From 677eeb04681faecfb5d4357b62a4f984cb62695f Mon Sep 17 00:00:00 2001
From: os-zhuang <zhuangjianguo@steedos.com>
Date: Mon, 3 Aug 2026 09:37:32 +0000
Subject: [PATCH] fix(approvals): hold the record lock for predicate (multi)
 updates (#4778)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The ADR-0019 record lock only ran for updates carrying an `input.id`, which the
engine extracts from a SCALAR `where.id` alone. Any other predicate is a
multi-row write that routes to `updateMany` and reached the hook with no id, so
`if (!id) return` read "no row was resolved" as "there is nothing to authorize"
when the truth was "nothing was ever queried" — the #4757 / #4630 fail-open
shape, here reachable with no privilege at all: rewriting the same edit as
`multi: true` bypassed the lock without admin, `isSystem`, `lockRecord: false`
or a whitelisted field.

The hook now resolves the rows a write touches before deciding. By-id writes are
unchanged. A predicate write is decided by intersecting the caller's predicate
with the object's LOCKED records, so the query is bounded by pending approvals
rather than by the update's match set; an unscoped whole-table `multi` update
reaches every locked row and is refused while any is held. Past 1 000 locked
records, or if the intersection query fails, the write fails closed.

Every exemption moves with the guard — `isSystem`, admin, the
`approvalStatusField` mirror, `lockRecord: false` and the owning run's
`flowRunId` (#3456 / #3712) — each pinned on both predicate shapes, plus a
real-engine integration test that reproduces the issue's three lines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
---
 .../approval-record-lock-predicate-updates.md |  57 ++++
 .../src/approval-service.test.ts              | 219 +++++++++++++++
 .../plugin-approvals/src/lifecycle-hooks.ts   | 261 ++++++++++++++++--
 ...cord-lock-multi-update.integration.test.ts | 248 +++++++++++++++++
 4 files changed, 767 insertions(+), 18 deletions(-)
 create mode 100644 .changeset/approval-record-lock-predicate-updates.md
 create mode 100644 packages/plugins/plugin-approvals/src/record-lock-multi-update.integration.test.ts

diff --git a/.changeset/approval-record-lock-predicate-updates.md b/.changeset/approval-record-lock-predicate-updates.md
new file mode 100644
index 0000000000..96b821abb3
--- /dev/null
+++ b/.changeset/approval-record-lock-predicate-updates.md
@@ -0,0 +1,57 @@
+---
+"@objectstack/plugin-approvals": patch
+---
+
+fix(approvals): the record lock now holds for predicate (`multi`) updates (#4778)
+
+The ADR-0019 record lock — "while a record has a pending `sys_approval_request`,
+block edits to it" — was enforced only for updates that reach the hook with an
+`input.id`. The engine extracts that id from a **scalar** `where.id` alone; an
+operator object (`{ $in: [...] }`) or any other predicate is a multi-row write
+that routes to `updateMany` and arrives with no id. The hook opened with
+`if (!id) return`, so it read *"no row was resolved"* as *"there is nothing to
+authorize"* when the truth was *"nothing was ever queried"*.
+
+Rewriting the very same edit as `multi: true` therefore walked straight past the
+lock:
+
+```ts
+// rec_1 carries a pending approval, lockRecord is not disabled
+await ql.update('crm_opportunity', { amount: 999 }, { where: { id: 'rec_1' } });                       // RECORD_LOCKED
+await ql.update('crm_opportunity', { amount: 999 }, { where: { id: { $in: ['rec_1'] } }, multi: true }); // went through
+await ql.update('crm_opportunity', { amount: 999 }, { where: { name: 'x' }, multi: true });              // went through
+```
+
+No privilege was needed for that bypass — not an `admin` role, not `isSystem`,
+not `lockRecord: false`, not a whitelisted `approvalStatusField`. Every caller
+shape that can spell a predicate (SDK, ObjectQL, a flow's `update_record`) could
+produce it. It is the same fail-open reasoning fixed for `sys_attachment`
+(#4757) and `sys_comment` (#4630), in the one place where it needed no
+privilege at all.
+
+**The hook now resolves the rows a write touches before deciding.** By-id writes
+are unchanged (the driver writes by primary key, so the rest of `where` must not
+narrow the verdict). A predicate write is decided by intersecting the caller's
+predicate with the records that are actually locked — which is also what keeps
+it cheap: the query is bounded by the object's **pending approvals**, never by
+the update's match set, so a mass update of 50 000 unlocked rows costs one
+bookkeeping probe and is allowed. An unscoped `multi` update over the whole
+table reaches every locked row of the object and is refused while any is held.
+
+**Fail-closed, both ways.** Past 1 000 locked records — the bound the attachment
+and comment guards use — or if the intersection query fails, the write is
+refused rather than allowed: the lock could not prove the write misses a locked
+row. The approvals bookkeeping being unreadable at all stays the one fail-open,
+as before: this hook is global over every object, so a kernel without
+`sys_approval_request` would otherwise refuse every update in the deployment.
+Both the bookkeeping and the match-set resolution are read under a **system**
+context — a guard's own input must never be narrowed by the caller's
+visibility, since a locked row you cannot read is still a row you may not write.
+
+**Every exemption moved with the guard**, which is the other way this class of
+fix goes wrong — a guard extended to more rows that carries only its deny rules
+turns a fail-open into a false-positive. `isSystem`, the `admin` override, the
+`approvalStatusField` status mirror, `lockRecord: false` and the owning run's
+`flowRunId` (#3456 / #3712) all decide a predicate write exactly as they decide
+a by-id write, each pinned by tests on both predicate shapes. Refusals now name
+the record and object that are locked.
diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts
index 8c0a0d559f..58a91d8a7a 100644
--- a/packages/plugins/plugin-approvals/src/approval-service.test.ts
+++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts
@@ -27,6 +27,12 @@ function makeFakeEngine() {
         if (!(v as any[]).some(sub => matches(row, sub))) return false;
         continue;
       }
+      // The record lock intersects the caller's predicate with the locked ids
+      // (#4778), so the fake has to compose branches the way a driver does.
+      if (k === '$and') {
+        if (!(v as any[]).every(sub => matches(row, sub))) return false;
+        continue;
+      }
       const rv = row[k];
       if (v != null && typeof v === 'object' && '$in' in (v as any)) {
         if (!(v as any).$in.includes(rv)) return false;
@@ -47,12 +53,16 @@ function makeFakeEngine() {
 
   /** Every `update` the service made, with the context it presented (#3783). */
   const writes: Array<{ object: string; data: any; context: any }> = [];
+  /** Every `find` anyone made — pins what a guard does NOT query (#4778). */
+  const finds: Array<{ object: string; options: any }> = [];
 
   return {
     _tables: tables,
     _hooks: hooks,
     _writes: writes,
+    _finds: finds,
     async find(object: string, options?: any) {
+      finds.push({ object, options });
       const rows = ensure(object).filter(r => matches(r, options?.filter ?? options?.where));
       if (options?.orderBy?.[0]) {
         // Canonical SortNode key only (spec/data/query.zod.ts): a sloppy
@@ -1914,6 +1924,215 @@ describe('record-lock hook (node era)', () => {
   });
 });
 
+// ── #4778: the lock has to survive a PREDICATE (multi) update ────────────
+//
+// `engine.update()` extracts `input.id` only from a SCALAR `where.id`; every
+// other predicate is a multi-row write that routes to `updateMany` and reaches
+// the hook with NO id. The hook used to open with `if (!id) return`, reading
+// "no row was resolved" as "nothing to authorize" when the truth was "nothing
+// was ever queried" (the #4757 / #4630 fail-open shape). Rewriting the very
+// same edit as `multi: true` then walked past the lock with NO privilege at
+// all — no admin, no isSystem, no `lockRecord: false`, no whitelisted field.
+//
+// Both halves are pinned here, because extending a guard to more rows fails
+// the other way just as easily: the refusals AND every exemption, on both
+// predicate shapes.
+describe('record-lock hook — predicate (multi) updates (#4778)', () => {
+  let engine: ReturnType<typeof makeFakeEngine>;
+  let svc: ApprovalService;
+  let n = 0;
+  const baseTime = new Date('2026-01-15T10:00:00Z').getTime();
+
+  /** The two shapes that carry no `input.id` and used to bypass the lock. */
+  const SHAPES: Array<[string, any]> = [
+    ['an id-operator predicate', { id: { $in: ['opp1'] } }],
+    ['a non-id predicate', { stage: 'new' }],
+  ];
+
+  const USER = { isSystem: false, positions: [], userId: 'u1' };
+
+  /** A `multi: true` update, i.e. the ctx the engine builds for `updateMany`. */
+  const predicateUpdate = (
+    where: any,
+    data: Record<string, unknown>,
+    rest: Record<string, unknown> = {},
+  ) =>
+    engine.fire('beforeUpdate', {
+      object: 'opportunity',
+      input: { data, options: { ...(where === undefined ? {} : { where }), multi: true } },
+      session: USER,
+      ...rest,
+    });
+
+  /** Re-open the pending request with a different node-config snapshot. */
+  const reopenWith = async (configExtra: Record<string, any>) => {
+    engine._tables['sys_approval_request'] = [];
+    engine._tables['sys_approval_action'] = [];
+    await svc.openNodeRequest(
+      openInput(['u9'], {}, { approvalStatusField: 'approval_status', ...configExtra }),
+      CTX,
+    );
+  };
+
+  beforeEach(async () => {
+    engine = makeFakeEngine();
+    n = 0;
+    svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
+    bindApprovalLockHook(engine as any);
+    await svc.openNodeRequest(openInput(['u9'], {}, { approvalStatusField: 'approval_status' }), CTX);
+    // `opp1` carries the pending request; `opp2` is an unlocked neighbour that
+    // the same predicates also match.
+    engine._tables['opportunity'] = [
+      { id: 'opp1', amount: 100, stage: 'new' },
+      { id: 'opp2', amount: 100, stage: 'new' },
+    ];
+  });
+
+  // ── the hole itself ───────────────────────────────────────────────
+
+  it.each(SHAPES)('blocks %s that reaches the locked record', async (_label, where) => {
+    await expect(predicateUpdate(where, { amount: 999 })).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('blocks an unscoped whole-table update — no predicate at all', async () => {
+    // `updateMany` gets an AST of `{ object }`, i.e. every row, so every locked
+    // row of the object is in reach. "No predicate" is the widest write there
+    // is; it must not be the one that reads as "nothing to authorize".
+    await expect(predicateUpdate(undefined, { amount: 999 })).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('names the locked record and its object in the refusal', async () => {
+    await expect(predicateUpdate({ stage: 'new' }, { amount: 999 }))
+      .rejects.toThrow(/record 'opp1' of 'opportunity' is locked/);
+  });
+
+  // ── and it must not over-block: a lock is a PER-ROW verdict ────────
+
+  it('allows a predicate that reaches only unlocked rows', async () => {
+    await expect(predicateUpdate({ id: { $in: ['opp2'] } }, { amount: 999 })).resolves.toBeUndefined();
+  });
+
+  it('allows a non-id predicate that matches no locked row', async () => {
+    // `opp1` is `stage: 'new'`, so this predicate misses it. Resolving the row
+    // set is what makes the difference between refusing this write and
+    // refusing every bulk update on an object that has any approval open.
+    await expect(predicateUpdate({ stage: 'closed' }, { amount: 999 })).resolves.toBeUndefined();
+  });
+
+  it('never scans an object that has no pending approval at all', async () => {
+    engine._finds.length = 0;
+    await expect(
+      engine.fire('beforeUpdate', {
+        object: 'other_object',
+        input: { data: { amount: 999 }, options: { where: { stage: 'new' }, multi: true } },
+        session: USER,
+      }),
+    ).resolves.toBeUndefined();
+    // One bookkeeping probe, and nothing else: the bound is on locked records,
+    // so a mass update of unlocked rows costs a single query.
+    expect(engine._finds.map(f => f.object)).toEqual(['sys_approval_request']);
+  });
+
+  // ── fail closed when the row set cannot be decided ─────────────────
+
+  it('fails closed past the 1000-record bound', async () => {
+    for (let i = 0; i < 1001; i++) {
+      engine._tables['sys_approval_request'].push({
+        id: `extra_${i}`,
+        object_name: 'opportunity',
+        record_id: `bulk_${i}`,
+        status: 'pending',
+        node_config_json: JSON.stringify({ lockRecord: true }),
+      });
+    }
+    await expect(predicateUpdate({ stage: 'new' }, { amount: 999 }))
+      .rejects.toThrow(/RECORD_LOCKED.*more than 1000/s);
+  });
+
+  it('fails closed when the match set cannot be resolved', async () => {
+    const realFind = engine.find.bind(engine);
+    engine.find = (async (object: string, options?: any) => {
+      if (object === 'opportunity') throw new Error('driver unavailable');
+      return realFind(object, options);
+    }) as typeof engine.find;
+    await expect(predicateUpdate({ stage: 'new' }, { amount: 999 }))
+      .rejects.toThrow(/RECORD_LOCKED.*cannot determine which rows/s);
+  });
+
+  // ── every exemption moves with the guard (the other failure mode) ──
+
+  it.each(SHAPES)('allows engine self-writes (system session) via %s', async (_label, where) => {
+    await expect(
+      predicateUpdate(where, { amount: 999 }, { session: { isSystem: true, positions: [] } }),
+    ).resolves.toBeUndefined();
+  });
+
+  it.each(SHAPES)('allows an admin override via %s', async (_label, where) => {
+    await expect(
+      predicateUpdate(where, { amount: 999 }, { session: { isSystem: false, roles: ['admin'] } }),
+    ).resolves.toBeUndefined();
+  });
+
+  it.each(SHAPES)('allows a status-mirror write via %s', async (_label, where) => {
+    await expect(predicateUpdate(where, { approval_status: 'approved' })).resolves.toBeUndefined();
+  });
+
+  it.each(SHAPES)('allows the OWNING run to write its own target record via %s', async (_label, where) => {
+    await expect(
+      predicateUpdate(where, { amount: 999 }, { provenance: { flowRunId: 'run_1' } }),
+    ).resolves.toBeUndefined();
+  });
+
+  it.each(SHAPES)('allows the write when the node opted out of the lock, via %s', async (_label, where) => {
+    await reopenWith({ lockRecord: false });
+    await expect(predicateUpdate(where, { amount: 999 })).resolves.toBeUndefined();
+  });
+
+  // ── …and the exemptions stay as narrow as on the by-id path ────────
+
+  it('still blocks a DIFFERENT run on the predicate path', async () => {
+    await expect(
+      predicateUpdate({ stage: 'new' }, { amount: 999 }, { provenance: { flowRunId: 'run_other' } }),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('still blocks a mirror write that changes anything else too', async () => {
+    await expect(
+      predicateUpdate({ stage: 'new' }, { approval_status: 'approved', amount: 999 }),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('still blocks an identity-less caller with no provenance at all', async () => {
+    await expect(
+      engine.fire('beforeUpdate', {
+        object: 'opportunity',
+        input: { data: { amount: 999 }, options: { where: { stage: 'new' }, multi: true } },
+      }),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('judges a multi-row write by EACH request it reaches', async () => {
+    // Two records, two independent approvals: one opted out of the lock, one
+    // did not. A predicate spanning both is refused by the one that locks.
+    engine._tables['sys_approval_request'].push({
+      id: 'req_2',
+      object_name: 'opportunity',
+      record_id: 'opp2',
+      status: 'pending',
+      flow_run_id: 'run_2',
+      node_config_json: JSON.stringify({ lockRecord: false }),
+    });
+    await expect(predicateUpdate({ id: { $in: ['opp2'] } }, { amount: 999 })).resolves.toBeUndefined();
+    await expect(predicateUpdate({ id: { $in: ['opp1', 'opp2'] } }, { amount: 999 }))
+      .rejects.toThrow(/record 'opp1'/);
+  });
+
+  it('ignores a request that is no longer pending', async () => {
+    engine._tables['sys_approval_request'][0].status = 'approved';
+    await expect(predicateUpdate({ stage: 'new' }, { amount: 999 })).resolves.toBeUndefined();
+  });
+});
+
 // ── #3456 recovery half: release records held by a dead approval run ──
 //
 // The prevention half above stops a run from dying on its own lock. This sweep
diff --git a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
index 42dc3bab13..80d839147d 100644
--- a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
+++ b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
@@ -12,12 +12,42 @@
  * of an approval node is only known at flow-run time). For each update it:
  *
  *   1. Skips engine self-writes (status mirror) and `sys_approval_*` bookkeeping.
- *   2. Looks up a pending request for `(object, recordId)`.
- *   3. Reads the lock policy from that request's `node_config_json` snapshot:
+ *   2. Resolves **which records the write would touch** and looks up the pending
+ *      requests gating them.
+ *   3. Reads the lock policy from each request's `node_config_json` snapshot:
  *      - `lockRecord === false` → allow.
  *      - otherwise block, EXCEPT when the only changed field is the configured
- *        `approvalStatusField` (so the status mirror is never blocked) or the
- *        caller is an `admin`.
+ *        `approvalStatusField` (so the status mirror is never blocked), the
+ *        caller is an `admin`, or the writer is the run that opened the request
+ *        (`flowRunId`, #3456 / #3712).
+ *
+ * ## Step 2 is per-row, and it covers PREDICATE writes (#4778)
+ *
+ * The engine extracts `input.id` only from a **scalar** `where.id`; an operator
+ * object (`{ $in: [...] }`) or any other predicate is a multi-row write that
+ * routes to `updateMany`, and reaches this hook with **no** `input.id`. The
+ * hook used to open with `if (!id) return`, i.e. it read *"no row was
+ * resolved"* as *"there is nothing to authorize"* when the truth was *"nothing
+ * was ever queried"* — the same fail-open reasoning as #4757 (`sys_attachment`)
+ * and #4630 (`sys_comment`). Rewriting the very same edit as `multi: true` then
+ * bypassed the lock with **no privilege at all**: no admin role, no `isSystem`,
+ * no `lockRecord: false`, no whitelisted field.
+ *
+ * So the hook now resolves the row set the way the attachment/comment guards
+ * do, with one difference the record lock forces: it is a **per-row** guard
+ * ("does THIS record have a pending approval"), not a "should this whole-table
+ * write be refused" guard — so an unscoped bulk update is *not* refused
+ * outright. Instead the resolution is inverted, which is what keeps it cheap:
+ * the bound is on the **pending requests for the object** (of which there are
+ * normally none, and the hook returns after one query), never on the update's
+ * match set. Only when some record of the object *is* locked does the hook ask
+ * the engine which of those locked rows the caller's predicate actually
+ * matches. Past {@link PENDING_LOCK_LIMIT} locked records — or if that
+ * intersection query fails — the write fails **CLOSED**.
+ *
+ * Every exemption above is evaluated on the multi-row path exactly as on the
+ * by-id path: extending a guard to more rows must move the *allow* rules with
+ * the *deny* rules, or fail-open merely becomes false-positive.
  *
  * Registered under `packageId: 'plugin-approvals:lock'` so it can be cleanly
  * unbound on plugin stop.
@@ -50,6 +80,52 @@ function parseJson<T = any>(raw: unknown, fallback: T): T {
   return raw as T;
 }
 
+/**
+ * Bound on the records one write may be authorized against — the sibling of
+ * `sys_attachment`'s `MULTI_DELETE_AUTH_LIMIT` and `sys_comment`'s
+ * `MULTI_WRITE_AUTH_LIMIT` (#4757 / #4630).
+ *
+ * It bounds LOCKED records (pending requests on the object) and ids the caller
+ * names, never the update's match set: a mass update of 50 000 unlocked rows
+ * costs one query and is allowed, while an object carrying more pending
+ * approvals than this cannot be decided row by row and fails CLOSED.
+ */
+const PENDING_LOCK_LIMIT = 1_000;
+
+/**
+ * The approvals bookkeeping — and the row set a predicate write would touch —
+ * are read as SYSTEM. A guard's own input must never be narrowed by the
+ * caller's visibility: a locked row the caller cannot READ is still a locked
+ * row they must not WRITE (the #4630 rule).
+ */
+const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
+
+function lockedError(message: string): never {
+  const err: any = new Error(`RECORD_LOCKED: ${message}`);
+  err.code = 'RECORD_LOCKED';
+  err.statusCode = 409;
+  throw err;
+}
+
+/**
+ * The record ids a write names outright — a scalar id or an `{ $in: [...] }` —
+ * or `null` when the ids cannot be read off it (any other predicate, or an
+ * `$in` carrying a non-scalar, which we refuse to guess at).
+ *
+ * Mirrors `asIdList` in the attachment/comment kits. An empty `$in` legitimately
+ * names *no* record, so it returns `[]` (nothing to gate), not `null`.
+ */
+function asIdList(id: unknown): Array<string | number> | null {
+  if (typeof id === 'number') return [id];
+  if (typeof id === 'string') return id === '' ? null : [id];
+  if (id && typeof id === 'object' && Array.isArray((id as any).$in)) {
+    const raw = (id as any).$in as unknown[];
+    const scalars = raw.filter((v): v is string | number => typeof v === 'string' || typeof v === 'number');
+    return scalars.length === raw.length ? scalars : null;
+  }
+  return null;
+}
+
 /** The pending request gating a record, plus its snapshotted node config. */
 async function pendingRequestFor(
   engine: MinimalEngine,
@@ -60,6 +136,7 @@ async function pendingRequestFor(
     const rows = await engine.find('sys_approval_request', {
       where: { object_name: objectName, record_id: String(recordId), status: 'pending' },
       limit: 1,
+      context: { ...SYSTEM_CTX },
     } as any);
     return Array.isArray(rows) && rows[0] ? rows[0] : null;
   } catch {
@@ -67,14 +144,151 @@ async function pendingRequestFor(
   }
 }
 
+/** Pending requests gating any of the records a write names by id. */
+async function pendingRequestsForRecords(
+  engine: MinimalEngine,
+  objectName: string,
+  recordIds: ReadonlyArray<string | number>,
+): Promise<any[]> {
+  if (recordIds.length === 0) return [];
+  if (recordIds.length > PENDING_LOCK_LIMIT) {
+    lockedError(
+      `refusing to authorize an update naming more than ${PENDING_LOCK_LIMIT} records of '${objectName}' — ` +
+      'the approval lock cannot check them row by row; scope the write',
+    );
+  }
+  if (recordIds.length === 1) {
+    const one = await pendingRequestFor(engine, objectName, String(recordIds[0]));
+    return one ? [one] : [];
+  }
+  try {
+    const rows = await engine.find('sys_approval_request', {
+      where: { object_name: objectName, record_id: { $in: recordIds.map(String) }, status: 'pending' },
+      limit: PENDING_LOCK_LIMIT + 1,
+      context: { ...SYSTEM_CTX },
+    } as any);
+    return Array.isArray(rows) ? rows : [];
+  } catch {
+    return [];
+  }
+}
+
+/**
+ * Every record of `objectName` currently held by a pending approval, capped one
+ * past the bound so the caller can tell "at the cap" from "over it".
+ *
+ * `null` means the bookkeeping could not be read at all. That reads as "no
+ * lock" — deliberately the ONE fail-open left, and the pre-existing behaviour
+ * of {@link pendingRequestFor}: this hook is GLOBAL over every object, so a
+ * kernel where `sys_approval_request` is absent or momentarily unreadable would
+ * otherwise refuse every update in the deployment. The fail-closed decisions
+ * below are the ones an attacker can actually steer (a predicate they choose);
+ * "is the approvals table readable" is not one of them.
+ */
+async function pendingRequestsForObject(
+  engine: MinimalEngine,
+  objectName: string,
+): Promise<any[] | null> {
+  try {
+    const rows = await engine.find('sys_approval_request', {
+      where: { object_name: objectName, status: 'pending' },
+      limit: PENDING_LOCK_LIMIT + 1,
+      context: { ...SYSTEM_CTX },
+    } as any);
+    return Array.isArray(rows) ? rows : [];
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Narrow `candidates` (pending requests) to the ones whose record the caller's
+ * predicate actually matches — asked of the engine as an intersection, so the
+ * query is bounded by the number of LOCKED rows, not by the update's match set.
+ *
+ * A failure here fails CLOSED: we know some record of this object is locked and
+ * we could not prove the write misses it.
+ */
+async function narrowToMatchedRecords(
+  engine: MinimalEngine,
+  objectName: string,
+  where: unknown,
+  candidates: any[],
+): Promise<any[]> {
+  const lockedIds = candidates.map((c) => String(c?.record_id ?? ''));
+  let rows: any[];
+  try {
+    rows = await engine.find(objectName, {
+      where: { $and: [where, { id: { $in: lockedIds } }] },
+      fields: ['id'],
+      limit: lockedIds.length,
+      context: { ...SYSTEM_CTX },
+    } as any);
+  } catch (err) {
+    lockedError(
+      `cannot determine which rows a predicate update on '${objectName}' would touch ` +
+      `(${(err as Error)?.message ?? String(err)}); ${candidates.length} record(s) of it carry a pending ` +
+      'approval, so the write is refused',
+    );
+  }
+  const matched = new Set((Array.isArray(rows) ? rows : []).map((r: any) => String(r?.id)));
+  return candidates.filter((c) => matched.has(String(c?.record_id ?? '')));
+}
+
+/**
+ * The pending requests gating the rows THIS write would touch.
+ *
+ * Four shapes, cheapest first:
+ *   - `input.id` (engine's scalar by-id fast path) → that record only. The
+ *     driver updates by primary key, so the rest of `where` never narrows it —
+ *     narrowing here would be a fail-open.
+ *   - predicate naming ids only (`{ id: { $in: [...] } }`) → those records.
+ *   - predicate with other keys → the object's locked records, intersected with
+ *     the predicate.
+ *   - no predicate at all (`updateMany` over the whole table) → every locked
+ *     record of the object.
+ */
+async function gatingRequests(
+  engine: MinimalEngine,
+  ctx: any,
+  objectName: string,
+): Promise<any[]> {
+  const byId = asIdList(ctx?.input?.id);
+  if (byId) return pendingRequestsForRecords(engine, objectName, byId);
+
+  // Predicate write. `where` is canonical here: the engine folds the `filter`
+  // alias into it before hooks run (PD #12 — no consumer-side `?? filter`).
+  const rawWhere = (ctx?.input?.options as any)?.where;
+  const hasWhere = rawWhere !== undefined && rawWhere !== null;
+  const whereObj = hasWhere && typeof rawWhere === 'object' && !Array.isArray(rawWhere)
+    ? rawWhere as Record<string, unknown>
+    : null;
+
+  const namedIds = whereObj ? asIdList(whereObj.id) : null;
+  const candidates = namedIds
+    ? await pendingRequestsForRecords(engine, objectName, namedIds)
+    : await pendingRequestsForObject(engine, objectName);
+  if (candidates === null) return [];          // bookkeeping unreadable — see the note above
+  if (candidates.length === 0) return [];      // nothing locked in reach
+  if (candidates.length > PENDING_LOCK_LIMIT) {
+    lockedError(
+      `refusing a predicate update on '${objectName}': more than ${PENDING_LOCK_LIMIT} of its records carry a ` +
+      'pending approval, so the lock cannot decide row by row; scope the write to the rows you mean',
+    );
+  }
+  // No predicate → the write touches every row, so every locked row is in reach.
+  if (!hasWhere) return candidates;
+  // The predicate was exactly the id list we already resolved against.
+  if (namedIds && whereObj && Object.keys(whereObj).every((k) => k === 'id')) return candidates;
+  return narrowToMatchedRecords(engine, objectName, rawWhere, candidates);
+}
+
 /**
  * Bind the global record-lock hook. Caller is responsible for calling
  * {@link unbindAllHooks} first if re-binding.
  */
 export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogger): void {
   engine.registerHook('beforeUpdate', async (ctx: any) => {
-    const id = String((ctx?.input?.id ?? '') as string);
-    if (!id) return;
     const object = (ctx?.object ?? ctx?.objectName) as string | undefined;
     // No object name (shouldn't happen) or our own bookkeeping objects → skip.
     if (!object || String(object).startsWith('sys_approval')) return;
@@ -83,6 +297,11 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
     const changedFields = Object.keys(data).filter((k) => k !== 'id' && k !== 'updated_at');
     if (changedFields.length === 0) return;
 
+    // ── Caller-level exemptions. Row-independent, so they are decided once,
+    // before any row is resolved — and they hold identically for a by-id and a
+    // predicate write (#4778: an exemption that only survives on one path turns
+    // a fail-open into a false-positive).
+
     // Allow engine self-writes (status mirror from the approvals service, etc).
     if ((ctx?.session as any)?.isSystem) return;
 
@@ -90,8 +309,9 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
     const roles = (ctx?.session?.roles ?? []) as string[];
     if (Array.isArray(roles) && roles.includes('admin')) return;
 
-    const pending = await pendingRequestFor(engine, object, id);
-    if (!pending) return;
+    // ── Which rows does this write touch, and which of them are locked?
+    const gating = await gatingRequests(engine, ctx, object);
+    if (gating.length === 0) return;
 
     // The run that OPENED this approval may still write its own target record
     // (#3456). Without this the lock cannot tell "the run that owns this pending
@@ -113,19 +333,24 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
     // session, and that shape is the one that used to die on its own lock
     // (#3712).
     const writerRun = (ctx?.provenance as any)?.flowRunId;
-    if (writerRun && pending.flow_run_id && String(writerRun) === String(pending.flow_run_id)) return;
 
-    const config = parseJson<any>(pending.node_config_json, {});
-    if (config?.lockRecord === false) return;
+    // Per-row verdict: the write is refused as soon as ONE touched record is
+    // locked against it. Each request carries its own snapshotted policy, so a
+    // multi-row write spanning two approvals is judged by each of them.
+    for (const pending of gating) {
+      if (writerRun && pending?.flow_run_id && String(writerRun) === String(pending.flow_run_id)) continue;
+
+      const config = parseJson<any>(pending?.node_config_json, {});
+      if (config?.lockRecord === false) continue;
 
-    // Allow when every changed field is the approval status mirror.
-    const mirror = config?.approvalStatusField;
-    if (typeof mirror === 'string' && mirror && changedFields.every((f) => f === mirror)) return;
+      // Allow when every changed field is the approval status mirror.
+      const mirror = config?.approvalStatusField;
+      if (typeof mirror === 'string' && mirror && changedFields.every((f) => f === mirror)) continue;
 
-    const err: any = new Error('RECORD_LOCKED: record is locked while an approval is in progress');
-    err.code = 'RECORD_LOCKED';
-    err.statusCode = 409;
-    throw err;
+      lockedError(
+        `record '${String(pending?.record_id ?? '')}' of '${object}' is locked while an approval is in progress`,
+      );
+    }
   }, { packageId: APPROVALS_HOOK_PACKAGE, priority: 50 });
 
   logger?.info?.('[approvals] record-lock hook bound');
diff --git a/packages/plugins/plugin-approvals/src/record-lock-multi-update.integration.test.ts b/packages/plugins/plugin-approvals/src/record-lock-multi-update.integration.test.ts
new file mode 100644
index 0000000000..e0dfcd000f
--- /dev/null
+++ b/packages/plugins/plugin-approvals/src/record-lock-multi-update.integration.test.ts
@@ -0,0 +1,248 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #4778 — the approvals record lock, through a REAL {@link ObjectQL} engine, on
+ * the write shape that used to walk straight past it.
+ *
+ * `engine.update()` extracts `input.id` only from a **scalar** `where.id`; an
+ * operator object (`{ $in: [...] }`) or any other predicate is a multi-row
+ * write that routes to `updateMany` and reaches `beforeUpdate` with no id at
+ * all. The lock hook opened with `if (!id) return`, so the *same edit* written
+ * as `multi: true` bypassed it — with no admin role, no `isSystem`, no
+ * `lockRecord: false` and no whitelisted field. The bypass cost was "spell the
+ * write differently", which is the worst bypass cost there is.
+ *
+ * The unit cases next to the other lock tests (`approval-service.test.ts`) fire
+ * the hook directly. This file refuses to stub the hop that produced the bug:
+ * the engine decides by-id vs `updateMany`, seeds the AST and builds the hook
+ * context — so it is the engine, not a fake, that hands the hook a write with
+ * no id. Same three lines as the issue's repro.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { ObjectQL } from '@objectstack/objectql';
+import { bindApprovalLockHook } from './lifecycle-hooks.js';
+
+const opportunity = {
+  name: 'opportunity',
+  label: 'Opportunity',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    name: { name: 'name', type: 'text' as const },
+    amount: { name: 'amount', type: 'number' as const },
+    approval_status: { name: 'approval_status', type: 'text' as const },
+  },
+};
+
+/** The lock hook reads pending requests off this object. */
+const approvalRequest = {
+  name: 'sys_approval_request',
+  label: 'Approval Request',
+  fields: {
+    id: { name: 'id', type: 'text' as const, primaryKey: true },
+    object_name: { name: 'object_name', type: 'text' as const },
+    record_id: { name: 'record_id', type: 'text' as const },
+    status: { name: 'status', type: 'text' as const },
+    flow_run_id: { name: 'flow_run_id', type: 'text' as const },
+    node_config_json: { name: 'node_config_json', type: 'text' as const },
+  },
+};
+
+/**
+ * A memory driver that understands the operators this path actually binds:
+ * `$in` (the caller's predicate) and `$and` (the lock's intersection of that
+ * predicate with the locked ids). An unknown operator matches nothing rather
+ * than comparing an object to a scalar — a fixture that silently matched
+ * everything would prove the opposite of what these tests claim.
+ */
+function makeMemoryDriver() {
+  const stores = new Map<string, Map<string, Record<string, unknown>>>();
+  const storeFor = (o: string) => {
+    let s = stores.get(o);
+    if (!s) { s = new Map(); stores.set(o, s); }
+    return s;
+  };
+  let nextId = 0;
+  const matches = (row: Record<string, unknown>, where: any): boolean => {
+    if (!where || typeof where !== 'object') return true;
+    for (const [k, v] of Object.entries(where)) {
+      if (k === '$and') { if (!(v as any[]).every((w) => matches(row, w))) return false; continue; }
+      if (k === '$or') { if (!(v as any[]).some((w) => matches(row, w))) return false; continue; }
+      if (v && typeof v === 'object' && !Array.isArray(v)) {
+        if ('$in' in (v as any)) {
+          if (!(v as any).$in.map(String).includes(String(row[k]))) return false;
+          continue;
+        }
+        if ('$eq' in (v as any)) {
+          if ((row[k] ?? null) !== ((v as any).$eq ?? null)) return false;
+          continue;
+        }
+        return false;
+      }
+      if ((row[k] ?? null) !== (v ?? null)) return false;
+    }
+    return true;
+  };
+  const driver: any = {
+    name: 'memory', version: '0.0.0', supports: {},
+    async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
+    async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); },
+    async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
+    async create(o: string, data: Record<string, unknown>) {
+      nextId += 1;
+      const id = (data.id as string) ?? `r_${nextId}`;
+      const row = { ...data, id };
+      storeFor(o).set(id, row);
+      return row;
+    },
+    async update(o: string, id: string, data: Record<string, unknown>) {
+      const s = storeFor(o);
+      const cur = s.get(id);
+      if (!cur) throw new Error(`nf ${o}/${id}`);
+      const up = { ...cur, ...data, id };
+      s.set(id, up);
+      return up;
+    },
+    async updateMany(o: string, ast: any, data: Record<string, unknown>) {
+      const s = storeFor(o);
+      const hits = Array.from(s.values()).filter((r) => matches(r, ast?.where));
+      for (const r of hits) s.set(String(r.id), { ...r, ...data, id: r.id });
+      return hits.length;
+    },
+    async upsert(o: string, data: Record<string, unknown>) {
+      const id = data.id as string | undefined;
+      return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data);
+    },
+    async delete(o: string, id: string) { return storeFor(o).delete(id); },
+    async count(o: string, ast: any) { return (await this.find(o, ast)).length; },
+    async bulkCreate(o: string, rows: Record<string, unknown>[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
+    async bulkUpdate() { return []; },
+    async bulkDelete() {},
+    async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
+    async commit() {}, async rollback() {},
+  };
+  return driver;
+}
+
+const USER_CTX = { isSystem: false, userId: 'u1', positions: [], permissions: [] };
+
+describe('approvals record lock — predicate (multi) updates (#4778)', () => {
+  let engine: ObjectQL;
+  /** Held by a pending approval. */
+  let lockedId: string;
+  /** Same object, same predicates, no approval. */
+  let freeId: string;
+
+  const amountOf = async (id: string) =>
+    (await engine.findOne('opportunity', { where: { id }, context: USER_CTX } as any))?.amount;
+
+  const openRequest = async (config: Record<string, unknown>, extra: Record<string, unknown> = {}) => {
+    await engine.insert('sys_approval_request', {
+      object_name: 'opportunity',
+      record_id: lockedId,
+      status: 'pending',
+      flow_run_id: 'run_1',
+      node_config_json: JSON.stringify(config),
+      ...extra,
+    }, { context: { isSystem: true } } as any);
+  };
+
+  beforeEach(async () => {
+    engine = new ObjectQL();
+    engine.registerDriver(makeMemoryDriver(), true);
+    await engine.init();
+    for (const o of [opportunity, approvalRequest]) engine.registry.registerObject(o as any);
+
+    lockedId = String((await engine.insert('opportunity', { name: 'Deal', amount: 100 })).id);
+    freeId = String((await engine.insert('opportunity', { name: 'Other', amount: 100 })).id);
+    await openRequest({ lockRecord: true, approvalStatusField: 'approval_status' });
+
+    bindApprovalLockHook(engine as any);
+  });
+
+  it('the issue verbatim: the by-id write is refused, and so are both predicate rewrites', async () => {
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { id: lockedId }, context: USER_CTX } as any),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { id: { $in: [lockedId] } }, multi: true, context: USER_CTX } as any),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Deal' }, multi: true, context: USER_CTX } as any),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+
+    // The refusal is a refusal: nothing reached the driver.
+    expect(await amountOf(lockedId)).toBe(100);
+  });
+
+  it('refuses an unscoped whole-table update', async () => {
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { multi: true, context: USER_CTX } as any),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+    expect(await amountOf(lockedId)).toBe(100);
+    expect(await amountOf(freeId)).toBe(100);
+  });
+
+  it('still lets a predicate update that misses the locked record through', async () => {
+    // The lock is a per-row verdict, not "this object has an approval open".
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Other' }, multi: true, context: USER_CTX } as any),
+    ).resolves.toBeDefined();
+    expect(await amountOf(freeId)).toBe(999);
+    expect(await amountOf(lockedId)).toBe(100);
+  });
+
+  it('lets a predicate update through once the request is no longer pending', async () => {
+    const [req] = await engine.find('sys_approval_request', { where: { record_id: lockedId }, context: { isSystem: true } } as any);
+    await engine.update('sys_approval_request', { id: req.id, status: 'approved' }, { context: { isSystem: true } } as any);
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Deal' }, multi: true, context: USER_CTX } as any),
+    ).resolves.toBeDefined();
+    expect(await amountOf(lockedId)).toBe(999);
+  });
+
+  // ── the exemptions, on the multi path, through the real engine ─────
+
+  it('exempts an engine self-write (isSystem)', async () => {
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Deal' }, multi: true, context: { isSystem: true, positions: [], permissions: [] } } as any),
+    ).resolves.toBeDefined();
+    expect(await amountOf(lockedId)).toBe(999);
+  });
+
+  it('exempts the run that opened the approval (flowRunId provenance)', async () => {
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Deal' }, multi: true, context: { ...USER_CTX, flowRunId: 'run_1' } } as any),
+    ).resolves.toBeDefined();
+    expect(await amountOf(lockedId)).toBe(999);
+
+    // …and only that run.
+    await expect(
+      engine.update('opportunity', { amount: 1 }, { where: { name: 'Deal' }, multi: true, context: { ...USER_CTX, flowRunId: 'run_other' } } as any),
+    ).rejects.toThrow(/RECORD_LOCKED/);
+  });
+
+  it('exempts a status-mirror write (only the approvalStatusField changes)', async () => {
+    await expect(
+      engine.update('opportunity', { approval_status: 'approved' }, { where: { name: 'Deal' }, multi: true, context: USER_CTX } as any),
+    ).resolves.toBeDefined();
+    expect(
+      (await engine.findOne('opportunity', { where: { id: lockedId }, context: USER_CTX } as any))?.approval_status,
+    ).toBe('approved');
+  });
+
+  it('exempts a node that opted out of the lock (lockRecord: false)', async () => {
+    const [req] = await engine.find('sys_approval_request', { where: { record_id: lockedId }, context: { isSystem: true } } as any);
+    await engine.update(
+      'sys_approval_request',
+      { id: req.id, node_config_json: JSON.stringify({ lockRecord: false }) },
+      { context: { isSystem: true } } as any,
+    );
+    await expect(
+      engine.update('opportunity', { amount: 999 }, { where: { name: 'Deal' }, multi: true, context: USER_CTX } as any),
+    ).resolves.toBeDefined();
+    expect(await amountOf(lockedId)).toBe(999);
+  });
+});