feat: detect free variables in rule expressions

Free variables are already forbidden in rule heads, but not in rule expressions.

biscuit-auth/src/parser.rs

@@ -154,13 +154,25 @@ mod tests {
     }
 
     #[test]
-    fn rule_with_unused_head_variables() {
+    fn rule_with_free_head_variables() {
         assert_eq!(
             biscuit_parser::parser::rule("right($0, $test) <- resource($0), operation(\"read\")"),
             Err( nom::Err::Failure(Error {
-                input: "right($0, $test)",
+                input: "right($0, $test) <- resource($0), operation(\"read\")",
                 code: ErrorKind::Satisfy,
-                message: Some("rule head contains variables that are not used in predicates of the rule's body: $test".to_string()),
+                message: Some("the rule contains variables that are not bound by predicates in the rule's body: $test".to_string()),
+            }))
+        );
+    }
+
+    #[test]
+    fn rule_with_free_expression_variables() {
+        assert_eq!(
+            biscuit_parser::parser::rule("right($0) <- resource($0), operation(\"read\"), $test"),
+            Err( nom::Err::Failure(Error {
+                input: "right($0) <- resource($0), operation(\"read\"), $test",
+                code: ErrorKind::Satisfy,
+                message: Some("the rule contains variables that are not bound by predicates in the rule's body: $test".to_string()),
             }))
         );
     }

biscuit-parser/src/builder.rs

@@ -1,6 +1,6 @@
 //! helper functions and structure to create tokens and blocks
 use std::{
-    collections::{BTreeSet, HashMap},
+    collections::{BTreeSet, HashMap, HashSet},
     time::{SystemTime, UNIX_EPOCH},
 };
 
@@ -328,33 +328,38 @@ impl Rule {
     }
 
     pub fn validate_variables(&self) -> Result<(), String> {
-        let mut head_variables: std::collections::HashSet<String> = self
-            .head
-            .terms
-            .iter()
-            .filter_map(|term| match term {
-                Term::Variable(s) => Some(s.to_string()),
-                _ => None,
-            })
-            .collect();
+        let mut free_variables: HashSet<String> = HashSet::default();
+        for term in self.head.terms.iter() {
+            if let Term::Variable(s) = term {
+                free_variables.insert(s.to_string());
+            }
+        }
+
+        for e in self.expressions.iter() {
+            for op in e.ops.iter() {
+                if let Op::Value(Term::Variable(s)) = op {
+                    free_variables.insert(s.to_string());
+                }
+            }
+        }
 
         for predicate in self.body.iter() {
             for term in predicate.terms.iter() {
                 if let Term::Variable(v) = term {
-                    head_variables.remove(v);
-                    if head_variables.is_empty() {
+                    free_variables.remove(v);
+                    if free_variables.is_empty() {
                         return Ok(());
                     }
                 }
             }
         }
 
-        if head_variables.is_empty() {
+        if free_variables.is_empty() {
             Ok(())
         } else {
             Err(format!(
-                    "rule head contains variables that are not used in predicates of the rule's body: {}",
-                    head_variables
+                    "the rule contains variables that are not bound by predicates in the rule's body: {}",
+                    free_variables
                     .iter()
                     .map(|s| format!("${}", s))
                     .collect::<Vec<_>>()

biscuit-parser/src/parser.rs

@@ -175,18 +175,22 @@ pub fn rule(i: &str) -> IResult<&str, builder::Rule, Error> {
 }
 
 pub fn rule_inner(i: &str) -> IResult<&str, builder::Rule, Error> {
-    let (i, (head_input, head)) = consumed(rule_head)(i)?;
-    let (i, _) = space0(i)?;
+    let (i, (input, (head, body, expressions, scopes))) = consumed(|i| {
+        let (i, head) = rule_head(i)?;
+        let (i, _) = space0(i)?;
+
+        let (i, _) = tag("<-")(i)?;
 
-    let (i, _) = tag("<-")(i)?;
+        let (i, (body, expressions, scopes)) = cut(rule_body)(i)?;
 
-    let (i, (body, expressions, scopes)) = cut(rule_body)(i)?;
+        Ok((i, (head, body, expressions, scopes)))
+    })(i)?;
 
     let rule = builder::Rule::new(head, body, expressions, scopes);
 
     if let Err(message) = rule.validate_variables() {
         return Err(nom::Err::Failure(Error {
-            input: head_input,
+            input,
             code: ErrorKind::Satisfy,
             message: Some(message),
         }));