Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 49 additions & 3 deletions litellm/proxy/auth/route_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@

class RouteChecks:
@staticmethod
def should_call_route(route: str, valid_token: UserAPIKeyAuth):
def should_call_route(
route: str,
valid_token: UserAPIKeyAuth,
request: Optional[Request] = None,
):
"""
Check if management route is disabled and raise exception
"""
Expand All @@ -77,13 +81,15 @@ def should_call_route(route: str, valid_token: UserAPIKeyAuth):

# Check if Virtual Key is allowed to call the route - Applies to all Roles
RouteChecks.is_virtual_key_allowed_to_call_route(
route=route, valid_token=valid_token
route=route, valid_token=valid_token, request=request
)
return True

@staticmethod
def is_virtual_key_allowed_to_call_route(
route: str, valid_token: UserAPIKeyAuth
route: str,
valid_token: UserAPIKeyAuth,
request: Optional[Request] = None,
) -> bool:
"""
Raises Exception if Virtual Key is not allowed to call the route
Expand Down Expand Up @@ -130,6 +136,21 @@ def is_virtual_key_allowed_to_call_route(
):
return True

# Method-aware carve-out: allow GET on the two
# read-only MCP-server discovery endpoints
# (`/v1/mcp/server` and `/v1/mcp/server/{server_id}`)
# so virtual keys with allowed_routes=["llm_api_routes"]
# can list/inspect MCP servers. The GET handlers in
# mcp_management_endpoints.py sanitize the response
# for restricted virtual keys (stripping url,
# headers, env, credentials). POST/PUT/DELETE on
# these paths are admin-only management writes and
# are intentionally not covered.
if RouteChecks._is_get_mcp_server_discovery_route(
route=route, request=request
):
return True

# check if wildcard pattern is allowed
for allowed_route in valid_token.allowed_routes:
if RouteChecks._route_matches_wildcard_pattern(
Expand Down Expand Up @@ -401,6 +422,31 @@ def is_llm_api_route(route: str) -> bool:
return True
return False

@staticmethod
def _is_get_mcp_server_discovery_route(
route: str, request: Optional[Request]
) -> bool:
"""
Returns True if `request` is a GET against one of the two read-only
MCP-server discovery paths:

- GET `/v1/mcp/server` (list)
- GET `/v1/mcp/server/{server_id}` (single server, single segment)

Multi-segment paths (`/v1/mcp/server/{id}/approve`, etc.) and any
non-GET method return False, so admin-only management writes on the
same path prefix are not reachable through this carve-out.
"""
if request is None or request.method.upper() != "GET":
return False
if route == "/v1/mcp/server":
return True
prefix = "/v1/mcp/server/"
if not route.startswith(prefix):
return False
remainder = route[len(prefix) :]
return bool(remainder) and "/" not in remainder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: MCP health endpoint exposed to LLM-only virtual keys

/v1/mcp/server/health is also a single segment after /v1/mcp/server/, so a virtual key restricted to allowed_routes=["llm_api_routes"] can now call that management health endpoint. Use the matched route template, or explicitly exclude static management paths like health and submissions, so this carve-out only covers the list handler and the /server/{server_id} handler.


@staticmethod
def is_management_route(route: str) -> bool:
"""
Expand Down
4 changes: 3 additions & 1 deletion litellm/proxy/auth/user_api_key_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -2200,7 +2200,9 @@ async def user_api_key_auth(
user_api_key_auth_obj.budget_reservation = None

## ENSURE DISABLE ROUTE WORKS ACROSS ALL USER AUTH FLOWS ##
RouteChecks.should_call_route(route=route, valid_token=user_api_key_auth_obj)
RouteChecks.should_call_route(
route=route, valid_token=user_api_key_auth_obj, request=request
)

# Single authorization point. Builder paths MUST NOT call common_checks.
# Route through the same exception handler the builder uses so
Expand Down
115 changes: 114 additions & 1 deletion tests/test_litellm/proxy/auth/test_route_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,125 @@ def test_virtual_key_mcp_routes_allows_v1_mcp_server_subpaths(route):
)
def test_mcp_management_routes_classified_as_management_not_llm_api(route):
"""MCP server CRUD must be management routes, not llm_api routes, so
DISABLE_LLM_API_ENDPOINTS on admin nodes does not block the Admin UI."""
DISABLE_LLM_API_ENDPOINTS on admin nodes does not block the Admin UI.

