From 2c41f3c291208c4f78f6c56a2f4b4587f335bd1f Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 18 Apr 2026 12:17:45 -0700 Subject: [PATCH] [Fix] UI - Keys: strip empty premium fields from key update payload The /key/update response echoes top-level defaults like policies:[] into client state. On a subsequent edit, the form resends policies:[], which the backend treats as "user is setting policies" and blocks with a 403 enterprise check regardless of value. Drop premium metadata fields from the update payload when the current form value and the previously persisted value are both empty. Genuine clears (non-empty -> empty) still pass through so premium users can clear policies as intended. --- .../templates/key_info_view.test.tsx | 78 +++++++++++++++++++ .../components/templates/key_info_view.tsx | 26 +++++++ 2 files changed, 104 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 f269ad96a2..6b5732436a 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 @@ -7,8 +7,20 @@ import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { useResetKeySpend } from "@/app/(dashboard)/hooks/keys/useResetKeySpend"; import { KeyResponse, Team } from "../key_team_helpers/key_list"; +import { keyUpdateCall } from "../networking"; import KeyInfoView from "./key_info_view"; +const editViewMocks = vi.hoisted(() => ({ + onSubmit: undefined as ((v: Record) => Promise) | undefined, +})); + +vi.mock("./key_edit_view", () => ({ + KeyEditView: ({ onSubmit }: { onSubmit: (v: Record) => Promise }) => { + editViewMocks.onSubmit = onSubmit; + return
; + }, +})); + vi.mock("@/app/(dashboard)/hooks/useTeams", () => ({ default: vi.fn(), })); @@ -680,4 +692,70 @@ describe("KeyInfoView", () => { }); }); }); + + describe("premium metadata 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 an empty policies field when the key previously had no policies", async () => { + // Reproduces the real bug: after a successful /key/update, the response echoes + // top-level `policies: []` into client state. Without stripping, the next save + // resends `[]` and trips the premium gate in prepare_metadata_fields. + const keyData: KeyResponse = { + ...MOCK_KEY_DATA, + user_id: "proxy-admin-user", + metadata: {}, + policies: [], + } as KeyResponse; + + await enterEditMode(keyData); + await editViewMocks.onSubmit!({ key: keyData.token, token: keyData.token, policies: [] }); + + expect(keyUpdateCall).toHaveBeenCalledWith( + expect.anything(), + expect.not.objectContaining({ policies: expect.anything() }), + ); + }); + + it("should keep an empty policies field when the key previously had policies set", async () => { + // Premium users must still be able to clear existing policies by sending `[]`. + const keyData: KeyResponse = { + ...MOCK_KEY_DATA, + user_id: "proxy-admin-user", + metadata: { policies: ["existing-policy"] }, + policies: ["existing-policy"], + } as KeyResponse; + + await enterEditMode(keyData); + await editViewMocks.onSubmit!({ key: keyData.token, token: keyData.token, policies: [] }); + + expect(keyUpdateCall).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ policies: [] }), + ); + }); + }); }); 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 5b5e7722c0..bdb8254c72 100644 --- a/ui/litellm-dashboard/src/components/templates/key_info_view.tsx +++ b/ui/litellm-dashboard/src/components/templates/key_info_view.tsx @@ -34,6 +34,21 @@ interface KeyInfoViewProps { backButtonText?: string; } +// Must stay in sync with LiteLLM_ManagementEndpoint_MetadataFields_Premium +// in litellm/proxy/_types.py — limited to fields the key-edit form submits. +const PREMIUM_METADATA_FIELDS = [ + "policies", + "guardrails", + "prompts", + "tags", + "allowed_passthrough_routes", +] as const; + +const isEmptyValue = (v: unknown): boolean => + v == null || + (Array.isArray(v) && v.length === 0) || + (typeof v === "string" && v.trim() === ""); + /** * ───────────────────────────────────────────────────────────────────────── * @deprecated @@ -146,6 +161,17 @@ export default function KeyInfoView({ delete formValues.prompts; } + // Drop premium metadata fields that are empty AND were empty before. + // The /key/update response echoes defaults like `policies: []` back into + // state; without this, the next save resends `[]` and trips the premium + // gate in prepare_metadata_fields for non-premium users. + for (const field of PREMIUM_METADATA_FIELDS) { + const previousValue = (currentKeyData.metadata as Record | undefined)?.[field]; + if (isEmptyValue(formValues[field]) && isEmptyValue(previousValue)) { + delete formValues[field]; + } + } + // Handle max budget empty string formValues.max_budget = mapEmptyStringToNull(formValues.max_budget);