fix(ui): outer deny precedence and prune overrides for deselected servers

Co-Authored-By: bot_apk <apk@cognition.ai>

tests/test_litellm/proxy/_experimental/mcp_server/fixtures/mcp_tool_classification_cases.json

@@ -1,132 +1,28 @@
 [
-  {
-    "name": "delete_link",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "delete-link",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "deleteLink",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "DeleteLink",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "DELETE_LINK",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "remove_user",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "purge_cache",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "get_removed_entries",
-    "description": null,
-    "expected": "read"
-  },
-  {
-    "name": "list_deleted_items",
-    "description": null,
-    "expected": "read"
-  },
-  {
-    "name": "find_deleted",
-    "description": null,
-    "expected": "read"
-  },
-  {
-    "name": "describe_purge_job",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "updateItem",
-    "description": null,
-    "expected": "update"
-  },
-  {
-    "name": "create-record",
-    "description": null,
-    "expected": "create"
-  },
-  {
-    "name": "foo",
-    "description": "Deletes the file",
-    "expected": "delete"
-  },
-  {
-    "name": "foo",
-    "description": "Lists files",
-    "expected": "read"
-  },
-  {
-    "name": "delete_link",
-    "description": "Reads a link",
-    "expected": "delete"
-  },
-  {
-    "name": "get_removed_entries",
-    "description": "Permanently deletes",
-    "expected": "read"
-  },
-  {
-    "name": "unlinkNode",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "checkDeletion",
-    "description": null,
-    "expected": "read"
-  },
-  {
-    "name": "rm_file",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "x",
-    "description": null,
-    "expected": "unknown"
-  },
-  {
-    "name": "info",
-    "description": null,
-    "expected": "read"
-  },
-  {
-    "name": "settings",
-    "description": null,
-    "expected": "unknown"
-  },
-  {
-    "name": "get_and_delete_item",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "fetchAndDelete",
-    "description": null,
-    "expected": "delete"
-  },
-  {
-    "name": "read_then_remove",
-    "description": null,
-    "expected": "delete"
-  }
+  {"name": "delete_link", "description": null, "expected": "delete"},
+  {"name": "delete-link", "description": null, "expected": "delete"},
+  {"name": "deleteLink", "description": null, "expected": "delete"},
+  {"name": "DeleteLink", "description": null, "expected": "delete"},
+  {"name": "DELETE_LINK", "description": null, "expected": "delete"},
+  {"name": "remove_user", "description": null, "expected": "delete"},
+  {"name": "purge_cache", "description": null, "expected": "delete"},
+  {"name": "get_removed_entries", "description": null, "expected": "read"},
+  {"name": "list_deleted_items", "description": null, "expected": "read"},
+  {"name": "find_deleted", "description": null, "expected": "read"},
+  {"name": "describe_purge_job", "description": null, "expected": "delete"},
+  {"name": "updateItem", "description": null, "expected": "update"},
+  {"name": "create-record", "description": null, "expected": "create"},
+  {"name": "foo", "description": "Deletes the file", "expected": "delete"},
+  {"name": "foo", "description": "Lists files", "expected": "read"},
+  {"name": "delete_link", "description": "Reads a link", "expected": "delete"},
+  {"name": "get_removed_entries", "description": "Permanently deletes", "expected": "read"},
+  {"name": "unlinkNode", "description": null, "expected": "delete"},
+  {"name": "checkDeletion", "description": null, "expected": "read"},
+  {"name": "rm_file", "description": null, "expected": "delete"},
+  {"name": "x", "description": null, "expected": "unknown"},
+  {"name": "info", "description": null, "expected": "read"},
+  {"name": "settings", "description": null, "expected": "unknown"},
+  {"name": "get_and_delete_item", "description": null, "expected": "delete"},
+  {"name": "fetchAndDelete", "description": null, "expected": "delete"},
+  {"name": "read_then_remove", "description": null, "expected": "delete"}
 ]

ui/litellm-dashboard/src/components/Teams.tsx

@@ -40,6 +40,9 @@ import { fetchAvailableModelsForTeamOrKey } from "./key_team_helpers/fetch_avail
 import type { Team } from "./key_team_helpers/key_list";
 import MCPServerSelector from "./mcp_server_management/MCPServerSelector";
 import MCPToolPermissions from "./mcp_server_management/MCPToolPermissions";
