Skip to content

Commit e471f39

Browse files
fix: separate public repo visibility from OAuth scopes
Keep public_repo as a fixed-token visibility capability while requiring repo for per-call OAuth challenges. Challenge only missing scopes for workflow pushes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent d0b863f commit e471f39

14 files changed

Lines changed: 73 additions & 47 deletions

README.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,7 @@ The following sets of tools are available:
875875
<summary><picture><source media="(prefers-color-scheme: dark)" srcset="pkg/octicons/icons/issue-opened-dark.png"><source media="(prefers-color-scheme: light)" srcset="pkg/octicons/icons/issue-opened-light.png"><img src="pkg/octicons/icons/issue-opened-light.png" width="20" height="20" alt="issue-opened"></picture> Issues</summary>
876876

877877
- **add_issue_comment** - Add comment to issue or pull request
878-
- **OAuth Challenge Scopes**: `public_repo`
878+
- **OAuth Challenge Scopes**: `repo`
879879
- `body`: Comment content. Required unless reaction is provided. (string, optional)
880880
- `comment_id`: The numeric ID of the issue or pull request comment to react to. Use this for reactions to comments; omit it to react to the issue or pull request itself. Cannot be combined with body. (integer, optional)
881881
- `issue_number`: Issue or pull request number to comment on or react to. (number, required)
@@ -906,7 +906,7 @@ The following sets of tools are available:
906906
- `repo`: The name of the repository (string, required)
907907

908908
- **issue_write** - Create or update issue/pull request
909-
- **OAuth Challenge Scopes**: `public_repo`
909+
- **OAuth Challenge Scopes**: `repo`
910910
- `assignees`: Usernames to assign to this issue (string[], optional)
911911
- `body`: Issue body content (string, optional)
912912
- `duplicate_of`: Issue number that this issue is a duplicate of. Required when state_reason is 'duplicate'. (number, optional)
@@ -1154,7 +1154,7 @@ The following sets of tools are available:
11541154
- `repo`: Repository name (string, required)
11551155

11561156
- **create_pull_request** - Open new pull request
1157-
- **OAuth Challenge Scopes**: `public_repo`
1157+
- **OAuth Challenge Scopes**: `repo`
11581158
- `base`: Branch to merge into (string, required)
11591159
- `body`: PR description (string, optional)
11601160
- `draft`: Create as draft PR (boolean, optional)
@@ -1257,7 +1257,7 @@ The following sets of tools are available:
12571257
<summary><picture><source media="(prefers-color-scheme: dark)" srcset="pkg/octicons/icons/repo-dark.png"><source media="(prefers-color-scheme: light)" srcset="pkg/octicons/icons/repo-light.png"><img src="pkg/octicons/icons/repo-light.png" width="20" height="20" alt="repo"></picture> Repositories</summary>
12581258

12591259
- **create_branch** - Create branch
1260-
- **OAuth Challenge Scopes**: `public_repo`
1260+
- **OAuth Challenge Scopes**: `repo`
12611261
- `branch`: Name for new branch (string, required)
12621262
- `from_branch`: Source branch (defaults to repo default) (string, optional)
12631263
- `owner`: Repository owner (string, required)
@@ -1296,7 +1296,7 @@ The following sets of tools are available:
12961296
- `repo`: Repository name (string, required)
12971297

12981298
- **fork_repository** - Fork repository
1299-
- **OAuth Challenge Scopes**: `public_repo`
1299+
- **OAuth Challenge Scopes**: `repo`
13001300
- `organization`: Organization to fork to (string, optional)
13011301
- `owner`: Repository owner (string, required)
13021302
- `repo`: Repository name (string, required)
@@ -1380,7 +1380,7 @@ The following sets of tools are available:
13801380
- `repo`: Repository name (string, required)
13811381

13821382
- **push_files** - Push files to repository
1383-
- **OAuth Challenge Scopes**: `public_repo`, `workflow`
1383+
- **OAuth Challenge Scopes**: `repo`, `workflow`
13841384
- `branch`: Branch to push to (string, required)
13851385
- `files`: Array of file objects to push, each object with path (string) and content (string) (object[], required)
13861386
- `message`: Commit message (string, required)

