mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
refactor(proxy): move the credential PATCH status and in-memory sync fixes out of this stack
Both changes are independent of admin-owned trace destinations and now ship as their own PRs, so this stack carries only the feature: the access-shape validation, the resolves_to_destination disclosure, and their tests. Merge order: the status-code PR must land first. Without it this stack's access validation is rejected by the handler and then returned as the response body, so FastAPI answers 200 and the dashboard reports a rejected access shape as saved.
This commit is contained in:
Yucheng Zhu
parent
6a12b77ac8
commit
616a700143
2 changed files with 32 additions and 187 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue