diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index d39437a0e2a..eec452a82fa 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -285,8 +285,12 @@ def update_db_credential( merged_credential.credential_values.update(encrypted_params) + # update model info if encrypted_credential.credential_info: - merged_credential.credential_info = encrypted_credential.credential_info + """Update credential info""" + if "credential_info" not in merged_credential.credential_info: + merged_credential.credential_info = {} + merged_credential.credential_info.update(encrypted_credential.credential_info) return merged_credential @@ -329,53 +333,32 @@ async def update_credential( "updated_by": user_api_key_dict.user_id, }, ) - _sync_in_memory_credential( - old_name=credential_name, - merged=merged_credential, - patch=credential, - ) + + # Sync in-memory credential_list (skip if not in memory - e.g., proxy restarted) + new_name = merged_credential.credential_name + existing_in_memory: CredentialItem | None = None + for cred in litellm.credential_list: + if cred.credential_name == credential_name: + existing_in_memory = cred + break + + if existing_in_memory is not None: + in_memory_values = dict(existing_in_memory.credential_values or {}) + if credential.credential_values: + in_memory_values.update(credential.credential_values) + in_memory_info = dict(existing_in_memory.credential_info or {}) + if credential.credential_info: + in_memory_info.update(credential.credential_info) + updated_in_memory = CredentialItem( + credential_name=new_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]) + return {"success": True, "message": "Credential updated successfully"} except Exception as e: - # Raised, not returned: returning the exception makes it the response body and - # FastAPI answers 200, so a rejected access shape reads as a successful write to - # any client checking the status (the destination edit modal does). - raise handle_exception_on_proxy(e) - - -def _sync_in_memory_credential( - *, - old_name: str, - merged: CredentialItem, - patch: CredentialItem, -) -> None: - """Mirror the DB write into ``litellm.credential_list``. - - Skips when the credential isn't resident in memory (e.g. created on - another scaled instance, restored from DB on the next reload). - ``credential_info`` is replaced exactly as the DB write replaces it, so the - routing-live copy and the stored row can't disagree; values merge, matching - the DB's ``update``. Diverging here would hide a lost field until the next - reload swapped the in-memory copy for the row that never had it. - """ - existing_in_memory: CredentialItem | None = None - for cred in litellm.credential_list: - if cred.credential_name == old_name: - existing_in_memory = cred - break - if existing_in_memory is None: - return - - in_memory_values = dict(existing_in_memory.credential_values or {}) - if patch.credential_values: - in_memory_values.update(patch.credential_values) - in_memory_info = ( - dict(patch.credential_info) if patch.credential_info else dict(existing_in_memory.credential_info or {}) - ) - updated_in_memory = CredentialItem( - credential_name=merged.credential_name, - credential_values=in_memory_values, - credential_info=in_memory_info, - ) - if merged.credential_name != old_name: - litellm.credential_list = [c for c in litellm.credential_list if c.credential_name != old_name] - CredentialAccessor.upsert_credentials([updated_in_memory]) + return handle_exception_on_proxy(e) diff --git a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py index 3d5c9487ef3..a5fcc3cd071 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py @@ -29,95 +29,6 @@ def _admin(): # --- credential_info replace semantics --- -def test_update_db_credential_replaces_info_for_a_logging_destination(): - """A destination's ``credential_info`` is replaced so an access edit actually applies. - - Narrowing a destination's scope has to be able to shrink ``access``; a subfield merge - would leave the widest previously-granted shape in place, so an admin re-scoping a - destination from global to one team would not revoke anyone. - """ - from litellm.proxy.credential_endpoints.endpoints import update_db_credential - - merged = update_db_credential( - CredentialItem( - credential_name="c", - credential_values={}, - credential_info={"credential_type": "logging", "description": "arize", "access": {"global": True}}, - ), - CredentialItem( - credential_name="c", - credential_values={}, - credential_info={"credential_type": "logging", "description": "arize", "access": {"teams": ["t1"]}}, - ), - ) - - assert merged.credential_info["access"] == {"teams": ["t1"]} - - -def test_partial_patch_of_a_provider_credential_drops_sibling_keys(): - """Documents a PRE-EXISTING data loss; this is NOT the intended contract. - - A PATCH carrying a fragment of ``credential_info`` drops the keys it omits, so a - one-key patch destroys siblings such as ``custom_llm_provider``. The merge-base has the - same loss at the DB layer: its merge branch guards on ``"credential_info" not in - merged_credential.credential_info``, testing for a key nested inside itself, which is - never true, so it wipes the dict and updates onto empty. Base only appeared to preserve - the key because its in-memory mirror merged and hid the row until the next reload. - - Asserted so the behaviour is visible rather than silently relied upon. Restoring a real - merge for non-logging credentials is tracked separately; when that lands, this test - should flip. - """ - from litellm.proxy.credential_endpoints.endpoints import update_db_credential - - merged = update_db_credential( - CredentialItem( - credential_name="c", - credential_values={}, - credential_info={"custom_llm_provider": "openai", "description": "prod"}, - ), - CredentialItem(credential_name="c", credential_values={}, credential_info={"description": "patched"}), - ) - - assert merged.credential_info == {"description": "patched"} - assert "custom_llm_provider" not in merged.credential_info - - -def test_sync_in_memory_credential_mirrors_the_db_row(monkeypatch): - """Regression: the routing-live copy must equal the row that was written. - - The in-memory mirror used to merge ``credential_info`` while the DB write replaced - it, so a field dropped from the row stayed visible in ``litellm.credential_list`` - and on ``GET /credentials`` -- the loss only surfaced on the next reload, long after - the request that caused it. - """ - from litellm.proxy.credential_endpoints.endpoints import _sync_in_memory_credential - - existing = CredentialItem( - credential_name="openai-prod", - credential_values={"api_key": "enc"}, - credential_info={"custom_llm_provider": "openai", "keepme": "important"}, - ) - monkeypatch.setattr(litellm, "credential_list", [existing]) - patch = CredentialItem( - credential_name="openai-prod", - credential_values={}, - credential_info={"description": "just a label"}, - ) - merged = CredentialItem( - credential_name="openai-prod", - credential_values={"api_key": "enc"}, - credential_info={"description": "just a label"}, - ) - - _sync_in_memory_credential(old_name="openai-prod", merged=merged, patch=patch) - - in_memory = next(c for c in litellm.credential_list if c.credential_name == "openai-prod") - assert in_memory.credential_info == merged.credential_info - - -# --- access-shape validation is scoped to logging destinations --------------- - @pytest.mark.asyncio async def test_create_credential_validates_access_only_for_logging(monkeypatch): """validate_credential_access runs for a logging destination but never for a @@ -186,55 +97,6 @@ def test_patch_credentials_route_targets_update_credential(): assert patch_route.endpoint is endpoints.update_credential -@pytest.mark.asyncio -async def test_update_credential_raises_rejections_instead_of_answering_200(monkeypatch): - """Regression: the handler returned the ProxyException instead of raising it, so - FastAPI serialized it as the body of a 200. A rejected access shape then read as a - successful write to any client checking the status, including the destination edit - modal. The rejection has to reach the caller as a 4xx. - """ - from litellm.proxy._types import ProxyException - - class _Repo: - def __init__(self, _client): - pass - - async def find_by_name(self, name): - return CredentialItem( - credential_name=name, - credential_values={}, - credential_info={"credential_type": "logging", "description": "generic"}, - ) - - monkeypatch.setattr(endpoints, "CredentialsRepository", _Repo) - monkeypatch.setattr(endpoints, "validate_credential_access", _boom_400) - import litellm.proxy.proxy_server as proxy_server - - monkeypatch.setattr(proxy_server, "prisma_client", object(), raising=False) - - with pytest.raises(ProxyException) as excinfo: - await endpoints.update_credential( - request=MagicMock(), - fastapi_response=MagicMock(), - credential=CredentialItem( - credential_name="dest", - credential_values={}, - credential_info={"credential_type": "logging", "access": "everyone"}, - ), - credential_name="dest", - user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN), - ) - assert excinfo.value.code in ("400", 400) - - -def _boom_400(_info): - from fastapi import HTTPException - - raise HTTPException(status_code=400, detail={"error": "credential_info.access must be an object"}) - - -# --- secret masking on read -------------------------------------------------- - @pytest.mark.asyncio async def test_get_credentials_masks_secret_values(monkeypatch): """GET /credentials masks secret-bearing values; in particular a destination's