fix(client): add SSRF redirect protection to httpx client factory - #3423
fix(client): add SSRF redirect protection to httpx client factory#3423Ethanz11-creat wants to merge 1 commit into
Conversation
The Streamable HTTP client factory unconditionally sets follow_redirects=True and never validates redirect targets. A compromised or attacker-influenced MCP server can return a 307/308 that bounces JSON-RPC traffic onto loopback/link-local/private hosts (local agents, metadata endpoints), and the client accepts the internal reply as the server's own — the client-side mirror of the server-side DNS-rebinding protection already present in transport_security.py. Introduce RedirectPolicy (NONE/SAME_HOST/SAFE/ALL, default SAFE) and a request event hook that records the caller-chosen origin and blocks redirect hops onto non-global addresses. Legitimate public redirects are still followed; ALL preserves legacy behavior explicitly. streamable_http_client gains a redirect_policy passthrough and warns when a caller-supplied httpx2.AsyncClient skips this protection.
|
This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #3358. If a maintainer assigns you to #3358, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take. You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way. CONTRIBUTING.md has the full reasoning, but in short:
Maintainers: reopen, remove |
Fixes #3358
Problem
create_mcp_http_client(src/mcp/shared/_httpx_utils.py) unconditionally setsfollow_redirects=Trueand never validates where server 3xx redirects land. A compromised or attacker-influenced MCP server can return307/308that bounces the client's JSON-RPC traffic onto loopback / link-local / private hosts (a local agent, a metadata endpoint, a Docker/K8s service proxy). Because the target answers JSON-RPC, the client accepts the internal service's reply as the MCP server's own — SSRF + protocol confusion. This is the client-side mirror of the server-side DNS-rebinding protection already shipped intransport_security.py, and it is currently the only unprotected direction.Fix
Introduce a
RedirectPolicyenum (NONE/SAME_HOST/SAFE/ALL) and a request event hook on the factory. The hook records the caller-chosen origin and blocks later redirect hops that land on non-global addresses (loopback, link-local, private, multicast, reserved, unspecified). Default isSAFE: legitimate public redirects are still followed, bouncing into internal hosts is refused with a clearConnectError.src/mcp/shared/_httpx_utils.py—RedirectPolicy,_is_internal_or_non_global,_make_redirect_guard, wired intocreate_mcp_http_client(redirect_policy=...).src/mcp/client/streamable_http.py—streamable_http_clientgains aredirect_policypassthrough and logs a debug note when a caller-suppliedAsyncClientbypasses this protection.tests/shared/test_httpx_utils.py— unit tests for host classification plus live integration tests proving SAFE blocks an internal redirect, ALL preserves legacy behavior, SAME_HOST still follows same-host redirects, NONE never follows.Verification
tests/client/test_streamable_http.py: 28 passed, no regressions.ruff check+ruff format --check: clean.