Skip to content

Commit 907e5cb

Browse files
committed
auth: fail closed startup on unmet token scopes (#2075)
1 parent b222072 commit 907e5cb

File tree

2 files changed

+164
-9
lines changed

2 files changed

+164
-9
lines changed

internal/ghmcp/server.go

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"os"
1010
"os/signal"
11+
"sort"
1112
"strings"
1213
"syscall"
1314
"time"
@@ -248,20 +249,49 @@ func RunStdioServer(cfg StdioServerConfig) error {
248249
logger := slog.New(slogHandler)
249250
logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode)
250251

251-
// Fetch token scopes for scope-based tool filtering (PAT tokens only)
252-
// Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header.
253-
// Fine-grained PATs and other token types don't support this, so we skip filtering.
252+
featureChecker := createFeatureChecker(cfg.EnabledFeatures)
253+
254+
// Fetch token scopes for scope-based tool filtering and startup validation.
255+
// We currently fail closed for classic PAT and OAuth access tokens where scopes
256+
// can be resolved deterministically.
254257
var tokenScopes []string
255-
if strings.HasPrefix(cfg.Token, "ghp_") {
258+
if shouldValidateTokenScopesAtStartup(cfg.Token) {
256259
fetchedScopes, err := fetchTokenScopesForHost(ctx, cfg.Token, cfg.Host)
257260
if err != nil {
258-
logger.Warn("failed to fetch token scopes, continuing without scope filtering", "error", err)
259-
} else {
260-
tokenScopes = fetchedScopes
261-
logger.Info("token scopes fetched for filtering", "scopes", tokenScopes)
261+
return fmt.Errorf("scope requirements check failed: unable to fetch token scopes: %w", err)
262262
}
263+
tokenScopes = fetchedScopes
264+
logger.Info("token scopes fetched for filtering", "scopes", tokenScopes)
263265
} else {
264-
logger.Debug("skipping scope filtering for non-PAT token")
266+
logger.Debug("skipping startup scope validation for token type")
267+
}
268+
269+
if shouldValidateTokenScopesAtStartup(cfg.Token) {
270+
startupInventory, err := github.NewInventory(t).
271+
WithDeprecatedAliases(github.DeprecatedToolAliases).
272+
WithReadOnly(cfg.ReadOnly).
273+
WithToolsets(github.ResolvedEnabledToolsets(cfg.DynamicToolsets, cfg.EnabledToolsets, cfg.EnabledTools)).
274+
WithTools(github.CleanTools(cfg.EnabledTools)).
275+
WithExcludeTools(cfg.ExcludeTools).
276+
WithServerInstructions().
277+
WithFeatureChecker(featureChecker).
278+
WithInsidersMode(cfg.InsidersMode).
279+
Build()
280+
if err != nil {
281+
return fmt.Errorf("failed to build inventory for scope validation: %w", err)
282+
}
283+
284+
missingScopes, blockedTools, err := evaluateScopeRequirements(startupInventory.AllTools(), tokenScopes)
285+
if err != nil {
286+
return fmt.Errorf("failed to evaluate token scope requirements: %w", err)
287+
}
288+
if len(blockedTools) > 0 {
289+
return fmt.Errorf(
290+
"scope requirements unmet at startup: missing scopes [%s]; blocked tools [%s]",
291+
strings.Join(missingScopes, ", "),
292+
strings.Join(blockedTools, ", "),
293+
)
294+
}
265295
}
266296

267297
ghServer, err := NewStdioMCPServer(ctx, github.MCPServerConfig{
@@ -327,6 +357,43 @@ func RunStdioServer(cfg StdioServerConfig) error {
327357
return nil
328358
}
329359

360+
func shouldValidateTokenScopesAtStartup(token string) bool {
361+
return strings.HasPrefix(token, "ghp_") || strings.HasPrefix(token, "gho_")
362+
}
363+
364+
func evaluateScopeRequirements(tools []inventory.ServerTool, tokenScopes []string) ([]string, []string, error) {
365+
filter := github.CreateToolScopeFilter(tokenScopes)
366+
missingScopeSet := make(map[string]struct{})
367+
blockedTools := make([]string, 0)
368+
369+
for i := range tools {
370+
allowed, err := filter(context.Background(), &tools[i])
371+
if err != nil {
372+
return nil, nil, err
373+
}
374+
if allowed {
375+
continue
376+
}
377+
378+
blockedTools = append(blockedTools, tools[i].Tool.Name)
379+
for _, required := range tools[i].RequiredScopes {
380+
if required == "" {
381+
continue
382+
}
383+
missingScopeSet[required] = struct{}{}
384+
}
385+
}
386+
387+
missingScopes := make([]string, 0, len(missingScopeSet))
388+
for scope := range missingScopeSet {
389+
missingScopes = append(missingScopes, scope)
390+
}
391+
sort.Strings(missingScopes)
392+
sort.Strings(blockedTools)
393+
394+
return missingScopes, blockedTools, nil
395+
}
396+
330397
// createFeatureChecker returns a FeatureFlagChecker that checks if a flag name
331398
// is present in the provided list of enabled features. For the local server,
332399
// this is populated from the --features CLI flag.
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package ghmcp
2+
3+
import (
4+
"testing"
5+
6+
"github.com/github/github-mcp-server/pkg/inventory"
7+
"github.com/modelcontextprotocol/go-sdk/mcp"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func makeScopeTestTool(
12+
name string,
13+
readOnly bool,
14+
requiredScopes []string,
15+
acceptedScopes []string,
16+
) inventory.ServerTool {
17+
return inventory.ServerTool{
18+
Tool: mcp.Tool{
19+
Name: name,
20+
Annotations: &mcp.ToolAnnotations{
21+
ReadOnlyHint: readOnly,
22+
},
23+
},
24+
RequiredScopes: requiredScopes,
25+
AcceptedScopes: acceptedScopes,
26+
}
27+
}
28+
29+
func TestShouldValidateTokenScopesAtStartup(t *testing.T) {
30+
require.True(t, shouldValidateTokenScopesAtStartup("ghp_test"))
31+
require.True(t, shouldValidateTokenScopesAtStartup("gho_test"))
32+
require.False(t, shouldValidateTokenScopesAtStartup("ghs_test"))
33+
require.False(t, shouldValidateTokenScopesAtStartup("github_pat_test"))
34+
}
35+
36+
func TestEvaluateScopeRequirementsReportsMissingScopesAndBlockedTools(t *testing.T) {
37+
tools := []inventory.ServerTool{
38+
makeScopeTestTool(
39+
"repo_write",
40+
false,
41+
[]string{"repo"},
42+
[]string{"repo"},
43+
),
44+
}
45+
46+
missingScopes, blockedTools, err := evaluateScopeRequirements(tools, []string{})
47+
require.NoError(t, err)
48+
require.Equal(t, []string{"repo"}, missingScopes)
49+
require.Equal(t, []string{"repo_write"}, blockedTools)
50+
}
51+
52+
func TestEvaluateScopeRequirementsAllowsReadOnlyRepoToolsWithoutScopes(t *testing.T) {
53+
tools := []inventory.ServerTool{
54+
makeScopeTestTool(
55+
"repo_read_only",
56+
true,
57+
[]string{"repo"},
58+
[]string{"repo", "public_repo"},
59+
),
60+
}
61+
62+
missingScopes, blockedTools, err := evaluateScopeRequirements(tools, []string{})
63+
require.NoError(t, err)
64+
require.Empty(t, missingScopes)
65+
require.Empty(t, blockedTools)
66+
}
67+
68+
func TestEvaluateScopeRequirementsSortsOutputDeterministically(t *testing.T) {
69+
tools := []inventory.ServerTool{
70+
makeScopeTestTool(
71+
"z_tool",
72+
false,
73+
[]string{"admin:org"},
74+
[]string{"admin:org"},
75+
),
76+
makeScopeTestTool(
77+
"a_tool",
78+
false,
79+
[]string{"repo"},
80+
[]string{"repo"},
81+
),
82+
}
83+
84+
missingScopes, blockedTools, err := evaluateScopeRequirements(tools, []string{})
85+
require.NoError(t, err)
86+
require.Equal(t, []string{"admin:org", "repo"}, missingScopes)
87+
require.Equal(t, []string{"a_tool", "z_tool"}, blockedTools)
88+
}

0 commit comments

Comments
 (0)