Skip to content

fix(plugin-security): explain fails closed when a dependency it shares with enforcement throws - #20030

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-20002-explain-sharing-fault
Sep 24, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-20002-explain-sharing-fault

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20002

Clause-②: no

The security/explain route promises that it runs 「the same code paths the enforcement middleware runs」. Enforcement does not catch a failure in those calls, so the request fails. The explain engine caught the same failure and turned it into a value that its record matcher reads as an answer. So a request that fails was reported as one that succeeds. This is the same defect class as #19963 and #19986 (PR #19984, PR #20000): explain and enforcement give two different answers. Enforcement is not touched. This PR changes the report only.

Measurement first (on origin/main fc6ddb87a4, before any fix)

The stack is the real SecurityPlugin + SharingService + both middlewares over one in-memory engine, in the shape of PR #20000's test file. For each caught fall-back in explain-engine.ts, the dependency was made to fail, and the same request was run through both middlewares. Commit 02067686d4 adds the test file that pins the fix. On the unfixed engine its 10 fault cells are red and its 6 controls are green. The commit message carries this table.

Site Caught into What explain reported when the dependency failed What enforcement did Verdict
:848 fetchRecord null Record not found: visible: false, no decidedBy The find throws on the same store fault Fails toward not visible. Left as is. The plugin's own binding already catches to null, so in this wiring this catch is never reached.
:858 computeLayeredRlsFilter { layer0: null, layer1: null } allowed: false (the object-level catch denies), but record.visible: true for a shared row, an owned row and an org-depth row The find throws the injected error Fails OPEN at row level, and contradicts allowed. Changed.
:941 listRecordShares [] Verdict unchanged, because the read filter decides it. rules[] is empty Unaffected: enforcement never calls listShares Only the attribution changes: it can only drop an admits rule. Left as is.
:958 sharingReadFilter (the card) null Share store down, own-depth reader: visible: true on the unshared, shared and owned rows. Sharing is admitted with rowFilter: null The find throws the store error Fails OPEN. Changed.
:966 write gate (canEdit / canDelete) undefined ("no gate wired") Update on an owned row: visible: true (private and public_read). On a row with only a READ share, sharing reported admitted The by-id PATCH throws the injected error Fails OPEN. Changed.
:1128 resolveSets [] allowed: false, decidedBy: object_crud 403 PERMISSION_DENIED Fails toward deny. Left as is.
:1140 resolveDelegatorContext (not on the dispatch list, found by the same sweep) { kind: 'none' } ("no delegation") Delegator's grant store down: allowed: true, principal neutral. The D10 intersection was dropped (a healthy delegator gives allowed: false) 503 SERVICE_UNAVAILABLE Fails OPEN (object-level allowed). Changed.
:1147 delegator resolveSets [] allowed: false, decidedBy: object_crud The find throws Fails toward deny. Left as is.

Two storage-level controls from the same probe:

  • With the share store down, an org-depth reader's read filter answers null before it reads a share, so both sides admit.
  • The per-record gate catches its own store faults (writeGateFailClosed), so an update during the same outage already got the same answer from both sides.

What changed

In packages/plugins/plugin-security/src/explain-engine.ts:

  • The helper. A settle helper keeps a failure apart from every value the call can return: it returns a DEPENDENCY_FAULT sentinel instead. It is used only at the four fail-open sites.
  • Sharing read filter and write gate. The sharing layer's record outcome is not_evaluated, with no rowFilter and no matchesRecord. Its detail says the layer could not be evaluated and that the request fails on the same call. record.visible is false, with decidedBy: 'sharing'.
    • The call that is checked is the one the operation's verdict depends on: the read filter for a read, and the per-record gate for a write.
    • It is checked before the OWD, because the sharing middleware calls it whatever the OWD is.
  • Layered RLS composition. The tenant_isolation and rls record attributions are not_evaluated, with a detail that names the failure. record.visible is false, with decidedBy: 'rls'. This matches the object-level rls layer, which already reports the same failure as a denial.
  • Delegator resolution. A new delegatorUnresolved flag fails closed like a missing delegator. principal and object_crud deny with their own wording, and allowed is false.
  • Order in the record verdict. Unchanged first: capability, CRUD, missing record, tenant exclusion, RLS exclusion. Then an RLS-composition failure, then a sharing failure. This follows the pipeline, where the RLS composition runs before the sharing middleware.

Outcome vocabulary. ExplainRecordAttributionSchema.outcome in packages/spec/src/security/explain.zod.ts is admitted | excluded | not_evaluated. The engine already uses not_evaluated for "Tenant layer split is unavailable on this engine build" and for a record that is not found. So not_evaluated plus a detail that names the failure says "could not be evaluated". The response has no new value and no new key, and packages/spec is not edited.

