Skip to content

sec(api): setPlanAccounts re-points ANY plan to ANY accounts (no requirePlanAccess, no allowed_accounts) #1552

Description

@cristim

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

  1. cristim commented on Sep 2, 2026

    @cristim
    MemberAuthor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions