scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-05 08:07:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 137

Merge pull request #36260 from eeshsaxena/fix-credential-endpoints-raise-not-return

Browse source 
fix(proxy): 404 a credential delete that matched nothing, and raise instead of return
This commit is contained in:
Mateo Wang 2026-09-03 06:17:56 -07:00 • committed by GitHub
commit 11a02b9581
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 148 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
litellm/proxy/credential_endpoints/endpoints.py
View file

@ -135,7 +135,7 @@ async def get_credentials(
]
return {"success": True, "credentials": masked_credentials}
except Exception as e:
return handle_exception_on_proxy(e)
raise handle_exception_on_proxy(e)
@router.get(
@ -239,13 +239,18 @@ async def delete_credential(
status_code=500,
detail={"error": CommonProxyErrors.db_not_connected_error.value},
)
await CredentialsRepository(prisma_client).delete_by_name(credential_name)
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,
)
## DELETE FROM LITELLM ##
litellm.credential_list = [cred for cred in litellm.credential_list if cred.credential_name != credential_name]
return {"success": True, "message": "Credential deleted successfully"}
except Exception as e:
return handle_exception_on_proxy(e)
raise handle_exception_on_proxy(e)
def update_db_credential(

171
tests/test_litellm/proxy/credential_endpoints/test_endpoints.py
View file

@ -6,6 +6,7 @@ import pytest
from fastapi.testclient import TestClient
import litellm
from litellm.proxy._types import UserAPIKeyAuth
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.proxy_server import app
@ -18,16 +19,12 @@ def _as_admin():
return UserAPIKeyAuth(api_key="test-key", user_role="proxy_admin")
def _patch_credential(name: str, body: dict):
def _call_as_admin(method: str, path: str, json_body: dict | None = None):
missing = object()
previous_override = app.dependency_overrides.get(user_api_key_auth, missing)
app.dependency_overrides[user_api_key_auth] = _as_admin
try:
return client.patch(
f"/credentials/{name}",
json=body,
headers={"Authorization": "Bearer test-key"},
)
return client.request(method, path, json=json_body, headers={"Authorization": "Bearer test-key"})
finally:
if previous_override is missing:
app.dependency_overrides.pop(user_api_key_auth, None)
@ -35,37 +32,69 @@ def _patch_credential(name: str, body: dict):
app.dependency_overrides[user_api_key_auth] = previous_override
def test_update_credential_answers_404_when_the_credential_does_not_exist():
def _patch_credential(name: str, body: dict):
return _call_as_admin("PATCH", f"/credentials/{name}", body)
def _delete_credential(name: str):
return _call_as_admin("DELETE", f"/credentials/{name}")
def _list_credentials():
return _call_as_admin("GET", "/credentials")
@pytest.fixture
def credential_store():
"""Stands the credential store up for one test: whether the database is reachable, what
the proxy is already serving from memory, and what each repository call hands back."""
def install(
*,
connected: bool = True,
in_memory: tuple[object, ...] = (),
**repository_calls: AsyncMock,
) -> None:
patch("litellm.proxy.proxy_server.prisma_client", MagicMock() if connected else None).start()
patch("litellm.proxy.proxy_server.master_key", "sk-test-master").start()
patch.object(litellm, "credential_list", list(in_memory)).start()
repository = patch("litellm.proxy.credential_endpoints.endpoints.CredentialsRepository").start()
for call_name, result in repository_calls.items():
setattr(repository.return_value, call_name, result)
yield install
patch.stopall()
def test_update_credential_answers_404_when_the_credential_does_not_exist(credential_store):
"""Regression: the handler used to ``return handle_exception_on_proxy(e)``, which makes
the exception the response body and lets FastAPI answer 200, so a write the handler
rejected read as a success to every caller that checks the status. The dashboard's API
client branches on the status, so it reported a failed edit as applied."""
with patch("litellm.proxy.proxy_server.prisma_client", MagicMock()), patch(
"litellm.proxy.credential_endpoints.endpoints.CredentialsRepository"
) as repository:
repository.return_value.find_by_name = AsyncMock(return_value=None)
credential_store(find_by_name=AsyncMock(return_value=None))
response = _patch_credential(
"definitely-not-there",
{"credential_name": "definitely-not-there", "credential_values": {"api_key": "sk-x"}, "credential_info": {}},
)
response = _patch_credential(
"definitely-not-there",
{"credential_name": "definitely-not-there", "credential_values": {"api_key": "sk-x"}, "credential_info": {}},
)
assert response.status_code == 404, f"rejected write answered {response.status_code}: {response.text}"
assert "error" in response.json()
def test_update_credential_answers_500_when_the_database_is_not_connected():
def test_update_credential_answers_500_when_the_database_is_not_connected(credential_store):
"""The other rejection this handler raises must carry its own status too."""
with patch("litellm.proxy.proxy_server.prisma_client", None):
response = _patch_credential(
"any-name",
{"credential_name": "any-name", "credential_values": {"api_key": "sk-x"}, "credential_info": {}},
)
credential_store(connected=False)
response = _patch_credential(
"any-name",
{"credential_name": "any-name", "credential_values": {"api_key": "sk-x"}, "credential_info": {}},
)
assert response.status_code == 500, f"rejected write answered {response.status_code}: {response.text}"
def test_update_credential_still_answers_200_on_a_successful_write():
def test_update_credential_still_answers_200_on_a_successful_write(credential_store):
"""The fix must not turn a legitimate update into an error; the dashboard and the
Playwright credentials spec both assert the success path."""
stored = CredentialItem(
@ -73,16 +102,96 @@ def test_update_credential_still_answers_200_on_a_successful_write():
credential_values={"api_key": "sk-old"},
credential_info={"custom_llm_provider": "openai"},
)
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)
credential_store(find_by_name=AsyncMock(return_value=stored), update_by_name=AsyncMock(return_value=None))
response = _patch_credential(
"existing",
{"credential_name": "existing", "credential_values": {"api_key": "sk-new"}, "credential_info": {}},
)
response = _patch_credential(
"existing",
{"credential_name": "existing", "credential_values": {"api_key": "sk-new"}, "credential_info": {}},
)
assert response.status_code == 200, response.text
assert response.json()["success"] is True
def test_delete_credential_answers_404_when_the_credential_does_not_exist(credential_store):
"""Regression: prisma's ``delete`` hands back None when the ``where`` clause matched no row
instead of raising, and the handler never looked. Deleting a name that was never stored
answered 200 "Credential deleted successfully", so an operator scripting cleanup could not
tell a real deletion from a typo."""
credential_store(delete_by_name=AsyncMock(return_value=None))
response = _delete_credential("definitely-not-there")
assert response.status_code == 404, f"delete of a missing credential answered {response.status_code}: {response.text}"
assert "definitely-not-there" in response.text
def test_delete_credential_still_answers_200_and_drops_the_credential_from_memory(credential_store):
"""The fix must not turn a real deletion into an error, and the deleted credential must
stop being served from the in-memory list the proxy routes on."""
stored = CredentialItem(
credential_name="doomed",
credential_values={"api_key": "sk-old"},
credential_info={"custom_llm_provider": "openai"},
)
survivor = CredentialItem(
credential_name="keeper",
credential_values={"api_key": "sk-keep"},
credential_info={},
)
credential_store(in_memory=(stored, survivor), delete_by_name=AsyncMock(return_value=MagicMock()))
response = _delete_credential("doomed")
assert response.status_code == 200, response.text
assert response.json()["success"] is True
assert [credential.credential_name for credential in litellm.credential_list] == ["keeper"]
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."""
config_only = CredentialItem(
credential_name="from-config-yaml",
credential_values={"api_key": "sk-config"},
credential_info={},
)
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 [credential.credential_name for credential in litellm.credential_list] == ["from-config-yaml"]
def test_delete_credential_answers_500_when_the_database_is_not_connected(credential_store):
"""The handler used to ``return handle_exception_on_proxy(e)``, which makes the exception the
response body and lets FastAPI answer 200. A DB-less proxy answered its own 500 as a success."""
credential_store(connected=False)
response = _delete_credential("any-name")
assert response.status_code == 500, f"rejected delete answered {response.status_code}: {response.text}"
class _CredentialThatCannotBeMasked:
"""Stands in for anything that fails while ``GET /credentials`` builds its response."""
credential_name = "unreadable"
credential_info: dict = {}
@property
def credential_values(self):
raise RuntimeError("credential store unreadable")
def test_get_credentials_answers_an_error_status_when_the_listing_fails(credential_store):
"""Same ``return`` instead of ``raise`` on the list route: a failed listing was serialized as
a 200 whose body happened to be an error, so a caller reading the status saw an empty success."""
credential_store(in_memory=(_CredentialThatCannotBeMasked(),))
response = _list_credentials()
assert response.status_code == 500, f"failed listing answered {response.status_code}: {response.text}"
assert response.json().get("success") is not True