Skip to content

Commit bb00a50

Browse files
os-zhuangclaude
andauthored
fix(formula): catch unknown functions in CEL conditions at build (#1877) (#1896)
* fix(formula): catch unknown functions in CEL conditions at build (#1877) cel-js's `check()` returns a `TypeCheckResult` object (`{ valid, error }`), not an array, so `compile()`'s `Array.isArray(checkErrors)` guard never matched and the type-check verdict was silently discarded. A condition calling an unknown function (`PRIOR(status)`, a typo'd `isBlnk(...)`) type-checks as `found no matching overload`, but that result never surfaced — so `objectstack compile`, `registerFlow`, and the `validate_expression` tool all accepted the predicate, which then silently no-op'd the flow at runtime. Read the documented `{ valid, error }` shape instead. Because every author surface routes through this one `compile()`, the fix closes the gap for flow conditions, validation rules, and field formulas at once. Verified cel-js's `check()` cleanly separates valid predicates from unknown-function ones with no false positives on existing conditions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(changeset): formula unknown-function condition validation --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 18b6041 commit bb00a50

4 files changed

Lines changed: 55 additions & 4 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@objectstack/formula": patch
3+
---
4+
5+
fix(formula): catch unknown functions in CEL conditions at build (#1877)
6+
7+
`compile()` discarded cel-js's type-check verdict because `check()` returns a `TypeCheckResult` object (`{ valid, error }`), not an array — so the `Array.isArray(checkErrors)` guard never matched. A condition calling an unknown function (`PRIOR(status)`, a typo'd `isBlnk(...)`) type-checks as `found no matching overload`, but that result never surfaced, so `objectstack compile`, `registerFlow`, and the `validate_expression` tool all accepted the predicate, which then silently no-op'd the flow at runtime. Now reads the documented `{ valid, error }` shape, closing the gap for flow conditions, validation rules, and field formulas at once.

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,22 @@ describe('celEngine', () => {
7070
expect(r.ok).toBe(true);
7171
});
7272

73+
// #1877 — cel-js `check()` returns a `{ valid, error }` object, not an array.
74+
// compile() must read that shape so an UNKNOWN function (here `PRIOR`) is
75+
// reported as a type fault at build time instead of slipping through.
76+
it('compile() rejects an unknown function as a type error (#1877)', () => {
77+
const r = celEngine.compile('PRIOR(status) != "promoted"');
78+
expect(r.ok).toBe(false);
79+
if (!r.ok) {
80+
expect(r.error.kind).toBe('type');
81+
expect(r.error.message).toMatch(/overload|PRIOR/);
82+
}
83+
});
84+
85+
it('compile() still accepts a registered stdlib function (#1877)', () => {
86+
expect(celEngine.compile('!isBlank(record.target_channels)').ok).toBe(true);
87+
});
88+
7389
it('handles timestamp + duration arithmetic', () => {
7490
const pinned = new Date('2026-01-01T00:00:00Z');
7591
const r = celEngine.evaluate(cel('now() + duration("720h")'), { now: pinned });

packages/formula/src/cel-engine.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,19 @@ export const celEngine: DialectEngine = {
146146
// type-checking; the function is never actually called.
147147
const env = buildEnv(() => new Date(0));
148148
const compiled = env.parse(source);
149-
// Surface check errors eagerly.
150-
const checkErrors = compiled.check?.();
151-
if (checkErrors && Array.isArray(checkErrors) && checkErrors.length > 0) {
149+
// Surface check errors eagerly. cel-js's `check()` returns a
150+
// `TypeCheckResult` object (`{ valid, type?, error? }`) — NOT an array —
151+
// so the type fault (including `found no matching overload for 'PRIOR(dyn)'`
152+
// when a condition calls an UNKNOWN function) only surfaces when we read
153+
// `valid === false`. The previous `Array.isArray(...)` guard never matched
154+
// an object, so unknown-function predicates type-checked clean and were
155+
// silently accepted by `objectstack build` / `registerFlow`, then no-op'd
156+
// the flow at runtime (#1877). Reading the documented shape closes that.
157+
const checkResult = compiled.check?.();
158+
if (checkResult && checkResult.valid === false) {
152159
return {
153160
ok: false,
154-
error: { kind: 'type', message: checkErrors.join('; ') },
161+
error: { kind: 'type', message: checkResult.error?.message ?? 'expression failed type checking' },
155162
};
156163
}
157164
return { ok: true, value: compiled.ast };

packages/formula/src/validate.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,27 @@ describe('validateExpression (ADR-0032)', () => {
2626
expect(validateExpression('predicate', '').ok).toBe(true);
2727
expect(validateExpression('predicate', null).ok).toBe(true);
2828
});
29+
30+
// #1877 — a predicate calling an UNKNOWN function (e.g. `PRIOR()`, a typo'd
31+
// `isBlnk()`) must be rejected at build/registration, not silently accepted
32+
// and then no-op the flow at runtime. cel-js's type checker reports these as
33+
// `found no matching overload`; the engine surfaces them as an invalid CEL
34+
// predicate.
35+
it('rejects an unknown function call (#1877)', () => {
36+
const r = validateExpression('predicate', 'PRIOR(status) != "promoted"');
37+
expect(r.ok).toBe(false);
38+
expect(r.errors[0].message).toMatch(/invalid CEL predicate/i);
39+
expect(r.errors[0].message).toMatch(/overload|PRIOR/);
40+
});
41+
42+
it('rejects an unknown function even when guarded by a short-circuit (#1877)', () => {
43+
const r = validateExpression('predicate', 'status == "promoted" && PRIOR(status) != "promoted"');
44+
expect(r.ok).toBe(false);
45+
});
46+
47+
it('still accepts a registered stdlib function (isBlank)', () => {
48+
expect(validateExpression('predicate', '!isBlank(record.target_channels)').ok).toBe(true);
49+
});
2950
});
3051

3152
describe('templates', () => {

0 commit comments

Comments
 (0)