Skip to content

Commit 442d43f

Browse files
feat(auth): model alternative scope policies
Represent authorization as alternative execution paths with conjunctive requirements and per-requirement accepted scopes. Resolve conditional tool policies from call arguments while preserving legacy scope metadata. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 6db7748 commit 442d43f

19 files changed

Lines changed: 558 additions & 168 deletions

docs/scope-filtering.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ When the server starts with a classic PAT, it makes a lightweight HTTP HEAD requ
2222

2323
With OAuth, the remote server can dynamically request additional scopes as needed. With PATs, scopes are fixed at token creation, so the server proactively hides tools you can't use.
2424

25+
## Scope Policies
26+
27+
Tool scopes are modeled as authorization paths rather than a flat list:
28+
29+
- A tool may support multiple paths, and satisfying any path is sufficient.
30+
- Every independent requirement within the selected path must be satisfied.
31+
- A requirement may accept multiple alternative scopes, including broader scopes from the hierarchy.
32+
- Argument-dependent tools resolve the exact path at call time before an OAuth challenge is issued.
33+
34+
For example, repository deletion requires `repo` **and** `delete_repo`, while listing issue fields requires `repo` for a repository request or `read:org` for an organization request. Updating an ordinary file requires `repo`; updating a workflow file requires both `repo` and `workflow`.
35+
36+
PAT filtering runs before call arguments are known, so it keeps a tool when any supported path is usable. OAuth challenges resolve the actual arguments and request only the preferred missing scopes for that call.
37+
2538
## OAuth Scope Challenges (Remote Server)
2639

2740
When using the [remote MCP server](./remote-server.md) with OAuth authentication, the server uses a different approach called **scope challenges**. Instead of hiding tools upfront, all tools are available, and the server requests additional scopes on-demand when you try to use a tool that requires them.

pkg/github/dependencies.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,10 @@ func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
230230
// The handler function receives deps extracted from context via MustDepsFromContext.
231231
// Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked.
232232
//
233-
// requiredScopes specifies the minimum OAuth scopes needed for this tool.
234-
// AcceptedScopes are automatically derived using the scope hierarchy (e.g., if
235-
// public_repo is required, repo is also accepted since repo grants public_repo).
233+
// requiredScopes specifies alternative minimum OAuth scopes for this tool.
234+
// NewTool preserves the established any-of behavior; tools with conjunctive or
235+
// argument-dependent requirements should set ScopePolicy and ScopeResolver.
236+
// AcceptedScopes are automatically derived using the scope hierarchy.
236237
func NewTool[In, Out any](
237238
toolset inventory.ToolsetMetadata,
238239
tool mcp.Tool,
@@ -245,6 +246,9 @@ func NewTool[In, Out any](
245246
})
246247
st.RequiredScopes = scopes.ToStringSlice(requiredScopes...)
247248
st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...)
249+
if len(requiredScopes) > 0 {
250+
st.ScopePolicy = scopes.AnyOfScopePolicy(requiredScopes...)
251+
}
248252
return st
249253
}
250254

@@ -254,8 +258,9 @@ func NewTool[In, Out any](
254258
// The handler function receives deps extracted from context via MustDepsFromContext.
255259
// Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked.
256260
//
257-
// requiredScopes specifies the minimum OAuth scopes needed for this tool.
258-
// AcceptedScopes are automatically derived using the scope hierarchy.
261+
// requiredScopes specifies alternative minimum OAuth scopes for this tool.
262+
// NewToolFromHandler preserves the established any-of behavior; tools with
263+
// richer requirements should set ScopePolicy and ScopeResolver.
259264
func NewToolFromHandler(
260265
toolset inventory.ToolsetMetadata,
261266
tool mcp.Tool,
@@ -268,6 +273,9 @@ func NewToolFromHandler(
268273
})
269274
st.RequiredScopes = scopes.ToStringSlice(requiredScopes...)
270275
st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...)
276+
if len(requiredScopes) > 0 {
277+
st.ScopePolicy = scopes.AnyOfScopePolicy(requiredScopes...)
278+
}
271279
return st
272280
}
273281

pkg/github/issue_fields.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool
167167
}
168168
return result, nil, nil
169169
})
170+
st.ScopeResolver = repositoryOrOrganizationScopePolicy
170171
return st
171172
}
172173

