Skip to content

Commit 80273c8

Browse files
authored
feat(formula,lint): warn on date field == temporal-function silent-miss (#3192)
A `Field.date` deserializes as a "YYYY-MM-DD" string (ADR-0053 Phase 1), and cel-js's equality hard-codes `string == <timestamp>` to false, so `record.due_date == today()` compiles clean but silently never matches. Adds a build-time advisory warning (ADR-0032 guardrail strategy) that flags `==`/`!=` between a date field and today()/daysFromNow()/daysAgo()/now() across every CEL predicate site, via an AST walk (no regex on uncontrolled input — ReDoS-safe). Warning severity — never fails the build. Runtime fix tracked in #3183.
1 parent 621b2cc commit 80273c8

6 files changed

Lines changed: 323 additions & 4 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
feat(formula): warn when a `date` field is compared to a temporal function with `==`/`!=` (#3183)
6+
7+
A `Field.date` deserializes as a `YYYY-MM-DD` **string** (ADR-0053 Phase 1), and
8+
cel-js's equality hard-codes `string == <timestamp>` to `false` — it returns
9+
`false` for a string left operand without ever consulting a registered overload,
10+
and refuses cross-type object equality (`@marcbachmann/cel-js` `overloads.js`
11+
`isEqual`). So the most natural "is it due today" predicate —
12+
13+
```cel
14+
record.due_date == today() // silently false, even when due_date IS today
15+
record.due_date != today() // silently true for a same-day record
16+
```
17+
18+
— compiles clean, throws nothing, and silently never matches. Same silent-miss
19+
family as #1928; **timezone-independent** (fails identically at UTC) and
20+
cross-cutting (formulas, validation, RLS, flow/action/sharing/hook predicates).
21+
22+
cel-js gives no operator-layer hook to fix the comparison, so this adds a
23+
**build-time advisory warning** (the established ADR-0032 guardrail strategy)
24+
rather than a runtime behavior change. `validateExpression` reuses the shared
25+
`ExprSchemaHint.fieldTypes` (the same per-field type map the #1928 tier-4
26+
soundness check already threads through `@objectstack/lint`) to flag a `==`/`!=`
27+
between a `date` field (`record.`/`previous.`/bare) and
28+
`today()`/`daysFromNow()`/`daysAgo()`/`now()`, with a self-correcting message
29+
pointing at the working idioms: `date(record.d) == today()`, a range
30+
(`>= … && <= …`), or `daysBetween(today(), record.d) == 0`.
31+
32+
Warning severity — never fails the build (the write/validation path may carry a
33+
real `Date`). Restricted to `type: 'date'` (unambiguously a string); `datetime`
34+
is excluded to avoid false positives. Ordering operators (`>=`/`<=`/`<`/`>`)
35+
already work — cel-js *throws* for them, tripping the engine's existing
36+
string-hydration retry — so they are not flagged.
37+
38+
A runtime fix (normalizing the peer of a temporal operand in the data layer)
39+
remains tracked in #3183; a naive "hydrate date fields to `Date`" version would
40+
trade this silent-miss for another (breaking `dateField == "2026-06-20"`), so it
41+
needs its own design.

packages/formula/src/cel-engine.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it } from 'vitest';
22

3-
import { celEngine } from './cel-engine';
3+
import { celEngine, temporalEqualityFields } from './cel-engine';
44
import { CEL_STDLIB_FUNCTIONS } from './validate';
55
import type { Expression } from '@objectstack/spec';
66

@@ -510,4 +510,36 @@ describe('celEngine', () => {
510510
expect(unresolved, `advertised functions that fault at runtime:\n${unresolved.join('\n')}`).toEqual([]);
511511
});
512512
});
513+
514+
// #3183 — AST walk backing the date-equality guardrail. Returns field names
515+
// compared with `==`/`!=` directly against a temporal function; the validator
516+
// filters these by field type. AST-based, so no ReDoS on adversarial source.
517+
describe('temporalEqualityFields (#3183)', () => {
518+
it('finds the field on either side, for all four temporal functions', () => {
519+
expect(temporalEqualityFields('record.due == today()')).toEqual(['due']);
520+
expect(temporalEqualityFields('today() != record.due')).toEqual(['due']);
521+
expect(temporalEqualityFields('record.due == daysFromNow(3)')).toEqual(['due']);
522+
expect(temporalEqualityFields('record.due != daysAgo(7)')).toEqual(['due']);
523+
expect(temporalEqualityFields('previous.due == now()')).toEqual(['due']);
524+
expect(temporalEqualityFields('due == today()')).toEqual(['due']); // bare (flattened)
525+
});
526+
527+
it('returns nothing for the working idioms or ordering comparisons', () => {
528+
expect(temporalEqualityFields('date(record.due) == today()')).toEqual([]);
529+
expect(temporalEqualityFields('record.due >= today()')).toEqual([]);
530+
expect(temporalEqualityFields('daysBetween(today(), record.due) == 0')).toEqual([]);
531+
expect(temporalEqualityFields('record.a == record.b')).toEqual([]);
532+
});
533+
534+
it('de-duplicates and finds fields nested in a compound predicate', () => {
535+
expect(temporalEqualityFields('record.due == today() || record.due == daysFromNow(1)')).toEqual(['due']);
536+
expect(temporalEqualityFields('record.a == today() && b != now()').sort()).toEqual(['a', 'b']);
537+
});
538+
539+
it('is linear on adversarial input (the CodeQL ReDoS repros) and returns []', () => {
540+
// These would drive the previous regex O(n²); the AST walk parses or bails fast.
541+
expect(temporalEqualityFields('$'.repeat(5000))).toEqual([]);
542+
expect(temporalEqualityFields('now('.repeat(2000))).toEqual([]);
543+
});
544+
});
513545
});

