mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
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.
This commit is contained in:
parent
c725017ef9
commit
577dd3b707
4 changed files with 92 additions and 15 deletions
|
|
@ -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,}/);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<CredentialsPanelProps> = ({ 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,
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>): Record<string, unknown> =>
|
||||
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];
|
||||
|
|
|
|||
10
ui/litellm-dashboard/src/utils/maskedSecretUtils.ts
Normal file
10
ui/litellm-dashboard/src/utils/maskedSecretUtils.ts
Normal file
|
|
@ -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<string, unknown>): Record<string, unknown> =>
|
||||
Object.fromEntries(Object.entries(params).filter(([, value]) => !isMaskedSecret(value)));
|
||||
Loading…
Add table
Reference in a new issue