Conversation
… API and /events The REST surface and the SSE stream sent `Access-Control-Allow-Origin: *` unconditionally, so any website could read responses to key-authenticated requests once a key reached a browser context — which happens in practice, since the Web UI is routinely opened with `?apikey=`. There is no `Access-Control-Allow-Credentials` anywhere, so this was a key-leak amplifier rather than a cookie-session hole, but the wildcard bought nothing: the embedded UI is same-origin under /ui/. CORS now echoes the request Origin only when it passes the allowlist the MCP surface already uses — loopback on any port, or a host in config `trusted_hosts` — and emits no CORS headers at all otherwise, including when there is no Origin header (every non-browser client). `Vary: Origin` is Add'ed on every response so a shared cache never serves one origin's response to another. No new config field: reusing `trusted_hosts` gives the REST and MCP surfaces one origin policy, and it is already documented as governing Origin validation, hot-reloadable, and overridable via MCPPROXY_TRUSTED_HOSTS. The duplicate wildcard write inside handleSSEEvents is deleted rather than converted: /events is a chi route on the same router, so the Use-registered middleware already covers it, and a second call site is only somewhere for the two policies to drift. The allowlist helpers move verbatim from internal/server/host_validation.go into a new leaf package internal/httpx that both surfaces import (internal/server already imports internal/httpapi, so it could not be shared in place). One-line delegates stay behind, and internal/server/host_validation_test.go passes unchanged — that is the proof the extraction is behavior-preserving. Two defects found in cross-model review are fixed alongside: OriginAllowed accepted a trailing `?` or `#` (url.Parse hides a bare `?` in ForceQuery and drops a bare `#`, so neither struct check saw it), and Allow-Methods omitted PATCH although the API registers PATCH routes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
e130340
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://44d05e38.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-cors-loopback-allowlist.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 35748051985 --repo smart-mcp-proxy/mcpproxy-go
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The REST API (
/api/v1/*) and the SSE stream (/events) sentAccess-Control-Allow-Origin: *on every response. CORS now echoes the requestOriginonly when it is loopback (any port) or a host in configtrusted_hosts, and sends no CORS headers at all otherwise — including when there is noOriginheader, which is every non-browser client.Vary: Originis added to every response.Why
With
*, any website could read responses to key-authenticated requests once an API key reached a browser context. That happens in practice: the Web UI is routinely opened with?apikey=. There is noAccess-Control-Allow-Credentialsanywhere in the tree, so this was a key-leak amplifier rather than a cookie-session hole — but the wildcard bought nothing either, since the embedded UI is same-origin under/ui/.No new config field.
trusted_hostsalready exists, is already documented as validating theOriginheader, is hot-reloadable, and has an env override (MCPPROXY_TRUSTED_HOSTS) and an escape hatch ("*"). Reusing it gives the REST and MCP surfaces one origin policy instead of two that can drift.The duplicate wildcard write inside
handleSSEEventsis deleted, not converted:/eventsis a chi route on the same router, so theUse-registered middleware already covers it. The test asserts the header on/events— that is what proves it.The allowlist helpers (
originAllowed/isLoopbackHost/hostMatchesTrusted) move verbatim out ofinternal/server/host_validation.gointo a new leaf packageinternal/httpx, becauseinternal/serverimportsinternal/httpapiand they could not be shared in place. One-line delegates stay behind andinternal/server/host_validation_test.gopasses unchanged — that is the proof the extraction is behavior-preserving.Upgrade note
If a separate web app calls the REST API cross-origin from a public domain, it now needs its host in
trusted_hosts. Unlike theHostcheck, which only ever ran on the MCP endpoints, this one applies to the REST surface — so a REST-only reverse-proxied deployment may have been running withtrusted_hostsempty. Documented indocs/operations/reverse-proxy.mdanddocs/configuration.md.How verified
go test -raceoninternal/httpapi,internal/httpx, andinternal/server(CI skip regex) — green.golangci-lintboth passes (bare +--build-tags server) — no new issues.https://evil.example,null,http://127.0.0.1.evil.comand a missingOriginget no CORS headers;/eventsbehaves identically; preflight still works without an API key;/ui/still loads.gpt-5.6-sol), 2 rounds, clean verdict. Two findings fixed:OriginAllowedaccepted a trailing?or#(url.Parsehides a bare?inForceQueryand drops a bare#), andAllow-MethodsomittedPATCHalthough the API registers PATCH routes. One finding rejected as out of scope: the middleware short-circuits everyOPTIONSwith 200 rather than only true preflights — that is byte-for-byte the pre-existing behavior and belongs in its own change.🤖 Generated with Claude Code