diff --git a/litellm/litellm_core_utils/credential_accessor.py b/litellm/litellm_core_utils/credential_accessor.py index b2310894c3a..ae00f8eb8c5 100644 --- a/litellm/litellm_core_utils/credential_accessor.py +++ b/litellm/litellm_core_utils/credential_accessor.py @@ -15,10 +15,17 @@ from litellm.types.utils import ( class CredentialAccessor: @staticmethod def find_credential(credential_name: str) -> CredentialItem | None: - return next( + exact: Final = next( (credential for credential in litellm.credential_list if credential.credential_name == credential_name), None, ) + if exact is not None: + return exact + stripped: Final = credential_name.strip() + padded: Final = tuple( + credential for credential in litellm.credential_list if credential.credential_name.strip() == stripped + ) + return padded[0] if len(padded) == 1 else None @staticmethod def get_credential_values( diff --git a/litellm/proxy/common_utils/credential_hydration.py b/litellm/proxy/common_utils/credential_hydration.py index f6a6c682cf0..5ac664c27be 100644 --- a/litellm/proxy/common_utils/credential_hydration.py +++ b/litellm/proxy/common_utils/credential_hydration.py @@ -12,6 +12,7 @@ from types import MappingProxyType from typing import Final, cast # noqa: TID251 # narrows untyped credential values at the decrypt boundary import litellm +from litellm.litellm_core_utils.credential_accessor import CredentialAccessor from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper from litellm.proxy.utils import PrismaClient from litellm.repositories.credentials_repository import CredentialsRepository @@ -128,10 +129,8 @@ async def named_credential_wif_fields( fields would see none and allow the write. This reads both and returns the union, so the gate refuses whenever either side says the credential is server-owned. """ - matching: Final = tuple(c for c in litellm.credential_list if c.credential_name == credential_name) - in_memory: Final = tuple( - chain.from_iterable(server_owned_wif_fields_written(c.credential_values) for c in matching) - ) + matching: Final = CredentialAccessor.find_credential(credential_name) + in_memory: Final = () if matching is None else server_owned_wif_fields_written(matching.credential_values) if prisma_client is None: return in_memory db_credential: Final = await CredentialsRepository(prisma_client).find_by_name(credential_name) diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index 953c944bfb2..a5bc82f5697 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -193,6 +193,18 @@ class CredentialHelperUtils: ) +def _normalized_credential_name(credential_name: str) -> str: + trimmed: Final = credential_name.strip() + if not trimmed: + raise ProxyException( + message="credential_name cannot be blank.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="credential_name", + ) + return trimmed + + def _normalized_display_name(display_name: str | None) -> str | None: if display_name is None: return None @@ -308,6 +320,7 @@ async def create_credential( status_code=500, detail={"error": CommonProxyErrors.db_not_connected_error.value}, ) + credential_name: Final = _normalized_credential_name(credential.credential_name) credential_values: Final = ( _resolve_deployment_credentials(llm_router, credential.model_id) if credential.model_id @@ -320,11 +333,11 @@ async def create_credential( ) _reject_non_admin_wif_fields(server_owned_wif_fields_written(credential_values), user_api_key_dict) _reject_non_admin_wif_fields( - await named_credential_wif_fields(credential.credential_name, prisma_client), user_api_key_dict + await named_credential_wif_fields(credential_name, prisma_client), user_api_key_dict ) - _reject_config_defined_name(credential.credential_name) + _reject_config_defined_name(credential_name) processed_credential: Final = CredentialItem( - credential_name=credential.credential_name, + credential_name=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, @@ -345,7 +358,7 @@ async def create_credential( except Exception as e: if not is_unique_violation(e): raise - raise HTTPException(status_code=409, detail=_credential_exists_detail(credential.credential_name)) + raise HTTPException(status_code=409, detail=_credential_exists_detail(credential_name)) ## ADD TO LITELLM ## CredentialAccessor.upsert_credentials([processed_credential]) @@ -395,15 +408,15 @@ async def get_credential_by_name( [BETA] endpoint. This might change unexpectedly. """ try: - for credential in litellm.credential_list: - if credential.credential_name == credential_name: - return _credential_view( - credential, - get_masked_values(credential.credential_values, unmasked_length=4, number_of_asterisks=4), - ) - raise HTTPException( - status_code=404, - detail="Credential not found. Got credential name: " + credential_name, + credential: Final = CredentialAccessor.find_credential(credential_name) + if credential is None: + raise HTTPException( + status_code=404, + detail="Credential not found. Got credential name: " + credential_name, + ) + return _credential_view( + credential, + get_masked_values(credential.credential_values, unmasked_length=4, number_of_asterisks=4), ) except Exception as e: verbose_proxy_logger.exception(e) @@ -822,15 +835,19 @@ async def delete_credential( _reject_non_admin_wif_fields( await named_credential_wif_fields(credential_name, prisma_client), user_api_key_dict ) - deleted: Final = await CredentialsRepository(prisma_client).delete_by_name(credential_name) - if deleted is None: + credentials_repository: Final = CredentialsRepository(prisma_client) + stored: Final = await credentials_repository.find_by_name(credential_name) + if stored is None: raise _not_found_unless_config_defined( credential_name, "Credential not found. Got credential name: " + credential_name ) + await credentials_repository.delete_by_name(stored.credential_name) ## DELETE FROM LITELLM ## - litellm.credential_list = [cred for cred in litellm.credential_list if cred.credential_name != credential_name] - await _purge_user_connections_for_credential(credential_name) + litellm.credential_list = [ + cred for cred in litellm.credential_list if cred.credential_name != stored.credential_name + ] + await _purge_user_connections_for_credential(stored.credential_name) return {"success": True, "message": "Credential deleted successfully"} except Exception as e: raise handle_exception_on_proxy(e) @@ -892,7 +909,7 @@ async def update_credential( from litellm.proxy.proxy_server import prisma_client try: - if credential.credential_name and credential.credential_name != credential_name: + if credential.credential_name and credential.credential_name.strip() != credential_name.strip(): raise ProxyException( message="credential_name is immutable. Set display_name to change how the credential is labeled.", type=ProxyErrorTypes.validation_error.value, @@ -917,8 +934,9 @@ async def update_credential( if db_credential is None: 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) + stored_name: Final = db_credential.credential_name patch: Final = CredentialItem( - credential_name=credential_name, + credential_name=stored_name, display_name=( requested_display_name if "display_name" in credential.model_fields_set else db_credential.display_name ), @@ -931,7 +949,7 @@ async def update_credential( "dict[str, object]", jsonify_object(merged_credential.model_dump(exclude_none=True)) ) await credentials_repository.update_by_name( - credential_name, + stored_name, data={ **credential_object_jsonified, "display_name": merged_credential.display_name, @@ -940,14 +958,14 @@ async def update_credential( ) # Sync in-memory credential_list (skip if not in memory - e.g., proxy restarted) - _sync_in_memory_credential(patch, credential_name) + _sync_in_memory_credential(patch, stored_name) merged_values: Final = cast( # cast-ok: credential_values is a plain dict at runtime Mapping[object, object], merged_credential.credential_values ) still_per_user: Final = merged_values.get(GITHUB_COPILOT_AUTH_TYPE_KEY) == GITHUB_COPILOT_PER_USER_AUTH_TYPE if not still_per_user: - await _purge_user_connections_for_credential(credential_name) + await _purge_user_connections_for_credential(stored_name) return {"success": True, "message": "Credential updated successfully"} except Exception as e: diff --git a/litellm/repositories/credentials_repository.py b/litellm/repositories/credentials_repository.py index cb1bc1d101b..ff0b47d9316 100644 --- a/litellm/repositories/credentials_repository.py +++ b/litellm/repositories/credentials_repository.py @@ -72,8 +72,15 @@ class CredentialsRepository: return await self.table.create(data=data) async def find_by_name(self, credential_name: str) -> CredentialItem | None: + """The row named ``credential_name``, else the single row whose name only differs by + surrounding whitespace, which older versions stored verbatim from the UI.""" record: Final = await self.table.find_unique(where={"credential_name": credential_name}) - return self._to_model(record) + if record is not None: + return self._to_model(record) + stripped: Final = credential_name.strip() + candidates: Final = await self.table.find_many(where={"credential_name": {"contains": stripped}}) + padded: Final = tuple(candidate for candidate in candidates if candidate.credential_name.strip() == stripped) + return self._to_model(padded[0]) if len(padded) == 1 else None async def update_by_name( self, credential_name: str, data: Mapping[str, object] diff --git a/tests/unit/proxy/common_utils/test_credential_hydration.py b/tests/unit/proxy/common_utils/test_credential_hydration.py index ad75b5b8760..91250ddb0b6 100644 --- a/tests/unit/proxy/common_utils/test_credential_hydration.py +++ b/tests/unit/proxy/common_utils/test_credential_hydration.py @@ -4,6 +4,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest import litellm +from litellm.litellm_core_utils.credential_accessor import CredentialAccessor from litellm.proxy.common_utils.credential_hydration import ( effective_server_owned_wif_fields, hydrate_named_credential_authoritative, @@ -34,6 +35,26 @@ async def test_authoritative_hydrate_returns_an_encrypted_empty_value_as_empty(m assert resolved.credential_values == {"api_base": "", "openai_service_account_id": "user-1"} +@pytest.mark.asyncio +async def test_wif_gate_sees_the_padded_credential_a_trimmed_name_resolves_to(): + """Resolution maps ``corp-wif`` onto a credential stored as ``corp-wif ``, so the federation + gate must see that credential's server-owned fields too, or a team admin could swap them out.""" + padded = CredentialItem( + credential_name="corp-wif ", + credential_values={"anthropic_keycloak_token_url": "https://keycloak.internal/token"}, + credential_info={}, + source="config", + ) + with patch.object( + litellm, "credential_list", [padded] + ): # test-quality-ok: the in-memory config credential under test + resolved = CredentialAccessor.find_credential("corp-wif") + gated = await named_credential_wif_fields("corp-wif", None) + + assert resolved is padded + assert "anthropic_keycloak_token_url" in gated + + @pytest.mark.asyncio @pytest.mark.parametrize( "credential_api_key, expected", diff --git a/tests/unit/proxy/credential_endpoints/test_endpoints.py b/tests/unit/proxy/credential_endpoints/test_endpoints.py index 6aeceaf4356..ef0af6f4d61 100644 --- a/tests/unit/proxy/credential_endpoints/test_endpoints.py +++ b/tests/unit/proxy/credential_endpoints/test_endpoints.py @@ -12,6 +12,7 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ec from fastapi import HTTPException, Request from fastapi.testclient import TestClient +from pydantic import BaseModel, ConfigDict, field_validator import litellm from litellm.models.credentials import CredentialSource @@ -65,6 +66,7 @@ def _list_credentials(): def _prisma_without_credential_rows() -> MagicMock: prisma_client = MagicMock() prisma_client.db.litellm_credentialstable.find_unique = AsyncMock(return_value=None) + prisma_client.db.litellm_credentialstable.find_many = AsyncMock(return_value=[]) return prisma_client @@ -1128,7 +1130,7 @@ 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 400 that follows is the rule for + """The gate lets the admin through to the row lookup. The 405 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", source="config") @@ -1139,7 +1141,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: assert response.status_code == 405, response.text assert response.headers["allow"] == "GET" assert "defined in config" in response.json()["error"]["message"] - repository.delete_by_name.assert_awaited_once_with("config-wif") + repository.delete_by_name.assert_not_awaited() assert litellm.credential_list == [config_credential] def test_non_admin_cannot_shadow_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): @@ -1280,6 +1282,7 @@ class TestManagementReadsTheStoredCredential: ) prisma = MagicMock() prisma.db.litellm_credentialstable.find_unique = AsyncMock(return_value=None) + prisma.db.litellm_credentialstable.find_many = AsyncMock(return_value=[]) with patch.object( litellm, "credential_list", [only_in_memory] @@ -1318,7 +1321,11 @@ def test_delete_credential_still_answers_200_and_drops_the_credential_from_memor credential_values={"api_key": "sk-keep"}, credential_info={}, ) - credential_store(in_memory=(stored, survivor), delete_by_name=AsyncMock(return_value=MagicMock())) + credential_store( + in_memory=(stored, survivor), + find_by_name=AsyncMock(return_value=stored), + delete_by_name=AsyncMock(return_value=MagicMock()), + ) response = _delete_credential("doomed") @@ -1716,6 +1723,7 @@ def test_deleting_the_credential_purges_user_connections(monkeypatch, credential rows = [_connection_row(user_id="u1"), _connection_row(user_id="u2")] credential_store( in_memory=[_per_user_credential()], + find_by_name=AsyncMock(return_value=_per_user_credential()), delete_by_name=AsyncMock(return_value=_per_user_credential()), ) prisma_client = MagicMock() @@ -1725,6 +1733,7 @@ def test_deleting_the_credential_purges_user_connections(monkeypatch, credential prisma_client.db.litellm_userprovidercredentials = table prisma_client.writer_db = prisma_client.db prisma_client.db.litellm_credentialstable.find_unique = AsyncMock(return_value=None) + prisma_client.db.litellm_credentialstable.find_many = AsyncMock(return_value=[]) monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma_client) monkeypatch.setattr("litellm.proxy.proxy_server.user_api_key_cache", DualCache()) @@ -2567,3 +2576,155 @@ class TestCredentialDisplayName: 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" + + +class _CredentialRow(BaseModel): + model_config = ConfigDict(frozen=True) + + credential_name: str + display_name: str | None = None + credential_values: dict[str, object] + credential_info: dict[str, object] + + @field_validator("credential_values", "credential_info", mode="before") + @classmethod + def _decode_json_column(cls, value: object) -> object: + return json.loads(value) if isinstance(value, str) else value + + +class _CredentialsTable: + """In-memory litellm_credentialstable keyed by the exact stored name, as the unique index is.""" + + def __init__(self, *names: str) -> None: + self.rows = { + name: _CredentialRow(credential_name=name, credential_values={}, credential_info={}) for name in names + } + + async def find_unique(self, where): + return self.rows.get(where["credential_name"]) + + async def find_many(self, where): + needle = where["credential_name"]["contains"] + return [row for name, row in self.rows.items() if needle in name] + + async def create(self, data): + row = _CredentialRow.model_validate(data) + self.rows[row.credential_name] = row + return row + + async def update(self, where, data): + name = where["credential_name"] + self.rows[name] = _CredentialRow.model_validate({**self.rows[name].model_dump(), **data}) + return self.rows[name] + + async def delete(self, where): + return self.rows.pop(where["credential_name"], None) + + +@pytest.fixture +def credentials_table(monkeypatch, restore_credential_list): + """A database holding the given credential names, served through the real repository.""" + + def install(*names: str) -> _CredentialsTable: + table = _CredentialsTable(*names) + prisma_client = MagicMock() + prisma_client.db.litellm_credentialstable = table + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma_client) + monkeypatch.setattr("litellm.proxy.proxy_server.master_key", "sk-test-master") + monkeypatch.setattr( + litellm, + "credential_list", + [ + CredentialItem(credential_name=name, credential_values={"api_key": "sk-old"}, credential_info={}) + for name in names + ], + ) + return table + + return install + + +class TestCredentialNameWhitespace: + def test_create_stores_the_name_without_surrounding_whitespace(self, credentials_table): + table = credentials_table() + + response = _post_credential( + {"credential_name": " test-cred ", "credential_values": {"api_key": "sk-new"}, "credential_info": {}} + ) + + assert response.status_code == 200, response.text + assert list(table.rows) == ["test-cred"] + assert [entry["credential_name"] for entry in _list_credentials().json()["credentials"]] == ["test-cred"] + + @pytest.mark.parametrize("credential_name", ["", " "]) + def test_create_rejects_a_blank_name(self, credentials_table, credential_name): + table = credentials_table() + + response = _post_credential( + {"credential_name": credential_name, "credential_values": {"api_key": "sk-new"}, "credential_info": {}} + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "credential_name" + assert table.rows == {} + assert litellm.credential_list == [] + + def test_get_by_trimmed_name_finds_a_legacy_padded_credential(self, credentials_table): + credentials_table("Openrouter 11/08 ") + + response = _call_as("GET", "/credentials/by_name/Openrouter%2011/08") + + assert response.status_code == 200, response.text + assert response.json()["credential_name"] == "Openrouter 11/08 " + + def test_delete_by_trimmed_name_removes_a_legacy_padded_credential(self, credentials_table): + table = credentials_table("Openrouter 11/08 ", "keeper") + + response = _delete_credential("Openrouter%2011/08") + + assert response.status_code == 200, response.text + assert list(table.rows) == ["keeper"] + assert [credential.credential_name for credential in litellm.credential_list] == ["keeper"] + + def test_patch_by_trimmed_name_updates_the_legacy_padded_row_in_place(self, credentials_table): + table = credentials_table("Openrouter 11/08 ") + + response = _patch_credential( + "Openrouter%2011/08", + { + "credential_name": "Openrouter 11/08 ", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + }, + ) + + assert response.status_code == 200, response.text + assert list(table.rows) == ["Openrouter 11/08 "] + assert "api_key" in table.rows["Openrouter 11/08 "].credential_values + assert [c.credential_values["api_key"] for c in litellm.credential_list] == ["sk-new"] + + def test_an_exact_match_wins_over_a_padded_one(self, credentials_table): + table = credentials_table("foo", "foo ") + + response = _delete_credential("foo") + + assert response.status_code == 200, response.text + assert list(table.rows) == ["foo "] + + def test_a_trimmed_name_matching_several_padded_rows_deletes_nothing(self, credentials_table): + table = credentials_table("foo ", " foo") + + response = _delete_credential("foo") + + assert response.status_code == 404, response.text + assert sorted(table.rows) == [" foo", "foo "] + + @pytest.mark.parametrize("method", ["GET", "DELETE"]) + def test_a_trimmed_name_does_not_match_a_longer_padded_name(self, credentials_table, method): + table = credentials_table("foobar ") + path = "/credentials/by_name/foo" if method == "GET" else "/credentials/foo" + + response = _call_as(method, path) + + assert response.status_code == 404, response.text + assert list(table.rows) == ["foobar "] diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx index d0e28a0e273..878891127d9 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx @@ -131,7 +131,7 @@ export default function CredentialModal({ } const values = projectMountedValues(registry, form.getValues); const meta = { - credential_name: String(values.credential_name), + credential_name: String(values.credential_name).trim(), custom_llm_provider: String(values.custom_llm_provider), ...displayNameChange(values.display_name, existingCredential), }; diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index e7a55870345..c8977e7363b 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -544,6 +544,29 @@ describe("teamInfoCall", () => { }); }); +describe("credential calls", () => { + const credentialName = "a b/c "; + + const requestedPath = (mockFetch: ReturnType): string => { + const [url] = mockFetch.mock.calls[0]; + return url instanceof Request ? new URL(url.url).pathname : new URL(url.toString(), "http://example.com").pathname; + }; + + it.each([ + ["delete", () => Networking.credentialDeleteCall("token", credentialName), "/credentials/a%20b%2Fc%20"], + ["update", () => Networking.credentialUpdateCall("token", credentialName, {}), "/credentials/a%20b%2Fc%20"], + ["get", () => Networking.credentialGetCall("token", credentialName, null), "/credentials/by_name/a%20b%2Fc%20"], + ])("%s keeps the trailing space and slash of the credential name in the path", async (_, call, expectedPath) => { + const mockFetch = vi.spyOn(global, "fetch").mockResolvedValue(new Response("{}", { status: 200 })); + try { + await call(); + expect(requestedPath(mockFetch).endsWith(expectedPath)).toBe(true); + } finally { + mockFetch.mockRestore(); + } + }); +}); + describe("uiSpendLogsCall exclude_internal_health_checks serialization", () => { const originalFetch = global.fetch; diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 4df34275923..535e7c557b8 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -2648,7 +2648,7 @@ export const credentialGetCall = async (accessToken: string, credentialName: str let path = `/credentials`; if (credentialName) { - path += `/by_name/${credentialName}`; + path += `/by_name/${encodeURIComponent(credentialName)}`; } else if (modelId) { path += `/by_model/${modelId}`; } @@ -2664,7 +2664,7 @@ export const credentialGetCall = async (accessToken: string, credentialName: str export const credentialDeleteCall = async (accessToken: string, credentialName: string) => { try { - const data = await apiClient.delete(`/credentials/${credentialName}`, { accessToken }); + const data = await apiClient.delete(`/credentials/${encodeURIComponent(credentialName)}`, { accessToken }); return data; // Handle success - you might want to update some state or UI based on the created key } catch (error) { @@ -2741,7 +2741,7 @@ export const credentialUpdateCall = async ( } } - const data = await apiClient.patch(`/credentials/${credentialName}`, { + const data = await apiClient.patch(`/credentials/${encodeURIComponent(credentialName)}`, { accessToken, body: { ...formValues, // Include formValues in the request body