From 77131a50db5bdc2f4d92f510a9cb96d59a7e57d0 Mon Sep 17 00:00:00 2001 From: Ishaan Jaffer Date: Mon, 23 Mar 2026 11:14:21 -0700 Subject: [PATCH] fix(mcp): use _user_has_admin_view; deny None mcp_toolsets for non-admin; use direct RecordNotFoundError import; fix docstring --- .../proxy/_experimental/mcp_server/server.py | 10 +++++++--- .../_experimental/mcp_server/toolset_db.py | 20 ++++++------------- litellm/proxy/proxy_server.py | 5 +++-- .../mcp/litellm_proxy_mcp_handler.py | 13 +++++++----- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index adda0b5aebb..f722cefaaff 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -2430,14 +2430,18 @@ if MCP_AVAILABLE: or set to a list that omits this toolset). Admin keys always pass. """ from litellm.proxy._types import LiteLLM_ObjectPermissionTable + from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view # Access control: non-admin keys must have this toolset in their grant list. - is_admin = getattr(user_api_key_auth, "user_role", None) == "proxy_admin" + # Use _user_has_admin_view so that PROXY_ADMIN_VIEW_ONLY is also treated as admin. + is_admin = _user_has_admin_view(user_api_key_auth) if not is_admin: op = user_api_key_auth.object_permission granted = getattr(op, "mcp_toolsets", None) if op else None - # granted=None → no restriction (allow); granted=[] or list without toolset_id → deny - if granted is not None and toolset_id not in granted: + # granted=None → key has no explicit toolset grants → deny (same semantics as + # fetch_mcp_toolsets which returns [] for non-admin keys with no grants configured). + # granted=[] or list without toolset_id → also deny. + if granted is None or toolset_id not in granted: raise HTTPException( status_code=403, detail=f"API key does not have access to toolset '{toolset_id}'.", diff --git a/litellm/proxy/_experimental/mcp_server/toolset_db.py b/litellm/proxy/_experimental/mcp_server/toolset_db.py index adea36d2dea..2d22b52e331 100644 --- a/litellm/proxy/_experimental/mcp_server/toolset_db.py +++ b/litellm/proxy/_experimental/mcp_server/toolset_db.py @@ -1,6 +1,8 @@ import json from typing import List, Optional +from prisma.errors import RecordNotFoundError + from litellm._logging import verbose_proxy_logger from litellm._uuid import uuid from litellm.proxy.utils import PrismaClient @@ -91,13 +93,8 @@ async def update_mcp_toolset( where={"toolset_id": data.toolset_id}, data=data_dict, ) - except Exception as e: - if ( - "RecordNotFoundError" in type(e).__name__ - or "record was not found" in str(e).lower() - ): - return None - raise + except RecordNotFoundError: + return None return _toolset_from_row(row) @@ -109,11 +106,6 @@ async def delete_mcp_toolset( row = await prisma_client.db.litellm_mcptoolsettable.delete( where={"toolset_id": toolset_id} ) - except Exception as e: - if ( - "RecordNotFoundError" in type(e).__name__ - or "record was not found" in str(e).lower() - ): - return None - raise + except RecordNotFoundError: + return None return _toolset_from_row(row) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 28c8d8d4cc5..5f1d7767b6c 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -13589,8 +13589,9 @@ async def toolset_mcp_route(toolset_name: str, request: Request): Namespace a toolset as its own MCP endpoint. Connecting to /toolset//mcp exposes exactly the tools defined in - the toolset, regardless of what other permissions the API key has. - Any valid API key can discover and call the toolset's tools here. + the toolset. Access is enforced: non-admin API keys must have the toolset + listed in their object_permission.mcp_toolsets grant list, or the request + will be rejected with a 403. """ try: from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index 6d0b6b87ce6..d8b218f9389 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -179,10 +179,11 @@ class LiteLLM_Proxy_MCP_Handler: if toolset is not None: # Access control: only allow if the key explicitly grants this toolset. if user_api_key_auth is not None: - is_admin = ( - getattr(user_api_key_auth, "user_role", None) - == "proxy_admin" + from litellm.proxy.management_endpoints.common_utils import ( + _user_has_admin_view, ) + + is_admin = _user_has_admin_view(user_api_key_auth) if not is_admin: op = user_api_key_auth.object_permission granted = ( @@ -190,9 +191,11 @@ class LiteLLM_Proxy_MCP_Handler: if op else None ) + # None means no grants configured → deny (consistent with + # fetch_mcp_toolsets which returns [] for unconfigured keys) if ( - granted is not None - and toolset.toolset_id not in granted + granted is None + or toolset.toolset_id not in granted ): verbose_logger.debug( f"Key does not have access to toolset '{name}', skipping."