Skip to content

Commit ffdc20d

Browse files
committed
feat(formula): dateField == today() now matches — AST temporal-comparison rewrite (#3183)
A `Field.date` reads back as a "YYYY-MM-DD" string (ADR-0053 Phase 1), and cel-js's equality treats a string and a timestamp as unequal without consulting any overload, so `record.due_date == today()` silently returned false (and `!= today()` silently true) even for a same-day record. celEngine.evaluate now rewrites the parsed AST: for each `==`/`!=` whose one operand is today()/daysFromNow()/daysAgo()/now(), the field operand is wrapped in date(...) (the stdlib coercion), then serialized and evaluated — so `record.due_date == today()` runs as `date(record.due_date) == today()`. - Per-occurrence: a mixed `d == "literal" || d == today()` keeps the literal comparison intact while fixing the temporal one. - Type-blind-safe: date() degrades gracefully (already-Date passes through; non-date string / null → Invalid Date → stays false), so no field types are needed and no currently-correct result is worsened. - Cheap: reserializes only when such a comparison is present (plain-includes gate) and memoizes source → rewritten source. AST-based, no regex on input. Covers every interpreter site (formulas, defaults, validation, hooks, flow conditions) via the single evaluate chokepoint. RLS/sharing unaffected (cel-to-filter rejects function calls loudly). Supersedes the #3192 advisory lint (checkTemporalDateEquality + the temporalEqualityFields helper), now removed — with the runtime fixed it would be a false alarm. Flips the #3181 KNOWN GAP characterization test to assert the fix, and adds an objectql end-to-end test (a date formula field read via find()). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuiM565BZ3TR1VD3prMguB
1 parent 5754a23 commit ffdc20d

7 files changed

Lines changed: 222 additions & 234 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
feat(formula): `dateField == today()` now matches — AST temporal-comparison rewrite (#3183)
6+
7+
**Behavior change (the fix):** a `Field.date` compared with `==`/`!=` against a
8+
temporal function now matches on the calendar day. Previously it **silently
9+
returned the wrong answer**`record.due_date == today()` was always `false`
10+
(and `!= today()` always `true`) even for a same-day record, because a
11+
`Field.date` reads back as a `YYYY-MM-DD` **string** (ADR-0053 Phase 1) and
12+
cel-js's equality (`overloads.js` `isEqual`) treats a string and a timestamp as
13+
unequal without consulting any overload.
14+
15+
`celEngine.evaluate` now rewrites the parsed AST: for each `==`/`!=` whose one
16+
operand is `today()`/`daysFromNow()`/`daysAgo()`/`now()`, the **field operand**
17+
is wrapped in `date(...)` (the stdlib coercion), then the expression is
18+
serialized and evaluated. So `record.due_date == today()` runs as
19+
`date(record.due_date) == today()`.
20+
21+
- **Per-occurrence**, not per-field: `record.d == "2026-06-20" || record.d == today()`
22+
keeps the string-literal comparison intact while fixing the temporal one.
23+
- **Type-blind-safe**: `date()` degrades gracefully — an already-`Date`
24+
(`Field.datetime`) operand passes through; a non-date string or null →
25+
`Invalid Date` → the comparison stays `false`, exactly as before. No
26+
field-type information is needed, and no currently-correct result is worsened.
27+
- **Cheap**: the rewrite only reserializes when such a comparison is present
28+
(a plain-`includes` gate skips the rest), and is memoized per source string.
29+
30+
Applies to every interpreter site — read-time `Field.formula`, default values,
31+
validation rules, hook conditions, and flow conditions — since all route through
32+
`celEngine.evaluate`. RLS/sharing conditions are unaffected: they compile via
33+
`cel-to-filter`, which already rejects function calls as a loud authoring error.
34+
35+
**Supersedes the #3192 advisory lint.** That build-time warning
36+
(`checkTemporalDateEquality`) flagged `dateField == today()` as a silent-miss;
37+
with the runtime fixed it would be a false alarm, so it (and the
38+
`temporalEqualityFields` helper it used) is removed. Authors can now write the
39+
natural `record.due_date == today()` directly; the `date(...)` /
40+
`daysBetween(...) == 0` / range idioms all keep working.

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

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

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

@@ -169,20 +169,23 @@ describe('celEngine', () => {
169169
.toEqual({ ok: true, value: true });
170170
});
171171

172-
it('KNOWN GAP: bare `date-string == today()` silently returns false (cel-js equality)', () => {
173-
// Characterization guard, NOT an endorsement. cel-js's `isEqual`
174-
// (overloads.js) hard-codes `string == X` to false and never consults a
175-
// registered overload, so a bare `Field.date` string compared with `==`
176-
// silently misses — independent of timezone (fails identically at UTC).
177-
// The fix must hydrate date fields to Date in the data layer (where field
178-
// types are known); tracked as a separate follow-up. Authors should use
179-
// the idioms in the test above until then. If this starts returning true,
180-
// the follow-up landed — update/remove this guard.
181-
const now = new Date('2026-11-02T04:30:00Z');
182-
const r = celEngine.evaluate(cel('record.due_date == today()'), {
183-
now, timezone: 'America/New_York', record: { due_date: '2026-11-01' },
184-
});
185-
expect(r).toEqual({ ok: true, value: false });
172+
it('bare `date-string == today()` now matches (the #3183 runtime fix)', () => {
173+
// Previously the KNOWN GAP: cel-js's `isEqual` hard-codes `string == X` to
174+
// false, so a bare `Field.date` string never equalled the Timestamp from
175+
// today(). The engine now rewrites the field operand to `date(record.d)`
176+
// (AST temporal-comparison rewrite, #3183), so it compares two Timestamps
177+
// and matches on the reference-tz calendar day.
178+
const now = new Date('2026-11-02T04:30:00Z'); // Nov 1 in NY
179+
const ny = { now, timezone: 'America/New_York', record: { due_date: '2026-11-01' } };
180+
expect(celEngine.evaluate(cel('record.due_date == today()'), ny))
181+
.toEqual({ ok: true, value: true });
182+
// The `!=` dual is now correctly false for a same-day record.
183+
expect(celEngine.evaluate(cel('record.due_date != today()'), ny))
184+
.toEqual({ ok: true, value: false });
185+
// A different day still compares unequal.
186+
expect(celEngine.evaluate(cel('record.due_date == today()'), {
187+
now, timezone: 'America/New_York', record: { due_date: '2026-10-31' },
188+
})).toEqual({ ok: true, value: false });
186189
});
187190
});
188191

@@ -511,35 +514,78 @@ describe('celEngine', () => {
511514
});
512515
});
513516

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)
517+
// #3183 — AST rewrite backing the runtime date-equality fix: wrap a field
518+
// operand compared with `==`/`!=` against a temporal function in `date(...)`.
519+
describe('rewriteTemporalEquality (#3183)', () => {
520+
it('wraps the field operand on either side, for all four temporal functions', () => {
521+
expect(rewriteTemporalEquality('record.due == today()')).toBe('date(record.due) == today()');
522+
expect(rewriteTemporalEquality('today() != record.due')).toBe('today() != date(record.due)');
523+
expect(rewriteTemporalEquality('record.due == daysFromNow(3)')).toBe('date(record.due) == daysFromNow(3)');
524+
expect(rewriteTemporalEquality('record.due != daysAgo(7)')).toBe('date(record.due) != daysAgo(7)');
525+
expect(rewriteTemporalEquality('previous.due == now()')).toBe('date(previous.due) == now()');
526+
expect(rewriteTemporalEquality('due == today()')).toBe('date(due) == today()'); // bare (flattened)
527+
});
528+
529+
it('leaves the working idioms, ordering comparisons, and non-temporal equality untouched', () => {
530+
for (const src of [
531+
'date(record.due) == today()', // already coerced — idempotent
532+
'record.due >= today()', // ordering (already works)
533+
'daysBetween(today(), record.due) == 0', // integer compare
534+
'record.a == record.b', // no temporal
535+
'record.due == "2026-06-20"', // string literal, no temporal
536+
]) {
537+
expect(rewriteTemporalEquality(src)).toBe(src);
538+
}
539+
});
540+
541+
it('rewrites per-occurrence — a mixed literal+temporal expression keeps the literal intact', () => {
542+
expect(rewriteTemporalEquality('record.d == "2026-06-20" || record.d == today()'))
543+
.toBe('record.d == "2026-06-20" || date(record.d) == today()');
525544
});
526545

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([]);
546+
it('returns the source unchanged (no throw) on adversarial input — no ReDoS', () => {
547+
// AST-based + a plain-`includes` gate; the parse either bails or is linear.
548+
expect(rewriteTemporalEquality('$'.repeat(5000))).toBe('$'.repeat(5000));
549+
expect(rewriteTemporalEquality('now('.repeat(2000))).toBe('now('.repeat(2000));
550+
});
551+
});
552+
553+
// #3183 — the end-to-end runtime behavior the rewrite delivers: a `Field.date`
554+
// string operand now matches a temporal function under `==`/`!=`, while string
555+
// literals and already-typed operands are unaffected.
556+
describe('date-string == temporal runtime fix (#3183)', () => {
557+
const now = new Date('2026-06-20T08:00:00Z');
558+
const rec = (due: unknown) => ({ now, record: { due } });
559+
560+
it('a date-only string field == today() matches on the same day', () => {
561+
expect(celEngine.evaluate(cel('record.due == today()'), rec('2026-06-20')))
562+
.toEqual({ ok: true, value: true });
563+
expect(celEngine.evaluate(cel('record.due == today()'), rec('2026-06-19')))
564+
.toEqual({ ok: true, value: false });
565+
// same-day record → `!=` is correctly false (previously silently true)
566+
expect(celEngine.evaluate(cel('record.due != today()'), rec('2026-06-20')))
567+
.toEqual({ ok: true, value: false });
532568
});
533569

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']);
570+
it('a string literal comparison is unchanged, even mixed with a temporal one', () => {
571+
// Pre-existing behavior: string == string literal works.
572+
expect(celEngine.evaluate(cel('record.due == "2026-06-20"'), rec('2026-06-20')))
573+
.toEqual({ ok: true, value: true });
574+
// Mixed: literal clause AND temporal clause both correct for a same-day record.
575+
expect(celEngine.evaluate(cel('record.due == "2026-06-20" || record.due == today()'), rec('2026-06-20')))
576+
.toEqual({ ok: true, value: true });
577+
// Mixed, record on neither day: both clauses false.
578+
expect(celEngine.evaluate(cel('record.due == "2026-06-20" || record.due == today()'), rec('2026-06-18')))
579+
.toEqual({ ok: true, value: false });
537580
});
538581

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([]);
582+
it('an already-Date operand and non-date/null operands are unaffected (graceful date() coercion)', () => {
583+
expect(celEngine.evaluate(cel('record.due == today()'), rec(new Date('2026-06-20T00:00:00Z'))))
584+
.toEqual({ ok: true, value: true });
585+
expect(celEngine.evaluate(cel('record.due == today()'), rec('not-a-date')))
586+
.toEqual({ ok: true, value: false });
587+
expect(celEngine.evaluate(cel('record.due == today()'), rec(null)))
588+
.toEqual({ ok: true, value: false });
543589
});
544590
});
545591
});

