diff --git a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py index 21682b4dd3e..cae304bf73a 100644 --- a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py @@ -1321,12 +1321,15 @@ if MCP_AVAILABLE: if isinstance(credentials, dict): mcp_auth_header = credentials.get("auth_value") + # Authorization doubles as the admission fallback (LITELLM_API_KEY_HEADER_NAME_SECONDARY): + # when the primary x-litellm-api-key header is absent, the Authorization value is the + # caller's LiteLLM key, not an upstream token, and must never be forwarded upstream. oauth2_headers: Optional[Dict[str, str]] = None if new_mcp_server_request.auth_type in { MCPAuth.oauth2, MCPAuth.true_passthrough, MCPAuth.oauth_delegate, - }: + } and headers.get(MCPRequestHandler.LITELLM_API_KEY_HEADER_NAME_PRIMARY): oauth2_headers = MCPRequestHandler._get_oauth2_headers_from_headers(headers) async def _list_tools_operation(client): diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py index 090b4711dc9..465cebcc12c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py @@ -37,10 +37,7 @@ def _build_request( body_bytes = body else: body_bytes = b"" - raw_headers = [ - (key.lower().encode("latin-1"), value.encode("latin-1")) - for key, value in headers.items() - ] + raw_headers = [(key.lower().encode("latin-1"), value.encode("latin-1")) for key, value in headers.items()] scope = { "type": "http", "http_version": "1.1", @@ -62,25 +59,18 @@ def _build_request( def _get_route(path: str, method: str): for route in rest_endpoints.router.routes: - if getattr(route, "path", None) == path and method in getattr( - route, "methods", set() - ): + if getattr(route, "path", None) == path and method in getattr(route, "methods", set()): return route raise AssertionError(f"Route {method} {path} not found") def _route_has_dependency(route, dependency) -> bool: - if any( - getattr(dep, "dependency", None) == dependency - for dep in getattr(route, "dependencies", []) - ): + if any(getattr(dep, "dependency", None) == dependency for dep in getattr(route, "dependencies", [])): return True dependant = getattr(route, "dependant", None) if dependant is None: return False - return any( - getattr(dep, "call", None) == dependency for dep in dependant.dependencies - ) + return any(getattr(dep, "call", None) == dependency for dep in dependant.dependencies) class TestExecuteWithMcpClient: @@ -104,9 +94,7 @@ class TestExecuteWithMcpClient: auth_type=MCPAuth.none, ) - result = await rest_endpoints._execute_with_mcp_client( - payload, failing_operation - ) + result = await rest_endpoints._execute_with_mcp_client(payload, failing_operation) assert result["status"] == "error" assert "stack_trace" not in result @@ -267,15 +255,10 @@ class TestExecuteWithMcpClient: assert result["status"] == "ok" # The incoming Authorization must be dropped — extra_headers should # contain no oauth2 headers (only static_headers, which are None here). - assert ( - captured["extra_headers"] is None - or "Authorization" not in captured["extra_headers"] - ) + assert captured["extra_headers"] is None or "Authorization" not in captured["extra_headers"] @pytest.mark.asyncio - async def test_interactive_oauth_resolves_forwarded_token_via_presented_store( - self, monkeypatch - ): + async def test_interactive_oauth_resolves_forwarded_token_via_presented_store(self, monkeypatch): """Interactive authorization_code preview (oauth2, no client credentials): the forwarded just-authorized token is resolved THROUGH the v2 resolver via a one-shot presented store (cred_provider), not the caller-override path. The bare token (Bearer stripped) is the @@ -433,9 +416,7 @@ class TestExecuteWithMcpClient: return None async def fake_create_client(*args, **kwargs): - raise BaseExceptionGroup( - "test group", [RuntimeError("Cancelled via cancel scope")] - ) + raise BaseExceptionGroup("test group", [RuntimeError("Cancelled via cancel scope")]) monkeypatch.setattr( rest_endpoints.global_mcp_server_manager, @@ -497,9 +478,7 @@ class TestTestToolsList: "message": "Successfully retrieved tools", } - monkeypatch.setattr( - rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False - ) + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) oauth_call_counter = {"count": 0} @@ -555,9 +534,7 @@ class TestTestToolsList: "message": "Successfully retrieved tools", } - monkeypatch.setattr( - rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False - ) + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) oauth_headers = {"Authorization": "Bearer oauth"} oauth_call_counter = {"count": 0} @@ -573,7 +550,7 @@ class TestTestToolsList: raising=False, ) - request = _build_request({"authorization": "Bearer incoming"}) + request = _build_request({"authorization": "Bearer incoming", "x-litellm-api-key": "sk-admission"}) payload = NewMCPServerRequest( server_name="example", url="https://example.com", @@ -627,7 +604,7 @@ class TestTestToolsList: raising=False, ) - request = _build_request({"authorization": "Bearer upstream-token"}) + request = _build_request({"authorization": "Bearer upstream-token", "x-litellm-api-key": "sk-admission"}) payload = NewMCPServerRequest( server_name="example", url="https://example.com", @@ -646,6 +623,48 @@ class TestTestToolsList: assert captured["mcp_auth_header"] is None assert captured["oauth2_headers"] == oauth_headers + @pytest.mark.parametrize("auth_type", [MCPAuth.oauth2, MCPAuth.true_passthrough, MCPAuth.oauth_delegate]) + async def test_does_not_forward_authorization_that_satisfied_admission(self, monkeypatch, auth_type): + """Authorization is also the admission fallback: with no x-litellm-api-key on the request, + the Authorization value is the caller's LiteLLM key, so forwarding it would send the + admission credential to the upstream.""" + + captured: dict = {} + + async def fake_execute( + request, + operation, + mcp_auth_header=None, + oauth2_headers=None, + raw_headers=None, + ): + captured["oauth2_headers"] = oauth2_headers + return { + "tools": [], + "error": None, + "message": "Successfully retrieved tools", + } + + monkeypatch.setattr(rest_endpoints, "_execute_with_mcp_client", fake_execute, raising=False) + + request = _build_request({"authorization": "Bearer sk-litellm-admission-key"}) + payload = NewMCPServerRequest( + server_name="example", + url="https://example.com", + auth_type=auth_type, + ) + + from litellm.proxy._types import LitellmUserRoles + + result = await rest_endpoints.test_tools_list( + request, + payload, + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN), + ) + + assert result["message"] == "Successfully retrieved tools" + assert captured["oauth2_headers"] is None + class TestListToolsRestAPI: pytestmark = pytest.mark.asyncio @@ -775,9 +794,7 @@ class TestListToolsRestAPI: stub_server = StubServer() captured = {} - async def fake_get_tools( - server, server_auth_header, *args, apply_tool_filters=True, **kwargs - ): + async def fake_get_tools(server, server_auth_header, *args, apply_tool_filters=True, **kwargs): captured["apply_tool_filters"] = apply_tool_filters return ["tool-1"] @@ -825,9 +842,7 @@ class TestListToolsRestAPI: assert captured["apply_tool_filters"] is True @pytest.mark.parametrize("upstream_status", [401, 403]) - async def test_upstream_auth_failure_surfaces_status_and_challenge( - self, monkeypatch, upstream_status - ): + async def test_upstream_auth_failure_surfaces_status_and_challenge(self, monkeypatch, upstream_status): """A single-server pass-through request whose upstream rejects the token must surface the upstream status (401 or 403) plus its WWW-Authenticate challenge, not collapse into a 200 ``unexpected_error`` body.""" @@ -1415,9 +1430,7 @@ class TestListToolsRestAPI: oauth_headers = {"Authorization": "Bearer user-oauth-token"} - async def fake_get_user_oauth_extra_headers( - server, user_api_key_dict, prefetched_creds=None - ): + async def fake_get_user_oauth_extra_headers(server, user_api_key_dict, prefetched_creds=None): return oauth_headers captured = {} @@ -1661,9 +1674,7 @@ class TestGetToolsForSingleServer: pytestmark = pytest.mark.asyncio - async def test_filters_tools_by_object_permission_mcp_tool_permissions( - self, monkeypatch - ): + async def test_filters_tools_by_object_permission_mcp_tool_permissions(self, monkeypatch): """Test that tools are filtered by user_api_key_auth.object_permission.mcp_tool_permissions""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -1826,9 +1837,7 @@ class TestGetToolsForSingleServer: # All tools should be returned assert len(result) == 2 - async def test_no_filtering_when_server_not_in_mcp_tool_permissions( - self, monkeypatch - ): + async def test_no_filtering_when_server_not_in_mcp_tool_permissions(self, monkeypatch): """Test that all tools are returned when server is not in mcp_tool_permissions""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -1881,9 +1890,7 @@ class TestGetToolsForSingleServer: # All tools should be returned since server is not in permissions assert len(result) == 2 - async def test_combines_server_allowed_tools_and_object_permission_filters( - self, monkeypatch - ): + async def test_combines_server_allowed_tools_and_object_permission_filters(self, monkeypatch): """Test that both server.allowed_tools and object_permission.mcp_tool_permissions filters are applied""" from litellm.proxy._experimental.mcp_server.server import MCPServer from litellm.proxy._types import LiteLLM_ObjectPermissionTable @@ -2201,9 +2208,7 @@ class TestPreviewOpenAPITools: "paths": { "/repos/{owner}/{repo}/actions/jobs/{job_id}/logs": { "get": { - "operationId": ( - "actions/download-job-logs-for-workflow-run" - ), + "operationId": ("actions/download-job-logs-for-workflow-run"), "summary": "Download job logs", } }, @@ -2246,9 +2251,7 @@ class TestPreviewOpenAPITools: names = [t["name"] for t in result["tools"]] anthropic_re = re.compile(r"^[a-zA-Z0-9_-]{1,128}$") for name in names: - assert anthropic_re.match( - name - ), f"preview tool name {name!r} violates ^[a-zA-Z0-9_-]+$" + assert anthropic_re.match(name), f"preview tool name {name!r} violates ^[a-zA-Z0-9_-]+$" assert "actions_download-job-logs-for-workflow-run" in names assert "pulls_list-files" in names @@ -2305,9 +2308,7 @@ class TestPreviewOpenAPITools: registered_summary_to_name: dict = {} - def fake_create_tool_function( - path, method, operation, base_url - ): # noqa: ANN001 + def fake_create_tool_function(path, method, operation, base_url): # noqa: ANN001 def _f(): return None @@ -2320,9 +2321,7 @@ class TestPreviewOpenAPITools: ) class _StubRegistry: - def register_tool( - self, name, description, input_schema, handler - ): # noqa: ANN001 + def register_tool(self, name, description, input_schema, handler): # noqa: ANN001 registered_summary_to_name[description] = name monkeypatch.setattr( @@ -2331,9 +2330,7 @@ class TestPreviewOpenAPITools: _StubRegistry(), ) - openapi_to_mcp_generator.register_tools_from_openapi( - spec, base_url="https://example.invalid" - ) + openapi_to_mcp_generator.register_tools_from_openapi(spec, base_url="https://example.invalid") assert preview_summary_to_name == registered_summary_to_name, ( f"preview {preview_summary_to_name} != " @@ -2361,15 +2358,11 @@ class TestConnectionErrorMessage: assert secret not in message def test_connect_error_points_at_reachability(self): - message = rest_endpoints._connection_error_message( - httpx.ConnectError("All connection attempts failed") - ) + message = rest_endpoints._connection_error_message(httpx.ConnectError("All connection attempts failed")) assert "unreachable" in message.lower() def test_timeout_error_message(self): - message = rest_endpoints._connection_error_message( - httpx.ConnectTimeout("timed out") - ) + message = rest_endpoints._connection_error_message(httpx.ConnectTimeout("timed out")) assert "unreachable" in message.lower() def test_http_status_error_includes_status_code(self):