mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-14 14:26:43 +00:00
[Fix] preserve service_account_id in metadata on /key/update
/key/update and /user/update wholesale-replaced the metadata JSON column whenever a caller passed a `metadata` field, silently dropping service_account_id. The pre-call check in litellm_pre_call_utils.py then stopped treating the key as a service account and bypassed service_account_settings.enforced_params. Add LiteLLM_Reserved_Metadata_Fields and have prepare_metadata_fields preserve these keys from the existing row when the caller omits them. Reject attempts to change an already-set value (400) since rebinding service_account_id would break spend attribution.
This commit is contained in:
@@ -4047,6 +4047,11 @@ LiteLLM_ManagementEndpoint_MetadataFields_Premium = [
|
||||
"allowed_passthrough_routes",
|
||||
]
|
||||
|
||||
# Metadata keys preserved from existing rows when an update omits them.
|
||||
LiteLLM_Reserved_Metadata_Fields = [
|
||||
"service_account_id",
|
||||
]
|
||||
|
||||
|
||||
class ProviderBudgetResponseObject(LiteLLMPydanticObjectBase):
|
||||
"""
|
||||
|
||||
@@ -1517,6 +1517,20 @@ def prepare_metadata_fields(
|
||||
|
||||
casted_metadata = cast(dict, non_default_values["metadata"])
|
||||
|
||||
# Reserved metadata fields are immutable once set. Preserve the existing value
|
||||
# when omitted, reject attempts to change it.
|
||||
for reserved_field in LiteLLM_Reserved_Metadata_Fields:
|
||||
existing_value = existing_metadata.get(reserved_field)
|
||||
if existing_value is None:
|
||||
continue
|
||||
incoming_value = casted_metadata.get(reserved_field)
|
||||
if incoming_value is not None and incoming_value != existing_value:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"{reserved_field} is immutable once set and cannot be changed via update.",
|
||||
)
|
||||
casted_metadata[reserved_field] = existing_value
|
||||
|
||||
data_json = data.model_dump(exclude_unset=True, exclude_none=True)
|
||||
|
||||
try:
|
||||
|
||||
@@ -1206,6 +1206,95 @@ async def test_update_service_account_works_with_team_id():
|
||||
await prepare_key_update_data(data=data, existing_key_row=existing_key)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_preserves_service_account_id_when_metadata_replaced():
|
||||
"""
|
||||
Regression: /key/update wholesale-replaced metadata, silently dropping
|
||||
service_account_id. The pre-call check then treated the key as a regular
|
||||
key and bypassed service_account_settings.enforced_params.
|
||||
"""
|
||||
data = UpdateKeyRequest(
|
||||
key="sk-1",
|
||||
metadata={"unrelated": "value"},
|
||||
team_id="IJ",
|
||||
)
|
||||
existing_key = LiteLLM_VerificationToken(
|
||||
token="hashed",
|
||||
team_id="IJ",
|
||||
metadata={"service_account_id": "sa-123"},
|
||||
)
|
||||
|
||||
result = await prepare_key_update_data(
|
||||
data=data, existing_key_row=existing_key
|
||||
)
|
||||
|
||||
assert result["metadata"]["service_account_id"] == "sa-123"
|
||||
assert result["metadata"]["unrelated"] == "value"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_rejects_service_account_id_overwrite():
|
||||
"""
|
||||
Once assigned, a key's service_account_id is an identity marker — rebinding
|
||||
it via update would break spend attribution. Reject rather than silently
|
||||
ignore so scripted callers surface the bug.
|
||||
"""
|
||||
data = UpdateKeyRequest(
|
||||
key="sk-1",
|
||||
metadata={"service_account_id": "sa-new"},
|
||||
team_id="IJ",
|
||||
)
|
||||
existing_key = LiteLLM_VerificationToken(
|
||||
token="hashed",
|
||||
team_id="IJ",
|
||||
metadata={"service_account_id": "sa-old"},
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await prepare_key_update_data(data=data, existing_key_row=existing_key)
|
||||
assert exc_info.value.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_allows_matching_service_account_id():
|
||||
"""Resending the same value (e.g. UI round-trip) is a no-op, not a conflict."""
|
||||
data = UpdateKeyRequest(
|
||||
key="sk-1",
|
||||
metadata={"service_account_id": "sa-123", "other": "value"},
|
||||
team_id="IJ",
|
||||
)
|
||||
existing_key = LiteLLM_VerificationToken(
|
||||
token="hashed",
|
||||
team_id="IJ",
|
||||
metadata={"service_account_id": "sa-123"},
|
||||
)
|
||||
|
||||
result = await prepare_key_update_data(
|
||||
data=data, existing_key_row=existing_key
|
||||
)
|
||||
|
||||
assert result["metadata"]["service_account_id"] == "sa-123"
|
||||
assert result["metadata"]["other"] == "value"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_without_metadata_still_preserves_existing():
|
||||
"""Omitting metadata entirely must not drop existing metadata fields."""
|
||||
data = UpdateKeyRequest(key="sk-1", max_budget=100)
|
||||
existing_key = LiteLLM_VerificationToken(
|
||||
token="hashed",
|
||||
team_id="IJ",
|
||||
metadata={"service_account_id": "sa-123", "other": "kept"},
|
||||
)
|
||||
|
||||
result = await prepare_key_update_data(
|
||||
data=data, existing_key_row=existing_key
|
||||
)
|
||||
|
||||
assert result["metadata"]["service_account_id"] == "sa-123"
|
||||
assert result["metadata"]["other"] == "kept"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_prepare_key_update_data_duration_never_expires():
|
||||
"""Test that duration="-1" sets expires to None (never expires)."""
|
||||
@@ -7961,6 +8050,7 @@ async def test_update_key_non_budget_fields_allowed_for_internal_user(monkeypatc
|
||||
mock_existing_key.max_budget = 10.0
|
||||
mock_existing_key.key_alias = None
|
||||
mock_existing_key.models = []
|
||||
mock_existing_key.metadata = {}
|
||||
mock_existing_key.model_dump.return_value = {
|
||||
"token": test_hashed_token,
|
||||
"user_id": "internal_user",
|
||||
|
||||
Reference in New Issue
Block a user