diff --git a/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel b/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel index b7050466c..67e791317 100644 --- a/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel +++ b/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel @@ -84,6 +84,23 @@ java_library( ], ) +java_library( + name = "comprehension_nesting_limit_validator", + srcs = [ + "ComprehensionNestingLimitValidator.java", + ], + tags = [ + ], + deps = [ + "//bundle:cel", + "//common/ast", + "//common/navigation", + "//common/navigation:common", + "//validator:ast_validator", + "@maven//:com_google_guava_guava", + ], +) + java_library( name = "literal_validator", srcs = [ diff --git a/validator/src/main/java/dev/cel/validator/validators/ComprehensionNestingLimitValidator.java b/validator/src/main/java/dev/cel/validator/validators/ComprehensionNestingLimitValidator.java new file mode 100644 index 000000000..55dadacc5 --- /dev/null +++ b/validator/src/main/java/dev/cel/validator/validators/ComprehensionNestingLimitValidator.java @@ -0,0 +1,85 @@ +// Copyright 2025 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 +// +// https://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 dev.cel.validator.validators; + +import static com.google.common.base.Preconditions.checkArgument; + +import dev.cel.bundle.Cel; +import dev.cel.common.ast.CelExpr.ExprKind; +import dev.cel.common.navigation.CelNavigableAst; +import dev.cel.common.navigation.CelNavigableExpr; +import dev.cel.common.navigation.TraversalOrder; +import dev.cel.validator.CelAstValidator; + +/** + * Checks that the nesting depth of comprehensions does not exceed the configured limit. Nesting + * occurs when a comprehension is used in the range expression or the body of a comprehension. + * + *

Trivial comprehensions (comprehensions over an empty range) do not count towards the limit. + */ +public final class ComprehensionNestingLimitValidator implements CelAstValidator { + private final int nestingLimit; + + /** + * Constructs a new instance of {@link ComprehensionNestingLimitValidator} with the configured + * maxNesting as its limit. A limit of 0 means no comprehensions are allowed. + */ + public static ComprehensionNestingLimitValidator newInstance(int maxNesting) { + checkArgument(maxNesting >= 0); + return new ComprehensionNestingLimitValidator(maxNesting); + } + + @Override + public void validate(CelNavigableAst navigableAst, Cel cel, IssuesFactory issuesFactory) { + navigableAst + .getRoot() + .allNodes(TraversalOrder.PRE_ORDER) + .filter(node -> node.getKind().equals(ExprKind.Kind.COMPREHENSION)) + .filter(node -> nestingLevel(node) > nestingLimit) + .forEach( + node -> + issuesFactory.addError( + node.id(), + String.format( + "comprehension nesting exceeds the configured limit: %s.", nestingLimit))); + } + + private static boolean isTrivialComprehension(CelNavigableExpr node) { + return (node.expr().comprehension().iterRange().getKind().equals(ExprKind.Kind.LIST) + && node.expr().comprehension().iterRange().list().elements().isEmpty()) + || (node.expr().comprehension().iterRange().getKind().equals(ExprKind.Kind.MAP) + && node.expr().comprehension().iterRange().map().entries().isEmpty()); + } + + private static int nestingLevel(CelNavigableExpr node) { + if (isTrivialComprehension(node)) { + return 0; + } + int count = 1; + while (node.parent().isPresent()) { + CelNavigableExpr parent = node.parent().get(); + + if (parent.getKind().equals(ExprKind.Kind.COMPREHENSION) && !isTrivialComprehension(parent)) { + count++; + } + node = parent; + } + return count; + } + + private ComprehensionNestingLimitValidator(int maxNesting) { + this.nestingLimit = maxNesting; + } +} diff --git a/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel b/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel index 9e88e0d16..cb35e7a6b 100644 --- a/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel +++ b/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel @@ -16,12 +16,15 @@ java_library( "//common:proto_ast", "//common/internal:proto_time_utils", "//common/types", + "//extensions", "//extensions:optional_library", + "//parser:macro", "//runtime", "//runtime:function_binding", "//validator", "//validator:validator_builder", "//validator/validators:ast_depth_limit_validator", + "//validator/validators:comprehension_nesting_limit_validator", "//validator/validators:duration", "//validator/validators:homogeneous_literal", "//validator/validators:regex", diff --git a/validator/src/test/java/dev/cel/validator/validators/ComprehensionNestingLimitValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/ComprehensionNestingLimitValidatorTest.java new file mode 100644 index 000000000..ccf625d88 --- /dev/null +++ b/validator/src/test/java/dev/cel/validator/validators/ComprehensionNestingLimitValidatorTest.java @@ -0,0 +1,176 @@ +// Copyright 2025 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 +// +// https://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 dev.cel.validator.validators; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.testing.junit.testparameterinjector.TestParameter; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import dev.cel.bundle.Cel; +import dev.cel.bundle.CelFactory; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelIssue.Severity; +import dev.cel.common.CelValidationException; +import dev.cel.common.CelValidationResult; +import dev.cel.common.types.SimpleType; +import dev.cel.extensions.CelExtensions; +import dev.cel.parser.CelStandardMacro; +import dev.cel.validator.CelValidator; +import dev.cel.validator.CelValidatorFactory; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(TestParameterInjector.class) +public class ComprehensionNestingLimitValidatorTest { + + private static final Cel CEL = + CelFactory.standardCelBuilder() + .addCompilerLibraries( + CelExtensions.optional(), CelExtensions.comprehensions(), CelExtensions.bindings()) + .addRuntimeLibraries(CelExtensions.optional(), CelExtensions.comprehensions()) + .setStandardMacros(CelStandardMacro.STANDARD_MACROS) + .addVar("x", SimpleType.DYN) + .build(); + + private static final CelValidator CEL_VALIDATOR = + CelValidatorFactory.standardCelValidatorBuilder(CEL) + .addAstValidators(ComprehensionNestingLimitValidator.newInstance(1)) + .build(); + + @Test + public void comprehensionNestingLimit_populatesErrors( + @TestParameter({ + "[1, 2, 3].map(x, [1, 2, 3].map(y, x + y))", + "[1, 2, 3].map(x, [1, 2, 3].map(y, x + y).size() > 0, x)", + "[1, 2, 3].all(x, [1, 2, 3].exists(y, x + y > 0))", + "[1, 2, 3].map(x, {x: [1, 2, 3].map(y, x + y)})", + "[1, 2, 3].exists(i, v, i < 3 && [1, 2, 3].all(j, v2, j < 3 && v2 > 0))", + "{1: 2}.all(k, {2: 3}.all(k2, k != k2))" + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getAllIssues().get(0).getSeverity()).isEqualTo(Severity.ERROR); + assertThat(result.getAllIssues().get(0).toDisplayString(ast.getSource())) + .contains("comprehension nesting exceeds the configured limit: 1."); + } + + @Test + public void comprehensionNestingLimit_accumulatesErrors( + @TestParameter({ + "[1, 2, 3].map(x, [1, 2, 3].map(y, [1, 2, 3].map(z, x + y + z)))", + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(2); + } + + @Test + public void comprehensionNestingLimit_limitConfigurable( + @TestParameter({ + "[1, 2, 3].map(x, [1, 2, 3].map(y, x + y))", + "[1, 2, 3].map(x, [1, 2, 3].map(y, x + y).size() > 0, x)", + "[1, 2, 3].all(x, [1, 2, 3].exists(y, x + y > 0))", + "[1, 2, 3].map(x, {x: [1, 2, 3].map(y, x + y)})", + "[1, 2, 3].exists(i, v, i < 3 && [1, 2, 3].all(j, v2, j < 3 && v2 > 0))", + "{1: 2}.all(k, {2: 3}.all(k2, k != k2))" + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + CelValidator celValidator = + CelValidatorFactory.standardCelValidatorBuilder(CEL) + .addAstValidators(ComprehensionNestingLimitValidator.newInstance(2)) + .build(); + + CelValidationResult result = celValidator.validate(ast); + + assertThat(result.hasError()).isFalse(); + } + + @Test + public void comprehensionNestingLimit_trivialLoopsDontCount( + @TestParameter({ + "cel.bind(x, [1, 2].map(x, x + 1), x + [1, 2].map(x, x + 1))", + "optional.of(1).optMap(x, [1, 2, 3].exists(y, y == x))", + "[].map(x, [1, 2, 3].map(y, x + y))", + "{}.map(k1, {1: 2, 3: 4}.map(k2, k1 + k2))", + "[1, 2, 3].map(x, cel.bind(y, 2, x + y))", + "[1, 2, 3].map(x, optional.of(1).optMap(y, x + y).orValue(0))", + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isFalse(); + } + + @Test + public void comprehensionNestingLimit_zeroLimitAcceptedComprehenions( + @TestParameter({ + "cel.bind(x, 1, x + 1)", + "optional.of(1).optMap(x, x + 1)", + "[].map(x, int(x))", + "cel.bind(x, 1 + [].map(x, int(x)).size(), x + 1)" + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + + CelValidator celValidator = + CelValidatorFactory.standardCelValidatorBuilder(CEL) + .addAstValidators(ComprehensionNestingLimitValidator.newInstance(0)) + .build(); + + CelValidationResult result = celValidator.validate(ast); + + assertThat(result.hasError()).isFalse(); + } + + @Test + public void comprehensionNestingLimit_zeroLimitRejectedComprehensions( + @TestParameter({ + "[1].map(x, x)", + "[1].exists(x, x > 0)", + "[].exists(x, [1].all(y, y > 0))", + }) + String expr) + throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(expr).getAst(); + + CelValidator celValidator = + CelValidatorFactory.standardCelValidatorBuilder(CEL) + .addAstValidators(ComprehensionNestingLimitValidator.newInstance(0)) + .build(); + + CelValidationException e = + assertThrows(CelValidationException.class, () -> celValidator.validate(ast).getAst()); + + assertThat(e.getMessage()).contains("comprehension nesting exceeds the configured limit: 0."); + } +} diff --git a/validator/validators/BUILD.bazel b/validator/validators/BUILD.bazel index b5498d682..ab0919f20 100644 --- a/validator/validators/BUILD.bazel +++ b/validator/validators/BUILD.bazel @@ -29,3 +29,8 @@ java_library( name = "ast_depth_limit_validator", exports = ["//validator/src/main/java/dev/cel/validator/validators:ast_depth_limit_validator"], ) + +java_library( + name = "comprehension_nesting_limit_validator", + exports = ["//validator/src/main/java/dev/cel/validator/validators:comprehension_nesting_limit_validator"], +)