Follow HTTP redirects in transparent proxy

[gkatz2]

Summary

• MCP clients cannot handle HTTP redirect responses (they expect JSON-RPC), so when a remote server redirects, the client silently fails. The transparent proxy now follows redirects in its forward method before returning the response to the client.
• Adds an isRedirectStatus helper that handles 301, 302, 307, and 308. The HTTP method and body are preserved across redirects via req.Clone(). A maxRedirects limit of 10 prevents infinite loops.
• Logs at WARN level on each redirect to nudge operators toward fixing the server URL.

Fixes #4453

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Tested with a local HTTP server returning 308 redirects to a running MCP server (design-system-mcp). Verified:

• initialize call succeeds through the redirect
• Proxy logs show the WARN message with from/to URLs and redirect count

Does this introduce a user-facing change?

Remote MCP servers behind HTTP redirects now work. Previously, the proxy passed redirect responses to the client, causing silent failures. No configuration needed — redirect following is automatic.

Special notes for reviewers

• The redirect logic lives in forward rather than RoundTrip so that session tracking, 401 handling, and DELETE cleanup in RoundTrip only see the final response after all redirects are resolved.
• 303 (See Other) is excluded from isRedirectStatus because it explicitly requires changing the method to GET, which would break JSON-RPC.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.88%. Comparing base (a7f347f) to head (2679b22).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...g/transport/proxy/transparent/transparent_proxy.go	87.50%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4454      +/-   ##
==========================================
+ Coverage   68.82%   68.88%   +0.05%     
==========================================
  Files         516      516              
  Lines       54153    54199      +46     
==========================================
+ Hits        37272    37336      +64     
+ Misses      14018    14000      -18     
  Partials     2863     2863              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[amirejaz]

The redirect-following logic is well-structured and the test coverage is thorough. Two high-severity security issues need to be addressed before merging, plus a few smaller observations.

Security — must fix

1. SSRF via open redirect (High)
The forward loop follows any Location header without validating whether it points to the same host as the original target. In a Kubernetes deployment a compromised or malicious MCP server can return a redirect to http://kubernetes.default.svc/api/v1/secrets, http://169.254.169.254/ (IMDS), or any reachable pod/service. The proxy's pod SA typically has broad network access by design, so the blast radius is significant.

Mitigation: compare redirectURL.Host (normalized) against the original req.URL.Host and break (returning the 3xx as-is) if they differ. Operators who intentionally rename a server's URL should just update the configured target, not rely on redirect following.

2. Authorization header forwarded to a different host (High)
The comment explicitly acknowledges this (// request headers (including Authorization) are preserved across redirects). Combined with the SSRF issue, a bearer token is silently exfiltrated to whatever host the attacker redirects to. The Go stdlib's own http.Client strips Authorization on cross-host redirects precisely to prevent this. See net/http/client.go:shouldCopyHeaderOnRedirect.

Mitigation: either block cross-host redirects (see above) or, if cross-host is ever needed, strip Authorization, Cookie, and Proxy-Authorization when redirectURL.Host != originalHost.

Correctness — looks good

• Body replay via bytes.NewReader(body) is correct; body is pre-read by readRequestBody before the session guard, so the original req.Body stream is intact on the first hop.
• Connection pool hygiene (drain + close on each hop) is correct.
• req.Host = redirectURL.Host correctly mirrors the existing host-rewrite pattern in RoundTrip.
• Interaction with post-forward logic (401 handling, DELETE cleanup, session tracking) is correct — all of it only sees the final response.
• 303 exclusion is correct and well-documented.

Minor

• maxRedirects = 10 matches http.Client but for a proxy that is already warning operators on every hop, 3 would be more conservative and sufficient for the intended use case (HTTP→HTTPS redirects, single-hop canonical URL normalisation).
• No test exercises a cross-host redirect. Adding one (even if it just asserts the current behavior) makes the security decision explicit and would catch a future regression if a host-check is added.

[gkatz2]

Thanks for the detailed review! Both security issues have been addressed in the follow-up commit 3e034e44.

SSRF / cross-host redirect (both issues addressed together): The forward() loop now extracts the original host before following redirects and compares each redirect destination's host against it. If the redirect target is a different host, we break and return the 3xx response as-is — the client decides what to do. This prevents both SSRF and auth header leakage to a different host in one check.

E2E failure: The failing test (should successfully delete running workload) is in api_workloads_test.go and exercises the workload deletion API — it has no interaction with the redirect-following code path. The failure appears to be a timing issue where Docker took longer than 60s to clean up the container on the CI runner. Happy to re-trigger if that would help.

[jhrozek]

First, sorry to leave you waiting, @amirejaz is on PTO this week and we didn't hand off properly. We should have done that!

There are some merging conflicts so I can't approve, those need to be resolved first. I think the PR looks good overall, I left two nits/observations inline.

[jhrozek]

@gkatz2 I'm sorry there's another conflict (I think this time with your own PR to add the timeout). If you rebase I'll re-ack right away.

[gkatz2]

@jhrozek, I resolved the conflict. Please take another look.

dismissing since Amir is on PTO