fix(plugin-security, objectql)!: a by-id update's row-level check holds for the row it stores

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

@@ -1,3 +1,3 @@
 ---
 "@objectstack/plugin-security": minor
 "@objectstack/objectql": minor
@@ -26,7 +26,7 @@
 
 **What does not change.**
 
-- A by-id update and a single-row insert are judged exactly as before.
+- A single-row insert is judged exactly as before. A by-id update is not changed by this entry; its judgement on the row it stores is its own entry (#19989).
 - 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/19989-by-id-update-post-hook-check.md

@@ -0,0 +1,31 @@
+---
+"@objectstack/plugin-security": minor
+"@objectstack/objectql": minor
+---
+
+fix(plugin-security, objectql)!: a by-id update's row-level `check` now holds for the row it stores, after the `beforeUpdate` chain (#19989)
+
+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 change the hook, the policy's `check`, or the data. -->
+
+**BREAKING**: this narrows the set of writes the write gate accepts. A by-id update that is admitted today can be refused after this change. It ships as `minor` under the launch-window convention, as the multi-row check did (#19950).
+
+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. An insert and a predicate update are judged on the row the driver stores. A by-id update was judged only on the change set as the caller sent it, merged with the stored row, before the `beforeUpdate` chain ran. A value a hook wrote into a checked field after that point was never judged, so the row it produced could be stored outside the policy.
+
+A by-id update is now also judged on the row it stores: the prior row merged with the final payload, after the `beforeUpdate` chain and both readonly strips, before the statement. The engine (`@objectstack/objectql`) runs that judgement through the seam the insert and predicate update already use (`OperationContext.postHookWriteImageCheck`). The existing judgement of the change set as sent stays, so this change only ever refuses more.
+
+**Writes that are now refused.** Each refusal is the existing row-level CHECK denial, `403 PERMISSION_DENIED`, and nothing is stored. There is no transition switch.
+
+- **A by-id update whose `beforeUpdate` chain writes a checked field to a value the check refuses**, including a value derived from a field the caller changed.
+- **A by-id 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 and a predicate update already are, with an `error` log saying the check was not evaluated.
+- **An update whose payload `id` addresses no row while `where.id` addresses one**, under a policy with a `check`. The engine writes the `where.id` row while the gate had judged the payload id. It used to be written and then refused; it is now refused before anything runs.
+
+**Remedy.** A hook that must store a value the caller's `check` refuses does so in a separate write under a system context, or the policy declares a `check` that admits it. Otherwise fix the data the write carries. For the last case, address the row with one id: `update(object, { id, ...fields })` or `update(object, fields, { where: { id } })`.
+
+**What does not change.**
+
+- A by-id update whose hooks leave the checked fields inside the check is admitted as before.
+- A change set the check refuses as sent is refused as before, even when a hook would have replaced the refused value.
+- Inserts and predicate updates are judged exactly as before.
+- A system-context write is not gated.

.changeset/insert-check-post-image.md

@@ -1,3 +1,3 @@
 ---
 "@objectstack/plugin-security": minor
 "@objectstack/objectql": minor
@@ -18,7 +18,7 @@
 
 Ruled 2026-09-07 (maintainer, verbatim 「同意」, director seat, summon #17, decision batch #3). The refused alternative — keep the order and write the contract that a checked field must arrive from the caller, plus an `os validate` rule to police it — institutionalises the contradiction and needs a permanent lint to hold it in place.
 
-**What changed, mechanically.** `OperationContext` gains `postHookWriteImageCheck` (`@objectstack/objectql`), an optional judgement an enforcement layer installs and `ObjectQL.insert` runs once the `beforeInsert` chain has produced the row — after the post-hook declared-field door, after the two value-changing strips (`stripRuntimeOwnedFields` and the static-`readonly` strip with its `defaultValue` re-default, both moved ahead of it), and before every producer with a side effect (the secret channel, the autonumber, validation, the statement), so a refusal still costs nothing. `@objectstack/plugin-security` installs its compiled `check` filter there for `insert` instead of matching it against `opCtx.data`; `update` is unchanged. The compiled filter is still built in the middleware, where the caller's permission sets, the ADR-0090 D10 delegator's, the staged membership and the request context are all resolved — only the IMAGE is deferred. A middleware that installed the judgement and finds the seam was never run refuses the write and logs at ERROR: an unjudged write is not an allowed one.
+**What changed, mechanically.** `OperationContext` gains `postHookWriteImageCheck` (`@objectstack/objectql`), an optional judgement an enforcement layer installs and `ObjectQL.insert` runs once the `beforeInsert` chain has produced the row — after the post-hook declared-field door, after the two value-changing strips (`stripRuntimeOwnedFields` and the static-`readonly` strip with its `defaultValue` re-default, both moved ahead of it), and before every producer with a side effect (the secret channel, the autonumber, validation, the statement), so a refusal still costs nothing. `@objectstack/plugin-security` installs its compiled `check` filter there for `insert` instead of matching it against `opCtx.data`; this entry leaves `update` unchanged, and the predicate and by-id updates move onto the same seam in their own entries (#19950, #19989). The compiled filter is still built in the middleware, where the caller's permission sets, the ADR-0090 D10 delegator's, the staged membership and the request context are all resolved — only the IMAGE is deferred. A middleware that installed the judgement and finds the seam was never run refuses the write and logs at ERROR: an unjudged write is not an allowed one.
 
 **Who is affected.** Only objects governed by a permission set that EXPLICITLY declares `check`, on single-row inserts by a non-system caller — the gate's existing scope, unchanged. Two behaviour changes to expect, and they are the two halves of the same correction: an insert that left a hook-stamped field off the payload now succeeds where it used to be refused, and an insert whose hook-stamped field lands outside the caller's scope is now refused where it used to be admitted. Callers that were duplicating the stamp to get past the gate keep working and may stop.
 

packages/objectql/src/engine.ts

@@ -2235,8 +2235,12 @@ export interface OperationContext {
    * 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.
+   *
+   * [#19989] A by-id `update` uses it too. A middleware can read that one row,
+   * but only BEFORE the `beforeUpdate` chain runs, so its image is the change
+   * set as sent and a hook that rewrites a judged field is never judged. So
+   * {@link ObjectQL.update} calls it on the by-id path as well, once the
+   * payload is final, with the prior row merged with the payload.
    *
    * ABSENT is the ordinary state — no enforcement layer is mounted, or the
    * write is one it does not gate. The engine never invents one.
@@ -2253,7 +2257,8 @@ export interface OperationContext {
  * 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.
+ * payload; on a by-id `update` ([#19989]) the one prior row 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
@@ -13420,6 +13425,41 @@ export class ObjectQL implements IObjectQLEngine {
                // "you sent a read-only field" should not depend on whether some
                // other field also failed a business rule.
                assertNoStrictDrops();
+               // ── [#19989] The post-image seam on the BY-ID path ─────────────
+               //
+               // The by-id twin of the predicate-path call below, placed at the
+               // same point and for the same reason: the payload is FINAL here.
+               // The `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.
+               //
+               // An enforcement layer used to judge this row only in its own
+               // middleware, on its read of the row merged with the change set
+               // AS SENT. That image is taken before `next()` runs the hooks, so
+               // a `beforeUpdate` that rewrote a checked field (a scoping column
+               // derived from a re-pointed parent, a status derived from another
+               // field) was never judged, and the row it produced was stored
+               // unjudged. The layer keeps that earlier judgement and installs
+               // this seam as well, so the row the driver stores is judged too.
+               //
+               // The image is the prior row (read once, above, under the
+               // not-found gate, so it is present) merged with the final
+               // payload: the row `driver.update` is about to produce, and the
+               // shape the predicate path hands over per matched row. The
+               // credential channel runs above on this branch too, so a `check`
+               // naming a secret field judges the stored reference, as on the
+               // predicate path.
+               //
+               // `honoured` is set BEFORE `evaluate`: it answers "did the seam
+               // run", never "did the write pass".
+               const byIdImageCheck = opCtx.postHookWriteImageCheck;
+               if (byIdImageCheck) {
+                   byIdImageCheck.honoured = true;
+                   const payload = hookContext.input.data as Record<string, unknown>;
+                   await byIdImageCheck.evaluate([
+                     coerceBooleanFields(updateSchema as any, { ...priorRecord, ...payload } as any) as Record<string, unknown>,
+                   ]);
+               }
                // [#18682] The reference FK a predicate traverses may come from
                // the PATCH or from the stored row, so the id is read off the
                // POST-strip merged view `evaluateValidationRules` evaluates.
@@ -13629,9 +13669,11 @@ export class ObjectQL implements IObjectQLEngine {
                //
                // 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
+               // path"). For a by-id update the enforcement middleware knows the
+               // one row, but can read it only before the hooks run, so that
+               // path hands the final row to the same seam ([#19989], the by-id
+               // branch above). For a predicate update the middleware cannot
+               // even name the rows: they 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

packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts

@@ -61,6 +61,7 @@
 import { describe, it, expect, vi } from 'vitest';
 import { SysUser } from '@objectstack/platform-objects/identity';
 import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security';
+import { assertEngineUpdateDispatch } from '@objectstack/objectql';
 import {
   registerIdentityWriteGuard,
   registerManagedUpdateWhitelist,
@@ -204,9 +205,31 @@ async function route(
     context,
   };
 
+  // [#19989] The engine's half of the write gate, which the terminal `next()`
+  // stands in for. `ObjectQL.update` runs the installed
+  // `postHookWriteImageCheck` on the row a by-id update writes (the stored row
+  // merged with the payload) before the statement; a terminal that skipped it
+  // would be refused fail-closed by the middleware, and would read here as a
+  // `row-scope` refusal. The row comes straight from the fixture, NOT through
+  // `findOne`, so `preImageWheres` still records only the middleware's reads.
+  // The guard below runs after this, so the image carries the pre-guard
+  // payload; the check this file composes (`sys_user_self`, whose `using` is
+  // the check: `id == current_user.id`) reads only `id`, which the guard never
+  // touches. Only the by-id path is modelled, through the producer's own
+  // dispatch predicate.
+  const runByIdWriteImageCheck = async () => {
+    const seam = opCtx.postHookWriteImageCheck;
+    if (!seam || operation !== 'update') return;
+    const dispatch = assertEngineUpdateDispatch(opCtx.data, opCtx.options);
+    if (dispatch.kind !== 'by-id') return;
+    seam.honoured = true;
+    const row = ROWS[String(dispatch.id)];
+    await seam.evaluate(row ? [{ ...row, ...opCtx.data }] : []);
+  };
+
   const snapshotWrites: Array<{ key: string; value: any }> = [];
   try {
-    await middleware(opCtx, async () => {});
+    await middleware(opCtx, runByIdWriteImageCheck);
   } catch (error: any) {
     return {
       // The pre-image re-read is the first engine call past the CRUD gate, so

packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts

@@ -231,12 +231,23 @@ function makeEngine(opts: { findOneThrows?: boolean } = {}) {
     },
     // Both write verbs open with the PRODUCER's own dispatch predicate
     // (#4550 / #5480 / #6277), never a hand-mirrored guard.
-    async update(object: string, data: any, options?: any) {
+    //
+    // [#19989] `opCtx` is the operation the middleware chain ran on. The engine
+    // runs an installed write-image check on every update before it writes
+    // (the rows it will store, each merged with the payload), so this double
+    // does too: one 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 = 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;
     },
@@ -303,7 +314,7 @@ async function makeStack(engineOpts: { findOneThrows?: boolean } = {}): Promise<
       let reached = false;
       try {
         await securityMw(opCtx, async () => {
-          await engine.update(opCtx.object, opCtx.data, opCtx.options);
+          await engine.update(opCtx.object, opCtx.data, opCtx.options, opCtx);
           reached = true;
         });
       } catch (e: any) {

packages/plugins/plugin-security/src/authz-matrix-gate.test.ts

@@ -46,6 +46,7 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { derivePosture, POSTURE_RANK } from '@objectstack/core';
+import { assertEngineUpdateDispatch } from '@objectstack/metadata-core';
 import { SecurityPlugin, hasPlatformAdminCapability } from './security-plugin.js';
 import { PermissionEvaluator } from './permission-evaluator.js';
 import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './objects/default-permission-sets.js';
@@ -154,7 +155,26 @@ function makeHarness(opts: {
       return services[name];
     },
   };
-  return { ctx, findOne, run: async (opCtx: any) => { await middleware(opCtx, async () => {}); return opCtx; } };
+  // [#19989] The engine's stored-row check on a by-id UPDATE, which the empty
+  // terminal stands in for: `ObjectQL.update` runs the installed
+  // `postHookWriteImageCheck` on the row it writes (here the stubbed pre-image,
+  // merged with the payload) before the statement, and a terminal that skipped
+  // it would be refused fail-closed by the middleware. Only the by-id path is
+  // modelled, through the producer's own dispatch predicate.
+  const runByIdWriteImageCheck = async (opCtx: any) => {
+    const seam = opCtx?.postHookWriteImageCheck;
+    if (!seam || opCtx.operation !== 'update') return;
+    const dispatch = assertEngineUpdateDispatch(opCtx.data, opCtx.options);
+    if (dispatch.kind !== 'by-id') return;
+    seam.honoured = true;
+    const prior = opts.findOneImpl ? opts.findOneImpl({ where: { id: dispatch.id } }) : null;
+    await seam.evaluate(prior ? [{ ...prior, ...opCtx.data }] : []);
+  };
+  return {
+    ctx,
+    findOne,
+    run: async (opCtx: any) => { await middleware(opCtx, () => runByIdWriteImageCheck(opCtx)); return opCtx; },
+  };
 }
 
 /** Effective READ filter the engine would AND onto a `find` (the visible-row set). */

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

@@ -293,15 +293,16 @@ function makeEngine() {
     // [#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.
+    // before it writes, and [#19989] on the BY-ID path the one row it writes,
+    // merged with the payload. This double does the same on both: 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;
+      const seam = opCtx?.postHookWriteImageCheck;
       if (seam) {
         seam.honoured = true;
         await seam.evaluate(targets.map((r) => ({ ...r, ...data })));