Keep following a relative redirect when the endpoint URL carries userinfo

[maxisbey]

Follow-up to #3397: an endpoint URL that carries userinfo (http://user:pass@host/mcp, which httpx2 turns into Basic auth) could no longer follow a redirect with a relative Location, because URL join keeps the base's userinfo and the transport refused any next request whose URL had userinfo. The check now refuses only userinfo the redirect itself introduces.

Motivation and Context

#3397 stops the transports from following a Location that carries user:pass@, since re-sending would put that pair in an Authorization header. The check was broader than its purpose: for a relative Location (/mcp/), next_request.url inherits the authority of the URL the caller configured, userinfo included, so the ordinary same-origin trailing-slash redirect was handed back unfollowed for such an endpoint and the call failed with "Redirect to … not followed". An absolute Location was unaffected. Found in review of the v1.x backport (#3448), which carries the same change.

How Has This Been Tested?

New helper test: a 307 with Location: /mcp/ for http://user:secret@mcp.example/mcp is followed and the second request goes to http://user:secret@mcp.example/mcp/; it fails before the change. The existing test that a Location introducing userinfo is not followed still passes. Full suite, pyright and ruff pass.

Breaking Changes

None.

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 am assigned to the linked issue (or it is labeled help wanted, or I'm a maintainer)
• I have disclosed any AI assistance and can explain the change in my own words
• 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

_{AI Disclaimer}

[claude]

I reviewed this PR and found no bugs, but since it loosens a credential-related redirect guard, a quick human confirmation of the new condition is still worthwhile.

What was reviewed:

• The new condition in next_request_within_origin (src/mcp/shared/_httpx_utils.py:104): a redirect is now refused only when the next request's userinfo is non-empty and differs from the just-sent request's — checked that every attacker-introduced-userinfo case (none configured, or a different pair in an absolute Location) still fails closed, including via RedirectAwareAuth.
• The newly-followed cases: relative Location inheriting the configured userinfo, and an absolute Location echoing the identical userinfo — both are same-origin (enforced by _within_origin) to a server that already received that credential, so no new exposure; the http-to-https upgrade branch only ever moves the same credential to TLS on the same host.
• Byte-exact userinfo comparison means percent-encoding or case variants mismatch and are refused, which fails closed.
• The new test (tests/shared/test_httpx_utils.py:211) and the existing introduced-userinfo test together pin both sides of the boundary.

Extended reasoning...

Overview

The PR changes one condition in next_request_within_origin in src/mcp/shared/_httpx_utils.py, a private module (underscore-prefixed, not exported via __all__ in src/mcp/__init__.py). Previously any redirect whose next-request URL carried userinfo was refused; now it is refused only when that userinfo is non-empty and differs from the userinfo of the request just sent. This fixes the regression from the earlier hardening: an endpoint URL configured with userinfo could not follow even an ordinary same-origin relative redirect (e.g. a 307 trailing-slash normalisation), because URL join preserves the base authority and the guard saw userinfo on the next request. The docstring is updated and a new test pins the fixed behaviour.

Security risks

This is the security-relevant part of the redirect policy — it exists to stop a server-controlled Location from injecting a user:pass@ pair that httpx2 would send as Basic auth. I walked the cases: (1) no userinfo configured, Location introduces some — next userinfo is non-empty and differs from empty, refused (existing test test_redirect_location_with_userinfo_is_not_followed still covers this); (2) userinfo configured, relative Location — same userinfo, followed, which is the fix; (3) userinfo configured, absolute same-origin Location echoing the byte-identical userinfo — now followed, but the credential goes to the same origin that already received it in the first request, so nothing new is exposed; (4) userinfo configured, Location with a different pair — refused; (5) the http-to-https same-host upgrade branch of _within_origin can only carry the caller's own credential to TLS on the same host, which is strictly safer than the http request that preceded it. The comparison is byte-exact on httpx2.URL.userinfo, so percent-encoding or case variants of the "same" credential mismatch and the redirect is refused — the guard fails closed on ambiguity. redirect_location still strips userinfo before anything reaches an error message or log line. The same helper drives RedirectAwareAuth for OAuth-flow requests; the identical reasoning applies there.

Level of scrutiny

High, despite the tiny diff: this is a deliberate loosening of a check whose whole purpose is preventing credential leakage on redirects. I read the full module rather than just the diff to check every consumer of next_request_within_origin (stream_within_origin, request_within_origin, sse_within_origin, RedirectAwareAuth.async_auth_flow) and confirmed the origin check is always applied alongside the userinfo check, so no followed redirect can leave the configured origin regardless of userinfo. Because the guidelines treat credential-handling paths as ones a human should still look at, I am deferring rather than approving, even though my own analysis found no way for the loosened condition to leak a credential to a party that does not already hold it.

Other factors

The test suite around this helper is thorough: parametrised within-origin and outside-origin cases, redirect-limit behaviour, and now both sides of the userinfo boundary (introduced userinfo refused, inherited userinfo followed). The new test follows the repo's test conventions (plain top-level test_* function, provenance-tagged docstring, in-memory MockTransport, no sleeps). The bug hunt exited on dry_streak with zero findings and zero ruled-out candidates. No docs update is needed — this restores the behaviour users of a userinfo-bearing endpoint URL would already expect, rather than changing documented behaviour. The PR description matches the code exactly.

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/shared/_httpx_utils.py">

<violation number="1" location="src/mcp/shared/_httpx_utils.py:104">
P2: When the endpoint already has userinfo, an absolute `Location` can repeat those credentials and bypass this check because the resolved userinfo is unchanged. Inspect the raw `Location` URL and reject authority userinfo supplied there, while still allowing relative locations to inherit the configured userinfo.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>