Skip to content

Commit 4340f13

Browse files
os-zhuangclaude
andauthored
feat(lint,cli): flag flow update_record writes to readonly fields at design time (#3425) (#3465)
A flow `update_record` node that writes a field the target object declares `readonly: true`, under the default `runAs: 'user'`, is a silent no-op: the objectql engine strips static-`readonly` fields from a non-system UPDATE payload (#2948) so the write never lands, while the step still reports success. #3407/#3413 surfaced the strip at run time; this shifts discovery left to `os validate` / `os build`. - New `@objectstack/lint` rule `validateReadonlyFlowWrites`: static readonly + literal field under runAs!=='system' → error (gates); readonlyWhen → warning. Skips create_record (INSERT is engine-exempt), runAs:'system' flows, templated object names, and non-literal fields maps to stay false-positive-free. - Wired into `os validate` and `os compile`/`os build` (mirrors the security posture gate). Verified: reds on a violating app-crm flow, clean on all example apps. - Documents the formal contract in the objectstack-data / -automation skills: readonly governs the user/API surface; system writers (runAs:'system', hooks, seeds) maintain it. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 37b1346 commit 4340f13

8 files changed

Lines changed: 573 additions & 0 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/cli": minor
4+
---
5+
6+
feat(lint,cli): flag flow `update_record` writes to readonly fields at design time (#3425)
7+
8+
A flow `update_record` node that writes a field the target object declares
9+
`readonly: true`, under the default `runAs: 'user'` identity, is a **silent
10+
no-op**: the objectql engine strips static-`readonly` fields from a non-system
11+
UPDATE payload (#2948), so the intended write never lands — yet the step still
12+
reports `success`. #3407/#3413 surfaced the strip as a run-time step warning;
13+
this moves the discovery **left** to `os validate` / `os build` so an author
14+
finds the mismatch at design time instead of by reading server WARN logs days
15+
later.
16+
17+
- New `@objectstack/lint` rule `validateReadonlyFlowWrites(stack)` — a pure
18+
`(stack) => Finding[]` check (ADR-0019). A static `readonly:true` field
19+
written by a literal `update_record` under `runAs !== 'system'` is a
20+
100%-certain no-op → **error** (gates the build). A `readonlyWhen` field is
21+
per-record-state → **warning** (advisory). Deliberately narrow to stay
22+
false-positive-free: `create_record` (INSERT is engine-exempt from the strip),
23+
`runAs: 'system'` flows (the intended "automation maintains it" channel),
24+
templated object names, and non-literal `fields` maps are all skipped.
25+
- Wired into `os validate` and `os compile`/`os build`, mirroring the existing
26+
security-posture gate (errors fail; advisories print dimmed).
27+
28+
The formal contract, unchanged in behavior: `readonly` governs the end-user /
29+
API surface (REST/UI and `runAs:'user'` flows strip it); trusted system writers
30+
(`runAs:'system'`, system hooks, seeds) maintain it. To let a flow maintain a
31+
readonly field, declare `runAs: 'system'`.

packages/cli/src/commands/compile.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { validateWidgetBindings } from '@objectstack/lint';
1414
import { validateDashboardActionRefs } from '@objectstack/lint';
1515
import { validateResponsiveStyles } from '@objectstack/lint';
1616
import { validateSecurityPosture, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
17+
import { validateReadonlyFlowWrites } from '@objectstack/lint';
1718
import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
1819
import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
1920
import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
@@ -451,6 +452,39 @@ export default class Compile extends Command {
451452
}
452453
}
453454

455+
// 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
456+
// writing a static-`readonly` field is a silent no-op — the engine
457+
// strips it from the UPDATE payload (#2948) while the step reports
458+
// success. This GATES the build (shift-left of the #3407/#3413
459+
// run-time strip warning); `readonlyWhen` writes are per-record-state,
460+
// so they are advisory, printed dimmed and never fatal.
461+
if (!flags.json) printStep('Checking readonly flow writes (#3425)...');
462+
const readonlyWriteFindings = validateReadonlyFlowWrites(result.data as Record<string, unknown>);
463+
const readonlyWriteErrors = readonlyWriteFindings.filter((f) => f.severity === 'error');
464+
const readonlyWriteAdvisories = readonlyWriteFindings.filter((f) => f.severity !== 'error');
465+
if (readonlyWriteErrors.length > 0) {
466+
if (flags.json) {
467+
console.log(JSON.stringify({ success: false, error: 'readonly flow-write validation failed', issues: readonlyWriteErrors }));
468+
this.exit(1);
469+
}
470+
console.log('');
471+
printError(`Readonly flow-write check failed (${readonlyWriteErrors.length} issue${readonlyWriteErrors.length > 1 ? 's' : ''})`);
472+
for (const f of readonlyWriteErrors.slice(0, 50)) {
473+
console.log(` • ${f.where}: ${f.message}`);
474+
console.log(chalk.dim(` ${f.hint}`));
475+
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
476+
}
477+
this.exit(1);
478+
}
479+
if (readonlyWriteAdvisories.length > 0 && !flags.json) {
480+
console.log('');
481+
for (const f of readonlyWriteAdvisories) {
482+
printWarning(`${f.where}: ${f.message}`);
483+
console.log(chalk.dim(` ${f.hint}`));
484+
console.log(chalk.dim(` rule: ${f.rule}`));
485+
}
486+
}
487+
454488
// 3f. [ADR-0090 D6] Access-matrix snapshot gate. Opt-in per app: when
455489
// `access-matrix.json` sits next to the config, the (permission set
456490
// × object) capability matrix derived from THIS build must match it

packages/cli/src/commands/validate.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { validateCapabilityReferences } from '@objectstack/lint';
1919
import { validateVisibilityPredicates } from '@objectstack/lint';
2020
import { validateSecurityPosture } from '@objectstack/lint';
2121
import { validateFlowTriggerReadiness } from '@objectstack/lint';
22+
import { validateReadonlyFlowWrites } from '@objectstack/lint';
2223
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
2324
import {
2425
printHeader,
@@ -427,6 +428,41 @@ export default class Validate extends Command {
427428
}
428429
}
429430

431+
// 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
432+
// that writes a static-`readonly` field is a SILENT no-op — the engine
433+
// strips it from the UPDATE payload (#2948) yet the step reports
434+
// success. This is the shift-left of the run-time strip warning
435+
// (#3407/#3413): a static readonly + literal field is a certain no-op
436+
// → error (gates); a `readonlyWhen` field is per-record-state → advisory.
437+
if (!flags.json) printStep('Checking readonly flow writes (#3425)...');
438+
const readonlyWriteFindings = validateReadonlyFlowWrites(normalized as Record<string, unknown>);
439+
const readonlyWriteErrors = readonlyWriteFindings.filter((f) => f.severity === 'error');
440+
const readonlyWriteWarnings = readonlyWriteFindings.filter((f) => f.severity === 'warning');
441+
if (readonlyWriteErrors.length > 0) {
442+
if (flags.json) {
443+
console.log(JSON.stringify({
444+
valid: false,
445+
errors: readonlyWriteErrors,
446+
duration: timer.elapsed(),
447+
}, null, 2));
448+
this.exit(1);
449+
}
450+
console.log('');
451+
printError(`Readonly flow-write check failed (${readonlyWriteErrors.length} issue${readonlyWriteErrors.length > 1 ? 's' : ''})`);
452+
for (const f of readonlyWriteErrors.slice(0, 50)) {
453+
console.log(` • ${f.where}: ${f.message}`);
454+
console.log(chalk.dim(` ${f.hint}`));
455+
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
456+
}
457+
this.exit(1);
458+
}
459+
if (!flags.json) {
460+
for (const f of readonlyWriteWarnings.slice(0, 50)) {
461+
console.log(chalk.yellow(` ⚠ ${f.where}: ${f.message}`));
462+
console.log(chalk.dim(` ${f.hint}`));
463+
}
464+
}
465+
430466
// 3f. [ADR-0090 D7] Security posture — the same gate `os compile`/`os build`
431467
// run. Without it here, `os validate` passed a stack (e.g. a custom
432468
// object with no explicit sharingModel) that the build then rejected,

packages/lint/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@ export type {
3939
FlowTriggerReadinessSeverity,
4040
} from './validate-flow-trigger-readiness.js';
4141

42+
export {
43+
validateReadonlyFlowWrites,
44+
FLOW_UPDATE_READONLY_FIELD,
45+
FLOW_UPDATE_READONLY_WHEN_FIELD,
46+
} from './validate-readonly-flow-writes.js';
47+
export type {
48+
ReadonlyFlowWriteFinding,
49+
ReadonlyFlowWriteSeverity,
50+
} from './validate-readonly-flow-writes.js';
51+
4252
export { validateViewContainers, VIEW_CONTAINER_SHAPE } from './validate-view-containers.js';
4353
export type { ViewContainerFinding, ViewContainerSeverity } from './validate-view-containers.js';
4454

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
validateReadonlyFlowWrites,
6+
FLOW_UPDATE_READONLY_FIELD,
7+
FLOW_UPDATE_READONLY_WHEN_FIELD,
8+
} from './validate-readonly-flow-writes.js';
9+
10+
// Target object: a static-readonly field, a conditional readonlyWhen field, and
11+
// a plain writable field. Map-shaped `fields` (the common authoring form).
12+
const opportunityObject = {
13+
name: 'crm_opportunity',
14+
label: 'Opportunity',
15+
fields: {
16+
approval_status: { type: 'text', readonly: true },
17+
amount: { type: 'currency', readonlyWhen: "record.stage == 'closed_won'" },
18+
notes: { type: 'text' },
19+
},
20+
};
21+
22+
/** A flow with a single `update_record` node (nodes[1]) writing `fields`. */
23+
function flowWith(
24+
fields: unknown,
25+
flowOverrides: Record<string, unknown> = {},
26+
nodeConfigOverrides: Record<string, unknown> = {},
27+
) {
28+
return {
29+
name: 'stamp_approval',
30+
type: 'record_change',
31+
nodes: [
32+
{ id: 'start', type: 'start', config: {} },
33+
{
34+
id: 'stamp',
35+
type: 'update_record',
36+
label: 'Stamp approval',
37+
config: { objectName: 'crm_opportunity', filter: { id: '{recordId}' }, fields, ...nodeConfigOverrides },
38+
},
39+
{ id: 'end', type: 'end' },
40+
],
41+
edges: [],
42+
...flowOverrides,
43+
};
44+
}
45+
46+
describe('validateReadonlyFlowWrites', () => {
47+
// ── static readonly → ERROR ──────────────────────────────────────────
48+
it('errors when a runAs:user update_record writes a static-readonly field', () => {
49+
const findings = validateReadonlyFlowWrites({
50+
objects: [opportunityObject],
51+
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
52+
});
53+
expect(findings).toHaveLength(1);
54+
expect(findings[0].severity).toBe('error');
55+
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
56+
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.approval_status');
57+
expect(findings[0].message).toContain('approval_status');
58+
expect(findings[0].message).toContain('crm_opportunity');
59+
expect(findings[0].message).toContain('#2948');
60+
expect(findings[0].where).toBe('flow "stamp_approval" › node "Stamp approval"');
61+
});
62+
63+
it('errors when runAs is unauthored (defaults to user)', () => {
64+
const findings = validateReadonlyFlowWrites({
65+
objects: [opportunityObject],
66+
flows: [flowWith({ approval_status: 'approved' })], // no runAs
67+
});
68+
expect(findings).toHaveLength(1);
69+
expect(findings[0].severity).toBe('error');
70+
});
71+
72+
it('resolves the target object via the `object` alias', () => {
73+
const findings = validateReadonlyFlowWrites({
74+
objects: [opportunityObject],
75+
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' }, { objectName: undefined, object: 'crm_opportunity' })],
76+
});
77+
expect(findings).toHaveLength(1);
78+
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
79+
});
80+
81+
it('flags each readonly field written in one node', () => {
82+
const twoReadonly = {
83+
name: 'crm_case',
84+
fields: {
85+
is_sla_violated: { type: 'boolean', readonly: true },
86+
closed_at: { type: 'datetime', readonly: true },
87+
subject: { type: 'text' },
88+
},
89+
};
90+
const flow = {
91+
name: 'close_case',
92+
type: 'record_change',
93+
runAs: 'user',
94+
nodes: [
95+
{ id: 'start', type: 'start', config: {} },
96+
{
97+
id: 'u',
98+
type: 'update_record',
99+
label: 'Close',
100+
config: { objectName: 'crm_case', fields: { is_sla_violated: true, closed_at: '{now}', subject: 'x' } },
101+
},
102+
],
103+
edges: [],
104+
};
105+
const findings = validateReadonlyFlowWrites({ objects: [twoReadonly], flows: [flow] });
106+
expect(findings).toHaveLength(2);
107+
expect(findings.every((f) => f.severity === 'error')).toBe(true);
108+
expect(findings.map((f) => f.path)).toEqual([
109+
'flows[0].nodes[1].config.fields.is_sla_violated',
110+
'flows[0].nodes[1].config.fields.closed_at',
111+
]);
112+
});
113+
114+
it('handles array-shaped object.fields', () => {
115+
const arrObject = {
116+
name: 'crm_lead',
117+
fields: [
118+
{ name: 'converted_account', type: 'lookup', readonly: true },
119+
{ name: 'company', type: 'text' },
120+
],
121+
};
122+
const flow = {
123+
name: 'convert',
124+
type: 'record_change',
125+
runAs: 'user',
126+
nodes: [
127+
{ id: 'start', type: 'start', config: {} },
128+
{ id: 'u', type: 'update_record', label: 'Convert', config: { objectName: 'crm_lead', fields: { converted_account: '{acct}' } } },
129+
],
130+
edges: [],
131+
};
132+
const findings = validateReadonlyFlowWrites({ objects: [arrObject], flows: [flow] });
133+
expect(findings).toHaveLength(1);
134+
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
135+
});
136+
137+
// ── readonlyWhen → WARNING ───────────────────────────────────────────
138+
it('warns (not errors) when writing a readonlyWhen field', () => {
139+
const findings = validateReadonlyFlowWrites({
140+
objects: [opportunityObject],
141+
flows: [flowWith({ amount: 5000 }, { runAs: 'user' })],
142+
});
143+
expect(findings).toHaveLength(1);
144+
expect(findings[0].severity).toBe('warning');
145+
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
146+
expect(findings[0].message).toContain('#3042');
147+
});
148+
149+
it('separates readonly (error) + readonlyWhen (warning) + plain (clean) in one node', () => {
150+
const findings = validateReadonlyFlowWrites({
151+
objects: [opportunityObject],
152+
flows: [flowWith({ approval_status: 'approved', amount: 1, notes: 'hi' }, { runAs: 'user' })],
153+
});
154+
expect(findings).toHaveLength(2);
155+
expect(findings.find((f) => f.severity === 'error')?.path).toBe('flows[0].nodes[1].config.fields.approval_status');
156+
expect(findings.find((f) => f.severity === 'warning')?.path).toBe('flows[0].nodes[1].config.fields.amount');
157+
});
158+
159+
// ── clean: runAs:system is the intended maintenance channel ───────────
160+
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
161+
const findings = validateReadonlyFlowWrites({
162+
objects: [opportunityObject],
163+
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
164+
});
165+
expect(findings).toEqual([]);
166+
});
167+
168+
// ── clean: create_record is engine-exempt from the readonly strip ─────
169+
it('does NOT flag create_record writing a readonly field', () => {
170+
const flow = {
171+
name: 'seed_opp',
172+
type: 'record_change',
173+
runAs: 'user',
174+
nodes: [
175+
{ id: 'start', type: 'start', config: {} },
176+
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { approval_status: 'approved' } } },
177+
],
178+
edges: [],
179+
};
180+
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
181+
expect(findings).toEqual([]);
182+
});
183+
184+
// ── clean: plain writable field ──────────────────────────────────────
185+
it('does NOT flag writes to a plain writable field', () => {
186+
const findings = validateReadonlyFlowWrites({
187+
objects: [opportunityObject],
188+
flows: [flowWith({ notes: 'updated' }, { runAs: 'user' })],
189+
});
190+
expect(findings).toEqual([]);
191+
});
192+
193+
// ── clean: not statically knowable ───────────────────────────────────
194+
it('skips a templated objectName (dynamic target)', () => {
195+
const findings = validateReadonlyFlowWrites({
196+
objects: [opportunityObject],
197+
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' }, { objectName: '{targetObject}' })],
198+
});
199+
expect(findings).toEqual([]);
200+
});
201+
202+
it('skips a non-literal fields map (dynamic write payload)', () => {
203+
const findings = validateReadonlyFlowWrites({
204+
objects: [opportunityObject],
205+
flows: [flowWith('{allFields}', { runAs: 'user' })],
206+
});
207+
expect(findings).toEqual([]);
208+
});
209+
210+
it('skips an object not defined in this stack (another package)', () => {
211+
const findings = validateReadonlyFlowWrites({
212+
objects: [], // crm_opportunity not present
213+
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
214+
});
215+
expect(findings).toEqual([]);
216+
});
217+
218+
it('does NOT flag an unknown field (not this rule’s concern)', () => {
219+
const findings = validateReadonlyFlowWrites({
220+
objects: [opportunityObject],
221+
flows: [flowWith({ nonexistent_field: 'x' }, { runAs: 'user' })],
222+
});
223+
expect(findings).toEqual([]);
224+
});
225+
226+
// ── shape robustness ─────────────────────────────────────────────────
227+
it('returns [] for a stack with no flows', () => {
228+
expect(validateReadonlyFlowWrites({ objects: [opportunityObject] })).toEqual([]);
229+
expect(validateReadonlyFlowWrites({})).toEqual([]);
230+
});
231+
232+
it('falls back to node id then index for the location label', () => {
233+
const flow = {
234+
name: 'f',
235+
runAs: 'user',
236+
nodes: [{ id: 'my_node', type: 'update_record', config: { objectName: 'crm_opportunity', fields: { approval_status: 'x' } } }],
237+
edges: [],
238+
};
239+
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
240+
expect(findings[0].where).toBe('flow "f" › node "my_node"');
241+
expect(findings[0].path).toBe('flows[0].nodes[0].config.fields.approval_status');
242+
});
243+
});

0 commit comments

Comments
 (0)