Skip to content

Commit 720bf47

Browse files
claude[bot]claude
andauthored
fix(lint): report a non-system create_record / ctx.api insert of a static-readonly field — the create-verb scan gap on the flow and hook readonly rules (#16248)
* wip: widen readonly flow/hook rules to the create verb (static shape) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 * fix(lint): report a non-system create of a static-readonly field on the flow and hook readonly rules Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 * fix(lint): the create-verb readonly finding skips platform objects, which the engine's insert strip does not judge Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6971170 commit 720bf47

7 files changed

Lines changed: 568 additions & 116 deletions
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
`flow-update-readonly-field` and `hook-api-update-readonly-field` now report a non-system **create** of a static-`readonly` field — a new **error**-severity finding that fails `os lint` / `os validate` / `os build` on a shape they used to accept.
6+
7+
Both rules scanned only the update verb (`update_record`; `ctx.api…update()` / `.updateById()`) and justified the omission with the same sentence: INSERT is engine-exempt from the author-declared `readonly` strip, so a create that seeds a `readonly` column is not a no-op. The maintainer ruling of 2026-09-03 (option C, #14147) made that false — `engine.insert` now runs the same `isSystem`-gated `stripReadonlyFields` the update path runs — so a flow `create_record` without `runAs: 'system'`, or a hook body's `ctx.api.object('…').insert()` under a non-system trigger, that writes a `readonly` field became a **silent no-op**: the row lands without the column (which falls back to its `defaultValue`), the step reports `success`, and only a run-time warning names the dropped field (measured end to end in `@objectstack/service-automation`'s `create-record-readonly-drop.test.ts`). Nothing reported it at build time. This closes that scan gap (#15394).
8+
9+
**What now fails that passed before.** Exactly one new shape per rule, at `error`:
10+
11+
- a flow `create_record` node whose literal `fields` map writes a field the target object declares `readonly: true`, on a flow that does not declare `runAs: 'system'`;
12+
- an L2 hook body's literal `ctx.api.object('<name>').insert({ … })` writing such a field, on a hook that does not declare `runAs: 'system'`.
13+
14+
The rule ids and severities are the update ones — one id per shape, not per verb — and each finding's message names the verb it was judged on and what actually happens to a create. Everything the rules already skipped is still skipped: a templated object name, a non-literal payload, an object outside the stack or declaring no fields, an unknown field (the unknown-field rules' question), and any `runAs: 'system'` flow or hook, because seeding a `readonly` column at create time is a system act and that write lands.
15+
16+
**Deliberately not reported.**
17+
18+
- No `readonlyWhen` (conditional) finding on a create, on either surface: a conditional lock is evaluated against the record being written over, which a create does not have, and the engine runs no conditional strip on INSERT ("INSERT stays exempt"). A warning there would state something false about a write that lands.
19+
- The hook rule judges `.insert()` only, not `.create()`. The host `ObjectRepository` aliases `create()` to `insert()`, but L2 bodies run in QuickJS and the VM-side `ctx.api.object()` installs no `create` leaf — a body calling `.create()` throws `TypeError: not a function` on its first run, a loud failure rather than the silent drop this rule reports. The silence is recorded as a reasoned method exclusion (`READONLY_HOOK_METHOD_EXCLUSIONS`) and pinned.
20+
- No create finding on a **platform object** — one declaring `managedBy`, or in the reserved `sys_` namespace. The engine's create-side strip does not judge those at all (`staticReadonlyInsertSubject`: their own ADR-0086 write guard governs them), so a finding there would describe a strip that never runs. The update verb keeps judging them, exactly as the engine's update path does.
21+
- `validate-readonly-action-writes` is unchanged: an action body runs system-elevated by design, so its create genuinely lands.
22+
23+
**Migration.** If your build reds on the new finding, the fix is one of: declare `runAs: 'system'` on the flow or hook when seeding the `readonly` column is the intent (the intended channel — `readonly` governs the end-user/API surface, not trusted system writers); remove the key from the `create_record` `fields` / `insert()` payload when it is not; or stamp it in a `beforeInsert` hook on the target object (`ctx.input.<field> = …`), which is a server value the strip does not touch. Measured over this repository's shipped examples (`app-crm`, `app-showcase`, `app-todo`): zero in-repo flows or hooks go red — the two `create_record` nodes that target an object carrying a `readonly` field write none of its `readonly` fields, and the one flow that creates unauthenticated already declares `runAs: 'system'`; no shipped hook body inserts through `ctx.api`.

content/docs/automation/hook-bodies.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,10 +265,10 @@ There is an asymmetry here that costs data if you learn it the hard way, so lear
265265

266266
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**:
267267

268-
- `hook-api-update-readonly-field`**error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
268+
- `hook-api-update-readonly-field`**error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there.
269269
- `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.
270270

271-
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`.
271+
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. `.create()` is skipped too, for a reason about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so a body calling `.create()` throws `TypeError: not a function` on its first run — a loud failure, not a silent drop — and the same payload spelled `.insert()` is what the rule judges. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create.
272272

273273
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.
274274

packages/lint/src/validate-flow-node-writes.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,12 @@ describe('validateFlowNodeWrites', () => {
407407
});
408408

409409
it('does NOT flag a readonly field on create_record — this rule asks whether the column EXISTS, not whether the write lands', () => {
410-
// The readonly sibling does not scan create_record today — a scan gap
411-
// (#15394) since the 2026-09-03 ruling put the static-`readonly` strip
412-
// inside `engine.insert` (#14147), not an exemption. This rule asks a
413-
// different question either way, so a DECLARED readonly field is clean here
414-
// for its own reason: it resolves to a column.
410+
// Since #15394 the readonly sibling (`flow-update-readonly-field`) DOES
411+
// report this exact write — a non-system create_record of a static-
412+
// readonly field is stripped inside `engine.insert` since the 2026-09-03
413+
// ruling (#14147). This rule asks a different question, so a DECLARED
414+
// readonly field is clean here for its own reason: it resolves to a
415+
// column. The two never double-report one key.
415416
const withReadonly = {
416417
name: 'deal',
417418
fields: { stage: { type: 'text' }, approval_status: { type: 'text', readonly: true } },

0 commit comments

Comments
 (0)