From 98a2464b335045d2ea4294b728dd6f969a23c178 Mon Sep 17 00:00:00 2001 From: Filippo Mattia Menghi Date: Wed, 10 Jun 2026 10:19:32 +0200 Subject: [PATCH] Require admin access for budget_limits changes on /key/update Clearing budget_limits via [] or null is a budget mutation, but _validate_update_key_data only counted max_budget and spend as budget changes before deciding whether to skip _check_key_admin_access. A non-admin key owner or a team member with /key/update could therefore remove a key's per-window spend caps without admin authorization. Treat any explicit budget_limits value in the request (set, change, or clear) as a budget change so it gates through the same admin check as max_budget. model_fields_set is used because an explicit null is indistinguishable from an omitted field by value alone. --- .../key_management_endpoints.py | 18 ++++--- .../test_key_management_endpoints.py | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index c2cbb2a353f..24bfb71a624 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -2254,14 +2254,18 @@ async def _validate_update_key_data( # - Anyone else (non-PROXY_ADMIN, not the owner, not a team member # on a team key): must pass _check_key_admin_access (PROXY_ADMIN # / key-owner / team-admin / org-admin of the key). - # - max_budget / spend: always require the admin check, even for the - # key owner or a team member (matches the existing admin-only - # budget semantics). + # - max_budget / spend / budget_limits: always require the admin + # check, even for the key owner or a team member (matches the + # existing admin-only budget semantics). budget_limits uses + # model_fields_set because an explicit null/[] clears the field + # and must gate the same as setting or changing it. _is_budget_change = ( - data.max_budget is not None and data.max_budget != existing_key_row.max_budget - ) or ( - data.spend is not None - and data.spend != getattr(existing_key_row, "spend", None) + (data.max_budget is not None and data.max_budget != existing_key_row.max_budget) + or ( + data.spend is not None + and data.spend != getattr(existing_key_row, "spend", None) + ) + or "budget_limits" in data.model_fields_set ) # Personal-key bypass: the caller both created the key AND still owns it 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 4802761f3e3..7ee9984485e 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 @@ -9671,6 +9671,58 @@ class TestKeyOwnerPrivilegeEscalation: ) mock_check.assert_called_once() + @pytest.mark.asyncio + @pytest.mark.parametrize("cleared_value", [[], None]) + async def test_creator_cannot_clear_own_budget_limits(self, cleared_value): + """Clearing budget_limits is a budget change and requires admin.""" + data = UpdateKeyRequest(key="sk-test", budget_limits=cleared_value) + existing = self._make_existing_key(created_by="creator-123") + auth = self._make_auth(user_id="creator-123") + + mock_check = AsyncMock( + side_effect=HTTPException(status_code=403, detail="Not authorized") + ) + with patch( + "litellm.proxy.management_endpoints.key_management_endpoints._check_key_admin_access", + mock_check, + ): + with pytest.raises(HTTPException): + await _validate_update_key_data( + data=data, + existing_key_row=existing, + user_api_key_dict=auth, + llm_router=None, + premium_user=False, + prisma_client=AsyncMock(), + user_api_key_cache=MagicMock(), + ) + mock_check.assert_called_once() + + @pytest.mark.asyncio + async def test_admin_can_clear_budget_limits(self): + data = UpdateKeyRequest(key="sk-test", budget_limits=[]) + existing = self._make_existing_key(created_by="someone-else") + auth = UserAPIKeyAuth( + user_id="admin-user", + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + + mock_check = AsyncMock() + with patch( + "litellm.proxy.management_endpoints.key_management_endpoints._check_key_admin_access", + mock_check, + ): + await _validate_update_key_data( + data=data, + existing_key_row=existing, + user_api_key_dict=auth, + llm_router=None, + premium_user=False, + prisma_client=AsyncMock(), + user_api_key_cache=MagicMock(), + ) + mock_check.assert_not_called() + @pytest.mark.asyncio async def test_admin_can_update_any_field(self): data = UpdateKeyRequest(key="sk-test", models=["gpt-4"], max_budget=999.0)