packages/formula/src/cel-engine.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,73 @@ export function firstTypeMismatch(
325325
}
326326
}
327327

328+
/** cel-js temporal functions that return a calendar Timestamp (for #3183). */
329+
const TEMPORAL_FNS = new Set(['today', 'daysFromNow', 'daysAgo', 'now']);
330+
331+
/** A cel-js AST node is `{ op, args }`; `args` is a node[], or a leaf string. */
332+
type CelNode = { op: string; args: unknown };
333+
334+
function isCelNode(v: unknown): v is CelNode {
335+
return typeof v === 'object' && v !== null && typeof (v as CelNode).op === 'string';
336+
}
337+
338+
/** True when `node` is a call to a temporal function (`today()`/`daysFromNow(…)`/…). */
339+
function isTemporalCall(node: unknown): boolean {
340+
return isCelNode(node) && node.op === 'call'
341+
&& Array.isArray(node.args) && typeof node.args[0] === 'string'
342+
&& TEMPORAL_FNS.has(node.args[0]);
343+
}
344+
345+
/**
346+
* If `node` is a field reference — `record.<f>` / `previous.<f>` (member access)
347+
* or a bare `<f>` (flattened flow scope) — return the field name `<f>`, else null.
348+
*/
349+
function fieldRefName(node: unknown): string | null {
350+
if (!isCelNode(node)) return null;
351+
if (node.op === 'id' && typeof node.args === 'string') return node.args; // bare `<f>`
352+
if (node.op === '.' && Array.isArray(node.args) && node.args.length === 2) {
353+
const [base, member] = node.args;
354+
if (isCelNode(base) && base.op === 'id'
355+
&& (base.args === 'record' || base.args === 'previous')
356+
&& typeof member === 'string') {
357+
return member;
358+
}
359+
}
360+
return null;
361+
}
362+
363+
/**
364+
* #3183 — field names compared with `==`/`!=` DIRECTLY against a temporal
365+
* function (`today()`/`daysFromNow()`/`daysAgo()`/`now()`), found by walking the
366+
* cel-js AST (never a regex on the source — no ReDoS). The caller filters these
367+
* by field type; a `Field.date` reads back as a string and cel-js equality never
368+
* matches it against a timestamp, so such a comparison silently misses. The
369+
* `date(…)`/`datetime(…)`/`timestamp(…)` coercions are NOT temporal calls, so the
370+
* fixed idiom `date(record.d) == today()` yields nothing. Returns `[]` on a parse
371+
* fault (compile() reports those) or when there is no such comparison.
372+
*/
373+
export function temporalEqualityFields(source: string): string[] {
374+
if (typeof source !== 'string' || !source.trim()) return [];
375+
let ast: unknown;
376+
try {
377+
ast = (recordScopeEnv ??= buildScopedEnv([])).parse(source).ast;
378+
} catch {
379+
return [];
380+
}
381+
const out = new Set<string>();
382+
const visit = (node: unknown): void => {
383+
if (!isCelNode(node)) return;
384+
if ((node.op === '==' || node.op === '!=') && Array.isArray(node.args) && node.args.length === 2) {
385+
const [left, right] = node.args;
386+
if (isTemporalCall(left)) { const f = fieldRefName(right); if (f) out.add(f); }
387+
if (isTemporalCall(right)) { const f = fieldRefName(left); if (f) out.add(f); }
388+
}
389+
if (Array.isArray(node.args)) for (const child of node.args) visit(child);
390+
};
391+
visit(ast);
392+
return [...out];
393+
}
394+
328395
/** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
329396
function coerce(value: unknown): unknown {
330397
if (typeof value === 'bigint') {

packages/formula/src/validate.test.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,12 @@ describe('validateExpression (ADR-0032)', () => {
200200
expect(validateExpression('value', 'record.amount * 2 - 50', schema).warnings).toHaveLength(0);
201201
});
202202

203-
it('does NOT warn on a date field compared to today()/daysFromNow()', () => {
203+
it('does NOT warn on a date field with an ORDERING comparison (they hydrate at runtime)', () => {
204+
// Ordering ops fault → the engine's string-hydration retry fires → they work.
205+
// (Equality `==`/`!=` does NOT — that is the #3183 silent-miss, covered in its
206+
// own block below; this tier-4 check leaves it to the #3183 guardrail.)
204207
expect(validateExpression('predicate', 'record.due <= daysFromNow(30)', schema).warnings).toHaveLength(0);
205-
expect(validateExpression('predicate', 'record.due == today()', schema).warnings).toHaveLength(0);
208+
expect(validateExpression('predicate', 'record.due >= today()', schema).warnings).toHaveLength(0);
206209
});
207210

208211
it('does NOT warn on a select field ordered against a number (option values may be numeric codes)', () => {
@@ -320,3 +323,61 @@ describe('inferExpressionType — coarse value-type of a formula', () => {
320323
expect(inferExpressionType('undeclared_field + 1')).toBe('unknown'); // bare ref, no fields given
321324
});
322325
});
326+
327+
// #3183 — a `Field.date` reads back as a "YYYY-MM-DD" string, and cel-js's
328+
// equality hard-codes `string == <timestamp>` to false, so `record.due_date ==
329+
// today()` silently never matches. Advisory warning that guides the author to a
330+
// working idiom. Derived from the shared `fieldTypes` hint (type === 'date').
331+
describe('temporal date-equality guardrail (#3183)', () => {
332+
const schema = {
333+
objectName: 'task',
334+
fields: ['due_date', 'status'] as const,
335+
fieldTypes: { due_date: 'date', status: 'text' },
336+
scope: 'record',
337+
} as const;
338+
const dateWarns = (src: string, s: Record<string, unknown> = schema) =>
339+
validateExpression('predicate', src, s as never).warnings
340+
.filter(w => /calendar-day \(date\) field/.test(w.message));
341+
342+
it('warns on `dateField == today()` (field on the left)', () => {
343+
const r = validateExpression('predicate', 'record.due_date == today()', schema);
344+
expect(r.ok).toBe(true); // advisory — never fails the build
345+
expect(r.errors).toHaveLength(0);
346+
const w = r.warnings.filter(x => /calendar-day \(date\) field/.test(x.message));
347+
expect(w).toHaveLength(1);
348+
expect(w[0].message).toMatch(/date\(record\.due_date\) == today\(\)/);
349+
});
350+
351+
it('warns on `today() == dateField` and on daysFromNow/daysAgo/now', () => {
352+
expect(dateWarns('today() == record.due_date')).toHaveLength(1);
353+
expect(dateWarns('record.due_date == daysFromNow(3)')).toHaveLength(1);
354+
expect(dateWarns('record.due_date != daysAgo(7)')).toHaveLength(1);
355+
expect(dateWarns('record.due_date == now()')).toHaveLength(1);
356+
});
357+
358+
it('warns on a bare date-field ref (flattened flow-condition scope)', () => {
359+
const flat = { objectName: 'task', fields: ['due_date'], fieldTypes: { due_date: 'date' } };
360+
expect(dateWarns('due_date == today()', flat)).toHaveLength(1);
361+
});
362+
363+
it('does NOT warn on the working idioms', () => {
364+
expect(dateWarns('date(record.due_date) == today()')).toHaveLength(0);
365+
expect(dateWarns('record.due_date >= today() && record.due_date <= today()')).toHaveLength(0);
366+
expect(dateWarns('daysBetween(today(), record.due_date) == 0')).toHaveLength(0);
367+
});
368+
369+
it('does NOT warn on ordering comparisons (they fault→hydrate and work)', () => {
370+
expect(dateWarns('record.due_date >= today()')).toHaveLength(0);
371+
expect(dateWarns('record.due_date < daysFromNow(30)')).toHaveLength(0);
372+
});
373+
374+
it('does NOT warn on a non-date field, or when fieldTypes is absent', () => {
375+
expect(dateWarns('record.status == today()')).toHaveLength(0); // status is text, not date
376+
expect(dateWarns('record.due_date == today()', { objectName: 'task', fields: ['due_date'], scope: 'record' }))
377+
.toHaveLength(0); // no fieldTypes → check skipped
378+
});
379+
380+
it('de-duplicates repeated references to the same field', () => {
381+
expect(dateWarns('record.due_date == today() || record.due_date == daysFromNow(1)')).toHaveLength(1);
382+
});
383+
});

packages/formula/src/validate.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* This validator detects that specific mistake and returns the exact fix.
1818
*/
1919

