Skip to content

Commit 662a019

Browse files
committed
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.
1 parent 6cdc6db commit 662a019

4 files changed

Lines changed: 38 additions & 27 deletions

File tree

cel/env.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -403,16 +403,6 @@ func (e *Env) Check(ast *Ast) (*Ast, *Issues) {
403403
return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo())
404404
}
405405

406-
// Guard against ASTs that bypass the parser depth limit (e.g. loaded via
407-
// ParsedExprToAst / CheckedExprToAst), since later recursive checking
408-
// would otherwise risk a Go stack overflow on adversarially deep inputs.
409-
if celast.ExceedsMaxDepth(ast.NativeRep().Expr(), celast.MaxNestingDepth) {
410-
errs := common.NewErrors(ast.Source())
411-
errs.ReportErrorString(common.NoLocation,
412-
fmt.Sprintf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth))
413-
return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo())
414-
}
415-
416406
checked, errs := checker.Check(ast.NativeRep(), ast.Source(), chk)
417407
if len(errs.GetErrors()) > 0 {
418408
return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo())
@@ -706,9 +696,15 @@ func (e *Env) PlanProgram(a *celast.AST, opts ...ProgramOption) (Program, error)
706696
// Guard against ASTs that bypass the parser depth limit (e.g. loaded via
707697
// ParsedExprToAst / CheckedExprToAst), since later recursive planning and
708698
// evaluation would otherwise risk a Go stack overflow on adversarially
709-
// deep inputs.
710-
if a != nil && celast.ExceedsMaxDepth(a.Expr(), celast.MaxNestingDepth) {
711-
return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", celast.MaxNestingDepth)
699+
// deep inputs. The limit is configurable via ExpressionNestingDepthLimit;
700+
// a zero value falls back to the parser-matching default and a negative
701+
// value disables the check.
702+
maxDepth := e.limits[limitMaxASTDepth]
703+
if maxDepth == 0 {
704+
maxDepth = defaultMaxASTDepth
705+
}
706+
if a != nil && celast.ExceedsMaxDepth(a.Expr(), maxDepth) {
707+
return nil, fmt.Errorf("input exceeds maximum expression nesting depth: %d", maxDepth)
712708
}
713709
optSet := e.progOpts
714710
if len(opts) != 0 {

cel/io_test.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -360,17 +360,21 @@ func TestLoadedAstDepthLimit(t *testing.T) {
360360
}
361361
deepAst := ParsedExprToAst(&exprpb.ParsedExpr{Expr: expr})
362362

363-
_, iss = env.Check(deepAst)
364-
if iss == nil || iss.Err() == nil {
365-
t.Fatalf("Check(deepAst) expected an error, got nil")
366-
}
367-
if !strings.Contains(iss.Err().Error(), "maximum expression nesting depth") {
368-
t.Errorf("Check(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", iss.Err())
369-
}
370-
363+
// Program enforces the default depth limit and returns a normal error
364+
// rather than overflowing the stack during planning.
371365
if _, err := env.Program(deepAst); err == nil {
372366
t.Fatalf("Program(deepAst) expected an error, got nil")
373367
} else if !strings.Contains(err.Error(), "maximum expression nesting depth") {
374368
t.Errorf("Program(deepAst) error = %v, want it to mention 'maximum expression nesting depth'", err)
375369
}
370+
371+
// The limit is configurable via the ExpressionNestingDepthLimit option: a
372+
// negative value disables the check so the same deep AST plans cleanly.
373+
unbounded, err := NewEnv(ExpressionNestingDepthLimit(-1))
374+
if err != nil {
375+
t.Fatalf("NewEnv(ExpressionNestingDepthLimit(-1)) failed: %v", err)
376+
}
377+
if _, err := unbounded.Program(deepAst); err != nil {
378+
t.Errorf("Program(deepAst) with depth checking disabled failed: %v", err)
379+
}
376380
}

cel/options.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,15 @@ const (
109109
limitCodePointSize
110110
// The number of attempts to recover from a parse error.
111111
limitParseErrorRecovery
112+
// The maximum nesting depth permitted for ASTs loaded outside the parser.
113+
limitMaxASTDepth
112114
)
113115

116+
// defaultMaxASTDepth mirrors the parser's default maxRecursionDepth (250) and
117+
// is applied to ASTs that enter through non-parser ingestion paths (e.g. via
118+
// ParsedExprToAst / CheckedExprToAst) when no explicit limit is configured.
119+
const defaultMaxASTDepth = 250
120+
114121
var limitIDsToNames = map[limitID]string{
115122
limitCodePointSize: "cel.limit.expression_code_points",
116123
limitParseErrorRecovery: "cel.limit.parse_error_recovery",
@@ -942,6 +949,16 @@ func ParserExpressionSizeLimit(limit int) EnvOption {
942949
return setLimit(limitCodePointSize, limit)
943950
}
944951

952+
// ExpressionNestingDepthLimit bounds the nesting depth permitted for ASTs that
953+
// are loaded outside the parser (e.g. via ParsedExprToAst / CheckedExprToAst),
954+
// which would otherwise bypass the parser's recursion limit. The limit is
955+
// enforced when a Program is planned, returning a normal error rather than
956+
// risking a Go stack overflow on adversarially deep inputs. It defaults to the
957+
// parser's recursion limit (250); a negative value disables the check.
958+
func ExpressionNestingDepthLimit(limit int) EnvOption {
959+
return setLimit(limitMaxASTDepth, limit)
960+
}
961+
945962
// EnableHiddenAccumulatorName sets the parser to use the identifier '@result' for accumulators
946963
// which is not normally accessible from CEL source.
947964
func EnableHiddenAccumulatorName(enabled bool) EnvOption {

common/ast/depth.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@
1414

1515
package ast
1616

17-
// MaxNestingDepth is the default maximum expression nesting depth applied to
18-
// ASTs that enter through non-parser ingestion paths. It mirrors the parser's
19-
// default maxRecursionDepth so loaded ASTs are validated against the same
20-
// bound as ASTs produced from CEL source.
21-
const MaxNestingDepth = 250
22-
2317
// ExceedsMaxDepth reports whether the given expression nests deeper than
2418
// maxDepth. The traversal itself is bounded: it never recurses past
2519
// maxDepth+1 levels, so it is safe to call on adversarially deep inputs that

0 commit comments

Comments
 (0)