From 1d73f1bac5e852d32b54e2fe732d1dc2f8bc03b2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 26 May 2026 19:21:05 +0000 Subject: [PATCH] fix(mcp): reconcile cold-start bypass with x-mcp-servers header and skip non-absolute WWW-Authenticate fabrication - _parse_mcp_server_names_from_path now fails closed when the x-mcp-servers header introduces any target not present in the path-derived target set, closing a header/path mismatch where the cold-start passthrough bypass could otherwise admit anonymously while the header advertises a non-passthrough server. - MCPUpstreamAuthError.to_http_exception no longer emits a relative resource_metadata URI when base_url is missing; per RFC 9728 3.2 the URI must be absolute, so we skip fabrication entirely rather than send a challenge strict MCP clients will reject. Co-authored-by: Yassin Kortam --- .../mcp_server/auth/user_api_key_auth_mcp.py | 27 ++++++++++++++++--- .../_experimental/mcp_server/exceptions.py | 11 ++++---- .../test_mcp_oauth_passthrough_tools.py | 9 ++++--- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index 0c3c6e0acd3..c2032e1a9f1 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -18,13 +18,22 @@ from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.auth.ip_address_utils import IPAddressUtils -def _parse_mcp_server_names_from_path(path: str) -> Optional[List[str]]: +def _parse_mcp_server_names_from_path( + path: str, mcp_servers_header: Optional[List[str]] = None +) -> Optional[List[str]]: """Resolve the single MCP server name a cold-start passthrough bypass may target. Delegates parsing to :meth:`MCPRequestHandler._extract_target_server_names_from_path` so the names used here always match the names downstream routing uses; returns ``None`` whenever the bypass must not activate (aggregate ``/mcp``, - multi-server CSV paths, or any other unrecognized path).""" + multi-server CSV paths, or any other unrecognized path). + + Also fails closed when the ``x-mcp-servers`` header introduces any server + not present in the path-derived target set. Downstream routing for + ``/mcp/...`` paths overrides the header with path-derived names, but a + header/path mismatch here is a sign of a confused or hostile caller — + refuse the cold-start bypass rather than admit anonymously based on the + path while the header advertises a stricter, non-passthrough target.""" servers = MCPRequestHandler._extract_target_server_names_from_path(path) if len(servers) != 1: verbose_logger.debug( @@ -34,6 +43,14 @@ def _parse_mcp_server_names_from_path(path: str) -> Optional[List[str]]: servers, ) return None + if mcp_servers_header is not None and (set(mcp_servers_header) - set(servers)): + verbose_logger.debug( + "MCP cold-start: x-mcp-servers header %r introduces target(s) not " + "in path-derived set %r; passthrough 401 bypass will not activate", + mcp_servers_header, + servers, + ) + return None return servers @@ -268,7 +285,7 @@ class MCPRequestHandler: # budget / rate limited) and must propagate so those # controls are not bypassed via anonymous admission. mcp_servers_from_path = _parse_mcp_server_names_from_path( - request_route + request_route, mcp_servers ) if ( mcp_servers_from_path is not None @@ -300,7 +317,9 @@ class MCPRequestHandler: # require unauthenticated requests to protected resources to receive # 401 + WWW-Authenticate. Defer to _raise_preemptive_401_for_unauthenticated_servers # for pass-through servers instead of surfacing a generic admission error. - mcp_servers_from_path = _parse_mcp_server_names_from_path(request_route) + mcp_servers_from_path = _parse_mcp_server_names_from_path( + request_route, mcp_servers + ) client_ip = IPAddressUtils.get_mcp_client_ip(request) if ( mcp_servers_from_path is not None diff --git a/litellm/proxy/_experimental/mcp_server/exceptions.py b/litellm/proxy/_experimental/mcp_server/exceptions.py index 1a9b7130ada..67bb9f5d387 100644 --- a/litellm/proxy/_experimental/mcp_server/exceptions.py +++ b/litellm/proxy/_experimental/mcp_server/exceptions.py @@ -39,13 +39,14 @@ class MCPUpstreamAuthError(Exception): that points at the gateway's standard-pattern well-known endpoint for this server, so MCP clients can still initiate RFC 9728 discovery against the upstream IdP via the gateway's proxied metadata. Callers - should pass ``base_url`` (the gateway origin, no trailing slash) so - the fabricated URI is absolute as RFC 9728 §3.2 requires; strict - clients reject relative URIs in the Bearer challenge. + must pass ``base_url`` (the gateway origin, no trailing slash) so the + fabricated URI is absolute as RFC 9728 §3.2 requires; if ``base_url`` + is missing we skip fabrication entirely rather than emit a relative + URI that strict clients reject in the Bearer challenge. """ challenge: Optional[str] = self.www_authenticate - if challenge is None and self.status_code == 401: - prefix = base_url.rstrip("/") if base_url else "" + if challenge is None and self.status_code == 401 and base_url: + prefix = base_url.rstrip("/") challenge = ( "Bearer resource_metadata=" f'"{prefix}/.well-known/oauth-protected-resource/mcp/{self.server_name}"' diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py index cfaaf21c0dc..d900f690c57 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough_tools.py @@ -126,7 +126,10 @@ def test_to_http_exception_preserves_upstream_www_authenticate(): } -def test_to_http_exception_fabricates_resource_metadata_when_upstream_omits_header(): +def test_to_http_exception_skips_fabrication_when_base_url_missing(): + """Without ``base_url`` we cannot build an RFC 9728 §3.2-compliant absolute + URI, so we omit the fabricated ``WWW-Authenticate`` challenge entirely + instead of emitting a relative URI strict clients reject.""" err = MCPUpstreamAuthError( status_code=401, www_authenticate=None, @@ -135,9 +138,7 @@ def test_to_http_exception_fabricates_resource_metadata_when_upstream_omits_head http_exc = err.to_http_exception() assert http_exc.status_code == 401 - assert http_exc.headers == { - "www-authenticate": 'Bearer resource_metadata="/.well-known/oauth-protected-resource/mcp/sample_docs"' - } + assert http_exc.headers is None def test_to_http_exception_fabricates_absolute_resource_metadata_with_base_url():