fix(mcp): follow redirects with per-hop SSRF re-validation - #344
Open
terpstra-209 wants to merge 1 commit into
Open
fix(mcp): follow redirects with per-hop SSRF re-validation#344terpstra-209 wants to merge 1 commit into
terpstra-209 wants to merge 1 commit into
Conversation
guardedFetch set redirect: "manual" and returned the 3xx as-is, so any MCP server whose OAuth discovery endpoints redirect was unusable. The MCP SDK continues its well-known URL search only on 4xx and treats anything else as fatal, so a single 302 aborted discovery even when a later candidate URL would have returned valid metadata. Follow redirects here rather than delegating to the runtime, so every hop is re-checked by validateRemoteMcpUrl (HTTPS, private-IP, DNS re-resolution). This is stricter than redirect: "follow", which would let a public URL bounce to an address the guard never saw. Bounded: GET/HEAD only (no POST body replay), 5 hops max, and the Authorization header is dropped when the origin changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
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
guardedFetchrefused to follow HTTP redirects, which made any MCP server whose OAuthdiscovery endpoints redirect impossible to connect. This follows redirects while
re-validating every hop against the same SSRF guard, so the protection is preserved
(and is stricter than plain
redirect: "follow").Found while connecting a public MCP server that serves its RFC 8414 metadata via a 302.
Why
guardedFetchsetredirect: "manual"and returned the 3xx response as-is. That helperis handed to the MCP SDK as its fetch implementation (
servers.ts:63), so the SDK saw theraw redirect during OAuth discovery.
The SDK's
discoverAuthorizationServerMetadatatries several well-known URLs in order,but continues only on 4xx:
A 302 is not 4xx, so the first redirecting candidate aborted the whole search — even when
a later candidate would have returned valid metadata. Concretely, for a server whose
protected-resource metadata advertises
https://host/mcpas its authorization server:https://host/.well-known/oauth-authorization-server/mcphttps://host/mcp/.well-known/oauth-authorization-serverThe connector could be created but never authorized, so it sat at 0 tools with an opaque
HTTP 302 trying to load OAuth metadataerror. The same server connects normally in otherMCP clients, which follow the redirect.
Changes
backend/src/lib/mcp/client.ts—guardedFetchnow follows redirects itself rather thandelegating to the runtime, so each hop is re-checked by
validateRemoteMcpUrl(HTTPS,private/reserved IP ranges, DNS re-resolution).
redirect: "manual"is retained on theunderlying
fetchso the runtime never follows anything unvalidated.Deliberately bounded:
that, and skipping it avoids reasoning about 307/308 body semantics.
Authorizationis dropped when the origin changes, so credentials can't leak to athird-party host via redirect.
Locationvalues are resolved against the current URL before validation.This is stricter than the
redirect: "follow"a normal client would use: withfollow,the runtime resolves the redirect chain internally and the guard only ever sees the
original URL, so a public hostname could bounce to a private address unchecked. Here every
hop is validated before it is fetched.
Testing
backend/src/lib/mcp/__tests__/client.ssrf.test.ts— 7 new cases:Locationagainst the current URL169.254.169.254(the guard still fires mid-chain)Authorizationon a cross-origin redirectAuthorizationon a same-origin redirectResults:
Manually verified end to end: the connector that previously failed with
HTTP 302 trying to load OAuth metadatanow completes OAuth and discovers its full toollist.
The investigation and code in this PR were done by Claude (Opus 5) via Claude Code; I
reviewed and am submitting it.