From 4fdeadbd276525ddea992f4e46de4c2d79dd84b3 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 15:09:52 +0200 Subject: [PATCH] refactor: condense lockdown comments in pull_request_read get_commits Trim the GetPullRequestCommits doc comment to a single terse sentence and remove a test comment that only restated the test name. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/pullrequests.go | 9 +++------ pkg/github/pullrequests_test.go | 3 --- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 3907ebc0ec..3e23927269 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -412,12 +412,9 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, deps ToolDe return MarshalledTextResult(minimalFiles), nil } -// GetPullRequestCommits returns the commits on a pull request. Commit messages -// are user-authored content like the PR diff and files, so under lockdown mode -// this applies the same PR-author check as GetPullRequestDiff/GetPullRequestFiles -// rather than filtering individual commits: all commits on a pull request are -// part of the same untrusted head branch, so a single check on the PR author is -// sufficient and avoids an extra permission lookup per commit. +// GetPullRequestCommits returns the commits on a pull request. Under lockdown +// mode it checks the PR author once rather than per commit, since every +// commit on the PR belongs to the same untrusted head branch. func GetPullRequestCommits(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { if restricted, err := enforcePullRequestLockdown(ctx, client, deps, owner, repo, pullNumber); restricted != nil || err != nil { return restricted, err diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index ec8fa8e86f..03ec851cf4 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1539,9 +1539,6 @@ func Test_GetPullRequestCommits(t *testing.T) { expectedCommits: mockCommits, }, { - // Trusted bot logins (e.g. github-actions[bot], copilot) are treated as - // safe content sources regardless of push access, matching the - // intentional exception documented for lockdown mode. name: "lockdown enabled - trusted bot author lacks push access", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &github.PullRequest{