sec(auth): group AllowedAccounts has no grant ceiling; a caller can widen a group past their own account scope

[cristim]

Discovered while implementing the group-permission grant ceiling in #1737 (fixes #1550). Deliberately left out of that PR to keep it proportionate.

What

PR #1737 added a grant ceiling to CreateGroupAPI / UpdateGroupAPI (internal/auth/service_api.go): a caller may only write permissions their own effective set holds, and the money verbs carved out of admin:* are not grantable at all.

The same request body also carries AllowedAccounts, and that field has no ceiling. applyUpdateGroupRequest writes it verbatim:

if req.AllowedAccounts != nil {
    group.AllowedAccounts = req.AllowedAccounts
}

A caller whose own groups restrict them to accounts A and B can therefore write allowed_accounts: ["*"] (or any account UUID they cannot see) onto a group and gain visibility into every registered cloud account. IsUnrestrictedAccess treats both an empty list and a "*" entry as unrestricted, so [] works as well as ["*"].

Blast radius

Data visibility, not money: AllowedAccounts feeds CanAccessAccount / GetAllowedAccountsAPI, which scope which cloud accounts a user sees. It does not grant a verb, so it cannot by itself reach a purchase. That is why it was scoped out of #1737 rather than folded in.

Note it is only reachable by a caller who already holds create:groups or update:groups, i.e. an admin today.

Fix direction

Mirror the permission ceiling: a group's AllowedAccounts may not name an account outside the union of the acting principal's own groups' AllowedAccounts, with IsUnrestrictedAccess(actor) meaning "any value is in ceiling". Fail closed if the actor's allowed set cannot be resolved, and refuse loudly naming the offending account rather than silently intersecting the list (same rule #1629 establishes for permissions).

The hook already exists: checkGrantCeiling in internal/auth/group_ceiling.go is called from both write paths with the acting principal already threaded through, so this is an added check inside an existing seam, not new plumbing.

Related

• sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request #1550 / sec(auth): enforce a grant ceiling and system-managed guard on group writes #1737 - the permission-side ceiling this mirrors.
• sec(frontend): group edit drops unrepresentable permissions and widens their resource to * #1629 - the "never silently narrow, always name what was refused" rule.
• sec: enforce view-only+write group-combination contradiction in the backend (guardGroupChange) cloud-commitments-platform#78 - the view-only / write-capable group combination check, likewise enforced only in the browser.

[cristim]

Fixed in PR #1737 rather than separately, on review. The finding there was sharper than this issue captured: an allowed_accounts-only PUT never reached the permission ceiling at all, because checkGrantCeiling opens with if len(requested) == 0 { return nil } and APIUpdateGroupRequest's "empty means not sent" contract makes that the natural request shape.

Verified by execution before fixing, with an actor holding only update:groups in one group scoped to one account:

widen to more accounts        -> ACCEPTED
widen to []  (= unrestricted) -> ACCEPTED
widen to ["*"]                -> ACCEPTED
CONTROL: same call WITH permissions -> REFUSED

The control is what made it decisive: the sibling dimension (Permissions[].Constraints.AccountIDs) was already correctly bounded on the same request, so the endpoint guarded one account axis and left the other open.

checkAccountCeiling is deliberately a separate call from checkGrantCeiling so it cannot inherit that early return. Two traps handled explicitly: [] and "*" both mean unrestricted, so it cannot be a subset test (the empty set is a subset of everything and means the opposite); and on create there is no prior scope, where a nil existing would read as unrestricted and swallow every check.

Every regression test sends allowed_accounts with no permissions key — a test including permissions alongside passes with the bug present, because the permission ceiling then runs and refuses for an unrelated reason.

Closes on merge of #1737.