Repository navigation
Conversation
…igin across redirects
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With
Config.Authset,tool/mcptoolsethands the configured credential to whatever host a redirect names.httpClientinstallsauth.Transport, which applies the credential to every request it sends, redirect hops included. That runs below the layer where net/http stripsAuthorizationon a cross-host redirect, and the client never setsCheckRedirect, so net/http's default policy follows the redirect and the credential goes with it. Anhttpstohttpredirect sends it in cleartext.The
auth/gcpprovider already requires that the client carryingauth.Transportrefuse cross-host redirects, andagent/remoteagent/v2enforces the same forA2AConfig.Auth.modelcontextprotocol/go-sdk#1440 restricts redirects to the same origin in the SDK, but it is not in a release yet, and it steps aside whenever the
http.Clientsets its ownCheckRedirectorMCPGODEBUG=followallredirects=1is set. Either one lets the credential follow a cross-origin redirect again, which is the open question on #1965.Summary
Config.Authset, the toolset's HTTP client refuses a redirect unless it stays on the endpoint's host and port and either keeps the scheme or upgrades http to https. The call fails with an error and the other host is never contacted. A same-origin redirect such as/mcpto/mcp/, which Starlette and FastMCP mounts send, is still followed with the credential.CheckRedirecton a caller-suppliedHTTPClientruns after this check, so it can refuse more redirects but cannot let the credential reach another origin. Without one, the client keeps net/http's limit of 10 requests per chain. Toolsets withoutConfig.Authare unchanged.agent/remoteagent/v2already applies, moved unchanged intointernal/credredirectso both auth paths share one implementation.remoteagent/v2keeps its behavior and error messages.The redirect is refused rather than followed without the credential, for three reasons. It matches
remoteagent/v2and the MCP SDKs (modelcontextprotocol/go-sdk#1440, modelcontextprotocol/python-sdk#3397). Following it would still send the JSON-RPC request body, tool arguments included, to the other host. And an endpoint that requires the credential would answer the unauthenticated request with an error anyway. adk-python is closer to the second option:_RefreshableAsyncCredentialsskips the google-auth token when the redirect's host and port differ from the endpoint's, and httpx strips a staticAuthorizationheader on a cross-origin redirect. Both still follow the redirect.Once a go-sdk release with #1440 is picked up, a toolset with
Config.Authuses this policy instead of the SDK's, because the SDK defers to anyCheckRedirect. The SDK's policy also refuses a redirect that changes the HTTP method or the URL userinfo. Those two checks will not apply withConfig.Authunless they are added here too.Fixes #1965