sec(auth): Standard Users and Read-Only Users are not system_managed, so #1737's guard does not protect them

[cristim]

Two seeded groups are not system_managed, so PUT /api/groups/{id} can corrupt their permissions even after PR LeanerCloud/cloud-commitments-cli#1737 lands its grant ceiling and system_managed guard.

group	id	system_managed
Administrators + 3 others	…000001–…000004	TRUE (000059)
Purchaser	…000007	TRUE (000064)
Standard Users	…000005	FALSE
Read-Only Users	…000006	FALSE

Traced through every migration touching the column: 000057 inserts both with an INSERT column list that omits system_managed, so they take DEFAULT FALSE. 000059's UPDATE … SET system_managed = TRUE covers only …000001–…000004. 000064 covers Purchaser. 000086 and 000088 modify those groups' permissions but never the column.

Why LeanerCloud/cloud-commitments-cli#1737's ceiling does not cover them

Two independent reasons, both read out of the committed code:

1. A narrowing write is invisible to the ceiling. checkGrantCeiling (internal/auth/group_ceiling.go:81) iterates the requested permissions only. It never diffs against existing to detect removals. sec(frontend): group edit drops unrepresentable permissions and widens their resource to * cloud-commitments-cli#1629's primary harm is the group-edit form silently dropping cancel-own / retry-own / approve-own on purchases — all three present on Standard Users (000057:30-32), none representable in the form's hardcoded action list. That write passes the ceiling untouched.
2. Admin widening is permitted for non-carved-out verbs, which is sec(frontend): group edit drops unrepresentable permissions and widens their resource to * cloud-commitments-cli#1629's exact scenario. grantCeilingAllows (:104) checks checkAdminPermission(held) first; view:* is not in adminCarvedOuts, so it returns true. view:history widens to view:*.

LeanerCloud/cloud-commitments-cli#1629 names this group explicitly: "The seeded user role-mirror group has the same shape."

What is and is not affected

• sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request cloud-commitments-cli#1550 is unaffected. The money separation-of-duties carve-out is enforced by the unconditional carve-out rule, which does not depend on system_managed. sec(auth): enforce a grant ceiling and system-managed guard on group writes cloud-commitments-cli#1737 closes sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request cloud-commitments-cli#1550 completely.
• sec(frontend): group edit drops unrepresentable permissions and widens their resource to * cloud-commitments-cli#1629 is only partly closed by the backend. Its frontend half (PR sec(frontend): stop group edit from dropping/widening permissions cloud-commitments-cli#1730) stops the form producing corrupt lists. Its backend half protects system_managed groups. Neither protects Standard Users or Read-Only Users from a direct API call.

The decision this needs — a product call, not an engineering one

Should Standard Users and Read-Only Users be system_managed?

• If they are meant to be fixed role mirrors, a one-line migration setting the column TRUE closes this entirely and is the smallest possible fix.
• If operators are meant to customise them — plausible, since "Standard Users" reads like a default an organisation would tailor — then locking them is wrong, and the gap must instead be closed by making the ceiling detect removals as well as additions. That is a larger change with its own risk: an over-strict removal check would block legitimate permission tidying.

This was deliberately not decided inside LeanerCloud/cloud-commitments-cli#1737. Marking a seeded group immutable changes what operators can configure, which is not a call to make as a side effect of a security PR.

If the removal-detection route is chosen

Note it is a genuinely different guard from the grant ceiling. "You may not grant what you do not hold" and "you may not silently drop what you did not intend to" are separate properties, and a single check that tries to be both will likely get one of them wrong. Any implementation needs a negative control proving deliberate permission removal still works.

Found during the pre-push review of LeanerCloud/cloud-commitments-cli#1737.

[cristim]

Correction to the issue body: the silently-dropped verbs on Standard Users are two, not three.

approve-own:purchases was already removed from that group by migration 000086 (issue LeanerCloud/cloud-commitments-cli#1407). So on a fully-migrated database the verbs the group-edit form silently drops are cancel-own and retry-own on purchases. 000057:30-32 describes the group as originally seeded, not as it stands today.

Also worth recording for whoever picks this up: 000074 re-adds the system_managed column with DEFAULT FALSE, which does not change the conclusion — Standard Users (…0005) and Read-Only Users (…0006) still end up FALSE — but it means the column has been touched by four migrations rather than three, and a future audit should check all four rather than assuming 000059 and 000064 are the only relevant ones.

Neither correction affects the substance: both groups remain non-system_managed and reachable via PUT /api/groups/{id} with the LeanerCloud/cloud-commitments-cli#1737 guard inactive, and both reasons the ceiling misses the damage still hold — it iterates requested and never diffs existing, so removals are invisible; and admin:* grants view:* because it is not carved out.

The product decision this issue exists for is unchanged.