packages/formula/src/cel-engine.ts

Lines changed: 68 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* third-party plugins can't ship runaway predicates.
1414
*/
1515

16-
import { Environment } from '@marcbachmann/cel-js';
16+
import { Environment, serialize } from '@marcbachmann/cel-js';
1717
import type { Expression } from '@objectstack/spec';
1818

1919
import { buildScope, registerNumericCoercions, registerStdLib } from './stdlib';
@@ -360,36 +360,80 @@ function fieldRefName(node: unknown): string | null {
360360
return null;
361361
}
362362

363+
/** Wrap an AST field-reference node in a `date(...)` call (the stdlib coercion). */
364+
function wrapInDate(node: CelNode): CelNode {
365+
return { op: 'call', args: ['date', [node]] };
366+
}
367+
363368
/**
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.
369+
* #3183 — rewrite each `<field> ==/!= <temporal>()` (either operand order) so the
370+
* FIELD operand is coerced with `date(...)`. A `Field.date` reads back as a
371+
* `YYYY-MM-DD` string and cel-js equality never matches a string against the
372+
* Timestamp that `today()` etc. return, so the bare comparison silently misses;
373+
* `date(record.d) == today()` compares two Timestamps and matches on the calendar
374+
* day. The rewrite is:
375+
* - **per-occurrence** — only the operand paired with a temporal call is wrapped,
376+
* so `record.d == "2026-06-20" || record.d == today()` keeps the string-literal
377+
* comparison intact while fixing the temporal one (no field-wide trade-off);
378+
* - **type-blind-safe** — `date()`/`toDate` degrades gracefully (an already-`Date`
379+
* datetime field passes through; a non-date string / null → `Invalid Date` →
380+
* the comparison stays `false`, exactly as today), so no field-type info is
381+
* needed and a currently-correct result is never worsened;
382+
* - **idempotent** — `date(record.d)` is a `call`, not a field ref, so it is not
383+
* re-wrapped.
384+
*
385+
* Returns the (possibly rewritten) source. Only reserializes when a rewrite
386+
* actually happened — the ~99% case that needs no rewrite evaluates the original
387+
* source untouched. Memoized per source string; a parse fault returns the source
388+
* unchanged (compile()/evaluate() report it).
372389
*/
373-
export function temporalEqualityFields(source: string): string[] {
374-
if (typeof source !== 'string' || !source.trim()) return [];
390+
export function rewriteTemporalEquality(source: string): string {
391+
if (typeof source !== 'string' || !source.trim()) return source;
392+
const cached = temporalRewriteCache.get(source);
393+
if (cached !== undefined) return cached;
394+
// Cheap gate: a rewrite needs an equality operator AND a temporal call.
395+
const gated = (source.includes('==') || source.includes('!='))
396+
&& (source.includes('today') || source.includes('daysFromNow')
397+
|| source.includes('daysAgo') || source.includes('now'));
398+
if (!gated) { rememberRewrite(source, source); return source; }
399+
375400
let ast: unknown;
376401
try {
377402
ast = (recordScopeEnv ??= buildScopedEnv([])).parse(source).ast;
378403
} catch {
379-
return [];
404+
rememberRewrite(source, source);
405+
return source;
380406
}
381-
const out = new Set<string>();
407+
let changed = false;
382408
const visit = (node: unknown): void => {
383409
if (!isCelNode(node)) return;
384410
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); }
411+
const args = node.args as unknown[];
412+
const [left, right] = args;
413+
// Wrap the field operand paired with a temporal call. Guard `fieldRefName`
414+
// so we never wrap a literal, another call, or an arithmetic sub-tree.
415+
if (isTemporalCall(left) && isCelNode(right) && fieldRefName(right)) { args[1] = wrapInDate(right); changed = true; }
416+
else if (isTemporalCall(right) && isCelNode(left) && fieldRefName(left)) { args[0] = wrapInDate(left); changed = true; }
388417
}
389418
if (Array.isArray(node.args)) for (const child of node.args) visit(child);
390419
};
391420
visit(ast);
392-
return [...out];
421+
const out = changed ? serialize(ast as Parameters<typeof serialize>[0]) : source;
422+
rememberRewrite(source, out);
423+
return out;
424+
}
425+
426+
/** Bounded memo of source → temporal-equality-rewritten source (#3183). */
427+
const temporalRewriteCache = new Map<string, string>();
428+
const TEMPORAL_REWRITE_CACHE_MAX = 500;
429+
function rememberRewrite(source: string, rewritten: string): void {
430+
// Simple FIFO cap — expression sources are few and long-lived; this only guards
431+
// against an unbounded set of one-off dynamic strings.
432+
if (temporalRewriteCache.size >= TEMPORAL_REWRITE_CACHE_MAX) {
433+
const first = temporalRewriteCache.keys().next().value;
434+
if (first !== undefined) temporalRewriteCache.delete(first);
435+
}
436+
temporalRewriteCache.set(source, rewritten);
393437
}
394438

395439
/** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
@@ -541,8 +585,13 @@ export const celEngine: DialectEngine = {
541585
try {
542586
const env = buildEnv(now, ctx.timezone ?? 'UTC');
543587
const scope = buildScope(ctx);
588+
// #3183 — coerce a date-field operand compared with `==`/`!=` against a
589+
// temporal function (`date(record.d) == today()`), so a `Field.date` string
590+
// matches the Timestamp instead of silently never equalling it. No-op (and
591+
// no reserialize) for any source without such a comparison.
592+
const evalSource = rewriteTemporalEquality(source);
544593
try {
545-
const raw = env.evaluate(source, scope);
594+
const raw = env.evaluate(evalSource, scope);
546595
return { ok: true, value: coerce(raw) as T };
547596
} catch (err) {
548597
// ADR-0032 §1c — string-serialized fields make CEL raise
@@ -558,7 +607,7 @@ export const celEngine: DialectEngine = {
558607
if (!isNumericOverloadError(err)) throw err;
559608
const hydrated = hydrateOverloadStrings(scope) as Record<string, unknown>;
560609
try {
561-
const raw = env.evaluate(source, hydrated);
610+
const raw = env.evaluate(evalSource, hydrated);
562611
return { ok: true, value: coerce(raw) as T };
563612
} catch {
564613
// Hydration did not resolve it — surface the original fault, not the

0 commit comments

Comments
 (0)