Skip to content

Commit 85ab10f

Browse files
committed
fix(interpreter): propagate unknown/error operands through == and !=
EvalEq and EvalNe evaluated their operands and called lhs.equal(rhs) directly, without first checking whether either operand was unknown or error. Because UnknownT.equal(...) returns BoolT.False, any "==" against an unknown collapsed to a concrete false (and "!=" to a concrete true) during partial evaluation, instead of propagating the unknown. The sibling EvalBinary (used by <, <=, >, >=) already guards with isUnknownOrError, and cel-go guards its equality evaluators the same way. Add the same guard to EvalEq and EvalNe so an undecidable equality is deferred rather than resolved. This fixes residual-AST / partial evaluation: a policy of the form `... && claims.email == "x"` evaluated with `email` unknown now yields a residual of the deferred comparison instead of collapsing the whole expression to false. ResidualAst_Complex is updated to assert the corrected (unknown-propagating) outcome, and a focused regression test, InterpreterTest.equalityWithUnknownOperandStaysUnknown, is added.
1 parent 11b8c8c commit 85ab10f

3 files changed

Lines changed: 42 additions & 3 deletions

File tree

core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,13 @@ final class EvalEq extends AbstractEvalLhsRhs implements InterpretableCall {
396396
public Val eval(org.projectnessie.cel.interpreter.Activation ctx) {
397397
Val lVal = lhs.eval(ctx);
398398
Val rVal = rhs.eval(ctx);
399+
// Early return if any argument to the function is unknown or error.
400+
if (isUnknownOrError(lVal)) {
401+
return lVal;
402+
}
403+
if (isUnknownOrError(rVal)) {
404+
return rVal;
405+
}
399406
return lVal.equal(rVal);
400407
}
401408

@@ -439,6 +446,13 @@ final class EvalNe extends AbstractEvalLhsRhs implements InterpretableCall {
439446
public Val eval(org.projectnessie.cel.interpreter.Activation ctx) {
440447
Val lVal = lhs.eval(ctx);
441448
Val rVal = rhs.eval(ctx);
449+
// Early return if any argument to the function is unknown or error.
450+
if (isUnknownOrError(lVal)) {
451+
return lVal;
452+
}
453+
if (isUnknownOrError(rVal)) {
454+
return rVal;
455+
}
442456
Val eqVal = lVal.equal(rVal);
443457
switch (eqVal.type().typeEnum()) {
444458
case Err:

core/src/test/java/org/projectnessie/cel/CELTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import static org.projectnessie.cel.ProgramOption.functions;
5050
import static org.projectnessie.cel.ProgramOption.globals;
5151
import static org.projectnessie.cel.Util.mapOf;
52-
import static org.projectnessie.cel.common.types.BoolT.False;
5352
import static org.projectnessie.cel.common.types.BoolT.True;
5453
import static org.projectnessie.cel.common.types.Err.isError;
5554
import static org.projectnessie.cel.common.types.Err.newErr;
@@ -657,10 +656,10 @@ void ResidualAst_Complex() {
657656
assertThat(astIss.hasIssues()).isFalse();
658657
Program prg = e.program(astIss.getAst(), evalOptions(OptTrackState, OptPartialEval));
659658
EvalResult outDet = prg.eval(unkVars);
660-
assertThat(outDet.getVal()).isSameAs(False);
659+
assertThat(outDet.getVal()).isInstanceOf(UnknownT.class);
661660
Ast residual = e.residualAst(astIss.getAst(), outDet.getEvalDetails());
662661
String expr = astToString(residual);
663-
assertThat(expr).isEqualTo("false");
662+
assertThat(expr).isEqualTo("request.auth.claims.email == \"wiley@acme.co\"");
664663
}
665664

666665
@Test

core/src/test/java/org/projectnessie/cel/interpreter/InterpreterTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,6 +1505,32 @@ void missingIdentInSelect() {
15051505
assertThat(result).isInstanceOf(Err.class);
15061506
}
15071507

1508+
@Test
1509+
void equalityWithUnknownOperandStaysUnknown() {
1510+
assertThat(evalWithUnknownStringX("x == 'foo'")).isInstanceOf(UnknownT.class);
1511+
assertThat(evalWithUnknownStringX("x != 'foo'")).isInstanceOf(UnknownT.class);
1512+
assertThat(evalWithUnknownStringX("false || x == 'foo'")).isInstanceOf(UnknownT.class);
1513+
assertThat(evalWithUnknownStringX("false && x == 'foo'")).isSameAs(False);
1514+
}
1515+
1516+
private static Val evalWithUnknownStringX(String expr) {
1517+
Source src = newTextSource(expr);
1518+
ParseResult parsed = Parser.parseAllMacros(src);
1519+
assertThat(parsed.hasErrors()).withFailMessage(parsed.getErrors()::toDisplayString).isFalse();
1520+
1521+
Container cont = testContainer("test");
1522+
TypeRegistry reg = newRegistry();
1523+
CheckerEnv env = newStandardCheckerEnv(cont, reg);
1524+
env.add(Decls.newVar("x", Decls.String));
1525+
CheckResult checkResult = Checker.Check(parsed, src, env);
1526+
1527+
AttributeFactory attrs = newPartialAttributeFactory(cont, reg, reg);
1528+
Interpreter interp = newStandardInterpreter(cont, reg, reg, attrs);
1529+
Interpretable i = interp.newInterpretable(checkResult.getCheckedExpr());
1530+
Activation vars = newPartialActivation(mapOf(), newAttributePattern("x"));
1531+
return i.eval(vars);
1532+
}
1533+
15081534
static class ConvTestCase {
15091535
final String in;
15101536
Val out;

0 commit comments

Comments
 (0)