diff --git a/litellm-proxy-extras/litellm_proxy_extras/migrations/20260622120000_make_mcp_servers_nullable_jsonb/migration.sql b/litellm-proxy-extras/litellm_proxy_extras/migrations/20260622120000_make_mcp_servers_nullable_jsonb/migration.sql new file mode 100644 index 00000000000..a27de9b655b --- /dev/null +++ b/litellm-proxy-extras/litellm_proxy_extras/migrations/20260622120000_make_mcp_servers_nullable_jsonb/migration.sql @@ -0,0 +1,15 @@ +-- AlterTable +-- Convert object_permission.mcp_servers from a non-nullable String[] to a nullable +-- JSONB column so an explicit empty list ([]) can mean "no servers" distinctly from +-- NULL ("inherit team scope"). Prisma's auto-diff would DROP/ADD the column (data +-- loss); this in-place USING cast preserves data. Existing empty arrays meant +-- "inherit" under the old semantics, so they map to NULL (which also means "inherit" +-- under the new semantics) -- no live key or team changes behavior. +ALTER TABLE "LiteLLM_ObjectPermissionTable" + ALTER COLUMN "mcp_servers" DROP DEFAULT, + ALTER COLUMN "mcp_servers" TYPE JSONB USING ( + CASE + WHEN "mcp_servers" IS NULL OR cardinality("mcp_servers") = 0 THEN NULL + ELSE to_jsonb("mcp_servers") + END + ); diff --git a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma index e21c0016491..30a66775523 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma +++ b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma @@ -269,7 +269,7 @@ model LiteLLM_UserTable { model LiteLLM_ObjectPermissionTable { object_permission_id String @id @default(uuid()) - mcp_servers String[] @default([]) + mcp_servers Json? // null = inherit team scope; [] = no servers; ["id", ...] = exactly those servers mcp_access_groups String[] @default([]) mcp_tool_permissions Json? // Tool-level permissions for MCP servers. Format: {"server_id": ["tool_name_1", "tool_name_2"]} vector_stores String[] @default([]) diff --git a/litellm/models/object_permission.py b/litellm/models/object_permission.py index 6c0d100046c..952f0e6bf5b 100644 --- a/litellm/models/object_permission.py +++ b/litellm/models/object_permission.py @@ -14,7 +14,10 @@ class LiteLLM_ObjectPermissionTable(LiteLLMPydanticObjectBase): """Represents a LiteLLM_ObjectPermissionTable record""" object_permission_id: str - mcp_servers: Optional[List[str]] = [] + # None = inherit team scope; [] = no servers; ["id", ...] = exactly those. The + # default must stay None so the inherit/zero distinction survives cache + # round-trips (serialize drops None via exclude_none, deserialize restores it). + mcp_servers: Optional[List[str]] = None mcp_access_groups: Optional[List[str]] = [] mcp_tool_permissions: Optional[Dict[str, List[str]]] = None vector_stores: Optional[List[str]] = [] 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 e47fc84b533..8ecd49249cf 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 @@ -657,19 +657,24 @@ class MCPRequestHandler: ######################################################### # Calculate key/team allowed servers using inheritance and intersection logic ######################################################### - key_set = set(allowed_mcp_servers_for_key) + # allowed_mcp_servers_for_key is None when the key declares no scope of + # its own (inherit the team) and a list -- possibly empty -- when it does. + key_declares_scope = allowed_mcp_servers_for_key is not None + key_set = set(allowed_mcp_servers_for_key or []) team_set = set(allowed_mcp_servers_for_team) grants_set = set(key_access_group_grants) - has_lower_level_mcp_restrictions = bool(key_set or team_set or grants_set) + has_lower_level_mcp_restrictions = bool( + key_declares_scope or team_set or grants_set + ) # 1. Key/team ceiling. An empty set means "this level does not restrict". - if not team_set: - base = key_set # no team restriction - elif not key_set: - base = team_set # key has no own perms → inherits team + if not key_declares_scope: + base = team_set # key inherits the team (team_set may be empty) + elif not team_set: + base = key_set # key restricts, team does not else: - base = key_set & team_set # both restrict → intersect + base = key_set & team_set # both restrict → intersect ([] → zero) # 2. Add the key's access-group grants on top. These are additive: # attaching a group to the key grants its servers regardless of the @@ -1014,18 +1019,23 @@ class MCPRequestHandler: @staticmethod async def _get_allowed_mcp_servers_for_key( user_api_key_auth: Optional[UserAPIKeyAuth] = None, - ) -> List[str]: + ) -> Optional[List[str]]: """ Get the key's own MCP ceiling from its object_permission (mcp_servers, tag-style mcp_access_groups, mcp_tool_permissions). + Returns None when the key declares no MCP scope of its own (so it inherits + the team's scope) and a list -- possibly empty -- when it does. An explicit + empty list means "no servers", which intersects the team down to nothing + instead of inheriting it. + Unified key.access_group_ids are NOT resolved here — they are additive grants handled by _get_key_access_group_mcp_server_extras and unioned on top of the key/team ceiling, so they must not enter this scope (which is intersected against the team). """ if user_api_key_auth is None: - return [] + return None try: from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( global_mcp_server_manager, @@ -1056,7 +1066,11 @@ class MCPRequestHandler: proxy_logging_obj=proxy_logging_obj, ) if key_object_permission is None: - return [] + return None + + # None means the key never set its own list (inherit); [] means it + # explicitly scoped itself to no servers. + direct_scope_declared = key_object_permission.mcp_servers is not None # Permission entries may be server_ids OR names/aliases — expand to ids. direct_mcp_servers = global_mcp_server_manager.expand_permission_list( @@ -1079,12 +1093,16 @@ class MCPRequestHandler: # Combine all lists all_servers = direct_mcp_servers + access_group_servers + tool_perm_servers - return list(set(all_servers)) + if all_servers: + return list(set(all_servers)) + + # Nothing resolved: preserve the inherit-vs-zero distinction. + return [] if direct_scope_declared else None except Exception as e: verbose_logger.warning( f"Failed to get allowed MCP servers for key: {str(e)}" ) - return [] + return None @staticmethod async def _get_allowed_mcp_servers_for_team( diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index 8edb831a9df..0f4f2ef23a3 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -516,19 +516,22 @@ async def get_objectpermissions_for_mcp_server( """ Get all the object permissions records and the associated team and verficiationtoken records that have access to the mcp server """ + # mcp_servers is a nullable JSON column (null = inherit, [] = none, [ids] = those), + # so it can't be filtered with the scalar-list `has` operator; match in Python. object_permission_records = await ObjectPermissionRepository( prisma_client ).table.find_many( - where={ - "mcp_servers": {"has": mcp_server_id}, - }, include={ "teams": True, "verification_tokens": True, }, ) - return object_permission_records + return [ + record + for record in object_permission_records + if isinstance(record.mcp_servers, list) and mcp_server_id in record.mcp_servers + ] async def get_virtualkeys_for_mcp_server( diff --git a/litellm/proxy/management_helpers/object_permission_utils.py b/litellm/proxy/management_helpers/object_permission_utils.py index f2ddae40d8c..46a6ef68622 100644 --- a/litellm/proxy/management_helpers/object_permission_utils.py +++ b/litellm/proxy/management_helpers/object_permission_utils.py @@ -136,6 +136,12 @@ async def handle_update_object_permission_common( existing_object_permissions_dict["mcp_tool_permissions"] ) + # mcp_servers is a JSON column; serialize the list (None stays NULL = inherit). + if isinstance(existing_object_permissions_dict.get("mcp_servers"), list): + existing_object_permissions_dict["mcp_servers"] = safe_dumps( + existing_object_permissions_dict["mcp_servers"] + ) + ######################################################### # Commit the update to the LiteLLM_ObjectPermissionTable ######################################################### @@ -185,6 +191,10 @@ async def _set_object_permission( clean_data["mcp_tool_permissions"] ) + # mcp_servers is a JSON column; serialize the list (None was already dropped above). + if isinstance(clean_data.get("mcp_servers"), list): + clean_data["mcp_servers"] = safe_dumps(clean_data["mcp_servers"]) + created_permission = await ObjectPermissionRepository(prisma_client).table.create( data=clean_data ) diff --git a/litellm/proxy/schema.prisma b/litellm/proxy/schema.prisma index e21c0016491..30a66775523 100644 --- a/litellm/proxy/schema.prisma +++ b/litellm/proxy/schema.prisma @@ -269,7 +269,7 @@ model LiteLLM_UserTable { model LiteLLM_ObjectPermissionTable { object_permission_id String @id @default(uuid()) - mcp_servers String[] @default([]) + mcp_servers Json? // null = inherit team scope; [] = no servers; ["id", ...] = exactly those servers mcp_access_groups String[] @default([]) mcp_tool_permissions Json? // Tool-level permissions for MCP servers. Format: {"server_id": ["tool_name_1", "tool_name_2"]} vector_stores String[] @default([]) diff --git a/schema.prisma b/schema.prisma index e21c0016491..30a66775523 100644 --- a/schema.prisma +++ b/schema.prisma @@ -269,7 +269,7 @@ model LiteLLM_UserTable { model LiteLLM_ObjectPermissionTable { object_permission_id String @id @default(uuid()) - mcp_servers String[] @default([]) + mcp_servers Json? // null = inherit team scope; [] = no servers; ["id", ...] = exactly those servers mcp_access_groups String[] @default([]) mcp_tool_permissions Json? // Tool-level permissions for MCP servers. Format: {"server_id": ["tool_name_1", "tool_name_2"]} vector_stores String[] @default([]) diff --git a/tests/mcp_tests/test_mcp_server.py b/tests/mcp_tests/test_mcp_server.py index eea2f2721ab..e17cf75879e 100644 --- a/tests/mcp_tests/test_mcp_server.py +++ b/tests/mcp_tests/test_mcp_server.py @@ -2649,7 +2649,7 @@ async def test_mcp_access_group_permission_inheritance_integration(): MCPRequestHandler, "_get_allowed_mcp_servers_for_team" ) as mock_team: # Key has no permissions, team has servers - mock_key.return_value = [] # Key inherits nothing directly + mock_key.return_value = None # Key declares no scope -> inherit team mock_team.return_value = [ "staff-server-1", "staff-server-2", diff --git a/tests/test_litellm/models/test_models.py b/tests/test_litellm/models/test_models.py index 786f6244930..f7c8c783780 100644 --- a/tests/test_litellm/models/test_models.py +++ b/tests/test_litellm/models/test_models.py @@ -161,6 +161,35 @@ class TestObjectPermission: ) assert perm.mcp_tool_permissions == {"server1": ["tool1", "tool2"]} + @pytest.mark.parametrize( + "mcp_servers,scenario", + [ + (None, "none_inherits_team"), + ([], "explicit_empty_is_zero"), + ], + ) + def test_object_permission_mcp_servers_survive_cache_round_trip( + self, mcp_servers, scenario + ): + """A Redis round-trip serializes through ``model_dump(mode='json', + exclude_none=True)`` and re-validates the dict. ``mcp_servers`` must + keep the inherit (None) vs explicit-zero ([]) distinction across that + boundary, since the MCP permission logic branches on exactly that.""" + from litellm.proxy.common_utils.cache_pydantic_utils import CacheCodec + + perm = LiteLLM_ObjectPermissionTable( + object_permission_id="x", + mcp_servers=mcp_servers, + ) + + payload = CacheCodec.serialize(perm) + restored = CacheCodec.deserialize( + payload, model_type=LiteLLM_ObjectPermissionTable + ) + + assert restored is not None + assert restored.mcp_servers == mcp_servers, f"scenario={scenario}" + class TestOrganization: def test_organization_creation(self): 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 ab42ee1e979..9678ad5c7a4 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 @@ -29,7 +29,7 @@ class TestMCPRequestHandler: (["server1", "server2"], [], ["server1", "server2"], "key_only"), # Test case 3: No key servers, team has servers (inherit from team) ( - [], + None, ["team_server1", "team_server2"], ["team_server1", "team_server2"], "inherit_from_team", @@ -100,7 +100,7 @@ class TestMCPRequestHandler: "team_servers,key_servers,expected_servers,scenario", [ # Test case 1: Key has no permissions, should inherit from team - (["server1", "server2"], [], ["server1", "server2"], "inherit_from_team"), + (["server1", "server2"], None, ["server1", "server2"], "inherit_from_team"), # Test case 2: Key has permissions, should use intersection with team ( ["server1", "server2", "server3"], @@ -203,7 +203,7 @@ class TestMCPRequestHandler: # granted only via key.access_group_ids → caller sees team's server # AND the grant (grant is added on top of the ceiling). ( - [], + None, ["test"], ["context7"], ["context7", "test"], @@ -251,6 +251,53 @@ class TestMCPRequestHandler: result = await MCPRequestHandler.get_allowed_mcp_servers(mock_user_auth) assert sorted(result) == sorted(expected) + @pytest.mark.parametrize( + "key_servers,expected,scenario", + [ + # Explicit [] means the key scoped itself to zero servers: it + # intersects the populated team down to nothing rather than + # inheriting it. This is the headline behavior the fix introduces. + ([], [], "explicit_empty_key_scopes_to_zero"), + # None means the key declares no scope of its own → inherit the team. + (None, ["alpha", "beta"], "none_key_inherits_team"), + # A real subset intersects against the team. + (["alpha"], ["alpha"], "subset_key_intersects_team"), + ], + ) + async def test_get_allowed_mcp_servers_key_scope_tristate( + self, key_servers, expected, scenario + ): + """The key's MCP scope is tri-state: explicit ``[]`` (zero servers), + ``None`` (inherit team), or a concrete list (intersect). A populated + team must NOT rescue an explicitly-zero-scoped key.""" + mock_user_auth = UserAPIKeyAuth( + api_key="test-key", + user_id="test-user", + team_id="test-team", + ) + with ( + patch.object( + MCPRequestHandler, + "_get_allowed_mcp_servers_for_key", + new_callable=AsyncMock, + return_value=key_servers, + ), + patch.object( + MCPRequestHandler, + "_get_allowed_mcp_servers_for_team", + new_callable=AsyncMock, + return_value=["alpha", "beta"], + ), + patch.object( + MCPRequestHandler, + "_get_key_access_group_mcp_server_extras", + new_callable=AsyncMock, + return_value=[], + ), + ): + result = await MCPRequestHandler.get_allowed_mcp_servers(mock_user_auth) + assert sorted(result) == sorted(expected), f"scenario={scenario}" + async def test_access_group_extras_returns_empty_when_no_auth(self): """No auth object → no additive grants.""" result = await MCPRequestHandler._get_key_access_group_mcp_server_extras(None) @@ -3113,7 +3160,7 @@ async def test_get_allowed_mcp_servers_for_team_without_team_id_returns_empty(): async def test_get_allowed_mcp_servers_for_key_guard_conditions( user_api_key_auth, prisma_client_value, scenario ): - """Ensure guard clauses return [] before hitting get_object_permission.""" + """Ensure guard clauses return None (inherit team) before hitting get_object_permission.""" with patch( "litellm.proxy.auth.auth_checks.get_object_permission", @@ -3124,13 +3171,13 @@ async def test_get_allowed_mcp_servers_for_key_guard_conditions( user_api_key_auth ) - assert result == [] + assert result is None mock_get_perm.assert_not_called() @pytest.mark.asyncio -async def test_get_allowed_mcp_servers_for_key_returns_empty_when_db_returns_none(): - """Ensure [] is returned when get_object_permission yields None.""" +async def test_get_allowed_mcp_servers_for_key_returns_none_when_db_returns_none(): + """Ensure None is returned (inherit team) when get_object_permission yields None.""" user_api_key_auth = UserAPIKeyAuth( api_key="test-key", @@ -3153,7 +3200,7 @@ async def test_get_allowed_mcp_servers_for_key_returns_empty_when_db_returns_non user_api_key_auth ) - assert result == [] + assert result is None mock_get_perm.assert_awaited_once() @@ -3210,6 +3257,62 @@ async def test_get_allowed_mcp_servers_for_key_prefers_in_memory_permission(): global_mcp_server_manager.registry.pop("direct-server", None) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "mcp_servers,expected,scenario", + [ + # None on the object_permission means the key never set its own list → + # inherit the team (the helper signals this with None). + (None, None, "mcp_servers_none_inherits_team"), + # An explicit empty list means the key scoped itself to zero servers → + # the helper returns [] so the combination logic intersects to zero. + ([], [], "mcp_servers_explicit_empty_is_zero"), + # A concrete list passes through (expand_permission_list keeps unknown + # ids as-is so we don't need a registered server here). + (["x"], ["x"], "mcp_servers_concrete_list"), + ], +) +async def test_get_allowed_mcp_servers_for_key_tristate_from_object_permission( + mcp_servers, expected, scenario +): + """``_get_allowed_mcp_servers_for_key`` must preserve the inherit (None) vs + explicit-zero ([]) vs concrete-list distinction from the key's + object_permission.mcp_servers when there are no access groups or tool + permissions.""" + from litellm.proxy._types import LiteLLM_ObjectPermissionTable + + perms = LiteLLM_ObjectPermissionTable( + object_permission_id="perm-tristate", + mcp_servers=mcp_servers, + mcp_access_groups=[], + mcp_tool_permissions=None, + ) + user_api_key_auth = UserAPIKeyAuth( + api_key="test-key", + user_id="test-user", + object_permission=perms, + ) + + with ( + patch( + "litellm.proxy.auth.auth_checks.get_object_permission", + new_callable=AsyncMock, + ) as mock_get_perm, + patch.object( + MCPRequestHandler, + "_get_mcp_servers_from_access_groups", + new_callable=AsyncMock, + return_value=[], + ), + ): + result = await MCPRequestHandler._get_allowed_mcp_servers_for_key( + user_api_key_auth + ) + + assert result == expected, f"scenario={scenario}" + mock_get_perm.assert_not_called() + + @pytest.mark.asyncio class TestAgentMCPPermissions: """Test agent-level MCP server and tool permission intersection.""" @@ -3526,7 +3629,7 @@ class TestOrgMCPPermissions: "org_empty_no_restriction", ), ( - [], + None, [], ["org_s1", "org_s2"], ["org_s1", "org_s2"], @@ -4005,7 +4108,7 @@ async def test_get_allowed_mcp_servers_unions_key_access_group_extras(): MCPRequestHandler, "_get_allowed_mcp_servers_for_key", new_callable=AsyncMock, - return_value=[], + return_value=None, ), patch.object( MCPRequestHandler, @@ -4222,7 +4325,7 @@ async def test_get_allowed_mcp_servers_includes_team_access_group_extras_end_to_ MCPRequestHandler, "_get_allowed_mcp_servers_for_key", new_callable=AsyncMock, - return_value=[], + return_value=None, ), patch.object( MCPRequestHandler, @@ -4264,7 +4367,7 @@ async def test_allowed_mcp_servers_for_key_excludes_access_group_ids(): ): result = await MCPRequestHandler._get_allowed_mcp_servers_for_key(auth) - assert result == [] + assert result is None mock_resolver.assert_not_called() diff --git a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx index ba7a1120e87..b15304c68f8 100644 --- a/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx +++ b/ui/litellm-dashboard/src/components/organisms/create_key_button.tsx @@ -451,25 +451,31 @@ const CreateKey: React.FC = ({ team, teams, data, addKey, autoOp delete formValues.allowed_vector_store_ids; } - // Transform allowed_mcp_servers_and_groups into object_permission format - if ( - formValues.allowed_mcp_servers_and_groups && - (formValues.allowed_mcp_servers_and_groups.servers?.length > 0 || - formValues.allowed_mcp_servers_and_groups.accessGroups?.length > 0) - ) { + // Transform the MCP scope mode + selector into object_permission format. + // The mode is tri-state: "inherit" omits mcp_servers (backend inherits the + // team's servers), "none" sets it to [] (explicitly zero), and "specific" + // sets the selected servers/access groups/toolsets. + const mcpScopeMode = formValues.mcp_scope_mode ?? "inherit"; + if (mcpScopeMode === "none") { if (!formValues.object_permission) { formValues.object_permission = {}; } - const { servers, accessGroups } = formValues.allowed_mcp_servers_and_groups; - if (servers && servers.length > 0) { - formValues.object_permission.mcp_servers = servers; + formValues.object_permission.mcp_servers = []; + } else if (mcpScopeMode === "specific") { + const { servers, accessGroups, toolsets } = formValues.allowed_mcp_servers_and_groups ?? {}; + if (!formValues.object_permission) { + formValues.object_permission = {}; } + formValues.object_permission.mcp_servers = servers ?? []; if (accessGroups && accessGroups.length > 0) { formValues.object_permission.mcp_access_groups = accessGroups; } - // Remove the original field as it's now part of object_permission - delete formValues.allowed_mcp_servers_and_groups; + if (toolsets && toolsets.length > 0) { + formValues.object_permission.mcp_toolsets = toolsets; + } } + delete formValues.mcp_scope_mode; + delete formValues.allowed_mcp_servers_and_groups; // Add MCP tool permissions to object_permission const mcpToolPermissions = formValues.mcp_tool_permissions || {}; @@ -1385,46 +1391,81 @@ const CreateKey: React.FC = ({ team, teams, data, addKey, autoOp - Allowed MCP Servers{" "} - + MCP Servers / Access Groups{" "} + } - name="allowed_mcp_servers_and_groups" - help="Select MCP servers or access groups this key can access" + name="mcp_scope_mode" + initialValue="inherit" > - form.setFieldValue("allowed_mcp_servers_and_groups", val)} - value={form.getFieldValue("allowed_mcp_servers_and_groups")} - accessToken={accessToken} - teamId={selectedCreateKeyTeam?.team_id ?? null} - placeholder="Select MCP servers or access groups (optional)" - /> - - - {/* Hidden field to register mcp_tool_permissions with the form */} - - prevValues.allowed_mcp_servers_and_groups !== currentValues.allowed_mcp_servers_and_groups || - prevValues.mcp_tool_permissions !== currentValues.mcp_tool_permissions + prevValues.mcp_scope_mode !== currentValues.mcp_scope_mode } > - {() => ( -
- form.setFieldsValue({ mcp_tool_permissions: toolPerms })} - /> -
- )} + {() => + form.getFieldValue("mcp_scope_mode") === "specific" && ( + <> + + Allowed MCP Servers{" "} + + + + + } + name="allowed_mcp_servers_and_groups" + help="Select MCP servers or access groups this key can access" + > + form.setFieldValue("allowed_mcp_servers_and_groups", val)} + value={form.getFieldValue("allowed_mcp_servers_and_groups")} + accessToken={accessToken} + teamId={selectedCreateKeyTeam?.team_id ?? null} + placeholder="Select MCP servers or access groups (optional)" + /> + + + + prevValues.allowed_mcp_servers_and_groups !== + currentValues.allowed_mcp_servers_and_groups || + prevValues.mcp_tool_permissions !== currentValues.mcp_tool_permissions + } + > + {() => ( +
+ form.setFieldsValue({ mcp_tool_permissions: toolPerms })} + /> +
+ )} + + + ) + } + + + {/* Hidden field to register mcp_tool_permissions with the form */} + diff --git a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx index fe0fa1ab0c2..cb9f6c7102b 100644 --- a/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_edit_view.tsx @@ -5,7 +5,7 @@ import { useUISettings } from "@/app/(dashboard)/hooks/uiSettings/useUISettings" import PolicySelector from "@/components/policies/PolicySelector"; import { InfoCircleOutlined } from "@ant-design/icons"; import { TextInput, Button as TremorButton } from "@tremor/react"; -import { Form, Input, Select, Switch, Tooltip } from "antd"; +import { Form, Input, Radio, Select, Switch, Tooltip } from "antd"; import { useEffect, useState } from "react"; import { rolesWithWriteAccess } from "../../utils/roles"; import AgentSelector from "../agent_management/AgentSelector"; @@ -78,6 +78,21 @@ const getKeyTypeFromRoutes = (allowedRoutes: string[] | null | undefined): strin return "default"; }; +type McpScopeMode = "inherit" | "none" | "specific"; + +const getMcpScopeMode = (keyData: KeyResponse): McpScopeMode => { + const mcpServers = keyData.object_permission?.mcp_servers; + const hasAccessGroups = (keyData.object_permission?.mcp_access_groups?.length ?? 0) > 0; + + if (mcpServers == null && !hasAccessGroups) { + return "inherit"; + } + if (Array.isArray(mcpServers) && mcpServers.length === 0 && !hasAccessGroups) { + return "none"; + } + return "specific"; +}; + export function KeyEditView({ keyData, onCancel, @@ -179,6 +194,7 @@ export function KeyEditView({ prompts: keyData.metadata?.prompts, tags: keyData.metadata?.tags, vector_stores: keyData.object_permission?.vector_stores || [], + mcp_scope_mode: getMcpScopeMode(keyData), mcp_servers_and_groups: { servers: keyData.object_permission?.mcp_servers || [], accessGroups: keyData.object_permission?.mcp_access_groups || [], @@ -212,6 +228,7 @@ export function KeyEditView({ prompts: keyData.metadata?.prompts, tags: keyData.metadata?.tags, vector_stores: keyData.object_permission?.vector_stores || [], + mcp_scope_mode: getMcpScopeMode(keyData), mcp_servers_and_groups: { servers: keyData.object_permission?.mcp_servers || [], accessGroups: keyData.object_permission?.mcp_access_groups || [], @@ -612,13 +629,22 @@ export function KeyEditView({ /> - - form.setFieldValue("mcp_servers_and_groups", val)} - value={form.getFieldValue("mcp_servers_and_groups")} - accessToken={accessToken || ""} - placeholder="Select MCP servers or access groups (optional)" - /> + + MCP Servers / Access Groups{" "} + + + + + } + name="mcp_scope_mode" + > + + Inherit from team + No MCP access + Specific servers + {/* Hidden field to register mcp_tool_permissions with the form */} @@ -629,20 +655,34 @@ export function KeyEditView({ + prevValues.mcp_scope_mode !== currentValues.mcp_scope_mode || prevValues.mcp_servers_and_groups !== currentValues.mcp_servers_and_groups || prevValues.mcp_tool_permissions !== currentValues.mcp_tool_permissions } > - {() => ( -
- form.setFieldsValue({ mcp_tool_permissions: toolPerms })} - /> -
- )} + {() => + form.getFieldValue("mcp_scope_mode") === "specific" && ( + <> + + form.setFieldValue("mcp_servers_and_groups", val)} + value={form.getFieldValue("mcp_servers_and_groups")} + accessToken={accessToken || ""} + placeholder="Select MCP servers or access groups (optional)" + /> + + +
+ form.setFieldsValue({ mcp_tool_permissions: toolPerms })} + /> +
+ + ) + } diff --git a/ui/litellm-dashboard/src/components/templates/key_info_view.tsx b/ui/litellm-dashboard/src/components/templates/key_info_view.tsx index 018880b70aa..85b2bafaf37 100644 --- a/ui/litellm-dashboard/src/components/templates/key_info_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_info_view.tsx @@ -195,7 +195,13 @@ export default function KeyInfoView({ delete formValues.vector_stores; } - if (formValues.mcp_servers_and_groups !== undefined) { + // MCP scope is tri-state: "inherit" sends mcp_servers = null (backend inherits + // the team's servers), "none" sends [] (explicitly zero), and "specific" sends + // the selected servers/access groups/toolsets. The mode is the source of truth, + // so switching specific -> inherit correctly resets a previously-set scope. + const mcpScopeMode = formValues.mcp_scope_mode; + const isSpecificMcpScope = mcpScopeMode === "specific"; + if (mcpScopeMode !== undefined) { const { servers, accessGroups, toolsets } = formValues.mcp_servers_and_groups || { servers: [], accessGroups: [], @@ -203,18 +209,18 @@ export default function KeyInfoView({ }; formValues.object_permission = { ...currentKeyData.object_permission, - mcp_servers: servers || [], - mcp_access_groups: accessGroups || [], - mcp_toolsets: toolsets || [], + mcp_servers: isSpecificMcpScope ? servers || [] : mcpScopeMode === "none" ? [] : null, + mcp_access_groups: isSpecificMcpScope ? accessGroups || [] : [], + mcp_toolsets: isSpecificMcpScope ? toolsets || [] : [], }; - // Remove mcp_servers_and_groups from the top level as it should be in object_permission - delete formValues.mcp_servers_and_groups; } + delete formValues.mcp_scope_mode; + delete formValues.mcp_servers_and_groups; - // Handle MCP tool permissions + // Tool permissions only apply when specific servers are chosen; clear them otherwise. if (formValues.mcp_tool_permissions !== undefined) { const mcpToolPermissions = formValues.mcp_tool_permissions || {}; - if (Object.keys(mcpToolPermissions).length > 0) { + if (isSpecificMcpScope && Object.keys(mcpToolPermissions).length > 0) { formValues.object_permission = { ...formValues.object_permission, mcp_tool_permissions: mcpToolPermissions,