Clamp expr ids for Trace to [1, std::numeric_limits<int32_t>::max()]]

eval/eval/evaluator_core.cc

@@ -15,6 +15,8 @@
 #include "eval/eval/evaluator_core.h"
 
 #include <cstddef>
+#include <cstdint>
+#include <limits>
 #include <memory>
 #include <utility>
 
@@ -132,7 +134,13 @@ absl::StatusOr<cel::Value> ExecutionFrame::Evaluate(
                            "Try to disable short-circuiting.";
         continue;
       }
-      if (EvaluationStatus status(listener(expr->id(), value_stack().Peek(),
+      const int64_t id = expr->id();
+      // Skip if the id is out of range.
+      // Will take advantage of this in a follow up to bit pack the id.
+      if (id < 0 || id >= std::numeric_limits<int32_t>::max()) {
+        continue;
+      }
+      if (EvaluationStatus status(listener(id, value_stack().Peek(),
                                            descriptor_pool(), message_factory(),
                                            arena()));
           !status.ok()) {

eval/public/cel_expression.h

@@ -24,6 +24,13 @@ namespace google::api::expr::runtime {
 // then the order of the callback invocations is guaranteed to correspond
 // the order of variable sub-elements (e.g. the order of elements returned
 // by Comprehension.iter_range).
+//
+// Expression IDs outside of the range [0, INT32_MAX] are not supported and
+// will not invoke the listener. While the AST allows any int64, supported
+// parser implementations should use a dense range starting at 1. In practice,
+// no AST should contain more than ~ 1e9 nodes.
+//
+// ID 0 should not be considered valid, but is supported for legacy reasons.
 using CelEvaluationListener = std::function<absl::Status(
     int64_t expr_id, const CelValue&, google::protobuf::Arena*)>;
 

runtime/runtime.h

@@ -130,6 +130,13 @@ class TraceableProgram : public Program {
   // to an AST expression node. The value provided is the top of the value
   // stack, corresponding to the result of evaluating the given sub expression.
   //
+  // Expression IDs outside of the range [0, INT32_MAX] are not supported and
+  // will not invoke the listener. While the AST allows any int64, supported
+  // parser implementations should use a dense range starting at 1. In practice,
+  // no AST should contain more than ~ 1e9 nodes.
+  //
+  // ID 0 should not be considered valid, but is supported for legacy reasons.
+  //
   // A returning a non-ok status stops evaluation and forwards the error.
   using EvaluationListener = absl::AnyInvocable<absl::Status(
       int64_t expr_id, const Value&, const google::protobuf::DescriptorPool* absl_nonnull,