diff --git a/.changeset/validate-record-scoped-predicates.md b/.changeset/validate-record-scoped-predicates.md new file mode 100644 index 0000000000..14dd1e5b7e --- /dev/null +++ b/.changeset/validate-record-scoped-predicates.md @@ -0,0 +1,32 @@ +--- +"@objectstack/cli": minor +"@objectstack/formula": patch +--- + +build: extend ADR-0032 predicate validation to all flat record-scoped sites + +Builds on the action-predicate guard. `os build` now also validates these +record-scoped predicates for bare field references (`status` instead of +`record.status`), which otherwise evaluate to nothing at runtime and silently +mis-behave: + +- **field conditional rules** — `requiredWhen`, `readonlyWhen`, + `conditionalRequired`, `visibleWhen` (server-enforced; a broken one is + fail-open — the required/readonly rule just never fires); +- **sharing-rule `condition`** (security-critical — decides which rows a + principal sees); +- **lifecycle hook `condition`** (skips the handler when false); +- **nested `when`** on `conditional` validation rules (previously only the + top-level rule predicate was checked). + +`@objectstack/formula`: adds `parent` to the record-scope namespace roots — +master-detail inline grids inject the header record as `parent` for a child +field's `readonlyWhen`/`requiredWhen` (ADR-0036, #1581), so `parent.status` is +legitimate, not a bare ref. Verified against the full monorepo build (76 tasks +clean). + +Not yet covered (separate follow-up — needs a recursive view/page tree walker +and per-node scope classification): deeply-nested UI visibility predicates +(`view` element/section `visibleOn`/`condition`, `page` component `visibility`), +object field-group `visibleOn`, and app-nav `visible` (user/feature-scoped, not +record-scoped). diff --git a/packages/cli/src/utils/validate-expressions.test.ts b/packages/cli/src/utils/validate-expressions.test.ts index 3eb8d35457..673ca7fc0b 100644 --- a/packages/cli/src/utils/validate-expressions.test.ts +++ b/packages/cli/src/utils/validate-expressions.test.ts @@ -287,4 +287,60 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues.filter(i => i.where.includes("action 'mark_done' visible"))).toHaveLength(1); }); }); + + describe('record-scoped coverage extensions (field rules / sharing / hooks / nested when)', () => { + it('flags a bare-field `readonlyWhen`/`requiredWhen` on a field', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_task', + fields: { + done: { type: 'boolean', readonlyWhen: 'done == true' }, + title: { type: 'text', requiredWhen: 'status == "x"' }, + }, + }], + }); + expect(issues.some(i => i.where.includes('readonlyWhen') && /bare reference `done`/.test(i.message))).toBe(true); + expect(issues.some(i => i.where.includes('requiredWhen') && /bare reference `status`/.test(i.message))).toBe(true); + }); + + it('accepts record-qualified field rules and the master-detail `parent` namespace', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'inv_line', + fields: { + qty: { type: 'number', readonlyWhen: "parent.status == 'paid'" }, + note: { type: 'text', requiredWhen: 'record.qty >= 100' }, + }, + }], + }); + expect(issues).toHaveLength(0); + }); + + it('flags a bare-field sharing-rule condition', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'crm_account', fields: { region: { type: 'text' } } }], + sharingRules: [{ name: 'sales_region', object: 'crm_account', condition: 'region == "EMEA"' }], + }); + expect(issues.some(i => i.where.includes("sharingRule 'sales_region'") && /bare reference `region`/.test(i.message))).toBe(true); + }); + + it('flags a bare-field hook condition', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'crm_lead', fields: { status: { type: 'select' } } }], + hooks: [{ name: 'on_close', object: 'crm_lead', condition: 'status == "closed"' }], + }); + expect(issues.some(i => i.where.includes("hook 'on_close'") && /bare reference `status`/.test(i.message))).toBe(true); + }); + + it('flags a bare-field nested `when` on a conditional validation rule', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'crm_account', + fields: { tier: { type: 'select' } }, + validations: [{ name: 'cond', type: 'conditional', when: 'tier == "gold"', then: { type: 'required' } }], + }], + }); + expect(issues.some(i => i.where.includes('when') && /bare reference `tier`/.test(i.message))).toBe(true); + }); + }); }); diff --git a/packages/cli/src/utils/validate-expressions.ts b/packages/cli/src/utils/validate-expressions.ts index 8d1c450b4e..b8a7c75baf 100644 --- a/packages/cli/src/utils/validate-expressions.ts +++ b/packages/cli/src/utils/validate-expressions.ts @@ -142,6 +142,8 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // Common predicate keys across rule shapes. Validation predicates are // `record`-scoped — no field flattening — so bare refs are flagged (#1928). check(where, rule.expression ?? rule.predicate ?? rule.condition ?? rule.formula, objectName, 'record'); + // `conditional` rules carry a nested `when` predicate (record-scoped). + check(`${where} when`, (rule as AnyRec).when, objectName, 'record'); } // Field-level formulas (computed fields) reference the same object. const fields = obj.fields; @@ -149,6 +151,15 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { ? (fields as AnyRec[]) : (fields && typeof fields === 'object' ? Object.values(fields as AnyRec) as AnyRec[] : []); for (const f of fieldList) { + // Field-level conditional rules are server-enforced (rule-validator) and + // record-scoped — a bare ref silently fails the rule (required/readonly + // not enforced = data-integrity hole). #1928 class, same as actions. + if (f && typeof f === 'object') { + const fname = (f.name as string) ?? '?'; + for (const key of ['requiredWhen', 'readonlyWhen', 'conditionalRequired', 'visibleWhen'] as const) { + check(`object '${objectName}' · field '${fname}' ${key}`, (f as AnyRec)[key], objectName, 'record'); + } + } if (f && typeof f === 'object' && f.formula) { // formulas are `value` role (any return type), still CEL. They are // `record`-scoped — `record.`, never bare — so flag bare refs (#1928). @@ -193,5 +204,23 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { } } + // ── Sharing-rule predicates (security-critical, record-scoped) ───── + // A criteria sharing rule's `condition` decides which rows a principal sees. + // It is evaluated against the record, so a bare ref silently changes access. + for (const rule of asArray(stack.sharingRules)) { + const ruleObj = typeof rule.object === 'string' ? rule.object : undefined; + const where = `sharingRule '${(rule.name as string) ?? '?'}'${ruleObj ? ` (${ruleObj})` : ''} condition`; + check(where, rule.condition ?? rule.criteria ?? rule.predicate, ruleObj, 'record'); + } + + // ── Hook `condition` predicates (record-scoped gate) ─────────────── + // A lifecycle hook's `condition` skips the handler when false; it is + // evaluated against the record, so a bare ref silently makes the hook + // run on every record (or never) instead of the intended subset. + for (const hook of asArray(stack.hooks)) { + const hookObj = typeof hook.object === 'string' ? hook.object : undefined; // array targets → no single field set + check(`hook '${(hook.name as string) ?? '?'}'${hookObj ? ` (${hookObj})` : ''} condition`, hook.condition, hookObj, 'record'); + } + return issues; } diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index 99edce72aa..56b0fcbac6 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -56,6 +56,9 @@ const SCOPE_ROOTS = [ // UI action / predicate context (ActionEngine, renderers): the current // record plus ambient globals exposed to `visible`/`disabled` predicates. 'ctx', 'features', + // Master-detail inline grids inject the header record as `parent` for a + // child field's `readonlyWhen`/`requiredWhen` predicate (ADR-0036, #1581). + 'parent', ] as const; /**