fix(backstage,beacon,types): board-seat delegation audit — one regression, the email hole, and a rules-parity test

[arkgast]

Summary

Audit of the board-seat / member-login delegation shipped in #224, and the fixes it turned up. #224 merged while this ran, so these land as a follow-up.

One regression #224 introduced, now in production. positionsLockedForEditor mirrors positionsAssignmentSafe()'s OLD-side conjunct (currentCargoGrantsEmpty, Admin-role only, deliberately never delegated). Both member forms keyed it on allowPowerGrants, which #224 widened from isAdmin to isAdmin || update:BoardSeat. So an update:BoardSeat delegate opening a member seated on a power-granting cargo gets a full picker, an enabled Guardar, and a guaranteed permission-denied — the render-then-die shape assignable-cargo.ts exists to prevent, reintroduced inside it. Only reachable once someone actually holds the delegated code.

The transversal-email rule had one hole. The profile page's "Invitar acceso" returned early on the action link and never sent the reset mail — the single surface where an invite produced no email. The row menu and the invite drawer were always correct.

Three server refusals rendered as "try again". beacon tags reprovision-requires-admin, granted-member-requires-admin and power-seat-requires-admin; the UI mapped only a fourth, so every delegate 403 read as transient and invited infinite retries.

Plus: the invite affordance mirrored one of beacon's four non-Admin refusals (now one shared predicate at all three entry points); assignedBy reached auth.getUser() unscreened, where a malformed value throws permanently under retry:false and kills that member's claims sync forever; member.email reached getUserByEmail unscreened with the same failure shape; the refusal reasons were five hand-copied string literals across two apps, now one typed union in @luminova/types.

The deeper fix

assignable-cargo.ts hand-mirrors firestore.rules, and its unit test never reads firestore.rules — change the rules and every assertion stays green. That mirror drifted twice on #224 alone (the option list, then the lock flag), both caught by review rather than by a test.

tests/firestore-rules/cargo-assignment-parity.test.ts asserts the implication instead: for every (principal, member fixture, cargo), if the editor would let it be submitted, the real emulator must allow the write. 17 principals — all built-in roles derived from ROLES, never hand-listed, plus 8 perms-only customs — × 6 member fixtures, both lanes, assignedBy self-stamped as the mapper does. 160 offered triples, each a real write. Both historical drifts turn it red with a message naming the drifted file.

Per docs/engineering-guardrails.md this is the "small parser + parity test" prescription with the parser half deliberately skipped: these are boolean predicates, not field lists, and the emulator is the parser.

Known, documented, not fixed

• Revocation is not immediate. Claims decode only in onAuthStateChanged off a cached token, and both the rules and the callable read request.auth.token — a revoked code keeps working until the ID token expires (~1h). Fixing it is app-wide auth-store surgery, outside this diff. Spec records the latency and revokeRefreshTokens as the immediate cut.
• The ceiling with all three codes held together is "everything but Admin". The self-assignment guard blocks one-write self-promotion, not the multi-write puppet form. Mutual exclusion would close it but contradicts the requirement that the seat delegate also send emails. Stated plainly in the spec, with the post-revocation checklist (a puppet's Auth account survives revocation; nothing in the codebase deletes or disables one).
• /permisos renders ~10 inert checkboxes for these two subjects. Pre-existing pattern, tripled by feat: delegable board-seat and member-login permissions #224. Tracked, not fixed.
• comisionIds: null drops the whole term entry, unseating the member. Kept fail-closed rather than fail-open, now logged. A one-line change if the owner prefers the other direction.
• The client mirror is deliberately stricter in one spot: parseDocs drops a position for any schema violation, so a cargo that exists with grants: [] but a malformed description locks the editor out of a takedown the rules would allow. Availability only, needs a malformed catalog doc, and an Admin can reassign. Documented at the predicate.
• Residual beacon items, recorded not fixed: auth/invalid-uid still surfaces opaque (different field, needs a console-written uid); the power-seat loop reads one cargo per distinct term serially with no ceiling (rules are term-pinned, and it fails closed); no beacon callable sets enforceAppCheck (consistent across all five, pre-existing).

Test plan

• backstage 913 · beacon 359 · types 331 · ui 110
• firestore-rules 541 (5 files, emulator on 4011 — the dev emulator on 4010 was never displaced)
• lint · typecheck · knip · prettier clean
• eager JS 160 kB gz (budget 162), CSS 13 kB gz (budget 15) — -9 B vs the pre-fix baseline, measured byte-for-byte against a scratch build of the base commit rather than the rounded gate number
• every new guard mutation-tested — see the table below. This line was false when first written, which is the finding I care most about in this PR; the two guards it was false for are fixed and now falsifiable.
• ci pass

Mutation evidence

Guard	Mutation applied	Result
CEL conjunct (firestore.rules)	delete && category != 'CEL'	10 rows red, both lanes
boardSeatDelegate()	widen hasPerm → canDo (so manage:all satisfies it)	3 rows red
fail-closed cargo lock	drop the unresolvable disjunct	2 rows red
link withholding + cache invalidation	return actionLink on success / drop onSettled	4 rows red
null-vs-throw port contract	also swallow auth/internal-error	its own row red
log-sample truncation	drop .map(truncateForLog)	its own row red
surrogate trim	neutralize the orphan check	its own row red

The last three were invisible to the entire suite before this round.

Reviews

• security-review
• firestore-security-reviewer
• firebase-functions-reviewer
• code-review
• simplify
• react-best-practices
• bundle-budget-watcher

/code-review cannot be launched programmatically, so the author ran it manually at max effort and an equivalent adversarial pass covered the delta afterwards. Every review above was re-run on the final delta; firestore-security-reviewer and firebase-functions-reviewer both returned SHIP with no Critical/High, and their Low/Medium items were taken.

What the last /code-review max found

Twelve findings, including three correctness bugs in member-profile-page.tsx alone — all in code this branch rewrote:

1. The invite mail could be dropped entirely. It was sent from a component-scoped mutate(id, {onSuccess}); TanStack v5 runs those only if (this.#mutateOptions && this.hasListeners()) — verified in the installed query-core 5.101.0 — so an operator who navigated away, or merely switched members (this branch's own key={member.id} made that an unmount), got the account created and the uid linked with no mail and no error. Provisioning and the mail are one mutationFn now; removeObserver never aborts it.
2. The Admin's action link was invalidated by the mail fired right after it — Firebase keeps only the most recent reset oobCode valid — while the new copy offered that link as the fallback. It is now returned only when the mail failed.
3. Both member forms lacked key={member.id} while a display-only banner had one, so a cache-warm A→B navigation could save A's identity onto B's document.

It also found the test-plan claim above was falsifiable and false, and that the BLOCKING test named for the #224 flag-conflation regression could not fail on flag conflation.

Reviews were run in four rounds. Every round found real defects in the previous round's fixes, and the last two found them in code written during this audit. The shape was the same almost every time: a claim the code did not have — "strict subset" (wasn't), "namespaced" (still collided), "make every fail-closed path say so" (missed the widest), "an Admin can still clear it" (they must reassign instead). What caught them was mechanical — the emulator parity test, mutation testing, reviewers treating comments as hypotheses. What kept creating them was prose. The later fixes therefore convert claims into things a machine can falsify: a required parameter instead of a convention, a compile error instead of a comment, an extracted module instead of a documented import cycle.