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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
21 changes: 21 additions & 0 deletions .changeset/lint-readonly-when-system-flows.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
'@objectstack/lint': patch
---

lint: `flow-update-readonly-when-field` now inspects `runAs:'system'` flows

The `runAs:'system'` exemption in `validate-readonly-flow-writes` was a single
flow-level early return, so it removed an elevated flow from **both** branches of
the rule. Only the static branch warrants it: the engine skips
`stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, but
`stripReadonlyWhenFields` runs on the update path with no `isSystem` guard at all
(`packages/objectql/src/engine.ts`, the #9107 note: "`isSystem` is still NOT an
exemption here, unlike the static strip below"), pinned as "LOCK 2 — isSystem does
NOT exempt a caller-supplied value".

The exemption now gates the static branch only. A `runAs:'system'` flow whose
`update_record` node writes a `readonlyWhen` field reports the branch's existing
`warning` — the same silent-no-op the rule exists to surface, on the flow class the
rule's own hint tells the author elevation cannot save. A system flow writing a
static `readonly:true` field stays silent, as before; rule ids and severities are
unchanged, and the new finding is advisory and never blocks a build.
121 changes: 120 additions & 1 deletion packages/lint/src/validate-readonly-flow-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,14 +197,133 @@ describe('validateReadonlyFlowWrites', () => {
});

// ── clean: runAs:system is the intended maintenance channel ───────────
it('does NOT flag a runAs:system flow (elevated writer bypasses the strip)', () => {
// …for the STATIC strip, and only for it. The engine skips
// `stripReadonlyFields` under `if (!opCtx.context?.isSystem)`, so an elevated
// flow maintaining a `readonly:true` column is the intended channel and stays
// silent. Paired with the `readonlyWhen` case below, which is the OTHER half
// of the same run identity — the two must not move together (#14201).
it('does NOT flag a runAs:system flow writing a STATIC readonly field (elevated writer bypasses that strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(findings).toEqual([]);
});

// ── runAs:system + readonlyWhen → still a WARNING (#14201) ────────────
// `stripReadonlyWhenFields` is called on the update path with NO `isSystem`
// guard at all (engine.ts, the #9107 note: "`isSystem` is still NOT an
// exemption here, unlike the static strip below"), pinned from both sides as
// "LOCK 2 — isSystem does NOT exempt a caller-supplied value"
// (`engine-readonly-when-derived-writes.test.ts`) and "covers readonlyWhen
// too — the arm a trusted (isSystem) caller can still hit"
// (`engine-readonly-strict-writes.test.ts`). So the elevated flow's write
// vanishes on a locked record exactly as a user run's does, and the rule that
// exists to surface that silent no-op has to say so on the very flow class
// its own hint tells the author elevation cannot save.
it('warns when a runAs:system flow writes a readonlyWhen field (elevation does NOT waive the conditional strip)', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ amount: 5000 }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
// The message states the run identity it was judged under, so a reader of
// the finding cannot mistake it for the user-run case.
expect(findings[0].message).toContain("runAs:'system'");
expect(findings[0].message).toContain('#3042');
expect(findings[0].hint).toContain('NOT waived by a system context');
});

it('reports ONLY the conditional half for a runAs:system node writing both kinds in one payload', () => {
const findings = validateReadonlyFlowWrites({
objects: [opportunityObject],
flows: [flowWith({ approval_status: 'approved', amount: 5000, notes: 'hi' }, { runAs: 'system' })],
});
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[1].config.fields.amount');
expect(findings.some((f) => f.rule === FLOW_UPDATE_READONLY_FIELD)).toBe(false);
});

// A field declaring BOTH flags: under `runAs:'system'` the static strip is
// skipped and the conditional one is not, so the truthful finding is the
// warning — not silence (the old flow-level skip) and not the error (which
// would state something false about an elevated write).
it('falls through to the conditional branch for a field declaring readonly AND readonlyWhen under runAs:system', () => {
const bothFlags = {
name: 'crm_opportunity',
fields: {
approval_status: { type: 'text', readonly: true, readonlyWhen: "record.stage == 'closed_won'" },
},
};
const systemFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'system' })],
});
expect(systemFindings).toHaveLength(1);
expect(systemFindings[0].severity).toBe('warning');
expect(systemFindings[0].rule).toBe(FLOW_UPDATE_READONLY_WHEN_FIELD);

// Unchanged for a user run: the static strip applies there, and the certain
// no-op outranks the conditional one.
const userFindings = validateReadonlyFlowWrites({
objects: [bothFlags],
flows: [flowWith({ approval_status: 'approved' }, { runAs: 'user' })],
});
expect(userFindings).toHaveLength(1);
expect(userFindings[0].severity).toBe('error');
expect(userFindings[0].rule).toBe(FLOW_UPDATE_READONLY_FIELD);
});

// Nesting is orthogonal to run identity: the walk reaches an elevated flow's
// nested regions on the conditional branch too.
it('reaches a readonlyWhen write nested in a loop body under runAs:system', () => {
const flow = {
name: 'sweep_system',
runAs: 'system',
nodes: [
{
id: 'each',
type: 'loop',
label: 'Each',
config: {
collection: '{items}',
body: {
nodes: [
{ id: 'u', type: 'update_record', label: 'U', config: { objectName: 'crm_opportunity', fields: { amount: 1 } } },
],
edges: [],
},
},
},
],
edges: [],
};
const findings = validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] });
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].path).toBe('flows[0].nodes[0].config.body.nodes[0].config.fields.amount');
});

// create_record stays exempt on BOTH branches under elevation: a
// `readonlyWhen` predicate has no prior record to evaluate on an insert.
it('does NOT flag create_record writing a readonlyWhen field under runAs:system', () => {
const flow = {
name: 'seed_opp_system',
type: 'record_change',
runAs: 'system',
nodes: [
{ id: 'start', type: 'start', config: {} },
{ id: 'c', type: 'create_record', label: 'Create', config: { objectName: 'crm_opportunity', fields: { amount: 10 } } },
],
edges: [],
};
expect(validateReadonlyFlowWrites({ objects: [opportunityObject], flows: [flow] })).toEqual([]);
});

// ── clean: create_record is engine-exempt from the readonly strip ─────
it('does NOT flag create_record writing a readonly field', () => {
const flow = {
Expand Down
49 changes: 31 additions & 18 deletions packages/lint/src/validate-readonly-flow-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,21 +17,26 @@
// by calling the data engine directly), so a create writing a readonly
// field is NOT a no-op and is never flagged.
//
// • Only `runAs !== 'system'`. A `runAs:'system'` run is elevated and the
// engine skips the STATIC `readonly` strip, so a system flow legitimately
// MAINTAINS readonly fields ("users can't edit this, but automation does").
// That is the intended channel, so it is never flagged.
// • `runAs:'system'` exempts the STATIC branch ONLY - it is not a flow-level
// skip. An elevated run bypasses the static `readonly` strip, so a system
// flow legitimately MAINTAINS readonly fields ("users can't edit this, but
// automation does"). That is the intended channel, so it is never flagged.
//
// ⚠️ That exemption is the STATIC strip's alone. `stripReadonlyWhenFields`
// runs with no `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem`
// is still NOT an exemption here, unlike the static strip below"), pinned as
// "LOCK 2 - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts`. So elevation is NOT a
// `readonlyWhen` remedy, and this rule's hint must never offer it. The skip
// above is therefore WIDER than the conditional lock warrants - a
// `runAs:'system'` flow writing a `readonlyWhen` field is still stripped on a
// locked record and goes unflagged. Left as-is deliberately: the match set is
// out of scope for the message-text correction that fixed the hint.
// ⚠️ The exemption stops there. `stripReadonlyWhenFields` runs with no
// `isSystem` guard at all (engine.ts, the #9107 note: "`isSystem` is still
// NOT an exemption here, unlike the static strip below"), pinned as "LOCK 2
// - isSystem does NOT exempt a caller-supplied value" in
// `engine-readonly-when-derived-writes.test.ts` and from the other side in
// `engine-readonly-strict-writes.test.ts` ("covers readonlyWhen too - the
// arm a trusted (isSystem) caller can still hit"). So a `runAs:'system'`
// flow writing a `readonlyWhen` field IS still stripped on a locked record,
// and the conditional branch inspects an elevated flow exactly like any
// other, at its usual `warning` severity. Narrowing this exemption to the
// branch it belongs to (#14201) is what stops the rule from going silent on
// the one flow class its own hint tells the author elevation cannot save -
// the same split the action sibling was born with
// (`validate-readonly-action-writes.ts`: an action body is system-elevated
// BY DESIGN, so it carries the conditional half and only that half).
//
// • Static `readonly:true` + a LITERAL field name is a 100%-certain no-op →
// ERROR (gates the build). `readonlyWhen` is per-record-state — it strips
Expand DownExpand Up@@ -150,10 +155,13 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind

flows.forEach((flow, flowIndex) => {
// `runAs` defaults to 'user' (schema default). Only an explicit 'system'
// run bypasses the strip, so treat anything else — including an unauthored
// (undefined) runAs — as strip-subject.
if (flow.runAs === 'system') return;
// run bypasses the STATIC strip, so treat anything else — including an
// unauthored (undefined) runAs — as subject to both strips. ⛔ Not a
// flow-level skip: the conditional strip has no `isSystem` guard, so an
// elevated flow stays in the walk and is judged on the `readonlyWhen`
// branch below (#14201).
const runAs = flow.runAs === 'user' || flow.runAs === 'system' ? flow.runAs : 'user';
const isSystemRun = runAs === 'system';

const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`;
// Every node, INCLUDING those nested in try_catch / loop / parallel regions.
Expand DownExpand Up@@ -189,7 +197,12 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind
// the two never double-report the same key.
if (!meta) continue;

if (meta.readonly) {
// The static branch is the one an elevated run really does bypass, so
// `isSystem` gates it HERE rather than at flow level. A field declaring
// BOTH flags therefore still falls through to the conditional branch
// under `runAs:'system'` — which is the truth about that write: the
// static strip is skipped, the conditional one is not.
if (meta.readonly && !isSystemRun) {
findings.push({
severity: 'error',
rule: FLOW_UPDATE_READONLY_FIELD,
Expand Down
Loading