Repository navigation
fix(plugin-security, objectql)!: a by-id update's row-level check holds for the row it stores - #20012
Conversation
…stored row Failing-first pins for a by-id update whose beforeUpdate chain changes a field a row-level check names, on both SQL driver families, with controls for the in-scope write, a hook that touches no checked field, the existing change-set judgement, the predicate twin and the fail-closed seam. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
…k on the row it stores The engine now runs OperationContext.postHookWriteImageCheck on the by-id update path too, once the payload is final (after the beforeUpdate chain, the hand-back, both readonly strips and the strict-drop refusal), over the prior row merged with that payload. The security middleware keeps its existing judgement of the change set as sent and installs the seam for every update, so a by-id update is judged twice and only ever refused more. A falsy payload id beside a truthy where.id makes the engine write a row other than the one the middleware judged; that write is now refused before anything runs, instead of after the driver stored it. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
…y-id update The real engine now runs an installed postHookWriteImageCheck on the by-id update path, so the doubles that stand in for it do the same (the row they write, merged with the payload) instead of being refused fail-closed. The in-tree note that recorded this gap as open now points at its pins. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
…e update never carries Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
…-id-update-post-hook-check # Conflicts: # packages/objectql/src/engine.ts
📓 Docs Drift CheckThis PR changes 2 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 — 27 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 d9207719966eaa4c98352fa2c27770570af11181 && git checkout d9207719966eaa4c98352fa2c27770570af11181
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fc6ddb87a4cd065171080f018cd865cd9c84c9d9 0474ea38f3626627a9052a0806b7d989c6cf08f9 && git checkout -B drift-repro fc6ddb87a4cd065171080f018cd865cd9c84c9d9 && git merge --no-ff 0474ea38f3626627a9052a0806b7d989c6cf08f9
node scripts/docs-audit/affected-docs.mjs --json fc6ddb87a4cd065171080f018cd865cd9c84c9d9
|
The multi-row check entry said a by-id update is judged exactly as before, and the insert post-image entry said update is unchanged. Both read as release-level claims, and the by-id stored-row judgement in the same release makes them false. Each sentence now says what its own entry does and points at the entry that moves the by-id update. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
…ed-row check The harness's terminal next() stood in for the engine without running the write-image check the security middleware now installs on a by-id update, so the middleware's fail-closed guard refused every own-row write it pins as admitted. The terminal now runs it on the fixture row merged with the payload, through the producer's dispatch predicate, without adding a read to the recorded pre-image wheres. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Read: card #19989 body and 5 comments (the two ① Derived judgments
Deliberate correctionNote 1:
Note 2:
No third changeset on the merge base is touched (diff stat: exactly these two ② Semver levelRIGHT. ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
|
Generated by Claude Code |
…s with enforcement throws (objectstack-ai#20030) Fixes objectstack-ai#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 objectstack-ai#19963 and objectstack-ai#19986 (PR objectstack-ai#19984, PR objectstack-ai#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 objectstack-ai#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:** - `security-plugin.ts`. PR objectstack-ai#20012 has since landed there, and this branch merges it. - `default-permission-sets.ts`. - `packages/spec`. - Enforcement. ## 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](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
… cleanup names the delete, the reference and the repair (objectstack-ai#20021) Fixes objectstack-ai#20006 Clause-②: no ## What this changes Deleting a record clears every `set_null` reference to it (the default for an optional `lookup`). `ObjectQL.cascadeDeleteRelations` does this with an UPDATE of each referencing record. That cleanup resolves no related record for a validation rule, by design (objectstack-ai#18682: resolving one would let a rule's verdict leak one bit of a related record to a deleter who cannot read it). So a `script` / `cross_field` rule on the referencing object that reads through a reference faults there. It refuses the cleanup, and the delete with it. The refusal stays. objectstack-ai#18682 ruled it fail-closed, and this PR does not touch it. Only the text changes, per round-11 Q1 option D, 「让外键清空的拒绝文字指对对象」. **Before**, measured at the merge base `fc6ddb87a`: the deleter got the generic fault text about the rule's own object: ```text Validation rule 'closed_account_frozen' could not be evaluated (runtime: No such key: status) — write rejected. The predicate reads 'status', which this object does not declare — fix the rule's condition, or declare the field. ``` The person who deleted the record did not write that rule. Following the advice adds a bogus `status` column to `crm_deal`, the wrong object. **After**: ```text Cannot delete crm_account (acc_1): the delete clears `account` on the crm_deal records that reference it, and validation rule 'closed_account_frozen' on crm_deal could not be evaluated on that write — it reads 'status' through `account`, and a rule is given no related record while a delete clears references. Guard the rule on `account` being set: make it the `then` of a `conditional` rule whose `when` is `record.account != null`. Or change `deleteBehavior` on crm_deal.account: 'cascade' deletes those records with the crm_account, 'restrict' refuses the delete while they exist. ``` It names all four things the triage asked for: the object and rule, the reference being cleared, the parent delete that is blocked, and the repairs. Each repair was measured to work, and each is pinned: - **The `conditional` guard** makes the same delete succeed and clears the reference. It is offered only to a rule that reads through the reference the cleanup empties. Such a rule already refuses every write that leaves that reference empty (`no single related record`), so the guard only lets those writes through. The guarded rule is still judged on an ordinary write while the reference is set, and a non-vacuity pin shows it refusing with the authored message. On a multi-value reference the other members stay, so the guard would still run the rule (measured: still refused). There the text offers only `deleteBehavior`. - **A rule that reads only through another reference** (the objectstack-ai#18682 plugin-security fixture's own shape) is offered only `deleteBehavior`. A guard on the cleared reference would stop judging that rule on every record whose cleared reference is empty, on every insert and update (patch round 1, F2). - **`deleteBehavior`** is a real, authorable `FieldSchema` key (`'set_null' | 'cascade' | 'restrict'`). With `'cascade'` the delete succeeds and the referencing records are deleted with it; measured on a single-value and a multi-value lookup. With `'restrict'` the delete is refused up front: `DELETE_RESTRICTED`, 409, nothing changed. **Unchanged, byte for byte:** the error (`VALIDATION_FAILED`), the field error (`rule_violation`, `_record`), the whole `constraint` (`reason: 'unevaluable'`, `fault`, `missingKey`), and the set of deletes refused. The new text is chosen only AFTER the predicate has already faulted. A rule that has a verdict without reading through the reference still lets the delete through, as before. That is pinned with `record.amount > 100 && record.account.status == 'closed'`; CEL's `false && FAULT` is `false`. Any other refusal keeps today's text, and two controls pin it byte for byte: - a rule that reads through no reference, on the same cleanup; - the same fault on an ordinary update. ## How - **`rule-validator.ts`.** On a CEL fault, `checkPredicate` asks `referentialClearRefusal` whether this is the cleanup. The answer is yes only when three things hold: - the `related` binding was minted by `referentialClearBinding`; - the fault's missing key is a column the rule reads through a reference field; - `readsAnOwnColumnItLacks` finds no own read of a key its holder does not hold. Optional spellings (`has(record.K)`, `record.?K`) count too, although they do not fault, so such a rule keeps the generic text: a conservative miss that changes no verdict. That last check covers EVERY own read, not only the key CEL reports; when both operands of `&&` / `||` fault, CEL reports the right-hand key. The own reads are: - a bare `record.K` the record does not hold; - a `record.F.K` through a field that is not a reference, whose value lacks `K`; - a bare `previous.K` the previous row does not hold; - a `previous.F.K` through ANY field, a reference included, whose value lacks `K`. `previous` is never hydrated, so a hop through a reference there meets its bare id. A declared column is materialised on `record` and `previous`, is always held, and never counts. If any own read faults, or the missing key is not read through a reference, the generic text stands byte for byte. `referenceGuardRepair(field)` holds the guard wording once. - **`engine.ts`, `cascadeDeleteRelations`.** Beside `__referentialFieldClear`, the cleanup UPDATE's context gets `__referentialFieldClearCause`: `{ object, id, referencingObject, referencingId, field }`. It uses the operation-private `__` prefix, so every consumer that forwards a context already strips it. It authorizes nothing, and the referencing row's id never reaches the text. - **`engine.ts`, `resolvePredicateRelated`.** This is the `__referentialFieldClear` short-circuit, the `⛔ A referential FK clear resolves NOTHING` block. It still resolves nothing. For the one record the cleanup was issued for (same object, same row id), it now hands back an EMPTY binding that carries the cause. A write that only inherits the envelope keeps today's text. That is a hook's write to another object or another row during the cleanup, and it is pinned. - **No public surface moves.** `RelatedRecordBinding`, `RelatedFieldBinding`, `RelatedUnavailableReason` and `EvaluateRulesOptions` are unchanged. The cause rides a module-private `WeakMap` keyed by the minted binding. Measured in the rebuilt `dist/*.d.ts`, `referentialClearBinding` and `ReferentialClearCause` have 0 hits in `index.d.ts`, `core.d.ts` and the `util` chunk; the control `RelatedRecordBinding` has 1, 1 and 3 hits. There is no new error code and no ledger change. Neither `OperationContext` nor `update()` is touched (draft PR objectstack-ai#20012), and nor are `packages/formula` or `packages/spec`. ## For objectstack-ai#20007 objectstack-ai#20007 rewrites the neighbouring `no single related record` prescription. It should reuse this sentence, with its own reference name: > Guard the rule on `REF` being set: make it the `then` of a `conditional` rule whose `when` is `record.REF != null`. In `rule-validator.ts` the clause after the colon is `referenceGuardRepair(field)`, a module-local function, so objectstack-ai#20007 can call it rather than respell it. This PR does not edit objectstack-ai#20007's prescriptions. objectstack-ai#20007 is not addressed here. ## Mechanism hypotheses (dispatch §2), measured at `fc6ddb87a` - **H1 holds.** The cleanup context is `{ ...(context ?? {}), __referentialFieldClear: true }`, and `resolvePredicateRelated` returns `unbound` for it. - **H2: the CEL-fault arm fires, not the `no single related record` arm.** When the rule reads the cleared reference, `null.status` gives `runtime: No such key: status`. When it reads another reference, the bare id `'reg_1'.kind` gives `No such key: kind`. Both fall to `unevaluableRuleError`. The `no-reference` arm fires only on an ordinary write that empties the reference, and that text is unchanged here. - **H3.** The validator could not learn which reference and which delete from a boolean. The cause is carried on the cleanup UPDATE's own context object only, not on the `OperationContext` interface, and there is no public option. - **H4 holds.** The `conditional` guard makes the delete succeed, and `deleteBehavior` is a real key. Both are named exactly, with what each does. ## Deviations, declared - **Option D's written mechanism was not taken, only its direction.** Option D sketched "an explicit unavailable binding with its own reason ... a new `RelatedUnavailableReason` member". That would refuse BEFORE evaluation, which grows the refused set. Measured at the merge base, two rules pass the cleanup today: `record.amount > 100 && record.account.status == 'closed'` with amount 50, and `has(record.account.status) && ...`. A pre-evaluation refusal would refuse both deletes. That is a narrowing, against the claim's `Clause-②: no`, and the new union member is a public type the dispatch rules out. So the text is chosen on the fault path, and the refused set does not move (pinned). - **The `engine.ts` edit spans two methods.** Besides `cascadeDeleteRelations` (the stamp), it edits the `__referentialFieldClear` short-circuit in `resolvePredicateRelated` (the read), plus one named import. There is no other seam where the cleanup's context reaches the validator without editing `update()`. Neither hunk overlaps objectstack-ai#20012's (`OperationContext` ~2235–2257, `update()` ~13420–13669). ## Patch round 2 (`b22174cd4`): contract review 5822275989 (FAIL at `9a31fc992`), item F3 - **F3, attribution keyed on the one key CEL reports.** When both operands of `&&` / `||` fault, CEL reports the right-hand key. So at `9a31fc992`, `record.kind == 'x' && record.account.status == 'closed'` on a `crm_deal` declaring no `kind` got the cleanup text with the guard, although the rule is broken on every write. So did its `||` form and `previous.kind == 'x' && …`. - **The fix:** `readsKeyOffTheTraversal` is replaced by the key-independent `readsAnOwnColumnItLacks`, which checks EVERY own read (see "How"). Those three shapes now keep today's generic text byte for byte, which is base's text for them. The round-1 refinement stands: a declared column never counts. - **Pins:** red at `7a57a1092` (3 failed / 67 passed, the three F3 cleanup legs), then green at `b22174cd4` (70/70). The ordinary-write control, where the rule refuses on its own `kind`, is green at both heads. - **Ablations:** each restore ended blob == HEAD. - G1, the bare-read loop removed: 5 red. - G2, the `previous` root unchecked: 2 red. - **Refused set and envelope, re-measured:** a base-revert probe over 22 engine-level shapes moved 0 verdicts and 0 envelopes. The message differs from base only in cleanup shapes where only the traversal faults: 5 in the author's 22-shape probe, and 18 in the contract review's 78-shape probe (5822996409). - **Suites and checks:** - the two pin files: 70/70; - the plugin-security fixture: 18/18; - objectql `--project local`: 311 files / 5262 tests; - `typecheck` exits 0, with the debt unchanged; - narrowed eslint: 0 / 0; - the module-local names have 0 hits in the rebuilt d.ts; - `dispatch-gates` gives the same 65 families (63 exit 0, 2 NOT MEASURED, dist-reading families re-run after the rebuild); - `check-issue-citations` exits 0. - **`engine.ts`** is untouched this round. - **Changeset:** only the "Unchanged" bullet is reworded, to the four reads the code checks. - **Accepted as is: a read through a reference into a column the related object does not declare** (`record.account.nope`, with no `nope` on crm_account). - It gets the cleanup text with the guard. Every sentence of that text is true, and the guard does let the delete through. - The rule's breakage stays loud on every ordinary linked write ("'crm_account' declares no 'nope'"). - The generic text would be worse there: it would say the rule's own object does not declare `nope`. ## Patch round 1 (`9a31fc992`): contract review 5821350148 (FAIL at `b1de69f91`), items F1 and F2 - **F1, attribution by key name.** At `b1de69f91`, `record.status == 'x' && record.account.status == 'closed'` on a `crm_deal` that declares no `status` got the cleanup text, although that rule is broken on every write. The attribution now also requires that the key cannot fault on the rule's own columns (`readsKeyOffTheTraversal`, module-local). That shape keeps today's generic text byte for byte, and so does a `previous.KEY` collision. - The review's literal rule, "a key also read bare at the root keeps the generic text", was measured and not taken. On an object that DOES declare `status`, `record.status == 'x' || record.account.status == 'closed'` would then say "which this object does not declare", which is false: the declared column reads null and cannot fault. - A bare read therefore counts only when the record does not hold the key. That case is pinned, and so is its ablation (F1lit). - **F2, the guard's cost: option (b).** The `conditional` guard is offered only to a rule that reads through the reference the cleanup empties. That rule already refuses every write leaving the reference empty (`no single related record`), so the guard gives up nothing it enforced (pinned). - A rule that reads only through ANOTHER reference is offered `deleteBehavior` alone. A guard there would stop judging it on every record whose cleared reference is empty. - Pinned: an account-less secret-region insert is refused with the authored message both unguarded and under `deleteBehavior` 'restrict' / 'cascade'. - **Pins:** red at `98cb79454` (4 failed / 61 passed, the new F1/F2 pins against round 0's implementation), then green at `9a31fc992` (66/66). - **Ablations:** each restore ended blob == HEAD. - F1, the off-traversal check disabled: 2 red. - F1lit, the bare read counted whether or not the record holds the key: 1 red, the declared-`status` leg. - F2, the guard gated on emptiness only: 2 red. - **Suites and checks:** - the two pin files: 66/66; - plugin-security `delete-reference-cleanup-system-identity.test.ts`: 18/18; - objectql `vitest run --project local`: 311 files / 5258 tests; - `typecheck` exits 0, with the debt unchanged; - narrowed eslint: 0 / 0; - the new module-local names have 0 hits in the shipped d.ts; - `dispatch-gates` derives the same 65 families: 63 exit 0, and 2 are NOT MEASURED (the workspace dist was not built; CI's Lint & Repo Gates and Type Check are green at this head); - `check-issue-citations --base fc6ddb8` exits 0. - **`engine.ts`** is untouched this round. - **The changeset:** the guard bullet and the "Unchanged" bullet are rewritten to match, and an other-reference bullet is added. The example block and the multi-value bullet are unchanged. The "Tests" section below is round 0's readings at `b1de69f91`. ## Tests (all at `b1de69f91`) - **Failing probe first.** `636550185` added the pins before any fix. At that commit, `engine-predicate-relationship.test.ts` gave 3 failed / 28 passed, and the failures were exactly the new-text pins. Each received today's generic text quoted above; the guard, cascade and control pins were green, as they measure existing behaviour. - **Two test files:** - `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/engine-predicate-relationship.test.ts src/validation/rule-relationship-traversal.test.ts` gives 63/63 passed. - The new blocks add 12 end-to-end cases (real engine, in-file driver double, no new double) and 5 evaluator cases. - **`@objectstack/objectql`:** - `vitest run --project local --maxWorkers=2`: 311 files / 5255 tests passed. - `test:repo`: 1 file / 5 tests passed. - `typecheck` exit 0, including `check:test-typecheck`. The debt is unchanged at 40 files / 234 errors / 65 pinned. - **plugin-security, end to end on the real `SecurityPlugin`.** `delete-reference-cleanup-system-identity.test.ts` stays green unchanged, 18/18. It includes THE CONTRACT: a secret and a public related row give identical outcomes, the message included, and zero reads. A scratch copy that only printed the message (deleted after the run) shows the new text arriving through the security middleware, so the cause key survives that path: ```text Cannot delete os_ehr_product (r_2): the delete clears `product` on the os_ehr_inspection records that reference it, and validation rule 'no_secret_line' on os_ehr_inspection could not be evaluated on that write — it reads 'kind' through `line`, ... ``` - **eslint.** `eslint --no-inline-config --format json` over the 4 touched TS files: 4 files, 0 errors, 0 warnings. The config has no type-aware linting: `parserOptions` is `{ ecmaVersion, sourceType }` only, with no `project`. So this diff cannot move the verdict on any untouched file. The repo-wide run is CI's. ### Ablations Each ablation went through `scripts/ablation-replace.mjs` in WRAP mode, running the two test files. Every mutation was verified on disk (anchor 1 → 0, blob changed). Every restore ended with blob == HEAD and `git diff HEAD` empty, and porcelain was 0 after the set. | Ablation | Mutation | Red | Tests that went red | |---|---|---|---| | A | Engine binding dropped (`return unbound`) | 3 / 63 | the two new-text pins, and the inherited-marker control leg | | B | Validator branch dropped (`return unevaluable`) | 6 / 63 | those 3, plus the evaluator guard / multi / decides-nothing cases | | C1 | Row-id match dropped | 1 / 63 | the inherited-marker pin | | C2 | Object match dropped | 1 / 63 | the inherited-marker pin | | D | Attribution check dropped | 2 / 63 | the byte-for-byte clear control, and the own-columns evaluator case | | E | Guard offered unconditionally | 1 / 63 | the multi-value case | - My first try at A was refused by the tool: the replacement was a substring of the anchor, so the count could not rise. Nothing ran. It was re-run with a distinct marker; the result above is from that re-run. ### Gates - **Derivation.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `b1de69f91` (merge base `fc6ddb87a`, 5 paths) derived 65 commands, and all 65 were run: 63 exit 0; 2 exit 3 (PREREQUISITE NOT MET, workspace `dist/` not built in this worktree), NOT MEASURED: `check:dual-build-cjs-loads`, `check:type-check-debt`. - `--ran` reconciliation: 65 derived, 63 run, 2 NOT-MEASURED, 0 UNRUN. - As a supplementary reading, not the gate: `require('./packages/objectql/dist/index.js')` loads. - **Citations.** `node scripts/check-issue-citations.mjs --base fc6ddb8`: exit 0. 7 citations were judged across 2 files, and all resolve. ## Acceptance notes - **Out of scope, class (a), not filed here** (the seat files it). The `ConditionalValidationSchema` TSDoc examples in `packages/spec/src/data/validation.zod.ts` (use cases 1–3) fail when copied: `when: 'account_type = "enterprise"'`, `condition: 'approval_status = null'` and `'shipping_address = null OR shipping_address = ""'` each give `parse: Unexpected character: =`. `'order_total > 10000'` gives `Unknown variable: order_total`, because it has no `record.` root. Those were measured through `ExpressionEngine.evaluate`. The examples ship in the spec's published `.d.ts` (`object.zod-*.d.ts`). - **Changeset.** `.changeset/20006-cascade-fk-clear-refusal-text.md`, `@objectstack/objectql: patch`, `Clause-②: no`. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…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>
…es them (objectstack-ai#20268) Fixes objectstack-ai#19967 Clause-②: no Text only. No schema shape, accepted value or runtime behaviour moves. Every sentence below was re-measured against the code on `origin/main` at `0d3ec47137`, where PR objectstack-ai#19962 (`b5853da1ca`), PR objectstack-ai#19988 (`009da14713`), PR objectstack-ai#20012 (`44639665ee`, objectstack-ai#19989) and PR objectstack-ai#20167 (`b276d4463f`) are all merged. Each `git merge-base --is-ancestor` probe returned exit 0. ## What `main` enforces (the measurement the texts now state) - **Write `check` selection, per operation.** `writeCheckPolicies` (`packages/plugins/plugin-security/src/security-plugin.ts:890-899`) returns the policies that declare `check` when any applicable policy does. Otherwise it returns every applicable policy with a `using`, and keeps the platform ownership floor only where the write's row gate kept it. `compileFilter` (`rls-compiler.ts:600-602`) reads `check` for a policy that declares one and `using` for the rest, then OR-combines (`:676`). - **A USING-only `insert` policy's `using` is the insert check** when no applicable policy declares `check`. Pinned by `rls-check-defaults-to-using.test.ts`, case "an insert-class USING-only policy gates INSERT the same way", which is green on this head. - **A check-only `update` policy is legal and enforced.** The schema accepts it (`rls.zod.ts`, the at-least-one rule). The row gate derives the write scope from `select` when no write-class `using` applies (`security-plugin.ts:7018`), and the post-image check judges the written rows. Pinned by `check-only-write-scope.test.ts`, which is green. The showcase's `invoice_owner_immutable` is one such policy. - **A non-blank `check` on `select` / `delete` is refused** (`rls.zod.ts`, the `superRefine` at the `check` path, from PR objectstack-ai#20167). - **OR-combination.** On reads, and on the rows an update or delete may target, the applicable policies' `using` clauses OR-combine (`compileFilter`). On the written rows, the check is chosen first and then OR-combined. So "most permissive wins" is true on reads and false as a statement about writes. - **Post-image scope: every insert and update shape.** The seam is installed for every insert and for every update (`security-plugin.ts:3191`, `:3261`, `:3508`). The engine runs it: - on each live row of an insert, after `beforeInsert` (`packages/objectql/src/engine.ts:12470`); - on the by-id row merged with the final payload, after `beforeUpdate` (`:14085`, PR objectstack-ai#20012); - on each matched row of a `multi: true` update (`:14345`, PR objectstack-ai#19988). The by-id update is also judged in the middleware on the change set as sent (`security-plugin.ts:3116`). Pinned by `rls-check-multi-row-writes.test.ts`, `rls-check-by-id-update-post-hook.test.ts` and `insert-check-post-image.test.ts`, all green on this head. ## Sites, old text, new text, and the code that makes the new text true | Site | Old text | New text | True because | |---|---|---|---| | `rls.zod.ts` `check` describe | "matched against the new row of a single-record INSERT or a by-id UPDATE ...; an array insert and a `multi: true` update are not post-image checked." | "judged on every row an insert or an update writes ...: each row of an insert, an array insert included, as its `beforeInsert` hooks leave it, and each row an update changes, by id or `multi: true`, as the prior row merged with the final payload after its `beforeUpdate` hooks. One failing row refuses the whole write. A by-id update is also judged, before its hooks, on the prior row merged with the change set as sent." | `engine.ts:12470`, `:14085`, `:14345`; `security-plugin.ts:3116` | | `rls.zod.ts` `check` TSDoc | "Validation of the new row of a single-record INSERT or a by-id UPDATE. An array insert and a `multi: true` update are not post-image checked." | Every row an insert or update writes, with the per-shape image. It also names the engine-filled values that are not on an insert's judged image (autonumber, a `secret` field's stored reference, an absent tenant column). | the same lines; the engine's closed list after the insert seam (`engine.ts`, the comment above `:12470`) | | `rls.zod.ts` `check` TSDoc, floor | "takes part only where the by-id pre-image gate kept it" | "only where the write's own row gate kept it" | `computeWriteCheckFilter`: a `multi: true` update has no by-id gate and keeps the floor unless the OWD yields (`platformFloorYieldsToObjectWriteModel`) | | `rls.zod.ts` `using` describe | "Filter condition for SELECT/UPDATE/DELETE ... Optional for INSERT-only policies." | "Row predicate ...: the rows a `select` policy lets a caller read, and the existing rows an `update` or `delete` policy lets a caller change or remove. On an insert or an update, when no applicable policy for that operation declares `check`, each applicable policy's `using` also stands in as its check on every row written ...; on an `insert` policy that is its only effect. ... Needed on a `select` or `delete` policy (a `check` there is refused); optional on an `insert`, `update` or `all` policy that declares `check`." | `writeCheckPolicies:896`; `getApplicablePolicies:846`; the `check` refusal | | `rls.zod.ts` `using` TSDoc | "For INSERT-only policies, USING is not required (only CHECK is needed). For SELECT/UPDATE/DELETE operations, USING is required." | A per-operation list. `update` does not require `using`: a check-only `update` policy is accepted and enforced, and its target rows come from the other `update` / `all` `using` or from `select`. An `insert` policy's `using` filters nothing, but it is the insert check when none is declared. | `security-plugin.ts:7018`; `check-only-write-scope.test.ts` | | `rls.zod.ts` `superRefine` message | "... For SELECT/UPDATE/DELETE operations, provide "using". For INSERT operations, provide "check"." | The same head. Then: `select` / `delete` take "using"; `insert` takes "check", or a "using" alone as that check when no applicable insert policy declares one; `update` / `all` take either or both. | the schema accepts each prescription (pinned below) | | `rls.zod.ts` schema TSDoc | "combined with OR logic (union of results)" | OR on reads; on writes, the per-operation choice (see `check`) | `compileFilter`; `writeCheckPolicies` | | `rls.zod.ts` `priority` TSDoc | "Applicable policies OR-combine (... the doc above `RLSCompiler.compileFilter` and this schema's own former describe both say most-permissive-wins)" | No outcome depends on an order: OR on reads; on writes, the check is chosen per operation and then OR-combined | the same | | `rls.zod.ts` overview, "Default Deny" | "If no policy matches, access is denied" | Default deny among the policies that apply. When no policy applies, the policies restrict nothing (the tenant wall still applies). | `compileFilter:656` (`applicable === 0` returns `null`, no filter); `content/docs/permissions/rls.mdx` callout | | `migrations/registry.ts` `--from 16` prose (and regenerated `docs/protocol-upgrade-guide.md`) | "applicable policies OR-combine (most permissive wins)" | "no outcome depends on an order: applicable policies OR-combine on reads, and a write's check is chosen once per operation across the applicable policies, then OR-combined" | the same | | `liveness/permission.json` `check` / `using` evidence | `compileFilter` "`(policy as { check?: string }).check ?? policy.using`" (the expression is gone) | `security-plugin.ts#writeCheckPolicies` plus `rls-compiler.ts#compileFilter`, with the live expression | `check:liveness` resolves both anchors (green) | | `security-plugin.ts` `writeCheckPolicies` docblock | "The published contract is `RowLevelSecurityPolicySchema.check`: "defaults to USING clause if not specified"." | It points at `RowLevelSecurityPolicySchema.check` without quoting it, so it cannot go stale. PostgreSQL's per-policy rule is named as the starting point. | the describe itself | | `rls-check-defaults-to-using.test.ts` header | "A policy that declares no `check` holds the write post-image to its `using`, which is what `RowLevelSecurityPolicySchema.check` publishes: "defaults to USING clause if not specified"." | When no applicable policy declares `check`, each `using` stands in. The choice is per operation, not per policy. | `writeCheckPolicies` | | `content/docs/permissions/authorization.mdx:108-109` | "Multiple row policies for the same object/operation OR-combine" | They OR-combine their `using` on reads and on the rows a write may target. The written-row `check` is chosen per operation first. Links the fail-closed contract. | the same | ### In-place fixes beyond the claim's file surface (same defect class: the same stale sentences) The dispatch asked for a grep for other copies of the old sentences. These hits are not governed and not pending changesets, so they are fixed here. **File-surface supplement for the claim:** `content/docs/permissions/rls.mdx`, `content/docs/protocol/objectql/security.mdx` and `packages/spec/src/conversions/registry.ts` (TSDoc only). The two changesets below are also outside it. - `content/docs/permissions/rls.mdx:63`: "after a single-record insert or a by-id update; an array insert and a `multi: true` update are not checked" now says every row an insert or update writes. - `content/docs/protocol/objectql/security.mdx:144`: the same parenthetical. `using` is no longer "for SELECT/UPDATE/DELETE" only. - `packages/spec/src/conversions/registry.ts`: the `permission-rls-priority-removed` TSDoc said "most permissive wins". Its `summary` (which feeds `spec-changes.json`) is unchanged and not false. - `packages/spec/src/security/rls.test.ts`: the `priority` test comment said "(most permissive wins)". ## Pin sweep for the `superRefine` message ① The repo-wide grep for `At least one of` and `provide "using"` finds pins only in `packages/spec/src/security/rls.test.ts`: `toContain('At least one of')` and its negation. Both still hold, because the head is unchanged. No other package, doc or translation carries the message. ② New load-bearing pins, in the `RowLevelSecurityPolicySchema — the "at least one" refusal` block of `rls.test.ts`. For each of the five operations they assert the refusal's `code` (`custom`), `path` (`[]`) and head, that every operation is named, and that the false sentence is absent. They then prove each prescription against the schema itself: `select` / `delete` + `using`, `insert` + `check`, `insert` + `using` alone, and `update` / `all` with check only, using only, and both. All parse. ## Pending release notes corrected (DELIBERATE CORRECTION: `check-empty-changeset` is red by design) - `.changeset/19953-rls-check-default-composition-text.md` (from PR objectstack-ai#19962). It said: "The check runs on the new row of a single-record insert and of a by-id update. An array insert and a `multi: true` update are not post-image checked; those are tracked in objectstack-ai#19964 and objectstack-ai#19950, and the texts now say so". That would ship false. objectstack-ai#19988 and objectstack-ai#20012 land in the same release, and this PR changes the texts it says "now say so". The rewrite dates the old scope to when that change was written, and states the release's scope. - `.changeset/rls-check-defaults-to-using.md` (from PR objectstack-ai#19952). It said: "`RowLevelSecurityPolicySchema.check` reads "defaults to USING clause if not specified"". The describe no longer reads that in the release this note ships in (PR objectstack-ai#19962). Changed to "read ... (it now states the default per operation across the applicable policies, objectstack-ai#19953)". The triage on objectstack-ai#19967 raised this note for the dev to judge. The gate's remedy is to say so here and get it confirmed. Restoring either note from base would put the false sentence back. ## Reported only, not edited - **Governed (Tier H):** - `docs/adr/0066-unified-authorization-model.md:93`: "Multiple row policies for the same object/operation are OR-combined". This is the ADR sentence that `authorization.mdx` paraphrases, and it has the same false-for-writes reading. - `skills/objectstack-data/rules/security.md:72-75`: calls `using` the "read filter" and `check` the "write filter", and does not state the stand-in check. - `docs/adr/0095-authz-kernel-tenant-layer-and-posture-ladder.md:79-81`: about the read filter, and not false. - **Release-owned (never edited in a code PR):** `packages/spec/CHANGELOG.md:46934`, `:73373` and `packages/plugins/plugin-security/CHANGELOG.md:6610`, `:10225`. These say "(the schema's own describe says most-permissive-wins)". That was true of the describe when they shipped. - `packages/spec/spec-changes.json:79`, `:971`: the generated `summary` "policies OR-combine". It is shorthand and not false. It is regenerated from the conversion registry, which this PR leaves as is. ## Published surface - **`@objectstack/spec`**: `patch`. It ships `src/**/*.zod.ts`, `dist`, `liveness` and the `--from 16` prose. - **`@objectstack/plugin-security`**: no changeset. The two edits are a comment in a non-exported function and a test header. Measured: tsup strips in-body comments. A positive control in `packages/objectql/dist/index.js` shows the code line `postHookWriteImageCheck.honoured = true` present (1 hit) and the comment above it, "INSERT POST-IMAGE seam", absent (0 hits). `writeCheckPolicies` is not exported, so no `.d.ts` carries its docblock. ## Verification Every reading below was taken on head `e2bd8f3680`, the final commit, with a clean tree. - **Gate union.** `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived the list. `--ran` reconciles it: "116 derived famil(ies) accounted for — 112 run, 4 NOT-MEASURED". The 116 includes 2 families the derivation added after the regeneration commit (`pnpm --filter @objectstack/spec run check:generated` and `pnpm check:quick-reference-counts`). Both were run: exit 0. - **111 green.** - **1 red by design:** `node scripts/check-empty-changeset.mjs --base origin/main` exits 1 with the DELIBERATE CORRECTION class, for the two notes named above. - **NOT MEASURED, each one a PREREQUISITE NOT MET (exit 3):** - `check:skill-examples` needs a built `@objectstack/client-react` (a 36-package closure); - `check:dual-build-cjs-loads` needs a full `pnpm build`; - `check:i18n` needs the built CLI closure; - `check:type-check-debt` needs a whole-workspace build. - **`@objectstack/spec` build:** exit 0, DTS included. `check:generated`: 2 of 15 stale (`docs/protocol-upgrade-guide.md`, `content/docs/references/**`). Regenerated with exactly `gen:upgrade-guide && gen:docs`, then "All 15 generated artifacts are up to date". - **`@objectstack/spec` tests:** `vitest run --project local --maxWorkers=2`: "Test Files 547 passed (547) · Tests 16086 passed | 2 todo". - **`@objectstack/plugin-security` tests:** `vitest run --maxWorkers=2`: "Test Files 139 passed (139) · Tests 2839 passed (2839)". This includes the semantics pins cited above: `rls-check-defaults-to-using` (22 cases), `check-only-write-scope` (21), `rls-check-multi-row-writes` (32), `rls-check-by-id-update-post-hook` (24) and `insert-check-post-image` (25), all green. - **`@objectstack/plugin-security` typecheck:** exit 0, test layer included ("check:test-typecheck: OK"). - **`@objectstack/spec` typecheck: declared narrowing.** The full `pnpm --filter @objectstack/spec typecheck` never got a turn on the shared verify lock in about 40 minutes of queueing: 8 attempts, each exit 99. Its three legs are covered as follows: - the src program: the build's DTS pass compiles the entry closure, which contains `rls.zod.ts`, `migrations/registry.ts` and `conversions/registry.ts`; - the test program: `check:test-typecheck` is green inside `check:generated` on this head, and `tsconfig.test.json` includes `src/**/*.test.ts`; - `check:scripts-typecheck`: no spec script is touched. CI's `TypeScript Type Check` runs the full command. - **eslint, a proven narrowing:** - ① Population: `eslint.config.mjs` lints `**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` minus build output, which contains all 6 changed `.ts` files. - ② `pnpm exec eslint --no-inline-config --format json` over those 6 files reports 6 files, 0 errors, 0 warnings, exit 0. - ③ No config object sets `parserOptions.project`, so no rule is type-aware, and this diff cannot move a verdict on an untouched file. - **Control bytes:** `check:nul-bytes` is green. The self-scan `grep -naP` for C0 control characters and DEL over the changed files exits 1 with no match. ## Acceptance notes - **Whitespace-only `check`: the schema and the runtime disagree** (observation, not filed; no public-door measurement, no named producer). The at-least-one rule tests `!data.check`, so `check: ' '` satisfies it. The runtime and the `select` / `delete` refusal read a blank clause as absent (`policyDeclaresClause`). So a policy with only a whitespace `check` parses and is inert. The `using` describe's "needed on a `select` or `delete` policy" is worded as the runtime reads it. - **`@objectstack/lint` `rls-predicate-*` messages understate the scope** (`packages/lint/src/validate-rls-predicate-enforceability.ts:272`, `:318`, `:943`, `:969`). They say "the single-record INSERT check" and "every single-record insert and by-id update ... fails". Since PR objectstack-ai#19988 and PR objectstack-ai#20012 this is true but narrow: array inserts and `multi: true` updates are refused too. These are not false, and they are outside this card's file surface. Carrier: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…ADR-0066 and the data skill (objectstack-ai#20298) Fixes objectstack-ai#20275 Clause-②: no Two governed texts said the RLS write check wrong. ADR-0066's combination rule (item 3) called row policies "OR-combined (any matching policy admits the row)" for every operation; the published data skill's RLS section called `using` the read filter and `check` the write filter, with no stand-in rule, no per-row statement and no refusal rule. Both now say what `main` enforces, in the wording the schema already carries. objectstack-ai#20268 landed the non-governed half (the `rls.zod.ts` overview and describe texts, the docs pages); this PR is the governed half and waits on the maintainer's hand (Tier H: `docs/adr/**` and `skills/**`). No code change. Provenance of the three facts the skill now states: every-row judging of inserts and updates landed in objectstack-ai#19988 and objectstack-ai#20012; the refusal of a non-blank `check` on a `select` / `delete` policy in objectstack-ai#20167; the per-operation default and the default-posture wording in objectstack-ai#20268. ## What changed ### `docs/adr/0066-unified-authorization-model.md` — item 3 of "Precedence / combination semantics" One sentence. It now reads: on a read, the applicable policies' `using` predicates OR-combine (any matching policy admits the row); on an insert or update, the check is chosen once per operation across the applicable policies — the declared `check` predicates when any declares one, else each applicable policy's `using` standing in — and the chosen predicates OR-combine over every row written. The tenant-isolation clause and the superuser-bypass sentence are unchanged; Status, headings and the other items are untouched. The enforcing code is cited as symbol anchors, so `check:adr-symbol-anchors` holds them: - `packages/plugins/plugin-security/src/rls-compiler.ts#RLSCompiler.compileFilter` — read side: OR-combines the applicable policies' `using` (its docblock: "Multiple policies for the same object/operation are OR-combined"). - `packages/plugins/plugin-security/src/security-plugin.ts#writeCheckPolicies` — write side: takes the applicable policies that declare `check`; when none does, the ones that declare `using` (the platform ownership floor kept exactly when the pre-image gate kept it, `keepOwnershipFloor`). `computeWriteCheckFilter` then hands that set to `compileFilter` with the `check` clause, which OR-combines. ### `skills/objectstack-data/rules/security.md` — the "Row-Level Security (RLS)" paragraph and its example comments The paragraph now states, in this order: 1. `using` admits rows — what a `select` policy lets the caller read, and the existing rows an `update`/`delete` policy lets it change or remove; on a read the applicable `using` OR-combine, then AND into the query. 2. `check` is judged on every row an `insert`/`update` writes (array inserts and `multi: true` included; one failing row refuses the write), chosen per operation: when any applicable policy declares `check`, only those decide (OR-combined); else each applicable `using` stands in. 3. A non-blank `check` on a `select`/`delete` policy is refused. 4. The default posture, in the schema overview's words ("Default deny, among the policies that apply … when none applies the policies restrict nothing (the tenant wall still applies)"), plus the one clause the review of the non-governed half named: an `update`/`delete` target with no write-class `using` is bounded by the caller's `select` policies (the by-id pre-image gate derives its scope from the caller's SELECT narrowing when no write-class `using` applies; not for `insert`, not under the read-side superuser bypass). The example's two comments (`// read scope` / `// write scope`) now read `// rows readable / targetable` and `// every row written`. Wording follows the `RowLevelSecurityPolicySchema.using` / `.check` describe texts and the `rls.zod.ts` overview as landed in `3f86dc52`; no second phrasing of the same fact was introduced. No issue or PR number appears in the skill text (`check:doc-authoring` refuses one under `skills/`). ### Paying the token ceiling `check:skills-token-ratchet` holds `security.md` at 2543 tokens with headroom 0. The RLS paragraph grew by 687 bytes and the two comments by 22; the difference is paid in the same file by removing sentences the file already states elsewhere — nothing moved to another file, the ceiling is untouched: - the `permissions`-vs-`permissionSets` bullet no longer repeats the code comment two lines above it (the refusal text, the `ObjectStackDefinitionSchema` source and "never a silent drop" stay); - the `permission_set_id` warning no longer says "record id" twice; - the RLS source line cites `rls.zod.ts` once (policy shape, grammar, `check` composition) instead of `permission.zod.ts` again (already cited under RBAC); - the owner-scoping bullet, the `requiredPermissions` paragraph (its enforcer was already named under `maskingRule`), the platform-global paragraph and its blockquote lose filler words, no facts. ## Readings Line/token budget (tokens = `ceil(utf8 bytes / 4)`, the ratchet's own unit; `3f86dc52` → `4b330f38`): | surface | lines before → after | tokens before → after | |---|---|---| | `skills/objectstack-data/rules/security.md` | 214 → 216 | 2543 → 2541 (ceiling 2543, headroom 2) | | `skills/objectstack-data/**` (17 files) | 3807 → 3809 | 40934 → 40932 | | `skills/**/SKILL.md` (10 files) | 4402 → 4402 | 51903 → 51903 | | whole `skills/` tree (65 files) | 13427 → 13429 | 155990 → 155988 | | ratchet "bundle total (whole shipped tree)" | — | 153970 → 153968 | | `docs/adr/0066-unified-authorization-model.md` | 114 → 114 | 5096 → 5224 (no token ratchet on `docs/adr/**`) | The +2 lines in the skill file are the natural wrapping of a longer paragraph; the gate that prices `skills/**` is the token ratchet, and it went down. Gates — derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` off the merge base at `4b330f38`: 29 commands; every exit code captured before any pipe; `--ran` reconciliation: "29 derived, 29 run, 0 NOT-MEASURED, 0 UNRUN … all 29 recorded an exit code and none of them is 3". - `node scripts/check-skills-token-ratchet.mjs` → 0 — "`skills/objectstack-data/rules/security.md` is 2541 tokens (ceiling 2543; headroom 2)"; `--self-test` → 0 (65 cases) - `node scripts/check-adr-symbol-anchors.mjs` → 0 — "2125 anchors across 140 records resolve"; `--self-test` → 0 - `node scripts/check-adr-links.mjs` → 0; `--self-test` → 0 - `node scripts/check-ci-filter-parity.mjs` → 0 - `node scripts/check-closing-keyword-parity.mjs` → 0; `--self-test` → 0 - `node scripts/check-comment-mask-corpus.mjs` → 0 - `node scripts/check-doc-route-spelling.mjs --advisory` → 0; `--self-test` → 0 - `pnpm --filter @objectstack/lint run check:doc-formula-expressions` → first run exit 3 (PREREQUISITE NOT MET: `@objectstack/formula` and `@objectstack/lint` not built — a refusal, not a measurement); after `turbo run build --filter=@objectstack/formula --filter=@objectstack/lint` under `os-verify-lock.sh` (VERDICT command-exit 0, held 240s) → 0 - `pnpm check:adr-anchors` → 0 · `check:agent-test-spelling` → 0 · `check:corpus-claim-drift` → 0 · `check:cross-package-test-inputs` → 0 · `check:doc-authoring` → 0 · `check:driver-memory-census` → 0 · `check:gitlink-declared` → 0 · `check:nul-bytes` → 0 · `check:pm-governed-merges` → 0 · `check:pm-prior-rulings` → 0 · `check:refd-timer-probe` → 0 · `check:role-word` → 0 · `check:skill-compatibility` → 0 · `check:skill-frame-sync` → 0 · `check:skill-identifier-liveness` → 0 · `check:watch-hint-literal` → 0 Reverse verification of the ADR anchors (the gate lists only findings, so resolution was proven by failure): with `writeCheckPolicies` mutated to `writeCheckPoliciesNOPE` in the committed ADR, `check-adr-symbol-anchors` exits 1 with `[unresolved-symbol] docs/adr/0066-unified-authorization-model.md:93`; restored with `git checkout HEAD -- PATH`, `git hash-object` of the path equals the HEAD blob (`01878adb…`) and `git diff HEAD` is empty. Package tests / typecheck: none owed — the diff touches no `packages/**` file, so there is no ① dependency closure and no ② package suite; the whole-repo `pnpm lint` sweep is CI's. Changeset: `skip-changeset` applies. Both paths are outside every published package: 0 of the 69 non-private `package.json` manifests list `skills/`, `docs/adr` or a parent path in `files[]`; positive control — `RowLevelSecurityPolicySchema` is found in `packages/spec/dist/security/index.d.ts` (grep exit 0) and the schema's describe phrase "decided per operation across the applicable policies" in 9 spec dist files (exit 0); negative — the new skill sentence "chosen per operation across the applicable" is in no built output under `packages/` (exit 1). `docs/adr/**` is on the fast track (never published). ## Acceptance notes - The dispatch order's suggested ADR phrasing named "the `using` of the applicable insert-class policies" as the stand-in. The code is wider: `writeCheckPolicies` runs for `insert` and `update` alike (an `all` policy included), and when no applicable policy declares `check`, every applicable policy's `using` stands in for that operation. The landed text says "each applicable policy's `using`", matching `RowLevelSecurityPolicySchema.using` ("on an insert or an update, when no applicable policy for that operation declares `check`, each applicable policy's `using` also stands in"). The triage note's "an insert policy's `using` stands in when no `check` is declared" is one instance of that rule, not the whole rule. - The example policy `org_isolation` (a hand-written `organization_id == current_user.organization_id` select policy) sits beside the file's own Multi-tenancy section ("⛔ never `single` + your own RLS") and duplicates the Layer 0 wall. Left as is: not this card, and the token ceiling was paid without touching it. Observation only, nothing to file. - `check-doc-formula-expressions` refuses (exit 3) on a fresh worktree until `@objectstack/formula` and `@objectstack/lint` are built; its refusal text names the fix. Not a finding. ## 维护者速读(草稿) **改了什么**:两处受管文本各改一段。ADR-0066「优先级/组合语义」第 3 项那一句,从「同一对象/操作的多条行策略 OR 合并(任一匹配即放行)」改为:读侧按 `using` OR 合并;写侧(insert/update)先按操作在适用策略中选出检查——有声明 `check` 的只用它们,否则每条适用策略的 `using` 顶上——再 OR 合并,逐行判定写出的每一行;并以符号锚引用 `compileFilter` 与 `writeCheckPolicies`。发布技能包 `objectstack-data` 的 RLS 段改写为四句:`using` 放行哪些行;`check` 逐行判定 insert/update 写出的每一行(数组插入与 `multi: true` 包含)、按操作选定、无 `check` 时 `using` 顶上;`select`/`delete` 策略上的非空 `check` 被拒;默认姿态(只在有策略适用时默认拒绝;无策略适用则不限制,租户墙照旧;update/delete 目标行在没有写侧 `using` 时受调用者的 `select` 策略约束)。示例块两行注释同步。 **为什么改**:这两段是 AI 写 RLS 策略时读的唯一说明,原文把 `check` 当成读过滤的对偶、且不写顶替规则,按它写出的策略与运行时真实执行不一致(NORTH-STAR 优先级规则 4:写给 AI 的文档与 skills 说错一句等于产品缺陷)。执行代码本身是对的,非受管文本已由 objectstack-ai#20268 修正;本 PR 只改受管的两处,不动代码。 **风险与代价(含回滚)**:纯文本;不改 schema、不改运行时。`security.md` 的 token 上限为 2543、余量 0,新增内容以删除同文件重复句付账(2543 → 2541),上限未动、未挪内容到别的文件。29 个派生门禁全绿,ADR 符号锚经反向验证(改坏符号名即变红)。回滚即 revert 本 PR 的单个 commit,无迁移、无数据影响。 **席位意见**:(留空) **你要做的**:审阅两处措辞后在本 PR 上 Approve(Tier H);落地由席位执行。 --- _Generated by [Claude Code](https://claude.ai/code/session_01MjvgiFAmjHqsxy1XLiVYfH)_ Co-authored-by: objectstack-fleet[bot] <noreply@anthropic.com>
Fixes #19989
Clause-②: no (narrowing)
What this fixes
A row-level security
check(declared on a policy, or defaulted from itsusing) is a guarantee about the row that is stored (RowLevelSecurityPolicySchema.check; ADR-0058 D4). An insert and a predicate update are judged on the row the driver stores, through the engine-runOperationContext.postHookWriteImageCheckseam. A by-id update was judged only inside the security middleware, on the caller's pre-image merged with the change set as sent, beforenext()runs thebeforeUpdatechain. A value a hook wrote into a checked field after that point was never judged, so the row could be stored outside the policy, including in an organization the caller does not hold.A by-id update is now also judged on the row it stores, with the one existing refusal (
PERMISSION_DENIED/ 403, nothing stored). The middleware's existing judgement of the change set as sent stays, so this change only ever refuses more.Landing: two packages
packages/objectql/src/engine.ts(domain:engine, declared cross-seat in the claim). The by-id branch ofupdate()runs the installed seam right afterassertNoStrictDrops(), the same point the predicate path uses. By then thebeforeUpdatechain, the hand-back of withheld read-only values, both readonly strips and the strict-drop refusal have run, and nothing below changes a value beforedriver.update. The image is the prior row (already read under the not-found gate) merged with the final payload, coerced as the predicate path's images are. The seam's doc comments now name the by-id path.packages/plugins/plugin-security/src/security-plugin.ts, step 3.6. The seam is installed for every update, not only a predicate update. The by-id change-set judgement is unchanged. The post-next()fail-closed guard therefore covers a by-id update too.Why the engine is the producer (measured). The middleware's only image of a by-id update is taken before the hooks run. The engine is the one place that holds the final payload and the prior row together, and it already runs this seam for the other two write shapes.
One adjacent edge, closed so that the change never admits more. The middleware reads a falsy scalar payload
idas a row address; the engine does not, and binds a truthy scalarwhere.idinstead. In that shape the middleware's by-id gates judged a different row than the one written. Before this change the write reached the driver and was refused only afterwards, by the fail-closed guard, because the seam had been installed for a predicate path the engine never took (measured: 403 with the row already stored). With the engine now running the seam on the by-id path, that guard would stop firing and the 403 would become a 200. So step 3.6 asks the engine's own dispatch predicate (resolveEngineUpdateDispatch, from@objectstack/metadata-core, already a runtime dependency) and refuses that shape beforenext(), with nothing stored. It applies only where acheckapplies, which is the set the old guard covered.Mechanism hypotheses (dispatch Section 2), measured
Base:
origin/maine8f163fc3aat worktree creation. It is one unrelated commit (service-analytics) after the dispatch's009da14713.// BY-ID UPDATE — unchanged. Build the post-image: the caller's pre-image merged with the change set, thenlet postImage = { ...(opCtx.data) },if (pre) postImage = { ...pre, ...(opCtx.data) },if (!satisfiesCheck(postImage)) denyCheck();. All of it runs beforenext().SecurityPluginandObjectQL.organization_idsupplied in the change set as sent. On the tenant-wall harness, re-pointing to a parent in another tenant is refused earlier, by the reference check (VALIDATION_FAILED), not by step 3.7. A related Layer 0 reading is handed to the seat as an out-of-scope finding; it is not fixed here.update()has the equivalent point: afterassertNoStrictDrops()and beforeevaluateValidationRules. The predicate path's seam call sits in the same position.Tests (code head
7efc9eb010; round 2 head0474ea38f3)New:
packages/plugins/plugin-security/src/rls-check-by-id-update-post-hook.test.ts, 24 cells (12 per driver). It uses the realSecurityPluginandObjectQLon both SQL drivers. Every refusal assertscodePERMISSION_DENIEDandstatus403 plus the gate's developer half, then reads the stored rows straight off the driver's table.idbeside a truthywhere.id(for both''and0).idwith nowhere.idis still a judged predicate update.36f1dff9d6). Then both source files were set back to the base blobs and restored fromHEAD(blobs equal HEAD,git diff HEADempty). The run gave 12 red (the six negative cells × two drivers) and 12 green controls.Ablations were run on the final head, and the counts were identical on the pre-merge head
414909be35. Each is one mutation throughscripts/ablation-replace.mjs: the anchor hit 1 → 0 and the blob changed, then the file was restored with blob == HEAD and an emptygit diff HEAD. On resolution, the plugin is imported relatively and@objectstack/objectqlis aliased tosrc/index.tsin this package'svitest.config.ts, so nodist/sits between a mutation and the run.next()fail-closed guard droppedSuites on
7efc9eb010, after mergingorigin/main, with the closure rebuilt:0474ea38f3: plugin-auth 114 files / 2439 tests passed (CI Test Core 6/6 had 8 red insys-user-self-service-route.test.tson7efc9eb010; see the surface section); runtime 277 files / 3911 passed, 1 skipped; plugin-dev 8 files / 80 passed; plugin-authtypecheckexit 0 (test-layer ledger held at 10 files / 94 errors).sys_userrow (realObjectQLon driver-sql, the realSysUserschema, the shipped permission sets, the real identity write guard) is admitted and stored forlocaleandname, with the seam installed and honoured. The peer row is refused by the by-id pre-image gate, androleby the identity guard. So the plugin-auth red was the harness, not the product path.typecheckgreen for both packages. The plugin-security test layer is at 0 files / 0 errors; objectql holds its ledger unchanged at 40 files / 234 errors.Gates
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackderived 69 families from this diff on0474ea38f3(68 on7efc9eb010, pluscheck-dev-prereqs --self-testfor the plugin-auth file). All 69 ran and--ranreconciled 69 derived / 69 run / 0 NOT-MEASURED. 68 exit 0;check-empty-changesetexits 1 by design (see Deliberate correction). That list is a superset of the dispatch-time list; the additions includecheck:engine-double-contract,check:adr-0087-registration,check:empty-changeset,check:durability-log-leveland the type-check ratchets.check:dual-build-cjs-loads,check:i18nandcheck:type-check-debtfirst answered PREREQUISITE NOT MET (exit 3) and were re-run green after a full package-closure build.GITHUB_TOKEN=… node scripts/check-issue-citations.mjspassed: 15 citations resolve. The dead tracker the in-tree note cited is removed, and is referred to in words.eslint --no-inline-config --format jsonover the 13 touched.tsfiles gave 13 files, 0 errors and 0 warnings. Three facts make it a full measurement for these files. The population is the**/*.{ts,…}andpackages/**blocks ofeslint.config.mjs. The count, 13, comes from the JSON output. The config never enables type-aware linting (noparserOptions.project, no typed rules), so this diff cannot move the verdict for any untouched file. The repo-widepnpm lintis left to CI.Surface beyond the claim, with reasons
authored-row-write-verdict,authz-matrix-gate,check-only-write-scope,controlled-by-parent-detail-write-authority,controlled-by-parent-master-widener,rls-check-membership-staging(its double had judged the payload alone),security-denial-user-copyandselect-only-write-visibility.security-plugin.test.tschanges a comment only.insert-check-post-image.test.ts: the in-tree note that recorded this gap as open now points at the new pins.packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts. Its terminalnext()stood in for the engine without running the seam, so the middleware's fail-closed guard refused every own-row write the file pins as admitted. The failing run's developer message was the guard's own ("the update on 'sys_user' was executed without the row-level CHECK being evaluated"), not the check refusal. The terminal now runs the seam on the fixture row merged with the payload, through the producer's dispatch predicate, without adding a read to the recorded pre-image wheres. The census ofSecurityPluginhosts outside its own package found one other hand-made engine double,runtime/src/domains/share-links-enforcement-context.test.ts. It passes unchanged (runtime suite green), so it is untouched.origin/mainbrought PR feat(formula,objectql): read one hop through a lookup in a validation predicate #19728, which touched both files. It left one conflict inengine.ts, at this exact point: the seam and feat(formula,objectql): read one hop through a lookup in a validation predicate #19728's related-record binding for validation both followassertNoStrictDrops(). The resolution keeps both, with the seam first, as on the predicate path.Behaviour that changes (all in the refusing direction)
beforeUpdatechain leaves a checked field at a value the check refuses is refused, and nothing is stored.errorlog), as inserts and predicate updates already are.idaddresses no row whilewhere.idaddresses one, under a policy with acheck, is refused before anything runs. It used to be written and then refused.Deliberate correction
This PR rewrites one sentence in each of two pending release notes it did not add. Both sentences read as release-level claims, and this PR makes them false in a release that ships it. Every other byte of both files is unchanged (reversing each replacement reproduces the base blob exactly).
.changeset/19950-rls-check-multi-row-writes.md, under "What does not change".checkis judged on the pre-hook change set, so abeforeUpdatestamp that rewrites a checked field (e.g. the organization derived from a re-pointed parent) is stored unjudged — the tracker #16790 now answers 404 #19989).".changeset/insert-check-post-image.md, under "What changed, mechanically".updateis unchanged."updateunchanged, and the predicate and by-id updates move onto the same seam in their own entries (security: acheck-only row-level policy does not gate a bulk update (update(…, { where, multi: true })): the post-image check is skipped as "governed by the using-scoped where", and nousingexists to scope it #19950, security: a by-id UPDATE's row-levelcheckis judged on the pre-hook change set, so abeforeUpdatestamp that rewrites a checked field (e.g. the organization derived from a re-pointed parent) is stored unjudged — the tracker #16790 now answers 404 #19989)."updateis unchanged" would be false in the release.check-empty-changesetgoes red on this PR, and that is expected. It refuses any change to a changeset present on the merge base, and names both files with the DELIBERATE CORRECTION class. Its remedy for that class is to keep the correction and have it confirmed on the PR, not to restore the old text. Local run on0474ea38f3: exit 1, "This PR changes a changeset it did not add", listing both files as "present on the merge base and CHANGED by this PR".Acceptance notes
authz-matrix-gatemodel only the by-id path, through the producer's dispatch predicate. Thesecurity-plugin.test.tsdouble holds no table and judges no rows on an update; this is documented there.