From 8da32a51a78a8c42e451a2933f9a059fdfa95906 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 20 May 2026 19:12:19 +0000 Subject: [PATCH] fix(mcp): skip JWT injection when per-user mcp_auth_header is set MCPClient._get_auth_headers() applies extra_headers AFTER writing Authorization from auth_value, so an injected JWT silently overwrites the user's per-server OAuth token. Guard the JWT signer with 'not mcp_auth_header' so per-user OAuth (and any dict-form per-user auth) takes precedence, mirroring the existing static_headers guard. Adds a regression test that the signer's inject helper is not called when mcp_auth_header is supplied. --- .../mcp_server/mcp_server_manager.py | 17 +++++-- .../mcp_server/test_mcp_server_manager.py | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 6eb76ea3f64..a1ef47909db 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -1436,10 +1436,13 @@ class MCPServerManager: # MCPJWTSigner: inject signed JWT for tools/list (list path skips pre_call_hook). # Skip entirely when the signer is not configured (avoid an unnecessary - # dict copy on every list call) and when the server has its own - # static Authorization header — admin-configured static auth must - # take precedence per-server so the signer doesn't silently - # overwrite e.g. an upstream API key. + # dict copy on every list call), when the server has its own static + # Authorization header, or when a per-user mcp_auth_header has already + # been resolved — admin-configured static auth and per-user OAuth must + # take precedence so the signer doesn't silently overwrite e.g. an + # upstream API key or a user's OAuth token (MCPClient._get_auth_headers + # applies extra_headers after writing Authorization from auth_value, so + # an injected JWT would otherwise clobber the per-user token). if user_api_key_auth is not None and not server.spec_path: from litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer import ( get_mcp_jwt_signer, @@ -1452,7 +1455,11 @@ class MCPServerManager: for k in static_headers.keys() ) - if get_mcp_jwt_signer() is not None and not has_static_authorization: + if ( + get_mcp_jwt_signer() is not None + and not has_static_authorization + and not mcp_auth_header + ): extra_headers = await inject_mcp_jwt_headers_for_upstream( user_api_key_dict=user_api_key_auth, extra_headers=extra_headers, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 7125f9729fe..3a9ba1388b7 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -1805,6 +1805,53 @@ class TestMCPServerManager: assert len(tools_unprefixed) == 1 assert tools_unprefixed[0].name == "send_email" + @pytest.mark.asyncio + async def test_get_tools_from_server_jwt_skipped_when_mcp_auth_header_set(self): + """When a per-user mcp_auth_header is resolved, JWT injection must be skipped. + + MCPClient._get_auth_headers() applies extra_headers AFTER writing + Authorization from auth_value, so an injected JWT would clobber the + user's per-server OAuth token. Regression test for that interaction. + """ + from litellm.proxy._types import UserAPIKeyAuth + + manager = MCPServerManager() + server = MCPServer( + server_id="zapier", + name="zapier", + transport=MCPTransport.http, + ) + + manager._create_mcp_client = AsyncMock(return_value=object()) + manager._fetch_tools_with_timeout = AsyncMock(return_value=[]) + + user_auth = UserAPIKeyAuth(api_key="sk-test", user_id="alice") + + with ( + patch( + "litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer.get_mcp_jwt_signer", + return_value=MagicMock(), + ), + patch( + "litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer.inject_mcp_jwt_headers_for_upstream", + new=AsyncMock(return_value={"Authorization": "Bearer signed-jwt"}), + ) as mock_inject, + ): + # Case A: mcp_auth_header present -> JWT must NOT be injected + await manager._get_tools_from_server( + server, + mcp_auth_header="oauth-user-token", + user_api_key_auth=user_auth, + ) + mock_inject.assert_not_called() + + # Case B: no mcp_auth_header -> JWT injection runs as before + await manager._get_tools_from_server( + server, + user_api_key_auth=user_auth, + ) + mock_inject.assert_awaited_once() + def test_create_prefixed_tools_updates_mapping_for_both_forms(self): """_create_prefixed_tools should populate mapping for prefixed and original names even when not adding prefix in output.""" manager = MCPServerManager()