Reject block indices that leads to cycles

optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java

@@ -52,6 +52,7 @@
 import dev.cel.parser.CelStandardMacro;
 import dev.cel.parser.CelUnparser;
 import dev.cel.parser.CelUnparserFactory;
+import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelFunctionBinding;
 import dev.cel.runtime.CelRuntime;
 import dev.cel.runtime.CelRuntimeFactory;
@@ -579,6 +580,28 @@ public void verifyOptimizedAstCorrectness_indexIsNotForwardReferencing_throws(St
         .contains("Illegal block index found. The index value must be less than");
   }
 
+  @Test
+  public void block_containsCycle_throws() throws Exception {
+    CelAbstractSyntaxTree ast = compileUsingInternalFunctions("cel.block([index1,index0],index0)");
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> CEL.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("Cycle detected: @index0");
+  }
+
+  @Test
+  public void block_lazyEvaluationContainsError_cleansUpCycleState() throws Exception {
+    CelAbstractSyntaxTree ast =
+        compileUsingInternalFunctions(
+            "cel.block([1/0 > 0], (index0 && false) || (index0 && true))");
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> CEL.createProgram(ast).eval());
+
+    assertThat(e).hasMessageThat().contains("/ by zero");
+    assertThat(e).hasMessageThat().doesNotContain("Cycle detected");
+  }
+
   /**
    * Converts AST containing cel.block related test functions to internal functions (e.g: cel.block
    * -> cel.@block)

runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java

@@ -57,6 +57,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 /** Default implementation of the CEL interpreter. */
 @ThreadSafe
@@ -343,7 +344,13 @@ private IntermediateResult resolveIdent(ExecutionFrame frame, CelExpr expr, Stri
       Object value = rawResult.value();
       boolean isLazyExpression = value instanceof LazyExpression;
       if (isLazyExpression) {
-        value = evalInternal(frame, ((LazyExpression) value).celExpr).value();
+        frame.markLazyEvaluationOrThrow(name);
+
+        try {
+          value = evalInternal(frame, ((LazyExpression) value).celExpr).value();
+        } finally {
+          frame.endLazyEvaluation(name);
+        }
       }
 
       // Value resolved from Binding, it could be Message, PartialMessage or unbound(null)
@@ -1063,9 +1070,14 @@ private IntermediateResult evalCelBlock(
             indexKey,
             IntermediateResult.create(new LazyExpression(exprList.elements().get(index))));
       }
+      frame.setRequireCycleCheck(true);
       frame.pushLazyScope(Collections.unmodifiableMap(blockList));
 
-      return evalInternal(frame, blockCall.args().get(1));
+      try {
+        return evalInternal(frame, blockCall.args().get(1));
+      } finally {
+        frame.popScope();
+      }
     }
 
     private CelType getCheckedTypeOrThrow(CelExpr expr) throws CelEvaluationException {
@@ -1115,8 +1127,10 @@ static class ExecutionFrame {
     private final int maxIterations;
     private final ArrayDeque<RuntimeUnknownResolver> resolvers;
     private final Optional<? extends CelFunctionResolver> lateBoundFunctionResolver;
+    private final Set<String> activeLazyAttributes = new HashSet<>();
     private RuntimeUnknownResolver currentResolver;
     private int iterations;
+    private boolean requireCycleCheck;
     @VisibleForTesting int scopeLevel;
 
     private ExecutionFrame(
@@ -1132,6 +1146,25 @@ private ExecutionFrame(
       this.maxIterations = maxIterations;
     }
 
+    private void markLazyEvaluationOrThrow(String name) {
+      if (!requireCycleCheck) {
+        return;
+      }
+
+      boolean added = activeLazyAttributes.add(name);
+      if (!added) {
+        throw new IllegalStateException(String.format("Cycle detected: %s", name));
+      }
+    }
+
+    private void endLazyEvaluation(String name) {
+      if (!requireCycleCheck) {
+        return;
+      }
+
+      activeLazyAttributes.remove(name);
+    }
+
     private Optional<CelEvaluationListener> getEvaluationListener() {
       return evaluationListener;
     }
@@ -1175,6 +1208,14 @@ private void cacheLazilyEvaluatedResult(
       currentResolver.cacheLazilyEvaluatedResult(name, result);
     }
 
+    /**
+     * If set, interpreter will check for potential cycles for lazily evaluable attributes. This
+     * only applies for cel.@block indices.
+     */
+    private void setRequireCycleCheck(boolean requireCycleCheck) {
+      this.requireCycleCheck = requireCycleCheck;
+    }
+
     private void pushLazyScope(Map<String, IntermediateResult> scope) {
       pushScope(scope);
       for (String lazyAttribute : scope.keySet()) {