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.
This commit is contained in:
Filippo Mattia Menghi 2026-06-10 10:19:32 +02:00
parent 8382e549e4
commit 98a2464b33
2 changed files with 63 additions and 7 deletions

View file

@ -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

View file

@ -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)