Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -922,13 +922,13 @@ The following sets of tools are available:
- `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional)

- **list_issue_fields** - List issue fields
- **Required OAuth Scopes (any of)**: `repo`, `read:org`
- **Required OAuth Scopes**: `repo`, `read:org`
- **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org`
- `owner`: The account owner of the repository or organization. The name is not case sensitive. (string, required)
- `repo`: The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly. (string, optional)

- **list_issue_types** - List available issue types
- **Required OAuth Scopes (any of)**: `repo`, `read:org`
- **Required OAuth Scopes**: `repo`, `read:org`
- **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org`
- `owner`: The account owner of the repository or organization. (string, required)
- `repo`: The name of the repository. When provided, returns issue types for this specific repository. When omitted, returns org-level issue types directly. (string, optional)
Expand Down
6 changes: 5 additions & 1 deletion cmd/github-mcp-server/feature_flag_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@ func generateFlaggedToolsDoc(flags []string, emptyMessage string) string {
if !hasAny {
return emptyMessage
}
// Clarify scope semantics for the rendered tools: every listed required
// scope is needed (AND), and a higher scope in the hierarchy also satisfies
// a required scope.
preamble := "> **OAuth scopes:** all listed required scopes are needed (AND). A higher scope in the hierarchy (e.g. `admin:org` for `read:org`, `repo` for `public_repo`) also satisfies a required scope.\n\n"
// Leading/trailing newlines around the body produce blank lines between
// our content and the surrounding marker comments, so the trailing comment
// doesn't get absorbed into the final list item by markdown renderers.
return "\n" + strings.TrimSuffix(buf.String(), "\n") + "\n"
return "\n" + preamble + strings.TrimSuffix(buf.String(), "\n") + "\n"
}

// flaggedToolDiff returns the tools whose definition (input schema or meta)
Expand Down
12 changes: 4 additions & 8 deletions cmd/github-mcp-server/generate_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,11 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {

// OAuth scopes if present
if len(tool.RequiredScopes) > 0 {
// Scope filtering uses "any of" semantics (see scopes.HasRequiredScopes),
// so when multiple required scopes are listed, render them as alternatives
// rather than implying all are required.
// All listed required scopes are needed (AND). A higher scope in the
// hierarchy also satisfies a required scope (see scopes.HasRequiredScopes).
// AcceptedScopes below is non-authoritative display metadata.
scopeList := "`" + strings.Join(tool.RequiredScopes, "`, `") + "`"
if len(tool.RequiredScopes) > 1 {
fmt.Fprintf(buf, " - **Required OAuth Scopes (any of)**: %s\n", scopeList)
} else {
fmt.Fprintf(buf, " - **Required OAuth Scopes**: %s\n", scopeList)
}
fmt.Fprintf(buf, " - **Required OAuth Scopes**: %s\n", scopeList)

// Only show accepted scopes if they differ from required scopes
if len(tool.AcceptedScopes) > 0 && !scopesEqual(tool.RequiredScopes, tool.AcceptedScopes) {
Expand Down
4 changes: 3 additions & 1 deletion docs/feature-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ runtime behavior (such as output formatting) won't appear here.

<!-- START AUTOMATED FEATURE FLAG TOOLS -->

> **OAuth scopes:** all listed required scopes are needed (AND). A higher scope in the hierarchy (e.g. `admin:org` for `read:org`, `repo` for `public_repo`) also satisfies a required scope.

### `remote_mcp_ui_apps`

- **create_pull_request** - Open new pull request
Expand Down Expand Up @@ -74,7 +76,7 @@ runtime behavior (such as output formatting) won't appear here.
- `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional)

- **ui_get** - Get UI data
- **Required OAuth Scopes (any of)**: `repo`, `read:org`
- **Required OAuth Scopes**: `repo`, `read:org`
- **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org`
- `method`: The type of data to fetch (string, required)
- `owner`: Repository owner (required for all methods) (string, required)
Expand Down
4 changes: 3 additions & 1 deletion docs/insiders-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ The list below is generated from the Go source. It covers tool **inventory and s

<!-- START AUTOMATED INSIDERS TOOLS -->

> **OAuth scopes:** all listed required scopes are needed (AND). A higher scope in the hierarchy (e.g. `admin:org` for `read:org`, `repo` for `public_repo`) also satisfies a required scope.

### `remote_mcp_ui_apps`

- **create_pull_request** - Open new pull request
Expand Down Expand Up @@ -68,7 +70,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
- `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional)

