diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index c0cadbbc63..7014a38e75 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -166,6 +166,11 @@ var ( Short: "Start HTTP server", Long: `Start an HTTP server that listens for MCP requests over HTTP.`, RunE: func(_ *cobra.Command, _ []string) error { + staticToken, err := resolveHTTPStaticToken(viper.GetBool("static-auth"), viper.GetString("personal_access_token")) + if err != nil { + return err + } + // Parse toolsets (same approach as stdio — see comment there) var enabledToolsets []string if viper.IsSet("toolsets") { @@ -199,6 +204,7 @@ var ( httpConfig := ghhttp.ServerConfig{ Version: version, Host: viper.GetString("host"), + StaticToken: staticToken, Port: viper.GetInt("port"), ListenHost: viper.GetString("listen-host"), BaseURL: viper.GetString("base-url"), @@ -268,6 +274,7 @@ func init() { httpCmd.Flags().String("authorization-server", "", "Override the authorization server URL in OAuth resource metadata. Useful when deploying behind an OAuth proxy (e.g. for GHES). Env: GITHUB_AUTHORIZATION_SERVER") httpCmd.Flags().Bool("scope-challenge", false, "Enable OAuth scope challenge responses") httpCmd.Flags().Bool("trust-proxy-headers", false, "Honor X-Forwarded-Host and X-Forwarded-Proto when constructing OAuth resource metadata URLs. Only enable when the server is deployed behind a trusted proxy that sets these headers. Ignored when --base-url is set.") + httpCmd.Flags().Bool("static-auth", false, "Allow requests without an Authorization header to use GITHUB_PERSONAL_ACCESS_TOKEN. For single-tenant deployments behind a trusted access boundary only. Env: GITHUB_STATIC_AUTH") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -297,6 +304,7 @@ func init() { _ = viper.BindPFlag("authorization-server", httpCmd.Flags().Lookup("authorization-server")) _ = viper.BindPFlag("scope-challenge", httpCmd.Flags().Lookup("scope-challenge")) _ = viper.BindPFlag("trust-proxy-headers", httpCmd.Flags().Lookup("trust-proxy-headers")) + _ = viper.BindPFlag("static-auth", httpCmd.Flags().Lookup("static-auth")) // Add subcommands rootCmd.AddCommand(stdioCmd) rootCmd.AddCommand(httpCmd) @@ -309,6 +317,19 @@ func initConfig() { viper.AutomaticEnv() } +func resolveHTTPStaticToken(enabled bool, token string) (string, error) { + if !enabled { + return "", nil + } + if token == "" { + return "", errors.New("HTTP static auth requires GITHUB_PERSONAL_ACCESS_TOKEN") + } + if _, err := utils.ParseToken(token); err != nil { + return "", fmt.Errorf("invalid GITHUB_PERSONAL_ACCESS_TOKEN for HTTP static auth: %w", err) + } + return token, nil +} + func main() { if err := rootCmd.Execute(); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) diff --git a/cmd/github-mcp-server/main_test.go b/cmd/github-mcp-server/main_test.go index a5b2b84967..5f7691ed68 100644 --- a/cmd/github-mcp-server/main_test.go +++ b/cmd/github-mcp-server/main_test.go @@ -57,6 +57,46 @@ func TestAuthorizationServerConfigurationIsHTTPOnly(t *testing.T) { assert.Equal(t, "https://oauth-proxy.example.com", viper.GetString("authorization-server")) } +func TestStaticAuthConfigurationIsHTTPOnly(t *testing.T) { + flag := httpCmd.Flags().Lookup("static-auth") + require.NotNil(t, flag) + assert.Equal(t, "false", flag.DefValue) + assert.Nil(t, stdioCmd.Flags().Lookup("static-auth")) + + t.Setenv("GITHUB_STATIC_AUTH", "true") + initConfig() + assert.True(t, viper.GetBool("static-auth")) +} + +func TestResolveHTTPStaticToken(t *testing.T) { + const validToken = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + t.Run("disabled ignores configured token", func(t *testing.T) { + token, err := resolveHTTPStaticToken(false, validToken) + require.NoError(t, err) + assert.Empty(t, token) + }) + + t.Run("enabled accepts configured token", func(t *testing.T) { + token, err := resolveHTTPStaticToken(true, validToken) + require.NoError(t, err) + assert.Equal(t, validToken, token) + }) + + t.Run("enabled rejects missing token", func(t *testing.T) { + _, err := resolveHTTPStaticToken(true, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "GITHUB_PERSONAL_ACCESS_TOKEN") + }) + + t.Run("enabled rejects invalid token without exposing it", func(t *testing.T) { + const invalidToken = "invalid-secret-value" + _, err := resolveHTTPStaticToken(true, invalidToken) + require.Error(t, err) + assert.NotContains(t, err.Error(), invalidToken) + }) +} + func TestWriteToolDocScopes(t *testing.T) { tool := inventory.ServerTool{ Tool: mcp.Tool{Name: "delete", Annotations: &mcp.ToolAnnotations{Title: "Delete"}}, diff --git a/docs/streamable-http.md b/docs/streamable-http.md index a1e6d9890a..b7b54fbcf2 100644 --- a/docs/streamable-http.md +++ b/docs/streamable-http.md @@ -22,6 +22,38 @@ github-mcp-server http The server will be available at `http://localhost:8082`. +### Single-tenant static authentication + +By default, every HTTP request must provide its own GitHub token in the +`Authorization` header. For a single-tenant deployment where a trusted gateway +cannot add that header, explicitly opt in to using the process credential when +the header is absent: + +```bash +export GITHUB_PERSONAL_ACCESS_TOKEN=ghp_yourtokenhere +# Bind the backend to loopback and put an authenticating gateway in front of it. +github-mcp-server http --static-auth --listen-host 127.0.0.1 --read-only +``` + +The environment equivalent of the flag is `GITHUB_STATIC_AUTH=true`. If static +authentication is enabled without a valid `GITHUB_PERSONAL_ACCESS_TOKEN`, the +server refuses to start. A request that includes an `Authorization` header +always uses that header instead; an empty, malformed, or unsupported header is +rejected rather than replaced with the process credential. + +Browser requests carrying an `Origin` header cannot use the static credential +fallback. They must explicitly request the `Authorization` header during CORS +preflight and send their own token with the actual request. + +> [!WARNING] +> Static authentication does not authenticate callers to the MCP server. Any +> caller that can reach the endpoint without an `Authorization` header receives +> the permissions of the shared GitHub credential. Use this mode only behind an +> authenticating trusted gateway or access boundary. Binding to a loopback +> interface reduces network exposure but does not authenticate local callers and +> is not sufficient by itself. Grant the token the least privileges possible, +> and enable `--read-only` unless write tools are required. + ### With Scope Challenge Enable scope validation to enforce GitHub permission checks: diff --git a/pkg/http/handler.go b/pkg/http/handler.go index e4a9d198ec..2ee02d73ca 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -134,7 +134,7 @@ func (h *Handler) RegisterMiddleware(r chi.Router) { r.Use( // Must run first: bounds the body before anything downstream reads it. middleware.WithMaxBodySize(h.maxRequestBodyBytes()), - middleware.ExtractUserToken(h.oauthCfg), + middleware.ExtractUserTokenWithFallback(h.oauthCfg, h.config.StaticToken), middleware.WithRequestConfig, middleware.WithMCPParse(), middleware.WithPATScopes(h.logger, h.scopeFetcher), diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index f051084785..5e968bad22 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -60,6 +60,19 @@ func (f allScopesFetcher) FetchTokenScopes(_ context.Context, _ string) ([]strin var _ scopes.FetcherInterface = allScopesFetcher{} +type recordingScopesFetcher struct { + token string + calls int +} + +func (f *recordingScopesFetcher) FetchTokenScopes(_ context.Context, token string) ([]string, error) { + f.token = token + f.calls++ + return []string{string(scopes.Repo)}, nil +} + +var _ scopes.FetcherInterface = (*recordingScopesFetcher)(nil) + func mockToolWithFeatureFlag(name, toolsetID string, readOnly bool, enableFlag, disableFlag string) inventory.ServerTool { tool := mockTool(name, toolsetID, readOnly) tool.FeatureFlagEnable = enableFlag @@ -1011,6 +1024,53 @@ func TestCrossOriginProtection(t *testing.T) { } } +func TestStaticTokenFallbackFlowsThroughPATScopeMiddleware(t *testing.T) { + const staticToken = "ghp_staticxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + jsonRPCBody := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"test","version":"0.1"}}}` + + apiHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + var capturedTokenInfo *ghcontext.TokenInfo + var capturedScopes []string + fetcher := &recordingScopesFetcher{} + handler := NewHTTPMcpHandler( + context.Background(), + &ServerConfig{Version: "test", StaticToken: staticToken}, + nil, + translations.NullTranslationHelper, + slog.Default(), + apiHost, + WithInventoryFactory(func(_ *http.Request) (*inventory.Inventory, error) { + return inventory.NewBuilder().Build() + }), + WithGitHubMCPServerFactory(func(r *http.Request, _ github.ToolDependencies, _ *inventory.Inventory, _ *github.MCPServerConfig) (*mcp.Server, error) { + capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + capturedScopes, _ = ghcontext.GetTokenScopes(r.Context()) + return mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil), nil + }), + WithScopeFetcher(fetcher), + ) + + router := chi.NewRouter() + handler.RegisterMiddleware(router) + handler.RegisterRoutes(router) + + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(jsonRPCBody)) + req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON) + req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", ")) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code, "unexpected status code; body: %s", rr.Body.String()) + require.NotNil(t, capturedTokenInfo) + assert.Equal(t, staticToken, capturedTokenInfo.Token) + assert.Equal(t, utils.TokenTypePersonalAccessToken, capturedTokenInfo.TokenType) + assert.Equal(t, staticToken, fetcher.token) + assert.Equal(t, 1, fetcher.calls) + assert.Equal(t, []string{string(scopes.Repo)}, capturedScopes) +} + func TestHTTPToolMinimumProtocolVersion(t *testing.T) { apiHost, err := utils.NewAPIHost("https://api.github.com") require.NoError(t, err) diff --git a/pkg/http/middleware/cors.go b/pkg/http/middleware/cors.go index 409d134127..a97311542b 100644 --- a/pkg/http/middleware/cors.go +++ b/pkg/http/middleware/cors.go @@ -10,7 +10,8 @@ import ( // SetCorsHeaders is middleware that sets CORS headers to allow browser-based // MCP clients to connect from any origin. This is safe because the server // authenticates via bearer tokens (not cookies), so cross-origin requests -// cannot exploit ambient credentials. +// cannot exploit ambient credentials. Static auth installs its browser guard +// before this middleware. func SetCorsHeaders(h http.Handler) http.Handler { allowHeaders := strings.Join([]string{ "Content-Type", diff --git a/pkg/http/middleware/static_auth.go b/pkg/http/middleware/static_auth.go new file mode 100644 index 0000000000..e95c41ce2f --- /dev/null +++ b/pkg/http/middleware/static_auth.go @@ -0,0 +1,61 @@ +package middleware + +import ( + "net/http" + "strings" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/http/oauth" +) + +// StaticAuthBrowserGuard prevents browser requests from implicitly consuming a +// shared static credential. It must run before CORS response headers are set. +func StaticAuthBrowserGuard(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if isOAuthMetadataPath(r.URL.Path) { + next.ServeHTTP(w, r) + return + } + if r.Header.Get("Origin") == "" { + next.ServeHTTP(w, r) + return + } + if _, ok := ghcontext.GetTokenInfo(r.Context()); ok { + next.ServeHTTP(w, r) + return + } + + if r.Method == http.MethodOptions { + if headerListContains(r.Header.Get("Access-Control-Request-Headers"), headers.AuthorizationHeader) { + next.ServeHTTP(w, r) + return + } + rejectStaticAuthBrowserRequest(w) + return + } + + if hasAuthorizationHeader(r.Header) { + next.ServeHTTP(w, r) + return + } + rejectStaticAuthBrowserRequest(w) + }) +} + +func isOAuthMetadataPath(path string) bool { + return path == oauth.OAuthProtectedResourcePrefix || strings.HasPrefix(path, oauth.OAuthProtectedResourcePrefix+"/") +} + +func headerListContains(value, target string) bool { + for header := range strings.SplitSeq(value, ",") { + if strings.EqualFold(strings.TrimSpace(header), target) { + return true + } + } + return false +} + +func rejectStaticAuthBrowserRequest(w http.ResponseWriter) { + http.Error(w, "Forbidden", http.StatusForbidden) +} diff --git a/pkg/http/middleware/static_auth_test.go b/pkg/http/middleware/static_auth_test.go new file mode 100644 index 0000000000..6fb593cb94 --- /dev/null +++ b/pkg/http/middleware/static_auth_test.go @@ -0,0 +1,112 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + "github.com/github/github-mcp-server/pkg/http/middleware" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/stretchr/testify/assert" +) + +func TestStaticAuthBrowserGuard(t *testing.T) { + tests := []struct { + name string + method string + origin string + authorization string + preflightHeaders string + withUpstreamToken bool + expectedStatus int + expectInnerCalled bool + expectWildcardCORS bool + }{ + { + name: "non-browser request is allowed", + method: http.MethodPost, + expectedStatus: http.StatusNoContent, + expectInnerCalled: true, + expectWildcardCORS: true, + }, + { + name: "headerless browser request is rejected before CORS", + method: http.MethodPost, + origin: "https://example.com", + expectedStatus: http.StatusForbidden, + }, + { + name: "browser request with authorization is allowed", + method: http.MethodPost, + origin: "https://example.com", + authorization: "Bearer github_pat_requesttoken", + expectedStatus: http.StatusNoContent, + expectInnerCalled: true, + expectWildcardCORS: true, + }, + { + name: "upstream token context is allowed", + method: http.MethodPost, + origin: "https://example.com", + withUpstreamToken: true, + expectedStatus: http.StatusNoContent, + expectInnerCalled: true, + expectWildcardCORS: true, + }, + { + name: "headerless static auth preflight is rejected", + method: http.MethodOptions, + origin: "https://example.com", + preflightHeaders: "content-type", + expectedStatus: http.StatusForbidden, + }, + { + name: "preflight requesting authorization is allowed", + method: http.MethodOptions, + origin: "https://example.com", + preflightHeaders: "content-type, AUTHORIZATION", + expectedStatus: http.StatusOK, + expectWildcardCORS: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + innerCalled := false + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + innerCalled = true + w.WriteHeader(http.StatusNoContent) + }) + handler := middleware.StaticAuthBrowserGuard(middleware.SetCorsHeaders(inner)) + + req := httptest.NewRequest(tt.method, "/", nil) + if tt.origin != "" { + req.Header.Set("Origin", tt.origin) + } + if tt.authorization != "" { + req.Header.Set("Authorization", tt.authorization) + } + if tt.preflightHeaders != "" { + req.Header.Set("Access-Control-Request-Headers", tt.preflightHeaders) + } + if tt.withUpstreamToken { + req = req.WithContext(ghcontext.WithTokenInfo(req.Context(), &ghcontext.TokenInfo{ + Token: "gho_upstreamtoken", + TokenType: utils.TokenTypeOAuthAccessToken, + })) + } + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + assert.Equal(t, tt.expectedStatus, rr.Code) + assert.Equal(t, tt.expectInnerCalled, innerCalled) + if tt.expectWildcardCORS { + assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) + } else { + assert.Empty(t, rr.Header().Get("Access-Control-Allow-Origin")) + } + }) + } +} diff --git a/pkg/http/middleware/token.go b/pkg/http/middleware/token.go index 012bbabef2..ae20ef2f9f 100644 --- a/pkg/http/middleware/token.go +++ b/pkg/http/middleware/token.go @@ -4,13 +4,22 @@ import ( "errors" "fmt" "net/http" + "strings" ghcontext "github.com/github/github-mcp-server/pkg/context" + "github.com/github/github-mcp-server/pkg/http/headers" "github.com/github/github-mcp-server/pkg/http/oauth" "github.com/github/github-mcp-server/pkg/utils" ) func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handler { + return ExtractUserTokenWithFallback(oauthCfg, "") +} + +// ExtractUserTokenWithFallback extracts a per-request token, falling back to a +// configured token only when the Authorization header is absent. An explicitly +// provided header, including an empty or malformed one, is never replaced. +func ExtractUserTokenWithFallback(oauthCfg *oauth.Config, fallbackToken string) 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() @@ -25,8 +34,22 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl tokenType, token, err := utils.ParseAuthorizationHeader(r) if err != nil { - // For missing Authorization header, return 401 with WWW-Authenticate header per MCP spec if errors.Is(err, utils.ErrMissingAuthorizationHeader) { + if !hasAuthorizationHeader(r.Header) && fallbackToken != "" { + tokenType, tokenErr := utils.ParseToken(fallbackToken) + if tokenErr != nil { + http.Error(w, "configured static token is invalid", http.StatusInternalServerError) + return + } + ctx = ghcontext.WithTokenInfo(ctx, &ghcontext.TokenInfo{ + Token: fallbackToken, + TokenType: tokenType, + }) + next.ServeHTTP(w, r.WithContext(ctx)) + return + } + + // For missing Authorization header, return 401 with WWW-Authenticate header per MCP spec sendAuthChallenge(w, r, oauthCfg) return } @@ -46,6 +69,15 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl } } +func hasAuthorizationHeader(header http.Header) bool { + for name := range header { + if strings.EqualFold(name, headers.AuthorizationHeader) { + return true + } + } + return false +} + // sendAuthChallenge sends a 401 Unauthorized response with WWW-Authenticate header // containing the OAuth protected resource metadata URL as per RFC 6750 and MCP spec. func sendAuthChallenge(w http.ResponseWriter, r *http.Request, oauthCfg *oauth.Config) { diff --git a/pkg/http/middleware/token_test.go b/pkg/http/middleware/token_test.go index fa8f0ee98e..5558a37330 100644 --- a/pkg/http/middleware/token_test.go +++ b/pkg/http/middleware/token_test.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" "net/http/httptest" + "strings" "testing" ghcontext "github.com/github/github-mcp-server/pkg/context" @@ -232,6 +233,118 @@ func TestExtractUserToken_NilOAuthConfig(t *testing.T) { assert.Equal(t, utils.TokenTypePersonalAccessToken, capturedTokenInfo.TokenType) } +func TestExtractUserTokenWithFallback(t *testing.T) { + const staticToken = "ghp_staticxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + const upstreamToken = "gho_upstreamxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + headerToken := strings.Join([]string{"github", "pat", "headerxxxxxxxxxxxxxxxxxxxxxxxx"}, "_") + + tests := []struct { + name string + fallbackToken string + authHeader string + setAuthHeader bool + upstreamTokenInfo *ghcontext.TokenInfo + expectedStatusCode int + expectedToken string + expectedTokenType utils.TokenType + expectTokenInfo bool + expectChallenge bool + }{ + { + name: "missing header uses static token", + fallbackToken: staticToken, + expectedStatusCode: http.StatusOK, + expectedToken: staticToken, + expectedTokenType: utils.TokenTypePersonalAccessToken, + expectTokenInfo: true, + }, + { + name: "missing header without static token preserves challenge", + expectedStatusCode: http.StatusUnauthorized, + expectChallenge: true, + }, + { + name: "request header takes precedence", + fallbackToken: staticToken, + authHeader: "Bearer " + headerToken, + setAuthHeader: true, + expectedStatusCode: http.StatusOK, + expectedToken: headerToken, + expectedTokenType: utils.TokenTypeFineGrainedPersonalAccessToken, + expectTokenInfo: true, + }, + { + name: "blank request header does not fall back", + fallbackToken: staticToken, + setAuthHeader: true, + expectedStatusCode: http.StatusUnauthorized, + expectChallenge: true, + }, + { + name: "malformed request header does not fall back", + fallbackToken: staticToken, + authHeader: "Bearer invalid-token", + setAuthHeader: true, + expectedStatusCode: http.StatusBadRequest, + }, + { + name: "unsupported request header does not fall back", + fallbackToken: staticToken, + authHeader: "GitHub-Bearer encrypted-token", + setAuthHeader: true, + expectedStatusCode: http.StatusBadRequest, + }, + { + name: "upstream token context takes precedence", + fallbackToken: staticToken, + authHeader: "Bearer " + headerToken, + setAuthHeader: true, + upstreamTokenInfo: &ghcontext.TokenInfo{ + Token: upstreamToken, + TokenType: utils.TokenTypeOAuthAccessToken, + }, + expectedStatusCode: http.StatusOK, + expectedToken: upstreamToken, + expectedTokenType: utils.TokenTypeOAuthAccessToken, + expectTokenInfo: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var capturedTokenInfo *ghcontext.TokenInfo + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + w.WriteHeader(http.StatusOK) + }) + + handler := ExtractUserTokenWithFallback(nil, tt.fallbackToken)(nextHandler) + req := httptest.NewRequest(http.MethodGet, "/test", nil) + if tt.setAuthHeader { + req.Header[headers.AuthorizationHeader] = []string{tt.authHeader} + } + if tt.upstreamTokenInfo != nil { + req = req.WithContext(ghcontext.WithTokenInfo(req.Context(), tt.upstreamTokenInfo)) + } + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + assert.Equal(t, tt.expectedStatusCode, rr.Code) + if tt.expectChallenge { + assert.NotEmpty(t, rr.Header().Get("WWW-Authenticate")) + } + if tt.expectTokenInfo { + require.NotNil(t, capturedTokenInfo) + assert.Equal(t, tt.expectedToken, capturedTokenInfo.Token) + assert.Equal(t, tt.expectedTokenType, capturedTokenInfo.TokenType) + } else { + assert.Nil(t, capturedTokenInfo) + } + }) + } +} + func TestExtractUserToken_MissingAuthHeader_WWWAuthenticateFormat(t *testing.T) { oauthCfg := &oauth.Config{ BaseURL: "https://api.example.com", diff --git a/pkg/http/server.go b/pkg/http/server.go index cc2d23d3ac..b4ba5ec6a1 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -38,6 +38,10 @@ type ServerConfig struct { // GitHub Host to target for API requests (e.g. github.com or github.enterprise.com) Host string + // StaticToken is a shared GitHub credential used only when a request has no + // Authorization header. A per-request token always takes precedence. + StaticToken string + // Port to listen on (default: 8082). Port int @@ -123,6 +127,10 @@ type ServerConfig struct { } func RunHTTPServer(cfg ServerConfig) error { + if err := validateStaticToken(cfg.StaticToken); err != nil { + return err + } + // Create app context ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() @@ -210,6 +218,11 @@ func RunHTTPServer(cfg ServerConfig) error { return fmt.Errorf("failed to create OAuth handler: %w", err) } + var mcpMiddleware []func(http.Handler) http.Handler + if cfg.StaticToken != "" { + mcpMiddleware = append(mcpMiddleware, middleware.StaticAuthBrowserGuard) + } + r := newHTTPRouter( func(r chi.Router) { // Register Middleware First, needs to be before route registration @@ -218,6 +231,7 @@ func RunHTTPServer(cfg ServerConfig) error { handler.RegisterRoutes(r) }, oauthHandler.RegisterRoutes, + mcpMiddleware..., ) logger.Info("MCP endpoints registered", "baseURL", cfg.BaseURL) logger.Info("OAuth protected resource endpoints registered", "baseURL", cfg.BaseURL) @@ -253,8 +267,19 @@ func RunHTTPServer(cfg ServerConfig) error { return nil } -func newHTTPRouter(registerMCPRoutes, registerOAuthRoutes func(chi.Router)) chi.Router { +func validateStaticToken(token string) error { + if token == "" { + return nil + } + if _, err := utils.ParseToken(token); err != nil { + return fmt.Errorf("invalid static token configuration: %w", err) + } + return nil +} + +func newHTTPRouter(registerMCPRoutes, registerOAuthRoutes func(chi.Router), mcpMiddleware ...func(http.Handler) http.Handler) chi.Router { r := chi.NewRouter() + r.Use(mcpMiddleware...) r.Use(middleware.SetCorsHeaders) r.Group(registerMCPRoutes) r.Group(registerOAuthRoutes) diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index a8c4e1a90b..de3873d676 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -51,6 +51,36 @@ func TestRunHTTPServerRejectsInvalidStaticTools(t *testing.T) { } } +func TestRunHTTPServerRejectsInvalidStaticToken(t *testing.T) { + const invalidToken = "invalid-static-token-secret" + + err := RunHTTPServer(ServerConfig{StaticToken: invalidToken}) + + require.Error(t, err) + assert.ErrorContains(t, err, "invalid static token configuration") + assert.NotContains(t, err.Error(), invalidToken) +} + +func TestValidateStaticToken(t *testing.T) { + t.Run("empty token keeps fallback disabled", func(t *testing.T) { + require.NoError(t, validateStaticToken("")) + }) + + t.Run("valid token is accepted", func(t *testing.T) { + require.NoError(t, validateStaticToken("ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")) + }) + + t.Run("invalid token is rejected without exposing it", func(t *testing.T) { + const invalidToken = "invalid-static-token-secret" + + err := validateStaticToken(invalidToken) + + require.Error(t, err) + assert.ErrorContains(t, err, "invalid static token configuration") + assert.NotContains(t, err.Error(), invalidToken) + }) +} + func TestNewOAuthConfig(t *testing.T) { tests := []struct { name string @@ -190,6 +220,121 @@ func TestHTTPRouterCORSContract(t *testing.T) { } } +func TestHTTPRouterStaticAuthBrowserGuard(t *testing.T) { + var mcpCalled bool + var metadataCalled bool + router := newHTTPRouter( + func(r chi.Router) { + r.Post("/", func(w http.ResponseWriter, _ *http.Request) { + mcpCalled = true + w.WriteHeader(http.StatusNoContent) + }) + }, + func(r chi.Router) { + r.Get(oauth.OAuthProtectedResourcePrefix, func(w http.ResponseWriter, _ *http.Request) { + metadataCalled = true + w.WriteHeader(http.StatusNoContent) + }) + }, + middleware.StaticAuthBrowserGuard, + ) + + tests := []struct { + name string + method string + path string + origin string + authorization string + preflightHeaders string + expectedStatus int + expectMCP bool + expectMetadata bool + expectWildcardCORS bool + }{ + { + name: "headerless browser request cannot reach MCP handler", + method: http.MethodPost, + path: "/", + origin: "https://example.com", + expectedStatus: http.StatusForbidden, + }, + { + name: "non-browser request can reach MCP handler", + method: http.MethodPost, + path: "/", + expectedStatus: http.StatusNoContent, + expectMCP: true, + expectWildcardCORS: true, + }, + { + name: "browser request with authorization can reach MCP handler", + method: http.MethodPost, + path: "/", + origin: "https://example.com", + authorization: "Bearer github_pat_requesttoken", + expectedStatus: http.StatusNoContent, + expectMCP: true, + expectWildcardCORS: true, + }, + { + name: "headerless preflight is rejected", + method: http.MethodOptions, + path: "/", + origin: "https://example.com", + preflightHeaders: "content-type", + expectedStatus: http.StatusForbidden, + }, + { + name: "preflight requesting authorization is allowed", + method: http.MethodOptions, + path: "/", + origin: "https://example.com", + preflightHeaders: "content-type, authorization", + expectedStatus: http.StatusOK, + expectWildcardCORS: true, + }, + { + name: "OAuth metadata keeps wildcard CORS", + method: http.MethodGet, + path: oauth.OAuthProtectedResourcePrefix, + origin: "https://example.com", + expectedStatus: http.StatusNoContent, + expectMetadata: true, + expectWildcardCORS: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mcpCalled = false + metadataCalled = false + req := httptest.NewRequest(tt.method, tt.path, nil) + if tt.origin != "" { + req.Header.Set("Origin", tt.origin) + } + if tt.authorization != "" { + req.Header.Set("Authorization", tt.authorization) + } + if tt.preflightHeaders != "" { + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", tt.preflightHeaders) + } + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, tt.expectedStatus, rr.Code) + assert.Equal(t, tt.expectMCP, mcpCalled) + assert.Equal(t, tt.expectMetadata, metadataCalled) + if tt.expectWildcardCORS { + assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) + } else { + assert.Empty(t, rr.Header().Get("Access-Control-Allow-Origin")) + } + }) + } +} + func TestOAuthChallengeMetadataRouteContracts(t *testing.T) { const baseURL = "https://mcp.example.com" oauthCfg := &oauth.Config{ diff --git a/pkg/utils/token.go b/pkg/utils/token.go index 8933fb0bda..d05ca8a49e 100644 --- a/pkg/utils/token.go +++ b/pkg/utils/token.go @@ -60,16 +60,24 @@ func ParseAuthorizationHeader(req *http.Request) (tokenType TokenType, token str } } + tokenType, err := ParseToken(token) + if err != nil { + return 0, "", err + } + return tokenType, token, nil +} + +// ParseToken identifies a GitHub token by its prefix or legacy format. +func ParseToken(token string) (TokenType, error) { for prefix, tokenType := range supportedGitHubPrefixes { if strings.HasPrefix(token, prefix) { - return tokenType, token, nil + return tokenType, nil } } - matchesOldTokenPattern := oldPatternRegexp.MatchString(token) - if matchesOldTokenPattern { - return TokenTypePersonalAccessToken, token, nil + if oldPatternRegexp.MatchString(token) { + return TokenTypePersonalAccessToken, nil } - return 0, "", ErrBadAuthorizationHeader + return TokenTypeUnknown, ErrBadAuthorizationHeader }