Add CORS support to the transparent MCP proxy

[DevamShah]

Summary

Add opt-in CORS support to the transparent MCP proxy so that browser-based clients (e.g. MCP Inspector) can connect without hitting a 405 on the OPTIONS preflight.

Fixes #4297

Problem / motivation

The transparent MCP proxy (/mcp endpoint) has no OPTIONS handler. When a browser-based client such as MCP Inspector (http://localhost:6274) performs a CORS preflight before a real request, the server returns 405 Method Not Allowed because Go's http.ServeMux does not automatically handle OPTIONS. The browser then blocks the actual request with net::ERR_FAILED / CORS policy: No 'Access-Control-Allow-Origin' header.

This blocks every web-based MCP client from using the ToolHive proxy for local development and testing, exactly as reported in #4297.

Change

• pkg/transport/middleware/cors.go — a new CORS(allowedOrigins []string) MiddlewareFunction that:
  • Intercepts all OPTIONS requests, returns 204, never forwarded to the backend.
  • Injects Access-Control-Allow-Origin/Methods/Headers/Expose-Headers + Vary: Origin on responses whose Origin header matches an allowed entry.
  • Supports exact match (http://localhost:6274), scheme+host prefix (http://localhost → any port), and wildcard (*).
  • Is a no-op when allowedOrigins is empty — zero behaviour change by default.
• pkg/transport/proxy/transparent/transparent_proxy.go — adds a corsOrigins field and a WithAllowedOrigins([]string) Option, and wires the middleware as the outermost handler in Start() (after the stateless method gate), so OPTIONS is intercepted before the gate or backend can reject it.
• cmd/thv/app/proxy.go — adds the --allow-origins flag to thv proxy and switches to NewTransparentProxyWithOptions to pass the option.

Does this introduce a user-facing change?

Yes. New optional flag on thv proxy:

--allow-origins <origin>   Allowed CORS origins (repeatable).
  Exact:       http://localhost:6274
  Scheme+host: http://localhost   (matches any port)
  Wildcard:    *

No default value — CORS is disabled unless the flag is passed. Existing deployments are unaffected.

Security rationale

CORS is a browser-enforced same-origin mechanism (OWASP A05:2021 Security Misconfiguration, CWE-942 Permissive Cross-domain Policy). This implementation:

• Defaults to off — no CORS headers are set unless --allow-origins is explicitly passed.
• Never combines * with credentials — no Access-Control-Allow-Credentials: true header is emitted, so a wildcard origin and credentials are never paired.
• Reflects only the exact request origin (or *) — partial path/query components cannot inject forged headers; the scheme+host prefix match requires a : boundary, so http://localhost does not match http://localhost.evil.com.
• No third-party CORS dependency — standard-library Go with straightforward origin matching, fully reviewable.

Test plan

Unit tests added (pkg/transport/middleware/cors_test.go): preflight handling, header injection, origin matching (exact/prefix/wildcard/negative), and no-op-when-disabled, plus table-driven matchCORSOrigin cases.

go build ./pkg/transport/middleware/... ./pkg/transport/proxy/transparent/... ./cmd/thv/app/...   # clean
go vet  ./pkg/transport/middleware/... ./pkg/transport/proxy/transparent/... ./cmd/thv/app/...     # clean
go test ./pkg/transport/middleware/...                # PASS (incl. all TestCORS cases)
go test ./pkg/transport/proxy/transparent/...         # PASS (no regressions)
go test ./cmd/thv/app/...                             # PASS
gofmt -l <changed files>                              # clean

Manual: thv proxy my-server --target-uri http://localhost:8080 --allow-origins http://localhost:6274, then point MCP Inspector at the proxy — the OPTIONS preflight returns 204 with Access-Control-Allow-Origin: http://localhost:6274 and the follow-up POST reaches the backend. Without --allow-origins, behaviour is unchanged.

Notes for reviewers

• CONTRIBUTING asks for E2E tests covering API and CLI flows. The fix is a browser-level CORS-preflight concern that is impractical to cover in the container-runtime E2E harness without a headless-browser fixture, so it is covered by middleware-level unit tests instead. Happy to add an E2E case if you can point me at a suitable fixture.
• --allow-origins is wired into thv proxy only. thv run's internal transparent proxy would need the same option plumbed through buildProxyOptions; left as a focused follow-up to keep this PR small.
• DCO: commit is signed off.

[codecov]

Codecov Report

❌ Patch coverage is 82.50000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.02%. Comparing base (ddbdcb8) to head (fc820f0).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...g/transport/proxy/transparent/transparent_proxy.go	0.00%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #5588    +/-   ##
========================================
  Coverage   70.01%   70.02%            
========================================
  Files         651      660     +9     
  Lines       66401    66903   +502     
========================================
+ Hits        46492    46850   +358     
- Misses      16533    16669   +136     
- Partials     3376     3384     +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[aponcedeleonch]

Thanks for the contribution. Clean implementation, well-structured tests, and the security defaults (opt-in, no credentials header) are all correct.

[aponcedeleonch]

suggestion: The entry + ":" colon boundary is the key guard here. It's what prevents http://localhost.evil.com from matching the entry http://localhost. That invariant isn't explicitly tested, though. Worth adding a case alongside the existing prefix tests:

{
    name:           "prefix match does not allow sibling hostname",
    requestOrigin:  "http://localhost.evil.com",
    allowed:        []string{"http://localhost"},
    expectedResult: "",
},

This documents the security property and protects against a future refactor accidentally weakening it.

[aponcedeleonch]

One thing to do before CI goes green: run task docs from the repo root to regenerate the CLI reference docs. Adding --allow-origins means the auto-generated content under docs/ is out of date, and the docs check will fail without it.

[JAORMX]

So, got a panel together on this one (spec, standards, security, architecture, MCP protocol, UX, devex) since CORS is one of those features that looks small but has a habit of biting you later. The security-sensitive ask got extra eyes.

Overall the shape is good. Opt-in, default-off, no credentials, origin matching that doesn't fall for the usual prefix-spoofing traps. The toggle you asked about is there and it works... --allow-origins is repeatable, empty by default, and there are guards at four layers so a deployment that handles CORS at a gateway is genuinely unaffected. That part's solid.

But there's one thing that'll break the headline use case, plus a few rough edges worth tightening before this merges.

Blocking: MCP-Protocol-Version is missing from the allowed headers

This one's the real problem. The MCP Streamable HTTP transport (2025-11-25) has clients send MCP-Protocol-Version on requests after initialize. ToolHive itself reads and validates it at pkg/transport/proxy/streamable/streamable_proxy.go:360... a bad version gets a 400. And pkg/telemetry/middleware.go:393 reads it too.

The catch: MCP-Protocol-Version is not a CORS-safelisted request header. So a spec-compliant browser client (MCP Inspector included) that sends it triggers a preflight, and Access-Control-Allow-Headers has to list it. Right now corsAllowedHeaders is "Content-Type, Accept, Mcp-Session-Id, Authorization" (cors.go:19), no protocol version. So the browser blocks the actual POST right after initialize. Which is exactly the scenario this PR exists to enable. Oops!

Fix:

corsAllowedHeaders = "Content-Type, Accept, Mcp-Session-Id, MCP-Protocol-Version, Authorization, Last-Event-ID"
corsExposedHeaders = "Mcp-Session-Id, MCP-Protocol-Version"

Last-Event-ID is for SSE stream resumption (also not safelisted). MCP-Protocol-Version belongs in Expose-Headers too, so the browser can read the negotiated version back from the response.

Security findings (extra-focus pass)

The origin matching is sound against the attacks I worried about most. matchCORSOrigin (cors.go:85) uses a : boundary for the scheme+host prefix, so http://localhost does NOT match http://localhost.evil.com (the next char is ., not :). No CWE-942. Wildcard * is never paired with credentials (no Access-Control-Allow-Credentials is ever set, anywhere). Default-off is real. No CRLF response-splitting path (Go's http.Header.Set sanitizes, and the request parser terminates Origin at CRLF). Vary: Origin is set correctly with Add so it composes. Good.

One medium worth a decision: the CORS middleware is wired outermost (transparent_proxy.go:1242), so OPTIONS preflights are intercepted before the auth middleware runs. That's actually spec-conformant (the Fetch standard expects preflight to be unauthenticated, and the response leaks nothing sensitive, just static headers), so I'd call it acceptable. But it does mean an unauthenticated OPTIONS advertises Authorization in Access-Control-Allow-Headers, which tells a prober "this endpoint expects bearer tokens." Minor capability leak, not a hole. Worth a one-line comment noting preflight is intentionally unauthenticated.

Also: on a stateless server, preflight advertises GET, POST, DELETE, OPTIONS but the statelessMethodGate (transparent_proxy.go:1510) only allows POST, OPTIONS. So a browser preflights DELETE successfully, then the actual DELETE hits 405. Preflight/actual asymmetry... confusing for clients, not a security issue. Either make the advertised methods conditional on stateless, or accept the mismatch with a comment.

UX / toggle

The toggle design itself is right. But --help doesn't actually tell the gateway-deployment operator that doing nothing is the correct choice. The "disabled by default" note is in a code comment (proxy.go:163), not the help string. An operator running thv proxy --help sees the flag with no indication that omitting it leaves CORS off. One line in the help text would fix it:

CORS is disabled by default; omit this flag when CORS is handled by an upstream gateway.

Second thing: --allow-origins "" (empty string) is a surprising edge. It passes the len > 0 check (length is 1), activates the middleware, but matchCORSOrigin matches nothing... so you get OPTIONS preflights eaten (204 instead of 405) with no CORS headers set. Half-configured and confusing. Validate/reject empty entries, or filter them before the guard.

The thv run omission is a real UX gap too... thv run is the primary way operators expose MCP servers, and it has no --allow-origins. The plumbing (WithAllowedOrigins) exists, it's just not wired through buildProxyOptions (pkg/transport/http.go:431). The PR scopes it to thv proxy only, which keeps the diff small, but an operator will reasonably expect parity. At least note the limitation in the help text if it's a follow-up.

Architecture / devex

A few structural notes from the architect and devex passes:

• NewTransparentProxy is now a thin delegator to NewTransparentProxyWithOptions, and after this PR it has zero non-test callers. Either deprecate it (it's dead weight) or commit to it as a stable alias. Right now it's boilerplate that has to stay in lockstep on every signature change (go-style "avoid parallel types that drift").
• Origins aren't validated. --allow-origins "localhost:6274" (no scheme) or "http://localhost:6274/" (trailing slash) silently never matches anything, and the deployer gets a broken browser experience with no startup signal. The go-style rule "Constructor Validation: Fail Loudly on Invalid Input" applies here... validate at WithAllowedOrigins/CORS and reject entries that aren't * or don't parse as a URL with scheme + host.
• CORS and WithAllowedOrigins retain the caller's slice without a defensive copy (cors.go:51, transparent_proxy.go:356). The closure lives for process lifetime. A one-line slices.Clone removes the footgun.
• The CORS doc comment should state the credentials constraint explicitly: Access-Control-Allow-Credentials is never set, so cookie-based browser auth won't work regardless of origin matching. Deployers wiring browser auth via cookies will hit this with no pointer in the docs.
• The triple empty-guard (in CORS, in WithAllowedOrigins, in Start) is redundant. Pick one layer to own the contract.

Standards

Minor: cmd/thv/app/proxy.go:280 uses the mutable-variable-then-conditional-append pattern; go-style prefers an immediately-invoked anonymous function. The E2E omission is disclosed and acceptable for this scope (browser-preflight is impractical in the container E2E harness), and the thv run follow-up is a reasonable scope boundary.

Summary

The toggle is there and the security posture against the usual CORS attacks is solid. But MCP-Protocol-Version missing from the allowed headers breaks the browser client case this PR exists to fix, so that's a request-changes from me. Once that's in, the UX help-text disclosure, the empty-string validation, and the origin validation are the next ones I'd want before merge. The rest (stateless method mismatch, thv run parity, constructor cleanup, slice copy, doc comment) can land as follow-ups if you want to keep this tight.

[JAORMX]

Ship-blocker: MCP-Protocol-Version is missing here. ToolHive reads and validates it at pkg/transport/proxy/streamable/streamable_proxy.go:360 (bad version = 400) and pkg/telemetry/middleware.go:393. It's not a CORS-safelisted header, so a spec-compliant browser client sending it triggers a preflight that this allowlist rejects... and the browser blocks the POST right after initialize. Which is the exact case this PR enables.

Add MCP-Protocol-Version (and Last-Event-ID for SSE resumption) to corsAllowedHeaders. Verified against the current branch.

[JAORMX]

MCP-Protocol-Version should be in Expose-Headers too, so a browser client can read the negotiated version back from the response. (Content-Type is already CORS-safelisted, so listing it here is harmless but redundant.)

[JAORMX]

On a stateless server, the statelessMethodGate (transparent_proxy.go:1510) only allows POST, OPTIONS, but preflight advertises GET, POST, DELETE, OPTIONS. So a browser preflights DELETE successfully, then the actual DELETE hits 405. Preflight/actual asymmetry... confusing for clients. Either make the advertised methods conditional on stateless, or note the mismatch with a comment.

[JAORMX]

Origins aren't validated. --allow-origins "localhost:6274" (no scheme) or "http://localhost:6274/" (trailing slash) silently never matches anything, and the deployer gets a broken browser experience with no startup signal. Per go-style "Constructor Validation: Fail Loudly on Invalid Input", reject entries that aren't * or don't parse as a URL with scheme + host.

Also: allowedOrigins is captured into the closure without a slices.Clone... the closure lives for process lifetime, so a caller mutating the backing array later would change matching at runtime. One-line slices.Clone removes the footgun.

[JAORMX]

The "disabled by default" note is in the code comment above, not the help string an operator sees in --help. An operator who handles CORS at a gateway gets no signal that doing nothing is the correct choice. One line in the help text would fix it:

CORS is disabled by default; omit this flag when CORS is handled by an upstream gateway.

Also watch the --allow-origins "" edge: it passes the len > 0 check (length 1), activates the middleware, but matchCORSOrigin matches nothing... so preflights get eaten (204 instead of 405) with no CORS headers. Half-configured and confusing. Validate/reject empty entries or filter them before the guard.

[JAORMX]

NewTransparentProxy now delegates to NewTransparentProxyWithOptions and has zero non-test callers after this PR. Either deprecate it (dead weight) or commit to it as a stable alias... right now it's boilerplate that has to stay in lockstep on every signature change (go-style "avoid parallel types that drift").

Also: WithAllowedOrigins silently no-ops on empty input and stores the caller's slice without a copy. Consider validating origins here (fail loudly on invalid) and slices.Clone-ing before storing.

[DevamShah]

Thanks @JAORMX for convening the panel and @aponcedeleonch for the careful read — the feedback was precise and made this materially better. Pushed f2df558 addressing every point. Build, go vet, gofmt, and the targeted test suites all pass locally.

1. Ship-blocker — MCP-Protocol-Version missing from Allow-Headers. Added it to corsAllowedHeaders. A browser MCP client now passes the CORS preflight for the header that ToolHive reads and validates at streamable_proxy.go:360 and records at telemetry/middleware.go:393. Without it the header was stripped and the client could never negotiate a version.

2. Expose-Headers. Added MCP-Protocol-Version to corsExposedHeaders so the browser can read the negotiated version back, and removed Content-Type since it's already a CORS-safelisted response header — no need to list it.

3. Stateless method mismatch. Reconciled against the real source of truth. The advertised preflight methods are now derived from the same constants the request path enforces: statelessAllowedMethods ("POST, OPTIONS") and statefulAllowedMethods ("GET, POST, DELETE, OPTIONS"), both defined alongside statelessMethodGate. CORS() takes the method set as a parameter and the proxy passes the stateless set when p.stateless is true, so a browser can no longer preflight DELETE successfully only to have the real request 405. Added TestCORSAllowedMethodsMatchGate to lock the invariant.

4. Origin validation at startup. New ValidateAndNormalizeOrigins runs before the proxy starts: a schemeless entry (e.g. localhost:6274) is rejected with a clear error because it can never match a browser Origin, and a trailing slash (e.g. http://localhost:6274/) is normalized away with a warning. Misconfiguration now fails loudly instead of silently producing a broken browser experience.

5. Help text. The --allow-origins flag help now states CORS is disabled by default (so an operator terminating CORS at a gateway knows doing nothing is correct) and documents the scheme / no-trailing-slash requirements.

6. NewTransparentProxy. Committed to it as a stable positional-arg convenience wrapper around NewTransparentProxyWithOptions and documented it as such, rather than deprecating. It's still the constructor used across ~25 in-repo test sites; a // Deprecated: marker would trip staticcheck SA1019 on all of them (no test exclusion for staticcheck in .golangci.yml). New knobs like WithAllowedOrigins remain Options-only, which the doc comment now points to.

7. Colon-boundary invariant test. Added explicit cases to TestMatchCORSOrigin proving http://localhost.evil.com (and its port-bearing variant) does not match the entry http://localhost — the entry + ":" boundary is what enforces this. Good catch @aponcedeleonch.

8. Docs. Regenerated the CLI reference for the new flag (docs/cli/thv_proxy.md) — the flag was previously absent from the generated docs, so the docs check would have failed.

Happy to adjust any of these — particularly the call on #6 if you'd prefer deprecation with a lint exclusion instead.

[aponcedeleonch]

Thanks for the quick turnaround on the feedback.

Good progress addressing the feedback. Two open items from JAORMX's inline comments that weren't picked up in the latest commit:

• Last-Event-ID was mentioned alongside MCP-Protocol-Version in the blocker inline comment as needed for SSE stream resumption, but only MCP-Protocol-Version landed.
• slices.Clone is still missing in both CORS() and WithAllowedOrigins. The risk is low given the slice comes from a CLI flag, but the inline comments are technically unresolved.

CI is also red, so another commit is coming anyway. Once that's in, I'm fine to approve unless @JAORMX wants those two items resolved first before closing out the review.

[JAORMX]

CI's still red on f2df558, so not quite ready to merge. Two things to fix:

• gocyclo flagged proxyCmdFunc (complexity 16, limit's 15)... the --allow-origins wiring pushed it over. Pull the option-building into a helper and it'll drop right back down.
• Race detector fired across the CORS tests. This is the slices.Clone we flagged... CORS() and WithAllowedOrigins still capture the caller's slice. Adding the clone kills the race.

And the two open items from before are still hanging:

• Last-Event-ID missing from corsAllowedHeaders (JAORMX mentioned it alongside MCP-Protocol-Version for SSE resumption).
• Same slices.Clone, now actively breaking CI.

Fix those three, re-run CI, and once it's green I'll approve.