fix(plugin-security): tell a read that did not answer from one that answered nothing, per site (#15840)

[os-trump]

Fixes #15840

Clause-②: no

Executes the ruling recorded at issue-15840#issuecomment-5595728039 — director seat, decision batch #105 item 5, option A: per-site separation of "read failed" from "read empty" at the two live seams. Not re-litigated; the options block earlier in the thread is superseded.

The p1 this closes

reconcileOrgAdminGrant's sys_member read went through tryFind, which mapped a thrown read onto []. [] is exactly what that function reads as "this user is not an admin of this organization" — shouldGrant === false — which is the input to the revoke branch, which deletes a standing grant. So a transient storage fault withdrew a sitting admin's org-admin capability, the store kept it withdrawn after the fault cleared, and only a debug line separated that run from a healthy one.

⚠️ A correction to the dispatch brief, measured. The brief (and the ruling's summary of it) states the faulted-membership leg answers {action: 'noop'}. It does not — that value belongs to a different leg, the one report 5553806224 measured, where the grant-table read faults and the revoke is missed. On the ruled leg the verdict is {action: 'revoked'} and a row is deleted. Both readings are in the thread; the ruling's own pointer :602 → :713 ff. names the wrongful-revoke leg, which is the one graded p1 by the regrade at 5552131360 ("this one writes"), and the one fixed here. Measured red-then-green below.

Per site — ⛔ not a uniform sweep

site	what changed
auto-org-admin-grant.ts — the sys_member read feeding the revoke branch	A read fault (thrown, or a non-array answer) reports at error and returns {action: 'skipped', reason: 'membership_unreadable'}. The revoke branch is never entered — asserted on the delete call count, not the outcome. A genuine empty read is byte-identical to today.
normalize-managed-by.ts:53	A read fault refuses the pass and reports at error. ⛔ It never answers "already canonical". A genuine empty read is byte-identical to today.
claim-seed-ownership.ts:177	⛔ UNTOUCHED — zero diff, asserted: git diff HEAD -- packages/plugins/plugin-security/src/claim-seed-ownership.ts is 0 bytes. The card's third row is falsified; its three inherited pins are kept verbatim as the proof.

tryFind keeps its a best-effort array-of-rows best-effort contract for every other read in the module. The fault is now available through a module-private readRows, and exactly one caller — the one that turns the value into a DELETE — asks for it. That is what makes this A and not C.

⭐ The positive controls, which are half of the fix

Without these the change is indistinguishable from "make everything refuse", i.e. option C — the one the maintainer refused. All pass:

• a genuine empty sys_member read still revokes ({action:'revoked'}, exactly one delete of sys_user_permission_set, zero error: lines);
• a genuine membership still grants ({action:'granted'}, zero error: lines);
• an already-canonical catalog still answers {positions: 0, permissionSets: 0} with zero lines at any level;
• an engine with no find/update is not a read fault and still returns zeros;
• ⛔ not-swept controls, kept verbatim from PR test(plugin-security): three read-fault readings for the seams #15598 fenced out — measurement only, no behaviour change #15998: a faulted grant-table read still reports noop at debug, and a faulted backfill sweep read still reports "backfill complete" at info. Both are missed revokes, not the wrongful one; see acceptance notes.

One stated consequence, pinned rather than left to be discovered

The ruling's disposition is "skips that user for the round" and "never enters the revoke branch", so the early return lands before the membership-independent superseded-revoke leg (1b). A round that could not read performs no write at all. Consequence: on a faulted round a superseded-variant grant lingers one round longer than it does today. It is pinned by its own test with a healthy control beside it. ⛔ It is not a relaxation of any boundary — nothing is granted, so no population widens; today's alternative on that same round is a wrongful revoke, which is not a baseline worth preserving; and the next round removes it. If a reviewer prefers the leg to run anyway, the change is the placement of one early return.

Surface — the no-widening declaration above, honoured

⛔ No exported symbol added, no key on a published payload, no packages/spec/src/** path, no error-code-ledger.zod.ts row.

• action: 'skipped' is already in the returned union; reason is already string, so 'membership_unreadable' is a value, not a widening. (Report 5553806224 measured reason has zero non-test readers — nothing breaks on a new value.)
• MaybeLogger and NormalizeOptions['logger'] are not exported; each gains an optional error? method — an input the module asks for, not a channel it publishes. Required by the ruling's own "reports at error". Every caller that compiles today still compiles.
• The refusal is a plain Error — no new class, no code, so no ledger row.
• The declaration was MEASURED, not inherited: action: 'skipped' already existed at six pre-existing sites (:614, :617, :653, :749, :793, :846), so no member was added to that closed union; reason is already an open string vocabulary (objectql_unavailable, missing_keys, permission_set_missing, insert_failed, delete_failed), so membership_unreadable is a seventh value in it and not a new key. The mechanical floor is a new key or a new exported symbol — neither happened. (check-widening-tells --declaration no exits 0 on this diff, but that is silence rather than corroboration: the tool's declared surfaces do not cover packages/plugins/**, [finding] check-widening-tells counts a file it structurally cannot judge as "read" — so exit 0 reads as corroboration for every published surface outside packages/spec #17112. The no rests on the key/symbol reading, not on the tool.)
• ⛔ No gate's population widened (The { records }-normalizer gate's population is app-showcase page modules, but all three instances found so far are outside it — #14460's stated evidence threshold is now met #15094). ⛔ Nothing relaxed: enforcement is unchanged in both directions, and the managed_by write gate recognises both vocabularies, so rows left un-normalised by a refusal are still gated.

Changeset — patch, argued

.changeset/plugin-security-read-fault-vs-empty.md. Same family and a strictly smaller blast radius than the sibling repair it descends from: PR #15838 made this exact fault-vs-empty change on the enforcement plane (SecurityPlugin's sys_permission_set loader, now refusing with DATABASE_ERROR) and shipped it as patch. These two are boot reconcilers, no type signature moves, and healthy-path behaviour is byte-identical — only the fault path changes. ⛔ Analysis does not say major / protocol:breaking.

Verification — union run at 411ac8d16, exit codes captured before any pipe

• pnpm --filter @objectstack/plugin-security exec vitest run → 105 files / 1977 tests passed, exit 0.
• pnpm --filter @objectstack/plugin-security run typecheck → exit 0; check:test-typecheck OK, 0 files / 0 errors. NOT-MEASURED trap closed: tsc -p tsconfig.test.json --listFiles shows all three edited files in the program.
• Repo-wide eslint . --no-inline-config --format json → 6433 files, 0 errors, 0 warnings, exit 0. Full population, so no narrowing to declare.
• Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on the final tree derives 90 families; 90 run, 0 UNRUN, reconciled with --ran. 87 exit 0. 3 exit 3 = PREREQUISITE NOT MET = NOT MEASURED, never a pass — check:dual-build-cjs-loads, check:i18n, check:type-check-debt, all three blocked on a whole-repo build this worktree does not carry. ⚠️ check:type-check-debt is not the The 4096 heavy-command prefix in os-dev.md makes check:type-check-debt OOM into exit 3 — the gate pins its own 6144 CI-shaped ceiling #17151 OOM: run at --max-old-space-size=6144 it cleared the heap and stopped on the built-closure prerequisite instead.
• The family grew 60 → 90 when the census re-tally below touched a docs page. All 30 new ones were run; two (check:docs, check:skill-examples) first refused on build prerequisites and were re-run green after building @objectstack/spec and @objectstack/client-react.

Red-then-green ablation — the pins bite

Both source files reverted to their pre-fix content on top of the same tree, mutated and measured in one shell (the inherited warning: a helper carrying its own EXIT trap restores before the measurement and produces a confident, wrong green).

• Mutation proven on disk, not by an editor's exit code: blob 1cea5386… → 49b0ac3d… and 9a6baa7a… → fa327193…, with anchors moving membership_unreadable 2→0, readRows 4→0, findOrRefuse 2→0.
• Result: 8 failed / 10 passed. All 8 failures are the [RULED] cases.
• ⭐ The 10 that stayed green on the PRE-FIX tree are exactly the positive controls and the not-swept controls. That is the direct measurement that this is option A: the pre-fix source already satisfies every healthy-path assertion, so the healthy paths did not move.
• ⚠️ P0, the premise, measured on that same red leg: with a qualifying admin membership present and only the sys_member read faulted, the pre-fix tree issued delete sys_user_permission_set where id = ups_standing — the standing grant of a sitting admin, deleted by a transient read fault. The call-count assertion is what catches it; the outcome alone reads as an ordinary {action:'revoked'}.
• Restore proven by blob equality back to 1cea5386… / 9a6baa7a… and a 0-byte git diff HEAD — never by an exit code.

Post-review: check:optional-error-sink (#9754)

Adding the error? member the ruling asks for put MaybeLogger into that gate's population for the first time, and it declared every channel optional — a contract that permits silence. Repaired with the gate's own stated remedy: warn loses its ?. ⛔ Not by making error REQUIRED (#9754 option C, falsified — hosts inject reduced sinks), ⛔ not by requiring info. Gate now exit 0; census moves 33 → 34 sinks with a guaranteed warn and "permit silence" 2 → 1, the remaining one being the pre-existing baseline entry. The sibling normalize-managed-by.ts sink was checked rather than assumed — its info/warn were already non-optional, so the gate never named it. Narrowing the type forced no call-site change: every existing caller already supplied warn, measured by tsc over both layers (typecheck exit 0, check:test-typecheck 0 files / 0 errors) with 105 files / 1977 tests still green.

Three follow-on commits worth reading

1. MaybeLogger.warn made non-optional — see the section directly above.
2. Logger.error arity. The canonical Logger contract (packages/spec/src/contracts/logger.ts) takes the Error in its own second argument — (message, error?, meta?) — unlike info/warn/debug. Declaring the sibling two-parameter shape made the real ctx.logger unassignable: three TS2322s at security-plugin.ts :3818, :3911, :3937. Fixed at the declaration, contract-first, rather than by widening the consumer.
3. check:system-context-census re-tally. readRows's context: { isSystem: true; tenantId?: string } is a 23rd isSystem declaration site, so the gate's [declared-count] clause went red. Repaired mechanically by the gate's own pnpm gen:system-context-census --fix — one number in content/docs/permissions/system-context.mdx, no prose, no anchors. ⚠️ Attribution was measured, not assumed: a first reading of the census JSON suggested the site was not mine; reverting only this branch's three source files on the same tree gives 22 and exit 0, restoring them gives 23 and exit 1. It is mine. gen:schema rewrote nothing tracked.

验收备注 (noted, not filed)

• Two missed-revoke legs in the same module, untouched by this ruling. A faulted sys_user_permission_set read (:723) and a faulted resolvePermissionSetIdsForName read both yield [] → {action:'noop'}, so a capability the platform decided to withdraw stays in force. That is the opposite direction from the p1 fixed here (a missed revoke, not a wrongful one), it is already measured and pinned by PR test(plugin-security): three read-fault readings for the seams #15598 fenced out — measurement only, no behaviour change #15998's cases which this PR deliberately leaves green, and the ruling names neither. Observation, not filed: it is documented in this card's own thread and filing it separately would duplicate that record.
• claim-seed-ownership.ts's non-array arm warns "the predicate matched no rows to page" while the envelope was in fact carrying the row — reported, but attributing a cause that did not happen. Unreached on the shipped engine (plugin-security: 7 { records } union-normalizer blocks on find() results — dead limbs, one of them also #15092's DROP shape (security-plugin.ts:1292) #15598), so trap-removal, not a live defect. The ruling says zero diff at this file; not touched.
• The dispatch brief's claim that "there is no code to inherit" is falsified in the useful direction: PR test(plugin-security): three read-fault readings for the seams #15598 fenced out — measurement only, no behaviour change #15998 merged, so its fault-injection harness (read-fault-distinguishability.test.ts, a real ObjectQL over a real SqlDriver with a one-verb Proxy) is on main and was reused rather than rebuilt. Only the branch is gone.
• Serial constraint checked: PR fix(security)!: an RLS predicate naming an undeclared column denies in every position #17115 is on security-plugin.ts / rls-compiler.ts and fix(plugin-security): stop letting org-admin row count decide whether a platform admin already exists #17116 on bootstrap-platform-admin.ts. This PR touches none of them — no conflict. .objectui-sha untouched.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 12 documentable anchor(s).

8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/deployment/environment-variables.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/deployment/tenancy-modes.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/permissions/authentication.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/permissions/authorization.mdx (via managed_by (literal, a string literal in findOrRefuse; a string literal in tryFind))
• content/docs/permissions/capabilities.mdx (via managed_by (literal, a string literal in findOrRefuse; a string literal in tryFind))
• content/docs/permissions/delegated-administration.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/permissions/permission-sets.mdx (via managed_by (literal, a string literal in findOrRefuse; a string literal in tryFind), sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/permissions/positions.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))

⛔ 7 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/implementation-status.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/releases/v12.mdx (via managed_by (literal, a string literal in findOrRefuse; a string literal in tryFind))
• content/docs/releases/v15.mdx (via managed_by (literal, a string literal in findOrRefuse; a string literal in tryFind))
• content/docs/releases/v16.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/releases/v17/17-0.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/releases/v17/17-1.mdx (via sys_member (literal, a string literal in reconcileOrgAdminGrant))
• content/docs/releases/v17/17-3.mdx (via tryFind (symbol, a top-level function), sys_member (literal, a string literal in reconcileOrgAdminGrant))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 2 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df8a16deeee88bc0609f0230263d703d0fb885a9 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 72339ffc8d4f418708179e1bffdc43c1a831e37e — the merge of head 4f096ae89d55bbe961cef158b319e4771943c871 into base df8a16deeee88bc0609f0230263d703d0fb885a9, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 72339ffc8d4f418708179e1bffdc43c1a831e37e && git checkout 72339ffc8d4f418708179e1bffdc43c1a831e37e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df8a16deeee88bc0609f0230263d703d0fb885a9 4f096ae89d55bbe961cef158b319e4771943c871 && git checkout -B drift-repro df8a16deeee88bc0609f0230263d703d0fb885a9 && git merge --no-ff 4f096ae89d55bbe961cef158b319e4771943c871

node scripts/docs-audit/affected-docs.mjs --json df8a16deeee88bc0609f0230263d703d0fb885a9

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df8a16deeee88bc0609f0230263d703d0fb885a9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.