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
32 changes: 32 additions & 0 deletions .changeset/validate-record-scoped-predicates.md
Original file line numberDiff line numberDiff line change
@@ -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).
56 changes: 56 additions & 0 deletions packages/cli/src/utils/validate-expressions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
});
});
});
29 changes: 29 additions & 0 deletions packages/cli/src/utils/validate-expressions.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -142,13 +142,24 @@ 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;
const fieldList = Array.isArray(fields)
? (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.<field>`, never bare — so flag bare refs (#1928).
Expand DownExpand Up@@ -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;
}
3 changes: 3 additions & 0 deletions packages/formula/src/cel-engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;

/**
Expand Down