From 6867aecb73f462accf19c2e5f92cbcf80d61a684 Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Fri, 7 Aug 2026 00:29:09 -0700 Subject: [PATCH] fix(proxy): keep the in-memory credential in step with the row it was written from A partial credential_info patch replaces the stored row wholesale but merged the in-memory copy, so litellm.credential_list kept advertising fields the row no longer had. The dashboard refetches from that list after an edit, so it displayed values that were already gone; the next config reload (30s) then dropped them with nothing to point at. The DB write has always replaced: its guard asked whether the stored info contained a key literally named credential_info, which nothing stores, so it emptied the dict and repopulated it from the patch every time. That dead guard is removed, which is behaviour-preserving, and the in-memory sync now replaces to match. --- .../proxy/credential_endpoints/endpoints.py | 13 +- .../credential_endpoints/test_endpoints.py | 121 ++++++++++++++++++ 2 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 tests/test_litellm/proxy/credential_endpoints/test_endpoints.py diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index a6141ac9217..4775ec2ad2a 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -274,10 +274,7 @@ def update_db_credential( # update model info if 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) + merged_credential.credential_info = encrypted_credential.credential_info return merged_credential @@ -331,9 +328,11 @@ async def update_credential( in_memory_values: Final = dict(existing_in_memory.credential_values or {}) if credential.credential_values: in_memory_values.update(credential.credential_values) - in_memory_info: Final = dict(existing_in_memory.credential_info or {}) - if credential.credential_info: - in_memory_info.update(credential.credential_info) + in_memory_info: Final = ( + dict(credential.credential_info) + if credential.credential_info + else dict(existing_in_memory.credential_info or {}) + ) updated_in_memory: Final = CredentialItem( credential_name=new_name, credential_values=in_memory_values, diff --git a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py new file mode 100644 index 00000000000..d0cafb507ec --- /dev/null +++ b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py @@ -0,0 +1,121 @@ +"""Tests for the credential management endpoints.""" + +import os +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from fastapi.testclient import TestClient + +sys.path.insert(0, os.path.abspath("../../../..")) + +import litellm +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 update_db_credential +from litellm.proxy.proxy_server import app +from litellm.types.utils import CredentialItem + +client = TestClient(app) + + +def _as_admin(): + return UserAPIKeyAuth(api_key="test-key", user_role="proxy_admin") + + +def test_update_db_credential_replaces_credential_info_wholesale(): + """The stored row has always been replaced, not merged: the old guard asked whether + the stored info contained a key literally named ``credential_info``, which nothing + stores, so it emptied the dict and repopulated it from the patch every time. This + pins that behaviour so the simplification cannot drift into a real merge.""" + stored = CredentialItem( + credential_name="c1", + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai", "description": "keep me"}, + ) + patch_item = CredentialItem( + credential_name="c1", + credential_values={}, + credential_info={"description": "patched"}, + ) + + merged = update_db_credential(stored, patch_item) + + assert merged.credential_info == {"description": "patched"} + assert "custom_llm_provider" not in merged.credential_info + + +def test_update_db_credential_keeps_credential_info_when_the_patch_carries_none(): + """An empty ``credential_info`` on the patch must leave the stored one alone; the + master-key rotation path relies on the untouched branch.""" + stored = CredentialItem( + credential_name="c1", + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai"}, + ) + patch_item = CredentialItem(credential_name="c1", credential_values={"api_key": "sk-new"}, credential_info={}) + + with patch("litellm.proxy.proxy_server.master_key", "sk-test-master"): + merged = update_db_credential(stored, patch_item) + + assert merged.credential_info == {"custom_llm_provider": "openai"} + + +def test_update_db_credential_is_unchanged_for_a_full_patch(): + """Master-key rotation passes the whole decrypted row as the patch + (``key_management_endpoints.py`` rotate loop), so replace and merge agree there.""" + info = {"custom_llm_provider": "openai", "description": "prod"} + stored = CredentialItem(credential_name="c1", credential_values={"api_key": "sk-old"}, credential_info=dict(info)) + full_patch = CredentialItem( + credential_name="c1", credential_values={"api_key": "sk-old"}, credential_info=dict(info) + ) + + with patch("litellm.proxy.proxy_server.master_key", "sk-test-master"): + merged = update_db_credential(stored, full_patch) + + assert merged.credential_info == info + + +def test_partial_patch_leaves_no_stale_fields_in_the_in_memory_credential(): + """Regression: the in-memory copy merged ``credential_info`` while the DB row replaced + it, so after a partial patch ``litellm.credential_list`` still advertised fields the + row no longer had. The dashboard refetches from that list, so it showed values that + were already gone, until the next config reload (30s) silently dropped them.""" + stored = CredentialItem( + credential_name="c1", + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai", "description": "keep me"}, + ) + original_list = litellm.credential_list + litellm.credential_list = [ + CredentialItem( + credential_name="c1", + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai", "description": "keep me"}, + ) + ] + app.dependency_overrides[user_api_key_auth] = _as_admin + try: + with patch("litellm.proxy.proxy_server.prisma_client", MagicMock()), patch( + "litellm.proxy.proxy_server.master_key", "sk-test-master" + ), patch("litellm.proxy.credential_endpoints.endpoints.CredentialsRepository") as repository: + repository.return_value.find_by_name = AsyncMock(return_value=stored) + repository.return_value.update_by_name = AsyncMock(return_value=None) + + response = client.patch( + "/credentials/c1", + json={ + "credential_name": "c1", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {"description": "patched"}, + }, + headers={"Authorization": "Bearer test-key"}, + ) + + assert response.status_code == 200, response.text + in_memory = next(c for c in litellm.credential_list if c.credential_name == "c1") + assert in_memory.credential_info == {"description": "patched"} + assert "custom_llm_provider" not in in_memory.credential_info + finally: + app.dependency_overrides.clear() + litellm.credential_list = original_list