Not changed:

Tests

The new file packages/plugins/plugin-security/src/explain-dependency-fault.test.ts uses the real SecurityPlugin + SharingService + both middlewares over one in-memory engine. Each fault cell asserts both sides:

  • Explain: record.visible === false with the named decidedBy, and the layer is not_evaluated with no rowFilter and no matchesRecord.
  • Enforcement: the same request through both real middlewares fails with the injected fault itself, compared by identity. For the delegator, the cell asserts the ADR-0112 envelope: SERVICE_UNAVAILABLE / 503.

The fault cells:

  • Share store down × unshared / shared / owned row (read), and buildReadFilter failing on a public_read object.
  • canEdit failing × owned row (private and public_read).
  • computeLayeredRlsFilter failing × read-shared / owned / org-depth row. These cells also assert allowed: false.
  • Delegator's grant store down (sys_user_position): allowed: false, and principal and object_crud deny.

The controls:

  • A healthy shared row: admitted, decidedBy: sharing.
  • A healthy unshared row: excluded.
  • An org-depth reader whose filter answers null while the share store is down: still admitted, because only a failure is a fault.
  • A healthy update gate, a healthy tenant wall, and a healthy delegator (principal neutral).

Ablation of every negative pin, on HEAD e8e677786a. Each leg put one swallowing catch back with scripts/ablation-replace.mjs. The anchor went from 1 hit to 0, and the file's blob changed on disk. The test file then went red on exactly that site's cells. Each leg restored the file: blob fca6431074ac equals HEAD, and git diff HEAD is empty.

Leg Catch put back Red Green
A1 :958 .catch(() => null) 4, the read-filter cells (record.visible: expected true to be false) 12
A2 :966 .catch(() => undefined) 2, the write-gate cells 14
A3 :858 .catch(() => ({ layer0: null, layer1: null })) 3, the layered-RLS cells 13
A4 :1140 .catch(() => ({ kind: 'none' })) 1 (allowed: expected true to be false) 15

The test reaches the code under test through relative src imports (./security-plugin.js → ./explain-engine.js), so no dist sits between the mutation and the run.

Runs on HEAD e8e677786a, after merging origin/main at 44639665ee:

  • @objectstack/plugin-security: typecheck exits 0, and tsconfig.test.json lists the new file. The full suite passes: 130 files, 2541 tests.
  • Consumers of the explain route, found with grep -rln "security/explain\|explainAccess" packages --include=*.test.ts:
    • rest: security-routes, security-explain-envelope, rest-write-response-internal-fields.tripwire: 45/45.
    • plugin-sharing: sharing-service: 131/131.
    • client: client.test: 217/217.
    • dogfood: api-key-owner-revoke, owd-public-read-write-write-floor, showcase-d7-default-profile: 23/23.
    • spec: type-alias-convention.pin, explain-zero-rows-sentinels.pin: 11/11.
    • objectql: engine-middleware-operation-vocabulary: 5/5.
  • Gates. node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands on this HEAD derives 71 commands, and all 71 ran.
    • check-changeset-fixed, check:error-code-casing and check:filter-alias-parity also ran, because their rosters sit under paths this diff touches.
    • Reconciliation, with every line recorded with its exit code: Run reconciliation — 71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN.
    • check:type-check-debt first answered PREREQUISITE NOT MET, because the plugin-security dist was older than its sources. After a rebuild it answered OK — 4 ledger entr(ies) re-measured … none above its recorded number.
  • Lint, narrowed to the changed files. eslint --no-inline-config --format json on the two changed TS files: 2 files, 0 errors, 0 warnings.
    • The changeset and the JSON ledger are outside eslint's configured file set ("File ignored because no matching configuration was supplied").
    • The config never enables type-aware linting (no parserOptions.project), so this diff cannot change the lint result of any other file.

Also in the diff

  • scripts/engine-double-contract.pinned.json: one row added (explain-dependency-fault.test.ts, findOne), regenerated with --write. check:engine-double-contract (RETAINED) needs the coverage ledger to record every new pinned double. No row was lost.
  • .changeset/20002-explain-sharing-fault.md: @objectstack/plugin-security: patch, Clause-②: no. The report now matches what enforcement already does, and no accept set moves.

