Skip to content

Validate redirect scheme and strip credentials on cross-origin redirects in MCP HTTP client#320347

Open
g0w6y wants to merge 2 commits into
microsoft:mainfrom
g0w6y:fix/mcp-redirect-scheme-and-credential-leak
Open

Validate redirect scheme and strip credentials on cross-origin redirects in MCP HTTP client#320347
g0w6y wants to merge 2 commits into
microsoft:mainfrom
g0w6y:fix/mcp-redirect-scheme-and-credential-leak

Conversation

@g0w6y
Copy link
Copy Markdown

@g0w6y g0w6y commented Jun 8, 2026

Summary

The MCP HTTP client in src/vs/workbench/api/common/extHostMcp.ts follows 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:

  1. 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, _fetchInternal dispatches unix:// and pipe:// URLs over a local socket (extHostMcpNode.ts). A Location: 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 to http(s) at configuration time (mcpConfiguration.ts, pattern: ^https?:\/\/.+); the redirect path did not enforce the same restriction.

  2. Cross-origin credential replay. For 307/308 the method, body and all headers are preserved across the redirect. The configured Authorization bearer token (OAuth or a static key from the mcp.json headers), the Mcp-Session-Id, and the JSON-RPC body are re-sent to the redirect target even when it is a different origin. Browser fetch and curl strip credentials on a cross-origin redirect; the MCP client did not.

Fix

In the redirect loop:

  • Only follow redirects whose resolved protocol is http: or https:; otherwise stop following and log a warning. This blocks unix://, pipe://, file://, etc.
  • When the redirect target origin differs from the current origin, drop credential-bearing headers (Authorization, Cookie, Proxy-Authorization, Mcp-Session-Id) before the request is re-sent, matching fetch/curl behavior.

Same-origin redirects are unaffected and keep their headers. The change is confined to the redirect loop plus two module-level constants.

Testing

  • Manual: a redirect server returning 307 Location: <cross-origin> no longer receives the configured Authorization header or Mcp-Session-Id.
  • Manual: a redirect server returning 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.

Copilot AI review requested due to automatic review settings June 8, 2026 06:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/vs/workbench/api/common/extHostMcp.ts
Comment thread src/vs/workbench/api/common/extHostMcp.ts
@vs-code-engineering
Copy link
Copy Markdown
Contributor

vs-code-engineering Bot commented Jun 8, 2026

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@TylerLeonhardt

Matched files:

  • src/vs/workbench/api/common/extHostMcp.ts

@g0w6y
Copy link
Copy Markdown
Author

g0w6y commented Jun 8, 2026

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants