Refine active-rule deferral path matching; expand interpreter docs with concrete examples

src/interpreter.rs

@@ -586,7 +586,7 @@ impl Interpreter {
                                 } else {
                                     format!("{ref_path}.{index}.{}", path.join("."))
                                 };
-                                self.ensure_rule_evaluated(ref_path)?;
+                                self.ensure_matching_rules_for_dynamic_data_index(&ref_path)?;
                             }
                         }
 
@@ -2960,6 +2960,103 @@ impl Interpreter {
         Ok(())
     }
 
+    /// Ensures all rule/default-rule paths matching a dynamic data lookup are evaluated.
+    /// Matches both exact path (`data.a.b`) and descendants (`data.a.b.*`).
+    fn ensure_matching_rules_for_dynamic_data_index(&mut self, path: &str) -> Result<()> {
+        self.check_execution_time()?;
+        let path_prefix = format!("{path}.");
+        let mut matching_paths: Vec<String> = self
+            .compiled_policy
+            .default_rules
+            .keys()
+            .chain(self.compiled_policy.rules.keys())
+            .filter(|rule_path| *rule_path == path || rule_path.starts_with(&path_prefix))
+            .cloned()
+            .collect();
+        matching_paths.sort();
+        matching_paths.dedup();
+
+        for rule_path in matching_paths {
+            self.ensure_rule_evaluated(rule_path)?;
+        }
+
+        Ok(())
+    }
+
+    /// Builds a canonical `data` path from field components.
+    /// For an empty field list, returns `"data"`.
+    fn build_data_path(fields: &[&str]) -> String {
+        if fields.is_empty() {
+            "data".to_string()
+        } else {
+            format!("data.{}", fields.join("."))
+        }
+    }
+
+    fn path_is_prefix(prefix: &str, path: &str) -> bool {
+        if path == prefix {
+            return true;
+        }
+        path.get(prefix.len()..)
+            .is_some_and(|suffix| suffix.starts_with('.'))
+    }
+
+    fn should_defer_module_eval_for_path(&self, requested_path: &str) -> Result<bool> {
+        for active_rule in &self.active_rules {
+            let module = self.get_rule_module(active_rule)?;
+            let module_path = get_path_string(&module.package.refr, Some("data"))?;
+            let rule_path = get_path_string(Self::get_rule_refr(active_rule), None)?;
+            let full_rule_path = format!("{}.{}", module_path, rule_path);
+            if Self::path_is_prefix(requested_path, &full_rule_path) {
+                return Ok(true);
+            }
+        }
+        Ok(false)
+    }
+
+    /// Resolves `data.<fields...>` while preserving correct rule semantics.
+    /// When a rule is already active, this avoids eager module-wide evaluation to prevent
+    /// false-positive recursion detection and only evaluates matching rule paths.
+    fn lookup_data_path(&mut self, fields: &[&str]) -> Result<Value> {
+        if self.is_processed(fields)? {
+            return Ok(Self::get_value_chained(self.data.clone(), fields));
+        }
+
+        // If "data" is used in a query, without any fields, then evaluate all the modules.
+        if fields.is_empty() && self.active_rules.is_empty() {
+            for module in self.compiled_policy.modules.clone().iter() {
+                for rule in &module.policy {
+                    self.eval_rule(module, rule)?;
+                }
+            }
+        }
+
+        // While a rule is active, avoid eagerly evaluating all matching modules.
+        // This prevents sibling/module re-entry from being misclassified as cyclic recursion.
+        let requested_path = Self::build_data_path(fields);
+        if self.active_rules.is_empty()
+            || !self.should_defer_module_eval_for_path(&requested_path)?
+        {
+            self.ensure_module_evaluated(requested_path.clone())?;
+        }
+
+        for i in (1..=fields.len()).rev() {
+            let prefix = fields.iter().take(i).copied().collect::<Vec<_>>();
+            let prefix_path = Self::build_data_path(&prefix);
+            if self.compiled_policy.rules.contains_key(&prefix_path)
+                || self
+                    .compiled_policy
+                    .default_rules
+                    .contains_key(&prefix_path)
+            {
+                self.ensure_rule_evaluated(prefix_path)?;
+                break;
+            }
+        }
+
+        Ok(Self::get_value_chained(self.data.clone(), fields))
+    }
+
     fn is_processed(&self, path: &[&str]) -> Result<bool> {
         let mut obj = &self.processed_paths;
         for p in path {
@@ -3010,39 +3107,7 @@ impl Interpreter {
 
         // Ensure that rules are evaluated
         if name.text() == "data" {
-            if self.is_processed(fields)? {
-                return Ok(Self::get_value_chained(self.data.clone(), fields));
-            }
-
-            // If "data" is used in a query, without any fields, then evaluate all the modules.
-            if fields.is_empty() && self.active_rules.is_empty() {
-                for module in self.compiled_policy.modules.clone().iter() {
-                    for rule in &module.policy {
-                        self.eval_rule(module, rule)?;
-                    }
-                }
-            }
-
-            // With modifiers may be used to specify part of a module that that not yet been
-            // evaluated. Therefore ensure that module is evaluated first.
-            let requested_path = format!("data.{}", fields.join("."));
-            self.ensure_module_evaluated(requested_path.clone())?;
-
-            for i in (1..=fields.len()).rev() {
-                let prefix = fields.iter().take(i).copied().collect::<Vec<_>>();
-                let prefix_path = format!("data.{}", prefix.join("."));
-                if self.compiled_policy.rules.contains_key(&prefix_path)
-                    || self
-                        .compiled_policy
-                        .default_rules
-                        .contains_key(&prefix_path)
-                {
-                    self.ensure_rule_evaluated(prefix_path)?;
-                    break;
-                }
-            }
-
-            Ok(Self::get_value_chained(self.data.clone(), fields))
+            self.lookup_data_path(fields)
         } else if !self.compiled_policy.modules.is_empty() {
             let module = self.current_module()?;
             let parsed_path = Parser::get_path_ref_components(&module.package.refr)?;
@@ -3092,6 +3157,17 @@ impl Interpreter {
 
             if !found {
                 if let Some(imported_var) = self.compiled_policy.imports.get(&rule_path).cloned() {
+                    if let Ok(import_path) = get_path_string(&imported_var, None) {
+                        if import_path == "data" || import_path.starts_with("data.") {
+                            let combined_path = if fields.is_empty() {
+                                import_path
+                            } else {
+                                format!("{}.{}", import_path, fields.join("."))
+                            };
+                            let data_fields: Vec<&str> = combined_path.split('.').skip(1).collect();
+                            return self.lookup_data_path(&data_fields);
+                        }
+                    }
                     return Ok(Self::get_value_chained(
                         self.eval_expr(&imported_var)?,
                         fields,

src/languages/rego/compiler/references.rs

@@ -338,6 +338,15 @@ impl<'a> Compiler<'a> {
         // No rule found; fall back to module-level imports.
         let import_key = format!("{}.{}", &self.current_package, root);
         if let Some(import_expr) = self.policy.inner.imports.get(&import_key) {
+            if let Ok(mut import_chain) = parse_reference_chain(import_expr) {
+                if let ReferenceRoot::Variable(import_root) = &import_chain.root {
+                    if import_root == "data" {
+                        import_chain.components.extend(chain.components.clone());
+                        return self.compile_data_chain(&import_chain, span);
+                    }
+                }
+            }
+
             let import_reg =
                 self.compile_rego_expr_with_span(import_expr, import_expr.span(), false)?;
             if chain.components.is_empty() {

tests/engine/mod.rs

@@ -197,3 +197,34 @@ fn get_policy_parameters() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn cross_package_import_lookup_does_not_false_cycle() -> Result<()> {
+    let mut engine = Engine::new();
+    engine.add_policy(
+        "registry.rego".to_string(),
+        r#"package registry
+           import data.registry.packages.package_a
+           import rego.v1
+
+           allow_stage1 if package_a.allow_stage1
+         "#
+        .to_string(),
+    )?;
+    engine.add_policy(
+        "package_a.rego".to_string(),
+        r#"package registry.packages.package_a
+           import data.registry
+           import rego.v1
+
+           allow_stage2 if registry.allow_stage1
+           allow_stage1 := true
+         "#
+        .to_string(),
+    )?;
+
+    let result = engine.eval_rule("data.registry.packages.package_a.allow_stage2".to_string())?;
+    assert_eq!(result, Value::Bool(true));
+
+    Ok(())
+}

tests/rvm/rego/cases/imports.yaml

@@ -95,3 +95,37 @@ cases:
         }
     query: data.rules.present
     want_result: true
+  - note: import_cross_package_no_false_cycle
+    modules:
+      - |
+        package registry
+        import data.registry.packages.package_a
+        import rego.v1
+
+        allow_stage1 if package_a.allow_stage1
+      - |
+        package registry.packages.package_a
+        import data.registry
+        import rego.v1
+
+        allow_stage2 if registry.allow_stage1
+        allow_stage1 := true
+    query: data.registry.packages.package_a.allow_stage2
+    want_result: true
+  - note: dynamic_data_index_evaluates_matching_rules
+    modules:
+      - |
+        package registry
+        import rego.v1
+
+        allow_stage1 if {
+          name := "package_a"
+          data.registry.packages[name].allow_stage2
+        }
+      - |
+        package registry.packages.package_a
+        import rego.v1
+
+        allow_stage2 := true
+    query: data.registry.allow_stage1
+    want_result: true