Fix a fuzzer issue involving nested comprehensions

PiperOrigin-RevId: 746534921

Author: l46kok

.bazelrc

@@ -5,3 +5,7 @@ build --java_language_version=11
 
 # Hide Java 8 deprecation warnings.
 common --javacopt=-Xlint:-options
+
+# MacOS Fix https://github.com/protocolbuffers/protobuf/issues/16944
+build --host_cxxopt=-std=c++14
+build --cxxopt=-std=c++14

runtime/BUILD.bazel

@@ -105,6 +105,12 @@ java_library(
     exports = ["//runtime/src/main/java/dev/cel/runtime:interpreter"],
 )
 
+java_library(
+    name = "interpretable",
+    visibility = ["//:internal"],
+    exports = ["//runtime/src/main/java/dev/cel/runtime:interpretable"],
+)
+
 java_library(
     name = "runtime_helpers",
     visibility = ["//:internal"],

runtime/src/main/java/dev/cel/runtime/BUILD.bazel

@@ -328,6 +328,7 @@ cel_android_library(
         "//common/types:types_android",
         "@maven//:com_google_code_findbugs_annotations",
         "@maven//:com_google_errorprone_error_prone_annotations",
+        "@maven//:com_google_guava_guava",
         "@maven_android//:com_google_guava_guava",
     ],
 )
