From b10557afc1f7b82a498708d4530b1a0b7ca6a1db Mon Sep 17 00:00:00 2001 From: Adam Preuss Date: Wed, 21 Jan 2026 15:11:59 -0700 Subject: [PATCH] Ensure that check queries have a consistent structure by replacing the original rule head with a placeholder predicate. Add an API that validates the Rule before including it in the biscuit. --- .../eclipse/biscuit/token/builder/Block.java | 14 ++++++ .../eclipse/biscuit/token/builder/Check.java | 8 ++- .../eclipse/biscuit/builder/BuilderTest.java | 49 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/eclipse/biscuit/token/builder/Block.java b/src/main/java/org/eclipse/biscuit/token/builder/Block.java index dc7fce0d..6a697a36 100644 --- a/src/main/java/org/eclipse/biscuit/token/builder/Block.java +++ b/src/main/java/org/eclipse/biscuit/token/builder/Block.java @@ -26,6 +26,8 @@ import org.eclipse.biscuit.datalog.SchemaVersion; import org.eclipse.biscuit.datalog.SymbolTable; import org.eclipse.biscuit.error.Error; +import org.eclipse.biscuit.error.LogicError; +import org.eclipse.biscuit.error.Result; import org.eclipse.biscuit.token.builder.parser.Parser; public final class Block { @@ -56,6 +58,18 @@ public Block addFact(String s) throws Error.Parser { return addFact(res.getOk()._2); } + public Result addRule(Rule rule, boolean validate) { + if (validate) { + var valid = rule.validateVariables(); + if (valid.isErr()) { + return Result.err( + new Error.FailedLogic(new LogicError.InvalidBlockRule(0, valid.getErr()))); + } + } + this.rules.add(rule); + return Result.ok(this); + } + public Block addRule(Rule rule) { this.rules.add(rule); return this; diff --git a/src/main/java/org/eclipse/biscuit/token/builder/Check.java b/src/main/java/org/eclipse/biscuit/token/builder/Check.java index 296c38d7..9cb21d33 100644 --- a/src/main/java/org/eclipse/biscuit/token/builder/Check.java +++ b/src/main/java/org/eclipse/biscuit/token/builder/Check.java @@ -24,8 +24,14 @@ public Check(org.eclipse.biscuit.datalog.Check.Kind kind, List queries) { public Check(org.eclipse.biscuit.datalog.Check.Kind kind, Rule query) { this.kind = kind; + // Checks are queries that match facts, not rules that generate facts. + // Replace the rule's head with a placeholder predicate since checks only use the body, + // expressions, and scopes. + Rule q = + new Rule( + new Predicate("query", new ArrayList<>()), query.body, query.expressions, query.scopes); ArrayList r = new ArrayList<>(); - r.add(query); + r.add(q); queries = r; } diff --git a/src/test/java/org/eclipse/biscuit/builder/BuilderTest.java b/src/test/java/org/eclipse/biscuit/builder/BuilderTest.java index 36474870..e7d71b7b 100644 --- a/src/test/java/org/eclipse/biscuit/builder/BuilderTest.java +++ b/src/test/java/org/eclipse/biscuit/builder/BuilderTest.java @@ -14,18 +14,23 @@ import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.eclipse.biscuit.crypto.KeyPair; import org.eclipse.biscuit.datalog.SymbolTable; import org.eclipse.biscuit.error.Error; import org.eclipse.biscuit.token.Biscuit; import org.eclipse.biscuit.token.builder.Block; +import org.eclipse.biscuit.token.builder.Check; import org.eclipse.biscuit.token.builder.Expression; +import org.eclipse.biscuit.token.builder.Rule; import org.eclipse.biscuit.token.builder.Term; import org.eclipse.biscuit.token.builder.Utils; +import org.eclipse.biscuit.token.builder.parser.Parser; import org.junit.jupiter.api.Test; public class BuilderTest { @@ -143,4 +148,48 @@ public void testArrayValueIsCopy() { System.identityHashCode(term.getValue()), "different objects"); } + + @Test + public void testCheckOnlyIncludesQuery() { + // Built `not_before` check: + var head = Utils.pred("nbf", List.of(Utils.var("0"), Utils.var("1"))); + var body = + List.of( + Utils.pred("time", List.of(Utils.var("0"))), + Utils.pred("nbf", List.of(Utils.var("1")))); + List expressions = + List.of( + new Expression.Binary( + Expression.Op.LessOrEqual, + new Expression.Value(Utils.var("1")), + new Expression.Value(Utils.var("0")))); + List scopes = new ArrayList<>(); + var nbfRule = new Rule(head, body, expressions, scopes); + Check builtCheck = Utils.check(nbfRule); + + // Parsed `not_before` check: + var res = Parser.check("check if time($0), nbf($1), $1 <= $0"); + + assertEquals(builtCheck, res.getOk()._2); + } + + @Test + public void testInvalidRuleFails() { + // Head must not include variables that are not in the body + var head = Utils.pred("nbf", List.of(Utils.var("x"))); + var body = + List.of( + Utils.pred("time", List.of(Utils.var("0"))), + Utils.pred("nbf", List.of(Utils.var("1")))); + List expressions = + List.of( + new Expression.Binary( + Expression.Op.LessOrEqual, + new Expression.Value(Utils.var("1")), + new Expression.Value(Utils.var("0")))); + List scopes = new ArrayList<>(); + var nbfRule = new Rule(head, body, expressions, scopes); + Block authorityBuilder = new Block(); + assertTrue(authorityBuilder.addRule(nbfRule, true).isErr()); + } }