+import { extractMcpEntitlement } from "./mcp_server_management/mcpEntitlement";
+import { useMCPServers } from "@/app/(dashboard)/hooks/mcpServers/useMCPServers";
+import { useMCPToolsets } from "@/app/(dashboard)/hooks/mcpServers/useMCPToolsets";
 import { toast } from "@/lib/toast";
 import { extractProxyErrorMessage } from "@/lib/http/client";
 import BudgetDurationDropdown, {
@@ -259,6 +262,8 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
   const watchedMcpSelection = form.watch("allowed_mcp_servers_and_groups");
   const watchedToolPermissions = form.watch("mcp_tool_permissions");
   const watchedToolOverrides = form.watch("mcp_tool_overrides");
+  const { data: allMcpServers = [] } = useMCPServers();
+  const { data: allMcpToolsets = [] } = useMCPToolsets();
 
   const [selectedTeam, setSelectedTeam] = useState<Team | null>(null);
   const [selectedTeamId, setSelectedTeamId] = useQueryState("team", parseAsString.withOptions({ history: "push" }));
@@ -485,7 +490,17 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
             delete formValues.mcp_tool_permissions;
           }
           if (formValues.mcp_tool_overrides && Object.keys(formValues.mcp_tool_overrides).length > 0) {
-            formValues.object_permission.mcp_tool_overrides = formValues.mcp_tool_overrides;
+            const entitlement = extractMcpEntitlement(
+              {
+                mcp_servers_and_groups: formValues.allowed_mcp_servers_and_groups,
+                mcp_tool_overrides: formValues.mcp_tool_overrides,
+              },
+              allMcpServers,
+              allMcpToolsets,
+            );
+            if (entitlement && Object.keys(entitlement.mcp_tool_overrides).length > 0) {
+              formValues.object_permission.mcp_tool_overrides = entitlement.mcp_tool_overrides;
+            }
             delete formValues.mcp_tool_overrides;
           }
         }

ui/litellm-dashboard/src/components/mcp_server_management/effectiveMcpServers.test.ts

@@ -12,6 +12,7 @@ import {
   mcpToolPermissionKeyFor,
   mcpToolState,
   resolveEffectiveMcpServers,
+  retainedMcpToolOverrides,
 } from "./effectiveMcpServers";
 
 const server = (overrides: Partial<MCPServer> & { server_id: string }): MCPServer =>
@@ -715,3 +716,29 @@ describe("convention servers and tool overrides", () => {
     expect(applyToolOverrideWrites(deselectAll)).toEqual({ "srv-1": { allow: [], deny: ["list_pages"] } });
   });
 });
+
+describe("retainedMcpToolOverrides", () => {
+  const catalog = [server({ server_id: "srv-1" }), server({ server_id: "srv-2", alias: "shared" })];
+
+  it("drops the override for a server the save no longer grants", () => {
+    expect(
+      retainedMcpToolOverrides(
+        { "srv-1": { allow: [], deny: ["list_pages"] }, "srv-2": { allow: ["delete_page"], deny: [] } },
+        new Set(["srv-2"]),
+        catalog,
+      ),
+    ).toEqual({ "srv-2": { allow: ["delete_page"], deny: [] } });
+  });
+
+  it("keeps an override while any server the key names stays granted", () => {
+    expect(
+      retainedMcpToolOverrides({ shared: { allow: [], deny: ["t"] } }, new Set(["srv-2"]), catalog),
+    ).toEqual({ shared: { allow: [], deny: ["t"] } });
+  });
+
+  it("keeps an override whose key resolves to nothing, so an unloaded catalog prunes nothing", () => {
+    expect(retainedMcpToolOverrides({ "not-yet-loaded": { allow: [], deny: ["t"] } }, new Set(), catalog)).toEqual(
+      { "not-yet-loaded": { allow: [], deny: ["t"] } },
+    );
+  });
+});

ui/litellm-dashboard/src/components/mcp_server_management/effectiveMcpServers.ts