20-
import { celEngine, firstUndeclaredReference, firstTypeMismatch, inferCelType, type FieldCelType } from './cel-engine';
20+
import { celEngine, firstUndeclaredReference, firstTypeMismatch, inferCelType, temporalEqualityFields, type FieldCelType } from './cel-engine';
2121
import { templateEngine } from './template-engine';
2222

2323
export type FieldRole = 'predicate' | 'value' | 'template';
@@ -144,6 +144,37 @@ function typeSoundnessWarning(
144144
};
145145
}
146146

147+
/**
148+
* #3183 — flag `==`/`!=` between a calendar-day (`date`) field and a temporal
149+
* function (`today()`/`daysFromNow()`/`daysAgo()`/`now()`). A `Field.date` reads
150+
* back as a `YYYY-MM-DD` string (ADR-0053 Phase 1), and cel-js's equality
151+
* (`overloads.js` `isEqual`) treats a string and a timestamp as unequal without
152+
* consulting any overload, so the comparison silently never matches. Advisory
153+
* `warning` only — on the write/validation path the value may be a real `Date` —
154+
* and no-op unless `schema.fieldTypes` marks the referenced field `date`. Derives
155+
* the date fields from the shared `fieldTypes` hint (no separate plumbing).
156+
*/
157+
function checkTemporalDateEquality(
158+
source: string,
159+
schema: ExprSchemaHint | undefined,
160+
warnings: ExprValidationError[],
161+
): void {
162+
const fieldTypes = schema?.fieldTypes;
163+
if (!fieldTypes) return;
164+
// AST-based (no regex on the raw source → no ReDoS); filter to `date` fields.
165+
for (const field of temporalEqualityFields(source)) {
166+
if (fieldTypes[field] !== 'date') continue;
167+
warnings.push({
168+
source,
169+
message:
170+
`\`${field}\` is a calendar-day (date) field, stored as a "YYYY-MM-DD" string, so comparing it to a ` +
171+
`timestamp function with \`==\`/\`!=\` silently never matches (CEL treats a string and a timestamp as ` +
172+
`unequal). Wrap the field to coerce it — \`date(record.${field}) == today()\` — or use a range ` +
173+
`(\`record.${field} >= today() && record.${field} <= today()\`) or \`daysBetween(today(), record.${field}) == 0\`.`,
174+
});
175+
}
176+
}
177+
147178
/** A bare `{x}` that is NOT part of a `{{x}}` mustache hole. */
148179
const SINGLE_BRACE_RE = /(?:^|[^{])\{\s*([A-Za-z_$][\w.$]*)\s*\}(?!\})/;
149180
/** `record.<field>` / `previous.<field>` head references for field-existence. */
@@ -310,6 +341,10 @@ export function validateExpression(
310341
} else {
311342
checkFieldExistence(source, schema, errors);
312343
checkRoleCatalog(source, schema, errors);
344+
// #3183 — date-field `==`/`!=` a temporal function silently never matches.
345+
// Scope-independent (wrong in both record and flattened sites), so run it
346+
// outside the scope branch below.
347+
checkTemporalDateEquality(source, schema, warnings);
313348
if (schema?.scope === 'record') {
314349
// In a `record`-scoped site a bare top-level identifier is a silent bug —
315350
// it must be `record.<field>` (#1928). Hard error.

packages/lint/src/validate-expressions.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,4 +472,87 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
472472
expect(issues.some(i => /columnName/.test(i.message))).toBe(false);
473473
});
474474
});
475+
476+
// #3183 — `date` field `== today()` silently never matches (cel-js equality).
477+
// The stack validator threads each object's field types into the shared
478+
// validator, which flags the pattern with an advisory (non-blocking) warning.
479+
describe('date-field equality guardrail (#3183)', () => {
480+
const objects = [{
481+
name: 'task',
482+
fields: { due_date: { type: 'date' }, title: { type: 'text' } },
483+
}];
484+
const dateWarns = (issues: { message: string }[]) =>
485+
issues.filter(i => /calendar-day \(date\) field/.test(i.message));
486+
487+
it('warns on a formula field comparing a date field to today()', () => {
488+
const issues = validateStackExpressions({
489+
objects: [{
490+
name: 'task',
491+
fields: {
492+
due_date: { type: 'date' },
493+
is_due_today: { type: 'formula', formula: 'record.due_date == today()' },
494+
},
495+
}],
496+
});
497+
const w = dateWarns(issues);
498+
expect(w).toHaveLength(1);
499+
expect(w[0].severity).toBe('warning');
500+
expect(w[0].where).toMatch(/object 'task'.*formula/);
501+
});
502+
503+
it('warns on a validation-rule predicate, and never fails the build (advisory)', () => {
504+
const issues = validateStackExpressions({
505+
objects: [{
506+
name: 'task',
507+
fields: { due_date: { type: 'date' } },
508+
validations: [{ name: 'due_check', expression: 'record.due_date != today()' }],
509+
}],
510+
});
511+
const w = dateWarns(issues);
512+
expect(w).toHaveLength(1);
513+
expect(w.every(i => i.severity === 'warning')).toBe(true);
514+
});
515+
516+
it('warns on a flattened flow condition using a bare date field', () => {
517+
const issues = validateStackExpressions({
518+
objects,
519+
flows: [{
520+
name: 'due_flow',
521+
nodes: [
522+
{ id: 'start', type: 'start', config: { objectName: 'task' } },
523+
{ id: 'check', type: 'decision', config: { condition: 'due_date == today()' } },
524+
],
525+
edges: [],
526+
}],
527+
});
528+
expect(dateWarns(issues)).toHaveLength(1);
529+
});
530+
531+
it('does not warn on the working idioms or ordering comparisons', () => {
532+
const issues = validateStackExpressions({
533+
objects: [{
534+
name: 'task',
535+
fields: {
536+
due_date: { type: 'date' },
537+
due_soon: { type: 'formula', formula: 'record.due_date >= today() && record.due_date <= daysFromNow(7)' },
538+
is_today: { type: 'formula', formula: 'date(record.due_date) == today()' },
539+
},
540+
}],
541+
});
542+
expect(dateWarns(issues)).toHaveLength(0);
543+
});
544+
545+
it('does not warn on a datetime field (may deserialize as a real instant)', () => {
546+
const issues = validateStackExpressions({
547+
objects: [{
548+
name: 'task',
549+
fields: {
550+
signed_at: { type: 'datetime' },
551+
just_signed: { type: 'formula', formula: 'record.signed_at == now()' },
552+
},
553+
}],
554+
});
555+
expect(dateWarns(issues)).toHaveLength(0);
556+
});
557+
});
475558
});

0 commit comments

Comments
 (0)