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 81.39535% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.38%. Comparing base (11cd266) to head (3e034e4).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
...g/transport/proxy/transparent/transparent_proxy.go	81.39%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4454      +/-   ##
==========================================
- Coverage   69.57%   69.38%   -0.19%     
==========================================
  Files         489      501      +12     
  Lines       50193    51385    +1192     
==========================================
+ Hits        34920    35654     +734     
- Misses      12590    12980     +390     
- Partials     2683     2751      +68     

☔ 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.

[amirejaz]

SSRF via open redirect — req.URL.Parse(location) accepts any absolute URL, including http://kubernetes.default.svc/... or http://169.254.169.254/. A compromised MCP server can redirect the proxy to any internal cluster service, and the proxy (with its typically permissive network policy) will dutifully forward the request.

Consider rejecting cross-host redirects:

Suggested change

	break
	redirectURL, parseErr := req.URL.Parse(location)
	if parseErr != nil {
	slog.Warn("failed to parse redirect Location header",
	"location", location, "error", parseErr)
	break
	}
	// Only follow same-host redirects to prevent SSRF. A redirect to a
	// different host indicates the operator should update the target URL.
	if redirectURL.Host != req.URL.Host {
	slog.Warn("refusing cross-host redirect from remote MCP server; update the configured target URL",
	"from_host", req.URL.Host, "to_host", redirectURL.Host)
	break
	}

Note: the if parseErr != nil block that follows this line in the original is being absorbed into the suggestion above — remove the duplicate below.

[amirejaz]

Authorization header forwarded to a different host — req.Clone() copies all headers verbatim, including Authorization. If a cross-host redirect is ever permitted, the bearer token is delivered to the new host. The Go stdlib explicitly strips Authorization on cross-host redirects for this reason (net/http/client.go:shouldCopyHeaderOnRedirect).

If the same-host check suggested above is added, this becomes a defense-in-depth note; if cross-host redirects are intentionally supported in the future, add:

if redirectURL.Host != originalHost {
    req.Header.Del("Authorization")
    req.Header.Del("Cookie")
    req.Header.Del("Proxy-Authorization")
}

[amirejaz]

No test covers a cross-host redirect scenario. It would be valuable to have a test that (a) sets up two separate servers, (b) has server A redirect to server B at a different host, and (c) asserts the expected behavior — either that the 3xx is returned as-is (if the same-host guard is added) or that the Authorization header is stripped (if cross-host is permitted). This makes the security decision explicit and prevents future regressions.

[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.