diff --git a/.changeset/tidy-cups-smile.md b/.changeset/tidy-cups-smile.md new file mode 100644 index 0000000000..584113db15 --- /dev/null +++ b/.changeset/tidy-cups-smile.md @@ -0,0 +1,18 @@ +--- +'@objectstack/objectql': minor +'@objectstack/metadata-protocol': minor +'@objectstack/service-automation': patch +'@objectstack/lint': patch +'@objectstack/spec': patch +--- + +**BREAKING (behaviour):** a static `readonly` field is now stripped from a **non-system caller's INSERT payload inside `engine.insert`**, exactly as it already was on `engine.update`. A non-system create that used to write a read-only column now has that column dropped, reported through `onFieldsDropped` / `droppedFields`, logged at `warn`, and refused outright under `strictReadonlyWrites`. Seeding a read-only column at create time is a **system** act — use `context.isSystem`, a flow's `runAs: 'system'`, a system hook or a seed. + +Until now the create-side strip lived only at the DataProtocol ingress (`stripReadonlyForInsert` in `@objectstack/metadata-protocol`), so `readonly` meant one thing on insert and another on update: every external REST/GraphQL/MCP create was stripped, while a caller reaching `engine.insert` directly — the automation engine's `create_record` among them — wrote the column with no refusal, no `WARN` and no dropped-field event. + +- `stripReadonlyForInsert` and its five call sites in `@objectstack/metadata-protocol` are **deleted**, not kept as a second implementation; every create face — `createData`, `cloneData`, `createManyData`, `insertManyData`, and `batchData`'s `create` rows and both arms of `upsert` that create — now hands the caller's payload to the engine whole, and every face whose response carries `droppedFields` (`createData`, `createManyData`, `insertManyData`, every `batchData` row that created) reports the engine's own verdict there, so `droppedFields` says the same thing at each of those seams. `cloneData` forwards whole but reports nothing on the wire: its response contract (`CloneDataResponseSchema`, declared as produced) has no `droppedFields` member, so a clone that carried or overrode a read-only column is stripped and logged at `warn` but not reported in the 201 body — adding that key is a spec change, not part of this one. +- `create_record` (`@objectstack/service-automation`) starts receiving readonly drops on the `onFieldsDropped` channel it has been wired for since #3407 — a flow without `runAs: 'system'` that seeds a read-only column now reports a node warning and `output.droppedFields` instead of a clean success. That package's own code changes only in prose; the traffic is new, the surface is not. +- Unchanged, deliberately: `isSystem` is still the exemption; `preserveAudit` is still an UPDATE-path exemption and a create that asks for it is told so out loud; runtime-owned types (`autonumber`) keep their own pass and their own wider whitelist; platform objects (`managedBy`, the `sys_` namespace) are still left to their own field-write guards; `readonlyWhen` still has no create-side strip. A stripped key's `defaultValue` is re-derived, so a forged `approval_status` becomes `draft` rather than NULL. +- `@objectstack/lint` and `@objectstack/spec` are `patch`: both change prose only. All three lint rules — `validate-readonly-action-writes`, `validate-readonly-flow-writes`, `validate-readonly-hook-writes` — drop the superseded "INSERT is exempt" premise from their docblocks and from the justification of their green control cases; the two non-elevated rules now name their `insert`/`create` silence as a scan gap rather than an exemption (the action rule additionally records its now-reasoned refusal as a module-local constant that its `index` does not re-export, so no public surface widens). The spec change is prose only: one docblock sentence that named the deleted function, the `strictReadonlyWrites` contract docblock (which now states what strict refuses on insert), and the `readonly` liveness-ledger verdict, whose evidence pointer named the deleted ingress strip. + + diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 4bf5090d7e..1983282f55 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -252,23 +252,23 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update ### Writing a `readonly` field -There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them. +There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but only through the own-hook stamp, or under a **system** context. A nested `ctx.api` write on a non-system trigger is **not** a channel, and since the maintainer ruling of 2026-09-03 ([#14147](https://github.com/objectstack-ai/objectstack/issues/14147)) that is as true of `insert` as of `update`. -`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So: +`readonly` governs the *caller* surface. On every non-system write — UPDATE, and INSERT too since that ruling moved the create-side strip into `engine.insert` — the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So: | How the body writes it | What happens | |:---|:---| | `ctx.input. = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. | | `ctx.api.object('x').update({ })` | **Silently dropped** — unless the hook declares `runAs: 'system'`. `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped, and the call still returns success. Declaring [`runAs: 'system'`](/docs/automation/hooks#elevation--runas) gives that `ctx.api` a system context, which the strip skips, so the write lands. | | `ctx.api.sudo().object('x').update({ })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. The knob to reach for is [`runAs: 'system'`](/docs/automation/hooks#elevation--runas) on the hook itself, which is declarative and works on **both** surfaces. | -| `ctx.api.object('x').insert({ })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. | +| `ctx.api.object('x').insert({ })` | **Silently dropped** — unless the hook declares `runAs: 'system'`, exactly as the `update` row. `engine.insert` runs the same static strip under the same `isSystem` gate, so a non-system create no longer seeds a read-only column: the key is removed, the field falls back to its `defaultValue`, and the call still returns success. Seeding a read-only column at create time is a **system** act — `runAs: 'system'` here, or a `beforeInsert` stamp on the target object. (Before the 2026-09-03 ruling this row read "Lands — INSERT is exempt"; that row is superseded.) | The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**: - `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`. - `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object. -Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425). +Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `insert`/`create` are skipped too — but since the 2026-09-03 ruling that is a **scan gap**, not an exemption: the write is dropped exactly as the table says, and nothing reports it at build time yet ([#15394](https://github.com/objectstack-ai/objectstack/issues/15394)). The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), with the same gap on `create_record`. The table above is about a **hook** body. An **action** body is the one surface where the answer changes, so read this before you move a body from one to the other: an action body runs **elevated** — its `ctx.api` is built over the caller's envelope with `isSystem` set, which is the same trusted posture that lets an action bypass row and field permissions — and the static strip applies only to non-system callers. So `ctx.api.object('x').update({ someReadonlyField })` **lands** in an action, and there is no finding for it. Elevation does not waive the *conditional* lock, though, so that half does carry across: `action-api-update-readonly-when-field` — a **warning** — on an action body's literal `ctx.api` update to a `readonlyWhen` field ([#13770](https://github.com/objectstack-ai/objectstack/issues/13770)). Net effect when you move a body: a `readonly` write changes behaviour, a `readonlyWhen` write does not. diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index cf37881c0c..2d4483f95a 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -316,7 +316,7 @@ These properties are available on all field types: | `description` | `string` | — | Developer documentation | | `inlineHelpText` | `string` | — | Help text shown in UI | | `hidden` | `boolean` | `false` | Hide from default views | -| `readonly` | `boolean` | `false` | Prevent editing — hidden from create/edit forms AND server-enforced on both write paths: a non-system write to the field is silently dropped on `UPDATE` (in the engine) and on `INSERT` through the data API (REST/MCP/import, at the DataProtocol ingress). A stripped field still falls back to its `defaultValue`; **seeding a `readonly` column at create requires a system context** (import/migration/programmatic seed). Platform (`sys_`/`managedBy`) objects are governed by their own write policy instead — the resolved-affordance write guard keyed off the object's [lifecycle bucket](/docs/data-modeling/objects#lifecycle-bucket-managedby) (ADR-0103), not this field-level flag. | +| `readonly` | `boolean` | `false` | Prevent editing — hidden from create/edit forms AND server-enforced on both write paths: a non-system write to the field is silently dropped on `UPDATE` and, since the maintainer ruling of 2026-09-03, on `INSERT` — both in the engine, so a direct `engine.insert` caller (a flow's `create_record`, a hook body's `ctx.api`) is covered exactly like the data API's REST/MCP/import faces. A stripped field still falls back to its `defaultValue`; **seeding a `readonly` column at create requires a system context** (import/migration/programmatic seed). Platform (`sys_`/`managedBy`) objects are governed by their own write policy instead — the resolved-affordance write guard keyed off the object's [lifecycle bucket](/docs/data-modeling/objects#lifecycle-bucket-managedby) (ADR-0103), not this field-level flag. | | `sortable` | `boolean` | `true` | Allow sorting by this field | | `group` | `string` | — | Group name for organizing in forms (e.g. `'billing'`) | diff --git a/content/docs/kernel/contracts/data-engine.mdx b/content/docs/kernel/contracts/data-engine.mdx index 967eb870f6..b11f74df0c 100644 --- a/content/docs/kernel/contracts/data-engine.mdx +++ b/content/docs/kernel/contracts/data-engine.mdx @@ -303,13 +303,14 @@ The strips these two options cover are the engine's legal ones: | Strip | `reason` | Verbs | Writers it skips | |:---|:---|:---|:---| -| Static `readonly: true` (#2948) | `readonly` | `update` | `isSystem` | +| Static `readonly: true` (#2948; on `insert` too since the 2026-09-03 ruling, #14147) | `readonly` | `insert` **and** `update` | `isSystem` | | A TRUE `readonlyWhen` predicate (#3042) | `readonly_when` | `update` | none at the API boundary — every caller, `isSystem` included; a value a `beforeUpdate` hook derived or overwrote is not a caller write and is never stripped (#9107) | | Implicitly-readonly runtime-owned type (#5503 — `RUNTIME_OWNED_FIELD_TYPES`, today `autonumber`) | `readonly` | `insert` **and** `update` | `isSystem`, `preserveAudit` (#3493) | | Primary-key strip of a payload `id` the update dispatch already ruled is not an identifier (#6437) | `primary_key` | `update` | none | -The two AUTHOR-DECLARED strips are insert-exempt at this seam by design (#3413) — -see **On `insert`** below. +Of the two AUTHOR-DECLARED strips only `readonlyWhen` is insert-exempt at this seam +(a conditional lock has no prior record on a create); the static `readonly` strip runs +on `insert` too since the 2026-09-03 ruling — see **On `insert`** below. {/* os:check */} ```typescript @@ -366,23 +367,33 @@ alternative outputs of one seam, not a sequence: `DroppedFieldsEvent` means "fields dropped and the write completed without them", and under strict the write does not complete. Quiet-and-observable or loud — pick one per call. -**On `insert`.** The two AUTHOR-DECLARED strips are deliberately insert-exempt at -this seam (#3413: an in-process create may seed a `readonly: true` field's initial -value, and `readonlyWhen` cannot lock anything on a create at all), so an insert -refusal can only ever be about a runtime-owned value — a caller-supplied record -number. With the option `true` that insert throws (`operation: 'insert'`) and -nothing is written; without it the value is stripped, the write completes, and -`onFieldsDropped` fires with `reason: 'readonly'`. The engine-level writers exempt -from that strip — and therefore never refused — are the two the error message -itself names: `isSystem`, and the `preserveAudit` historical import reinstating -legacy record numbers (#3493). +**On `insert`.** Until the maintainer ruling of 2026-09-03 (option C, #14147) this +paragraph said the two AUTHOR-DECLARED strips were insert-exempt at this seam +(#3413: an in-process create may seed a `readonly: true` field's initial value). +That row is superseded: `engine.insert` now runs the static `readonly` strip for a +non-system caller — the same `stripReadonlyFields`, under the same `isSystem` gate, +as `engine.update` — and the DataProtocol ingress copy that used to cover external +callers only is deleted. One semantics, one enforcement point. `readonlyWhen` alone +stays insert-exempt (a conditional lock has no prior record on a create). So an +insert refusal is about a runtime-owned value — a caller-supplied record number — +**or** a static `readonly` value from a non-system caller. With the option `true` +that insert throws (`operation: 'insert'`, every taken field in one list) and +nothing is written; without it the values are stripped, the write completes, a +stripped `readonly` field falls back to its `defaultValue`, and `onFieldsDropped` +fires once with `reason: 'readonly'`. The writers exempt — and therefore never +refused — differ per strip, and that difference is the 2026-08-08 ruling, not this +one: `isSystem` exempts both; the `preserveAudit` historical import reinstating +legacy record numbers (#3493) exempts the runtime-owned strip only, and a +non-system create that asks for it still has its static `readonly` fields stripped, +with a `warn` saying the exemption is UPDATE-only (#6640). -**Layering — this is the engine seam.** The exemption pair above is *this* -in-process seam's. The DataProtocol ingress enforces its own author-declared -`readonly` policy on create (#3043), where `preserveAudit` is UPDATE-only (#6640) -and runtime-owned types are left to the engine strip — see `FieldSchema.readonly`. -Nothing on this page widens or narrows that ingress policy. +**Layering — this is the engine seam, and since the 2026-09-03 ruling the only +one.** The DataProtocol create faces (`createData`, `cloneData`, `createManyData`, +`insertManyData`, `batchData`) forward the caller's payload whole and surface this +seam's `onFieldsDropped` as their response `droppedFields` wherever their contract +declares one; none carries a `readonly` policy of its own any more — see +`FieldSchema.readonly`. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d00b4d8e25..a057a16cde 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -9,8 +9,8 @@ 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 **106 -distinct sites across 20 packages**, and knowing three of those behaviours gives +because the flag is not one concept: it is a single boolean read at **105 +distinct sites across 19 packages**, and knowing three of those behaviours gives no hint that the other hundred-and-three exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, and the gap was observable only by querying the resulting rows. @@ -100,7 +100,7 @@ that silently does not happen. | 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1412` | | 12 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `perf-timing.ts:474` | | 13 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `permission-set-overlay-discard.ts:142` | -| 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | +| 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | | 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1580` | @@ -109,67 +109,66 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 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:11450` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11633` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10183` | -| 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:1795` | -| 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:10231`, `readonly-strict-errors.ts:66` | -| 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:6050` | -| 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:3799`, `:3809`, `:3836` | -| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | -| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` | -| 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:6748` | -| 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:12249` | -| 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:12178` | +| 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:11545` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11728` | +| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `objectql/src/engine.ts:10193` | +| 21 | 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:10326`, `readonly-strict-errors.ts:66` | +| 22 | **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:6052` | +| 23 | 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:3799`, `:3809`, `:3836` | +| 24 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | +| 25 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` | +| 26 | 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:6751` | +| 27 | 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:12344` | +| 28 | 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:12273` | ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 106 sites**. +The largest single consumer — **17 of the 105 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| -| 30 | **Sharing-rule REVOCATION is skipped on the record-`afterDelete` hook** — and on that hook only | Lose: nothing permanently — the revoke is **delivered, but deferred on the unbounded shape**. The payload belongs to another subscriber: `record-share-cascade.ts` binds on every sharing-capable object and stashes for system writes on its own account (#5103). When the deleted ids are enumerable it revokes inline; when they are not — a predicate delete whose row set the stash could not resolve — it hands the reclaim to a queued background orphan sweep instead, so the share rows outlive the deleted records until that sweep runs, with the boot orphan sweep behind it. No surviving record loses access either way, and a restart re-runs the same sweep. This is one subscriber declining work another owns, not elevation silencing a consequence. ⚠️ **Grant MATERIALISATION no longer asks** — the `afterInsert` / `afterUpdate` skips, and the `before*` stash skip that fed them, were removed by the 2026-08-31 ruling on #13533; a system write materialises exactly as a user write does | `rule-hooks.ts:292` | -| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:677` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1787` | -| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1238` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1476` (guard at `:1501`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | -| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | -| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:202`, `:427` | +| 29 | **Sharing-rule REVOCATION is skipped on the record-`afterDelete` hook** — and on that hook only | Lose: nothing permanently — the revoke is **delivered, but deferred on the unbounded shape**. The payload belongs to another subscriber: `record-share-cascade.ts` binds on every sharing-capable object and stashes for system writes on its own account (#5103). When the deleted ids are enumerable it revokes inline; when they are not — a predicate delete whose row set the stash could not resolve — it hands the reclaim to a queued background orphan sweep instead, so the share rows outlive the deleted records until that sweep runs, with the boot orphan sweep behind it. No surviving record loses access either way, and a restart re-runs the same sweep. This is one subscriber declining work another owns, not elevation silencing a consequence. ⚠️ **Grant MATERIALISATION no longer asks** — the `afterInsert` / `afterUpdate` skips, and the `before*` stash skip that fed them, were removed by the 2026-08-31 ruling on #13533; a system write materialises exactly as a user write does | `rule-hooks.ts:292` | +| 30 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:677` | +| 31 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1787` | +| 32 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1238` | +| 33 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1476` (guard at `:1501`) | +| 34 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | +| 35 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | +| 36 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | +| 37 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | +| 38 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:202`, `:427` | ### 4. Approvals, reports, attachments, comments, knowledge | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` | -| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` | -| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3196`, `:3342`, `:3509`, `:3580`, `:3769`, `:3809` | -| 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` | -| 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` | -| 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` | -| 46 | Comment access hooks return early (insert + update + delete, and the read AST) | plugin-audit | Lose: comment visibility scoping | `comment-access-hooks.ts:322`, `:449`, `:488`, `:540` | -| 47 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `service-knowledge/src/knowledge-service.ts:316` | +| 39 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` | +| 40 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` | +| 41 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3196`, `:3342`, `:3509`, `:3580`, `:3769`, `:3809` | +| 42 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` | +| 43 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` | +| 44 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` | +| 45 | Comment access hooks return early (insert + update + delete, and the read AST) | plugin-audit | Lose: comment visibility scoping | `comment-access-hooks.ts:322`, `:449`, `:488`, `:540` | +| 46 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `service-knowledge/src/knowledge-service.ts:316` | ### 5. Actions, metadata plane, provenance | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | -| 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4967`, `:6381`, `:6629`, `:7060`, `:7253` | -| 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:422`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | -| 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | -| 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | -| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:241`, `:274` | -| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` | -| 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | -| 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | -| 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:59`, `webhook-provenance.ts:50` | -| 60 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `runtime-identity.ts:279`, called from `builtin/crud-nodes.ts:319` | -| 61 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `inbox-caller.ts:148` | +| 47 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | +| 48 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | +| 49 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4967`, `:6381`, `:6629`, `:7060`, `:7253` | +| 50 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 49's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | +| 51 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:422`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | +| 52 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | +| 53 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | +| 54 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:241`, `:274` | +| 55 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` | +| 56 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | +| 57 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | +| 58 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:59`, `webhook-provenance.ts:50` | +| 59 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `runtime-identity.ts:279`, called from `builtin/crud-nodes.ts:319` | +| 60 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `inbox-caller.ts:148` | ### 6. Reads that only carry the flag onward @@ -179,10 +178,10 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3606` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14693` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | -| 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | -| 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | +| 61 | `objectql/src/engine.ts:3606` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 62 | `objectql/src/engine.ts:14788` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 63 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | +| 64 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | --- @@ -195,8 +194,8 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "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:2032` (rationale at `:1942`–`1944`, #3760), `flow.zod.ts:702` | | "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) | -| "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:10166`–`10183` | -| "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:1580` (#3493 / #6640) | +| "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:10176`–`10193` | +| "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:1581` (#3493 / #6640) | | "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` | | "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:299` | | "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1548`, `:1577`; `domains/actions.ts:414` | @@ -235,7 +234,7 @@ should recognise it instead of re-deriving it. a **bug**, because a sharing rule's declared semantics is a published promise and `isSystem` names the operator, never a consequence that need not happen. Both materialisation skips and the notice that announced them are - gone; row 30 is now the `afterDelete` skip alone, which survives on the + gone; row 29 is now the `afterDelete` skip alone, which survives on the separate ground that another subscriber delivers that payload. ⚠️ The observability half of that reading is worth keeping in mind @@ -244,11 +243,11 @@ should recognise it instead of re-deriving it. see it?". A compensating path that exists but that nobody can be expected to know about is not a compensating path. -3. **Strict write observability is inert under elevation.** Row 22: a caller +3. **Strict write observability is inert under elevation.** Row 21: a caller that asked to be told loudly about dropped fields is told nothing, because nothing was dropped. The two facts are indistinguishable from the outside. -4. **`revoke()` skips its own conflict guard.** Row 34 is correct for the rule +4. **`revoke()` skips its own conflict guard.** Row 33 is correct for the rule evaluator and surprising for anything else: a system caller can delete a rule-materialised grant that the next reconcile silently restores. @@ -269,8 +268,8 @@ 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 106 read sites - in 20 packages. Splitting it is a breaking contract change across all of them. +- **Shipped semantics.** `isSystem` is a published contract with 105 read sites + in 19 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.) - **No business pull.** No app has asked for the combinations a split would @@ -326,15 +325,15 @@ 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 | 22 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 112 | ✅ | +| — parsed as a property **read** | 111 | ✅ | | — 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` | **106** | ✅ | -| — behaviour-bearing (rows 1–61 above) | 102 | ✅ | -| — carry the flag onward only (rows 62–65 above) | 4 | ✅ | -| Packages containing at least one elevation read | **20** | ✅ | -| Files containing at least one elevation read | 45 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **105** | ✅ | +| — behaviour-bearing (rows 1–60 above) | 101 | ✅ | +| — carry the flag onward only (rows 61–64 above) | 4 | ✅ | +| Packages containing at least one elevation read | **19** | ✅ | +| Files containing at least one elevation read | 44 | ✅ | The six rows marked — are a **dated decomposition, not a live claim**: they were measured on 2026-08-29 at `ca1965f2b5` and CI does not re-derive them. They count @@ -420,4 +419,4 @@ that introduces it — CI will say so if it is not. - [Authorization Architecture](/docs/permissions/authorization) — the six-gate enforcement chain this flag short-circuits - [Security & Access Control](/docs/protocol/objectql/security) — the `readonly` write strip and its exemptions - [State Machine](/docs/protocol/objectql/state-machine) — `skipStateMachine`, `preserveAudit`, `treatAsHistorical` -- [Sharing Rules](/docs/permissions/sharing-rules) — what row 30 is skipping +- [Sharing Rules](/docs/permissions/sharing-rules) — what row 29 is skipping diff --git a/content/docs/protocol/objectql/security.mdx b/content/docs/protocol/objectql/security.mdx index e83cb601d3..11c85423be 100644 --- a/content/docs/protocol/objectql/security.mdx +++ b/content/docs/protocol/objectql/security.mdx @@ -274,11 +274,11 @@ field, but cannot *rescue* one the caller supplied. **`preserveAudit` is an UPDATE-path exemption. It does not apply on INSERT (#6640).** -The two write paths run two different strips: UPDATE is stripped inside the engine -(`stripReadonlyFields`), which consults `preserveAudit`; CREATE is stripped earlier, at the -DataProtocol ingress (`stripReadonlyForInsert`, #3043), whose only exemption is -`context.isSystem`. So one historical import that *upserts* keeps an author-declared -`readonly` column on the rows it **updates** and strips it from the rows it **creates**. +Both write paths run the same strip inside the engine (`stripReadonlyFields`, #14147), but +they read it differently: UPDATE consults `preserveAudit`, while CREATE is passed no such +flag and keeps `context.isSystem` as its only exemption. So one historical import that +*upserts* keeps an author-declared `readonly` column on the rows it **updates** and strips +it from the rows it **creates**. That asymmetry is deliberate, not an oversight: `treatAsHistorical` arrives on an ordinary (non-system) REST import request, so honouring it on create would let any caller seed the diff --git a/content/docs/protocol/objectql/state-machine.mdx b/content/docs/protocol/objectql/state-machine.mdx index 3cdb53e9dd..383094f370 100644 --- a/content/docs/protocol/objectql/state-machine.mdx +++ b/content/docs/protocol/objectql/state-machine.mdx @@ -112,7 +112,7 @@ transitions: { - **Seed writes are exempt** (#3433). Curated seed data — package bootstrap fixtures, marketplace templates, per-org replay, all loaded by `SeedLoaderService` — is a snapshot of established facts, not a record walking its lifecycle, so it bypasses the `state_machine` rule entirely: a seed may be born mid-lifecycle (a `completed` project, a `closed_won` opportunity) and neither `initialStates` (insert) nor `transitions` (update) is enforced. Every *other* validation still runs, so a seed must still satisfy field shape, `format`, `script`, and the rest. `os lint` warns when a seeded value is not a state the machine declares, so a typo is still caught before boot. - **A "historical" data import is exempt too** (#3479). Migrating established facts — a batch of already-`closed` tickets, `closed_won` deals — is the same "snapshot, not a lifecycle event" situation. Set `treatAsHistorical: true` on the import request (default **off**) and the runner puts `skipStateMachine` on the write context, so `initialStates` doesn't reject those mid-lifecycle rows. A normal import leaves it off and still walks the FSM — the strict behavior is the default, so the exemption is always an explicit opt-in. - **`treatAsHistorical` also preserves the original audit timeline** (#3493) — **on the rows an import UPDATES** (#6640). Skipping the FSM is only half of migrating established facts; the other half is keeping *when* they happened and *who* did them. Under the same flag the write context also carries `preserveAudit`, which (1) makes `updated_at` / `updated_by` **client-preferred** — a supplied historical last-modified survives instead of being stamped with the import instant — and (2) admits a **whitelist** through the static-`readonly` write strip: the audit/timestamp family plus author-declared business `readonly` fields (`closed_at`, `resolved_by`, …) — but never the record's own primary key (`id`), which is the address of the write rather than a fact being restored (#8215). Platform-managed `system` columns outside that family (`organization_id` and other tenancy/generated columns) stay stripped — a historical import reinstates facts, it does not forge tenancy. Like the FSM exemption this is opt-in: a normal write still auto-stamps `updated_at`/`updated_by` and strips `readonly` exactly as before, and permissions / RLS / field-level security are unchanged. -- **…but a historical `upsert` still drops those columns from the rows it CREATES** (#6640). `preserveAudit` is an **UPDATE-path exemption and nothing else reads it**, because the two write paths run two different strips: UPDATE is stripped inside the engine (`stripReadonlyFields`), which consults `preserveAudit`; CREATE is stripped earlier, at the DataProtocol ingress (`stripReadonlyForInsert`, #3043) that every REST-import create travels, and that one's only exemption is `context.isSystem`. So a single `treatAsHistorical` upsert keeps `closed_at` on the rows it **matches** and strips it — together with a supplied `created_at` / `updated_at`, which the injected audit columns also declare `readonly` — from the rows it **inserts**. The asymmetry is deliberate, not an oversight: `treatAsHistorical` arrives on an ordinary (non-system) import request, so honouring it on create would let any caller seed the approval/status columns that create-side strip exists to protect. The ignored request is at least no longer silent — the server logs a `WARN` naming the object, the stripped fields and this UPDATE-only rule — but the strip still applies. **To replay archival read-only facts on the rows an import creates, write from a system context** (`isSystem`). Full rule and rationale: [Security & Access Control](/docs/protocol/objectql/security). +- **…but a historical `upsert` still drops those columns from the rows it CREATES** (#6640). `preserveAudit` is an **UPDATE-path exemption and nothing else reads it**: both write paths run the same in-engine strip (`stripReadonlyFields`, #14147), but only the UPDATE call site passes `preserveAudit` — the create side is given no such flag and keeps `context.isSystem` as its only exemption. So a single `treatAsHistorical` upsert keeps `closed_at` on the rows it **matches** and strips it — together with a supplied `created_at` / `updated_at`, which the injected audit columns also declare `readonly` — from the rows it **inserts**. The asymmetry is deliberate, not an oversight: `treatAsHistorical` arrives on an ordinary (non-system) import request, so honouring it on create would let any caller seed the approval/status columns that create-side strip exists to protect. The ignored request is at least no longer silent — the server logs a `WARN` naming the object, the stripped fields and this UPDATE-only rule — but the strip still applies. **To replay archival read-only facts on the rows an import creates, write from a system context** (`isSystem`). Full rule and rationale: [Security & Access Control](/docs/protocol/objectql/security). - **Undoing a historical import is symmetric** (#3549 / #3556). The import undo (`POST /api/v1/data/import/jobs/:jobId/undo`) logically rolls back a finished job — deleting the rows it created and restoring the captured pre-import snapshot on the rows it updated. That restore write now carries `preserveAudit` too, but **only** when the job was flagged `treatAsHistorical`, so the snapshotted `updated_at` / `updated_by` and business `readonly` fields (`closed_at`, …) are reinstated verbatim instead of being re-stamped to the undo instant. The undo is unaffected by the create-side carve-out above: it only ever *deletes* the rows the import created and *updates* the rows it touched, so every write it makes is on the path where the exemption is real. Without it the undo would silently overwrite the very timeline the historical import preserved; a normal (non-historical) import's undo keeps the default stamp/strip. ### Conditional transitions diff --git a/content/docs/references/api/batch.mdx b/content/docs/references/api/batch.mdx index 927ea113ca..baebda6792 100644 --- a/content/docs/references/api/batch.mdx +++ b/content/docs/references/api/batch.mdx @@ -67,7 +67,7 @@ const result = BatchConfigSchema.parse(data); | **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error. A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back. | | **data** | `Record` | optional | Full record data (if returnRecords=true) | | **index** | `number` | optional | Index of the record in the request array | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | ### Nested Shape: `BatchOperationResult.errors[number]` @@ -201,7 +201,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error. A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back. | | **data** | `Record` | optional | Full record data (if returnRecords=true) | | **index** | `number` | optional | Index of the record in the request array | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | --- diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index 5435ac2bce..a1914be5d7 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -443,7 +443,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018) | **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error. A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back. | | **data** | `Record` | optional | Full record data (if returnRecords=true) | | **index** | `number` | optional | Index of the record in the request array | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | --- @@ -522,7 +522,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018) | **object** | `string` | ✅ | The object name. | | **id** | `string` | ✅ | The ID of the newly created record. | | **record** | `Record` | ✅ | The created record, including server-generated fields (created_at, owner). | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields that were LEGALLY stripped before the record was written — a non-system create cannot seed a static `readonly` column (ingress strip), so those keys are dropped and the field re-derives its default. Present ONLY when ≥1 field was dropped; the create still succeeded without them (status/success semantics unchanged). REST additionally surfaces this as the `X-ObjectStack-Dropped-Fields` response header. Optional — omit-when-empty keeps the shape backward-compatible for existing clients. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields that were LEGALLY stripped before the record was written — a non-system create cannot seed a static `readonly` column (the strip runs inside `engine.insert`, after the `beforeInsert` hooks, `isSystem`-gated), so those keys are dropped and the field re-derives its default. Present ONLY when ≥1 field was dropped; the create still succeeded without them (status/success semantics unchanged). REST additionally surfaces this as the `X-ObjectStack-Dropped-Fields` response header. Optional — omit-when-empty keeps the shape backward-compatible for existing clients. | ### Nested Shape: `CreateDataResponse.droppedFields[number]` @@ -558,7 +558,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | **object** | `string` | ✅ | Object name | | **records** | `Record[]` | ✅ | Created records | | **count** | `number` | ✅ | Number of records created | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied `readonly` fields the create-ingress strip removed before the rows were written. AGGREGATED across the batch (one event per object/reason with the union of dropped field names) rather than per-row, because the insert-time strip is static-`readonly` only — schema-uniform, so every row drops the same set. Present ONLY when ≥1 field was dropped; the creates still succeeded without them (count/success unchanged). Optional — omit-when-empty keeps the shape backward-compatible. (The per-row `insertMany`/`batch` paths carry per-row `droppedFields` on each result instead — see BatchOperationResultSchema.) | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied `readonly` fields the in-engine create-side strip (`engine.insert`, `isSystem`-gated) removed before the rows were written. AGGREGATED across the batch (one event per object/reason with the union of dropped field names) rather than per-row, because the insert-time strip is static-`readonly` only — schema-uniform, so every row drops the same set. Present ONLY when ≥1 field was dropped; the creates still succeeded without them (count/success unchanged). Optional — omit-when-empty keeps the shape backward-compatible. (The per-row `insertMany`/`batch` paths carry per-row `droppedFields` on each result instead — see BatchOperationResultSchema.) | ### Nested Shape: `CreateManyDataResponse.droppedFields[number]` @@ -658,7 +658,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error. A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back. | | **data** | `Record` | optional | Full record data (if returnRecords=true) | | **index** | `number` | optional | Index of the record in the request array | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | --- @@ -2835,7 +2835,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | **errors** | `{ code: Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| …>; declaredCode?: string; message: string; userMessage?: string; … }[]` | optional | Array of errors if operation failed. Branch on `errors[0].code` — an atomic batch that rolled back marks rows that were written then undone with code ROLLED_BACK and rows never reached with NOT_ATTEMPTED, while the causal row keeps its own error. A NON-atomic batch that stopped (the `continueOnError: false` default) marks its un-attempted tail with the same NOT_ATTEMPTED code — rows before the failure stay written and keep reporting success, since nothing was rolled back. | | **data** | `Record` | optional | Full record data (if returnRecords=true) | | **index** | `number` | optional | Index of the record in the request array | -| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the create-ingress strip. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when' \| 'primary_key'> }[]` | optional | Write-observability: caller-supplied fields LEGALLY stripped from THIS row before it was written — static `readonly` / TRUE `readonlyWhen` on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop different fields on different rows (`readonlyWhen` is record-state-dependent). Present ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). A single response header cannot express per-row drops, so this body field is the canonical bulk channel — REST does not emit `X-ObjectStack-Dropped-Fields` for batches. Optional — omit-when-empty keeps the shape backward-compatible. | --- diff --git a/content/docs/references/data/data-engine.mdx b/content/docs/references/data/data-engine.mdx index 725db503ba..d4b71ee1ea 100644 --- a/content/docs/references/data/data-engine.mdx +++ b/content/docs/references/data/data-engine.mdx @@ -68,7 +68,7 @@ const result = BaseEngineOptionsSchema.parse(data); | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -120,7 +120,7 @@ Options for DataEngine.aggregate operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -195,7 +195,7 @@ Options for DataEngine.count operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -267,7 +267,7 @@ Options for DataEngine.delete operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -446,7 +446,7 @@ Options for DataEngine.insert operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -516,7 +516,7 @@ Query options for IDataEngine.find() operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -809,7 +809,7 @@ Options for DataEngine.update operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -921,7 +921,7 @@ QueryAST-aligned options for DataEngine.aggregate operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -989,7 +989,7 @@ QueryAST-aligned options for DataEngine.count operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -1040,7 +1040,7 @@ QueryAST-aligned options for DataEngine.delete operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -1100,7 +1100,7 @@ QueryAST-aligned query options for IDataEngine.find() operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | @@ -1188,7 +1188,7 @@ QueryAST-aligned options for DataEngine.update operations | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | diff --git a/content/docs/references/kernel/execution-context.mdx b/content/docs/references/kernel/execution-context.mdx index 5b5b43b611..5f97168c87 100644 --- a/content/docs/references/kernel/execution-context.mdx +++ b/content/docs/references/kernel/execution-context.mdx @@ -53,7 +53,7 @@ const result = ExecutionContextSchema.parse(data); | **skipAutomations** | `boolean` | optional | | | **seedReplay** | `boolean` | optional | | | **skipStateMachine** | `boolean` | optional | | -| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | +| **preserveAudit** | `boolean` | optional | Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected. | | **oauthScopes** | `string[]` | optional | | | **accessToken** | `string` | optional | | | **transaction** | `any` | optional | | diff --git a/docs/qa/platform-checklist/areas/records-forms.json b/docs/qa/platform-checklist/areas/records-forms.json index 3a5d9b6d81..c0c4b9ed03 100644 --- a/docs/qa/platform-checklist/areas/records-forms.json +++ b/docs/qa/platform-checklist/areas/records-forms.json @@ -136,7 +136,7 @@ { "clause": "clone (POST /data/:object/:id/clone, gated by enable.clone default-on) returns 201 with a NEW id and the source's field VALUES copied, but engine-owned columns (id, audit, autonumber, formula, summary) and readonly columns (e.g. approval_status) RE-DERIVED not carried, and the clone is owned by the CLONER — not the source's owner", "oracle": "api", - "verify": "the 201 result carries {id (new ≠ sourceId), sourceId, record}; field-by-field diff shows business values copied and system/readonly columns re-derived (#3043 CLONE_STRIP_FIELDS + stripReadonlyForInsert); owner_id resolves to the signed-in cloner (the clone is a create in the caller's context — packages/metadata-protocol/src/protocol.ts cloneData)", + "verify": "the 201 result carries {id (new ≠ sourceId), sourceId, record}; field-by-field diff shows business values copied and system/readonly columns re-derived (#3043 CLONE_STRIP_FIELDS + the engine's create-side static-readonly strip, #14147); owner_id resolves to the signed-in cloner (the clone is a create in the caller's context — packages/metadata-protocol/src/protocol.ts cloneData)", "evidence": "clone response + source-vs-clone field diff + owner_id read" }, { @@ -160,7 +160,7 @@ "dogfood-verification skill §3", "examples/app-showcase/src/data/objects/account.object.ts (requiredness + format/conditional validations)", "packages/runtime/src/route-ledger.ts (/data CRUD routes)", - "packages/rest/src/rest-server.ts#registerDataActionEndpoints (POST /data/:object/:id/clone → registerDataActionEndpoints) + packages/metadata-protocol/src/protocol.ts#cloneData (cloneData: enable.clone gate, findOne-in-caller-context, CLONE_STRIP_FIELDS, stripReadonlyForInsert)", + "packages/rest/src/rest-server.ts#registerDataActionEndpoints (POST /data/:object/:id/clone → registerDataActionEndpoints) + packages/metadata-protocol/src/protocol.ts#cloneData (cloneData: enable.clone gate, findOne-in-caller-context, CLONE_STRIP_FIELDS; the static-readonly strip is engine.insert's since #14147)", "packages/rest/src/rest-route-ledger.ts (POST /api/v1/data/:object/:id/clone, client data.clone)", "objectui: e2e/live/record-history-display.spec.ts", "cross-ref: the inline-edit atomic two-surface behavior (ONE Save bar / ONE PATCH carrying exactly the changed keys + ifMatch) is folded into records-forms.concurrent-edit-conflict, not here" @@ -3860,7 +3860,7 @@ "evidence": "both responses + the unchanged count" }, { - "clause": "overrides WIN over copied business values — the override name lands on the clone — but cannot FORGE protected columns: stripReadonlyForInsert runs AFTER Object.assign(data, overrides) (protocol.ts), so a readonly/engine-owned key smuggled through overrides is dropped and re-derived, same as #3043's carried-over case", + "clause": "overrides WIN over copied business values — the override name lands on the clone — but cannot FORGE protected columns: the create-side static-readonly strip runs AFTER Object.assign(data, overrides) — the overrides are applied in protocol.ts cloneData and judged in engine.insert (#14147) — so a readonly/engine-owned key smuggled through overrides is dropped and re-derived, same as #3043's carried-over case", "oracle": "api", "verify": "the clone's re-read shows the override name AND the forged key re-derived (defaultValue / fresh audit stamp), not the smuggled value", "evidence": "the overrides payload + the clone re-read diff" @@ -3898,7 +3898,7 @@ "ref": "packages/metadata-protocol — search-clone-schema-conformance.test.ts (parses the real cloneData producer); packages/rest — search-clone-route-schema-conformance.test.ts (drives the live mount); named on the ledger row rest-route-ledger.ts" }, "source": [ - "packages/metadata-protocol/src/protocol.ts#cloneData (cloneData: registration gate #3770, CLONE_DISABLED, findOne-in-context, CLONE_STRIP_FIELDS + system/autonumber/formula/summary strip, overrides, stripReadonlyForInsert #3043, omitInternalFieldsFromWriteResponse #7823)", + "packages/metadata-protocol/src/protocol.ts#cloneData (cloneData: registration gate #3770, CLONE_DISABLED, findOne-in-context, CLONE_STRIP_FIELDS + system/autonumber/formula/summary strip, overrides, omitInternalFieldsFromWriteResponse #7823; the static-readonly strip moved to engine.insert #14147)", "packages/rest/src/rest-route-ledger.ts (POST /api/v1/data/:object/:id/clone, client data.clone, bare-201 note #11924)", "objectui: packages/app-shell/src/views/studio-design/ObjectSettingsPanel.tsx (Studio authors the enable.clone opt-out switch)", "cross-ref: the clone HAPPY path + engine-column re-derivation + RLS-gated 404 are crud-roundtrip clauses 7-8 — this item drives only the contract edges that item does not, and deliberately re-states none of its oracles" diff --git a/examples/app-todo/test/task-completion-trigger.test.ts b/examples/app-todo/test/task-completion-trigger.test.ts index 5387819508..dbafeebe6a 100644 --- a/examples/app-todo/test/task-completion-trigger.test.ts +++ b/examples/app-todo/test/task-completion-trigger.test.ts @@ -89,6 +89,10 @@ async function bootTodoKernel(): Promise<{ // so without this line the completion stamp would not exist here and every // completion below would be refused — which is exactly the bug, and exactly // why the old version of this file seeded `completed_date` on CREATE. + // [#14147] That seed is no longer available as a workaround either: a + // non-system create has a `readonly` column stripped just as an update does, + // so binding the hook is the only way this harness can complete a task. + // `task-recurrence.test.ts` bound it for the same reason. objectql.bindHooks([taskHook], { packageId: 'app:com.example.todo' }); for (const flow of allFlows) automation.registerFlow(flow.name, flow); @@ -227,15 +231,27 @@ describe('#6882 — app-todo `task_completion` is armed, not dead', () => { * update status+completed_date (user ctx): REJECTED -> Completed date is required when status is Completed * update status only (user ctx): REJECTED -> Completed date is required when status is Completed * update status+completed_date (isSystem): OK - * insert already-completed: OK + * insert already-completed (user ctx): OK * * — i.e. every escape was a NON-user path, and `completeTask` always failed. * + * ⚠️ [#14147] The FOURTH row of that table is history, not a live escape. It + * held because the create path was exempt from the static-`readonly` strip; the + * maintainer ruling of 2026-09-03 overturned that exemption, so a non-system + * create that seeds `completed_date` now has it stripped exactly as an update + * does, and an already-completed insert from a user context is refused by the + * same rule. The table is left as measured — it is dated evidence of the + * defect, and rewriting it would be rewriting the measurement — but nothing + * below may be read as saying a create may still seed a server-owned column. + * Seeding one at create time is a SYSTEM act (`context.isSystem`, + * `runAs: 'system'`, a system hook or a seed). + * * The repair is the server owning the column: `task.hook.ts` stamps it on the * transition, and the strip lets a hook's write through because it only - * deletes a key that still holds the *caller's own* value (#2948/#5591). The - * assertions below are written against that seam rather than against the - * message, so they stay meaningful if the wording changes. + * deletes a key that still holds the *caller's own* value (#2948/#5591) — the + * same guard on both write paths now. The assertions below are written against + * that seam rather than against the message, so they stay meaningful if the + * wording changes. */ describe('#7036 — completing a task is possible for a normal user', () => { const ctx = { context: { userId: 'u_todo' } }; diff --git a/examples/app-todo/test/task-recurrence.test.ts b/examples/app-todo/test/task-recurrence.test.ts index 74ee26990d..e15e0dfe54 100644 --- a/examples/app-todo/test/task-recurrence.test.ts +++ b/examples/app-todo/test/task-recurrence.test.ts @@ -45,6 +45,7 @@ import { RecordChangeTriggerPlugin } from '@objectstack/trigger-record-change'; import { allFlows, TaskCompletionFlow } from '../src/flows/index.js'; import { todoFunctions, computeNextTaskDueDate } from '../src/functions/index.js'; import { Task } from '../src/objects/task.object.js'; +import taskHook from '../src/objects/task.hook.js'; const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); @@ -57,10 +58,19 @@ afterEach(async () => { }); /** - * A real kernel with the app's real `todo_task` object and real flows — the + * A real kernel with the app's real `todo_task` object, hooks and flows — the * same harness `test/task-completion-trigger.test.ts` boots, plus the one wire * that card did not need: the function resolver. * + * [#14147] The hook wire is new here, and its absence is what this sentence used + * to overstate: the harness claimed to boot the same stack as its sibling while + * binding no hooks at all, so `task.hook.ts`'s `beforeUpdate` completion stamp + * — the app's real answer to `completed_date_required` since #7036 — never ran + * in this file. That is why it carried a `completed_date` CREATE-seed the + * sibling had already deleted. Binding the app's own hook is what lets the + * completion below travel the app's real user path; the app ships this hook, so + * a harness that omits it was testing a stack the app does not have. + * * `defineStack({ functions })` is bridged to `AutomationEngine.resolveFunction` * by the automation plugin when an app bundle is loaded through `AppPlugin`. * This suite registers the app's metadata directly (no bundle), so it wires the @@ -87,6 +97,7 @@ async function bootTodoKernel(): Promise<{ objectql.registerDriver(driver, true); openDrivers.push(driver); objectql.registry.registerObject(Task, 'todo', 'todo'); + objectql.bindHooks([taskHook], { packageId: 'app:com.example.todo' }); await objectql.syncSchemas(); automation.setFunctionResolver( @@ -112,12 +123,24 @@ function calendarDate(value: unknown): string { * Complete one recurring task through the real write path and return the task * the flow spawned, plus the run that spawned it. * - * `completed_date` is seeded on CREATE for the same reason - * `test/task-completion-trigger.test.ts` seeds it: the object's - * `completed_date_required` rule refuses a completion write without it, and the - * ordinary-user completion path is a separate card's subject (#7036). This card - * is about what the flow does once it fires, so the completion is driven the way - * the #6882 suite already drives it. + * [#14147] NO `completed_date` CREATE-seed. This helper used to carry one, + * citing `test/task-completion-trigger.test.ts` — but that file had already + * DELETED the same workaround at #7036 ("the CREATE-seed workaround this test + * used to carry existed only because the completion UPDATE was impossible"), + * and its `GATES:` case drives this identical write shape (a user-context + * predicate update to `status: 'completed'` on a task created without the + * column) green today — because that harness BINDS the app's hook, which this + * one did not. The seed was a stale copy of a workaround whose reason had + * already been fixed: `task.hook.ts`'s `beforeUpdate` leg stamps + * `completed_date` on the transition into `completed`, which is what satisfies + * the object's `completed_date_required` rule. Both halves had to change here: + * bind the hook (above) and drop the seed. + * + * It had to go now because it was also a NON-SYSTEM caller seeding a + * `readonly: true`, server-owned column on create — the act the maintainer + * ruling of 2026-09-03 makes the engine strip. Dropping it removes a workaround, + * not an assertion: the completion below now travels the app's real user path + * rather than around it. */ async function completeRecurringTask(opts: { subject: string; @@ -137,7 +160,6 @@ async function completeRecurringTask(opts: { is_recurring: true, recurrence_type: opts.recurrenceType, recurrence_interval: opts.interval ?? 1, - completed_date: '2026-08-09T10:00:00.000Z', }, ctx); const id = Array.isArray(created) ? created[0].id : created.id; @@ -323,10 +345,12 @@ describe('#7037 — the recurrence branch computes a real next due date', () => const { automation, data } = await bootTodoKernel(); const ctx = { context: { userId: 'u_todo' } }; + // [#14147] No `completed_date` seed — see `completeRecurringTask` above: + // the hook stamps it on the transition, and a non-system create may not + // seed a server-owned column. const created = await data.insert('todo_task', { subject: 'One-off task', status: 'not_started', priority: 'normal', owner: 'u_todo', due_date: '2026-08-10', is_recurring: false, - completed_date: '2026-08-09T10:00:00.000Z', }, ctx); const id = Array.isArray(created) ? created[0].id : created.id; @@ -375,11 +399,20 @@ describe('#7037 — the recurrence branch computes a real next due date', () => broken.type = 'autolaunched'; automation.registerFlow(broken.name, broken); + // [#14147] Unlike the cases above, this fixture must START from an + // already-completed row — there is no transition to stamp on, and the + // object's `completed_date_required` rule refuses a completed task with a + // blank `completed_date`. Seeding a server-owned column at create time is + // a SYSTEM act (maintainer ruling, 2026-09-03: `context.isSystem`, + // `runAs: 'system'`, a system hook or a seed), so this one write declares + // itself trusted. Nothing else about the case changes: the subject is + // still what the BROKEN flow fixture does with an uncomputed date, and + // the assertions below are untouched. const created = await data.insert('todo_task', { subject: 'Reverse fixture task', status: 'completed', priority: 'normal', owner: 'u_todo', due_date: '2026-08-10', is_recurring: true, recurrence_type: 'daily', recurrence_interval: 1, completed_date: '2026-08-09T10:00:00.000Z', - }, ctx); + }, { context: { userId: 'u_todo', isSystem: true } }); const id = Array.isArray(created) ? created[0].id : created.id; // Driven directly rather than through the record-change hook, so the diff --git a/packages/lint/src/validate-flow-node-writes.test.ts b/packages/lint/src/validate-flow-node-writes.test.ts index 049ed95eb8..7844e5a237 100644 --- a/packages/lint/src/validate-flow-node-writes.test.ts +++ b/packages/lint/src/validate-flow-node-writes.test.ts @@ -406,10 +406,12 @@ describe('validateFlowNodeWrites', () => { ).toEqual([]); }); - it('does NOT flag a readonly field on create_record — INSERT is engine-exempt from that strip', () => { - // The readonly rule skips create_record entirely (a create may legitimately - // seed readonly columns). This rule asks a different question, so a DECLARED - // readonly field is clean here for its own reason: it resolves to a column. + it('does NOT flag a readonly field on create_record — this rule asks whether the column EXISTS, not whether the write lands', () => { + // The readonly sibling does not scan create_record today — a scan gap + // (#15394) since the 2026-09-03 ruling put the static-`readonly` strip + // inside `engine.insert` (#14147), not an exemption. This rule asks a + // different question either way, so a DECLARED readonly field is clean here + // for its own reason: it resolves to a column. const withReadonly = { name: 'deal', fields: { stage: { type: 'text' }, approval_status: { type: 'text', readonly: true } }, diff --git a/packages/lint/src/validate-readonly-action-writes.test.ts b/packages/lint/src/validate-readonly-action-writes.test.ts index 1910d87957..126eaf5a09 100644 --- a/packages/lint/src/validate-readonly-action-writes.test.ts +++ b/packages/lint/src/validate-readonly-action-writes.test.ts @@ -22,6 +22,12 @@ // when it does. `flags nothing on a static readonly field` below is that // measurement's pin: if the engine ever stops exempting system callers, this is // the test that should be revisited first. +// +// [#14147] The engine DID change on one axis and this table did not move: the +// static strip now runs on the CREATE path too, under the same `isSystem` gate. +// An action body is elevated on both verbs, so every row above is unchanged — +// what changed is that "INSERT is exempt" stopped being a true reason for +// anything, which the INSERT block at the bottom of this file now pins. import { describe, expect, it } from 'vitest'; import { HOOK_BODY_WRITE_PATTERNS } from './validate-hook-body-writes.js'; @@ -30,6 +36,7 @@ import { ACTION_API_UPDATE_READONLY_WHEN_FIELD, READONLY_ACTION_WRITE_PATTERN_IDS, READONLY_ACTION_WRITE_EXCLUSIONS, + READONLY_ACTION_INSERT_SILENCE, } from './validate-readonly-action-writes.js'; /** @@ -278,12 +285,41 @@ describe('validateReadonlyActionWrites - GREEN: ctx.input is the params bag', () }); }); -describe('validateReadonlyActionWrites - GREEN: INSERT is exempt from both strips', () => { - // Measured on the same harness: an elevated insert seeding a `readonly` AND a - // `readonlyWhen`-locked column keeps both values. A `readonlyWhen` predicate - // has no prior record to evaluate on a create, which is also why the flow - // sibling never reads a `create_record` node. - it('never flags insert()', () => { +describe('validateReadonlyActionWrites - the INSERT silence is a REASONED refusal, not a gap', () => { + // [#14147] This block used to be titled "INSERT is exempt from both strips" + // and rested on exactly that sentence. The maintainer ruling of 2026-09-03 + // (option C) SUPERSEDED it: `engine.insert` now runs the static-`readonly` + // strip for a non-system caller, `isSystem`-gated, exactly as `engine.update` + // does, and the metadata-protocol boundary copy the old reason cited is + // deleted. A green case whose justification has been overturned is + // indistinguishable from a scan gap, so the verdicts below are kept — they + // are still TRUE — and the REASON is pinned alongside them. + // + // Re-measured after the ruling, on the harness this file's header describes: + // an ELEVATED insert seeding a `readonly` AND a `readonlyWhen`-locked column + // still keeps both values, because `buildActionExecutionContext` is + // `{ ...ec, isSystem: true }`. That is a fact about elevation, not INSERT. + + it('pins WHY it refuses to report on insert/create — and that neither reason is the superseded one', () => { + expect(READONLY_ACTION_INSERT_SILENCE.methods).toEqual(['insert', 'create']); + expect(READONLY_ACTION_INSERT_SILENCE.reasons.map((r) => r.id)).toEqual([ + 'conditional-lock-has-no-prior-record', + 'action-body-is-system-elevated', + ]); + for (const { reason } of READONLY_ACTION_INSERT_SILENCE.reasons) { + expect(reason.length, 'a refusal with no stated reason is a gap').toBeGreaterThan(40); + // The refusal pin proper: the overturned sentence may never come back as + // the justification, in any of its spellings. + expect(reason).not.toMatch(/INSERT is exempt/i); + expect(reason).not.toMatch(/exempt from both strips/i); + } + // ...and one of them must still name the surviving engine fact, so a future + // author cannot read the silence as "the engine does not strip on create". + expect(READONLY_ACTION_INSERT_SILENCE.reasons.map((r) => r.reason).join(' ')) + .toContain('isSystem: true'); + }); + + it('never flags insert() — the conditional lock has no prior record on a create', () => { expect( validateReadonlyActionWrites( invoiceStack("await ctx.api.object('showcase_invoice').insert({ tax_rate: 8 });"), @@ -291,10 +327,10 @@ describe('validateReadonlyActionWrites - GREEN: INSERT is exempt from both strip ).toEqual([]); }); - it('never flags create()', () => { + it('never flags create() — same, and the static half is skipped by ELEVATION, not by INSERT', () => { expect( validateReadonlyActionWrites( - invoiceStack("await ctx.api.object('showcase_invoice').create({ tax_rate: 8 });"), + invoiceStack("await ctx.api.object('showcase_invoice').create({ invoice_number: 'INV-1' });"), ), ).toEqual([]); }); diff --git a/packages/lint/src/validate-readonly-action-writes.ts b/packages/lint/src/validate-readonly-action-writes.ts index 7543585433..57877afa64 100644 --- a/packages/lint/src/validate-readonly-action-writes.ts +++ b/packages/lint/src/validate-readonly-action-writes.ts @@ -33,9 +33,10 @@ // both production dispatch paths build it that way: REST `/actions` in // `domains/actions.ts` and MCP `run_action` in `action-execution.ts`. The // engine's static strip runs under `if (!opCtx.context?.isSystem)`, so it is -// SKIPPED for an action body. The conditional strip is not: it runs before that -// guard, over the caller-supplied keys, and `isSystem` is explicitly NOT an -// exemption there (#9107's LOCK 2, pinned in +// SKIPPED for an action body - on the create path as well, since the 2026-09-03 ruling put a +// static strip there under the SAME gate. The conditional strip is not: it runs +// before that guard, over the caller-supplied keys, and `isSystem` is explicitly +// NOT an exemption there (#9107's LOCK 2, pinned in // `engine-readonly-when-derived-writes.test.ts`). // // So a static-`readonly` finding on this surface would state something FALSE - @@ -54,12 +55,30 @@ // // --- SCOPE - deliberately narrow, so a finding is worth reporting ---------- // -// - Only `update` / `updateById`. INSERT is exempt from BOTH strips: a -// `readonlyWhen` predicate has no prior record to evaluate on a create, and -// the static one is skipped for the author-declared reason the flow sibling -// skips `create_record` (#3043/#3413). Measured on the same harness - an -// elevated `insert` seeding a `readonly` AND a `readonlyWhen`-locked column -// keeps both values. +// - Only `update` / `updateById`, and since the 2026-09-03 ruling for ONE reason rather than +// two. The premise this bullet used to carry - "INSERT is exempt from BOTH +// strips" - is SUPERSEDED: the maintainer ruling of 2026-09-03 (option C, +// overturning their own 2026-07-24 "INSERT (all callers) exempt" row) put +// the static-`readonly` strip inside `engine.insert`, `isSystem`-gated, +// exactly as on update, and DELETED the metadata-protocol boundary copy +// this bullet cited. So INSERT is no longer engine-exempt from anything. +// +// What actually keeps `insert` / `create` out of this rule's match set is +// the pair of facts in {@link READONLY_ACTION_INSERT_SILENCE}, and neither +// is the superseded row: the CONDITIONAL lock has no prior record to +// evaluate on a create (`stripReadonlyWhenFields` is update-path-only, +// engine.ts: "INSERT stays exempt"), and the STATIC one is skipped on THIS +// surface for the same reason it is skipped on `update` here - an action +// body runs `isSystem`-elevated. Re-measured on the harness above after the +// ruling landed: an elevated `insert` seeding a `readonly` AND a +// `readonlyWhen`-locked column still keeps both values. +// +// ⚠️ That is a fact about ELEVATION, not about INSERT, and it does not +// travel: a NON-elevated create - a flow `create_record` without +// `runAs:'system'`, a hook body's `ctx.api.insert` - now IS stripped. The +// flow sibling's own `create_record` gap rests on the superseded premise +// and is reported rather than widened here (a new error-severity finding +// class is not this card's to introduce). // // - Only the `api-crud-literal` shape. `ctx.record` is not a write surface at // all here, and that is the whole false-positive class this rule had to @@ -149,11 +168,44 @@ export const READONLY_ACTION_WRITE_EXCLUSIONS: readonly BodyWritePatternExclusio const APPLICABLE_PATTERN_IDS: ReadonlySet = new Set(READONLY_ACTION_WRITE_PATTERN_IDS); /** - * `ctx.api` write methods whose payload is subject to the conditional strip. + * The REFUSAL this rule owes a reason for, declared as data (the 2026-09-03 ruling). * - * `insert` / `create` are absent BY DECISION, not by omission: INSERT is exempt - * from both strips, which is the same reason the flow sibling never looks at a - * `create_record` node. + * `insert` / `create` are absent from {@link STRIP_SUBJECT_METHODS}, and until + * the 2026-09-03 ruling the reason was a one-liner - "INSERT is exempt from both + * strips" - that has since been SUPERSEDED on its static half. A silence whose + * stated reason has been overturned is indistinguishable from a scan gap, so the + * surviving reasons are written down here and pinned, the same way + * {@link READONLY_ACTION_WRITE_EXCLUSIONS} pins the shapes this rule leaves + * alone. ⛔ Neither reason may be restated as "INSERT is exempt": that sentence + * is false about the engine as of the 2026-09-03 ruling. + */ +export const READONLY_ACTION_INSERT_SILENCE = { + methods: ['insert', 'create'] as readonly string[], + reasons: [ + { + id: 'conditional-lock-has-no-prior-record', + reason: + 'the CONDITIONAL strip this rule reports (`stripReadonlyWhenFields`) is update-path-only - a ' + + '`readonlyWhen` predicate is evaluated against the record being written over, which a create ' + + 'does not have. `engine.ts` states it at the bulk strip: "INSERT stays exempt". Unchanged by ' + + 'the 2026-09-03 ruling, which moved the STATIC strip only', + }, + { + id: 'action-body-is-system-elevated', + reason: + 'the STATIC strip now runs inside `engine.insert` for a non-system caller (the 2026-09-03 ruling), but an ' + + "action body's `ctx.api` is `ql.createContext(buildActionExecutionContext(ec))` and that is " + + '`{ ...ec, isSystem: true }` - so it is skipped on THIS surface for exactly the reason it is ' + + 'skipped on `update` here, and a finding would state something false about a write that lands. ' + + '⚠️ A fact about ELEVATION, not about INSERT: a non-elevated create IS stripped', + }, + ] as readonly { id: string; reason: string }[], +} as const; + +/** + * `ctx.api` write methods whose payload is subject to the conditional strip. + * `insert` / `create` are absent for the reasons {@link + * READONLY_ACTION_INSERT_SILENCE} records - never for the superseded one. */ const STRIP_SUBJECT_METHODS: ReadonlySet = new Set(['update', 'updateById']); diff --git a/packages/lint/src/validate-readonly-flow-writes.test.ts b/packages/lint/src/validate-readonly-flow-writes.test.ts index aec906ead7..09ce288e72 100644 --- a/packages/lint/src/validate-readonly-flow-writes.test.ts +++ b/packages/lint/src/validate-readonly-flow-writes.test.ts @@ -308,8 +308,10 @@ describe('validateReadonlyFlowWrites', () => { expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount'); }); - // create_record stays exempt on BOTH branches under elevation: a - // `readonlyWhen` predicate has no prior record to evaluate on an insert. + // create_record under elevation is clean on BOTH branches, for two different + // reasons: the static strip is skipped by `runAs:'system'` (elevation, not + // INSERT — see the block below), and a `readonlyWhen` predicate has no prior + // record to evaluate on an insert. it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => { const flow = { name: 'seed_opp_system', @@ -324,8 +326,18 @@ describe('validateReadonlyFlowWrites', () => { expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]); }); - // ── clean: create_record is engine-exempt from the readonly strip ───── - it('does NOT flag create_record writing a readonly field', () => { + // ── clean TODAY: create_record is a SCAN GAP, not an exemption (#15394) ── + // [#14147] This case used to be justified by "create_record is engine-exempt + // from the readonly strip". The maintainer ruling of 2026-09-03 (option C) + // SUPERSEDED that row: `engine.insert` runs the static-`readonly` strip for a + // non-system caller, and a `create_record` node without `runAs:'system'` is + // exactly that caller — the write below is a silent no-op at run time + // (measured end to end in `create-record-readonly-drop.test.ts`, + // service-automation). The verdict is kept because it is still TRUE of what + // this rule scans (`update_record` only); the reason is that the scan gap is + // filed as #15394, not that the engine exempts anything. When that lands, + // this case flips to RED with severity `error`. + it('does NOT flag create_record writing a readonly field — the #15394 scan gap, not an exemption', () => { const flow = { name: 'seed_opp', type: 'record_change', diff --git a/packages/lint/src/validate-readonly-flow-writes.ts b/packages/lint/src/validate-readonly-flow-writes.ts index daace3043d..8423167923 100644 --- a/packages/lint/src/validate-readonly-flow-writes.ts +++ b/packages/lint/src/validate-readonly-flow-writes.ts @@ -11,11 +11,19 @@ // // Scope — deliberately narrow to keep it false-positive-free: // -// • Only `update_record`. INSERT is engine-exempt from the readonly strip (a -// `create_record` may legitimately seed readonly columns; the ingress strip -// added in #3043 lives in metadata-protocol, which the flow engine bypasses -// by calling the data engine directly), so a create writing a readonly -// field is NOT a no-op and is never flagged. +// • Only `update_record`, and ⚠️ this bullet's REASON is spent. It used to +// be that INSERT was engine-exempt from the author-declared static-`readonly` +// strip (#3043/#3413: "a `create_record` may legitimately seed readonly +// columns", with an ingress copy in metadata-protocol that the flow engine +// bypassed by calling the data engine directly). The maintainer ruling of +// 2026-09-03 (option C, #14147) SUPERSEDED that row: `engine.insert` runs +// the static strip for a non-system caller, the ingress copy is deleted, +// and a `create_record` node without `runAs:'system'` is exactly such a +// caller. So a create writing a readonly field IS a silent no-op now, and +// this rule does not yet report it — a scan gap, not a decision, recorded +// here and filed rather than widened inside #14147's PR (a new +// error-severity finding class is its own change). The hook sibling's +// `insert`/`create` gap rests on the same superseded premise. // // • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level // skip. An elevated run bypasses the static `readonly` strip, so a system diff --git a/packages/lint/src/validate-readonly-hook-writes.test.ts b/packages/lint/src/validate-readonly-hook-writes.test.ts index b654c1b6b1..41ce72bc13 100644 --- a/packages/lint/src/validate-readonly-hook-writes.test.ts +++ b/packages/lint/src/validate-readonly-hook-writes.test.ts @@ -212,11 +212,22 @@ describe('validateReadonlyHookWrites - GREEN: the elevated channel', () => { }); }); -describe('validateReadonlyHookWrites - GREEN: INSERT is engine-exempt', () => { - // A create may legitimately seed read-only columns: the engine's static - // readonly strip is deliberately absent from the insert path (#3043/#3413), - // which is the same reason the flow sibling never reads a create_record node. - it('never flags insert()', () => { +describe('validateReadonlyHookWrites - GREEN today: insert()/create() are a SCAN GAP, not an exemption', () => { + // [#14147] This block used to be titled "INSERT is engine-exempt" and rested + // on exactly that sentence ("a create may legitimately seed read-only + // columns", #3043/#3413). The maintainer ruling of 2026-09-03 (option C) + // SUPERSEDED it: `engine.insert` now runs the static-`readonly` strip for a + // non-system caller, `isSystem`-gated, exactly as `engine.update` does — and + // a hook body's `ctx.api` under a non-system trigger IS such a caller. A green + // case whose justification has been overturned is indistinguishable from a + // scan gap, so the verdicts below are kept — they are still TRUE of what this + // rule scans (`update` / `updateById` only) — and the reason is restated: + // - the CONDITIONAL lock has no prior record to evaluate on a create, so + // `hook-api-update-readonly-when-field` is right to stay silent; + // - the STATIC half IS a silent no-op now and is NOT reported — a scan gap, + // filed as #15394. When that lands, `insert()` of a static-`readonly` + // column flips to RED here, and this block's title goes with it. + it('never flags insert() — the conditional lock has no prior record on a create; the static half is the #15394 gap', () => { expect( validateReadonlyHookWrites( crmStack("await ctx.api.object('crm_account').insert({ last_activity_date: now });"), @@ -224,7 +235,7 @@ describe('validateReadonlyHookWrites - GREEN: INSERT is engine-exempt', () => { ).toEqual([]); }); - it('never flags create()', () => { + it('never flags create() — same two facts, same gap', () => { expect( validateReadonlyHookWrites( crmStack("await ctx.api.object('crm_account').create({ last_activity_date: now });"), diff --git a/packages/lint/src/validate-readonly-hook-writes.ts b/packages/lint/src/validate-readonly-hook-writes.ts index 33c871e708..5dc2572252 100644 --- a/packages/lint/src/validate-readonly-hook-writes.ts +++ b/packages/lint/src/validate-readonly-hook-writes.ts @@ -39,11 +39,17 @@ // // --- SCOPE - deliberately narrow, so a finding is worth gating on ---------- // -// - Only `update` / `updateById`. INSERT is engine-exempt from the -// author-declared static-`readonly` strip (`stripReadonlyForInsert`'s note -// in rule-validator.ts, #3043/#3413: "a create may legitimately seed -// read-only columns"), so `insert`/`create` are not no-ops and are never -// flagged. Exactly the reason the flow sibling skips `create_record`. +// - Only `update` / `updateById`, and ⚠️ this bullet's REASON is spent. It +// used to be that INSERT was engine-exempt from the author-declared +// static-`readonly` strip (#3043/#3413: "a create may legitimately seed +// read-only columns"). The maintainer ruling of 2026-09-03 (option C, +// #14147) SUPERSEDED that row: `engine.insert` runs the static strip for a +// non-system caller, and a hook body's `ctx.api` under a non-system trigger +// is exactly that. So a hook `insert` of a static-`readonly` column IS a +// silent no-op now, and this rule does not yet report it — a scan gap, not +// a decision, recorded here and filed rather than widened inside #14147's +// PR (a new error-severity finding class is its own change). The flow +// sibling's `create_record` gap rests on the same superseded premise. // // - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context // with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain @@ -198,12 +204,18 @@ export const READONLY_HOOK_WRITE_EXCLUSIONS: readonly BodyWritePatternExclusion[ const APPLICABLE_PATTERN_IDS: ReadonlySet = new Set(READONLY_HOOK_WRITE_PATTERN_IDS); /** - * `ctx.api` write methods whose payload is subject to the update-path strip. + * `ctx.api` write methods whose payload this rule judges against the strip. * - * `insert` / `create` are absent BY DECISION, not by omission: the engine - * exempts INSERT from the author-declared static-`readonly` strip so a create - * may legitimately seed read-only columns (#3043/#3413), which is the same - * reason the flow sibling never looks at a `create_record` node. + * `insert` / `create` are absent as a SCAN GAP, not by decision. This docblock + * used to say the opposite — that the engine exempts INSERT from the + * author-declared static-`readonly` strip so a create may legitimately seed + * read-only columns (#3043/#3413). The maintainer ruling of 2026-09-03 + * (option C, #14147) superseded that row — the header above carries the full + * reading: `engine.insert` now runs the same strip under the same `isSystem` + * gate, so a non-system hook `insert` of a static-`readonly` column is a + * silent no-op this rule does not yet report. Widening the set is filed as its + * own change rather than ridden in here, and the flow sibling's `create_record` + * gap is the same finding one surface over. */ const STRIP_SUBJECT_METHODS: ReadonlySet = new Set(['update', 'updateById']); diff --git a/packages/mcp/src/stdio-data-bridge.ts b/packages/mcp/src/stdio-data-bridge.ts index 7d625c34bd..e222052e8b 100644 --- a/packages/mcp/src/stdio-data-bridge.ts +++ b/packages/mcp/src/stdio-data-bridge.ts @@ -47,8 +47,12 @@ * * `callData` prefers the `protocol` service (metadata-protocol) and falls back * to the engine; this bridge is engine-only. So the HTTP tools additionally get - * that layer's ingress `readonly` strip, its existence probes, its spec-shaped - * receipts and `expand`/`select`. None of those is the authorization boundary + * that layer's existence probes, its spec-shaped receipts and `expand`/`select`. + * (That layer's create-side `readonly` strip used to head this list; since the + * maintainer ruling of 2026-09-03 — option C, #14147 — the static `readonly` + * strip runs inside `engine.insert` for every non-system caller and the + * ingress copy is deleted, so on that point the two transports no longer + * differ.) None of those is the authorization boundary * — every call here still passes the engine's CRUD/FLS/RLS — but the two * transports should not differ at all, and unifying them behind one * transport-neutral data seam is filed as follow-up work rather than forked diff --git a/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts b/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts index 38cab925c8..6e736116d7 100644 --- a/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts +++ b/packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts @@ -2,7 +2,8 @@ // // [#3455] Extends the single-write drop-observability of #3431 to the BULK // write paths. Each bulk method must (a) surface the same LEGAL strips -// (static `readonly` #2948 / `readonlyWhen` #3042 / #3043 create ingress) that +// (static `readonly` #2948 / `readonlyWhen` #3042 / the create-side static +// strip, in the engine since #14147) that // single-write now reports, and (b) thread the caller's execution `context` to // the engine so RLS/FLS/`readonlyWhen` run under the caller — a gap the // pre-#3455 `updateManyData`/`batchData` loops had. Channels: @@ -70,12 +71,23 @@ describe('createManyData — aggregated top-level droppedFields (#3455)', () => function makeProtocol() { const engine = { registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) }, - insert: vi.fn(async (_object: string, rows: any[]) => rows.map((r, i) => ({ id: `rec-${i + 1}`, ...r }))), + // [#14147] The strip is the ENGINE's, and its event is the batch UNION — + // one call, one listener invocation, however many rows forged. + insert: vi.fn(async (object: string, rows: any[], options?: any) => { + const system = options?.context?.isSystem === true; + const dropped = !system && rows.some((r) => r && 'approval_status' in r); + if (dropped) options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' }); + return rows.map((r, i) => { + if (system || !(r && 'approval_status' in r)) return { id: `rec-${i + 1}`, ...r }; + const { approval_status: _forged, ...kept } = r; + return { id: `rec-${i + 1}`, ...kept }; + }); + }), }; return { p: new ObjectStackProtocolImplementation(engine as any), engine }; } - it('aggregates the schema-uniform ingress strip across rows into one event', async () => { + it('aggregates the schema-uniform create strip across rows into one event', async () => { const { p } = makeProtocol(); const res: any = await p.createManyData({ object: 'approval_case', @@ -116,10 +128,19 @@ describe('createManyData — aggregated top-level droppedFields (#3455)', () => }); describe('insertManyData — per-row droppedFields on outcomes (#3455)', () => { - it('attaches the ingress strip to the matching outcome row only', async () => { - const insertMany = vi.fn(async (_object: string, rows: any[]) => - rows.map((r, i) => ({ ok: true, record: { id: `rec-${i + 1}`, ...r } })), - ); + it('attaches the create strip to the matching outcome row only', async () => { + // [#14147] The engine's listener carries no row index — it reports the + // batch UNION — so row precision here is recovered by asking which row + // SUPPLIED each dropped name. That recovery is what this case pins. + const insertMany = vi.fn(async (object: string, rows: any[], options?: any) => { + if (rows.some((r) => r && 'approval_status' in r)) { + options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' }); + } + return rows.map((r, i) => { + const { approval_status: _forged, ...kept } = r ?? {}; + return { ok: true, record: { id: `rec-${i + 1}`, ...kept } }; + }); + }); const engine = { registry: { getObject: () => SCHEMA }, insertMany }; const p = new ObjectStackProtocolImplementation(engine as any); @@ -136,14 +157,21 @@ describe('insertManyData — per-row droppedFields on outcomes (#3455)', () => { expect(res.outcomes[1].droppedFields).toEqual([ { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, ]); - // The strip really removed the field from what the engine inserted. - expect(insertMany.mock.calls[0][1][1]).not.toHaveProperty('approval_status'); + // [#14147] The ingress hands the caller's row over WHOLE — judging it is + // the engine's job now, and this assertion is what would catch a + // reintroduced second strip at this seam. + expect(insertMany.mock.calls[0][1][1]).toHaveProperty('approval_status'); }); }); describe('batchData — per-row droppedFields + context threading (#3455)', () => { - it('create rows surface the ingress strip and honour a system context', async () => { - const insert = vi.fn(async (_object: string, data: any, _options?: any) => ({ id: 'rec-1', ...data })); + it('create rows surface the engine strip and honour a system context', async () => { + const insert = vi.fn(async (object: string, data: any, options?: any) => { + if (options?.context?.isSystem || !(data && 'approval_status' in data)) return { id: 'rec-1', ...data }; + const { approval_status: _forged, ...kept } = data; + options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' }); + return { id: 'rec-1', ...kept }; + }); const engine = { registry: { getObject: () => SCHEMA }, insert, update: vi.fn(), findOne: vi.fn() }; const p = new ObjectStackProtocolImplementation(engine as any); @@ -165,7 +193,11 @@ describe('batchData — per-row droppedFields + context threading (#3455)', () = }); it('a system-context batch create is exempt from the strip (context now threaded to it)', async () => { - const insert = vi.fn(async (_object: string, data: any) => ({ id: 'rec-1', ...data })); + const insert = vi.fn(async (_object: string, data: any, options?: any) => { + if (options?.context?.isSystem) return { id: 'rec-1', ...data }; + const { approval_status: _forged, ...kept } = data ?? {}; + return { id: 'rec-1', ...kept }; + }); const engine = { registry: { getObject: () => SCHEMA }, insert, update: vi.fn(), findOne: vi.fn() }; const p = new ObjectStackProtocolImplementation(engine as any); diff --git a/packages/metadata-protocol/src/protocol.dropped-fields.test.ts b/packages/metadata-protocol/src/protocol.dropped-fields.test.ts index 0e451917c8..8a7bbec5d7 100644 --- a/packages/metadata-protocol/src/protocol.dropped-fields.test.ts +++ b/packages/metadata-protocol/src/protocol.dropped-fields.test.ts @@ -6,9 +6,10 @@ // the strip back to the REST layer: // - updateData forwards the engine's onFieldsDropped events (readonly / // readonly_when) onto the response as `droppedFields`; -// - createData surfaces the #3043 static-`readonly` INGRESS strip, which runs -// BEFORE the engine (so it is recovered by diffing the payload, not via the -// engine listener) — symmetric with update; +// - createData surfaces the static-`readonly` create strip — which since +// #14147 runs INSIDE `engine.insert`, so it arrives on the SAME +// `onFieldsDropped` listener the update side uses (it used to run at this +// ingress and be recovered by diffing the payload) — symmetric with update; // - no strip → NO `droppedFields` key, so the response shape stays // backward-compatible for clients that only read `record`. @@ -88,11 +89,21 @@ describe('updateData — forwards engine write strips as droppedFields (#3431)', }); }); -describe('createData — surfaces the #3043 ingress readonly strip as droppedFields (#3431)', () => { +describe('createData — surfaces the engine readonly create strip as droppedFields (#3431/#14147)', () => { function makeProtocol() { const engine = { registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) }, - insert: vi.fn(async (_object: string, data: any) => ({ id: 'rec-1', ...data })), + // Stands in for `engine.insert` AFTER #14147: it strips a non-system + // caller's readonly key and reports it through the listener, which is the + // same shape the update stand-ins above have always had. + insert: vi.fn(async (object: string, data: any, options?: any) => { + if (options?.context?.isSystem || !(data && 'approval_status' in data)) { + return { id: 'rec-1', ...data }; + } + const { approval_status: _forged, ...kept } = data; + options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' }); + return { id: 'rec-1', ...kept }; + }), }; return { p: new ObjectStackProtocolImplementation(engine as any), engine }; } @@ -107,7 +118,8 @@ describe('createData — surfaces the #3043 ingress readonly strip as droppedFie expect(res.droppedFields).toEqual([ { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, ]); - // Stripped field is absent from the persisted payload (existing #3043 behaviour). + // Stripped field is absent from the record the engine returns (#3043's + // behaviour, enforced one layer down since #14147). expect(res.record).not.toHaveProperty('approval_status'); }); diff --git a/packages/metadata-protocol/src/protocol.readonly-insert.test.ts b/packages/metadata-protocol/src/protocol.readonly-insert.test.ts index efdbf2af64..ef08bee07b 100644 --- a/packages/metadata-protocol/src/protocol.readonly-insert.test.ts +++ b/packages/metadata-protocol/src/protocol.readonly-insert.test.ts @@ -1,279 +1,274 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// #3043 — static `readonly: true` fields must not be SEEDABLE via a non-system -// create through the external data API. The strip lives at the DataProtocol -// ingress (createData / createManyData / batchData / cloneData) — the seam every -// external REST/GraphQL/MCP create funnels through — while trusted internal -// writers call engine.insert directly and are unaffected. It runs BEFORE -// engine.insert, so a stripped field falls back to its defaultValue (re-derived -// by the engine, which the mock stands in for). System context is exempt. +// #3043 → #14147 — where the create-side static-`readonly` strip LIVES, pinned +// from the ingress side. +// +// #3043 put the strip at this DataProtocol ingress because the engine was +// INSERT-readonly-exempt (#3413), and the ingress is the seam every external +// REST/GraphQL/MCP create funnels through. That left the exemption's other half +// standing: a non-system caller reaching `engine.insert` DIRECTLY wrote the +// read-only column with no refusal, no WARN and no `onFieldsDropped` event. +// +// Maintainer ruling, 2026-09-03 (option C), superseding their own 2026-07-24 +// "INSERT (all callers) exempt" row: the strip is `stripReadonlyFields` inside +// `engine.insert` — the same function, the same `isSystem` gate and the same +// reporting the UPDATE path has had since #2948 — and the ingress copy is +// DELETED rather than kept as a second implementation. +// +// ⇒ What this file may still pin is DELEGATION, and only that: +// - every create face hands the caller's payload to `engine.insert` WHOLE — +// `createData`, `cloneData`, `createManyData`, `insertManyData`, and +// `batchData`'s `create` rows AND both arms of `upsert` that create; +// - every create face whose RESPONSE carries `droppedFields` surfaces the +// ENGINE's `onFieldsDropped` there, which is the channel the ingress used +// to FAKE with a before/after payload diff. `cloneData` is the one face +// that does not: `CloneDataResponseSchema` declares no such member (pinned +// in the firing-control block at the bottom). +// The enforcement itself is pinned where it now runs, against a real engine: +// `packages/objectql/src/engine-insert-static-readonly-strip.test.ts`. This +// package does not depend on `@objectstack/objectql`, so a strip assertion here +// could only ever re-测 a mock — which is exactly how the direct-`engine.insert` +// hole survived four rulings. -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; import { ObjectStackProtocolImplementation } from './protocol.js'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; const SCHEMA = { name: 'approval_case', fields: { title: { name: 'title', type: 'text' }, - // readonly approval column — the #3003 attack target + // readonly approval column — the #3003/#3043 attack target approval_status: { name: 'approval_status', type: 'text', readonly: true, defaultValue: 'draft' }, // readonly provenance stamp with no default source: { name: 'source', type: 'text', readonly: true }, }, }; -function makeProtocol() { +/** + * A stand-in engine that behaves like the real one AFTER #14147: it strips the + * declared `readonly` keys the caller supplied and reports them through + * `options.onFieldsDropped`, exactly once per call. + * + * The strip here is a STUB of the engine's verdict, never a second copy of its + * rules — the assertions below are about what the ingress FORWARDS and what it + * SURFACES, and a stub that never dropped anything would make every + * `droppedFields` assertion vacuously green (the firing control is + * `engine listener wiring` at the bottom of this file). + */ +function makeProtocol(schema: any = SCHEMA) { const inserts: Array<{ object: string; data: any; options: any }> = []; + const strip = (row: any) => { + if (!row || typeof row !== 'object') return { row, dropped: [] as string[] }; + const dropped: string[] = []; + const out: any = { ...row }; + for (const [name, def] of Object.entries(schema.fields)) { + if (!def?.readonly || !(name in out)) continue; + delete out[name]; + dropped.push(name); + } + return { row: out, dropped }; + }; + const run = (object: string, data: any, options?: any) => { + inserts.push({ object, data, options }); + const rows = Array.isArray(data) ? data : [data]; + const stripped = rows.map(strip); + const union = [...new Set(stripped.flatMap((s) => s.dropped))]; + if (union.length > 0 && typeof options?.onFieldsDropped === 'function') { + options.onFieldsDropped({ object, fields: union, reason: 'readonly' } as DroppedFieldsEvent); + } + return stripped.map((s, i) => ({ id: `rec-${i + 1}`, ...s.row })); + }; const engine = { - registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) }, + registry: { getObject: (n: string) => (n === schema.name ? schema : undefined) }, insert: vi.fn(async (object: string, data: any, options?: any) => { - inserts.push({ object, data, options }); - const rows = Array.isArray(data) ? data : [data]; - const out = rows.map((r, i) => ({ id: `rec-${i + 1}`, ...r })); + const out = run(object, data, options); return Array.isArray(data) ? out : out[0]; }), + insertMany: vi.fn(async (object: string, rows: any[], options?: any) => + run(object, rows, options).map((record) => ({ ok: true, record }))), + // `cloneData` reads the source through the engine's find path — opened with + // the shared predicate so this double cannot be looser than `ObjectQL.findOne` + // (`check:engine-double-contract`). + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { + assertEngineFindOnePredicate(object, query); + // The clone source exists; any other id names no row, which is what sends + // `batchData`'s upsert fork (`probeRecord`, #5099) down its CREATE arm. + const id = (query as any)?.where?.id; + return id === 'src-1' ? { id: 'src-1', title: 'Source', approval_status: 'approved' } : null; + }), }; const p = new ObjectStackProtocolImplementation(engine as any); return { p, engine, inserts }; } -describe('createData — static readonly INSERT strip (#3043)', () => { - it('drops a non-system caller forging a readonly field; editable sibling lands', async () => { +describe('#14147 — the create ingress DELEGATES the readonly strip to engine.insert', () => { + it('createData forwards the caller payload WHOLE — the forged key is the engine’s to judge', async () => { const { p, inserts } = makeProtocol(); - await p.createData({ + const res: any = await p.createData({ object: 'approval_case', data: { title: 'Case A', approval_status: 'approved' }, - context: { userId: 'u1' }, }); expect(inserts).toHaveLength(1); - expect(inserts[0].data).toEqual({ title: 'Case A' }); // approval_status stripped - expect(inserts[0].data).not.toHaveProperty('approval_status'); + expect(inserts[0].data, 'nothing is removed before the engine sees it') + .toEqual({ title: 'Case A', approval_status: 'approved' }); + // ...and the engine's verdict is what the 201 body reports. + expect(res.droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, + ]); + expect(res.record).not.toHaveProperty('approval_status'); }); - it('ALLOWS a system-context caller to seed the readonly field', async () => { + it('createData passes a SYSTEM context through untouched — the exemption is the engine’s too', async () => { const { p, inserts } = makeProtocol(); await p.createData({ object: 'approval_case', - data: { title: 'Seed', approval_status: 'approved' }, + data: { title: 'Case B', approval_status: 'approved' }, context: { isSystem: true }, }); - expect(inserts[0].data.approval_status).toBe('approved'); + expect(inserts[0].data.approval_status, 'the ingress never pre-empts the isSystem gate').toBe('approved'); + expect(inserts[0].options.context).toEqual({ isSystem: true }); }); - it('strips a forged readonly field even when no context is supplied (non-system default)', async () => { + it('cloneData forwards the copied row AND the caller overrides whole', async () => { const { p, inserts } = makeProtocol(); - await p.createData({ object: 'approval_case', data: { title: 'X', source: 'attacker' } }); - expect(inserts[0].data).not.toHaveProperty('source'); + await p.cloneData({ + object: 'approval_case', + id: 'src-1', + overrides: { source: 'forged' }, + } as any); + expect(inserts).toHaveLength(1); + // `overrides` are applied BEFORE the insert, so a readonly key smuggled + // through them is still the engine's to strip (#3043's carried-over case). + expect(inserts[0].data.source).toBe('forged'); + expect(inserts[0].data.approval_status, 'the copied readonly column travels too').toBe('approved'); }); - it('does NOT strip a PLATFORM object — defers to its own field guards (ADR-0086 / #3004)', async () => { - // A `sys_`/managedBy object carries dedicated write governance (e.g. the - // ADR-0086 provenance guard REJECTS a forged managed_by/package_id with 403); - // the generic silent strip must not pre-empt that. Proven with both markers. - const platformSchema = { - name: 'sys_permission_set', - fields: { managed_by: { name: 'managed_by', type: 'select', readonly: true } }, - }; - const managedSchema = { - name: 'crm_thing', managedBy: 'package', - fields: { locked: { name: 'locked', type: 'text', readonly: true } }, - }; - const inserts: any[] = []; - const engine = { - registry: { getObject: (n: string) => (n === 'sys_permission_set' ? platformSchema : managedSchema) }, - insert: vi.fn(async (object: string, data: any) => { inserts.push({ object, data }); return { id: 'x', ...data }; }), - }; - const p = new ObjectStackProtocolImplementation(engine as any); - await p.createData({ object: 'sys_permission_set', data: { managed_by: 'package' }, context: { userId: 'u1' } }); - await p.createData({ object: 'crm_thing', data: { locked: 'forged' }, context: { userId: 'u1' } }); - expect(inserts[0].data.managed_by, 'sys_ object: readonly field passed through to its guard').toBe('package'); - expect(inserts[1].data.locked, 'managedBy object: readonly field passed through to its guard').toBe('forged'); - }); -}); - -describe('createManyData / batchData — per-row readonly INSERT strip (#3043)', () => { - it('createManyData strips the forged readonly column on every row', async () => { + it('createManyData forwards every row whole and AGGREGATES the engine’s event', async () => { const { p, inserts } = makeProtocol(); - await p.createManyData({ + const res: any = await p.createManyData({ object: 'approval_case', - records: [ - { title: 'A', approval_status: 'approved' }, - { title: 'B', approval_status: 'approved' }, - ], - context: { userId: 'u1' }, + records: [{ title: 'A', approval_status: 'approved' }, { title: 'B', source: 'x' }], }); - expect(inserts[0].data).toEqual([{ title: 'A' }, { title: 'B' }]); + expect(inserts[0].data).toEqual([ + { title: 'A', approval_status: 'approved' }, + { title: 'B', source: 'x' }, + ]); + expect(res.droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status', 'source'], reason: 'readonly' }, + ]); }); - it('batchData create strips the forged readonly column', async () => { + it('batchData create forwards the row whole and hangs the engine’s event on that row', async () => { const { p, inserts } = makeProtocol(); - await p.batchData({ + const res: any = await p.batchData({ object: 'approval_case', - request: { operation: 'create', records: [{ data: { title: 'A', approval_status: 'approved' } }] } as any, - }); - expect(inserts).toHaveLength(1); - expect(inserts[0].data).toEqual({ title: 'A' }); + request: { operation: 'create', records: [{ data: { title: 'A', approval_status: 'approved' } }] }, + } as any); + expect(inserts[0].data).toEqual({ title: 'A', approval_status: 'approved' }); + expect(res.results[0].droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, + ]); }); -}); - -// --------------------------------------------------------------------------- -// #6640 — `preserveAudit` is an UPDATE-path exemption, and a non-system INSERT -// that asks for it is TOLD so. -// -// The contract used to promise the historical-import exemption (#3493) on both -// write paths (FieldSchema.readonly's `.describe()`, security.mdx), but only -// UPDATE ever read it: this ingress knows `isSystem` and nothing else. REST -// import's `treatAsHistorical` puts `preserveAudit: true` on the write context -// and creates through `createData` — so one import preserved an author-declared -// readonly column on the rows it updated and silently dropped it on the rows it -// created. Maintainer ruling 2026-08-08 (option 2 + loudness rider): the -// contract narrows to the enforcement, and the ignored request stops being -// silent. -// -// Every pin below goes through the REAL entry (`createData` / `createManyData` / -// `batchData` → `stripReadonlyForInsert`). That is the binding half of the test -// note: the pre-existing `preserveAudit` pins all call `engine.insert` directly, -// bypassing this ingress, which is how the gap survived. -// --------------------------------------------------------------------------- - -/** - * A historical-import target: two author-declared business `readonly` columns - * (the `closed_at` / `resolved_by` family the issue names) plus the injected - * audit column, which the registry's `AUDIT_FIELD_DEFS` also marks - * `readonly: true` — so an ordinary export→import round-trip trips this on - * every row, not only a hand-built payload. - */ -const TICKET = { - name: 'ticket', - fields: { - subject: { name: 'subject', type: 'text' }, - closed_at: { name: 'closed_at', type: 'datetime', readonly: true }, - resolved_by: { name: 'resolved_by', type: 'lookup', reference: 'sys_user', readonly: true }, - created_at: { name: 'created_at', type: 'datetime', readonly: true, system: true }, - }, -}; - -function makeTicketProtocol() { - const inserts: Array<{ object: string; data: any }> = []; - const engine = { - registry: { getObject: (n: string) => (n === 'ticket' ? TICKET : undefined) }, - insert: vi.fn(async (object: string, data: any) => { - inserts.push({ object, data }); - const rows = Array.isArray(data) ? data : [data]; - const out = rows.map((r, i) => ({ id: `t-${i + 1}`, ...r })); - return Array.isArray(data) ? out : out[0]; - }), - }; - return { p: new ObjectStackProtocolImplementation(engine as any), inserts }; -} - -/** Capture `console.warn` for one test without letting it reach the reporter. */ -function captureWarn() { - return vi.spyOn(console, 'warn').mockImplementation(() => {}); -} - -const HISTORICAL_CTX = { userId: 'importer', skipStateMachine: true, preserveAudit: true }; - -describe('#6640 — a non-system INSERT requesting preserveAudit is stripped AND warned', () => { - afterEach(() => { vi.restoreAllMocks(); }); - it('strips the readonly fields (enforcement unchanged) and names them in one loud warning', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - - await p.createData({ - object: 'ticket', - data: { - subject: 'legacy ticket', - closed_at: '2019-04-01T00:00:00Z', - resolved_by: 'usr_alice', - created_at: '2019-03-28T00:00:00Z', - }, - context: HISTORICAL_CTX, - }); - - // 1. The strip still applies — the ruling narrowed the CONTRACT, not the guard. - expect(inserts[0].data, 'readonly fields still stripped on a non-system create') - .toEqual({ subject: 'legacy ticket' }); - - // 2. …and it is no longer silent about the exemption it refused to honour. - expect(warn, 'exactly one signal per ingress call, not one per field').toHaveBeenCalledTimes(1); - const msg = String(warn.mock.calls[0]?.[0]); - // The rule, stated: the reader must learn WHY, not just THAT. - expect(msg).toContain('preserveAudit is UPDATE-only and was IGNORED on this INSERT'); - expect(msg).toContain("(object 'ticket')"); - // Every field it cost them, by name. - expect(msg).toContain('closed_at'); - expect(msg).toContain('resolved_by'); - expect(msg).toContain('created_at'); - // The remedy, so the warning is actionable rather than merely loud. - expect(msg).toContain('context.isSystem'); - expect(msg).toContain('#6640'); + it('batchData upsert-CREATE (row with no id) forwards the row whole and hangs the engine’s event on that row', async () => { + const { p, inserts } = makeProtocol(); + const res: any = await p.batchData({ + object: 'approval_case', + request: { operation: 'upsert', records: [{ data: { title: 'A', approval_status: 'approved' } }] }, + } as any); + expect(inserts).toHaveLength(1); + expect(inserts[0].data).toEqual({ title: 'A', approval_status: 'approved' }); + expect(res.results[0].success).toBe(true); + expect(res.results[0].droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, + ]); }); - it('leaves a SYSTEM insert untouched — preserveAudit or not, isSystem is still the exemption', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - await p.createData({ - object: 'ticket', - data: { subject: 'seed', closed_at: '2019-04-01T00:00:00Z' }, - context: { ...HISTORICAL_CTX, isSystem: true }, - }); - expect(inserts[0].data.closed_at, 'system context replays archival readonly facts').toBe('2019-04-01T00:00:00Z'); - expect(warn, 'nothing was ignored, so nothing is reported').not.toHaveBeenCalled(); + it('batchData upsert-CREATE (id names no row) forwards `{ id, ...data }` whole and hangs the engine’s event on that row', async () => { + const { p, engine, inserts } = makeProtocol(); + const res: any = await p.batchData({ + object: 'approval_case', + request: { operation: 'upsert', records: [{ id: 'new-1', data: { title: 'A', approval_status: 'approved' } }] }, + } as any); + // The fork asked existence first (#5099) and was answered null, so this is + // the CREATE arm — pinned, because the update arm five lines above it in + // `runBatchDataLoop` already reported drops and would make this green for + // the wrong reason. + expect(engine.findOne).toHaveBeenCalledTimes(1); + expect(inserts).toHaveLength(1); + expect(inserts[0].data).toEqual({ id: 'new-1', title: 'A', approval_status: 'approved' }); + expect(res.results[0].success).toBe(true); + expect(res.results[0].droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, + ]); }); - it('stays SILENT for an ordinary create that never asked for the exemption (#3043 unchanged)', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - await p.createData({ - object: 'ticket', - data: { subject: 'x', closed_at: '2019-04-01T00:00:00Z' }, - context: { userId: 'u1' }, + it('insertManyData forwards every row whole and keeps ROW precision from the union', async () => { + const { p, inserts } = makeProtocol(); + const res: any = await p.insertManyData({ + object: 'approval_case', + records: [{ title: 'A', approval_status: 'approved' }, { title: 'B' }], }); - expect(inserts[0].data).toEqual({ subject: 'x' }); - // This is what makes the signal INFORMATIVE: it distinguishes "your fields - // were stripped by the ordinary rule" (already reported via droppedFields) - // from "the exemption you requested does not exist on this path". - expect(warn).not.toHaveBeenCalled(); + expect(inserts[0].data).toEqual([{ title: 'A', approval_status: 'approved' }, { title: 'B' }]); + // The engine's event is the batch UNION (its listener carries no row + // index); row precision is recovered by asking which row SUPPLIED the key. + expect(res.outcomes[0].droppedFields).toEqual([ + { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }, + ]); + expect(res.outcomes[1].droppedFields, 'row B supplied none of the dropped names').toBeUndefined(); }); +}); - it('stays SILENT when preserveAudit was requested but nothing was actually stripped', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - await p.createData({ object: 'ticket', data: { subject: 'plain row' }, context: HISTORICAL_CTX }); - expect(inserts[0].data).toEqual({ subject: 'plain row' }); - expect(warn, 'a request that loses nothing has nothing to report').not.toHaveBeenCalled(); +describe('#14147 — engine listener wiring (the firing control for every assertion above)', () => { + // The faces enumerated here are the ones whose RESPONSE carries + // `droppedFields`: `CreateDataResponse`, `CreateManyDataResponse`, and the + // per-row results of `insertManyData` / `batchData`. `cloneData` is + // deliberately NOT among them — its contract has no such member; the case + // after this one pins that exclusion so "every" stays true of what is listed. + it('every create face whose response carries droppedFields passes an onFieldsDropped listener to the engine', async () => { + const { p, inserts } = makeProtocol(); + await p.createData({ object: 'approval_case', data: { title: 'A' } }); + await p.createManyData({ object: 'approval_case', records: [{ title: 'A' }] }); + await p.batchData({ + object: 'approval_case', + request: { operation: 'create', records: [{ data: { title: 'A' } }] }, + } as any); + await p.batchData({ + object: 'approval_case', + request: { operation: 'upsert', records: [{ data: { title: 'A' } }] }, + } as any); + await p.batchData({ + object: 'approval_case', + request: { operation: 'upsert', records: [{ id: 'new-1', data: { title: 'A' } }] }, + } as any); + await p.insertManyData({ object: 'approval_case', records: [{ title: 'A' }] }); + expect(inserts, 'createData · createManyData · batchData create · batchData upsert-create ×2 (no id / unknown id) · insertManyData') + .toHaveLength(6); + for (const call of inserts) { + expect(typeof call.options?.onFieldsDropped, 'a face with no listener reports a silent drop').toBe('function'); + } }); - it('createManyData warns ONCE with the UNION of what every row lost', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - await p.createManyData({ - object: 'ticket', - records: [ - { subject: 'A', closed_at: '2019-04-01T00:00:00Z' }, - { subject: 'B', resolved_by: 'usr_bob' }, - ], - context: HISTORICAL_CTX, - }); - expect(inserts[0].data).toEqual([{ subject: 'A' }, { subject: 'B' }]); - expect(warn, 'one signal per ingress call — the strip is schema-uniform').toHaveBeenCalledTimes(1); - const msg = String(warn.mock.calls[0]?.[0]); - expect(msg).toContain('closed_at'); - expect(msg).toContain('resolved_by'); + it('cloneData is the one create face that passes NO listener — its response contract declares no droppedFields', async () => { + // `CloneDataResponseSchema` (#11924, declared AS PRODUCED) is exactly + // `{ object, id, sourceId, record }`; `search-clone-schema-conformance.test.ts` + // holds the producer to that key set and asserts `droppedFields` in + // particular is absent. So a listener here would have nowhere contracted + // to report to. The engine still strips a copied-over or overridden + // readonly column and still logs the `warn` line — the clone simply does + // not carry the event on the wire. Reporting it means a new response key, + // which is a spec change with its own card, not a delegation detail. + const { p, inserts } = makeProtocol(); + await p.cloneData({ object: 'approval_case', id: 'src-1' } as any); + expect(inserts).toHaveLength(1); + expect(inserts[0].options?.onFieldsDropped).toBeUndefined(); }); - it('batchData create carries the same signal (every ingress, by construction)', async () => { - const warn = captureWarn(); - const { p, inserts } = makeTicketProtocol(); - await p.batchData({ - object: 'ticket', - request: { operation: 'create', records: [{ data: { subject: 'A', closed_at: '2019-04-01T00:00:00Z' } }] } as any, - context: HISTORICAL_CTX, - } as any); - expect(inserts[0].data).toEqual({ subject: 'A' }); - expect(warn).toHaveBeenCalledTimes(1); - expect(String(warn.mock.calls[0]?.[0])).toContain('preserveAudit is UPDATE-only'); + it('a create that drops NOTHING reports no droppedFields at all', async () => { + const { p } = makeProtocol(); + const res: any = await p.createData({ object: 'approval_case', data: { title: 'clean' } }); + expect(res.droppedFields).toBeUndefined(); }); }); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 6e66fe99b5..3bf888dd1d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -78,7 +78,6 @@ import { SEARCHABLE_TEXTUAL_TYPES, SEARCHABLE_ENUM_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS, isVirtualSearchField, classifyDottedFilterHead, - RUNTIME_OWNED_FIELD_TYPES, RPC_QUERY_ALIAS_SLOTS, foldQueryAliasSlots, type QueryAliasConflict, type QueryAliasSlot, type DroppedFieldsEvent, type QueryAST, type EngineQueryOptionsParsed, @@ -1736,200 +1735,16 @@ const CLONE_STRIP_FIELDS: readonly string[] = [ 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', ]; -/** - * [#3043] Drop caller-supplied writes to statically `readonly: true` fields from - * an INSERT payload, at the external DATA-WRITE INGRESS. - * - * #2948/#3003 made static `readonly` server-enforced on UPDATE (the engine strips - * a non-system caller's write). INSERT was left exempt — but for approval/status - * columns that exemption is the SHORTER attack: instead of the #3003 - * draft-then-PATCH move, a non-system caller can POST a record already - * `approval_status: 'approved'` in one step. This closes it symmetrically, but at - * the INGRESS rather than in the engine: every EXTERNAL programmatic create — the - * REST CRUD route, the GraphQL/MCP dispatcher (`bridge.create` → `callData` → - * here), and bulk import — lands in the DataProtocol, while TRUSTED internal - * writers (better-auth's adapter, the metadata repository, the seed loader) call - * `engine.insert` DIRECTLY and never pass through here. Keeping the strip at the - * ingress therefore protects every agent/caller path at once WITHOUT stripping - * the internal writers that legitimately seed read-only columns on create - * (identity provisioning, provenance stamps, event-log cursors) — the blast - * radius an engine-level insert strip would have. - * - * Silent by contract (like the UPDATE / `readonlyWhen` strips): the forged key is - * dropped, the create still succeeds, and the engine re-derives the field's - * `defaultValue` (a forged `approval_status` becomes `draft`, the enforced - * initial state, not NULL). `isSystem` writes are exempt. `readonlyWhen` stays - * INSERT-exempt (a conditional lock needs a prior record, which a create lacks). - * Handles a single record or a batch array. - * - * SCOPE — author-defined business objects only. PLATFORM objects (`managedBy` - * set, or the reserved `sys_` namespace) carry their OWN field-write governance - * that a silent strip must not pre-empt: e.g. ADR-0086 REJECTS (403) a forged - * `managed_by:'package'` / `package_id` on `sys_permission_set`, and #3004 - * rejects a forged `owner_id` anchor — several of those columns are `readonly`, - * so stripping them here would silently swallow the payload the guard is meant to - * reject. The #3043 threat is app approval/status/verdict fields (the issue's - * `sporadic_application` / `assessment`), never `sys_`; this is the same - * platform-vs-authored boundary `applySystemFields` uses for ownership. - * - * SCOPE, second boundary — RUNTIME-OWNED field types - * ({@link RUNTIME_OWNED_FIELD_TYPES}: today `autonumber`) are left to the - * ENGINE's own insert strip (`stripRuntimeOwnedFields`, #5503), which runs on - * every insert path including the direct `engine.insert` callers this ingress - * never sees. Skipping them here removes no protection and prevents this seam - * from PRE-EMPTING an exemption it does not implement: the engine strip honours - * `preserveAudit` (#3493 — a historical import reinstating legacy record - * numbers) while this one knows only `isSystem`. Before #5628 the distinction - * was academic, because an `autonumber` field carried no `readonly` flag for the - * loop below to notice; now that `Field.autonumber` injects one, stripping here - * would silently delete the value a historical import is entitled to keep, - * BEFORE the engine could apply the whitelist. Author-declared `readonly` on - * every other type is untouched — the #3043 strip is exactly as wide as it was. - * - * SCOPE, third boundary — `preserveAudit` IS NOT READ HERE, DELIBERATELY (#6640). - * The historical-import exemption (#3493) is an **UPDATE-path rule only**; see - * {@link warnPreserveAuditIgnoredOnInsert} for the ruling, the reason, and the - * loud signal a non-system INSERT gets for asking. - */ -function stripReadonlyForInsert(schema: any, data: any, context: any): any { - if (context?.isSystem) return data; - if (!schema || schema.managedBy || String(schema.name ?? '').startsWith('sys_')) return data; - const fields = schema?.fields; - if (!fields || data == null) return data; - // [#6640] The UNION of names actually removed, across every row of a batch — - // the same aggregation `mergeDroppedFieldEvents` applies, and for the same - // reason: the strip is schema-uniform, so one signal per ingress call is - // faithful where one per row would be noise. - const stripped = new Set(); - const stripRow = (row: any): any => { - if (row == null || typeof row !== 'object') return row; - let out = row; - for (const name of Object.keys(fields)) { - if (!fields[name]?.readonly) continue; - // [#5628] The engine's runtime-owned strip owns these, with the - // wider exemption set. See the note above. - if (RUNTIME_OWNED_FIELD_TYPES.has(String(fields[name]?.type ?? ''))) continue; - if (!(name in out)) continue; - if (out === row) out = { ...row }; - delete out[name]; - stripped.add(name); - } - return out; - }; - const result = Array.isArray(data) ? data.map(stripRow) : stripRow(data); - if (context?.preserveAudit && stripped.size > 0) { - warnPreserveAuditIgnoredOnInsert(String(schema.name ?? ''), Array.from(stripped)); - } - return result; -} - -/** - * [#6640] THE loud half of the `preserveAudit` ruling — a non-system INSERT that - * asks for the historical-import exemption is TOLD it does not exist here. - * - * ## The contradiction this closes - * - * `FieldSchema.readonly`'s `.describe()` promised the `preserveAudit` exemption - * (#3493) on BOTH write paths, and `docs/protocol/objectql/security.mdx` agreed. - * Only UPDATE ever implemented it: `stripReadonlyFields` (objectql's - * rule-validator) consults `isPreservableUnderAudit`, while this INSERT ingress - * has never read `preserveAudit` at all — `isSystem` is its only exemption. REST - * import's `treatAsHistorical` (`rest/src/import-runner.ts`) puts - * `preserveAudit: true` on the write context and creates through `createData`, - * i.e. through exactly this seam. So ONE historical import PRESERVED an - * author-declared `readonly` business column (`closed_at`, `resolved_by`) on the - * rows it updated and SILENTLY DROPPED it on the rows it created. - * - * ## Which half the ruling kept (maintainer, 2026-08-08 — option 2) - * - * The **enforcement** is the truth and the **contract** was narrowed to it: the - * exemption is UPDATE-only, and this entry keeps honouring `isSystem` alone. - * Honouring `preserveAudit` here instead would have handed a NON-system caller — - * `treatAsHistorical` arrives on an ordinary REST import request — the ability to - * seed the approval/status columns #3043 exists to protect, in one POST. That is - * the #3043 threat model reversed, for a capability with no measured consumer: - * replaying archival readonly facts on INSERT is available today, from a system - * context, which is what the in-repo importer can run as. - * - * ## Why it is a WARNING and not a throw — measured, not assumed - * - * The ruling made loudness binding and left the SHAPE to whichever one can be - * both loud and non-breaking. A throw cannot: `runImport`'s per-row writer - * collects a write error into `toFailedResult(rowNo, res.error)` rather than - * aborting the run, so refusing here would not stop a historical import — it - * would convert every row it CREATES into a failed row, while the rows it - * updates still succeed. And the trigger is not exotic: the audit family itself - * (`created_at` / `created_by` / `updated_at` / `updated_by`) is `readonly: true` - * in the registry's `AUDIT_FIELD_DEFS`, so an ordinary export→historical-import - * round-trip carries readonly columns on every row. Measured on this branch, a - * throwing variant took the historical import of 2 new rows from - * `{created: 2, errors: 0}` to `{created: 0, errors: 2}`. Breaking the shipped - * `treatAsHistorical` flow for new rows is precisely the condition under which - * the ruling names the loud WARNING — strip still applied — as the - * containment-correct landing. - * - * The silence this replaces was specific: the drop itself already surfaces - * through `droppedFields` (#3431), but a caller who EXPLICITLY asked for the - * exemption could not tell "your fields were stripped by the ordinary #3043 - * rule" from "the exemption you requested does not exist on this path". This - * says the second one, by name. It fires ONLY when `preserveAudit` was requested - * AND something was actually removed — a request that loses nothing has nothing - * to report, and the ordinary non-`preserveAudit` strip is left exactly as quiet - * as #3043 designed it. - * - * Family precedent #5714/#5931: a declared key silently ignored on one branch - * joins the loud set by default. Those two could reject outright because they - * judge AUTHORING input, before anything runs; this one sits on a live write - * path, which is what moves it from throw to warn. - */ -function warnPreserveAuditIgnoredOnInsert(object: string, fields: readonly string[]): void { - console.warn( - `[Protocol] preserveAudit is UPDATE-only and was IGNORED on this INSERT` + - `${object ? ` (object '${object}')` : ''}: the historical-import exemption (#3493) applies when a ` + - `record is UPDATED, never when it is created, so the readonly field(s) ${fields.join(', ')} were ` + - `STRIPPED from this create rather than preserved. To replay archival readonly facts on INSERT, ` + - `write from a system context (\`context.isSystem\`) — a non-system create may not seed a readonly ` + - `column (#3043/#6640).`, - ); -} - -/** - * [#3431] Recover a `DroppedFieldsEvent` from a before/after write-payload diff. - * - * The UPDATE strips (static `readonly` / `readonlyWhen`) run INSIDE the engine, - * which reports them via the `onFieldsDropped` listener (wired in `updateData`). - * The CREATE `readonly` strip, however, runs at THIS protocol ingress - * (`stripReadonlyForInsert`, #3043) — BEFORE the engine — so the engine listener - * never sees it. Diffing the caller-supplied keys against the stripped payload - * recovers exactly which supplied fields the ingress strip removed, so the create - * path can surface them symmetrically with update. - * - * Returns `null` when nothing was dropped (same reference, non-object, array, or - * no key delta) so callers can `if (ev) dropped.push(ev)` without emitting empty - * events. Mirrors the engine's own before/after key-set diff (`reportDroppedFields` - * in objectql/engine.ts) so both channels agree on what "dropped" means. - */ -function diffDroppedFields( - object: string, - before: unknown, - after: unknown, - reason: DroppedFieldsEvent['reason'], -): DroppedFieldsEvent | null { - if (before === after || before == null || typeof before !== 'object' || Array.isArray(before)) return null; - const afterObj = (after ?? {}) as Record; - const fields = Object.keys(before as Record).filter((k) => !(k in afterObj)); - return fields.length > 0 ? { object, fields, reason } : null; -} - /** * [#3455] Collapse a batch's per-row `DroppedFieldsEvent`s into one event per * `(object, reason)` with the UNION of dropped field names. * * Used by the bulk-create surface (`createManyData`), whose `{ object, records, * count }` response has no per-row slot to hang a `droppedFields` on. The - * insert-ingress strip (#3043) is static-`readonly` only — schema-uniform, so - * every row drops the same set — which makes an aggregated view faithful rather - * than lossy. Returns `[]` when nothing was dropped so callers can spread + * create-side static-`readonly` strip is schema-uniform — every row drops the + * same set — which makes an aggregated view faithful rather than lossy. (Since + * #14147 that strip is the ENGINE's, which reports one event per CALL for it, + * so the aggregation is over the runtime-owned per-row events.) Returns `[]` when nothing was dropped so callers can spread * `...(x.length ? { droppedFields: x } : {})` and keep the omit-when-empty shape. * The per-row `insertMany`/`batch` paths keep row precision instead (they have a * per-row result to carry it). @@ -10571,26 +10386,25 @@ export class ObjectStackProtocolImplementation implements async createData(request: { object: string, data: any, context?: any }) { this.assertObjectRegistered(request.object); // [#3770] - // [#3043] Ingress-level static-`readonly` strip — a non-system caller - // cannot seed a read-only column (e.g. `approval_status`) on create. - const data = stripReadonlyForInsert( - this.engine.registry?.getObject(request.object), - request.data, - request.context, - ); - // [#3431] The #3043 ingress strip above is SILENT by contract; surface it - // so a REST/API caller learns which supplied fields were dropped, symmetric - // with `updateData`. The strip lives at THIS ingress (not the engine, which - // is INSERT-readonly-exempt, #3413), so recover it by diffing the supplied - // payload against the stripped one. The engine's `onFieldsDropped` is ALSO - // wired below so a FUTURE insert-side engine strip surfaces automatically - // through the same list instead of going silent. + // [#14147] The static-`readonly` create strip is the ENGINE's now + // (`engine.insert` → `stripReadonlyFields`, `isSystem`-gated, exactly as + // on update), so this ingress hands the caller's payload over WHOLE and + // reports the drop through the listener #3431 already wired here. The + // ingress copy it replaces (`stripReadonlyForInsert`, #3043) is deleted + // rather than kept as a second implementation — maintainer ruling, + // 2026-09-03, superseding the 2026-07-24 "INSERT (all callers) exempt" + // row. Two consequences worth stating, because they are what the + // before/after diff used to fake: + // - the event now carries the ENGINE's verdict rather than this seam's + // reconstruction of it, so `droppedFields` says the same thing here, + // on `data.insert`, and in a `create_record` flow node; + // - and it is the engine's guards that decide, which are wider than a + // payload diff can be: a `beforeInsert` hook's own stamp is not a + // caller forgery (#5591/#14259), and the ingress ran before the hooks. const dropped: DroppedFieldsEvent[] = []; - const ingressDropped = diffDroppedFields(request.object, request.data, data, 'readonly'); - if (ingressDropped) dropped.push(ingressDropped); const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; if (request.context !== undefined) opts.context = request.context; - const result = await this.engine.insert(request.object, data, opts); + const result = await this.engine.insert(request.object, request.data, opts); // [#7823] The 201 body is a GENERIC-DATA-PATH surface: strip // `internal: true` fields here, at the ingress, per the A-prime ruling // (2026-08-13). The engine deliberately no longer strips its own write @@ -10666,13 +10480,14 @@ export class ObjectStackProtocolImplementation implements Object.assign(data, request.overrides); } - // [#3043] A clone is a create: a non-system caller must not carry over (or - // override in) a read-only column — copying the source's `approval_status` - // or forging one via `overrides` would mint an approved record. Strip them - // so the insert re-derives their `defaultValue`, symmetric with createData. - const insertData = stripReadonlyForInsert(schema, data, ctx); - - const result = await this.engine.insert(request.object, insertData, ctxOpt as any); + // [#3043/#14147] A clone is a create: a non-system caller must not carry + // over (or override in) a read-only column — copying the source's + // `approval_status` or forging one via `overrides` would mint an approved + // record. That strip is `engine.insert`'s since #14147, so the payload + // goes over whole and the insert re-derives the field's `defaultValue`, + // symmetric with createData. `overrides` are applied ABOVE this line, so + // a readonly key smuggled through them is still judged by the strip. + const result = await this.engine.insert(request.object, data, ctxOpt as any); // [#7823] Same ingress strip as `createData` — a clone's 201 body is // the same generic-data-path surface. (The SOURCE row was read through // the engine's find path, which already omits internal fields, so the @@ -11618,12 +11433,15 @@ export class ObjectStackProtocolImplementation implements this.assertObjectRegistered(object); // [#3770] const { operation, records, options } = batchReq; - // [#3043] The batch endpoint is an external ingress: strip forged - // read-only columns on create. [#3455] It DOES resolve an execution - // context (threaded by REST); thread it to every engine call so RLS/FLS - // and `readonlyWhen` run under the caller, and pass it to the strip so a - // system caller is correctly exempt (the pre-#3455 code hard-coded the - // strip context to `undefined`, treating every batch create as non-system). + // [#3043 → #14147] The batch endpoint is an external ingress, and it + // used to strip forged read-only columns on create HERE; that strip is + // `engine.insert`'s since the 2026-09-03 ruling, reported back per row + // through `onFieldsDropped`. [#3455] It DOES resolve an execution + // context (threaded by REST); thread it to every engine call so RLS/FLS, + // `readonlyWhen` AND the create-side readonly strip run under the + // caller, so a system caller is correctly exempt (the pre-#3455 code + // hard-coded the strip context to `undefined`, treating every batch + // create as non-system). const batchSchema = this.engine.registry?.getObject(object); // ADR-0119 D4 — `atomic` is REAL or REFUSED, never silent best-effort. @@ -11779,10 +11597,14 @@ export class ObjectStackProtocolImplementation implements let failed = 0; // Spread form for options objects that already carry `where`/`onFieldsDropped` - // (`{}` spread is a safe no-op); arg form for `insert`, whose whole options - // arg is `undefined` when there is no context — exact parity with createData. + // (`{}` spread is a safe no-op). [#14147] EVERY `engine.insert` in this + // loop — `create`, and both arms of `upsert` that create — builds an + // options object: the readonly strip they report moved into + // `engine.insert` and reaches this seam only through `onFieldsDropped`, + // so the arg form (`undefined` when there is no context) is gone from + // here. A create-shaped arm that passed no listener reported a silent + // drop — the upsert arms did exactly that until the patch round. const ctxOpt = context !== undefined ? { context } : {}; - const insertCtx = context !== undefined ? { context } : undefined; // [#4793] `index` is the row's position in the REQUEST array — the // correlation a caller needs for failure rows that carry no id. @@ -11790,14 +11612,19 @@ export class ObjectStackProtocolImplementation implements try { switch (operation) { case 'create': { - // [#3455] Diff the supplied row against the stripped one so a - // batch-create caller sees the same `droppedFields` a - // single-write create surfaces (#3431). - const stripped = stripReadonlyForInsert(batchSchema, record.data || record, context); - const ev = diffDroppedFields(object, record.data || record, stripped, 'readonly'); - const created = await this.engine.insert(object, stripped, insertCtx as any); + // [#3455/#14147] A batch-create caller sees the same + // `droppedFields` a single-write create surfaces (#3431) — + // read off the ENGINE's listener now that the readonly + // create strip lives there, instead of reconstructed from a + // before/after payload diff at this seam. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, record.data || record, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - results.push({ id: created.id, success: true, data: created, index, ...(ev ? { droppedFields: [ev] } : {}) }); + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); succeeded++; break; } @@ -11847,14 +11674,28 @@ export class ObjectStackProtocolImplementation implements omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823] results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) }); } else { - const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, insertCtx as any); + // [#14147] The upsert's CREATE arm is a create face + // too: same listener, same per-row `droppedFields`, + // as `case 'create'` above and the update arm just up. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - results.push({ id: created.id, success: true, data: created, index }); + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); } } else { - const created = await this.engine.insert(object, record.data || record, insertCtx as any); + // [#14147] Same: an id-less upsert row IS a create. + const rowDropped: DroppedFieldsEvent[] = []; + const created = await this.engine.insert(object, record.data || record, { + ...ctxOpt, + onFieldsDropped: (e: DroppedFieldsEvent) => { rowDropped.push(e); }, + } as any); omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823] - results.push({ id: created.id, success: true, data: created, index }); + const ev = mergeDroppedFieldEvents(rowDropped); + results.push({ id: created.id, success: true, data: created, index, ...(ev.length > 0 ? { droppedFields: ev } : {}) }); } succeeded++; break; @@ -12049,36 +11890,17 @@ export class ObjectStackProtocolImplementation implements async createManyData(request: { object: string, records: any[], context?: any }): Promise { this.assertObjectRegistered(request.object); // [#3770] - // [#3043] Ingress-level static-`readonly` strip (per row) — mirrors - // createData for the bulk-create / import surface. - const rows = stripReadonlyForInsert( - this.engine.registry?.getObject(request.object), - request.records, - request.context, - ); - // [#3455] Surface the #3043 ingress strip, symmetric with single-write - // createData. Diff each supplied row against its stripped form, then - // AGGREGATE — the `{ records, count }` response has no per-row slot, so - // a union is the only representable view here. (It used to be lossless - // as well, the ingress strip being static-`readonly` and therefore - // schema-uniform; the engine strip #5503 adds is per-row, so the union - // now genuinely aggregates. `insertManyData`, which HAS a per-row slot, - // keeps row precision for both sources.) + // [#3455/#5503/#14147] Both create-side strips are the engine's now — + // runtime-owned `autonumber` (#5503) and, since #14147, static + // author-declared `readonly` — so ONE listener carries both, and this + // seam no longer diffs payloads to recover a strip it performed itself. + // AGGREGATED: the `{ records, count }` response has no per-row slot, so + // a union is the only representable view here. (`insertManyData`, which + // HAS a per-row slot, recovers row precision from the same union.) const dropped: DroppedFieldsEvent[] = []; - if (Array.isArray(request.records)) { - for (let i = 0; i < request.records.length; i++) { - const ev = diffDroppedFields(request.object, request.records[i], Array.isArray(rows) ? rows[i] : rows, 'readonly'); - if (ev) dropped.push(ev); - } - } - // [#5503] The engine gained an INSERT-side strip of its own (runtime-owned - // `autonumber` values a non-system caller supplied). Forward the listener - // here as `createData` already does, so a bulk create / import learns - // which record numbers were refused instead of only the server log seeing - // it. Merging AFTER the write is what lets both sources land in one list. const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } }; if (request.context !== undefined) opts.context = request.context; - const records = await this.engine.insert(request.object, rows, opts); + const records = await this.engine.insert(request.object, request.records, opts); // [#7823] Bulk create is the same generic-data-path surface as the // single-record 201 — one strip over the returned rows, at the // ingress. (Today's `internal`-flagged objects grant no `bulk` @@ -12108,37 +11930,23 @@ export class ObjectStackProtocolImplementation implements if (typeof engineInsertMany !== 'function') { throw new Error('insertManyData requires an engine with insertMany (framework#3172)'); } - // Same ingress strip as createManyData (#3043). - const rows = stripReadonlyForInsert( - this.engine.registry?.getObject(request.object), - request.records, - request.context, - ); - // [#3455] Per-row #3043 ingress-strip observability. Unlike createManyData, - // this partial-success path HAS a per-row slot (`outcomes[i]`), so keep - // row precision: `stripReadonlyForInsert` maps 1:1 in order, so the i-th - // supplied row diffs against the i-th stripped row and rides the i-th - // outcome. Computed BEFORE the insert so a per-row engine failure never - // hides which fields the ingress had already dropped. - const rowsArr = Array.isArray(rows) ? rows : [rows]; - const perRowDropped: Array = Array.isArray(request.records) - ? request.records.map((rec, i) => diffDroppedFields(request.object, rec, rowsArr[i], 'readonly')) - : []; - // [#5503] The ENGINE now strips too (runtime-owned `autonumber` values a - // non-system caller supplied), and its `onFieldsDropped` event is the - // UNION over the batch — the listener signature carries no row index. Row - // precision is recoverable without one: the engine strip only removes - // keys the ROW ITSELF supplied, so a dropped name belongs to exactly the - // rows whose supplied payload carried it. Without this the import - // surface (which prefers this partial-success path over createManyData) - // would drop record numbers with nothing but a server log to show for it. + // [#5503/#14147] Every create-side strip is the ENGINE's — runtime-owned + // `autonumber` (#5503) and static author-declared `readonly` (#14147) — + // and its `onFieldsDropped` event is the UNION over the batch, the + // listener signature carrying no row index. This partial-success path HAS + // a per-row slot (`outcomes[i]`), and row precision is recoverable + // without an index: the strip only removes keys the ROW ITSELF supplied, + // so a dropped name belongs to exactly the rows whose supplied payload + // carried it. Without this the import surface (which prefers this path + // over createManyData) would drop columns with nothing but a server log + // to show for it. const engineDropped = new Set(); const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { for (const f of e.fields) engineDropped.add(f); } }; if (request.context !== undefined) opts.context = request.context; const outcomes: Array<{ ok: boolean; record?: any; error?: unknown; droppedFields?: DroppedFieldsEvent[] }> = await engineInsertMany.call( this.engine, request.object, - rows, + request.records, opts, ); // [#7823] Per-outcome ingress strip — the partial-success face hands @@ -12156,7 +11964,6 @@ export class ObjectStackProtocolImplementation implements const supplied = (request.records?.[i] ?? {}) as Record; const mine = [...engineDropped].filter((f) => f in supplied); const events: DroppedFieldsEvent[] = []; - if (perRowDropped[i]) events.push(perRowDropped[i]!); if (mine.length > 0) events.push({ object: request.object, fields: mine, reason: 'readonly' }); const merged = mergeDroppedFieldEvents(events); if (merged.length > 0) outcomes[i].droppedFields = merged; diff --git a/packages/objectql/src/engine-autonumber-runtime-owned.test.ts b/packages/objectql/src/engine-autonumber-runtime-owned.test.ts index 22de28069e..9dce2cb645 100644 --- a/packages/objectql/src/engine-autonumber-runtime-owned.test.ts +++ b/packages/objectql/src/engine-autonumber-runtime-owned.test.ts @@ -546,19 +546,24 @@ describe('#5503 — autonumber is runtime-owned: UPDATE', () => { * them sees what that flag changes on the way in. * * What it changes is WHICH strip gets to the field first. A `readonly` field is - * also stripped at the DataProtocol create INGRESS (`stripReadonlyForInsert`, - * #3043) — a seam that knows only the `isSystem` exemption, while the engine's - * runtime-owned strip also honours `preserveAudit` (#3493: a historical import - * reinstating legacy record numbers). Left alone, the flag would therefore have - * SILENTLY narrowed a documented exemption: the ingress would delete the legacy - * number before the engine could keep it, with no test anywhere going red, - * because every existing preserveAudit pin calls `engine.insert` directly. + * also subject to the AUTHOR-declared static strip — at the DataProtocol create + * ingress when this note was written (`stripReadonlyForInsert`, #3043), and + * inside `engine.insert` itself since the maintainer ruling of 2026-09-03 + * (option C, #14147), which deleted that ingress copy. Either way it is a strip + * that knows only the `isSystem` exemption, while the engine's runtime-owned + * strip also honours `preserveAudit` (#3493: a historical import reinstating + * legacy record numbers). Left alone, the flag would therefore have SILENTLY + * narrowed a documented exemption: the static strip would delete the legacy + * number before the runtime-owned pass could keep it, with no test anywhere + * going red, because every existing preserveAudit pin calls `engine.insert` + * directly. * - * So the ingress skips runtime-owned types outright (they are covered by the - * engine strip on EVERY insert path, including the direct `engine.insert` - * callers the ingress never sees) and these cases pin both halves of that: the - * ordinary caller is still stripped, and the historical import still keeps its - * value — flagged or not, the verdicts are identical. + * So the static strip skips runtime-owned types outright — the ingress copy did + * by rule, and the in-engine pass does by construction: it runs over + * `staticReadonlyInsertSubject`, a schema view with those types removed, AFTER + * the runtime-owned pass has had its say — and these cases pin both halves of + * that: the ordinary caller is still stripped, and the historical import still + * keeps its value — flagged or not, the verdicts are identical. */ describe('#5628 — a `readonly: true` autonumber keeps the #5503 exemption set', () => { // What `Field.autonumber({ label: 'Invoice No.' })` produces since #5628. @@ -603,9 +608,12 @@ describe('#5628 — a `readonly: true` autonumber keeps the #5503 exemption set' }); it('keeps a legacy number for a `preserveAudit` historical import THROUGH THE INGRESS (#3493)', async () => { - // The regression this whole describe exists for: the ingress strip has no - // `preserveAudit` exemption, so if it acted on the flag the value would be - // gone before the engine's whitelist ran. + // The regression this whole describe exists for: the static strip has no + // `preserveAudit` exemption — at the ingress when this was written, inside + // `engine.insert` since ruling C — so if it acted on the flagged autonumber + // the value would be gone before the runtime-owned whitelist could keep it. + // It does not: the static pass runs over `staticReadonlyInsertSubject`, + // with runtime-owned types removed, after the runtime-owned pass. const created = await rig.protocol.createData({ object: 'an_invoice', data: { name: 'legacy', invoice_number: 'LEGACY-0007' }, @@ -623,7 +631,7 @@ describe('#5628 — a `readonly: true` autonumber keeps the #5503 exemption set' expect(created.record.invoice_number).toBe('INV-000042'); }); - it('an author-declared `readonly` field of an ORDINARY type is still stripped at the ingress', async () => { + it('an author-declared `readonly` field of an ORDINARY type is still stripped on a create through the ingress — inside `engine.insert` since ruling C', async () => { // The #3043 strip keeps its full width — only runtime-owned types moved. rig.engine.registry.registerObject({ name: 'an_case', diff --git a/packages/objectql/src/engine-insert-static-readonly-strip.test.ts b/packages/objectql/src/engine-insert-static-readonly-strip.test.ts new file mode 100644 index 0000000000..f322753c3a --- /dev/null +++ b/packages/objectql/src/engine-insert-static-readonly-strip.test.ts @@ -0,0 +1,345 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #14147 — a static `readonly` field is stripped from a NON-SYSTEM caller's +// INSERT payload INSIDE `engine.insert`, exactly as on `engine.update`. +// +// ## The measurement this file inverts +// +// Reported from an application against 17.2.0, on a real booted kernel: +// +// write face context forged readonly col +// ------------------------------------ --------------- ------------------- +// MetadataProtocolService.createData none stripped +// MetadataProtocolService.createData { isSystem:false } stripped +// MetadataProtocolService.createData { isSystem:true } preserved (intended) +// engine.insert (the `data` service) none PRESERVED ← the hole +// +// The create-side strip lived at the DataProtocol ingress +// (`stripReadonlyForInsert`, #3043) while the update-side one lived in the +// engine (`stripReadonlyFields`, #2948). So `readonly` meant one thing on +// insert and another on update, three consequences followed — +// +// 1. a non-system caller reaching `engine.insert` DIRECTLY wrote the column +// with no refusal, no WARN and no `onFieldsDropped` event; +// 2. `assertReferencesResolve`'s own doc sentence ("like every other +// write-path guard in this engine") was false about the create path; +// 3. `create_record` (`@objectstack/service-automation`) passes +// `onFieldsDropped` to `data.insert` and surfaces `output.droppedFields` + +// node `warnings` — a channel that could never carry a readonly drop. +// +// **Maintainer ruling, 2026-09-03 (option C)** — presented as overturning their +// own 2026-07-24 "INSERT (all callers) exempt" row, verbatim 「同意」: one +// semantics, one enforcement point. `stripReadonlyFields` runs on both write +// paths, `isSystem`-gated, with the same `onFieldsDropped` / warn behaviour; +// seeding a readonly column at create time is done under system context, the +// exemption the 2026-07-24 table already grants. The boundary copy is DELETED +// rather than kept as a second implementation. +// +// ## What this file must keep telling apart +// +// The strip is a FORGERY strip, not a column ban. Three exemptions are +// load-bearing and each has a pin below: `isSystem`; a server-side stamp (a +// `beforeInsert` hook's own write is not caller-supplied); and the platform +// objects whose own 403 guards a silent strip must not pre-empt (ADR-0086 / +// #3004 — carried over from the deleted copy, and NOT part of what ruling C +// superseded). Two neighbouring rules must stay where they are: `preserveAudit` +// is an UPDATE-path exemption (maintainer, 2026-08-08) and `readonlyWhen` has no +// prior record on a create, so both keep their INSERT posture unchanged. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; + +function makeCapturingLogger() { + const lines: Array<{ level: string; msg: string }> = []; + const logger: any = { + lines, + trace() {}, fatal() {}, + debug() {}, info() {}, + warn(msg: string) { lines.push({ level: 'warn', msg: String(msg) }); }, + error(msg: string) { lines.push({ level: 'error', msg: String(msg) }); }, + child() { return logger; }, + }; + return logger; +} + +/** Records what actually reaches the driver — the payload is the verdict. */ +function makeRecordingDriver() { + const creates: Array> = []; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(_o: string, data: Record) { + creates.push({ ...data }); + return { id: 'rec_1', ...data }; + }, + async update(_o: string, id: string, data: Record) { return { id, ...data }; }, + async updateMany() { return 0; }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 0; }, + async bulkCreate(o: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(o, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, creates }; +} + +/** + * `completed_at` is the application-reported shape (an author-declared business + * `readonly` column). `approval_status` is #3043's original attack target and + * carries a `defaultValue`, so a stripped forgery falls back to the enforced + * initial state rather than to NULL. `account_number` is the RUNTIME-owned type + * whose own pass must keep owning it, and `locked_note` is the second + * author-declared column used for the batch/multi-key cases. + */ +async function makeEngine(objectName = 'duly_task') { + const logger = makeCapturingLogger(); + const engine = new ObjectQL({ logger }); + const { driver, creates } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: objectName, + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + title: { name: 'title', type: 'text' }, + completed_at: { name: 'completed_at', type: 'datetime', readonly: true }, + approval_status: { name: 'approval_status', type: 'text', readonly: true, defaultValue: 'draft' }, + locked_note: { name: 'locked_note', type: 'text', readonly: true }, + account_number: { name: 'account_number', type: 'autonumber' }, + status: { name: 'status', type: 'text' }, + frozen_total: { name: 'frozen_total', type: 'number', readonlyWhen: "record.status == 'paid'" }, + }, + } as any, 'test'); + return { engine, creates, logger }; +} + +interface Observed { + readonly created: Record | undefined; + readonly creates: number; + readonly dropped: DroppedFieldsEvent[]; + readonly warns: string[]; + readonly refusedCode: string | null; +} + +async function observeInsert( + data: unknown, + options: Record = {}, + arrange?: (engine: ObjectQL) => void, +): Promise { + const { engine, creates, logger } = await makeEngine(); + arrange?.(engine); + const dropped: DroppedFieldsEvent[] = []; + let refusedCode: string | null = null; + try { + await engine.insert('duly_task', data as any, { + onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, + ...options, + } as any); + } catch (e: any) { + refusedCode = e?.code ?? e?.name ?? null; + } + return { + created: creates[0], + creates: creates.length, + dropped, + warns: logger.lines.filter((l: any) => l.level === 'warn').map((l: any) => l.msg), + refusedCode, + }; +} + +describe('#14147 — THE REPRO, inverted: engine.insert enforces static readonly', () => { + it('a NO-CONTEXT caller no longer writes the readonly column — the application’s exact shape', async () => { + const o = await observeInsert({ title: 'T', completed_at: '2019-04-01T00:00:00Z' }); + expect(o.creates, 'the write still succeeds — a strip shrinks the payload, it does not refuse').toBe(1); + expect(o.created, 'the forged column never reaches the driver').not.toHaveProperty('completed_at'); + expect(o.created?.title).toBe('T'); + }); + + it('an explicitly NON-system caller is judged identically', async () => { + const o = await observeInsert( + { title: 'T', completed_at: '2019-04-01T00:00:00Z' }, + { context: { isSystem: false, userId: 'u1' } }, + ); + expect(o.created).not.toHaveProperty('completed_at'); + }); + + it('reports the drop through onFieldsDropped — the channel create_record was wired for', async () => { + const o = await observeInsert({ title: 'T', completed_at: 'x', approval_status: 'approved' }); + expect(o.dropped).toHaveLength(1); + expect(o.dropped[0].object).toBe('duly_task'); + expect(o.dropped[0].reason, 'the same vocabulary the update path reports under').toBe('readonly'); + expect([...o.dropped[0].fields].sort()).toEqual(['approval_status', 'completed_at']); + }); + + it('WARNs at `warn`, naming the field — and every claim the line makes is true of a CREATE', async () => { + const o = await observeInsert({ title: 'T', completed_at: 'x' }); + const line = o.warns.find((m) => m.includes('completed_at')); + expect(line, 'a silent drop is the #4632 second-class shape this ruling closes').toBeDefined(); + expect(line).toContain('duly_task'); + // The update-path message would have said three things that are false here. + // #8141/#8214's rule: a strip line may state only what is true of the call + // in front of it, and a remedy it names must be one that would have worked. + expect(line, 'this is a create, and the column takes its default rather than keeping a stored value') + .toContain('the create is being COMMITTED WITHOUT IT'); + expect(line).toContain('beforeInsert'); + expect(line, '⛔ preserveAudit is UPDATE-only — offering it here is the #8141 defect') + .not.toContain('preserveAudit: true'); + }); + + it('a stripped forgery falls back to the field’s defaultValue, not to NULL', async () => { + const o = await observeInsert({ title: 'T', approval_status: 'approved' }); + expect(o.created?.approval_status, 'the enforced initial state, re-derived by the engine').toBe('draft'); + }); +}); + +describe('#14147 — the exemptions, each one load-bearing', () => { + it('isSystem seeds the readonly column — the exemption the ruling names', async () => { + const o = await observeInsert( + { title: 'T', completed_at: '2019-04-01T00:00:00Z' }, + { context: { isSystem: true } }, + ); + expect(o.created?.completed_at, 'seed replay / runAs:system / system hooks').toBe('2019-04-01T00:00:00Z'); + expect(o.dropped, 'nothing was dropped, so nothing is reported').toEqual([]); + }); + + it('a beforeInsert hook’s OWN stamp survives — only CALLER-supplied keys are candidates', async () => { + const o = await observeInsert({ title: 'T' }, {}, (engine) => { + engine.registerHook('beforeInsert', async (ctx: any) => { + ctx.input.data.completed_at = '2026-01-01T00:00:00Z'; + }); + }); + expect(o.created?.completed_at, 'a server-side stamp is not a forgery').toBe('2026-01-01T00:00:00Z'); + expect(o.dropped).toEqual([]); + }); + + it('...and it survives even when the caller ECHOED the same key back (#14259’s record)', async () => { + const o = await observeInsert({ title: 'T', completed_at: 'forged' }, {}, (engine) => { + engine.registerHook('beforeInsert', async (ctx: any) => { + ctx.input.data.completed_at = '2026-01-01T00:00:00Z'; + }); + }); + expect(o.created?.completed_at, 'the hook wrote it — provenance, not value equality').toBe('2026-01-01T00:00:00Z'); + }); + + it('a PLATFORM object is left to its own 403 guard (ADR-0086 / #3004, carried over)', async () => { + // `managedBy` / the reserved `sys_` namespace carry dedicated write + // governance — a forged `managed_by: 'package'` is REFUSED, and silently + // stripping it would swallow the payload that guard exists to reject. That + // boundary was ruled on its own merits and is NOT the row ruling C + // superseded, so it comes across with the strip. + const logger = makeCapturingLogger(); + const engine = new ObjectQL({ logger }); + const { driver, creates } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'sys_permission_set', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + managed_by: { name: 'managed_by', type: 'text', readonly: true }, + }, + } as any, 'test'); + engine.registry.registerObject({ + name: 'crm_thing', + managedBy: 'package', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + locked: { name: 'locked', type: 'text', readonly: true }, + }, + } as any, 'test'); + await engine.insert('sys_permission_set', { managed_by: 'package' }, { context: { userId: 'u1' } } as any); + await engine.insert('crm_thing', { locked: 'forged' }, { context: { userId: 'u1' } } as any); + expect(creates[0].managed_by, 'sys_ object: passed through to its guard').toBe('package'); + expect(creates[1].locked, 'managedBy object: passed through to its guard').toBe('forged'); + }); +}); + +describe('#14147 — the neighbouring rules keep their INSERT posture', () => { + it('preserveAudit does NOT exempt a static readonly on create (maintainer, 2026-08-08)', async () => { + const o = await observeInsert( + { title: 'T', completed_at: '2019-04-01T00:00:00Z' }, + { context: { userId: 'importer', preserveAudit: true } }, + ); + expect(o.created, 'the historical-import exemption is an UPDATE-path rule').not.toHaveProperty('completed_at'); + const line = o.warns.find((m) => m.includes('preserveAudit is UPDATE-only')); + expect(line, 'the request is refused OUT LOUD, not silently').toBeDefined(); + expect(line).toContain('completed_at'); + expect(line).toContain('context.isSystem'); + expect(line, 'the remedy the line offers must be the one that actually works on a create') + .toContain('may not seed a readonly column'); + }); + + it('...but preserveAudit still reinstates a RUNTIME-owned autonumber (#3493/#5503)', async () => { + const o = await observeInsert( + { title: 'T', account_number: 'LEGACY-7' }, + { context: { userId: 'importer', preserveAudit: true } }, + ); + expect(o.created?.account_number, 'the runtime-owned pass owns this key, with the wider whitelist') + .toBe('LEGACY-7'); + expect(o.warns.filter((m) => m.includes('preserveAudit is UPDATE-only')), + 'nothing static was stripped, so the line has nothing to report').toEqual([]); + }); + + it('a caller-seeded autonumber is still reported as RUNTIME-owned, not as an author lock', async () => { + const o = await observeInsert({ title: 'T', account_number: 'FORGED-1' }); + // Stripped and then RE-ISSUED from the sequence (`applyAutonumbers` runs + // after validation), so the assertion is on the forgery, not on the key. + expect(o.created?.account_number, 'the runtime-owned strip is unchanged by this card') + .not.toBe('FORGED-1'); + const line = o.warns.find((m) => m.includes('account_number')); + expect(line, 'its own message states the true, actionable reason — not an author-declared lock') + .toContain('autonumber'); + }); + + it('readonlyWhen stays INSERT-exempt — a conditional lock has no prior record on a create', async () => { + const o = await observeInsert({ title: 'T', status: 'paid', frozen_total: 99 }); + expect(o.created?.frozen_total, 'unchanged by this card').toBe(99); + }); +}); + +describe('#14147 — strictReadonlyWrites refuses before any driver dispatch', () => { + it('ERR_READONLY_FIELD_REJECTED, zero creates, and the listener deliberately silent', async () => { + const o = await observeInsert( + { title: 'T', completed_at: 'forged' }, + { strictReadonlyWrites: true }, + ); + expect(o.refusedCode).toBe('ERR_READONLY_FIELD_REJECTED'); + expect(o.creates, 'refused BEFORE the driver — nothing was written').toBe(0); + expect(o.dropped, 'a refused write did not complete, so `dropped and committed` must not fire').toEqual([]); + }); + + it('strict adds NO second policy — an isSystem write it would not strip is still accepted', async () => { + const o = await observeInsert( + { title: 'T', completed_at: 'x' }, + { strictReadonlyWrites: true, context: { isSystem: true } }, + ); + expect(o.refusedCode).toBeNull(); + expect(o.creates).toBe(1); + }); +}); + +describe('#14147 — the batch path is judged per row', () => { + it('strips only the rows that forged, and reports the batch union once', async () => { + const { engine, creates, logger } = await makeEngine(); + const dropped: DroppedFieldsEvent[] = []; + await engine.insert('duly_task', [ + { title: 'A', completed_at: 'forged' }, + { title: 'B' }, + { title: 'C', locked_note: 'forged' }, + ] as any, { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } } as any); + expect(creates).toHaveLength(3); + expect(creates[0]).not.toHaveProperty('completed_at'); + expect(creates[1].title).toBe('B'); + expect(creates[2]).not.toHaveProperty('locked_note'); + expect(dropped, 'one event per CALL — the listener signature carries no row index').toHaveLength(1); + expect([...dropped[0].fields].sort()).toEqual(['completed_at', 'locked_note']); + expect(logger.lines.filter((l: any) => l.level === 'warn').length).toBeGreaterThan(0); + }); +}); diff --git a/packages/objectql/src/engine-lookup-referential-integrity.test.ts b/packages/objectql/src/engine-lookup-referential-integrity.test.ts index 279fd594a0..044911f056 100644 --- a/packages/objectql/src/engine-lookup-referential-integrity.test.ts +++ b/packages/objectql/src/engine-lookup-referential-integrity.test.ts @@ -309,18 +309,25 @@ describe('[#4441] a lookup id that resolves to nothing is refused', () => { }); it('a READONLY lookup is not the caller\'s to answer for', async () => { - // By construction, not by exemption: `stripReadonlyFields` / - // `stripReadonlyForInsert` remove a non-system caller's value from a - // readonly field before the write, so anything still there was written by - // the PLATFORM — outside this check's stated scope. + // By construction, not by exemption: `stripReadonlyFields` removes a + // non-system caller's value from a readonly field before the write — on + // BOTH write paths since #14147 — so anything still there was written by + // the PLATFORM, outside this check's stated scope. // // The real case that found this: `sys_metadata_history.recorded_by` is a // `lookup('sys_user', { readonly: true })` the metadata repository fills // with `actor ?? 'system'` — a SENTINEL STRING, not a user id — on a write // that carries no `isSystem`. Checking it rejected ordinary metadata // authoring (package create / publish / clone) in the dogfood gate. + // + // [#14147] The fixture is `sys_`-NAMED, as the real object is, and that is + // now load-bearing rather than cosmetic: the create-side static strip + // leaves platform objects to their own field guards (ADR-0086 / #3004), so + // a `sys_` row is exactly where a readonly lookup still HOLDS a + // caller-supplied value when this check runs. The author-object half is + // the sibling case below. engine.registry.registerObject({ - name: 'ref_history', + name: 'sys_ref_history', label: 'History', fields: { id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, @@ -333,11 +340,38 @@ describe('[#4441] a lookup id that resolves to nothing is refused', () => { } as any); const row: any = await engine.insert( - 'ref_history', { note: 'n', recorded_by: 'system' }, { context: userCtx } as any, + 'sys_ref_history', { note: 'n', recorded_by: 'system' }, { context: userCtx } as any, ); expect(row.recorded_by).toBe('system'); }); + it('[#14147] on an AUTHOR object the same value is STRIPPED, so the check never sees it', async () => { + // The same conclusion — "not the caller's to answer for" — reached one step + // EARLIER. Before #14147 a non-system `engine.insert` wrote the readonly + // lookup verbatim and this narrowing was the only thing standing between + // that value and a `reference_not_found` refusal; now the create-side strip + // takes it first, exactly as the update path always did. Both halves are + // pinned because they fail differently: a lost narrowing REJECTS a platform + // write, a lost strip ACCEPTS a forged one. + engine.registry.registerObject({ + name: 'ref_audit_note', + label: 'Audit Note', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + note: { name: 'note', label: 'Note', type: 'text' as const }, + recorded_by: { + name: 'recorded_by', label: 'Recorded By', + type: 'lookup' as const, reference: 'ref_permission_set', readonly: true, + }, + }, + } as any); + + const row: any = await engine.insert( + 'ref_audit_note', { note: 'n', recorded_by: 'ps_does_not_exist' }, { context: userCtx } as any, + ); + expect(row.recorded_by, 'stripped before the reference check, not refused by it').toBeUndefined(); + }); + it('…and the issue\'s own fields are NOT readonly, so they stay enforced', () => { // The narrowing above must not quietly cover the two fields #4441 names. for (const [obj, field] of [ diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 6a73f5a86a..a655d6f78f 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -193,7 +193,7 @@ import { deriveViewContainerObject } from '@objectstack/metadata/view-container' import { bindHooksToEngine } from './hook-binder.js'; import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; -import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields } from './validation/rule-validator.js'; +import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; // [#14088] The before-phase write recorder — the provenance channel the static // `readonly` strip needs to tell a hook's write from a caller's echo of the // SAME value. Armed and sealed in `update()`; the module owns the argument for @@ -6014,7 +6014,9 @@ export class ObjectQL implements IObjectQLEngine { * written by hooks and middleware, not by the request, and re-validating * them here would turn a platform stamp into a caller-facing rejection. * - **Non-system writes only**, like every other write-path guard in this - * engine (`stripReadonlyFields`, `stripReadonlyForInsert`). Seed replay, + * engine (`stripReadonlyFields` — which since #14147 runs on the create + * path as well as the update one — and `stripRuntimeOwnedFields`). Seed + * replay, * package install and boot-time provisioning legitimately write rows in an * order that resolves only once the batch completes; failing them closed * would turn an ordering detail into a boot failure. This leaves a real @@ -6058,8 +6060,9 @@ export class ObjectQL implements IObjectQLEngine { // // This check answers for exactly one thing: "the reference the CALLER // named". `stripReadonlyFields` removes a non-system caller's value from - // a readonly field before the write, and the create ingress does the same - // (`stripReadonlyForInsert`, #3043). So a value still sitting in one at + // a readonly field before the write — on BOTH write paths since #14147 + // moved the create-side strip into this engine and deleted the boundary + // copy it used to live in. So a value still sitting in one at // this point was minted by the PLATFORM — outside this check's own stated // scope, whatever it happens to hold. That argument stands on its own and // depends on no particular field: deleting the `continue` would start @@ -9611,21 +9614,28 @@ export class ObjectQL implements IObjectQLEngine { * (no number-range gaps from a rejected batch). */ // [#3407 / #5126] BOTH members of `WriteObservabilityOptions` are live here, - // for exactly ONE strip: the runtime-owned (`autonumber`) strip added by - // #5503, wired at its strip site below. Each arrived carrying the same + // for TWO strips that share one report site (`insertDropped`, below): the + // runtime-owned (`autonumber`) strip added by #5503, and — since the + // maintainer ruling of 2026-09-03 (option C, #14147) — the AUTHOR-declared + // static-`readonly` strip, the same `stripReadonlyFields` the update path + // runs, under the same `isSystem` gate. Each member arrived carrying the same // standing condition — #3407's "if insert ever gains a silent strip, wire the // listener at that strip site", #5126's "it is inert here only because insert // strips nothing; if insert ever gains a strip, both members wire up together - // at that site". #5503 is that strip, so both are discharged together: - // quiet-and-observable by default (`onFieldsDropped`), refused outright under - // `strictReadonlyWrites` — the same one-per-call choice update offers. + // at that site". #5503 was the first such strip and #14147 the second; both + // discharge through the one site: quiet-and-observable by default + // (`onFieldsDropped`), refused outright under `strictReadonlyWrites` — the + // same one-per-call choice update offers. // - // INSERT remains deliberately exempt from the AUTHOR-declared - // readonly/readonlyWhen strips (a create may legitimately seed read-only - // columns; the #3043 ingress strip covers external callers instead), and the - // FLS write gate throws rather than stripping. So neither member reports on - // those here — only on what this path actually strips. Any FURTHER strip added - // here must wire both members at its own site too. + // What INSERT still does NOT strip: `readonlyWhen` (a conditional lock has no + // prior record to evaluate on a create — the update-path note below says + // "INSERT stays exempt" for exactly that strip), and the FLS write gate throws + // rather than stripping. So neither member reports on those here — only on + // what this path actually strips. The 2026-07-24 row "INSERT (all callers) + // exempt" this note used to rest on is SUPERSEDED, and the metadata-protocol + // ingress copy it pointed external callers at is deleted: there is one + // create-side enforcement point, and it is this one. Any FURTHER strip added + // here must feed `insertDropped` (or wire both members at its own site) too. /** * Validate-only (#6037, #4633 ruling D) — run the write path's own verdict * over candidate rows and report it, WITHOUT persisting anything. @@ -10179,7 +10189,7 @@ export class ObjectQL implements IObjectQLEngine { // hook that RE-ISSUES the record number lost its write to any caller // that had also submitted the key, while the same hook's write survived // on a caller that had not. The update path's twin (#5591). - const autonumberDropped: string[] = []; + const insertDropped: string[] = []; if (!opCtx.context?.isSystem) { const preserveAudit = opCtx.context?.preserveAudit === true; for (let i = 0; i < rows.length; i++) { @@ -10207,11 +10217,96 @@ export class ObjectQL implements IObjectQLEngine { ) as Record; if (stripped === rows[i]) continue; for (const k of Object.keys(rows[i])) { - if (!(k in stripped) && !autonumberDropped.includes(k)) autonumberDropped.push(k); + if (!(k in stripped) && !insertDropped.includes(k)) insertDropped.push(k); } rows[i] = stripped; rowHookContexts[i].input.data = stripped; } + // [#14147] STATIC author-declared `readonly`, enforced HERE — one + // semantics, one enforcement point, per the maintainer ruling of + // 2026-09-03 (option C) which SUPERSEDED the 2026-07-24 row "INSERT + // (all callers) exempt". Until it landed, a non-system caller + // reaching `engine.insert` DIRECTLY wrote a read-only column with no + // refusal, no WARN and no `onFieldsDropped` event, while the very + // same payload through the DataProtocol was stripped — and + // `create_record`'s listener (`@objectstack/service-automation`) was + // wired for a readonly drop it could never receive. The boundary copy + // that produced that asymmetry (`stripReadonlyForInsert`, + // metadata-protocol) is DELETED in the same change rather than kept + // as a second implementation. + // + // The strip is {@link stripReadonlyFields} — the SAME function + // `update` runs, under the SAME `isSystem` gate (the branch above), + // reporting through the SAME channels: `readonlyStripWarning` at + // `warn`, `onFieldsDropped` under reason `readonly`, and + // `strictReadonlyWrites` refusing before any driver dispatch. Its + // guards therefore come across too, and they are wider than the + // deleted ingress copy's: a hook stamp is not caller-supplied + // (`suppliedPerRow`), and a key a `beforeInsert` hook ASSIGNED is the + // hook's write, not a forgery (`rowHookWrittenKeys`, #14259). The + // ingress copy ran BEFORE the hooks and could judge neither. + // + // ⛔ `preserveAudit` is deliberately NOT forwarded — see + // {@link preserveAuditIgnoredOnInsertWarning}: the 2026-08-08 ruling + // narrowed that exemption to the UPDATE path and left `isSystem` as + // the create side's only one. Ruling C moved WHERE this strip runs; + // it did not widen WHAT exempts it. + // + // WHICH fields it may judge is {@link staticReadonlyInsertSubject}'s + // (runtime-owned types belong to the pass above, platform objects to + // their own 403 guards); `null` — no such field on this object — is + // the cheap exit every ordinary insert takes. + const readonlySubject = staticReadonlyInsertSubject(schemaForValidation as any); + if (readonlySubject) { + const preserveAuditIgnored: string[] = []; + for (let i = 0; i < rows.length; i++) { + if (rowErrors[i] !== undefined) continue; + const stripped = stripReadonlyFields( + readonlySubject as any, rows[i], suppliedPerRow[i] ?? {}, this.logger, + { + strictReadonlyWrites: options?.strictReadonlyWrites === true, + hookWrittenKeys: rowHookWrittenKeys[i], + verb: 'insert', + }, + ) as Record; + if (stripped === rows[i]) continue; + const takenFromRow: string[] = []; + for (const k of Object.keys(rows[i])) { + if (k in stripped) continue; + takenFromRow.push(k); + if (!insertDropped.includes(k)) insertDropped.push(k); + if (preserveAudit && !preserveAuditIgnored.includes(k)) preserveAuditIgnored.push(k); + } + // The field's `defaultValue` is RE-DERIVED for every key this + // pass took, which is #3043's stated contract and a guarantee in + // its own right: a forged `approval_status` becomes `draft` — the + // enforced initial state — never NULL, so a stripped forgery + // cannot leave a row in a state the object's own rules + // (`requiredWhen`, the state machine) were written to exclude. + // The deleted ingress copy got this for free by running BEFORE + // `applyFieldDefaults`; a strip that runs after the hooks has to + // ask. Asked over the STRIPPED row, so a `defaultValue` + // expression reads the payload it will really be stored beside, + // and copied back key by key: `applyFieldDefaults` also fills + // every OTHER absent field, and a hook that deliberately wrote + // `null` must keep its null (the first defaults pass, ahead of + // the hooks, is the one that owns those keys). + if (takenFromRow.length > 0) { + const redefaulted = this.applyFieldDefaults(object, stripped, opCtx.context, nowSnap); + for (const k of takenFromRow) { + if (redefaulted[k] !== undefined) stripped[k] = redefaulted[k]; + } + } + rows[i] = stripped; + rowHookContexts[i].input.data = stripped; + } + // One line per CALL, not per row, and only when the exemption was + // ASKED FOR and something was actually removed — the union is + // faithful because the strip is schema-uniform. + if (preserveAuditIgnored.length > 0) { + this.logger.warn(preserveAuditIgnoredOnInsertWarning(object, preserveAuditIgnored)); + } + } } // [#3407 / #5126] This is the strip site both standing notes on // `insert()` pointed at, so both members of `WriteObservabilityOptions` @@ -10226,14 +10321,14 @@ export class ObjectQL implements IObjectQLEngine { // an implicitly read-only field is dropped for exactly the reason a // declared one is, and inventing a parallel reason code would fork the // vocabulary (`packages/spec`) for a distinction no consumer acts on. - if (autonumberDropped.length > 0) { - const drop: DroppedFieldsEvent = { object, fields: autonumberDropped, reason: 'readonly' }; + if (insertDropped.length > 0) { + const drop: DroppedFieldsEvent = { object, fields: insertDropped, reason: 'readonly' }; if (options?.strictReadonlyWrites === true) { // Before the driver write and before validation — nothing is // written, and "you sent a runtime-owned field" should not depend on // whether some other field also failed a business rule (#5126's // ordering on the update path, mirrored). - throw new ReadonlyFieldRejectedError(object, autonumberDropped, [drop], 'insert'); + throw new ReadonlyFieldRejectedError(object, insertDropped, [drop], 'insert'); } if (typeof options?.onFieldsDropped === 'function') { // Under strict the listener deliberately does NOT fire (above): diff --git a/packages/objectql/src/integrity/dangling-reference-audit.ts b/packages/objectql/src/integrity/dangling-reference-audit.ts index 5c389d8173..02e6d09cdc 100644 --- a/packages/objectql/src/integrity/dangling-reference-audit.ts +++ b/packages/objectql/src/integrity/dangling-reference-audit.ts @@ -77,7 +77,7 @@ import { PLATFORM_OBJECTS_BY_PACKAGE } from '@objectstack/spec/system'; * * `readonly` reference fields used to be skipped outright, on two grounds. The * first is #4441's and still holds: a non-system caller's value is stripped - * before the write (`stripReadonlyFields` / `stripReadonlyForInsert`), so what + * before the write (`stripReadonlyFields`, on both write paths since #14147), so what * survives was minted by the platform and was never the caller's to answer for. * The second was the `recorded_by` sentinel above — the platform wrote a * NON-ID into a reference column, and probing it could only ever produce a diff --git a/packages/objectql/src/validation/rule-validator.test.ts b/packages/objectql/src/validation/rule-validator.test.ts index 60a4050a50..4a1cd7be4c 100644 --- a/packages/objectql/src/validation/rule-validator.test.ts +++ b/packages/objectql/src/validation/rule-validator.test.ts @@ -1351,11 +1351,17 @@ describe('stripRuntimeOwnedFields — the INSERT-side strip (#5503)', () => { expect(out).toEqual({ title: 'x' }); }); - it('leaves AUTHOR-declared readonly fields alone — insert keeps its #3413 exemption', () => { - // The engine is deliberately NOT the place the static-`readonly` insert - // strip lives (that is the #3043 protocol ingress); this narrower helper - // must not quietly take over that job and start stripping columns the - // trusted internal writers legitimately seed on create. + it('leaves AUTHOR-declared readonly fields alone — that strip is a SEPARATE pass, never this helper’s', () => { + // [#14147] This case used to say "insert keeps its #3413 exemption" and + // that the engine was "deliberately NOT the place the static-`readonly` + // insert strip lives (that is the #3043 protocol ingress)". The maintainer + // ruling of 2026-09-03 (option C) superseded that: the static strip now + // runs inside `engine.insert` — but as `stripReadonlyFields` over + // `staticReadonlyInsertSubject`, a second pass with its own gate and its + // own report. The verdict here is unchanged and still load-bearing: this + // narrower runtime-owned helper must not quietly take over that job, or + // `preserveAudit` (which this helper honours and the static pass does not) + // would start reinstating author-declared columns it was never ruled to. const supplied = { title: 'x', closed_at: '2021-01-01T00:00:00Z' }; const out = stripRuntimeOwnedFields(numberedFields, { ...supplied }, supplied); expect(out).toEqual({ title: 'x', closed_at: '2021-01-01T00:00:00Z' }); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 7c03fe5491..a277ff1a8a 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -997,11 +997,15 @@ export function stripReadonlyWhenFieldsMulti( * * The set itself now lives in `@objectstack/spec` (`RUNTIME_OWNED_FIELD_TYPES`, * #5628) — the protocol's one statement of the ownership — because a SECOND - * consumer needs it: the DataProtocol create ingress, whose `readonly` strip - * carries a NARROWER exemption set than this module's (no `preserveAudit`), and - * which therefore has to recognise these types to stay out of their way. A - * literal copied over there is the drift `AUDIT_TIMELINE_FIELDS` below stopped - * paying for. This module keeps the reasoning; the membership is imported. + * consumer needs it: the create-side static-`readonly` strip, whose exemption + * set is NARROWER than this helper's (no `preserveAudit`) and which therefore + * has to recognise these types to stay out of their way. When #5628 landed + * that consumer was the DataProtocol create ingress in another package; since + * the maintainer ruling of 2026-09-03 (option C, #14147) it is + * {@link staticReadonlyInsertSubject} in this module, feeding `engine.insert`, + * and the ingress copy is deleted. A literal copied over there was the drift + * `AUDIT_TIMELINE_FIELDS` below stopped paying for; the membership stays + * imported from the spec. This module keeps the reasoning. */ /** @@ -1223,6 +1227,13 @@ export function stripReadonlyFields( * proof, because a key in this set is a key this strip stops defending. */ hookWrittenKeys?: ReadonlySet; + /** + * Which write path is calling. Omitted means `'update'`, so every + * pre-existing call site logs byte-identical text; `engine.insert` passes + * `'insert'` so the line describes a create and drops the `preserveAudit` + * remedy, which is UPDATE-only. + */ + verb?: 'update' | 'insert'; }, ): Record | undefined | null { const fields = objectSchema?.fields; @@ -1287,6 +1298,7 @@ export function stripReadonlyFields( const warnOptions: StripWarningOptions = { strict, preserveAuditApplies: isPreservableUnderAudit(name, def), + ...(options?.verb !== undefined ? { verb: options.verb } : {}), }; logger?.warn?.( def?.readonly @@ -1302,12 +1314,17 @@ export function stripReadonlyFields( * isRuntimeOwnedField}) only — the INSERT-side counterpart of * {@link stripReadonlyFields} (#5503). * - * Why a separate, narrower function rather than reusing the one above: INSERT is - * deliberately exempt from the author-declared static-`readonly` strip inside - * the engine (#3413). A create may legitimately seed read-only columns, and the - * trusted internal writers (identity provisioning, the metadata repository, the - * event-log cursor) call `engine.insert` DIRECTLY — which is why that strip - * lives at the DataProtocol ingress instead (`stripReadonlyForInsert`, #3043). + * Why a separate, narrower function rather than reusing the one above. It was + * originally because INSERT was exempt from the author-declared static-`readonly` + * strip inside the engine — that exemption is GONE (ruling C, 2026-09-03), and + * `engine.insert` now runs {@link stripReadonlyFields} over + * {@link staticReadonlyInsertSubject} for a non-system caller. What keeps this + * function separate is what it always also did: it owns the runtime-owned types + * under a WIDER `preserveAudit` exemption than the create side grants an + * author-declared column, and its message states the runtime-owned reason + * rather than an author-declared lock — the spec injects `readonly: true` onto + * every `autonumber`, so the two would otherwise be indistinguishable to the + * reader of a log line. * Runtime-owned fields carry none of that ambiguity: nobody may seed a record * number on create, because the engine (or the driver's persistent sequence) * issues it. So this one CAN live in the engine, and living there is the point — @@ -1455,6 +1472,138 @@ export function stripRuntimeOwnedFields( return result; } +/** + * The INSERT-side subject of {@link stripReadonlyFields}: the schema + * VIEW an `engine.insert` static-`readonly` pass may judge, or `null` when the + * object has nothing for it to judge (the common case, and the cheap exit). + * + * ## Why the insert side needs a view at all + * + * The maintainer ruling of 2026-09-03 (option C) made a static `readonly` field + * enforced IN THE ENGINE on INSERT for non-system callers, exactly as on UPDATE, + * and superseded the 2026-07-24 row "INSERT (all callers) exempt". The + * enforcement is {@link stripReadonlyFields} itself — one implementation, one + * semantics — and the boundary copy that used to run at the DataProtocol ingress + * (`stripReadonlyForInsert`, metadata-protocol) is deleted rather than kept as a + * second one. + * + * What does NOT come across is that copy's job of deciding WHICH fields the + * create-side strip owns, and those two exclusions are load-bearing: + * + * - **Runtime-owned types** ({@link isRuntimeOwnedField}) are already stripped + * on this path by {@link stripRuntimeOwnedFields}, under the WIDER + * `preserveAudit` exemption a historical import needs to reinstate its legacy + * record numbers. Re-judging them here — without that flag, see + * below — would delete exactly what the pass before it legitimately kept. The + * warning would also be wrong: the spec injects `readonly: true` onto every + * `autonumber`, so `stripReadonlyFields` would report an author-declared lock + * where `runtimeOwnedStripWarning` states the true, actionable reason. + * + * - **Platform objects** (`managedBy` set, or the reserved `sys_` namespace) + * carry their OWN field-write governance that a silent strip must not + * pre-empt (ADR-0086): a forged `managed_by: 'package'` or + * `package_id` on `sys_permission_set` is REFUSED with a 403, and several of + * those columns are `readonly`, so stripping them would silently swallow the + * payload the guard exists to reject. That boundary is the deleted copy's + * (`applySystemFields` draws the same platform-vs-authored line) and it was + * ruled on its own merits, NOT on the "INSERT is exempt" row that ruling C + * superseded — so it is carried over rather than dropped in passing. The + * threat this strip answers is an app's approval/status/verdict column, never + * `sys_`. + * + * ⚠️ The UPDATE path applies NEITHER exclusion, deliberately: it has no + * runtime-owned sibling pass, and its own platform-object posture predates both + * rulings. This asymmetry is the create side's, and it is stated here rather + * than re-derived at the call site. + */ +export function staticReadonlyInsertSubject( + objectSchema: { name?: string; managedBy?: unknown; fields?: Record } | undefined | null, +): { name?: string; fields: Record } | null { + const fields = objectSchema?.fields; + if (!fields) return null; + if (objectSchema?.managedBy) return null; + if (String(objectSchema?.name ?? '').startsWith('sys_')) return null; + const subject: Record = {}; + let any = false; + for (const [name, def] of Object.entries(fields)) { + if (!def?.readonly) continue; + if (isRuntimeOwnedField(def)) continue; + subject[name] = def; + any = true; + } + if (!any) return null; + return { name: objectSchema?.name, fields: subject }; +} + +/** + * THE loud half of the `preserveAudit` ruling — a + * non-system INSERT that asks for the historical-import exemption is TOLD it + * does not exist on this path. + * + * ## Why the create side refuses an exemption the update side grants + * + * `FieldSchema.readonly`'s `.describe()` promised the `preserveAudit` exemption + * on BOTH write paths, and `docs/protocol/objectql/security.mdx` agreed. + * Only UPDATE ever implemented it: {@link stripReadonlyFields} consults + * {@link isPreservableUnderAudit} when the caller passes the flag, while the + * create-side strip has never read `preserveAudit` at all — `isSystem` is its + * only exemption. REST import's `treatAsHistorical` puts `preserveAudit: true` + * on the write context and creates through that seam, so ONE historical import + * PRESERVED an author-declared `readonly` business column (`closed_at`, + * `resolved_by`) on the rows it updated and SILENTLY DROPPED it on the rows it + * created. + * + * **Maintainer ruling, 2026-08-08 (option 2):** the ENFORCEMENT is the truth and + * the contract was narrowed to it — the exemption is UPDATE-only, and the create + * side keeps honouring `isSystem` alone. Honouring `preserveAudit` here instead + * would hand a NON-system caller (`treatAsHistorical` arrives on an ordinary + * REST import request) the ability to seed the approval/status columns the strip + * exists to protect, in one POST — the create-side threat model reversed, for a + * capability with no measured consumer. Replaying archival readonly facts on + * INSERT is available today from a system context. + * + * ⚠️ **That ruling is untouched by the 2026-09-03 ruling** which moved this + * strip into the engine: ruling C changed WHERE the create-side static strip + * runs, not WHAT exempts it. So `engine.insert` calls {@link + * stripReadonlyFields} WITHOUT `preserveAudit` over + * {@link staticReadonlyInsertSubject}, and emits this line when the flag was + * requested and something was actually removed. + * + * ## Why it is a WARNING and not a throw — measured, not assumed + * + * The ruling made loudness binding and left the SHAPE to whichever one can be + * both loud and non-breaking. A throw cannot: `runImport`'s per-row writer + * collects a write error into `toFailedResult(rowNo, res.error)` rather than + * aborting the run, so refusing here would not stop a historical import — it + * would convert every row it CREATES into a failed row while the rows it updates + * still succeed. And the trigger is not exotic: the audit family itself + * (`created_at` / `created_by` / `updated_at` / `updated_by`) is `readonly: true` + * in the registry's `AUDIT_FIELD_DEFS`, so an ordinary export→historical-import + * round-trip carries readonly columns on every row. Measured when the ruling + * landed: a throwing variant took the historical import of 2 new rows from + * `{created: 2, errors: 0}` to `{created: 0, errors: 2}`. + * + * The silence this replaces was specific: the drop itself already surfaces + * through the dropped-field channel, but a caller who EXPLICITLY asked for the + * exemption could not tell "your fields were stripped by the ordinary rule" from + * "the exemption you requested does not exist on this path". This says the + * second one, by name. It fires ONLY when `preserveAudit` was requested AND + * something was actually removed — a request that loses nothing has nothing to + * report — and one line per CALL, not per row: the strip is schema-uniform, so + * the union of what the batch lost is the faithful signal (the same aggregation + * `onFieldsDropped` already applies). + */ +export function preserveAuditIgnoredOnInsertWarning(object: string, fields: readonly string[]): string { + return ( + `preserveAudit is UPDATE-only and was IGNORED on this INSERT` + + `${object ? ` (object '${object}')` : ''}: the historical-import exemption applies when a ` + + `record is UPDATED, never when it is created, so the readonly field(s) ${fields.join(', ')} were ` + + `STRIPPED from this create rather than preserved. To replay archival readonly facts on INSERT, ` + + `write from a system context (\`context.isSystem\`) — a non-system create may not seed a readonly ` + + `column.` + ); +} + /** * What the strip knows about the write at the moment it composes a line (#8214). * @@ -1495,6 +1644,21 @@ export interface StripWarningOptions { * the other would falsify this line; keep the two fed from one fact. */ readonly strict?: boolean; + /** + * Which write this line is about. Default `'update'`, which is what + * every pre-existing caller means and what keeps their bytes identical. + * + * It exists because ruling C put the static strip on the CREATE path too, and + * three of this message's sentences were update-shaped: "the update is being + * COMMITTED WITHOUT IT", "a beforeUpdate hook does NOT need this", and the + * `preserveAudit` remedy — which on a create is not merely mis-worded but + * FALSE, the exemption being UPDATE-only by the 2026-08-08 ruling. Offering a + * remedy that would not have worked is the defect already removed once from this very + * message (it told a caller to pass `isSystem: true`, a strictly worse posture, + * to silence a line that should never have printed). Rather than repeat it, the + * create path says `verb: 'insert'` and this message drops the sentence. + */ + readonly verb?: 'update' | 'insert'; /** * `{ context: { preserveAudit: true } }` (#3493) would have KEPT this exact * field — so naming it is a remedy the reader can act on. @@ -1521,6 +1685,12 @@ export interface StripWarningOptions { * have kept the field this line names. See {@link StripWarningOptions}. */ function preserveAuditRemedySentence(options?: StripWarningOptions): string { + // ⛔ Never on a create: `preserveAudit` is an UPDATE-path exemption + // (maintainer, 2026-08-08), so on the insert path this whole sentence would + // advertise a remedy that cannot work. The create side's own line — + // `preserveAuditIgnoredOnInsertWarning` — says the opposite, by name, and + // only to the caller who actually asked for the exemption. + if (options?.verb === 'insert') return ''; if (options?.preserveAuditApplies !== true) return ''; return ( ` A historical import restoring this record's own earlier values does NOT need that blanket ` + @@ -1633,22 +1803,28 @@ export function readonlyStripWarning( options?: StripWarningOptions, ): string { const on = object ? ` on '${object}'` : ''; + const insert = options?.verb === 'insert'; + const noun = insert ? 'create' : 'update'; const consequence = options?.strict === true - ? `the caller-supplied value was DROPPED and the update is being REFUSED ENTIRELY — this ` + + ? `the caller-supplied value was DROPPED and the ${noun} is being REFUSED ENTIRELY — this ` + `write passed options.strictReadonlyWrites, so NOTHING is written: not this column, and ` + `not the fields that would have survived the strip. The call throws ` + `ERR_READONLY_FIELD_REJECTED rather than returning success (#5126).` - : `the caller-supplied value was DROPPED and the update ` + - `is being COMMITTED WITHOUT IT — the call returns success while this column keeps its stored ` + - `value (#2948).`; + : insert + ? `the caller-supplied value was DROPPED and the create ` + + `is being COMMITTED WITHOUT IT — the call returns success while this column takes its ` + + `declared defaultValue instead of the value you sent.` + : `the caller-supplied value was DROPPED and the update ` + + `is being COMMITTED WITHOUT IT — the call returns success while this column keeps its stored ` + + `value (#2948).`; return ( `Field '${field}'${on} is read-only: ` + consequence + ` Server-side code that legitimately writes read-only columns (a plugin, a cron / ` + `background job persisting a system-computed value) must declare itself trusted by passing ` + - `{ context: { isSystem: true } } on the write; a beforeUpdate hook does NOT need this because ` + - `hook-written keys are not caller-supplied.` + + `{ context: { isSystem: true } } on the write; a ${insert ? 'beforeInsert' : 'beforeUpdate'} ` + + `hook does NOT need this because hook-written keys are not caller-supplied.` + preserveAuditRemedySentence(options) + observeInsteadSentence(options) + ` Forged read-only keys from untrusted client ` + diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index cd694fb00b..2c82d52f44 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -278,10 +278,10 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ note: 'ADR-0101. Unlike the raw pre-ADR-0101 bridge (which fed the long-lived server the RAW metadata service + data engine with no ExecutionContext), record_by_id now reads only under a principal resolved from OS_MCP_STDIO_API_KEY, re-resolved per call so a revoked/expired key stops working live. Unit-proven in @objectstack/mcp plugin.test.ts (fail-closed: no key / no objectql / unresolvable key each refuse to start) + mcp-server-runtime.test.ts (record_by_id registered only with a principal-bound reader); the scoped read shares the exact ql.find({context}) enforcement path proven end-to-end by mcp-http-identity (showcase-mcp-http-identity.dogfood.test.ts) and the RLS fixtures. Dropping the principal binding (the resolveStdioExecutionContext threading) makes the stdio-principal-bound key STALE → red CI. NOT high-risk: driving a real stdio transport in-process is impractical, but the reader path is the same RLS-applying engine call the HTTP e2e already exercises.' }, { id: 'default-profile', summary: 'app-declared default profile (isDefault)', state: 'enforced', enforcement: 'plugin-security/security-plugin.ts fallback resolution', proof: 'showcase-default-profile.dogfood.test.ts' }, - { id: 'readonly-static-write', summary: 'static `readonly: true` stripped from non-system UPDATE (#2948 / #3003) AND INSERT (#3043) payloads — neither a direct PATCH nor a direct POST can forge approval/status/amount columns the UI never renders', state: 'enforced', - enforcement: 'UPDATE: objectql/engine.ts stripReadonlyFields on the single-id + multi-row paths (#2948, caller-supplied VALUES only — the entry snapshot carries the caller payload, so a server stamp survives whether the hook ADDED the key or OVERWROTE one the caller also sent, #5591). INSERT: metadata-protocol/protocol.ts strips read-only keys at the DataProtocol create INGRESS (createData / createManyData / batchData / cloneData) — the single seam every external REST/GraphQL/MCP create funnels through, while trusted internal engine.insert writers (better-auth adapter, metadata repo, seed loader) bypass it; stripped before the engine so the field re-derives its defaultValue. isSystem exempt on both; symmetric with the readonlyWhen strip', + { id: 'readonly-static-write', summary: 'static `readonly: true` stripped from non-system UPDATE (#2948 / #3003) AND INSERT (#3043 at the ingress; in-engine for every caller since the 2026-09-03 ruling) payloads — neither a direct PATCH nor a direct POST can forge approval/status/amount columns the UI never renders', state: 'enforced', + enforcement: 'UPDATE: objectql/engine.ts stripReadonlyFields on the single-id + multi-row paths (#2948, caller-supplied VALUES only — the entry snapshot carries the caller payload, so a server stamp survives whether the hook ADDED the key or OVERWROTE one the caller also sent, #5591). INSERT: objectql/engine.ts runs the SAME stripReadonlyFields inside engine.insert (maintainer ruling 2026-09-03, option C — one semantics, one enforcement point; the metadata-protocol ingress copy that used to cover only the DataProtocol faces while a direct engine.insert caller bypassed it is deleted), after the beforeInsert hooks and before validation, over staticReadonlyInsertSubject (runtime-owned types keep their own pass, platform objects their own 403 guards); the field re-derives its defaultValue. isSystem exempt on both. An internal writer that seeds a readonly column on create does so by one of exactly two mechanisms: (1) a system context — identity provisioning (plugin-auth objectql-adapter.ts wraps its engine in withSystemContext); (2) the platform-object carve-out in staticReadonlyInsertSubject (rule-validator.ts) — a sys_-prefixed or managedBy object is outside the strip subject, so the metadata repository (sys-metadata-repository.ts) seeds sys_metadata_history provenance (recorded_by, readonly lookup) and its event-log cursor (event_seq, readonly number) under the CALLER context, and those columns are policed by the object guards of that platform object rather than swallowed by the strip — as is every other platform-object seeder; symmetric with the readonlyWhen strip', proof: 'showcase-static-readonly.dogfood.test.ts', - note: 'The #3003 field report: `readonly: true` used to be UI-only, so a logged-in non-admin self-approved a 4-stage approval (approval_status/approval_stage/confirmed_total) with one same-session REST PATCH on a draft record — RECORD_LOCKED only guards pending flows, and the draft never entered one. #3043 is the INSERT face: the same non-admin could skip the draft entirely and POST a record already `approval_status:"approved"` — a step SHORTER than #3003, and one the UPDATE strip never reached. Enforced at the DATA-WRITE INGRESS (not the engine) so it covers every external caller — REST, the GraphQL/MCP dispatcher, bulk import — without stripping the internal writers that legitimately seed readonly columns on create (identity provisioning, provenance, event-log cursors). The strip is SILENT on both paths (HTTP 2xx, forged value dropped; a stripped INSERT field falls back to its defaultValue). `readonlyWhen` stays INSERT-exempt (a conditional lock needs a prior record). System-context writes (import, seed replay, migration) still seed readonly columns. Ingress unit proof in metadata-protocol protocol.readonly-insert.test.ts (forge stripped, default re-seeded, system context allowed, batch rows covered, internal engine.insert unaffected).' }, + note: 'The #3003 field report: `readonly: true` used to be UI-only, so a logged-in non-admin self-approved a 4-stage approval (approval_status/approval_stage/confirmed_total) with one same-session REST PATCH on a draft record — RECORD_LOCKED only guards pending flows, and the draft never entered one. #3043 is the INSERT face: the same non-admin could skip the draft entirely and POST a record already `approval_status:"approved"` — a step SHORTER than #3003, and one the UPDATE strip never reached. It was first enforced at the DATA-WRITE INGRESS (not the engine) so it covered every external caller — REST, the GraphQL/MCP dispatcher, bulk import — without stripping internal writers; the maintainer ruling of 2026-09-03 (option C) moved it INTO the engine, so a direct engine.insert caller is covered too. A writer that seeds a readonly column on create does so under a system context (identity provisioning) or, for a platform object (sys_-prefixed or managedBy), under the carve-out in staticReadonlyInsertSubject that leaves such objects to their own guards — the metadata repository writing sys_metadata_history.recorded_by provenance and its event_seq event-log cursor under the caller context is that second mechanism, not a system context. The strip is SILENT on both paths (HTTP 2xx, forged value dropped; a stripped INSERT field falls back to its defaultValue). `readonlyWhen` stays INSERT-exempt (a conditional lock needs a prior record). System-context writes (import, seed replay, migration) still seed readonly columns. Engine proof in objectql engine-insert-static-readonly-strip.test.ts (forge stripped against a real ObjectQL, default re-seeded, system context allowed, hook stamps survive, strict refuses); delegation proof in metadata-protocol protocol.readonly-insert.test.ts (every create face forwards whole and surfaces the engine verdict as droppedFields where its contract carries one).' }, // ── ADR-0057 — ERP authorization core (enforced + e2e proven) ────────── { id: 'scope-depth', summary: 'permission-grant access DEPTH (own/own_and_reports/unit/unit_and_below/org)', state: 'enforced', diff --git a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts index 77645d39cf..b67bba3b79 100644 --- a/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts @@ -18,13 +18,15 @@ // used to be EXEMPT, so the same non-admin could skip the draft entirely and // POST a record already `approval_status:'approved'` — a step SHORTER than // #3003, and one the UPDATE strip never reached. It is now closed on both -// paths: UPDATE in the engine (`stripReadonlyFields`, objectql/engine.ts, -// #2948) and INSERT at the DataProtocol create INGRESS -// (metadata-protocol/protocol.ts `stripReadonlyForInsert`, #3043 — the single -// seam every external REST/GraphQL/MCP create funnels through, while trusted -// internal engine.insert writers are untouched). On a non-system INSERT or -// UPDATE, caller-supplied writes to statically-readonly fields are silently -// dropped (HTTP 2xx; a stripped INSERT field falls back to its `defaultValue`). +// paths, at ONE enforcement point: `stripReadonlyFields` in objectql/engine.ts, +// on UPDATE since #2948 and on INSERT since the maintainer ruling of 2026-09-03 +// (option C, #14147). #3043 first closed the INSERT face at the DataProtocol +// create INGRESS (`stripReadonlyForInsert`, metadata-protocol) — the seam every +// external REST/GraphQL/MCP create funnels through, while a direct +// engine.insert caller was untouched; that copy is deleted and the engine +// strips every non-system caller now. On a non-system INSERT or UPDATE, +// caller-supplied writes to statically-readonly fields are silently dropped +// (HTTP 2xx; a stripped INSERT field falls back to its `defaultValue`). // System-context writes (import, seed replay, migration) stay exempt, as does // `readonlyWhen` on INSERT (a conditional lock needs a prior record). // diff --git a/packages/rest/src/import-runner-historical-readonly-insert.test.ts b/packages/rest/src/import-runner-historical-readonly-insert.test.ts index 088b1f1a2e..7faa6dbbb0 100644 --- a/packages/rest/src/import-runner-historical-readonly-insert.test.ts +++ b/packages/rest/src/import-runner-historical-readonly-insert.test.ts @@ -1,37 +1,42 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * #6640 — the historical import, end to end, through the REAL DataProtocol - * ingress. + * #6640 — the historical import, end to end, through the REAL write path. * * `runImport` with `treatAsHistorical: true` puts `preserveAudit: true` on the * write context (pinned next door in `import-runner-historical.test.ts`) and - * then creates through `p.createData`. That call lands in - * `stripReadonlyForInsert` — the entry that has never read `preserveAudit` — - * so an author-declared `readonly` business column survived on the rows the - * import UPDATED and vanished from the rows it CREATED. One import, two - * answers. + * then creates through `p.createData`. The create-side static-`readonly` strip + * has never read `preserveAudit`, so an author-declared `readonly` business + * column survived on the rows the import UPDATED and vanished from the rows it + * CREATED. One import, two answers — and the 2026-08-08 ruling kept the + * enforcement and narrowed the contract to it, with the ignored request made + * loud. * - * This file exists because every pre-existing `preserveAudit` pin drives - * `engine.insert` directly and therefore cannot see the ingress at all; the - * ruling's test note makes going through the real entry binding. So the - * protocol here is a real `ObjectStackProtocolImplementation` over a mock - * ENGINE, not a mock protocol: the runner, the ingress strip and the loud - * signal are all the shipped code, and only the storage below them is faked. + * [#14147] WHERE that strip lives moved — from the DataProtocol ingress into + * `engine.insert` (maintainer ruling, 2026-09-03) — and WHAT it does did not. + * So this file's harness moved with it: the engine below the protocol is a REAL + * `ObjectQL` over a recording driver, not a mock that records payloads. That is + * not incidental. A mock engine cannot strip, so under the new architecture the + * old harness would have reported the historical column landing on a create and + * called it green — the same class of blind spot the ruling's own test note was + * written against ("every pre-existing `preserveAudit` pin drives + * `engine.insert` directly and therefore cannot see the ingress at all"). * * What it pins is the ruling's landing, not a wish: the create-side strip is * UNCHANGED (`preserveAudit` is an UPDATE-path exemption), the import still - * SUCCEEDS, and the request that was silently ignored is now reported. + * SUCCEEDS, and the request that was silently ignored is reported by name. */ import { describe, it, expect, vi, afterEach } from 'vitest'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { ObjectQL } from '@objectstack/objectql'; import { runImport, type ImportProtocolLike } from './import-runner'; import type { ExportFieldMeta } from './export-format.js'; const TICKET = { name: 'ticket', fields: { + id: { name: 'id', type: 'text', primaryKey: true }, subject: { name: 'subject', type: 'text' }, // The author-declared business `readonly` column the issue names. closed_at: { name: 'closed_at', type: 'datetime', readonly: true }, @@ -43,19 +48,54 @@ const metaMap = new Map([ ['closed_at', { name: 'closed_at', type: 'datetime' }], ]); -/** A real protocol over a mock engine — everything above the store is shipped code. */ -function makeRealProtocol() { - const inserted: any[] = []; - const engine = { - registry: { getObject: (n: string) => (n === 'ticket' ? TICKET : undefined) }, - insert: vi.fn(async (_object: string, data: any) => { - const rows = Array.isArray(data) ? data : [data]; - const out = rows.map((r, i) => { const rec = { id: `t-${inserted.length + i + 1}`, ...r }; return rec; }); - inserted.push(...out); - return Array.isArray(data) ? out : out[0]; - }), - find: vi.fn(async () => []), +/** Captures the engine's own WARN channel — where the loud half now prints. */ +function makeCapturingLogger() { + const lines: string[] = []; + const logger: any = { + lines, + trace() {}, fatal() {}, debug() {}, info() {}, + warn(msg: string) { lines.push(String(msg)); }, + error() {}, + child() { return logger; }, + }; + return logger; +} + +/** Records the rows that actually reach the store — nothing above it is faked. */ +function makeRecordingDriver(inserted: any[]) { + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(_o: string, data: Record) { + const rec = { id: `t-${inserted.length + 1}`, ...data }; + inserted.push(rec); + return rec; + }, + async update(_o: string, id: string, data: Record) { return { id, ...data }; }, + async updateMany() { return 0; }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 0; }, + async bulkCreate(o: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(o, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, }; + return driver; +} + +/** A real protocol over a REAL engine — everything above the store is shipped code. */ +async function makeRealProtocol() { + const inserted: any[] = []; + const logger = makeCapturingLogger(); + const engine = new ObjectQL({ logger }); + engine.registerDriver(makeRecordingDriver(inserted), true); + await engine.init(); + engine.registry.registerObject(TICKET as any, 'test'); const impl = new ObjectStackProtocolImplementation(engine as any); // `runImport` needs find/create only for an insert-mode run; delegate both to // the real implementation so the ingress is genuinely on the path. @@ -64,7 +104,7 @@ function makeRealProtocol() { createData: (args: any) => impl.createData(args as any) as any, updateData: (args: any) => impl.updateData(args as any) as any, }; - return { p, inserted }; + return { p, inserted, logger }; } const baseOpts = { @@ -88,8 +128,7 @@ describe('runImport (treatAsHistorical) → the REAL insert ingress (#6640)', () afterEach(() => { vi.restoreAllMocks(); }); it('still CREATES every row — the loud signal replaces the silence, not the flow', async () => { - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const { p, inserted } = makeRealProtocol(); + const { p, inserted, logger } = await makeRealProtocol(); const summary = await runImport({ ...baseOpts, p, rows: ROWS, treatAsHistorical: true }); @@ -108,7 +147,7 @@ describe('runImport (treatAsHistorical) → the REAL insert ingress (#6640)', () } // …and the ignored request is now reported, by name and with the rule. - const messages = warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('preserveAudit')); + const messages = logger.lines.filter((m: string) => m.includes('preserveAudit is UPDATE-only')); expect(messages.length, 'the historical create path emits the signal').toBeGreaterThan(0); expect(messages[0]).toContain('preserveAudit is UPDATE-only and was IGNORED on this INSERT'); expect(messages[0]).toContain('closed_at'); @@ -116,8 +155,7 @@ describe('runImport (treatAsHistorical) → the REAL insert ingress (#6640)', () }); it('a NON-historical import of the same rows is stripped just as quietly as before (#3043)', async () => { - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const { p, inserted } = makeRealProtocol(); + const { p, inserted, logger } = await makeRealProtocol(); const summary = await runImport({ ...baseOpts, p, rows: ROWS, treatAsHistorical: false }); @@ -125,12 +163,11 @@ describe('runImport (treatAsHistorical) → the REAL insert ingress (#6640)', () for (const rec of inserted) expect(rec).not.toHaveProperty('closed_at'); // No exemption was requested, so there is no ignored request to report — // the new signal is specific to the contradiction, not to the strip. - expect(warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('preserveAudit'))).toHaveLength(0); + expect(logger.lines.filter((m: string) => m.includes('preserveAudit is UPDATE-only'))).toHaveLength(0); }); it('the same import from a SYSTEM context replays the archival value — the documented remedy', async () => { - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const { p, inserted } = makeRealProtocol(); + const { p, inserted, logger } = await makeRealProtocol(); const summary = await runImport({ ...baseOpts, p, rows: ROWS, treatAsHistorical: true, context: { isSystem: true }, @@ -144,6 +181,6 @@ describe('runImport (treatAsHistorical) → the REAL insert ingress (#6640)', () expect(inserted[0].closed_at, 'system context is how archival readonly facts reach a create') .toBeDefined(); expect(new Date(inserted[0].closed_at).toISOString()).toBe(new Date('2019-04-01T00:00:00Z').toISOString()); - expect(warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('preserveAudit'))).toHaveLength(0); + expect(logger.lines.filter((m: string) => m.includes('preserveAudit is UPDATE-only'))).toHaveLength(0); }); }); diff --git a/packages/rest/src/rest-dropped-fields.test.ts b/packages/rest/src/rest-dropped-fields.test.ts index 081669fac9..219b8466ea 100644 --- a/packages/rest/src/rest-dropped-fields.test.ts +++ b/packages/rest/src/rest-dropped-fields.test.ts @@ -94,7 +94,7 @@ describe('PATCH /data/:object/:id — X-ObjectStack-Dropped-Fields (#3431)', () }); describe('POST /data/:object — X-ObjectStack-Dropped-Fields on create (#3431)', () => { - it('sets the header (status still 201) when the create ingress stripped a readonly field', async () => { + it('sets the header (status still 201) when `createData` relays a readonly field dropped by the create-side strip in `engine.insert`', async () => { const createData = vi.fn().mockResolvedValue({ object: 'approval_case', id: 'rec-1', record: { id: 'rec-1', title: 'A' }, droppedFields: DROPPED, }); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 87db8036c9..57948b11de 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -8005,9 +8005,10 @@ export class RestServer { ...(environmentId ? { environmentId } : {}), ...(context ? { context } : {}), } as any); - // [#3431] Advertise fields the #3043 create ingress strip - // dropped via the response header (body also carries - // `droppedFields`). Status stays 201. + // [#3431] Advertise fields the engine's create-side static- + // `readonly` strip dropped (`engine.insert`, relayed by + // `createData` as `droppedFields`) via the response header + // (body also carries `droppedFields`). Status stays 201. applyDroppedFieldsHeader(res, result); res.status(201).json(result); } catch (error: any) { @@ -12354,22 +12355,25 @@ export class RestServer { if (op.action === 'create') { // [#3835] Go through the protocol's create ingress — // the SAME one `POST /data/:object` uses — rather than - // calling `ql.insert` directly. The engine's INSERT path - // is static-`readonly`-exempt by design (#3413), so the - // #3043 strip that stops a non-system caller from seeding - // a read-only column lives at that ingress. Bypassing it - // here made `readonly` mean two different things on two - // create paths: rejected on the single route, written - // through the batch. `createData` also owns the platform- - // object carve-out (a `sys_`/`managedBy` object's own - // guard must REJECT a forged value, not silently swallow - // it), which is why this routes to the ingress instead of - // re-implementing the strip here — one create ingress, - // and a future change to its policy covers the batch for - // free. `trxCtx` carries the caller's context (including - // `isSystem`) plus the open transaction, so the strip - // decides exactly as it does on the single route and the - // insert still joins this transaction. + // calling `ql.insert` directly. When this was written the + // #3043 static-`readonly` strip lived at that ingress and + // a direct `ql.insert` bypassed it, so `readonly` meant + // two different things on two create paths. Since the + // maintainer ruling of 2026-09-03 (option C, #14147) the + // strip runs inside `engine.insert` for every non-system + // caller, so both routes are stripped identically, and the + // platform-object carve-out (a `sys_`/`managedBy` object's + // own guard must REJECT a forged value, not silently + // swallow it) is the engine's as well. The routing stands + // on what the ingress still owns: the #3770 object- + // existence gate, the #7823 `internal: true` response + // strip and the `droppedFields` relay — one create + // ingress, one response contract, and a future change to + // its policy covers the batch for free. `trxCtx` carries + // the caller's context (including `isSystem`) plus the + // open transaction, so the engine's strip decides exactly + // as it does on the single route and the insert still + // joins this transaction. const created: any = await p.createData({ object: op.object, data, context: trxCtx } as any); for (const e of (created?.droppedFields ?? []) as DroppedFieldsEvent[]) { dropped.push({ ...e, index }); diff --git a/packages/services/service-automation/src/builtin/create-record-readonly-drop.test.ts b/packages/services/service-automation/src/builtin/create-record-readonly-drop.test.ts new file mode 100644 index 0000000000..75688f2d47 --- /dev/null +++ b/packages/services/service-automation/src/builtin/create-record-readonly-drop.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #14147 — `create_record`'s `onFieldsDropped` channel, driven END TO END + * against a real ObjectQL engine. + * + * The card's second finding: `create_record` passes `onFieldsDropped` to + * `data.insert` and surfaces the result as `output.droppedFields` + node + * `warnings` (#3407, symmetric with `update_record`) — but the engine's insert + * path ran no readonly strip, so that channel could never carry a readonly + * drop. A flow without `runAs: 'system'` could set a `readonly` column at + * insert and the run reported a clean success over a write that did not happen + * the way the author wrote it. + * + * The maintainer ruling of 2026-09-03 (option C) closed the strip half; this + * file is the proof that the SIGNAL half arrives with it. ⛔ A unit test on the + * strip alone does not demonstrate the channel — the stub-engine pins in + * `crud-dropped-fields.test.ts` prove the node forwards an event it is GIVEN, + * which is exactly what stayed green for the entire life of the defect. So the + * `data` service here is a real `ObjectQL` over a recording driver, and the + * assertions are on what a FLOW RUN reports. + */ +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { AutomationEngine } from '../engine.js'; +import { registerCrudNodes } from './crud-nodes.js'; + +function makeLogger(): any { + const l: any = { info() {}, warn() {}, error() {}, debug() {}, trace() {}, fatal() {} }; + l.child = () => l; + return l; +} + +/** Records what actually reaches the store — the row is the verdict. */ +function makeRecordingDriver() { + const creates: Array> = []; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(_o: string, data: Record) { + creates.push({ ...data }); + return { id: 'rec_1', ...data }; + }, + async update(_o: string, id: string, data: Record) { return { id, ...data }; }, + async updateMany() { return 0; }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 0; }, + async bulkCreate(o: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(o, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, creates }; +} + +/** A real data engine, registered as the `data` service the CRUD nodes resolve. */ +async function makeStack() { + const logger = makeLogger(); + const engine = new ObjectQL({ logger }); + const { driver, creates } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'duly_task', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + title: { name: 'title', type: 'text' }, + // The application-reported column: author-declared, statically readonly. + completed_at: { name: 'completed_at', type: 'datetime', readonly: true }, + }, + } as any, 'test'); + + const automation = new AutomationEngine(logger); + registerCrudNodes(automation, { + logger, + getService: (n: string) => (n === 'data' ? engine : undefined), + } as any); + return { automation, engine, creates }; +} + +function seedFlow(name: string, runAs?: 'system' | 'user') { + return { + name, label: name, type: 'autolaunched', + ...(runAs ? { runAs } : {}), + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'mk', type: 'create_record', label: 'Create', + config: { + objectName: 'duly_task', + fields: { title: 'T', completed_at: '2019-04-01T00:00:00Z' }, + outputVariable: 'made', + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'mk' }, + { id: 'e2', source: 'mk', target: 'end' }, + ], + } as any; +} + +describe('#14147 — a NON-system create_record now receives the readonly drop', () => { + it('the column does not land, and the run says so — a node warning naming the field', async () => { + const { automation, creates } = await makeStack(); + automation.registerFlow('seed', seedFlow('seed')); + const res = await automation.execute('seed', { userId: 'u1' }); + expect(res.success, 'a strip is legal semantics, not a failure').toBe(true); + + // 1. The write happened, and it happened WITHOUT the forged column. + expect(creates).toHaveLength(1); + expect(creates[0], 'a non-system flow may not seed a readonly column').not.toHaveProperty('completed_at'); + expect(creates[0].title).toBe('T'); + + // 2. ...and the RUN reports it — the half that could never fire before. + const runs = await automation.listRuns('seed'); + const step = runs[0].steps.find((s: any) => s.nodeId === 'mk')!; + expect(step.status).toBe('success'); + expect(step.warnings, 'the channel #3407 wired and #14147 filled').toHaveLength(1); + expect(step.warnings![0]).toContain('create_record(duly_task)'); + expect(step.warnings![0]).toContain('completed_at'); + }); + + it('runAs:system still seeds it, with no drop and no warning — the intended channel', async () => { + const { automation, creates } = await makeStack(); + automation.registerFlow('seed_sys', seedFlow('seed_sys', 'system')); + const res = await automation.execute('seed_sys', { userId: 'u1' }); + expect(res.success).toBe(true); + + expect(creates[0].completed_at, 'seeding a readonly column at create time is a SYSTEM act') + .toBe('2019-04-01T00:00:00Z'); + const runs = await automation.listRuns('seed_sys'); + const step = runs[0].steps.find((s: any) => s.nodeId === 'mk')!; + expect(step.warnings, 'no warning is manufactured for a write that landed').toBeUndefined(); + }); +}); diff --git a/packages/services/service-automation/src/builtin/crud-nodes.ts b/packages/services/service-automation/src/builtin/crud-nodes.ts index 177f4caea7..25bc1cdb68 100644 --- a/packages/services/service-automation/src/builtin/crud-nodes.ts +++ b/packages/services/service-automation/src/builtin/crud-nodes.ts @@ -318,11 +318,17 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext): const dataCtx = resolveRunDataContext(context); stampSystemInsertOwner(fields, dataCtx, data, objectName); try { - // #3407 — symmetric with update_record. Today the engine's - // insert path strips nothing (INSERT is readonly-exempt and - // FLS write denial throws), so this listener never fires; - // wired anyway so a future insert-side strip surfaces here - // automatically instead of going silent. + // #3407 — symmetric with update_record, and LIVE since + // #14147. It was wired in #3407 against an insert path that + // stripped nothing (INSERT was readonly-exempt; FLS write + // denial throws), i.e. for a signal it could not then + // receive — the maintainer ruling of 2026-09-03 put the + // static-`readonly` strip inside `engine.insert` under an + // `isSystem` gate, so a flow WITHOUT `runAs: 'system'` that + // seeds a readonly column now lands here: `output.dropped- + // Fields` plus a node warning, instead of a clean success + // over a column that never landed. Driven end to end in + // `create-record-readonly-drop.test.ts`. const dropped: DroppedFieldsEvent[] = []; const created = await data.insert(objectName, fields, { context: dataCtx, diff --git a/packages/spec/liveness/field.json b/packages/spec/liveness/field.json index 9d48a657ac..741c80df8a 100644 --- a/packages/spec/liveness/field.json +++ b/packages/spec/liveness/field.json @@ -97,9 +97,9 @@ }, "readonly": { "status": "live", - "evidence": "packages/objectql/src/validation/rule-validator.ts (UPDATE strip); packages/metadata-protocol/src/protocol.ts (INSERT ingress strip)", + "evidence": "packages/objectql/src/validation/rule-validator.ts (stripReadonlyFields — the one strip, UPDATE and INSERT); packages/objectql/src/engine.ts (INSERT call site, isSystem-gated, since the 2026-09-03 ruling)", "proof": "packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts#readonly-static-write", - "note": "renderer + server write path: a non-system write to the field is silently dropped on BOTH UPDATE (#2948/#3003 — engine stripReadonlyFields; was renderer-only, i.e. false compliance for approval/status columns) and INSERT (#3043 — stripped at the DataProtocol createData/import ingress so external REST/GraphQL/MCP creates can't seed approval_status:'approved' a step shorter than #3003, while trusted internal engine writers that legitimately seed readonly columns are unaffected); a stripped INSERT field falls back to its defaultValue; symmetric with readonlyWhen (which stays INSERT-exempt, needing a prior record)." + "note": "renderer + server write path: a non-system write to the field is silently dropped on BOTH UPDATE (#2948/#3003 — engine stripReadonlyFields; was renderer-only, i.e. false compliance for approval/status columns) and INSERT — first (#3043) at the DataProtocol createData/import ingress only, so external REST/GraphQL/MCP creates couldn't seed approval_status:'approved' a step shorter than #3003 while a direct engine.insert caller still could; since the maintainer ruling of 2026-09-03 (option C, #14147) inside engine.insert itself, by the SAME stripReadonlyFields under the SAME isSystem gate as UPDATE, with the ingress copy deleted, so every non-system caller is stripped and seeding a readonly column at create time is a system act (isSystem / runAs:'system' / seeds); a stripped INSERT field falls back to its defaultValue; symmetric with readonlyWhen (which stays INSERT-exempt, needing a prior record)." }, "hidden": { "status": "live", diff --git a/packages/spec/src/api/batch.zod.ts b/packages/spec/src/api/batch.zod.ts index e3e75d56b7..36b39a6c04 100644 --- a/packages/spec/src/api/batch.zod.ts +++ b/packages/spec/src/api/batch.zod.ts @@ -206,7 +206,7 @@ export const BatchOperationResultSchema = lazySchema(() => z.object({ droppedFields: z.array(DroppedFieldsEventSchema).optional().describe( 'Write-observability: caller-supplied fields LEGALLY stripped from ' + 'THIS row before it was written — static `readonly` / TRUE `readonlyWhen`' + - ' on update, or the create-ingress strip. Per-row because a batch can drop ' + + ' on update, or the in-engine static `readonly` strip on create. Per-row because a batch can drop ' + 'different fields on different rows (`readonlyWhen` is record-state-dependent). Present ' + 'ONLY when ≥1 field was dropped for this row; the row still succeeded (success unchanged). ' + 'A single response header cannot express per-row drops, so this body field is the ' + diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index de14269c93..84d329b3a0 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -1973,7 +1973,8 @@ export const CreateDataResponseSchema = lazySchema(() => z.object({ droppedFields: z.array(DroppedFieldsEventSchema).optional().describe( 'Write-observability: caller-supplied fields that were LEGALLY stripped ' + 'before the record was written — a non-system create cannot seed a static `readonly` ' + - 'column (ingress strip), so those keys are dropped and the field re-derives its ' + + 'column (the strip runs inside `engine.insert`, after the `beforeInsert` hooks, ' + + '`isSystem`-gated), so those keys are dropped and the field re-derives its ' + 'default. Present ONLY when ≥1 field was dropped; the create still succeeded without ' + 'them (status/success semantics unchanged). REST additionally surfaces this as the ' + '`X-ObjectStack-Dropped-Fields` response header. Optional — omit-when-empty keeps the ' + @@ -1993,9 +1994,11 @@ export const CreateDataResponseSchema = lazySchema(() => z.object({ * stable and server-produced): `cloneData` builds exactly * `{ object, id, sourceId, record }` — the structural sibling of * {@link CreateDataResponseSchema} plus `sourceId`. A clone IS a create (the - * copy runs the insert path: engine-owned columns re-derived, the #3043 - * readonly ingress strip applied, internal fields omitted from the response, - * #7823) — but unlike `createData` the producer emits no `droppedFields` + * copy runs the insert path: engine-owned columns re-derived, the static + * `readonly` strip applied inside `engine.insert` for a non-system caller — + * the 2026-09-03 ruling, #14147; the #3043 ingress copy is deleted — and + * internal fields omitted from the response, #7823) — but unlike `createData` + * the producer emits no `droppedFields` * member, so none is declared: a key the producer never writes would be a * promise conformance cannot measure. */ @@ -2311,8 +2314,8 @@ export const CreateManyDataResponseSchema = lazySchema(() => z.object({ records: z.array(z.record(z.string(), z.unknown())).describe('Created records'), count: z.number().describe('Number of records created'), droppedFields: z.array(DroppedFieldsEventSchema).optional().describe( - 'Write-observability: caller-supplied `readonly` fields the ' + - 'create-ingress strip removed before the rows were written. AGGREGATED across the batch ' + + 'Write-observability: caller-supplied `readonly` fields the in-engine create-side ' + + 'strip (`engine.insert`, `isSystem`-gated) removed before the rows were written. AGGREGATED across the batch ' + '(one event per object/reason with the union of dropped field names) rather than per-row, ' + 'because the insert-time strip is static-`readonly` only — schema-uniform, so every row ' + 'drops the same set. Present ONLY when ≥1 field was dropped; the creates still succeeded ' + diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index c6fb6bc0e4..991aa2038e 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -136,30 +136,38 @@ export interface WriteObservabilityOptions { * client toggle write-refusal on a security-adjacent path. Widening strict to * the wire is a SEPARATE decision, not a side effect of this one. * - * ## INSERT — refuses runtime-owned values (since #5503) + * ## INSERT — refuses runtime-owned values (since #5503) and static + * `readonly` values (since the 2026-09-03 ruling, #14147) * * Until #5503 this paragraph declared the option inert on insert — true - * when written (#5126 predates the runtime-owned strip), false since. At - * this seam insert remains deliberately exempt from the two AUTHOR-DECLARED - * strips (#3413: an in-process create may seed a `readonly: true` field's - * initial value, and `readonlyWhen` cannot lock anything on a create at - * all), but the implicitly-readonly runtime-owned strip #5503 added runs on - * insert too — and it is exactly the one strict refuses. An insert whose - * payload carries a runtime-owned value (`RUNTIME_OWNED_FIELD_TYPES`, today - * `autonumber` — a caller-supplied record number) behaves like update at - * this seam: with this option `true` it throws `ReadonlyFieldRejectedError` - * (`operation: 'insert'`) and nothing is written; without it the value is - * stripped, the write completes, and `onFieldsDropped` fires with - * `reason: 'readonly'`. The engine-level writers exempt from that strip — - * and therefore never refused — are the two the error message itself names: - * `isSystem`, and the `preserveAudit` historical import reinstating legacy - * record numbers (#3493). Layer note: that exemption pair is THIS - * in-process seam's. The DataProtocol ingress enforces its own - * author-declared `readonly` policy on create (#3043), where - * `preserveAudit` is UPDATE-only (#6640) — see `FieldSchema.readonly`; - * nothing here widens or narrows it. `ReadonlyFieldRejectedError`'s own doc - * records the same contract from the error's side: "Thrown by - * `engine.update` — and, since #5503, by `engine.insert`". + * when written (#5126 predates the runtime-owned strip), false since. Until + * the maintainer ruling of 2026-09-03 (option C, #14147) it then declared + * insert exempt from the two AUTHOR-DECLARED strips (#3413: "an in-process + * create may seed a `readonly: true` field's initial value") — true when + * written, SUPERSEDED since: `engine.insert` now runs the static-`readonly` + * strip for a non-system caller, the same `stripReadonlyFields` under the + * same `isSystem` gate as `engine.update`, and the metadata-protocol ingress + * copy that used to cover external callers only is deleted. One semantics, + * one enforcement point. What insert still does NOT strip is `readonlyWhen` + * (a conditional lock has no prior record to evaluate on a create). + * + * So at this seam an insert behaves like update for BOTH reported strips: a + * payload carrying a runtime-owned value (`RUNTIME_OWNED_FIELD_TYPES`, today + * `autonumber` — a caller-supplied record number) or a static `readonly` + * value from a non-system caller, with this option `true`, throws + * `ReadonlyFieldRejectedError` (`operation: 'insert'`, every taken field in + * one list) and nothing is written; without it the values are stripped, the + * write completes, and `onFieldsDropped` fires once with + * `reason: 'readonly'`. The writers exempt — and therefore never refused — + * differ per strip, and the difference is the 2026-08-08 ruling, not this + * one: `isSystem` exempts both; the `preserveAudit` historical import + * reinstating legacy record numbers (#3493) exempts the runtime-owned strip + * only, and a non-system create that asks for it still has its static + * `readonly` fields stripped, with a `warn` saying the exemption is + * UPDATE-only (#6640) — see `FieldSchema.readonly`. + * `ReadonlyFieldRejectedError`'s own doc records the same contract from the + * error's side: "Thrown by `engine.update` — and, since #5503, by + * `engine.insert`". */ strictReadonlyWrites?: boolean; } diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 5b6f87db72..63dbc4836b 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -205,9 +205,10 @@ const MULTILINE_EDITOR_FIELD_TYPES: ReadonlySet = new Set([ * This is the PROTOCOL's statement of that ownership, so the consumers that act * on it read one vocabulary instead of each carrying its own literal: objectql's * write-path strips (`isRuntimeOwnedField` / `stripRuntimeOwnedFields`, which - * treat these types as implicitly read-only), and the DataProtocol create - * ingress, which defers to those strips rather than pre-empting them with its - * own narrower exemption set (`stripReadonlyForInsert`, #5628). + * treat these types as implicitly read-only), and the create-side static + * `readonly` strip, which EXCLUDES these types rather than pre-empting a + * whitelist it does not implement (`staticReadonlyInsertSubject`, #5628/#14147 + * — the exclusion the deleted DataProtocol ingress copy used to carry). * * Keep the set to types whose value is (a) persisted, (b) issued by the runtime, * and (c) never legitimately supplied by a caller. `formula` and `summary` are diff --git a/packages/spec/src/kernel/execution-context.zod.ts b/packages/spec/src/kernel/execution-context.zod.ts index b5707420bc..d604a39ee4 100644 --- a/packages/spec/src/kernel/execution-context.zod.ts +++ b/packages/spec/src/kernel/execution-context.zod.ts @@ -389,7 +389,7 @@ export const ExecutionContextSchema = lazySchema(() => z.object({ * field-level security are unaffected: this changes only which audit/readonly * values the runtime overwrites, never who may write the record. */ - preserveAudit: z.boolean().optional().describe('Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: a create is stripped earlier, at the DataProtocol ingress, whose only exemption is `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected.'), + preserveAudit: z.boolean().optional().describe('Historical import: preserve the ORIGINAL audit timeline for this write instead of stamping it "now". Opt-in and server-constructed only, never client-supplied. On the UPDATE path it admits a whitelist — the audit/timestamp family (created_at / created_by / updated_at / updated_by) plus author-declared business `readonly` fields — while platform-managed `system` columns (tenancy, generated) stay stripped. On INSERT the exemption does NOT apply: the create-side static `readonly` strip runs inside `engine.insert` itself (after the `beforeInsert` hooks, before validation — the 2026-09-03 ruling; the DataProtocol ingress copy it replaced is deleted) and reads only `context.isSystem`, so a non-system create carrying `preserveAudit` still has those fields stripped and is warned (WARN) that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. Permissions / RLS / field-level security are unaffected.'), /** * OAuth 2.1 scopes granted to the access token that authenticated this diff --git a/scripts/check-system-context-census.mjs b/scripts/check-system-context-census.mjs index 873e073139..f9863a5379 100644 --- a/scripts/check-system-context-census.mjs +++ b/scripts/check-system-context-census.mjs @@ -320,37 +320,37 @@ export const NON_READ_ANCHORS = [ { file: 'packages/objectql/src/engine.ts', needle: 'if (!hasTx && !hasTenant && !isSystem && !hasTz && !preserveAudit) return base;', - why: 'row 24 -- the early return the tenant-audit read feeds', + why: 'row 23 -- the early return the tenant-audit read feeds', }, { file: 'packages/objectql/src/engine.ts', needle: 'if (isSystem && opts.bypassTenantAudit === undefined && !isTenantAuditInScope) {', - why: 'row 24 -- where `bypassTenantAudit` is threaded to the driver', + why: 'row 23 -- where `bypassTenantAudit` is threaded to the driver', }, { file: 'packages/objectql/src/engine.ts', needle: 'if (options?.strictReadonlyWrites === true) {', - why: 'row 22 -- the strict-drop refusal that never fires under elevation', + why: 'row 21 -- the strict-drop refusal that never fires under elevation', }, { file: 'packages/objectql/src/readonly-strict-errors.ts', needle: 'const READONLY_CLASS_REASONS', - why: 'row 22 -- the reason set the silent refusal would have used', + why: 'row 21 -- the reason set the silent refusal would have used', }, { file: 'packages/plugins/plugin-security/src/system-write-guard.ts', needle: 'if (!isUserContextWrite(context)) return;', - why: 'row 25 -- the bypass expressed through a helper rather than a direct read', + why: 'row 24 -- the bypass expressed through a helper rather than a direct read', }, { file: 'packages/plugins/plugin-sharing/src/sharing-service.ts', needle: "if (row.source != null && row.source !== 'manual') {", - why: 'row 34 -- the CONFLICT guard `revoke()` deletes in front of', + why: 'row 33 -- the CONFLICT guard `revoke()` deletes in front of', }, { file: 'packages/services/service-automation/src/builtin/crud-nodes.ts', needle: 'stampSystemInsertOwner(fields, dataCtx, data, objectName);', - why: 'row 60 -- the call site of the compensating owner stamp', + why: 'row 59 -- the call site of the compensating owner stamp', }, { file: 'packages/objectql/src/registry.ts', diff --git a/scripts/doc-authoring-prose-id.baseline.json b/scripts/doc-authoring-prose-id.baseline.json index 0b9441cf44..1ad330ad58 100644 --- a/scripts/doc-authoring-prose-id.baseline.json +++ b/scripts/doc-authoring-prose-id.baseline.json @@ -376,14 +376,11 @@ }, "packages/metadata-protocol/src/protocol.ts": { "#10377": 1, - "#3043": 1, "#3172": 1, - "#3493": 1, "#3770": 1, "#4196": 1, "#4432": 1, "#6190": 3, - "#6640": 1, "#6992": 1, "#7894": 2, "#8502": 1, diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 9141601666..1ede6d0a2a 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1031,6 +1031,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.readonly-insert.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.record-not-found.test.ts", "verb": "delete",