Repository navigation
Conversation
…ries Tool calls and repo:// resource reads are checked against an owner/repo allowlist before any GitHub API request is made. Owner-wide calls need an owner/* entry, search queries must be scoped with repo:/org:/user: qualifiers inside the list, and tools that can't be tied to a repository are rejected unless they never return repository content. Fixes github#1685
There was a problem hiding this comment.
🟡 Changes recommended
Critical allowlist enforcement gaps in tool scoping, search queries, completions, and secondary or destination targets permit reads and writes involving excluded repositories.
9 open issues
Filter linked project items by repository access · New Require owner and repo for notification operations · New Verify scope using tool-specific target policies · New Validate secondary and batch repository targets · New Validate fork destination against the allowlist · New Require a positive allowed-repository search qualifier · New Require provable repository scope in search queries · New Apply allowlist checks to repository completions · New Allow known static UI resources with an allowlist · New
What changed in this PR
Introduces an optional repository allowlist for the local stdio server, intended to limit agent access independently of the token’s broader permissions and address #1685.
Changes:
- Adds repository-pattern validation and authorization checks for tool calls, searches, and resource reads.
- Wires
--allowed-reposandGITHUB_ALLOWED_REPOSinto stdio configuration. - Adds behavioral tests and documents configuration, scope rules, and limitations.
| File | Description |
|---|---|
README.md |
Documents allowlist rules and usage examples. |
internal/ghmcp/server.go |
Parses configuration and installs allowlist middleware. |
internal/ghmcp/allowed_repos.go |
Implements pattern matching and request authorization. |
internal/ghmcp/allowed_repos_test.go |
Tests parsing, scope checks, and middleware behavior. |
docs/server-configuration.md |
Adds the option to configuration guidance. |
cmd/github-mcp-server/main.go |
Registers the flag and passes configured entries to stdio. |
Review effort: Max (auto)
| case owner != "": | ||
| scoped = true | ||
| if !l.allowsOwner(owner) { | ||
| return fmt.Sprintf("owner-wide access to %s requires %s/* in the allowed repositories", owner, owner) |
There was a problem hiding this comment.
Fixed in 796a537: list_project_items and get_project_item are rejected while an allowlist is set, since item repositories are not known from the arguments. The other project methods stay available under owner/*.
| case owner != "": | ||
| scoped = true | ||
| if !l.allowsOwner(owner) { | ||
| return fmt.Sprintf("owner-wide access to %s requires %s/* in the allowed repositories", owner, owner) | ||
| } |
There was a problem hiding this comment.
Fixed in 796a537: list_notifications and mark_all_notifications_read now need both owner and repo; dismiss_notification, get_notification_details and manage_notification_subscription are rejected because the thread ID cannot be tied to a repository.
| return "" | ||
| } | ||
|
|
||
| if reason := checkPair(str("owner"), str("repo")); reason != "" { |
There was a problem hiding this comment.
Fixed in 796a537: tools with a level argument are checked at that level only (repository needs owner+repo, organization needs org with org/*, enterprise is rejected), the ID-only notification and project methods are rejected, and unrelated owner/repo arguments no longer establish scope for them.
| if reason := checkPair(str("item_owner"), str("item_repo")); reason != "" { | ||
| return reason | ||
| } |
There was a problem hiding this comment.
Fixed in 796a537: parent_owner/parent_repo and related_owner/related_repo are checked with the handlers' defaults (missing parts fall back to owner/repo), and every items[] entry of update_project_items must name item_owner/item_repo in the allowlist; node_id/item_id entries are rejected.
| return reason | ||
| } | ||
| } | ||
| if name == "create_repository" { |
There was a problem hiding this comment.
Fixed in 796a537: fork_repository now requires an organization covered by an owner/* entry, and the implicit personal-account destination is rejected since its login is not known here.
| if query, ok := args["query"].(string); ok && strings.HasPrefix(name, "search_") { | ||
| found, reason := l.checkSearchQuery(query) | ||
| if reason != "" { | ||
| return reason | ||
| } | ||
| scoped = scoped || found | ||
| } |
There was a problem hiding this comment.
Fixed in 796a537: the owner/repo arguments of search_issues and search_pull_requests only count as scope when the query has no repo: qualifier at all (positive or negated), mirroring hasRepoFilter in prepareSearchArgs. Otherwise the query itself must carry an allowed positive qualifier.
| for field := range strings.FieldsSeq(query) { | ||
| key, value, ok := strings.Cut(field, ":") | ||
| if !ok { | ||
| continue | ||
| } | ||
| value = strings.Trim(value, `"'()`) | ||
| switch strings.ToLower(strings.TrimLeft(key, "(")) { |
There was a problem hiding this comment.
Fixed in 796a537: quoted qualifiers are treated as free text and never count as scope (an outside repo in quotes is still rejected, which also covers the semantic quote stripping), and queries with OR, NOT, AND or parentheses are rejected because one branch cannot vouch for the others.
| return utils.NewToolResultError(fmt.Sprintf("access denied: %s (allowed: %s)", reason, list)), nil | ||
| } | ||
| } | ||
| case *mcp.ReadResourceRequest: |
There was a problem hiding this comment.
Fixed in 796a537: completion/complete for repo:// templates is now checked in the middleware. Owner suggestions are rejected, repo suggestions need owner/*, and every other argument is completed only for an allowed resolved owner/repo.
|
|
||
| // checkResourceURI validates repo:// resource URIs. | ||
| func (l *repoAllowlist) checkResourceURI(uri string) string { | ||
| u, err := url.Parse(uri) |
There was a problem hiding this comment.
Fixed in 796a537: ui:// resources are allowed (they only serve bundled HTML); repo:// is still checked and other schemes are still rejected.
The repository allowlist now rejects calls whose target cannot be told from the arguments instead of trusting any owner/repo it finds: - list_notifications and mark_all_notifications_read need both owner and repo; thread-level notification tools are rejected - rulesets and custom properties are checked at the selected level, and the enterprise level is rejected - parent_owner/parent_repo, related_owner/related_repo and every items[].item_owner/item_repo entry are checked, with the handlers' defaults - fork_repository needs an allowed organization destination - project item reads, item writes without an issue reference, and views or status updates addressed by node ID are rejected - search queries: negated qualifiers never count as scope, quoted ones are free text, boolean operators and parentheses are rejected, and the owner/repo arguments of search_issues/search_pull_requests only count when the query has no repo: qualifier (matching prepareSearchArgs) - completion/complete is checked for repo:// templates - the bundled ui:// app resources are allowed
796a537 to
e3c092c
Compare


Summary
Adds
--allowed-repos/GITHUB_ALLOWED_REPOSto the local (stdio) server, so an agent can be limited to a fixed set of repositories (owner/repoorowner/*) no matter what the token can see.Why
Fixes #1685. Fine-grained tokens limited to selected repositories can still read every public repository, and prompt instructions aren't a reliable boundary. This gives local users a deterministic way to sandbox an agent to, for example, their org's private repos.
What changed
internal/ghmcp/allowed_repos.go: a receiving middleware that checkstools/callandresources/readbefore any GitHub API request:owner+repoarguments (anditem_owner/item_repoinprojects_write) must match an entry; the repo part acceptspath.Matchwildcards (octo-org/svc-*); matching is case-insensitive.ownerwithoutrepo,org,create_repository'sorganization) need anowner/*entry.search_*queries: every positiverepo:/org:/user:qualifier must be allowed (GitHub ORs them), and the call must be scoped either by those qualifiers or byowner/repoargs. Negated qualifiers are ignored.list_notifications,get_notification_detailsby ID) are rejected. A short list of tools that never return repository content (get_me, teams,search_users,search_orgs, global advisories, gists) is exempt.cmd/github-mcp-server/main.go, entries validated at startup.docs/server-configuration.mddocument the option and its rules.MCP impact
Prompts tested (tool changes only)
Ran the built binary over stdio with
GITHUB_ALLOWED_REPOS=octocat/hello-world:get_file_contentsonoctocat/spoon-knife→access denied: repository octocat/spoon-knife is not in the allowed repositories (allowed: octocat/hello-world)search_codewithfunc mainand no qualifier → rejected with a hint to addrepo:owner/nameSecurity / limits
I kept this to the stdio server, since the issue is about local deployments. Happy to add an
X-MCP-Allowed-Reposheader for HTTP mode in a follow-up if that's wanted.Tool renaming
deprecated_tool_aliases.goLint & tests
./script/lint(0 issues)./script/test/go test ./...(all packages pass); new table tests ininternal/ghmcp/allowed_repos_test.goDocs