mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
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.
This commit is contained in:
parent
4ddbb91825
commit
f5a193f29b
3 changed files with 40 additions and 36 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue