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
23 changes: 23 additions & 0 deletions .changeset/readonly-action-api-write-lint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@objectstack/lint': minor
---

Add `validateReadonlyActionWrites` — an author-time warning on an action body writing a `readonlyWhen` field through `ctx.api`.

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:

| channel | static `readonly` | `readonlyWhen`, predicate TRUE |
| --- | --- | --- |
| action body `ctx.api` | lands | **stripped** |
| hook body `ctx.api`, non-system trigger | stripped | stripped |
| `ctx.api.sudo()` | lands | **stripped** |

So exactly one shape is a silent no-op on this surface, and that is what the new rule reports:

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

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.

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

`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).
2 changes: 2 additions & 0 deletions content/docs/automation/hook-bodies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ The dropped case is the dangerous one: nothing fails, the step reports success,

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

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.

### 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: 11 additions & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,17 @@ export type {
ReadonlyHookWriteSeverity,
} from './validate-readonly-hook-writes.js';

export {
validateReadonlyActionWrites,
ACTION_API_UPDATE_READONLY_WHEN_FIELD,
READONLY_ACTION_WRITE_PATTERN_IDS,
READONLY_ACTION_WRITE_EXCLUSIONS,
} from './validate-readonly-action-writes.js';
export type {
ReadonlyActionWriteFinding,
ReadonlyActionWriteSeverity,
} from './validate-readonly-action-writes.js';

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

Expand Down
27 changes: 27 additions & 0 deletions packages/lint/src/reference-integrity-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ describe('reference-integrity suite — membership', () => {
// `ctx.api` update to a declared-`readonly` field, placed beside the flow
// twin that asks the identical question one surface over.
'validateReadonlyHookWrites',
// [#13770] The third write surface. Same question, and the one place the
// family's answer differs: an action body is elevated, so only the
// CONDITIONAL half of the readonly judgement survives there.
'validateReadonlyActionWrites',
'validateReactPageProps',
]);
});
Expand All @@ -68,6 +72,12 @@ describe('reference-integrity suite — every member actually runs', () => {
fields: {
name: { type: 'text', label: 'Name' },
locked: { type: 'boolean', label: 'Locked', readonly: true },
// validateReadonlyActionWrites (#13770): a CONDITIONAL lock, which is
// the one readonly shape an ACTION body cannot write — an action runs
// elevated, and `isSystem` exempts the static strip but never the
// conditional one. A separate field from `locked` on purpose: the two
// rules must be able to go silent independently.
frozen_note: { type: 'text', label: 'Frozen note', readonlyWhen: "record.locked == true" },
// validateSortableFields (#9257): a virtual field, so it is a REAL
// field name (existence passes) with no stored column behind it.
days_open: { type: 'formula', label: 'Days Open' },
Expand Down Expand Up @@ -118,6 +128,22 @@ describe('reference-integrity suite — every member actually runs', () => {
"ctx.record.name = 'scored'; await ctx.api.object('crm_lead').update({ lead_score: 100 });",
},
},
// validateReadonlyActionWrites (#13770): `frozen_note` EXISTS on crm_lead
// and is `readonlyWhen`, so this is not an existence question — on a
// record whose predicate is TRUE the engine drops the key from the UPDATE
// payload and the action still returns success. A SEPARATE action from
// `score_now` on purpose, mirroring the hook split above: one body
// carrying both defects would let either rule go silent behind the
// other's finding.
{
name: 'freeze_now',
label: 'Freeze Now',
objectName: 'crm_lead',
body: {
language: 'js',
source: "await ctx.api.object('crm_lead').update({ id: ctx.recordId, frozen_note: 'x' });",
},
},
],
views: [
{
Expand Down Expand Up @@ -308,6 +334,7 @@ describe('reference-integrity suite — every member actually runs', () => {
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('action-api-update-readonly-when-field');
expect(rules).toContain('react-prop-missing-required');
});

Expand Down
20 changes: 20 additions & 0 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ 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 { validateReadonlyActionWrites } from './validate-readonly-action-writes.js';
import { validateReactPageProps } from './validate-react-page-props.js';

export type ReferenceIntegritySeverity = 'error' | 'warning';
Expand Down Expand Up @@ -292,6 +293,25 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
// caller-supplied values (#5591) — so the rule keys on the write CHANNEL,
// and both directions are pinned in its tests.
{ name: 'validateReadonlyHookWrites', run: validateReadonlyHookWrites },
// [#13770] The THIRD write surface, and the one place the family's answer
// differs. An action body's `ctx.api` is `createContext({ ...ec, isSystem:
// true })` — elevated by design (#3914) — so the engine's STATIC readonly
// strip, which runs only under `!opCtx.context?.isSystem`, is skipped and a
// `readonly:true` write LANDS here. The conditional strip is not skipped
// (`isSystem` is explicitly not an exemption for it, #9107 LOCK 2), so what
// this member reports is exactly one shape: a `readonlyWhen` field written
// through a literal `ctx.api` update, which silently does not land on records
// whose predicate is TRUE. It advises rather than gates for the reason the
// flow and hook siblings advise on the same shape — the outcome depends on
// the ROW, not on anything this stack declares.
//
// `ctx.record` is excluded from its match set entirely, and that exclusion is
// the rule's load-bearing decision rather than an omission: an action's
// `ctx.record` is a dead snapshot the runtime never writes back, so no strip
// is ever consulted and a readonly verdict there would be false on every
// occurrence. `action-record-write-discarded` already owns that shape and
// states its real reason.
{ name: 'validateReadonlyActionWrites', run: validateReadonlyActionWrites },
// 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
20 changes: 17 additions & 3 deletions packages/lint/src/validate-action-body-writes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,14 @@ function asArray(v: unknown): AnyRec[] {
return [];
}

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

Expand Down
Loading
Loading