Skip to content

Stop forwarding inbound HTTP headers to unrelated remote servers#3837

Merged
jlowin merged 2 commits intomainfrom
fix/no-implicit-header-forwarding
Apr 11, 2026
Merged

Stop forwarding inbound HTTP headers to unrelated remote servers#3837
jlowin merged 2 commits intomainfrom
fix/no-implicit-header-forwarding

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Apr 11, 2026

StreamableHttpTransport and SSETransport unconditionally call get_http_headers(include={"authorization"}) during connect_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 any Client used 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_headers flag on both transports (default False), and ProxyClient enables it after construction. This keeps header forwarding working for proxies while making standalone clients behave correctly inside tool handlers.

@mcp.tool()
async def my_tool() -> str:
    # This now works — the client won't inherit inbound request headers
    async with Client("http://other-server:8080/mcp") as client:
        result = await client.call_tool("remote_tool", {})
        return result[0].text

Closes #3799

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. security Security fixes: input validation, SSRF/LFI prevention, auth hardening, injection defenses. client Related to the FastMCP client SDK or client-side functionality. http Related to HTTP transport, networking, or web server functionality. labels Apr 11, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +1006 to +1007
if isinstance(self.transport, StreamableHttpTransport | SSETransport):
self.transport.forward_incoming_headers = True
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: 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: TestSupabaseProviderIntegration::test_unauthorized_access in tests/server/auth/providers/test_supabase.py exceeded the 5-second default pytest timeout while the mcp_server_url fixture was trying to start a server subprocess. On Windows, multiprocessing.Process uses the spawn start method (unlike Linux's fork), which requires launching a fresh Python interpreter and reimporting all modules — this is significantly slower and can occasionally exceed 5s. The most recent successful main-branch Windows run completed the same test setup in 2.61 seconds; this run happened to land over the threshold.

Suggested Solution: Add a @pytest.mark.timeout(30) marker to the integration tests in tests/server/auth/providers/test_supabase.py that use the mcp_server_url fixture (or the whole TestSupabaseProviderIntegration class). The 5-second default is too tight for Windows subprocess startup.

@pytest.mark.timeout(30)
class TestSupabaseProviderIntegration:
    async def test_unauthorized_access(self, mcp_server_url: str):
        ...

Other test files that use run_server_in_process on Windows may have the same latent risk.

Detailed Analysis

Only failing job: Tests: Python 3.10 on windows-latest
All other jobs passed (ubuntu, Python 3.13, conformance tests, integration tests).

Relevant log excerpt:

tests\server\auth\providers\test_supabase.py .............+++++ Timeout +++++
  File "tests/server/auth/providers/test_supabase.py", line 180, in mcp_server_url
    with run_server_in_process(run_mcp_server) as url:
  File "src/fastmcp/utilities/tests.py", line 117, in run_server_in_process
    s.connect((host, port))

13 tests pass (all of TestSupabaseProvider), then the fixture for the 14th test (TestSupabaseProviderIntegration::test_unauthorized_access) times out waiting for the spawned server process to start.

Evidence this is not caused by the PR:

  • tests/server/auth/providers/test_supabase.py is not in the PR's changed files
  • src/fastmcp/utilities/tests.py (contains run_server_in_process) is not changed
  • The PR only touches client transport files (http.py, sse.py) and server/providers/proxy.py — none of which are on the server startup path
  • The immediately prior main-branch Windows run (run ID 24285996612) passed this exact test with 2.61s setup time
Related Files
  • tests/server/auth/providers/test_supabase.py:178mcp_server_url fixture (uses run_server_in_process)
  • tests/server/auth/providers/test_supabase.py:196TestSupabaseProviderIntegration (the failing class)
  • src/fastmcp/utilities/tests.py:75run_server_in_process (polls socket for server readiness, hits the 5s pytest timeout on slow Windows runners)

@jlowin jlowin merged commit 95102c7 into main Apr 11, 2026
7 of 8 checks passed
@jlowin jlowin deleted the fix/no-implicit-header-forwarding branch April 11, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality. http Related to HTTP transport, networking, or web server functionality. security Security fixes: input validation, SSRF/LFI prevention, auth hardening, injection defenses.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise an error when calling the external mcp tool in the server tool

1 participant