From 04f77388081b8bb33d99d1e73f0b8498f86ec815 Mon Sep 17 00:00:00 2001
From: Jonathan Tatum <jdtatum@google.com>
Date: Wed, 9 Sep 2026 11:39:59 -0700
Subject: [PATCH] Clamp expr ids for Trace to [1,
 std::numeric_limits<int32_t>::max()]] Test to see if anything depends on full
 range of expr ids.

PiperOrigin-RevId: 978663689
---
 eval/eval/evaluator_core.cc  | 10 +++++++++-
 eval/public/cel_expression.h |  7 +++++++
 runtime/runtime.h            |  7 +++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/eval/eval/evaluator_core.cc b/eval/eval/evaluator_core.cc
index 05dbed854..4f2e193fd 100644
--- a/eval/eval/evaluator_core.cc
+++ b/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()) {
diff --git a/eval/public/cel_expression.h b/eval/public/cel_expression.h
index 4cf029e89..af28e2ae6 100644
--- a/eval/public/cel_expression.h
+++ b/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*)>;
 
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 8c76236dd..02ad9fd06 100644
--- a/runtime/runtime.h
+++ b/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,