mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-19 14:23:49 +00:00
[Fix] reject explicit null when clearing reserved metadata field
Addresses Greptile review feedback: - Clarify LiteLLM_Reserved_Metadata_Fields comment to describe both preserve-on-omit and reject-on-change behaviors. - Treat explicit null as a change attempt so callers trying to clear service_account_id get a 400 instead of a silent no-op.
This commit is contained in:
@@ -4047,7 +4047,8 @@ LiteLLM_ManagementEndpoint_MetadataFields_Premium = [
|
||||
"allowed_passthrough_routes",
|
||||
]
|
||||
|
||||
# Metadata keys preserved from existing rows when an update omits them.
|
||||
# Metadata keys that are immutable once set: preserved when an update omits them,
|
||||
# and rejected (400) when an update tries to change them.
|
||||
LiteLLM_Reserved_Metadata_Fields = [
|
||||
"service_account_id",
|
||||
]
|
||||
|
||||
@@ -1518,13 +1518,15 @@ 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.
|
||||
# when omitted, reject any explicit attempt to change it (including null).
|
||||
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:
|
||||
if (
|
||||
reserved_field in casted_metadata
|
||||
and casted_metadata[reserved_field] != existing_value
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"{reserved_field} is immutable once set and cannot be changed via update.",
|
||||
|
||||
@@ -1277,6 +1277,29 @@ async def test_update_allows_matching_service_account_id():
|
||||
assert result["metadata"]["other"] == "value"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_rejects_explicit_null_service_account_id():
|
||||
"""
|
||||
Explicit null is an attempt to clear — not an omission. Silently ignoring
|
||||
it would let a caller think they cleared the field when they didn't, so
|
||||
treat it the same as any other rebind attempt and return 400.
|
||||
"""
|
||||
data = UpdateKeyRequest(
|
||||
key="sk-1",
|
||||
metadata={"service_account_id": None},
|
||||
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_without_metadata_still_preserves_existing():
|
||||
"""Omitting metadata entirely must not drop existing metadata fields."""
|
||||
|
||||
Reference in New Issue
Block a user