Skip to content

Fix lockdown mode permission check#2361

Draft
kerobbi wants to merge 4 commits intomainfrom
kerobbi/lockdown-fix
Draft

Fix lockdown mode permission check#2361
kerobbi wants to merge 4 commits intomainfrom
kerobbi/lockdown-fix

Conversation

@kerobbi
Copy link
Copy Markdown
Contributor

@kerobbi kerobbi commented Apr 21, 2026

Summary

Replaces the GraphQL repository.collaborators permission check in lockdown mode with the REST GetPermissionLevel endpoint, and add github-actions[bot] to the trusted bots list.

Lockdown filtering behaviour is preserved; same trust model (push access = trusted), same fail-closed error handling.

Why

Lockdown mode filters issue and PR content on public repos based on whether the author has push access to the repository. It does this using the GraphQL repository.collaborators field, but this field requires the calling token to have admin/write access on the repo to enumerate collaborators. GITHUB_TOKEN and GitHub App installation tokens typically don't have this, so the query silently returns empty results (no error), and lockdown ends up treating every author as untrusted, blocking all content.

The REST GetPermissionLevel endpoint works with any authenticated token and returns the exact permission level.

What changed

  • Replaced GraphQL collaborators query with REST GetPermissionLevel for permission checks (kept GraphQL for repo metadata only: viewer.login, isPrivate)
  • Added restClient as a positional parameter on GetInstance and wired it through
  • Optimised cache-hit path: when repo metadata is already cached for a repo, new user permission lookups skip GraphQL and go straight to REST
  • Added NewRepoAccessCache constructor for test isolation
  • Added github-actions[bot] to trustedBotLogins
  • Moved isTrustedBot check before API calls in IsSafeContent to skip unnecessary lookups for trusted bots
  • Updated all lockdown tests to reflect the new REST-based flow

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Security / limits

  • No security or limits impact
  • Auth / permissions considered - REST GetPermissionLevel requires only metadata: read (included by default on all token types as far as I know) and works with any authenticated token on public repos.
  • Data exposure, filtering, or token/size limits considered - all error paths remain fail-closed (content blocked). The REST endpoint is publicly callable by any authenticated user, so lockdown doesn't expose new information.

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant