fix(objectql): name the withheld read-only key when a hook faults reaching through it

.changeset/hook-withheld-readonly-key-diagnostic.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/objectql": patch
+---
+
+fix(objectql): a hook that faults reaching through a withheld read-only key now names the key, says the platform withheld it, and points at `ctx.previous` (#17219)
+
+Since #16344 the update path hides a caller-supplied static `readonly` value from `before*` hooks. A hook body that reaches **through** such a key — `ctx.input.locked_meta.who = 'hook'`, where `locked_meta` is a caller-supplied read-only `json` column — therefore dereferences `undefined` and throws, and a `body` hook's default `onError: abort` refuses the caller's whole write.
+
+**The refusal is correct and is unchanged.** What it replaced is a write that succeeded while persisting a value derived from the caller's forgery, and #16344 exists to close exactly that route. What this fixes is the diagnostic. Measured before this change, at both doors:
+
+```
+direct   SandboxError: hook 'guard_task_body' threw:
+           TypeError: cannot set property 'who' of undefined
+REST     500 {"error":"Internal server error","code":"INTERNAL_ERROR"}
+```
+
+The REST reading is the one that matters, and it is the worse of the two: a leading `TypeError:` is correctly classified as a script fault and sanitised (#7543), so an author was told nothing at all — not which key, not that the platform had taken it away, not what to read instead.
+
+### Who is affected
+
+Anyone whose `beforeUpdate` hook reads a read-only field that the caller may also send. The write was already being refused; only the message changes. A hook that needs the stored value reads it from **`ctx.previous.<field>`** — the same remedy PR #17195's changeset documents.
+
+### What the message says now
+
+```
+A `beforeUpdate` hook faulted while `locked_meta` was withheld from it. That field is
+`readonly: true`, and the engine withholds a caller-supplied value for a read-only field
+from `beforeUpdate` hooks, so `ctx.input.locked_meta` reads `undefined` — withheld by the
+platform, not missing by accident. Read the stored value from `ctx.previous.locked_meta`
+instead. Original fault: TypeError: cannot set property 'who' of undefined
+```
+
+The error declares **HTTP 400**, which is what carries it past the script-fault sanitiser onto the same "message verbatim" channel a body's own authored refusal already rides; REST callers who previously saw `500 INTERNAL_ERROR` for this case now see 400 with the text above. The original fault is carried inside the message rather than replaced.
+
+### Deliberate limits
+
+No new error code is registered and no key is added to any published payload — a dedicated `ERROR_CODE_LEDGER` entry for this refusal is a separate decision. The explanation claims only what is knowable at the seam: *faulted while these keys were withheld*, never a proven cause. An **authored** refusal (`throw new Error('…')`) is never rewritten, and a crash on an operation where nothing was withheld passes through untouched.

packages/objectql/src/engine-readonly-hook-input.test.ts

@@ -407,3 +407,180 @@ describe('#16344 — caller-forged readonly values are hidden from beforeUpdate'
     expect(afterSubmitted).toEqual([{ id: 'kpi_1', actual_value: 380, target_value: 1 }]);
   });
 });
+
+/**
+ * [#17219] The OTHER half of the same hide pass: what an author is told when a
+ * hook reaches THROUGH a key #16344 withheld.
+ *
+ * ⛔ The refusal itself is not under test here and is not moved: a body's
+ * default `onError: abort` refuses the caller's whole write, and what that
+ * replaced is a write that succeeded while persisting a value derived from the
+ * caller's forgery. Every case below re-asserts that the row is untouched, so a
+ * future repair of the DIAGNOSTIC cannot quietly restore the old write.
+ *
+ * Measured on `origin/main` `501959b72a` before this fix, both doors:
+ *
+ *   direct   SandboxError: hook '…' threw: TypeError: cannot set property 'who' of undefined
+ *   REST     500 {"error":"Internal server error","code":"INTERNAL_ERROR"}
+ *
+ * The REST reading is the one that decides the shape of the fix: a leading
+ * `TypeError:` is correctly classified as a crash (#7543) and sanitised, so at
+ * the door an author actually authors against, the old behaviour said nothing
+ * at all — not the key, not the reason, not the remedy.
+ *
+ * A code hook is the subject rather than a sandboxed body deliberately: the
+ * dispatch sites and the hide pass are what this card wraps, and they are
+ * common to both. The sandbox spelling is measured end to end, through a real
+ * QuickJS, in `runtime`'s `hook-input-writeback-readonly-provenance` suite.
+ */
+describe('#17219 — a hook that faults reaching THROUGH a withheld readonly key names it', () => {
+  let engine: ObjectQL;
+  let storeFor: ReturnType<typeof makeDriver>['storeFor'];
+
+  const OBJECT = 'guard_task';
+
+  /** Reaches through the read-only `locked_meta`, which the hide pass removed. */
+  const reachThrough = async (ctx: any) => {
+    ctx.input.data.locked_meta.who = 'hook';
+  };
+
+  async function boot(
+    hook: (ctx: any) => Promise<void>,
+    opts?: { unscopedMulti?: boolean },
+  ) {
+    engine = new ObjectQL({ logger: {
+      warn() {}, debug() {}, info() {}, error() {}, trace() {}, fatal() {},
+      child() { return this as any; },
+    } as any });
+    const d = makeDriver();
+    storeFor = d.storeFor;
+    engine.registerDriver(d.driver, true);
+    await engine.init();
+    engine.registry.registerObject({
+      name: OBJECT,
+      fields: {
+        status: { type: 'text' },
+        bucket: { type: 'text' },
+        locked_meta: { type: 'json', readonly: true },
+      },
+    } as any);
+    storeFor(OBJECT).set('t1', {
+      id: 't1', status: 'open', bucket: 'b1', locked_meta: { seeded: true },
+    });
+    engine.registerHook('beforeUpdate', hook, {
+      object: OBJECT,
+      priority: 50,
+      ...(opts?.unscopedMulti ? { dispatchUnscopedMultiWrite: true } : {}),
+    } as any);
+  }
+
+  const row = () => storeFor(OBJECT).get('t1');
+
+  /** Every requirement the card places on the message, asserted as one set. */
+  const expectActionable = (err: any) => {
+    // ① the withheld KEY is named — the old message named nothing.
+    expect(err.message).toContain('`locked_meta`');
+    // ② WITHHELD BY THE PLATFORM, not absent by accident.
+    expect(err.message).toContain('withheld by the platform, not missing by accident');
+    // ③ the documented remedy, reachable from the message itself.
+    expect(err.message).toContain('`ctx.previous.locked_meta`');
+    // ④ and it reaches the author at the REST door: `declaredHttpStatus` reads
+    //    this, and without it the body is the sanitised 500.
+    expect(err.status).toBe(400);
+    // The original fault is carried through, never swallowed.
+    expect(err.message).toMatch(/cannot set propert|Cannot set propert/);
+  };
+
+  it('BY-ID: the refusal stands and now names the key, the reason and the remedy', async () => {
+    await boot(reachThrough);
+
+    const err = await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    // ⛔ RULING 1, re-pinned: the write is still refused WHOLE. Neither the
+    // forged read-only value nor the writable `status` reached the row.
+    expect(row().locked_meta).toEqual({ seeded: true });
+    expect(row().status).toBe('open');
+  });
+
+  it('PREDICATE: the per-row dispatch site answers on the same terms', async () => {
+    await boot(reachThrough);
+
+    const err = await engine.update(
+      OBJECT,
+      { status: 'done', locked_meta: { who: 'caller' } } as any,
+      { multi: true, where: { bucket: 'b1' } } as any,
+    ).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    expect(row().locked_meta).toEqual({ seeded: true });
+    expect(row().status).toBe('open');
+  });
+
+  it('UNSCOPED-MULTI: the third dispatch site inside the hide window answers too', async () => {
+    await boot(reachThrough, { unscopedMulti: true });
+
+    const err = await engine.update(
+      OBJECT,
+      { status: 'done', locked_meta: { who: 'caller' } } as any,
+      { multi: true } as any,
+    ).then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expectActionable(err);
+    expect(row().locked_meta).toEqual({ seeded: true });
+  });
+
+  it('CONTROL — nothing withheld: an ordinary crash keeps its own raw words', async () => {
+    // The caller sends NO read-only key, so the hide pass never runs and
+    // `readonlyHiddenFromHooks` stays unset. The hook still faults (the column
+    // is simply absent from this payload), and that fault must pass through
+    // untouched: the diagnostic is tied to the WITHHOLDING, not to any crash
+    // that happens to occur on an object with a read-only field. Without this
+    // leg the case above would pass just as well for a wrapper that rewrote
+    // every hook error it saw.
+    await boot(reachThrough);
+
+    const err = await engine.update(OBJECT, { id: 't1', status: 'done' } as any)
+      .then(() => null, (e) => e);
+
+    expect(err).toBeTruthy();
+    expect(err.message).not.toContain('withheld by the platform');
+    expect(err.status).toBeUndefined();
+    expect(row().status).toBe('open');
+  });
+
+  it('CONTROL — an AUTHORED refusal is never rewritten, even while a key is withheld', async () => {
+    // The regression this guards is the card's own defect aimed the other way:
+    // `mapDataError` serves an authored message to the caller verbatim, so
+    // overwriting it would destroy the author's words to explain a key they
+    // never asked about.
+    await boot(async () => { throw new Error('仍有未结清的发票'); });
+
+    const err = await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any).then(() => null, (e) => e);
+
+    expect(err.message).toBe('仍有未结清的发票');
+    expect(err.status).toBeUndefined();
+    expect(row().status).toBe('open');
+  });
+
+  it('CONTROL — a hook that does NOT fault still runs, and the strip still refuses the forgery', async () => {
+    // The over-narrowing guard: if the wrapper had broken the dispatch, every
+    // case above would pass for the wrong reason. Here the same withheld key is
+    // in play, the hook completes, and the write lands MINUS the forgery.
+    await boot(async (ctx: any) => { ctx.input.data.status = 'hooked'; });
+
+    await engine.update(OBJECT, {
+      id: 't1', status: 'done', locked_meta: { who: 'caller' },
+    } as any);
+
+    expect(row().status).toBe('hooked');
+    expect(row().locked_meta).toEqual({ seeded: true });
+  });
+});

packages/objectql/src/engine.ts

@@ -214,6 +214,11 @@ import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, str
 // SAME value. Armed and sealed in `update()`; the module owns the argument for
 // why neither end may move.
 import { recordHookPayloadWrites } from './hook-write-provenance.js';
+// [#17219] The hide pass's other half: when a hook faults reaching THROUGH a
+// key that pass withheld, this names the key, says the platform withheld it,
+// and points at `ctx.previous` — the module owns the measurement and the
+// reason the explanation cannot be composed any further downstream.
+import { dispatchHooksExplainingWithheldReadonly } from './hook-withheld-readonly-fault.js';
 import {
   divergingHookPayloadKeys,
   MultiUpdateHookKeyDivergenceError,
@@ -11701,7 +11706,13 @@ export class ObjectQL implements IObjectQLEngine {
            // permanently true here: it states the invariant, and the invariant
            // outlives this call site.
            if (priorRecord) hookContext.previous = coerceBooleanFields(updateSchema as any, priorRecord as any) as any;
-           await this.triggerHooks('beforeUpdate', hookContext);
+           // [#17219] All three `beforeUpdate` dispatch sites inside the hide
+           // window share one wrapper, so a hook that faults reaching THROUGH a
+           // key this pass withheld names that key instead of surfacing the
+           // platform's own contract enforcement as the author's crash. It
+           // rethrows the original error untouched on every other path.
+           await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+             () => this.triggerHooks('beforeUpdate', hookContext));
            // The retired lever, refused. Everything above — `previous`, and
            // below it the `readonlyWhen` strip and every validation rule — was
            // computed against the row the ladder chose.
@@ -11772,7 +11783,8 @@ export class ObjectQL implements IObjectQLEngine {
            // predicate is unscoped.
            const rawWhere = (hookContext.input.options as { where?: unknown } | undefined)?.where;
            if (rawWhere === undefined || rawWhere === null) {
-               await this.dispatchUnscopedMultiWriteHooks('beforeUpdate', object, hookContext);
+               await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+                 () => this.dispatchUnscopedMultiWriteHooks('beforeUpdate', object, hookContext));
            }
            const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any);
            readPriorRows = async () => {
@@ -11804,7 +11816,8 @@ export class ObjectQL implements IObjectQLEngine {
                // [D1] Zero matched rows is zero dispatches — a batch that
                // changed nothing is not a record change.
                if (perRowBeforeHooks && rows.length > 0) {
-                   await this.dispatchPerRowBeforeHooks(object, 'beforeUpdate', rows, hookContext);
+                   await dispatchHooksExplainingWithheldReadonly(readonlyHiddenFromHooks, 'beforeUpdate',
+                     () => this.dispatchPerRowBeforeHooks(object, 'beforeUpdate', rows, hookContext));
                }
            }
        }

packages/objectql/src/hook-withheld-readonly-fault.test.ts

@@ -0,0 +1,122 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#17219] The composer's contract, at the seam rather than through a driver.
+ *
+ * The three DECLINE conditions carry as much weight as the accept case, and for
+ * the reason the card is about: a diagnostic that fires on the wrong error is
+ * the same defect this fixes, aimed the other way. So each decline is asserted
+ * with the SAME error the accept case uses wherever the condition permits it,
+ * which is what makes the assertions about the condition rather than about the
+ * error.
+ */
+
+import { describe, it, expect } from 'vitest';
+import {
+  withheldReadonlyHookFault,
+  dispatchHooksExplainingWithheldReadonly,
+  HookWithheldReadonlyFaultError,
+} from './hook-withheld-readonly-fault.js';
+
+/** The real shape `quickjs-runner.ts` throws — name `SandboxError`, native prefix kept on `innerMessage`. */
+class SandboxErrorLike extends Error {
+  innerMessage?: string;
+  constructor(message: string, innerMessage?: string) {
+    super(message);
+    this.name = 'SandboxError';
+    this.innerMessage = innerMessage;
+  }
+}
+
+/** Measured verbatim on `origin/main` `501959b72a` — see the module header. */
+const REAL_CRASH = () =>
+  new SandboxErrorLike(
+    "hook 'guard_task_body' threw: TypeError: cannot set property 'who' of undefined",
+    "TypeError: cannot set property 'who' of undefined",
+  );
+
+const HIDDEN = { locked_meta: { who: 'caller' } };
+
+describe('#17219 withheldReadonlyHookFault', () => {
+  it('names the key, says the platform withheld it, and points at ctx.previous', () => {
+    const out = withheldReadonlyHookFault(REAL_CRASH(), HIDDEN, 'beforeUpdate');
+    expect(out).toBeInstanceOf(HookWithheldReadonlyFaultError);
+    const msg = out!.message;
+    // ① the withheld KEY is named …
+    expect(msg).toContain('`locked_meta`');
+    // ② … as WITHHELD BY THE PLATFORM, not absent by accident …
+    expect(msg).toContain('withheld by the platform, not missing by accident');
+    expect(msg).toContain('`readonly: true`');
+    // ③ … and the documented remedy is reachable from the message itself.
+    expect(msg).toContain('`ctx.previous.locked_meta`');
+    // The original fault is carried, never replaced: an author debugging the
+    // body still gets the line that actually threw.
+    expect(msg).toContain("TypeError: cannot set property 'who' of undefined");
+    expect(out!.withheldKeys).toEqual(['locked_meta']);
+    expect(out!.cause).toBeDefined();
+  });
+
+  it('declares 400 — the whole envelope change, and what makes the message reachable', () => {
+    // Measured: without a declared status `mapDataError` answers
+    // `UNCLASSIFIED_FAULT` (500, sanitised body) and the message above never
+    // reaches the author at the REST door. `packages/rest`'s
+    // `declaredHttpStatus` reads exactly this property.
+    expect(withheldReadonlyHookFault(REAL_CRASH(), HIDDEN, 'beforeUpdate')!.status).toBe(400);
+  });
+
+  it('names every withheld key when the pass hid more than one', () => {
+    const out = withheldReadonlyHookFault(
+      REAL_CRASH(), { locked_meta: {}, locked_note: 'CALLER' }, 'beforeUpdate',
+    );
+    expect(out!.message).toContain('`locked_meta`, `locked_note`');
+    expect(out!.message).toContain('`ctx.previous.locked_meta`, `ctx.previous.locked_note`');
+    expect(out!.withheldKeys).toEqual(['locked_meta', 'locked_note']);
+  });
+
+  it('DECLINES when nothing was withheld — an unrelated crash keeps its own words', () => {
+    expect(withheldReadonlyHookFault(REAL_CRASH(), undefined, 'beforeUpdate')).toBeUndefined();
+    expect(withheldReadonlyHookFault(REAL_CRASH(), {}, 'beforeUpdate')).toBeUndefined();
+  });
+
+  it('DECLINES on an AUTHORED refusal, so a business message is never rewritten', () => {
+    // The one that would be a real regression: `mapDataError` serves this text
+    // to the caller verbatim at 400, and overwriting it would destroy the
+    // author's own words while a readonly key happened to be hidden.
+    const authored = new SandboxErrorLike("hook 'guard' threw: 仍有未结清的发票", '仍有未结清的发票');
+    expect(withheldReadonlyHookFault(authored, HIDDEN, 'beforeUpdate')).toBeUndefined();
+    // And the non-sandboxed spelling of the same thing.
+    expect(withheldReadonlyHookFault(new Error('仍有未结清的发票'), HIDDEN, 'beforeUpdate')).toBeUndefined();
+  });
+
+  it('ACCEPTS a CODE hook crash, which carries the native name in `name` instead', () => {
+    const out = withheldReadonlyHookFault(
+      new TypeError("Cannot set properties of undefined (setting 'who')"), HIDDEN, 'beforeUpdate',
+    );
+    expect(out).toBeInstanceOf(HookWithheldReadonlyFaultError);
+    expect(out!.message).toContain('`locked_meta`');
+  });
+
+  it('declines on a non-object throw rather than fabricating a shape', () => {
+    expect(withheldReadonlyHookFault('boom', HIDDEN, 'beforeUpdate')).toBeUndefined();
+  });
+});
+
+describe('#17219 dispatchHooksExplainingWithheldReadonly', () => {
+  it('is transparent on success', async () => {
+    await expect(dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => 'ok'))
+      .resolves.toBe('ok');
+  });
+
+  it('rethrows the ORIGINAL error object when the composer declines', async () => {
+    const authored = new Error('仍有未结清的发票');
+    await expect(
+      dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => { throw authored; }),
+    ).rejects.toBe(authored);
+  });
+
+  it('replaces an anonymous crash with the named refusal', async () => {
+    await expect(
+      dispatchHooksExplainingWithheldReadonly(HIDDEN, 'beforeUpdate', async () => { throw REAL_CRASH(); }),
+    ).rejects.toBeInstanceOf(HookWithheldReadonlyFaultError);
+  });
+});