From 57ca48a863b8dfb2bebef996fd723ea4191e3c79 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 3 Jul 2026 13:59:28 -0700 Subject: [PATCH] feat(mcp): add all-proxy-mcpservers sentinel to grant teams every MCP server (#32012) * feat(mcp): add all-proxy-mcpservers sentinel to grant every MCP server Teams can now be scoped to the all-proxy-mcpservers sentinel so they gain access to every MCP server on the proxy without listing each id. The sentinel expands to the live registry at request time, so a server added later is picked up with no change to the team's stored permission. The team ceiling that validates a key's MCP scope expands the sentinel too, so a key can be scoped to any server (including one registered after the team) and still pass subset validation Expose the option in the team create and edit forms via a new exclusive "All Proxy MCP Servers" choice in MCPServerSelector, mirroring the existing "No MCP Servers" sentinel * Update litellm/proxy/management_helpers/object_permission_utils.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * fix(mcp): honor all-proxy-mcpservers only on the team path, never per-key The sentinel was expanded inside the shared expand_permission_list, which also feeds the key, org, end_user and agent resolvers. A key whose stored object_permission ever held all-proxy-mcpservers (a stale write, a configured default, or a bug) would silently resolve to every MCP server at runtime, and a teamless key had nothing to cap it, so all servers got injected. Only write-time validation stripping the value stood between that value and a full grant Move the expansion out of expand_permission_list and into _get_allowed_mcp_servers_for_team so the sentinel is honored only where it is settable (a team). Anywhere else it now passes through as an inert literal that matches no registered server and is denied downstream. Reserved-id protection already blocks a real server from taking that id * fix(mcp): require proxy admin to grant a team the all-proxy MCP sentinel Granting a team every MCP server on the proxy is a proxy-wide authorization decision, but team create/update let any caller who can manage a team set object_permission.mcp_servers, with no ceiling check. Org admins reach /team/update by default (org_admin_allowed_routes) and _verify_team_access also admits team admins, so a non-proxy-admin could set all-proxy-mcpservers and self-grant their team access to every MCP server on the proxy, including servers never assigned to that team Gate the grant in new_team and update_team: a non-proxy-admin cannot add the all-proxy-mcpservers sentinel. The check is scoped to newly adding it, so a team a proxy admin already scoped to all-proxy can still be edited by a team admin without being forced to strip the sentinel. The UI only offers the "All Proxy MCP Servers" option to proxy admins in the team create and edit forms * fix(ui): render friendly all-proxy MCP label for non-admins editing an all-proxy team A team scoped to the all-proxy-mcpservers sentinel could be opened in the team edit form by a team admin or org admin (canEditTeam admits them), but the "All Proxy MCP Servers" option in MCPServerSelector was rendered only behind the proxy-admin-gated allowAllProxyMcpServers flag. For a non-proxy-admin the stored sentinel was hydrated into the selected value with no matching Select.Option, so antd showed the raw all-proxy-mcpservers literal as a chip, and adding another server could persist a mixed [all-proxy-mcpservers, ] value. Render the option whenever the sentinel is present in the value, not only when the caller may grant it, and drive the real-option disabling off presence too so the selection stays exclusive. A non-proxy-admin now sees the friendly label read-only and cannot build a mixed state; only a proxy admin can newly add it, which the backend already enforces. Adds regression tests: the selector shows the friendly option (not the raw literal) when the sentinel is stored but the grant flag is off, plus exclusive emit and disabled-real-options coverage, and MCPServerPermissions renders the green "All" state instead of the raw sentinel string. * fix(ui): drop redundant "All servers" hint from the all-proxy MCP chip antd renders a Select option's children inside the selected tag, so the all-proxy option showed both "All Proxy MCP Servers" and the green "All servers" type-hint in the chip, which say the same thing. Collapse the option to a single green "All Proxy MCP Servers" label so the dropdown row and the chip read cleanly without the duplication. * fix(ui): color the all-proxy MCP label blue to match server chips Use the same blue (#1890ff) as regular MCP server entries for the "All Proxy MCP Servers" option/chip instead of green. * fix(ui): make the all-proxy MCP permissions display blue, not green Match the blue used by the selector chip and regular server entries so the "All Proxy MCP Servers" badge and row in MCPServerPermissions are consistent across the team/key/org detail views. The red "Blocked" state for no-mcp-servers is unchanged. --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- .../mcp_server/auth/user_api_key_auth_mcp.py | 4 + .../management_endpoints/team_endpoints.py | 13 + .../object_permission_utils.py | 60 ++++- .../auth/test_user_api_key_auth_mcp.py | 239 ++++++++++++++++++ .../test_object_permission_utils.py | 174 ++++++++++++- .../src/components/OldTeams.tsx | 1 + .../MCPServerSelector.test.tsx | 85 ++++++- .../MCPServerSelector.tsx | 26 +- .../src/components/mcp_tools/constants.ts | 2 + .../permissions/MCPServerPermissions.test.tsx | 18 ++ .../permissions/MCPServerPermissions.tsx | 12 +- .../src/components/team/TeamInfo.tsx | 1 + 12 files changed, 621 insertions(+), 14 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 2520c7e82a1..387843ee5b2 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 @@ -11,6 +11,7 @@ from litellm.proxy._types import ( LiteLLM_TeamTable, ProxyException, SpecialHeaders, + SpecialMCPServerName, SpecialMCPServerNames, UserAPIKeyAuth, ) @@ -1041,6 +1042,9 @@ class MCPRequestHandler: if object_permissions is None: return list(set(team_access_group_servers)) + if SpecialMCPServerName.all_proxy_servers.value in (object_permissions.mcp_servers or []): + return list(global_mcp_server_manager.get_registry().keys()) + direct_mcp_servers = global_mcp_server_manager.expand_permission_list(object_permissions.mcp_servers or []) legacy_access_group_servers = await MCPRequestHandler._get_mcp_servers_from_access_groups( diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index 0ea2b9e05f9..5c41c60fcb1 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -93,6 +93,7 @@ from litellm.proxy.management_endpoints.tag_management_endpoints import ( ) from litellm.proxy.management_helpers.object_permission_utils import ( _set_object_permission, + enforce_all_proxy_mcp_servers_grant_is_admin_only, handle_update_object_permission_common, ) from litellm.proxy.management_helpers.team_member_permission_checks import ( @@ -1144,6 +1145,12 @@ async def new_team( data_json = data.json() ## Handle Object Permission - MCP, Vector Stores etc. + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=(data.object_permission.mcp_servers if data.object_permission is not None else None), + existing_object_permission_id=None, + is_proxy_admin=user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN, + prisma_client=prisma_client, + ) data_json = await _set_object_permission( data_json=data_json, prisma_client=prisma_client, @@ -1846,6 +1853,12 @@ async def update_team( # Check object permission if data.object_permission is not None: + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=data.object_permission.mcp_servers, + existing_object_permission_id=existing_team_row.object_permission_id, + is_proxy_admin=user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN, + prisma_client=prisma_client, + ) updated_kv = await handle_update_object_permission( data_json=updated_kv, existing_team_row=existing_team_row, diff --git a/litellm/proxy/management_helpers/object_permission_utils.py b/litellm/proxy/management_helpers/object_permission_utils.py index fe96d9c260a..6bbd41b93ed 100644 --- a/litellm/proxy/management_helpers/object_permission_utils.py +++ b/litellm/proxy/management_helpers/object_permission_utils.py @@ -11,7 +11,7 @@ from fastapi import HTTPException, status from litellm._logging import verbose_proxy_logger from litellm._uuid import uuid from litellm.litellm_core_utils.safe_json_dumps import safe_dumps -from litellm.proxy._types import ObjectPermissionDict, SpecialMCPServerNames +from litellm.proxy._types import ObjectPermissionDict, SpecialMCPServerName, SpecialMCPServerNames from litellm.proxy.utils import PrismaClient from litellm.repositories.object_permission_repository import ObjectPermissionRepository from litellm.repositories.table_repositories import MCPServerRepository @@ -334,6 +334,8 @@ async def _resolve_team_allowed_mcp_servers( ) direct_servers: List[str] = team_object_permission.mcp_servers or [] + if SpecialMCPServerName.all_proxy_servers.value in direct_servers: + return _get_all_mcp_server_ids() access_group_servers: List[str] = await MCPRequestHandler._get_mcp_servers_from_access_groups( team_object_permission.mcp_access_groups or [] ) @@ -359,6 +361,62 @@ def _get_allow_all_keys_server_ids() -> Set[str]: return set(global_mcp_server_manager.get_allow_all_keys_server_ids()) +def _get_all_mcp_server_ids() -> set[str]: + """Return every MCP server id registered on the proxy (config + DB union).""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + + return set(global_mcp_server_manager.get_registry().keys()) + + +async def _existing_object_permission_mcp_servers( + object_permission_id: Optional[str], + prisma_client: Optional[PrismaClient], +) -> list[str]: + if not object_permission_id or prisma_client is None: + return [] + existing = await ObjectPermissionRepository(prisma_client).table.find_unique( + where={"object_permission_id": object_permission_id}, + ) + if existing is None: + return [] + return existing.mcp_servers or [] + + +async def enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers: Optional[list[str]], + existing_object_permission_id: Optional[str], + is_proxy_admin: bool, + prisma_client: Optional[PrismaClient], +) -> None: + """ + Only a proxy admin may newly grant the all-proxy MCP sentinel. + + Scoping a team to every MCP server on the proxy is a proxy-wide authorization + decision, so a caller who is not a proxy admin (e.g. a team admin managing their + own team) cannot add ``all-proxy-mcpservers``. A sentinel a proxy admin already + granted is left untouched, so unrelated edits to such a team still succeed. + + Raises HTTPException(403) when a non-admin tries to add the sentinel. + """ + sentinel = SpecialMCPServerName.all_proxy_servers.value + if is_proxy_admin or sentinel not in (requested_mcp_servers or []): + return + existing_mcp_servers = await _existing_object_permission_mcp_servers( + object_permission_id=existing_object_permission_id, + prisma_client=prisma_client, + ) + if sentinel in existing_mcp_servers: + return + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "error": "Only a proxy admin can grant a team access to all proxy MCP servers ('all-proxy-mcpservers')." + }, + ) + + async def _get_team_allowed_mcp_servers( team_obj: Optional["LiteLLM_TeamTableCachedObj"], prisma_client: Optional[PrismaClient] = None, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py index b3b0e8adcf6..3607d448aad 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -4495,3 +4495,242 @@ async def test_get_allowed_mcp_servers_surfaces_ungated_key_access_group_grant_e assert result == ["srv-deepwiki"] finally: _stop_patches(patches) + + +def test_expand_permission_list_does_not_honor_all_proxy_sentinel(): + """The all-proxy sentinel is a team-only grant. The shared expand_permission_list + also feeds the key/org/end_user/agent resolvers, so it must NOT expand the + sentinel to the full registry; it passes through as an inert literal (denied + downstream). Concrete ids still resolve normally. If the sentinel were expanded + here, any stored key/org/end_user permission holding it would silently gain every + server.""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import SpecialMCPServerName + from litellm.types.mcp import MCPTransport + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + sentinel = SpecialMCPServerName.all_proxy_servers.value + for sid in ("srv-x", "srv-y"): + global_mcp_server_manager.registry[sid] = MCPServer( + server_id=sid, + name=sid, + server_name=sid, + url=f"https://{sid}.example.com", + transport=MCPTransport.http, + ) + try: + result = global_mcp_server_manager.expand_permission_list([sentinel]) + assert set(result).isdisjoint({"srv-x", "srv-y"}) + assert result == [sentinel] + assert global_mcp_server_manager.expand_permission_list(["srv-x"]) == ["srv-x"] + finally: + for sid in ("srv-x", "srv-y"): + global_mcp_server_manager.registry.pop(sid, None) + + +@pytest.mark.asyncio +async def test_get_allowed_mcp_servers_for_team_expands_all_proxy_sentinel_dynamically(): + """The TEAM resolver expands the all-proxy sentinel to every registered server and + picks up a server registered later, so a team scoped to all-proxy tracks the live + registry without any change to its stored permission. Reverting the team-side + expansion collapses this to the inert literal and the result no longer contains the + real servers.""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import ( + LiteLLM_ObjectPermissionTable, + LiteLLM_TeamTable, + SpecialMCPServerName, + ) + from litellm.types.mcp import MCPTransport + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + for sid in ("srv-x", "srv-y"): + global_mcp_server_manager.registry[sid] = MCPServer( + server_id=sid, + name=sid, + server_name=sid, + url=f"https://{sid}.example.com", + transport=MCPTransport.http, + ) + try: + team_perm = LiteLLM_ObjectPermissionTable( + object_permission_id="team-perm", + mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + mcp_access_groups=[], + vector_stores=[], + ) + team_obj = LiteLLM_TeamTable( + team_id="team-1", + access_group_ids=[], + object_permission_id="team-perm", + ) + team_obj.object_permission = team_perm + auth = UserAPIKeyAuth(token="test-token", api_key="sk-test", team_id="team-1") + + patches = _patch_proxy_server_globals_for_mcp() + [ + patch( + "litellm.proxy.auth.auth_checks.get_team_object", + new_callable=AsyncMock, + return_value=team_obj, + ), + patch( + "litellm.proxy.auth.auth_checks._get_mcp_server_ids_from_access_groups", + new_callable=AsyncMock, + return_value=[], + ), + ] + _start_patches(patches) + try: + result = await MCPRequestHandler._get_allowed_mcp_servers_for_team(auth) + assert set(result) == {"srv-x", "srv-y"} + + global_mcp_server_manager.registry["srv-z"] = MCPServer( + server_id="srv-z", + name="srv-z", + server_name="srv-z", + url="https://srv-z.example.com", + transport=MCPTransport.http, + ) + result_after = await MCPRequestHandler._get_allowed_mcp_servers_for_team(auth) + assert "srv-z" in result_after + finally: + _stop_patches(patches) + finally: + for sid in ("srv-x", "srv-y", "srv-z"): + global_mcp_server_manager.registry.pop(sid, None) + + +@pytest.mark.asyncio +async def test_key_with_all_proxy_sentinel_does_not_grant_all_servers(): + """Security regression: the all-proxy sentinel is a team-only grant. A KEY whose + stored object_permission holds the sentinel (via a stale write, a configured + default, or a bug) must NOT be silently widened to every server at runtime. A + teamless key with the sentinel resolves to no real server — never srv-secret or the + full registry. On the pre-hardening code the key path expanded the sentinel and + this key would reach srv-secret.""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import ( + LiteLLM_ObjectPermissionTable, + SpecialMCPServerName, + ) + from litellm.types.mcp import MCPTransport + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + for sid in ("srv-x", "srv-y", "srv-secret"): + global_mcp_server_manager.registry[sid] = MCPServer( + server_id=sid, + name=sid, + server_name=sid, + url=f"https://{sid}.example.com", + transport=MCPTransport.http, + ) + try: + key_perm = LiteLLM_ObjectPermissionTable( + object_permission_id="key-perm", + mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + mcp_access_groups=[], + vector_stores=[], + ) + auth = UserAPIKeyAuth(token="test-token", api_key="sk-test", object_permission=key_perm) + + patches = _patch_proxy_server_globals_for_mcp() + _start_patches(patches) + try: + result = await MCPRequestHandler.get_allowed_mcp_servers(auth) + finally: + _stop_patches(patches) + + assert "srv-secret" not in result + assert set(result).isdisjoint(global_mcp_server_manager.get_registry().keys()) + finally: + for sid in ("srv-x", "srv-y", "srv-secret"): + global_mcp_server_manager.registry.pop(sid, None) + + +@pytest.mark.asyncio +async def test_get_allowed_mcp_servers_team_all_proxy_key_scoped_to_one_end_to_end(): + """End-to-end: a team scoped to the all-proxy sentinel is a ceiling of every + registered server, so a key scoped to a single server (srv-x) resolves to + exactly that server (key ∩ all-servers == key). If the sentinel branch is + reverted the team ceiling collapses to the literal marker, the intersection + empties, and the result is [] instead of ["srv-x"].""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + from litellm.proxy._types import ( + LiteLLM_ObjectPermissionTable, + LiteLLM_TeamTable, + SpecialMCPServerName, + ) + from litellm.types.mcp import MCPTransport + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + for sid in ("srv-x", "srv-y"): + global_mcp_server_manager.registry[sid] = MCPServer( + server_id=sid, + name=sid, + server_name=sid, + url=f"https://{sid}.example.com", + transport=MCPTransport.http, + ) + try: + key_perm = LiteLLM_ObjectPermissionTable( + object_permission_id="key-perm", + mcp_servers=["srv-x"], + mcp_access_groups=[], + vector_stores=[], + ) + team_perm = LiteLLM_ObjectPermissionTable( + object_permission_id="team-perm", + mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + mcp_access_groups=[], + vector_stores=[], + ) + team_obj = LiteLLM_TeamTable( + team_id="team-1", + access_group_ids=[], + object_permission_id="team-perm", + ) + team_obj.object_permission = team_perm + + auth = UserAPIKeyAuth( + token="test-token", + api_key="sk-test", + team_id="team-1", + object_permission=key_perm, + ) + + patches = _patch_proxy_server_globals_for_mcp() + [ + patch( + "litellm.proxy.auth.auth_checks.get_team_object", + new_callable=AsyncMock, + return_value=team_obj, + ), + patch( + "litellm.proxy.auth.auth_checks._get_mcp_server_ids_from_access_groups", + new_callable=AsyncMock, + return_value=[], + ), + patch.object( + MCPRequestHandler, + "_get_mcp_servers_from_access_groups", + new_callable=AsyncMock, + return_value=[], + ), + ] + _start_patches(patches) + try: + result = await MCPRequestHandler.get_allowed_mcp_servers(auth) + finally: + _stop_patches(patches) + + assert result == ["srv-x"] + finally: + for sid in ("srv-x", "srv-y"): + global_mcp_server_manager.registry.pop(sid, None) diff --git a/tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py b/tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py index 0981c4239ee..d797a27aa67 100644 --- a/tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py +++ b/tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py @@ -9,13 +9,19 @@ sys.path.insert(0, os.path.abspath("../../../..")) from unittest.mock import AsyncMock, MagicMock, patch -from litellm.proxy._types import LiteLLM_ObjectPermissionBase, LiteLLM_ObjectPermissionTable, ObjectPermissionDict +from litellm.proxy._types import ( + LiteLLM_ObjectPermissionBase, + LiteLLM_ObjectPermissionTable, + ObjectPermissionDict, + SpecialMCPServerName, +) from litellm.proxy.management_helpers.object_permission_utils import ( _extract_requested_mcp_access_groups, _extract_requested_mcp_server_ids, _resolve_team_allowed_mcp_servers, _rewrite_object_permission_mcp_servers, _set_object_permission, + enforce_all_proxy_mcp_servers_grant_is_admin_only, validate_key_mcp_servers_against_team, validate_key_search_tools_against_team, validate_key_vector_stores_against_team, @@ -876,6 +882,172 @@ async def test_resolve_team_allowed_mcp_servers_dict_tool_permissions( assert result == {"server-a"} +# ---- Tests for the all-proxy-mcpservers sentinel (team scoped to every server) ---- + + +@pytest.mark.asyncio +@patch( + "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.MCPRequestHandler._get_mcp_servers_from_access_groups", + new_callable=AsyncMock, + return_value=[], +) +async def test_resolve_team_all_proxy_sentinel_resolves_dynamically(mock_access_groups): + """A team whose object_permission.mcp_servers holds the all-proxy sentinel + resolves to every registered server id, and picks up a server registered + later without any change to the team's stored permission (this kills the + early-return that maps the sentinel to the live registry).""" + registry = { + "srv-x": _make_mock_mcp_server("srv-x"), + "srv-y": _make_mock_mcp_server("srv-y"), + } + mock_mgr = MagicMock() + mock_mgr.get_registry.return_value = registry + + team_perm = MagicMock(spec=LiteLLM_ObjectPermissionTable) + team_perm.mcp_servers = [SpecialMCPServerName.all_proxy_servers.value] + team_perm.mcp_access_groups = [] + team_perm.mcp_tool_permissions = {} + + with patch( + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager", + mock_mgr, + ): + assert await _resolve_team_allowed_mcp_servers(team_perm) == {"srv-x", "srv-y"} + + registry["srv-z"] = _make_mock_mcp_server("srv-z") + assert await _resolve_team_allowed_mcp_servers(team_perm) == { + "srv-x", + "srv-y", + "srv-z", + } + + +@pytest.mark.asyncio +@patch( + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager", + new=_make_mock_mcp_manager("srv-x", "srv-y", "srv-z"), +) +@patch( + "litellm.proxy.management_helpers.object_permission_utils._get_allow_all_keys_server_ids", + return_value=set(), +) +@patch( + "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.MCPRequestHandler._get_mcp_servers_from_access_groups", + new_callable=AsyncMock, + return_value=[], +) +async def test_validate_key_scoped_to_server_added_after_team_all_proxy( + mock_access_groups, mock_allow_all +): + """The exact user scenario: a team scoped to the all-proxy sentinel, a server + (srv-z) registered afterwards, and a key scoped to just srv-z. Because the + team ceiling resolves to every registered server, the key passes validation + and keeps srv-z in its normalized permission.""" + team_obj = _make_team_obj(mcp_servers=[SpecialMCPServerName.all_proxy_servers.value]) + object_permission = {"mcp_servers": ["srv-z"]} + result = await validate_key_mcp_servers_against_team( + object_permission=object_permission, + team_obj=team_obj, + ) + assert result is not None + assert result["mcp_servers"] == ["srv-z"] + + +@pytest.mark.asyncio +@patch( + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager", + new=_make_mock_mcp_manager("srv-x", "srv-z"), +) +@patch( + "litellm.proxy.management_helpers.object_permission_utils._get_allow_all_keys_server_ids", + return_value=set(), +) +@patch( + "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.MCPRequestHandler._get_mcp_servers_from_access_groups", + new_callable=AsyncMock, + return_value=[], +) +async def test_validate_key_scoped_to_server_rejected_when_team_not_all_proxy( + mock_access_groups, mock_allow_all +): + """Contrast with the sentinel case: a team scoped to a concrete server list + (srv-x, not the sentinel) does NOT unlock srv-z for a key. It is the sentinel + specifically, not a blanket allow, that widens the team ceiling.""" + team_obj = _make_team_obj(mcp_servers=["srv-x"]) + with pytest.raises(HTTPException) as exc_info: + await validate_key_mcp_servers_against_team( + object_permission={"mcp_servers": ["srv-z"]}, + team_obj=team_obj, + ) + assert exc_info.value.status_code == 403 + assert "srv-z" in str(exc_info.value.detail) + + +# ---- Tests for the proxy-admin gate on granting a team the all-proxy sentinel ---- + + +@pytest.mark.asyncio +async def test_enforce_all_proxy_mcp_grant_blocks_non_admin_adding_sentinel(): + """A non-proxy-admin (e.g. a team admin) cannot newly grant a team the all-proxy + MCP sentinel. Without this gate a team admin could self-escalate their team to + every MCP server on the proxy via team create/update.""" + with pytest.raises(HTTPException) as exc_info: + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + existing_object_permission_id=None, + is_proxy_admin=False, + prisma_client=None, + ) + assert exc_info.value.status_code == 403 + assert "all-proxy-mcpservers" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_enforce_all_proxy_mcp_grant_allows_proxy_admin(): + """A proxy admin may grant the sentinel — the intended way to scope a team to all + proxy MCP servers.""" + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + existing_object_permission_id=None, + is_proxy_admin=True, + prisma_client=None, + ) + + +@pytest.mark.asyncio +async def test_enforce_all_proxy_mcp_grant_allows_non_admin_without_sentinel(): + """A non-admin scoping a team to concrete servers is unaffected by the gate.""" + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=["srv-x", "srv-y"], + existing_object_permission_id=None, + is_proxy_admin=False, + prisma_client=None, + ) + + +@pytest.mark.asyncio +async def test_enforce_all_proxy_mcp_grant_allows_non_admin_when_sentinel_already_set(): + """The gate blocks only NEW grants: a non-admin editing a team a proxy admin + already scoped to all-proxy is not forced to strip the sentinel, so unrelated + edits still succeed. The existing permission is read from the DB by id.""" + existing_row = MagicMock() + existing_row.mcp_servers = [SpecialMCPServerName.all_proxy_servers.value] + mock_repo = MagicMock() + mock_repo.table.find_unique = AsyncMock(return_value=existing_row) + + with patch( + "litellm.proxy.management_helpers.object_permission_utils.ObjectPermissionRepository", + return_value=mock_repo, + ): + await enforce_all_proxy_mcp_servers_grant_is_admin_only( + requested_mcp_servers=[SpecialMCPServerName.all_proxy_servers.value], + existing_object_permission_id="op-1", + is_proxy_admin=False, + prisma_client=MagicMock(), + ) + mock_repo.table.find_unique.assert_awaited_once() + + # ---- Tests for validate_key_search_tools_against_team ---- diff --git a/ui/litellm-dashboard/src/components/OldTeams.tsx b/ui/litellm-dashboard/src/components/OldTeams.tsx index 9c2106fa2fb..0ecbf39541b 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.tsx @@ -1479,6 +1479,7 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser value={form.getFieldValue("allowed_mcp_servers_and_groups")} accessToken={accessToken || ""} placeholder="Select MCP servers or access groups (optional)" + allowAllProxyMcpServers={isProxyAdminRole(userRole || "")} /> diff --git a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx index 1517b2dfa27..e6e778e49e5 100644 --- a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx @@ -3,7 +3,7 @@ import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders } from "../../../tests/test-utils"; import MCPServerSelector from "./MCPServerSelector"; -import { NO_MCP_SERVERS_SENTINEL } from "../mcp_tools/constants"; +import { ALL_PROXY_MCP_SERVERS_SENTINEL, NO_MCP_SERVERS_SENTINEL } from "../mcp_tools/constants"; vi.mock("@/app/(dashboard)/hooks/mcpServers/useMCPServers", () => ({ useMCPServers: vi.fn(), @@ -45,15 +45,19 @@ const mockUseMCPServers = vi.mocked(useMCPServers); const mockUseMCPAccessGroups = vi.mocked(useMCPAccessGroups); const mockUseMCPToolsets = vi.mocked(useMCPToolsets); +const setupMcpMocks = () => { + mockUseMCPServers.mockReturnValue({ + data: [{ server_id: "srv-1", server_name: "Server One" }], + isLoading: false, + } as any); + mockUseMCPAccessGroups.mockReturnValue({ data: [], isLoading: false } as any); + mockUseMCPToolsets.mockReturnValue({ data: [], isLoading: false } as any); +}; + describe("MCPServerSelector no-mcp-servers option", () => { beforeEach(() => { vi.clearAllMocks(); - mockUseMCPServers.mockReturnValue({ - data: [{ server_id: "srv-1", server_name: "Server One" }], - isLoading: false, - } as any); - mockUseMCPAccessGroups.mockReturnValue({ data: [], isLoading: false } as any); - mockUseMCPToolsets.mockReturnValue({ data: [], isLoading: false } as any); + setupMcpMocks(); }); const optionByValue = (value: string) => @@ -98,3 +102,70 @@ describe("MCPServerSelector no-mcp-servers option", () => { expect(optionByValue(NO_MCP_SERVERS_SENTINEL)?.disabled).toBe(false); }); }); + +describe("MCPServerSelector all-proxy-mcpservers option", () => { + beforeEach(() => { + vi.clearAllMocks(); + setupMcpMocks(); + }); + + const optionByValue = (value: string) => + Array.from(screen.getByTestId("mcp-select").querySelectorAll("option")).find( + (o) => (o as HTMLOptionElement).value === value, + ) as HTMLOptionElement | undefined; + + it("hides the All Proxy MCP Servers option by default", () => { + renderWithProviders( + , + ); + expect(optionByValue(ALL_PROXY_MCP_SERVERS_SENTINEL)).toBeUndefined(); + }); + + it("emits an exclusive sentinel when All Proxy MCP Servers is selected", async () => { + const onChange = vi.fn(); + renderWithProviders( + , + ); + expect(optionByValue(ALL_PROXY_MCP_SERVERS_SENTINEL)).toBeDefined(); + + await userEvent.selectOptions(screen.getByTestId("mcp-select"), [ALL_PROXY_MCP_SERVERS_SENTINEL]); + + expect(onChange).toHaveBeenCalledWith({ + servers: [ALL_PROXY_MCP_SERVERS_SENTINEL], + accessGroups: [], + toolsets: [], + }); + }); + + it("disables real server options while the sentinel is selected", () => { + renderWithProviders( + , + ); + expect(optionByValue("srv-1")?.disabled).toBe(true); + expect(optionByValue(ALL_PROXY_MCP_SERVERS_SENTINEL)?.disabled).toBe(false); + }); + + it("renders the friendly option, not the raw literal, when the sentinel is already stored but the flag is off", () => { + renderWithProviders( + , + ); + const option = optionByValue(ALL_PROXY_MCP_SERVERS_SENTINEL); + expect(option).toBeDefined(); + expect(option?.textContent).toContain("All Proxy MCP Servers"); + expect(optionByValue("srv-1")?.disabled).toBe(true); + }); +}); diff --git a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx index bbda761938e..f31f60d2084 100644 --- a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx +++ b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx @@ -3,7 +3,7 @@ import { useMCPServers } from "@/app/(dashboard)/hooks/mcpServers/useMCPServers" import { useMCPToolsets } from "@/app/(dashboard)/hooks/mcpServers/useMCPToolsets"; import { Select } from "antd"; import React from "react"; -import { NO_MCP_SERVERS_SENTINEL } from "@/components/mcp_tools/constants"; +import { ALL_PROXY_MCP_SERVERS_SENTINEL, NO_MCP_SERVERS_SENTINEL } from "@/components/mcp_tools/constants"; interface MCPServerSelectorProps { onChange: (selected: { servers: string[]; accessGroups: string[]; toolsets: string[] }) => void; @@ -18,6 +18,7 @@ interface MCPServerSelectorProps { disabled?: boolean; teamId?: string | null; allowNoMcpServers?: boolean; + allowAllProxyMcpServers?: boolean; } const TOOLSET_PREFIX = "toolset:"; @@ -31,6 +32,7 @@ const MCPServerSelector: React.FC = ({ disabled = false, teamId, allowNoMcpServers = false, + allowAllProxyMcpServers = false, }) => { const { data: mcpServers = [], isLoading: serversLoading } = useMCPServers(teamId); const { data: accessGroups = [], isLoading: groupsLoading } = useMCPAccessGroups(); @@ -81,9 +83,14 @@ const MCPServerSelector: React.FC = ({ ]; const hasNoMcpServersSelected = allowNoMcpServers && selectedValues.includes(NO_MCP_SERVERS_SENTINEL); + const hasAllProxyMcpServersSelected = selectedValues.includes(ALL_PROXY_MCP_SERVERS_SENTINEL); // Handle selection const handleChange = (selected: string[]) => { + if (allowAllProxyMcpServers && selected.includes(ALL_PROXY_MCP_SERVERS_SENTINEL)) { + onChange({ servers: [ALL_PROXY_MCP_SERVERS_SENTINEL], accessGroups: [], toolsets: [] }); + return; + } // "No MCP Servers" is exclusive: picking it clears everything else. if (allowNoMcpServers && selected.includes(NO_MCP_SERVERS_SENTINEL)) { onChange({ servers: [NO_MCP_SERVERS_SENTINEL], accessGroups: [], toolsets: [] }); @@ -113,10 +120,20 @@ const MCPServerSelector: React.FC = ({ disabled={disabled} filterOption={(input, option) => { if (option?.value === NO_MCP_SERVERS_SENTINEL) return true; + if (option?.value === ALL_PROXY_MCP_SERVERS_SENTINEL) return true; const searchText = options.find((opt) => opt.value === option?.value)?.searchText || ""; return searchText.toLowerCase().includes(input.toLowerCase()); }} > + {(allowAllProxyMcpServers || hasAllProxyMcpServersSelected) && ( + + All Proxy MCP Servers + + )} {allowNoMcpServers && (
@@ -126,7 +143,12 @@ const MCPServerSelector: React.FC = ({ )} {options.map((opt) => ( - +
{ // API should not be called without token expect(networking.fetchMCPServers).not.toHaveBeenCalled(); }); + + it("should display the All Proxy MCP Servers state instead of the raw sentinel string", async () => { + vi.mocked(networking.fetchMCPServers).mockResolvedValue([]); + + render( + , + ); + + expect(await screen.findByText("All Proxy MCP Servers")).toBeInTheDocument(); + expect(screen.getByText("All")).toBeInTheDocument(); + expect(screen.queryByText(ALL_PROXY_MCP_SERVERS_SENTINEL)).not.toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/permissions/MCPServerPermissions.tsx b/ui/litellm-dashboard/src/components/permissions/MCPServerPermissions.tsx index 9172ff92c69..4dfe41bb515 100644 --- a/ui/litellm-dashboard/src/components/permissions/MCPServerPermissions.tsx +++ b/ui/litellm-dashboard/src/components/permissions/MCPServerPermissions.tsx @@ -4,7 +4,7 @@ import { ServerIcon, ChevronDownIcon, ChevronRightIcon } from "@heroicons/react/ import { Tooltip } from "antd"; import { fetchMCPServers, fetchMCPToolsets } from "../networking"; import { MCPServer, MCPToolset } from "../mcp_tools/types"; -import { NO_MCP_SERVERS_SENTINEL } from "../mcp_tools/constants"; +import { ALL_PROXY_MCP_SERVERS_SENTINEL, NO_MCP_SERVERS_SENTINEL } from "../mcp_tools/constants"; interface MCPServerPermissionsProps { mcpServers: string[]; @@ -96,11 +96,12 @@ export function MCPServerPermissions({ }; const blocksAllMcpServers = mcpServers.includes(NO_MCP_SERVERS_SENTINEL); + const grantsAllProxyMcpServers = mcpServers.includes(ALL_PROXY_MCP_SERVERS_SENTINEL); // Merge servers and access groups into one list const mergedItems = [ ...mcpServers - .filter((server) => server !== NO_MCP_SERVERS_SENTINEL) + .filter((server) => server !== NO_MCP_SERVERS_SENTINEL && server !== ALL_PROXY_MCP_SERVERS_SENTINEL) .map((server) => ({ type: "server", value: server })), ...mcpAccessGroups.map((group) => ({ type: "accessGroup", value: group })), ]; @@ -112,7 +113,7 @@ export function MCPServerPermissions({ MCP Servers - {blocksAllMcpServers ? "Blocked" : totalCount} + {blocksAllMcpServers ? "Blocked" : grantsAllProxyMcpServers ? "All" : totalCount}
@@ -123,6 +124,11 @@ export function MCPServerPermissions({ No MCP servers — this key is blocked from all MCP servers, including its team's servers
+ ) : grantsAllProxyMcpServers ? ( +
+ + All Proxy MCP Servers +
) : totalCount > 0 ? (
{mergedItems.map((item, index) => { diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index c5b934d5432..3b80c598d34 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -1356,6 +1356,7 @@ const TeamInfoView: React.FC = ({ value={form.getFieldValue("mcp_servers_and_groups")} accessToken={accessToken || ""} placeholder="Select MCP servers or access groups (optional)" + allowAllProxyMcpServers={is_proxy_admin} />