mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-23 00:41:40 +00:00
fix(ui): save MCP allowed clients independently of private IP ranges
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
a705e0396e
commit
5e7888f44d
2 changed files with 102 additions and 28 deletions
|
|
@ -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(<MCPNetworkSettings accessToken="tok" />);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<MCPNetworkSettingsProps> = ({ accessToken }) => {
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [privateRanges, setPrivateRanges] = useState<string[]>([]);
|
||||
const [allowedClients, setAllowedClients] = useState<string[]>([]);
|
||||
const [storedRanges, setStoredRanges] = useState<string[]>([]);
|
||||
const [storedClients, setStoredClients] = useState<string[]>([]);
|
||||
const [currentIp, setCurrentIp] = useState<string | null>(null);
|
||||
const [rangeDraft, setRangeDraft] = useState("");
|
||||
const [clientDraft, setClientDraft] = useState("");
|
||||
|
|
@ -48,9 +53,11 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ 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<MCPNetworkSettingsProps> = ({ 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<MCPNetworkSettingsProps> = ({ accessToken })
|
|||
<div>
|
||||
<p className="text-lg font-semibold">Allowed Client Applications</p>
|
||||
<p className="mt-1 text-sm text-muted-foreground">
|
||||
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.
|
||||
</p>
|
||||
</div>
|
||||
|
||||
|
|
@ -244,8 +268,8 @@ const MCPNetworkSettings: React.FC<MCPNetworkSettingsProps> = ({ accessToken })
|
|||
}}
|
||||
/>
|
||||
<p className="mt-2 text-xs text-muted-foreground">
|
||||
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.
|
||||
</p>
|
||||
</Card>
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue