From 6cdc6db67be6276b6b2e815ae5da7fa1ddf52018 Mon Sep 17 00:00:00 2001 From: William Date: Thu, 4 Jun 2026 00:18:10 -0700 Subject: [PATCH 1/3] common/ast: depth-validate ASTs from non-parser ingestion ASTs loaded via ParsedExprToAst / CheckedExprToAst bypass the parser's recursion limit, so a deeply nested loaded AST could exhaust the Go stack during checking or planning and crash the process instead of returning a normal error. Add ast.ExceedsMaxDepth, a bounded traversal (default limit 250, the same as the parser's maxRecursionDepth) that Env.Check and Env.PlanProgram run before recursing, returning an ordinary error when the limit is exceeded. Includes a regression test that loads a synthetic over-deep AST. Closes #1333 --- cel/env.go | 17 ++++++++ cel/io_test.go | 58 +++++++++++++++++++++++++ common/ast/depth.go | 101 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) create mode 100644 common/ast/depth.go diff --git a/cel/env.go b/cel/env.go index 7139e415e..0486a0a48 100644 --- a/cel/env.go +++ b/cel/env.go @@ -403,6 +403,16 @@ func (e *Env) Check(ast *Ast) (*Ast, *Issues) { return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) } + // Guard against ASTs that bypass the parser depth limit (e.g. loaded via + // ParsedExprToAst / CheckedExprToAst), since later recursive checking + // would otherwise risk a Go stack overflow on adversarially deep inputs. + if celast.ExceedsMaxDepth(ast.NativeRep().Expr(), celast.MaxNestingDepth) { + errs := common.NewErrors(ast.Source()) + errs.ReportErrorString(common.NoLocation, + fmt.Sprintf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth)) + return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) + } + checked, errs := checker.Check(ast.NativeRep(), ast.Source(), chk) if len(errs.GetErrors()) > 0 { return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) @@ -693,6 +703,13 @@ func (e *Env) Program(ast *Ast, opts ...ProgramOption) (Program, error) { // PlanProgram generates an evaluable instance of the AST in the go-native representation within // the environment (Env). func (e *Env) PlanProgram(a *celast.AST, opts ...ProgramOption) (Program, error) { + // Guard against ASTs that bypass the parser depth limit (e.g. loaded via + // ParsedExprToAst / CheckedExprToAst), since later recursive planning and + // evaluation would otherwise risk a Go stack overflow on adversarially + // deep inputs. + if a != nil && celast.ExceedsMaxDepth(a.Expr(), celast.MaxNestingDepth) { + return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth) + } optSet := e.progOpts if len(opts) != 0 { mergedOpts := []ProgramOption{} diff --git a/cel/io_test.go b/cel/io_test.go index 9da7ce43d..a31f03a93 100644 --- a/cel/io_test.go +++ b/cel/io_test.go @@ -316,3 +316,61 @@ func TestCheckedExprToAstMissingInfo(t *testing.T) { t.Fatalf("ast2.ResultType() got %v, wanted 'int'", ast.ResultType()) } } + +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) + } + + // Build a synthetic deeply nested AST (depth ~300) using iteratively + // stacked unary `!` calls, well above the 250 default but far below the + // Go stack limit so the test itself never crashes. + const depth = 300 + 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}, + }, + }, + } + } + deepAst := ParsedExprToAst(&exprpb.ParsedExpr{Expr: expr}) + + _, iss = env.Check(deepAst) + if iss == nil || iss.Err() == nil { + t.Fatalf("Check(deepAst) expected an error, got nil") + } + if !strings.Contains(iss.Err().Error(), "maximum expression nesting depth") { + t.Errorf("Check(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", iss.Err()) + } + + if _, err := env.Program(deepAst); err == nil { + t.Fatalf("Program(deepAst) expected an error, got nil") + } else if !strings.Contains(err.Error(), "maximum expression nesting depth") { + t.Errorf("Program(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", err) + } +} diff --git a/common/ast/depth.go b/common/ast/depth.go new file mode 100644 index 000000000..e12d742bf --- /dev/null +++ b/common/ast/depth.go @@ -0,0 +1,101 @@ +// Copyright 2026 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 +// +// http://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 ast + +// MaxNestingDepth is the default maximum expression nesting depth applied to +// ASTs that enter through non-parser ingestion paths. It mirrors the parser's +// default maxRecursionDepth so loaded ASTs are validated against the same +// bound as ASTs produced from CEL source. +const MaxNestingDepth = 250 + +// ExceedsMaxDepth reports whether the given expression nests deeper than +// maxDepth. The traversal itself is bounded: it never recurses past +// maxDepth+1 levels, so it is safe to call on adversarially deep inputs that +// would otherwise blow the Go stack during later checking or planning. +// +// A non-positive maxDepth disables the check and always returns false. +func ExceedsMaxDepth(e Expr, maxDepth int) bool { + if maxDepth <= 0 { + return false + } + return exceedsMaxDepth(e, 0, maxDepth) +} + +func exceedsMaxDepth(e Expr, depth, maxDepth int) bool { + if e == nil { + return false + } + if depth > maxDepth { + return true + } + switch e.Kind() { + case CallKind: + c := e.AsCall() + if c.IsMemberFunction() { + if exceedsMaxDepth(c.Target(), depth+1, maxDepth) { + return true + } + } + for _, arg := range c.Args() { + if exceedsMaxDepth(arg, depth+1, maxDepth) { + return true + } + } + case ComprehensionKind: + c := e.AsComprehension() + if exceedsMaxDepth(c.IterRange(), depth+1, maxDepth) { + return true + } + if exceedsMaxDepth(c.AccuInit(), depth+1, maxDepth) { + return true + } + if exceedsMaxDepth(c.LoopCondition(), depth+1, maxDepth) { + return true + } + if exceedsMaxDepth(c.LoopStep(), depth+1, maxDepth) { + return true + } + if exceedsMaxDepth(c.Result(), depth+1, maxDepth) { + return true + } + case ListKind: + for _, elem := range e.AsList().Elements() { + if exceedsMaxDepth(elem, depth+1, maxDepth) { + return true + } + } + case MapKind: + for _, entry := range e.AsMap().Entries() { + me := entry.AsMapEntry() + if exceedsMaxDepth(me.Key(), depth+1, maxDepth) { + return true + } + if exceedsMaxDepth(me.Value(), depth+1, maxDepth) { + return true + } + } + case SelectKind: + if exceedsMaxDepth(e.AsSelect().Operand(), depth+1, maxDepth) { + return true + } + case StructKind: + for _, f := range e.AsStruct().Fields() { + if exceedsMaxDepth(f.AsStructField().Value(), depth+1, maxDepth) { + return true + } + } + } + return false +} From 662a01941f70d48b79a7554ce73dcb00b6be978b Mon Sep 17 00:00:00 2001 From: William Date: Thu, 4 Jun 2026 20:07:01 -0700 Subject: [PATCH 2/3] cel: make loaded-AST depth limit a configurable NewEnv option The depth bound for ASTs ingested outside the parser (via ParsedExprToAst / CheckedExprToAst) was an exported package constant, common/ast.MaxNestingDepth. Replace it with a cel.ExpressionNestingDepthLimit functional option on NewEnv so the behavior is user-configurable with a sensible default rather than a public package-level knob. The option defaults to 250, matching the parser's maxRecursionDepth, and a negative value disables the check. Enforce it once during Program planning (PlanProgram) and drop the separate Env.Check guard, leaving a single configurable enforcement point. --- cel/env.go | 22 +++++++++------------- cel/io_test.go | 20 ++++++++++++-------- cel/options.go | 17 +++++++++++++++++ common/ast/depth.go | 6 ------ 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/cel/env.go b/cel/env.go index 0486a0a48..a25e55c4d 100644 --- a/cel/env.go +++ b/cel/env.go @@ -403,16 +403,6 @@ func (e *Env) Check(ast *Ast) (*Ast, *Issues) { return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) } - // Guard against ASTs that bypass the parser depth limit (e.g. loaded via - // ParsedExprToAst / CheckedExprToAst), since later recursive checking - // would otherwise risk a Go stack overflow on adversarially deep inputs. - if celast.ExceedsMaxDepth(ast.NativeRep().Expr(), celast.MaxNestingDepth) { - errs := common.NewErrors(ast.Source()) - errs.ReportErrorString(common.NoLocation, - fmt.Sprintf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth)) - return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) - } - checked, errs := checker.Check(ast.NativeRep(), ast.Source(), chk) if len(errs.GetErrors()) > 0 { return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo()) @@ -706,9 +696,15 @@ func (e *Env) PlanProgram(a *celast.AST, opts ...ProgramOption) (Program, error) // Guard against ASTs that bypass the parser depth limit (e.g. loaded via // ParsedExprToAst / CheckedExprToAst), since later recursive planning and // evaluation would otherwise risk a Go stack overflow on adversarially - // deep inputs. - if a != nil && celast.ExceedsMaxDepth(a.Expr(), celast.MaxNestingDepth) { - return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth) + // deep inputs. The limit is configurable via ExpressionNestingDepthLimit; + // a zero value falls back to the parser-matching default and a negative + // value disables the check. + maxDepth := e.limits[limitMaxASTDepth] + if maxDepth == 0 { + maxDepth = defaultMaxASTDepth + } + if a != nil && celast.ExceedsMaxDepth(a.Expr(), maxDepth) { + return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", maxDepth) } optSet := e.progOpts if len(opts) != 0 { diff --git a/cel/io_test.go b/cel/io_test.go index a31f03a93..1086de693 100644 --- a/cel/io_test.go +++ b/cel/io_test.go @@ -360,17 +360,21 @@ func TestLoadedAstDepthLimit(t *testing.T) { } deepAst := ParsedExprToAst(&exprpb.ParsedExpr{Expr: expr}) - _, iss = env.Check(deepAst) - if iss == nil || iss.Err() == nil { - t.Fatalf("Check(deepAst) expected an error, got nil") - } - if !strings.Contains(iss.Err().Error(), "maximum expression nesting depth") { - t.Errorf("Check(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", iss.Err()) - } - + // Program enforces the default depth limit and returns a normal error + // rather than overflowing the stack during planning. if _, err := env.Program(deepAst); err == nil { t.Fatalf("Program(deepAst) expected an error, got nil") } else if !strings.Contains(err.Error(), "maximum expression nesting depth") { t.Errorf("Program(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", err) } + + // The limit is configurable via the ExpressionNestingDepthLimit option: a + // negative value disables the check so the same deep AST plans cleanly. + unbounded, err := NewEnv(ExpressionNestingDepthLimit(-1)) + if err != nil { + t.Fatalf("NewEnv(ExpressionNestingDepthLimit(-1)) failed: %v", err) + } + if _, err := unbounded.Program(deepAst); err != nil { + t.Errorf("Program(deepAst) with depth checking disabled failed: %v", err) + } } diff --git a/cel/options.go b/cel/options.go index d7d2ab034..f70c54cfc 100644 --- a/cel/options.go +++ b/cel/options.go @@ -109,8 +109,15 @@ 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", @@ -942,6 +949,16 @@ func ParserExpressionSizeLimit(limit int) EnvOption { return setLimit(limitCodePointSize, limit) } +// ExpressionNestingDepthLimit bounds the nesting depth permitted for ASTs that +// are loaded outside the parser (e.g. via ParsedExprToAst / CheckedExprToAst), +// which would otherwise bypass the parser's recursion limit. The limit is +// enforced when a Program is planned, returning a normal error rather than +// risking a Go stack overflow on adversarially deep inputs. It defaults to the +// parser's recursion limit (250); a negative value disables the check. +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 { diff --git a/common/ast/depth.go b/common/ast/depth.go index e12d742bf..f4a42dd55 100644 --- a/common/ast/depth.go +++ b/common/ast/depth.go @@ -14,12 +14,6 @@ package ast -// MaxNestingDepth is the default maximum expression nesting depth applied to -// ASTs that enter through non-parser ingestion paths. It mirrors the parser's -// default maxRecursionDepth so loaded ASTs are validated against the same -// bound as ASTs produced from CEL source. -const MaxNestingDepth = 250 - // ExceedsMaxDepth reports whether the given expression nests deeper than // maxDepth. The traversal itself is bounded: it never recurses past // maxDepth+1 levels, so it is safe to call on adversarially deep inputs that From d33c83ab2279cb4aa9d31a174bbc095ff443c7a7 Mon Sep 17 00:00:00 2001 From: William Date: Sat, 6 Jun 2026 04:04:22 -0700 Subject: [PATCH 3/3] cel: enforce loaded-AST depth at conversion via common/ast visitor Address review feedback on the loaded-AST depth guard: - common/ast: replace the bespoke depth.go traversal with ExceedsDepth in navigable.go, built on the existing NavigableExpr.Depth() visitor. The walk is bounded to maxDepth+1 levels so it stays safe on the adversarially deep inputs it guards against. - cel: move the depth check out of the PlanProgram hot path and into the ParsedExprToAst / CheckedExprToAst proto-conversion helpers, the actual entry points for ASTs that bypass the parser. The expensive traversal now runs once at load; Check/Program only read the recorded error. Embedders in full control of their AST inputs can skip the check by constructing the AST through the common/ast package directly. - cel: register limitMaxASTDepth in limitIDsToNames so the depth limit round-trips through env.Config export/import. Updates the regression test to cover the parsed and checked conversion paths, Check surfacing, the low-level bypass, and the env-config round-trip. --- cel/env.go | 30 +++++++----- cel/io.go | 28 ++++++++++- cel/io_test.go | 106 ++++++++++++++++++++++++++++------------ cel/options.go | 15 +++--- common/ast/depth.go | 95 ----------------------------------- common/ast/navigable.go | 23 +++++++++ 6 files changed, 149 insertions(+), 148 deletions(-) delete mode 100644 common/ast/depth.go diff --git a/cel/env.go b/cel/env.go index a25e55c4d..b226e2a86 100644 --- a/cel/env.go +++ b/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,25 +698,18 @@ 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...) } // PlanProgram generates an evaluable instance of the AST in the go-native representation within // the environment (Env). func (e *Env) PlanProgram(a *celast.AST, opts ...ProgramOption) (Program, error) { - // Guard against ASTs that bypass the parser depth limit (e.g. loaded via - // ParsedExprToAst / CheckedExprToAst), since later recursive planning and - // evaluation would otherwise risk a Go stack overflow on adversarially - // deep inputs. The limit is configurable via ExpressionNestingDepthLimit; - // a zero value falls back to the parser-matching default and a negative - // value disables the check. - maxDepth := e.limits[limitMaxASTDepth] - if maxDepth == 0 { - maxDepth = defaultMaxASTDepth - } - if a != nil && celast.ExceedsMaxDepth(a.Expr(), maxDepth) { - return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", maxDepth) - } optSet := e.progOpts if len(opts) != 0 { mergedOpts := []ProgramOption{} diff --git a/cel/io.go b/cel/io.go index 2e611228d..c991c95c3 100644 --- a/cel/io.go +++ b/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. diff --git a/cel/io_test.go b/cel/io_test.go index 1086de693..60a858125 100644 --- a/cel/io_test.go +++ b/cel/io_test.go @@ -317,6 +317,32 @@ 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 { @@ -335,46 +361,62 @@ func TestLoadedAstDepthLimit(t *testing.T) { t.Fatalf("Program(shallow) failed: %v", err) } - // Build a synthetic deeply nested AST (depth ~300) using iteratively - // stacked unary `!` calls, well above the 250 default but far below the - // Go stack limit so the test itself never crashes. const depth = 300 - expr := &exprpb.Expr{ - Id: 1, - ExprKind: &exprpb.Expr_ConstExpr{ - ConstExpr: &exprpb.Constant{ - ConstantKind: &exprpb.Constant_BoolValue{BoolValue: true}, - }, - }, + 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) } - 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}, - }, - }, - } + 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()) } - deepAst := ParsedExprToAst(&exprpb.ParsedExpr{Expr: expr}) - // Program enforces the default depth limit and returns a normal error - // rather than overflowing the stack during planning. - if _, err := env.Program(deepAst); err == nil { - t.Fatalf("Program(deepAst) expected an error, got nil") + // 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("Program(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", err) + 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) + } +} - // The limit is configurable via the ExpressionNestingDepthLimit option: a - // negative value disables the check so the same deep AST plans cleanly. - unbounded, err := NewEnv(ExpressionNestingDepthLimit(-1)) +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("NewEnv(ExpressionNestingDepthLimit(-1)) failed: %v", err) + 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 _, err := unbounded.Program(deepAst); err != nil { - t.Errorf("Program(deepAst) with depth checking disabled failed: %v", err) + if !found { + t.Errorf("env config limits %v missing 'cel.limit.max_ast_depth'", conf.Limits) } } diff --git a/cel/options.go b/cel/options.go index f70c54cfc..e7b6f2439 100644 --- a/cel/options.go +++ b/cel/options.go @@ -122,6 +122,7 @@ 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) { @@ -949,12 +950,14 @@ func ParserExpressionSizeLimit(limit int) EnvOption { return setLimit(limitCodePointSize, limit) } -// ExpressionNestingDepthLimit bounds the nesting depth permitted for ASTs that -// are loaded outside the parser (e.g. via ParsedExprToAst / CheckedExprToAst), -// which would otherwise bypass the parser's recursion limit. The limit is -// enforced when a Program is planned, returning a normal error rather than -// risking a Go stack overflow on adversarially deep inputs. It defaults to the -// parser's recursion limit (250); a negative value disables the check. +// 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) } diff --git a/common/ast/depth.go b/common/ast/depth.go deleted file mode 100644 index f4a42dd55..000000000 --- a/common/ast/depth.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2026 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 -// -// http://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 ast - -// ExceedsMaxDepth reports whether the given expression nests deeper than -// maxDepth. The traversal itself is bounded: it never recurses past -// maxDepth+1 levels, so it is safe to call on adversarially deep inputs that -// would otherwise blow the Go stack during later checking or planning. -// -// A non-positive maxDepth disables the check and always returns false. -func ExceedsMaxDepth(e Expr, maxDepth int) bool { - if maxDepth <= 0 { - return false - } - return exceedsMaxDepth(e, 0, maxDepth) -} - -func exceedsMaxDepth(e Expr, depth, maxDepth int) bool { - if e == nil { - return false - } - if depth > maxDepth { - return true - } - switch e.Kind() { - case CallKind: - c := e.AsCall() - if c.IsMemberFunction() { - if exceedsMaxDepth(c.Target(), depth+1, maxDepth) { - return true - } - } - for _, arg := range c.Args() { - if exceedsMaxDepth(arg, depth+1, maxDepth) { - return true - } - } - case ComprehensionKind: - c := e.AsComprehension() - if exceedsMaxDepth(c.IterRange(), depth+1, maxDepth) { - return true - } - if exceedsMaxDepth(c.AccuInit(), depth+1, maxDepth) { - return true - } - if exceedsMaxDepth(c.LoopCondition(), depth+1, maxDepth) { - return true - } - if exceedsMaxDepth(c.LoopStep(), depth+1, maxDepth) { - return true - } - if exceedsMaxDepth(c.Result(), depth+1, maxDepth) { - return true - } - case ListKind: - for _, elem := range e.AsList().Elements() { - if exceedsMaxDepth(elem, depth+1, maxDepth) { - return true - } - } - case MapKind: - for _, entry := range e.AsMap().Entries() { - me := entry.AsMapEntry() - if exceedsMaxDepth(me.Key(), depth+1, maxDepth) { - return true - } - if exceedsMaxDepth(me.Value(), depth+1, maxDepth) { - return true - } - } - case SelectKind: - if exceedsMaxDepth(e.AsSelect().Operand(), depth+1, maxDepth) { - return true - } - case StructKind: - for _, f := range e.AsStruct().Fields() { - if exceedsMaxDepth(f.AsStructField().Value(), depth+1, maxDepth) { - return true - } - } - } - return false -} diff --git a/common/ast/navigable.go b/common/ast/navigable.go index 13e5777b5..364edfa3a 100644 --- a/common/ast/navigable.go +++ b/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 (