docs/feature-flags.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ runtime behavior (such as output formatting) won't appear here.
3535
### `remote_mcp_ui_apps`
3636

3737
- **create_pull_request** - Open new pull request
38-
- **OAuth Challenge Scopes**: `public_repo`
38+
- **OAuth Challenge Scopes**: `repo`
3939
- **MCP App UI**: `ui://github-mcp-server/pr-write`
4040
- `base`: Branch to merge into (string, required)
4141
- `body`: PR description (string, optional)
@@ -52,7 +52,7 @@ runtime behavior (such as output formatting) won't appear here.
5252
- No parameters required
5353

5454
- **issue_write** - Create or update issue/pull request
55-
- **OAuth Challenge Scopes**: `public_repo`
55+
- **OAuth Challenge Scopes**: `repo`
5656
- **MCP App UI**: `ui://github-mcp-server/issue-write`
5757
- `assignees`: Usernames to assign to this issue (string[], optional)
5858
- `body`: Issue body content (string, optional)

docs/insiders-features.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
2929
### `remote_mcp_ui_apps`
3030

3131
- **create_pull_request** - Open new pull request
32-
- **OAuth Challenge Scopes**: `public_repo`
32+
- **OAuth Challenge Scopes**: `repo`
3333
- **MCP App UI**: `ui://github-mcp-server/pr-write`
3434
- `base`: Branch to merge into (string, required)
3535
- `body`: PR description (string, optional)
@@ -46,7 +46,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
4646
- No parameters required
4747

4848
- **issue_write** - Create or update issue/pull request
49-
- **OAuth Challenge Scopes**: `public_repo`
49+
- **OAuth Challenge Scopes**: `repo`
5050
- **MCP App UI**: `ui://github-mcp-server/issue-write`
5151
- `assignees`: Usernames to assign to this issue (string[], optional)
5252
- `body`: Issue body content (string, optional)

pkg/github/issues.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,7 +1403,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
14031403
Required: []string{"owner", "repo", "issue_number"},
14041404
},
14051405
},
1406-
scopes.RequireAll(scopes.PublicRepo),
1406+
publicRepositoryWriteScopeAccess(),
14071407
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
14081408
owner, err := RequiredParam[string](args, "owner")
14091409
if err != nil {
@@ -2524,7 +2524,7 @@ Options are:
25242524
Required: []string{"method", "owner", "repo"},
25252525
},
25262526
},
2527-
scopes.RequireAll(scopes.PublicRepo),
2527+
publicRepositoryWriteScopeAccess(),
25282528
func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
25292529
method, err := RequiredParam[string](args, "method")
25302530
if err != nil {

pkg/github/public_repo_scopes_test.go

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/stretchr/testify/require"
1111
)
1212

