diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 7fff640c49..0bcd57119d 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -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): """ diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index f69d9d2f8d..2d9fc4e89b 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -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: diff --git a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py index 479defbff5..bece11aa66 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py @@ -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",