Skip to content

Commit 46b53a2

Browse files
claude[bot]claude
andauthored
lint: warn when an action body writes a readonlyWhen field through ctx.api (#13844)
* wip(lint): action-surface readonly rule + wiring * feat(lint): warn when an action body writes a readonlyWhen field through ctx.api Adds validateReadonlyActionWrites, the action-surface member of the readonly write family, wired through REFERENCE_INTEGRITY_RULES. An action body's ctx.api is createContext({ ...callerEnvelope, isSystem: true }), so the engine's static readonly strip - which runs only under !opCtx.context?.isSystem - is skipped and a readonly:true write LANDS there. The conditional strip takes no isSystem exemption, so a readonlyWhen field written through ctx.api is still dropped on records whose predicate is TRUE. Only that second shape is reported, as a warning. ctx.record is excluded from the match set: an action's ctx.record is a dead snapshot the runtime never writes back, so no strip is ever consulted on it and a readonly verdict there would be false on every occurrence. Reuses buildReadonlyIndex from the flow rule and collectActionBodies from the action rule rather than growing a second walk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC * docs(automation): name the action surface in the readonly write-gate enumeration The 'Writing a readonly field' section's table is hook-scoped and its closing sentence enumerated the surfaces carrying the gate (hook, flow). Landing action-api-update-readonly-when-field would have left that enumeration one short, and left the hook-scoped table readable as covering actions on a page titled 'Hook & Action Bodies'. States the measured difference: an action body runs elevated, so the static strip does not apply and a readonly write LANDS there, while the conditional lock is not waived by elevation and does carry across. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 47878f1 commit 46b53a2

8 files changed

Lines changed: 891 additions & 3 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+
Add `validateReadonlyActionWrites` — an author-time warning on an action body writing a `readonlyWhen` field through `ctx.api`.
6+
7+
The action surface is the third write surface in the readonly family, after `flow-update-readonly-field` and `hook-api-update-readonly-field`, and it is the one where the family's answer differs. An action body's `ctx.api` is `createContext({ ...callerEnvelope, isSystem: true })` — elevated by design, so RLS/FLS-bypassing trusted execution is the documented posture — and the engine's **static** readonly strip runs only for non-system callers. Measured against a real engine over a memory driver:
8+
9+
| channel | static `readonly` | `readonlyWhen`, predicate TRUE |
10+
| --- | --- | --- |
11+
| action body `ctx.api` | lands | **stripped** |
12+
| hook body `ctx.api`, non-system trigger | stripped | stripped |
13+
| `ctx.api.sudo()` | lands | **stripped** |
14+
15+
So exactly one shape is a silent no-op on this surface, and that is what the new rule reports:
16+
17+
- `action-api-update-readonly-when-field`**warning**. A literal `ctx.api.object('…').update()` / `.updateById()` in an action body writing a field the named object declares `readonlyWhen`. The conditional strip takes no `isSystem` exemption, so elevation is not a workaround and the hint does not offer one: confirm the call only targets records whose predicate is FALSE, or derive the field in a `beforeUpdate` hook on the target object (a hook-written value is not caller-supplied and does land).
18+
19+
A static-`readonly` counterpart is deliberately **not** shipped: an elevated action write lands on such a field, so the finding would state a falsehood and, at the hook rule's `error` grade, would gate a build over working code.
20+
21+
Wired through `REFERENCE_INTEGRITY_RULES`, so it runs on `os validate`, `os lint` and `os compile` at once. It reuses the existing machinery rather than adding any: `buildReadonlyIndex` from the flow rule for the field metadata, and `collectActionBodies` from the action rule for the body walk (both registration sites, with the merged-action de-duplication that walk owns).
22+
23+
`ctx.record` is excluded from the match set, and that exclusion is the rule's load-bearing decision: an action's `ctx.record` is a dead snapshot the runtime never writes back, so no readonly strip is ever consulted on it and a readonly verdict there would be false on every occurrence. `action-record-write-discarded` already owns that shape and states its real reason. Also skipped, each for a stated reason: `insert` / `create` (INSERT is exempt from both strips), `ctx.input` writes (an action's `ctx.input` is its params bag), 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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ The dropped case is the dangerous one: nothing fails, the step reports success,
265265

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

268+
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.
269+
268270
### Errors from `ctx.api`
269271

270272
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: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,17 @@ export type {
136136
ReadonlyHookWriteSeverity,
137137
} from './validate-readonly-hook-writes.js';
138138

139+
export {
140+
validateReadonlyActionWrites,
141+
ACTION_API_UPDATE_READONLY_WHEN_FIELD,
142+
READONLY_ACTION_WRITE_PATTERN_IDS,
143+
READONLY_ACTION_WRITE_EXCLUSIONS,
144+
} from './validate-readonly-action-writes.js';
145+
export type {
146+
ReadonlyActionWriteFinding,
147+
ReadonlyActionWriteSeverity,
148+
} from './validate-readonly-action-writes.js';
149+
139150
export { validateViewContainers, VIEW_CONTAINER_SHAPE } from './validate-view-containers.js';
140151
export type { ViewContainerFinding, ViewContainerSeverity } from './validate-view-containers.js';
141152

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ describe('reference-integrity suite — membership', () => {
4242
// `ctx.api` update to a declared-`readonly` field, placed beside the flow
4343
// twin that asks the identical question one surface over.
4444
'validateReadonlyHookWrites',
45+
// [#13770] The third write surface. Same question, and the one place the
46+
// family's answer differs: an action body is elevated, so only the
47+
// CONDITIONAL half of the readonly judgement survives there.
48+
'validateReadonlyActionWrites',
4549
'validateReactPageProps',
4650
]);
4751
});
@@ -68,6 +72,12 @@ describe('reference-integrity suite — every member actually runs', () => {
6872
fields: {
6973
name: { type: 'text', label: 'Name' },
7074
locked: { type: 'boolean', label: 'Locked', readonly: true },
75+
// validateReadonlyActionWrites (#13770): a CONDITIONAL lock, which is
76+
// the one readonly shape an ACTION body cannot write — an action runs
77+
// elevated, and `isSystem` exempts the static strip but never the
78+
// conditional one. A separate field from `locked` on purpose: the two
79+
// rules must be able to go silent independently.
80+
frozen_note: { type: 'text', label: 'Frozen note', readonlyWhen: "record.locked == true" },
7181
// validateSortableFields (#9257): a virtual field, so it is a REAL
7282
// field name (existence passes) with no stored column behind it.
7383
days_open: { type: 'formula', label: 'Days Open' },
@@ -118,6 +128,22 @@ describe('reference-integrity suite — every member actually runs', () => {
118128
"ctx.record.name = 'scored'; await ctx.api.object('crm_lead').update({ lead_score: 100 });",
119129
},
120130
},
131+
// validateReadonlyActionWrites (#13770): `frozen_note` EXISTS on crm_lead
132+
// and is `readonlyWhen`, so this is not an existence question — on a
133+
// record whose predicate is TRUE the engine drops the key from the UPDATE
134+
// payload and the action still returns success. A SEPARATE action from
135+
// `score_now` on purpose, mirroring the hook split above: one body
136+
// carrying both defects would let either rule go silent behind the
137+
// other's finding.
138+
{
139+
name: 'freeze_now',
140+
label: 'Freeze Now',
141+
objectName: 'crm_lead',
142+
body: {
143+
language: 'js',
144+
source: "await ctx.api.object('crm_lead').update({ id: ctx.recordId, frozen_note: 'x' });",
145+
},
146+
},
121147
],
122148
views: [
123149
{
@@ -308,6 +334,7 @@ describe('reference-integrity suite — every member actually runs', () => {
308334
expect(rules).toContain('flow-node-write-unknown-field');
309335
expect(rules).toContain('flow-update-readonly-field');
310336
expect(rules).toContain('hook-api-update-readonly-field');
337+
expect(rules).toContain('action-api-update-readonly-when-field');
311338
expect(rules).toContain('react-prop-missing-required');
312339
});
313340

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ 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';
9999
import { validateReadonlyHookWrites } from './validate-readonly-hook-writes.js';
100+
import { validateReadonlyActionWrites } from './validate-readonly-action-writes.js';
100101
import { validateReactPageProps } from './validate-react-page-props.js';
101102

102103
export type ReferenceIntegritySeverity = 'error' | 'warning';
@@ -292,6 +293,25 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
292293
// caller-supplied values (#5591) — so the rule keys on the write CHANNEL,
293294
// and both directions are pinned in its tests.
294295
{ name: 'validateReadonlyHookWrites', run: validateReadonlyHookWrites },
296+
// [#13770] The THIRD write surface, and the one place the family's answer
297+
// differs. An action body's `ctx.api` is `createContext({ ...ec, isSystem:
298+
// true })` — elevated by design (#3914) — so the engine's STATIC readonly
299+
// strip, which runs only under `!opCtx.context?.isSystem`, is skipped and a
300+
// `readonly:true` write LANDS here. The conditional strip is not skipped
301+
// (`isSystem` is explicitly not an exemption for it, #9107 LOCK 2), so what
302+
// this member reports is exactly one shape: a `readonlyWhen` field written
303+
// through a literal `ctx.api` update, which silently does not land on records
304+
// whose predicate is TRUE. It advises rather than gates for the reason the
305+
// flow and hook siblings advise on the same shape — the outcome depends on
306+
// the ROW, not on anything this stack declares.
307+
//
308+
// `ctx.record` is excluded from its match set entirely, and that exclusion is
309+
// the rule's load-bearing decision rather than an omission: an action's
310+
// `ctx.record` is a dead snapshot the runtime never writes back, so no strip
311+
// is ever consulted and a readonly verdict there would be false on every
312+
// occurrence. `action-record-write-discarded` already owns that shape and
313+
// states its real reason.
314+
{ name: 'validateReadonlyActionWrites', run: validateReadonlyActionWrites },
295315
// The `kind:'react'` page surface. Every prop a react block binds BY FIELD
296316
// NAME is resolved against the object it names (#4340) — `<ListView columns>`,
297317
// `<ObjectForm fields>`, `<Block type="element:…">` through the SAME

packages/lint/src/validate-action-body-writes.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,14 @@ function asArray(v: unknown): AnyRec[] {
206206
return [];
207207
}
208208

209-
/** One L2 action body found in the stack, with the location to report it at. */
210-
interface ActionBodySite {
209+
/**
210+
* One L2 action body found in the stack, with the location to report it at.
211+
*
212+
* Exported alongside {@link collectActionBodies} for
213+
* `validate-readonly-action-writes.ts` (#13770), which walks the identical set
214+
* of bodies to ask a different question about them.
215+
*/
216+
export interface ActionBodySite {
211217
name: string;
212218
source: string;
213219
path: string;
@@ -259,8 +265,16 @@ function actionObjectBinding(action: AnyRec, parentObject?: string): string | un
259265
* a non-`script` body here would produce advice about writes that provably
260266
* never happen — noise pointing at metadata whose real defect is the `type`,
261267
* which the publish gate already names with its own prescription.
268+
*
269+
* Exported for `validate-readonly-action-writes.ts` (#13770) — shared rather
270+
* than copied, for the reason `buildReadonlyIndex` is shared with the hook
271+
* rule: two readings of "which bodies are there, and where do I report them?"
272+
* that drift produce two rules disagreeing about the same body, and the
273+
* disagreement is silent. Every subtlety above (both registration sites, the
274+
* by-VALUE de-duplication of a merged action, the `type: 'script'` default, the
275+
* authored-location path) is one this rule's sibling must get identically right.
262276
*/
263-
function collectActionBodies(stack: AnyRec): ActionBodySite[] {
277+
export function collectActionBodies(stack: AnyRec): ActionBodySite[] {
264278
const sites: ActionBodySite[] = [];
265279
const seen = new Set<string>();
266280

0 commit comments

Comments
 (0)