Skip to content

Apply lockdown check to pull_request_read get_diff/get_files/get_check_runs #2883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kerobbi wants to merge 2 commits into
base: kerobbi/lockdown-shared-enforcement
Choose a base branch
from
+275 −27
Open

Apply lockdown check to pull_request_read get_diff/get_files/get_check_runs #2883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b743422
enforce lockdown on pr diff/files/check_runs and fix reviews fail-open
kerobbi Jul 14, 2026
da131c9
fix flaky test
kerobbi Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 63 additions & 15 deletions pkg/github/pullrequests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,13 @@ Possible options:
result, err := GetPullRequest(ctx, client, deps, owner, repo, pullNumber)
return attachIFC(result), nil, err
case "get_diff":
result, err := GetPullRequestDiff(ctx, client, owner, repo, pullNumber)
result, err := GetPullRequestDiff(ctx, client, deps, owner, repo, pullNumber)
return attachIFC(result), nil, err
case "get_status":
result, err := GetPullRequestStatus(ctx, client, owner, repo, pullNumber)
return attachIFC(result), nil, err
case "get_files":
result, err := GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination)
result, err := GetPullRequestFiles(ctx, client, deps, owner, repo, pullNumber, pagination)
return attachIFC(result), nil, err
case "get_commits":
result, err := GetPullRequestCommits(ctx, client, owner, repo, pullNumber, pagination)
Expand All @@ -152,7 +152,7 @@ Possible options:
result, err := GetIssueComments(ctx, client, deps, owner, repo, pullNumber, pagination)
return attachIFC(result), nil, err
case "get_check_runs":
result, err := GetPullRequestCheckRuns(ctx, client, owner, repo, pullNumber, pagination)
result, err := GetPullRequestCheckRuns(ctx, client, deps, owner, repo, pullNumber, pagination)
return attachIFC(result), nil, err
default:
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
Expand Down Expand Up @@ -206,7 +206,40 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende
return MarshalledTextResult(minimalPR), nil
}

func GetPullRequestDiff(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {
// enforcePullRequestLockdown returns a restricted tool result when lockdown mode is
// enabled and the pull request author is not a safe content source for owner/repo,
// and (nil, nil) otherwise. It fetches the pull request to resolve the author and is
// a no-op that performs no request when lockdown mode is disabled.
func enforcePullRequestLockdown(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {
if !deps.GetFlags(ctx).LockdownMode {
return nil, nil
}
cache, err := deps.GetRepoAccessCache(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get repo access cache: %w", err)
}
pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get pull request", resp, err), nil
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusOK {
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to read response body: %w", err)
}
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil
}

return authorLockdownResult(ctx, cache, owner, repo, pr.GetUser().GetLogin(), lockdownPullRequestRestrictedMessage)
}

func GetPullRequestDiff(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {
Comment thread
kerobbi marked this conversation as resolved.
if restricted, err := enforcePullRequestLockdown(ctx, client, deps, owner, repo, pullNumber); restricted != nil || err != nil {
return restricted, err
}

raw, resp, err := client.PullRequests.GetRaw(
ctx,
owner,
Expand Down Expand Up @@ -282,7 +315,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep
return utils.NewToolResultText(string(r)), nil
}

func GetPullRequestCheckRuns(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) {
func GetPullRequestCheckRuns(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) {
Comment thread
kerobbi marked this conversation as resolved.
// First get the PR to get the head SHA
pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber)
if err != nil {
Expand All @@ -302,6 +335,16 @@ func GetPullRequestCheckRuns(ctx context.Context, client *github.Client, owner,
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil
}

if deps.GetFlags(ctx).LockdownMode {
cache, err := deps.GetRepoAccessCache(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get repo access cache: %w", err)
}
if restricted, err := authorLockdownResult(ctx, cache, owner, repo, pr.GetUser().GetLogin(), lockdownPullRequestRestrictedMessage); restricted != nil || err != nil {
return restricted, err
}
}

// Get check runs for the head SHA
opts := &github.ListCheckRunsOptions{
ListOptions: github.ListOptions{
Expand Down Expand Up @@ -347,7 +390,11 @@ func GetPullRequestCheckRuns(ctx context.Context, client *github.Client, owner,
return utils.NewToolResultText(string(r)), nil
}

func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) {
func GetPullRequestFiles(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) {
Comment thread
kerobbi marked this conversation as resolved.
if restricted, err := enforcePullRequestLockdown(ctx, client, deps, owner, repo, pullNumber); restricted != nil || err != nil {
return restricted, err
}

opts := &github.ListOptions{
PerPage: pagination.PerPage,
Page: pagination.Page,
Expand Down Expand Up @@ -552,17 +599,18 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, deps Tool
filteredReviews := make([]*github.PullRequestReview, 0, len(reviews))
for _, review := range reviews {
login := review.GetUser().GetLogin()
if login != "" {
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
if err != nil {
return nil, fmt.Errorf("failed to check lockdown mode: %w", err)
}
if isSafeContent {
filteredReviews = append(filteredReviews, review)
}
reviews = filteredReviews
if login == "" {
continue
}
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
if err != nil {
return nil, fmt.Errorf("failed to check lockdown mode: %w", err)
}
if isSafeContent {
filteredReviews = append(filteredReviews, review)
}
}
reviews = filteredReviews
}

minimalReviews := make([]MinimalPullRequestReview, 0, len(reviews))
Expand Down
Loading
Loading