fix(mcp): run the route gate on bridge admission so allowed_routes are enforced

The envelope arm reloaded the identity and ran _run_centralized_common_checks
but skipped RouteChecks.should_call_route, which the standard pipeline runs
between the builder and common_checks. Because the centralized checks treat MCP
as an inference route and never re-check allowed_routes, a key barred from MCP
routes could mint an envelope at the token endpoint (not itself an MCP route)
and replay it against MCP. Run the route gate before admitting, and clear the
request-scoped budget_reservation, matching the wrapper's sequence; a disallowed
route now surfaces the gate's own 403.
This commit is contained in:
Tin Chi Lo 2026-07-11 13:34:23 -07:00
parent f5f03cbd63
commit 688f535bbf
2 changed files with 47 additions and 14 deletions

View file

@ -636,24 +636,32 @@ class MCPRequestHandler:
@staticmethod
async def _enforce_admitted_live_policy(admitted: UserAPIKeyAuth, request: Request, route: str) -> None:
"""Run the standard pipeline's single authorization point over the admitted identity.
"""Run the standard pipeline's authorization checks over the admitted identity.
``_run_centralized_common_checks`` is the same gate ``user_api_key_auth`` applies
after every builder path, so the envelope identity gets team-block, project-block,
org, and budget enforcement identical to the same key presented directly, and any
policy dimension added to the standard pipeline applies here without this arm
mirroring it.
Mirrors the ``user_api_key_auth`` wrapper between the builder and its return: clear the
request-scoped ``budget_reservation`` on the reloaded identity, run the route gate
(``RouteChecks.should_call_route``) to enforce the identity's ``allowed_routes`` and any
disabled/admin-only route, then run ``_run_centralized_common_checks`` (the same gate every
builder path funnels through) for team-block, project-block, org, and budget. The route gate
closes a bypass: a key barred from MCP routes could otherwise mint an envelope at the token
endpoint (not itself an MCP route) and replay it against MCP, because the centralized checks
treat MCP as an inference route and never re-check ``allowed_routes``.
Failures surface with the status the standard pipeline would give them, mirroring
``UserAPIKeyAuthExceptionHandler``: an over-budget identity is a 429, a sub-check that
raised its own ``HTTPException``/``ProxyException`` keeps that status, a transient
database outage is a retryable 503, and only a genuinely unresolvable failure (a
blocked team/project raises a bare ``Exception``, same as the standard pipeline's
fallback) becomes the fail-closed 401. Collapsing every failure to 401 was misleading:
it told an over-budget but validly-authenticated caller their credential was invalid,
which on a DCR client reads as broken auth and can trigger a pointless re-authorize
loop that cannot fix a budget problem, and it masked a DB outage as an auth error."""
``UserAPIKeyAuthExceptionHandler``: a disallowed route is the route gate's own 403, an
over-budget identity is a 429, a sub-check that raised its own ``HTTPException``/
``ProxyException`` keeps that status, a transient database outage is a retryable 503, and
only a genuinely unresolvable failure (a blocked team/project raises a bare ``Exception``,
same as the standard pipeline's fallback) becomes the fail-closed 401. Collapsing every
failure to 401 was misleading: it told an over-budget but validly-authenticated caller their
credential was invalid, which on a DCR client reads as broken auth and can trigger a
pointless re-authorize loop that cannot fix a budget problem, and it masked a DB outage as an
auth error."""
from litellm.proxy.auth.route_checks import RouteChecks
admitted.budget_reservation = None
try:
RouteChecks.should_call_route(route=route, valid_token=admitted, request=request)
await _run_centralized_common_checks(
user_api_key_auth_obj=admitted,
request=request,

View file

@ -5297,6 +5297,31 @@ class TestMCPDcrBridgeDelegateAdmission:
assert exc_info.value.status_code == 503
async def test_envelope_for_key_barred_from_mcp_routes_is_rejected_403(self):
"""A key whose allowed_routes exclude MCP must not reach tools via an envelope: the arm runs
RouteChecks.should_call_route before admitting, exactly as the standard pipeline does between
the builder and common_checks. A route-restricted key can mint an envelope at the token
endpoint (not itself an MCP route) and would otherwise replay it against MCP, because the
centralized checks treat MCP as an inference route and never re-check allowed_routes; the
route gate rejects it with its own 403."""
envelope = self._mint_bridge_envelope(key_hash=self._KEY_HASH)
scope = {
"type": "http",
"method": "POST",
"path": "/mcp/bridge_delegate_server",
"headers": [(b"authorization", f"Bearer {envelope}".encode("latin-1"))],
}
with (
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
self._patch_key_reload(return_value=self._reloaded_key(allowed_routes=["/chat/completions"])),
):
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
with pytest.raises(HTTPException) as exc_info:
await MCPRequestHandler.process_mcp_request(scope)
assert exc_info.value.status_code == 403
async def test_blocked_state_bare_exception_stays_401(self):
"""A blocked team/project raises a bare Exception (no status) in common_checks, which the
standard pipeline renders as 401; the arm keeps failing those closed as 401, never a 500."""