fix(proxy): enforce org budget ceilings on /team/update

update_team loaded the org without its budget row, so the org max_budget,
tpm_limit and rpm_limit checks silently passed. It now loads the budget the
same way /team/new does
This commit is contained in:
ryan-crabbe-berri 2026-09-16 11:09:44 -07:00
parent a44a58e91a
commit 6e2ae19670
3 changed files with 100 additions and 9 deletions

View file

@ -2330,6 +2330,7 @@ async def update_team(
org_id=org_id_to_check,
user_api_key_cache=user_api_key_cache,
prisma_client=prisma_client,
include_budget_table=True,
)
if org_table is not None:
await _check_org_team_limits(

View file

@ -10,12 +10,11 @@ Pins the five helpers
Driven through /team/new + /team/update.
Structural finding, updated: /team/new loads the org via `get_org_object`
WITH `include_budget_table=True`, so the org max_budget / org tpm / org rpm
guards inside `_check_org_team_limits` are live there and are pinned as
enforced below. /team/update still loads the org without the budget
relation, so its budget guards remain no-ops. The `models` subset guard IS
reachable on both because it reads `org_table.models` directly. The
Structural finding, updated: /team/new and /team/update both load the org
via `get_org_object` WITH `include_budget_table=True`, so the org max_budget /
org tpm / org rpm guards inside `_check_org_team_limits` are live on both and
are pinned as enforced below. The `models` subset guard reads
`org_table.models` directly. The
`_check_user_team_limits` guards reach all branches through
`user_api_key_dict`, no relation include needed.
"""
@ -139,9 +138,8 @@ async def test_check_org_team_limits_models_subset(
# ---------------------------------------------------------------------------
# _check_org_team_limits — budget / tpm / rpm live on /team/new since its
# get_org_object call passes include_budget_table=True. (/team/update still
# loads the org without the budget relation, so its guards remain no-ops.)
# _check_org_team_limits — budget / tpm / rpm live on /team/new and
# /team/update since both get_org_object calls pass include_budget_table=True.
# ---------------------------------------------------------------------------
_ORG_BUDGET_ENFORCED_SCENARIOS = [
@ -216,6 +214,35 @@ async def test_check_org_team_limits_budget_enforced(
assert len(rows) == (1 if expected_status == 200 else 0)
@pytest.mark.parametrize(
"org_budget,body_extras,expected_status",
[(b, c, d) for (_id, b, c, d) in _ORG_BUDGET_ENFORCED_SCENARIOS],
ids=[s[0] for s in _ORG_BUDGET_ENFORCED_SCENARIOS],
)
async def test_check_org_team_limits_budget_enforced_on_update(
org_budget,
body_extras: Dict[str, Any],
expected_status: int,
proxy_client,
prisma,
scratch,
world,
):
org_id = await create_scratch_org(prisma, scratch.prefix, **org_budget)
team_id = await create_scratch_team(prisma, scratch.tag("team"), organization_id=org_id)
seeder = world.keys[Actor.PROXY_ADMIN].cleartext
resp = await proxy_client.post(
"/team/update",
headers={"Authorization": f"Bearer {seeder}"},
json={"team_id": team_id, **body_extras},
)
assert resp.status_code == expected_status, f"{body_extras!r}{resp.status_code}: {resp.text}"
row = await prisma.db.litellm_teamtable.find_unique(where={"team_id": team_id})
assert row is not None
persisted = {field: getattr(row, field) for field in body_extras}
assert (persisted == body_extras) == (expected_status == 200)
# ---------------------------------------------------------------------------
# _check_user_team_limits — fires for standalone (no-org) teams created by
# a non-admin caller. Each guard reads from user_api_key_dict / user_obj.

View file

@ -15078,6 +15078,69 @@ async def test_update_team_team_admin_changes_tpm_limit_once_a_proxy_admin_enabl
assert "'rpm_limit'" in str(refused.value.message)
@pytest.mark.asyncio
async def test_update_team_holds_a_team_admin_to_the_org_tpm_limit(disable_audit_logging_for_mocked_team):
"""The org ceiling lives on the org's budget row, so /team/update must load it to enforce the cap."""
import contextlib
capped_org = LiteLLM_OrganizationTable(
organization_id="capped-org",
budget_id="capped-budget",
created_by="admin",
updated_by="admin",
litellm_budget_table=LiteLLM_BudgetTable(tpm_limit=10000),
)
async def org_lookup(**kwargs):
return capped_org if kwargs.get("include_budget_table") else capped_org.model_copy(
update={"litellm_budget_table": None}
)
org_team = MagicMock()
org_team.metadata = {}
org_team.organization_id = "capped-org"
org_team.model_dump.return_value = {
"team_id": "test_team_id",
"team_alias": "test_team",
"organization_id": "capped-org",
"metadata": {},
"members_with_roles": [{"user_id": "team-admin", "role": "admin"}],
}
with contextlib.ExitStack() as stack:
prisma = _wire_update_team(stack, {})
prisma.db.litellm_teamtable.find_unique = AsyncMock(return_value=org_team)
stack.enter_context(_team_admin_may_edit("tpm_limit"))
stack.enter_context(
patch( # test-quality-ok: the org-admin lookup needs a real prisma client this file's MagicMock cannot provide
"litellm.proxy.management_endpoints.team_endpoints._is_user_org_admin_for_team",
AsyncMock(return_value=False),
)
)
stack.enter_context(
patch( # test-quality-ok: update_team reads orgs through this module-level import; no seam to inject
"litellm.proxy.management_endpoints.team_endpoints.get_org_object",
AsyncMock(side_effect=org_lookup),
)
)
with pytest.raises(ProxyException) as over_cap:
await update_team(
data=UpdateTeamRequest(team_id="test_team_id", tpm_limit=20000),
http_request=_update_request_stub(),
user_api_key_dict=_TEAM_ADMIN_CALLER,
)
await update_team(
data=UpdateTeamRequest(team_id="test_team_id", tpm_limit=8000),
http_request=_update_request_stub(),
user_api_key_dict=_TEAM_ADMIN_CALLER,
)
assert str(over_cap.value.code) == "400"
assert "exceeds organization's tpm_limit (10000)" in str(over_cap.value.message)
assert prisma.db.litellm_teamtable.update.await_count == 1
assert prisma.db.litellm_teamtable.update.call_args.kwargs["data"]["tpm_limit"] == 8000
@pytest.mark.asyncio
async def test_update_team_org_admin_is_not_filtered_by_the_team_admin_field_list(
disable_audit_logging_for_mocked_team,