From 52c9cf6058998816f82e7f7c5c61bd88fb86367e Mon Sep 17 00:00:00 2001 From: Murali Date: Fri, 6 Feb 2026 14:50:24 -0500 Subject: [PATCH] fix(proxy): skip premium check for empty metadata fields on team/key update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #20534 The UI sends the full form on every team update, including premium metadata fields like `policies: []` and `team_member_key_duration: ""`. The backend's `_update_metadata_fields` treated any non-None value as premium feature usage and returned 403 for non-enterprise users — even when the fields were empty and the user was just updating basic settings like team name or budget. Added `_has_non_empty_value` helper and use it in the premium field gate in `_update_metadata_fields` so empty lists, blank strings, and None values skip the premium check entirely. Non-empty values still enforce the enterprise requirement as before. --- .../management_endpoints/common_utils.py | 13 +- .../management_endpoints/test_common_utils.py | 159 ++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 tests/litellm/proxy/management_endpoints/test_common_utils.py diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 8f7dd4f8dfa..0fa943d96e8 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -227,6 +227,17 @@ def _update_metadata_field(updated_kv: dict, field_name: str) -> None: updated_kv["metadata"] = {field_name: _value} +def _has_non_empty_value(value: Any) -> bool: + """Check if a value has real content (not None, not empty list, not blank string).""" + if value is None: + return False + if isinstance(value, list) and len(value) == 0: + return False + if isinstance(value, str) and value.strip() == "": + return False + return True + + def _update_metadata_fields(updated_kv: dict) -> None: """ Helper function to update all metadata fields (both premium and standard). @@ -235,7 +246,7 @@ def _update_metadata_fields(updated_kv: dict) -> None: updated_kv: The key-value dict being used for the update """ for field in LiteLLM_ManagementEndpoint_MetadataFields_Premium: - if field in updated_kv and updated_kv[field] is not None: + if field in updated_kv and _has_non_empty_value(updated_kv[field]): _update_metadata_field(updated_kv=updated_kv, field_name=field) for field in LiteLLM_ManagementEndpoint_MetadataFields: diff --git a/tests/litellm/proxy/management_endpoints/test_common_utils.py b/tests/litellm/proxy/management_endpoints/test_common_utils.py new file mode 100644 index 00000000000..f857db770d0 --- /dev/null +++ b/tests/litellm/proxy/management_endpoints/test_common_utils.py @@ -0,0 +1,159 @@ +""" +Tests for litellm/proxy/management_endpoints/common_utils.py + +Specifically tests that _update_metadata_fields does not trigger premium +user checks when premium fields are present but empty. + +Related: https://github.com/BerriAI/litellm/issues/20534 +""" + +from unittest.mock import patch + +import pytest + +from litellm.proxy.management_endpoints.common_utils import ( + _has_non_empty_value, + _update_metadata_fields, +) + + +class TestHasNonEmptyValue: + """Tests for the _has_non_empty_value helper.""" + + def test_none_is_empty(self): + assert _has_non_empty_value(None) is False + + def test_empty_list_is_empty(self): + assert _has_non_empty_value([]) is False + + def test_empty_string_is_empty(self): + assert _has_non_empty_value("") is False + + def test_blank_string_is_empty(self): + assert _has_non_empty_value(" ") is False + + def test_non_empty_list_has_value(self): + assert _has_non_empty_value(["policy-a"]) is True + + def test_non_empty_string_has_value(self): + assert _has_non_empty_value("30d") is True + + def test_dict_has_value(self): + assert _has_non_empty_value({"key": "val"}) is True + + def test_empty_dict_has_value(self): + # empty dict is not None/list/str, so it counts as non-empty + assert _has_non_empty_value({}) is True + + +class TestUpdateMetadataFieldsPremiumCheck: + """ + Tests that _update_metadata_fields skips premium user checks for empty + values but still enforces them for real values. + + Issue: The UI sends the full form on every team update, including premium + fields like `policies: []`. The backend was treating these empty values + as premium feature usage and returning 403. + """ + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + side_effect=Exception("Should not be called"), + ) + def test_empty_policies_skips_premium_check(self, mock_check): + """policies: [] should NOT trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "team_alias": "my-team", + "policies": [], + } + _update_metadata_fields(updated_kv) + mock_check.assert_not_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + side_effect=Exception("Should not be called"), + ) + def test_empty_guardrails_skips_premium_check(self, mock_check): + """guardrails: [] should NOT trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "guardrails": [], + } + _update_metadata_fields(updated_kv) + mock_check.assert_not_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + side_effect=Exception("Should not be called"), + ) + def test_empty_string_team_member_key_duration_skips_premium_check( + self, mock_check + ): + """team_member_key_duration: '' should NOT trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "team_member_key_duration": "", + } + _update_metadata_fields(updated_kv) + mock_check.assert_not_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + side_effect=Exception("Should not be called"), + ) + def test_full_ui_payload_with_empty_premium_fields_skips_premium_check( + self, mock_check + ): + """A realistic UI payload with all empty premium fields should not 403.""" + updated_kv = { + "team_id": "team-123", + "team_alias": "renamed-team", + "models": ["gpt-4o"], + "max_budget": 200, + "policies": [], + "guardrails": [], + "logging": [], + "team_member_key_duration": "", + "prompts": [], + } + _update_metadata_fields(updated_kv) + mock_check.assert_not_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + ) + def test_non_empty_policies_triggers_premium_check(self, mock_check): + """policies: ['real-policy'] SHOULD trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "policies": ["real-policy"], + } + _update_metadata_fields(updated_kv) + mock_check.assert_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + ) + def test_non_empty_guardrails_triggers_premium_check(self, mock_check): + """guardrails: ['my-guardrail'] SHOULD trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "guardrails": ["my-guardrail"], + } + _update_metadata_fields(updated_kv) + mock_check.assert_called() + + @patch( + "litellm.proxy.management_endpoints.common_utils._premium_user_check", + ) + def test_non_empty_team_member_key_duration_triggers_premium_check( + self, mock_check + ): + """team_member_key_duration: '30d' SHOULD trigger premium user check.""" + updated_kv = { + "team_id": "team-123", + "team_member_key_duration": "30d", + } + _update_metadata_fields(updated_kv) + mock_check.assert_called()