diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx index 92f6f7554d4..d27c18c5ae3 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx @@ -154,16 +154,39 @@ describe("MCPNetworkSettings", () => { expect(screen.getByRole("textbox", { name: "Client 2 value" })).toHaveValue("codex-mcp-client"); }); - it("ignores a stored allowlist in the old plain-string shape instead of rendering it", async () => { + it("warns that a stored allowlist in the old plain-string shape denies every client and lets Save remove it", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, ]); renderSettings(); - await screen.findByText("Allowed Clients"); + expect(await screen.findByText(/stored allowlist is not a list of alias and value pairs/)).toBeVisible(); expect(screen.queryByRole("textbox", { name: "Client 1 value" })).not.toBeInTheDocument(); - expect(screen.queryByText(/every client is denied/)).not.toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients")); + expect(updateConfigFieldSetting).not.toHaveBeenCalled(); + await waitFor(() => expect(screen.queryByText(/stored allowlist is not a list/)).not.toBeInTheDocument()); + }); + + it("replaces a stored allowlist in the old plain-string shape with the clients the admin adds", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, + ]); + + renderSettings(); + + await screen.findByText(/stored allowlist is not a list of alias and value pairs/); + await addClient(ANTIGRAVITY.alias, ANTIGRAVITY.value); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY]), + ); + expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients"); + await waitFor(() => expect(screen.queryByText(/stored allowlist is not a list/)).not.toBeInTheDocument()); }); it("adds clients as alias and value pairs and saves them under mcp_allowed_clients", async () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx index 2ef3ee8707d..ae1fad36599 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx @@ -42,10 +42,20 @@ const isAllowedClient = (entry: unknown): entry is AllowedClient => { return typeof alias === "string" && typeof value === "string"; }; -const parseStoredClients = (fieldValue: unknown): AllowedClient[] | null => - Array.isArray(fieldValue) && fieldValue.every(isAllowedClient) - ? fieldValue.map(({ alias, value }) => ({ alias, value })) - : null; +type StoredAllowlist = + | { readonly kind: "absent" } + | { readonly kind: "clients"; readonly clients: AllowedClient[] } + | { readonly kind: "malformed" }; + +const ABSENT: StoredAllowlist = { kind: "absent" }; + +const parseStoredClients = (fieldValue: unknown): StoredAllowlist => { + if (fieldValue === null || fieldValue === undefined) return ABSENT; + if (Array.isArray(fieldValue) && fieldValue.every(isAllowedClient)) { + return { kind: "clients", clients: fieldValue.map(({ alias, value }) => ({ alias, value })) }; + } + return { kind: "malformed" }; +}; let nextRowKey = 0; const newRow = (client: AllowedClient = { alias: "", value: "" }): AllowedClientRow => ({ @@ -66,8 +76,16 @@ const sameClients = (a: AllowedClient[], b: AllowedClient[]) => const unchangedSinceLoad = (value: string[], stored: string[] | null) => stored === null ? value.length === 0 : value.length > 0 && sameList(value, stored); -const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: AllowedClient[] | null) => - stored === null ? value.length === 0 : value.length > 0 && sameClients(value, stored); +const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: StoredAllowlist) => { + switch (stored.kind) { + case "absent": + return value.length === 0; + case "clients": + return value.length > 0 && sameClients(value, stored.clients); + case "malformed": + return false; + } +}; const headerUnchangedSinceLoad = (value: string, stored: string | null) => stored === null ? value === "" : value !== "" && value === stored; @@ -79,7 +97,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const [allowedClients, setAllowedClients] = useState([]); const [clientIdHeader, setClientIdHeader] = useState(""); const [storedRanges, setStoredRanges] = useState(null); - const [storedClients, setStoredClients] = useState(null); + const [storedClients, setStoredClients] = useState(ABSENT); const [storedClientIdHeader, setStoredClientIdHeader] = useState(null); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); @@ -100,11 +118,9 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) setStoredRanges(field.field_value); } if (field.field_name === "mcp_allowed_clients") { - const clients = parseStoredClients(field.field_value); - if (clients !== null) { - setAllowedClients(clients.map(newRow)); - setStoredClients(clients); - } + const stored = parseStoredClients(field.field_value); + setAllowedClients(stored.kind === "clients" ? stored.clients.map(newRow) : []); + setStoredClients(stored); } if (field.field_name === "mcp_client_id_header" && typeof field.field_value === "string") { setClientIdHeader(field.field_value); @@ -145,11 +161,11 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) if (clientsUnchangedSinceLoad(clients, storedClients)) return; if (clients.length > 0) { await updateConfigFieldSetting(token, "mcp_allowed_clients", clients); - setStoredClients(clients); + setStoredClients({ kind: "clients", clients }); return; } await deleteConfigFieldSetting(token, "mcp_allowed_clients"); - setStoredClients(null); + setStoredClients(ABSENT); }; const persistClientIdHeader = async (token: string) => { @@ -216,7 +232,8 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) } const suggestedRange = currentIp ? ipToSlash24(currentIp) : null; - const storedAllowlistDeniesEveryone = storedClients !== null && storedClients.length === 0; + const storedAllowlistIsMalformed = storedClients.kind === "malformed"; + const storedAllowlistIsEmpty = storedClients.kind === "clients" && storedClients.clients.length === 0; return (
@@ -303,7 +320,13 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

Allowed Clients

- {storedAllowlistDeniesEveryone && ( + {storedAllowlistIsMalformed && ( +

+ The stored allowlist is not a list of alias and value pairs, so every client is denied. Add the clients you + want and save to replace it, or save with the list empty to remove it and allow every client again. +

+ )} + {storedAllowlistIsEmpty && (

An empty allowlist is currently stored, so every client is denied. Save with the list empty to remove it and allow every client again.