From 9dfbad0d695e8f191503b2f052497373439a9771 Mon Sep 17 00:00:00 2001 From: Tristan Swadell Date: Wed, 22 Jul 2026 22:13:55 +0000 Subject: [PATCH] Introduce cel.bind nesting limit --- cel/env_test.go | 20 +++++++++ cel/validator.go | 103 ++++++++++++++++++++++++++++++++++++++++--- ext/bindings_test.go | 65 +++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 7 deletions(-) diff --git a/cel/env_test.go b/cel/env_test.go index 6dbc13696..eb4c20850 100644 --- a/cel/env_test.go +++ b/cel/env_test.go @@ -439,8 +439,10 @@ func TestEnvToConfig(t *testing.T) { opts: []EnvOption{ ExtendedValidations(), ASTValidators(ValidateComprehensionNestingLimit(1)), + ASTValidators(ValidateBindNestingLimit(2)), }, want: env.NewConfig("validators").AddValidators( + env.NewValidator("cel.validator.bind_nesting_limit").SetConfig(map[string]any{"limit": 2}), env.NewValidator("cel.validator.comprehension_nesting_limit").SetConfig(map[string]any{"limit": 1}), env.NewValidator("cel.validator.duration"), env.NewValidator("cel.validator.homogeneous_literals"), @@ -1023,6 +1025,24 @@ func TestEnvFromConfigErrors(t *testing.T) { AddValidators(env.NewValidator("cel.validator.comprehension_nesting_limit").SetConfig(map[string]any{"limit": 2.5})), want: errors.New("invalid validator: cel.validator.comprehension_nesting_limit, limit value is not a whole number: 2.5"), }, + { + name: "invalid cel_bind validator config", + conf: env.NewConfig("invalid validator config"). + AddValidators(env.NewValidator("cel.validator.bind_nesting_limit")), + want: errors.New("invalid validator"), + }, + { + name: "invalid cel_bind validator config type - unsupported type", + conf: env.NewConfig("invalid validator config"). + AddValidators(env.NewValidator("cel.validator.bind_nesting_limit").SetConfig(map[string]any{"limit": "2"})), + want: errors.New("invalid validator"), + }, + { + name: "invalid cel_bind validator config type - fractional", + conf: env.NewConfig("invalid validator config"). + AddValidators(env.NewValidator("cel.validator.bind_nesting_limit").SetConfig(map[string]any{"limit": 2.5})), + want: errors.New("invalid validator: cel.validator.bind_nesting_limit, limit value is not a whole number: 2.5"), + }, } for _, tst := range tests { tc := tst diff --git a/cel/validator.go b/cel/validator.go index 905b99990..505e00445 100644 --- a/cel/validator.go +++ b/cel/validator.go @@ -26,11 +26,12 @@ import ( ) const ( - durationValidatorName = "cel.validator.duration" - regexValidatorName = "cel.validator.matches" - timestampValidatorName = "cel.validator.timestamp" - homogeneousValidatorName = "cel.validator.homogeneous_literals" - nestingLimitValidatorName = "cel.validator.comprehension_nesting_limit" + durationValidatorName = "cel.validator.duration" + regexValidatorName = "cel.validator.matches" + timestampValidatorName = "cel.validator.timestamp" + homogeneousValidatorName = "cel.validator.homogeneous_literals" + nestingLimitValidatorName = "cel.validator.comprehension_nesting_limit" + bindNestingLimitValidatorName = "cel.validator.bind_nesting_limit" // HomogeneousAggregateLiteralExemptFunctions is the ValidatorConfig key used to configure // the set of function names which are exempt from homogeneous type checks. The expected type @@ -61,6 +62,23 @@ var ( } return nil, fmt.Errorf("invalid validator: %s missing limit", nestingLimitValidatorName) }, + bindNestingLimitValidatorName: func(val *env.Validator) (ASTValidator, error) { + if limit, found := val.ConfigValue("limit"); found { + // In case of protos, config value is of type by google.protobuf.Value, which numeric values are always a double. + if val, isDouble := limit.(float64); isDouble { + if val != float64(int64(val)) { + return nil, fmt.Errorf("invalid validator: %s, limit value is not a whole number: %v", bindNestingLimitValidatorName, limit) + } + return ValidateBindNestingLimit(int(val)), nil + } + + if val, isInt := limit.(int); isInt { + return ValidateBindNestingLimit(val), nil + } + return nil, fmt.Errorf("invalid validator: %s unsupported limit type: %v", bindNestingLimitValidatorName, limit) + } + return nil, fmt.Errorf("invalid validator: %s missing limit", bindNestingLimitValidatorName) + }, durationValidatorName: func(*env.Validator) (ASTValidator, error) { return ValidateDurationLiterals(), nil }, @@ -233,6 +251,13 @@ func ValidateComprehensionNestingLimit(limit int) ASTValidator { return nestingLimitValidator{limit: limit} } +// ValidateBindNestingLimit ensures that cel.bind() macro nesting does not exceed the specified limit. +// +// This validator can be useful for preventing arbitrarily nested cel.bind() macro calls. +func ValidateBindNestingLimit(limit int) ASTValidator { + return bindNestingLimitValidator{limit: limit} +} + type argChecker func(env *Env, call, arg ast.Expr) error func newFormatValidator(funcName string, argNum int, check argChecker) formatValidator { @@ -432,8 +457,7 @@ func (v nestingLimitValidator) Validate(e *Env, _ ValidatorConfig, a *ast.AST, i } // When the comprehension has an empty range, continue to the next ancestor // as this comprehension does not have any associated cost. - iterRange := e.AsComprehension().IterRange() - if iterRange.Kind() == ast.ListKind && iterRange.AsList().Size() == 0 { + if isEmptyRangeComprehension(e) { e, hasParent = e.Parent() continue } @@ -447,3 +471,68 @@ func (v nestingLimitValidator) Validate(e *Env, _ ValidatorConfig, a *ast.AST, i } } } + +type bindNestingLimitValidator struct { + limit int +} + +// Name returns the name of the cel.bind nesting limit validator. +func (v bindNestingLimitValidator) Name() string { + return bindNestingLimitValidatorName +} + +// ToConfig converts the ASTValidator to an env.Validator specifying the validator name and the nesting limit +// as an integer value: {"limit": int} +func (v bindNestingLimitValidator) ToConfig() *env.Validator { + return env.NewValidator(v.Name()).SetConfig(map[string]any{"limit": v.limit}) +} + +// Validate implements the ASTValidator interface method. +func (v bindNestingLimitValidator) Validate(e *Env, _ ValidatorConfig, a *ast.AST, iss *Issues) { + root := ast.NavigateAST(a) + comprehensions := ast.MatchDescendants(root, ast.KindMatcher(ast.ComprehensionKind)) + var celBinds []ast.NavigableExpr + for _, comp := range comprehensions { + if isCelBind(comp) { + celBinds = append(celBinds, comp) + } + } + if len(celBinds) <= v.limit { + return + } + for _, comp := range celBinds { + count := 0 + e := comp + hasParent := true + for hasParent { + if isCelBind(e) { + count++ + if count > v.limit { + iss.ReportErrorAtID(comp.ID(), "cel.bind exceeds nesting limit") + break + } + } + e, hasParent = e.Parent() + } + } +} + +func isEmptyRangeComprehension(e ast.NavigableExpr) bool { + if e.Kind() != ast.ComprehensionKind { + return false + } + iterRange := e.AsComprehension().IterRange() + return iterRange.Kind() == ast.ListKind && iterRange.AsList().Size() == 0 +} + +func isCelBind(e ast.NavigableExpr) bool { + if !isEmptyRangeComprehension(e) { + return false + } + compre := e.AsComprehension() + loopCond := compre.LoopCondition() + loopStep := compre.LoopStep() + return compre.IterVar() == unusedIterVar && + loopCond.Kind() == ast.LiteralKind && loopCond.AsLiteral().Value() == false && + loopStep.Kind() == ast.IdentKind && loopStep.AsIdent() == compre.AccuVar() +} diff --git a/ext/bindings_test.go b/ext/bindings_test.go index b02e1208f..4999cc416 100644 --- a/ext/bindings_test.go +++ b/ext/bindings_test.go @@ -27,6 +27,7 @@ import ( "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/interpreter" + "github.com/google/cel-go/test" ) var bindingTests = []struct { @@ -563,3 +564,67 @@ func BenchmarkBlockEval(b *testing.B) { prg.Eval(input) } } + +func TestValidateBindNestingLimit(t *testing.T) { + env, err := cel.NewEnv( + Bindings(), + cel.ASTValidators(cel.ValidateBindNestingLimit(2)), + ) + if err != nil { + t.Fatalf("cel.NewEnv() failed: %v", err) + } + tests := []struct { + expr string + iss string + }{ + { + expr: `cel.bind(a, 1, a + 1)`, + }, + { + expr: `cel.bind(a, 1, cel.bind(b, 2, a + b))`, + }, + { + // two cel.binds, but in separate branches + expr: `cel.bind(a, 1, a) + cel.bind(b, 2, b)`, + }, + { + // empty iteration range comprehension (e.g. cel.bind) does not count against comprehension limit, + // but counts against cel.bind nesting limit. + expr: `[1, 2, 3].exists(i, cel.bind(a, i, cel.bind(b, a, a + b) > 0))`, + }, + { + // three cel.binds, three levels deep + expr: `cel.bind(a, 1, cel.bind(b, 2, cel.bind(c, 3, a + b + c)))`, + iss: ` + ERROR: :1:39: cel.bind exceeds nesting limit + | cel.bind(a, 1, cel.bind(b, 2, cel.bind(c, 3, a + b + c))) + | ......................................^`, + }, + { + // three cel.binds, three levels deep with non-comprehension ancestor (+) + expr: `cel.bind(a, 1, cel.bind(b, 2, 1 + cel.bind(c, 3, a + b + c)))`, + iss: ` + ERROR: :1:43: cel.bind exceeds nesting limit + | cel.bind(a, 1, cel.bind(b, 2, 1 + cel.bind(c, 3, a + b + c))) + | ..........................................^`, + }, + } + for _, tst := range tests { + tc := tst + t.Run(tc.expr, func(t *testing.T) { + _, iss := env.Compile(tc.expr) + if tc.iss != "" { + if iss.Err() == nil { + t.Fatalf("env.Compile(%v) returned ast, expected error: %v", tc.expr, tc.iss) + } + if !test.Compare(iss.Err().Error(), tc.iss) { + t.Fatalf("env.Compile(%v) returned %v, expected error: %v", tc.expr, iss.Err(), tc.iss) + } + return + } + if iss.Err() != nil { + t.Fatalf("env.Compile(%v) failed: %v", tc.expr, iss.Err()) + } + }) + } +}