From 0ad5748b36a09c08a4d485773c69600adf084c9f Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 12 Feb 2026 16:22:29 -0800 Subject: [PATCH] Revert "Merge pull request #20598 from muraliavarma/fix/team-update-empty-premium-fields-403" This reverts commit df15456bccd9b30dd1061abc30ab04d54f1305f1, reversing changes made to 89565c97ccde3d61d60c8f42b2caaed0883e3eae. --- .../management_endpoints/common_utils.py | 13 +- .../management_endpoints/test_common_utils.py | 159 ------------------ 2 files changed, 1 insertion(+), 171 deletions(-) delete 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 d6f6d1337a3..942758e3bab 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -383,17 +383,6 @@ 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). @@ -402,7 +391,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 _has_non_empty_value(updated_kv[field]): + if field in updated_kv and updated_kv[field] is not None: _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 deleted file mode 100644 index f857db770d0..00000000000 --- a/tests/litellm/proxy/management_endpoints/test_common_utils.py +++ /dev/null @@ -1,159 +0,0 @@ -""" -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()