@@ -271,8 +271,14 @@ export const mcpToolState = (
   isDeleteTool: boolean,
 ): McpToolState => {
   const denied = (entry.overrides?.deny ?? []).includes(toolName);
+  if (denied) {
+    if (entry.keyedTools !== undefined) {
+      return { checked: entry.keyedTools.includes(toolName), locked: false };
+    }
+    return { checked: false, locked: entry.toolsetTools !== undefined };
+  }
   if ((entry.toolsetTools ?? []).includes(toolName)) {
-    return { checked: !denied, locked: true };
+    return { checked: true, locked: true };
   }
   if (entry.keyedTools !== undefined) {
     return { checked: entry.keyedTools.includes(toolName), locked: false };
@@ -281,12 +287,24 @@ export const mcpToolState = (
     return { checked: false, locked: true };
   }
   const allowed = (entry.overrides?.allow ?? []).includes(toolName);
-  return { checked: (isDeleteTool ? false : !denied) || (allowed && !denied), locked: false };
+  return { checked: !isDeleteTool || allowed, locked: false };
 };
 
 export const isConventionServer = (entry: EffectiveMcpServer): boolean =>
   entry.keyedTools === undefined && entry.toolsetTools === undefined;
 
+export const retainedMcpToolOverrides = (
+  toolOverrides: Readonly<Record<string, McpToolOverrideEntry>>,
+  grantedServerIds: ReadonlySet<string>,
+  knownServers: readonly MCPServer[],
+): Record<string, McpToolOverrideEntry> =>
+  Object.fromEntries(
+    Object.entries(toolOverrides).filter(([permissionKey]) => {
+      const named = mcpServersForIdentifier(knownServers, permissionKey);
+      return named.length === 0 || named.some((server) => grantedServerIds.has(server.server_id));
+    }),
+  );
+
 export const applyToolOverrideWrite = ({
   toolOverrides,
   permissionKey,

ui/litellm-dashboard/src/components/team/TeamInfo.tsx

@@ -94,6 +94,7 @@ import {
   mcpServersForIdentifier,
   normalizeMcpToolOverrides,
   resolveEffectiveMcpServers,
+  retainedMcpToolOverrides,
   type EffectiveMcpServer,
 } from "../mcp_server_management/effectiveMcpServers";
 import type { MCPServer } from "../mcp_tools/types";
@@ -1109,6 +1110,11 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
         mcpResolution.kind === "resolved"
           ? retainedMcpToolPermissions(submittedToolPermissions, mcpResolution.serverIds, allMcpServers)
           : submittedToolPermissions;
+      const submittedToolOverrides = values.mcp_tool_overrides || {};
+      const mcpToolOverrides =
+        mcpResolution.kind === "resolved"
+          ? retainedMcpToolOverrides(submittedToolOverrides, mcpResolution.serverIds, allMcpServers)
+          : submittedToolOverrides;
 
       updateData.object_permission = {};
       if (servers) {
@@ -1120,8 +1126,8 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
       if (mcpToolPermissions) {
         updateData.object_permission.mcp_tool_permissions = mcpToolPermissions;
       }
-      if (values.mcp_tool_overrides && Object.keys(values.mcp_tool_overrides).length > 0) {
-        updateData.object_permission.mcp_tool_overrides = values.mcp_tool_overrides;
+      if (Object.keys(mcpToolOverrides).length > 0) {
+        updateData.object_permission.mcp_tool_overrides = mcpToolOverrides;
       }
       if (toolsets) {
         updateData.object_permission.mcp_toolsets = toolsets;

ui/litellm-dashboard/src/utils/mcpToolCrudClassification.ts

@@ -100,6 +100,7 @@ const tokenVariants = (token: string): string[] =>
 
 // Matches litellm/proxy/_experimental/mcp_server/tool_classification.py, fixture-pinned.
 const classifyTokens = (tokens: string[]): CrudOp => {
+  if (tokens.some((token) => DELETE_TOKENS.has(token))) return "delete";
   const variants = new Set(tokens.flatMap((token) => tokenVariants(token)));
   if ([...variants].some((variant) => READ_TOKENS.has(variant))) return "read";
   if ([...variants].some((variant) => DELETE_TOKENS.has(variant))) return "delete";