Skip to content

Commit 68c0a2b

Browse files
committed
feat(lint): gate a hook body's ctx.api write to a readonly field (#13653)
A hook's `ctx.api` is a ScopedContext over the TRIGGERING operation's execution context, so `ctx.api.object('x').update({ readonlyField })` reaches the engine as an ordinary non-system caller and the update path strips the key. The call returns success and the column stays null - a failure only an end-to-end read-back detects. This completes the hook side of the flow-side gate that shipped as `flow-update-readonly-field`. The rule keys on the write CHANNEL, not on the field: a beforeInsert / beforeUpdate body stamping `ctx.input.<field> = ...` writes a server value that survives the strip (#5591) and is never flagged - `readonly` plus a before-hook is a correct and widely used pairing. Also skipped, each for a stated reason: `ctx.api.sudo()` chains (elevated, the intended channel), insert/create (INSERT is engine-exempt, #3043/#3413), dynamic object names, non-literal payloads, objects or fields this stack does not declare, and `id` in an update payload (the row address, #8141). - `hook-api-update-readonly-field` - error - `hook-api-update-readonly-when-field` - warning (per-record state) Wired through REFERENCE_INTEGRITY_RULES so it runs on `os validate`, `os lint` and `os compile` at once. `buildReadonlyIndex` is now shared from the flow rule rather than copied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
1 parent 787d757 commit 68c0a2b

8 files changed

Lines changed: 837 additions & 3 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/lint': minor
3+
---
4+
5+
Add `validateReadonlyHookWrites` — an author-time gate on a hook body writing a `readonly` field through `ctx.api`.
6+
7+
A hook's `ctx.api` is a `ScopedContext` over the **triggering** operation's execution context, so `ctx.api.object('x').update({ someReadonlyField })` reaches the engine as an ordinary non-system caller and the update path strips the key. The call returns success, the step looks clean, and the column is simply always null — a failure only an end-to-end read-back detects. This completes the hook side of the flow-side gate that shipped as `flow-update-readonly-field`.
8+
9+
Two new rule ids, wired through `REFERENCE_INTEGRITY_RULES` so they run on `os validate`, `os lint` and `os compile`:
10+
11+
- `hook-api-update-readonly-field` — **error**. A literal `ctx.api.object('…').update()` / `.updateById()` writing a field the named object declares `readonly: true`.
12+
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record state.
13+
14+
The rule keys on the write **channel**, not on the field, so the correct and widely used pairing is untouched: a `beforeInsert`/`beforeUpdate` body stamping `ctx.input.<field> = …` writes a server value that survives the strip and is **never** flagged. Also skipped, each for a stated reason: `ctx.api.sudo()` chains (elevated — the intended channel), `insert`/`create` (INSERT is engine-exempt), dynamic object names, non-literal payloads, objects this stack does not declare, fields the object does not declare, and `id` in an `update` payload (the row address, not a field write).

‎content/docs/automation/hook-bodies.mdx‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ Because the checking is advisory and literal-only:
215215

216216
- **Treat `hook-body-write-unknown-field` as a build failure by convention.** It does not gate, but the rule is tuned for near-zero false positives — in practice a warning is a real typo.
217217
- **Check by hand what the parser cannot see.** Computed keys, spreads, aliased input and dynamic object names are invisible to the rule; for an array or `"*"` hook, every field must exist on every target.
218-
- **Prefer a flow `update_record` node when the write set is fixed — and for *this* check most of all.** A flow node's writes are structured config: they diff field-by-field, render in the Console designer, and a write to a `readonly:true` field is a **gating error** (`flow-update-readonly-field`) that hooks have no counterpart for. Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271) the field-existence check gates there too — `flow-node-write-unknown-field` is an **error**, not the advisory warning a body gets, because a node's `fields` is a literal map next to a literal `objectName`: there is no parser in between that could have mis-extracted it, so a finding is a certainty rather than a best effort.
218+
- **Prefer a flow `update_record` node when the write set is fixed — and for *this* check most of all.** A flow node's writes are structured config: they diff field-by-field, render in the Console designer, and since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271) the field-existence check gates there too — `flow-node-write-unknown-field` is an **error**, not the advisory warning a body gets, because a node's `fields` is a literal map next to a literal `objectName`: there is no parser in between that could have mis-extracted it, so a finding is a certainty rather than a best effort. (The *writability* check now has a hook-side counterpart — see [Writing a `readonly` field](#writing-a-readonly-field) below — but it covers only the `ctx.api` channel.)
219219
- **Exercise the hook against a real object before shipping** — on SQL drivers the mistake surfaces on the first write; schemaless drivers won't tell you.
220220

221221
### Signature conventions
@@ -245,6 +245,26 @@ Per-invocation budgets default to **250ms** (hooks) / **5000ms** (actions) of **
245245

246246
A body may write *other* objects — e.g. `await ctx.api.object('parent').update({ ... })` from a child's `afterInsert`/`afterUpdate` (requires `api.write`). The target's own hooks fire too: the nested write runs in a **fresh sandbox VM** while the calling body is suspended, and this composes to any depth. This is the natural "when a child changes, roll the total up to the parent" automation — it does **not** need a denormalized, hand-maintained mirror field. Because each body's budget is **CPU time** (ADR-0102), the caller is **not** charged for the nested write's own run — so the stock 250ms default comfortably covers deep rollup chains, and you rarely need to raise `timeoutMs` (the spec still permits up to 30_000ms for a genuinely CPU-heavy body).
247247

248+
### Writing a `readonly` field
249+
250+
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** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
251+
252+
`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:
253+
254+
| How the body writes it | What happens |
255+
|:---|:---|
256+
| `ctx.input.<field> = …` 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. |
257+
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `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. The call still returns success. |
258+
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
259+
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |
260+
261+
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**:
262+
263+
- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
264+
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
265+
266+
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).
267+
248268
### Errors from `ctx.api`
249269

250270
A rejected `ctx.api` call gives your body the host error's `name` and `message`, plus two structured properties when the host supplied them:

‎packages/lint/src/index.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ export type {
124124
ReadonlyFlowWriteSeverity,
125125
} from './validate-readonly-flow-writes.js';
126126

127+
export {
128+
validateReadonlyHookWrites,
129+
HOOK_API_UPDATE_READONLY_FIELD,
130+
HOOK_API_UPDATE_READONLY_WHEN_FIELD,
131+
READONLY_HOOK_WRITE_PATTERN_IDS,
132+
READONLY_HOOK_WRITE_EXCLUSIONS,
133+
} from './validate-readonly-hook-writes.js';
134+
export type {
135+
ReadonlyHookWriteFinding,
136+
ReadonlyHookWriteSeverity,
137+
} from './validate-readonly-hook-writes.js';
138+
127139
export { validateViewContainers, VIEW_CONTAINER_SHAPE } from './validate-view-containers.js';
128140
export type { ViewContainerFinding, ViewContainerSeverity } from './validate-view-containers.js';
129141

‎packages/lint/src/reference-integrity-suite.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ describe('reference-integrity suite — membership', () => {
3838
'validateActionBodyWrites',
3939
'validateFlowNodeWrites',
4040
'validateReadonlyFlowWrites',
41+
// [#13653] The hook-side half of the readonly write judgement: a body's
42+
// `ctx.api` update to a declared-`readonly` field, placed beside the flow
43+
// twin that asks the identical question one surface over.
44+
'validateReadonlyHookWrites',
4145
'validateReactPageProps',
4246
]);
4347
});
@@ -214,6 +218,22 @@ describe('reference-integrity suite — every member actually runs', () => {
214218
events: ['beforeInsert'],
215219
body: { language: 'js', source: "ctx.input.lead_score = 100;" },
216220
},
221+
// validateReadonlyHookWrites (#13653): `locked` EXISTS on crm_lead and is
222+
// static-`readonly`, so this is not an existence question — the engine
223+
// strips the key from the ctx.api UPDATE payload on every non-system
224+
// trigger and the call still returns success. A separate hook from
225+
// `score_lead` on purpose, mirroring the `stamp`/`lock` flow-node split
226+
// below: one body carrying both defects would let either rule go silent
227+
// behind the other's finding.
228+
{
229+
name: 'lock_lead',
230+
object: 'crm_lead',
231+
events: ['afterUpdate'],
232+
body: {
233+
language: 'js',
234+
source: "await ctx.api.object('crm_lead').update({ id: ctx.recordId, locked: true });",
235+
},
236+
},
217237
],
218238
flows: [
219239
{
@@ -287,6 +307,7 @@ describe('reference-integrity suite — every member actually runs', () => {
287307
expect(rules).toContain('action-record-write-discarded');
288308
expect(rules).toContain('flow-node-write-unknown-field');
289309
expect(rules).toContain('flow-update-readonly-field');
310+
expect(rules).toContain('hook-api-update-readonly-field');
290311
expect(rules).toContain('react-prop-missing-required');
291312
});
292313

‎packages/lint/src/reference-integrity-suite.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import { validateHookBodyWrites } from './validate-hook-body-writes.js';
9696
import { validateActionBodyWrites } from './validate-action-body-writes.js';
9797
import { validateFlowNodeWrites } from './validate-flow-node-writes.js';
9898
import { validateReadonlyFlowWrites } from './validate-readonly-flow-writes.js';
99+
import { validateReadonlyHookWrites } from './validate-readonly-hook-writes.js';
99100
import { validateReactPageProps } from './validate-react-page-props.js';
100101

101102
export type ReferenceIntegritySeverity = 'error' | 'warning';
@@ -275,6 +276,22 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
275276
// build the other command would have stopped. Joining the suite is the whole
276277
// fix; the two hand-wired call sites are deleted with it (#4345 follow-up).
277278
{ name: 'validateReadonlyFlowWrites', run: validateReadonlyFlowWrites },
279+
// [#13653] The SAME question as the member above, on the surface that had no
280+
// answer for it: a hook body's `ctx.api.object('x').update({ readonlyField })`.
281+
// A hook's `ctx.api` is a ScopedContext over the TRIGGERING operation's
282+
// context, so on a non-system trigger the engine strips the key and the call
283+
// still returns success — the flow rule's silent no-op, reached through JS
284+
// instead of through `config.fields`.
285+
//
286+
// It gates for the flow member's reason and NOT for its neighbour
287+
// `validateHookBodyWrites`': both halves of the judgement are declared in
288+
// THIS stack (the field's `readonly`, the body's literal `ctx.api` update),
289+
// so the finding does not depend on a package the build cannot see. What it
290+
// must never touch is the `ctx.input` stamp — a before-hook writing a
291+
// `readonly` field is CORRECT and widely used, because the strip drops only
292+
// caller-supplied values (#5591) — so the rule keys on the write CHANNEL,
293+
// and both directions are pinned in its tests.
294+
{ name: 'validateReadonlyHookWrites', run: validateReadonlyHookWrites },
278295
// The `kind:'react'` page surface. Every prop a react block binds BY FIELD
279296
// NAME is resolved against the object it names (#4340) — `<ListView columns>`,
280297
// `<ObjectForm fields>`, `<Block type="element:…">` through the SAME

‎packages/lint/src/validate-readonly-flow-writes.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ function asArray(v: unknown): AnyRec[] {
6565
return [];
6666
}
6767

68-
interface FieldReadonlyMeta {
68+
export interface FieldReadonlyMeta {
6969
/** Static `readonly: true`. */
7070
readonly: boolean;
7171
/** A non-empty `readonlyWhen` predicate is declared. */
@@ -77,8 +77,17 @@ interface FieldReadonlyMeta {
7777
* (array of `{name, readonly, readonlyWhen}` and name-keyed map). A field with
7878
* neither flag is recorded as `{false, false}` so callers can distinguish a
7979
* "known-writable field" from an "unknown field" (absent from the map).
80+
*
81+
* Exported for `validate-readonly-hook-writes.ts` (#13653), which asks the
82+
* IDENTICAL question one surface over — "is this declared field writable
83+
* through this channel?" — about a hook body's `ctx.api` update instead of a
84+
* flow node's `config.fields`. Shared rather than copied for the reason #4330
85+
* collapsed five hand-copied lists: two readings of `readonly`/`readonlyWhen`
86+
* that drift produce two rules that disagree about the same field, and the
87+
* disagreement is silent. `IMPLICIT_FIELDS` in `validate-hook-body-writes.ts`
88+
* is shared across its three surfaces on exactly this reasoning.
8089
*/
81-
function buildReadonlyIndex(objects: AnyRec[]): Map<string, Map<string, FieldReadonlyMeta>> {
90+
export function buildReadonlyIndex(objects: AnyRec[]): Map<string, Map<string, FieldReadonlyMeta>> {
8291
const idx = new Map<string, Map<string, FieldReadonlyMeta>>();
8392
for (const obj of objects) {
8493
const name = typeof obj.name === 'string' ? obj.name : undefined;

0 commit comments

Comments
 (0)