13-
func TestPublicRepoContributionToolOAuthScopes(t *testing.T) {
13+
func TestPublicRepoContributionToolScopeAccess(t *testing.T) {
1414
t.Parallel()
1515

1616
tools := []struct {
@@ -26,14 +26,16 @@ func TestPublicRepoContributionToolOAuthScopes(t *testing.T) {
2626

2727
for _, tt := range tools {
2828
t.Run(tt.name, func(t *testing.T) {
29-
assert.Equal(t, []string{string(scopes.PublicRepo)}, tt.tool.ScopeAccess.Scopes)
30-
assert.Equal(t, []string{string(scopes.PublicRepo)}, tt.tool.ScopeAccess.Challenge(nil, nil))
31-
for _, activeScopes := range [][]string{
32-
{string(scopes.PublicRepo)},
33-
{string(scopes.Repo)},
34-
} {
35-
assert.Empty(t, tt.tool.ScopeAccess.Challenge(nil, activeScopes))
36-
}
29+
assert.Equal(t, []string{string(scopes.Repo)}, tt.tool.ScopeAccess.Scopes)
30+
require.NotNil(t, tt.tool.ScopeAccess.Visible)
31+
assert.False(t, tt.tool.ScopeAccess.Visible(nil))
32+
assert.True(t, tt.tool.ScopeAccess.Visible([]string{string(scopes.PublicRepo)}))
33+
assert.True(t, tt.tool.ScopeAccess.Visible([]string{string(scopes.Repo)}))
34+
35+
require.NotNil(t, tt.tool.ScopeAccess.Challenge)
36+
assert.Equal(t, []string{string(scopes.Repo)}, tt.tool.ScopeAccess.Challenge(nil, nil))
37+
assert.Equal(t, []string{string(scopes.Repo)}, tt.tool.ScopeAccess.Challenge(nil, []string{string(scopes.PublicRepo)}))
38+
assert.Empty(t, tt.tool.ScopeAccess.Challenge(nil, []string{string(scopes.Repo)}))
3739
})
3840
}
3941
}
@@ -50,13 +52,23 @@ func TestPublicRepoContributionToolsVisibleToPATs(t *testing.T) {
5052
AddIssueComment(translations.NullTranslationHelper),
5153
}
5254

53-
for _, tokenScope := range []scopes.Scope{scopes.PublicRepo, scopes.Repo} {
54-
t.Run(string(tokenScope), func(t *testing.T) {
55-
filter := CreateToolScopeFilter([]string{string(tokenScope)})
55+
tests := []struct {
56+
name string
57+
tokenScopes []string
58+
wantVisible bool
59+
}{
60+
{name: "no scopes"},
61+
{name: "public_repo", tokenScopes: []string{string(scopes.PublicRepo)}, wantVisible: true},
62+
{name: "repo", tokenScopes: []string{string(scopes.Repo)}, wantVisible: true},
63+
}
64+
65+
for _, tt := range tests {
66+
t.Run(tt.name, func(t *testing.T) {
67+
filter := CreateToolScopeFilter(tt.tokenScopes)
5668
for i := range tools {
5769
included, err := filter(t.Context(), &tools[i])
5870
require.NoError(t, err)
59-
assert.True(t, included, "%s should be visible", tools[i].Tool.Name)
71+
assert.Equal(t, tt.wantVisible, included, tools[i].Tool.Name)
6072
}
6173
})
6274
}
@@ -73,12 +85,19 @@ func TestPushFilesOAuthScopeChallenges(t *testing.T) {
7385
"files": []any{map[string]any{"path": ".github/workflows/ci.yml"}},
7486
}
7587

76-
assert.Equal(t, []string{string(scopes.PublicRepo), string(scopes.Workflow)}, tool.ScopeAccess.Scopes)
77-
assert.Equal(t, []string{string(scopes.PublicRepo)}, tool.ScopeAccess.Challenge(regularFiles, nil))
78-
assert.Equal(t, []string{string(scopes.PublicRepo), string(scopes.Workflow)}, tool.ScopeAccess.Challenge(workflowFiles, nil))
79-
assert.Equal(t, []string{string(scopes.PublicRepo), string(scopes.Workflow)}, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.PublicRepo)}))
80-
assert.Empty(t, tool.ScopeAccess.Challenge(regularFiles, []string{string(scopes.PublicRepo)}))
88+
assert.Equal(t, []string{string(scopes.Repo), string(scopes.Workflow)}, tool.ScopeAccess.Scopes)
89+
require.NotNil(t, tool.ScopeAccess.Visible)
90+
assert.True(t, tool.ScopeAccess.Visible([]string{string(scopes.PublicRepo)}))
91+
assert.True(t, tool.ScopeAccess.Visible([]string{string(scopes.Repo)}))
92+
93+
assert.Equal(t, []string{string(scopes.Repo)}, tool.ScopeAccess.Challenge(regularFiles, nil))
94+
assert.Equal(t, []string{string(scopes.Repo)}, tool.ScopeAccess.Challenge(regularFiles, []string{string(scopes.PublicRepo)}))
8195
assert.Empty(t, tool.ScopeAccess.Challenge(regularFiles, []string{string(scopes.Repo)}))
82-
assert.Empty(t, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.PublicRepo), string(scopes.Workflow)}))
96+
97+
assert.Equal(t, []string{string(scopes.Repo), string(scopes.Workflow)}, tool.ScopeAccess.Challenge(workflowFiles, nil))
98+
assert.Equal(t, []string{string(scopes.Repo), string(scopes.Workflow)}, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.PublicRepo)}))
99+
assert.Equal(t, []string{string(scopes.Workflow)}, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.Repo)}))
100+
assert.Equal(t, []string{string(scopes.Repo)}, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.Workflow)}))
101+
assert.Equal(t, []string{string(scopes.Repo)}, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.PublicRepo), string(scopes.Workflow)}))
83102
assert.Empty(t, tool.ScopeAccess.Challenge(workflowFiles, []string{string(scopes.Repo), string(scopes.Workflow)}))
84103
}

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.RequireAll(scopes.PublicRepo),
709+
publicRepositoryWriteScopeAccess(),
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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,7 +1229,7 @@ func ForkRepository(t translations.TranslationHelperFunc) inventory.ServerTool {
12291229
Required: []string{"owner", "repo"},
12301230
},
12311231
},
1232-
scopes.RequireAll(scopes.PublicRepo),
1232+
publicRepositoryWriteScopeAccess(),
12331233
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
12341234
owner, err := RequiredParam[string](args, "owner")
12351235
if err != nil {
@@ -1526,7 +1526,7 @@ func CreateBranch(t translations.TranslationHelperFunc) inventory.ServerTool {
15261526
Required: []string{"owner", "repo", "branch"},
15271527
},
15281528
},
1529-
scopes.RequireAll(scopes.PublicRepo),
1529+
publicRepositoryWriteScopeAccess(),
15301530
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
15311531
owner, err := RequiredParam[string](args, "owner")
15321532
if err != nil {
@@ -1658,7 +1658,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
16581658
Required: []string{"owner", "repo", "branch", "files", "message"},
16591659
},
16601660
},
1661-
scopes.RequireAll(scopes.PublicRepo),
1661+
publicRepositoryWriteScopeAccess(),
16621662
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
16631663
owner, err := RequiredParam[string](args, "owner")
16641664
if err != nil {
@@ -1840,7 +1840,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
18401840
},
18411841
)
18421842
tool.ScopeAccess = scopes.DynamicChallenge(
1843-
[]scopes.Scope{scopes.PublicRepo, scopes.Workflow},
1843+
[]scopes.Scope{scopes.Repo, scopes.Workflow},
18441844
tool.ScopeAccess.Visible,
18451845
workflowScopeChallengeForFiles,
18461846
)

