Skip to content

fix: block redirect header leaks - #936

Merged
DaleSeo merged 1 commit into
mainfrom
fix/redirect-header-leak
Jul 1, 2026
Merged

DaleSeo merged 1 commit into
mainfrom
fix/redirect-header-leak

Conversation

@DaleSeo

@DaleSeo DaleSeo commented Jun 27, 2026 •

Copy link
Copy Markdown
Member

Motivation and Context

This fixes a security issue where the default reqwest-backed streamable HTTP transport could follow redirects while preserving caller-provided custom headers. Disabling automatic redirects keeps secrets such as custom API key headers scoped to the intended MCP server instead of replaying them to an attacker-controlled redirect target.

How Has This Been Tested?

Added a regression test

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Jun 27, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review June 27, 2026 03:51
@DaleSeo
DaleSeo requested a review from a team as a code owner June 27, 2026 03:51
@DaleSeo
DaleSeo merged commit 496902b into main Jul 1, 2026
19 checks passed
@DaleSeo
DaleSeo deleted the fix/redirect-header-leak branch July 1, 2026 20:28
@DaleSeo DaleSeo self-assigned this Jul 10, 2026
gocamille added a commit to gocamille/rust-sdk that referenced this pull request Oct 16, 2026
…treamable HTTP client

The default reqwest client used `Policy::none()`, so any redirect from an
MCP server failed with `UnexpectedServerResponse("HTTP 307: ...")`.

It now follows 307 and 308 when the target has the same origin as the
original request, up to 10 hops. Those keep the method and body, so the
JSON-RPC message and the session, auth and custom headers arrive intact.
301, 302 and 303 are still returned to the caller, because following them
turns the POST into a bodyless GET. Cross-origin redirects are still
returned too, which keeps the header-leak guard.

Refs modelcontextprotocol#936
Reported-by: Mo (@trickyfalcon)
DaleSeo pushed a commit that referenced this pull request Oct 19, 2026
…treamable HTTP client (#1367)

* fix(transport): follow same-origin 307/308 redirects in the default Streamable HTTP client

The default reqwest client used `Policy::none()`, so any redirect from an
MCP server failed with `UnexpectedServerResponse("HTTP 307: ...")`.

It now follows 307 and 308 when the target has the same origin as the
original request, up to 10 hops. Those keep the method and body, so the
JSON-RPC message and the session, auth and custom headers arrive intact.
301, 302 and 303 are still returned to the caller, because following them
turns the POST into a bodyless GET. Cross-origin redirects are still
returned too, which keeps the header-leak guard.

Refs #936
Reported-by: Mo (@trickyfalcon)

* docs(transport): describe redirect handling in default_http_client

Document on `from_uri` and `from_config` which redirects the default
client follows, that a 308 does not change the stored URI, and that a
client passed to `with_client` keeps its own redirect policy.

* docs(transport): move redirect rationale to same_origin_redirect_policy

The rationale explains the policy, so it now sits on
`same_origin_redirect_policy`, and the `default_http_client` doc points
there instead of repeating the `from_uri` section.

* test(transport): drop unused Default derive on CapturedRequest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants