From b2cc5f25f62555abb1fa1dc2ee286a5fb779c21f Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Sat, 10 Oct 2026 16:26:54 -0700 Subject: [PATCH] fix(credentials): keep credential labels and edits consistent across UI, API, and replicas (#45835) * fix(credentials): keep credential labels and edits consistent across UI, API, and replicas The credential modal flattened stored credential values into the same form object as the credential's own name, label, and provider, so a stored value keyed credential_name or display_name replaced the real one and the edit went to the wrong credential or relabeled it. The modal now submits the top-level fields and credential_values separately PATCH /credentials now reads the row it merges onto from the writer, so an edit inside the read replica's lag window no longer writes back an older label or key POST /credentials answers 409 for a name defined in config instead of creating a DB copy that shadows it display_name rules live on the server only. It rejects control, formatting, and line separator characters, and the UI stops enforcing its own length check and shows the server's error The models table, model detail view, and model edit picker show the credential's technical name under its label whenever the two differ * fix(ui): show credential source in its own column like models The config marker sat under the credential name. The credentials table now has a Source column with a Config or DB status badge, matching the Source column on the models table * test(credentials): unit-test label and prefill helpers and keep the full credential name on hover Moves displayNameChange and initialFormValues out of the modal so their rules are unit tested, keeps only the wiring cases in the modal's integration test, makes the label acceptance test write through the real credentials repository, and adds the full credential name as a hover title where the models pages truncate it --- .../proxy/credential_endpoints/endpoints.py | 25 ++- .../credential_endpoints/test_endpoints.py | 147 +++++++++++++++++- .../components/AllModelsTable.test.tsx | 29 +++- .../components/ModelsTableColumns.tsx | 11 +- .../src/components/ModelInfoEditForm.test.tsx | 17 +- .../src/components/ModelInfoEditForm.tsx | 37 +++-- .../src/components/add_model/AddModelForm.tsx | 6 +- .../CredentialModal.integration.test.tsx | 139 +++++++++++++---- .../model_add/CredentialModal.test.tsx | 141 +---------------- .../components/model_add/CredentialModal.tsx | 68 ++------ .../model_add/CredentialsPanel.test.tsx | 41 +++-- .../components/model_add/CredentialsPanel.tsx | 17 +- .../model_add/CredentialsTable.test.tsx | 10 +- .../model_add/CredentialsTableColumns.tsx | 17 +- .../model_add/credential_form_helpers.test.ts | 87 ++++++++++- .../model_add/credential_form_helpers.ts | 49 +++++- .../src/components/model_info_view.test.tsx | 2 +- .../src/components/model_info_view.tsx | 12 +- .../components/shared/CredentialIdentity.tsx | 52 +++++++ 19 files changed, 612 insertions(+), 295 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/shared/CredentialIdentity.tsx diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index 83af63bf24a..1584b12fd68 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -3,6 +3,7 @@ CRUD endpoints for storing reusable credentials. """ import time +import unicodedata from collections.abc import Mapping from datetime import datetime from typing import ( @@ -57,6 +58,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( decrypt_value_helper, encrypt_value_helper, ) +from litellm.proxy.db.routing_prisma_wrapper import WriterPinnedClient from litellm.proxy.utils import handle_exception_on_proxy, jsonify_object from litellm.repositories.base_repository import is_unique_violation from litellm.repositories.credentials_repository import CredentialsRepository @@ -79,6 +81,7 @@ router: Final = APIRouter() _CREDENTIAL_DICT_ADAPTER: Final = TypeAdapter(dict[str, object]) _GITHUB_COPILOT_PROVIDER: Final = "github_copilot" _DISPLAY_NAME_MAX_LENGTH: Final = 255 +_DISPLAY_NAME_FORBIDDEN_CATEGORIES: Final = frozenset({"Cc", "Cf", "Cs", "Zl", "Zp"}) def _reject_non_admin_wif_fields( @@ -208,9 +211,28 @@ def _normalized_display_name(display_name: str | None) -> str | None: code=status.HTTP_400_BAD_REQUEST, param="display_name", ) + if any(unicodedata.category(character) in _DISPLAY_NAME_FORBIDDEN_CATEGORIES for character in trimmed): + raise ProxyException( + message="display_name cannot contain control, formatting, or line separator characters.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="display_name", + ) return trimmed +def _reject_config_defined_name(credential_name: str) -> None: + in_memory: Final = CredentialAccessor.find_credential(credential_name) + if in_memory is None or in_memory.source != "config": + return + raise ProxyException( + message=f"Credential '{credential_name}' is defined in config. Edit config.yaml to change it.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_409_CONFLICT, + param="credential_name", + ) + + def _not_found_unless_config_defined(credential_name: str, not_found_detail: str) -> HTTPException | ProxyException: in_memory: Final = CredentialAccessor.find_credential(credential_name) if in_memory is None or in_memory.source != "config": @@ -300,6 +322,7 @@ async def create_credential( _reject_non_admin_wif_fields( await named_credential_wif_fields(credential.credential_name, prisma_client), user_api_key_dict ) + _reject_config_defined_name(credential.credential_name) processed_credential: Final = CredentialItem( credential_name=credential.credential_name, display_name=_normalized_display_name(credential.display_name), @@ -894,7 +917,7 @@ async def update_credential( status_code=500, detail={"error": CommonProxyErrors.db_not_connected_error.value}, ) - credentials_repository: Final = CredentialsRepository(prisma_client) + credentials_repository: Final = CredentialsRepository(WriterPinnedClient(prisma_client.db)) db_credential: Final = await credentials_repository.find_by_name(credential_name) if db_credential is None: raise _not_found_unless_config_defined(credential_name, "Credential not found in DB.") diff --git a/tests/unit/proxy/credential_endpoints/test_endpoints.py b/tests/unit/proxy/credential_endpoints/test_endpoints.py index 3c4e8b20f72..4359ea6592b 100644 --- a/tests/unit/proxy/credential_endpoints/test_endpoints.py +++ b/tests/unit/proxy/credential_endpoints/test_endpoints.py @@ -2,6 +2,7 @@ import json from contextlib import contextmanager +from types import SimpleNamespace from typing import Final from unittest.mock import AsyncMock, MagicMock, patch @@ -188,6 +189,56 @@ def test_update_credential_still_answers_200_on_a_successful_write(credential_st assert response.json()["success"] is True +def _credential_row(display_name: str, api_key: str, name: str = "replicated") -> dict: + return { + "credential_name": name, + "display_name": display_name, + "credential_values": {"api_key": api_key}, + "credential_info": {"custom_llm_provider": "openai"}, + } + + +def _credentials_table(row: dict | None) -> SimpleNamespace: + return SimpleNamespace( + find_many=AsyncMock(), + create=AsyncMock(), + find_unique=AsyncMock(return_value=row), + update=AsyncMock(return_value=None), + ) + + +def test_update_credential_merges_onto_the_writer_row_not_a_lagging_replica(restore_credential_list): + """Regression: PATCH read the row through the read replica and wrote the merged row to the + writer, so an edit inside the replica lag window wrote back the replica's older label and + secrets, undoing a relabel or key rotation that had already committed.""" + from litellm.proxy.db.prisma_client import PrismaWrapper + from litellm.proxy.db.routing_prisma_wrapper import RoutingPrismaWrapper + + writer_table = _credentials_table(_credential_row("Relabeled", "sk-B")) + reader_table = _credentials_table(_credential_row("Original", "sk-A")) + writer_inner = SimpleNamespace(litellm_credentialstable=writer_table) + reader_inner = SimpleNamespace(litellm_credentialstable=reader_table) + prisma_client = MagicMock() + prisma_client.db = RoutingPrismaWrapper( + writer=PrismaWrapper(original_prisma=writer_inner, iam_token_db_auth=False), + reader=PrismaWrapper(original_prisma=reader_inner, iam_token_db_auth=False), + ) + + with ( + patch("litellm.proxy.proxy_server.prisma_client", prisma_client), + patch("litellm.proxy.proxy_server.master_key", "sk-test-master"), + ): + response = _patch_credential( + "replicated", {"credential_values": {"api_base": "https://example.test/v1"}, "credential_info": {}} + ) + + assert response.status_code == 200, response.text + written = writer_table.update.await_args.kwargs["data"] + assert written["display_name"] == "Relabeled" + assert json.loads(written["credential_values"])["api_key"] == "sk-B" + reader_table.find_unique.assert_not_awaited() + + def _get_jwks(name: str): return _call_as("GET", f"/credentials/{name}/jwks") @@ -1063,8 +1114,9 @@ class TestNonAdminCannotTouchAStoredWifCredential: assert litellm.credential_list == [config_credential] assert litellm.credential_list[0].credential_values["api_key"] == "sk-old" - def test_proxy_admin_can_post_over_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): - monkeypatch.setattr(litellm, "credential_list", [_wif_credential("config-wif", source="config")]) + def test_proxy_admin_cannot_post_over_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): + config_credential = _wif_credential("config-wif", source="config") + monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _post_credential( { @@ -1075,9 +1127,11 @@ class TestNonAdminCannotTouchAStoredWifCredential: auth=_as_admin, ) - assert response.status_code == 200, response.text - repository.create.assert_awaited_once() - assert litellm.credential_list[0].credential_values == {"api_key": "sk-rotated"} + assert response.status_code == 409, response.text + assert response.json()["error"]["param"] == "credential_name" + assert "defined in config" in response.json()["error"]["message"] + repository.create.assert_not_awaited() + assert litellm.credential_list == [config_credential] def test_non_admin_cannot_post_a_null_wif_field(self, restore_credential_list): """Same key-presence rule on the create path: ``{"anthropic_issuer_url": null}`` persists @@ -2270,6 +2324,52 @@ class TestCredentialDisplayName: repository.update_by_name.assert_not_awaited() assert litellm.credential_list == [_labeled_credential()] + @pytest.mark.parametrize("display_name", ["\u200b", "Prod\u202eAI", "\ufeffProd", "Prod\u0085AI", "Prod\u2028AI"]) + def test_patch_rejects_invisible_or_control_characters_in_display_name( + self, restore_credential_list, monkeypatch, display_name + ): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": display_name, "credential_info": {}}) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "display_name" + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [_labeled_credential()] + + @pytest.mark.parametrize("display_name", ["\U0001f600" * 255, "Prod\u00a0OpenAI", "Équipe 東京"]) + def test_patch_accepts_visible_unicode_up_to_the_limit_in_code_points( + self, restore_credential_list, monkeypatch, display_name + ): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + table = _credentials_table(_credential_row("Prod OpenAI", "sk-old", name="openai-prod")) + prisma_client = MagicMock() + prisma_client.db = SimpleNamespace(litellm_credentialstable=table) + with ( + patch("litellm.proxy.proxy_server.prisma_client", prisma_client), + patch("litellm.proxy.proxy_server.master_key", "sk-test-master"), + ): + response = _patch_credential("openai-prod", {"display_name": display_name, "credential_info": {}}) + + assert response.status_code == 200, response.text + assert table.update.await_args.kwargs["where"] == {"credential_name": "openai-prod"} + assert table.update.await_args.kwargs["data"]["display_name"] == display_name + + def test_create_rejects_a_control_character_in_display_name(self, restore_credential_list): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "new-cred", + "display_name": "Prod\u200bOpenAI", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "display_name" + repository.create.assert_not_awaited() + def test_patch_stores_the_trimmed_display_name(self, restore_credential_list, monkeypatch): monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) with _repository_holding(_labeled_credential()) as repository: @@ -2333,6 +2433,43 @@ class TestCredentialDisplayName: repository.update_by_name.assert_not_awaited() assert litellm.credential_list == [config_credential] + def test_post_on_a_config_credential_name_is_rejected_and_does_not_shadow_it( + self, restore_credential_list, monkeypatch + ): + config_credential = _labeled_credential(display_name=None).model_copy(update={"source": "config"}) + monkeypatch.setattr(litellm, "credential_list", [config_credential]) + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "credential_values": {"api_key": "sk-shadow"}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + + assert response.status_code == 409, response.text + assert "defined in config" in response.json()["error"]["message"] + repository.create.assert_not_awaited() + assert litellm.credential_list == [config_credential] + + def test_post_with_a_display_name_matching_a_config_credential_name_is_allowed( + self, restore_credential_list, monkeypatch + ): + config_credential = _labeled_credential(display_name=None).model_copy(update={"source": "config"}) + monkeypatch.setattr(litellm, "credential_list", [config_credential]) + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-staging", + "display_name": "openai-prod", + "credential_values": {"api_key": "sk-staging"}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + + assert response.status_code == 200, response.text + repository.create.assert_awaited_once() + def test_patch_on_an_unknown_credential_still_answers_404(self, restore_credential_list): with _repository_holding(None): response = _patch_credential("nowhere", {"display_name": "Prod", "credential_info": {}}) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx index adbeb70a9c5..768fedf2201 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx @@ -157,7 +157,7 @@ describe("AllModelsTable", () => { expect(screen.getByText("Manual")).toBeInTheDocument(); }); - it("renders the credential's display name when one is set and keeps the name as the tooltip", () => { + it("renders the credential's display name with its credential name beneath", () => { render( { />, ); expect(screen.getByText("Prod OpenAI")).toBeInTheDocument(); - expect(screen.queryByText("openai-prod")).not.toBeInTheDocument(); - expect(screen.getByTitle("openai-prod")).toBeInTheDocument(); + expect(screen.getByText("openai-prod")).toBeInTheDocument(); + }); + + it("tells apart a credential whose label is another credential's name", () => { + render( + , + ); + expect(screen.getAllByText("openai-staging")).toHaveLength(2); + expect(screen.getByText("openai-prod")).toBeInTheDocument(); }); it("shows 'Defined in config' for a config model and the creator for a DB model", () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx index c277f527f86..1b7369c9520 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx @@ -1,6 +1,7 @@ "use client"; import { ColumnDef } from "@tanstack/react-table"; +import { CredentialIdentity } from "@/components/shared/CredentialIdentity"; import { Copy, Info, Loader2, Pencil, RefreshCw, Trash2 } from "lucide-react"; import { ProviderLogo } from "@/components/molecules/models/ProviderLogo"; @@ -171,10 +172,12 @@ function CredentialsCell({ credentialName, label }: { credentialName: string | u } return ( - - - {label ?? credentialName} - + } + className="text-xs font-medium text-info" + /> ); } diff --git a/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx b/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx index 1a05374d883..ecc0cea8bdf 100644 --- a/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx +++ b/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx @@ -96,10 +96,23 @@ describe("ModelInfoEditForm existing-credentials picker", () => { expect(onSubmit.mock.calls[1][0].litellm_credential_name).toBeNull(); }); - it("shows the attached credential's display name when not editing", () => { + it("shows the attached credential's display name with its credential name beneath when not editing", () => { renderForm({ isEditing: false, litellmParams: { model: "gpt-4o", litellm_credential_name: "openai-main" } }); expect(screen.getByText("Main OpenAI")).toBeInTheDocument(); + expect(screen.getByText("openai-main")).toBeInTheDocument(); + }); + + it("shows the selected credential's name under the picker only when its label differs", async () => { + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + renderForm({ litellmParams: { model: "gpt-4o", litellm_credential_name: "openai-main" } }); + + const picker = await screen.findByPlaceholderText("Select or search for existing credentials"); + expect(picker).toHaveValue("Main OpenAI"); + expect(screen.getByText("openai-main")).toBeInTheDocument(); + + await user.click(picker); + await user.click(await screen.findByRole("option", { name: "None" })); expect(screen.queryByText("openai-main")).not.toBeInTheDocument(); }); @@ -108,7 +121,7 @@ describe("ModelInfoEditForm existing-credentials picker", () => { isEditing: false, litellmParams: { model: "gpt-4o", litellm_credential_name: "not-listed" }, }); - expect(screen.getByText("not-listed")).toBeInTheDocument(); + expect(screen.getAllByText("not-listed")).toHaveLength(1); unmount(); renderForm({ isEditing: false }); diff --git a/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx b/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx index 2a451711407..c7860f35100 100644 --- a/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx +++ b/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx @@ -15,7 +15,8 @@ import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { SearchSelect } from "@/components/shared/SearchSelect"; -import { credentialLabelsByName, credentialOptions } from "@/components/shared/credentialOptions"; +import { credentialOptions } from "@/components/shared/credentialOptions"; +import { AttachedCredential, CredentialNameHint } from "@/components/shared/CredentialIdentity"; import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; @@ -347,9 +348,6 @@ const ChipList: React.FC<{ values: unknown; emptyLabel: string }> = ({ values, e ); }; -const attachedCredentialLabel = (credentialName: string | null | undefined, credentials: CredentialItem[]): string => - credentialName ? credentialLabelsByName(credentials).get(credentialName) ?? credentialName : "Manual"; - const ModelInfoEditForm: React.FC = ({ localModelData, modelData, @@ -640,21 +638,30 @@ const ModelInfoEditForm: React.FC = ({ {isEditing ? ( {({ id, value, onChange }) => ( - { - if (selected !== null) onChange(selected === "" ? null : selected); - }} - /> + <> + { + if (selected !== null) onChange(selected === "" ? null : selected); + }} + /> + + )} ) : ( - {attachedCredentialLabel(localModelData.litellm_params?.litellm_credential_name, credentialsList)} + )} diff --git a/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx b/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx index 2a93b642d6e..5313df6153a 100644 --- a/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx +++ b/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx @@ -28,7 +28,7 @@ import type { Team } from "../key_team_helpers/key_list"; import { type CredentialItem, type ProviderCreateInfo, credentialCreateCall, modelAvailableCall } from "../networking"; import CredentialModal from "../model_add/CredentialModal"; import { federatedProviderOf } from "../model_add/credential_federation"; -import { buildCredential, withoutRestrictedFields } from "../model_add/credential_form_helpers"; +import { buildCredential, type CredentialSubmission } from "../model_add/credential_form_helpers"; import { ProviderLogo } from "../molecules/models/ProviderLogo"; import AccessGroupTagsCombobox from "./AccessGroupTagsCombobox"; import AdvancedSettings from "./advanced_settings"; @@ -114,8 +114,8 @@ const AddModelForm: React.FC = ({ isProxyAdminRole(userRole ?? "") && federatedProviderOf(selectedProvider) !== null; const selectedAuthType = authTypesFor(selectedProvider).find(({ id }) => id === selectedAuthTypeId); - const handleCreateCredential = async (values: Record) => { - const credential = buildCredential(values, withoutRestrictedFields(values)); + const handleCreateCredential = async (submission: CredentialSubmission) => { + const credential = buildCredential(submission, submission.credential_values); try { await credentialCreateCall(accessToken, credential); } catch (error) { diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx index c28260ca095..e1e259063bf 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.integration.test.tsx @@ -167,7 +167,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { expect(values).toEqual({ credential_name: "microsoft-copilot", custom_llm_provider: "MICROSOFT_365_COPILOT", - api_key: "delegated-graph-token", + credential_values: { api_key: "delegated-graph-token" }, }); expect([...valuesToDelete].sort()).toEqual([ "client_id", @@ -192,7 +192,10 @@ describe("CredentialModal with Anthropic workload identity federation", () => { expect(await screen.findByLabelText("Upstream API Base")).toHaveValue(""); fill("Credential Name:", "anthropic-key"); await user.click(screen.getByRole("button", { name: "Add Credential" })); - expect(onSubmit).toHaveBeenCalledWith({ credential_name: "anthropic-key", custom_llm_provider: "Anthropic" }, []); + expect(onSubmit).toHaveBeenCalledWith( + { credential_name: "anthropic-key", custom_llm_provider: "Anthropic", credential_values: {} }, + [], + ); }); it("creates a federated credential from the federation fields and never sends an API key", async () => { @@ -213,9 +216,11 @@ describe("CredentialModal with Anthropic workload identity federation", () => { const expectedPayload = { credential_name: "anthropic-federated", custom_llm_provider: "Anthropic", - anthropic_federation_rule_id: "fdrl_new", - anthropic_organization_id: "org-new", - anthropic_identity_token_file: "/var/run/secrets/anthropic/token", + credential_values: { + anthropic_federation_rule_id: "fdrl_new", + anthropic_organization_id: "org-new", + anthropic_identity_token_file: "/var/run/secrets/anthropic/token", + }, }; expect(onSubmit).toHaveBeenCalledWith(expectedPayload, []); }); @@ -238,8 +243,10 @@ describe("CredentialModal with Anthropic workload identity federation", () => { expect(onSubmit).toHaveBeenCalledWith( expect.objectContaining({ - anthropic_identity_source: "keycloak", - anthropic_keycloak_client_secret_ref: "os.environ/KEYCLOAK_CLIENT_SECRET", + credential_values: expect.objectContaining({ + anthropic_identity_source: "keycloak", + anthropic_keycloak_client_secret_ref: "os.environ/KEYCLOAK_CLIENT_SECRET", + }), }), [], ); @@ -273,7 +280,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-federated", custom_llm_provider: "anthropic" }, + { credential_name: "anthropic-federated", custom_llm_provider: "anthropic", credential_values: {} }, [], ); }); @@ -290,7 +297,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { { credential_name: "anthropic-federated", custom_llm_provider: "anthropic", - anthropic_organization_id: "org-edited", + credential_values: { anthropic_organization_id: "org-edited" }, }, [], ); @@ -310,7 +317,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { expect(values).toEqual({ credential_name: "anthropic-federated", custom_llm_provider: "anthropic", - api_key: "sk-ant-replacement", + credential_values: { api_key: "sk-ant-replacement" }, }); expect([...valuesToDelete].sort()).toEqual(Object.keys(federatedCredential.credential_values).sort()); }); @@ -332,9 +339,11 @@ describe("CredentialModal with Anthropic workload identity federation", () => { const expectedValues = { credential_name: "azure-prod", custom_llm_provider: "Anthropic", - anthropic_federation_rule_id: "fdrl_new", - anthropic_organization_id: "org-new", - anthropic_identity_token_file: "/var/run/secrets/anthropic/token", + credential_values: { + anthropic_federation_rule_id: "fdrl_new", + anthropic_organization_id: "org-new", + anthropic_identity_token_file: "/var/run/secrets/anthropic/token", + }, }; expect(values).toEqual(expectedValues); expect([...valuesToDelete].sort()).toEqual(["api_base", "api_key", "api_version"]); @@ -351,7 +360,10 @@ describe("CredentialModal with Anthropic workload identity federation", () => { expect(await screen.findByLabelText("Azure API Base")).toHaveValue("https://corp.openai.azure.com"); await user.click(screen.getByRole("button", { name: "Update Credential" })); - expect(onSubmit).toHaveBeenCalledWith({ credential_name: "azure-prod", custom_llm_provider: "Azure" }, []); + expect(onSubmit).toHaveBeenCalledWith( + { credential_name: "azure-prod", custom_llm_provider: "Azure", credential_values: {} }, + [], + ); }); it("deletes the stored base URL when the admin clears it on a federated credential", async () => { @@ -367,7 +379,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-federated", custom_llm_provider: "anthropic" }, + { credential_name: "anthropic-federated", custom_llm_provider: "anthropic", credential_values: {} }, ["api_base"], ); }); @@ -383,7 +395,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); const [values, valuesToDelete] = onSubmit.mock.calls[0]; - expect(values).toMatchObject({ custom_llm_provider: "OpenAI", api_key: "sk-openai-new" }); + expect(values).toMatchObject({ custom_llm_provider: "OpenAI", credential_values: { api_key: "sk-openai-new" } }); expect([...valuesToDelete].sort()).toEqual(Object.keys(federatedCredential.credential_values).sort()); }); @@ -398,7 +410,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-key-and-federation", custom_llm_provider: "anthropic" }, + { credential_name: "anthropic-key-and-federation", custom_llm_provider: "anthropic", credential_values: {} }, ["api_key"], ); }); @@ -452,7 +464,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { { credential_name: "anthropic-unknown-source", custom_llm_provider: "anthropic", - anthropic_organization_id: "org-edited", + credential_values: { anthropic_organization_id: "org-edited" }, }, [], ); @@ -467,7 +479,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-unknown-source", custom_llm_provider: "anthropic" }, + { credential_name: "anthropic-unknown-source", custom_llm_provider: "anthropic", credential_values: {} }, ["anthropic_identity_source"], ); }); @@ -488,7 +500,11 @@ describe("CredentialModal with Anthropic workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Add Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-env", custom_llm_provider: "Anthropic", anthropic_organization_id: "org-new" }, + { + credential_name: "anthropic-env", + custom_llm_provider: "Anthropic", + credential_values: { anthropic_organization_id: "org-new" }, + }, [], ); }); @@ -506,7 +522,7 @@ describe("CredentialModal with Anthropic workload identity federation", () => { { credential_name: "anthropic-env-ids", custom_llm_provider: "Anthropic", - anthropic_identity_token_file: "/var/run/secrets/anthropic/token", + credential_values: { anthropic_identity_token_file: "/var/run/secrets/anthropic/token" }, }, [], ); @@ -536,7 +552,7 @@ describe("CredentialModal with GitHub Copilot auth types", () => { { credential_name: "copilot-per-user", custom_llm_provider: "GITHUB_COPILOT", - github_copilot_auth_type: "per_user_oauth", + credential_values: { github_copilot_auth_type: "per_user_oauth" }, }, [], ); @@ -551,7 +567,7 @@ describe("CredentialModal with GitHub Copilot auth types", () => { await user.click(screen.getByRole("button", { name: "Add Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "copilot-shared", custom_llm_provider: "GITHUB_COPILOT" }, + { credential_name: "copilot-shared", custom_llm_provider: "GITHUB_COPILOT", credential_values: {} }, [], ); }); @@ -565,7 +581,11 @@ describe("CredentialModal with GitHub Copilot auth types", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); const [values, valuesToDelete] = onSubmit.mock.calls[0]; - expect(values).toEqual({ credential_name: "copilot-per-user", custom_llm_provider: "GITHUB_COPILOT" }); + expect(values).toEqual({ + credential_name: "copilot-per-user", + custom_llm_provider: "GITHUB_COPILOT", + credential_values: {}, + }); expect(valuesToDelete).toEqual(["github_copilot_auth_type"]); }); }); @@ -698,9 +718,11 @@ describe("CredentialModal with OpenAI workload identity federation", () => { const expectedPayload = { credential_name: "openai-federated", custom_llm_provider: "OpenAI", - openai_identity_provider_id: "idp_new", - openai_service_account_id: "svc_new", - openai_identity_token_file: "/var/run/secrets/kubernetes.io/serviceaccount/token", + credential_values: { + openai_identity_provider_id: "idp_new", + openai_service_account_id: "svc_new", + openai_identity_token_file: "/var/run/secrets/kubernetes.io/serviceaccount/token", + }, }; expect(onSubmit).toHaveBeenCalledWith(expectedPayload, []); }); @@ -717,7 +739,10 @@ describe("CredentialModal with OpenAI workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); - expect(onSubmit).toHaveBeenCalledWith({ credential_name: "openai-federated", custom_llm_provider: "openai" }, []); + expect(onSubmit).toHaveBeenCalledWith( + { credential_name: "openai-federated", custom_llm_provider: "openai", credential_values: {} }, + [], + ); }); it("refuses a base URL the proxy would not federate with, and drops that check once the admin picks an API key", async () => { @@ -742,8 +767,7 @@ describe("CredentialModal with OpenAI workload identity federation", () => { const expectedPayload = { credential_name: "openai-gateway", custom_llm_provider: "OpenAI", - api_base: "https://gateway.example.com/v1", - api_key: "sk-proj-new", + credential_values: { api_base: "https://gateway.example.com/v1", api_key: "sk-proj-new" }, }; expect(onSubmit).toHaveBeenCalledWith(expectedPayload, []); }); @@ -761,7 +785,10 @@ describe("CredentialModal with OpenAI workload identity federation", () => { await user.click(screen.getByRole("button", { name: "Update Credential" })); - expect(onSubmit).toHaveBeenCalledWith({ credential_name: "openai-federated", custom_llm_provider: "openai" }, []); + expect(onSubmit).toHaveBeenCalledWith( + { credential_name: "openai-federated", custom_llm_provider: "openai", credential_values: {} }, + [], + ); }); it("deletes the stored OpenAI federation values when the admin switches the credential to an API key", async () => { @@ -778,7 +805,7 @@ describe("CredentialModal with OpenAI workload identity federation", () => { expect(values).toEqual({ credential_name: "openai-federated", custom_llm_provider: "openai", - api_key: "sk-proj-replacement", + credential_values: { api_key: "sk-proj-replacement" }, }); expect([...valuesToDelete].sort()).toEqual([ "openai_identity_provider_id", @@ -801,7 +828,53 @@ describe("CredentialModal with OpenAI workload identity federation", () => { ); await user.click(screen.getByRole("button", { name: "Update Credential" })); expect(onSubmit).toHaveBeenCalledWith( - { credential_name: "anthropic-federated", custom_llm_provider: "Anthropic" }, + { credential_name: "anthropic-federated", custom_llm_provider: "Anthropic", credential_values: {} }, + [], + ); + }); +}); + +describe("CredentialModal display name wiring", () => { + it("submits the typed label alongside the credential name and stored values", async () => { + const user = userEvent.setup(); + const onSubmit = renderModal({ initialProvider: Providers.OpenAI }); + await screen.findByLabelText("OpenAI API Key"); + fill("Credential Name:", "openai-prod"); + fill("Display Name:", "Prod OpenAI"); + fill("OpenAI API Key", "sk-new"); + + await user.click(screen.getByRole("button", { name: "Add Credential" })); + + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + credential_name: "openai-prod", + display_name: "Prod OpenAI", + credential_values: expect.objectContaining({ api_key: "sk-new" }), + }), + [], + ); + }); + + it("edits the opened credential even when its stored values reuse the top-level keys", async () => { + const user = userEvent.setup(); + const onSubmit = renderModal({ + mode: "edit", + existingCredential: { + credential_name: "legacy-a", + display_name: "Legacy A", + credential_values: { api_key: "sk-a****", credential_name: "other-credential", display_name: "Inner label" }, + credential_info: { custom_llm_provider: "openai" }, + }, + }); + await screen.findByLabelText("OpenAI API Key"); + + expect(screen.getByLabelText("Credential Name:")).toHaveValue("legacy-a"); + expect(screen.getByLabelText("Credential Name:")).toBeDisabled(); + expect(screen.getByLabelText("Display Name:")).toHaveValue("Legacy A"); + await user.click(screen.getByRole("button", { name: "Update Credential" })); + + expect(onSubmit).toHaveBeenCalledWith( + { credential_name: "legacy-a", custom_llm_provider: "openai", credential_values: {} }, [], ); }); diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx index 4fa3a3cc6bd..8d94ae0d213 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx @@ -1,6 +1,5 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; +import { render, screen, waitFor } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { Providers } from "../provider_info_helpers"; import { CredentialItem } from "../networking"; @@ -126,142 +125,4 @@ describe("CredentialModal", () => { expect(screen.getByLabelText("Credential Name:")).toBeDisabled(); }); }); - - describe("display name", () => { - const fillRequiredAddFields = async (user: ReturnType) => { - fireEvent.change(screen.getByLabelText("Credential Name:"), { target: { value: "new-cred" } }); - const providerInput = await screen.findByPlaceholderText("Select a provider"); - await user.click(providerInput); - await user.click(await screen.findByText("OpenAI")); - fireEvent.change(await screen.findByLabelText("OpenAI API Key"), { target: { value: "sk-test" } }); - }; - - it("submits the display name with the credential name in add mode", async () => { - const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); - const onSubmit = vi.fn(); - renderModal({ mode: "add", onSubmit }); - - await fillRequiredAddFields(user); - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "Prod" } }); - fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0]).toMatchObject({ credential_name: "new-cred", display_name: "Prod" }); - }); - - it("submits no display_name key when the display name is left blank in add mode", async () => { - const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); - const onSubmit = vi.fn(); - renderModal({ mode: "add", onSubmit }); - - await fillRequiredAddFields(user); - fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0]).not.toHaveProperty("display_name"); - }); - - it("prefills the display name and submits an edited one", async () => { - const onSubmit = vi.fn(); - renderModal({ - mode: "edit", - onSubmit, - existingCredential: { ...mockCredential, display_name: "Prod" }, - }); - - const displayNameInput = screen.getByLabelText("Display Name:") as HTMLInputElement; - expect(displayNameInput.value).toBe("Prod"); - - fireEvent.change(displayNameInput, { target: { value: "Staging" } }); - fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0]).toMatchObject({ - credential_name: "test-credential", - display_name: "Staging", - }); - }); - - it("submits display_name: null when the display name is cleared in edit mode", async () => { - const onSubmit = vi.fn(); - renderModal({ - mode: "edit", - onSubmit, - existingCredential: { ...mockCredential, display_name: "Prod" }, - }); - - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "" } }); - fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0].display_name).toBeNull(); - }); - - it("trims the display name before submitting in add mode", async () => { - const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); - const onSubmit = vi.fn(); - renderModal({ mode: "add", onSubmit }); - - await fillRequiredAddFields(user); - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " Prod " } }); - fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0]).toMatchObject({ credential_name: "new-cred", display_name: "Prod" }); - }); - - it("submits display_name: null when the display name is whitespace-only in edit mode", async () => { - const onSubmit = vi.fn(); - renderModal({ - mode: "edit", - onSubmit, - existingCredential: { ...mockCredential, display_name: "Prod OpenAI" }, - }); - - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " " } }); - fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0].display_name).toBeNull(); - }); - - it("leaves display_name out of the edit when it was not changed", async () => { - const onSubmit = vi.fn(); - renderModal({ - mode: "edit", - onSubmit, - existingCredential: { ...mockCredential, display_name: "Prod" }, - }); - - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " Prod " } }); - fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); - - await waitFor(() => expect(onSubmit).toHaveBeenCalled()); - expect(onSubmit.mock.calls[0][0]).not.toHaveProperty("display_name"); - }); - - it("accepts a 255-character display name and blocks a 256-character one", async () => { - const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); - const onSubmit = vi.fn(); - renderModal({ mode: "add", onSubmit }); - - await fillRequiredAddFields(user); - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "x".repeat(256) } }); - fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); - expect(await screen.findByText("Display name must be at most 255 characters")).toBeInTheDocument(); - expect(onSubmit).not.toHaveBeenCalled(); - - fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "x".repeat(255) } }); - fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); - await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); - expect(onSubmit.mock.calls[0][0].display_name).toBe("x".repeat(255)); - }); - - it("keeps the credential name read-only while the display name stays editable in edit mode", () => { - renderModal({ mode: "edit", existingCredential: { ...mockCredential, display_name: "Prod" } }); - - expect(screen.getByLabelText("Credential Name:")).toBeDisabled(); - expect(screen.getByLabelText("Display Name:")).toBeEnabled(); - }); - }); }); diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx index c97e4658817..d0e28a0e273 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx @@ -18,7 +18,13 @@ import { import { CredentialItem } from "../networking"; import { Providers } from "../provider_info_helpers"; import { Logo } from "@/components/molecules/logo/Logo"; -import { resetCredentialFormOnProviderChange, withoutRestrictedFields } from "./credential_form_helpers"; +import { + displayNameChange, + initialFormValues, + resetCredentialFormOnProviderChange, + withoutRestrictedFields, + type CredentialSubmission, +} from "./credential_form_helpers"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import FederationFields from "./FederationFields"; import InternalIssuerJwks from "./InternalIssuerJwks"; @@ -55,7 +61,7 @@ const authMethodItems: { value: AuthMethod; label: string }[] = [ interface CredentialModalProps { open: boolean; onCancel: () => void; - onSubmit: (values: Record, valuesToDelete: readonly string[]) => void; + onSubmit: (submission: CredentialSubmission, valuesToDelete: readonly string[]) => void; mode: "add" | "edit"; existingCredential?: CredentialItem | null; initialProvider?: string | null; @@ -67,39 +73,6 @@ interface CredentialModalProps { const sameProvider = (left: string | null | undefined, right: string | null | undefined): boolean => (left ?? "").toLowerCase() === (right ?? "").toLowerCase(); -const DISPLAY_NAME_MAX_LENGTH = 255; - -const displayNameChange = ( - value: unknown, - existingCredential: CredentialItem | null | undefined, -): { display_name?: string | null } => { - const trimmed = typeof value === "string" ? value.trim() : ""; - if (!existingCredential) { - return trimmed ? { display_name: trimmed } : {}; - } - if (trimmed === (existingCredential.display_name ?? "")) { - return {}; - } - return { display_name: trimmed || null }; -}; - -const initialFormValues = ( - existingCredential: CredentialItem | null | undefined, - initialProvider: string | null | undefined, -): MountedFormValues | undefined => { - if (existingCredential) { - return { - credential_name: existingCredential.credential_name, - display_name: existingCredential.display_name ?? "", - custom_llm_provider: existingCredential.credential_info.custom_llm_provider, - ...Object.fromEntries( - Object.entries(existingCredential.credential_values || {}).map(([key, value]) => [key, value ?? null]), - ), - }; - } - return initialProvider ? { custom_llm_provider: initialProvider } : undefined; -}; - export default function CredentialModal({ open, onCancel, @@ -158,12 +131,15 @@ export default function CredentialModal({ } const values = projectMountedValues(registry, form.getValues); const meta = { - credential_name: values.credential_name, - custom_llm_provider: values.custom_llm_provider, + credential_name: String(values.credential_name), + custom_llm_provider: String(values.custom_llm_provider), ...displayNameChange(values.display_name, existingCredential), }; if (!isEdit) { - onSubmit({ ...meta, ...buildCreateCredentialValues(withoutRestrictedFields(values), selection) }, []); + onSubmit( + { ...meta, credential_values: buildCreateCredentialValues(withoutRestrictedFields(values), selection) }, + [], + ); return; } const sameStoredProvider = sameProvider(selectedProvider, storedProvider); @@ -175,7 +151,7 @@ export default function CredentialModal({ ? authTypeFieldKeys(selectedProvider).filter((key) => key in storedValues && !(key in projectedValues)) : []; const valuesToDelete = Array.from(new Set([...patch.credential_values_to_delete, ...authTypeValuesToDelete])); - onSubmit({ ...meta, ...patch.credential_values }, valuesToDelete); + onSubmit({ ...meta, credential_values: patch.credential_values }, valuesToDelete); }; const closeAndReset = () => { @@ -216,19 +192,7 @@ export default function CredentialModal({ )} - - typeof value !== "string" || - value.trim().length <= DISPLAY_NAME_MAX_LENGTH || - `Display name must be at most ${DISPLAY_NAME_MAX_LENGTH} characters`, - }, - }} - className="mb-4" - > + {(control) => ( ({ }: { mode: "add" | "edit"; open: boolean; - onSubmit: (values: Record, valuesToDelete?: readonly string[]) => void; + onSubmit: (submission: CredentialSubmission, valuesToDelete?: readonly string[]) => void; }) { if (!open) { return null; } - const values = + const values: CredentialSubmission = mode === "edit" ? { credential_name: "openai-key", custom_llm_provider: "openai", - api_key: "sk-1****2345", - api_base: "https://proxy.e2e.example.com/v1", + credential_values: { + api_key: "sk-1****2345", + api_base: "https://proxy.e2e.example.com/v1", + credential_name: "other-credential", + }, } - : { credential_name: "new-cred", custom_llm_provider: "openai" }; + : { credential_name: "new-cred", custom_llm_provider: "openai", credential_values: {} }; return ( <>