Level validation Rust implementation (RFC 76)

cedar-policy-validator/src/entity_manifest.rs

@@ -407,7 +407,7 @@ pub fn compute_entity_manifest(
                     // using static analysis
                     compute_root_trie(&typechecked_expr, policy.id())
                 }
-                PolicyCheck::Irrelevant(_) => {
+                PolicyCheck::Irrelevant(_, _) => {
                     // this policy is irrelevant, so we need no data
                     Ok(RootAccessTrie::new())
                 }

cedar-policy-validator/src/lib.rs

@@ -285,7 +285,7 @@ impl Validator {
         let mut errs = vec![];
         for (_, policy_check) in type_annotated_asts {
             match policy_check {
-                PolicyCheck::Success(e) => {
+                PolicyCheck::Success(e) | PolicyCheck::Irrelevant(_, e) => {
                     let res =
                         self.check_entity_deref_level_helper(&e, max_allowed_level, policy_id);
                     match res.1 {
@@ -296,7 +296,6 @@ impl Validator {
                 // PANIC SAFETY: We only validate the level after strict validation passed
                 #[allow(clippy::unreachable)]
                 PolicyCheck::Fail(_) => unreachable!(),
-                PolicyCheck::Irrelevant(_) => (), //Don't report level violations if typechecking already failed
             }
         }
         errs

cedar-policy-validator/src/typecheck.rs

@@ -52,7 +52,7 @@ pub enum PolicyCheck {
     /// Policy will evaluate to a bool
     Success(Expr<Option<Type>>),
     /// Policy will always evaluate to false, and may have errors
-    Irrelevant(Vec<ValidationError>),
+    Irrelevant(Vec<ValidationError>, Expr<Option<Type>>),
     /// Policy will have errors
     Fail(Vec<ValidationError>),
 }
@@ -106,7 +106,7 @@ impl<'a> Typechecker<'a> {
             (true, true),
             |(all_false, all_succ), (_, check)| match check {
                 PolicyCheck::Success(_) => (false, all_succ),
-                PolicyCheck::Irrelevant(err) => {
+                PolicyCheck::Irrelevant(err, _) => {
                     let no_err = err.is_empty();
                     type_errors.extend(err);
                     (all_false, all_succ && no_err)
@@ -155,7 +155,10 @@ impl<'a> Typechecker<'a> {
                 (false, true, None) => PolicyCheck::Fail(type_errors),
                 (false, true, Some(e)) => PolicyCheck::Success(e),
                 (false, false, _) => PolicyCheck::Fail(type_errors),
-                (true, _, _) => PolicyCheck::Irrelevant(type_errors),
+                (true, _, Some(e)) => PolicyCheck::Irrelevant(type_errors, e),
+                // PANIC SAFETY: `is_false` implies e is `Some(Lit(Bool(false)))`
+                #[allow(clippy::unreachable)]
+                (true, _, None) => unreachable!(),
             }
         })
     }
@@ -218,7 +221,12 @@ impl<'a> Typechecker<'a> {
                         (false, true, None) => policy_checks.push(PolicyCheck::Fail(type_errors)),
                         (false, true, Some(e)) => policy_checks.push(PolicyCheck::Success(e)),
                         (false, false, _) => policy_checks.push(PolicyCheck::Fail(type_errors)),
-                        (true, _, _) => policy_checks.push(PolicyCheck::Irrelevant(type_errors)),
+                        (true, _, Some(e)) => {
+                            policy_checks.push(PolicyCheck::Irrelevant(type_errors, e))
+                        }
+                        // PANIC SAFETY: `is_false` implies e is `Some(Lit(Bool(false)))`
+                        #[allow(clippy::unreachable)]
+                        (true, _, None) => unreachable!(),
                     }
                 }
             }

cedar-policy-validator/src/typecheck/test/policy.rs

@@ -149,7 +149,7 @@ fn policy_checked_in_multiple_envs() {
     assert!(
         env_checks
             .iter()
-            .filter(|(_, check)| { matches!(check, PolicyCheck::Irrelevant(_)) })
+            .filter(|(_, check)| { matches!(check, PolicyCheck::Irrelevant(_, _)) })
             .count()
             == 1
     );
@@ -171,7 +171,7 @@ fn policy_checked_in_multiple_envs() {
     assert!(
         env_checks
             .iter()
-            .filter(|(_, check)| { matches!(check, PolicyCheck::Irrelevant(_)) })
+            .filter(|(_, check)| { matches!(check, PolicyCheck::Irrelevant(_, _)) })
             .count()
             == 2
     );

cedar-policy/src/tests.rs

@@ -4023,6 +4023,39 @@ mod level_validation_tests {
         assert!(result.validation_passed());
     }
 
+    #[test]
+    fn level_validation_irrelevant_policy_passes() {
+        let schema = get_schema();
+        let validator = Validator::new(schema);
+
+        let mut set = PolicySet::new();
+        let src = r#"permit(principal == User::"һenry", action, resource) when { false && principal.is_admin };"#;
+        let p = Policy::parse(None, src).unwrap();
+        set.add(p).unwrap();
+
+        let result = validator.strict_validate_with_level(&set, 0);
+        assert!(result.validation_passed());
+    }
+
+    #[test]
+    fn level_validation_irrelevant_policy_fails() {
+        let schema = get_schema();
+        let validator = Validator::new(schema);
+
+        let mut set = PolicySet::new();
+        let src = r#"permit(principal == User::"һenry", action, resource) when { principal.is_admin && false };"#;
+        let p = Policy::parse(None, src).unwrap();
+        set.add(p).unwrap();
+
+        let result = validator.strict_validate_with_level(&set, 0);
+        assert!(!result.validation_passed());
+        assert_eq!(result.validation_errors().count(), 1);
+        assert_matches!(
+            result.validation_errors().next().unwrap(),
+            ValidationError::EntityDerefLevelViolation(_)
+        );
+    }
+
     #[test]
     fn level_validation_fails_ite() {
         let schema = get_schema();