pkg/github/issues.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1256,7 +1256,7 @@ func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string,
12561256
// ListIssueTypes creates a tool to list defined issue types for an organization or repository.
12571257
// This can be used to understand supported issue type values for creating or updating issues.
12581258
func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool {
1259-
return NewTool(
1259+
st := NewTool(
12601260
ToolsetMetadataIssues,
12611261
mcp.Tool{
12621262
Name: "list_issue_types",
@@ -1354,6 +1354,8 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool {
13541354
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoMetadata(true))
13551355
return result, nil, nil
13561356
})
1357+
st.ScopeResolver = repositoryOrOrganizationScopePolicy
1358+
return st
13571359
}
13581360

13591361
// AddIssueComment creates a tool to add a comment or reaction to an issue.

pkg/github/repositories.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ SHA MUST be provided for existing file updates.
625625
return MarshalledTextResult(minimalResponse), nil, nil
626626
},
627627
)
628-
tool.ScopeResolver = workflowScopeForPath
628+
tool.ScopeResolver = workflowScopePolicyForPath
629629
return tool
630630
}
631631

@@ -911,6 +911,7 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
911911
tool.MinimumProtocolVersion = inventory.ProtocolVersionMultiRoundTrip
912912
tool.RequiredElicitationMode = inventory.ElicitationModeForm
913913
tool.RequiredScopeGroups = scopes.ExpandScopeGroups(scopes.DeleteRepo, scopes.Repo)
914+
tool.ScopePolicy = scopes.AllOfScopePolicy(scopes.DeleteRepo, scopes.Repo)
914915
return tool
915916
}
916917

@@ -1481,7 +1482,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool {
14811482
return utils.NewToolResultText(string(r)), nil, nil
14821483
},
14831484
)
1484-
tool.ScopeResolver = workflowScopeForPath
1485+
tool.ScopeResolver = workflowScopePolicyForPath
14851486
return tool
14861487
}
14871488

@@ -1832,7 +1833,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
18321833
return utils.NewToolResultText(string(r)), nil, nil
18331834
},
18341835
)
1835-
tool.ScopeResolver = workflowScopeForFiles
1836+
tool.ScopeResolver = workflowScopePolicyForFiles
18361837
return tool
18371838
}
18381839

pkg/github/repository_path.go

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import (
66
"slices"
77
"strings"
88

9+
"github.com/github/github-mcp-server/pkg/inventory"
910
"github.com/github/github-mcp-server/pkg/scopes"
1011
)
1112

1213
const workflowPathPrefix = ".github/workflows/"
1314

