feat: delegable board-seat and member-login permissions - #224
Merged
Conversation
Two delegable capabilities an Admin can grant temporarily and revoke: update:BoardSeat (seat a member on any vacant cargo, CEL and power-granting alike) and create:MemberLogin (provision a member's Auth login). Both gates are exact hasPerm code tests, so manage:all does not satisfy them and the sibling codes the cross-product generates stay inert. Spec and plan record the three guards adversarial review required: the trust gate must be non-reflexive, currentCargoGrantsEmpty stays Admin-only, and a non-Admin provision caller may only mint a new Auth account. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
boardSeatDelegate() lifts the NEW-side cargo conjunct so an update:BoardSeat holder may seat a member on a CEL or power-granting cargo. It confers nothing alone — the principal still needs update:Member, create:Member or update:Position to reach an arm at all. The OLD side (currentCargoGrantsEmpty) stays Admin-role-only. computeMemberRoles derives the roles claim exclusively from cargo grants, so a principal able to overwrite a sitting Admin's cargo could strip every Admin claim in the chapter, and setUserRoles / roles/* / permissionOverrides are all Admin-only — the state would be unrecoverable outside the Firebase console. Falsified: neutralizing boardSeatDelegate() to Admin-only turns exactly the three new ALLOW tests red, 481 others green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…MemberLogin Trust gate: resolveTrustedGrants now honors a cargo's grants when the assigner holds the Admin role OR the exact update:BoardSeat perm. Without the second disjunct a delegate's seat would publish on the public Directiva and mint nothing, since the rules make them stamp their own uid into assignedBy. The gate is NON-REFLEXIVE on a self-assignment: when the assigner is the member being seated, only the perm is trusted. Otherwise a delegate self-seats Presidente, gets minted Admin, and that minted Admin satisfies the gate that minted it — revoking the perm would re-fire the trigger and re-honor the grants forever. recomputeAllClaims runs the same code and would not break the loop. provisionMemberLogin moves to requireAdminOrPerm(create:MemberLogin), with a hard adoption guard: a non-Admin caller may only mint a brand-new Auth account or re-provision one already linked to that member. members.email is not constrained by firestore.rules and has no uniqueness check, so without the guard a create:Member + create:MemberLogin holder could file a member doc carrying an Admin's email and have the callable strip that Admin's claims, bind their uid via the admin SDK, and return a password-reset link for their mailbox. callerIsAdmin defaults to false so a new call site opts into adoption. callable-auth gains its first direct unit tests, incl. the manage:all and manage:<subject> wildcards failing the exact-code gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… empty cargo list canAssignPowerGrants is replaced by three flags. It had five consumers and one of them — positions-page's PositionForm canEditGrants — governs the /positions CATALOG, not member seating. Widening it in place would have handed a seat delegate the ability to mint a grant-free CEL 'Presidente' and then seat itself on it at public board rank 0. canAssignBoardSeat = Admin role || update:BoardSeat (member create+update lanes) canEditCargoCatalog = Admin role (/positions, unchanged) canProvisionLogin = Admin role || create:MemberLogin (invite affordances) The invite affordances (drawer checkbox, row menu, profile header) move off the Admin role onto canProvisionLogin. All three flags use the exact-code hasPerm, never CASL, so manage:all cannot satisfy them and produce a render-then-403. NoAssignableCargosNote explains a Cargo picker the permission ceiling emptied — previously a bare "Sin resultados" indistinguishable from an empty catalog, which is the permanent state of a chapter whose every cargo carries grants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- extract noAssignableCargos() into assignable-cargo.ts. The three-clause render guard was typed out at both member forms, immediately next to the two sibling predicates that file exists to hold so the forms cannot disagree. - extract adminOrPerm() in use-can.ts at its third occurrence. - rename provisionMember's callerIsAdmin parameter to callerHoldsAdminRole: it shadowed the imported callerIsAdmin function in the same module. Skipped: hoisting canProvisionLogin out of member-row-menu into member-table as a prop. It saves one buildCan() per row, bounded by the page size (max 32) and each a trivial object construction, at the cost of prop plumbing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-lane ride-along Adversarial review found the first form of the trust guard was both unsafe and a production outage. Replaced, not patched. Trust gate: a cargo whose grants include Admin is honored only for an assigner holding the Admin ROLE; every other cargo is honored for an update:BoardSeat delegate too. The previous self-assignment form was wrong twice over: - it missed the two-write puppet loop (delegate creates a second member on a mailbox they control, seats IT on Presidente — not a self-assignment, so the perm was trusted — and that puppet is Admin forever), so revocation, the premise the whole delegation was accepted on, did not work; and - it stripped the SITTING PRESIDENT. seed-president.mjs stamps assignedBy with the president's own uid and an Admin's perms are manage:all, never the exact code — verified against the live production member doc, which is self-assigned on an Admin-granting cargo. Pinned by a regression test. provisionMemberLogin: a non-Admin may provision only a member with NO login yet. The resend path was the hole — passwordResetLink is generatePasswordResetLink, which returns the oobCode URL to the CALLER, so a delegate could pass the president's memberId and receive a live reset link for their address. createPositionsSafe: restrict to the current term key, mirroring the update arm. assignedBySelf() and cargoAssignableByNonAdmin() read only the current term, so a second term key rode along unvalidated — a next-term power cargo attributed to a real Admin, minted on the year rollover. Pre-existing, in the function this branch rewrites. Tests: retargeted three delegate pins that were silently denied by G3 rather than by the conjunct they named; added rules coverage for hasPerm-vs-canDo, the create-lane self-stamp and the create-lane non-vacuity pin. Every new guard mutation-tested: neutralizing each turns exactly its own test red and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…member /security-review found a third takeover path, and the strongest: the adoption guard asked whether this was a NEW login, never WHOSE member doc it was. Every uid-less member is reachable by memberId — including one an Admin already seated on an Admin-granting cargo, which is the normal state between being seated and being invited. The delegate forges nothing. linkUid fires onMemberWritten, resolveTrustedGrants reads the STORED assignedBy (a genuine Admin), honors the grants, and mints Admin onto the account the call just created; the attacker then reaches it through the invite. So the mint is refused at the source: a non-Admin provision is denied when the member's current-term cargo carries grants, and fails closed when that cargo cannot be read. Suppressing the returned link would NOT have been sufficient on its own — with manage:Member the attacker rewrites members.email first, which the rules never pin, and the ordinary reset mail lands in their inbox. The link is withheld from non-Admin callers anyway as defence in depth; the drawer's copy-link fallback is hidden when there is no link rather than offering to copy "". Both guards mutation-tested: neutralizing each turns exactly its own tests red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rgo read Verification pass found readCurrentCargoId treated a PRESENT but unreadable shape as "no cargo" and therefore ALLOWED a non-Admin provision — a non-object term entry, a non-string cargoId, or an empty one. That is the power-seat guard's own bypass, and it contradicted the docstring's fail-closed promise. Three outcomes now, explicitly: null for a genuinely absent cargo (allow), the id for a usable one, "" for present-but-unreadable (refuse, since "" fails isSafeDocId at the port too). readPositionGrants extracted: getPositionGrants and claims-sync's getPosition were the same isSafeDocId + read + isValidRole filter with different wrappers, and they must not drift — both decide whether a cargo confers power, one before minting claims and one before creating a login. Mutation-tested: restoring the pre-fix null returns turns the malformed-shape test red, and the paired "genuinely absent cargo is unseated" test keeps the guard from passing for a rule that just refuses every delegate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… comment
Delta re-review found the retargeted "past term" pin was still over-determined:
on an unseated fixture a lone 2099 write is denied by assignedBySelf() (the
post-merge current term is {}), so the term conjunct was not the denier and the
comment claiming otherwise was a guardrail #6 claim with nothing behind it. Now
paired with a valid current-term entry so only positionsDelta().hasOnly() denies.
Added the missing Admin pin on the create arm. The term restriction binds every
principal, and the tempting future edit is `hasAnyRole(['Admin']) ||` in front of
it the first time a migration looks blocked — which silently reopens the ride-along
for anyone who can get an Admin to run a create. Nothing went red on that mutation
before; now exactly this test does.
Also de-escaped HTML entities that leaked into a firestore.rules comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…grants Final security pass found the power-seat guard closed only one of the two claims-mint sources. syncMemberClaims mints `roles` from trusted cargo grants AND `perms` from roleIds + permissionOverrides via resolveMemberPerms — the second path needs no cargo at all, and it is precisely what the Admin-only roles panel writes, so "granted but not yet invited" is as ordinary a state as "seated but not yet invited". Exploit it closes: a manage:Member + create:MemberLogin holder finds an un-invited member an Admin granted a custom role or an override to, rewrites their email (the rules never pin it), provisions them, and the account they now control is minted that member's entire granted perm set — which may itself include update:BoardSeat, chaining into the seating lane. hasDirectGrants fails closed on any shape that is not a clean empty, while absent / null / [] stay ungranted: the rules' unchanged()/touched() gap admits an explicit null and parseMember resolves it to [], so treating null as malformed would make ordinary members un-invitable by a delegate. Mutation-tested both ways: dropping the guard and softening its fail-closed branches each turn exactly their own tests red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… one Final verification found the last gap: syncMemberClaims reads positions[currentTermKey()] at TRIGGER time, so a future-term power cargo was invisible to the guard and would mint on the UTC-year rollover — a genuine Admin in assignedBy, the cargo's grants honored, onto an account a delegate caused to exist and whose mailbox they control. Every client write lane is term-pinned (positionsDelta().hasOnly() on update, keys().hasOnly() on create, both binding Admins), so the shape needs a console edit, an admin-SDK write or a legacy migration — the same reachability this file already fail-closes on for a malformed cargoId, and a console-authored next-term board slate is the more plausible of the two. readCargoIds now yields every cargo in the map, deduped, with "" for any present-but-unreadable entry so the port's isSafeDocId still refuses it. Mutation-tested: restricting the loop back to the current term turns exactly the future-term test red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e delegate invite Functions re-review, both Mediums: M1 — readPositionGrants called .filter on snap.data().grants, which is a TypeError when that field is a string or a map. firestore.rules short-circuits every grants check on hasAnyRole(['Admin']) and never type-checks the field, so a console edit or a migration can store one. onMemberWritten is retry:false and the bad value persists, so the throw would kill claims sync for every member seated on that cargo, permanently and silently. Returns null now — fail-closed for both callers. The module had no test file; it has one. M2 — member-profile-page's InviteAccess still assumed an actionLink. For a delegate (who is withheld one) it opened a dialog over an empty code block with a copy button that copied nothing, having already created the account. It now sends the reset mail itself, exactly as the invite drawer does. Also: validateProvisionInput uses isSafeDocId rather than a weaker hand-rolled subset; createUser's catch is narrowed to auth/email-already-exists instead of swallowing quota and invalid-email errors; the post-createUser recovery comment now says a delegate's retry is refused; a duplicated comment fragment removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviews: 2c328f5 security-review,firestore-security-reviewer,firebase-functions-reviewer,code-review,simplify,react-best-practices,bundle-budget-watcher
…ever on themselves /code-review (the real skill, run by the user) found a HIGH the subagent passes missed, plus four stale-claim/dead-affordance issues. HIGH — update:BoardSeat was a self-service grant of every built-in role but Admin. The spec's own recommended pairing (update:Position + update:BoardSeat) could write a Secretario/ProjectManager cargo onto its OWN member doc through the positions-only lane — one write, no puppet — and claims-sync minted those roles onto the author. resolveTrustedGrants now requires the Admin ROLE for a self-assignment of any granting cargo, alongside the existing Admin-grant rule. The seeded president is unaffected (both halves defer to the Admin role, which he holds), and seating someone ELSE still works, which is the feature. Also: - firestore.rules' positions-only lane still claimed "CEL is NOT in that grant" and "the power-cargo restriction is NOT relaxed" — false since this branch lifted the new-side conjunct. boardSeatDelegate()'s comment still described the abandoned non-reflexive design, which would have led a reader to re-introduce the check that broke the president. - The invite drawer offered "Enviar acceso" on a cargo whose provisioning beacon always refuses: it created the member, 403'd, and pointed at a row action that failed identically forever. It now decides before writing anything and says so. - "Reenviar invitación/acceso" was shown to delegates, whom the adoption guard rejects on every click. Gated on isAdmin || !member.uid. - hasDirectGrants failed OPEN on an array permissionOverrides (typeof [] is "object"), contradicting its own fail-closed docblock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviews: 58266d3 security-review,firestore-security-reviewer,firebase-functions-reviewer,code-review,simplify,react-best-practices,bundle-budget-watcher
13 tasks
arkgast
added a commit
that referenced
this pull request
Aug 29, 2026
positionsLockedForEditor took `Position | undefined` and asked only whether it
confers power, so a held cargo whose id resolves to NOTHING read as "no cargo" and
unlocked the picker. The rules have no such gap: currentCargoGrantsEmpty() get()s the
real doc and a missing one errors the rule, which denies. So the client offered a full
picker and an enabled Guardar for a write firestore.rules always rejects — the
render-then-die shape this module exists to prevent, one input short.
Reachable without a console edit. The catalog is parseDocs(positionDocSchema, …),
which DROPS any doc failing the schema, so the very corruption that makes a cargo's
power unknowable is what removes it from the array the client searches.
`heldCargo(positions, cargoId)` is now the one way to build that input — it keeps the
id alongside the lookup, so "seated on something unreadable" and "not seated" stop
being the same value. It also replaces the `positions.find(...)` each form had typed
for itself. `cargoSlotsForEditor` and `draftProvisionBlocked` lose their truthiness
tests on the id for the same reason: "" is an id that resolves to nothing, not "no
cargo", and beacon's readCargoIds manufactures exactly that value so its own guard
refuses it. The provision-gate test enshrined that divergence as deliberate; the
reasoning it gave ("the draft schema cannot produce one") makes the case unreachable,
not the fail-open answer correct.
MemberForm's four authority props are REQUIRED now. They were optional with `= false`
defaults, which are not safe in the same direction: `isSelfAssignment = false`
suppresses the mint-pending warning, `allowReplacePowerCargo = false` locks an Admin's
picker, and a call site that forgot either compiled clean.
The BLOCKING test named for the #224 flag conflation could not fail on flag
conflation — one assertion duplicated an earlier test verbatim and the other was
decided by the cargo alone. It now pins that the two flags DISAGREE for the same
principal, which is the property collapsing them destroys. cargoTakedownOnly gets the
truth table it never had.
ui: MultiSelect takes aria-describedby, like Combobox. Both forms disable the
comisiones picker on the same `locked` flag, and only the cargo picker was explaining
itself — a screen-reader user met a dead control with no way to tell a permission
ceiling from a broken widget.
Mutation-tested: dropping the unresolvable clause turns exactly the two new BLOCKING
rows red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
arkgast
added a commit
that referenced
this pull request
Aug 29, 2026
…irroring buildCan
Two defects in this branch's headline deliverable.
It asserted `client offers => rules allow` and nothing else, which catches a rules
TIGHTENING and structurally cannot catch a LOOSENING. Delete `&& category != 'CEL'`
from cargoAssignableByNonAdmin(), or widen boardSeatDelegate() from
hasPerm('update:BoardSeat') to canDo('update','BoardSeat') so manage:all satisfies it,
and all 429 lines stayed green — including the row named "the delegation is live",
which reads as a rules property and asserts only about the client. The converse cannot
be asserted wholesale (the client is deliberately stricter about comisiones, retired
and inactive cargos, and flagging that curation would make the test an obstacle to
it), but it can be for the three cargos the delegation is ABOUT, where the gap is not
curation but the boundary. Both loosenings now turn those rows red — verified by
making each mutation against firestore.rules and re-running the suite.
And `gatesFor` hand-re-implemented buildCan's claims -> flags mapping: the exact
mirror class this file exists to delete, of the exact flag whose widening caused the
#224 regression. It would have kept agreeing with itself while use-can.ts drifted. The
derivation is now `capabilityFlags()`, split out of use-can.ts (React, so this package
cannot load it) and spread back into buildCan — one function, both sides.
Also corrects a comment that justified the local category union by claiming CargoLike
widens it to `string`. It does not; assignable-cargo-core declares PositionCategory
and documents at length why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
both surfaced as checkboxes in
/permisosand as chips in the per-member overrides panel:update:BoardSeat("Asientos de directiva") — seat a member on any vacant cargo, CELand power-granting alike.
create:MemberLogin("Acceso de miembros") — provision a new member's Auth login. Thisis cargo-agnostic: it applies to every new member, board seat or not.
manage:Memberalone the Cargo picker is empty in production. Every cargo in thecatalog carries grants, so
cargoAssignableByNonAdmin()filtered out all of them and thedelegation lane shipped in feat(rules,backstage,beacon,auth): position-assignment lane + role-lifecycle follow-through #222 was unreachable on real data — the picker just said
"Sin resultados" with no explanation. That empty state is now explained too.
Design and the full threat model:
docs/specs/board-seat-delegation.md.What the delegation deliberately does NOT reach
resolveTrustedGrantshonors anAdmin-granting cargo only for an assigner holding the Admin role.currentCargoGrantsEmpty()stays Admin-role-only. Therolesclaim derives exclusively from cargo grants, so displacing sitting Admins would strip every Admin claim in the chapter — unrecoverable outside the Firebase console./positionscatalogcanAssignPowerGrantswas split intocanAssignBoardSeat/canEditCargoCatalogprecisely because one flag drove both.roleIds/permissionOverridesgeneratePasswordResetLinkreturns a bearer credential to the caller.Security findings fixed during review
Five escalation paths were found by adversarial review and closed. None was part of the accepted
risk (which is only: a delegate may seat power-granting cargos).
members.emailis unpinned by the rules and has no uniqueness check,so a delegate could file a member doc carrying an Admin's email and have the callable adopt
that account, strip its claims, link its uid onto their own doc, and return its reset link.
memberIdreturned a live reset link for thatmember's address.
uid-less member was reachable, including one an Admin had already seated on an Admin cargo;
linkUidthen fired the trigger, which read the Admin's storedassignedByand minted Adminonto the account the delegate just caused to exist.
roleIds/permissionOverridesmintpermswith no cargo involved.update:BoardSeatlet its holder seat a Secretario/ProjectManager cargoon their own member doc and be minted those roles. Found by
/code-review; the fourabove were found by subagent passes.
next-term power cargo with a real Admin's
assignedByrode along unvalidated at create andminted on the year rollover.
A production outage was also caught before it shipped. The first form of the trust guard
keyed on self-assignment, which would have stripped the sitting president's Admin: the seeded
president self-stamps
assignedByand an Admin's perms aremanage:all, never the exact code.Verified against the live member doc (
Presidente,grants: ["Admin"],assignedBy= own uid).Keying on the grant instead fixes it with no owner-op and no bootstrap change.
Every guard is mutation-tested: neutralizing each turns exactly its own tests red and nothing
else.
Test plan
@luminova/types329backstage766 (eslint + tsc + build + vitest + knip + size-limit)beacon327 unit + 37 emulatortests/firestore-rules489main(160/162 kB budget), CSS unchanged,knipcleanpnpm pr-tests— fails only on the Firestore emulator port: 4010 is held by a runningdev emulator, which I did not kill. 11/11 non-emulator packages pass; both emulator suites
pass on a free port. Stop the dev emulator and re-run to get a clean single command.
Reviews
missed (self-promotion, feat(backstage): auth foundation + protected shell #5 above) plus four stale-claim / dead-affordance issues; all six
are fixed in
fix(beacon,rules,backstage): a delegate may confer power on others….Operator notes
update:BoardSeatconfers nothing alone — pair it withMembresía(manage:Member) orupdate:Position. Neither surface hints at the dependency.read
permsoff the ID token andauth-store.tsdoes not force-refresh.their doc.
🤖 Generated with Claude Code