Validate redirect scheme and strip credentials on cross-origin redirects in MCP HTTP client#320347
Open
g0w6y wants to merge 2 commits into
Open
Validate redirect scheme and strip credentials on cross-origin redirects in MCP HTTP client#320347g0w6y wants to merge 2 commits into
g0w6y wants to merge 2 commits into
Conversation
…cts in MCP HTTP client
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens MCP HTTP redirect handling to prevent redirects to non-HTTP(S) schemes and to avoid replaying sensitive headers across origins.
Changes:
- Restricts followed redirects to
http:/https:targets only. - Strips credential-bearing headers when redirects cross origin.
- Adds constants documenting and enforcing these redirect policies.
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @TylerLeonhardtMatched files:
|
Author
|
@microsoft-github-policy-service agree |
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 MCP HTTP client in
src/vs/workbench/api/common/extHostMcp.tsfollows HTTP redirects (301/302/303/307/308) without validating the scheme of the redirect target and without stripping credential headers when the target is a different origin. Two issues result:Arbitrary scheme follow. The redirect target is resolved with
new URL(location, currentUrl)and passed straight back into the fetch path. On the Node extension host,_fetchInternaldispatchesunix://andpipe://URLs over a local socket (extHostMcpNode.ts). ALocation: unix:///var/run/docker.sock#/...header therefore causes the client to issue requests to local sockets such as the Docker daemon socket. MCP server URLs are already restricted tohttp(s)at configuration time (mcpConfiguration.ts,pattern: ^https?:\/\/.+); the redirect path did not enforce the same restriction.Cross-origin credential replay. For 307/308 the method, body and all headers are preserved across the redirect. The configured
Authorizationbearer token (OAuth or a static key from themcp.jsonheaders), theMcp-Session-Id, and the JSON-RPC body are re-sent to the redirect target even when it is a different origin. Browserfetchandcurlstrip credentials on a cross-origin redirect; the MCP client did not.Fix
In the redirect loop:
http:orhttps:; otherwise stop following and log a warning. This blocksunix://,pipe://,file://, etc.Authorization,Cookie,Proxy-Authorization,Mcp-Session-Id) before the request is re-sent, matchingfetch/curlbehavior.Same-origin redirects are unaffected and keep their headers. The change is confined to the redirect loop plus two module-level constants.
Testing
307 Location: <cross-origin>no longer receives the configuredAuthorizationheader orMcp-Session-Id.302 Location: unix:///.../docker.sock#/...is no longer followed; a warning is logged and the redirect response is returned to the caller.Disclosure
This issue was reported to the Microsoft Security Response Center (MSRC) via the researcher portal — Case number 116632 (VULN-187202) — by Gouri Sankar A.