Repository navigation
sec(api): setPlanAccounts re-points ANY plan to ANY accounts (no requirePlanAccess, no allowed_accounts) #1552
Description
Reviewed commit: be11bdcb5 (origin/main), from the 2026-07-28 full-repo review.
Severity: HIGH. Privilege escalation into another tenant's accounts via a plan re-point.
Where
internal/api/handler_accounts.go:1304-1348 (setPlanAccounts)
What
PUT /api/plans/{id}/accounts calls requirePermission(ctx, httpReq, "update", "plans") at :1309 and then goes straight to UUID-shape validation and validatePlanAccountProviders.
It never calls h.requirePlanAccess(ctx, session, id), which every other plan mutation does (handler_plans.go:169/209/267/293/491), and it never checks the supplied account_ids against the caller's allowed_accounts. It also discards the session: requirePermission's return value is assigned to _.
Failure scenario
User U has allowed_accounts = [acct-A] and update:plans. Plan P belongs to acct-B, and GET /api/plans/P correctly 404s for U.
U sends PUT /api/plans/P/accounts {"account_ids":["<acct-A>","<acct-B>"]}, which returns 200. P now intersects acct-A, so requirePlanAccess passes for U from that point on, and POST /api/plans/P/purchases (handler_plans.go:278) creates scheduled executions against a plan that still targets acct-B.
The reverse direction is equally available: U re-points their own plan onto acct-B and schedules purchases there.
Fix direction
Add requirePlanAccess(session, id) plus a per-account_id requireAccountAccess loop before SetPlanAccounts.
Related
Same family as LeanerCloud/cloud-commitments-platform#29, which covers only listPlans read scoping. Also related: #959 (dashboard calculateCommitmentMetrics allowed-accounts intersection).
Activity
cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: PUT /api/plans/{id}/accounts now keeps the session from requirePermission and calls requirePlanAccountsAccess, which enforces requirePlanAccess on the plan and requireAccountAccess on every supplied account_id for scoped sessions (enumeration-safe not-found on refusal), before provider validation and the SetPlanAccounts write. Evidence: internal/api/handler_accounts.go:1329 and internal/api/scoping.go:100 (#1813, which closed the duplicate #1769). Residual axes checked: listPlanAccounts read path (fixed in the same PR); createPlan target_accounts (still open, tracked as LeanerCloud/cloud-commitments-platform#95). Closing as completed; reopen if the behaviour recurs.