From 577dd3b7073467c1ec6d4afba7f88134a5747efb Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 17 Jul 2026 18:25:24 -0700 Subject: [PATCH] fix(ui): stop credential edit from persisting the masked api key (#33797) Editing an existing LLM credential and changing only the api_base also overwrote the stored api_key with its masked display value (e.g. sk****IA). The edit form pre-fills fields from the credential the backend returns, whose secrets come back masked, and the update handler sent every field straight back; the endpoint then encrypted and stored the asterisks over the real key. Run credential_values through stripMaskedSecrets before the PATCH so masked placeholders are never sent, mirroring the guard the model edit form already uses. The isMaskedSecret / stripMaskedSecrets helpers move out of model_info_view into a shared utils module so both call sites share one implementation. Add a Playwright e2e that seeds a credential, edits only the api base in the LLM Credentials tab, and asserts the outgoing PATCH no longer carries the masked api_key while the new base persists. --- .../tests/modelsPage/credentials.spec.ts | 75 +++++++++++++++++++ .../src/components/model_add/credentials.tsx | 9 ++- .../src/components/model_info_view.tsx | 13 +--- .../src/utils/maskedSecretUtils.ts | 10 +++ 4 files changed, 92 insertions(+), 15 deletions(-) create mode 100644 ui/litellm-dashboard/e2e_tests/tests/modelsPage/credentials.spec.ts create mode 100644 ui/litellm-dashboard/src/utils/maskedSecretUtils.ts diff --git a/ui/litellm-dashboard/e2e_tests/tests/modelsPage/credentials.spec.ts b/ui/litellm-dashboard/e2e_tests/tests/modelsPage/credentials.spec.ts new file mode 100644 index 00000000000..8b7824813a4 --- /dev/null +++ b/ui/litellm-dashboard/e2e_tests/tests/modelsPage/credentials.spec.ts @@ -0,0 +1,75 @@ +import { test, expect } from "@playwright/test"; +import { ADMIN_STORAGE_PATH } from "../../constants"; +import { Role, users } from "../../fixtures/users"; + +test.describe("Edit LLM credential", () => { + test.use({ storageState: ADMIN_STORAGE_PATH }); + + const masterKey = users[Role.ProxyAdmin].password; + const SEED_API_KEY = "sk-e2e-credential-ABCDEFGHIJKLMNOP"; + const SEED_API_BASE = "https://api.openai.com/v1"; + const NEW_API_BASE = "https://proxy.e2e.example.com/v1"; + + let credentialName: string; + + test.beforeEach(async ({ page }) => { + credentialName = `e2e-cred-${Date.now()}`; + const res = await page.request.post("/credentials", { + headers: { Authorization: `Bearer ${masterKey}` }, + data: { + credential_name: credentialName, + credential_values: { api_key: SEED_API_KEY, api_base: SEED_API_BASE }, + credential_info: { custom_llm_provider: "openai" }, + }, + }); + expect(res.ok(), `POST /credentials for ${credentialName}`).toBe(true); + }); + + test.afterEach(async ({ page }) => { + await page.request.delete(`/credentials/${credentialName}`, { + headers: { Authorization: `Bearer ${masterKey}` }, + }); + }); + + test("changing only the api base does not overwrite the stored api key with its masked value", async ({ page }) => { + await page.goto("/ui"); + await page.getByText("Models + Endpoints").click(); + await page.getByRole("tab", { name: "LLM Credentials" }).click(); + + const row = page.locator("tr", { hasText: credentialName }); + await expect(row).toBeVisible({ timeout: 15_000 }); + await row.getByRole("button").first().click(); + + const modal = page.locator(".ant-modal-content").filter({ hasText: "Edit Credential" }); + await expect(modal).toBeVisible({ timeout: 10_000 }); + + const apiKeyField = modal.locator("#api_key"); + const apiBaseField = modal.locator("#api_base"); + await expect(apiKeyField).toBeVisible({ timeout: 15_000 }); + + await expect(apiKeyField, "form pre-fills the api key with the backend's masked value").toHaveValue(/\*{2,}/); + await expect(apiKeyField).not.toHaveValue(SEED_API_KEY); + + await apiBaseField.fill(NEW_API_BASE); + + const patchPromise = page.waitForRequest( + (req) => req.method() === "PATCH" && req.url().includes(`/credentials/${credentialName}`), + ); + await modal.getByRole("button", { name: "Update Credential" }).click(); + const patchReq = await patchPromise; + const patchBody = JSON.parse(patchReq.postData() ?? "{}"); + + expect(patchBody.credential_values.api_base, "UI sends the edited api base").toBe(NEW_API_BASE); + expect("api_key" in patchBody.credential_values, "UI must not send the masked api key back on update").toBe(false); + + await expect(page.getByText("Credential updated successfully")).toBeVisible({ timeout: 10_000 }); + + const infoRes = await page.request.get(`/credentials/by_name/${credentialName}`, { + headers: { Authorization: `Bearer ${masterKey}` }, + }); + expect(infoRes.ok()).toBe(true); + const cred = await infoRes.json(); + expect(cred.credential_values.api_base, "edited api base persisted to the backend").toBe(NEW_API_BASE); + expect(cred.credential_values.api_key, "a stored api key is still present (returned masked)").toMatch(/\*{2,}/); + }); +}); diff --git a/ui/litellm-dashboard/src/components/model_add/credentials.tsx b/ui/litellm-dashboard/src/components/model_add/credentials.tsx index 53df9015c97..82320b7ff8d 100644 --- a/ui/litellm-dashboard/src/components/model_add/credentials.tsx +++ b/ui/litellm-dashboard/src/components/model_add/credentials.tsx @@ -27,6 +27,7 @@ import EditCredentialsModal from "./EditCredentialModal"; import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import { isProxyAdminRole } from "@/utils/roles"; +import { stripMaskedSecrets } from "@/utils/maskedSecretUtils"; interface CredentialsPanelProps { uploadProps: UploadProps; } @@ -52,9 +53,11 @@ const CredentialsPanel: React.FC = ({ uploadProps }) => { return; } - const filter_credential_values = Object.entries(values) - .filter(([key]) => !restrictedFields.includes(key)) - .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}); + const filter_credential_values = stripMaskedSecrets( + Object.entries(values) + .filter(([key]) => !restrictedFields.includes(key)) + .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}), + ); // Transform form values into credential structure const newCredential = { credential_name: values.credential_name, diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index 5b7f82c44b4..8aaabdc50a2 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -22,6 +22,7 @@ import VectorStoreSelector from "./vector_store_management/VectorStoreSelector"; import { CheckIcon, CopyIcon } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; import { copyToClipboard as utilCopyToClipboard } from "../utils/dataUtils"; +import { isMaskedSecret, stripMaskedSecrets } from "../utils/maskedSecretUtils"; import { formItemValidateJSON, truncateString } from "../utils/textUtils"; import AutoRouterConnectionTest from "./add_model/auto_router_connection_test"; import { AutoRouterTestTarget, buildAutoRouterTestTargets } from "./add_model/build_auto_router_test_targets"; @@ -58,18 +59,6 @@ interface ModelInfoViewProps { modelAccessGroups: string[] | null; } -// The /model/info response redacts secrets by masking them (e.g. "sk-1****2345"), -// not by removing them. The edit form must never echo a masked value back on save: -// the backend would encrypt the asterisks and overwrite the real secret. A run of -// 2+ mask chars only appears in masker output (real config — incl. wildcard model -// names like "openai/*" — carries at most a single "*"), so this reliably detects a -// redacted value without a provider-metadata lookup. API-key rotation goes through -// UpdateModelCredentialsModal instead, which sends only the new key. -const isMaskedSecret = (value: unknown): boolean => typeof value === "string" && /\*{2,}/.test(value); - -const stripMaskedSecrets = (params: Record): Record => - Object.fromEntries(Object.entries(params).filter(([, value]) => !isMaskedSecret(value))); - const normalizeTierModels = (value: unknown): string[] => { if (Array.isArray(value)) return value; if (typeof value === "string" && value) return [value]; diff --git a/ui/litellm-dashboard/src/utils/maskedSecretUtils.ts b/ui/litellm-dashboard/src/utils/maskedSecretUtils.ts new file mode 100644 index 00000000000..101316bbd82 --- /dev/null +++ b/ui/litellm-dashboard/src/utils/maskedSecretUtils.ts @@ -0,0 +1,10 @@ +// The proxy redacts secrets in API responses by masking them (e.g. "sk-1****2345"), +// not by removing them. Edit forms must never echo a masked value back on save: the +// backend would encrypt the asterisks and overwrite the real secret. A run of 2+ mask +// chars only appears in masker output (real config -- incl. wildcard model names like +// "openai/*" -- carries at most a single "*"), so this reliably detects a redacted +// value without a provider-metadata lookup. +export const isMaskedSecret = (value: unknown): boolean => typeof value === "string" && /\*{2,}/.test(value); + +export const stripMaskedSecrets = (params: Record): Record => + Object.fromEntries(Object.entries(params).filter(([, value]) => !isMaskedSecret(value)));