fix(http): allow projected MCP headers in preflights - #3167
fix(http): allow projected MCP headers in preflights#3167SamMorrowDrums wants to merge 2 commits into
Conversation
Sorry, something went wrong.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Auto-generated by license-check workflow
There was a problem hiding this comment.
Pull request overview
Extends browser CORS preflight support for MCP routing and projected parameter headers.
Changes:
- Adds fixed MCP method, name, owner, and repo headers.
- Validates, deduplicates, sorts, and reflects future Mcp-Param-* headers.
- Expands middleware and router-level CORS tests.
| File | Description |
|---|---|
| pkg/http/server_test.go | Tests projected headers through the full router. |
| pkg/http/middleware/cors.go | Implements projected-header preflight handling. |
| pkg/http/middleware/cors_test.go | Covers validation, ordering, deduplication, and preserved behavior. |
| pkg/http/headers/headers.go | Defines MCP projection header constants. |
| go.mod | Promotes x/net to a direct dependency. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 8/9 changed files
- Comments generated: 0
- Review effort level: Balanced
Sorry, something went wrong.
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
There's no cap on how many projected names we reflect here. Within Go's 1MB request header limit, a client can send thousands of distinct Mcp-Param-x names on a preflight and get every one of them echoed back, growing both seen and the response header. That's a cheap request producing a large response — a small amplification vector.
Could we bound this? Stopping after something like 32–64 projected names would close it off at no practical cost, since real clients will only ever send a handful.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
Summary
Exact projected-header contract
CORS has no prefix wildcard, so preflights validate each requested name with httpguts.ValidHeaderFieldName. Matching is case-insensitive against the exact Mcp-Param- prefix and requires a non-empty suffix. Fixed owner/repo entries remain first; future names are case-insensitively deduplicated, sorted, and emitted with http.CanonicalHeaderKey. Arbitrary headers, bare Mcp-Param-, malformed/control-character names, and lookalike prefixes are omitted.
Mcp-Param-* headers are request-only and are not added to Access-Control-Expose-Headers; Mcp-Session-Id and WWW-Authenticate remain exposed.
Cross-repository dependencies
Validation