From 67a38f2c432b84495d6502b5a294822af8f8f79a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 09:54:03 +0000 Subject: [PATCH 1/2] fix(formula): hydrate string-serialized numeric fields in CEL comparisons (#1534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Numeric fields that serialize as strings (Field.rating -> "5.0", Field.currency -> "250000.00", Field.percent) made predicates like `record.rating >= 4` fault under strict CEL (`no such overload: dyn >= int`). In automation flow decision/edge conditions this silently dead-ended the run (no edge matched); in objectql applyFormulaPlan it swallowed to null. The shared CEL engine now retries an evaluation once with purely-numeric strings hydrated to numbers, but only after a `no such overload` fault — so a comparison that already type-checks is never re-interpreted, and a genuinely non-numeric operand still faults loudly. Because both the automation condition path and the objectql formula path route through ExpressionEngine.evaluate, both are fixed consistently. https://claude.ai/code/session_01FjtHqsTWWPThCP6jsVdvPa --- .changeset/fix-numeric-string-cel-coercion.md | 9 +++ packages/formula/src/cel-engine.test.ts | 65 +++++++++++++++++ packages/formula/src/cel-engine.ts | 71 ++++++++++++++++++- .../service-automation/src/engine.test.ts | 53 ++++++++++++++ 4 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-numeric-string-cel-coercion.md diff --git a/.changeset/fix-numeric-string-cel-coercion.md b/.changeset/fix-numeric-string-cel-coercion.md new file mode 100644 index 000000000..be87d1b46 --- /dev/null +++ b/.changeset/fix-numeric-string-cel-coercion.md @@ -0,0 +1,9 @@ +--- +"@objectstack/formula": patch +--- + +fix(formula): hydrate string-serialized numeric fields in CEL comparisons (#1534) + +Numeric fields that serialize as strings — `Field.rating(allowHalf)` → `"5.0"`, `Field.currency(scale)` → `"250000.00"`, `Field.percent` — made comparisons like `record.rating >= 4` fault under strict CEL with `no such overload: dyn >= int`. In flow decision/edge conditions this silently dead-ended the run (no edge matched), and in objectql `applyFormulaPlan` it swallowed to `null`. + +The CEL engine now retries an evaluation **once** with purely-numeric strings hydrated to numbers, but only after a `no such overload` fault — so a comparison that already type-checks is never re-interpreted (a zip like `"02134"` stays a string in `record.zip == "02134"`). Because both the automation condition path (`service-automation` `evaluateCondition`) and the objectql formula path route through `ExpressionEngine.evaluate`, both are fixed consistently. A genuinely non-numeric operand (e.g. `record.rating >= 4` where `rating` is `"high"`) still faults loudly rather than being silently rescued. diff --git a/packages/formula/src/cel-engine.test.ts b/packages/formula/src/cel-engine.test.ts index cc31f2670..2180a750b 100644 --- a/packages/formula/src/cel-engine.test.ts +++ b/packages/formula/src/cel-engine.test.ts @@ -82,4 +82,69 @@ describe('celEngine', () => { expect(r.ok).toBe(true); if (r.ok) expect(typeof r.value === 'string' || typeof r.value === 'number').toBe(true); }); + + // ADR-0032 §1c — string-serialized numeric fields (#1530, #1534). + describe('numeric-string field hydration', () => { + it('compares a rating that serializes as "5.0" against an int literal', () => { + const r = celEngine.evaluate(cel('record.rating >= 4'), { + record: { rating: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('compares a currency string against an int literal', () => { + const r = celEngine.evaluate(cel('record.amount > 100000'), { + record: { amount: '250000.00' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('returns false (not a fault) when the hydrated compare is unmet', () => { + const r = celEngine.evaluate(cel('record.rating >= 4'), { + record: { rating: '2.5' }, + }); + expect(r).toEqual({ ok: true, value: false }); + }); + + it('compares a percent string against a number literal', () => { + const r = celEngine.evaluate(cel('record.completion >= 0.8'), { + record: { completion: '0.95' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('hydrates within a compound predicate (the real flow-condition shape)', () => { + const r = celEngine.evaluate( + cel('record.rating >= 4 && record.status == "new"'), + { record: { rating: '5.0', status: 'new' } }, + ); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('hydrates nested numeric strings (e.g. previous.* transition gates)', () => { + const r = celEngine.evaluate(cel('record.amount > previous.amount'), { + record: { amount: '600000.00' }, + previous: { amount: '500000.00' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('leaves genuine string equality untouched (no spurious coercion)', () => { + // string == string already type-checks, so the retry path never runs + // and a numeric-looking string stays a string. + const r = celEngine.evaluate(cel('record.zip == "02134"'), { + record: { zip: '02134' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('does not coerce non-numeric strings', () => { + // "high" is not a number literal, so the compare still faults loudly + // rather than being silently rescued. + const r = celEngine.evaluate(cel('record.rating >= 4'), { + record: { rating: 'high' }, + }); + expect(r.ok).toBe(false); + }); + }); }); diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index 79b5f9aee..dfe646c8f 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -59,6 +59,54 @@ function coerce(value: unknown): unknown { return value; } +/** + * A string that is *entirely* a JS number literal: optional sign, integer + * and/or fractional part, optional exponent. Deliberately strict — `"5.0"`, + * `"250000.00"`, `"-3"`, `"1e3"` match; `"5px"`, `"0x10"`, `" "`, `""`, + * `"1,000"`, `"v2"` do not. + */ +const NUMERIC_STRING_RE = /^[+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?$/; + +/** + * cel-js raises `no such overload: dyn int` (and kin) when a comparison + * or arithmetic operator sees a `string` on one side and a number on the + * other. ADR-0032 §1c — numeric fields that serialize as strings (`Field.rating` + * → `"5.0"`, `Field.currency` → `"250000.00"`, `Field.percent`) trip this in + * flow conditions / formulas (#1530, #1534) even though the schema and the + * build-time validator treat them as numeric. + */ +function isNumericOverloadError(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return /no such overload/i.test(message); +} + +/** + * Recursively coerce string values that are *entirely* numeric literals into + * numbers. Used only on the {@link isNumericOverloadError} retry path, so it + * can never change a comparison that already evaluated cleanly — it only + * rescues one that already faulted. Dates and non-numeric strings pass through + * untouched (a zip like `"02134"` only changes if the surrounding expression + * already faulted, in which case the original loud error is preserved when the + * retry still cannot type-check). + */ +function hydrateNumericStrings(value: unknown): unknown { + if (typeof value === 'string') { + const trimmed = value.trim(); + if (trimmed.length > 0 && NUMERIC_STRING_RE.test(trimmed)) { + const n = Number(trimmed); + if (Number.isFinite(n)) return n; + } + return value; + } + if (Array.isArray(value)) return value.map(hydrateNumericStrings); + if (value && typeof value === 'object' && !(value instanceof Date)) { + const out: Record = {}; + for (const [k, v] of Object.entries(value)) out[k] = hydrateNumericStrings(v); + return out; + } + return value; +} + function classifyError(err: unknown): EvalResult { const message = err instanceof Error ? err.message : String(err); let kind: 'parse' | 'type' | 'runtime' | 'bounds' = 'runtime'; @@ -114,8 +162,27 @@ export const celEngine: DialectEngine = { try { const env = buildEnv(now); const scope = buildScope(ctx); - const raw = env.evaluate(source, scope); - return { ok: true, value: coerce(raw) as T }; + try { + const raw = env.evaluate(source, scope); + return { ok: true, value: coerce(raw) as T }; + } catch (err) { + // ADR-0032 §1c — string-serialized numeric fields (`rating` → `"5.0"`, + // `amount` → `"250000.00"`) make `record.rating >= 4` raise CEL's + // `no such overload: dyn >= int`. Hydrate purely-numeric strings to + // numbers and retry ONCE. This only runs after a fault, so a comparison + // that already evaluated cleanly is never re-interpreted; if the retry + // still cannot type-check, the original loud error is reported (#1534). + if (!isNumericOverloadError(err)) throw err; + const hydrated = hydrateNumericStrings(scope) as Record; + try { + const raw = env.evaluate(source, hydrated); + return { ok: true, value: coerce(raw) as T }; + } catch { + // Hydration did not resolve it — surface the original fault, not the + // retry's, so the message reflects what the author actually wrote. + throw err; + } + } } catch (err) { return classifyError(err); } diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index e4ef5fb30..7c6fcf029 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -376,6 +376,59 @@ describe('AutomationEngine', () => { expect(() => engine.evaluateCondition({ dialect: 'cel', source: '{record.rating} >= 4' }, vars)) .toThrow(/source:|template braces|bare CEL/); }); + + // ADR-0032 §1c / #1534 — numeric fields that serialize as strings + // (`Field.rating` → `"5.0"`, `Field.currency` → `"250000.00"`) used to + // fault under strict CEL (`no such overload: dyn >= int`) and silently + // dead-end the flow at the decision node. The condition must now compare + // as a number so the matching edge is taken. + it('takes a decision edge gated on a string-serialized numeric field — #1534', async () => { + const vars = new Map([['record', { rating: '5.0', amount: '250000.00' }]]); + expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.rating >= 4' }, vars)).toBe(true); + expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.amount > 100000' }, vars)).toBe(true); + expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.rating >= 4' }, + new Map([['record', { rating: '2.5' }]]))).toBe(false); + }); + + it('routes through a decision instead of dead-ending when rating is "5.0" — #1534', async () => { + const executed: string[] = []; + engine.registerNodeExecutor({ + type: 'decision', + async execute() { return { success: true }; }, + }); + engine.registerNodeExecutor({ + type: 'assignment', + async execute(node) { + executed.push(node.id); + return { success: true }; + }, + }); + + engine.registerFlow('hot_lead', { + name: 'hot_lead', + label: 'Hot Lead Routing', + type: 'record_change', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'check', type: 'decision', label: 'Check' }, + { id: 'hot', type: 'assignment', label: 'Hot' }, + { id: 'cold', type: 'assignment', label: 'Cold' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'check' }, + { id: 'e2', source: 'check', target: 'hot', condition: 'record.rating >= 4' }, + { id: 'e3', source: 'check', target: 'cold', condition: 'record.rating < 4' }, + { id: 'e4', source: 'hot', target: 'end' }, + { id: 'e5', source: 'cold', target: 'end' }, + ], + }); + + const result = await engine.execute('hot_lead', { record: { rating: '5.0' }, object: 'crm_lead' }); + expect(result.success).toBe(true); + expect(executed).toContain('hot'); + expect(executed).not.toContain('cold'); + }); }); describe('Durable Suspend / Resume (ADR-0019)', () => { From fc1a19a23f3711ea2fc291a6744807e248efc9b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 09:58:55 +0000 Subject: [PATCH 2/2] fix(formula): remove ReDoS hazard in numeric-string regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged polynomial backtracking in NUMERIC_STRING_RE: the `\d+\.?\d*` form degrades to adjacent unbounded quantifiers (`\d+\d*`) when the dot is absent, backtracking on long digit runs. Anchor the fractional part as a single optional `(?:\.\d*)?` group — same matched language, no hazard. https://claude.ai/code/session_01FjtHqsTWWPThCP6jsVdvPa --- packages/formula/src/cel-engine.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index dfe646c8f..9081357fa 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -65,7 +65,11 @@ function coerce(value: unknown): unknown { * `"250000.00"`, `"-3"`, `"1e3"` match; `"5px"`, `"0x10"`, `" "`, `""`, * `"1,000"`, `"v2"` do not. */ -const NUMERIC_STRING_RE = /^[+-]?(?:\d+\.?\d*|\.\d+)(?:[eE][+-]?\d+)?$/; +// The fractional part is a single optional `(?:\.\d*)?` group anchored by the +// literal `.` — never the ambiguous `\d+\.?\d*`, whose adjacent unbounded +// quantifiers (`\d+\d*` when the dot is absent) backtrack polynomially on long +// digit runs (CodeQL ReDoS). This matches the same strings without the hazard. +const NUMERIC_STRING_RE = /^[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/; /** * cel-js raises `no such overload: dyn int` (and kin) when a comparison