fix(plugin-security): propagate engine faults from readRowById instead of flattening to null

[os-help]

Fixes #7505

What was wrong

SecurityPlugin's shared by-id probe, readRowById, answered null for three different facts — the row does not exist, the engine threw (driver down, table missing, timeout), and no engine is wired — and every gate that probes with it read all three as "no such row". Its own contract note claimed a null "always DENIES downstream". That was true of one caller and false of the other three, in two opposite directions:

caller	steady state	under a store fault (before)
assertControlledByParentWrite	404 RECORD_NOT_FOUND for an absent row	404 — terminal to an SDK, when the truthful answer was a transient fault to back off on
assertPackageManagedWriteGate (ADR-0086)	403 on a package-managed row	admitted — fail-OPEN
assertSystemRowWriteGate (ADR-0066)	403 on a platform-provided row	admitted — fail-OPEN
getCallerPreImage (owner-anchor echo)	403 on a real transfer	403 accusing the caller of an ownership grab they never attempted

The two fail-open cases had no middleware-level coverage at all, which is why they went unmeasured. Both provenance gates leaked the same way on their bulk-filter branch too, one if above the by-id one, via a sibling .catch(() => null).

The ruling this implements

Maintainer ruling recorded on the issue 2026-08-11T08:08Z:

Ruling: fail-closed. The probe distinguishes "row absent" from "could not be read"; an engine fault propagates instead of flattening to null. Gates answer fail-closed on a fault (refusal, or ERR_DATASOURCE_UNAVAILABLE/503 where surfacing the outage is the right answer) — never 404 for an outage, which teaches SDKs to treat a transient fault as a terminal absent-row.

What changed

An engine fault now propagates out of the probe and out of the gate. The write is still refused — the throw happens before next(), so nothing reaches the driver — and the error is re-thrown exactly as the engine threw it rather than re-badged. objectql's DatasourceUnavailableError keeps its ERR_DATASOURCE_UNAVAILABLE code and mapDataError turns that into 503, which is the answer a client can act on.

Re-badging was considered and rejected on two grounds: it would relabel a dependency outage as an authorization event, and it would register a second spelling of an existing ADR-0112 ledger entry under a package that does not own it. plugin-security is not the producer of this condition and has nothing to add to it — so this PR adds no new error class, no new code, and no ledger entry.

null from the probe now means one thing: the row is genuinely absent.

Four behaviour changes, all inside readRowById and its direct callers:

1. readRowById no longer catches. The third collapsed fact — no engine wired — throws rather than reading as absence; it is unreachable in a real deployment, because start() registers no security middleware at all without a query engine.
2. The owner-anchor echo's catch { unchanged = false } is removed. It predates the probe distinguishing the two facts: with both arriving as null it was the only fail-closed answer available. The oracle it protected is untouched — an absent row and a row the caller cannot read both still return null and still deny.
3. • 4. Both provenance gates' bulk-filter .catch(() => null) removed, so one gate cannot answer two ways for one outage.

Deliberately unchanged: the master-visibility probe inside the same controlled-by-parent gate still treats a throw as "not visible" and answers 403. The two probes ask different questions — "does this row exist", which an outage leaves unanswered and which must not be answered "no", versus "is this master visible to you under your own write policy", whose fail-closed default genuinely is "not visible". The issue and the ruling both name that probe as the house posture to match, not a site to change. Pinned as a decision rather than left to read as an oversight.

Tests

17 new cases across two files, pinning both directions per caller — the steady-state answer and the fault answer, in the same describe, so "the fault propagates" cannot be satisfied by a probe that throws on everything.

• controlled-by-parent-sharing.test.ts — 5 cases beside the assertControlledByParentWrite answers a metadata defect and a missing row with the same 403 PERMISSION_DENIED "requires edit access to its master record" #7474 legs they correct, including a code-less engine error (a timeout arrives as a bare Error) and the deliberate 403 on the master probe.
• store-fault-fail-closed.test.ts (new) — 12 cases for the two provenance gates (by-id and bulk) and the owner-anchor echo, plus a blast-radius pin that an ordinary field-only update is unaffected by a faulting store.

The store double is modelled field-for-field on the real DatasourceUnavailableError: code, name, datasource, and no status, because that class declares none and rest routes it off the code. Giving the double a status its producer never sets would let these cases pass against an error no engine can throw.

Reverse verification (ablation). All four behaviour changes reverted:

× FAULT (by id): the outage propagates — it does NOT read as "no package row here"
× FAULT (bulk filter): the same answer — one gate cannot answer two ways for one outage
× FAULT (by id): the outage propagates instead of admitting the delete
× FAULT (bulk filter): the outage propagates there too
× FAULT: the outage propagates instead of being read as "you are not the owner"
× the fault is NOT memoized as a verdict — a retry re-probes the store
× the detail-row probe faulting propagates ERR_DATASOURCE_UNAVAILABLE, not 404
× an engine error with NO code still never becomes a 404
Tests  8 failed | 36 passed (44)

Exactly the eight fault-path cases flip; all 36 steady-state cases stay green — which is the shape that makes them a pin rather than a restatement. The headline assertion fails as expected 'RECORD_NOT_FOUND' to be 'ERR_DATASOURCE_UNAVAILABLE', i.e. the reverted code reproduces the reported defect verbatim.

Suites run (all green, on top of origin/main merged in):

package	result
@objectstack/plugin-security	47 files, 969 tests
@objectstack/runtime	127 files, 2016 tests
@objectstack/plugin-approvals	21 files, 458 tests
@objectstack/http-conformance	4 files, 72 tests

Consumer sweep used the prefix filter (...@objectstack/plugin-security — downstream consumers, not upstream deps). Also green: typecheck, eslint on the changed files, check:nul-bytes, check:engine-double-contract (the new double declares reads only, so it is out of that gate's scope for the same reason the sibling file's is), check:error-code-casing.

In-flight neighbor: #7626

Same file, and no conflict — verified rather than assumed. git merge-tree against claude/issue-7626-expand-crud-bypass auto-merges security-plugin.ts cleanly: their hunks are lines 1194-1310, mine are 1807+, 3525+, 3631+, 3678+, 4137+ and 4196+ in origin/main coordinates.

No behavioural interaction either. Their expandSkipCrud region is gated on opCtx.operation === 'find'; every readRowById caller is on a write path (insert/update/delete/transfer/restore/purge, or the insert/update-only owner-anchor and RLS-check steps), so no operation can be in both regions at once. Their landed patch deletes the waiver outright and does not touch the isPrivate derivation — their only other change to shared vocabulary, in packages/core/src/security/operation-private-keys.ts, is comment-only.

That trial merge does surface one conflict, in packages/objectql/src/engine.ts — a file this PR does not touch. It is between their branch and #7594 (245d1dc), which landed on main after their base. Theirs to resolve, flagged here only so it is not misread as coming from this card.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 11, 2026 10:49am

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/deployment/cli.mdx (via @objectstack/plugin-security)
• content/docs/kernel/runtime-services/sharing-service.mdx (via @objectstack/plugin-security)
• content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-security)
• content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
• content/docs/permissions/authorization.mdx (via @objectstack/plugin-security)
• content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
• content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security)
• content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security)
• content/docs/plugins/index.mdx (via @objectstack/plugin-security)
• content/docs/plugins/packages.mdx (via @objectstack/plugin-security)
• content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
• content/docs/ui/dashboards.mdx (via @objectstack/plugin-security)

⛔ 1 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/implementation-status.mdx (via @objectstack/plugin-security)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.