Skip to content

Commit a65e4bb

Browse files
committed
fix(scopes): allow public_repo scope for public repository write tools
Several repository write tools declared the broad `repo` scope, which hid them from tokens limited to `public_repo` and forced public-only OAuth deployments to request private-repository access. Lower the required scope to public_repo for tools that only operate on repositories the token can already reach: - add_issue_comment - issue_write - create_branch - push_files - create_pull_request - fork_repository Because RequiredScopes are expanded through the scope hierarchy, a full repo token remains accepted for every tool. GitHub continues to enforce actual per-repository permissions at the API layer. Fixes #3136
1 parent 64a49f3 commit a65e4bb

4 files changed

Lines changed: 95 additions & 6 deletions

File tree

pkg/github/issues.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,7 +1401,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
14011401
Required: []string{"owner", "repo", "issue_number"},
14021402
},
14031403
},
1404-
[]scopes.Scope{scopes.Repo},
1404+
[]scopes.Scope{scopes.PublicRepo},
14051405
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
14061406
owner, err := RequiredParam[string](args, "owner")
14071407
if err != nil {
@@ -2511,7 +2511,7 @@ Options are:
25112511
Required: []string{"method", "owner", "repo"},
25122512
},
25132513
},
2514-
[]scopes.Scope{scopes.Repo},
2514+
[]scopes.Scope{scopes.PublicRepo},
25152515
func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
25162516
method, err := RequiredParam[string](args, "method")
25172517
if err != nil {
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package github
2+
3+
import (
4+
"testing"
5+
6+
"github.com/github/github-mcp-server/pkg/inventory"
7+
"github.com/github/github-mcp-server/pkg/scopes"
8+
"github.com/github/github-mcp-server/pkg/translations"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// TestPublicRepoWriteToolsDeclareLeastPrivilegeScopes asserts that the public
14+
// repository write tools advertise `public_repo` as their required scope while
15+
// still accepting a full `repo` token (via the scope hierarchy). This enables
16+
// least-privilege, public-only OAuth deployments instead of forcing the broad
17+
// `repo` scope, which also grants private-repository access.
18+
// See https://github.com/github/github-mcp-server/issues/3136
19+
func TestPublicRepoWriteToolsDeclareLeastPrivilegeScopes(t *testing.T) {
20+
t.Parallel()
21+
22+
builders := map[string]func(translations.TranslationHelperFunc) *inventory.ServerTool{
23+
"add_issue_comment": wrapToolBuilder(AddIssueComment),
24+
"issue_write": wrapToolBuilder(IssueWrite),
25+
"create_branch": wrapToolBuilder(CreateBranch),
26+
"push_files": wrapToolBuilder(PushFiles),
27+
"create_pull_request": wrapToolBuilder(CreatePullRequest),
28+
"fork_repository": wrapToolBuilder(ForkRepository),
29+
}
30+
31+
for name, build := range builders {
32+
t.Run(name, func(t *testing.T) {
33+
t.Parallel()
34+
35+
st := build(translations.NullTranslationHelper)
36+
37+
assert.Equal(t, []string{string(scopes.PublicRepo)}, st.RequiredScopes,
38+
"%s should require only public_repo", name)
39+
assert.ElementsMatch(t,
40+
[]string{string(scopes.PublicRepo), string(scopes.Repo)}, st.AcceptedScopes,
41+
"%s should accept both public_repo and repo tokens", name)
42+
})
43+
}
44+
}
45+
46+
// TestPublicRepoWriteToolsVisibleToPublicRepoToken asserts the PAT tool filter
47+
// shows these tools to a public_repo-only token and keeps them visible for a
48+
// full repo token.
49+
func TestPublicRepoWriteToolsVisibleToPublicRepoToken(t *testing.T) {
50+
t.Parallel()
51+
52+
publicRepoToken := []string{string(scopes.PublicRepo)}
53+
repoToken := []string{string(scopes.Repo)}
54+
55+
filterForPublicToken := CreateToolScopeFilter(publicRepoToken)
56+
filterForRepoToken := CreateToolScopeFilter(repoToken)
57+
58+
builders := map[string]func(translations.TranslationHelperFunc) *inventory.ServerTool{
59+
"add_issue_comment": wrapToolBuilder(AddIssueComment),
60+
"issue_write": wrapToolBuilder(IssueWrite),
61+
"create_branch": wrapToolBuilder(CreateBranch),
62+
"push_files": wrapToolBuilder(PushFiles),
63+
"create_pull_request": wrapToolBuilder(CreatePullRequest),
64+
"fork_repository": wrapToolBuilder(ForkRepository),
65+
}
66+
67+
for name, build := range builders {
68+
t.Run(name, func(t *testing.T) {
69+
t.Parallel()
70+
71+
st := build(translations.NullTranslationHelper)
72+
73+
allowed, err := filterForPublicToken(t.Context(), st)
74+
require.NoError(t, err)
75+
assert.True(t, allowed, "%s should be visible with a public_repo-only token", name)
76+
77+
allowed, err = filterForRepoToken(t.Context(), st)
78+
require.NoError(t, err)
79+
assert.True(t, allowed, "%s should remain visible with a full repo token", name)
80+
})
81+
}
82+
}
83+
84+
func wrapToolBuilder(fn func(translations.TranslationHelperFunc) inventory.ServerTool) func(translations.TranslationHelperFunc) *inventory.ServerTool {
85+
return func(t translations.TranslationHelperFunc) *inventory.ServerTool {
86+
st := fn(t)
87+
return &st
88+
}
89+
}

pkg/github/pullrequests.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
706706
Required: []string{"owner", "repo", "title", "head", "base"},
707707
},
708708
},
709-
[]scopes.Scope{scopes.Repo},
709+
[]scopes.Scope{scopes.PublicRepo},
710710
func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
711711
owner, err := RequiredParam[string](args, "owner")
712712
if err != nil {

pkg/github/repositories.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ func ForkRepository(t translations.TranslationHelperFunc) inventory.ServerTool {
12221222
Required: []string{"owner", "repo"},
12231223
},
12241224
},
1225-
[]scopes.Scope{scopes.Repo},
1225+
[]scopes.Scope{scopes.PublicRepo},
12261226
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
12271227
owner, err := RequiredParam[string](args, "owner")
12281228
if err != nil {
@@ -1509,7 +1509,7 @@ func CreateBranch(t translations.TranslationHelperFunc) inventory.ServerTool {
15091509
Required: []string{"owner", "repo", "branch"},
15101510
},
15111511
},
1512-
[]scopes.Scope{scopes.Repo},
1512+
[]scopes.Scope{scopes.PublicRepo},
15131513
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
15141514
owner, err := RequiredParam[string](args, "owner")
15151515
if err != nil {
@@ -1641,7 +1641,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
16411641
Required: []string{"owner", "repo", "branch", "files", "message"},
16421642
},
16431643
},
1644-
[]scopes.Scope{scopes.Repo},
1644+
[]scopes.Scope{scopes.PublicRepo},
16451645
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
16461646
owner, err := RequiredParam[string](args, "owner")
16471647
if err != nil {

0 commit comments

Comments
 (0)