Add depth validation for ASTs loaded through ParsedExprToAst / Checke...

cel/env.go

@@ -48,6 +48,10 @@ type Source = common.Source
 type Ast struct {
 	source Source
 	impl   *celast.AST
+	// loadErr captures an error detected while loading the AST (e.g. an over-deep AST ingested via
+	// ParsedExprToAst / CheckedExprToAst) so it can be surfaced when the Ast is checked or planned
+	// instead of recursing into the checker or planner on adversarially deep input.
+	loadErr error
 }
 
 // NativeRep converts the AST to a Go-native representation.
@@ -395,6 +399,13 @@ func NewCustomEnv(opts ...EnvOption) (*Env, error) {
 // It is possible to have both non-nil Ast and Issues values returned from this call: however,
 // the mere presence of an Ast does not imply that it is valid for use.
 func (e *Env) Check(ast *Ast) (*Ast, *Issues) {
+	// Surface any error recorded while the Ast was loaded (e.g. an over-deep AST rejected by
+	// ParsedExprToAst / CheckedExprToAst) before recursing into the type checker on it.
+	if ast != nil && ast.loadErr != nil {
+		errs := common.NewErrors(ast.Source())
+		errs.ReportErrorString(common.NoLocation, ast.loadErr.Error())
+		return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo())
+	}
 	// Construct the internal checker env, erroring if there is an issue adding the declarations.
 	chk, err := e.initChecker()
 	if err != nil {
@@ -687,6 +698,12 @@ func (e *Env) ParseSource(src Source) (*Ast, *Issues) {
 
 // Program generates an evaluable instance of the Ast within the environment (Env).
 func (e *Env) Program(ast *Ast, opts ...ProgramOption) (Program, error) {
+	// Surface any error recorded while the Ast was loaded (e.g. an over-deep AST rejected by
+	// ParsedExprToAst / CheckedExprToAst) rather than recursing into the planner on it. This is a
+	// cheap field read; the depth traversal itself runs once at conversion time, not here.
+	if ast != nil && ast.loadErr != nil {
+		return nil, ast.loadErr
+	}
 	return e.PlanProgram(ast.NativeRep(), opts...)
 }
 

cel/io.go

@@ -52,7 +52,12 @@ func CheckedExprToAstWithSource(checkedExpr *exprpb.CheckedExpr, src Source) (*A
 	if err != nil {
 		return nil, err
 	}
-	return &Ast{source: src, impl: checked}, nil
+	out := &Ast{source: src, impl: checked}
+	if err := checkLoadedASTDepth(checked); err != nil {
+		out.loadErr = err
+		return out, err
+	}
+	return out, nil
 }
 
 // AstToCheckedExpr converts an Ast to an protobuf CheckedExpr value.
@@ -83,7 +88,26 @@ func ParsedExprToAstWithSource(parsedExpr *exprpb.ParsedExpr, src Source) *Ast {
 		src = common.NewInfoSource(parsedExpr.GetSourceInfo())
 	}
 	e, _ := ast.ProtoToExpr(parsedExpr.GetExpr())
-	return &Ast{source: src, impl: ast.NewAST(e, info)}
+	out := &Ast{source: src, impl: ast.NewAST(e, info)}
+	// ParsedExprToAstWithSource has no error return, so record an over-depth violation on the Ast
+	// to be surfaced when it is later checked or planned.
+	out.loadErr = checkLoadedASTDepth(out.impl)
+	return out
+}
+
+// checkLoadedASTDepth guards ASTs that enter through the proto conversion helpers
+// (ParsedExprToAst / CheckedExprToAst) against nesting deeper than the parser's recursion limit.
+// Those entry points bypass the parser, so without this check a deeply nested loaded AST could
+// exhaust the Go stack during later checking or planning. It returns a normal error rather than
+// risking that overflow; the traversal itself is bounded so it stays safe on the same input.
+//
+// Embedders that fully control their AST inputs can skip this by building the AST through the
+// common/ast package directly instead of these conversion helpers.
+func checkLoadedASTDepth(a *ast.AST) error {
+	if ast.ExceedsDepth(a, defaultMaxASTDepth) {
+		return fmt.Errorf("input exceeds maximum expression nesting depth: %d", defaultMaxASTDepth)
+	}
+	return nil
 }
 
 // AstToParsedExpr converts an Ast to an protobuf ParsedExpr value.

cel/io_test.go

@@ -317,6 +317,110 @@ func TestCheckedExprToAstMissingInfo(t *testing.T) {
 	}
 }
 
+// deepBoolExpr builds a synthetic deeply nested proto Expr by stacking `depth` unary `!` calls
+// on top of a boolean literal. A depth well above the 250 default but far below the Go stack
+// limit keeps the test itself from overflowing while still exercising the depth guard.
+func deepBoolExpr(depth int) *exprpb.Expr {
+	expr := &exprpb.Expr{
+		Id: 1,
+		ExprKind: &exprpb.Expr_ConstExpr{
+			ConstExpr: &exprpb.Constant{
+				ConstantKind: &exprpb.Constant_BoolValue{BoolValue: true},
+			},
+		},
+	}
+	for i := 0; i < depth; i++ {
+		expr = &exprpb.Expr{
+			Id: int64(i + 2),
+			ExprKind: &exprpb.Expr_CallExpr{
+				CallExpr: &exprpb.Expr_Call{
+					Function: operators.LogicalNot,
+					Args:     []*exprpb.Expr{expr},
+				},
+			},
+		}
+	}
+	return expr
+}
+
+func TestLoadedAstDepthLimit(t *testing.T) {
+	env, err := NewEnv()
+	if err != nil {
+		t.Fatalf("NewEnv() failed: %v", err)
+	}
+
+	// Sanity check: a shallow parsed expression still checks and plans clean.
+	shallow, iss := env.Parse("1 + 2")
+	if iss.Err() != nil {
+		t.Fatalf("Parse('1 + 2') failed: %v", iss.Err())
+	}
+	if _, iss := env.Check(shallow); iss.Err() != nil {
+		t.Fatalf("Check(shallow) failed: %v", iss.Err())
+	}
+	if _, err := env.Program(shallow); err != nil {
+		t.Fatalf("Program(shallow) failed: %v", err)
+	}
+
+	const depth = 300
+	deepExpr := deepBoolExpr(depth)
+
+	// ParsedExprToAst flags the over-deep AST at conversion time. Because the conversion helper
+	// has no error return, the violation is surfaced as a normal error when the AST is later
+	// planned or checked, rather than recursing into a Go stack overflow.
+	deepParsed := ParsedExprToAst(&exprpb.ParsedExpr{Expr: deepExpr})
+	if _, err := env.Program(deepParsed); err == nil {
+		t.Errorf("Program(deepParsed) expected an error, got nil")
+	} else if !strings.Contains(err.Error(), "maximum expression nesting depth") {
+		t.Errorf("Program(deepParsed) error = %v, want it to mention 'maximum expression nesting depth'", err)
+	}
+	if _, iss := env.Check(deepParsed); iss.Err() == nil {
+		t.Errorf("Check(deepParsed) expected an error, got nil")
+	} else if !strings.Contains(iss.Err().Error(), "maximum expression nesting depth") {
+		t.Errorf("Check(deepParsed) error = %v, want it to mention 'maximum expression nesting depth'", iss.Err())
+	}
+
+	// CheckedExprToAstWithSource returns the depth error directly since it has an error return.
+	if _, err := CheckedExprToAstWithSource(&exprpb.CheckedExpr{Expr: deepExpr}, nil); err == nil {
+		t.Errorf("CheckedExprToAstWithSource(deep) expected an error, got nil")
+	} else if !strings.Contains(err.Error(), "maximum expression nesting depth") {
+		t.Errorf("CheckedExprToAstWithSource(deep) error = %v, want it to mention 'maximum expression nesting depth'", err)
+	}
+
+	// Embedders in full control of their AST inputs can skip the check by building the AST through
+	// the common/ast package directly rather than the cel conversion helpers.
+	nativeExpr, err := celast.ProtoToExpr(deepExpr)
+	if err != nil {
+		t.Fatalf("celast.ProtoToExpr() failed: %v", err)
+	}
+	bypass := &Ast{impl: celast.NewAST(nativeExpr, celast.NewSourceInfo(nil))}
+	if _, err := env.Program(bypass); err != nil {
+		t.Errorf("Program(bypass) built directly via common/ast failed: %v", err)
+	}
+}
+
+func TestExpressionNestingDepthLimitConfigRoundTrip(t *testing.T) {
+	env, err := NewEnv(ExpressionNestingDepthLimit(128))
+	if err != nil {
+		t.Fatalf("NewEnv(ExpressionNestingDepthLimit(128)) failed: %v", err)
+	}
+	conf, err := env.ToConfig("depth-limit")
+	if err != nil {
+		t.Fatalf("env.ToConfig() failed: %v", err)
+	}
+	found := false
+	for _, limit := range conf.Limits {
+		if limit.Name == "cel.limit.max_ast_depth" {
+			found = true
+			if limit.Value != 128 {
+				t.Errorf("limit %q value = %d, wanted 128", limit.Name, limit.Value)
+			}
+		}
+	}
+	if !found {
+		t.Errorf("env config limits %v missing 'cel.limit.max_ast_depth'", conf.Limits)
+	}
+}
+
 func TestRefValueToValue_Error(t *testing.T) {
 	_, err := RefValueToValue(types.NewErr("test error"))
 	if err == nil {
@@ -344,4 +448,4 @@ func TestRefValToExprValue_Wrappers(t *testing.T) {
 	if res.GetValue() == nil {
 		t.Error("RefValToExprValue() returned nil value")
 	}
-}
+}

cel/options.go

@@ -109,12 +109,20 @@ const (
 	limitCodePointSize
 	// The number of attempts to recover from a parse error.
 	limitParseErrorRecovery
+	// The maximum nesting depth permitted for ASTs loaded outside the parser.
+	limitMaxASTDepth
 )
 
+// defaultMaxASTDepth mirrors the parser's default maxRecursionDepth (250) and
+// is applied to ASTs that enter through non-parser ingestion paths (e.g. via
+// ParsedExprToAst / CheckedExprToAst) when no explicit limit is configured.
+const defaultMaxASTDepth = 250
+
 var limitIDsToNames = map[limitID]string{
 	limitCodePointSize:       "cel.limit.expression_code_points",
 	limitParseErrorRecovery:  "cel.limit.parse_error_recovery",
 	limitParseRecursionDepth: "cel.limit.parse_recursion_depth",
+	limitMaxASTDepth:         "cel.limit.max_ast_depth",
 }
 
 func limitNameByID(id limitID) (string, bool) {
@@ -942,6 +950,18 @@ func ParserExpressionSizeLimit(limit int) EnvOption {
 	return setLimit(limitCodePointSize, limit)
 }
 
+// ExpressionNestingDepthLimit records the maximum nesting depth permitted for ASTs in the
+// environment configuration so that the value round-trips through env.Config export/import.
+//
+// ASTs loaded outside the parser (e.g. via ParsedExprToAst / CheckedExprToAst) bypass the
+// parser's recursion limit, so those conversion paths validate nesting depth against the
+// parser-matching default (250) to avoid a Go stack overflow during later checking or planning.
+// Embedders that fully control their AST inputs and want to skip the check can construct the AST
+// through the common/ast package directly rather than the cel conversion helpers.
+func ExpressionNestingDepthLimit(limit int) EnvOption {
+	return setLimit(limitMaxASTDepth, limit)
+}
+
 // EnableHiddenAccumulatorName sets the parser to use the identifier '@result' for accumulators
 // which is not normally accessible from CEL source.
 func EnableHiddenAccumulatorName(enabled bool) EnvOption {

common/ast/navigable.go

@@ -181,6 +181,29 @@ func PreOrderVisit(expr Expr, visitor Visitor) {
 	visit(expr, visitor, preOrder, 0, 0)
 }
 
+// ExceedsDepth determines whether the AST contains expressions nested deeper than the specified
+// maxDepth. The root expression has depth 0, so a maxDepth of 250 permits expressions nested up
+// to and including 250 levels deep.
+//
+// The traversal is bounded: it descends at most maxDepth+1 levels, so it remains safe to call on
+// adversarially deep inputs that could otherwise exhaust the Go stack during later checking or
+// planning. A non-positive maxDepth disables the check and returns false.
+func ExceedsDepth(a *AST, maxDepth int) bool {
+	if a == nil || maxDepth <= 0 {
+		return false
+	}
+	exceedsDepth := false
+	visitor := NewExprVisitor(func(e Expr) {
+		if nav, ok := e.(NavigableExpr); ok && nav.Depth() >= maxDepth {
+			exceedsDepth = true
+		}
+	})
+	// Bound the walk to maxDepth+1 levels so it never recurses past the first level that exceeds
+	// the limit, keeping the check itself safe on the deep inputs it guards against.
+	visit(NavigateAST(a), visitor, postOrder, 0, maxDepth+1)
+	return exceedsDepth
+}
+
 type visitOrder int
 
 const (