@@ -506,7 +507,6 @@ java_library(
 java_library(
     name = "interpretable",
     srcs = INTERPRABLE_SOURCES,
-    visibility = ["//visibility:private"],
     deps = [
         ":evaluation_exception",
         ":evaluation_listener",

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

@@ -17,9 +17,11 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.Immutable;
 import javax.annotation.concurrent.ThreadSafe;
 import dev.cel.common.CelAbstractSyntaxTree;
@@ -51,9 +53,7 @@
 import java.util.Map;
 import java.util.Optional;
 
-/**
- * Default implementation of the CEL interpreter.
- */
+/** Default implementation of the CEL interpreter. */
 @ThreadSafe
 final class DefaultInterpreter implements Interpreter {
 
@@ -111,8 +111,8 @@ public Interpretable createInterpretable(CelAbstractSyntaxTree ast) {
   }
 
   @Immutable
-  private static final class DefaultInterpretable
-      implements Interpretable, UnknownTrackingInterpretable {
+  @VisibleForTesting
+  static final class DefaultInterpretable implements Interpretable, UnknownTrackingInterpretable {
     private final TypeResolver typeResolver;
     private final RuntimeTypeProvider typeProvider;
     private final Dispatcher.ImmutableCopy dispatcher;
@@ -165,14 +165,44 @@ public Object evalTrackingUnknowns(
         Optional<? extends FunctionResolver> functionResolver,
         CelEvaluationListener listener)
         throws CelEvaluationException {
-      int comprehensionMaxIterations =
-          celOptions.enableComprehension() ? celOptions.comprehensionMaxIterations() : 0;
-      ExecutionFrame frame =
-          new ExecutionFrame(listener, resolver, functionResolver, comprehensionMaxIterations);
+      ExecutionFrame frame = newExecutionFrame(resolver, functionResolver, listener);
       IntermediateResult internalResult = evalInternal(frame, ast.getExpr());
       return internalResult.value();
     }
 
+    /**
+     * Evaluates this interpretable and returns the resulting execution frame populated with
+     * evaluation state. This method is specifically designed for testing the interpreter's internal
+     * invariants.
+     *
+     * <p><b>Do not expose to public. This method is strictly for internal testing purposes
+     * only.</b>
+     */
+    @VisibleForTesting
+    @CanIgnoreReturnValue
+    ExecutionFrame populateExecutionFrame(ExecutionFrame frame) throws CelEvaluationException {
+      evalInternal(frame, ast.getExpr());
+
+      return frame;
+    }
+
+    @VisibleForTesting
+    ExecutionFrame newTestExecutionFrame(GlobalResolver resolver) {
+      return newExecutionFrame(
+          RuntimeUnknownResolver.fromResolver(resolver),
+          Optional.empty(),
+          CelEvaluationListener.noOpListener());
+    }
+
+    private ExecutionFrame newExecutionFrame(
+        RuntimeUnknownResolver resolver,
+        Optional<? extends FunctionResolver> functionResolver,
+        CelEvaluationListener listener) {
+      int comprehensionMaxIterations =
+          celOptions.enableComprehension() ? celOptions.comprehensionMaxIterations() : 0;
+      return new ExecutionFrame(listener, resolver, functionResolver, comprehensionMaxIterations);
+    }
+
     private IntermediateResult evalInternal(ExecutionFrame frame, CelExpr expr)
         throws CelEvaluationException {
       try {
@@ -927,8 +957,12 @@ private IntermediateResult evalComprehension(
       }
 
       frame.pushScope(Collections.singletonMap(accuVar, accuValue));
-      IntermediateResult result = evalInternal(frame, compre.result());
-      frame.popScope();
+      IntermediateResult result;
+      try {
+        result = evalInternal(frame, compre.result());
+      } finally {
+        frame.popScope();
+      }
       return result;
     }
 
@@ -976,13 +1010,14 @@ private LazyExpression(CelExpr celExpr) {
   }
 
   /** This class tracks the state meaningful to a single evaluation pass. */
-  private static class ExecutionFrame {
+  static class ExecutionFrame {
     private final CelEvaluationListener evaluationListener;
     private final int maxIterations;
     private final ArrayDeque<RuntimeUnknownResolver> resolvers;
     private final Optional<? extends FunctionResolver> lateBoundFunctionResolver;
     private RuntimeUnknownResolver currentResolver;
     private int iterations;
+    @VisibleForTesting int scopeLevel;
 
     private ExecutionFrame(
         CelEvaluationListener evaluationListener,
@@ -1040,12 +1075,14 @@ private void cacheLazilyEvaluatedResult(
 
     /** Note: we utilize a HashMap instead of ImmutableMap to make lookups faster on string keys. */
     private void pushScope(Map<String, IntermediateResult> scope) {
+      scopeLevel++;
       RuntimeUnknownResolver scopedResolver = currentResolver.withScope(scope);
       currentResolver = scopedResolver;
       resolvers.addLast(scopedResolver);
     }
 
     private void popScope() {
+      scopeLevel--;
       if (resolvers.isEmpty()) {
         throw new IllegalStateException("Execution frame error: more scopes popped than pushed");
       }

runtime/src/test/java/dev/cel/runtime/BUILD.bazel

@@ -72,6 +72,7 @@ java_library(
         "//common:cel_descriptors",
         "//common:cel_exception",
         "//common:cel_source",
+        "//common:compiler_common",
         "//common:error_codes",
         "//common:options",
         "//common:proto_ast",
@@ -100,6 +101,8 @@ java_library(
         "//runtime:evaluation_listener",
         "//runtime:function_binding",
         "//runtime:function_overload_impl",
+        "//runtime:interpretable",
+        "//runtime:interpreter",
         "//runtime:interpreter_util",
         "//runtime:lite_runtime",
         "//runtime:lite_runtime_factory",

runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java

@@ -0,0 +1,88 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dev.cel.runtime;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
+import dev.cel.common.CelAbstractSyntaxTree;
+import dev.cel.common.CelFunctionDecl;
+import dev.cel.common.CelOptions;
+import dev.cel.common.CelOverloadDecl;
+import dev.cel.common.types.SimpleType;
+import dev.cel.compiler.CelCompiler;
+import dev.cel.compiler.CelCompilerFactory;
+import dev.cel.parser.CelStandardMacro;
+import dev.cel.runtime.DefaultInterpreter.DefaultInterpretable;
+import dev.cel.runtime.DefaultInterpreter.ExecutionFrame;
+import java.util.Map;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * Exercises tests for the internals of the {@link DefaultInterpreter}. Tests should only be added
+ * here if we absolutely must add coverage for internal state of the interpreter that's otherwise
+ * impossible with the CEL public APIs.
+ */
+@RunWith(TestParameterInjector.class)
+public class DefaultInterpreterTest {
+
+  @Test
+  public void nestedComprehensions_accuVarContainsErrors_scopeLevelInvariantNotViolated()
+      throws Exception {
+    CelCompiler celCompiler =
+        CelCompilerFactory.standardCelCompilerBuilder()
+            .addFunctionDeclarations(
+                CelFunctionDecl.newFunctionDeclaration(
+                    "error", CelOverloadDecl.newGlobalOverload("error_overload", SimpleType.DYN)))
+            .setStandardMacros(CelStandardMacro.STANDARD_MACROS)
+            .build();
+    RuntimeTypeProvider emptyProvider =
+        new RuntimeTypeProvider() {
+          @Override
+          public Object createMessage(String messageName, Map<String, Object> values) {
+            return null;
+          }
+
+          @Override
+          public Object selectField(Object message, String fieldName) {
+            return null;
+          }
+
+          @Override
+          public Object hasField(Object message, String fieldName) {
+            return null;
+          }
+
+          @Override
+          public Object adapt(Object message) {
+            return message;
+          }
+        };
+    CelAbstractSyntaxTree ast = celCompiler.compile("[1].all(x, [2].all(y, error()))").getAst();
+    DefaultDispatcher dispatcher = DefaultDispatcher.create();
+    dispatcher.add("error", long.class, (args) -> new IllegalArgumentException("Always throws"));
+    DefaultInterpreter defaultInterpreter =
+        new DefaultInterpreter(new TypeResolver(), emptyProvider, dispatcher, CelOptions.DEFAULT);
+    DefaultInterpretable interpretable =
+        (DefaultInterpretable) defaultInterpreter.createInterpretable(ast);
+
+    ExecutionFrame frame = interpretable.newTestExecutionFrame(GlobalResolver.EMPTY);
+
+    assertThrows(CelEvaluationException.class, () -> interpretable.populateExecutionFrame(frame));
+    assertThat(frame.scopeLevel).isEqualTo(0);
+  }
+}