Skip to content

Commit 00ff228

Browse files
os-muskclaude
os-musk
and
claude
authored
fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance (#14472)
* wip(objectql): thread hook-write provenance into the readonlyWhen and runtime-owned strips Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * test(objectql): pin the two sibling seams on hook-write provenance Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * chore(changeset): hook-write provenance for the two sibling strips Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * fix(objectql): decide the insert-side runtime-owned strip by hook-write provenance Withholds the readonlyWhen sibling seam: threading the record into isCallerSuppliedValue was measured to let a caller value survive a TRUE readonlyWhen predicate (#9107 pin LOCK 3b), which #14259's fork clause routes to the decision inbox rather than to a unilateral choice. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * fix(objectql): honour the caller's limit in the new pin's driver double; re-anchor the system-context census Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent e5812fa commit 00ff228

5 files changed

Lines changed: 594 additions & 32 deletions

File tree

‎.changeset/hook-provenance-sibling-seams.md‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): decide the insert-side runtime-owned strip by hook-write PROVENANCE, not `Object.is` (#14259)
6+
7+
#14088 replaced `Object.is(payload[k], supplied[k])` inside `stripReadonlyFields`
8+
with a recording of the keys the before-phase hook chain actually assigned
9+
(`recordHookPayloadWrites`). Its argument was never about `null`: value equality
10+
cannot separate *the hook deliberately wrote the value the caller also sent* from
11+
*the hook never touched the key*, and those two demand opposite verdicts.
12+
13+
`stripRuntimeOwnedFields` — the INSERT-side twin — was left on the comparison
14+
that argument retired, and #6339's own prose is the finding: it argued a key SET
15+
made the contract true "only BY ACCIDENT" and moved to values, which is
16+
accidental in the identical way. A `beforeInsert` hook that re-issues or
17+
normalises a record number therefore still lost its write to any caller that
18+
submitted the same value — the caller who omitted the key kept the hook's number,
19+
the caller who echoed it got the sequence value, and the two differed in nothing
20+
else.
21+
22+
`engine.insert` now arms one recording **per row** at hook-context construction
23+
and seals each immediately after that row's `beforeInsert` chain, and
24+
`stripRuntimeOwnedFields` consults the sealed record before the value test. Per
25+
row, never per call, so a hook stamping one row of a batch confers nothing on the
26+
next.
27+
28+
⛔ **Not a relaxation of #5503, and the accept set for callers does not move.** A
29+
caller-seeded record number that no hook assigned is still stripped, still warns
30+
with the same text, and still reports through `onFieldsDropped` /
31+
`strictReadonlyWrites`; `isSystem` and `preserveAudit` are untouched. What
32+
changed is only the EVIDENCE for the hook-write exemption that already existed —
33+
a record of which keys were assigned, instead of an inference from the values
34+
afterwards.
35+
36+
The forgery boundary is inherited verbatim: a caller-supplied value must never
37+
become hook-owned. The new insert-side recording is armed after the caller's
38+
payload has arrived and been snapshotted, sealed before any engine-owned pass
39+
touches the row, and records that an assignment ran rather than anything about
40+
the payload's contents — a caller cannot execute an assignment, so no key it
41+
sends can enter the record. A hook that REPLACES the payload object leaves no
42+
attributable record and falls back to the pre-existing value test, which
43+
over-strips: keeping the old bug is the only safe direction, because reading a
44+
replacement's keys as hook-owned would launder a caller's forgery.
45+
46+
The `readonlyWhen` sibling seam #14259 also names (`isCallerSuppliedValue`,
47+
behind `stripReadonlyWhenFields` / `stripReadonlyWhenFieldsMulti`) is **not**
48+
included: threading the record there was measured to let a caller's value survive
49+
a TRUE `readonlyWhen` predicate, which is a maintainer decision rather than a
50+
mechanical follow-through. Nothing about that seam's behaviour changes here.

‎content/docs/permissions/system-context.mdx‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,18 @@ that silently does not happen.
109109

110110
| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
111111
|:--|:---|:---|:---|:---|
112-
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11049` |
113-
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11217` |
114-
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9827` |
112+
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11128` |
113+
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11296` |
114+
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9895` |
115115
| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` |
116-
| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9864`, `readonly-strict-errors.ts:66` |
117-
| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5761` |
118-
| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3605`, `:3615`, `:3642` |
116+
| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9943`, `readonly-strict-errors.ts:66` |
117+
| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5762` |
118+
| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3606`, `:3616`, `:3643` |
119119
| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
120120
| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` |
121-
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6459` |
122-
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11810` |
123-
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11739` |
121+
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6460` |
122+
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11889` |
123+
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11818` |
124124

125125
### 3. Sharing (`plugin-sharing`)
126126

@@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them.
179179

180180
| # | Site | Package | What it does |
181181
|:--|:---|:---|:---|
182-
| 62 | `objectql/src/engine.ts:3412` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183-
| 63 | `objectql/src/engine.ts:14159` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
182+
| 62 | `objectql/src/engine.ts:3413` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183+
| 63 | `objectql/src/engine.ts:14238` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
184184
| 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
185185
| 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
186186

@@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
195195
|:---|:---|:---|
196196
| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:685` |
197197
| "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
198-
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9810`–`9827` |
198+
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9878`–`9895` |
199199
| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) |
200200
| "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` |
201201
| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` |

0 commit comments

Comments
 (0)