Acceptance notes

  • No log line is added at the four sites. The suggested route said to log the failure the way enforcement does, but I did not, for two reasons:
    • The explain engine has no logger dependency, and wiring one would mean editing security-plugin.ts, which is out of scope here.
    • AGENTS.md "Degradation log levels" says that a failure delivered to the caller is not a degradation. The report delivers it: not_evaluated, a detail, and a fail-closed verdict. The real request's failure shows up on the request that fails.
  • The detail names the failure, not the thrown message. Explaining your own access needs no capability, so a raw store error message would reach an ordinary caller.
  • :848 and :941 word a failure imprecisely. They still say "Record not found" or "0 share(s) attached". Their verdicts fail toward not visible, so they are left as is.
  • The tenant_isolation layer-level verdict stays not_applicable under a layered-RLS failure. That code is unchanged, and the verdict enum has no "unknown". The record attribution carries the failure.
  • A spec describe text does not mention failures. ExplainRecordAttributionSchema.outcome's describe text reads "not_evaluated (skipped/not row-scoped)". The engine already uses the value for "unavailable", so this is a docs detail in packages/spec, and it is not edited here.

Generated by Claude Code

… rejects

Measurement first, on origin/main fc6ddb8, before any fix. The new file
wires the real SecurityPlugin, SharingService and both middlewares over one
in-memory engine; its 10 fault cells are RED here and its 6 controls green.

