fix(plugin-security, objectql): judge every row a predicate update stores against the row-level check

A row-level `check` (declared, or defaulted from `using`) guarantees
that no stored row fails it. A predicate update (`multi: true`, no row
address) was never judged: the write gate skipped it on the assumption
that a `using`-scoped `where` governed it. A policy that declares only
`check` scopes nothing, and a scoped `where` says nothing about the new
row, so the guarantee did not hold on that path for any policy.

The rows such an update changes are the ones the middleware-composed
query selects, which is complete only once every middleware has run. So
the security layer installs its judgement on the existing
`OperationContext.postHookWriteImageCheck` seam, and the engine runs it
on the predicate path once the payload is final. It hands the seam
every matched row merged with that payload, read by the one matched-row
read the path already makes. One failing row refuses the whole update
with the existing PERMISSION_DENIED / 403 refusal. A seam the engine
never runs fails closed, as it does for an insert. A falsy payload id,
which the engine does not treat as a row address, is judged both ways.

The by-id update and the single-row insert are unchanged. The pending
release note that said bulk updates are not checked row by row is
corrected in place.

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

Author: claude

.changeset/19950-rls-check-multi-row-writes.md

@@ -0,0 +1,26 @@
+---
+"@objectstack/plugin-security": patch
+"@objectstack/objectql": patch
+---
+
+fix(plugin-security): a row-level `check` now holds for every row a multi-row write stores — an array insert and a predicate (`multi: true`) update (#19950, #19964)
+
+A row-level security `check` (declared on the policy, or defaulted from its `using`) is the write-side half of the policy: a row the check refuses is never stored. The write gate enforced it for a single-row insert and for a by-id update. It did not enforce it for the two multi-row write shapes:
+
+- **An array insert** (`insert(object, [rows])`, which the create-many data route calls). No check was installed for an array payload, so the rows were stored unjudged.
+- **A predicate update** (`update(object, changes, { where, multi: true })`). The new rows were never judged. The gate assumed a `using`-scoped `where` covered the write, but a policy that declares only `check` scopes nothing, and a scoped `where` says nothing about the new row in any case.
+
+Both shapes are now judged row by row, with the same refusal the single-row shapes give: `403 PERMISSION_DENIED`, and nothing is stored. One failing row refuses the whole write.
+
+- **Array insert.** Every row is judged on the image the `beforeInsert` chain produced, as a single insert is.
+- **Predicate update.** Every row the write selects is judged on its new image: the matched row merged with the final payload. The engine supplies the rows (`@objectstack/objectql`): the security layer installs its judgement on `OperationContext.postHookWriteImageCheck`, the seam the insert check already uses, and the engine runs it on the predicate path over the rows its composed query selects, once the payload is final. That is the one matched-row read the path already makes for validation and per-row hooks, not a second one.
+
+**Writes that are now refused.** A multi-row write is refused where the same row written alone would be:
+
+- a predicate update under a policy that declares only `check`, when any matched row's new image fails that check;
+- a predicate update that moves a matched row out of a policy's `using` when no applicable policy declares `check`. The `using` is the defaulted check, and this is the answer the by-id update already gives;
+- an array insert when any row fails the check, including every configuration that already refused each single insert (a `using` or `check` that does not compile).
+
+**What does not change.** A by-id update and a single-row insert are judged exactly as before. A predicate update still touches only the rows its scoped `where` selects; the check refuses a write, it never widens or narrows which rows are selected. A predicate update whose new rows all pass is admitted as before. A system-context write is not gated.
+
+**Hosts that run the security plugin with their own write executor.** A predicate update that installs the judgement and returns without the engine having run it is now refused, as an insert already is: `403`, and an `error` log saying the check was not evaluated.

.changeset/rls-check-defaults-to-using.md

@@ -24,5 +24,5 @@ The published contract has always said this. `RowLevelSecurityPolicySchema.check
 - If any applicable policy declares `check`, only the declared checks decide, exactly as before. A policy with only a `using` alongside them adds nothing to the check.
 - The platform's ownership floor (`owner_only_writes`) is part of a defaulted check only when the by-id write gate kept it for that write. A record share at edit depth, a `public_read_write` object, or a covering controlled-by-parent master gate still replaces the floor. Those writes are not refused again on the new row.
 - `select` policies never gate a write's new row.
-- Bulk updates without a single id are still scoped by the `using` where clause and are not checked row by row.
+- Bulk updates without a single id are still scoped by the `using` where clause. Their new rows are now checked row by row as well, by the separate multi-row entry (#19950).
 - The `modifyAllRecords` bypass on private and platform-global objects still skips the check.

packages/objectql/src/engine.ts

@@ -2224,8 +2224,16 @@ export interface OperationContext {
    * against `opCtx.data`, and {@link ObjectQL.insert} calls it once the
    * `beforeInsert` chain has produced the row — before the first producer with
    * a side effect (the secret channel, the autonumber, the statement), so a
-   * refusal still costs nothing. `update` needs no seam: that path already
-   * merges its pre-image with the change set, which is the same proposition.
+   * refusal still costs nothing. An ARRAY insert is one operation: every live
+   * row is judged in the same call.
+   *
+   * [#19950] A PREDICATE `update` (`multi: true`, no row address) uses the same
+   * seam. The middleware cannot know which rows the write will change: they
+   * are the rows the COMPOSED AST selects, and that AST is complete only after
+   * every middleware has run. So {@link ObjectQL.update} calls it on that path,
+   * once the payload is final, with every matched row merged with the payload.
+   * A by-id `update` is never handed to the seam: the middleware judges that
+   * one row itself, by merging its pre-image with the change set.
    *
    * ABSENT is the ordinary state — no enforcement layer is mounted, or the
    * write is one it does not gate. The engine never invents one.
@@ -2237,11 +2245,12 @@ export interface OperationContext {
  * [#16608] The judgement {@link OperationContext.postHookWriteImageCheck}
  * carries, and the acknowledgement its installer reads back.
  *
- * `evaluate` receives the rows exactly as the `beforeInsert` chain left them —
- * the images the driver is about to be handed — and REFUSES by throwing. It is
- * called at most once per operation, and only for rows still live (a row the
- * declared-field door culled from a partial batch is never judged: it will not
- * be written).
+ * `evaluate` receives the images the driver is about to store, and REFUSES by
+ * throwing. On an `insert` those are the rows exactly as the `beforeInsert`
+ * chain left them, only the live ones (a row the declared-field door culled
+ * from a partial batch is never judged: it will not be written). On a
+ * predicate `update` they are the matched rows, each merged with the final
+ * payload. It is called at most once per operation.
  *
  * `honoured` is set by the engine immediately before `evaluate` runs. It exists
  * so the installer can fail CLOSED on a seam that was never called: an
@@ -13222,6 +13231,56 @@ export class ObjectQL implements IObjectQLEngine {
                // caller is told before N rows are written with a column missing
                // — the failure mode a bulk write makes N times larger.
                assertNoStrictDrops();
+               // ── [#19950] The post-image seam on the PREDICATE path ─────────
+               //
+               // An enforcement layer's write `check` must hold for EVERY row a
+               // write stores (ADR-0058 D4: "and on the AST-injected bulk
+               // path"). For a by-id update the enforcement middleware can judge
+               // the new row itself: it knows the one row and reads it. For a
+               // predicate update it cannot: the rows are the ones the
+               // middleware-COMPOSED AST selects, and that AST is complete only
+               // once every middleware has run (the enforcement layer's own
+               // scope, a sharing layer's editable-rows filter, the tenant
+               // wall). So the layer installs its judgement on
+               // `opCtx.postHookWriteImageCheck`, as it does for an insert, and
+               // the engine hands it the rows here.
+               //
+               // Each image is one matched row merged with the payload, the
+               // row `updateMany` is about to produce, and it is the same shape
+               // the per-row `afterUpdate` context calls `result`
+               // (`buildPerRowAfterContexts`). The rows come from the D7 read,
+               // the one `readPriorRows` memo that validation, the
+               // `readonlyWhen` strip and both hook phases share, bound to the
+               // same composed AST the statement binds. That is the one read the
+               // ruling allows, never a second fetch.
+               //
+               // Placement: the payload is FINAL here. The per-row
+               // `beforeUpdate` chain, the hand-back, both readonly strips and
+               // the strict-drop refusal have all run, and nothing below
+               // changes a value before the statement. The seam judges the rows
+               // that will be stored, which is the rule the insert seam was
+               // held to. The credential channel (`encryptSecretFields`) runs
+               // above on this branch, so a refused write that carried a secret
+               // field has already minted its `sys_secret` row. A validation
+               // refusal two lines down already pays the same cost here, and
+               // moving that channel is a separate change. A `check` naming a
+               // secret field judges the stored reference.
+               //
+               // `honoured` is set BEFORE `evaluate`, exactly as on the insert
+               // path: it answers "did the seam run", never "did the write
+               // pass". Zero matched rows is an empty judgement, not a skipped
+               // one.
+               const predicateImageCheck = opCtx.postHookWriteImageCheck;
+               if (predicateImageCheck) {
+                   predicateImageCheck.honoured = true;
+                   const matchedRows = (await readPriorRows()) ?? [];
+                   const payload = hookContext.input.data as Record<string, unknown>;
+                   await predicateImageCheck.evaluate(
+                     matchedRows.map(
+                       (row) => coerceBooleanFields(updateSchema as any, { ...row, ...payload } as any) as Record<string, unknown>,
+                     ),
+                   );
+               }
                // [#3106] Same enforcement the single-id branch runs at its
                // `evaluateValidationRules` call, applied per matched row: any
                // error-severity violation rejects the WHOLE batch before

packages/plugins/plugin-security/src/check-only-write-scope.test.ts

@@ -289,12 +289,23 @@ function makeEngine() {
     },
     // Both write verbs open with the PRODUCER's own dispatch predicate
     // (`assertEngine*Dispatch`), never a hand-mirrored guard.
-    async update(object: string, data: any, options?: any) {
+    //
+    // [#19950] `opCtx` is the operation the middleware chain ran on, as the
+    // real engine holds it. On the PREDICATE path the engine hands an
+    // installed write-image check every matched row merged with the payload
+    // before it writes, so this double does the same: a double that skipped
+    // it would be refused, fail-closed, by the security middleware.
+    async update(object: string, data: any, options?: any, opCtx?: any) {
       const dispatch = assertEngineUpdateDispatch(data, options);
       const rows = (tables[object] ??= []);
       const targets = dispatch.kind === 'by-id'
         ? rows.filter((r) => r.id === dispatch.id)
         : rows.filter((r) => matches(r, options?.where));
+      const seam = dispatch.kind === 'by-id' ? undefined : opCtx?.postHookWriteImageCheck;
+      if (seam) {
+        seam.honoured = true;
+        await seam.evaluate(targets.map((r) => ({ ...r, ...data })));
+      }
       for (const r of targets) Object.assign(r, data);
       return dispatch.kind === 'by-id' ? (targets[0] ?? null) : targets.length;
     },
@@ -379,7 +390,7 @@ async function makeStack(opts: { orgScoping?: boolean } = {}): Promise<Stack> {
         await sharingMw(opCtx, async () => {
           if (opCtx.operation === 'delete') await engine.delete(opCtx.object, opCtx.options);
           else if (opCtx.operation === 'insert') await engine.insert(opCtx.object, opCtx.data);
-          else await engine.update(opCtx.object, opCtx.data, opCtx.options);
+          else await engine.update(opCtx.object, opCtx.data, opCtx.options, opCtx);
           reached = true;
         });
       });
@@ -594,11 +605,13 @@ describe('[#8059 site 1] Layer 1 actually DERIVES for a check-only policy — th
   let stack: Stack;
   beforeEach(async () => { stack = await makeStack(); });
 
-  it('the bulk UPDATE path touches only READABLE rows — a path step 3.6 explicitly declines to check', async () => {
-    // Step 3.6 logs "not post-image validated" and skips for a write with no
-    // single id, so belt 2 contributes NOTHING here by its own construction.
+  it('the bulk UPDATE path touches only READABLE rows — a path step 3.6 can refuse but never scope', async () => {
+    // Since #19950 step 3.6 judges every row a bulk update stores, but a check
+    // can only REFUSE a write; it cannot choose which rows the write touches.
     // The only thing that can scope this write is Layer 1 injected into the
-    // AST — i.e. the derivation. On a site-1 revert both rows are rewritten.
+    // AST — i.e. the derivation. On a site-1 revert the match set grows to the
+    // other contributor's row, whose new image fails the owner check, so the
+    // whole write is refused and the caller's own row is not rewritten either.
     const opCtx: any = {
       object: 'qa_invoice',
       operation: 'update',
@@ -609,7 +622,7 @@ describe('[#8059 site 1] Layer 1 actually DERIVES for a check-only policy — th
     };
     const securityMw = stack.engine._middlewares[0];
     await securityMw(opCtx, async () => {
-      await stack.engine.update(opCtx.object, opCtx.data, { ...opCtx.options, where: opCtx.ast.where, multi: true });
+      await stack.engine.update(opCtx.object, opCtx.data, { ...opCtx.options, where: opCtx.ast.where, multi: true }, opCtx);
     });
     expect(stack.rows('qa_invoice').find((r) => r.id === INVOICE_C2.id)?.subject).toBe('bulk-edit');
     expect(
@@ -632,7 +645,7 @@ describe('[#8059 site 1] Layer 1 actually DERIVES for a check-only policy — th
     };
     const securityMw = stack.engine._middlewares[0];
     await securityMw(opCtx, async () => {
-      await stack.engine.update(opCtx.object, opCtx.data, { ...opCtx.options, where: opCtx.ast.where, multi: true });
+      await stack.engine.update(opCtx.object, opCtx.data, { ...opCtx.options, where: opCtx.ast.where, multi: true }, opCtx);
     });
     expect(stack.rows('qa_invoice').find((r) => r.id === INVOICE_C2.id)?.subject).toBe('own-bulk-edit');
   });