Skip to content

Commit 9a26d73

Browse files
Merge branch 'main' into main
2 parents 9d83bc8 + 7d13a7a commit 9a26d73

5 files changed

Lines changed: 114 additions & 10 deletions

File tree

‎docs/streamable-http.md‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,23 @@ The OAuth protected resource metadata's `resource` attribute will be populated w
7272
],
7373
"scopes_supported": [
7474
"repo",
75-
...
75+
"read:org",
76+
"read:user",
77+
"user:email",
78+
"read:packages",
79+
"write:packages",
80+
"read:project",
81+
"project",
82+
"gist",
83+
"notifications"
7684
],
7785
...
7886
}
7987
```
8088

8189
This allows OAuth clients to discover authentication requirements and endpoint information automatically.
90+
Scopes excluded from this default set, such as `delete_repo`, are requested only
91+
through a per-tool OAuth authorization challenge when needed.
8292

8393
The HTTP server is the OAuth protected resource, not the authorization server. It
8494
therefore serves `/.well-known/oauth-protected-resource` but does not serve

‎pkg/github/copilot.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,19 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) inventory.Server
214214
return utils.NewToolResultError(err.Error()), nil, nil
215215
}
216216

217+
// owner, repo and issue_number are required, but WeakDecode zero-fills a
218+
// missing value, so a missing arg reached the query as a confusing
219+
// "Could not resolve to a Repository" error. Reject the zero values.
220+
if params.Owner == "" {
221+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
222+
}
223+
if params.Repo == "" {
224+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
225+
}
226+
if params.IssueNumber == 0 {
227+
return utils.NewToolResultError("missing required parameter: issue_number"), nil, nil
228+
}
229+
217230
client, err := deps.GetGQLClient(ctx)
218231
if err != nil {
219232
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
@@ -588,6 +601,19 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent
588601
return utils.NewToolResultError(err.Error()), nil, nil
589602
}
590603

604+
// owner, repo and issue_number are required, but WeakDecode zero-fills a
605+
// missing value, so reject the zero values (as with rationale/confidence
606+
// below) before they reach the query as a confusing repository error.
607+
if params.Owner == "" {
608+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
609+
}
610+
if params.Repo == "" {
611+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
612+
}
613+
if params.IssueNumber == 0 {
614+
return utils.NewToolResultError("missing required parameter: issue_number"), nil, nil
615+
}
616+
591617
// Validate rationale length (rune count, matching the granular assignee tools).
592618
rationale := strings.TrimSpace(params.Rationale)
593619
if rationale == "" {

‎pkg/github/copilot_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,36 @@ func TestAssignCopilotToIssue(t *testing.T) {
5959
expectToolError bool
6060
expectedToolErrMsg string
6161
}{
62+
{
63+
name: "missing owner is rejected",
64+
requestArgs: map[string]any{
65+
"repo": "repo",
66+
"issue_number": float64(123),
67+
},
68+
mockedClient: githubv4mock.NewMockedHTTPClient(),
69+
expectToolError: true,
70+
expectedToolErrMsg: "missing required parameter: owner",
71+
},
72+
{
73+
name: "missing repo is rejected",
74+
requestArgs: map[string]any{
75+
"owner": "owner",
76+
"issue_number": float64(123),
77+
},
78+
mockedClient: githubv4mock.NewMockedHTTPClient(),
79+
expectToolError: true,
80+
expectedToolErrMsg: "missing required parameter: repo",
81+
},
82+
{
83+
name: "missing issue_number is rejected",
84+
requestArgs: map[string]any{
85+
"owner": "owner",
86+
"repo": "repo",
87+
},
88+
mockedClient: githubv4mock.NewMockedHTTPClient(),
89+
expectToolError: true,
90+
expectedToolErrMsg: "missing required parameter: issue_number",
91+
},
6292
{
6393
name: "successful assignment when there are no existing assignees",
6494
requestArgs: map[string]any{
@@ -1228,6 +1258,19 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
12281258
expectedToolErrMsg string
12291259
expectSuggestion bool
12301260
}{
1261+
{
1262+
name: "missing owner is rejected",
1263+
requestArgs: map[string]any{
1264+
"repo": "repo",
1265+
"issue_number": float64(123),
1266+
"rationale": "Well-scoped task.",
1267+
"confidence": "HIGH",
1268+
"is_suggestion": false,
1269+
},
1270+
mockedClient: githubv4mock.NewMockedHTTPClient(),
1271+
expectToolError: true,
1272+
expectedToolErrMsg: "missing required parameter: owner",
1273+
},
12311274
{
12321275
name: "direct assignment with rationale and confidence preserves existing assignees",
12331276
requestArgs: map[string]any{

‎pkg/http/oauth/oauth.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,12 @@ const (
2020
OAuthProtectedResourcePrefix = "/.well-known/oauth-protected-resource"
2121
)
2222

23-
// SupportedScopes lists every OAuth scope that an MCP tool may require. HTTP
24-
// protected-resource metadata advertises this full set so clients can step up
25-
// authorization for tools excluded from the default grant.
23+
// SupportedScopes lists every OAuth scope that an MCP tool may require.
2624
var SupportedScopes = scopes.SupportedOAuthScopes()
2725

28-
// DefaultScopes are requested by stdio OAuth unless the operator explicitly
29-
// supplies --oauth-scopes. High-risk scopes such as delete_repo require opt-in.
26+
// DefaultScopes are advertised in protected-resource metadata and requested by
27+
// stdio OAuth unless the operator explicitly supplies --oauth-scopes. Other
28+
// scopes require opt-in through a per-tool authorization challenge.
3029
var DefaultScopes = scopes.DefaultOAuthScopes()
3130

3231
// Config holds the OAuth configuration for the MCP server.
@@ -128,7 +127,7 @@ func (h *AuthHandler) metadataHandler() http.Handler {
128127
Resource: resourceURL,
129128
AuthorizationServers: []string{authorizationServerURL},
130129
ResourceName: "GitHub MCP Server",
131-
ScopesSupported: SupportedScopes,
130+
ScopesSupported: DefaultScopes,
132131
BearerMethodsSupported: []string{"header"},
133132
}
134133

‎pkg/http/oauth/oauth_test.go‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,18 @@ func TestHandleProtectedResource(t *testing.T) {
436436
host: "api.example.com",
437437
method: http.MethodGet,
438438
expectedStatusCode: http.StatusOK,
439-
expectedScopes: SupportedScopes,
439+
expectedScopes: []string{
440+
"repo",
441+
"read:org",
442+
"read:user",
443+
"user:email",
444+
"read:packages",
445+
"write:packages",
446+
"read:project",
447+
"project",
448+
"gist",
449+
"notifications",
450+
},
440451
validateResponse: func(t *testing.T, body map[string]any) {
441452
t.Helper()
442453
assert.Equal(t, "GitHub MCP Server", body["resource_name"])
@@ -573,7 +584,12 @@ func TestHandleProtectedResource(t *testing.T) {
573584
if tc.expectedScopes != nil {
574585
scopes, ok := body["scopes_supported"].([]any)
575586
require.True(t, ok)
576-
assert.Len(t, scopes, len(tc.expectedScopes))
587+
actualScopes := make([]string, len(scopes))
588+
for i, scope := range scopes {
589+
actualScopes[i], ok = scope.(string)
590+
require.True(t, ok)
591+
}
592+
assert.Equal(t, tc.expectedScopes, actualScopes)
577593
}
578594
}
579595
})
@@ -671,10 +687,20 @@ func TestSupportedScopes(t *testing.T) {
671687
assert.Equal(t, expectedScopes, SupportedScopes)
672688
}
673689

674-
func TestDefaultScopesRequiresExplicitDeleteRepoOptIn(t *testing.T) {
690+
func TestDefaultScopesRequireExplicitOptIn(t *testing.T) {
675691
assert.Subset(t, SupportedScopes, DefaultScopes)
676692
assert.Contains(t, SupportedScopes, "delete_repo")
677693
assert.NotContains(t, DefaultScopes, "delete_repo")
694+
assert.Contains(t, SupportedScopes, "workflow")
695+
assert.NotContains(t, DefaultScopes, "workflow")
696+
assert.Contains(t, SupportedScopes, "codespace")
697+
assert.NotContains(t, DefaultScopes, "codespace")
698+
assert.Contains(t, SupportedScopes, "admin:org")
699+
assert.NotContains(t, DefaultScopes, "admin:org")
700+
assert.Contains(t, SupportedScopes, "read:enterprise")
701+
assert.NotContains(t, DefaultScopes, "read:enterprise")
702+
assert.Contains(t, SupportedScopes, "admin:enterprise")
703+
assert.NotContains(t, DefaultScopes, "admin:enterprise")
678704
assert.Contains(t, DefaultScopes, "repo")
679705
}
680706

0 commit comments

Comments
 (0)