fix(approvals): isOverrideActor resolves TENANT-admin standing from the ADR-0095 rung, never from a position name - #18252
Conversation
…rg built-in name still confers override Step 1 of the card: measure before deciding what to build. This harness resolves a principal through the REAL `resolveUserAuthzGrants` and drives three doors (`decideNode`, `recall`, `listRequests`) for a non-slate, non-submitter actor in the request's own organization. Committed RED on purpose, so the fix's before/after readings are taken from committed states rather than from a working tree. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
…r a position NAME `isOverrideActor`'s tenant arm ORed `positions.includes(org_owner)` and `positions.includes(org_admin)` onto the capability rung. `positions[]` carries ADR-0057 D4 assignment values alongside the ADR-0068 D2 projection, so a stored row spelling either name conferred tenant-admin override authority on approvals inside that organization — measured on three doors with the harness committed in the previous commit. The tenant rung is established from the resolver's own source rather than copied from the platform side: ADR-0095 D3's `derivePosture` resolves `TENANT_ADMIN` from `ORGANIZATION_ADMIN_GRANTS` and from nothing else, and the spec declares those grants that rung's source of truth — so the capability arm IS an authority read, and only the two name arms go. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
…-arm pin The new pin declares its own `update`/`delete` doubles through the engine's dispatch predicates, so `check:engine-double-contract` needs the ledger row or the file is never protected. Additive only: 2 rows added, 0 lost. Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 87c0341ede5395bafe9e64ed87358216798b2c89 && git checkout 87c0341ede5395bafe9e64ed87358216798b2c89
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cb648cb1b29f12bfbb44fae66510dbb9d2ca8b8d 783180eba89e942b9103830303c2475d48e5593e && git checkout -B drift-repro cb648cb1b29f12bfbb44fae66510dbb9d2ca8b8d && git merge --no-ff 783180eba89e942b9103830303c2475d48e5593e
node scripts/docs-audit/affected-docs.mjs --json cb648cb1b29f12bfbb44fae66510dbb9d2ca8b8d
|
Fixes #16166
Clause-②: no
ApprovalService.isOverrideActor's TENANT arm derived override authority from a built-in identity NAME oncurrent_user.positions— the half PR #16148 deliberately left when it closed the PLATFORM arm of the same predicate. This lands the tenant half the same way: read the rung, never the name.Step 1 was to DRIVE it — the three questions, in order, each with a reading
The card and the triage comment both required the mechanism be measured before anything was built, and required each step be reported rather than concluded from shape. It was, on the tree at
68fea8bc(the branch base).Q1 — can a
manageAssignments-only delegate mint an assignment row spellingorg_owner? Measured on both halves, in a throwaway harness (no permanent file: #15972's landed suite already pins this door).org_ownerandorg_adminalike — the vacuity fix(plugin-auth)!: session payloadpositions[]is the security axis, not the better-auth role scalar #15948 recorded, re-measured here for the org names rather than carried. Control: the same gate refuses when the scope does not carrymanageAssignments.plugin-security'sreserved_identity_positionobject validation on a real engine over a real SQL driver —VALIDATION_FAILED, for both names. Negative control: an ordinary position name (org_manager) still writes, so the object is not simply refusing everything.⇒ The minting route the card names is gated upstream. That is a FINDING, not a failure — and it is exactly the shape the triage comment reserved that word for. Two neighbouring routes were checked and are closed too:
sys_member.roleis a closed, write-enforced select whose HTTP surface is read-only, so the projection cannot be poisoned from there either.Q2 — does such a row, once it exists, move
isOverrideActor's verdict? Yes. Measured through the REALresolveUserAuthzGrantswith the actor resolved from stored rows, on three doors —decideNode,recall, and the console's participant-visibility read. All three admitted a non-slate, non-submitter actor whose resolved posture wasMEMBERand who held no org-administration capability at all. The pin's premise legs assert exactly that separation before any door is driven, so the result cannot be an artifact of the actor accidentally holding standing.⇒ Q1 and Q2 together are why the reader still has to be fixed. The write-side ruling refused NEW writes and explicitly declined a migration, so a row predating it resolves into
positions[]on every request; and a reader that trusts a name is not an invariant in any case, which is the whole reason the write door was built.Q3 — is any site already gated such that the name-read reaches only a misreport? No. The console read here is not an explain panel: it returns the rows, with
can_overrideset.The tenant rung, established from the resolver's own source — ⛔ not copied across
The platform arm's expression is deliberately not reused. ADR-0095 D3 resolves the rung in
derivePosture(packages/core/src/security/posture-ladder.ts) from held CAPABILITY grants:PLATFORM_ADMINfrom the unscopedadmin_full_accessgrant, andTENANT_ADMINfromORGANIZATION_ADMIN_GRANTS.some(n => permissions.includes(n))and from nothing else — the identical expression the predicate's second arm already spelled.packages/spec/src/identity/eval-user.zod.tsdeclares those two grants "the source of truth for theTENANT_ADMINposture rung".⇒ The question the card left open — whether the
ORGANIZATION_ADMIN_GRANTScapability arm is itself a tenant-authority rung or another weak arm — resolves to rung. It and the derivedpostureare one authority read in two spellings, kept apart only so a transport that never resolvedposturestill reads the held grant. Both survive.The two NAMES are the other thing entirely: ADR-0068 D2 declares them "a normalized PROJECTION into
current_user.positions" whose sources of truth live elsewhere, while the same array also carries ADR-0057 D4 assignment values. A projection is not an authority. Both name arms are removed, and with them the predicate's last read ofpositions[]on either rung.The whole predicate was read, not just the arm.
posture === 'TENANT_ADMIN'sat first in that OR and protected nothing — an OR is only as strong as its weakest arm.What does not change
The #3424 stuck-approval escape hatch is intact for anyone who actually holds org-admin standing: a genuine
organization_admingrant still overrides, still only within its own organization, and the decision is still audited asvia_override. Three CONTROL legs assert that on all three doors, and they were green before this change as well as after — they are the floor, not the result.Verification
Reverse verification, both legs taken from COMMITTED states. The pin was committed RED first (
bd157492c), then the fix (d40b9afd5):bd157492c(fix absent):Tests 10 failed | 6 passed (16)d40b9afd5(fix present):Tests 16 passed (16)The 6 that passed RED are the premise legs and the three controls — i.e. the harness was already discriminating before the fix, and the 10 failures are the escalation itself, not a broken harness.
Package suites (
@objectstack/plugin-approvals):pnpm test→Test Files 46 passed (46) · Tests 754 passed (754);pnpm typecheck→ exit 0, includingcheck:test-typecheck.Gate families — derived mechanically from the change set by
node scripts/pm/dispatch-gates.mjs, then reconciled with--rancarrying each recorded exit code: 73 derived · 70 run green · 3 NOT MEASURED · 0 UNRUN. The three arecheck:dual-build-cjs-loads,check:i18nandcheck:type-check-debt, each exit 3 = PREREQUISITE NOT MET (they read a full-repo build that this container does not hold). ⛔ Exit 3 is not a pass; those three are CI's.check:engine-double-contractfailed first (exit 1) because the new pin declares its ownupdate/deletedoubles and the ledger had not learned about the file. Regenerated with--write— additive only, 2 rows added, 0 lost — and the gate is green at783180eba.Repo-wide lint:
pnpm lint(eslint . --no-inline-config) run in full, exit 0, 91s, at783180eba. No narrowing was used, so no narrowing evidence is owed.Base freshness: branched from
68fea8bca; four commits have landed onmainsince and none touchesplugin-approvals,plugin-securityor the authz resolver, somainwas not merged in. The merge queue rebuilds this as merged onto currentmainanyway.Clause-②: no — derived from the DELIVERED diff, with controls
Derived by REACHABILITY FROM THE PUBLISHED ENTRY (the barrel's re-export list plus this package's
exports/files), ⛔ not from the wordexportand ⛔ not from a bundle grep. The package was built at the branch base and at HEAD and the twodist/index.d.tswere compared:private isOverrideActor;. That declaration line is byte-identical, and it carries no signature to widen — nothing was added, removed or narrowed on the published surface.ApprovalService: 43 occurrences in both builds, so the artifact really is the published surface and the reading discriminates.BUILTIN_IDENTITY_ORG_OWNER: 0 in both builds. The constants this diff stopped importing were never reachable from the published entry, so dropping the import moves nothing published.The behavioural direction is a NARROWING — an authority path removed — which is the opposite of the widening tell. The changeset is
patch, on@objectstack/plugin-approvals.The before/after measurement mutated the service file on disk and restored it; the restore is evidenced by
git hash-objectequalling the HEAD blob (bbae6c8f…) and by an emptygit diff HEAD, anddist/was rebuilt at HEAD afterwards so nothing is left standing at the base build.Acceptance notes
Two observations, noted and ⛔ not filed — neither is a reproducible defect, a contract violation, or a metadata-authoring trap:
plugin-security'ssys_invitation_org_adminRLS policy takes apositionsdomain on the same two built-in names. It is a different kind of read — a row-visibility WIDENING inside an already org-scoped select, with its rationale written out at the declaration (the domain only ever widens, so a principal it does not match fails closed). Its reach is bounded by Layer 0 and it grants no override authority. Successor: any future sweep of built-in-name reads.boundSets.every(...)is vacuous for a position that distributes no permission set. That is a real property, it is already recorded inplugin-security's own landed suite as the reason the gate cannot be where a reserved name is refused, and it is not this card's to change. Successor: A tenant can mint asys_user_positionrow spelling any built-in identity name — PR #15948 closed every reader, nothing stops the row #15972's write-side lane.⛔ Per the card's family this body carries no reproduction recipe; the driving harness lives in the test suite, which is where it belongs.
Authored by Claude Code in session https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj — recorded here in prose because this body's trailing footer block is the platform's to write.
Generated by Claude Code