From 31f95d9117cc85ce2ccd60b878bf4b16961daf3c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 5 May 2026 00:44:42 +0000 Subject: [PATCH] fix(ui): omit unchanged allowed_routes on key update Non-admin team admins could not save key edit settings because the UI always submitted the current allowed_routes value in the update payload, triggering the backend permission check added in PR #25445. This follows the established precedent from commit 2c41f3c29 (policies field fix): strip allowed_routes from the update payload when the form value equals the previously persisted value so non-admin editors don't trip the backend 'setting allowed_routes' permission check on a no-op save. Genuine route changes (including clears) still pass through. Adds normalizeStringList and areStringListsEqual helpers for robust comparison of the route list values. Closes #27005 Co-authored-by: Krrish Dholakia --- .../templates/key_info_view.test.tsx | 88 +++++++++++++++++++ .../components/templates/key_info_view.tsx | 30 +++++++ 2 files changed, 118 insertions(+) diff --git a/ui/litellm-dashboard/src/components/templates/key_info_view.test.tsx b/ui/litellm-dashboard/src/components/templates/key_info_view.test.tsx index 724c961bbdc..7f71ee2f08e 100644 --- a/ui/litellm-dashboard/src/components/templates/key_info_view.test.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_info_view.test.tsx @@ -777,4 +777,92 @@ describe("KeyInfoView", () => { ); }); }); + + describe("allowed_routes payload normalization", () => { + const enterEditMode = async (keyData: KeyResponse) => { + vi.mocked(useAuthorized).mockReturnValue({ + ...baseUseAuthorizedMock, + userId: "proxy-admin-user", + userRole: "proxy_admin", + }); + renderWithProviders( + {}} + keyId="test-key-id" + onKeyDataUpdate={() => {}} + teams={[]} + />, + ); + await userEvent.click(screen.getByRole("tab", { name: /settings/i })); + await userEvent.click(screen.getByRole("button", { name: /edit settings/i })); + await waitFor(() => expect(editViewMocks.onSubmit).toBeDefined()); + }; + + beforeEach(() => { + editViewMocks.onSubmit = undefined; + vi.mocked(keyUpdateCall).mockClear(); + vi.mocked(keyUpdateCall).mockResolvedValue({}); + }); + + it("should drop allowed_routes when the submitted value matches the existing key", async () => { + const keyData: KeyResponse = { + ...MOCK_KEY_DATA, + user_id: "proxy-admin-user", + allowed_routes: ["management_routes"], + } as KeyResponse; + + await enterEditMode(keyData); + await editViewMocks.onSubmit!({ + key: keyData.token, + token: keyData.token, + allowed_routes: ["management_routes"], + }); + + expect(keyUpdateCall).toHaveBeenCalledWith( + expect.anything(), + expect.not.objectContaining({ allowed_routes: expect.anything() }), + ); + }); + + it("should drop empty allowed_routes when the key previously had no route override", async () => { + const keyData: KeyResponse = { + ...MOCK_KEY_DATA, + user_id: "proxy-admin-user", + allowed_routes: [], + } as KeyResponse; + + await enterEditMode(keyData); + await editViewMocks.onSubmit!({ + key: keyData.token, + token: keyData.token, + allowed_routes: [], + }); + + expect(keyUpdateCall).toHaveBeenCalledWith( + expect.anything(), + expect.not.objectContaining({ allowed_routes: expect.anything() }), + ); + }); + + it("should keep allowed_routes when the user clears an existing route override", async () => { + const keyData: KeyResponse = { + ...MOCK_KEY_DATA, + user_id: "proxy-admin-user", + allowed_routes: ["management_routes"], + } as KeyResponse; + + await enterEditMode(keyData); + await editViewMocks.onSubmit!({ + key: keyData.token, + token: keyData.token, + allowed_routes: [], + }); + + expect(keyUpdateCall).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ allowed_routes: [] }), + ); + }); + }); }); 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 492e43cbc81..1ca71b6e060 100644 --- a/ui/litellm-dashboard/src/components/templates/key_info_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_info_view.tsx @@ -49,6 +49,30 @@ const isEmptyValue = (v: unknown): boolean => (Array.isArray(v) && v.length === 0) || (typeof v === "string" && v.trim() === ""); +const normalizeStringList = (value: unknown): string[] => { + if (Array.isArray(value)) { + return value + .map((entry) => (typeof entry === "string" ? entry.trim() : "")) + .filter((entry) => entry.length > 0); + } + if (typeof value === "string") { + return value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + } + return []; +}; + +const areStringListsEqual = (left: unknown, right: unknown): boolean => { + const normalizedLeft = normalizeStringList(left); + const normalizedRight = normalizeStringList(right); + return ( + normalizedLeft.length === normalizedRight.length && + normalizedLeft.every((entry, index) => entry === normalizedRight[index]) + ); +}; + /** * ───────────────────────────────────────────────────────────────────────── * @deprecated @@ -174,6 +198,12 @@ export default function KeyInfoView({ } } + // Strip unchanged allowed_routes so non-admin editors don't trip the + // backend "setting allowed_routes" permission check on a no-op save. + if (areStringListsEqual(formValues.allowed_routes, currentKeyData.allowed_routes)) { + delete formValues.allowed_routes; + } + // Handle max budget empty string formValues.max_budget = mapEmptyStringToNull(formValues.max_budget);