fix(plugin-security, objectql): a row-level check holds for every row of an array insert and a predicate update

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

@@ -0,0 +1,32 @@
+---
+"@objectstack/plugin-security": minor
+"@objectstack/objectql": minor
+---
+
+fix(plugin-security, objectql)!: 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)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) an enforcement change on the write gate: no authorable key, spelling or stored shape moves, so a stored `sys_metadata` row needs no conversion and an upgrader has nothing to hand-edit. The remedy for a newly refused write is to declare `check` on the policy, or to fix the data. -->
+
+**BREAKING**: this narrows the set of writes the write gate accepts. A multi-row write that is admitted today can be refused after this change. It ships as `minor` under the launch-window convention, as the using-defaulted check did (#19942).
+
+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 a by-id update, but not for the two multi-row write shapes. An **array insert** (`insert(object, [rows])`, which the create-many data route calls) installed no check, so its rows were stored unjudged. A **predicate update** (`update(object, changes, { where, multi: true })`) never judged its new rows. 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. An array insert judges each row on the image the `beforeInsert` chain produced. A predicate update judges each row the write selects on its new image: the matched row merged with the final payload. The engine (`@objectstack/objectql`) supplies those rows through the seam the insert check already uses (`OperationContext.postHookWriteImageCheck`). It runs the judgement on the predicate path over the rows its composed query selects, reusing the matched-row read that path already makes.
+
+**Writes that are now refused.** Each refusal is the existing row-level CHECK denial, `403 PERMISSION_DENIED`, and nothing is stored. One failing row refuses the whole write. There is no transition switch.
+
+- **A predicate update under a policy that declares `check`**, when any matched row's new image fails that check, including when the policy has no `using` at all.
+- **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; a by-id update already gives this answer.
+- **An array insert** when any row fails the check. This includes every configuration that already refused each single insert, such as a `using` or `check` that does not compile.
+- **A predicate update on a host that installs the judgement and never runs it**, for example a custom write executor in place of the engine. It is refused as an insert already is, with an `error` log saying the check was not evaluated.
+
+**Remedy.** To let a write store a row outside a policy's scope, declare a `check` on that policy that admits it; otherwise fix the data the write carries.
+
+**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 changes which rows are selected.
+- A predicate update or array insert whose rows all pass is admitted as before.
+- A system-context write is not gated.

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

@@ -1,3 +1,3 @@
 ---
 "@objectstack/plugin-security": minor
 ---
@@ -24,5 +24,5 @@
 - 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');
   });

packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts

@@ -164,7 +164,7 @@ const attempt = async (run: () => Promise<unknown>): Promise<Outcome> => {
   }
 };
 
-/** ⚠️ A SINGLE object, never an array: step 3.6 skips a bulk payload. */
+/** A single-row insert; the array shape is pinned in `rls-check-multi-row-writes.test.ts`. */
 const insert = (engine: ObjectQL, row: Record<string, unknown>) =>
   engine.insert('qa_ticket', { id: 't1', title: 't', ...row } as never, { context: CALLER } as never);