[dan1d]

Linked issue

• Closes: tool/mcptoolset: credential forwarded across cross-host redirect — httpClient never sets CheckRedirect (contradicts auth/gcp provider contract) #1965

Describe the change

Problem:

tool/mcptoolset's httpClient installs auth.Transport when Config.Auth is set, but never
sets CheckRedirect. auth.Transport.RoundTrip resolves and applies the credential on every
request it handles — below the layer where net/http strips Authorization on a cross-host
redirect — so a redirect from the configured MCP endpoint carries the freshly-applied credential
to whatever host the Location header names. This contradicts the transport requirement the
repo itself documents in auth/gcp/provider.go ("The transport must not follow a cross-host
redirect… Set CheckRedirect on the http.Client that carries the transport") and already
implements for the ADC-backed client in auth/gcp/client.go (http.ErrUseLastResponse).

Security-relevant when the configured server is honest but the redirect is not under its
control: edge/CDN rewrite rules, header injection that sets Location, domain lapse after
configuration, and https → http downgrades (net/http follows them; the bearer token transits
in cleartext). The failure is silent — no error, no log, no operator signal.

Solution:

Pin redirects in httpClient (http.ErrUseLastResponse) at the step that wraps every
streamable/SSE transport copy. Placing the guard there — rather than only on the Config.Auth
path — also covers user-supplied credential-bearing HTTPClients (e.g. the documented
oauth2.NewClient wiring in examples/mcp/main.go), whose oauth2.Transport re-signs every
hop the same way. An operator-provided CheckRedirect still wins (nil-guard), so the change
only removes a footgun, never a capability someone configured.

Alternatives considered: a same-origin-allowing guard (permits in-origin redirects like
trailing-slash normalization), and an allowed-host on auth.Transport matching adk-python's
_RefreshableAsyncCredentials behavior (skip injection on a foreign host instead of refusing
the redirect). The ErrUseLastResponse variant is the minimal, repo-consistent one; happy to
switch if maintainers prefer either alternative.

Behavior change

Yes, for affected configurations: a toolset whose MCP endpoint answers a redirect now surfaces
the redirect response (e.g. 302 Found) as an error instead of silently following it. Servers
that do not redirect behave identically. Users who intentionally rely on redirect-following can
restore it by setting their own CheckRedirect on a custom HTTPClient.

Testing Plan

• New regression test redirect_regression_test.go drives the public API (mcptoolset.New +
  Tools) with a built-in control: serverA (localhost) 302s to serverB (127.0.0.1) — a
  genuinely cross-host hop, both loopback. A plain http.DefaultClient control proves net/http
  strips Authorization on this exact hop; the toolset path is then exercised on the same hop.
  The test fails without the fix (serverB receives the configured bearer token, which the
  control shows net/http itself never forwards) and passes with it; it also asserts serverA
  was contacted so a pass cannot be vacuous.
• go test ./tool/mcptoolset/ — full package suite green with the fix.
• go vet ./tool/mcptoolset/ — clean.

Fail-without-fix output:

--- FAIL: TestCredentialNotForwardedOnCrossHostRedirect (0.01s)
    the redirect target received the credential as "Bearer SECRET_TOKEN_MUST_NOT_LEAK"
    while the control proved net/http strips it on this hop

Pass-with-fix:

--- PASS: TestCredentialNotForwardedOnCrossHostRedirect (0.00s)
ok  	google.golang.org/adk/v2/tool/mcptoolset

[indurireddy-TF]

Hi @dan1d I verified the changes locally, and everything works as expected. The issue is resolved, and the changes look good to me. Thanks for the fix!

@baptmont Could you please take a look at the changes?

[baptmont]

Thanks for the report and the regression test. The leak is real, as @indurireddy-TF confirmed. We're closing this PR in favor of picking up the fix from the MCP Go SDK, for two reasons.

Refusing every redirect is stricter than needed. It also breaks same-origin redirects that work today, most commonly the 307 from /mcp to /mcp/ that Starlette/FastMCP mounts send. With this change, initialize against such a server fails with Temporary Redirect, with or without Config.Auth.

go-sdk has already fixed this. modelcontextprotocol/go-sdk#1440 (for modelcontextprotocol/go-sdk#1429), merged on Oct 23, makes StreamableClientTransport and SSEClientTransport follow a redirect only when it stays within the request's origin (http→https on the same host is allowed) and keeps the request method, unless the client sets its own CheckRedirect. The Python (modelcontextprotocol/python-sdk#3397), TypeScript and Rust MCP SDKs made the same change, and that SDK policy is also what adk-python relies on; it adds no redirect policy of its own for MCP. I ran adk-go main against go-sdk at that commit with no ADK changes: your regression test passes, a same-origin redirect with Config.Auth is followed, and a cross-origin one is refused before the target is contacted.

go-sdk leaves the decision to any CheckRedirect already set on the client, so merging this would also permanently turn off that same-origin following once we upgrade.

We'll bump go-sdk once a release containing #1440 ships and add a regression test along the lines of yours. That work, and one open question about Config.Auth, is tracked in #1965.