fix(mcp): forward configured headers to OAuth discovery on the server host#3159
Conversation
… host
Remote MCP `headers` were applied only to the main channel, never to the OAuth flow's discovery/token requests. Servers like Grafana Cloud need a routing header (X-Grafana-URL) on the protected-resource-metadata request to scope the OAuth flow to the right instance; without it the auth screen re-prompts for the instance.
The OAuth HTTP client now forwards configured headers, but only to requests targeting the MCP server's own host, so credentials are never leaked to a third-party authorization server advertised in (untrusted) server metadata. SSRF safety, allow_private_ips, timeout and ${headers.NAME} placeholders are preserved; the shared client singleton is never mutated.
Fixes #3148
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One low-probability finding in newly added code: the host-scoping check may silently drop configured headers when the MCP server URL includes an explicit standard port (e.g., :443). The fix logic and security model (no credential leakage to third-party auth servers) are sound. All other security properties — SSRF protection, timeout, singleton non-mutation — are preserved correctly.
url.Parse keeps an explicit standard port verbatim, so a config URL like https://host:443/mcp stored host "host:443" while servers usually advertise port-less discovery URLs (host). The host comparison then missed and headers were silently dropped, reproducing #3148 for users who spell out :443/:80. Normalize both sides by stripping the scheme's default port.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The host-scoping mechanism (hostScopedHeaderTransport) correctly forwards configured MCP server headers only to the MCP server's own host, preventing credential leakage to third-party authorization servers. The core security property is sound and well-tested.
Minor observations (no action required):
-
oauthHTTPClientWithHeadersomitsJarfrom base client (oauth_helpers.go:82) — The newly constructed*http.Clientdoes not copybase.Jar. Today all base clients have a nilJar, so this is harmless. If a cookie jar is ever attached to the base OAuth client in the future, it would silently disappear in the wrapped client. -
req.URL.Hostmay be empty on redirect requests (oauth_helpers.go:101) — Go's HTTP client may reissue requests withRequest.URL.Host == ""(host inRequest.Host) on redirects. In that casehostWithoutDefaultPortreturns"", the host check fails, and headers are silently dropped even if the redirect target is the MCP server itself. This only triggers on same-host redirects and is not the primary OAuth discovery path.
Both are low-risk edge cases that don't affect the primary use case described in this PR.
…covery scoped to server host
Fixes #3148
Problem
A remote MCP server's
headerswere applied only to the main MCP channel, never to the OAuth flow. Grafana Cloud needsX-Grafana-URLon the OAuth discovery request to scope the flow to the right instance; without it the auth screen keeps prompting for the instance.Root cause
oauthTransport.base(header transport)oauthHTTPClient(bare singleton)The
protected-resource-metadatarequest (the one Grafana uses to scope the instance) went out with none of the configured headers.Fix
Authorization/ API keys can't leak.allow_private_ips, timeout, and${headers.NAME}placeholders. The shared client singleton is never mutated.Tests
...ScopesHeadersToMCPHost...NoHeadersReusesBaseClientTestInitialize_CustomHeadersReachOAuthDiscoveryInitialize()path; reverting the fix makes it fail (reproduces #3148)Validation:
go build ./...,golangci-lint run ./pkg/tools/mcp/...(0 issues),go test -race ./pkg/tools/mcp/all pass.Caveat
Scoping is on the exact host of the configured URL. If a server ever required the header on a host different from its MCP endpoint, the scope would need widening. Started strict, for safety.