Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/readonly-hook-api-write-lint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'@objectstack/lint': minor
---

Add `validateReadonlyHookWrites` — an author-time gate on a hook body writing a `readonly` field through `ctx.api`.

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`.

Two new rule ids, wired through `REFERENCE_INTEGRITY_RULES` so they run on `os validate`, `os lint` and `os compile`:

- `hook-api-update-readonly-field` — **error**. A literal `ctx.api.object('…').update()` / `.updateById()` writing 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 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).
26 changes: 23 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Static validation around a hook is asymmetric, and it is worth knowing exactly w

- **Checked — read side.** `hook.condition` is validated at build time against the target object's fields by the expression validator (`@objectstack/lint`), including array-valued `hook.object` targets. A condition referencing a nonexistent field fails the lint.
- **Checked — capability side.** `body.capabilities` gates which `ctx` APIs the body may call at all; the sandbox throws on an undeclared call.
- **Checked — write side, advisory and literal-only.** Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271), `body.source` is **parsed** (never executed, never type-checked) and the field names it writes are resolved against the target object's declarations. An unknown field raises `hook-body-write-unknown-field` — a **warning** carrying a did-you-mean suggestion, which never blocks a build. Action bodies get the same check on their `ctx.api` writes (`action-body-write-unknown-field`). Both run under `os validate`, `os lint` and `os compile`.
- **Checked — write side, literal-only.** Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271), `body.source` is **parsed** (never executed, never type-checked) and the field names it writes are resolved against the target object's declarations. An unknown field raises `hook-body-write-unknown-field` — a **warning** carrying a did-you-mean suggestion, which never blocks a build. Action bodies get the same check on their `ctx.api` writes (`action-body-write-unknown-field`). Both run under `os validate`, `os lint` and `os compile`. The *existence* question is advisory like this because the answer can depend on a package the build cannot see; the separate *writability* question — [writing a `readonly` field through `ctx.api`](#writing-a-readonly-field) — **does** gate, because both halves of that judgement are declared in the stack being checked.
- **Checked — writes to a system column the object has no storage for.** Since [#8663](https://github.com/objectstack-ai/objectstack/issues/8663), a write to an injected system column is no longer exempted on the strength of its NAME alone. The registry injects `owner_id` / `organization_id` / the audit family onto an ADR-0015 [`external` object](/docs/data-modeling/external-datasources) exactly as onto a local one, but the remote database owns that schema and no column exists behind them. Writing one raises `hook-body-write-unprovisioned-anchor` (or `action-body-write-unprovisioned-anchor` / `flow-node-write-unprovisioned-anchor` on the other two surfaces) — a **warning** on all three, including the flow-node rule that otherwise gates, because the claim is about a remote schema the build cannot see. A column you **declare** yourself is untouched: on a federated object a declared `owner_id` maps a remote column you vouch for. Why it matters more than an ordinary typo: an undeclared name is refused upstream by the engine's own write-path validator (`INVALID_FIELD`), whereas the injected anchor is in the registered schema and passes it — so it is the one payload key that reaches the remote database raw, where a SQL remote aborts the **whole statement** with an untyped `no such column` and takes the correctly named fields of the same payload with it.
- **Checked — writes that reach nothing at all.** Since [#4345](https://github.com/objectstack-ai/objectstack/issues/4345), an action body assigning to `ctx.record` raises `action-record-write-discarded`, also a warning. This one is **not** a field-resolution question: an action's `ctx.record` is a snapshot the runtime never writes back, so the assignment is discarded whether or not the field is declared — see [Signature conventions](#signature-conventions) below.

Expand Down Expand Up @@ -211,11 +211,11 @@ An unknown field is **not** caught at runtime, and it does not fail quietly eith

Neither outcome is the one you wanted, and the advisory warning is the earliest signal you get.

Because the checking is advisory and literal-only:
Because the existence check is advisory, and every write-side check here is literal-only:

- **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.
- **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.
- **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.
- **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.)
- **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.

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

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).

### 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** — but only through two channels, and a nested `ctx.api` write is **not** one of them.

`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:

