sec(auth): account scoping fails OPEN when a user's groups cannot be resolved (unrestricted access to all accounts) #1748

Found while answering a review check on PR #1737. Not fixed there — #1737 hardens the group write path; this is the read path, and folding it in would mix an access-control fix into a privilege-escalation review.

What

A user scoped to specific cloud accounts gets unrestricted access to all of them whenever their group memberships fail to resolve.

The chain is Handler.getAllowedAccounts (internal/api/handler.go:527) -> authServiceAdapter.GetAllowedAccountsAPI (internal/server/app.go:1159) -> Service.BuildAuthContext -> authCtx.AllowedAccounts.

collectGroupsAndAccounts (internal/auth/service_group.go) deliberately skips a group it cannot load, both on pgx.ErrNoRows and on a (nil, nil) return:

if errors.Is(err, pgx.ErrNoRows) {
    // Group was deleted; skip it rather than failing the entire request.
    continue
}
if group == nil {
    continue
}

So a user whose groups all fail to load ends with AllowedAccounts == [], and auth.IsUnrestrictedAccess documents empty as "all accounts" (a deliberate backward-compat default). The failure is silent: no error is returned and nothing is logged.

Verified by execution

scoped user, group loads fine (control)  -> AllowedAccounts=[acct-A]  IsUnrestrictedAccess=false
scoped user, group missing (ErrNoRows)   -> AllowedAccounts=[]        IsUnrestrictedAccess=true   <<< ALL ACCOUNTS
scoped user, group returns (nil, nil)    -> AllowedAccounts=[]        IsUnrestrictedAccess=true   <<< ALL ACCOUNTS

Blast radius

Every account-scoped handler, because they all funnel through the same helper — 17 getAllowedAccounts call sites across 9 files: handler_accounts.go (2), handler_analytics.go (1), handler_dashboard.go (2), handler_history.go (1), handler_ladder.go (1), handler_marketplace.go (1), handler_recommendations.go (1), handler_ri_exchange.go (4), scoping.go (4) — plus a direct GetAllowedAccountsAPI call in handler_purchases_revoke.go. scoping.go's four cover requireAccountAccess and requirePlanAccess, which the per-record scoping depends on.

Triggering conditions are ordinary rather than exotic: a group deleted while a session is live, a replica lagging behind a group insert, or any store path returning (nil, nil).

Why the write path is already fixed and this is not

PR #1737 hit the identical shape in grantCeilingAccounts and closed it by requiring at least one group to actually resolve:

if len(authCtx.Groups) == 0 {
    return nil, fmt.Errorf("%w: the acting user's account scope could not be established (no group resolved)", ErrPermissionCeiling)
}

The same guard belongs on the read path. Note the asymmetry that makes this easy to miss: the permission side already fails closed on the same input, because an empty permission set grants nothing. Only the account side fails open, and it does so without erroring.

Fix direction

Distinguish "resolved to unrestricted" from "could not resolve". Options, in rough order of preference:

1. Make BuildAuthContext report when a user has GroupIDs but none resolved, and have getAllowedAccounts fail closed on it. Keeps the backward-compat empty-means-all default intact for genuinely unrestricted groups.
2. Represent unrestricted explicitly (a sentinel or a *[]string) so absent and unrestricted stop sharing a representation. Larger, but removes the trap rather than guarding it.

Whichever is chosen, the regression test must force a group-resolution failure for a scoped user and assert access is refused — a test using an unrestricted user passes with the bug present.

Related

• sec(auth): enforce a grant ceiling and system-managed guard on group writes #1737 — the same shape on the write path, fixed there.
• sec(purchases): standard user can pause/resume other users' scheduled purchases (no ownership gate) #950 / fix(api): filter purchases by account UUID AND external id (closes #701, #498, #866) #956 — the account-filter regression class this sits in.
• fix(test): grantAdmin authorization-decision stub — remainder after #1744 (pair pinning, .Maybe() per-handler, scoped-account conversion) #1596 — the test helper that made restricted-account paths untestable, so this class had no coverage.