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 b6521acd7e2..298e23526a4 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 @@ -2,6 +2,7 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import MCPNetworkSettings from "./MCPNetworkSettings"; +import { toast } from "@/lib/toast"; import { getGeneralSettingsCall, updateConfigFieldSetting, @@ -16,6 +17,10 @@ vi.mock("@/components/networking", () => ({ fetchMCPClientIp: vi.fn(), })); +vi.mock("@/lib/toast", () => ({ + toast: { success: vi.fn(), fromError: vi.fn() }, +})); + const renderSettings = () => render(); describe("MCPNetworkSettings", () => { @@ -82,25 +87,44 @@ describe("MCPNetworkSettings", () => { expect(screen.getByText("203.0.113.0/24")).toBeInTheDocument(); }); - it("saves the configured ranges", async () => { + it("saves the configured ranges once they change", async () => { + vi.mocked(fetchMCPClientIp).mockResolvedValue("203.0.113.45"); vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_internal_ip_ranges", field_value: ["10.0.0.0/8"] }, ]); renderSettings(); + await userEvent.click(await screen.findByText("203.0.113.0/24")); await userEvent.click(await screen.findByRole("button", { name: /Save/ })); await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", ["10.0.0.0/8"]), + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", [ + "10.0.0.0/8", + "203.0.113.0/24", + ]), ); expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges"); }); - it("clears the setting instead of saving an empty list", async () => { + it("clears a stored range setting instead of saving an empty list", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_internal_ip_ranges", field_value: ["10.0.0.0/8"] }, + ]); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: "Remove 10.0.0.0/8" })); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges")); + expect(updateConfigFieldSetting).not.toHaveBeenCalled(); + }); + + it("does not write settings that were never stored and are still empty", async () => { renderSettings(); await userEvent.click(await screen.findByRole("button", { name: /Save/ })); - await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges")); + await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved")); + expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); expect(updateConfigFieldSetting).not.toHaveBeenCalled(); }); @@ -159,12 +183,38 @@ describe("MCPNetworkSettings", () => { ]); renderSettings(); - await userEvent.click(await screen.findByRole("button", { name: /Save/ })); + await userEvent.type( + await screen.findByRole("textbox", { name: "Allowed client names" }), + "codex-mcp-client{Enter}", + ); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", ["antigravity-cli"]), + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ + "antigravity-cli", + "codex-mcp-client", + ]), ); - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", ["10.0.0.0/8"]); + expect(updateConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", expect.anything()); expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); }); + + it("still saves the allowed clients when the private range write fails, and reports the failure", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_internal_ip_ranges", field_value: ["10.0.0.0/8"] }, + ]); + const rangeFailure = new Error("Field name=mcp_internal_ip_ranges not in config"); + vi.mocked(deleteConfigFieldSetting).mockRejectedValue(rangeFailure); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: "Remove 10.0.0.0/8" })); + await userEvent.type(screen.getByRole("textbox", { name: "Allowed client names" }), "codex-mcp-client{Enter}"); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", ["codex-mcp-client"]), + ); + await waitFor(() => expect(toast.fromError).toHaveBeenCalledWith(rangeFailure)); + expect(toast.success).not.toHaveBeenCalled(); + }); }); 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 18377a4bb82..74f203227b7 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 @@ -6,6 +6,7 @@ import { Card } from "@/components/ui/card"; import { Input } from "@/components/ui/input"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import { DeprecationBanner } from "@/components/DeprecationBanner"; +import { toast } from "@/lib/toast"; import { getGeneralSettingsCall, updateConfigFieldSetting, @@ -26,11 +27,15 @@ function ipToSlash24(ip: string): string { return `${parts[0]}.${parts[1]}.${parts[2]}.0/24`; } +const sameList = (a: string[], b: string[]) => a.length === b.length && a.every((value, i) => value === b[i]); + const MCPNetworkSettings: React.FC = ({ accessToken }) => { const [loading, setLoading] = useState(true); const [saving, setSaving] = useState(false); const [privateRanges, setPrivateRanges] = useState([]); const [allowedClients, setAllowedClients] = useState([]); + const [storedRanges, setStoredRanges] = useState([]); + const [storedClients, setStoredClients] = useState([]); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); const [clientDraft, setClientDraft] = useState(""); @@ -48,9 +53,11 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) for (const field of settings) { if (field.field_name === "mcp_internal_ip_ranges" && field.field_value) { setPrivateRanges(field.field_value); + setStoredRanges(field.field_value); } if (field.field_name === "mcp_allowed_clients" && field.field_value) { setAllowedClients(field.field_value); + setStoredClients(field.field_value); } } } catch (error) { @@ -68,25 +75,42 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) } }; + const persistList = async ( + token: string, + fieldName: "mcp_internal_ip_ranges" | "mcp_allowed_clients", + { value, stored, setStored }: { value: string[]; stored: string[]; setStored: (value: string[]) => void }, + ) => { + if (sameList(value, stored)) return; + if (value.length > 0) { + await updateConfigFieldSetting(token, fieldName, value); + } else { + await deleteConfigFieldSetting(token, fieldName); + } + setStored(value); + }; + const handleSave = async () => { if (!accessToken) return; setSaving(true); - try { - if (privateRanges.length > 0) { - await updateConfigFieldSetting(accessToken, "mcp_internal_ip_ranges", privateRanges); - } else { - await deleteConfigFieldSetting(accessToken, "mcp_internal_ip_ranges"); - } - if (allowedClients.length > 0) { - await updateConfigFieldSetting(accessToken, "mcp_allowed_clients", allowedClients); - } else { - await deleteConfigFieldSetting(accessToken, "mcp_allowed_clients"); - } - } catch (error) { - console.error("Failed to save MCP network settings:", error); - } finally { - setSaving(false); + const results = await Promise.allSettled([ + persistList(accessToken, "mcp_internal_ip_ranges", { + value: privateRanges, + stored: storedRanges, + setStored: setStoredRanges, + }), + persistList(accessToken, "mcp_allowed_clients", { + value: allowedClients, + stored: storedClients, + setStored: setStoredClients, + }), + ]); + setSaving(false); + const failures = results.filter((result): result is PromiseRejectedResult => result.status === "rejected"); + if (failures.length === 0) { + toast.success("MCP network settings saved"); + return; } + failures.forEach((failure) => toast.fromError(failure.reason)); }; const addSuggestedRange = (range: string) => { @@ -202,10 +226,10 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

Allowed Client Applications

- Only the MCP client applications listed here can connect to the gateway. Names are matched exactly against - the clientInfo.name each client sends in its MCP initialize request (for example claude-code or - codex-mcp-client). Leave empty to allow every client. Clients choose the name they send, so treat this as a - policy control rather than a security boundary. + Only the MCP client applications listed here can connect to the gateway. Names are matched exactly against the + clientInfo.name each client sends in its MCP initialize request (for example claude-code or codex-mcp-client). + Leave empty to allow every client. Clients choose the name they send, so treat this as a policy control rather + than a security boundary.

@@ -244,8 +268,8 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) }} />

- Enter the clientInfo.name values to admit. Any other client, or one that does not identify itself, gets a - 403 on its MCP initialize request. + Enter the clientInfo.name values to admit. Any other client, or one that does not identify itself, gets a 403 + on its MCP initialize request.