Repository navigation
fix(rooms): a Workspace-only account cannot set its own room budget; cap room cost (#3495) - #3882
Conversation
…cap room cost (#3495) ent#387 made a room's budget an operator control, but drew the line at "platform principal" — which since #3488 includes the Workspace-only `user` role. Such an account kept whatever it sent: an uncapped cost, 500 messages, or a TTL of 0 (a room that never expires), while a portal client sending the same values was cut back to the operator defaults. - `_resolve_budget` now also requires a role at or above `operator` (`dependencies.is_workspace_only_role`, ent#837). Below it — the `user` rung, an MCP key such an account owns, a role outside the ladder, or no role at all — the caller's values are ignored and the operator defaults apply: the ent#387 rule, not a clamp. Operator and above may still bound a room, TTL 0 included. - `max_cost_usd` gets an upper bound, `MAX_COST_USD_CEILING` = $1,000 per room, on `RoomCreate` and on the operator default (`RoomBudgetDefaultsUpdate`; the two ranges mirror each other). A stored default above it is CLAMPED to it by `_coerce_cost` — reading it as invalid would mean no cap at all. The defaults response reports the ceiling beside the message and TTL limits. - MCP `create_room` declares the same 500 / $1,000 / 168 h bounds, and its stale "default 60" / "default 24" descriptions now point at the operator defaults. Fixes #3495 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Approving. The fix is in the right place: one role check in _resolve_budget using is_workspace_only_role. Portal clients and Workspace-only-owned agent keys can't get around it, and there's no other create or update path. I ran the new tests on the head (they pass), then reverted the role gate, the clamp and the le= bound one at a time; each made its own test fail.
Two small things, ideally in this PR:
docs/user-docs/collaboration/rooms.md:58(and line 34) still say any platform caller can set its own budget and that the cost cap has no upper limit. Please update both for theuserrule and the $1,000 ceiling._coerce_costnow lets a stored"nan"through (min(nan, 1000)isnan), which gives an uncapped room. Before this PR it fell back to the default. Please addmath.isfiniteto the check.
Nits: the settings panel doesn't use max_cost_usd_ceiling yet, and it's worth saying in the PR body that the default cost is still no cap and that rooms created before this fix keep their budgets.
|
merge-train follow-up (non-blocking, merged via train #3887): |
Summary
userrole. Those accounts kept whatever budget they sent: an uncapped cost, up to 500 messages, or a TTL of 0 (a room that never expires)._resolve_budgetnow also requires a role at or aboveoperator, usingdependencies.is_workspace_only_role(ent#837).operator, the caller's values are ignored and the operator defaults apply. That covers theuserrole, an MCP key such an account owns, a role outside the ladder, and a principal with no role at all. This is the ent#387 rule (ignore), not a clamp.operatorand above can still set a budget, TTL 0 included.MAX_COST_USD_CEILING, $1,000 per room. It's enforced onRoomCreateand on the operator default (RoomBudgetDefaultsUpdate), so the two ranges match.create_roomnow declares the same limits (500 messages, $1,000, 168 h). Its stale "default 60" and "default 24" descriptions now point to the operator defaults instead.The $1,000 figure is a judgement call: nothing in the codebase implied one. It's one constant plus the two model bounds that a test keeps in sync with it.
Changes
src/backend/shared_sessions/service.py: role gate in_resolve_budget,MAX_COST_USD_CEILING, clamp in_coerce_cost.src/backend/shared_sessions/models.py:le=1000.0on both cost fields,max_cost_usd_ceilingonRoomBudgetDefaults.src/backend/shared_sessions/router.py: reports the ceiling.src/mcp-server/src/tools/rooms.ts: matching bounds and descriptions.tests/unit/test_3495_room_budget_roles.py(new; reuses the ent#387 fixtures).Test Plan
cd tests && pytest -p no:randomly unit/test_3495_room_budget_roles.py -v: 11 tests. 8 failed before the fix; the 3 operator-and-above cases already passed. Covers:uservia JWT, auser's MCP key, an unknown role and a missing role all get the defaults;operator/creator/admincan set a budget including TTL 0; both models enforce the ceiling; a stored default above it is clamped; the response reports the ceiling.test_ent387_room_budget_defaults.pyis unchanged and passes, 25 passed with the new file.tsc --noEmitis clean.Fixes #3495
🤖 Generated with Claude Code