From 930de4f4577acdd5b8169c409da211ae4ae97298 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Thu, 8 Oct 2026 21:44:38 -0700 Subject: [PATCH] feat(credentials): replace credential_alias with display_name and reject edits to config credentials credential_name stays the immutable reference key. display_name is a nullable, trimmed, max 255 character label set on POST or PATCH (omit keeps it, null clears it, blank is a 400). Reads return display_name plus an in-memory source tag (db or config), and PATCH or DELETE on a config-defined credential answers 400 instead of a misleading 404 --- .../migration.sql | 2 +- .../litellm_proxy_extras/schema.prisma | 2 +- litellm/models/credentials.py | 13 +- .../common_utils/credential_hydration.py | 1 + .../proxy/credential_endpoints/endpoints.py | 109 +++++--- .../key_management_endpoints.py | 2 +- litellm/proxy/proxy_server.py | 2 +- litellm/proxy/schema.prisma | 2 +- .../repositories/credentials_repository.py | 2 +- schema.prisma | 2 +- ...ias.py => test_credential_display_name.py} | 69 ++--- .../credential_endpoints/test_endpoints.py | 250 ++++++++++++++---- .../proxy/proxy_server/test_proxy_config.py | 43 ++- 13 files changed, 371 insertions(+), 128 deletions(-) rename litellm-proxy-extras/litellm_proxy_extras/migrations/{20260925000000_add_credential_alias => 20261009000000_add_credential_display_name}/migration.sql (75%) rename tests/integration/management/{test_credential_alias.py => test_credential_display_name.py} (50%) diff --git a/litellm-proxy-extras/litellm_proxy_extras/migrations/20260925000000_add_credential_alias/migration.sql b/litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql similarity index 75% rename from litellm-proxy-extras/litellm_proxy_extras/migrations/20260925000000_add_credential_alias/migration.sql rename to litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql index a9b21ebdc1e..264bd88a807 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/migrations/20260925000000_add_credential_alias/migration.sql +++ b/litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql @@ -1,2 +1,2 @@ -- AlterTable -ALTER TABLE "LiteLLM_CredentialsTable" ADD COLUMN IF NOT EXISTS "credential_alias" TEXT; +ALTER TABLE "LiteLLM_CredentialsTable" ADD COLUMN IF NOT EXISTS "display_name" TEXT; diff --git a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma index 87fd38e0f53..59ffb037177 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma +++ b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma @@ -42,7 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique - credential_alias String? + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/litellm/models/credentials.py b/litellm/models/credentials.py index b91ced275ff..563f25d1646 100644 --- a/litellm/models/credentials.py +++ b/litellm/models/credentials.py @@ -6,14 +6,18 @@ layer; ``litellm.types.utils`` re-exports them for backwards compatibility. """ from collections.abc import Mapping +from typing import Literal, TypeAlias from pydantic import Field, model_validator from litellm.types.llms.base import LiteLLMBaseModel +CredentialSource: TypeAlias = Literal["db", "config"] + class CredentialBase(LiteLLMBaseModel): credential_name: str + display_name: str | None = None credential_info: dict @@ -23,6 +27,12 @@ class CredentialItem(CredentialBase): # edit rather than the credential, so it stays out of dumps: those feed config loading, the DB # write, and the in-memory list, none of which have a place for it. credential_values_to_delete: tuple[str, ...] | None = Field(default=None, exclude=True) + source: CredentialSource = Field(default="db", exclude=True) + + +class CredentialView(CredentialBase): + credential_values: dict[str, object] + source: CredentialSource class CreateCredentialItem(CredentialBase): @@ -38,7 +48,8 @@ class CreateCredentialItem(CredentialBase): class UpdateCredentialItem(LiteLLMBaseModel): - credential_name: str + credential_name: str | None = None + display_name: str | None = None credential_info: Mapping[str, object] credential_values: Mapping[str, object] | None = None model_id: str | None = None diff --git a/litellm/proxy/common_utils/credential_hydration.py b/litellm/proxy/common_utils/credential_hydration.py index 2aabe8cad5c..c2a9fb25553 100644 --- a/litellm/proxy/common_utils/credential_hydration.py +++ b/litellm/proxy/common_utils/credential_hydration.py @@ -71,6 +71,7 @@ def _decrypted(db_credential: CredentialItem) -> CredentialItem: ) return CredentialItem( credential_name=db_credential.credential_name, + display_name=db_credential.display_name, credential_values=decrypted_values, # pyright: ignore[reportArgumentType] # declared dict[str, str], and pydantic copies this mapping into one on validation; LIT002 rules out building that dict here credential_info=db_credential.credential_info, ) diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index 6d25307db96..330ddd2ebbd 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -22,7 +22,7 @@ from litellm.llms.anthropic.wif import ( UnbuildableIdentitySource, anthropic_internal_issuer_jwks, ) -from litellm.models.credentials import UpdateCredentialItem +from litellm.models.credentials import CredentialView, UpdateCredentialItem from litellm.proxy._types import ( CommonProxyErrors, LitellmUserRoles, @@ -32,7 +32,6 @@ from litellm.proxy._types import ( ) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.credential_hydration import ( - hydrate_named_credential, hydrate_named_credential_authoritative, named_credential_wif_fields, stored_credential_provider, @@ -46,6 +45,7 @@ from litellm.types.utils import CreateCredentialItem, CredentialItem router: Final = APIRouter() _CREDENTIAL_DICT_ADAPTER: Final = TypeAdapter(dict[str, object]) +_DISPLAY_NAME_MAX_LENGTH: Final = 255 def _reject_non_admin_wif_fields( @@ -105,7 +105,7 @@ def _without_null_values(credential_values: Mapping[str, object]) -> dict[str, o return {key: value for key, value in credential_values.items() if value is not None} -def _sync_in_memory_credential(credential: CredentialItem, credential_name: str, new_name: str) -> None: +def _sync_in_memory_credential(credential: CredentialItem, credential_name: str) -> None: """Mirror a DB credential update into the in-memory ``credential_list`` used by request-time resolution; a no-op if the credential isn't loaded in memory (e.g. proxy restarted since boot). """ @@ -127,13 +127,11 @@ def _sync_in_memory_credential(credential: CredentialItem, credential_name: str, if credential.credential_info: in_memory_info.update(credential.credential_info) updated_in_memory: Final = CredentialItem( - credential_name=new_name, + credential_name=credential_name, + display_name=credential.display_name, credential_values=in_memory_values, credential_info=in_memory_info, ) - # Remove old entry if renamed, then use upsert_credentials to handle duplicates - if new_name != credential_name: - litellm.credential_list = [c for c in litellm.credential_list if c.credential_name != credential_name] CredentialAccessor.upsert_credentials([updated_in_memory]) @@ -149,11 +147,55 @@ class CredentialHelperUtils: # is kept in memory and should remain unencrypted. return CredentialItem( credential_name=credential.credential_name, + display_name=credential.display_name, credential_values=encrypted_credential_values, credential_info=credential.credential_info or {}, ) +def _normalized_display_name(display_name: str | None) -> str | None: + if display_name is None: + return None + trimmed: Final = display_name.strip() + if not trimmed: + raise ProxyException( + message="display_name cannot be blank. Send null to clear it or omit the field to leave it unchanged.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="display_name", + ) + if len(trimmed) > _DISPLAY_NAME_MAX_LENGTH: + raise ProxyException( + message=f"display_name cannot be longer than {_DISPLAY_NAME_MAX_LENGTH} characters.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="display_name", + ) + return trimmed + + +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": + return HTTPException(status_code=404, detail=not_found_detail) + return ProxyException( + message=f"Credential '{credential_name}' is defined in config and cannot be edited from the API or UI.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="credential_name", + ) + + +def _credential_view(credential: CredentialItem, credential_values: dict[str, object]) -> CredentialView: + return CredentialView( + credential_name=credential.credential_name, + display_name=credential.display_name, + credential_values=credential_values, + credential_info=credential.credential_info, + source=credential.source, + ) + + def _credential_exists_detail(credential_name: str) -> str: return ( f"Credential '{credential_name}' already exists. " @@ -222,6 +264,7 @@ async def create_credential( ) processed_credential: Final = CredentialItem( credential_name=credential.credential_name, + display_name=_normalized_display_name(credential.display_name), credential_values=_without_null_values(_CREDENTIAL_DICT_ADAPTER.validate_python(credential_values)), credential_info=credential.credential_info, ) @@ -267,11 +310,7 @@ async def get_credentials( """ try: masked_credentials: Final = [ - { - "credential_name": credential.credential_name, - "credential_values": get_masked_values(credential.credential_values), - "credential_info": credential.credential_info, - } + _credential_view(credential, get_masked_values(credential.credential_values)) for credential in litellm.credential_list ] return {"success": True, "credentials": masked_credentials} @@ -283,7 +322,7 @@ async def get_credentials( "/credentials/by_name/{credential_name:path}", dependencies=[Depends(user_api_key_auth)], tags=["credential management"], - response_model=CredentialItem, + response_model=CredentialView, ) async def get_credential_by_name( request: Request, @@ -297,16 +336,10 @@ async def get_credential_by_name( try: for credential in litellm.credential_list: if credential.credential_name == credential_name: - masked_credential = CredentialItem( - credential_name=credential.credential_name, - credential_values=get_masked_values( - credential.credential_values, - unmasked_length=4, - number_of_asterisks=4, - ), - credential_info=credential.credential_info, + return _credential_view( + credential, + get_masked_values(credential.credential_values, unmasked_length=4, number_of_asterisks=4), ) - return masked_credential raise HTTPException( status_code=404, detail="Credential not found. Got credential name: " + credential_name, @@ -444,9 +477,8 @@ async def delete_credential( ) deleted: Final = await CredentialsRepository(prisma_client).delete_by_name(credential_name) if deleted is None: - raise HTTPException( - status_code=404, - detail="Credential not found. Got credential name: " + credential_name, + raise _not_found_unless_config_defined( + credential_name, "Credential not found. Got credential name: " + credential_name ) ## DELETE FROM LITELLM ## @@ -466,6 +498,7 @@ def update_db_credential( """ merged_credential: Final = CredentialItem( credential_name=db_credential.credential_name, + display_name=updated_patch.display_name, credential_info=db_credential.credential_info, credential_values=db_credential.credential_values, ) @@ -474,10 +507,6 @@ def update_db_credential( updated_patch, new_encryption_key, ) - # update model name - if encrypted_credential.credential_name: - merged_credential.credential_name = encrypted_credential.credential_name - # update litellm params if encrypted_credential.credential_values: # Encrypt any sensitive values @@ -515,6 +544,14 @@ async def update_credential( from litellm.proxy.proxy_server import prisma_client try: + if credential.credential_name is not None and credential.credential_name != credential_name: + raise ProxyException( + message="credential_name is immutable. Set display_name to change how the credential is labeled.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="credential_name", + ) + requested_display_name: Final = _normalized_display_name(credential.display_name) _reject_overlapping_credential_values(credential) incoming_values: Final = _CREDENTIAL_DICT_ADAPTER.validate_python( _resolve_deployment_credentials(llm_router, credential.model_id) @@ -530,14 +567,13 @@ async def update_credential( credentials_repository: Final = CredentialsRepository(prisma_client) db_credential: Final = await credentials_repository.find_by_name(credential_name) if db_credential is None: - raise HTTPException(status_code=404, detail="Credential not found in DB.") + raise _not_found_unless_config_defined(credential_name, "Credential not found in DB.") _reject_non_admin_wif_fields(_stored_wif_fields(db_credential), user_api_key_dict) - if credential.credential_name != credential_name: - shadowed_credential: Final = await hydrate_named_credential(credential.credential_name, prisma_client) - if shadowed_credential is not None: - _reject_non_admin_wif_fields(_stored_wif_fields(shadowed_credential), user_api_key_dict) patch: Final = CredentialItem( - credential_name=credential.credential_name, + credential_name=credential_name, + display_name=( + requested_display_name if "display_name" in credential.model_fields_set else db_credential.display_name + ), credential_info=_CREDENTIAL_DICT_ADAPTER.validate_python(credential.credential_info), credential_values=incoming_values, credential_values_to_delete=credential.credential_values_to_delete, @@ -550,12 +586,13 @@ async def update_credential( credential_name, data={ **credential_object_jsonified, + "display_name": merged_credential.display_name, "updated_by": user_api_key_dict.user_id, }, ) # Sync in-memory credential_list (skip if not in memory - e.g., proxy restarted) - _sync_in_memory_credential(patch, credential_name, merged_credential.credential_name) + _sync_in_memory_credential(patch, credential_name) return {"success": True, "message": "Credential updated successfully"} except Exception as e: diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 821c38937bc..ac9dfeb136d 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -5400,7 +5400,7 @@ async def rotate_master_key( try: decrypted_cred = proxy_config.decrypt_credentials(cred) encrypted_cred = update_db_credential( - db_credential=decrypted_cred, + db_credential=cred, updated_patch=decrypted_cred, new_encryption_key=new_master_key, ) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index e035f061db9..3b1942be62c 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -6313,7 +6313,7 @@ class ProxyConfig: credential_list_dict: Final = config.get("credential_list") credential_list = [] if credential_list_dict: - credential_list = [CredentialItem(**cred) for cred in credential_list_dict] + credential_list = [CredentialItem(**{**cred, "source": "config"}) for cred in credential_list_dict] return credential_list def parse_search_tools(self, config: dict) -> list[SearchToolTypedDict] | None: diff --git a/litellm/proxy/schema.prisma b/litellm/proxy/schema.prisma index 87fd38e0f53..59ffb037177 100644 --- a/litellm/proxy/schema.prisma +++ b/litellm/proxy/schema.prisma @@ -42,7 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique - credential_alias String? + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/litellm/repositories/credentials_repository.py b/litellm/repositories/credentials_repository.py index 1d8b4e0179e..fa6aeca534d 100644 --- a/litellm/repositories/credentials_repository.py +++ b/litellm/repositories/credentials_repository.py @@ -58,7 +58,7 @@ class CredentialsRepository: return CredentialItem.model_validate( { "credential_name": data["credential_name"], - "credential_alias": data.get("credential_alias"), + "display_name": data.get("display_name"), "credential_values": data.get("credential_values") or {}, "credential_info": data.get("credential_info") or {}, } diff --git a/schema.prisma b/schema.prisma index 87fd38e0f53..59ffb037177 100644 --- a/schema.prisma +++ b/schema.prisma @@ -42,7 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique - credential_alias String? + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/tests/integration/management/test_credential_alias.py b/tests/integration/management/test_credential_display_name.py similarity index 50% rename from tests/integration/management/test_credential_alias.py rename to tests/integration/management/test_credential_display_name.py index ff97893fba1..0e72dcf8b2f 100644 --- a/tests/integration/management/test_credential_alias.py +++ b/tests/integration/management/test_credential_display_name.py @@ -7,13 +7,13 @@ from tests.integration._support.client import Gateway, Scenario from tests.integration._support.database import read_rows -def _credential_with_alias(gateway: Gateway, scenario: Scenario, alias: str) -> str: +def _credential_with_display_name(gateway: Gateway, scenario: Scenario, display_name: str) -> str: name: Final = f"credential-{uuid.uuid4().hex}" gateway.post( "/credentials", { "credential_name": name, - "credential_alias": alias, + "display_name": display_name, "credential_values": {"api_key": "synthetic-credential"}, "credential_info": {"custom_llm_provider": "openai"}, }, @@ -27,9 +27,9 @@ def _delete_credential_if_present(gateway: Gateway, name: str) -> None: assert response.status_code in (200, 404), response.text -def _stored_alias(name: str) -> list[dict[str, JsonValue]]: +def _stored_display_name(name: str) -> list[dict[str, JsonValue]]: return read_rows( - 'SELECT credential_name, credential_alias FROM "LiteLLM_CredentialsTable" WHERE credential_name = %s', + 'SELECT credential_name, display_name FROM "LiteLLM_CredentialsTable" WHERE credential_name = %s', (name,), ) @@ -39,7 +39,7 @@ def _served_credential(gateway: Gateway, name: str) -> dict[str, JsonValue]: listed: Final = [entry for entry in gateway.get("/credentials")["credentials"] if isinstance(entry, dict)] from_list: Final = [entry for entry in listed if entry["credential_name"] == name] assert len(from_list) == 1, listed - assert from_list[0]["credential_alias"] == by_name["credential_alias"], (from_list[0], by_name) + assert from_list[0]["display_name"] == by_name["display_name"], (from_list[0], by_name) return by_name @@ -51,51 +51,60 @@ def _model_using(gateway: Gateway, scenario: Scenario, credential: str) -> str: ) -def test_alias_round_trips_and_patch_keeps_clears_and_rejects_blank(gateway: Gateway) -> None: +def test_display_name_round_trips_and_patch_keeps_clears_and_rejects_blank(gateway: Gateway) -> None: with gateway.scenario() as scenario: - name: Final = _credential_with_alias(gateway, scenario, "Prod OpenAI") - assert _stored_alias(name) == [{"credential_name": name, "credential_alias": "Prod OpenAI"}] - assert _served_credential(gateway, name)["credential_alias"] == "Prod OpenAI" + name: Final = _credential_with_display_name(gateway, scenario, "Prod OpenAI") + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Prod OpenAI"}] + assert _served_credential(gateway, name)["display_name"] == "Prod OpenAI" kept: Final = gateway.request("PATCH", f"/credentials/{name}", {"credential_info": {"description": "d"}}) assert kept.status_code == 200, kept.text - assert _served_credential(gateway, name)["credential_alias"] == "Prod OpenAI" + assert _served_credential(gateway, name)["display_name"] == "Prod OpenAI" - renamed_alias: Final = gateway.request( - "PATCH", f"/credentials/{name}", {"credential_alias": "Staging OpenAI", "credential_info": {}} + relabeled: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": "Staging OpenAI", "credential_info": {}} ) - assert renamed_alias.status_code == 200, renamed_alias.text - assert _stored_alias(name) == [{"credential_name": name, "credential_alias": "Staging OpenAI"}] - assert _served_credential(gateway, name)["credential_alias"] == "Staging OpenAI" + assert relabeled.status_code == 200, relabeled.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] + assert _served_credential(gateway, name)["display_name"] == "Staging OpenAI" - blank: Final = gateway.request( - "PATCH", f"/credentials/{name}", {"credential_alias": " ", "credential_info": {}} - ) + blank: Final = gateway.request("PATCH", f"/credentials/{name}", {"display_name": " ", "credential_info": {}}) assert blank.status_code == 400, blank.text - assert _stored_alias(name) == [{"credential_name": name, "credential_alias": "Staging OpenAI"}] + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] - cleared: Final = gateway.request( - "PATCH", f"/credentials/{name}", {"credential_alias": None, "credential_info": {}} + too_long: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": "x" * 256, "credential_info": {}} ) + assert too_long.status_code == 400, too_long.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] + + trimmed: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": " Staging OpenAI v2 ", "credential_info": {}} + ) + assert trimmed.status_code == 200, trimmed.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI v2"}] + assert _served_credential(gateway, name)["source"] == "db" + + cleared: Final = gateway.request("PATCH", f"/credentials/{name}", {"display_name": None, "credential_info": {}}) assert cleared.status_code == 200, cleared.text - assert _stored_alias(name) == [{"credential_name": name, "credential_alias": None}] - assert _served_credential(gateway, name)["credential_alias"] is None + assert _stored_display_name(name) == [{"credential_name": name, "display_name": None}] + assert _served_credential(gateway, name)["display_name"] is None -def test_blank_alias_on_create_is_rejected(gateway: Gateway) -> None: +def test_blank_display_name_on_create_is_rejected(gateway: Gateway) -> None: name: Final = f"credential-{uuid.uuid4().hex}" rejected: Final = gateway.request( "POST", "/credentials", - {"credential_name": name, "credential_alias": "", "credential_values": {"api_key": "k"}, "credential_info": {}}, + {"credential_name": name, "display_name": "", "credential_values": {"api_key": "k"}, "credential_info": {}}, ) assert rejected.status_code == 400, rejected.text - assert _stored_alias(name) == [] + assert _stored_display_name(name) == [] def test_patch_rename_is_rejected_and_model_keeps_working(gateway: Gateway) -> None: with gateway.scenario() as scenario: - name: Final = _credential_with_alias(gateway, scenario, "Prod OpenAI") + name: Final = _credential_with_display_name(gateway, scenario, "Prod OpenAI") model: Final = _model_using(gateway, scenario, name) assert gateway.chat(model)["object"] == "chat.completion" @@ -104,9 +113,9 @@ def test_patch_rename_is_rejected_and_model_keeps_working(gateway: Gateway) -> N "PATCH", f"/credentials/{name}", {"credential_name": other, "credential_info": {"description": "x"}} ) assert renamed.status_code == 400, renamed.text - assert "credential_alias" in renamed.text, renamed.text - assert _stored_alias(name) == [{"credential_name": name, "credential_alias": "Prod OpenAI"}] - assert _stored_alias(other) == [] + assert "display_name" in renamed.text, renamed.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Prod OpenAI"}] + assert _stored_display_name(other) == [] assert gateway.request("GET", f"/credentials/by_name/{other}").status_code == 404 same_name: Final = gateway.request( diff --git a/tests/unit/proxy/credential_endpoints/test_endpoints.py b/tests/unit/proxy/credential_endpoints/test_endpoints.py index aff7babe424..e8d39a5380a 100644 --- a/tests/unit/proxy/credential_endpoints/test_endpoints.py +++ b/tests/unit/proxy/credential_endpoints/test_endpoints.py @@ -14,6 +14,7 @@ from litellm.proxy._types import UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.credential_endpoints.endpoints import get_llm_router from litellm.proxy.proxy_server import app +from litellm.models.credentials import CredentialSource from litellm.types.utils import CredentialItem client = TestClient(app) @@ -771,7 +772,7 @@ class TestNonAdminCannotPersistWifFieldsOnCredential: update_mock.assert_awaited_once() -def _wif_credential(name: str = "federated-cred") -> CredentialItem: +def _wif_credential(name: str = "federated-cred", source: CredentialSource = "db") -> CredentialItem: return CredentialItem( credential_name=name, credential_values={ @@ -779,6 +780,7 @@ def _wif_credential(name: str = "federated-cred") -> CredentialItem: "api_key": "sk-old", }, credential_info={"custom_llm_provider": "anthropic"}, + source=source, ) @@ -978,7 +980,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: def test_non_admin_cannot_delete_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): """A ``credential_list`` entry from config.yaml has no DB row, so a gate that consulted only the DB let a non-admin evict the admin-owned federation settings from memory.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _delete_credential("config-wif", auth=_as_non_admin) @@ -989,15 +991,16 @@ class TestNonAdminCannotTouchAStoredWifCredential: assert litellm.credential_list == [config_credential] def test_proxy_admin_can_delete_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): - """The gate lets the admin through to the row delete. The 404 that follows is the rule for + """The gate lets the admin through to the row delete. The 400 that follows is the rule for every config-only credential (no row to delete, the entry is back on the next boot), so the in-memory entry stays put too.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _delete_credential("config-wif", auth=_as_admin) - assert response.status_code == 404, response.text + assert response.status_code == 400, response.text + assert "defined in config" in response.json()["error"]["message"] repository.delete_by_name.assert_awaited_once_with("config-wif") assert litellm.credential_list == [config_credential] @@ -1005,7 +1008,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: """POST with the same name carries no WIF field and collides with no DB row, yet ``CredentialAccessor.upsert_credentials`` would replace the admin entry in memory and the periodic config sync would then make the takeover permanent.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _post_credential( @@ -1024,7 +1027,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: 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")]) + monkeypatch.setattr(litellm, "credential_list", [_wif_credential("config-wif", source="config")]) with _repository_holding(None) as repository: response = _post_credential( { @@ -1039,50 +1042,6 @@ class TestNonAdminCannotTouchAStoredWifCredential: repository.create.assert_awaited_once() assert litellm.credential_list[0].credential_values == {"api_key": "sk-rotated"} - def test_non_admin_cannot_rename_a_credential_onto_a_config_only_wif_credential( - self, restore_credential_list, monkeypatch - ): - """PATCH is the other way to shadow: renaming an ordinary credential onto the WIF - credential's name makes ``_sync_in_memory_credential`` upsert the attacker's values over - the admin entry, with no WIF field in the payload and no DB row to collide with.""" - config_credential = _wif_credential("config-wif") - monkeypatch.setattr(litellm, "credential_list", [_plain_credential("mine"), config_credential]) - with _repository_holding(_plain_credential("mine")) as repository: - response = _patch_credential( - "mine", - { - "credential_name": "config-wif", - "credential_values": {"api_key": "sk-attacker"}, - "credential_info": {}, - }, - auth=_as_non_admin, - ) - - assert response.status_code == 403, response.text - assert "anthropic_keycloak_token_url" in response.text - repository.update_by_name.assert_not_awaited() - assert config_credential in litellm.credential_list - assert litellm.credential_list[1].credential_values["api_key"] == "sk-old" - - def test_proxy_admin_can_rename_a_credential_onto_a_config_only_wif_credential( - self, restore_credential_list, monkeypatch - ): - monkeypatch.setattr(litellm, "credential_list", [_plain_credential("mine"), _wif_credential("config-wif")]) - with _repository_holding(_plain_credential("mine")) as repository: - response = _patch_credential( - "mine", - { - "credential_name": "config-wif", - "credential_values": {"api_key": "sk-rotated"}, - "credential_info": {}, - }, - auth=_as_admin, - ) - - assert response.status_code == 200, response.text - repository.update_by_name.assert_awaited_once() - assert [c.credential_name for c in litellm.credential_list] == ["config-wif"] - 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 the key, and the resolver reacts to the key.""" @@ -1228,17 +1187,19 @@ def test_delete_credential_still_answers_200_and_drops_the_credential_from_memor def test_delete_credential_leaves_a_credential_that_only_exists_in_memory_in_place(credential_store): """A credential declared in the config yaml is never written to the table, so the delete matches no row. Reporting success would be the same lie: it comes straight back on the next - proxy boot. ``PATCH /credentials/{name}`` already answers 404 for that credential.""" + proxy boot.""" config_only = CredentialItem( credential_name="from-config-yaml", credential_values={"api_key": "sk-config"}, credential_info={}, + source="config", ) credential_store(in_memory=(config_only,), delete_by_name=AsyncMock(return_value=None)) response = _delete_credential("from-config-yaml") - assert response.status_code == 404, response.text + assert response.status_code == 400, response.text + assert "defined in config" in response.json()["error"]["message"] assert [credential.credential_name for credential in litellm.credential_list] == ["from-config-yaml"] @@ -1399,3 +1360,186 @@ def test_update_credential_still_accepts_a_body_without_credential_values(creden written = update_by_name.await_args.kwargs["data"] assert json.loads(written["credential_info"]) == {"custom_llm_provider": "openai"} assert set(json.loads(written["credential_values"])) == {"api_key"}, "stored values survive an info-only patch" + + +def _labeled_credential(name: str = "openai-prod", display_name: str | None = "Prod OpenAI") -> CredentialItem: + return CredentialItem( + credential_name=name, + display_name=display_name, + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai"}, + ) + + +class TestCredentialDisplayName: + def test_create_stores_the_trimmed_display_name_in_the_row_and_in_memory(self, restore_credential_list): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": " Prod OpenAI ", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + + assert response.status_code == 200, response.text + assert repository.create.await_args.kwargs["data"]["display_name"] == "Prod OpenAI" + assert [(c.credential_name, c.display_name) for c in litellm.credential_list] == [ + ("openai-prod", "Prod OpenAI") + ] + + @pytest.mark.parametrize("display_name", ["", " ", "x" * 256]) + def test_create_rejects_an_unusable_display_name(self, restore_credential_list, display_name): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": display_name, + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + } + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "display_name" + repository.create.assert_not_awaited() + assert litellm.credential_list == [] + + def test_create_accepts_a_display_name_of_exactly_the_maximum_length(self, restore_credential_list): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": "x" * 255, + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + } + ) + + assert response.status_code == 200, response.text + assert repository.create.await_args.kwargs["data"]["display_name"] == "x" * 255 + + def test_patch_relabels_without_touching_the_name_or_the_values(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": "Staging OpenAI", "credential_info": {}}) + + assert response.status_code == 200, response.text + repository.update_by_name.assert_awaited_once() + assert repository.update_by_name.await_args.args[0] == "openai-prod" + written = repository.update_by_name.await_args.kwargs["data"] + assert written["credential_name"] == "openai-prod" + assert written["display_name"] == "Staging OpenAI" + assert json.loads(written["credential_values"]) == {"api_key": "sk-old"} + assert [(c.credential_name, c.display_name, c.credential_values) for c in litellm.credential_list] == [ + ("openai-prod", "Staging OpenAI", {"api_key": "sk-old"}) + ] + + def test_patch_without_display_name_keeps_the_stored_one(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"credential_info": {"description": "rotated"}}) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.kwargs["data"]["display_name"] == "Prod OpenAI" + assert litellm.credential_list[0].display_name == "Prod OpenAI" + + def test_patch_with_null_display_name_clears_it(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": None, "credential_info": {}}) + + assert response.status_code == 200, response.text + written = repository.update_by_name.await_args.kwargs["data"] + assert "display_name" in written + assert written["display_name"] is None + assert litellm.credential_list[0].display_name is None + + @pytest.mark.parametrize("display_name", ["", " ", "x" * 256]) + def test_patch_rejects_an_unusable_display_name_and_changes_nothing( + 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()] + + def test_patch_rejects_a_different_credential_name_and_changes_nothing(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential( + "openai-prod", + { + "credential_name": "openai-production", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + }, + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "credential_name" + assert "display_name" in response.json()["error"]["message"] + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [_labeled_credential()] + + def test_patch_accepts_the_same_credential_name_echoed_back(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential( + "openai-prod", + {"credential_name": "openai-prod", "credential_values": {"api_key": "sk-new"}, "credential_info": {}}, + ) + + assert response.status_code == 200, response.text + assert [(c.credential_name, c.credential_values) for c in litellm.credential_list] == [ + ("openai-prod", {"api_key": "sk-new"}) + ] + + def test_patch_on_a_config_credential_is_rejected_as_config_owned(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 = _patch_credential("openai-prod", {"display_name": "Prod", "credential_info": {}}) + + assert response.status_code == 400, response.text + assert "defined in config" in response.json()["error"]["message"] + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [config_credential] + + 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": {}}) + + assert response.status_code == 404, response.text + + def test_patch_on_a_name_stored_in_the_db_and_config_applies_to_the_row(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential(display_name=None)]) + with _repository_holding(_labeled_credential(display_name=None)) as repository: + response = _patch_credential("openai-prod", {"display_name": "Prod", "credential_info": {}}) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.kwargs["data"]["display_name"] == "Prod" + + def test_reads_return_display_name_and_source(self, restore_credential_list, monkeypatch): + config_credential = CredentialItem( + credential_name="from-config", + credential_values={"api_key": "sk-config-value"}, + credential_info={}, + source="config", + ) + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential(), config_credential]) + + listed = {entry["credential_name"]: entry for entry in _list_credentials().json()["credentials"]} + by_name = _call_as("GET", "/credentials/by_name/openai-prod").json() + config_by_name = _call_as("GET", "/credentials/by_name/from-config").json() + + assert (listed["openai-prod"]["display_name"], listed["openai-prod"]["source"]) == ("Prod OpenAI", "db") + assert (listed["from-config"]["display_name"], listed["from-config"]["source"]) == (None, "config") + assert (by_name["display_name"], by_name["source"]) == ("Prod OpenAI", "db") + assert config_by_name["source"] == "config" + assert listed["openai-prod"]["credential_values"]["api_key"] != "sk-old" diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index 08799ab2318..04a06454a8b 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -1851,12 +1851,29 @@ def test_ProxyConfig_load_credential_list_returns_items(): dumped = creds[0].model_dump() assert dumped == { "credential_name": "openai-key", - "credential_alias": None, + "display_name": None, "credential_info": {"provider": "openai"}, "credential_values": {"api_key": "sk-x"}, } +def test_ProxyConfig_load_credential_list_tags_every_entry_as_config_defined(): + creds = ProxyConfig().load_credential_list( + { + "credential_list": [ + {"credential_name": "plain", "credential_info": {}, "credential_values": {"api_key": "sk-x"}}, + { + "credential_name": "claims-db", + "source": "db", + "credential_info": {}, + "credential_values": {"api_key": "sk-y"}, + }, + ] + } + ) + assert [(cred.credential_name, cred.source) for cred in creds] == [("plain", "config"), ("claims-db", "config")] + + def test_ProxyConfig_load_credential_list_invalid_entry_raises(): pc = ProxyConfig() with pytest.raises(ValidationError): @@ -4022,6 +4039,30 @@ async def test_ProxyConfig_get_credentials_reads_from_writer_not_replica(monkeyp reader_inner.litellm_credentialstable.find_many.assert_not_awaited() +@pytest.mark.asyncio +async def test_ProxyConfig_get_credentials_carries_the_stored_display_name_and_marks_rows_as_db( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from litellm.litellm_core_utils.credential_accessor import CredentialAccessor + + pc = ProxyConfig() + fake_prisma = MagicMock() + fake_prisma.db.litellm_credentialstable.find_many = AsyncMock( + return_value=[{**_encrypted_credential_row("labeled-cred", "sk-labeled"), "display_name": "Prod OpenAI"}] + ) + _stub_add_deployment_collaborators(monkeypatch, pc, fake_prisma) + + await pc.get_credentials(prisma_client=fake_prisma) + + loaded = CredentialAccessor.find_credential("labeled-cred") + assert loaded is not None + assert (loaded.display_name, loaded.source, loaded.credential_values) == ( + "Prod OpenAI", + "db", + {"api_key": "sk-labeled"}, + ) + + # --------------------------------------------------------------------------- # ProxyConfig._reschedule_spend_log_cleanup_job # ---------------------------------------------------------------------------