From 5c44a659604a36f7409f8caf326df8f5e7cffc8d Mon Sep 17 00:00:00 2001 From: Karim Baba Date: Thu, 20 Aug 2026 02:42:03 -0700 Subject: [PATCH] fix(proxy): only gate premium metadata fields when a key update changes them The key update path ran the enterprise check whenever a premium metadata field was present in the request. The Admin UI echoes a key's stored fields back on every save, so any key already carrying tags, guardrails or policies could not be edited without a license, and prepare_metadata_fields swallowed the raised 403 and dropped every metadata field ordered after it. Both update gates now compare against the stored value, so re-submitting an unchanged field is a no-op. Creation paths are unchanged and still gate on presence. --- .../management_endpoints/common_utils.py | 8 ++- .../key_management_endpoints.py | 3 +- .../test_key_management_endpoints.py | 70 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 2241884faf1..4aa6edb47aa 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -1,4 +1,5 @@ import math +from collections.abc import Mapping from typing import TYPE_CHECKING, Any, Final, Optional, Union from fastapi import HTTPException, status @@ -410,6 +411,7 @@ def _set_object_metadata_field( ], field_name: str, value: Any, + existing_metadata: Mapping[str, object] | None = None, ) -> None: """ Helper function to set metadata fields that require premium user checks @@ -418,8 +420,12 @@ def _set_object_metadata_field( object_data: The team/key/organization/project data object to modify field_name: Name of the metadata field to set value: Value to set for the field + existing_metadata: Stored metadata on update flows. When the submitted value + already matches what is stored, the premium check is skipped so that + re-submitting an unchanged field is not treated as enabling the feature. """ - if field_name in LiteLLM_ManagementEndpoint_MetadataFields_Premium and value: + is_new_value: Final = existing_metadata is None or existing_metadata.get(field_name) != value + if field_name in LiteLLM_ManagementEndpoint_MetadataFields_Premium and value and is_new_value: _premium_user_check(field_name) object_data.metadata = object_data.metadata or {} diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 71218d6114b..868cd343f61 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -2022,7 +2022,7 @@ def prepare_metadata_fields(data: BaseModel, non_default_values: dict, existing_ if k in LiteLLM_ManagementEndpoint_MetadataFields_Premium: from litellm.proxy.utils import _premium_user_check - if v: + if v and v != existing_metadata.get(k): _premium_user_check(k) casted_metadata[k] = v @@ -2061,6 +2061,7 @@ async def prepare_key_update_data( object_data=data, field_name=field, value=getattr(data, field), + existing_metadata=existing_key_row.metadata, ) for k, v in data_json.items(): if k in LiteLLM_ManagementEndpoint_MetadataFields or k in LiteLLM_ManagementEndpoint_MetadataFields_Premium: 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 8e661af8daa..d117b1710a7 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 @@ -2074,6 +2074,76 @@ async def test_prepare_key_update_data_disable_global_guardrails_true_premium_pe assert result["metadata"]["disable_global_guardrails"] is True +@pytest.mark.asyncio +async def test_prepare_key_update_data_unchanged_premium_field_no_premium(monkeypatch): + """ + Regression #15230: the UI echoes a key's stored premium fields back on every save, + so editing an unrelated field on a key that already carries tags or guardrails must + not 403 a non-premium user, and the stored values must survive the update. + """ + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False) + existing_key = LiteLLM_VerificationToken( + token="hashed", + metadata={"tags": ["prod"], "guardrails": ["pii-guard"]}, + ) + data = UpdateKeyRequest( + key="sk-1", + max_budget=10.0, + tags=["prod"], + guardrails=["pii-guard"], + ) + + result = await prepare_key_update_data(data=data, existing_key_row=existing_key) + + assert result["metadata"]["tags"] == ["prod"] + assert result["metadata"]["guardrails"] == ["pii-guard"] + assert result["max_budget"] == 10.0 + + +@pytest.mark.asyncio +async def test_prepare_key_update_data_changed_premium_field_requires_premium(monkeypatch): + """Control: changing a stored premium value without a license still 403s.""" + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False) + existing_key = LiteLLM_VerificationToken(token="hashed", metadata={"tags": ["prod"]}) + data = UpdateKeyRequest(key="sk-1", tags=["prod", "eu"]) + + 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 == 403 + + +@pytest.mark.asyncio +async def test_prepare_key_update_data_new_premium_field_requires_premium(monkeypatch): + """Control: setting a premium field a key does not have yet still 403s.""" + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False) + existing_key = LiteLLM_VerificationToken(token="hashed", metadata={}) + data = UpdateKeyRequest(key="sk-1", tags=["prod"]) + + 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 == 403 + + +@pytest.mark.asyncio +async def test_prepare_key_update_data_unchanged_premium_field_keeps_later_metadata(monkeypatch): + """ + Regression #15230: prepare_metadata_fields swallows the premium HTTPException, so the + gate firing on an unchanged field aborted the loop and silently dropped every metadata + field ordered after it. Editing enforced_params on a key that already carries tags must + persist enforced_params. + """ + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False) + existing_key = LiteLLM_VerificationToken(token="hashed", metadata={"tags": ["prod"]}) + data = UpdateKeyRequest(key="sk-1", tags=["prod"], enforced_params=["user"]) + + result = await prepare_key_update_data(data=data, existing_key_row=existing_key) + + assert result["metadata"]["enforced_params"] == ["user"] + assert result["metadata"]["tags"] == ["prod"] + + @pytest.mark.asyncio async def test_validate_team_id_used_in_service_account_request_requires_team_id(): """