1415
func validateRelativePath(value string) (string, error) {
16+
value = strings.TrimPrefix(value, "/")
1517
if value == "" {
1618
return "", fmt.Errorf("path must not be empty")
1719
}
@@ -36,31 +38,42 @@ func isWorkflowPath(value string) bool {
3638
return strings.HasPrefix(value, workflowPathPrefix) && len(value) > len(workflowPathPrefix)
3739
}
3840

39-
func workflowScopeForPath(arguments map[string]any) []string {
41+
func workflowScopePolicyForPath(arguments map[string]any) inventory.ScopePolicy {
4042
value, ok := arguments["path"].(string)
4143
if !ok {
42-
return nil
44+
return scopes.UnscopedScopePolicy()
4345
}
4446
cleaned, err := validateRelativePath(value)
45-
if err != nil || !isWorkflowPath(cleaned) {
46-
return nil
47+
if err != nil {
48+
return scopes.UnscopedScopePolicy()
4749
}
48-
return []string{string(scopes.Workflow)}
50+
if !isWorkflowPath(cleaned) {
51+
return scopes.AllOfScopePolicy(scopes.Repo)
52+
}
53+
return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow)
4954
}
5055

51-
func workflowScopeForFiles(arguments map[string]any) []string {
56+
func workflowScopePolicyForFiles(arguments map[string]any) inventory.ScopePolicy {
5257
files, ok := arguments["files"].([]any)
5358
if !ok {
54-
return nil
59+
return scopes.UnscopedScopePolicy()
5560
}
5661
for _, file := range files {
5762
fileMap, ok := file.(map[string]any)
5863
if !ok {
59-
continue
64+
return scopes.UnscopedScopePolicy()
65+
}
66+
value, ok := fileMap["path"].(string)
67+
if !ok {
68+
return scopes.UnscopedScopePolicy()
69+
}
70+
cleaned, err := validateRelativePath(value)
71+
if err != nil {
72+
return scopes.UnscopedScopePolicy()
6073
}
61-
if len(workflowScopeForPath(fileMap)) > 0 {
62-
return []string{string(scopes.Workflow)}
74+
if isWorkflowPath(cleaned) {
75+
return scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow)
6376
}
6477
}
65-
return nil
78+
return scopes.AllOfScopePolicy(scopes.Repo)
6679
}

pkg/github/repository_path_test.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/github/github-mcp-server/pkg/inventory"
8+
"github.com/github/github-mcp-server/pkg/scopes"
89
"github.com/github/github-mcp-server/pkg/translations"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
@@ -18,11 +19,12 @@ func TestValidateRelativePath(t *testing.T) {
1819
wantErr string
1920
}{
2021
{name: "file", value: "docs/readme.md", want: "docs/readme.md"},
22+
{name: "normalizes leading slash", value: "/docs/readme.md", want: "docs/readme.md"},
2123
{name: "normalizes dot segment", value: "./.github/workflows/ci.yml", want: ".github/workflows/ci.yml"},
2224
{name: "normalizes duplicate separator", value: ".github//workflows/ci.yml", want: ".github/workflows/ci.yml"},
2325
{name: "empty", value: "", wantErr: "must not be empty"},
2426
{name: "current directory", value: ".", wantErr: "must identify a file"},
25-
{name: "absolute", value: "/.github/workflows/ci.yml", wantErr: "must be relative"},
27+
{name: "double leading slash", value: "//.github/workflows/ci.yml", wantErr: "must be relative"},
2628
{name: "parent traversal", value: "docs/../.github/workflows/ci.yml", wantErr: "parent directory traversal"},
2729
{name: "leading traversal", value: "../.github/workflows/ci.yml", wantErr: "parent directory traversal"},
2830
{name: "backslash traversal", value: `docs\..\.github\workflows\ci.yml`, wantErr: "forward slashes"},
@@ -47,34 +49,37 @@ func TestFileWriteWorkflowScopeResolvers(t *testing.T) {
4749
name string
4850
tool inventory.ServerTool
4951
args map[string]any
50-
want []string
52+
want inventory.ScopePolicy
5153
}{
5254
{
5355
name: "create regular file",
5456
tool: CreateOrUpdateFile(translations.NullTranslationHelper),
5557
args: map[string]any{"path": "docs/readme.md"},
58+
want: scopes.AllOfScopePolicy(scopes.Repo),
5659
},
5760
{
5861
name: "create workflow",
5962
tool: CreateOrUpdateFile(translations.NullTranslationHelper),
6063
args: map[string]any{"path": ".github/workflows/ci.yml"},
61-
want: []string{"workflow"},
64+
want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow),
6265
},
6366
{
6467
name: "delete normalized workflow",
6568
tool: DeleteFile(translations.NullTranslationHelper),
6669
args: map[string]any{"path": "./.github/workflows/ci.yml"},
67-
want: []string{"workflow"},
70+
want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow),
6871
},
6972
{
7073
name: "reject traversal instead of resolving it",
7174
tool: DeleteFile(translations.NullTranslationHelper),
7275
args: map[string]any{"path": "docs/../.github/workflows/ci.yml"},
76+
want: scopes.UnscopedScopePolicy(),
7377
},
7478
{
7579
name: "push regular files",
7680
tool: PushFiles(translations.NullTranslationHelper),
7781
args: map[string]any{"files": []any{map[string]any{"path": "README.md"}}},
82+
want: scopes.AllOfScopePolicy(scopes.Repo),
7883
},
7984
{
8085
name: "push includes workflow",
@@ -83,7 +88,7 @@ func TestFileWriteWorkflowScopeResolvers(t *testing.T) {
8388
map[string]any{"path": "README.md"},
8489
map[string]any{"path": ".github/workflows/ci.yml"},
8590
}},
86-
want: []string{"workflow"},
91+
want: scopes.AllOfScopePolicy(scopes.Repo, scopes.Workflow),
8792
},
8893
}
8994

@@ -113,7 +118,7 @@ func TestFileWriteToolsRejectUnsafePathsBeforeAPICalls(t *testing.T) {
113118
name: "delete",
114119
tool: DeleteFile(translations.NullTranslationHelper),
115120
args: map[string]any{
116-
"owner": "owner", "repo": "repo", "path": "/.github/workflows/ci.yml",
121+
"owner": "owner", "repo": "repo", "path": "//.github/workflows/ci.yml",
117122
"message": "message", "branch": "main",
118123
},
119124
},

pkg/github/scope_filter.go

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,35 @@ var repoScopesSet = map[string]bool{
1515
string(scopes.PublicRepo): true,
1616
}
1717

18-
// onlyRequiresRepoScopes returns true if all of the tool's accepted scopes
19-
// are repo-related scopes (repo, public_repo). Such tools work on public
20-
// repositories without needing any scope.
21-
func onlyRequiresRepoScopes(acceptedScopes []string) bool {
22-
if len(acceptedScopes) == 0 {
23-
return false
24-
}
25-
for _, scope := range acceptedScopes {
26-
if !repoScopesSet[scope] {
27-
return false
18+
// hasRepoOnlyScopeAlternative returns true if at least one authorization path
19+
// uses only repo-related scopes. Such a path may work on a public repository
20+
// without any token scope.
21+
func hasRepoOnlyScopeAlternative(policy inventory.ScopePolicy) bool {
22+
for _, alternative := range policy.AnyOf {
23+
if len(alternative.AllOf) == 0 {
24+
continue
25+
}
26+
repoOnly := true
27+
for _, requirement := range alternative.AllOf {
28+
accepted := requirement.AcceptedScopes
29+
if len(accepted) == 0 {
30+
accepted = []string{requirement.RequiredScope}
31+
}
32+
for _, scope := range accepted {
33+
if !repoScopesSet[scope] {
34+
repoOnly = false
35+
break
36+
}
37+
}
38+
if !repoOnly {
39+
break
40+
}
41+
}
42+
if repoOnly {
43+
return true
2844
}
2945
}
30-
return true
46+
return false
3147
}
3248

3349
// CreateToolScopeFilter creates an inventory.ToolFilter that filters tools
@@ -41,9 +57,9 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool {
4157
// token is known at startup and won't change during the session.
4258
//
4359
// The filter returns true (include tool) if:
44-
// - The tool has no scope requirements (AcceptedScopes is empty)
60+
// - The tool has an unscoped authorization path
4561
// - The tool is read-only and only requires repo/public_repo scopes (works on public repos)
46-
// - The token has at least one of the tool's accepted scopes
62+
// - The token satisfies every requirement in any authorization path
4763
//
4864
// Example usage:
4965
//
@@ -55,13 +71,11 @@ func onlyRequiresRepoScopes(acceptedScopes []string) bool {
5571
// inventory := github.NewInventory(t).WithFilter(filter).Build()
5672
func CreateToolScopeFilter(tokenScopes []string) inventory.ToolFilter {
5773
return func(_ context.Context, tool *inventory.ServerTool) (bool, error) {
74+
policy := scopes.ScopePolicyForTool(tool)
5875
// Read-only tools requiring only repo/public_repo work on public repos without any scope
59-
if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && onlyRequiresRepoScopes(tool.AcceptedScopes) {
76+
if tool.Tool.Annotations != nil && tool.Tool.Annotations.ReadOnlyHint && hasRepoOnlyScopeAlternative(policy) {
6077
return true, nil
6178
}
62-
if len(tool.RequiredScopeGroups) > 0 {
63-
return scopes.HasRequiredScopeGroups(tokenScopes, tool.RequiredScopeGroups), nil
64-
}
65-
return scopes.HasRequiredScopes(tokenScopes, tool.AcceptedScopes), nil
79+
return scopes.ScopePolicySatisfied(tokenScopes, policy), nil
6680
}
6781
}

pkg/github/scope_filter_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) {
211211

212212
func TestCreateToolScopeFilterPreservesExistingMultiScopeSemantics(t *testing.T) {
213213
filter := CreateToolScopeFilter([]string{"repo"})
214+
unscopedFilter := CreateToolScopeFilter(nil)
214215
tools := []inventory.ServerTool{
215216
ListIssueFields(translations.NullTranslationHelper),
216217
ListIssueTypes(translations.NullTranslationHelper),
@@ -222,5 +223,9 @@ func TestCreateToolScopeFilterPreservesExistingMultiScopeSemantics(t *testing.T)
222223
require.NoError(t, err)
223224
assert.True(t, allowed, "%s should remain visible with a repo-only token", tools[i].Tool.Name)
224225
assert.Empty(t, tools[i].RequiredScopeGroups, "%s should retain legacy any-of scope semantics", tools[i].Tool.Name)
226+
227+
allowed, err = unscopedFilter(context.Background(), &tools[i])
228+
require.NoError(t, err)
229+
assert.True(t, allowed, "%s should remain visible when a public-repository path is plausible", tools[i].Tool.Name)
225230
}
226231
}

0 commit comments

Comments
 (0)