From e24ea2897a9b8217eb89c004782afdb34db85022 Mon Sep 17 00:00:00 2001 From: Varun Chawla <34209028+veeceey@users.noreply.github.com> Date: Sat, 7 Feb 2026 22:22:12 -0800 Subject: [PATCH] fix: empty guardrails/policies arrays should not trigger enterprise license check (#20567) * fix: empty guardrails/policies arrays should not trigger enterprise license check (#20304) The UI sends empty arrays for enterprise-only fields (guardrails, policies, logging) even when the user has not configured these features. The backend `is not None` check treated `[]` as a truthy intent to use the feature, falsely requiring an enterprise license for basic team operations. Backend: Add `and updated_kv[field] != [] and updated_kv[field] != {}` guards in `_update_metadata_fields` so empty collections are skipped. UI: Conditionally omit guardrails, logging, and policies from the payload when empty instead of defaulting to `[]`. Fixes #20304 * fix: allow clearing fields with empty collections while skipping enterprise check Address PR review feedback: 1. Move the empty-collection guard into _update_metadata_field (singular) so that empty lists/dicts skip only the premium license check but still get written into metadata. This lets users intentionally clear a previously-set field (e.g. guardrails: []) without being blocked, while the UI's default empty arrays still don't trigger a false enterprise error. 2. Remove sys.path hack from test file; use standard imports that work with pytest discovery. 3. Add tests verifying that empty collections are moved into metadata (field clearing works) even though they bypass the premium check. Fixes #20304 --- .../management_endpoints/common_utils.py | 9 +- .../management_endpoints/test_common_utils.py | 162 ++++++++++++++++++ .../src/components/team/team_info.tsx | 6 +- 3 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 tests/test_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..24a41a2361b 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -216,7 +216,14 @@ def _update_metadata_field(updated_kv: dict, field_name: str) -> None: field_name: Name of the metadata field being updated """ if field_name in LiteLLM_ManagementEndpoint_MetadataFields_Premium: - _premium_user_check() + value = updated_kv.get(field_name) + # Skip the premium check for empty collections ([] or {}). + # The UI sends these as defaults even when the user hasn't configured + # any enterprise features (see issue #20304). However, we still + # proceed with the update so that users can intentionally clear a + # previously-set field by sending an empty list/dict. + if value is not None and value != [] and value != {}: + _premium_user_check() if field_name in updated_kv and updated_kv[field_name] is not None: # remove field from updated_kv diff --git a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py new file mode 100644 index 00000000000..b372476c3d6 --- /dev/null +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -0,0 +1,162 @@ +""" +Tests for litellm/proxy/management_endpoints/common_utils.py + +Covers the fix for GitHub issue #20304: +Empty guardrails/policies arrays sent by the UI should NOT trigger the +enterprise (premium) license check, but should still be applied so that +users can intentionally clear previously-set fields. +""" + +from unittest.mock import patch + +from litellm.proxy.management_endpoints.common_utils import ( + _update_metadata_fields, +) + + +class TestUpdateMetadataFieldsEmptyCollections: + """ + Regression tests for issue #20304. + + The UI sends empty arrays (`[]`) for enterprise-only fields like + guardrails, policies, and logging even when the user hasn't configured + these features. The backend must not treat empty collections as an + intent to use the feature, and therefore must not trigger the premium + license check. + + However, empty collections must still be written into metadata so that + users can intentionally clear a previously-set field (e.g. removing all + guardrails by sending `guardrails: []`). + """ + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_empty_list_does_not_trigger_premium_check(self, mock_premium_check): + """Empty lists for premium fields must not trigger the premium check.""" + updated_kv = { + "team_id": "test-team", + "guardrails": [], + "policies": [], + "logging": [], + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_not_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_empty_list_still_updates_metadata(self, mock_premium_check): + """ + Empty lists must still be moved into metadata so users can clear + previously-set fields (e.g. remove all guardrails). + """ + updated_kv = { + "team_id": "test-team", + "guardrails": [], + "policies": [], + } + _update_metadata_fields(updated_kv=updated_kv) + # The fields should have been moved into metadata + assert "guardrails" not in updated_kv, ( + "guardrails should be popped from top-level" + ) + assert "policies" not in updated_kv, ( + "policies should be popped from top-level" + ) + assert updated_kv["metadata"]["guardrails"] == [] + assert updated_kv["metadata"]["policies"] == [] + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_empty_dict_does_not_trigger_premium_check(self, mock_premium_check): + """Empty dicts for premium fields must not trigger the premium check.""" + updated_kv = { + "team_id": "test-team", + "secret_manager_settings": {}, + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_not_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_empty_dict_still_updates_metadata(self, mock_premium_check): + """ + Empty dicts must still be moved into metadata so users can clear + previously-set fields. + """ + updated_kv = { + "team_id": "test-team", + "secret_manager_settings": {}, + } + _update_metadata_fields(updated_kv=updated_kv) + assert "secret_manager_settings" not in updated_kv, ( + "secret_manager_settings should be popped from top-level" + ) + assert updated_kv["metadata"]["secret_manager_settings"] == {} + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_none_value_does_not_trigger_premium_check(self, mock_premium_check): + """None values for premium fields should be silently ignored.""" + updated_kv = { + "team_id": "test-team", + "guardrails": None, + "policies": None, + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_not_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_absent_fields_do_not_trigger_premium_check(self, mock_premium_check): + """Fields not present in the dict should not trigger premium check.""" + updated_kv = { + "team_id": "test-team", + "team_alias": "example-team", + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_not_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_non_empty_list_triggers_premium_check(self, mock_premium_check): + """Non-empty lists for premium fields should trigger the premium check.""" + updated_kv = { + "team_id": "test-team", + "guardrails": ["my-guardrail"], + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_non_empty_value_triggers_premium_check(self, mock_premium_check): + """Non-empty string values for premium fields should trigger the premium check.""" + updated_kv = { + "team_id": "test-team", + "tags": ["production"], + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_called() + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_non_empty_list_updates_metadata(self, mock_premium_check): + """Non-empty lists should be moved into metadata.""" + updated_kv = { + "team_id": "test-team", + "guardrails": ["my-guardrail"], + } + _update_metadata_fields(updated_kv=updated_kv) + assert "guardrails" not in updated_kv + assert updated_kv["metadata"]["guardrails"] == ["my-guardrail"] + + @patch("litellm.proxy.management_endpoints.common_utils._premium_user_check") + def test_ui_typical_payload_does_not_trigger_premium_check(self, mock_premium_check): + """ + Simulate the exact payload the UI sends when no enterprise features + are configured. This must NOT trigger the premium check. + """ + # This is the payload structure the UI sends (from issue #20304) + updated_kv = { + "team_id": "67848772-1a8b-4343-938c-17e60f1db860", + "team_alias": "example-team", + "models": ["gpt-4"], + "metadata": { + "guardrails": [], + "logging": [], + }, + "policies": [], + } + _update_metadata_fields(updated_kv=updated_kv) + mock_premium_check.assert_not_called() diff --git a/ui/litellm-dashboard/src/components/team/team_info.tsx b/ui/litellm-dashboard/src/components/team/team_info.tsx index 35f5b87e071..014f8fb9010 100644 --- a/ui/litellm-dashboard/src/components/team/team_info.tsx +++ b/ui/litellm-dashboard/src/components/team/team_info.tsx @@ -465,8 +465,8 @@ const TeamInfoView: React.FC = ({ budget_duration: values.budget_duration, metadata: { ...parsedMetadata, - guardrails: values.guardrails || [], - logging: values.logging_settings || [], + ...(values.guardrails?.length > 0 ? { guardrails: values.guardrails } : {}), + ...(values.logging_settings?.length > 0 ? { logging: values.logging_settings } : {}), disable_global_guardrails: values.disable_global_guardrails || false, soft_budget_alerting_emails: typeof values.soft_budget_alerting_emails === "string" @@ -477,7 +477,7 @@ const TeamInfoView: React.FC = ({ : values.soft_budget_alerting_emails || [], ...(secretManagerSettings !== undefined ? { secret_manager_settings: secretManagerSettings } : {}), }, - policies: values.policies || [], + ...(values.policies?.length > 0 ? { policies: values.policies } : {}), organization_id: values.organization_id, };