Skip to content

Commit 25fc0e4

Browse files
xuyushun441-sysos-zhuangclaude
authored
feat(cli): validate all flat record-scoped predicates at build time (ADR-0032) (#2187)
Extends the action-predicate guard to the remaining flat record-scoped sites, catching bare field references that silently mis-behave at runtime: - field conditional rules (requiredWhen/readonlyWhen/conditionalRequired/visibleWhen) — fail-open when broken - sharing-rule condition (security: which rows a principal sees) - lifecycle hook condition (skips handler when false) - nested `when` on conditional validation rules formula: add `parent` to record-scope namespace roots — master-detail inline grids inject the header record as `parent` for a child field's readonlyWhen (ADR-0036, #1581), so `parent.status` is legitimate, not a bare ref. Verified: full monorepo build 76 tasks clean (zero false positives across all examples + platform bundles); cli validator suite 25 cases (5 new). Deeply-nested UI visibility (view/page tree, object field-group visibleOn, app-nav) is deferred — it needs a recursive walker + per-node scope rules. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e2601c1 commit 25fc0e4

4 files changed

Lines changed: 120 additions & 0 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/cli": minor
3+
"@objectstack/formula": patch
4+
---
5+
6+
build: extend ADR-0032 predicate validation to all flat record-scoped sites
7+
8+
Builds on the action-predicate guard. `os build` now also validates these
9+
record-scoped predicates for bare field references (`status` instead of
10+
`record.status`), which otherwise evaluate to nothing at runtime and silently
11+
mis-behave:
12+
13+
- **field conditional rules**`requiredWhen`, `readonlyWhen`,
14+
`conditionalRequired`, `visibleWhen` (server-enforced; a broken one is
15+
fail-open — the required/readonly rule just never fires);
16+
- **sharing-rule `condition`** (security-critical — decides which rows a
17+
principal sees);
18+
- **lifecycle hook `condition`** (skips the handler when false);
19+
- **nested `when`** on `conditional` validation rules (previously only the
20+
top-level rule predicate was checked).
21+
22+
`@objectstack/formula`: adds `parent` to the record-scope namespace roots —
23+
master-detail inline grids inject the header record as `parent` for a child
24+
field's `readonlyWhen`/`requiredWhen` (ADR-0036, #1581), so `parent.status` is
25+
legitimate, not a bare ref. Verified against the full monorepo build (76 tasks
26+
clean).
27+
28+
Not yet covered (separate follow-up — needs a recursive view/page tree walker
29+
and per-node scope classification): deeply-nested UI visibility predicates
30+
(`view` element/section `visibleOn`/`condition`, `page` component `visibility`),
31+
object field-group `visibleOn`, and app-nav `visible` (user/feature-scoped, not
32+
record-scoped).

packages/cli/src/utils/validate-expressions.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,4 +287,60 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
287287
expect(issues.filter(i => i.where.includes("action 'mark_done' visible"))).toHaveLength(1);
288288
});
289289
});
290+
291+
describe('record-scoped coverage extensions (field rules / sharing / hooks / nested when)', () => {
292+
it('flags a bare-field `readonlyWhen`/`requiredWhen` on a field', () => {
293+
const issues = validateStackExpressions({
294+
objects: [{
295+
name: 'showcase_task',
296+
fields: {
297+
done: { type: 'boolean', readonlyWhen: 'done == true' },
298+
title: { type: 'text', requiredWhen: 'status == "x"' },
299+
},
300+
}],
301+
});
302+
expect(issues.some(i => i.where.includes('readonlyWhen') && /bare reference `done`/.test(i.message))).toBe(true);
303+
expect(issues.some(i => i.where.includes('requiredWhen') && /bare reference `status`/.test(i.message))).toBe(true);
304+
});
305+
306+
it('accepts record-qualified field rules and the master-detail `parent` namespace', () => {
307+
const issues = validateStackExpressions({
308+
objects: [{
309+
name: 'inv_line',
310+
fields: {
311+
qty: { type: 'number', readonlyWhen: "parent.status == 'paid'" },
312+
note: { type: 'text', requiredWhen: 'record.qty >= 100' },
313+
},
314+
}],
315+
});
316+
expect(issues).toHaveLength(0);
317+
});
318+
319+
it('flags a bare-field sharing-rule condition', () => {
320+
const issues = validateStackExpressions({
321+
objects: [{ name: 'crm_account', fields: { region: { type: 'text' } } }],
322+
sharingRules: [{ name: 'sales_region', object: 'crm_account', condition: 'region == "EMEA"' }],
323+
});
324+
expect(issues.some(i => i.where.includes("sharingRule 'sales_region'") && /bare reference `region`/.test(i.message))).toBe(true);
325+
});
326+
327+
it('flags a bare-field hook condition', () => {
328+
const issues = validateStackExpressions({
329+
objects: [{ name: 'crm_lead', fields: { status: { type: 'select' } } }],
330+
hooks: [{ name: 'on_close', object: 'crm_lead', condition: 'status == "closed"' }],
331+
});
332+
expect(issues.some(i => i.where.includes("hook 'on_close'") && /bare reference `status`/.test(i.message))).toBe(true);
333+
});
334+
335+
it('flags a bare-field nested `when` on a conditional validation rule', () => {
336+
const issues = validateStackExpressions({
337+
objects: [{
338+
name: 'crm_account',
339+
fields: { tier: { type: 'select' } },
340+
validations: [{ name: 'cond', type: 'conditional', when: 'tier == "gold"', then: { type: 'required' } }],
341+
}],
342+
});
343+
expect(issues.some(i => i.where.includes('when') && /bare reference `tier`/.test(i.message))).toBe(true);
344+
});
345+
});
290346
});

