From beb728a646338d214ec9f4230aa1e058bbe1b57d Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Tue, 10 Feb 2026 15:47:18 +0100 Subject: [PATCH 1/8] Move scope storage into its own context key, separately from token info. This allows us to provide scopes seperately in the remote server, where we have scopes before we do the auth. --- pkg/context/token.go | 23 +++++++++++++++++++---- pkg/http/handler.go | 6 ++++-- pkg/http/middleware/pat_scope.go | 12 ++++++++---- pkg/http/middleware/pat_scope_test.go | 19 +++++++++++-------- pkg/http/middleware/scope_challenge.go | 4 +--- 5 files changed, 43 insertions(+), 21 deletions(-) diff --git a/pkg/context/token.go b/pkg/context/token.go index beddb02b2..3cb3707da 100644 --- a/pkg/context/token.go +++ b/pkg/context/token.go @@ -12,10 +12,8 @@ type tokenCtx string var tokenCtxKey tokenCtx = "tokenctx" type TokenInfo struct { - Token string - TokenType utils.TokenType - ScopesFetched bool - Scopes []string + Token string + TokenType utils.TokenType } // WithTokenInfo adds TokenInfo to the context @@ -30,3 +28,20 @@ func GetTokenInfo(ctx context.Context) (*TokenInfo, bool) { } return nil, false } + +type TokenScopesKey tokenCtx + +var tokenScopesKey TokenScopesKey = "tokenscopesctx" + +// WithTokenScopes adds token scopes to the context +func WithTokenScopes(ctx context.Context, scopes []string) context.Context { + return context.WithValue(ctx, tokenScopesKey, scopes) +} + +// GetTokenScopes retrieves token scopes from the context +func GetTokenScopes(ctx context.Context) ([]string, bool) { + if scopes, ok := ctx.Value(tokenScopesKey).([]string); ok { + return scopes, true + } + return nil, false +} diff --git a/pkg/http/handler.go b/pkg/http/handler.go index df0b819fc..a550c203f 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -271,8 +271,10 @@ func PATScopeFilter(b *inventory.Builder, r *http.Request, fetcher scopes.Fetche // Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header. // Fine-grained PATs and other token types don't support this, so we skip filtering. if tokenInfo.TokenType == utils.TokenTypePersonalAccessToken { - if tokenInfo.ScopesFetched { - return b.WithFilter(github.CreateToolScopeFilter(tokenInfo.Scopes)) + // Check if scopes are already in context (should be set by WithPATScopes). If not, fetch them. + existingScopes, ok := ghcontext.GetTokenScopes(ctx) + if ok { + return b.WithFilter(github.CreateToolScopeFilter(existingScopes)) } scopesList, err := fetcher.FetchTokenScopes(ctx, tokenInfo.Token) diff --git a/pkg/http/middleware/pat_scope.go b/pkg/http/middleware/pat_scope.go index 8b77b3d32..bb1efdc01 100644 --- a/pkg/http/middleware/pat_scope.go +++ b/pkg/http/middleware/pat_scope.go @@ -26,6 +26,13 @@ func WithPATScopes(logger *slog.Logger, scopeFetcher scopes.FetcherInterface) fu // Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header. // Fine-grained PATs and other token types don't support this, so we skip filtering. if tokenInfo.TokenType == utils.TokenTypePersonalAccessToken { + existingScopes, ok := ghcontext.GetTokenScopes(ctx) + if ok { + logger.Debug("using existing scopes from context", "scopes", existingScopes) + next.ServeHTTP(w, r) + return + } + scopesList, err := scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) if err != nil { logger.Warn("failed to fetch PAT scopes", "error", err) @@ -33,11 +40,8 @@ func WithPATScopes(logger *slog.Logger, scopeFetcher scopes.FetcherInterface) fu return } - tokenInfo.Scopes = scopesList - tokenInfo.ScopesFetched = true - // Store fetched scopes in context for downstream use - ctx := ghcontext.WithTokenInfo(ctx, tokenInfo) + ctx = ghcontext.WithTokenScopes(ctx, scopesList) next.ServeHTTP(w, r.WithContext(ctx)) return diff --git a/pkg/http/middleware/pat_scope_test.go b/pkg/http/middleware/pat_scope_test.go index eb472bcf1..0607b8cf2 100644 --- a/pkg/http/middleware/pat_scope_test.go +++ b/pkg/http/middleware/pat_scope_test.go @@ -111,12 +111,13 @@ func TestWithPATScopes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var capturedTokenInfo *ghcontext.TokenInfo + var capturedScopes []string + var scopesFound bool var nextHandlerCalled bool nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { nextHandlerCalled = true - capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + capturedScopes, scopesFound = ghcontext.GetTokenScopes(r.Context()) w.WriteHeader(http.StatusOK) }) @@ -141,10 +142,9 @@ func TestWithPATScopes(t *testing.T) { assert.Equal(t, tt.expectNextHandlerCalled, nextHandlerCalled, "next handler called mismatch") - if tt.expectNextHandlerCalled && tt.tokenInfo != nil { - require.NotNil(t, capturedTokenInfo, "expected token info in context") - assert.Equal(t, tt.expectScopesFetched, capturedTokenInfo.ScopesFetched) - assert.Equal(t, tt.expectedScopes, capturedTokenInfo.Scopes) + if tt.expectNextHandlerCalled { + assert.Equal(t, tt.expectScopesFetched, scopesFound, "scopes found mismatch") + assert.Equal(t, tt.expectedScopes, capturedScopes) } }) } @@ -154,9 +154,12 @@ func TestWithPATScopes_PreservesExistingTokenInfo(t *testing.T) { logger := slog.Default() var capturedTokenInfo *ghcontext.TokenInfo + var capturedScopes []string + var scopesFound bool nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + capturedScopes, scopesFound = ghcontext.GetTokenScopes(r.Context()) w.WriteHeader(http.StatusOK) }) @@ -182,6 +185,6 @@ func TestWithPATScopes_PreservesExistingTokenInfo(t *testing.T) { require.NotNil(t, capturedTokenInfo) assert.Equal(t, originalTokenInfo.Token, capturedTokenInfo.Token) assert.Equal(t, originalTokenInfo.TokenType, capturedTokenInfo.TokenType) - assert.True(t, capturedTokenInfo.ScopesFetched) - assert.Equal(t, []string{"repo", "user"}, capturedTokenInfo.Scopes) + assert.True(t, scopesFound) + assert.Equal(t, []string{"repo", "user"}, capturedScopes) } diff --git a/pkg/http/middleware/scope_challenge.go b/pkg/http/middleware/scope_challenge.go index 526797241..2b8402561 100644 --- a/pkg/http/middleware/scope_challenge.go +++ b/pkg/http/middleware/scope_challenge.go @@ -102,9 +102,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter } // Store active scopes in context for downstream use - tokenInfo.Scopes = activeScopes - tokenInfo.ScopesFetched = true - ctx = ghcontext.WithTokenInfo(ctx, tokenInfo) + ctx = ghcontext.WithTokenScopes(ctx, activeScopes) r = r.WithContext(ctx) // Check if user has the required scopes From 2022dee74309c41d6f4286eb47d8df7c5ae1616a Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Tue, 10 Feb 2026 15:50:38 +0100 Subject: [PATCH 2/8] Skip token extraction if token info already exists in context. This is to avoid redundant token extraction in remote setup where token info may have already been extracted earlier in the request lifecycle. --- pkg/http/middleware/token.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/http/middleware/token.go b/pkg/http/middleware/token.go index c362ea201..012bbabef 100644 --- a/pkg/http/middleware/token.go +++ b/pkg/http/middleware/token.go @@ -13,6 +13,16 @@ import ( func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Check if token info already exists in context, if it does, skip extraction. + // In remote setup, we may have already extracted token info earlier. + if _, ok := ghcontext.GetTokenInfo(ctx); ok { + // Token info already exists in context, skip extraction + next.ServeHTTP(w, r) + return + } + tokenType, token, err := utils.ParseAuthorizationHeader(r) if err != nil { // For missing Authorization header, return 401 with WWW-Authenticate header per MCP spec @@ -25,7 +35,6 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl return } - ctx := r.Context() ctx = ghcontext.WithTokenInfo(ctx, &ghcontext.TokenInfo{ Token: token, TokenType: tokenType, From f1d372e2fdc11cfc7b2f24519e2cba646bfc0dd7 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Tue, 10 Feb 2026 16:06:18 +0100 Subject: [PATCH 3/8] Check for existing scopes in context before fetching from GitHub API in scope challenge middleware --- pkg/http/middleware/scope_challenge.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/http/middleware/scope_challenge.go b/pkg/http/middleware/scope_challenge.go index 2b8402561..d738cc603 100644 --- a/pkg/http/middleware/scope_challenge.go +++ b/pkg/http/middleware/scope_challenge.go @@ -94,11 +94,15 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter return } - // Get OAuth scopes from GitHub API - activeScopes, err := scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) - if err != nil { - next.ServeHTTP(w, r) - return + // Get OAuth scopes for Token. First check if scopes are already in context, then fetch from GitHub if not present. + // This allows Remote Server to pass scope info to avoid redundant GitHub API calls. + activeScopes, ok := ghcontext.GetTokenScopes(ctx) + if !ok { + activeScopes, err = scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) + if err != nil { + next.ServeHTTP(w, r) + return + } } // Store active scopes in context for downstream use From a209b8e3584770414282f233b5b27af63478fc9e Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Fri, 13 Feb 2026 17:04:06 +0100 Subject: [PATCH 4/8] Just do the check if the scopes are empty. --- pkg/http/middleware/scope_challenge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/http/middleware/scope_challenge.go b/pkg/http/middleware/scope_challenge.go index d738cc603..1a86bf93c 100644 --- a/pkg/http/middleware/scope_challenge.go +++ b/pkg/http/middleware/scope_challenge.go @@ -97,7 +97,7 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter // Get OAuth scopes for Token. First check if scopes are already in context, then fetch from GitHub if not present. // This allows Remote Server to pass scope info to avoid redundant GitHub API calls. activeScopes, ok := ghcontext.GetTokenScopes(ctx) - if !ok { + if !ok || (len(activeScopes) == 0 && tokenInfo.Token != "") { activeScopes, err = scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) if err != nil { next.ServeHTTP(w, r) From 683641d5638789e4fd40d34e3ce57a0c356f4c76 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 16 Feb 2026 11:03:16 +0100 Subject: [PATCH 5/8] Return error type for unknown tools in inventory builder and handle it in HTTP handler --- pkg/http/handler.go | 7 +++++++ pkg/inventory/builder.go | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 753c62fa8..e7743ff86 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -2,6 +2,7 @@ package http import ( "context" + "errors" "log/slog" "net/http" @@ -178,6 +179,12 @@ func withInsiders(next http.Handler) http.Handler { func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { inv, err := h.inventoryFactoryFunc(r) if err != nil { + if errors.Is(err, inventory.UnknownToolsErr) { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(err.Error())) + return + } + w.WriteHeader(http.StatusInternalServerError) return } diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index ff2d06d5d..b610d6134 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -2,11 +2,16 @@ package inventory import ( "context" + "errors" "fmt" "sort" "strings" ) +var ( + UnknownToolsErr = errors.New("unknown tools specified in WithTools") +) + // ToolFilter is a function that determines if a tool should be included. // Returns true if the tool should be included, false to exclude it. type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error) @@ -204,7 +209,7 @@ func (b *Builder) Build() (*Inventory, error) { // Error out if there are unrecognized tools if len(unrecognizedTools) > 0 { - return nil, fmt.Errorf("unrecognized tools: %s", strings.Join(unrecognizedTools, ", ")) + return nil, fmt.Errorf("%w: %s", UnknownToolsErr, strings.Join(unrecognizedTools, ", ")) } } From f1407b7ae7c3689deb1cee65adc6f3c27a140ebb Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 16 Feb 2026 12:20:31 +0100 Subject: [PATCH 6/8] Appease the linter --- pkg/http/handler.go | 4 +++- pkg/inventory/builder.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index e7743ff86..7e28939cb 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -181,7 +181,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { if errors.Is(err, inventory.UnknownToolsErr) { w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + if _, writeErr := w.Write([]byte(err.Error())); writeErr != nil { + h.logger.Error("failed to write response", "error", writeErr) + } return } diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index b610d6134..2504e96c0 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -9,6 +9,7 @@ import ( ) var ( + // UnknownToolsErr is returned when tools specified via WithTools() are not recognized. UnknownToolsErr = errors.New("unknown tools specified in WithTools") ) From 2d6e43d83b237b4914842ee97f08b7a54cda7c92 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 16 Feb 2026 12:30:33 +0100 Subject: [PATCH 7/8] Rename error for tools for linter --- pkg/http/handler.go | 2 +- pkg/inventory/builder.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 7e28939cb..3c6c5302e 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -179,7 +179,7 @@ func withInsiders(next http.Handler) http.Handler { func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { inv, err := h.inventoryFactoryFunc(r) if err != nil { - if errors.Is(err, inventory.UnknownToolsErr) { + if errors.Is(err, inventory.ErrUnknownTools) { w.WriteHeader(http.StatusBadRequest) if _, writeErr := w.Write([]byte(err.Error())); writeErr != nil { h.logger.Error("failed to write response", "error", writeErr) diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index 2504e96c0..4d4597c1c 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -9,8 +9,8 @@ import ( ) var ( - // UnknownToolsErr is returned when tools specified via WithTools() are not recognized. - UnknownToolsErr = errors.New("unknown tools specified in WithTools") + // ErrUnknownTools is returned when tools specified via WithTools() are not recognized. + ErrUnknownTools = errors.New("unknown tools specified in WithTools") ) // ToolFilter is a function that determines if a tool should be included. @@ -210,7 +210,7 @@ func (b *Builder) Build() (*Inventory, error) { // Error out if there are unrecognized tools if len(unrecognizedTools) > 0 { - return nil, fmt.Errorf("%w: %s", UnknownToolsErr, strings.Join(unrecognizedTools, ", ")) + return nil, fmt.Errorf("%w: %s", ErrUnknownTools, strings.Join(unrecognizedTools, ", ")) } } From 3ce3c3bd8090c45e95e901e57c1b5ecb6fbf6802 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 16 Feb 2026 12:34:04 +0100 Subject: [PATCH 8/8] Use anonyamous structs as context keys to avoid collisions and ensure type safety. --- pkg/context/token.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pkg/context/token.go b/pkg/context/token.go index 3cb3707da..97091a922 100644 --- a/pkg/context/token.go +++ b/pkg/context/token.go @@ -6,10 +6,7 @@ import ( "github.com/github/github-mcp-server/pkg/utils" ) -// tokenCtxKey is a context key for authentication token information -type tokenCtx string - -var tokenCtxKey tokenCtx = "tokenctx" +type tokenCtxKey struct{} type TokenInfo struct { Token string @@ -18,29 +15,27 @@ type TokenInfo struct { // WithTokenInfo adds TokenInfo to the context func WithTokenInfo(ctx context.Context, tokenInfo *TokenInfo) context.Context { - return context.WithValue(ctx, tokenCtxKey, tokenInfo) + return context.WithValue(ctx, tokenCtxKey{}, tokenInfo) } // GetTokenInfo retrieves the authentication token from the context func GetTokenInfo(ctx context.Context) (*TokenInfo, bool) { - if tokenInfo, ok := ctx.Value(tokenCtxKey).(*TokenInfo); ok { + if tokenInfo, ok := ctx.Value(tokenCtxKey{}).(*TokenInfo); ok { return tokenInfo, true } return nil, false } -type TokenScopesKey tokenCtx - -var tokenScopesKey TokenScopesKey = "tokenscopesctx" +type tokenScopesKey struct{} // WithTokenScopes adds token scopes to the context func WithTokenScopes(ctx context.Context, scopes []string) context.Context { - return context.WithValue(ctx, tokenScopesKey, scopes) + return context.WithValue(ctx, tokenScopesKey{}, scopes) } // GetTokenScopes retrieves token scopes from the context func GetTokenScopes(ctx context.Context) ([]string, bool) { - if scopes, ok := ctx.Value(tokenScopesKey).([]string); ok { + if scopes, ok := ctx.Value(tokenScopesKey{}).([]string); ok { return scopes, true } return nil, false