jerryshao commented on code in PR #13478: URL: https://github.com/apache/gravitino/pull/13478#discussion_r4236271585
########## core/src/main/java/org/apache/gravitino/policy/expression/RestrictedRegoExpressionParserFacade.java: ########## @@ -0,0 +1,756 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.gravitino.policy.expression; + +import com.google.common.base.Preconditions; +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.policy.ReadRestrictionContent; +import org.apache.gravitino.policy.expression.CanonicalExpression.Comparison; +import org.apache.gravitino.policy.expression.CanonicalExpression.ComparisonOperator; +import org.apache.gravitino.policy.expression.CanonicalExpression.GroupMembership; +import org.apache.gravitino.policy.expression.CanonicalExpression.Literal; +import org.apache.gravitino.policy.expression.CanonicalExpression.LiteralArray; +import org.apache.gravitino.policy.expression.CanonicalExpression.LiteralType; +import org.apache.gravitino.policy.expression.CanonicalExpression.LogicalExpression; +import org.apache.gravitino.policy.expression.CanonicalExpression.LogicalOperator; +import org.apache.gravitino.policy.expression.CanonicalExpression.Not; +import org.apache.gravitino.policy.expression.CanonicalExpression.SessionUser; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.ColumnMask; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.FilterBranch; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.MaskAction; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.MaskBranch; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.RowFilter; + +/** Parses and validates complete programs in the {@code restricted-rego-v1} source dialect. */ +public final class RestrictedRegoExpressionParserFacade { + + private static final int MAX_SOURCE_DEPTH = 8; + private static final int MAX_AST_NODES = 256; + private static final int MAX_STRING_BYTES = 4 * 1024; + private static final int MAX_ARRAY_ELEMENTS = 256; + + private RestrictedRegoExpressionParserFacade() {} + + /** + * Parses and validates one complete row-filter or column-mask program. + * + * @param source program in {@code restricted-rego-v1} + * @return validated unresolved program + * @throws IllegalArgumentException if syntax or semantics are invalid + */ + public static RestrictedRegoProgram parse(String source) { + Preconditions.checkArgument( + StringUtils.isNotBlank(source), "restricted-rego-v1 program cannot be blank"); + Preconditions.checkArgument( + source.getBytes(StandardCharsets.UTF_8).length + <= ReadRestrictionContent.MAX_SOURCE_LENGTH_BYTES, + "restricted-rego-v1 source must not exceed %s UTF-8 bytes", + ReadRestrictionContent.MAX_SOURCE_LENGTH_BYTES); + + RestrictedRegoProgram program = new SourceParser(source).parseProgram(); + validateProgram(program); + return program; + } + + /** + * Parses and validates one complete {@code filter := ...} program. + * + * @param source row-filter program in {@code restricted-rego-v1} + * @return validated unresolved row-filter rule + * @throws IllegalArgumentException if syntax or semantics are invalid or the rule head is not + * {@code filter} + */ + public static RowFilter parseRowFilter(String source) { + RestrictedRegoProgram program = parse(source); + Preconditions.checkArgument( + program instanceof RowFilter, "row-filter program must declare a filter rule"); + return (RowFilter) program; + } + + /** + * Parses and validates one complete {@code mask := action(...)} program. + * + * @param source column-mask program in {@code restricted-rego-v1} + * @return validated unresolved column-mask rule + * @throws IllegalArgumentException if syntax or semantics are invalid or the rule head is not + * {@code mask} + */ + public static ColumnMask parseColumnMask(String source) { + RestrictedRegoProgram program = parse(source); + Preconditions.checkArgument( + program instanceof ColumnMask, "column-mask program must declare a mask rule"); + return (ColumnMask) program; + } + + private static void validateProgram(RestrictedRegoProgram program) { + program.validate(); + List<CanonicalExpression> expressions = expressions(program); + int nodeCount = 0; + for (CanonicalExpression expression : expressions) { + Preconditions.checkArgument( + expression.depth() <= MAX_SOURCE_DEPTH, + "restricted-rego-v1 source depth must not exceed %s", + MAX_SOURCE_DEPTH); + nodeCount += countNodes(expression); + } + Preconditions.checkArgument( + nodeCount <= MAX_AST_NODES, + "restricted-rego-v1 AST must not exceed %s nodes", + MAX_AST_NODES); + if (program instanceof RowFilter) { + CanonicalExpression lowered = ((RowFilter) program).lower(); + Preconditions.checkArgument( + lowered.depth() <= MAX_SOURCE_DEPTH, + "lowered row-filter depth must not exceed %s", + MAX_SOURCE_DEPTH); + Preconditions.checkArgument( + countNodes(lowered) <= MAX_AST_NODES, + "lowered row-filter AST must not exceed %s nodes", + MAX_AST_NODES); + } + } + + private static List<CanonicalExpression> expressions(RestrictedRegoProgram program) { + List<CanonicalExpression> expressions = new ArrayList<>(); + if (program instanceof RowFilter) { + RowFilter filter = (RowFilter) program; + for (FilterBranch branch : filter.branches()) { + expressions.add(branch.result()); + expressions.add(branch.condition()); + } + expressions.add(filter.fallback()); + } else { + ColumnMask mask = (ColumnMask) program; + for (MaskBranch branch : mask.branches()) { + expressions.add(branch.condition()); + } + } + return expressions; + } + + private static int countNodes(CanonicalExpression expression) { + if (expression instanceof Comparison) { + Comparison comparison = (Comparison) expression; + return 1 + countNodes(comparison.left()) + countNodes(comparison.right()); + } + if (expression instanceof Not) { + return 1 + countNodes(((Not) expression).operand()); + } + if (expression instanceof LogicalExpression) { + int count = 1; + for (CanonicalExpression child : ((LogicalExpression) expression).operands()) { + count += countNodes(child); + } + return count; + } + return 1; Review Comment: [Nit] The node budget does not bound the literal payload: an array counts as one node. `countNodes` falls through to `return 1` for a `LiteralArray` whatever its length, and for a `Literal` of any precision, so the only effective bound on the values that reach a resolved predicate is the 16 KiB source limit, not `MAX_AST_NODES`. Two measured cases: a disjunction of 12 `col("cN") in [0..255]` comparisons is 37 counted nodes but carries 3072 literals (11192-byte source, accepted); and `col("x") == 1.<15000 fives>` is one counted node holding a 15000-digit `BigDecimal` (accepted). Since this round added the lowered-node check specifically so that lowering cannot produce an oversized wire predicate, the same reasoning suggests counting array elements in `countNodes` (or capping total `in` values) and saying which it is in the Limits section of the design doc, which today lists only `array elements: 256` per array. Verified by: ran both sources through the standalone parser; both were accepted. The 37 is `countNodes`'s own arithmetic on the returned tree: 1 for the `or` node plus 12 x (comparison + column + array) = 37, because each 256-element array is a single node. ########## docs/policies.md: ########## @@ -47,6 +47,51 @@ The row-filter and column-mask policy types can be created and associated with t does not enforce them yet. Until enforcement is available, these policy types do not restrict data access. +### Read-Restriction Expressions + +Row-filter and column-mask expressions use `restricted-rego-v1`, a small, fail-closed Rego-like +syntax owned by Gravitino. Each expression is one complete rule, not an arbitrary Rego module. A +row filter starts with `filter :=` and produces a Boolean predicate. A column mask starts with +`mask := action("...")` and produces an allowlisted masking action. + +For example: + +```text +filter := col("region") == "US" if is_group_member("auditors") +else := col("owner") == session_user() + +mask := action("show-last-4") if is_group_member("support") +else := action("replace-with-null") +``` + +Conditional branches are evaluated from left to right. The first true condition selects its result, +and every conditional rule must end with an unconditional `else`. Row-filter conditions and results +must be Boolean. Column-mask conditions may use only request context; they cannot reference columns. + +The syntax supports these constructs: + +- `col("name")`, `session_user()`, and `is_group_member("group")`; +- JSON string literals, exact decimal numbers, `true`, `false`, `null`, and homogeneous non-null + literal arrays; +- comparisons `==`, `!=`, `<`, `<=`, `>`, `>=`, and `in`; and Review Comment: [Nit] The new docs section lists the operators but not the operand shapes, so a reader can still write a filter that is rejected. The design doc has the operand table (design-docs/tag-based-read-restrictions-for-iceberg-rest.md:342-354), but here only the operators appear, and the rejection paragraph at line 91 covers syntax rather than operand shapes. All of these are rejected at save time with nothing in this page to predict it: `col("a") == col("b")` and `1 == 1` (`Unsupported operands`), `col("a") > true` (`boolean literal supports only == and !=`), `session_user() < "bob"` (`session_user() supports only == and !=`), and `col("x") in []`. One sentence per shape would close it: column vs literal for all six comparisons, column vs `session_user()` and `session_user()` vs string literal for `==`/`!=` only, column vs `null` for `==`/`!=` only, and `in` with the column on the left and a non-empty homogeneous array. Separately, the single fenced block at lines 59-65 holds two complete programs. An `expression` field carries exactly one rule, so pasting that block as-is is rejected; two blocks, one per rule, would avoid the trap. Verified by: ran the five expressions and the concatenated doc block through the standalone parser - each expression rejected with the message quoted above, and the concatenated block rejected with `Invalid restricted-rego-v1 program at line 4, column 1: expected end of program`. ########## core/src/main/java/org/apache/gravitino/policy/expression/CanonicalExpression.java: ########## @@ -0,0 +1,686 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.gravitino.policy.expression; + +import com.google.common.base.Preconditions; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * A node in an unresolved {@code restricted-rego-v1} expression tree. + * + * <p>The tree contains only the source constructs allowlisted by the read-restriction contract. + * Column and context references remain symbolic until the effective-policy resolver binds them to a + * table schema and request context. + */ +public interface CanonicalExpression { + + /** + * Validates this expression node and all of its children. + * + * @throws IllegalArgumentException if the expression is malformed or outside the allowlisted + * source profile + */ + void validate() throws IllegalArgumentException; + + /** + * Returns the operation depth defined by {@code restricted-rego-v1}. + * + * <p>Value leaves such as a column, string, or number have depth 0. Atomic predicates such as a + * comparison, Boolean literal, or group-membership check have depth 1. Negation and a logical + * expression add one to their deepest operand. A consecutive n-ary logical chain counts as one + * operation, so {@code true and false and true} has depth 2. + * + * @return expression operation depth + */ + int depth(); + + /** Comparison operators supported by {@code restricted-rego-v1}. */ + enum ComparisonOperator { + /** Equality comparison. */ + EQ("=="), + /** Inequality comparison. */ + NEQ("!="), + /** Less-than comparison. */ + LT("<"), + /** Less-than-or-equal comparison. */ + LTE("<="), + /** Greater-than comparison. */ + GT(">"), + /** Greater-than-or-equal comparison. */ + GTE(">="), + /** Literal-array membership comparison. */ + IN("in"); + + private final String sourceToken; + + ComparisonOperator(String sourceToken) { + this.sourceToken = sourceToken; + } + + /** + * Parses an operator token from restricted Rego source. + * + * @param token source operator token + * @return parsed operator + */ + public static ComparisonOperator fromSourceToken(String token) { + Preconditions.checkArgument(token != null && !token.isEmpty(), "operator cannot be empty"); + switch (token) { + case "==": + return EQ; + case "!=": + return NEQ; + case "<": + return LT; + case "<=": + return LTE; + case ">": + return GT; + case ">=": + return GTE; + case "in": + return IN; + default: + throw new IllegalArgumentException("Unsupported restricted-rego-v1 operator: " + token); + } + } + + /** + * Returns the token used in restricted Rego source. + * + * @return source token + */ + public String sourceToken() { + return sourceToken; + } + } + + /** N-ary logical operators supported by {@code restricted-rego-v1}. */ + enum LogicalOperator { + /** Boolean conjunction. */ + AND("and"), + /** Boolean disjunction. */ + OR("or"); + + private final String sourceToken; + + LogicalOperator(String sourceToken) { + this.sourceToken = sourceToken; + } + + /** + * Returns the token used in restricted Rego source. + * + * @return source token + */ + public String sourceToken() { + return sourceToken; + } + } + + /** Source literal types supported by {@code restricted-rego-v1}. */ + enum LiteralType { + /** String literal. */ + STRING, + /** Exact base-10 numeric literal. */ + NUMBER, + /** Boolean literal. */ + BOOLEAN, + /** Null literal. */ + NULL + } + + /** A binary comparison operation. */ + final class Comparison implements CanonicalExpression { + private final ComparisonOperator operator; + private final CanonicalExpression left; + private final CanonicalExpression right; + + Comparison(ComparisonOperator operator, CanonicalExpression left, CanonicalExpression right) { + this.operator = operator; + this.left = left; + this.right = right; + } + + /** + * Returns the comparison operator. + * + * @return comparison operator + */ + public ComparisonOperator operator() { + return operator; + } + + /** + * Returns the left operand. + * + * @return left operand + */ + public CanonicalExpression left() { + return left; + } + + /** + * Returns the right operand. + * + * @return right operand + */ + public CanonicalExpression right() { + return right; + } + + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument(operator != null, "comparison operator cannot be null"); + Preconditions.checkArgument( + left != null && right != null, "comparison operands cannot be null"); + left.validate(); + right.validate(); + try { + ExpressionValidation.validateComparison(operator, left, right); + } catch (IllegalArgumentException exception) { + throw new IllegalArgumentException( + exception.getMessage() + "; offending comparison: " + this, exception); + } + } + + @Override + public int depth() { + return 1; + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof Comparison)) { + return false; + } + Comparison that = (Comparison) other; + return operator == that.operator + && Objects.equals(left, that.left) + && Objects.equals(right, that.right); + } + + @Override + public int hashCode() { + return Objects.hash(operator, left, right); + } + + @Override + public String toString() { + return "Comparison{" + "operator=" + operator + ", left=" + left + ", right=" + right + '}'; + } + } + + /** A Boolean negation operation. */ + final class Not implements CanonicalExpression { + private final CanonicalExpression operand; + + Not(CanonicalExpression operand) { + this.operand = operand; + } + + /** + * Returns the negated predicate. + * + * @return negated predicate + */ + public CanonicalExpression operand() { + return operand; + } + + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument(operand != null, "not requires an operand"); + operand.validate(); + Preconditions.checkArgument( + ExpressionValidation.isPredicate(operand), "not operand must be a boolean predicate"); + } + + @Override + public int depth() { + return 1 + (operand == null ? 0 : operand.depth()); + } + + @Override + public boolean equals(Object other) { + return other instanceof Not && Objects.equals(operand, ((Not) other).operand); + } + + @Override + public int hashCode() { + return Objects.hash(operand); + } + + @Override + public String toString() { + return "Not{" + "operand=" + operand + '}'; + } + } + + /** An n-ary Boolean conjunction or disjunction. */ + final class LogicalExpression implements CanonicalExpression { + private final LogicalOperator operator; + private final List<CanonicalExpression> operands; + + LogicalExpression(LogicalOperator operator, List<CanonicalExpression> operands) { + this.operator = operator; + this.operands = + operands == null ? null : Collections.unmodifiableList(new ArrayList<>(operands)); + } + + static CanonicalExpression of(LogicalOperator operator, List<CanonicalExpression> operands) { + if (operands == null) { + return new LogicalExpression(operator, null); + } + + List<CanonicalExpression> flattened = new ArrayList<>(); + for (CanonicalExpression operand : operands) { + if (operand instanceof LogicalExpression + && ((LogicalExpression) operand).operator() == operator) { + flattened.addAll(((LogicalExpression) operand).operands()); + } else { + flattened.add(operand); + } + } + return flattened.size() == 1 ? flattened.get(0) : new LogicalExpression(operator, flattened); + } + + /** + * Returns the logical operator. + * + * @return logical operator + */ + public LogicalOperator operator() { + return operator; + } + + /** + * Returns the logical operands. + * + * @return immutable operands + */ + public List<CanonicalExpression> operands() { + return operands; + } + + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument(operator != null, "logical expression operator cannot be null"); + Preconditions.checkArgument( + operands != null && operands.size() >= 2, + "%s requires at least two operands", + operator.sourceToken()); + for (CanonicalExpression child : operands) { + Preconditions.checkArgument( + child != null, "%s operand cannot be null", operator.sourceToken()); + child.validate(); + Preconditions.checkArgument( + ExpressionValidation.isPredicate(child), + "%s operands must be boolean predicates", + operator.sourceToken()); + } + } + + @Override + public int depth() { + int childDepth = 0; + if (operands != null) { + for (CanonicalExpression child : operands) { + if (child != null) { + childDepth = Math.max(childDepth, child.depth()); + } + } + } + return 1 + childDepth; + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof LogicalExpression)) { + return false; + } + LogicalExpression that = (LogicalExpression) other; + return operator == that.operator && Objects.equals(operands, that.operands); + } + + @Override + public int hashCode() { + return Objects.hash(operator, operands); + } + + @Override + public String toString() { + return "LogicalExpression{" + "operator=" + operator + ", operands=" + operands + '}'; + } + } + + /** A symbolic top-level column reference. */ + final class Column implements CanonicalExpression { + private final String name; + + Column(String name) { + this.name = name; + } + + /** + * Returns the exact decoded column name. + * + * @return column name + */ + public String name() { + return name; + } + + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument(name != null && !name.isEmpty(), "column name cannot be empty"); + Preconditions.checkArgument(name.indexOf('\0') < 0, "column name cannot contain NUL"); Review Comment: [Nit] NUL is rejected in column names but allowed in group names and string values. This line rejects NUL in a column name, but `GroupMembership.validate()` (line 475) and the STRING case of `Literal.validate()` (line 556) only call `validateUnicodeScalars`, which checks for isolated surrogates and says nothing about NUL (ExpressionValidation.java:116-129). So `filter := is_group_member("a\u0000b")` and `filter := col("x") == "a\u0000b"` both parse and are stored, while the `col()` form is rejected. If NUL is unsafe enough to reject in an identifier that will be bound to a schema field, it is hard to see why a group name resolved against metalake membership, or a literal that ends up in a wire predicate, should keep it. Moving the check into `validateUnicodeScalars` would cover all three with one message and remove the special case here. Verified by: ran the standalone parser on all three inputs - the two above parsed (returning `GroupMembership` and `Comparison`), the `col()` form failed with `column name cannot contain NUL`. ########## core/src/main/java/org/apache/gravitino/policy/expression/RestrictedRegoExpressionParserFacade.java: ########## @@ -0,0 +1,756 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.gravitino.policy.expression; + +import com.google.common.base.Preconditions; +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.policy.ReadRestrictionContent; +import org.apache.gravitino.policy.expression.CanonicalExpression.Comparison; +import org.apache.gravitino.policy.expression.CanonicalExpression.ComparisonOperator; +import org.apache.gravitino.policy.expression.CanonicalExpression.GroupMembership; +import org.apache.gravitino.policy.expression.CanonicalExpression.Literal; +import org.apache.gravitino.policy.expression.CanonicalExpression.LiteralArray; +import org.apache.gravitino.policy.expression.CanonicalExpression.LiteralType; +import org.apache.gravitino.policy.expression.CanonicalExpression.LogicalExpression; +import org.apache.gravitino.policy.expression.CanonicalExpression.LogicalOperator; +import org.apache.gravitino.policy.expression.CanonicalExpression.Not; +import org.apache.gravitino.policy.expression.CanonicalExpression.SessionUser; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.ColumnMask; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.FilterBranch; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.MaskAction; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.MaskBranch; +import org.apache.gravitino.policy.expression.RestrictedRegoProgram.RowFilter; + +/** Parses and validates complete programs in the {@code restricted-rego-v1} source dialect. */ +public final class RestrictedRegoExpressionParserFacade { + + private static final int MAX_SOURCE_DEPTH = 8; + private static final int MAX_AST_NODES = 256; + private static final int MAX_STRING_BYTES = 4 * 1024; + private static final int MAX_ARRAY_ELEMENTS = 256; + + private RestrictedRegoExpressionParserFacade() {} + + /** + * Parses and validates one complete row-filter or column-mask program. + * + * @param source program in {@code restricted-rego-v1} + * @return validated unresolved program + * @throws IllegalArgumentException if syntax or semantics are invalid + */ + public static RestrictedRegoProgram parse(String source) { + Preconditions.checkArgument( + StringUtils.isNotBlank(source), "restricted-rego-v1 program cannot be blank"); + Preconditions.checkArgument( + source.getBytes(StandardCharsets.UTF_8).length + <= ReadRestrictionContent.MAX_SOURCE_LENGTH_BYTES, + "restricted-rego-v1 source must not exceed %s UTF-8 bytes", + ReadRestrictionContent.MAX_SOURCE_LENGTH_BYTES); + + RestrictedRegoProgram program = new SourceParser(source).parseProgram(); + validateProgram(program); + return program; + } + + /** + * Parses and validates one complete {@code filter := ...} program. + * + * @param source row-filter program in {@code restricted-rego-v1} + * @return validated unresolved row-filter rule + * @throws IllegalArgumentException if syntax or semantics are invalid or the rule head is not + * {@code filter} + */ + public static RowFilter parseRowFilter(String source) { + RestrictedRegoProgram program = parse(source); + Preconditions.checkArgument( + program instanceof RowFilter, "row-filter program must declare a filter rule"); + return (RowFilter) program; + } + + /** + * Parses and validates one complete {@code mask := action(...)} program. + * + * @param source column-mask program in {@code restricted-rego-v1} + * @return validated unresolved column-mask rule + * @throws IllegalArgumentException if syntax or semantics are invalid or the rule head is not + * {@code mask} + */ + public static ColumnMask parseColumnMask(String source) { + RestrictedRegoProgram program = parse(source); + Preconditions.checkArgument( + program instanceof ColumnMask, "column-mask program must declare a mask rule"); + return (ColumnMask) program; + } + + private static void validateProgram(RestrictedRegoProgram program) { + program.validate(); + List<CanonicalExpression> expressions = expressions(program); + int nodeCount = 0; + for (CanonicalExpression expression : expressions) { + Preconditions.checkArgument( + expression.depth() <= MAX_SOURCE_DEPTH, + "restricted-rego-v1 source depth must not exceed %s", + MAX_SOURCE_DEPTH); + nodeCount += countNodes(expression); + } + Preconditions.checkArgument( + nodeCount <= MAX_AST_NODES, + "restricted-rego-v1 AST must not exceed %s nodes", + MAX_AST_NODES); + if (program instanceof RowFilter) { + CanonicalExpression lowered = ((RowFilter) program).lower(); + Preconditions.checkArgument( + lowered.depth() <= MAX_SOURCE_DEPTH, + "lowered row-filter depth must not exceed %s", + MAX_SOURCE_DEPTH); + Preconditions.checkArgument( + countNodes(lowered) <= MAX_AST_NODES, + "lowered row-filter AST must not exceed %s nodes", + MAX_AST_NODES); + } + } + + private static List<CanonicalExpression> expressions(RestrictedRegoProgram program) { + List<CanonicalExpression> expressions = new ArrayList<>(); + if (program instanceof RowFilter) { + RowFilter filter = (RowFilter) program; + for (FilterBranch branch : filter.branches()) { + expressions.add(branch.result()); + expressions.add(branch.condition()); + } + expressions.add(filter.fallback()); + } else { + ColumnMask mask = (ColumnMask) program; + for (MaskBranch branch : mask.branches()) { + expressions.add(branch.condition()); + } + } + return expressions; + } + + private static int countNodes(CanonicalExpression expression) { + if (expression instanceof Comparison) { + Comparison comparison = (Comparison) expression; + return 1 + countNodes(comparison.left()) + countNodes(comparison.right()); + } + if (expression instanceof Not) { + return 1 + countNodes(((Not) expression).operand()); + } + if (expression instanceof LogicalExpression) { + int count = 1; + for (CanonicalExpression child : ((LogicalExpression) expression).operands()) { + count += countNodes(child); + } + return count; + } + return 1; + } + + private static final class SourceParser { + private static final int MAX_PARENTHESIS_NESTING = 256; + + private final Lexer lexer; + private Token current; + private int parenthesisNesting; + + private SourceParser(String source) { + this.lexer = new Lexer(source); + this.current = lexer.nextToken(); + } + + private RestrictedRegoProgram parseProgram() { + RestrictedRegoProgram program; + if (check(TokenType.FILTER)) { + program = parseRowFilter(); + } else if (check(TokenType.MASK)) { + program = parseColumnMask(); + } else { + throw error("expected filter or mask rule"); + } + expect(TokenType.EOF, "expected end of program"); + return program; + } + + private RowFilter parseRowFilter() { + expect(TokenType.FILTER, "expected filter"); + expect(TokenType.ASSIGN, "expected := after filter"); + CanonicalExpression result = parseExpression(); + if (!match(TokenType.IF)) { + return new RowFilter(new ArrayList<>(), result); + } + + List<FilterBranch> branches = new ArrayList<>(); + branches.add(new FilterBranch(result, parseExpression())); + while (true) { + expect(TokenType.ELSE, "expected final else branch"); + expect(TokenType.ASSIGN, "expected := after else"); + CanonicalExpression nextResult = parseExpression(); + if (!match(TokenType.IF)) { + return new RowFilter(branches, nextResult); + } + branches.add(new FilterBranch(nextResult, parseExpression())); + } + } + + private ColumnMask parseColumnMask() { + expect(TokenType.MASK, "expected mask"); + expect(TokenType.ASSIGN, "expected := after mask"); + MaskAction action = parseMaskAction(); + if (!match(TokenType.IF)) { + return new ColumnMask(new ArrayList<>(), action); + } + + List<MaskBranch> branches = new ArrayList<>(); + branches.add(new MaskBranch(action, parseExpression())); + while (true) { + expect(TokenType.ELSE, "expected final else branch"); + expect(TokenType.ASSIGN, "expected := after else"); + MaskAction nextAction = parseMaskAction(); + if (!match(TokenType.IF)) { + return new ColumnMask(branches, nextAction); + } + branches.add(new MaskBranch(nextAction, parseExpression())); + } + } + + private MaskAction parseMaskAction() { + expect(TokenType.ACTION, "expected action"); + expect(TokenType.LPAREN, "expected ( after action"); + Token actionToken = expect(TokenType.STRING, "expected mask action string"); + String action = decodeString(actionToken); + expect(TokenType.RPAREN, "expected ) after mask action"); + try { + return MaskAction.fromValue(action); + } catch (IllegalArgumentException exception) { + throw error(actionToken, exception.getMessage()); + } + } + + private CanonicalExpression parseExpression() { + return parseOrExpression(); + } + + private CanonicalExpression parseOrExpression() { + List<CanonicalExpression> operands = new ArrayList<>(); + operands.add(parseAndExpression()); + while (match(TokenType.OR)) { + operands.add(parseAndExpression()); + } + return LogicalExpression.of(LogicalOperator.OR, operands); + } + + private CanonicalExpression parseAndExpression() { + List<CanonicalExpression> operands = new ArrayList<>(); + operands.add(parseNotExpression()); + while (match(TokenType.AND)) { + operands.add(parseNotExpression()); + } + return LogicalExpression.of(LogicalOperator.AND, operands); + } + + private CanonicalExpression parseNotExpression() { + int notCount = 0; + while (match(TokenType.NOT)) { + notCount++; + } + + CanonicalExpression result = parseComparisonExpression(); + for (int index = 0; index < notCount; index++) { + result = new Not(result); + } + return result; + } + + private CanonicalExpression parseComparisonExpression() { + CanonicalExpression left = parsePrimary(); + if (!isComparisonOperator(current.type)) { + return left; + } + + ComparisonOperator operator = ComparisonOperator.fromSourceToken(advance().text); + CanonicalExpression right = parsePrimary(); + if (isComparisonOperator(current.type)) { + throw error("chained comparisons are not supported"); + } + return new Comparison(operator, left, right); + } + + private CanonicalExpression parsePrimary() { + if (match(TokenType.COL)) { + expect(TokenType.LPAREN, "expected ( after col"); + String column = decodeString(expect(TokenType.STRING, "expected column name string")); + expect(TokenType.RPAREN, "expected ) after column name"); + return new CanonicalExpression.Column(column); + } + if (match(TokenType.SESSION_USER)) { + expect(TokenType.LPAREN, "expected ( after session_user"); + expect(TokenType.RPAREN, "expected ) after session_user("); + return new SessionUser(); + } + if (match(TokenType.IS_GROUP_MEMBER)) { + expect(TokenType.LPAREN, "expected ( after is_group_member"); + String group = decodeString(expect(TokenType.STRING, "expected group name string")); + expect(TokenType.RPAREN, "expected ) after group name"); + return new GroupMembership(group); + } + if (isLiteral(current.type)) { + return parseLiteral(); + } + if (match(TokenType.LBRACKET)) { + return parseLiteralArray(); + } + if (match(TokenType.LPAREN)) { + parenthesisNesting++; + if (parenthesisNesting > MAX_PARENTHESIS_NESTING) { + throw error("parenthesis nesting must not exceed " + MAX_PARENTHESIS_NESTING); + } + CanonicalExpression expression = parseExpression(); + expect(TokenType.RPAREN, "expected ) after expression"); + parenthesisNesting--; + return expression; + } + throw error("expected expression"); + } + + private Literal parseLiteral() { + Token token = advance(); + switch (token.type) { + case STRING: + return new Literal(LiteralType.STRING, decodeString(token)); + case NUMBER: + BigDecimal value = new BigDecimal(token.text); + return new Literal( + LiteralType.NUMBER, value, token.text.startsWith("-") && value.signum() == 0); + case TRUE: + return new Literal(LiteralType.BOOLEAN, true); + case FALSE: + return new Literal(LiteralType.BOOLEAN, false); + case NULL: + return new Literal(LiteralType.NULL, null); + default: + throw error(token, "expected literal"); + } + } + + private LiteralArray parseLiteralArray() { + List<Literal> values = new ArrayList<>(); + LiteralType elementType = null; + do { + if (values.size() >= MAX_ARRAY_ELEMENTS) { + throw error("array literal must not exceed " + MAX_ARRAY_ELEMENTS + " elements"); + } + if (!isLiteral(current.type)) { + throw error("expected a literal array element"); + } + Literal literal = parseLiteral(); + if (elementType == null) { + elementType = literal.literalType(); + } + values.add(literal); + } while (match(TokenType.COMMA)); + expect(TokenType.RBRACKET, "expected ] after array literal"); + return new LiteralArray(elementType, values); + } + + private boolean match(TokenType type) { + if (!check(type)) { + return false; + } + advance(); + return true; + } + + private boolean check(TokenType type) { + return current.type == type; + } + + private Token expect(TokenType type, String message) { + if (!check(type)) { + throw error(message); + } + return advance(); + } + + private Token advance() { + Token previous = current; + current = lexer.nextToken(); + return previous; + } + + private IllegalArgumentException error(String message) { + return error(current, message); + } + + private static IllegalArgumentException error(Token token, String message) { + return new IllegalArgumentException( + String.format( + "Invalid restricted-rego-v1 program at line %s, column %s: %s", + token.line, token.column, message)); + } + + private static boolean isComparisonOperator(TokenType type) { + return type == TokenType.EQ + || type == TokenType.NEQ + || type == TokenType.LT + || type == TokenType.LTE + || type == TokenType.GT + || type == TokenType.GTE + || type == TokenType.IN; + } + + private static boolean isLiteral(TokenType type) { + return type == TokenType.STRING + || type == TokenType.NUMBER + || type == TokenType.TRUE + || type == TokenType.FALSE + || type == TokenType.NULL; + } + + private static String decodeString(Token token) { + StringBuilder result = new StringBuilder(); + for (int index = 1; index < token.text.length() - 1; index++) { + char current = token.text.charAt(index); + if (current != '\\') { + result.append(current); + continue; + } + + char escaped = token.text.charAt(++index); + switch (escaped) { + case '"': + case '\\': + case '/': + result.append(escaped); + break; + case 'b': + result.append('\b'); + break; + case 'f': + result.append('\f'); + break; + case 'n': + result.append('\n'); + break; + case 'r': + result.append('\r'); + break; + case 't': + result.append('\t'); + break; + case 'u': + result.append((char) Integer.parseInt(token.text.substring(index + 1, index + 5), 16)); + index += 4; + break; + default: + throw error(token, "unsupported string escape: \\" + escaped); + } + } + String decoded = result.toString(); + if (decoded.getBytes(StandardCharsets.UTF_8).length > MAX_STRING_BYTES) { + throw error( + token, "decoded string literal must not exceed " + MAX_STRING_BYTES + " UTF-8 bytes"); + } + return decoded; + } + } + + private enum TokenType { + FILTER, + MASK, + IF, + ELSE, + OR, + AND, + NOT, + IN, + COL, + SESSION_USER, + IS_GROUP_MEMBER, + ACTION, + TRUE, + FALSE, + NULL, + ASSIGN, + EQ, + NEQ, + LTE, + LT, + GTE, + GT, + LPAREN, + RPAREN, + LBRACKET, + RBRACKET, + COMMA, + NUMBER, + STRING, + EOF + } + + private static final class Token { + private final TokenType type; + private final String text; + private final int line; + private final int column; + + private Token(TokenType type, String text, int line, int column) { + this.type = type; + this.text = text; + this.line = line; + this.column = column; + } + } + + private static final class Lexer { + private final String source; + private int offset; + private int line = 1; + private int column = 1; + + private Lexer(String source) { + this.source = source; + } + + private Token nextToken() { + skipWhitespace(); + int tokenOffset = offset; + int tokenLine = line; + int tokenColumn = column; + if (atEnd()) { + return new Token(TokenType.EOF, "", tokenLine, tokenColumn); + } + + char current = peek(); + if (current == '"') { + scanString(tokenLine, tokenColumn); + return token(TokenType.STRING, tokenOffset, tokenLine, tokenColumn); + } + if (isIdentifierStart(current)) { + do { + advance(); + } while (!atEnd() && isIdentifierPart(peek())); + String text = source.substring(tokenOffset, offset); + return new Token(keywordType(text, tokenLine, tokenColumn), text, tokenLine, tokenColumn); + } + if (isDigit(current) || (current == '-' && isDigit(peekNext()))) { + scanNumber(tokenLine, tokenColumn); + if (!atEnd() && isIdentifierStart(peek())) { + throw lexicalError( + tokenLine, tokenColumn, "numeric literal must be separated from identifiers"); + } + return token(TokenType.NUMBER, tokenOffset, tokenLine, tokenColumn); + } + + advance(); + switch (current) { + case ':': + return symbol(TokenType.ASSIGN, '=', tokenOffset, tokenLine, tokenColumn); + case '=': + return symbol(TokenType.EQ, '=', tokenOffset, tokenLine, tokenColumn); + case '!': + return symbol(TokenType.NEQ, '=', tokenOffset, tokenLine, tokenColumn); + case '<': + return optionalEquals(TokenType.LT, TokenType.LTE, tokenOffset, tokenLine, tokenColumn); + case '>': + return optionalEquals(TokenType.GT, TokenType.GTE, tokenOffset, tokenLine, tokenColumn); + case '(': + return token(TokenType.LPAREN, tokenOffset, tokenLine, tokenColumn); + case ')': + return token(TokenType.RPAREN, tokenOffset, tokenLine, tokenColumn); + case '[': + return token(TokenType.LBRACKET, tokenOffset, tokenLine, tokenColumn); + case ']': + return token(TokenType.RBRACKET, tokenOffset, tokenLine, tokenColumn); + case ',': + return token(TokenType.COMMA, tokenOffset, tokenLine, tokenColumn); + default: + throw lexicalError(tokenLine, tokenColumn, "unsupported character " + current); + } + } + + private void scanString(int tokenLine, int tokenColumn) { + advance(); + while (!atEnd()) { + char current = advance(); + if (current == '"') { + return; + } + if (current < 0x20) { + throw lexicalError(tokenLine, tokenColumn, "unescaped control character in string"); + } + if (current != '\\') { + continue; + } + if (atEnd()) { + break; + } + char escaped = advance(); + if (escaped == 'u') { + for (int index = 0; index < 4; index++) { + if (atEnd() || Character.digit(advance(), 16) < 0) { Review Comment: [Important] `\u` escapes accept any Unicode decimal digit, not JSON hex digits. `Character.digit(c, 16)` here, and `Integer.parseInt(text.substring(...), 16)` in `decodeString` (line 461), are both Unicode-aware, so characters from any decimal-digit script pass as hex digits. A `\u` escape whose four digits are U+0660 U+0660 U+0664 U+0661 (Arabic-Indic `0041`) is accepted and decodes to the column named `A` (U+0041); the fullwidth (U+FF10 ...) and Devanagari (U+0966 ...) forms behave the same, and a mixed `\u00` + two Arabic-Indic digits works too. RFC 8259 and the design doc (`Strings use JSON double-quoted syntax`, design-docs/tag-based-read-restrictions-for-iceberg-rest.md:275) allow only `[0-9A-Fa-f]`, and the point of this parser is to reject everything outside the subset, so this is source the documented grammar does not define. It also means two different source texts canonicalize to the same name, which matters for the signature and dedup steps planned in the design doc. Suggest validating the digits explicitly in `scanString`, e.g. accept only `(c >= '0' && c <= '9') || ((c | 0x20) >= 'a' && (c | 0x20) <= 'f')`, and keeping `decodeString` on the same check so the two layers cannot diverge. A test alongside `testRejectsInvalidUnicodeScalarsAndColumnNul` would pin it. Verified by: compiled the four new expression files standalone (stubbing `Preconditions`, `StringUtils`, `ReadRestrictionContent`) and ran the parser; the Arabic-Indic, fullwidth, Devanagari and mixed forms each returned `Column{name=U+0041}`, while non-digit junk in a `\u` escape is still rejected with `invalid Unicode escape in string`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