pkg/github/repository_path.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,12 @@ func workflowScopeChallengeForFiles(arguments map[string]any, activeScopes []str
7575
containsWorkflow = true
7676
}
7777
}
78-
if containsWorkflow {
79-
return scopes.ChallengeAll(activeScopes, scopes.PublicRepo, scopes.Workflow)
78+
var challenge []string
79+
if !scopes.HasAll(activeScopes, scopes.Repo) {
80+
challenge = append(challenge, string(scopes.Repo))
8081
}
81-
return scopes.ChallengeAll(activeScopes, scopes.PublicRepo)
82+
if containsWorkflow && !scopes.HasAll(activeScopes, scopes.Workflow) {
83+
challenge = append(challenge, string(scopes.Workflow))
84+
}
85+
return challenge
8286
}

pkg/github/repository_path_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestFileWriteWorkflowScopeChallenges(t *testing.T) {
8484
map[string]any{"path": "README.md"},
8585
map[string]any{"path": ".github/workflows/ci.yml"},
8686
}},
87-
want: []string{"public_repo", "workflow"},
87+
want: []string{"workflow"},
8888
},
8989
{
9090
name: "push validates entries after workflow",

pkg/github/tool_scopes.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ import (
55
"github.com/github/github-mcp-server/pkg/scopes"
66
)
77

8+
func publicRepositoryWriteScopeAccess() inventory.ScopeAccess {
9+
access := scopes.RequireAll(scopes.Repo)
10+
access.Visible = func(activeScopes []string) bool {
11+
return scopes.HasAll(activeScopes, scopes.PublicRepo)
12+
}
13+
return access
14+
}
15+
816
func repositoryOrOrganizationScopeAccess() inventory.ScopeAccess {
917
return scopes.DynamicChallenge(
1018
[]scopes.Scope{scopes.Repo, scopes.ReadOrg},

0 commit comments

Comments
 (0)