mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(proxy): require admin for any /key/update spend, reject non-finite
Gate the admin check on the presence of `spend` (not a value diff): the DB spend lags the live cross-pod counter, so an "unchanged" spend on the non-admin path let a key owner / team member overwrite the live counter below real usage. Also reject NaN/+-inf spend before the DB write.
This commit is contained in:
parent
e50c12e97b
commit
eeb13259af
2 changed files with 43 additions and 4 deletions
|
|
@ -69,6 +69,7 @@ from litellm.proxy.management_endpoints.common_utils import (
|
|||
_is_user_team_admin,
|
||||
_set_object_metadata_field,
|
||||
_team_member_has_permission,
|
||||
validate_finite_spend,
|
||||
)
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
_add_model_to_db,
|
||||
|
|
@ -2204,6 +2205,9 @@ async def _validate_update_key_data(
|
|||
user_api_key_cache: Any,
|
||||
) -> None:
|
||||
"""Validate permissions and constraints for key update."""
|
||||
# Reject NaN/±inf spend before it can reach the DB / spend counter.
|
||||
validate_finite_spend(data.spend)
|
||||
|
||||
_is_proxy_admin = user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value
|
||||
|
||||
_check_allowed_routes_caller_permission(
|
||||
|
|
@ -2263,12 +2267,14 @@ async def _validate_update_key_data(
|
|||
# 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.
|
||||
# - spend gates on presence alone (not a value diff): the DB spend
|
||||
# lags the live cross-pod counter, so letting an "unchanged" spend
|
||||
# through the non-admin path would let a key owner / team member
|
||||
# overwrite the live counter below real usage and silently weaken
|
||||
# enforcement.
|
||||
_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)
|
||||
)
|
||||
or data.spend is not None
|
||||
or "budget_limits" in data.model_fields_set
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -9718,6 +9718,39 @@ class TestKeyOwnerPrivilegeEscalation:
|
|||
assert exc_info.value.status_code == 403
|
||||
mock_check.assert_called_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_creator_cannot_reset_own_spend_to_stale_value(self):
|
||||
"""Submitting `spend` equal to the stale DB value must still require
|
||||
admin. The DB spend lags the live cross-pod counter, so an
|
||||
"unchanged" spend on the non-admin path would let the creator
|
||||
overwrite the live counter below real usage. Any explicit `spend`
|
||||
is a budget change, regardless of value match."""
|
||||
existing = self._make_existing_key(created_by="creator-123")
|
||||
existing.spend = 0.0
|
||||
# spend equals the stale DB value (0.0) — the old `!=` gate skipped
|
||||
# the admin check here.
|
||||
data = UpdateKeyRequest(key="sk-test", spend=0.0)
|
||||
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_assigned_user_blocked_from_model_escalation(self):
|
||||
data = UpdateKeyRequest(key="sk-test", models=["gpt-4", "claude-opus"])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue