fix(http): apply CORS headers to all top-level responses - #3148
fix(http): apply CORS headers to all top-level responses#3148CAOShurong wants to merge 1 commit into
Conversation
Sorry, something went wrong.
The SetCorsHeaders middleware was only mounted on the MCP route group, so responses from the OAuth protected resource metadata endpoints and any router-level error fell through without Access-Control-* headers. Browser clients then masked the real failure (a 401 auth challenge or 404) behind an opaque "missing CORS headers" error, making the server appear unreachable from web-based MCP clients. Move the middleware to the top-level chi router so every response class — MCP auth challenges (401), OAuth metadata (200), and fall-through errors — carries CORS headers. The server authenticates via bearer tokens rather than cookies, so wildcard origins remain safe at this layer.
|
Thank you for investigating this and for correctly identifying that the CORS middleware needed to move to the top-level router. That core diagnosis and fix direction are sound.\n\nWe are consolidating the complete change in #3147, which includes the same root-router CORS fix plus the missing readonly/insiders/toolset protected-resource metadata variants, terminal 404 handling for unknown metadata paths, additive exposed-header behavior, the full challenge-to-metadata route contract, and the cross-repository hosted rollout dependencies. Closing this PR as superseded so review and release can proceed through one implementation. Thanks again for the useful contribution. |
Sorry, something went wrong.
Closes #3095.
Problem
#3095 reports that browser-based MCP clients see "This MCP server doesn't support web access. Missing CORS headers." when connecting to the hosted server — even though a raw OPTIONS preflight succeeds with the right headers. The reporter's follow-up captures it precisely: the preflight passes, but subsequent non-2xx responses (401 from the auth challenge, and errors from unauthenticated metadata lookups) come back without CORS headers, so the browser hides the real error behind an opaque CORS failure.
Root cause
In pkg/http/server.go, middleware.SetCorsHeaders was mounted only on the MCP route group:
The MCP endpoints themselves were fine — which is why curl-based checks passed while real browser clients failed.
Fix
Move r.Use(middleware.SetCorsHeaders) from the MCP group to the top-level chi router, so every response class carries CORS headers:
Wildcard origins remain safe at this layer: the server authenticates via bearer tokens, not cookies, so cross-origin requests cannot exploit ambient credentials (unchanged rationale from the original middleware).
Testing
New TestTopLevelCORSHeadersOnAllResponses + TestTopLevelCORSPreflightOnMetadataRoute in pkg/http/server_cors_test.go, building the same two-group router layout as RunHTTPServer:
Gates: go build ./... clean; go vet ./pkg/http/... clean; go test ./pkg/http/... ./pkg/context/... -count=1 all pass; golangci-lint v2.13.1 reports only the 6 pre-existing gosec hits in token_test.go (identical on clean main).
Note for browser-client authors (re #3095)
With this fix the browser can finally see the 401 + WWW-Authenticate: Bearer resource_metadata="..." challenge and the RFC 9728 metadata document. Authentication itself is GitHub OAuth outside this repo's scope — clients should implement the standard MCP authorization flow triggered by that challenge.