From a1602789ce8966beb47025e83b6f5482067ec9fd Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 27 Aug 2026 11:54:23 +0200 Subject: [PATCH 1/2] fix(http): allow projected MCP headers in preflights Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- go.mod | 2 +- pkg/http/headers/headers.go | 8 ++ pkg/http/middleware/cors.go | 78 +++++++++++++++---- pkg/http/middleware/cors_test.go | 127 +++++++++++++++++++++---------- pkg/http/server_test.go | 41 +++++----- 5 files changed, 181 insertions(+), 75 deletions(-) diff --git a/go.mod b/go.mod index 45e0dcebdc..f0237a1625 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.12.1 github.com/yosida95/uritemplate/v3 v3.0.2 + golang.org/x/net v0.55.0 golang.org/x/oauth2 v0.36.0 ) @@ -38,7 +39,6 @@ require ( github.com/stretchr/objx v0.5.3 // indirect github.com/subosito/gotenv v1.6.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect - golang.org/x/net v0.55.0 // indirect golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.45.0 // indirect golang.org/x/text v0.37.0 // indirect diff --git a/pkg/http/headers/headers.go b/pkg/http/headers/headers.go index 280875ef5b..94981081a7 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -41,6 +41,14 @@ const ( // MCPMethodHeader mirrors the JSON-RPC method for request routing. MCPMethodHeader = "Mcp-Method" + // MCPNameHeader identifies the requested MCP primitive. + MCPNameHeader = "Mcp-Name" + // MCPParamHeaderPrefix prefixes request headers projected from MCP parameters. + MCPParamHeaderPrefix = "Mcp-Param-" + // MCPParamOwnerHeader carries the projected owner parameter. + MCPParamOwnerHeader = MCPParamHeaderPrefix + "owner" + // MCPParamRepoHeader carries the projected repo parameter. + MCPParamRepoHeader = MCPParamHeaderPrefix + "repo" // MCPReadOnlyHeader indicates whether the MCP is in read-only mode. MCPReadOnlyHeader = "X-MCP-Readonly" // MCPToolsetsHeader is a comma-separated list of MCP toolsets that the request is for. diff --git a/pkg/http/middleware/cors.go b/pkg/http/middleware/cors.go index 409d134127..e1b9aff230 100644 --- a/pkg/http/middleware/cors.go +++ b/pkg/http/middleware/cors.go @@ -2,42 +2,90 @@ package middleware import ( "net/http" + "sort" "strings" "github.com/github/github-mcp-server/pkg/http/headers" + "golang.org/x/net/http/httpguts" ) +var corsAllowedRequestHeaders = []string{ + headers.ContentTypeHeader, + "Mcp-Session-Id", + "Mcp-Protocol-Version", + headers.MCPMethodHeader, + headers.MCPNameHeader, + "Last-Event-ID", + headers.AuthorizationHeader, + headers.MCPReadOnlyHeader, + headers.MCPToolsetsHeader, + headers.MCPToolsHeader, + headers.MCPExcludeToolsHeader, + headers.MCPFeaturesHeader, + headers.MCPLockdownHeader, + headers.MCPInsidersHeader, + headers.MCPParamOwnerHeader, + headers.MCPParamRepoHeader, +} + // 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. func SetCorsHeaders(h http.Handler) http.Handler { - allowHeaders := strings.Join([]string{ - "Content-Type", - "Mcp-Session-Id", - "Mcp-Protocol-Version", - "Last-Event-ID", - headers.AuthorizationHeader, - headers.MCPReadOnlyHeader, - headers.MCPToolsetsHeader, - headers.MCPToolsHeader, - headers.MCPExcludeToolsHeader, - headers.MCPFeaturesHeader, - headers.MCPLockdownHeader, - headers.MCPInsidersHeader, - }, ", ") + fixedAllowHeaders := strings.Join(corsAllowedRequestHeaders, ", ") return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Access-Control-Allow-Origin", "*") w.Header().Set("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS") w.Header().Set("Access-Control-Max-Age", "86400") w.Header().Add("Access-Control-Expose-Headers", "Mcp-Session-Id, WWW-Authenticate") - w.Header().Set("Access-Control-Allow-Headers", allowHeaders) + w.Header().Set("Access-Control-Allow-Headers", fixedAllowHeaders) if r.Method == http.MethodOptions { + w.Header().Set("Access-Control-Allow-Headers", corsPreflightAllowedRequestHeaders(r.Header)) w.WriteHeader(http.StatusOK) return } h.ServeHTTP(w, r) }) } + +// corsPreflightAllowedRequestHeaders reflects validated projected arguments because CORS has no prefix wildcard. +func corsPreflightAllowedRequestHeaders(requestHeaders http.Header) string { + allowed := make([]string, 0, len(corsAllowedRequestHeaders)) + allowed = append(allowed, corsAllowedRequestHeaders...) + + seen := make(map[string]struct{}, len(allowed)) + for _, header := range allowed { + seen[strings.ToLower(header)] = struct{}{} + } + + prefix := strings.ToLower(headers.MCPParamHeaderPrefix) + var projected []string + for _, value := range requestHeaders.Values("Access-Control-Request-Headers") { + for header := range strings.SplitSeq(value, ",") { + header = strings.TrimSpace(header) + if !httpguts.ValidHeaderFieldName(header) { + continue + } + + key := strings.ToLower(header) + if !strings.HasPrefix(key, prefix) || len(key) == len(prefix) { + continue + } + if _, ok := seen[key]; ok { + continue + } + + seen[key] = struct{}{} + projected = append(projected, key) + } + } + + sort.Strings(projected) + for _, header := range projected { + allowed = append(allowed, http.CanonicalHeaderKey(header)) + } + return strings.Join(allowed, ", ") +} diff --git a/pkg/http/middleware/cors_test.go b/pkg/http/middleware/cors_test.go index 67305e0ac8..a6aa9a9766 100644 --- a/pkg/http/middleware/cors_test.go +++ b/pkg/http/middleware/cors_test.go @@ -8,54 +8,101 @@ import ( "github.com/github/github-mcp-server/pkg/http/middleware" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestSetCorsHeaders(t *testing.T) { +const fixedAllowedRequestHeaders = "Content-Type, Mcp-Session-Id, Mcp-Protocol-Version, Mcp-Method, Mcp-Name, Last-Event-ID, Authorization, X-MCP-Readonly, X-MCP-Toolsets, X-MCP-Tools, X-MCP-Exclude-Tools, X-MCP-Features, X-MCP-Lockdown, X-MCP-Insiders, Mcp-Param-owner, Mcp-Param-repo" + +func TestSetCorsHeadersPreflight(t *testing.T) { + tests := []struct { + name string + requestedHeadersValues []string + expectedAllowedHeaders string + }{ + { + name: "current MCP request headers", + requestedHeadersValues: []string{ + "authorization, content-type, mcp-protocol-version, mcp-method, mcp-name, mcp-param-owner, mcp-param-repo", + }, + expectedAllowedHeaders: fixedAllowedRequestHeaders, + }, + { + name: "future projected parameter", + requestedHeadersValues: []string{"Mcp-Param-region"}, + expectedAllowedHeaders: fixedAllowedRequestHeaders + ", Mcp-Param-Region", + }, + { + name: "mixed case duplicates across values", + requestedHeadersValues: []string{ + "mCp-PaRaM-ReGiOn, MCP-PARAM-ZONE", + "MCP-PARAM-REGION, mcp-param-zone", + }, + expectedAllowedHeaders: fixedAllowedRequestHeaders + ", Mcp-Param-Region, Mcp-Param-Zone", + }, + { + name: "invalid unrelated bare and lookalike names", + requestedHeadersValues: []string{ + "Mcp-Param-, X-Evil, XMcp-Param-region, Mcp_Param-region, Mcp-Param-\x00region, Mcp-Param-bad name", + }, + expectedAllowedHeaders: fixedAllowedRequestHeaders, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + innerCalled := false + handler := middleware.SetCorsHeaders(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + innerCalled = true + })) + req := httptest.NewRequest(http.MethodOptions, "/", nil) + req.Header.Set("Origin", "https://confer.to") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + for _, value := range tt.requestedHeadersValues { + req.Header.Add("Access-Control-Request-Headers", value) + } + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.False(t, innerCalled) + assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) + assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials")) + assert.Equal(t, "GET, POST, DELETE, OPTIONS", rr.Header().Get("Access-Control-Allow-Methods")) + assert.Equal(t, "86400", rr.Header().Get("Access-Control-Max-Age")) + assert.Equal(t, tt.expectedAllowedHeaders, rr.Header().Get("Access-Control-Allow-Headers")) + assert.Equal(t, "Mcp-Session-Id, WWW-Authenticate", rr.Header().Get("Access-Control-Expose-Headers")) + assert.NotContains(t, rr.Header().Get("Access-Control-Expose-Headers"), "Mcp-Param-") + }) + } +} + +func TestSetCorsHeadersPostPreservesBehavior(t *testing.T) { innerCalled := false inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { innerCalled = true w.Header().Add("Access-Control-Expose-Headers", "X-Existing-Response") - w.WriteHeader(http.StatusOK) + w.WriteHeader(http.StatusCreated) + _, err := w.Write([]byte("created")) + require.NoError(t, err) }) handler := middleware.SetCorsHeaders(inner) + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.Header.Set("Origin", "https://confer.to") + req.Header.Set("Access-Control-Request-Headers", "Mcp-Param-region") + rr := httptest.NewRecorder() - t.Run("OPTIONS preflight returns 200 with CORS headers", func(t *testing.T) { - innerCalled = false - req := httptest.NewRequest(http.MethodOptions, "/", nil) - req.Header.Set("Origin", "https://confer.to") - req.Header.Set("Access-Control-Request-Method", http.MethodPost) - req.Header.Set("Access-Control-Request-Headers", "content-type") - rr := httptest.NewRecorder() - handler.ServeHTTP(rr, req) - - assert.Equal(t, http.StatusOK, rr.Code) - assert.False(t, innerCalled) - assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) - assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials")) - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Methods"), "POST") - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "Authorization") - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "Content-Type") - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "Mcp-Session-Id") - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "X-MCP-Lockdown") - assert.Contains(t, rr.Header().Get("Access-Control-Allow-Headers"), "X-MCP-Insiders") - assert.Contains(t, rr.Header().Get("Access-Control-Expose-Headers"), "Mcp-Session-Id") - assert.Contains(t, rr.Header().Get("Access-Control-Expose-Headers"), "WWW-Authenticate") - }) + handler.ServeHTTP(rr, req) - t.Run("POST request includes CORS headers without replacing existing exposed headers", func(t *testing.T) { - innerCalled = false - req := httptest.NewRequest(http.MethodPost, "/", nil) - req.Header.Set("Origin", "https://confer.to") - rr := httptest.NewRecorder() - handler.ServeHTTP(rr, req) - - assert.Equal(t, http.StatusOK, rr.Code) - assert.True(t, innerCalled) - assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) - assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials")) - exposedHeaders := strings.Join(rr.Header().Values("Access-Control-Expose-Headers"), ", ") - assert.Contains(t, exposedHeaders, "Mcp-Session-Id") - assert.Contains(t, exposedHeaders, "WWW-Authenticate") - assert.Contains(t, exposedHeaders, "X-Existing-Response") - }) + assert.Equal(t, http.StatusCreated, rr.Code) + assert.True(t, innerCalled) + assert.Equal(t, "created", rr.Body.String()) + assert.Equal(t, "*", rr.Header().Get("Access-Control-Allow-Origin")) + assert.Empty(t, rr.Header().Get("Access-Control-Allow-Credentials")) + assert.Equal(t, fixedAllowedRequestHeaders, rr.Header().Get("Access-Control-Allow-Headers")) + exposedHeaders := strings.Join(rr.Header().Values("Access-Control-Expose-Headers"), ", ") + assert.Contains(t, exposedHeaders, "Mcp-Session-Id") + assert.Contains(t, exposedHeaders, "WWW-Authenticate") + assert.Contains(t, exposedHeaders, "X-Existing-Response") + assert.NotContains(t, exposedHeaders, "Mcp-Param-") } diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index a8c4e1a90b..58e1caa003 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -106,26 +106,29 @@ func TestHTTPRouterCORSContract(t *testing.T) { ) tests := []struct { - name string - method string - path string - expectedStatus int - expectChallenge bool - expectAllowHeader bool + name string + method string + path string + requestHeaders string + expectedStatus int + expectChallenge bool + expectedAllow []string }{ { - name: "MCP preflight", - method: http.MethodOptions, - path: "/", - expectedStatus: http.StatusOK, - expectAllowHeader: true, + name: "MCP preflight", + method: http.MethodOptions, + path: "/", + requestHeaders: "content-type, mcp-method, mcp-name, mcp-param-owner, mcp-param-region", + expectedStatus: http.StatusOK, + expectedAllow: []string{"Content-Type", "Mcp-Method", "Mcp-Name", "Mcp-Param-owner", "Mcp-Param-Region"}, }, { - name: "metadata preflight", - method: http.MethodOptions, - path: "/metadata", - expectedStatus: http.StatusOK, - expectAllowHeader: true, + name: "metadata preflight", + method: http.MethodOptions, + path: "/metadata", + requestHeaders: "content-type", + expectedStatus: http.StatusOK, + expectedAllow: []string{"Content-Type"}, }, { name: "authentication challenge", @@ -166,7 +169,7 @@ func TestHTTPRouterCORSContract(t *testing.T) { req.Header.Set("Origin", "https://confer.to") if tt.method == http.MethodOptions { req.Header.Set("Access-Control-Request-Method", http.MethodPost) - req.Header.Set("Access-Control-Request-Headers", "content-type") + req.Header.Set("Access-Control-Request-Headers", tt.requestHeaders) } rec := httptest.NewRecorder() @@ -177,8 +180,8 @@ func TestHTTPRouterCORSContract(t *testing.T) { assert.Empty(t, rec.Header().Get("Access-Control-Allow-Credentials")) assert.Contains(t, rec.Header().Get("Access-Control-Expose-Headers"), "Mcp-Session-Id") assert.Contains(t, rec.Header().Get("Access-Control-Expose-Headers"), "WWW-Authenticate") - if tt.expectAllowHeader { - assert.Contains(t, rec.Header().Get("Access-Control-Allow-Headers"), "Content-Type") + for _, header := range tt.expectedAllow { + assert.Contains(t, rec.Header().Get("Access-Control-Allow-Headers"), header) } if tt.expectChallenge { assert.Equal(t, From c6be6e022775e4c3d21f1bc9c6667a07354cab5e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 27 Aug 2026 09:56:04 +0000 Subject: [PATCH 2/2] chore: regenerate license files Auto-generated by license-check workflow --- third-party-licenses.darwin.md | 2 +- third-party-licenses.linux.md | 2 +- third-party-licenses.windows.md | 2 +- third-party/golang.org/x/net/{html => }/LICENSE | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename third-party/golang.org/x/net/{html => }/LICENSE (100%) diff --git a/third-party-licenses.darwin.md b/third-party-licenses.darwin.md index f1d33c5130..9e8cd0b794 100644 --- a/third-party-licenses.darwin.md +++ b/third-party-licenses.darwin.md @@ -42,7 +42,7 @@ The following packages are included for the amd64, arm64 architectures. - [github.com/subosito/gotenv](https://pkg.go.dev/github.com/subosito/gotenv) ([MIT](https://github.com/subosito/gotenv/blob/v1.6.0/LICENSE)) - [github.com/yosida95/uritemplate/v3](https://pkg.go.dev/github.com/yosida95/uritemplate/v3) ([BSD-3-Clause](https://github.com/yosida95/uritemplate/blob/v3.0.2/LICENSE)) - [go.yaml.in/yaml/v3](https://pkg.go.dev/go.yaml.in/yaml/v3) ([MIT](https://github.com/yaml/go-yaml/blob/v3.0.5/LICENSE)) - - [golang.org/x/net/html](https://pkg.go.dev/golang.org/x/net/html) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) + - [golang.org/x/net](https://pkg.go.dev/golang.org/x/net) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) - [golang.org/x/oauth2](https://pkg.go.dev/golang.org/x/oauth2) ([BSD-3-Clause](https://cs.opensource.google/go/x/oauth2/+/v0.36.0:LICENSE)) - [golang.org/x/sync/errgroup](https://pkg.go.dev/golang.org/x/sync/errgroup) ([BSD-3-Clause](https://cs.opensource.google/go/x/sync/+/v0.20.0:LICENSE)) - [golang.org/x/sys](https://pkg.go.dev/golang.org/x/sys) ([BSD-3-Clause](https://cs.opensource.google/go/x/sys/+/v0.45.0:LICENSE)) diff --git a/third-party-licenses.linux.md b/third-party-licenses.linux.md index bd98a92cb8..f5b267f28d 100644 --- a/third-party-licenses.linux.md +++ b/third-party-licenses.linux.md @@ -42,7 +42,7 @@ The following packages are included for the 386, amd64, arm64 architectures. - [github.com/subosito/gotenv](https://pkg.go.dev/github.com/subosito/gotenv) ([MIT](https://github.com/subosito/gotenv/blob/v1.6.0/LICENSE)) - [github.com/yosida95/uritemplate/v3](https://pkg.go.dev/github.com/yosida95/uritemplate/v3) ([BSD-3-Clause](https://github.com/yosida95/uritemplate/blob/v3.0.2/LICENSE)) - [go.yaml.in/yaml/v3](https://pkg.go.dev/go.yaml.in/yaml/v3) ([MIT](https://github.com/yaml/go-yaml/blob/v3.0.5/LICENSE)) - - [golang.org/x/net/html](https://pkg.go.dev/golang.org/x/net/html) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) + - [golang.org/x/net](https://pkg.go.dev/golang.org/x/net) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) - [golang.org/x/oauth2](https://pkg.go.dev/golang.org/x/oauth2) ([BSD-3-Clause](https://cs.opensource.google/go/x/oauth2/+/v0.36.0:LICENSE)) - [golang.org/x/sync/errgroup](https://pkg.go.dev/golang.org/x/sync/errgroup) ([BSD-3-Clause](https://cs.opensource.google/go/x/sync/+/v0.20.0:LICENSE)) - [golang.org/x/sys](https://pkg.go.dev/golang.org/x/sys) ([BSD-3-Clause](https://cs.opensource.google/go/x/sys/+/v0.45.0:LICENSE)) diff --git a/third-party-licenses.windows.md b/third-party-licenses.windows.md index 05086f41c2..7e834bc36f 100644 --- a/third-party-licenses.windows.md +++ b/third-party-licenses.windows.md @@ -43,7 +43,7 @@ The following packages are included for the 386, amd64, arm64 architectures. - [github.com/subosito/gotenv](https://pkg.go.dev/github.com/subosito/gotenv) ([MIT](https://github.com/subosito/gotenv/blob/v1.6.0/LICENSE)) - [github.com/yosida95/uritemplate/v3](https://pkg.go.dev/github.com/yosida95/uritemplate/v3) ([BSD-3-Clause](https://github.com/yosida95/uritemplate/blob/v3.0.2/LICENSE)) - [go.yaml.in/yaml/v3](https://pkg.go.dev/go.yaml.in/yaml/v3) ([MIT](https://github.com/yaml/go-yaml/blob/v3.0.5/LICENSE)) - - [golang.org/x/net/html](https://pkg.go.dev/golang.org/x/net/html) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) + - [golang.org/x/net](https://pkg.go.dev/golang.org/x/net) ([BSD-3-Clause](https://cs.opensource.google/go/x/net/+/v0.55.0:LICENSE)) - [golang.org/x/oauth2](https://pkg.go.dev/golang.org/x/oauth2) ([BSD-3-Clause](https://cs.opensource.google/go/x/oauth2/+/v0.36.0:LICENSE)) - [golang.org/x/sync/errgroup](https://pkg.go.dev/golang.org/x/sync/errgroup) ([BSD-3-Clause](https://cs.opensource.google/go/x/sync/+/v0.20.0:LICENSE)) - [golang.org/x/sys](https://pkg.go.dev/golang.org/x/sys) ([BSD-3-Clause](https://cs.opensource.google/go/x/sys/+/v0.45.0:LICENSE)) diff --git a/third-party/golang.org/x/net/html/LICENSE b/third-party/golang.org/x/net/LICENSE similarity index 100% rename from third-party/golang.org/x/net/html/LICENSE rename to third-party/golang.org/x/net/LICENSE