- **ui_get** - Get UI data
- **Required OAuth Scopes (any of)**: `repo`, `read:org`
- **Required OAuth Scopes**: `repo`, `read:org`
- **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org`
- `method`: The type of data to fetch (string, required)
- `owner`: Repository owner (required for all methods) (string, required)
Expand Down
23 changes: 14 additions & 9 deletions pkg/github/scope_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ var repoScopesSet = map[string]bool{
string(scopes.PublicRepo): true,
}

// onlyRequiresRepoScopes returns true if all of the tool's accepted scopes
// onlyRequiresRepoScopes returns true if all of the tool's required scopes
// are repo-related scopes (repo, public_repo). Such tools work on public
// repositories without needing any scope.
func onlyRequiresRepoScopes(acceptedScopes []string) bool {
if len(acceptedScopes) == 0 {
func onlyRequiresRepoScopes(requiredScopes []string) bool {
if len(requiredScopes) == 0 {
return false
}
for _, scope := range acceptedScopes {
for _, scope := range requiredScopes {
if !repoScopesSet[scope] {
return false
}
Expand All @@ -41,9 +41,13 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool {
// token is known at startup and won't change during the session.
//
// The filter returns true (include tool) if:
// - The tool has no scope requirements (AcceptedScopes is empty)
// - The tool has no scope requirements (RequiredScopes is empty)
// - The tool is read-only and only requires repo/public_repo scopes (works on public repos)
// - The token has at least one of the tool's accepted scopes
// - The token satisfies ALL of the tool's required scopes (AND-of-ORs, where
// each required scope may be met directly or by a higher scope)
//
// RequiredScopes is the single source of truth here; AcceptedScopes is
// display-only metadata and is intentionally not consulted.
//
// Example usage:
//
Expand All @@ -55,10 +59,11 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool {
// inventory := github.NewInventory(t).WithFilter(filter).Build()
func CreateToolScopeFilter(tokenScopes []string) inventory.ToolFilter {
return func(_ context.Context, tool *inventory.ServerTool) (bool, error) {
// Read-only tools requiring only repo/public_repo work on public repos without any scope
if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && onlyRequiresRepoScopes(tool.AcceptedScopes) {
// Read-only tools requiring only repo/public_repo work on public repos without any scope.
// Tools that also require a non-repo scope (e.g. {repo, read:org}) fall through to the AND check.
if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && onlyRequiresRepoScopes(tool.RequiredScopes) {
return true, nil
}
return scopes.HasRequiredScopes(tokenScopes, tool.AcceptedScopes), nil
return scopes.HasRequiredScopes(tokenScopes, tool.RequiredScopes), nil
}
}
66 changes: 45 additions & 21 deletions pkg/github/scope_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,51 +11,56 @@ import (
)

func TestCreateToolScopeFilter(t *testing.T) {
// Create test tools with various scope requirements
// Create test tools with various scope requirements.
// RequiredScopes is the single source of truth for filtering.
toolNoScopes := &inventory.ServerTool{
Tool: mcp.Tool{Name: "no_scopes_tool"},
AcceptedScopes: nil,
RequiredScopes: nil,
}

toolEmptyScopes := &inventory.ServerTool{
Tool: mcp.Tool{Name: "empty_scopes_tool"},
AcceptedScopes: []string{},
RequiredScopes: []string{},
}

toolRepoScope := &inventory.ServerTool{
Tool: mcp.Tool{Name: "repo_tool"},
AcceptedScopes: []string{"repo"},
RequiredScopes: []string{"repo"},
}

toolRepoScopeReadOnly := &inventory.ServerTool{
Tool: mcp.Tool{
Name: "repo_tool_readonly",
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true},
},
AcceptedScopes: []string{"repo"},
RequiredScopes: []string{"repo"},
}

toolPublicRepoScope := &inventory.ServerTool{
Tool: mcp.Tool{Name: "public_repo_tool"},
AcceptedScopes: []string{"public_repo", "repo"}, // repo is parent, also accepted
RequiredScopes: []string{"public_repo"},
}

toolPublicRepoScopeReadOnly := &inventory.ServerTool{
Tool: mcp.Tool{
Name: "public_repo_tool_readonly",
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true},
},
AcceptedScopes: []string{"public_repo", "repo"},
RequiredScopes: []string{"public_repo"},
}

toolGistScope := &inventory.ServerTool{
Tool: mcp.Tool{Name: "gist_tool"},
AcceptedScopes: []string{"gist"},
RequiredScopes: []string{"gist"},
}

toolMultiScope := &inventory.ServerTool{
Tool: mcp.Tool{Name: "multi_scope_tool"},
AcceptedScopes: []string{"repo", "admin:org"},
// Models ui_get / list_issue_fields: read-only, but requires repo AND read:org.
toolRepoAndReadOrg := &inventory.ServerTool{
Tool: mcp.Tool{
Name: "ui_get",
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true},
},
RequiredScopes: []string{"repo", "read:org"},
}

tests := []struct {
Expand Down Expand Up @@ -100,12 +105,6 @@ func TestCreateToolScopeFilter(t *testing.T) {
tool: toolGistScope,
expected: false,
},
{
name: "token with one of multiple accepted scopes can see tool",
tokenScopes: []string{"admin:org"},
tool: toolMultiScope,
expected: true,
},
{
name: "empty token scopes cannot see scoped tools",
tokenScopes: []string{},
Expand All @@ -130,6 +129,24 @@ func TestCreateToolScopeFilter(t *testing.T) {
tool: toolPublicRepoScope,
expected: true,
},
{
name: "AND: repo-only classic PAT hides {repo, read:org} tool",
tokenScopes: []string{"repo"},
tool: toolRepoAndReadOrg,
expected: false,
},
{
name: "AND: {repo, read:org} token shows {repo, read:org} tool",
tokenScopes: []string{"repo", "read:org"},
tool: toolRepoAndReadOrg,
expected: true,
},
{
name: "AND: {repo, admin:org} token shows {repo, read:org} tool via hierarchy",
tokenScopes: []string{"repo", "admin:org"},
tool: toolRepoAndReadOrg,
expected: true,
},
}

for _, tt := range tests {
Expand All @@ -149,17 +166,23 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) {
{
Tool: mcp.Tool{Name: "public_tool"},
Toolset: inventory.ToolsetMetadata{ID: "test"},
AcceptedScopes: nil, // No scopes required
RequiredScopes: nil, // No scopes required
},
{
Tool: mcp.Tool{Name: "repo_tool"},
Toolset: inventory.ToolsetMetadata{ID: "test"},
AcceptedScopes: []string{"repo"},
RequiredScopes: []string{"repo"},
},
{
Tool: mcp.Tool{Name: "gist_tool"},
Toolset: inventory.ToolsetMetadata{ID: "test"},
AcceptedScopes: []string{"gist"},
RequiredScopes: []string{"gist"},
},
{
// Requires repo AND read:org; hidden for a {repo}-only token.
Tool: mcp.Tool{Name: "list_issue_fields"},
Toolset: inventory.ToolsetMetadata{ID: "test"},
RequiredScopes: []string{"repo", "read:org"},
},
}

Expand All @@ -177,7 +200,7 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) {
// Get available tools
availableTools := inv.AvailableTools(context.Background())

// Should see public_tool and repo_tool, but not gist_tool
// Should see public_tool and repo_tool, but not gist_tool or list_issue_fields
assert.Len(t, availableTools, 2)

toolNames := make([]string, len(availableTools))
Expand All @@ -188,4 +211,5 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) {
assert.Contains(t, toolNames, "public_tool")
assert.Contains(t, toolNames, "repo_tool")
assert.NotContains(t, toolNames, "gist_tool")
assert.NotContains(t, toolNames, "list_issue_fields")
}
16 changes: 8 additions & 8 deletions pkg/http/middleware/scope_challenge.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,31 +109,31 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter
ctx = ghcontext.WithTokenScopes(ctx, activeScopes)
r = r.WithContext(ctx)

// Check if user has the required scopes
if toolScopeInfo.HasAcceptedScope(activeScopes...) {
// Check if user satisfies all required scopes
if toolScopeInfo.Satisfies(activeScopes...) {
next.ServeHTTP(w, r)
return
}

// User lacks required scopes - get the scopes they need
requiredScopes := toolScopeInfo.GetRequiredScopesSlice()
// User lacks one or more required scopes - request only the missing ones
missingScopes := toolScopeInfo.MissingScopes(activeScopes...)

// Build the resource metadata URL using the shared utility
// GetEffectiveResourcePath returns the original path (e.g., /mcp or /mcp/x/all)
// which is used to construct the well-known OAuth protected resource URL
resourcePath := oauth.ResolveResourcePath(r, oauthCfg)
resourceMetadataURL := oauth.BuildResourceMetadataURL(r, oauthCfg, resourcePath)

// Build recommended scopes: existing scopes + required scopes
recommendedScopes := make([]string, 0, len(activeScopes)+len(requiredScopes))
// Build recommended scopes: existing scopes + missing required scopes
recommendedScopes := make([]string, 0, len(activeScopes)+len(missingScopes))
recommendedScopes = append(recommendedScopes, activeScopes...)
recommendedScopes = append(recommendedScopes, requiredScopes...)
recommendedScopes = append(recommendedScopes, missingScopes...)

// Build the WWW-Authenticate header value
wwwAuthenticateHeader := fmt.Sprintf(`Bearer error="insufficient_scope", scope=%q, resource_metadata=%q, error_description=%q`,
strings.Join(recommendedScopes, " "),
resourceMetadataURL,
"Additional scopes required: "+strings.Join(requiredScopes, ", "),
"Additional scopes required: "+strings.Join(missingScopes, ", "),
)

// Send scope challenge response with the superset of existing and required scopes
Expand Down
Loading
Loading