packages/cli/src/utils/validate-expressions.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,24 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
142142
// Common predicate keys across rule shapes. Validation predicates are
143143
// `record`-scoped — no field flattening — so bare refs are flagged (#1928).
144144
check(where, rule.expression ?? rule.predicate ?? rule.condition ?? rule.formula, objectName, 'record');
145+
// `conditional` rules carry a nested `when` predicate (record-scoped).
146+
check(`${where} when`, (rule as AnyRec).when, objectName, 'record');
145147
}
146148
// Field-level formulas (computed fields) reference the same object.
147149
const fields = obj.fields;
148150
const fieldList = Array.isArray(fields)
149151
? (fields as AnyRec[])
150152
: (fields && typeof fields === 'object' ? Object.values(fields as AnyRec) as AnyRec[] : []);
151153
for (const f of fieldList) {
154+
// Field-level conditional rules are server-enforced (rule-validator) and
155+
// record-scoped — a bare ref silently fails the rule (required/readonly
156+
// not enforced = data-integrity hole). #1928 class, same as actions.
157+
if (f && typeof f === 'object') {
158+
const fname = (f.name as string) ?? '?';
159+
for (const key of ['requiredWhen', 'readonlyWhen', 'conditionalRequired', 'visibleWhen'] as const) {
160+
check(`object '${objectName}' · field '${fname}' ${key}`, (f as AnyRec)[key], objectName, 'record');
161+
}
162+
}
152163
if (f && typeof f === 'object' && f.formula) {
153164
// formulas are `value` role (any return type), still CEL. They are
154165
// `record`-scoped — `record.<field>`, never bare — so flag bare refs (#1928).
@@ -193,5 +204,23 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
193204
}
194205
}
195206

207+
// ── Sharing-rule predicates (security-critical, record-scoped) ─────
208+
// A criteria sharing rule's `condition` decides which rows a principal sees.
209+
// It is evaluated against the record, so a bare ref silently changes access.
210+
for (const rule of asArray(stack.sharingRules)) {
211+
const ruleObj = typeof rule.object === 'string' ? rule.object : undefined;
212+
const where = `sharingRule '${(rule.name as string) ?? '?'}'${ruleObj ? ` (${ruleObj})` : ''} condition`;
213+
check(where, rule.condition ?? rule.criteria ?? rule.predicate, ruleObj, 'record');
214+
}
215+
216+
// ── Hook `condition` predicates (record-scoped gate) ───────────────
217+
// A lifecycle hook's `condition` skips the handler when false; it is
218+
// evaluated against the record, so a bare ref silently makes the hook
219+
// run on every record (or never) instead of the intended subset.
220+
for (const hook of asArray(stack.hooks)) {
221+
const hookObj = typeof hook.object === 'string' ? hook.object : undefined; // array targets → no single field set
222+
check(`hook '${(hook.name as string) ?? '?'}'${hookObj ? ` (${hookObj})` : ''} condition`, hook.condition, hookObj, 'record');
223+
}
224+
196225
return issues;
197226
}

packages/formula/src/cel-engine.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ const SCOPE_ROOTS = [
5656
// UI action / predicate context (ActionEngine, renderers): the current
5757
// record plus ambient globals exposed to `visible`/`disabled` predicates.
5858
'ctx', 'features',
59+
// Master-detail inline grids inject the header record as `parent` for a
60+
// child field's `readonlyWhen`/`requiredWhen` predicate (ADR-0036, #1581).
61+
'parent',
5962
] as const;
6063

6164
/**

0 commit comments

Comments
 (0)