Note: virtual keys with allowed_routes=["llm_api_routes"] can still call
*GET* `/v1/mcp/server` and *GET* `/v1/mcp/server/{server_id}` — that
carve-out is enforced method-aware inside
`is_virtual_key_allowed_to_call_route`, not by adding the paths to
`llm_api_routes`. So `is_llm_api_route()` still returns False here and
`DISABLE_LLM_API_ENDPOINTS` still does not block these paths.
"""

assert RouteChecks.is_llm_api_route(route=route) is False
assert RouteChecks.is_management_route(route=route) is True


def _mock_request(method: str) -> Request:
request = MagicMock(spec=Request)
request.method = method
return request


@pytest.mark.parametrize(
"route",
[
"/v1/mcp/server",
"/v1/mcp/server/abc-123",
],
)
def test_virtual_key_llm_api_routes_allows_get_mcp_server_discovery(route):
"""
Regression test: virtual keys with allowed_routes=["llm_api_routes"] must
be able to list/inspect MCP servers via GET /v1/mcp/server[/{server_id}].

The handlers strip credential-bearing fields via
`_sanitize_mcp_server_list_for_virtual_key` when the caller is a
restricted virtual key, so GET is safe to expose. The carve-out is
method-aware (see below) — non-GET requests to the same paths are
rejected at this layer, so admin-only writes remain gated.
"""

valid_token = UserAPIKeyAuth(
user_id="test_user",
allowed_routes=["llm_api_routes"],
)

result = RouteChecks.is_virtual_key_allowed_to_call_route(
route=route,
valid_token=valid_token,
request=_mock_request("GET"),
)

assert result is True


@pytest.mark.parametrize(
"route",
[
"/v1/mcp/server",
"/v1/mcp/server/abc-123",
],
)
@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"])
def test_virtual_key_llm_api_routes_rejects_non_get_mcp_server_discovery(route, method):
"""Method-aware: the MCP server discovery carve-out is GET-only.

POST/PUT/PATCH/DELETE on `/v1/mcp/server[/{server_id}]` are admin-only
management writes and must not be reachable via llm_api_routes.
"""

valid_token = UserAPIKeyAuth(
user_id="test_user",
allowed_routes=["llm_api_routes"],
)

with pytest.raises(HTTPException) as exc_info:
RouteChecks.is_virtual_key_allowed_to_call_route(
route=route,
valid_token=valid_token,
request=_mock_request(method),
)

assert exc_info.value.status_code == 403


@pytest.mark.parametrize(
"route",
[
# Multi-segment admin-only sub-paths must NOT be reachable via
# llm_api_routes, even on GET.
"/v1/mcp/server/abc-123/approve",
"/v1/mcp/server/abc-123/reject",
"/v1/mcp/server/oauth/session",
"/v1/mcp/server/abc-123/user-credential",
],
)
def test_virtual_key_llm_api_routes_rejects_mcp_multi_segment_admin_subpaths(
route,
):
"""Multi-segment admin-only MCP sub-paths are not reachable via llm_api_routes.

The discovery carve-out only matches `/v1/mcp/server` and
`/v1/mcp/server/{server_id}` (single segment after `/server/`), so any
path with additional segments is rejected even when the request is GET.
"""

valid_token = UserAPIKeyAuth(
user_id="test_user",
allowed_routes=["llm_api_routes"],
)

with pytest.raises(HTTPException) as exc_info:
RouteChecks.is_virtual_key_allowed_to_call_route(
route=route,
valid_token=valid_token,
request=_mock_request("GET"),
)

assert exc_info.value.status_code == 403


def test_spend_logs_v2_classified_as_management_not_llm_api():
"""Paginated spend logs are a management/spend read route, not an LLM API."""

Expand Down
Loading