roryqi commented on code in PR #13478:
URL: https://github.com/apache/gravitino/pull/13478#discussion_r4245718245


##########
core/src/main/java/org/apache/gravitino/policy/rego/RestrictedRegoExpressionParserFacade.java:
##########
@@ -0,0 +1,819 @@
+/*
+ * 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.rego;
+
+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.rego.CanonicalExpression.Comparison;
+import org.apache.gravitino.policy.rego.CanonicalExpression.ComparisonOperator;
+import org.apache.gravitino.policy.rego.CanonicalExpression.GroupMembership;
+import org.apache.gravitino.policy.rego.CanonicalExpression.Literal;
+import org.apache.gravitino.policy.rego.CanonicalExpression.LiteralArray;
+import org.apache.gravitino.policy.rego.CanonicalExpression.LiteralType;
+import org.apache.gravitino.policy.rego.CanonicalExpression.LogicalExpression;
+import org.apache.gravitino.policy.rego.CanonicalExpression.LogicalOperator;
+import org.apache.gravitino.policy.rego.CanonicalExpression.Not;
+import org.apache.gravitino.policy.rego.CanonicalExpression.SessionUser;
+import org.apache.gravitino.policy.rego.RestrictedRegoProgram.ColumnMask;
+import org.apache.gravitino.policy.rego.RestrictedRegoProgram.FilterBranch;
+import org.apache.gravitino.policy.rego.RestrictedRegoProgram.MaskAction;
+import org.apache.gravitino.policy.rego.RestrictedRegoProgram.MaskBranch;
+import org.apache.gravitino.policy.rego.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 static final int MAX_TOTAL_ARRAY_ELEMENTS = 256;

Review Comment:
   Addressed in 3ad6e25461 by changing the model rather than documenting 
branch-dependent 128/86 allowances. The 256-element total is now a source limit 
only; generated predicates do not re-apply source depth, node, or array 
budgets. Lowering was rewritten from quadratic flat DNF to a linear nested 
form, so every condition appears at most twice and 256 source array elements 
produce at most 512 generated occurrences regardless of branch position. The 
design/user docs and boundary tests now pin this behavior.



##########
core/src/main/java/org/apache/gravitino/policy/rego/CanonicalExpression.java:
##########
@@ -0,0 +1,685 @@
+/*
+ * 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.rego;
+
+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 {
+        ExpressionValidator.validateComparison(operator, left, right);
+      } catch (IllegalArgumentException exception) {
+        throw new IllegalArgumentException(
+            exception.getMessage() + "; offending comparison: " + this, 
exception);

Review Comment:
   Fixed in 3ad6e25461. Semantic comparison errors now report only the operator 
and operand kinds, never the expanded operand values. Added a regression using 
a 256-element invalid comparison that asserts the message stays below 512 
characters and does not contain literal payload values.



##########
docs/policies.md:
##########
@@ -47,6 +47,76 @@ 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, a row-filter program can select the first matching predicate:
+
+```restricted-rego-v1
+filter := col("region") == "US" if is_group_member("auditors")
+else := col("owner") == session_user()
+```
+
+This column-mask program selects a masking action in the same way:
+
+```restricted-rego-v1
+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
+- Boolean operators `and`, `or`, and `not`.
+
+Keywords and built-in function names are case-sensitive. For example, only 
lowercase `in` is an
+operator; `In`, `IN`, and `iN` are invalid.
+
+Supported operand shapes are:
+
+| Form | Operators | Requirements |
+| --- | --- | --- |
+| Boolean predicates | `and`, `or`, `not` | Every operand must be Boolean. |
+| Column and non-null literal | `==`, `!=`, `<`, `<=`, `>`, `>=` | Either 
operand order; types must be compatible. Boolean literals support only `==` and 
`!=`. |
+| String column and `session_user()` | `==`, `!=` | Either operand order. |
+| `session_user()` and string literal | `==`, `!=` | Either operand order. |
+| Column and `null` | `==`, `!=` | Either operand order. |
+| Column and literal array | `in` | Column on the left; array must be 
non-empty and homogeneous. |
+| `session_user()` and string array | `in` | `session_user()` on the left; 
array must be non-empty. |
+| `is_group_member("group")` | none | Produces a Boolean request-context 
predicate. |
+
+Column-to-column and literal-to-literal comparisons, ordering on 
`session_user()`, null array
+elements, nested arrays, and comparisons on `is_group_member(...)` are invalid.
+
+Column-mask actions are `mask-alphanum`, `mask-to-fixed-value`, 
`replace-with-null`, `show-first-4`,
+`show-last-4`, `truncate-to-year`, `truncate-to-month`, `sha-256-global`, and
+`sha-256-query-local`.
+
+Validation applies these limits when a policy is created or its content is 
updated:
+
+- 16 KiB of UTF-8 source;
+- operation depth 8 for the source and lowered row-filter predicate;
+- 256 nodes for the source AST and lowered row-filter predicate;
+- 4 KiB of UTF-8 for each decoded string literal; and
+- 256 bytes for each numeric literal;
+- 256 elements in a literal array; and
+- 256 array elements in total in the source and lowered row-filter predicate.
+
+Bare identifiers, arbitrary functions, comments, packages, imports, variables, 
chained
+comparisons, nested arrays, and a conditional rule without a final `else` are 
rejected. Read paths
+do not re-parse stored policy expressions.
+

Review Comment:
   Fixed in 3ad6e25461. The Policies guide now includes the complete 
filter/mask grammar plus string escaping and Unicode behavior, exact-number 
syntax, case sensitivity, supported operand shapes, action allowlist, rejection 
rules, and source limits. The documented executable programs remain covered by 
the docs-drift test.



-- 
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]

Reply via email to