Compare commits

..

5 Commits

Author SHA1 Message Date
copilot-swe-agent[bot]
6cf77bb5ea Clarify deferral comments and dynamic path docs 2026-06-09 21:10:05 +00:00
copilot-swe-agent[bot]
6dc3a3e2dc Improve active-rule deferral matching docs and path checks 2026-06-09 21:04:19 +00:00
copilot-swe-agent[bot]
e8f126d479 Refine module-evaluation deferral logic for active rules 2026-06-09 19:18:20 +00:00
copilot-swe-agent[bot]
bc2fcc1cee Fix import-driven cross-package lookup behavior in interpreter and RVM 2026-06-09 19:15:12 +00:00
copilot-swe-agent[bot]
e865f13102 Initial plan 2026-06-09 17:05:19 +00:00
6 changed files with 229 additions and 45 deletions

View File

@@ -837,9 +837,9 @@ dependencies = [
[[package]]
name = "pyo3"
version = "0.29.0"
version = "0.28.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cd274650b21d4bfc26a0a47587962c1edb425f69287324355cd040c3ea66071c"
checksum = "91fd8e38a3b50ed1167fb981cd6fd60147e091784c427b8f7183a7ee32c31c12"
dependencies = [
"anyhow",
"libc",
@@ -852,18 +852,18 @@ dependencies = [
[[package]]
name = "pyo3-build-config"
version = "0.29.0"
version = "0.28.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c5e2a7d2f0d013342f295c048ad19237add5154a55b1c5a254c0ec93d4109078"
checksum = "e368e7ddfdeb98c9bca7f8383be1648fd84ab466bf2bc015e94008db6d35611e"
dependencies = [
"target-lexicon",
]
[[package]]
name = "pyo3-ffi"
version = "0.29.0"
version = "0.28.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ca85c467da1bbc8d866eea5deff9cf29ea5f7785054a17da36e65bda9c05845b"
checksum = "7f29e10af80b1f7ccaf7f69eace800a03ecd13e883acfacc1e5d0988605f651e"
dependencies = [
"libc",
"pyo3-build-config",
@@ -871,9 +871,9 @@ dependencies = [
[[package]]
name = "pyo3-macros"
version = "0.29.0"
version = "0.28.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9ac53762fd065daa3194dd09337a38bd793a188100fd1a9304c4ab312d901771"
checksum = "df6e520eff47c45997d2fc7dd8214b25dd1310918bbb2642156ef66a67f29813"
dependencies = [
"proc-macro2",
"pyo3-macros-backend",
@@ -883,12 +883,13 @@ dependencies = [
[[package]]
name = "pyo3-macros-backend"
version = "0.29.0"
version = "0.28.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4ca3a1557399783172dc5bf39cfca835157732532cba56b71d2292161e53b362"
checksum = "c4cdc218d835738f81c2338f822078af45b4afdf8b2e33cbb5916f108b813acb"
dependencies = [
"heck",
"proc-macro2",
"pyo3-build-config",
"quote",
"syn",
]

View File

@@ -23,7 +23,7 @@ coverage = ["regorus/coverage"]
[dependencies]
anyhow = "1.0"
ordered-float = "5.3.0"
pyo3 = { version = "0.29.0", features = ["abi3-py310", "anyhow", "extension-module"] }
pyo3 = { version = "0.28.3", features = ["abi3-py310", "anyhow", "extension-module"] }
regorus = { path = "../..", default-features = false, features = ["arc", "rvm"] }
serde_json = "1.0.150"

View File

@@ -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,136 @@ 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 with the `data.a.b.` prefix.
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("."))
}
}
/// Returns `true` when `prefix` matches `path` on segment boundaries.
///
/// Examples:
/// - `path_is_prefix("data.auth", "data.auth") == true`
/// - `path_is_prefix("data.auth", "data.auth.allow") == true`
/// - `path_is_prefix("data.auth", "data.authorization") == false`
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('.'))
}
/// Checks whether a `requested_path` can contain values produced by an active rule.
///
/// The active rule path is logically `module_path.rule_path`, but this check avoids
/// allocating that joined string in tight evaluation loops.
///
/// Examples:
/// - request `data` matches module `data.authz` (module expansion needed)
/// - request `data.authz` matches rule `allow`
/// - request `data.authz.allow` matches rule `allow`
/// - request `data.auth` does not match module `data.authz`
fn request_matches_active_rule_path(
requested_path: &str,
module_path: &str,
rule_path: &str,
) -> bool {
if Self::path_is_prefix(requested_path, module_path) {
return true;
}
requested_path
.strip_prefix(module_path)
.and_then(|suffix| suffix.strip_prefix('.'))
.is_some_and(|requested_rule_prefix| {
Self::path_is_prefix(requested_rule_prefix, rule_path)
})
}
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)?;
if Self::request_matches_active_rule_path(requested_path, &module_path, &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 so legitimate
/// cross-package references are not misidentified as cyclic recursion.
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 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 re-entry through sibling rules or other modules 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 +3140,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 +3190,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,

View File

@@ -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() {

View File

@@ -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(())
}

View File

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