Stop forwarding inbound HTTP headers to unrelated remote servers#3837
Stop forwarding inbound HTTP headers to unrelated remote servers#3837
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82c393776a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(self.transport, StreamableHttpTransport | SSETransport): | ||
| self.transport.forward_incoming_headers = True |
There was a problem hiding this comment.
Enable forwarding for proxies built from plain Client
This change enables the new header-forwarding flag only when the backend client is a ProxyClient, but create_proxy() also accepts a regular Client and _create_client_factory returns client.new() for that path. After this commit, those proxies run with forward_incoming_headers=False, so a documented flow like create_proxy(Client("https://...")) (including the CLI URL proxy path) no longer propagates inbound Authorization headers to upstream MCP servers and will start returning auth failures for per-caller protected backends.
Useful? React with 👍 / 👎.
Test Failure AnalysisSummary: A single test timed out on the Windows Python 3.10 runner during fixture setup — not in the test body itself. This failure is unrelated to the PR's changes. Root Cause: Suggested Solution: Add a @pytest.mark.timeout(30)
class TestSupabaseProviderIntegration:
async def test_unauthorized_access(self, mcp_server_url: str):
...Other test files that use Detailed AnalysisOnly failing job: Relevant log excerpt: 13 tests pass (all of Evidence this is not caused by the PR:
Related Files
|
StreamableHttpTransportandSSETransportunconditionally callget_http_headers(include={"authorization"})duringconnect_session, which reads headers from the active server request context. This was designed for the proxy pattern, where auth headers need to flow through to the upstream server — but it fires for anyClientused inside a server tool handler. If a tool calls out to an unrelated MCP server, the inbound caller's authorization header leaks into the outbound request, causing 400s or worse.The fix adds a
forward_incoming_headersflag on both transports (defaultFalse), andProxyClientenables it after construction. This keeps header forwarding working for proxies while making standalone clients behave correctly inside tool handlers.Closes #3799