| How the body writes it | What happens |
|:---|:---|
| `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. |
| `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. |
| `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. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

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*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.

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).

### Errors from `ctx.api`

A rejected `ctx.api` call gives your body the host error's `name` and `message`, plus two structured properties when the host supplied them:
Expand Down
11 changes: 9 additions & 2 deletions content/docs/automation/hooks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,19 @@ Two structural reasons to prefer the flow when either could work:
`update_record` node's `fields` is structural config that `os validate`
checks — readonly targets, template dialects, declared expression slots. A
hook body's write set is checked too, but only for the literal patterns a
parser can recognise and only as an advisory warning (see
parser can recognise (see
[Hook & Action Bodies](/docs/automation/hook-bodies#write-set-checking)).
Field existence is checked on both surfaces, at different strengths: a hook
body gets a warning, an `update_record` node a **gating error**
(`flow-node-write-unknown-field`) — its `fields` is a literal map next to a
literal `objectName`, so nothing could have mis-read it.
literal `objectName`, so nothing could have mis-read it. **Writability** is
now gated on both: writing a `readonly` field through `ctx.api` is
`hook-api-update-readonly-field`, the hook-side sibling of the flow node's
`flow-update-readonly-field` (see
[Writing a `readonly` field](/docs/automation/hook-bodies#writing-a-readonly-field)) —
the two questions differ because a `readonly` declaration and a literal
`ctx.api` update are both visible in the stack, while a field's existence may
depend on a package the build cannot see.
- **A flow reviews as data.** The node graph diffs field-by-field and renders
in the Console designer with per-node run history; a hook body reviews as
code only.
Expand Down
12 changes: 12 additions & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,18 @@ export type {
ReadonlyFlowWriteSeverity,
} from './validate-readonly-flow-writes.js';

export {
validateReadonlyHookWrites,
HOOK_API_UPDATE_READONLY_FIELD,
HOOK_API_UPDATE_READONLY_WHEN_FIELD,
READONLY_HOOK_WRITE_PATTERN_IDS,
READONLY_HOOK_WRITE_EXCLUSIONS,
} from './validate-readonly-hook-writes.js';
export type {
ReadonlyHookWriteFinding,
ReadonlyHookWriteSeverity,
} from './validate-readonly-hook-writes.js';

export { validateViewContainers, VIEW_CONTAINER_SHAPE } from './validate-view-containers.js';
export type { ViewContainerFinding, ViewContainerSeverity } from './validate-view-containers.js';

Expand Down
21 changes: 21 additions & 0 deletions packages/lint/src/reference-integrity-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ describe('reference-integrity suite — membership', () => {
'validateActionBodyWrites',
'validateFlowNodeWrites',
'validateReadonlyFlowWrites',
// [#13653] The hook-side half of the readonly write judgement: a body's
// `ctx.api` update to a declared-`readonly` field, placed beside the flow
// twin that asks the identical question one surface over.
'validateReadonlyHookWrites',
'validateReactPageProps',
]);
});
Expand Down Expand Up @@ -214,6 +218,22 @@ describe('reference-integrity suite — every member actually runs', () => {
events: ['beforeInsert'],
body: { language: 'js', source: "ctx.input.lead_score = 100;" },
},
// validateReadonlyHookWrites (#13653): `locked` EXISTS on crm_lead and is
// static-`readonly`, so this is not an existence question — the engine
// strips the key from the ctx.api UPDATE payload on every non-system
// trigger and the call still returns success. A separate hook from
// `score_lead` on purpose, mirroring the `stamp`/`lock` flow-node split
// below: one body carrying both defects would let either rule go silent
// behind the other's finding.
{
name: 'lock_lead',
object: 'crm_lead',
events: ['afterUpdate'],
body: {
language: 'js',
source: "await ctx.api.object('crm_lead').update({ id: ctx.recordId, locked: true });",
},
},
],
flows: [
{
Expand Down Expand Up @@ -287,6 +307,7 @@ describe('reference-integrity suite — every member actually runs', () => {
expect(rules).toContain('action-record-write-discarded');
expect(rules).toContain('flow-node-write-unknown-field');
expect(rules).toContain('flow-update-readonly-field');
expect(rules).toContain('hook-api-update-readonly-field');
expect(rules).toContain('react-prop-missing-required');
});

Expand Down
17 changes: 17 additions & 0 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ import { validateHookBodyWrites } from './validate-hook-body-writes.js';
import { validateActionBodyWrites } from './validate-action-body-writes.js';
import { validateFlowNodeWrites } from './validate-flow-node-writes.js';
import { validateReadonlyFlowWrites } from './validate-readonly-flow-writes.js';
import { validateReadonlyHookWrites } from './validate-readonly-hook-writes.js';
import { validateReactPageProps } from './validate-react-page-props.js';

export type ReferenceIntegritySeverity = 'error' | 'warning';
Expand Down Expand Up @@ -275,6 +276,22 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
// build the other command would have stopped. Joining the suite is the whole
// fix; the two hand-wired call sites are deleted with it (#4345 follow-up).
{ name: 'validateReadonlyFlowWrites', run: validateReadonlyFlowWrites },
// [#13653] The SAME question as the member above, on the surface that had no
// answer for it: a hook body's `ctx.api.object('x').update({ readonlyField })`.
// A hook's `ctx.api` is a ScopedContext over the TRIGGERING operation's
// context, so on a non-system trigger the engine strips the key and the call
// still returns success — the flow rule's silent no-op, reached through JS
// instead of through `config.fields`.
//
// It gates for the flow member's reason and NOT for its neighbour
// `validateHookBodyWrites`': both halves of the judgement are declared in
// THIS stack (the field's `readonly`, the body's literal `ctx.api` update),
// so the finding does not depend on a package the build cannot see. What it
// must never touch is the `ctx.input` stamp — a before-hook writing a
// `readonly` field is CORRECT and widely used, because the strip drops only
// caller-supplied values (#5591) — so the rule keys on the write CHANNEL,
// and both directions are pinned in its tests.
{ name: 'validateReadonlyHookWrites', run: validateReadonlyHookWrites },
// The `kind:'react'` page surface. Every prop a react block binds BY FIELD
// NAME is resolved against the object it names (#4340) — `<ListView columns>`,
// `<ObjectForm fields>`, `<Block type="element:…">` through the SAME
Expand Down
13 changes: 11 additions & 2 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function asArray(v: unknown): AnyRec[] {
return [];
}

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