From f5a193f29b2fdb36a378055e9d991b322bf55560 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 21 May 2026 18:55:17 +0000 Subject: [PATCH] fix(mcp/jwt): dedupe cold-start path parser; reject conflicting audience flags - _parse_mcp_server_names_from_path now delegates to MCPRequestHandler._extract_target_server_names_from_path so the names used by the cold-start passthrough bypass cannot drift from the names used by downstream routing. - JWTIssuerConfig now rejects the combination of audience and disable_audience_validation=True at validation time instead of silently ignoring the flag. --- .../mcp_server/auth/user_api_key_auth_mcp.py | 51 ++++++------------- litellm/proxy/_types.py | 4 ++ .../proxy/auth/test_handle_jwt.py | 21 ++++++++ 3 files changed, 40 insertions(+), 36 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 d3242bbb856..d55261982aa 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 @@ -19,43 +19,22 @@ from litellm.proxy.auth.ip_address_utils import IPAddressUtils def _parse_mcp_server_names_from_path(path: str) -> Optional[List[str]]: - """Parse a single MCP server name from /mcp/{name} or /{name}/mcp path patterns. - Returns None for the aggregate /mcp route (no bypass for multi-server paths). - - Multi-server CSV paths like ``/mcp/server1,server2`` also return ``None`` — - the cold-start bypass must not activate when any of the co-targeted servers - might not be passthrough-eligible. - - Server names may contain a single slash (e.g. ``custom_solutions/user_123``), - so the ``/mcp/...`` form must mirror - :meth:`MCPRequestHandler._extract_target_server_names_from_path` rather than - stop at the first ``/``. Otherwise the cold-start lookup would miss - slashed-name servers and fall through to the generic admission error - instead of the spec-compliant 401 challenge.""" - mcp_path_match = re.match(r"^/mcp/([^?#]+)", path) - if mcp_path_match: - servers_and_path = mcp_path_match.group(1) - if not servers_and_path: - return None - if "," in servers_and_path: - return None - # Server name may contain at most one slash; strip any trailing - # path segments so the registry lookup uses the canonical name. - single_server_match = re.match( - r"^([^/]+(?:/[^/]+)?)(?:/.*)?$", servers_and_path + """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).""" + servers = MCPRequestHandler._extract_target_server_names_from_path(path) + if len(servers) != 1: + verbose_logger.debug( + "MCP cold-start: path %r resolved to %r; passthrough 401 bypass " + "requires exactly one target and will not activate", + path, + servers, ) - if single_server_match: - return [single_server_match.group(1)] - return [servers_and_path] - m = re.match(r"^/([^/,?#]+)/mcp", path) - if m: - return [m.group(1)] - verbose_logger.debug( - "MCP cold-start: path %r does not match /mcp/{name} or /{name}/mcp; " - "passthrough 401 bypass will not activate", - path, - ) - return None + return None + return servers def _is_mcp_passthrough_cold_start( diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 19cd14622f8..d4d11457a35 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -4476,6 +4476,10 @@ class JWTIssuerConfig(BaseModel): raise ValueError( f"JWT issuer {self.issuer} must configure audience or set disable_audience_validation=True" ) + if self.audience is not None and self.disable_audience_validation: + raise ValueError( + f"JWT issuer {self.issuer} cannot set audience and disable_audience_validation=True together" + ) return self diff --git a/tests/test_litellm/proxy/auth/test_handle_jwt.py b/tests/test_litellm/proxy/auth/test_handle_jwt.py index 0cae0b56f7f..4becac13ccf 100644 --- a/tests/test_litellm/proxy/auth/test_handle_jwt.py +++ b/tests/test_litellm/proxy/auth/test_handle_jwt.py @@ -3173,6 +3173,27 @@ def test_multi_issuer_jwt_requires_audience_unless_explicitly_disabled( assert "must configure audience" in str(exc.value) +def test_multi_issuer_jwt_rejects_audience_with_disable_audience_validation(): + issuer = "https://issuer.example.com" + jwks_url = f"{issuer}/keys" + + with pytest.raises(Exception) as exc: + LiteLLM_JWTAuth( + issuers=[ + { + "issuer": issuer, + "jwks_url": jwks_url, + "audience": "some-audience", + "disable_audience_validation": True, + } + ] + ) + + assert "cannot set audience and disable_audience_validation=True together" in str( + exc.value + ) + + @pytest.mark.asyncio async def test_global_jwt_ignores_user_supplied_internal_claims(monkeypatch): from litellm.caching.dual_cache import DualCache