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 ( <>