Repository navigation
Commit f05649f
Fixes #22306
Clause-②: no (narrowing)
## What was wrong
On a non-system create, `ObjectQL.insert` dispatched `beforeInsert` on
the caller's payload. It ran the static `readonly` strip
(`staticReadonlyCreateStrip`) only after the hooks. Take a hook that
stamps a read-only column only when the column is absent, the hotcrm `if
(!input.stage_entry_date)` shape. It saw the caller's value and did
nothing, then the strip dropped that value, so the row stored NULL where
the hook would have stamped today. The update path has withheld these
values from its hooks since #16344. The insert path did not.
## Base and head
The same object and hook as hotcrm: `stage_entry_date` (date,
`readonly`), `days_in_stage` (number, `readonly`), and a `beforeInsert`
hook that stamps both when `stage_entry_date` is absent.
| case | base: hook saw `stage_entry_date` | base: stored | head: hook
saw | head: stored |
|---|---|---|---|---|
| caller sends the readonly key (non-system) | `'2020-01-01'` | NULL /
NULL | absent | `'2026-10-09'` / 0 |
| key absent (non-system) | absent | `'2026-10-09'` / 0 | absent |
`'2026-10-09'` / 0 |
| system writer (`isSystem`) sends the key | `'2020-01-01'` |
`'2020-01-01'` / NULL | `'2020-01-01'` | `'2020-01-01'` / NULL |
| a hook stamps the key unconditionally, and the caller sent it |
`'2020-01-01'` | `'2026-10-09'` | absent | `'2026-10-09'` |
How each row was measured:
- **Rows 1 to 3, real engine over `driver-sql` (better-sqlite3):** base
is `c8c803c293` with objectql built from that commit. Head is this
branch.
- **Row 1, REST door:** `POST /api/v1/data/rro_opp` through `bootStack`
with a sandboxed `body` hook. The base call answered 201 and stored NULL
/ NULL. The head call answered 201 and stored `'2026-10-09'` / 0.
- The caller was the seeded admin, a non-system principal. The strip ran
on the base call, which shows this: the forged value was not stored.
- A freshly signed-up user had no create permission on the fixture
object (403), so that user could not be used.
- **Row 4:** the engine over a recording driver, at head and with the
hide ablated. The stored value is unchanged.
- **Batch:** the same on both paths. At base, row 0 of a batch (forged)
stored NULL and row 1 stored the stamp. At head, both rows store the
stamp.
## The change (`packages/objectql/src/engine.ts`)
- **Withhold, not strip, as on update.** Before the defaults and before
`beforeInsert`, `withholdInsertReadonlyFromHooks` takes out every value
that the post-hook static strip will take. It uses the same function
(`stripReadonlyFields`), the same subject
(`staticReadonlyInsertSubject`) and the same options (no
`preserveAudit`), so the hide and the strip take the same caller keys.
There is one named exception. Under `preserveAudit` the hide leaves the
audit timeline visible, while the strip takes it unless the audit binder
reinstates it. On a bare engine without `ObjectQLPlugin`, or for
`created_by` / `updated_by` with no session user, such a value is shown
to the hook and then stripped. The defaults then fill a withheld key the
way they fill any absent key. So a forged `approval_status: 'approved'`
is shown to the hook as its default `'draft'`, exactly what an honest
caller's hook sees.
- **The strip does not move.** After the seal,
`handBackWithheldInsertReadonly` puts the caller's values back wherever
no hook wrote the key. The strip, its re-default, the WARN,
`onFieldsDropped` and `strictReadonlyWrites` then judge the same payload
as before.
- A hook's own write is kept, and is never overwritten by a hand-back.
- A `data.x = data.x` self-assignment of a withheld key follows the
engine's rule:
- with no `defaultValue` the hook reads `undefined`, the write is undone
as on update, and the caller's value is stripped and reported.
`undefined` is never stored.
- with a `defaultValue` the hook re-assigns the default, which counts as
the hook's write. The default is stored and nothing is reported.
- **Faults are explained.** The dispatch goes through
`dispatchHooksExplainingWithheldReadonly`, the update path's wrapper. A
hook that faults reaching through a withheld key gets a 400 that names
the key.
- `hook-withheld-readonly-fault.ts` gets a `beforeInsert` prescription,
because a create has no `ctx.previous`.
- The update message is byte-identical.
What update withholds (M2), measured on `origin/main`:
- **The set:** `stripReadonlyFields` over the full object schema. That
covers static `readonly` and runtime-owned `autonumber`, but not
`readonlyWhen`.
- **Exemptions:** `isSystem` (the whole pass) and the `preserveAudit`
whitelist. A key the caller did not send (own property of the entry
snapshot) is never a candidate, and `id` is excluded.
- **After the hooks:** seal, then hand back what the payload does not
hold, undo a set-to-undefined, and narrow `hookWrittenKeys`. The
caller's submission travels on `ctx.submitted`.
The insert set differs from that in three measured places:
- **Runtime-owned `autonumber` stays visible.** I first included it, and
three existing pins went red. One is "the hook can still SEE the
caller-submitted record number"
(`engine-insert-runtime-owned-strip.test.ts`, #6339). The other two are
the insert half of #14259's provenance seam
(`engine-hook-provenance-sibling-seams.test.ts`). Moving that decision
is not this card's, so the hide covers the static strip only.
- **The audit timeline stays visible under `preserveAudit`.** On a
create the static strip takes `created_at`, `created_by`, `updated_at`
and `updated_by` whatever the flag says. But the historical-import
channel that the #15964 ruling kept working reinstates them through the
audit binder's `preserveAudit` branch (`record.created_at ?? now`).
Withholding them would erase every historical `created_at`. A business
`readonly` column under `preserveAudit` is still withheld.
- **`sys_` and platform-internal (`managedBy`) objects keep their own
guards.** The create strip's subject excludes them already, so nothing
is withheld there.
There is no `ctx.submitted` on a create. The one in-repo `beforeInsert`
reader that needs a caller's read-only value is the audit binder's
`preserveAudit` branch, and it keeps seeing it.
Entry points (M3): `ObjectQL.insert` is the only `beforeInsert` dispatch
site in the repo. Single insert, batch insert and `insertMany` all go
through it. The engine has no upsert of its own. The protocol batch
`upsert` create arm, REST `POST`, import and flows `create_record` all
call `engine.insert`. Seeds write with `SEED_WRITE_EXECUTION_CONTEXT`
(`isSystem: true`), so nothing is withheld for them.
## Docs touched
- `packages/spec/src/data/hook.zod.ts`: TSDoc only. Two sentences said
`beforeInsert` "is untouched" and "still receives the caller's own
values". The `submitted` describe ("update only") stays true and is
unchanged.
- `content/docs/protocol/objectql/security.mdx`: rule 5 and the #16344
callout now cover insert.
- `content/docs/permissions/system-context.mdx`: regenerated by `pnpm
gen:system-context-census`, 118 to 119 elevation read sites. The +1 is
the hide pass's non-system gate.
## Tests and gates
Head is `aa115f1c60`. The suites ran at `975f4b6690`; the only commit
since then is `aa115f1c60`, which regenerates the census page and
changes no code.
- **`@objectstack/objectql`:**
- `--project local`: 390 files, 7676 tests passed;
- `--project repo`: 1 file, 5 tests passed;
- `typecheck` (tsc plus `check:test-typecheck`): exit 0.
- **`@objectstack/rest`:**
- `--project local`: 264 files, 4954 passed, 326 skipped;
- `--project repo`: 5 files, 191 passed, 1 skipped.
- **Dogfood shards** (`OS_TEST_SHARD=k/3`):
- 1/3: 77 files, 569 passed;
- 2/3: 77 files, 545 passed, 1 skipped;
- 3/3: 76 files passed, 1 skipped; 679 tests passed, 8 skipped.
- **Gates:** `node scripts/pm/dispatch-gates.mjs --commands` at
`aa115f1c60` derived 113 commands. All 113 ran and exited 0, and `--ran`
reconciles them as 113 run, 0 NOT-MEASURED.
- `engine-insert-readonly-hook-input.test.ts` (new, 13 cases):
- the ordering probe with a writable-key control;
- the hotcrm shape;
- a default shown in place of a forgery;
- controls for `autonumber`, the system writer, strip reporting and the
strict refusal;
- a self-assignment no-op;
- batch and `insertMany`;
- `preserveAudit`;
- the explained fault;
- an update control.
- `hook-withheld-readonly-fault.test.ts`: one `beforeInsert` case.
- **Reverse verification:** `node scripts/ablation-replace.mjs`, anchor
`hookViewSource[i] = hidden.row;` replaced with `void hidden.row;`.
Result: 9 failed and 4 passed of 13. The four that stayed green are the
controls that do not depend on the hide. The restore was verified: blob
equals HEAD and `git diff HEAD` is empty.
## Acceptance notes
- **Insert and update still differ on `autonumber`.** `beforeUpdate`
does not see a caller-supplied `autonumber`, and `beforeInsert` still
does (#6339's pin). A hook that issues its own record number only when
the key is absent stands down on a forged one, and the sequence fills
it. Not measured through a public door, and nobody has picked this up.
- **Changeset level.** Settled by the contract-tier review:
`@objectstack/objectql` `minor` with `Clause-②: no (narrowing)`, a
BREAKING note and an ADR-0087 disposition, as the update-side twin
(#16344) was. See Patch round 1.
## Patch round 1
This round changes text only, after the contract-tier review graded the
narrowing BREAKING (FAIL on the semver level only; the engine change
passed).
- **The changeset**
`.changeset/22306-insert-readonly-withheld-from-before-insert.md`:
- is now `'@objectstack/objectql': minor` with `Clause-②: no
(narrowing)`. That is the launch-window spelling of a breaking change,
and pre mode stays on (`.changeset/pre.json`, tag `next`).
- carries a BREAKING paragraph. It names what a `beforeInsert` handler
reads on `ctx.input.data` on a non-system create, and the withheld set:
`staticReadonlyInsertSubject`'s, minus the `autonumber` and
`preserveAudit` audit-timeline edges.
- names two answers that change at the REST door. Under
`strictReadonlyWrites`, a self-assigning hook's launder is now refused
(201 to 400). A hook or body that dereferences through a withheld key
answers 400 where it answered 201.
- carries one ADR-0087 disposition marker, `not-required
(no-migration-prescription)`, in the comment form the gate reads.
- **Two sentences corrected:** the self-assignment sentence now states
the engine's rule for a withheld key with a `defaultValue`, and "the two
cannot disagree" now names the `preserveAudit` shown-but-stripped edge.
Both corrections are in the changeset and in this body.
- **The two docs lines the review named as owed:**
`content/docs/automation/hooks.mdx` extends the persist-image sentence
to `beforeInsert`, and census row 21 in
`content/docs/permissions/system-context.mdx` names the second
`isSystem` read.
- **`@objectstack/spec` is not added to the changeset.** Its diff is
TSDoc that documents objectql's behaviour. Listing it would print
objectql's BREAKING note into spec's CHANGELOG under a `patch`, and an
upgrader greps objectql's entry.
- **Gates:**
- `check-adr-0087-registration --base origin/main` reads the changeset
as `BREAKING+clause-②-narrowing` with a `not-required
(no-migration-prescription)` disposition.
- `check-changeset-no-major --base origin/main` exits 0, and its level
axis, run against this body as the PR event, is reported in the os-dev
report.
- `dispatch-gates --commands` was re-derived on the final commit, run
and reconciled with `--ran`. The figures are in the os-dev report on the
card.
- No code or test changed in this round.
## Merge round
This round merges `origin/main` `ca135dcc40` into the branch (merge
commit `2008591e47`) and regenerates one page in `76bd4fd399`. Head is
`76bd4fd399`.
- **Census skew.** `main`'s `ca135dcc40` and this branch each took
`content/docs/permissions/system-context.mdx` from 118 to 119, for
different read sites.
- The merge driver deferred the page (os-regen), so the merged page read
119 while the merged tree holds 120 sites. `check:system-context-census`
was red on the merge commit with 6 declared-count mismatches (119 vs
120).
- `pnpm gen:system-context-census` rewrote the counts, never by hand:
119 to 120 sites, 125 to 126 property reads, 116 to 117
behaviour-bearing.
- The gate now exits 0 at 120 sites. Both sides' rows survive: against
`main`, the page differs only by the counts and this branch's row-21
clause.
- **Readings at `76bd4fd399`:**
- `@objectstack/objectql` `--project local`: 390 files, 7676 passed;
- `--project repo`: 1 file, 5 passed;
- `typecheck` (tsc plus `check:test-typecheck`): exit 0;
- the pin files `engine-insert-readonly-hook-input.test.ts` and
`hook-withheld-readonly-fault.test.ts`: 2 files, 24 passed.
- `dispatch-gates --commands` derived the same 113 commands. All 113 ran
and exited 0, and `--ran` reconciles them as 113 run, 0 NOT-MEASURED.
- **Nothing else changed:** no code, test or changeset edit.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3054516 commit f05649f
9 files changed
Lines changed: 709 additions & 25 deletions
File tree
- .changeset
- content/docs
- automation
- permissions
- protocol/objectql
- packages
- objectql/src
- spec/src/data
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
184 | 184 | | |
185 | 185 | | |
186 | 186 | | |
187 | | - | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
188 | 190 | | |
189 | 191 | | |
190 | 192 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
123 | 123 | | |
124 | 124 | | |
125 | 125 | | |
126 | | - | |
| 126 | + | |
127 | 127 | | |
128 | 128 | | |
129 | 129 | | |
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
144 | | - | |
| 144 | + | |
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
| |||
284 | 284 | | |
285 | 285 | | |
286 | 286 | | |
287 | | - | |
| 287 | + | |
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
| |||
358 | 358 | | |
359 | 359 | | |
360 | 360 | | |
361 | | - | |
| 361 | + | |
362 | 362 | | |
363 | 363 | | |
364 | 364 | | |
365 | | - | |
366 | | - | |
| 365 | + | |
| 366 | + | |
367 | 367 | | |
368 | 368 | | |
369 | 369 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
276 | 276 | | |
277 | 277 | | |
278 | 278 | | |
279 | | - | |
| 279 | + | |
280 | 280 | | |
281 | 281 | | |
282 | 282 | | |
| |||
304 | 304 | | |
305 | 305 | | |
306 | 306 | | |
307 | | - | |
308 | | - | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
309 | 317 | | |
310 | 318 | | |
311 | 319 | | |
| |||
0 commit comments