From ec3c67084fb6fabd1122fec5bc79b0b3442be2b0 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 21 May 2026 17:46:51 +0000 Subject: [PATCH] security(mcp): strip Authorization in call_tool when LiteLLM admission used legacy header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the OAuth pass-through admission check from _prepare_mcp_server_headers (list-tools path) in _call_regular_mcp_tool (tool-call path): when the server is OAuth pass-through and the caller did not supply x-litellm-api-key, Authorization on the inbound request may itself be the LiteLLM API key — so strip it before forwarding instead of leaking the gateway credential upstream. When x-litellm-api-key is present, admission is unambiguous and Authorization continues to carry the upstream OAuth bearer (transparent pass-through). --- .../mcp_server/mcp_server_manager.py | 27 ++- .../mcp_server/test_mcp_server_manager.py | 161 ++++++++++++++++++ 2 files changed, 183 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 e33d15e7cfd..53ca189905e 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -2762,6 +2762,7 @@ class MCPServerManager: proxy_logging_obj: Optional[ProxyLogging], host_progress_callback: Optional[Callable] = None, hook_extra_headers: Optional[Dict[str, str]] = None, + user_api_key_auth: Optional[UserAPIKeyAuth] = None, ) -> CallToolResult: """ Call a regular MCP tool using the MCP client. @@ -2828,14 +2829,29 @@ class MCPServerManager: normalized_raw_headers = { str(k).lower(): v for k, v in raw_headers.items() if isinstance(k, str) } + has_explicit_litellm_admission_header = ( + normalized_raw_headers.get("x-litellm-api-key") is not None + ) + admission_consumed_authorization_as_litellm_key = ( + user_api_key_auth is not None + and bool(getattr(user_api_key_auth, "api_key", None)) + and not has_explicit_litellm_admission_header + ) + for header in mcp_server.extra_headers: if not isinstance(header, str): continue - if ( - mcp_server.has_client_credentials - and header.lower() == "authorization" - ): - continue + if header.lower() == "authorization": + if mcp_server.has_client_credentials: + continue + if mcp_server.is_oauth_passthrough and ( + admission_consumed_authorization_as_litellm_key + or ( + user_api_key_auth is None + and not has_explicit_litellm_admission_header + ) + ): + continue header_value = normalized_raw_headers.get(header.lower()) if header_value is None: continue @@ -3126,6 +3142,7 @@ class MCPServerManager: proxy_logging_obj=proxy_logging_obj, host_progress_callback=host_progress_callback, hook_extra_headers=hook_result.get("extra_headers"), + user_api_key_auth=user_api_key_auth, ) return await self._gather_openapi_tool_tasks(tasks, proxy_logging_obj) 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 d7078412a44..bbf0f7df7bb 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 @@ -480,6 +480,167 @@ class TestMCPServerManager: assert captured_extra_headers == {"Authorization": "Bearer token"} assert isinstance(result, CallToolResult) + @pytest.mark.asyncio + async def test_call_regular_mcp_tool_passthrough_strips_authorization_when_admission_consumed_litellm_key( + self, + ): + """OAuth pass-through must not forward the caller's Authorization to upstream + when LiteLLM admission consumed the bearer as its API key — otherwise the + LiteLLM key the caller used for admission would leak upstream.""" + from litellm.proxy._types import UserAPIKeyAuth + + manager = MCPServerManager() + server = MCPServer( + server_id="server-passthrough-call", + name="passthrough-server", + url="https://example.com", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + extra_headers=["Authorization", "x-request-id"], + delegate_auth_to_upstream=True, + ) + + mock_client = AsyncMock() + mock_client.call_tool = AsyncMock( + return_value=CallToolResult(content=[], isError=False) + ) + captured_extra_headers = None + + async def capture_create_mcp_client( + server, mcp_auth_header, extra_headers, stdio_env, subject_token=None + ): # pragma: no cover - helper + nonlocal captured_extra_headers + captured_extra_headers = extra_headers + return mock_client + + manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client) + + await manager._call_regular_mcp_tool( + mcp_server=server, + original_tool_name="tool", + arguments={}, + tasks=[], + mcp_auth_header=None, + mcp_server_auth_headers=None, + oauth2_headers=None, + raw_headers={ + "authorization": "Bearer sk-litellm-key", + "x-request-id": "req-123", + }, + proxy_logging_obj=None, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-key"), + ) + + assert captured_extra_headers == {"x-request-id": "req-123"} + + @pytest.mark.asyncio + async def test_call_regular_mcp_tool_passthrough_forwards_authorization_with_admission_header( + self, + ): + """OAuth pass-through forwards Authorization upstream when x-litellm-api-key + provides admission — in that case Authorization carries the upstream OAuth + bearer, not the LiteLLM key.""" + from litellm.proxy._types import UserAPIKeyAuth + + manager = MCPServerManager() + server = MCPServer( + server_id="server-passthrough-call-admission", + name="passthrough-server", + url="https://example.com", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + extra_headers=["Authorization"], + delegate_auth_to_upstream=True, + ) + + mock_client = AsyncMock() + mock_client.call_tool = AsyncMock( + return_value=CallToolResult(content=[], isError=False) + ) + captured_extra_headers = None + + async def capture_create_mcp_client( + server, mcp_auth_header, extra_headers, stdio_env, subject_token=None + ): # pragma: no cover - helper + nonlocal captured_extra_headers + captured_extra_headers = extra_headers + return mock_client + + manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client) + + await manager._call_regular_mcp_tool( + mcp_server=server, + original_tool_name="tool", + arguments={}, + tasks=[], + mcp_auth_header=None, + mcp_server_auth_headers=None, + oauth2_headers=None, + raw_headers={ + "x-litellm-api-key": "Bearer sk-litellm-key", + "authorization": "Bearer upstream-oauth-bearer", + }, + proxy_logging_obj=None, + user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-key"), + ) + + assert captured_extra_headers == { + "Authorization": "Bearer upstream-oauth-bearer" + } + + @pytest.mark.asyncio + async def test_call_regular_mcp_tool_passthrough_forwards_authorization_for_anonymous_admission( + self, + ): + """OAuth pass-through cold-start return (RFC 9728): the caller's only + credential is the upstream bearer in Authorization, and LiteLLM admission + is anonymous (no api_key on user_api_key_auth). Authorization must be + forwarded so the delegated flow can complete.""" + from litellm.proxy._types import UserAPIKeyAuth + + manager = MCPServerManager() + server = MCPServer( + server_id="server-passthrough-call-anon", + name="passthrough-server", + url="https://example.com", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + extra_headers=["Authorization"], + delegate_auth_to_upstream=True, + ) + + mock_client = AsyncMock() + mock_client.call_tool = AsyncMock( + return_value=CallToolResult(content=[], isError=False) + ) + captured_extra_headers = None + + async def capture_create_mcp_client( + server, mcp_auth_header, extra_headers, stdio_env, subject_token=None + ): # pragma: no cover - helper + nonlocal captured_extra_headers + captured_extra_headers = extra_headers + return mock_client + + manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client) + + await manager._call_regular_mcp_tool( + mcp_server=server, + original_tool_name="tool", + arguments={}, + tasks=[], + mcp_auth_header=None, + mcp_server_auth_headers=None, + oauth2_headers=None, + raw_headers={"authorization": "Bearer upstream-oauth-bearer"}, + proxy_logging_obj=None, + user_api_key_auth=UserAPIKeyAuth(api_key=None), + ) + + assert captured_extra_headers == { + "Authorization": "Bearer upstream-oauth-bearer" + } + @pytest.mark.asyncio async def test_get_prompts_from_server_success(self): """Ensure prompts are fetched and prefixed when requested."""