fix(auth): do not trust proxy headers by default when deriving public origin#163
Open
sebastiondev wants to merge 1 commit into
Open
fix(auth): do not trust proxy headers by default when deriving public origin#163sebastiondev wants to merge 1 commit into
sebastiondev wants to merge 1 commit into
Conversation
… origin Previously getPublicOrigin/getPublicUrl unconditionally honoured X-Forwarded-Host / X-Forwarded-Proto / Forwarded headers, which let clients spoof the public origin in deployments not fronted by a proxy that strips/overwrites these headers. The spoofed origin was used to build the resource_metadata URL in WWW-Authenticate responses and the resource identifier in OAuth Protected Resource Metadata (RFC 9728), potentially redirecting OAuth clients to attacker-controlled servers (CWE-918). Make trust opt-in via a new `trustProxy` option on getPublicOrigin, getPublicUrl, withMcpAuth, and protectedResourceHandler. Default is `false` (origin derived from req.url only). Existing tests that exercised proxy-header behaviour are updated to opt in.
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
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.
Summary
The
getPublicOrigin()andgetPublicUrl()functions insrc/lib/url.tsunconditionally trustX-Forwarded-Host,X-Forwarded-Proto, andForwardedheaders from any client. When an MCP server built withmcp-handleris deployed without a reverse proxy that sanitizes these headers (or when it's directly exposed), any HTTP client can inject arbitrary values into these headers to control the origin used in:resourcefield of the OAuth Protected Resource Metadata response (/.well-known/oauth-protected-resource)resource_metadataURL inWWW-Authenticateheaders returned bywithMcpAuth()This is a CWE-918 (Server-Side Request Forgery) variant — specifically origin spoofing. An attacker who sends a request with
X-Forwarded-Host: attacker.examplecauses the server to advertiseattacker.exampleas the resource origin. An OAuth client that follows theresource_metadataURL in aWWW-Authenticateresponse would then fetch metadata from the attacker-controlled domain, potentially leaking tokens or accepting a malicious authorization server.Affected functions
getPublicOrigin()insrc/lib/url.tsgetPublicUrl()insrc/lib/url.tsprotectedResourceHandler()insrc/auth/auth-metadata.tswithMcpAuth()insrc/auth/auth-wrapper.tsPoC
With the fix applied (default
trustProxy: false), the same request returns the actual server origin derived fromreq.url.Fix description
This PR adds a
trustProxyoption (defaulting tofalse) togetPublicOrigin(),getPublicUrl(),protectedResourceHandler(), andwithMcpAuth(). WhentrustProxyisfalse(the new default), proxy headers are ignored entirely and the origin is derived solely fromreq.url. Deployments behind a trusted reverse proxy can opt in withtrustProxy: true.This is a secure-by-default change. The
trustProxyoption mirrors the well-established pattern from Express.js and other frameworks where trusting forwarding headers requires explicit opt-in.The
resourceUrlexplicit override (already supported) continues to take precedence over all auto-detection, and its use is now more strongly recommended in the JSDoc.Testing
All 15 existing tests pass, plus 2 new tests that verify the fix:
X-Forwarded-Host: attacker.exampleand confirmsresourceuses the real origin fromreq.urlForwardedheaderExisting proxy-header tests were updated to pass
trustProxy: true, confirming the opt-in path still works correctly.Adversarial review
Before submitting, we considered whether existing mitigations prevent exploitation. The library does support an explicit
resourceUrloverride, but it's optional and not set by default — deployments that rely on auto-detection (the default path) are vulnerable. We also considered whether typical deployment environments (e.g., Vercel) sanitize forwarding headers, but sincemcp-handleris a general-purpose npm library used across diverse hosting environments, the library itself must default to safe behavior regardless of deployment context.Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.