mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
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
This commit is contained in:
parent
5a084cef41
commit
e24ea2897a
3 changed files with 173 additions and 4 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
@ -465,8 +465,8 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
|||
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<TeamInfoProps> = ({
|
|||
: 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,
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue