fix(objectql)!: beforeUpdate receives the persist image; the caller submission moves to ctx.submitted (#16344)

.changeset/hook-input-is-the-persist-image.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/spec": minor
+"@objectstack/objectql": minor
+"@objectstack/plugin-auth": patch
+---
+
+fix(objectql)!: `beforeUpdate` receives the record the engine intends to persist, and the caller's submission travels on `ctx.submitted` (#16344)
+
+<!-- adr-0087: not-required (no-migration-prescription) an enforcement-ORDER change plus one ADDITIVE optional key on a runtime context schema. 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. What changes is which image a `beforeUpdate` handler is shown; the remedy for a handler that depended on seeing a refused value is to read `ctx.submitted`, which is a code edit in the handler, not a metadata migration. Nothing is retired: `HookContext.submitted` is new and optional. -->
+
+**BREAKING** — what a `beforeUpdate` handler reads on `ctx.input.data` changes. A `readonly` field the caller supplied a value for is no longer there. The hidden set is the update strip's own subject set: author-declared `readonly: true` **and** the types whose value the runtime owns end to end (`autonumber`, implicitly read-only since #5503). `readonlyWhen` locks are deliberately not hidden.
+
+## The defect
+
+On update, a value sent for a field declared `readonly: true` was correctly **not persisted** — and was still handed to the object's `beforeUpdate` hook. A hook deriving columns from the incoming record therefore derived them from a value the row would never contain, and **those derived writes persisted**, because they are the hook's own.
+
+Measured on a real app (17.2.0, sqlite, dev runtime) and reproduced in `packages/objectql/src/engine-readonly-hook-input.test.ts`. One `PATCH { actual_value: 380, target_value: 1, weight: 1 }` against a `readonly` `target_value`:
+
+```
+read back: target_value 400   weight 10        ← the strip worked
+           score 1.2  calc_trace "实际 380 / 目标 1 … 权重 1%"
+```
+
+The row's own audit trail cites values the row does not hold. No error, no warning, 200, and `droppedFields` correctly reporting the strip the whole time — every channel said the write was fine, because by every channel's own lights it was. The only way for an application to be safe was for every hook to re-read its read-only columns and ignore the incoming record, which defeats declaring them read-only at all.
+
+## What changed
+
+**`ctx.input.data` on `beforeUpdate` is now the record the engine intends to persist.** Caller-supplied values for `readonly` fields are taken out of the hooks' view before the before phase is dispatched, and handed back at the engine's post-hook confluence — so the payload every engine-owned consumer below reads is byte-for-byte what it read before. `onFieldsDropped` reports the same fields with the same `readonly` reason, the read-only WARN says the same sentence, and `strictReadonlyWrites` refuses exactly the same writes.
+
+**The caller's submission travels on a new `HookContext` member, `ctx.submitted`** (`@objectstack/spec`, `HookContextSchema`) — the payload as sent, snapshotted at engine entry before any middleware or hook stamp, frozen, and documented as *diagnostics only, never the persist image*. It is bound on the update verb, both phases, and every per-row dispatch of one caller write.
+
+Two things deliberately did **not** move:
+
+- **The enforcement pass is still after the hooks.** It is the only point that can tell a hook's stamp from a caller's forgery (`hookWrittenKeys`), so a `beforeUpdate` that stamps a read-only column still lands — including when the caller echoed the same key back, which is the whole subject of #5591 / #14088.
+- **`beforeInsert` is untouched.** The create side's strip position is settled post-hook by ruling C (#14147, "one semantics, one enforcement point"), and `readonlyWhen`-locked fields stay hook-writable per #9107.
+
+`@objectstack/plugin-auth`'s ADR-0092 identity write guard is migrated onto the new member in the same change, which is why nothing degrades: its 403 and its security warn still name the non-whitelisted field the caller sent. Without that migration the identical request answers `None of the submitted fields (—) are editable` — as strong a refusal, saying nothing about what was refused. Both readings are pinned side by side in `identity-write-guard.test.ts`.
+
+Ruled 2026-09-08 (maintainer, verbatim 「批 #87 同意」, director seat, decision batch #87). The refused primary was the same strip move **without** the new member: the ADR-0092 diagnostic degrades and every third-party `beforeUpdate` guard reading `ctx.input.data` degrades with it, silently. The refused alternative on the other side was documenting that hooks must read read-only columns from `ctx.previous` — which outsources the invariant to every application, the exact shape triage had already rejected.
+
+## Who is affected
+
+A `beforeUpdate` handler that **reads a `readonly` field (declared, or runtime-owned) out of `ctx.input.data`**, on a non-`isSystem` write. Three shapes, and the fix is one line each:
+
+- **deriving a value from it** — this is the defect; the handler now derives from `ctx.previous`, or from `ctx.input.data` with the payload's absence meaning "unchanged", which is what it always meant for a field the caller never sent.
+- **reporting on what the caller sent** (a guard naming the offending key) — read `ctx.submitted`.
+- **a self-assignment** (`data.x = data.x`) on such a field — this used to promote the caller's forged value to hook-owned and commit it. It is now a **no-op**: the key the hook reads is gone, so the line re-creates it holding `undefined`, and the engine treats set-to-undefined of a hidden read-only key as the no-op it is — deleting the key, dropping it from the hook-write record, and letting the ordinary hand-back put the caller's value back for the strip to judge. **The stored value stands**, and the write reports exactly as it would with no hook at all (stripped, `onFieldsDropped`, the WARN, `strictReadonlyWrites` refusing). Persisting the `undefined` instead would erase the stored value on the memory driver and hand knex an undefined binding on a SQL one — neither is the record the engine intends to persist. That laundering route closing is intended, and it is re-pinned in both directions rather than removed.
+
+⚠️ **The sharpest edge is a sandboxed `body` hook, and it is a refusal rather than a quiet change.** A body that reaches *through* such a key — `ctx.input.locked_meta.who = 'hook'` — now dereferences `undefined` and throws, and a `body`'s default `onError` is `abort`, so the caller's **whole write is rejected** where it used to succeed. What that body used to do was persist a value derived from the caller's forgery, so refusing is the correct direction; but the message the author sees is a raw `TypeError` from their own dereference and names nothing actionable. Measured end to end through a real QuickJS sandbox and pinned in `packages/runtime/src/sandbox/hook-input-writeback-readonly-provenance.integration.test.ts`.
+
+A body hook cannot read `ctx.submitted`: it is deliberately not marshalled onto the sandbox face, for the reason `dispatch.scope` is not — that face is assembled key by key, and a key added there is a second published contract with its own compatibility story. A body deriving a column from a read-only field reads **`ctx.previous`**, the stored row, which is the correct source either way.
+
+⚠️ **One ADR-0092 boundary changes a status code, and no in-repo object hits it today.** On an object whose UPDATE whitelist admits a field that is ALSO declared `readonly`, a whitelist-only payload now answers **403** where it used to answer **200 having written nothing**. The identity write guard composes its refused list from what the engine left it, and a whitelisted key is excluded from that list by design, so the refusal reads `None of the submitted fields (—) are editable` — naming nothing. The write was already being dropped by the read-only strip before this change; what moves is that the caller is now told, and told imprecisely. `sys_user`'s three writable fields are not read-only, so nothing in this repository is on that boundary; an application that puts a `readonly` field in an UPDATE whitelist should take it out, which is what the whitelist meant either way.
+
+An `isSystem` caller sees no change at all: the strip has never applied to one, and neither does the hide.

content/docs/automation/hooks.mdx

@@ -181,6 +181,12 @@ record's fields **directly on `ctx.input`** (a flat view over the internal
 `{ data, options }` wrapper — reads and writes of record fields route through
 `ctx.input.data`):
 
+On **update**, "the incoming record" means *the record the engine intends to
+persist* — a caller-supplied value for a `readonly` field is not on it, and the
+caller's submission is on `ctx.submitted` instead (diagnostics only). See
+[Static `readonly` fields on the write path](/docs/protocol/objectql/security#static-readonly-fields-on-the-write-path)
+for the five rules and the migration.
+
 <Callout type="warn" title="`id` / `options` / `ast` / `data` are RESERVED on the flat `ctx.input` face">
 These four names always resolve to the envelope, never to a record field —
 even if your object declares a field with one of those names. A field named

content/docs/permissions/system-context.mdx

@@ -9,7 +9,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
 service self-write, a migration.
 
 This page is **the authority** for what that flag actually does. It exists
-because the flag is not one concept: it is a single boolean read at **108
+because the flag is not one concept: it is a single boolean read at **109
 distinct sites across 20 packages**, and knowing three of those behaviours gives
 no hint that the other hundred-and-four exist. Every documented app-side bug
 traced to `isSystem` had the same shape — the metadata was complete and correct,
@@ -132,7 +132,7 @@ that silently does not happen.
 
 ### 3. Sharing (`plugin-sharing`)
 
-The largest single consumer — **17 of the 108 sites**.
+The largest single consumer — **17 of the 109 sites**.
 
 | # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
 |:--|:---|:---|:---|
@@ -278,7 +278,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are
 independent decisions, and a seed loader plausibly wants the first two but not
 the third. The concept is nevertheless **staying as one boolean**:
 
-- **Shipped semantics.** `isSystem` is a published contract with 108 read sites
+- **Shipped semantics.** `isSystem` is a published contract with 109 read sites
   in 20 packages. Splitting it is a breaking contract change across all of them.
   (The ruling was taken when the census read 80 sites in 18 packages; the count
   has grown, which strengthens rather than weakens the argument.)
@@ -352,12 +352,12 @@ still holds equal to the census on every pull request:
 | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
 | — parsed as a declaration | 23 | ✅ |
 | — parsed as an object-literal / type key (producers and option objects) | 310 | — |
-| — parsed as a property **read** | 114 | ✅ |
+| — parsed as a property **read** | 115 | ✅ |
 | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
 | — the remainder: text inside comments and string literals | 358 | — |
 | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
-| Of those reads: reads of `ExecutionContext.isSystem` | **108** | ✅ |
-| — behaviour-bearing (rows 1–63 above) | 104 | ✅ |
+| Of those reads: reads of `ExecutionContext.isSystem` | **109** | ✅ |
+| — behaviour-bearing (rows 1–63 above) | 105 | ✅ |
 | — carry the flag onward only (rows 64–67 above) | 4 | ✅ |
 | Packages containing at least one elevation read | **20** | ✅ |
 | Files containing at least one elevation read | 45 | ✅ |

content/docs/protocol/objectql/security.mdx

@@ -259,18 +259,45 @@ rejecting — the offending key is removed from the payload and the rest of the
 committed. The write therefore **succeeds** (REST answers `200`), and the read-only column
 simply keeps its stored value.
 
-Four rules decide whether a given value survives:
+Five rules decide whether a given value survives:
 
 | # | Rule | Effect |
 |:--|:---|:---|
 | 1 | **Trusted context is exempt** | A write carrying `context.isSystem === true` skips the strip entirely and may set read-only columns. |
 | 2 | **Only *caller-supplied* keys are candidates** | The engine snapshots the payload's keys at entry (`suppliedKeys`), *before* middleware and `beforeUpdate` hooks run. Only keys in that snapshot can be stripped. |
 | 3 | **Hook / middleware backfill survives** | A key a `beforeUpdate` hook *adds* to `data` is absent from the entry snapshot, so it is not a candidate — this is why the built-in `updated_by` / `updated_at` stamps land even though those columns are `readonly`. |
 | 4 | **`context.preserveAudit` admits a whitelist — on UPDATE only** | An opt-in historical import reinstates the audit/timestamp family and author-declared business `readonly` fields; platform-managed `system` columns (tenancy, generated) stay stripped. This exemption exists on the **UPDATE** path and nowhere else — see below. |
-
-Rule 2 is scoped to keys, not values: a key the caller sent stays a strip candidate even if
-a hook later overwrites its value. So a `beforeUpdate` hook can *backfill* a read-only
-field, but cannot *rescue* one the caller supplied.
+| 5 | **Hooks are shown the persist image, not the submission** (#16344) | On UPDATE, a caller-supplied value for a `readonly` field is hidden from `ctx.input.data` *before* `beforeUpdate` is dispatched, so a hook cannot derive a persisted column from a value the row will never hold. What the caller actually sent is on **`ctx.submitted`** — diagnostics only, never the persist image. |
+
+Rule 2 selects the *candidates*; rules 3 and 5 decide what a hook can do about one. A key
+the caller never sent is not a candidate at all, which is why the built-in `updated_by` /
+`updated_at` stamps land. A key the caller **did** send stays a candidate — but a hook that
+**assigns** it owns the value standing on it and the strip keeps that write (#5591 /
+#14088: authorship is *recorded* while the hook writes happen, not inferred from value
+equality afterwards). So a `beforeUpdate` hook can both *backfill* a read-only field and
+*overwrite* one the caller supplied; what it can no longer do is *rescue the caller's own
+value*, because since #16344 that value is not on `ctx.input.data` for it to echo back.
+
+<Callout type="warn" title="`beforeUpdate` no longer sees the caller's read-only values (#16344)">
+This is a **breaking** change to what a hook reads, not to what is stored: the accept /
+refuse set is unchanged, `onFieldsDropped` reports the same fields under the same
+`readonly` reason, the WARN says the same sentence, and `strictReadonlyWrites` refuses the
+same writes. What moved is the hook's view.
+
+- A handler **deriving** a column from a read-only field reads the stored row on
+  `ctx.previous`, or treats the key's absence as "unchanged" — which is what absence
+  always meant for a field the caller never sent.
+- A handler **reporting on what the caller sent** (a guard naming an offending key) reads
+  `ctx.submitted`.
+- A self-assignment (`data.x = data.x`) on such a field is now a **no-op** — the stored
+  value stands, and the write is stripped and reported exactly as an un-hooked one is.
+- `ctx.submitted` is **not** marshalled onto the sandboxed `body` face. A `body` deriving
+  from a read-only column reads `ctx.previous`, the stored row, which is the correct
+  source either way.
+
+`beforeInsert` is untouched (#14147), and `readonlyWhen` locks are deliberately still
+hook-writable (#9107).
+</Callout>
 
 <Callout type="warn">
 **`preserveAudit` is an UPDATE-path exemption. It does not apply on INSERT (#6640).**

content/docs/references/data/hook.mdx

@@ -93,6 +93,7 @@ L2 sandboxed JS body — runs inside an isolated VM with declared capabilities
 | **input** | `Record<string, any>` | ✅ | Mutable input parameters |
 | **result** | `any` | optional | Operation result (After hooks only) |
 | **previous** | `Record<string, any>` | optional | Record state before operation |
+| **submitted** | `Record<string, any>` | optional | What the caller submitted, as sent (update only) — diagnostics only, never the persist image |
 | **dispatch** | `{ mode: Enum<'record' \| 'per-row'>; index: integer; scope: Record<string, any> }` | optional | How this hook call relates to the caller's write (engine-produced) |
 | **session** | `{ userId?: string; actor?: string; organizationId?: string; accessToken?: string; … }` | optional | Current session context |
 | **provenance** | `{ flowRunId?: string; attributedUserId?: string }` | optional | Server-stamped write provenance (never client-supplied, never an authorization input) |