What explain reported when each caught dependency rejected, against what the
same request did through the real middlewares (throwaway probe, same stack):

  :848  fetchRecord -> null. Record reported not found, visible false; the
        find throws on the same store fault. Fails toward not-visible.
        Leave. (The plugin's own binding already catches to null.)
  :858  computeLayeredRlsFilter -> {layer0: null, layer1: null}. allowed
        false (the object-level catch denies) but record.visible TRUE for a
        shared, an owned and an org-depth row; the find throws. FAIL OPEN.
  :941  listRecordShares -> []. Attribution only: the read filter still
        decides; enforcement never calls it. Can only drop an admits rule.
        Leave.
  :958  sharingReadFilter -> null (the card). Share store down, own-depth
        reader: visible TRUE on unshared, shared and owned rows, sharing
        admitted with rowFilter null; the find throws. FAIL OPEN.
  :966  writeGate -> undefined. canEdit rejects: update on an owned row
        visible TRUE (private and public_read); the PATCH throws. FAIL OPEN.
  :1128 resolveSets -> []. allowed false, decidedBy object_crud; the find
        answers 403. Fails toward deny. Leave.
  :1140 resolveDelegatorContext -> {kind: none}. Delegator grant store
        down: allowed TRUE, principal neutral, the D10 intersection
        dropped; the find answers 503 SERVICE_UNAVAILABLE. FAIL OPEN.
  :1147 delegator resolveSets -> []. allowed false; the find throws.
        Fails toward deny. Leave.

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
…hrows

security/explain calls the functions the enforcement middleware calls, and
enforcement runs them un-caught: when one rejects, the request fails. The
engine caught the same rejection into a value its matcher reads as an
answer, so a request that fails was explained as one that succeeds.

A `settle` helper now keeps a rejection apart from every value the call
could resolve to, at the four sites where the fold widened the report:

- the sharing read filter (null read as "no filter");
- the sharing per-record update/delete gate ("no gate wired");
- the layered RLS composition ("no tenant wall, no business RLS");
- the on-behalf-of delegator resolution ("no delegation").

Each reports its layer not_evaluated with no rowFilter and no
matchesRecord, the record not visible (decidedBy sharing / rls), and for
the delegator a principal and object_crud denial. No new response keys;
not_evaluated is an existing outcome value. The four fall-backs that
already failed toward deny are left as they were.

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
…gine double

check:engine-double-contract (RETAINED) asks for the coverage ledger to learn
the new pinned double; regenerated with --write, one row added, none lost.

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
check:objectql-double-limit could not judge a find that threw from its own
body. The outage now lives in the table (its filter throws), so find is an
ordinary double that applies limit by presence, after the filter.

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
Brings in the plugin-security / objectql by-id update change that landed
on main, before the PR opens (AGENTS.md multi-agent section 10).

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

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 26550c6603a920c28f04ca94b039388069ecbaad → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 67fc7fe2eecaf1ec6df85c828af4e8703cf53910 — the merge of head e8e677786ae7f15ecf64426657874d962f528a2d into base 26550c6603a920c28f04ca94b039388069ecbaad, 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 67fc7fe2eecaf1ec6df85c828af4e8703cf53910 && git checkout 67fc7fe2eecaf1ec6df85c828af4e8703cf53910
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 26550c6603a920c28f04ca94b039388069ecbaad e8e677786ae7f15ecf64426657874d962f528a2d && git checkout -B drift-repro 26550c6603a920c28f04ca94b039388069ecbaad && git merge --no-ff e8e677786ae7f15ecf64426657874d962f528a2d

node scripts/docs-audit/affected-docs.mjs --json 26550c6603a920c28f04ca94b039388069ecbaad

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 24, 2026
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 24, 2026 21:28
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit 8e9a425 Sep 24, 2026
35 of 36 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-20002-explain-sharing-fault branch September 24, 2026 22:01
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…w a write stores (objectstack-ai#20013) (objectstack-ai#20043)

Fixes objectstack-ai#20013
Clause-②: no (narrowing)

## What this fixes

Step 3.7 of the security middleware is the Layer 0 tenant write wall
(ADR-0095 D1, ADR-0105 D5). Its contract,
`packages/plugins/plugin-security/src/security-plugin.ts` lines
3239-3248 on `origin/main` `246314dffe`:

> Both close identically here: a SUPPLIED (non-empty) `organization_id`
in the write payload must satisfy the SAME Layer 0 filter the read side
uses (isolation active, tenant object, platform-admin posture exemption,
fail-closed on a missing active org). For UPDATE this makes
`organization_id` effectively immutable in non-platform user contexts:
the only value that passes is the caller's active org (which — since the
pre-image already scoped the target to that org — equals the row's
current org), so a re-point to any OTHER tenant is denied. A bulk update
carrying a cross-tenant `organization_id` change-set is caught too (the
check inspects the change-set value, not a per-row post-image).

The wall judged `opCtx.data`, the payload as the caller sent it, before
`next()` runs the engine's `beforeInsert` / `beforeUpdate` chain. A
value a hook wrote into `organization_id` was never judged by the wall,
so the row was stored in whatever organization the hook named. The
engine records the insert twin of the same gap at
`packages/objectql/src/engine.ts` lines 11846-11847 on `246314dffe`:
「The Layer 0 tenant wall still judges the PRE-hook image — filed
separately, and the fix's host is this seam.」

The wall now also judges the row the engine is about to store, through
the seam the row-level `check` already uses
(`OperationContext.postHookWriteImageCheck`). The refusal is the wall's
existing one: `PERMISSION_DENIED` / 403, nothing stored. The judgement
of the payload as sent stays, so this change only ever refuses more. No
engine change.

## Mechanism hypotheses (dispatch Section 2), measured

Base: `origin/main` `26550c6603`. Real `SecurityPlugin` and `ObjectQL`,
`isolated` posture, driver-sql (better-sqlite3) and driver-sqlite-wasm.
The fixture is a tenant object whose `beforeInsert` and `beforeUpdate`
hook copies another payload field into `organization_id`, and a caller
whose active organization is `org_a`.

1. **Held.** Step 3.7 judged only rows whose `organization_id` was
present in `opCtx.data` (the `suppliedRows` filter), before `next()`.
2. **Reproduced on both drivers, all three legs** (identical on both):

| leg | outcome on the base | stored `organization_id` |
|:--|:--|:--|
| control: by-id update supplies `org_b` | refused, `PERMISSION_DENIED`
/ 403 ("the update would place 'qa_account' in another tenant") |
`org_a`, unchanged |
| control: insert supplies `org_b` | refused, `PERMISSION_DENIED` / 403
| no row |
| by-id update, hook writes `org_b` | **admitted** | **`org_b`** |
| insert, hook writes `org_b` | **admitted** | **`org_b`** |
| array insert, hook writes `org_b` on the second row | **admitted** |
first row `org_a`, second **`org_b`** |
| predicate update (`multi: true`), hook writes `org_b` | **admitted** |
**`org_b`** |
| control: by-id update, hook writes `org_a` | admitted | `org_a` |

3. **The seam is installed only where a business `check` applies**,
measured: with no row-level policy on the object, an inner middleware
saw `postHookWriteImageCheck` absent on every leg above. So the wall
gets its own installation condition: a walled posture, a tenant object,
a caller Layer 0 does not exempt (that is,
`computeWriteTenantCheckFilter` returns a filter). When step 3.6 also
installed its judgement, the two are composed into the one handle the
engine runs, in the order the middleware judges in (the `check`, then
the wall).

**A second finding shaped the fail-closed guard.** The post-`next()`
guard now covers the new installation. The ADR-0094 permission-set data
door executes an insert or update of `sys_permission_set` itself,
through the metadata protocol, and never calls `next()`. Measured on the
base with a platform administrator (active organization `org_a`) under
`isolated`: Layer 0 walls the object (`organization_id = org_a`), the
insert is admitted, and the engine's write never runs. A guard-covered
wall seam alone would turn that into a 403. No engine write runs there,
so no hook chain runs, and the payload judgement has already cleared the
only `organization_id` such a write can carry. The guard therefore
stands down for a seam that carries the wall alone on a write the door
executed itself. The fact is observed by a wrapper around the door's
registration, which records a write the door never passed to `next()` in
a plugin-private `WeakSet`. No operation field is involved that another
middleware could set. A seam carrying a row-level `check` is not stood
down: the data door keeps failing closed under one, exactly as before.

**ADR-0095 D1 "Not touched"** records that D1 added no tenant post-image
check to `computeWriteCheckFilter`. Its reason is that such a check
"would risk denying legitimate inserts before the auto-stamp runs". This
change does not touch `computeWriteCheckFilter`, and it judges an image
only when the image names an organization. An absent value (the
auto-stamp's to fill) is never judged, and a control pins that. ADR-0105
D5 affirms the direction: an explicit value is validated against the
membership set or equality.

## What changed

- `packages/plugins/plugin-security/src/security-plugin.ts`
- Step 3.7 computes the wall when a payload names an organization (as
before) or the posture walls. A `single` posture pays for nothing new.
- One refusal (`denyTenantPlacement`) serves both halves. The step
installs the stored-row judgement whenever the wall applies (insert, or
a non-array update, as step 3.6 scopes it), composed after step 3.6's
judgement when one is installed.
- The post-`next()` guard covers the new installation with the data-door
stand-down above. Its developer message names what was not evaluated:
the business-check sentence is unchanged byte for byte, and a wall-only
seam gets its own sentence.
  - The data door is registered through the observing wrapper.
-
`packages/plugins/plugin-security/src/tenant-wall-post-hook-image.test.ts`
(new): 34 cells, 17 per driver.
- 11 existing plugin-security test files: engine doubles (see "Surface
beyond the claim").
- `.changeset/20013-tenant-wall-post-hook.md`:
`@objectstack/plugin-security` minor, BREAKING, remedy, `not-required
(no-migration-prescription)`.

## Tests

New file, real `SecurityPlugin` and `ObjectQL` on both SQL drivers,
`isolated` posture unless a cell says otherwise. Every refusal asserts
`code` `PERMISSION_DENIED`, `status` 403 and the wall's message ("the
insert/update would place 'OBJECT' in another tenant"), then reads the
table straight off the driver.

- **Negative cells:**
- a hook-written out-of-scope organization, refused on four paths: a
by-id update, an insert, an array insert (the whole write refused) and a
predicate update;
- the same with a business `check` installed and passing (one composed
seam);
- the composed seam still runs the `check` (an in-scope insert the check
refuses is refused);
- `group` posture: outside the membership set refused, inside admitted;
- fail-closed: a host that strips the installed wall-only seam is
refused, with the guard's wall sentence.
- **Controls:**
  - an in-scope hook write is admitted on all three paths;
- a supplied out-of-scope value is refused before the hook chain, as
before;
- only refuses more: a supplied out-of-scope value stays refused when a
hook would replace it with an in-scope one;
- an update that does not touch the column is admitted on both update
paths;
- an insert that leaves the column absent is admitted and lands in
`org_a`;
  - a platform administrator on a `private` object is exempt;
  - a system-context write is ungated;
  - the `single` posture is unchanged;
- the data door under `isolated`: admitted, and the engine's write never
ran.

**Failing first.** The file was committed before the fix (`8afaec51a1`).
On the base it gave 14 red (the 7 negative cells that existed then, × 2)
and 18 green. Every red read "expected a refusal, got a completed
write".

**Ablations**, each through `scripts/ablation-replace.mjs` in WRAP mode,
with the anchor hit 1 → 0 and a blob change reported by the tool. Each
ran under an outer `trap` restoring from `HEAD`, and was then proven
restored (blob == HEAD `278b25f19e`, empty `git diff HEAD`). They ran on
`e43cda19b4`, whose blobs for `security-plugin.ts` and the pin file
equal the final head's. The subject is imported relatively by the test
file and `@objectstack/objectql` is aliased to `src/` in this package's
`vitest.config.ts`, so no `dist/` sits between a mutation and the run.

| # | mutation | red (of 34) |
|---|---|---|
| A1 | the stored-row wall judgement never refuses | 12: by-id, insert,
array insert, predicate, composed-with-check, `group`, × 2 |
| A2 | the wall's seam installed only when a business `check` is (the
old condition) | 12: by-id, insert, array insert, predicate, `group`,
fail-closed, × 2 |
| A3 | the guard stands down for every wall-only seam | 2: fail-closed,
× 2 |
| A4 | no data-door stand-down | 2: the data door, × 2 |
| A5 | the payload judgement dropped | 4: supplied-value control and
only-refuses-more, × 2 |
| A6 | an absent `organization_id` judged too | 2: the absent-value
insert control, × 2 |
| A7 | the composed seam drops the business `check` | 2: the
composed-check cell, × 2 |

A5 has an extra reading. Without the payload judgement, a *supplied*
out-of-scope value is admitted but lands nowhere. The engine's static
`readonly` strip drops a caller-sent `organization_id`, which the
registry injects as `readonly: true`, while a hook-written value is
exempt from that strip. That exemption is why the hook path reached the
store, and why the payload refusal is the loud half of the supplied
case.

**Suites** (final head `cce969cf4d`, after merging `origin/main`
`7766b62282`, closure rebuilt):
- plugin-security: 133 files, 2648 tests passed;
- plugin-auth: 114 files, 2439 tests passed;
- runtime: 278 files, 3962 passed, 1 skipped;
- plugin-security `typecheck`: exit 0; the test layer compiles all 131
test files (`--listFiles`), and its ledger holds 0 files / 0 errors.

## Gates

`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` on `cce969cf4d` derived 62 families: the dispatch-time 49
plus 13 more. The 13 are `check-adr-0087-registration` and
`check-empty-changeset` (each with its self-test),
`release-rehearsal-clone --self-test`, `check:engine-double-contract`,
`check:objectql-double-limit`, `check:objectui-changeset`,
`check:pm-changeset-deadline-census`, `check:query-options-erasure`,
`check:type-check-coverage`, `check:type-check-debt` and
`check:where-matcher`. All 62 ran, and `--ran` reconciled 62 derived /
62 run / 0 NOT-MEASURED, every line carrying its exit code, all 0.
`check:dual-build-cjs-loads`, `check:i18n` and `check:type-check-debt`
first answered PREREQUISITE NOT MET (exit 3) and were re-run green after
a full workspace build. The board-probing `GITHUB_TOKEN=… node
scripts/check-issue-citations.mjs` passed: 9 citations resolve.

Narrowed lint: `eslint --no-inline-config --format json` over the 13
touched `.ts` files reported 13 files, 0 errors, 0 warnings. It is a
full measurement for these files for three reasons. The population is
the `**/*.{ts,…}` and `packages/**` blocks of `eslint.config.mjs`. The
count, 13, comes from the JSON output. The config never enables
type-aware linting (no `parserOptions.project`, no typed rules), so this
diff cannot move the verdict for any untouched file. The repo-wide `pnpm
lint` and the Dogfood Regression Gate are left to CI.

## Surface beyond the claim, with reasons

The new installation runs on every walled insert and update. So every
plugin-security test harness that stands in for the engine with a
terminal that skips the seam, under a walled posture, was refused by the
fail-closed guard (48 red on the first full run). Each double now runs
the seam the way the engine does: an insert's rows as sent (no hooks in
a double), and on an update the by-id row or the matched rows merged
with the payload, through the producer's dispatch predicate where the
double dispatches updates.

- The 10 files that went red on the first run: `authz-matrix-gate`,
`can-write-object-admission`, `check-only-write-scope`,
`controlled-by-parent-detail-write-authority`,
`controlled-by-parent-master-widener`, `explain-write-verdict-inputs`,
`no-active-organization-write-refusal`, `row-write-widener-composition`,
`select-only-write-visibility` and `tenant-layer0-verdict-on-operation`.
- `explain-dependency-fault`, which arrived with the merge of
`origin/main` (PR objectstack-ai#20030) and went red on the merged tree for the same
reason.
- `check:engine-double-contract` is green, with no ledger change.
- Census outside the package: `grep -rln postHookWriteImageCheck
packages --include=*.test.ts` names only plugin-auth's
`sys-user-self-service-route.test.ts` (PR objectstack-ai#20012's). The hand-made
SecurityPlugin hosts under a walled posture are runtime's
`share-links-enforcement-context` and
`standalone-stack-seeder-declaration-copy`. All pass unchanged
(plugin-auth and runtime suites green), so none is touched.

## Behaviour that changes (all in the refusing direction)

- Under `isolated` or `group`, an insert, by-id update or predicate
update whose hook chain leaves `organization_id` outside the caller's
organization scope (or the delegator's, ADR-0090 D10) is refused, and
nothing is stored.
- A walled write on a host that installs the wall's judgement and never
runs it is refused after the write, with an `error` log. The ADR-0094
data door is the stated exception, for a wall-only seam.

## Pending changesets

This change makes no sentence in a pending changeset false. None says
the tenant wall is unchanged. The "admitted as before" and "judged
exactly as before" sentences in `19950-rls-check-multi-row-writes.md`
and `19989-by-id-update-post-hook-check.md` are scoped to the row-level
`check`, whose judgement this change does not alter. So no deliberate
correction was made, and `check-empty-changeset` is green.

## Acceptance notes

- `packages/objectql/src/engine.ts` lines 11846-11847 still read "The
Layer 0 tenant wall still judges the PRE-hook image — filed separately".
After this change that sentence is false: the wall judges the stored row
through that same seam. The file was held by the engine lane at
dispatch, and this PR does not touch it. Suggested replacement for the
engine lane: "(The Layer 0 tenant wall judges this row too, through the
same seam; objectstack-ai#20013.)".
- Cost:
- Under a walled posture, every non-system insert and update now
computes the Layer 0 filter.
- A walled predicate update now always pays the engine's memoized
matched-row read, because the seam receives matched rows merged with the
payload. On an object with an update hook or a row-reading rule that
read already happened. On one with neither it is new. There it is one
`driver.find` over the composed `where`, with no row ceiling on that
path. The row-level `check` seam (objectstack-ai#19950) already imposes the same cost
where a `check` applies. NOT MEASURED: bulk-update latency or memory.
- An absent or emptied `organization_id` is not judged by either half.
That mirrors step 3.7's "supplied (non-empty)" scope and ADR-0095 D1's
reason. A hook that clears the column on an update therefore lands a row
with no organization. It is not measured here, and no declared contract
covers it.
- An array UPDATE payload gets no stored-row wall judgement, as step 3.6
gets none (the engine takes one payload per update). The payload
judgement still covers it.
- A by-id update is now judged by the wall twice: the payload as sent,
before `next()`, and the stored row, in the engine. The first is what
keeps this change refusing only more (A5).
- The merge commit `fe1579223a` carries no `Claude-Session` trailer; the
other commits carry the model-free pair.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
veigajoao pushed a commit to veigajoao/objectstack that referenced this pull request Sep 29, 2026
…for a row-level policy comparing two fields of no shared comparison class (objectstack-ai#20598)

Fixes objectstack-ai#20431

Clause-②: no

## What was wrong

A row-level policy can compare two fields that share no comparison
class, for example a text field against a number field. Enforcement
refuses every request such a policy scopes. The find answers
`INVALID_FILTER` / 400. A by-id update or delete fails closed at its
row-level gate (403), because that gate's pre-image read is the same
refused read.

The record-grained explanation judged the same predicate in-process
without the object's declared columns. It compared the two raw values
and reported a record verdict: `visible: true` for one ordering of a
pair, and `visible: false` (rls `excluded`) for the other. Both answers
covered a request that enforcement refuses.

## What changed

The landing point is
`packages/plugins/plugin-security/src/explain-engine.ts`, as the
dispatch expected; the other files are the pin file and the changeset.
There are no changes to `security-plugin.ts`, `packages/formula`,
`packages/spec`, the REST layer, or enforcement.

- The record matcher (`matchesFilterCondition`) now receives the
object's declared columns (`options.fields`), as the RLS write check
does. They are read from `ql.getSchema(object)`: the schema the engine
already reads for the OWD, and the ObjectQL registry that the find's
driver compiles against. A schema that cannot be read hands over no
columns, and the matcher judges values only, as before.
- With the columns, the matcher refuses the comparison. Explain answers
with that refusal: the explanation fails with `INVALID_FILTER` / 400
(the matcher's code and status, the envelope the find answers with), and
no record verdict is reported. The message names the policy and both
fields with their declared types. The matcher's own error rides as
`cause`.
- Naming the fields discloses nothing new. The report explain gives the
same caller for the same object already publishes that predicate
(`readFilter`, or the `rls` layer's `rowFilter`).

## Why a refusal and not a fail-closed report: dispatch assumption A3
did not hold

A3 said to reuse PR objectstack-ai#20030's shape (layer `not_evaluated`,
`record.visible: false`) for "enforcement refuses this read".
Measurement on `main` says otherwise:

- Explain already answers the matcher's other `INVALID_FILTER` refusals
as a refusal.
- `rls-stored-list-ordering-fails-closed.test.ts` (landed in `de091b50`,
PR objectstack-ai#20310) pins it: "explain read 400 = find 400; explain update 400,
the by-id update 403". One of its cells is a field-to-field comparison
against a list-holding field.
- PR objectstack-ai#20030's shape covers a dependency call that fails, not a predicate
the matcher refuses.

My first commit used the report shape. The full `plugin-security` suite
then turned 2 cells of that landed pin red, because its field-to-field
cell is now caught first by the comparison-class rule. Keeping the
report shape would have added the second refusal dialect the dispatch
forbids. So this PR follows the ruling's intent: "the read is refused …
both orderings answer the same refusal as find".

## Measurement: before and after (better-sqlite3, the same stack as the
pins)

| policy class | find | by-id update / delete | explain read / update /
delete, before | after |
|---|---|---|---|---|
| text vs number | 400 `INVALID_FILTER` | 403 `PERMISSION_DENIED` |
`visible: true`, `decidedBy: 'rls'`, rls `admitted` | refused, 400
`INVALID_FILTER` |
| number vs text (the other ordering) | 400 `INVALID_FILTER` | 403
`PERMISSION_DENIED` | `visible: false`, `decidedBy: 'rls'`, rls
`excluded` | refused, 400 `INVALID_FILTER` |
| text vs text (control) | the row | admitted | `visible: true`,
`decidedBy: 'rls'` | unchanged |

## Tests

New file:
`packages/plugins/plugin-security/src/explain-cross-class-refusal.test.ts`.
It uses the real `SecurityPlugin`, `ObjectQL` and SQL drivers
(better-sqlite3 and sqlite-wasm; PostgreSQL when `OS_TEST_POSTGRES_URL`
is set), on PR objectstack-ai#20427's harness. Every refused cell asserts both halves
with their envelope `code` and `status`: explain's answer, and the
caller's real request.

- Five cells: text vs number, text vs image, text vs formula, text vs
json, and number vs text. Each checks read, update and delete. Explain
answers `{ code: 'INVALID_FILTER', status: 400 }` and its message names
the policy and both fields. Find answers `INVALID_FILTER` / 400, update
and delete answer `PERMISSION_DENIED` / 403, and nothing is stored.
- Both orderings of one pair get `{ find: INVALID, explain: INVALID }`.
- Control, same class: find returns only the matching row. Explain
reports `visible: true` / `admitted` for it and `visible: false` /
`excluded` for the other row. The update is admitted and matches
explain.

Pre-fix run: `main`'s `explain-engine.ts` restored from the base blob
`92716c91`, under a trap whose restore is proven by the HEAD blob and an
empty `git diff HEAD`. Result: `Tests 12 failed | 2 passed | 7 skipped
(21)`. The 2 passes are the controls.

**Ablation:** only the declared-columns argument was removed, through
`scripts/ablation-replace.mjs`. The anchor hit 1 → 0 and the blob went
`a46456db` → `5a314958`. Result: `Tests 12 failed | 2 passed | 7 skipped
(21)`. Every refused cell on both drivers failed:

```text
AssertionError: expected 'answered' not to be 'answered' // Object.is equality
AssertionError: expected { find: { …(2) }, explain: 'admitted' } to deeply equal { find: { …(2) }, explain: { …(2) } }
```

Restore: `ok restored: blob == HEAD (a46456d) and git diff HEAD is
empty`.

All figures below were measured at `5e48f52c`, the head after merging
`origin/main` `c876a742`:

- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: `Test Files 144 passed (144)`, `Tests 3066 passed | 23
skipped (3089)`.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, with
the test layer OK. `tsc -p tsconfig.test.json --listFiles` counts the
new file once.
- Gates: `node scripts/pm/dispatch-gates.mjs --commands` derived 64
commands, and all 64 ran with exit 0. Three first answered exit 3
`PREREQUISITE NOT MET` (`check:dual-build-cjs-loads`, `check:i18n`,
`check:type-check-debt`). I rebuilt with `turbo run build
--filter='./packages/*' --filter='./packages/*/*'` (71/71 tasks) and
re-ran them; all three answered exit 0. `dispatch-gates --ran`: `64
derived, 64 run, 0 NOT-MEASURED, 0 UNRUN`.
- Lint, narrowed: `eslint --no-inline-config --format json` over the two
touched `.ts` files gives 2 files, 0 errors, 0 warnings. `eslint
--print-config` shows no `parserOptions.project` / `projectService`.
Linting is not type-aware, so this diff cannot move any untouched file's
verdict.

## Acceptance notes

- **The REST door answers 500 for this refusal.** The explain route's
catch maps only `PERMISSION_DENIED` → 403 and `OBJECT_NOT_FOUND` → 404;
every other throw becomes `500 EXPLAIN_FAILED`. I measured it through
the real handler (`security-explain-envelope.test.ts` harness): a
service refusal carrying `INVALID_FILTER` / 400 comes back as `{ status:
500, error: { code: 'EXPLAIN_FAILED', message: … } }`. The refusal's
message survives. PR objectstack-ai#20310's refusals were already answered this way.
It lives in `packages/rest/src/rest-server.ts`, outside this card's
surface, so it is reported, not fixed here.
- **The object-level answer is unchanged.** An explanation without a
`recordId` runs no record matcher. For a read under such a policy, it
still reports `allowed: true` and rls `narrows`, where the find answers
400. This PR does not change that; it is reported separately.
- **Missing record, not measured.** When the record does not exist, the
matcher never runs, so explain keeps its missing-record answer
(`visible: false`, no `decidedBy`) for a policy the find would refuse.
- **Duplicated attribution.** The policy-name attribution
(`refusedPolicyNamesOf`) copies the RLS write check's attribution in
`security-plugin.ts`. That file is held by objectstack-ai#20555, so one shared helper
is left to whoever next touches both files.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant