From 32f3367eb7dfd7bdf271928d0a6f910cfcf3f60b Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Wed, 30 Sep 2026 21:53:33 +0200 Subject: [PATCH 1/7] fix: preserve literal namespace components across evaluation paths Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/csharp/Regorus.Tests/RegorusTests.cs | 20 + bindings/ffi/src/engine.rs | 107 +++++ docs/grammar.md | 5 + src/compiled_policy.rs | 1 + src/engine.rs | 39 +- src/interpreter.rs | 431 ++++++++++-------- src/interpreter/target/resolve.rs | 45 +- src/languages/rego/compiler/function_calls.rs | 21 +- src/languages/rego/compiler/references.rs | 88 ++-- src/languages/rego/compiler/rules.rs | 117 +++-- src/parser.rs | 59 ++- src/rvm/vm/virtual_data.rs | 75 ++- src/tests/interpreter/mod.rs | 37 ++ src/utils.rs | 201 +++++++- tests/engine/mod.rs | 329 +++++++++++++ tests/interpreter/cases/import/tests.yaml | 48 ++ tests/interpreter/cases/refr/tests.yaml | 27 ++ .../cases/target/azure_policy.yaml | 25 + tests/rvm/rego/cases/namespace_paths.yaml | 207 +++++++++ tests/rvm/rego/mod.rs | 150 +++++- 20 files changed, 1680 insertions(+), 352 deletions(-) create mode 100644 tests/rvm/rego/cases/namespace_paths.yaml diff --git a/bindings/csharp/Regorus.Tests/RegorusTests.cs b/bindings/csharp/Regorus.Tests/RegorusTests.cs index 6fabdd4fd..e4b46c808 100644 --- a/bindings/csharp/Regorus.Tests/RegorusTests.cs +++ b/bindings/csharp/Regorus.Tests/RegorusTests.cs @@ -51,6 +51,26 @@ public void Rule_conflict_preserves_error_status_and_reports_previous_location() Assert.IsFalse(ex.Message.Contains('"')); } + [TestMethod] + public void Dotted_package_metadata_and_rule_paths_remain_canonical() + { + using var engine = new Engine(); + var package = engine.AddPolicy( + "namespace.rego", + """ + package graph.defUniqueName["1.0.0"] + + default deny := false + deny := true if { input.blocked == true } + """); + + Assert.AreEqual("data.graph.defUniqueName[\"1.0.0\"]", package); + engine.SetInputJson("""{"blocked":false}"""); + Assert.AreEqual( + "false", + engine.EvalRule("data.graph.defUniqueName[\"1.0.0\"].deny")); + } + [TestMethod] public void Evaluation_using_file_policies_succeeds() { diff --git a/bindings/ffi/src/engine.rs b/bindings/ffi/src/engine.rs index 7bdd143de..8f9d04afb 100644 --- a/bindings/ffi/src/engine.rs +++ b/bindings/ffi/src/engine.rs @@ -863,3 +863,110 @@ pub extern "C" fn regorus_engine_compile_program_with_entrypoints( } }) } + +#[cfg(all(test, feature = "std"))] +mod namespace_tests { + use super::{ + regorus_engine_add_policy, regorus_engine_compile_program_with_entrypoints, + regorus_engine_drop, regorus_engine_eval_rule, regorus_engine_new, + }; + use crate::common::{regorus_result_drop, RegorusStatus}; + use core::ffi::CStr; + use std::ffi::CString; + + #[test] + fn canonical_dotted_namespace_paths_cross_the_ffi_boundary() { + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let file = CString::new("namespace.rego").expect("valid file path"); + let policy = CString::new( + "package graph.defUniqueName[\"1.0.0\"]\n\ + default deny := false\n\ + deny := true if { input.blocked == true }\n", + ) + .expect("valid policy"); + let added = regorus_engine_add_policy(engine, file.as_ptr(), policy.as_ptr()); + assert!(matches!(added.status, RegorusStatus::Ok)); + unsafe { + let package = CStr::from_ptr(added.output) + .to_str() + .expect("UTF-8 package"); + assert_eq!(package, "data.graph.defUniqueName[\"1.0.0\"]"); + } + regorus_result_drop(added); + + let rule = + CString::new("data.graph.defUniqueName[\"1.0.0\"].deny").expect("valid rule path"); + let result = regorus_engine_eval_rule(engine, rule.as_ptr()); + assert!(matches!(result.status, RegorusStatus::Ok)); + unsafe { + let value = CStr::from_ptr(result.output).to_str().expect("UTF-8 value"); + assert_eq!(value, "false"); + } + regorus_result_drop(result); + regorus_engine_drop(engine); + } + + #[cfg(feature = "rvm")] + #[test] + fn bracketed_identifier_entrypoint_compiles_and_executes_through_engine_ffi() { + use crate::rvm::{ + regorus_program_drop, regorus_rvm_drop, regorus_rvm_execute_entry_point_by_index, + regorus_rvm_execute_entry_point_by_name, regorus_rvm_load_program, regorus_rvm_new, + RegorusProgram, + }; + + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let file = CString::new("namespace.rego").expect("valid file path"); + let policy = CString::new("package graph.version\nvalue := 7").expect("valid policy"); + let added = regorus_engine_add_policy(engine, file.as_ptr(), policy.as_ptr()); + assert!(matches!(added.status, RegorusStatus::Ok)); + regorus_result_drop(added); + + let entrypoint = CString::new("data.graph[\"version\"].value").expect("valid entry point"); + let entrypoints = [entrypoint.as_ptr()]; + let compiled = + regorus_engine_compile_program_with_entrypoints(engine, entrypoints.as_ptr(), 1); + assert!( + matches!(compiled.status, RegorusStatus::Ok), + "RVM compilation failed with {:?}", + compiled.status + ); + let program = compiled.pointer_value as *mut RegorusProgram; + assert!(!program.is_null()); + regorus_result_drop(compiled); + + let vm = regorus_rvm_new(); + assert!(!vm.is_null()); + let loaded = regorus_rvm_load_program(vm, program); + assert!(matches!(loaded.status, RegorusStatus::Ok)); + regorus_result_drop(loaded); + + let named = regorus_rvm_execute_entry_point_by_name(vm, entrypoint.as_ptr()); + assert!(matches!(named.status, RegorusStatus::Ok)); + assert_eq!( + unsafe { CStr::from_ptr(named.output) } + .to_str() + .expect("UTF-8 result"), + "7" + ); + regorus_result_drop(named); + + let indexed = regorus_rvm_execute_entry_point_by_index(vm, 0); + assert!(matches!(indexed.status, RegorusStatus::Ok)); + assert_eq!( + unsafe { CStr::from_ptr(indexed.output) } + .to_str() + .expect("UTF-8 result"), + "7" + ); + regorus_result_drop(indexed); + + regorus_rvm_drop(vm); + regorus_program_drop(program); + regorus_engine_drop(engine); + } +} diff --git a/docs/grammar.md b/docs/grammar.md index 9056dde1b..7f69abbd5 100644 --- a/docs/grammar.md +++ b/docs/grammar.md @@ -155,6 +155,11 @@ referenced by: path-ref ::= IDENT ( NO_WS ( '.' NO_WS IDENT | '[' STRING ']' ) )* ``` +String components in brackets retain their literal identity, including dots. +For example, `graph["a.b"]` is distinct from `graph.a.b`. When a package path +is returned by the Engine API or used as a rule entry point, non-identifier +string components use JSON-escaped bracket notation. + referenced by: * call-expr diff --git a/src/compiled_policy.rs b/src/compiled_policy.rs index edaa4d35a..f401c5545 100644 --- a/src/compiled_policy.rs +++ b/src/compiled_policy.rs @@ -215,6 +215,7 @@ pub(crate) struct CompiledPolicyData { pub(crate) schedule: Option>, pub(crate) rules: Map>>, pub(crate) default_rules: Map>, + pub(crate) rule_path_components: Map>, pub(crate) imports: BTreeMap>, pub(crate) functions: FunctionTable, pub(crate) rule_paths: MapSet, diff --git a/src/engine.rs b/src/engine.rs index 97aa6ef97..1ba70bf07 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -8,9 +8,9 @@ use crate::interpreter::*; use crate::lexer::*; use crate::parser::*; use crate::scheduler::*; -use crate::utils::gather_functions; use crate::utils::limits::PolicyLengthConfig; use crate::utils::limits::{self, fallback_execution_timer_config, ExecutionTimerConfig}; +use crate::utils::{gather_functions, get_path_string}; use crate::value::*; use crate::*; use crate::{Extension, QueryResults}; @@ -217,7 +217,9 @@ impl Engine { /// /// The policy file will be parsed and converted to AST representation. /// Multiple policy files may be added to the engine. - /// Returns the Rego package name declared in the policy. + /// Returns the Rego package name declared in the policy, formatted as a + /// canonical Rego path. String components that are not identifiers use + /// bracket notation, for example `data.graph["1.0.0"]`. /// /// * `path`: A filename to be associated with the policy. /// * `rego`: The rego policy code. @@ -252,14 +254,16 @@ impl Engine { Rc::make_mut(&mut self.modules).push(module.clone()); // if policies change, interpreter needs to be prepared again self.prepared = false; - Interpreter::get_path_string(&module.package.refr, Some("data")) + get_path_string(&module.package.refr, Some("data")) } /// Add a policy from a given file. /// /// The policy file will be parsed and converted to AST representation. /// Multiple policy files may be added to the engine. - /// Returns the Rego package name declared in the policy. + /// Returns the Rego package name declared in the policy, formatted as a + /// canonical Rego path (using bracket notation for non-identifier string + /// components). /// /// * `path`: Path to the policy file (.rego). /// @@ -290,11 +294,16 @@ impl Engine { Rc::make_mut(&mut self.modules).push(module.clone()); // if policies change, interpreter needs to be prepared again self.prepared = false; - Interpreter::get_path_string(&module.package.refr, Some("data")) + get_path_string(&module.package.refr, Some("data")) } /// Get the list of packages defined by loaded policies. /// + /// Each package is returned as a canonical Rego path. String components + /// that are not identifiers use JSON-escaped bracket notation, so a + /// literal component such as `"1.0.0"` is not confused with three nested + /// identifier components. + /// /// ``` /// # use regorus::*; /// # fn main() -> anyhow::Result<()> { @@ -314,7 +323,7 @@ impl Engine { pub fn get_packages(&self) -> Result> { self.modules .iter() - .map(|m| Interpreter::get_path_string(&m.package.refr, Some("data"))) + .map(|m| get_path_string(&m.package.refr, Some("data"))) .collect() } @@ -796,8 +805,13 @@ impl Engine { /// The `rule` parameter should follow the Rego rule path format: /// - `"data.package.rule"` - For rules in a specific package /// - `"data.package.subpackage.rule"` - For nested packages + /// - `"data.graph.defUniqueName[\"1.0.0\"].deny"` - For a rule under a + /// package with a literal string component /// - `"allow"` - For rules in the default package (though this is not recommended) /// + /// Bracketed string components are matched by their literal value; an + /// identifier in brackets is equivalent to dot notation. + /// /// # Notes /// /// - Automatically enables print gathering for debugging purposes @@ -851,6 +865,9 @@ impl Engine { /// let r = engine.eval_rule("data.exaample.x".to_string()); /// assert!(r.is_err()); /// + /// // Literal string components use bracket notation. + /// // engine.eval_rule("data.graph[\"1.0.0\"].deny".to_string())?; + /// /// // Path must be valid rule paths. /// assert!( engine.eval_rule("data".to_string()).is_err()); /// assert!( engine.eval_rule("data.example".to_string()).is_err()); @@ -1203,8 +1220,8 @@ impl Engine { // Ensure that empty modules are created. for m in self.modules.iter().filter(|m| m.policy.is_empty()) { - let path = Parser::get_path_ref_components(&m.package.refr)?; - let path: Vec<&str> = path.iter().map(|s| s.text()).collect(); + let path = Parser::get_static_string_path_components(&m.package.refr)?; + let path: Vec<&str> = path.iter().map(String::as_str).collect(); let vref = Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?; if *vref == Value::Undefined { @@ -1229,8 +1246,8 @@ impl Engine { // Ensure that all modules are created. for m in self.modules.iter() { - let path = Parser::get_path_ref_components(&m.package.refr)?; - let path: Vec<&str> = path.iter().map(|s| s.text()).collect(); + let path = Parser::get_static_string_path_components(&m.package.refr)?; + let path: Vec<&str> = path.iter().map(String::as_str).collect(); let vref = Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?; if *vref == Value::Undefined { @@ -1490,7 +1507,7 @@ impl Engine { pub fn get_policy_package_names(&self) -> Result> { let mut package_names = vec![]; for m in self.modules.iter() { - let package_name = Interpreter::get_path_string(&m.package.refr, None)?; + let package_name = get_path_string(&m.package.refr, None)?; package_names.push(PolicyPackageNameDefinition { source_file: m.package.span.source.file().to_string(), package_name, diff --git a/src/interpreter.rs b/src/interpreter.rs index 0ac20eb75..e2ae23152 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -19,7 +19,11 @@ use crate::utils::limits::{monotonic_now, ExecutionTimer, ExecutionTimerConfig}; #[cfg(feature = "std")] use crate::utils::*; #[cfg(not(feature = "std"))] -use crate::utils::{get_extra_arg, get_path_string, get_root_var, FunctionTable}; +use crate::utils::{ + append_path_component, append_path_value_component, format_path_components, format_string_path, + get_extra_arg, get_path_string, get_root_var, get_rule_path_components, + split_canonical_path_root, FunctionTable, PathComponent, +}; use crate::value::*; use crate::*; use crate::{Expression, Extension, Location, QueryResult, QueryResults}; @@ -574,9 +578,9 @@ impl Interpreter { } Expr::RefBrack { refr, index, .. } => match index.as_ref() { // refr["field"] is the same as refr.field - Expr::String { span, .. } => { + Expr::String { value, .. } => { expr = refr; - path.push(span.text()); + path.push(value.as_string()?.as_ref()); } // Handle other forms of refr. // Note, we have the choice to evaluate a non-string index @@ -586,18 +590,42 @@ impl Interpreter { let index = self.eval_expr(index)?; // Handle indexing into data. - if let Ok(ref_path) = get_path_string(refr, None) { - if get_root_var(refr)?.text() == "data" && index != Value::Undefined { - let index = match &index { - Value::String(s) => s.to_string(), - _ => index.to_string(), - }; - let ref_path = if path.is_empty() { - format!("{ref_path}.{index}") - } else { - format!("{ref_path}.{index}.{}", path.join(".")) - }; - self.ensure_rule_evaluated(ref_path)?; + if index != Value::Undefined { + if let Ok(ref_components) = + Parser::get_static_string_path_components(refr) + { + if ref_components.first().map(String::as_str) == Some("data") { + let ref_component_refs: Vec<&str> = + ref_components.iter().map(String::as_str).collect(); + if let Ok(ref_path) = format_string_path(&ref_component_refs) { + let mut rule_path = + append_path_value_component(&ref_path, &index)?; + for component in &path { + rule_path = + append_path_component(&rule_path, component)?; + } + self.ensure_rule_evaluated(rule_path)?; + + if matches!(&index, Value::Number(_)) { + let mut string_path = append_path_component( + &ref_path, + &index.to_string(), + )?; + for component in &path { + string_path = + append_path_component(&string_path, component)?; + } + if self.compiled_policy.rules.contains_key(&string_path) + || self + .compiled_policy + .default_rules + .contains_key(&string_path) + { + self.ensure_rule_evaluated(string_path)?; + } + } + } + } } } @@ -1257,28 +1285,21 @@ impl Interpreter { let mut skip_exec = false; // Apply with modifiers. for wm in &stmt.with_mods { - let path = Parser::get_path_ref_components(&wm.refr)?; - let mut path: Vec = path.iter().map(|s| s.text().to_string()).collect(); + let mut path = get_rule_path_components(&wm.refr)?; // Matching OPA, a leading import alias is rewritten before // any lookups: functions register as overrides below, // anything else becomes a data override. Only the alias // component is replaced so bracketed keys containing dots // survive the rewrite. - let rewritten: Option> = match path.split_first() { - Some((head, rest)) if head.as_str() != "data" => { + let rewritten: Option> = match path.split_first() { + Some((PathComponent::String(head), rest)) if head.as_str() != "data" => { self.lookup_import(head).and_then(|import_expr| { // Use the import target's parsed components, not // its dot-joined string, so bracketed keys // containing dots survive in the import path too. - let comps = Parser::get_path_ref_components(import_expr).ok()?; - Some( - comps - .iter() - .map(|s| s.text().to_string()) - .chain(rest.iter().cloned()) - .collect(), - ) + let comps = get_rule_path_components(import_expr).ok()?; + Some(comps.into_iter().chain(rest.iter().cloned()).collect()) }) } _ => None, @@ -1286,16 +1307,13 @@ impl Interpreter { if let Some(new_path) = rewritten { path = new_path; } - let mut target = path.join("."); + let mut target = format_path_components(&path)?; let mut target_is_function = self.lookup_function_by_name(&target).is_some() || Self::is_builtin(wm.refr.span(), &target); - if !target_is_function - && !target.starts_with("data.") - && !target.starts_with("input.") - && target != "input" - { + let root = path.first().map(PathComponent::value).unwrap_or_default(); + if !target_is_function && root != "data" && root != "input" { // target must be a function. if self.lookup_function_by_name(&target).is_none() && !Self::is_builtin(wm.refr.span(), &target) @@ -1347,16 +1365,16 @@ impl Interpreter { let Some(first) = path.first() else { bail!(wm.refr.span().error("empty path in with modifier")); }; - if *first == "input" || *first == "data" { + if first.value() == "input" || first.value() == "data" { // Override existing values in case of conflict. let mut obj = &mut self.with_document; - for p in &path { + for component in &path { if !matches!(obj, Value::Object(_)) { *obj = Value::new_object(); } obj = obj.as_object_mut()?.get_or_insert_with( - Value::String(p.to_string().into()), + Value::String(component.value().to_string().into()), Value::new_object, ); } @@ -2395,6 +2413,8 @@ impl Interpreter { fn lookup_function_by_name(&self, path: &str) -> Option<(&Vec>, &Ref)> { let mut path = path.to_owned(); if !path.starts_with("data.") { + // `path` is already a canonical Rego path; its first component is an + // identifier, so the separator here cannot flatten a string key. path = format!("{}.{}", self.current_module_path, path); } @@ -2410,33 +2430,25 @@ impl Interpreter { if self.compiled_policy.imports.is_empty() { return None; } - let import_key = format!("{}.{}", self.current_module_path, alias); + let import_key = append_path_component(&self.current_module_path, alias).ok()?; self.compiled_policy.imports.get(&import_key) } - /// Look up the dot-joined target path of an import of the current module - /// with the given alias, e.g. `data.a.b` for `b` after `import data.a.b`. - fn lookup_import_alias(&self, alias: &str) -> Option { - get_path_string(self.lookup_import(alias)?, None).ok() - } - /// Rewrite a path whose leading component is an import alias of the /// current module to the import's target, e.g. `b.f` to `data.a.b.f` /// after `import data.a.b`. fn rewrite_path_through_imports(&self, path: &str) -> Option { - if path.starts_with("data.") { + if self.compiled_policy.imports.is_empty() { return None; } - - let (alias, rest) = match path.split_once('.') { - Some((alias, rest)) => (alias, Some(rest)), - None => (path, None), - }; - let target = self.lookup_import_alias(alias)?; - Some(match rest { - Some(rest) => format!("{target}.{rest}"), - None => target, - }) + let (alias, suffix) = split_canonical_path_root(path)?; + if alias == "data" { + return None; + } + let import = self.lookup_import(alias)?; + let mut target_path = get_path_string(import.as_ref(), None).ok()?; + target_path.push_str(suffix); + Some(target_path) } /// Rewrite an import-aliased call path to its target, e.g. `b.f(1)` to @@ -3003,26 +3015,25 @@ impl Interpreter { None } - fn ensure_module_evaluated(&mut self, path: String) -> Result<()> { + fn ensure_module_evaluated(&mut self, path: &[String]) -> Result<()> { self.check_execution_time()?; for module in self.compiled_policy.modules.clone().iter().cloned() { if Some(&module) == self.module.as_ref() { // Prevent cyclic evaluation. continue; } - let module_path_str = get_path_string(&module.package.refr, Some("data"))?; - let has_dot_after_prefix = module_path_str - .get(path.len()..) - .is_some_and(|suffix| suffix.starts_with('.')); + let mut module_path_components = vec!["data".to_string()]; + module_path_components.extend(Parser::get_static_string_path_components( + &module.package.refr, + )?); - if module_path_str.starts_with(&path) - && (module_path_str.len() == path.len() || has_dot_after_prefix) - { + if path.len() > 1 && module_path_components.starts_with(path) { // Ensure that the module is created. - let module_path_components = Parser::get_path_ref_components(&module.package.refr)?; - let module_path_components: Vec<&str> = - module_path_components.iter().map(|s| s.text()).collect(); - let vref = Self::make_or_get_value_mut(&mut self.data, &module_path_components)?; + let module_value_components = + Parser::get_static_string_path_components(&module.package.refr)?; + let module_value_components: Vec<&str> = + module_value_components.iter().map(String::as_str).collect(); + let vref = Self::make_or_get_value_mut(&mut self.data, &module_value_components)?; if *vref == Value::Undefined { *vref = Value::new_object(); } @@ -3040,7 +3051,7 @@ impl Interpreter { } } self.set_current_module(prev_module)?; - self.mark_processed(&module_path_components)?; + self.mark_processed(&module_value_components)?; } } @@ -3074,10 +3085,17 @@ impl Interpreter { } if matched { - let comps: Vec<&str> = path.split('.').collect(); - if let Some((_, tail)) = comps.split_first() { - self.mark_processed(tail)?; - } + let components = self + .compiled_policy + .rule_path_components + .get(&path) + .cloned() + .ok_or_else(|| anyhow!("missing components for registered rule path {path}"))?; + let comps: Vec<&str> = components.iter().map(String::as_str).collect(); + let (_, tail) = comps + .split_first() + .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; + self.mark_processed(tail)?; } Ok(()) } @@ -3147,12 +3165,17 @@ impl Interpreter { // 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())?; + let requested_path: Vec = core::iter::once("data".to_string()) + .chain(fields.iter().map(|field| (*field).to_string())) + .collect(); + self.ensure_module_evaluated(&requested_path)?; for i in (1..=fields.len()).rev() { let prefix = fields.iter().take(i).copied().collect::>(); - let prefix_path = format!("data.{}", prefix.join(".")); + let prefix_components = core::iter::once("data") + .chain(prefix.iter().copied()) + .collect::>(); + let prefix_path = format_string_path(&prefix_components)?; if self.compiled_policy.rules.contains_key(&prefix_path) || self .compiled_policy @@ -3167,17 +3190,22 @@ impl Interpreter { Ok(Self::get_value_chained(self.data.clone(), 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)?; - let mut module_var_path: Vec<&str> = parsed_path.iter().map(|s| s.text()).collect(); - module_var_path.push(name.text()); - - if self.is_processed(&module_var_path)? { - let value = Self::get_value_chained(self.data.clone(), &module_var_path); + let mut module_var_path = + Parser::get_static_string_path_components(&module.package.refr)?; + module_var_path.push(name.text().to_string()); + let module_var_path_refs: Vec<&str> = + module_var_path.iter().map(String::as_str).collect(); + + if self.is_processed(&module_var_path_refs)? { + let value = Self::get_value_chained(self.data.clone(), &module_var_path_refs); return Ok(Self::get_value_chained(value, fields)); } // Ensure that all the rules having common prefix (name) are evaluated. - let rule_path = format!("data.{}", module_var_path.join(".")); + let rule_path_components = core::iter::once("data") + .chain(module_var_path.iter().map(String::as_str)) + .collect::>(); + let rule_path = format_string_path(&rule_path_components)?; if !no_error && !self.compiled_policy.rules.contains_key(&rule_path) @@ -3197,11 +3225,10 @@ impl Interpreter { let mut found = false; for i in (0..=fields.len()).rev() { let comps = fields.iter().take(i).copied().collect::>(); - let path = if comps.is_empty() { - rule_path.clone() - } else { - format!("{}.{}", rule_path, comps.join(".")) - }; + let mut path = rule_path.clone(); + for component in comps { + path = append_path_component(&path, component)?; + } if self.compiled_policy.rules.contains_key(&path) || self.compiled_policy.default_rules.contains_key(&path) @@ -3221,7 +3248,7 @@ impl Interpreter { } } - let value = Self::get_value_chained(self.data.clone(), &module_var_path[..]); + let value = Self::get_value_chained(self.data.clone(), &module_var_path_refs[..]); Ok(Self::get_value_chained(value, fields)) } else { Ok(Value::Undefined) @@ -3589,51 +3616,13 @@ impl Interpreter { } } - pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { - let mut comps = vec![]; - let mut expr_opt = Some(refr); - while let Some(expr) = expr_opt { - match expr { - Expr::RefDot { - refr: nested_refr, - field, - .. - } => { - comps.push(field.0.text()); - expr_opt = Some(nested_refr); - } - Expr::RefBrack { - refr: nested_refr, - index, - .. - } if matches!(index.as_ref(), Expr::String { .. }) => { - if let Expr::String { span: s, .. } = index.as_ref() { - comps.push(s.text()); - expr_opt = Some(nested_refr); - } - } - Expr::Var { span: v, .. } => { - comps.push(v.text()); - expr_opt = None; - } - _ => bail!(expr.span().error("invalid ref expression")), - } - } - if let Some(doc_component) = document { - comps.push(doc_component); - }; - comps.reverse(); - Ok(comps.join(".")) - } - pub fn set_current_module( &mut self, module: Option>, ) -> Result>> { let previous_module = self.module.clone(); if let Some(new_module) = &module { - self.current_module_path = - Self::get_path_string(&new_module.package.refr, Some("data"))?; + self.current_module_path = get_path_string(&new_module.package.refr, Some("data"))?; self.current_module_index = self.find_module_index(new_module); } self.module = module; @@ -3753,7 +3742,7 @@ impl Interpreter { let scopes = core::mem::take(&mut self.scopes); let module = self.current_module()?; - let mut path = Parser::get_path_ref_components(&module.package.refr)?; + let mut path = get_rule_path_components(&module.package.refr)?; let (refr, index) = match refr.as_ref() { Expr::RefBrack { refr, index, .. } => (refr, Some(index.clone())), @@ -3764,8 +3753,8 @@ impl Interpreter { .error(&format!("invalid token {refr:?} with the default keyword"))), }; - Parser::get_path_ref_components_into(refr, &mut path)?; - let paths: Vec<&str> = path.iter().map(|s| s.text()).collect(); + path.extend(get_rule_path_components(refr)?); + let paths: Vec<&str> = path.iter().map(PathComponent::value).collect(); Self::check_default_value(value)?; let value = self.eval_expr(value)?; @@ -3821,8 +3810,24 @@ impl Interpreter { self.set_current_module(prev_module)?; if result.is_ok() { - let components: Vec<&str> = rule_path.split('.').skip(1).collect(); - let value = Self::get_value_chained(self.data.clone(), &components); + let registered_components = self + .compiled_policy + .rule_path_components + .get(rule_path) + .ok_or_else(|| { + anyhow!( + "missing components for registered rule path {rule_path}" + ) + })?; + let (root, tail) = + registered_components.split_first().ok_or_else(|| { + anyhow!("internal error: expected rule path components") + })?; + if root != "data" { + bail!("internal error: rule path must start with data"); + } + let value_path: Vec<&str> = tail.iter().map(String::as_str).collect(); + let value = Self::get_value_chained(self.data.clone(), &value_path); if value != Value::Undefined { return Ok(value); @@ -3959,11 +3964,11 @@ impl Interpreter { RuleHead::Func { refr, args, assign, .. } => { + let current_module = self.current_module()?; let mut path = - Parser::get_path_ref_components(&self.current_module()?.package.refr)?; - - Parser::get_path_ref_components_into(refr, &mut path)?; - let path: Vec<&str> = path.iter().map(|s| s.text()).collect(); + Self::get_rule_path_components(¤t_module.package.refr)?; + path.extend(Self::get_rule_path_components(refr)?); + let path: Vec<&str> = path.iter().map(|s| s.as_ref()).collect(); // Ensure that for functions with a nesting level (e.g: a.foo), // `a` is created as an empty object. @@ -4188,8 +4193,8 @@ impl Interpreter { break; } Expr::RefBrack { refr, index, .. } => { - if let Expr::String { span: s, .. } = index.as_ref() { - components.push(s.text().into()); + if let Expr::String { value, .. } = index.as_ref() { + components.push(value.as_string()?.clone()); } else { components.clear(); } @@ -4249,25 +4254,32 @@ impl Interpreter { Ok(()) } - fn record_rule(&mut self, refr: &Ref, rule: Ref) -> Result<()> { - let comps = Parser::get_path_ref_components(refr)?; - let comps: Vec<&str> = comps.iter().map(|s| s.text()).collect(); - for (c, _) in comps.iter().enumerate() { - let path = format!( - "{}.{}", - self.current_module_path, - comps - .iter() - .take(c.saturating_add(1)) - .copied() - .collect::>() - .join(".") - ); - if c == comps.len().saturating_sub(1) { - self.compiled_policy_mut().rule_paths.insert(path.clone()); - } + fn current_module_data_path_components(&self) -> Result> { + let module = self.current_module()?; + let mut components = vec![PathComponent::String("data".to_string())]; + components.extend(get_rule_path_components(&module.package.refr)?); + Ok(components) + } - match self.compiled_policy_mut().rules.entry(path) { + fn record_rule(&mut self, refr: &Ref, rule: Ref) -> Result<()> { + let rule_components = get_rule_path_components(refr)?; + let mut path_components = self.current_module_data_path_components()?; + for (index, component) in rule_components.iter().enumerate() { + path_components.push(component.clone()); + let path = format_path_components(&path_components)?; + let string_components = path_components + .iter() + .map(|path_component| path_component.value().to_string()) + .collect(); + let compiled_policy = self.compiled_policy_mut(); + compiled_policy + .rule_path_components + .insert(path.clone(), string_components); + if index == rule_components.len().saturating_sub(1) { + compiled_policy.rule_paths.insert(path.clone()); + } + + match compiled_policy.rules.entry(path) { MapEntry::Occupied(o) => { o.into_mut().push(rule.clone()); } @@ -4286,26 +4298,26 @@ impl Interpreter { rule: &Ref, index: Option, ) -> Result<()> { - let comps = Parser::get_path_ref_components(refr)?; - let comps: Vec<&str> = comps.iter().map(|s| s.text()).collect(); - for (idx, comp_idx) in (0..comps.len()).enumerate() { - let path = format!( - "{}.{}", - self.current_module_path, - comps - .iter() - .take(comp_idx.saturating_add(1)) - .copied() - .collect::>() - .join(".") - ); - if comp_idx == comps.len().saturating_sub(1) { - self.compiled_policy_mut().rule_paths.insert(path.clone()); - } - - match self.compiled_policy_mut().default_rules.entry(path) { + let rule_components = get_rule_path_components(refr)?; + let mut path_components = self.current_module_data_path_components()?; + for (idx, component) in rule_components.iter().enumerate() { + path_components.push(component.clone()); + let path = format_path_components(&path_components)?; + let string_components = path_components + .iter() + .map(|path_component| path_component.value().to_string()) + .collect(); + let compiled_policy = self.compiled_policy_mut(); + compiled_policy + .rule_path_components + .insert(path.clone(), string_components); + if idx == rule_components.len().saturating_sub(1) { + compiled_policy.rule_paths.insert(path.clone()); + } + + match compiled_policy.default_rules.entry(path) { MapEntry::Occupied(o) => { - if idx == comps.len().saturating_sub(1) { + if idx == rule_components.len().saturating_sub(1) { for (_, i) in o.get() { if let (Some(old), Some(new)) = (i, &index) { if old == new { @@ -4332,12 +4344,12 @@ impl Interpreter { let module_path = get_path_string(&module.package.refr, Some("data"))?; for import in &module.imports { let target = match &import.r#as { - Some(s) => s.text(), + Some(s) => s.text().to_string(), _ => match import.refr.as_ref() { - Expr::RefDot { field, .. } => field.0.text(), + Expr::RefDot { field, .. } => field.0.text().to_string(), Expr::RefBrack { index, .. } => match index.as_ref() { - Expr::String { span: s, .. } => s.text(), - _ => "", + Expr::String { value, .. } => value.as_string()?.as_ref().to_string(), + _ => String::new(), }, Expr::Var { span: v, .. } if v.text() == "input" => { // Warn redundant import of input. Ignore it. @@ -4351,7 +4363,7 @@ impl Interpreter { ); continue; } - _ => "", + _ => String::new(), }, }; if target.is_empty() { @@ -4360,9 +4372,10 @@ impl Interpreter { .span() .message("warning", "invalid ref in import")); } + let import_key = append_path_component(&module_path, &target)?; self.compiled_policy_mut() .imports - .insert(format!("{}.{}", module_path, target), import.refr.clone()); + .insert(import_key, import.refr.clone()); } } Ok(()) @@ -4570,19 +4583,42 @@ impl Interpreter { Ok(core::mem::take(&mut self.prints)) } - pub fn eval_rule_in_path(&mut self, path: String) -> Result { - self.check_execution_time()?; - if !self.compiled_policy.rule_paths.contains(&path) { + fn canonical_rule_path(&self, path: &str) -> Result { + if self.compiled_policy.rule_paths.contains(path) { + return Ok(path.to_string()); + } + + let requested_components = Parser::parse_static_path_components(path) + .map_err(|_| anyhow!("not a valid rule path"))?; + let component_refs: Vec<&str> = requested_components.iter().map(String::as_str).collect(); + let canonical_path = + format_string_path(&component_refs).map_err(|_| anyhow!("not a valid rule path"))?; + if !self.compiled_policy.rule_paths.contains(&canonical_path) { bail!("not a valid rule path"); } - self.ensure_rule_evaluated(path.clone())?; - let parts: Vec<&str> = path.split('.').collect(); + Ok(canonical_path) + } - let (_, tail) = parts + pub fn eval_rule_in_path(&mut self, path: String) -> Result { + self.check_execution_time()?; + let canonical_path = self.canonical_rule_path(&path)?; + self.ensure_rule_evaluated(canonical_path.clone())?; + let registered_components = self + .compiled_policy + .rule_path_components + .get(&canonical_path) + .ok_or_else(|| { + anyhow!("missing components for registered rule path {canonical_path}") + })?; + let (root, tail) = registered_components .split_first() - .ok_or_else(|| anyhow!("internal error: expected rule path"))?; + .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; + if root != "data" { + bail!("internal error: rule path must start with data"); + } + let tail: Vec<&str> = tail.iter().map(String::as_str).collect(); - let value = Self::get_value_chained(self.data.clone(), tail); + let value = Self::get_value_chained(self.data.clone(), &tail); #[cfg(feature = "azure_policy")] { if let Some(target_info) = &self.compiled_policy.target_info { @@ -4598,22 +4634,23 @@ impl Interpreter { pub fn compile(&mut self, rule: Option>) -> Result> { let data = Some(self.init_data.clone()); let extensions = self.extensions.clone(); - let compiled_policy = self.compiled_policy_mut(); - - compiled_policy.data = data; - compiled_policy.extensions = extensions; + { + let compiled_policy = self.compiled_policy_mut(); + compiled_policy.data = data; + compiled_policy.extensions = extensions; + } if let Some(rule) = rule { - if !compiled_policy.rule_paths.contains(rule.as_ref()) { - bail!("not a valid rule path"); - } - compiled_policy.rule_to_evaluate = rule; + let canonical_path = self.canonical_rule_path(rule.as_ref())?; + let compiled_policy = self.compiled_policy_mut(); + compiled_policy.rule_to_evaluate = canonical_path.into(); } else { - compiled_policy.rule_to_evaluate = "".into(); + self.compiled_policy_mut().rule_to_evaluate = "".into(); } // Populate loop hoisting lookup table use crate::compiler::hoist::LoopHoister; // Re-run hoisting with the analyzer's schedule so statement order is preserved. + let compiled_policy = self.compiled_policy_mut(); let hoister = compiled_policy .schedule .clone() diff --git a/src/interpreter/target/resolve.rs b/src/interpreter/target/resolve.rs index fc3750c8b..70181fe13 100644 --- a/src/interpreter/target/resolve.rs +++ b/src/interpreter/target/resolve.rs @@ -6,6 +6,7 @@ use super::super::error::TargetCompileError; #[cfg(feature = "azure_policy")] use super::super::TargetInfo; use super::super::*; +use crate::parser::Parser; fn format_effect_names(names: &[String]) -> String { match names.len() { @@ -22,6 +23,23 @@ fn format_effect_names(names: &[String]) -> String { } } +fn target_effect_path( + package: &str, + effect_name: &str, + target_name: &str, +) -> Result<(Vec, String), TargetCompileError> { + let mut components = vec!["data".to_string()]; + components.extend( + Parser::parse_static_path_components(package) + .map_err(|_| TargetCompileError::TargetNotFound(target_name.into()))?, + ); + components.push(effect_name.to_string()); + let refs: Vec<&str> = components.iter().map(String::as_str).collect(); + let path = crate::utils::format_string_path(&refs) + .map_err(|_| TargetCompileError::TargetNotFound(target_name.into()))?; + Ok((components, path)) +} + pub fn resolve_target(interpreter: &mut Interpreter) -> Result<(), TargetCompileError> { use crate::registry::targets; @@ -32,7 +50,7 @@ pub fn resolve_target(interpreter: &mut Interpreter) -> Result<(), TargetCompile for module in interpreter.compiled_policy.modules.iter() { if let Some(ref module_target) = module.target { // Get the package path for this module - let module_package = Interpreter::get_path_string(&module.package.refr, None) + let module_package = crate::utils::get_path_string(&module.package.refr, None) .map_err(|_| TargetCompileError::TargetNotFound(module_target.clone().into()))?; match &target_name { @@ -125,11 +143,20 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile // For each effect defined in the target, check if rules exist for effect_name in target.effects.keys() { // Rule keys are stored with "data." prefix in CompiledPolicy - let expected_path = format!("data.{}.{}", package, effect_name); + let (expected_components, expected_path) = + target_effect_path(package, effect_name, &target.name)?; // Disallow sub-paths for effects in rules. for rule_path in interpreter.compiled_policy.rules.keys() { - if rule_path.starts_with(&expected_path) && rule_path.len() > expected_path.len() { + let is_sub_path = interpreter + .compiled_policy + .rule_path_components + .get(rule_path) + .is_some_and(|components| { + components.len() > expected_components.len() + && components.starts_with(&expected_components) + }); + if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only // This prevents effect rules from being defined at deeper nested paths let all_effect_names: Vec = @@ -145,7 +172,15 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile // Disallow sub-paths for effects in default_rules. for rule_path in interpreter.compiled_policy.default_rules.keys() { - if rule_path.starts_with(&expected_path) && rule_path.len() > expected_path.len() { + let is_sub_path = interpreter + .compiled_policy + .rule_path_components + .get(rule_path) + .is_some_and(|components| { + components.len() > expected_components.len() + && components.starts_with(&expected_components) + }); + if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only let all_effect_names: Vec = target.effects.keys().map(|k| k.to_string()).collect(); @@ -218,7 +253,7 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile }; // Update the target info with the correct effect schema, name, and path - let expected_path = format!("data.{}.{}", package, effect_name); + let (_, expected_path) = target_effect_path(package, effect_name, &target.name)?; if let Some(ref mut target_info_mut) = interpreter.compiled_policy_mut().target_info { target_info_mut.effect_schema = effect_schema; diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index a9ef6e803..ef93804a7 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -16,7 +16,7 @@ use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; use crate::rvm::instructions::{BuiltinCallParams, FunctionCallParams}; use crate::rvm::Instruction; -use crate::utils::get_path_string; +use crate::utils::{append_path_component, get_path_string, split_canonical_path_root}; use crate::value::Value; use alloc::{ format, @@ -233,20 +233,17 @@ impl<'a> Compiler<'a> { /// aliases unconditionally and rejects calls to a missing target at /// compile time. fn resolve_fcn_path_through_imports(&self, path: &str) -> Option { - if self.policy.inner.imports.is_empty() || path.starts_with("data.") { + if self.policy.inner.imports.is_empty() { return None; } - let (alias, rest) = match path.split_once('.') { - Some((alias, rest)) => (alias, Some(rest)), - None => (path, None), - }; - let import_key = format!("{}.{}", self.current_package, alias); + let (alias, suffix) = split_canonical_path_root(path)?; + if alias == "data" { + return None; + } + let import_key = append_path_component(&self.current_package, alias).ok()?; let import_expr = self.policy.inner.imports.get(&import_key)?; - let target = get_path_string(import_expr, None).ok()?; - let candidate = match rest { - Some(rest) => format!("{target}.{rest}"), - None => target, - }; + let mut candidate = get_path_string(import_expr.as_ref(), None).ok()?; + candidate.push_str(suffix); self.policy .inner .functions diff --git a/src/languages/rego/compiler/references.rs b/src/languages/rego/compiler/references.rs index e5bfc7ad2..ca8d8c5bd 100644 --- a/src/languages/rego/compiler/references.rs +++ b/src/languages/rego/compiler/references.rs @@ -14,8 +14,9 @@ use crate::rvm::instructions::{ ChainedIndexParams, LiteralOrRegister, VirtualDataDocumentLookupParams, }; use crate::rvm::Instruction; +use crate::utils::{append_path_component, format_string_path}; use crate::Value; -use alloc::format; +use alloc::collections::BTreeSet; use alloc::string::{String, ToString}; use alloc::vec; use alloc::vec::Vec; @@ -87,9 +88,15 @@ pub(super) fn parse_reference_chain(expr: &ExprRef) -> Result { Expr::RefBrack { refr, index, .. } => { // Bracket access - check if it's a string literal or dynamic match index.as_ref() { - Expr::String { span, .. } => { + Expr::String { value, .. } => { // String literal - treat as static field - components.push(AccessComponent::Field(span.text().to_string())); + components.push(AccessComponent::Field( + value + .as_string() + .map_err(CompilerError::from)? + .as_ref() + .to_string(), + )); } _ => { // Dynamic expression @@ -159,7 +166,8 @@ impl<'a> Compiler<'a> { // Start from the full path and work backwards for i in (1..static_prefix.len()).rev() { // Start from 1 to skip just "data" - let rule_candidate = static_prefix[0..=i].join("."); + let rule_candidate = + format_string_path(&static_prefix[0..=i]).map_err(CompilerError::from)?; if let Ok(rule_index) = self.get_or_assign_rule_index(&rule_candidate) { // Found a rule match! Call the rule @@ -186,11 +194,12 @@ impl<'a> Compiler<'a> { // Check if this path could be a prefix of any rules (for virtual document lookup) // Convert the full chain to a pattern that includes wildcards for dynamic components let path_pattern = self.create_path_pattern(&chain.components); - let matching_rules: Vec = self + let matching_rules: BTreeSet = self .policy .inner .rules .keys() + .chain(self.policy.inner.default_rules.keys()) .filter(|rule_path| self.matches_path_pattern(rule_path, &path_pattern)) .cloned() .collect(); @@ -221,54 +230,38 @@ impl<'a> Compiler<'a> { /// Create a path pattern from access components, using '*' for dynamic components /// e.g., [Field("a"), Expression(...), Field("b")] becomes "data.a.*.b" - fn create_path_pattern(&self, components: &[AccessComponent]) -> String { - let mut pattern_parts = vec!["data"]; + fn create_path_pattern(&self, components: &[AccessComponent]) -> Vec> { + let mut pattern_parts = vec![Some("data".to_string())]; for component in components { match component { - AccessComponent::Field(field) => pattern_parts.push(field.as_str()), - AccessComponent::Expression(_) => pattern_parts.push("*"), + AccessComponent::Field(field) => pattern_parts.push(Some(field.clone())), + AccessComponent::Expression(_) => pattern_parts.push(None), } } - pattern_parts.join(".") + pattern_parts } /// Check if a rule path matches the given pattern with wildcards /// e.g., "data.test.users.alice_profile" matches "data.test.users.*" - fn matches_path_pattern(&self, rule_path: &str, pattern: &str) -> bool { - // Use simple string matching implementation that handles wildcard patterns - if pattern.contains('*') { - self.simple_wildcard_match(rule_path, pattern) - } else { - // Simple prefix match for patterns without wildcards - rule_path.starts_with(&format!("{}.", pattern)) || rule_path == pattern - } - } - - /// Simple wildcard matching without regex dependencies - /// Checks if a rule path could be a prefix of the access pattern - /// e.g., rule "data.test.users.alice_data" matches pattern "data.*.*.*.*.* because - /// the rule could be accessed with the first 4 components of the pattern - /// Ensures exact component matching - "fee" will NOT match "feed" - fn simple_wildcard_match(&self, rule_path: &str, access_pattern: &str) -> bool { - let rule_parts: Vec<&str> = rule_path.split('.').collect(); - let pattern_parts: Vec<&str> = access_pattern.split('.').collect(); - + fn matches_path_pattern(&self, rule_path: &str, pattern: &[Option]) -> bool { + let Some(rule_parts) = self.policy.inner.rule_path_components.get(rule_path) else { + return false; + }; // Check how many components of the pattern the rule can match - let match_length = rule_parts.len().min(pattern_parts.len()); + let match_length = rule_parts.len().min(pattern.len()); // Check if the rule matches the pattern up to the available components for i in 0..match_length { - let rule_part = rule_parts[i]; - let pattern_part = pattern_parts[i]; - - if pattern_part == "*" { + let rule_part = &rule_parts[i]; + let pattern_part = &pattern[i]; + if pattern_part.is_none() { // Wildcard in pattern matches any non-empty rule component exactly if rule_part.is_empty() { return false; } - } else if rule_part != pattern_part { + } else if pattern_part.as_ref() != Some(rule_part) { return false; } } @@ -276,11 +269,8 @@ impl<'a> Compiler<'a> { // Rule matches if either: // 1. It's at least as long as the pattern, OR // 2. It matches all available components and the remaining pattern parts are wildcards - rule_parts.len() >= pattern_parts.len() - || (match_length > 0 - && pattern_parts[match_length..] - .iter() - .all(|&part| part == "*")) + rule_parts.len() >= pattern.len() + || (match_length > 0 && pattern[match_length..].iter().all(Option::is_none)) } /// Compile local variable access chain @@ -299,20 +289,25 @@ impl<'a> Compiler<'a> { } // Check if there's a rule in the current package that matches - let current_pkg_prefix = format!("{}.{}", &self.current_package, root); + let current_pkg_prefix = + append_path_component(&self.current_package, root).map_err(CompilerError::from)?; // Build static path for rule matching - let mut rule_path_parts = vec![current_pkg_prefix.as_str()]; + let mut static_fields = Vec::new(); for component in &chain.components { match component { - AccessComponent::Field(field) => rule_path_parts.push(field.as_str()), + AccessComponent::Field(field) => static_fields.push(field.as_str()), AccessComponent::Expression(_) => break, // Stop at first dynamic component } } // Try to find the longest matching rule prefix - for i in (0..rule_path_parts.len()).rev() { - let rule_candidate = rule_path_parts[0..=i].join("."); + for i in (0..=static_fields.len()).rev() { + let mut rule_candidate = current_pkg_prefix.clone(); + for field in static_fields.iter().take(i) { + rule_candidate = + append_path_component(&rule_candidate, field).map_err(CompilerError::from)?; + } if let Ok(rule_index) = self.get_or_assign_rule_index(&rule_candidate) { let rule_result_reg = self.alloc_register(); @@ -336,7 +331,8 @@ impl<'a> Compiler<'a> { } // No rule found; fall back to module-level imports. - let import_key = format!("{}.{}", &self.current_package, root); + let import_key = + append_path_component(&self.current_package, root).map_err(CompilerError::from)?; if let Some(import_expr) = self.policy.inner.imports.get(&import_key) { let import_reg = self.compile_rego_expr_with_span(import_expr, import_expr.span(), false)?; diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index e18f4b720..b59d98c76 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -14,9 +14,10 @@ use super::{CompilationContext, Compiler, CompilerError, ContextType, Result, Wo use crate::ast::{Expr, ExprRef, Rule, RuleHead}; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; +use crate::parser::Parser; use crate::rvm::program::{Program, RuleType}; use crate::rvm::Instruction; -use crate::utils::get_path_string; +use crate::utils::{format_string_path, get_path_string}; use crate::Map; use crate::{CompiledPolicy, Value}; use alloc::collections::BTreeSet; @@ -222,15 +223,33 @@ impl<'a> Compiler<'a> { } } - let package = if let Some(last_dot) = rule_path.rfind('.') { - rule_path[..last_dot].to_string() - } else { - "data".to_string() - }; + let package_components = self + .policy + .inner + .rule_path_components + .get(rule_path) + .ok_or_else(|| CompilerError::General { + message: format!("missing components for registered rule path '{rule_path}'"), + })?; + let (_, package_parts) = + package_components + .split_last() + .ok_or_else(|| CompilerError::General { + message: format!("invalid registered rule path '{rule_path}'"), + })?; + let package_refs: Vec<&str> = package_parts.iter().map(String::as_str).collect(); + let package = + format_string_path(&package_refs).map_err(|error| CompilerError::General { + message: format!("failed to format package path for '{rule_path}': {error}"), + })?; Ok((package, 0)) } - /// Compile from a CompiledPolicy to RVM Program + /// Compile from a CompiledPolicy to RVM Program. + /// + /// Entry-point names are canonical Rego rule paths. Use bracketed string + /// components for literal keys that are not identifiers, such as + /// `data.graph.defUniqueName["1.0.0"].deny`. pub fn compile_from_policy( policy: &CompiledPolicy, entry_points: &[&str], @@ -254,7 +273,27 @@ impl<'a> Compiler<'a> { for &entry_point_name in entry_points { let instruction_index = compiler.program.instructions.len(); let result_reg = compiler.alloc_register(); - let rule_idx = compiler.get_or_assign_rule_index(entry_point_name)?; + let rule_path = + if policy.inner.rule_paths.contains(entry_point_name) { + entry_point_name.to_string() + } else { + let components = Parser::parse_static_path_components(entry_point_name) + .map_err(|error| CompilerError::General { + message: error.to_string(), + })?; + let component_refs: Vec<&str> = components.iter().map(String::as_str).collect(); + let canonical_path = format_string_path(&component_refs).map_err(|error| { + CompilerError::General { + message: error.to_string(), + } + })?; + if policy.inner.rule_paths.contains(&canonical_path) { + canonical_path + } else { + entry_point_name.to_string() + } + }; + let rule_idx = compiler.get_or_assign_rule_index(&rule_path)?; compiler .entry_points .insert(entry_point_name.to_string(), instruction_index); @@ -689,17 +728,26 @@ impl<'a> Compiler<'a> { self.rule_function_param_count[rule_index as usize] = rule_param_count; if rule_param_count.is_none() { - let rule_path_parts: Vec<&str> = rule_path.split('.').collect(); - if let Some((rule_name, package_parts)) = rule_path_parts.split_last() { - let package_path: Vec = - package_parts.iter().map(|s| s.to_string()).collect(); - - let _ = self.program.add_rule_to_tree( - &package_path, - rule_name, - rule_index as usize, - ); - } + let components = self + .policy + .inner + .rule_path_components + .get(rule_path) + .ok_or_else(|| CompilerError::General { + message: format!( + "missing components for registered rule path '{rule_path}'" + ), + })?; + let (rule_name, package_parts) = + components + .split_last() + .ok_or_else(|| CompilerError::General { + message: format!("invalid registered rule path '{rule_path}'"), + })?; + let package_path = package_parts.to_vec(); + let _ = + self.program + .add_rule_to_tree(&package_path, rule_name, rule_index as usize); } self.register_counter = saved_register_counter; @@ -714,17 +762,26 @@ impl<'a> Compiler<'a> { } // Add the rule to the data tree so it is discoverable. - let rule_path_parts: Vec<&str> = rule_path.split('.').collect(); - if let Some((rule_name, package_parts)) = rule_path_parts.split_last() { - let package_path: Vec = - package_parts.iter().map(|s| s.to_string()).collect(); - - let _ = self.program.add_rule_to_tree( - &package_path, - rule_name, - rule_index as usize, - ); - } + let components = self + .policy + .inner + .rule_path_components + .get(rule_path) + .ok_or_else(|| CompilerError::General { + message: format!( + "missing components for registered rule path '{rule_path}'" + ), + })?; + let (rule_name, package_parts) = + components + .split_last() + .ok_or_else(|| CompilerError::General { + message: format!("invalid registered rule path '{rule_path}'"), + })?; + let package_path = package_parts.to_vec(); + let _ = + self.program + .add_rule_to_tree(&package_path, rule_name, rule_index as usize); } self.register_counter = saved_register_counter; diff --git a/src/parser.rs b/src/parser.rs index 8f7f73ee7..6d07027b5 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -59,7 +59,14 @@ const DEFAULT_MAX_EXPR_DEPTH: usize = 32; impl<'source> Parser<'source> { pub fn new(source: &'source Source) -> Result { + Self::new_with_max_col(source, None) + } + + fn new_with_max_col(source: &'source Source, max_col: Option) -> Result { let mut lexer = Lexer::new(source); + if let Some(max_col) = max_col { + lexer.set_max_col(max_col); + } let tok = lexer.next_token()?; Ok(Self { source: source.clone(), @@ -236,6 +243,48 @@ impl<'source> Parser<'source> { Ok(comps) } + pub(crate) fn get_static_string_path_components(refr: &Expr) -> Result> { + fn collect(refr: &Expr, components: &mut Vec) -> Result<()> { + match refr { + Expr::Var { span, .. } => components.push(span.text().to_string()), + Expr::RefDot { refr, field, .. } => { + collect(refr, components)?; + components.push(field.0.text().to_string()); + } + Expr::RefBrack { refr, index, .. } => { + collect(refr, components)?; + if let Expr::String { value, .. } = index.as_ref() { + components.push(value.as_string()?.as_ref().to_string()); + } else { + bail!(refr.span().error("not a static string path")); + } + } + _ => bail!(refr.span().error("not a static string path")), + } + Ok(()) + } + + let mut components = Vec::new(); + collect(refr, &mut components)?; + Ok(components) + } + + pub(crate) fn parse_static_path_components(path: &str) -> Result> { + let max_col = path + .len() + .checked_add(1) + .and_then(|max_col| u32::try_from(max_col).ok()) + .and_then(NonZeroU32::new) + .ok_or_else(|| anyhow!("path exceeds maximum supported length"))?; + let source = Source::from_contents("entrypoint.rego".to_string(), path.to_string())?; + let mut parser = Parser::<'_>::new_with_max_col(&source, Some(max_col))?; + let expression = parser.parse_path_ref()?; + if parser.tok.0 != TokenKind::Eof { + bail!(parser.tok.1.error("expecting EOF")); + } + Self::get_static_string_path_components(&expression) + } + fn handle_import_future_keywords(&mut self, comps: &[Span]) -> Result { if comps.len() >= 2 && comps[0].text() == "future" && comps[1].text() == "keywords" { match comps.len().saturating_sub(2) { @@ -1473,14 +1522,7 @@ impl<'source> Parser<'source> { "[" => { self.next_token()?; let index = match &self.tok.0 { - TokenKind::String => { - let (span, value) = Self::span_and_value(self.tok.1.clone()); - Expr::String { - span, - value, - eidx: self.next_eidx(), - } - } + TokenKind::String => self.parse_scalar_or_var()?, _ => { return Err(self.source.error( self.tok.1.line, @@ -1489,7 +1531,6 @@ impl<'source> Parser<'source> { )); } }; - self.next_token()?; self.expect("]", "while parsing bracketed reference")?; span.end = self.end; refr = Expr::RefBrack { diff --git a/src/rvm/vm/virtual_data.rs b/src/rvm/vm/virtual_data.rs index 12125091d..6356b7245 100644 --- a/src/rvm/vm/virtual_data.rs +++ b/src/rvm/vm/virtual_data.rs @@ -3,6 +3,7 @@ use crate::rvm::instructions::LiteralOrRegister; use crate::value::Value; +use alloc::string::ToString as _; use alloc::vec::Vec; use core::convert::TryFrom as _; @@ -13,23 +14,27 @@ impl RegoVM { pub(super) fn execute_virtual_data_document_lookup_subobject( &mut self, path_components: &[LiteralOrRegister], + string_fallback_indices: &[usize], + data_subobject: Value, rule_tree_subobject: &Value, ) -> Result { let mut root_path = Vec::new(); - for component in path_components { - let key_value = self.literal_or_register_value(component)?; + let mut fallback_indices = string_fallback_indices.iter().copied().peekable(); + for (index, component) in path_components.iter().enumerate() { + let mut key_value = self.literal_or_register_value(component)?; + if fallback_indices + .peek() + .is_some_and(|fallback_index| *fallback_index == index) + { + let _ = fallback_indices.next(); + key_value = Value::from(key_value.to_string()); + } root_path.push(key_value); } - // Walk data tree by reference, only clone the leaf. - let mut data_ref = &self.data; - for path_component in &root_path { - data_ref = &data_ref[path_component]; - } - - let mut result_subobject = match *data_ref { + let mut result_subobject = match data_subobject { Value::Undefined => Value::new_object(), - _ => data_ref.clone(), + subobject => subobject, }; self.traverse_rule_tree_subobject(rule_tree_subobject, &mut result_subobject, &root_path)?; @@ -215,12 +220,26 @@ impl RegoVM { .clone(); let mut current_node = &self.program.rule_tree["data"]; + let mut data_ref = &self.data; + let mut string_fallback_indices = Vec::new(); let mut components_consumed = 0; for (i, component) in params.path_components.iter().enumerate() { - let key_value = self.literal_or_register_value(component)?; + let mut key_value = self.literal_or_register_value(component)?; - current_node = ¤t_node[&key_value]; + let mut rule_child = ¤t_node[&key_value]; + let mut data_child = &data_ref[&key_value]; + if *rule_child == Value::Undefined + && *data_child == Value::Undefined + && matches!(key_value, Value::Number(_)) + { + string_fallback_indices.push(i); + key_value = Value::from(key_value.to_string()); + rule_child = ¤t_node[&key_value]; + data_child = &data_ref[&key_value]; + } + current_node = rule_child; + data_ref = data_child; components_consumed = self.checked_add_one(i, "path components traversed")?; match *current_node { @@ -259,25 +278,43 @@ impl RegoVM { }); } } - Value::Undefined | Value::Object(_) - if components_consumed != params.path_components.len() => - { + Value::Undefined => { + let mut result_ref = data_ref; + for component in params.path_components.iter().skip(components_consumed) { + let key_value = self.literal_or_register_value(component)?; + let mut child = &result_ref[&key_value]; + if *child == Value::Undefined && matches!(key_value, Value::Number(_)) { + let string_key = Value::from(key_value.to_string()); + child = &result_ref[&string_key]; + } + result_ref = child; + } + self.set_register(params.dest, result_ref.clone())?; + } + Value::Object(_) if components_consumed != params.path_components.len() => { // Walk data tree by reference, clone only the leaf. - let mut data_ref = &self.data; + let mut result_data_ref = &self.data; for component in ¶ms.path_components { let key_value = self.literal_or_register_value(component)?; - data_ref = &data_ref[&key_value]; + let mut child = &result_data_ref[&key_value]; + if *child == Value::Undefined && matches!(key_value, Value::Number(_)) { + let string_key = Value::from(key_value.to_string()); + child = &result_data_ref[&string_key]; + } + result_data_ref = child; } - let result = data_ref.clone(); + let result = result_data_ref.clone(); self.set_register(params.dest, result)?; } Value::Object(_) => { let rule_tree_subobject = current_node.clone(); - + let data_subobject = data_ref.clone(); let result = self.execute_virtual_data_document_lookup_subobject( ¶ms.path_components, + &string_fallback_indices, + data_subobject, &rule_tree_subobject, )?; self.set_register(params.dest, result)?; diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs index 4bdaa7017..fc135edae 100644 --- a/src/tests/interpreter/mod.rs +++ b/src/tests/interpreter/mod.rs @@ -15,6 +15,8 @@ use std::env; use crate::test_utils::{check_output, ValueOrVec}; +#[cfg(feature = "azure_policy")] +use crate::utils::limits::PolicyLengthConfig; use crate::utils::limits::{ acquire_limits_test_lock, fallback_execution_timer_config, ExecutionTimerConfig, }; @@ -439,6 +441,41 @@ mod load_target_definitions { Ok(()) } + + #[test] + fn compile_for_target_accepts_a_long_first_package_component() -> Result<()> { + load()?; + assert!(crate::registry::targets::contains( + "target.tests.azure_policy" + )); + + let package_component = "g".repeat(1300); + let mut engine = Engine::new(); + engine.set_policy_length_config(PolicyLengthConfig { + max_col: NonZeroU32::new(2048).expect("non-zero column limit"), + ..PolicyLengthConfig::default() + }); + engine.add_policy( + "long-target.rego".to_string(), + format!( + r#"package {package_component} +import rego.v1 +__target__ := "target.tests.azure_policy" +default allow := false +allow if {{ + input.type == "Microsoft.Storage/storageAccounts" +}}"# + ), + )?; + + let compiled = engine.compile_for_target()?; + let input = Value::from_json_str( + r#"{"type":"Microsoft.Storage/storageAccounts","name":"long-target","location":"East US"}"#, + )?; + assert_eq!(compiled.eval_with_input(input)?, Value::Bool(true)); + + Ok(()) + } } fn push_query_results(query_results: QueryResults, results: &mut Vec) { diff --git a/src/utils.rs b/src/utils.rs index 37193140a..e47669e91 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -12,39 +12,164 @@ pub mod limits; use crate::ast::*; use crate::builtins::*; -use crate::lexer::*; +use crate::lexer::SourceStr; +use crate::parser::Parser; use crate::*; use alloc::collections::BTreeMap; use anyhow::{bail, Result}; -pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { - let mut comps: Vec<&str> = vec![]; - let mut expr = Some(refr); - while expr.is_some() { - match expr { - Some(Expr::RefDot { refr, field, .. }) => { - comps.push(field.0.text()); - expr = Some(refr); +#[derive(Clone, Debug)] +pub(crate) enum PathComponent { + String(String), + Raw(String), +} + +impl PathComponent { + pub(crate) fn value(&self) -> &str { + match self { + Self::String(value) | Self::Raw(value) => value, + } + } +} + +fn is_identifier_component(component: &str) -> bool { + let mut bytes = component.bytes(); + matches!(bytes.next(), Some(first) if first.is_ascii_alphabetic() || first == b'_') + && bytes.all(|byte| byte.is_ascii_alphanumeric() || byte == b'_') +} + +pub(crate) fn append_path_component(path: &str, component: &str) -> Result { + if is_identifier_component(component) { + if path.is_empty() { + Ok(component.to_string()) + } else { + Ok(format!("{path}.{component}")) + } + } else { + let literal = Value::from(component).to_json_str()?; + if path.is_empty() { + Ok(format!("[{literal}]")) + } else { + Ok(format!("{path}[{literal}]")) + } + } +} + +pub(crate) fn append_path_value_component(path: &str, component: &Value) -> Result { + match component { + Value::String(value) => append_path_component(path, value.as_ref()), + value => { + if path.is_empty() { + Ok(value.to_string()) + } else { + Ok(format!("{path}.{value}")) } - Some(Expr::RefBrack { refr, index, .. }) => { - if let Expr::String { span: s, .. } = index.as_ref() { - comps.push(s.text()); + } + } +} + +pub(crate) fn format_path_components(components: &[PathComponent]) -> Result { + let mut path = String::new(); + for component in components { + path = match component { + PathComponent::String(value) => append_path_component(&path, value)?, + PathComponent::Raw(value) if path.is_empty() => value.clone(), + PathComponent::Raw(value) => format!("{path}.{value}"), + }; + } + Ok(path) +} + +pub(crate) fn format_string_path(components: &[&str]) -> Result { + let components: Vec = components + .iter() + .map(|component| PathComponent::String((*component).to_string())) + .collect(); + format_path_components(&components) +} + +pub(crate) fn split_canonical_path_root(path: &str) -> Option<(&str, &str)> { + let root_end = path.find(['.', '[']).unwrap_or(path.len()); + if root_end == 0 { + return None; + } + Some(path.split_at(root_end)) +} + +pub(crate) fn get_rule_path_components(refr: &Expr) -> Result> { + fn collect(refr: &Expr, components: &mut Vec) -> Result<()> { + match refr { + Expr::Var { span, .. } => { + components.push(PathComponent::String(span.text().to_string())); + } + Expr::RefDot { refr, field, .. } => { + collect(refr, components)?; + components.push(PathComponent::String(field.0.text().to_string())); + } + Expr::RefBrack { refr, index, .. } => { + collect(refr, components)?; + match index.as_ref() { + Expr::String { value, .. } => components.push(PathComponent::String( + value.as_string()?.as_ref().to_string(), + )), + Expr::Number { span, .. } + | Expr::Bool { span, .. } + | Expr::Null { span, .. } => { + components.push(PathComponent::Raw(span.text().to_string())); + } + _ => { + let index_components = Parser::get_path_ref_components(index)?; + components.extend( + index_components + .iter() + .map(|component| PathComponent::Raw(component.text().to_string())), + ); + } } - expr = Some(refr); } - Some(Expr::Var { span: v, .. }) => { - comps.push(v.text()); - expr = None; + _ => bail!("internal error: not a simple ref {refr:?}"), + } + Ok(()) + } + + let mut components = Vec::new(); + collect(refr, &mut components)?; + Ok(components) +} + +pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { + fn collect_simple(refr: &Expr, components: &mut Vec) -> Result<()> { + match refr { + Expr::Var { span, .. } => { + components.push(PathComponent::String(span.text().to_string())); + } + Expr::RefDot { refr, field, .. } => { + collect_simple(refr, components)?; + components.push(PathComponent::String(field.0.text().to_string())); } - _ => bail!("internal error: not a simple ref {expr:?}"), + Expr::RefBrack { refr, index, .. } => { + collect_simple(refr, components)?; + if let Expr::String { value, .. } = index.as_ref() { + components.push(PathComponent::String( + value.as_string()?.as_ref().to_string(), + )); + } + } + _ => bail!("internal error: not a simple ref {refr:?}"), } + Ok(()) } - if let Some(d) = document { - comps.push(d); - }; - comps.reverse(); - Ok(comps.join(".")) + + let mut components = Vec::new(); + collect_simple(refr, &mut components)?; + let mut path = document.unwrap_or_default().to_string(); + for component in components { + if let PathComponent::String(component) = component { + path = append_path_component(&path, &component)?; + } + } + Ok(path) } pub type FunctionTable = BTreeMap>, u8, Ref)>; @@ -127,3 +252,35 @@ pub fn get_root_var(mut expr: &Expr) -> Result { } } } + +#[cfg(test)] +mod tests { + use super::append_path_component; + use alloc::format; + use anyhow::Result; + + #[test] + fn path_component_formatting_matches_rego_identifier_grammar() -> Result<()> { + let cases = [ + ("ordinary_name_9", "data.ordinary_name_9"), + ("if", "data.if"), + ("9ordinary", r#"data["9ordinary"]"#), + ("with-hyphen", r#"data["with-hyphen"]"#), + ("a.b", r#"data["a.b"]"#), + (r#"escaped".dot"#, r#"data["escaped\".dot"]"#), + ("", r#"data[""]"#), + ("éclair", r#"data["éclair"]"#), + ]; + + for (component, expected) in cases { + let actual = append_path_component("data", component)?; + if actual != expected { + return Err(anyhow::Error::msg(format!( + "expected {expected}, got {actual}" + ))); + } + } + + Ok(()) + } +} diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 292bcb9b9..81679c18b 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -4,6 +4,335 @@ use anyhow::{bail, Result}; use regorus::*; +#[test] +fn namespace_literal_dot_rule_path_uses_bracketed_component() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "namespace.rego".to_string(), + r#" + package graph.defUniqueName["1.0.0"] + + default deny := false + deny := true if { input.blocked == true } + "# + .to_string(), + )?; + let path = "data.graph.defUniqueName[\"1.0.0\"].deny"; + let entrypoint: Rc = path.into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + + let inputs = [ + (Value::from_json_str(r#"{"blocked":true}"#)?, true), + (Value::from_json_str(r#"{"blocked":false}"#)?, false), + (Value::new_object(), false), + (Value::Null, false), + (Value::Undefined, false), + ]; + + for (input, expected) in inputs { + engine.set_input(input.clone()); + assert_eq!(engine.eval_rule(path.to_string())?, Value::Bool(expected)); + assert_eq!( + compiled.eval_with_input(input.clone())?, + Value::Bool(expected) + ); + + let query = engine.eval_query(path.to_string(), false)?; + assert_eq!(query.result[0].expressions[0].value, Value::Bool(expected)); + let data = engine.eval_query("data".to_string(), false)?; + let expected_data = Value::from_json_str(&format!( + r#"{{"graph":{{"defUniqueName":{{"1.0.0":{{"deny":{expected}}}}}}}}}"# + ))?; + assert_eq!(data.result[0].expressions[0].value, expected_data); + } + + Ok(()) +} + +#[test] +fn namespace_literal_dot_package_metadata_is_unambiguous() -> Result<()> { + let mut engine = Engine::new(); + let package = engine.add_policy( + "namespace.rego".to_string(), + r#"package graph.defUniqueName["1.0.0"]"#.to_string(), + )?; + + assert_eq!(package, "data.graph.defUniqueName[\"1.0.0\"]"); + assert_eq!( + engine.get_packages()?, + vec!["data.graph.defUniqueName[\"1.0.0\"]"] + ); + + Ok(()) +} + +#[test] +fn namespace_escaped_literal_component_uses_json_path_escaping() -> Result<()> { + let mut engine = Engine::new(); + let package = engine.add_policy( + "escaped.rego".to_string(), + r#"package graph["a\".b"] + value := 1"# + .to_string(), + )?; + + assert_eq!(package, r#"data.graph["a\".b"]"#); + assert_eq!( + engine.eval_rule(r#"data.graph["a\".b"].value"#.to_string())?, + Value::from(1) + ); + + Ok(()) +} + +#[test] +fn namespace_bracketed_identifier_path_is_equivalent_to_dotted_path() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "identifier.rego".to_string(), + "package graph.defUniqueName\nvalue := true".to_string(), + )?; + + assert_eq!( + engine.eval_rule("data.graph[\"defUniqueName\"].value".to_string())?, + Value::Bool(true) + ); + Ok(()) +} + +#[test] +fn namespace_escaped_package_materializes_only_the_decoded_key() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "escaped.rego".to_string(), + r#"package graph["a\".b"] + value := 7"# + .to_string(), + )?; + + assert_eq!( + engine.eval_modules(false)?, + Value::from_json_str(r#"{"graph":{"a\".b":{"value":7}}}"#)? + ); + Ok(()) +} + +#[test] +fn namespace_escaped_function_projection_uses_decoded_package_components() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "escaped.rego".to_string(), + r#"package graph["a\".b"] + value() := 7"# + .to_string(), + )?; + engine.add_policy( + "consumer.rego".to_string(), + r#"package consumer + import data.graph["a\".b"] as escaped + result := escaped.value"# + .to_string(), + )?; + + assert_eq!( + engine.eval_rule("data.consumer.result".to_string())?, + Value::from(7) + ); + Ok(()) +} + +#[test] +fn namespace_escaped_empty_package_materializes_only_the_decoded_key() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "escaped.rego".to_string(), + r#"package graph["a\".b"]"#.to_string(), + )?; + + assert_eq!( + engine.eval_modules(false)?, + Value::from_json_str(r#"{"graph":{"a\".b":{}}}"#)? + ); + Ok(()) +} + +#[test] +fn long_registered_entrypoint_and_bracketed_identifier_compile() -> Result<()> { + let package_component = "g".repeat(987); + let package_path = format!("graph.{package_component}"); + let rule_name = format!("value_{}", "r".repeat(57)); + let canonical_path = format!("data.{package_path}.{rule_name}"); + #[cfg(feature = "rvm")] + let bracketed_path = format!("data[\"graph\"][\"{package_component}\"][\"{rule_name}\"]"); + assert!(canonical_path.len() > 1024); + + let mut engine = Engine::new(); + engine.add_policy( + "long.rego".to_string(), + format!("package {package_path}\n{rule_name} := 7"), + )?; + + let compiled = engine.compile_with_entrypoint(&canonical_path.into())?; + assert_eq!(compiled.eval_with_input(Value::Null)?, Value::from(7)); + + #[cfg(feature = "rvm")] + { + let program = regorus::languages::rego::compiler::Compiler::compile_from_policy( + &compiled, + &[&bracketed_path], + )?; + let mut vm = regorus::rvm::vm::RegoVM::new(); + vm.load_program(program); + assert_eq!( + vm.execute_entry_point_by_name(&bracketed_path)?, + Value::from(7) + ); + assert_eq!(vm.execute_entry_point_by_index(0)?, Value::from(7)); + } + Ok(()) +} + +#[test] +fn wider_policy_column_limit_does_not_restrict_registered_entrypoints() -> Result<()> { + use core::num::NonZeroU32; + + let package_component = "g".repeat(1300); + let package_path = format!("graph.{package_component}"); + let rule_name = format!("value_{}", "r".repeat(57)); + let canonical_path = format!("data.{package_path}.{rule_name}"); + #[cfg(feature = "rvm")] + let bracketed_path = format!("data[\"graph\"][\"{package_component}\"][\"{rule_name}\"]"); + + let mut engine = Engine::new(); + engine.set_policy_length_config(regorus::utils::limits::PolicyLengthConfig { + max_col: NonZeroU32::new(2048).expect("non-zero column limit"), + ..Default::default() + }); + engine.add_policy( + "long.rego".to_string(), + format!("package {package_path}\n{rule_name} := 7"), + )?; + + let compiled = engine.compile_with_entrypoint(&canonical_path.into())?; + assert_eq!(compiled.eval_with_input(Value::Null)?, Value::from(7)); + + #[cfg(feature = "rvm")] + { + let program = regorus::languages::rego::compiler::Compiler::compile_from_policy( + &compiled, + &[&bracketed_path], + )?; + let mut vm = regorus::rvm::vm::RegoVM::new(); + vm.load_program(program); + assert_eq!( + vm.execute_entry_point_by_name(&bracketed_path)?, + Value::from(7) + ); + } + Ok(()) +} + +#[test] +fn namespace_rule_without_default_remains_undefined() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "no-default.rego".to_string(), + r#" + package graph.defUniqueName["1.0.0"] + deny := true if { input.blocked == true } + "# + .to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"blocked":false}"#)?); + + assert_eq!( + engine.eval_rule("data.graph.defUniqueName[\"1.0.0\"].deny".to_string())?, + Value::Undefined + ); + Ok(()) +} + +#[test] +fn namespace_invalid_and_missing_rule_paths_keep_the_existing_error() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "namespace.rego".to_string(), + r#" + package graph["1.0.0"] + deny := true + "# + .to_string(), + )?; + + for path in [ + "data.graph[1.0.0].deny", + "data.graph[\"1.0.1\"].deny", + "data.graph[\"1.0.0\"].deny trailing", + ] { + let error = engine.eval_rule(path.to_string()).unwrap_err(); + assert_eq!(error.to_string(), "not a valid rule path"); + } + Ok(()) +} + +#[test] +fn dynamic_data_root_lookup_does_not_evaluate_unrelated_modules() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := data[input.namespace].value + "# + .to_string(), + )?; + engine.add_policy( + "policy.rego".to_string(), + r#" + package policy + result := data.framework.result + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + r#" + package target + value := 7 + "# + .to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + assert_eq!( + engine.eval_rule("data.framework.result".to_string())?, + Value::from(7) + ); + Ok(()) +} + +#[test] +fn nested_dynamic_data_lookup_does_not_prefetch_a_dotted_suffix_rule() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "dynamic.rego".to_string(), + r#" + package dynamic + foo := {"bar": 7} + bar := data.dynamic.value + value := data.dynamic[input.first][input.second] + "# + .to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"first":"foo","second":"bar"}"#)?); + + assert_eq!( + engine.eval_rule("data.dynamic.value".to_string())?, + Value::from(7) + ); + Ok(()) +} + #[test] fn extension() -> Result<()> { fn repeat(mut params: Vec) -> Result { diff --git a/tests/interpreter/cases/import/tests.yaml b/tests/interpreter/cases/import/tests.yaml index 3efc551c4..964f9092c 100644 --- a/tests/interpreter/cases/import/tests.yaml +++ b/tests/interpreter/cases/import/tests.yaml @@ -113,6 +113,22 @@ cases: query: data.rules.lookup want_result: "value" + - note: import escaped literal namespace and project its zero argument function + modules: + - | + package graph["a\".b"] + import rego.v1 + + value() := 7 + - | + package consumer + import rego.v1 + import data.graph["a\".b"] as escaped + + result := escaped.value + query: data.consumer.result + want_result: 7 + - note: import alias shadowed by rule modules: - | @@ -408,6 +424,38 @@ cases: query: data.rules.main want_result: from: lib2 + - note: literal-dot package imports keep rule and function identities separate + modules: + - | + package graph["a.b"] + import rego.v1 + + default value := "literal default" + suffix() := "literal function" + - | + package graph.a.b + import rego.v1 + + default value := "nested default" + suffix() := "nested function" + - | + package consumer + import rego.v1 + import data.graph["a.b"] as dotted + import data.graph.a.b as nested + + result := { + "dotted": dotted.value, + "nested": nested.value, + "dotted_function": dotted.suffix(), + "nested_function": nested.suffix() + } + query: data.consumer.result + want_result: + dotted: literal default + nested: nested default + dotted_function: literal function + nested_function: nested function - note: with override target aliasing a non-function rule overrides the value modules: - | diff --git a/tests/interpreter/cases/refr/tests.yaml b/tests/interpreter/cases/refr/tests.yaml index c5a48e80d..1ed8b641d 100644 --- a/tests/interpreter/cases/refr/tests.yaml +++ b/tests/interpreter/cases/refr/tests.yaml @@ -12,3 +12,30 @@ cases: query: data.test want_result: x: [0, 1] + - note: literal-dot package and nested package materialize as distinct components + modules: + - | + package graph["a.b"] + literal := 1 + - | + package graph.a.b + nested := 2 + query: data + want_result: + graph: + a: + b: + nested: 2 + a.b: + literal: 1 + - note: missing literal-dot package does not read a nested package + modules: + - | + package graph.a.b + value := "nested" + - | + package consumer + import rego.v1 + result := data.graph["a.b"].value + query: data.consumer.result + no_result: true diff --git a/tests/interpreter/cases/target/azure_policy.yaml b/tests/interpreter/cases/target/azure_policy.yaml index 4e9062634..be722dacb 100644 --- a/tests/interpreter/cases/target/azure_policy.yaml +++ b/tests/interpreter/cases/target/azure_policy.yaml @@ -77,3 +77,28 @@ cases: } query: data.azure.policy.invalid.allow want_result: false + + - note: "Azure Policy target discovers an effect below a literal-dot package component" + data: {} + input: + type: "Microsoft.Storage/storageAccounts" + name: "literal-dot-account" + location: "East US" + properties: + supportsHttpsTrafficOnly: true + modules: + - | + package azure.policy["version.1"] + + import rego.v1 + + __target__ := "target.tests.azure_policy" + + default allow := false + + allow if { + input.type == "Microsoft.Storage/storageAccounts" + input.properties.supportsHttpsTrafficOnly == true + } + query: data.azure.policy["version.1"].allow + want_result: true diff --git a/tests/rvm/rego/cases/namespace_paths.yaml b/tests/rvm/rego/cases/namespace_paths.yaml new file mode 100644 index 000000000..14f2de581 --- /dev/null +++ b/tests/rvm/rego/cases/namespace_paths.yaml @@ -0,0 +1,207 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +cases: + - note: dotted package entrypoint returns the conditional true value + modules: + - | + package graph.defUniqueName["1.0.0"] + default deny := false + deny := true if { input.blocked == true } + query: data.graph.defUniqueName["1.0.0"].deny + input: + blocked: true + want_result: true + + - note: dotted package entrypoint preserves its false default + modules: + - | + package graph.defUniqueName["1.0.0"] + default deny := false + deny := true if { input.blocked == true } + query: data.graph.defUniqueName["1.0.0"].deny + input: + blocked: false + want_result: false + + - note: default-only dotted package rule returns false + modules: + - | + package graph.defUniqueName["1.0.0"] + default deny := false + query: data.graph.defUniqueName["1.0.0"].deny + input: {} + want_result: false + + - note: conditional dotted package rule without a default stays undefined + modules: + - | + package graph.defUniqueName["1.0.0"] + deny := true if { input.blocked == true } + query: data.graph.defUniqueName["1.0.0"].deny + input: + blocked: false + want_result: "#undefined" + + - note: literal-dot and nested packages keep imported defaults and functions separate + modules: + - | + package graph["a.b"] + import rego.v1 + + default value := "literal default" + suffix() := "literal function" + - | + package graph.a.b + import rego.v1 + + default value := "nested default" + suffix() := "nested function" + - | + package consumer + import rego.v1 + import data.graph["a.b"] as dotted + import data.graph.a.b as nested + + result := { + "dotted": dotted.value, + "nested": nested.value, + "dotted_function": dotted.suffix(), + "nested_function": nested.suffix() + } + query: data.consumer.result + input: {} + want_result: + dotted: literal default + nested: nested default + dotted_function: literal function + nested_function: nested function + + - note: absent literal-dot namespace does not route to nested package rule + modules: + - | + package graph.a.b + value := "nested" + - | + package consumer + import rego.v1 + result := data.graph["a.b"].value + query: data.consumer.result + input: {} + want_result: "#undefined" + + - note: input-selected literal-dot version reaches the matching rule + modules: + - | + package graph["1.0.0"] + deny := true if { input.blocked == true } + - | + package consumer + import rego.v1 + result := data.graph[input.version].deny + query: data.consumer.result + input: + version: "1.0.0" + blocked: true + want_result: true + + - note: dotted package entrypoint also works in suspendable mode + modules: + - | + package graph.defUniqueName["1.0.0"] + default deny := false + deny := true if { input.blocked == true } + query: data.graph.defUniqueName["1.0.0"].deny + input: + blocked: true + execution_mode: suspendable + want_result: true + + - note: numeric selector finds the canonical numeric string package + modules: + - | + package graph["1"] + value := 7 + result := data.graph[1].value + query: data.graph["1"].result + want_result: 7 + + - note: numeric data key remains preferred over a numeric string namespace + data: + graph: + 1: numeric data + modules: + - | + package graph["1"] + value := 7 + - | + package consumer + result := data.graph[1] + query: data.consumer.result + want_result: numeric data + + - note: numeric namespace subobject preserves base fields in both selectors + data: + graph: + "1": + extra: 9 + modules: + - | + package graph["1"] + value := 7 + - | + package consumer + result := { + "numeric": data.graph[input.numeric], + "string": data.graph[input.string] + } + query: data.consumer.result + input: + numeric: 1 + string: "1" + want_result: + numeric: + extra: 9 + value: 7 + string: + extra: 9 + value: 7 + + - note: actual numeric data object takes precedence over a string namespace + data: + graph: + 1: + extra: numeric data + "1": + extra: string data + modules: + - | + package graph["1"] + value := 7 + - | + package consumer + result := data.graph[1] + query: data.consumer.result + want_result: + extra: numeric data + + - note: escaped package zero argument function projection uses decoded keys + modules: + - | + package graph["a\".b"] + value() := 7 + - | + package consumer + result := data.graph["a\".b"].value + query: data.consumer.result + want_result: 7 + + - note: bracketed identifier entrypoint retains its caller-visible name + modules: + - | + package graph.version + value := 7 + query: data.graph["version"].value + entry_points: + - data.graph["version"].value + want_result: 7 diff --git a/tests/rvm/rego/mod.rs b/tests/rvm/rego/mod.rs index 6a16db671..26ce6c8ed 100644 --- a/tests/rvm/rego/mod.rs +++ b/tests/rvm/rego/mod.rs @@ -4,7 +4,9 @@ use anyhow::Result; use regorus::languages::rego::compiler::Compiler; -use regorus::rvm::program::{generate_tabular_assembly_listing, AssemblyListingConfig, Program}; +use regorus::rvm::program::{ + generate_tabular_assembly_listing, AssemblyListingConfig, DeserializationResult, Program, +}; use regorus::rvm::tests::test_utils::test_round_trip_serialization; use regorus::rvm::vm::{ExecutionMode, ExecutionState, RegoVM, SuspendReason}; use regorus::test_utils::{check_output, process_value, value_or_vec_to_vec, ValueOrVec}; @@ -12,6 +14,7 @@ use regorus::{CompiledPolicy, Engine, Rc, Value}; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, VecDeque}; use std::fs; +use std::sync::Arc; use test_generator::test_resources; #[derive(Serialize, Deserialize, PartialEq, Debug)] @@ -830,6 +833,151 @@ fn yaml_test_impl(file: &str) -> Result<()> { Ok(()) } +#[test] +fn namespace_entrypoint_roundtrips_and_reuses_vm_state() -> Result<()> { + let entrypoint = "data.graph.defUniqueName[\"1.0.0\"].deny"; + let mut engine = Engine::new(); + engine.add_policy( + "namespace.rego".to_string(), + r#" + package graph.defUniqueName["1.0.0"] + + default deny := false + deny := true if { input.blocked == true } + "# + .to_string(), + )?; + let entrypoint_ref: Rc = entrypoint.into(); + let compiled = engine.compile_with_entrypoint(&entrypoint_ref)?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let json_program = Program::deserialize_json(&json).map_err(anyhow::Error::msg)?; + let binary = program.serialize_binary().map_err(anyhow::Error::msg)?; + let binary_program = match Program::deserialize_binary(&binary).map_err(anyhow::Error::msg)? { + DeserializationResult::Complete(program) => program, + DeserializationResult::Partial(_) => { + anyhow::bail!("binary namespace program unexpectedly needs recompilation") + } + }; + + for (program, mode) in [ + (json_program, ExecutionMode::RunToCompletion), + (binary_program, ExecutionMode::Suspendable), + ] { + let mut vm = RegoVM::new(); + vm.load_program(Arc::new(program)); + vm.set_data(Value::new_object())?; + vm.set_execution_mode(mode); + + vm.set_input(Value::from_json_str(r#"{"blocked":true}"#)?); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + Value::Bool(true) + ); + + vm.set_input(Value::from_json_str(r#"{"blocked":false}"#)?); + assert_eq!(vm.execute_entry_point_by_index(0)?, Value::Bool(false)); + + vm.set_input(Value::Undefined); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + Value::Bool(false) + ); + } + + Ok(()) +} + +#[test] +fn numeric_namespace_subobject_preserves_base_fields_on_reused_vm() -> Result<()> { + let entrypoint = "data.consumer.result"; + let mut engine = Engine::new(); + engine.add_data(Value::from_json_str(r#"{"graph":{"1":{"extra":9}}}"#)?)?; + engine.add_policy( + "namespace.rego".to_string(), + r#" + package graph["1"] + value := 7 + "# + .to_string(), + )?; + engine.add_policy( + "consumer.rego".to_string(), + r#" + package consumer + result := data.graph[input.selector] + "# + .to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + let expected = Value::from_json_str(r#"{"extra":9,"value":7}"#)?; + let inputs = [ + Value::from_json_str(r#"{"selector":"1"}"#)?, + Value::from_json_str(r#"{"selector":1}"#)?, + Value::from_json_str(r#"{"selector":"1"}"#)?, + Value::from_json_str(r#"{"selector":1}"#)?, + ]; + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + + for input in &inputs { + vm.set_input(input.clone()); + assert_eq!(vm.execute_entry_point_by_name(entrypoint)?, expected); + } + } + + Ok(()) +} + +#[test] +fn bracketed_identifier_entrypoint_roundtrips_and_executes_by_name_and_index() -> Result<()> { + let requested_entrypoint = "data.graph[\"version\"].value"; + let mut engine = Engine::new(); + engine.add_policy( + "namespace.rego".to_string(), + "package graph.version\nvalue := 7".to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&"data.graph.version.value".into())?; + for invalid_entrypoint in [ + "data.graph[1].value", + "data.graph.version.value trailing", + "data.graph.missing.value", + ] { + assert!(Compiler::compile_from_policy(&compiled, &[invalid_entrypoint]).is_err()); + } + let program = Compiler::compile_from_policy(&compiled, &[requested_entrypoint])?; + + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let json_program = Program::deserialize_json(&json).map_err(anyhow::Error::msg)?; + let binary = program.serialize_binary().map_err(anyhow::Error::msg)?; + let binary_program = match Program::deserialize_binary(&binary).map_err(anyhow::Error::msg)? { + DeserializationResult::Complete(program) => program, + DeserializationResult::Partial(_) => { + anyhow::bail!("binary identifier program unexpectedly needs recompilation") + } + }; + + for program in [json_program, binary_program] { + let mut vm = RegoVM::new(); + vm.load_program(Arc::new(program)); + vm.set_data(Value::new_object())?; + assert_eq!( + vm.execute_entry_point_by_name(requested_entrypoint)?, + Value::from(7) + ); + assert_eq!(vm.execute_entry_point_by_index(0)?, Value::from(7)); + } + + Ok(()) +} + #[test_resources("tests/rvm/rego/cases/*.yaml")] fn run_rego_compiler_yaml(file: &str) { yaml_test_impl(file).unwrap(); From 40d61766ddaef13fa9b6d44cd684f1a3b7dfb244 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Thu, 1 Oct 2026 18:51:18 +0200 Subject: [PATCH 2/7] test: cover numeric namespace and input selectors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/parser/cases/package/package.yaml | 4 + tests/rvm/rego/cases/namespace_paths.yaml | 39 ++++++- tests/rvm/rego/mod.rs | 120 ++++++++++++++++++++-- 3 files changed, 150 insertions(+), 13 deletions(-) diff --git a/tests/parser/cases/package/package.yaml b/tests/parser/cases/package/package.yaml index 0d20250c1..4da8860e5 100644 --- a/tests/parser/cases/package/package.yaml +++ b/tests/parser/cases/package/package.yaml @@ -170,3 +170,7 @@ cases: - note: non-string-index rego: package a[1] error: expected string + + - note: non-string-decimal-index + rego: package a[1.0] + error: expected string diff --git a/tests/rvm/rego/cases/namespace_paths.yaml b/tests/rvm/rego/cases/namespace_paths.yaml index 14f2de581..242359c30 100644 --- a/tests/rvm/rego/cases/namespace_paths.yaml +++ b/tests/rvm/rego/cases/namespace_paths.yaml @@ -126,6 +126,39 @@ cases: query: data.graph["1"].result want_result: 7 + - note: numeric literal selectors canonicalize while quoted versions stay distinct + data: + graph: + "1": + base: integer + "1.0": + base: decimal + modules: + - | + package graph["1"] + value := "integer namespace" + - | + package graph["1.0"] + value := "decimal namespace" + - | + package consumer + result := [ + data.graph["1"], + data.graph["1.0"], + data.graph[1], + data.graph[1.0] + ] + query: data.consumer.result + want_result: + - base: integer + value: integer namespace + - base: decimal + value: decimal namespace + - base: integer + value: integer namespace + - base: integer + value: integer namespace + - note: numeric data key remains preferred over a numeric string namespace data: graph: @@ -136,9 +169,11 @@ cases: value := 7 - | package consumer - result := data.graph[1] + result := [data.graph[1], data.graph[1.0]] query: data.consumer.result - want_result: numeric data + want_result: + - numeric data + - numeric data - note: numeric namespace subobject preserves base fields in both selectors data: diff --git a/tests/rvm/rego/mod.rs b/tests/rvm/rego/mod.rs index 26ce6c8ed..5d5b179d8 100644 --- a/tests/rvm/rego/mod.rs +++ b/tests/rvm/rego/mod.rs @@ -890,10 +890,12 @@ fn namespace_entrypoint_roundtrips_and_reuses_vm_state() -> Result<()> { } #[test] -fn numeric_namespace_subobject_preserves_base_fields_on_reused_vm() -> Result<()> { +fn numeric_namespace_selectors_keep_string_identity_on_reused_vm() -> Result<()> { let entrypoint = "data.consumer.result"; let mut engine = Engine::new(); - engine.add_data(Value::from_json_str(r#"{"graph":{"1":{"extra":9}}}"#)?)?; + engine.add_data(Value::from_json_str( + r#"{"graph":{"1":{"extra":9},"1.0":{"extra":90}}}"#, + )?)?; engine.add_policy( "namespace.rego".to_string(), r#" @@ -902,6 +904,14 @@ fn numeric_namespace_subobject_preserves_base_fields_on_reused_vm() -> Result<() "# .to_string(), )?; + engine.add_policy( + "decimal_namespace.rego".to_string(), + r#" + package graph["1.0"] + value := 70 + "# + .to_string(), + )?; engine.add_policy( "consumer.rego".to_string(), r#" @@ -913,23 +923,111 @@ fn numeric_namespace_subobject_preserves_base_fields_on_reused_vm() -> Result<() let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; let data = engine.get_data(); - let expected = Value::from_json_str(r#"{"extra":9,"value":7}"#)?; - let inputs = [ - Value::from_json_str(r#"{"selector":"1"}"#)?, - Value::from_json_str(r#"{"selector":1}"#)?, - Value::from_json_str(r#"{"selector":"1"}"#)?, - Value::from_json_str(r#"{"selector":1}"#)?, + let cases = [ + (r#"{"selector":"1"}"#, r#"{"extra":9,"value":7}"#), + (r#"{"selector":1}"#, r#"{"extra":9,"value":7}"#), + (r#"{"selector":1.0}"#, r#"{"extra":9,"value":7}"#), + (r#"{"selector":"1.0"}"#, r#"{"extra":90,"value":70}"#), + (r#"{"selector":1.0}"#, r#"{"extra":9,"value":7}"#), + (r#"{"selector":"1"}"#, r#"{"extra":9,"value":7}"#), + (r#"{"selector":"1.0"}"#, r#"{"extra":90,"value":70}"#), + ]; + + for (input_json, expected_json) in cases { + let input = Value::from_json_str(input_json)?; + let expected = Value::from_json_str(expected_json)?; + engine.set_input(input); + assert_eq!( + engine.eval_rule(entrypoint.to_string())?, + expected, + "interpreter result for input {input_json}" + ); + } + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + + for (input_json, expected_json) in cases { + vm.set_input(Value::from_json_str(input_json)?); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + Value::from_json_str(expected_json)?, + "{mode:?} result for input {input_json}" + ); + } + } + + Ok(()) +} + +#[test] +fn numeric_object_selectors_do_not_use_data_namespace_fallback() -> Result<()> { + let entrypoint = "data.consumer.result"; + let mut engine = Engine::new(); + engine.add_policy( + "consumer.rego".to_string(), + r#" + package consumer + result := input.object[input.selector] + "# + .to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + let cases = [ + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":"1"}"#, + Value::String("integer string".into()), + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":1}"#, + Value::Undefined, + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":1.0}"#, + Value::Undefined, + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":"1.0"}"#, + Value::String("decimal string".into()), + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":1.0}"#, + Value::Undefined, + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":"1"}"#, + Value::String("integer string".into()), + ), + ( + r#"{"object":{"1":"integer string","1.0":"decimal string"},"selector":"1.0"}"#, + Value::String("decimal string".into()), + ), ]; + for (input_json, expected) in &cases { + engine.set_input(Value::from_json_str(input_json)?); + let actual = engine.eval_rule(entrypoint.to_string())?; + assert_eq!( + &actual, expected, + "interpreter result for input {input_json}" + ); + } + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { let mut vm = RegoVM::new(); vm.load_program(program.clone()); vm.set_data(data.clone())?; vm.set_execution_mode(mode); - for input in &inputs { - vm.set_input(input.clone()); - assert_eq!(vm.execute_entry_point_by_name(entrypoint)?, expected); + for (input_json, expected) in &cases { + vm.set_input(Value::from_json_str(input_json)?); + let actual = vm.execute_entry_point_by_name(entrypoint)?; + assert_eq!(&actual, expected, "{mode:?} result for input {input_json}"); } } From 37b8490c06e9956ba6bcbf4308e9049b0ebfa544 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Tue, 6 Oct 2026 16:45:33 +0200 Subject: [PATCH 3/7] fix: preserve dynamic namespace reads and bound static paths Load only resolved packages while preserving nested WITH overrides, Undefined behavior, and modifier-error rollback. Reject overdeep static paths before AST construction and cover the fixes through Engine, parser, RVM, and FFI regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/ffi/src/engine.rs | 104 ++++++ docs/grammar.md | 6 + src/interpreter.rs | 397 +++++++++++++++------ src/parser.rs | 14 + tests/engine/mod.rs | 706 ++++++++++++++++++++++++++++++++++++- tests/parser/mod.rs | 24 ++ tests/rvm/rego/mod.rs | 148 ++++++++ 7 files changed, 1288 insertions(+), 111 deletions(-) diff --git a/bindings/ffi/src/engine.rs b/bindings/ffi/src/engine.rs index 8f9d04afb..ea097e0c4 100644 --- a/bindings/ffi/src/engine.rs +++ b/bindings/ffi/src/engine.rs @@ -908,6 +908,110 @@ mod namespace_tests { regorus_engine_drop(engine); } + #[test] + fn overdeep_policy_path_returns_an_error_and_keeps_the_engine_usable() { + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let stable_file = CString::new("stable.rego").expect("valid file path"); + let stable_policy = CString::new("package stable\nvalue := true").expect("valid policy"); + let added = regorus_engine_add_policy(engine, stable_file.as_ptr(), stable_policy.as_ptr()); + assert!(matches!(&added.status, RegorusStatus::Ok)); + regorus_result_drop(added); + + let deep_package = ["segment"; 33].join("."); + let deep_file = CString::new("deep.rego").expect("valid file path"); + let deep_policy = + CString::new(format!("package {deep_package}\nvalue := true")).expect("valid policy"); + let rejected = regorus_engine_add_policy(engine, deep_file.as_ptr(), deep_policy.as_ptr()); + let is_error = matches!(&rejected.status, RegorusStatus::Error); + let error_message = if rejected.error_message.is_null() { + None + } else { + Some( + unsafe { CStr::from_ptr(rejected.error_message) } + .to_str() + .expect("UTF-8 error message") + .to_owned(), + ) + }; + regorus_result_drop(rejected); + assert!( + is_error, + "overdeep policy was not rejected: {error_message:?}" + ); + + let query = CString::new("data.stable.value").expect("valid rule path"); + let result = regorus_engine_eval_rule(engine, query.as_ptr()); + let succeeded = matches!(&result.status, RegorusStatus::Ok); + let output = if result.output.is_null() { + None + } else { + Some( + unsafe { CStr::from_ptr(result.output) } + .to_str() + .expect("UTF-8 result") + .to_owned(), + ) + }; + regorus_result_drop(result); + assert!( + succeeded, + "engine failed after rejecting the policy: {output:?}" + ); + assert_eq!(output.as_deref(), Some("true")); + + let long_dotted = format!("data{}", ".a".repeat(300_000)); + let long_bracketed = format!("data{}", r#"["a"]"#.repeat(180_000)); + for path in [long_dotted, long_bracketed] { + assert!(path.len() < 1024 * 1024); + let rule = CString::new(path).expect("valid rule path"); + let rejected = regorus_engine_eval_rule(engine, rule.as_ptr()); + let is_error = matches!(&rejected.status, RegorusStatus::Error); + let error_message = if rejected.error_message.is_null() { + None + } else { + Some( + unsafe { CStr::from_ptr(rejected.error_message) } + .to_str() + .expect("UTF-8 error message") + .to_owned(), + ) + }; + regorus_result_drop(rejected); + assert!( + is_error, + "huge rule path was not rejected: {error_message:?}" + ); + assert_eq!( + error_message.as_deref(), + Some("not a valid rule path"), + "expected the path validation error, not an engine-poisoned error" + ); + + let result = regorus_engine_eval_rule(engine, query.as_ptr()); + let succeeded = matches!(&result.status, RegorusStatus::Ok); + let output = if result.output.is_null() { + None + } else { + Some( + unsafe { CStr::from_ptr(result.output) } + .to_str() + .expect("UTF-8 result") + .to_owned(), + ) + }; + regorus_result_drop(result); + assert!( + succeeded, + "engine failed after rejecting the rule: {output:?}" + ); + assert_eq!(output.as_deref(), Some("true")); + } + + regorus_engine_drop(engine); + } + #[cfg(feature = "rvm")] #[test] fn bracketed_identifier_entrypoint_compiles_and_executes_through_engine_ffi() { diff --git a/docs/grammar.md b/docs/grammar.md index 7f69abbd5..d72d0370f 100644 --- a/docs/grammar.md +++ b/docs/grammar.md @@ -160,6 +160,12 @@ For example, `graph["a.b"]` is distinct from `graph.a.b`. When a package path is returned by the Engine API or used as a rule entry point, non-identifier string components use JSON-escaped bracket notation. +Each parsed path reference is limited to 32 components, counting its initial +identifier as one. Every dotted identifier or bracketed string adds one +component; dots inside a bracketed string do not. A 33rd component is rejected +during parsing. `Engine::eval_rule` reports `not a valid rule path` for an +overlong entry point, while parser and compiler APIs return an error. + referenced by: * call-expr diff --git a/src/interpreter.rs b/src/interpreter.rs index e2ae23152..9ed31cee6 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -77,6 +77,11 @@ enum RuleValueMerge { type RuleValues = BTreeMap, (Value, Ref)>; +// Path components in processed_paths are strings, and the ordinary processed marker uses +// Undefined. This boolean key records an explicit data WITH replacement without colliding +// with either. +const EXPLICIT_DATA_WITH_OVERRIDE_MARKER: Value = Value::Bool(true); + #[derive(Debug)] pub struct Interpreter { compiled_policy: Rc, @@ -624,6 +629,23 @@ impl Interpreter { self.ensure_rule_evaluated(string_path)?; } } + + let index_component = match &index { + Value::String(value) => { + Some(value.as_ref().to_string()) + } + Value::Number(_) => Some(index.to_string()), + _ => None, + }; + if let Some(index_component) = index_component { + let mut module_path = ref_components.clone(); + module_path.push(index_component); + module_path.extend( + path.iter() + .map(|component| (*component).to_string()), + ); + self.ensure_module_evaluated(&module_path)?; + } } } } @@ -1267,6 +1289,78 @@ impl Interpreter { }) } + fn copy_explicit_data_with_override_markers( + &mut self, + paths: &Value, + path: &mut Vec, + overridden_paths: &mut Vec>, + ) -> Result { + let mut inherited_paths = Value::new_object(); + let Value::Object(entries) = paths else { + return Ok(inherited_paths); + }; + + for (key, value) in entries.iter() { + self.check_execution_time()?; + self.memory_check()?; + if key == &EXPLICIT_DATA_WITH_OVERRIDE_MARKER { + inherited_paths + .as_object_mut()? + .insert(key.clone(), value.clone()); + let mut overridden_path = vec!["data".to_string()]; + overridden_path.extend(path.iter().cloned()); + overridden_paths.push(overridden_path); + self.memory_check()?; + } else if let Value::String(component) = key { + path.push(component.to_string()); + let child = + self.copy_explicit_data_with_override_markers(value, path, overridden_paths)?; + let _ = path.pop(); + if !child.as_object()?.is_empty() { + inherited_paths.as_object_mut()?.insert(key.clone(), child); + self.memory_check()?; + } + } + } + + Ok(inherited_paths) + } + + fn suppress_rules_under_data_override(&mut self, overridden_path: &[String]) -> Result<()> { + let compiled_policy = self.compiled_policy.clone(); + for (rule_path, rules) in &compiled_policy.rules { + self.check_execution_time()?; + let rule_components = compiled_policy + .rule_path_components + .get(rule_path) + .ok_or_else(|| { + anyhow!("missing components for registered rule path {rule_path}") + })?; + if rule_components.starts_with(overridden_path) { + for rule in rules { + self.processed.insert(rule.clone()); + self.memory_check()?; + } + } + } + for (rule_path, rules) in &compiled_policy.default_rules { + self.check_execution_time()?; + let rule_components = compiled_policy + .rule_path_components + .get(rule_path) + .ok_or_else(|| { + anyhow!("missing components for registered default rule path {rule_path}") + })?; + if rule_components.starts_with(overridden_path) { + for (rule, _) in rules { + self.processed.insert(rule.clone()); + self.memory_check()?; + } + } + } + Ok(()) + } + fn apply_with_modifiers(&mut self, stmt: &LiteralStmt) -> Result<(Option, bool)> { if !stmt.with_mods.is_empty() { // Save state; @@ -1277,134 +1371,171 @@ impl Interpreter { let with_functions = self.with_functions.clone(); let rule_values = self.rule_values.clone(); - self.processed.clear(); let processed_paths = core::mem::replace(&mut self.processed_paths, Value::new_object()); + let mut inherited_overrides = Vec::new(); + let inherited_paths = match self.copy_explicit_data_with_override_markers( + &processed_paths, + &mut Vec::new(), + &mut inherited_overrides, + ) { + Ok(paths) => paths, + Err(error) => { + self.processed_paths = processed_paths; + return Err(error); + } + }; + self.processed.clear(); + self.processed_paths = inherited_paths; self.rule_values.clear(); - let mut skip_exec = false; - // Apply with modifiers. - for wm in &stmt.with_mods { - let mut path = get_rule_path_components(&wm.refr)?; - - // Matching OPA, a leading import alias is rewritten before - // any lookups: functions register as overrides below, - // anything else becomes a data override. Only the alias - // component is replaced so bracketed keys containing dots - // survive the rewrite. - let rewritten: Option> = match path.split_first() { - Some((PathComponent::String(head), rest)) if head.as_str() != "data" => { - self.lookup_import(head).and_then(|import_expr| { - // Use the import target's parsed components, not - // its dot-joined string, so bracketed keys - // containing dots survive in the import path too. - let comps = get_rule_path_components(import_expr).ok()?; - Some(comps.into_iter().chain(rest.iter().cloned()).collect()) - }) + let saved_state = Some(( + with_document, + input, + data, + processed, + processed_paths, + with_functions, + rule_values, + )); + let apply_result = (|| -> Result { + let mut skip_exec = false; + for overridden_path in &inherited_overrides { + self.suppress_rules_under_data_override(overridden_path)?; + } + // Apply with modifiers. + for wm in &stmt.with_mods { + let mut path = get_rule_path_components(&wm.refr)?; + + // Matching OPA, a leading import alias is rewritten before + // any lookups: functions register as overrides below, + // anything else becomes a data override. Only the alias + // component is replaced so bracketed keys containing dots + // survive the rewrite. + let rewritten: Option> = match path.split_first() { + Some((PathComponent::String(head), rest)) if head.as_str() != "data" => { + self.lookup_import(head).and_then(|import_expr| { + // Use the import target's parsed components, not + // its dot-joined string, so bracketed keys + // containing dots survive in the import path too. + let comps = get_rule_path_components(import_expr).ok()?; + Some(comps.into_iter().chain(rest.iter().cloned()).collect()) + }) + } + _ => None, + }; + if let Some(new_path) = rewritten { + path = new_path; } - _ => None, - }; - if let Some(new_path) = rewritten { - path = new_path; - } - let mut target = format_path_components(&path)?; + let mut target = format_path_components(&path)?; - let mut target_is_function = self.lookup_function_by_name(&target).is_some() - || Self::is_builtin(wm.refr.span(), &target); + let mut target_is_function = self.lookup_function_by_name(&target).is_some() + || Self::is_builtin(wm.refr.span(), &target); - let root = path.first().map(PathComponent::value).unwrap_or_default(); - if !target_is_function && root != "data" && root != "input" { - // target must be a function. - if self.lookup_function_by_name(&target).is_none() - && !Self::is_builtin(wm.refr.span(), &target) - { - // Prefix target with current module path. - target = format!("{}.{}", self.current_module_path, target); - if self.lookup_function_by_name(&target).is_none() { - bail!(wm.refr.span().error("undefined rule")); + let root = path.first().map(PathComponent::value).unwrap_or_default(); + if !target_is_function && root != "data" && root != "input" { + // target must be a function. + if self.lookup_function_by_name(&target).is_none() + && !Self::is_builtin(wm.refr.span(), &target) + { + // Prefix target with current module path. + target = format!("{}.{}", self.current_module_path, target); + if self.lookup_function_by_name(&target).is_none() { + bail!(wm.refr.span().error("undefined rule")); + } + target_is_function = true; } - target_is_function = true; } - } - if target_is_function { - match self.eval_expr(&wm.r#as) { - Ok(v) if v != Value::Undefined => { - // Function replaced by value. - self.with_functions - .insert(target, FunctionModifier::Value(v)); - } - _ => { - // Function replaced by another function. - // Lookup by with current module path prefixed. - let mut function_path = - get_path_string(&wm.r#as, Some(&self.current_module_path))?; - if self.lookup_function_by_name(&function_path).is_none() { - // Lookup without current module path prefixed. - function_path = get_path_string(&wm.r#as, None)?; + if target_is_function { + match self.eval_expr(&wm.r#as) { + Ok(v) if v != Value::Undefined => { + // Function replaced by value. + self.with_functions + .insert(target, FunctionModifier::Value(v)); + } + _ => { + // Function replaced by another function. + // Lookup by with current module path prefixed. + let mut function_path = + get_path_string(&wm.r#as, Some(&self.current_module_path))?; if self.lookup_function_by_name(&function_path).is_none() { - // Resolve an aliased replacement before builtins. - let resolved = self - .resolve_fcn_path_through_imports(&function_path) - .filter(|r| self.compiled_policy.functions.contains_key(r)); - if let Some(resolved) = resolved { - function_path = resolved; - } else if !Self::is_builtin(wm.r#as.span(), &function_path) { - // bail!(wm.r#as.span().error("could not evaluate expression")); - skip_exec = true; + // Lookup without current module path prefixed. + function_path = get_path_string(&wm.r#as, None)?; + if self.lookup_function_by_name(&function_path).is_none() { + // Resolve an aliased replacement before builtins. + let resolved = self + .resolve_fcn_path_through_imports(&function_path) + .filter(|r| { + self.compiled_policy.functions.contains_key(r) + }); + if let Some(resolved) = resolved { + function_path = resolved; + } else if !Self::is_builtin(wm.r#as.span(), &function_path) + { + // bail!(wm.r#as.span().error("could not evaluate expression")); + skip_exec = true; + } } } + self.with_functions + .insert(target, FunctionModifier::Function(function_path)); } - self.with_functions - .insert(target, FunctionModifier::Function(function_path)); } - } - } else { - let value = self.eval_expr(&wm.r#as)?; - skip_exec = value == Value::Undefined; - let Some(first) = path.first() else { - bail!(wm.refr.span().error("empty path in with modifier")); - }; - if first.value() == "input" || first.value() == "data" { - // Override existing values in case of conflict. - let mut obj = &mut self.with_document; - for component in &path { - if !matches!(obj, Value::Object(_)) { - *obj = Value::new_object(); - } + } else { + let value = self.eval_expr(&wm.r#as)?; + skip_exec = value == Value::Undefined; + let Some(first) = path.first() else { + bail!(wm.refr.span().error("empty path in with modifier")); + }; + if first.value() == "input" || first.value() == "data" { + // Override existing values in case of conflict. + let mut obj = &mut self.with_document; + for component in &path { + if !matches!(obj, Value::Object(_)) { + *obj = Value::new_object(); + } - obj = obj.as_object_mut()?.get_or_insert_with( - Value::String(component.value().to_string().into()), - Value::new_object, - ); - } - *obj = value; - // Mark modified rules as processed. - if let Some(rules) = self.compiled_policy.rules.get(&target) { - for r in rules { - self.processed.insert(r.clone()); + obj = obj.as_object_mut()?.get_or_insert_with( + Value::String(component.value().to_string().into()), + Value::new_object, + ); + } + *obj = value; + + if first.value() == "data" { + let overridden_path: Vec = path + .iter() + .map(|component| component.value().to_string()) + .collect(); + let (_, overridden_subtree) = overridden_path + .split_first() + .ok_or_else(|| anyhow!("empty path in with modifier"))?; + self.mark_data_with_override( + &overridden_subtree + .iter() + .map(String::as_str) + .collect::>(), + )?; + self.suppress_rules_under_data_override(&overridden_path)?; } + } else { + bail!(wm.refr.span().error("not a valid target for with modifier")); } - } else { - bail!(wm.refr.span().error("not a valid target for with modifier")); } } - } - self.data = self.with_document["data"].clone(); - self.input = self.with_document["input"].clone(); - Ok(( - Some(( - with_document, - input, - data, - processed, - processed_paths, - with_functions, - rule_values, - )), - skip_exec, - )) + self.data = self.with_document["data"].clone(); + self.input = self.with_document["input"].clone(); + Ok(skip_exec) + })(); + match apply_result { + Ok(skip_exec) => Ok((saved_state, skip_exec)), + Err(error) => { + self.restore_state(saved_state)?; + Err(error) + } + } } else { Ok((None, false)) } @@ -3033,6 +3164,22 @@ impl Interpreter { Parser::get_static_string_path_components(&module.package.refr)?; let module_value_components: Vec<&str> = module_value_components.iter().map(String::as_str).collect(); + if self.has_explicit_data_with_override_ancestor(&module_value_components)? { + continue; + } + + // A parent package can be processed before its child modules are materialized. + if self.is_processed(&[])? + || (!module.policy.is_empty() + && module + .policy + .iter() + .all(|rule| self.processed.contains(rule)) + && self.is_processed(&module_value_components)?) + { + continue; + } + let vref = Self::make_or_get_value_mut(&mut self.data, &module_value_components)?; if *vref == Value::Undefined { *vref = Value::new_object(); @@ -3127,6 +3274,42 @@ impl Interpreter { Ok(()) } + fn mark_data_with_override(&mut self, path: &[&str]) -> Result<()> { + self.mark_processed(path)?; + let obj = self.processed_paths.make_or_get_value_mut(path)?; + obj.as_object_mut()? + .insert(EXPLICIT_DATA_WITH_OVERRIDE_MARKER.clone(), Value::Null); + Ok(()) + } + + fn has_explicit_data_with_override_ancestor(&self, path: &[&str]) -> Result { + let mut obj = &self.processed_paths; + if obj + .as_object()? + .get(&EXPLICIT_DATA_WITH_OVERRIDE_MARKER) + .is_some() + { + return Ok(true); + } + + for component in path { + match &obj[*component] { + Value::Undefined => return Ok(false), + value => obj = value, + } + + if obj + .as_object()? + .get(&EXPLICIT_DATA_WITH_OVERRIDE_MARKER) + .is_some() + { + return Ok(true); + } + } + + Ok(false) + } + fn lookup_var(&mut self, span: &Span, fields: &[&str], no_error: bool) -> Result { let name = span.source_str(); diff --git a/src/parser.rs b/src/parser.rs index 6d07027b5..b82800415 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -285,6 +285,15 @@ impl<'source> Parser<'source> { Self::get_static_string_path_components(&expression) } + fn check_path_component_limit(&self, current_count: usize) -> Result<()> { + if current_count >= DEFAULT_MAX_EXPR_DEPTH { + let message = + alloc::format!("path exceeds maximum depth of {DEFAULT_MAX_EXPR_DEPTH} components"); + bail!("{}", self.tok.1.error(&message)); + } + Ok(()) + } + fn handle_import_future_keywords(&mut self, comps: &[Span]) -> Result { if comps.len() >= 2 && comps[0].text() == "future" && comps[1].text() == "keywords" { match comps.len().saturating_sub(2) { @@ -1472,6 +1481,7 @@ impl<'source> Parser<'source> { fn parse_path_ref(&mut self) -> Result { let start = self.tok.1.start; let var = self.parse_var()?; + let mut component_count = 1; let (span, value) = Self::span_and_value(var); let mut refr = Expr::Var { @@ -1495,6 +1505,8 @@ impl<'source> Parser<'source> { ); } "." => { + self.check_path_component_limit(component_count)?; + component_count = component_count.saturating_add(1); // Read identifier. Keywords are allowed as field names in // dot-notation refs (e.g. `import data.my.package`). self.next_token()?; @@ -1520,6 +1532,8 @@ impl<'source> Parser<'source> { }; } "[" => { + self.check_path_component_limit(component_count)?; + component_count = component_count.saturating_add(1); self.next_token()?; let index = match &self.tok.0 { TokenKind::String => self.parse_scalar_or_var()?, diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 81679c18b..c5320705c 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -304,10 +304,12 @@ fn dynamic_data_root_lookup_does_not_evaluate_unrelated_modules() -> Result<()> )?; engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); - assert_eq!( - engine.eval_rule("data.framework.result".to_string())?, - Value::from(7) - ); + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::from(7)); + + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::from(7)); Ok(()) } @@ -333,6 +335,702 @@ fn nested_dynamic_data_lookup_does_not_prefetch_a_dotted_suffix_rule() -> Result Ok(()) } +#[test] +fn dynamic_namespace_lookup_does_not_prefetch_a_trailing_sibling_rule() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + "package framework\nresult := data[input.namespace].value\nnested := data[input.namespace].nested" + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nvalue := 7\nunrelated := data.framework.result".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested\nvalue := 11".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + assert_eq!( + engine.eval_rule("data.framework.result".to_string())?, + Value::from(7) + ); + assert_eq!( + engine.eval_rule("data.framework.nested".to_string())?, + Value::from_json_str(r#"{"value":11}"#)? + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_nested_input_with_preserves_outer_data_override() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + outer := cfg if { cfg := inner with data.target as {"blocked": false} } + inner := cfg if { cfg := data[input.namespace] with input as {"namespace": "target"} } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.outer".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"blocked":false}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true), + "the data replacement must not persist after evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_materializes_a_trailing_subpackage_without_parent_siblings( +) -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + "package framework\nconfig := data[input.namespace].nested".to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nunrelated := data.framework.config".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested\nvalue := 7".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.config".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"value":7}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_replacement_of_selected_parent_data() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := cfg if { + cfg := data[input.namespace] with data.target as {"blocked": false} + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"blocked":false}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true), + "the data replacement must not persist after evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_data_root_replacement_and_restores_original_data( +) -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := cfg if { + cfg := data[input.namespace] with data as {"target": {"blocked": false}} + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"blocked":false}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true), + "the data-root replacement must not persist after evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_data_root_replacement_without_materializing_modules( +) -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := cfg if { + cfg := data[input.namespace] with data as {} + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nvalue := 7".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Undefined); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::Undefined); + assert_eq!( + engine.eval_rule("data.target.value".to_string())?, + Value::from(7), + "the root replacement must be restored before ordinary evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_with_empty_parent_replacement_does_not_materialize_empty_child( +) -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + config := cfg if { + cfg := data[input.namespace].nested with data.target as {} + } + "# + .to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.config".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Undefined); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::Undefined); + + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_with_scalar_parent_replacement_does_not_materialize_child() -> Result<()> +{ + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + config := cfg if { + cfg := data[input.namespace].nested with data.target as 7 + } + "# + .to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested\nvalue := 7".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.config".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Undefined); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::Undefined); + assert_eq!( + engine.eval_rule("data.target.nested.value".to_string())?, + Value::from(7), + "the scalar replacement must be restored before ordinary evaluation" + ); + + Ok(()) +} + +#[test] +fn dynamic_namespace_prefetch_materializes_an_ordinary_empty_child_package() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + config := cfg.nested if { + cfg := data[input.namespace] + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nvalue := 7".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.config".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::new_object(); + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_parent_replacement_of_default_rule() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := cfg if { + cfg := data[input.namespace] with data.target as {"blocked": false} + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\ndefault blocked := true".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"blocked":false}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true), + "the default rule must remain unchanged after evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_nested_parent_replacement() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + config := cfg if { + cfg := data[input.namespace].nested with data.target as {"nested": {"value": 7}} + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nunrelated := data.framework.config".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested\nvalue := 11".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.config".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"value":7}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule("data.target.nested.value".to_string())?, + Value::from(11), + "the nested package must remain unchanged after evaluation" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_honors_with_exact_leaf_replacement() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := blocked if { + blocked := data[input.namespace].blocked with data.target.blocked as false + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true\nvalue := 7".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Bool(false)); + assert_eq!( + engine.eval_rule(entrypoint.to_string())?, + Value::Bool(false) + ); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true) + ); + assert_eq!( + engine.eval_rule("data.target.value".to_string())?, + Value::from(7) + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_with_literal_dot_override_preserves_sibling_identity() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + result := config if { + config := data.graph[input.namespace] with data.graph["a.b"] as {"value": 7} + } + "# + .to_string(), + )?; + engine.add_policy( + "literal.rego".to_string(), + "package graph[\"a.b\"]\nvalue := 11".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package graph.a.b\nvalue := 13".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"a.b"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"a.b"}"#)?; + let expected = Value::from_json_str(r#"{"value":7}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + assert_eq!( + engine.eval_rule(r#"data.graph["a.b"].value"#.to_string())?, + Value::from(11), + "the literal-dot override must be restored" + ); + assert_eq!( + engine.eval_rule("data.graph.a.b.value".to_string())?, + Value::from(13), + "the dotted sibling must not be marked as part of the override" + ); + Ok(()) +} + +#[test] +fn dynamic_namespace_prefetch_merges_initial_data_with_virtual_children() -> Result<()> { + let mut engine = Engine::new(); + engine.add_data(Value::from_json_str(r#"{"target":{"existing":1}}"#)?)?; + engine.add_policy( + "framework.rego".to_string(), + "package framework\nresult := data[input.namespace]".to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nvirtual := 7".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"existing":1,"virtual":7}"#)?; + assert_eq!(compiled.eval_with_input(input)?, expected); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, expected); + Ok(()) +} + +#[test] +fn dynamic_namespace_package_prefetch_evaluates_only_the_selected_package() -> Result<()> { + let mut engine = Engine::new(); + engine.add_extension( + "unrelated_failure".to_string(), + 0, + Box::new(|_| Err(anyhow::anyhow!("unrelated module was evaluated"))), + )?; + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data[input.namespace] + cfg.blocked + } + config := data[input.namespace] + literal_config := data.graph[input.namespace] + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true".to_string(), + )?; + engine.add_policy( + "unblocked.rego".to_string(), + "package unblocked\nvalue := 7".to_string(), + )?; + engine.add_policy( + "literal.rego".to_string(), + "package graph[\"target.v1\"]\nblocked := true".to_string(), + )?; + engine.add_policy( + "unrelated.rego".to_string(), + "package unrelated\nfailure := unrelated_failure()".to_string(), + )?; + + let entrypoint: Rc = "data.framework.allow".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let cases = [ + ( + Value::from_json_str(r#"{"namespace":"target"}"#)?, + Value::Bool(false), + ), + (Value::new_object(), Value::Bool(true)), + (Value::Undefined, Value::Bool(true)), + ( + Value::from_json_str(r#"{"namespace":"unblocked"}"#)?, + Value::Bool(true), + ), + ( + Value::from_json_str(r#"{"namespace":"nonexistent"}"#)?, + Value::Bool(true), + ), + ( + Value::from_json_str(r#"{"namespace":"target"}"#)?, + Value::Bool(false), + ), + ]; + + for (input, expected) in cases { + engine.set_input(input.clone()); + assert_eq!( + engine.eval_rule(entrypoint.to_string())?, + expected, + "interpreter result for input {input:?}" + ); + assert_eq!( + compiled.eval_with_input(input)?, + expected, + "compiled interpreter result" + ); + } + + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + assert_eq!( + engine.eval_rule("data.framework.config".to_string())?, + Value::from_json_str(r#"{"blocked":true}"#)? + ); + + engine.set_input(Value::from_json_str(r#"{"namespace":"target.v1"}"#)?); + assert_eq!( + engine.eval_rule("data.framework.literal_config".to_string())?, + Value::from_json_str(r#"{"blocked":true}"#)? + ); + + engine.set_input(Value::from_json_str(r#"{"namespace":"nonexistent"}"#)?); + assert_eq!( + engine.eval_rule("data.framework.config".to_string())?, + Value::Undefined + ); + + Ok(()) +} + +#[cfg(feature = "std")] +#[test] +fn failed_with_modifier_application_restores_document_before_engine_reuse() -> Result<()> { + use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }; + use std::time::Duration; + + use regorus::utils::limits::ExecutionTimerConfig; + + let replacement_reached = Arc::new(AtomicBool::new(false)); + let extension_reached = Arc::clone(&replacement_reached); + let mut engine = Engine::new(); + engine.add_policy( + "target.rego".to_string(), + "package target\nvalue := 42".to_string(), + )?; + engine.add_policy( + "test.rego".to_string(), + r#" + package test + result := value if { + value := data.target.value with data.target as slow_replacement() + } + later := value if { + value := data.target.value with input.flag as true + } + "# + .to_string(), + )?; + engine.add_extension( + "slow_replacement".to_string(), + 0, + Box::new(move |_| { + extension_reached.store(true, Ordering::SeqCst); + std::thread::sleep(Duration::from_secs(2)); + Value::from_json_str(r#"{"value":99}"#) + }), + )?; + + assert_eq!( + engine.eval_rule("data.test.later".to_string())?, + Value::from(42), + "prepare the engine before starting the timer" + ); + + engine.set_execution_timer_config(ExecutionTimerConfig { + limit: Duration::from_secs(1), + check_interval: std::num::NonZeroU32::new(1).unwrap(), + }); + let error = engine + .eval_rule("data.test.result".to_string()) + .unwrap_err(); + assert!( + error.to_string().contains("execution exceeded time limit"), + "expected the descendant scan timer to fail, got {error:#}" + ); + assert!( + replacement_reached.load(Ordering::SeqCst), + "the replacement extension must run before the timer expires" + ); + + engine.clear_execution_timer_config(); + let later_result = engine.eval_rule("data.test.later".to_string()); + assert!( + matches!(&later_result, Ok(value) if value == &Value::from(42)), + "failed modifier application must preserve original virtual data; got {later_result:?}" + ); + + Ok(()) +} + +#[test] +fn rule_path_component_limit_preserves_engine_behavior() -> Result<()> { + let package_path = (0..30) + .map(|index| format!("p{index}")) + .collect::>() + .join("."); + let mut engine = Engine::new(); + engine.add_policy( + "deep.rego".to_string(), + format!("package {package_path}\nvalue := 7"), + )?; + let remaining_package_path = package_path + .split('.') + .skip(1) + .collect::>() + .join("."); + let at_limit = format!("data[\"p0\"].{remaining_package_path}.value"); + assert_eq!( + package_path.split('.').count(), + 30, + "the policy package contributes 30 components" + ); + assert_eq!( + engine.eval_rule(at_limit.clone())?, + Value::from(7), + "a 32-component entrypoint remains valid" + ); + let compiled = engine.compile_with_entrypoint(&at_limit.clone().into())?; + assert_eq!( + compiled.eval_with_input(Value::Null)?, + Value::from(7), + "the compiled interpreter accepts a 32-component entrypoint" + ); + assert_eq!( + engine + .eval_rule(format!("{at_limit}.extra")) + .unwrap_err() + .to_string(), + "not a valid rule path" + ); + + Ok(()) +} + +#[test] +fn engine_rejects_huge_dotted_and_bracketed_rule_paths() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "stable.rego".to_string(), + "package stable\nvalue := true".to_string(), + )?; + let long_dotted = format!("data{}", ".a".repeat(300_000)); + let long_bracketed = format!("data{}", r#"["a"]"#.repeat(180_000)); + for path in [long_dotted, long_bracketed] { + assert!(path.len() < 1024 * 1024); + assert_eq!( + engine.eval_rule(path).unwrap_err().to_string(), + "not a valid rule path" + ); + assert_eq!( + engine.eval_rule("data.stable.value".to_string())?, + Value::Bool(true), + "engine remains usable after rejecting a huge rule path" + ); + } + + Ok(()) +} + #[test] fn extension() -> Result<()> { fn repeat(mut params: Vec) -> Result { diff --git a/tests/parser/mod.rs b/tests/parser/mod.rs index da3ba5355..d792affc8 100644 --- a/tests/parser/mod.rs +++ b/tests/parser/mod.rs @@ -810,3 +810,27 @@ fn yaml_test(file: &str) -> Result<()> { fn run(path: &str) { yaml_test(path).unwrap() } + +#[test] +fn path_ref_component_count_is_bounded_before_ast_growth() -> Result<()> { + for path in [ + format!("package {}", ["segment"; 32].join(".")), + format!("package graph{}", r#"["literal.dot"]"#.repeat(31)), + ] { + let source = Source::from_contents("path-limit.rego".to_string(), path)?; + let mut parser = Parser::new(&source)?; + let package = parser.parse_package()?; + assert_eq!(Parser::get_path_ref_components(&package.refr)?.len(), 32); + } + + for path in [ + format!("package {}", ["segment"; 33].join(".")), + format!("package graph{}", r#"["literal.dot"]"#.repeat(32)), + ] { + let source = Source::from_contents("path-limit.rego".to_string(), path)?; + let mut parser = Parser::new(&source)?; + assert!(parser.parse_package().is_err()); + } + + Ok(()) +} diff --git a/tests/rvm/rego/mod.rs b/tests/rvm/rego/mod.rs index 5d5b179d8..2190ebb13 100644 --- a/tests/rvm/rego/mod.rs +++ b/tests/rvm/rego/mod.rs @@ -963,6 +963,154 @@ fn numeric_namespace_selectors_keep_string_identity_on_reused_vm() -> Result<()> Ok(()) } +#[test] +fn dynamic_namespace_package_prefetch_matches_in_both_vm_modes() -> Result<()> { + let entrypoint = "data.framework.allow"; + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data.config[input.namespace] + cfg.blocked + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package config.target\nblocked := true".to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + let cases = [ + ( + Value::from_json_str(r#"{"namespace":"target"}"#)?, + Value::Bool(false), + ), + (Value::new_object(), Value::Bool(true)), + (Value::Undefined, Value::Bool(true)), + ( + Value::from_json_str(r#"{"namespace":"target"}"#)?, + Value::Bool(false), + ), + ]; + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + + for (input, expected) in &cases { + vm.set_input(input.clone()); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + *expected, + "{mode:?} result for input {input:?}" + ); + } + } + + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_does_not_prefetch_a_trailing_sibling_rule() -> Result<()> { + let entrypoint = "data.framework.result"; + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + "package framework\nresult := data[input.namespace].value".to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nvalue := 7\nunrelated := data.framework.result".to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + vm.set_input(input.clone()); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + Value::from(7), + "{mode:?} result" + ); + } + + Ok(()) +} + +#[test] +fn dynamic_namespace_lookup_materializes_a_trailing_subpackage_without_parent_siblings( +) -> Result<()> { + let entrypoint = "data.framework.config"; + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + "package framework\nconfig := data[input.namespace].nested".to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nunrelated := data.framework.config".to_string(), + )?; + engine.add_policy( + "nested.rego".to_string(), + "package target.nested\nvalue := 7".to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + let expected = Value::from_json_str(r#"{"value":7}"#)?; + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + vm.set_input(input.clone()); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + expected, + "{mode:?} result" + ); + } + + Ok(()) +} + +#[test] +fn rvm_compiler_rejects_large_entrypoints_with_many_components() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "base.rego".to_string(), + "package base\nvalue := true".to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&"data.base.value".into())?; + let paths = [ + format!("data{}", ".a".repeat(300_000)), + format!("data{}", r#"["a"]"#.repeat(180_000)), + ]; + + for path in &paths { + assert!(path.len() < 1024 * 1024); + assert!(Compiler::compile_from_policy(&compiled, &[path]).is_err()); + } + + Ok(()) +} + #[test] fn numeric_object_selectors_do_not_use_data_namespace_fallback() -> Result<()> { let entrypoint = "data.consumer.result"; From 963f2f64ba3ffd8a4adff12807530a3a338ecfc3 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Tue, 6 Oct 2026 22:26:02 +0200 Subject: [PATCH 4/7] fix: order selected-package defaults across policy files Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/interpreter.rs | 28 ++++++- tests/engine/mod.rs | 175 ++++++++++++++++++++++++++++++++++++++++++ tests/rvm/rego/mod.rs | 51 ++++++++++++ 3 files changed, 250 insertions(+), 4 deletions(-) diff --git a/src/interpreter.rs b/src/interpreter.rs index 9ed31cee6..9ba57524b 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -3192,12 +3192,32 @@ impl Interpreter { } let prev_module = self.set_current_module(Some(module.clone()))?; - for rule in &module.policy { - if !self.processed.contains(rule) { - self.eval_default_rule(rule)?; + let default_result = (|| -> Result<()> { + for rule in &module.policy { + if !self.processed.contains(rule) { + if let Rule::Default { refr, args, .. } = rule.as_ref() { + if args.is_empty() { + let refr = match refr.as_ref() { + Expr::RefBrack { refr, .. } => refr, + _ => refr, + }; + let mut rule_path = + vec![PathComponent::String("data".to_string())]; + rule_path + .extend(get_rule_path_components(&module.package.refr)?); + rule_path.extend(get_rule_path_components(refr)?); + self.ensure_rule_evaluated(format_path_components( + &rule_path, + )?)?; + } + } + self.eval_default_rule(rule)?; + } } - } + Ok(()) + })(); self.set_current_module(prev_module)?; + default_result?; self.mark_processed(&module_value_components)?; } } diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index c5320705c..9c272ac97 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -313,6 +313,181 @@ fn dynamic_data_root_lookup_does_not_evaluate_unrelated_modules() -> Result<()> Ok(()) } +#[test] +fn dynamic_namespace_defaults_follow_ordinary_rules_across_file_orders() -> Result<()> { + for default_first in [true, false] { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data[input.namespace] + cfg.blocked + } + "# + .to_string(), + )?; + + let default_policy = "package target\ndefault blocked := false".to_string(); + let ordinary_policy = "package target\nblocked := true".to_string(); + if default_first { + engine.add_policy("default.rego".to_string(), default_policy)?; + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + } else { + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + engine.add_policy("default.rego".to_string(), default_policy)?; + } + engine.add_policy( + "unrelated.rego".to_string(), + "package unrelated\nfirst := second\nsecond := first".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.allow".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Bool(false)); + assert_eq!( + engine.eval_rule(entrypoint.to_string())?, + Value::Bool(false) + ); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(true), + "ordinary target rule must win regardless of file order" + ); + } + + Ok(()) +} + +#[test] +fn dynamic_namespace_with_failure_restores_default_module_context() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + f(x) := x + result := "primary" if { f(0) with f as data[input.namespace] } else := "fallback" if { true } + "# + .to_string(), + )?; + engine.add_policy( + "target-default.rego".to_string(), + "package target\ndefault blocked := false".to_string(), + )?; + engine.add_policy( + "target-true.rego".to_string(), + "package target\nblocked := true".to_string(), + )?; + engine.add_policy( + "target-false.rego".to_string(), + "package target\nblocked := false".to_string(), + )?; + + let entrypoint: Rc = "data.framework.result".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!( + compiled.eval_with_input(input.clone())?, + Value::from("fallback") + ); + + engine.set_input(input); + assert_eq!( + engine.eval_rule(entrypoint.to_string())?, + Value::from("fallback") + ); + + let followup = engine.eval_query("data.framework.f(7)".to_string(), false)?; + assert_eq!( + followup.result[0].expressions[0].value, + Value::from(7), + "a subsequent ordinary function call should remain usable on the same Engine" + ); + + Ok(()) +} + +#[test] +fn dynamic_namespace_default_applies_when_selected_ordinary_rule_is_undefined() -> Result<()> { + for default_first in [true, false] { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data[input.namespace] + cfg.blocked + } + "# + .to_string(), + )?; + + let default_policy = "package target\ndefault blocked := false".to_string(); + let ordinary_policy = "package target\nblocked := true if { input.enabled }".to_string(); + if default_first { + engine.add_policy("default.rego".to_string(), default_policy)?; + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + } else { + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + engine.add_policy("default.rego".to_string(), default_policy)?; + } + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.allow".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Bool(true)); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::Bool(true)); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Bool(false) + ); + } + + Ok(()) +} + +#[test] +fn dynamic_namespace_ordinary_rule_without_default_stays_undefined() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data[input.namespace] + cfg.blocked + } + "# + .to_string(), + )?; + engine.add_policy( + "target.rego".to_string(), + "package target\nblocked := true if { input.enabled }".to_string(), + )?; + engine.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + + let entrypoint: Rc = "data.framework.allow".into(); + let compiled = engine.compile_with_entrypoint(&entrypoint)?; + let input = Value::from_json_str(r#"{"namespace":"target"}"#)?; + assert_eq!(compiled.eval_with_input(input)?, Value::Bool(true)); + assert_eq!(engine.eval_rule(entrypoint.to_string())?, Value::Bool(true)); + assert_eq!( + engine.eval_rule("data.target.blocked".to_string())?, + Value::Undefined + ); + + Ok(()) +} + #[test] fn nested_dynamic_data_lookup_does_not_prefetch_a_dotted_suffix_rule() -> Result<()> { let mut engine = Engine::new(); diff --git a/tests/rvm/rego/mod.rs b/tests/rvm/rego/mod.rs index 2190ebb13..29ef13963 100644 --- a/tests/rvm/rego/mod.rs +++ b/tests/rvm/rego/mod.rs @@ -1018,6 +1018,57 @@ fn dynamic_namespace_package_prefetch_matches_in_both_vm_modes() -> Result<()> { Ok(()) } +#[test] +fn dynamic_namespace_selected_package_defaults_follow_normal_rules_in_both_vm_modes() -> Result<()> +{ + let entrypoint = "data.framework.allow"; + + for default_first in [true, false] { + let mut engine = Engine::new(); + engine.add_policy( + "framework.rego".to_string(), + r#" + package framework + default allow := true + allow := false if { + cfg := data.config[input.namespace] + cfg.blocked + } + "# + .to_string(), + )?; + + let default_policy = "package config.target\ndefault blocked := false".to_string(); + let ordinary_policy = "package config.target\nblocked := true".to_string(); + if default_first { + engine.add_policy("default.rego".to_string(), default_policy)?; + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + } else { + engine.add_policy("ordinary.rego".to_string(), ordinary_policy)?; + engine.add_policy("default.rego".to_string(), default_policy)?; + } + + let compiled = engine.compile_with_entrypoint(&entrypoint.into())?; + let program = Compiler::compile_from_policy(&compiled, &[entrypoint])?; + let data = engine.get_data(); + + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_execution_mode(mode); + vm.set_input(Value::from_json_str(r#"{"namespace":"target"}"#)?); + assert_eq!( + vm.execute_entry_point_by_name(entrypoint)?, + Value::Bool(false), + "{mode:?} result with default_first={default_first}" + ); + } + } + + Ok(()) +} + #[test] fn dynamic_namespace_lookup_does_not_prefetch_a_trailing_sibling_rule() -> Result<()> { let entrypoint = "data.framework.result"; From df47a8f841c9b923bfe59c909fc6019754fa529f Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Wed, 7 Oct 2026 13:50:20 +0200 Subject: [PATCH 5/7] fix: preserve typed overrides and deep entrypoints Keep scalar rule identities distinct during WITH suppression, resolve equivalent static entrypoint spellings at registered depths, and make registration retries and compiled publication respect memory limits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/ffi/src/engine.rs | 72 ++++++ docs/grammar.md | 17 +- src/compiled_policy.rs | 4 +- src/interpreter.rs | 204 +++++++++-------- src/interpreter/target/resolve.rs | 12 +- src/languages/rego/compiler/references.rs | 4 +- src/languages/rego/compiler/rules.rs | 70 +++--- src/parser.rs | 87 +++++++- src/utils.rs | 103 +++++++-- tests/engine/mod.rs | 260 ++++++++++++++++++++++ tests/memory_limits.rs | 160 +++++++++++++ tests/rvm/rego/mod.rs | 63 ++++++ 12 files changed, 901 insertions(+), 155 deletions(-) diff --git a/bindings/ffi/src/engine.rs b/bindings/ffi/src/engine.rs index ea097e0c4..f4cbe4537 100644 --- a/bindings/ffi/src/engine.rs +++ b/bindings/ffi/src/engine.rs @@ -908,6 +908,78 @@ mod namespace_tests { regorus_engine_drop(engine); } + #[test] + fn deep_bracketed_registered_path_matches_dotted_path_and_reuses_ffi_engine() { + let _poison_test_lock = crate::panic_guard::lock_poison_test_state(); + crate::panic_guard::reset_poison(); + + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let package_components = (0..31).map(|index| format!("p{index}")).collect::>(); + let package_path = package_components.join("."); + let dotted_path = format!("data.{package_path}.value"); + let mut bracketed_path = String::from("data"); + for component in &package_components { + bracketed_path.push_str(&format!("[\"{component}\"]")); + } + bracketed_path.push_str("[\"value\"]"); + + let file = CString::new("deep.rego").expect("valid policy path"); + let policy = + CString::new(format!("package {package_path}\nvalue := 7")).expect("valid policy"); + let added = regorus_engine_add_policy(engine, file.as_ptr(), policy.as_ptr()); + assert!(matches!(&added.status, RegorusStatus::Ok)); + regorus_result_drop(added); + + let dotted = CString::new(dotted_path).expect("valid dotted path"); + let dotted_result = regorus_engine_eval_rule(engine, dotted.as_ptr()); + assert!(matches!(&dotted_result.status, RegorusStatus::Ok)); + assert_eq!( + unsafe { CStr::from_ptr(dotted_result.output) } + .to_str() + .expect("UTF-8 result"), + "7" + ); + regorus_result_drop(dotted_result); + + let bracketed = CString::new(bracketed_path.clone()).expect("valid bracketed path"); + let bracketed_result = regorus_engine_eval_rule(engine, bracketed.as_ptr()); + assert!( + matches!(&bracketed_result.status, RegorusStatus::Ok), + "equivalent bracketed lookup failed: {:?}", + bracketed_result.status + ); + assert_eq!( + unsafe { CStr::from_ptr(bracketed_result.output) } + .to_str() + .expect("UTF-8 result"), + "7" + ); + regorus_result_drop(bracketed_result); + + let missing_path = bracketed_path.replace("[\"value\"]", "[\"missing\"]"); + let missing = CString::new(missing_path).expect("valid missing path"); + let rejected = regorus_engine_eval_rule(engine, missing.as_ptr()); + assert!(matches!(&rejected.status, RegorusStatus::Error)); + regorus_result_drop(rejected); + assert!( + !crate::panic_guard::is_poisoned(), + "a lookup error must not poison the FFI engine" + ); + + let reused_result = regorus_engine_eval_rule(engine, bracketed.as_ptr()); + assert!(matches!(&reused_result.status, RegorusStatus::Ok)); + assert_eq!( + unsafe { CStr::from_ptr(reused_result.output) } + .to_str() + .expect("UTF-8 result"), + "7" + ); + regorus_result_drop(reused_result); + regorus_engine_drop(engine); + } + #[test] fn overdeep_policy_path_returns_an_error_and_keeps_the_engine_usable() { let engine = regorus_engine_new(); diff --git a/docs/grammar.md b/docs/grammar.md index d72d0370f..1de4f2841 100644 --- a/docs/grammar.md +++ b/docs/grammar.md @@ -160,11 +160,18 @@ For example, `graph["a.b"]` is distinct from `graph.a.b`. When a package path is returned by the Engine API or used as a rule entry point, non-identifier string components use JSON-escaped bracket notation. -Each parsed path reference is limited to 32 components, counting its initial -identifier as one. Every dotted identifier or bracketed string adds one -component; dots inside a bracketed string do not. A 33rd component is rejected -during parsing. `Engine::eval_rule` reports `not a valid rule path` for an -overlong entry point, while parser and compiler APIs return an error. +Policy-source path references are limited to 32 components, counting the +initial identifier as one. Every dotted identifier or bracketed string adds +one component; dots inside a bracketed string do not. A 33rd component is +rejected by the general policy parser. + +Static public entry-point lookup uses a separate iterative parser rather than +building the recursively nested policy AST. It accepts dotted and bracketed +string spellings up to the longest registered rule path, so equivalent +spellings remain consistent even when that path exceeds the policy parser's +32-component limit. Paths that exceed the registered depth or do not identify +a rule remain invalid; `Engine::eval_rule` reports `not a valid rule path`, +while parser and compiler APIs return an error. referenced by: diff --git a/src/compiled_policy.rs b/src/compiled_policy.rs index f401c5545..90bfe70d6 100644 --- a/src/compiled_policy.rs +++ b/src/compiled_policy.rs @@ -215,7 +215,9 @@ pub(crate) struct CompiledPolicyData { pub(crate) schedule: Option>, pub(crate) rules: Map>>, pub(crate) default_rules: Map>, - pub(crate) rule_path_components: Map>, + pub(crate) rule_path_components: Map>, + // Canonical string paths can coalesce rules with distinct typed selectors. + pub(crate) rule_path_components_by_rule: BTreeMap, Vec>, pub(crate) imports: BTreeMap>, pub(crate) functions: FunctionTable, pub(crate) rule_paths: MapSet, diff --git a/src/interpreter.rs b/src/interpreter.rs index 9ba57524b..6ea3dc2a6 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -1293,7 +1293,7 @@ impl Interpreter { &mut self, paths: &Value, path: &mut Vec, - overridden_paths: &mut Vec>, + overridden_paths: &mut Vec>, ) -> Result { let mut inherited_paths = Value::new_object(); let Value::Object(entries) = paths else { @@ -1307,8 +1307,8 @@ impl Interpreter { inherited_paths .as_object_mut()? .insert(key.clone(), value.clone()); - let mut overridden_path = vec!["data".to_string()]; - overridden_path.extend(path.iter().cloned()); + let mut overridden_path = vec![PathComponent::String("data".to_string())]; + overridden_path.extend(path.iter().cloned().map(PathComponent::String)); overridden_paths.push(overridden_path); self.memory_check()?; } else if let Value::String(component) = key { @@ -1326,36 +1326,26 @@ impl Interpreter { Ok(inherited_paths) } - fn suppress_rules_under_data_override(&mut self, overridden_path: &[String]) -> Result<()> { - let compiled_policy = self.compiled_policy.clone(); - for (rule_path, rules) in &compiled_policy.rules { - self.check_execution_time()?; - let rule_components = compiled_policy - .rule_path_components - .get(rule_path) - .ok_or_else(|| { - anyhow!("missing components for registered rule path {rule_path}") - })?; - if rule_components.starts_with(overridden_path) { - for rule in rules { - self.processed.insert(rule.clone()); - self.memory_check()?; - } - } + fn suppress_rules_under_data_override( + &mut self, + overridden_path: &[PathComponent], + ) -> Result<()> { + fn path_starts_with(path: &[PathComponent], prefix: &[PathComponent]) -> bool { + path.len() >= prefix.len() + && path + .iter() + .zip(prefix) + .all(|(path_component, prefix_component)| { + path_component.matches_path_component(prefix_component) + }) } - for (rule_path, rules) in &compiled_policy.default_rules { + + let compiled_policy = self.compiled_policy.clone(); + for (rule, rule_components) in &compiled_policy.rule_path_components_by_rule { self.check_execution_time()?; - let rule_components = compiled_policy - .rule_path_components - .get(rule_path) - .ok_or_else(|| { - anyhow!("missing components for registered default rule path {rule_path}") - })?; - if rule_components.starts_with(overridden_path) { - for (rule, _) in rules { - self.processed.insert(rule.clone()); - self.memory_check()?; - } + if path_starts_with(rule_components, overridden_path) { + self.processed.insert(rule.clone()); + self.memory_check()?; } } Ok(()) @@ -1373,7 +1363,7 @@ impl Interpreter { let processed_paths = core::mem::replace(&mut self.processed_paths, Value::new_object()); - let mut inherited_overrides = Vec::new(); + let mut inherited_overrides: Vec> = Vec::new(); let inherited_paths = match self.copy_explicit_data_with_override_markers( &processed_paths, &mut Vec::new(), @@ -1504,17 +1494,14 @@ impl Interpreter { *obj = value; if first.value() == "data" { - let overridden_path: Vec = path - .iter() - .map(|component| component.value().to_string()) - .collect(); + let overridden_path = path.clone(); let (_, overridden_subtree) = overridden_path .split_first() .ok_or_else(|| anyhow!("empty path in with modifier"))?; self.mark_data_with_override( &overridden_subtree .iter() - .map(String::as_str) + .map(PathComponent::value) .collect::>(), )?; self.suppress_rules_under_data_override(&overridden_path)?; @@ -3258,7 +3245,7 @@ impl Interpreter { .get(&path) .cloned() .ok_or_else(|| anyhow!("missing components for registered rule path {path}"))?; - let comps: Vec<&str> = components.iter().map(String::as_str).collect(); + let comps: Vec<&str> = components.iter().map(PathComponent::value).collect(); let (_, tail) = comps .split_first() .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; @@ -4026,10 +4013,11 @@ impl Interpreter { registered_components.split_first().ok_or_else(|| { anyhow!("internal error: expected rule path components") })?; - if root != "data" { + if root.value() != "data" { bail!("internal error: rule path must start with data"); } - let value_path: Vec<&str> = tail.iter().map(String::as_str).collect(); + let value_path: Vec<&str> = + tail.iter().map(PathComponent::value).collect(); let value = Self::get_value_chained(self.data.clone(), &value_path); if value != Value::Undefined { @@ -4467,17 +4455,18 @@ impl Interpreter { fn record_rule(&mut self, refr: &Ref, rule: Ref) -> Result<()> { let rule_components = get_rule_path_components(refr)?; let mut path_components = self.current_module_data_path_components()?; + let mut full_path_components = path_components.clone(); + full_path_components.extend(rule_components.iter().cloned()); + self.compiled_policy_mut() + .rule_path_components_by_rule + .insert(rule.clone(), full_path_components); for (index, component) in rule_components.iter().enumerate() { path_components.push(component.clone()); let path = format_path_components(&path_components)?; - let string_components = path_components - .iter() - .map(|path_component| path_component.value().to_string()) - .collect(); let compiled_policy = self.compiled_policy_mut(); compiled_policy .rule_path_components - .insert(path.clone(), string_components); + .insert(path.clone(), path_components.clone()); if index == rule_components.len().saturating_sub(1) { compiled_policy.rule_paths.insert(path.clone()); } @@ -4503,17 +4492,18 @@ impl Interpreter { ) -> Result<()> { let rule_components = get_rule_path_components(refr)?; let mut path_components = self.current_module_data_path_components()?; + let mut full_path_components = path_components.clone(); + full_path_components.extend(rule_components.iter().cloned()); + self.compiled_policy_mut() + .rule_path_components_by_rule + .insert(rule.clone(), full_path_components); for (idx, component) in rule_components.iter().enumerate() { path_components.push(component.clone()); let path = format_path_components(&path_components)?; - let string_components = path_components - .iter() - .map(|path_component| path_component.value().to_string()) - .collect(); let compiled_policy = self.compiled_policy_mut(); compiled_policy .rule_path_components - .insert(path.clone(), string_components); + .insert(path.clone(), path_components.clone()); if idx == rule_components.len().saturating_sub(1) { compiled_policy.rule_paths.insert(path.clone()); } @@ -4542,6 +4532,15 @@ impl Interpreter { Ok(()) } + fn clear_rule_registrations(&mut self) { + let compiled_policy = self.compiled_policy_mut(); + compiled_policy.rules.clear(); + compiled_policy.default_rules.clear(); + compiled_policy.rule_paths.clear(); + compiled_policy.rule_path_components.clear(); + compiled_policy.rule_path_components_by_rule.clear(); + } + pub fn process_imports(&mut self) -> Result<()> { for module in self.compiled_policy.modules.clone().iter() { let module_path = get_path_string(&module.package.refr, Some("data"))?; @@ -4585,49 +4584,64 @@ impl Interpreter { } pub fn gather_rules(&mut self) -> Result<()> { - for module in self.compiled_policy.modules.clone().iter() { - let prev_module = self.set_current_module(Some(module.clone()))?; - for rule in &module.policy { - let refr = Self::get_rule_refr(rule); - - if let Rule::Spec { .. } = rule.as_ref() { - // Adjust refr to ensure simple ref. - // TODO: refactor. - let refr = match refr.as_ref() { - Expr::RefBrack { index, .. } - if matches!(index.as_ref(), Expr::String { .. }) => - { - refr - } - Expr::RefBrack { refr, .. } => refr, - _ => refr, - }; - self.record_rule(refr, rule.clone())?; - } else if let Rule::Default { .. } = rule.as_ref() { - let (refr, index) = match refr.as_ref() { - // TODO: Validate the index - Expr::RefBrack { refr, index, .. } => { - if !matches!( - index.as_ref(), - Expr::Bool { .. } | Expr::Number { .. } | Expr::String { .. } - ) { - // OPA's behavior is ignoring the non-scalar index - bail!(index.span().error("index is not a scalar value")); - } + self.clear_rule_registrations(); + let result = (|| -> Result<()> { + for module in self.compiled_policy.modules.clone().iter() { + let prev_module = self.set_current_module(Some(module.clone()))?; + let registration_result = (|| -> Result<()> { + for rule in &module.policy { + let refr = Self::get_rule_refr(rule); + + if let Rule::Spec { .. } = rule.as_ref() { + // Adjust refr to ensure simple ref. + // TODO: refactor. + let refr = match refr.as_ref() { + Expr::RefBrack { index, .. } + if matches!(index.as_ref(), Expr::String { .. }) => + { + refr + } + Expr::RefBrack { refr, .. } => refr, + _ => refr, + }; + self.record_rule(refr, rule.clone())?; + self.memory_check()?; + } else if let Rule::Default { .. } = rule.as_ref() { + let (refr, index) = match refr.as_ref() { + // TODO: Validate the index + Expr::RefBrack { refr, index, .. } => { + if !matches!( + index.as_ref(), + Expr::Bool { .. } + | Expr::Number { .. } + | Expr::String { .. } + ) { + // OPA's behavior is ignoring the non-scalar index + bail!(index.span().error("index is not a scalar value")); + } - let index = self.eval_expr(index)?; + let index = self.eval_expr(index)?; - (refr, Some(index.to_string())) - } - _ => (refr, None), - }; + (refr, Some(index.to_string())) + } + _ => (refr, None), + }; - self.record_default_rule(refr, rule, index)?; - } + self.record_default_rule(refr, rule, index)?; + self.memory_check()?; + } + } + Ok(()) + })(); + self.set_current_module(prev_module)?; + registration_result?; } - self.set_current_module(prev_module)?; + Ok(()) + })(); + if result.is_err() { + self.clear_rule_registrations(); } - Ok(()) + result } pub fn add_extension( @@ -4791,7 +4805,14 @@ impl Interpreter { return Ok(path.to_string()); } - let requested_components = Parser::parse_static_path_components(path) + let max_components = self + .compiled_policy + .rule_path_components + .values() + .map(Vec::len) + .max() + .unwrap_or_default(); + let requested_components = Parser::parse_static_path_components(path, max_components) .map_err(|_| anyhow!("not a valid rule path"))?; let component_refs: Vec<&str> = requested_components.iter().map(String::as_str).collect(); let canonical_path = @@ -4816,10 +4837,10 @@ impl Interpreter { let (root, tail) = registered_components .split_first() .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; - if root != "data" { + if root.value() != "data" { bail!("internal error: rule path must start with data"); } - let tail: Vec<&str> = tail.iter().map(String::as_str).collect(); + let tail: Vec<&str> = tail.iter().map(PathComponent::value).collect(); let value = Self::get_value_chained(self.data.clone(), &tail); #[cfg(feature = "azure_policy")] @@ -4861,6 +4882,7 @@ impl Interpreter { let loop_lookup = hoister.populate(compiled_policy.modules.as_ref())?; compiled_policy.loop_hoisting_table = loop_lookup; + crate::utils::limits::enforce_memory_limit().map_err(|err| anyhow!(err))?; Ok(self.compiled_policy.clone()) } } diff --git a/src/interpreter/target/resolve.rs b/src/interpreter/target/resolve.rs index 70181fe13..ff879f26d 100644 --- a/src/interpreter/target/resolve.rs +++ b/src/interpreter/target/resolve.rs @@ -30,7 +30,7 @@ fn target_effect_path( ) -> Result<(Vec, String), TargetCompileError> { let mut components = vec!["data".to_string()]; components.extend( - Parser::parse_static_path_components(package) + Parser::parse_static_path_components(package, Parser::MAX_PATH_COMPONENTS) .map_err(|_| TargetCompileError::TargetNotFound(target_name.into()))?, ); components.push(effect_name.to_string()); @@ -154,7 +154,10 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile .get(rule_path) .is_some_and(|components| { components.len() > expected_components.len() - && components.starts_with(&expected_components) + && components + .iter() + .zip(&expected_components) + .all(|(component, expected)| component.value() == expected) }); if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only @@ -178,7 +181,10 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile .get(rule_path) .is_some_and(|components| { components.len() > expected_components.len() - && components.starts_with(&expected_components) + && components + .iter() + .zip(&expected_components) + .all(|(component, expected)| component.value() == expected) }); if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only diff --git a/src/languages/rego/compiler/references.rs b/src/languages/rego/compiler/references.rs index ca8d8c5bd..6643583aa 100644 --- a/src/languages/rego/compiler/references.rs +++ b/src/languages/rego/compiler/references.rs @@ -258,10 +258,10 @@ impl<'a> Compiler<'a> { let pattern_part = &pattern[i]; if pattern_part.is_none() { // Wildcard in pattern matches any non-empty rule component exactly - if rule_part.is_empty() { + if rule_part.value().is_empty() { return false; } - } else if pattern_part.as_ref() != Some(rule_part) { + } else if pattern_part.as_deref() != Some(rule_part.value()) { return false; } } diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index b59d98c76..614c18d4c 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -237,7 +237,10 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_refs: Vec<&str> = package_parts.iter().map(String::as_str).collect(); + let package_refs: Vec<&str> = package_parts + .iter() + .map(crate::utils::PathComponent::value) + .collect(); let package = format_string_path(&package_refs).map_err(|error| CompilerError::General { message: format!("failed to format package path for '{rule_path}': {error}"), @@ -269,30 +272,37 @@ impl<'a> Compiler<'a> { } compiler.current_rule_path = "".to_string(); let rules = policy.get_rules(); + let max_components = policy + .inner + .rule_path_components + .values() + .map(Vec::len) + .max() + .unwrap_or_default(); for &entry_point_name in entry_points { let instruction_index = compiler.program.instructions.len(); let result_reg = compiler.alloc_register(); - let rule_path = - if policy.inner.rule_paths.contains(entry_point_name) { - entry_point_name.to_string() - } else { - let components = Parser::parse_static_path_components(entry_point_name) + let rule_path = if policy.inner.rule_paths.contains(entry_point_name) { + entry_point_name.to_string() + } else { + let components = + Parser::parse_static_path_components(entry_point_name, max_components) .map_err(|error| CompilerError::General { message: error.to_string(), })?; - let component_refs: Vec<&str> = components.iter().map(String::as_str).collect(); - let canonical_path = format_string_path(&component_refs).map_err(|error| { - CompilerError::General { - message: error.to_string(), - } - })?; - if policy.inner.rule_paths.contains(&canonical_path) { - canonical_path - } else { - entry_point_name.to_string() + let component_refs: Vec<&str> = components.iter().map(String::as_str).collect(); + let canonical_path = format_string_path(&component_refs).map_err(|error| { + CompilerError::General { + message: error.to_string(), } - }; + })?; + if policy.inner.rule_paths.contains(&canonical_path) { + canonical_path + } else { + entry_point_name.to_string() + } + }; let rule_idx = compiler.get_or_assign_rule_index(&rule_path)?; compiler .entry_points @@ -744,10 +754,15 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_path = package_parts.to_vec(); - let _ = - self.program - .add_rule_to_tree(&package_path, rule_name, rule_index as usize); + let package_path: Vec = package_parts + .iter() + .map(|component| component.value().to_string()) + .collect(); + let _ = self.program.add_rule_to_tree( + &package_path, + rule_name.value(), + rule_index as usize, + ); } self.register_counter = saved_register_counter; @@ -778,10 +793,15 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_path = package_parts.to_vec(); - let _ = - self.program - .add_rule_to_tree(&package_path, rule_name, rule_index as usize); + let package_path: Vec = package_parts + .iter() + .map(|component| component.value().to_string()) + .collect(); + let _ = self.program.add_rule_to_tree( + &package_path, + rule_name.value(), + rule_index as usize, + ); } self.register_counter = saved_register_counter; diff --git a/src/parser.rs b/src/parser.rs index b82800415..cb89e5b5d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -58,6 +58,8 @@ const FUTURE_KEYWORDS: [&str; 4] = ["contains", "every", "if", "in"]; const DEFAULT_MAX_EXPR_DEPTH: usize = 32; impl<'source> Parser<'source> { + pub(crate) const MAX_PATH_COMPONENTS: usize = DEFAULT_MAX_EXPR_DEPTH; + pub fn new(source: &'source Source) -> Result { Self::new_with_max_col(source, None) } @@ -269,7 +271,10 @@ impl<'source> Parser<'source> { Ok(components) } - pub(crate) fn parse_static_path_components(path: &str) -> Result> { + pub(crate) fn parse_static_path_components( + path: &str, + max_components: usize, + ) -> Result> { let max_col = path .len() .checked_add(1) @@ -278,17 +283,87 @@ impl<'source> Parser<'source> { .ok_or_else(|| anyhow!("path exceeds maximum supported length"))?; let source = Source::from_contents("entrypoint.rego".to_string(), path.to_string())?; let mut parser = Parser::<'_>::new_with_max_col(&source, Some(max_col))?; - let expression = parser.parse_path_ref()?; + let mut components = Vec::new(); + + let root = parser.parse_var()?; + Self::push_static_path_component(&mut components, root.text().to_string(), max_components)?; + + loop { + match parser.token_text() { + "." | "[" if parser.tok.1.start != parser.end => { + bail!( + "{}", + parser.source.error( + parser.tok.1.line, + parser.tok.1.col.saturating_sub(1), + format!("invalid whitespace before {}", parser.token_text()).as_str() + ) + ); + } + "." => { + let separator = parser.tok.1.start; + parser.next_token()?; + let field = parser.parse_ref_field()?; + if field.start != separator.saturating_add(1) { + bail!( + "{}", + parser.source.error( + field.line, + field.col.saturating_sub(1), + "invalid whitespace between . and identifier" + ) + ); + } + Self::push_static_path_component( + &mut components, + field.text().to_string(), + max_components, + )?; + } + "[" => { + parser.next_token()?; + if parser.tok.0 != TokenKind::String { + bail!(parser.tok.1.error("expected string")); + } + let index = parser.parse_scalar_or_var()?; + parser.expect("]", "while parsing bracketed reference")?; + let Expr::String { value, .. } = index else { + bail!("internal error: static path component is not a string"); + }; + Self::push_static_path_component( + &mut components, + value.as_string()?.as_ref().to_string(), + max_components, + )?; + } + _ => break, + } + } if parser.tok.0 != TokenKind::Eof { bail!(parser.tok.1.error("expecting EOF")); } - Self::get_static_string_path_components(&expression) + Ok(components) + } + + fn push_static_path_component( + components: &mut Vec, + component: String, + max_components: usize, + ) -> Result<()> { + if components.len() >= max_components { + bail!("path exceeds maximum supported component count of {max_components}"); + } + components.push(component); + check_memory_limit()?; + Ok(()) } fn check_path_component_limit(&self, current_count: usize) -> Result<()> { - if current_count >= DEFAULT_MAX_EXPR_DEPTH { - let message = - alloc::format!("path exceeds maximum depth of {DEFAULT_MAX_EXPR_DEPTH} components"); + if current_count >= Self::MAX_PATH_COMPONENTS { + let message = alloc::format!( + "path exceeds maximum depth of {} components", + Self::MAX_PATH_COMPONENTS + ); bail!("{}", self.tok.1.error(&message)); } Ok(()) diff --git a/src/utils.rs b/src/utils.rs index e47669e91..555f752ee 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -13,6 +13,7 @@ pub mod limits; use crate::ast::*; use crate::builtins::*; use crate::lexer::SourceStr; +use crate::number::Number; use crate::parser::Parser; use crate::*; @@ -23,12 +24,23 @@ use anyhow::{bail, Result}; pub(crate) enum PathComponent { String(String), Raw(String), + Number { lexeme: String, value: Number }, } impl PathComponent { pub(crate) fn value(&self) -> &str { match self { Self::String(value) | Self::Raw(value) => value, + Self::Number { lexeme, .. } => lexeme, + } + } + + pub(crate) fn matches_path_component(&self, other: &Self) -> bool { + match (self, other) { + (Self::String(left), Self::String(right)) => left == right, + (Self::Raw(left), Self::Raw(right)) => left == right, + (Self::Number { value: left, .. }, Self::Number { value: right, .. }) => left == right, + _ => false, } } } @@ -40,20 +52,31 @@ fn is_identifier_component(component: &str) -> bool { } pub(crate) fn append_path_component(path: &str, component: &str) -> Result { + let mut path = path.to_string(); + append_path_component_to(&mut path, component)?; + Ok(path) +} + +fn append_path_component_to(path: &mut String, component: &str) -> Result<()> { if is_identifier_component(component) { - if path.is_empty() { - Ok(component.to_string()) - } else { - Ok(format!("{path}.{component}")) + if !path.is_empty() { + path.push('.'); } + path.push_str(component); } else { let literal = Value::from(component).to_json_str()?; - if path.is_empty() { - Ok(format!("[{literal}]")) - } else { - Ok(format!("{path}[{literal}]")) - } + path.push('['); + path.push_str(&literal); + path.push(']'); + } + Ok(()) +} + +fn append_raw_path_component_to(path: &mut String, component: &str) { + if !path.is_empty() { + path.push('.'); } + path.push_str(component); } pub(crate) fn append_path_value_component(path: &str, component: &Value) -> Result { @@ -72,21 +95,23 @@ pub(crate) fn append_path_value_component(path: &str, component: &Value) -> Resu pub(crate) fn format_path_components(components: &[PathComponent]) -> Result { let mut path = String::new(); for component in components { - path = match component { - PathComponent::String(value) => append_path_component(&path, value)?, - PathComponent::Raw(value) if path.is_empty() => value.clone(), - PathComponent::Raw(value) => format!("{path}.{value}"), - }; + match component { + PathComponent::String(value) => append_path_component_to(&mut path, value)?, + PathComponent::Raw(value) => append_raw_path_component_to(&mut path, value), + PathComponent::Number { lexeme, .. } => { + append_raw_path_component_to(&mut path, lexeme); + } + } } Ok(path) } pub(crate) fn format_string_path(components: &[&str]) -> Result { - let components: Vec = components - .iter() - .map(|component| PathComponent::String((*component).to_string())) - .collect(); - format_path_components(&components) + let mut path = String::new(); + for component in components { + append_path_component_to(&mut path, component)?; + } + Ok(path) } pub(crate) fn split_canonical_path_root(path: &str) -> Option<(&str, &str)> { @@ -113,9 +138,16 @@ pub(crate) fn get_rule_path_components(refr: &Expr) -> Result Expr::String { value, .. } => components.push(PathComponent::String( value.as_string()?.as_ref().to_string(), )), - Expr::Number { span, .. } - | Expr::Bool { span, .. } - | Expr::Null { span, .. } => { + Expr::Number { span, value, .. } => { + let Value::Number(number) = value else { + bail!("internal error: number expression has non-numeric value"); + }; + components.push(PathComponent::Number { + lexeme: span.text().to_string(), + value: number.clone(), + }); + } + Expr::Bool { span, .. } | Expr::Null { span, .. } => { components.push(PathComponent::Raw(span.text().to_string())); } _ => { @@ -138,6 +170,33 @@ pub(crate) fn get_rule_path_components(refr: &Expr) -> Result Ok(components) } +#[cfg(test)] +mod path_component_tests { + use super::*; + + #[test] + fn path_component_matching_preserves_scalar_identity_and_numeric_equality() { + let integer = PathComponent::Number { + lexeme: "1".to_string(), + value: Number::from(1_i64), + }; + let decimal = PathComponent::Number { + lexeme: "1.0".to_string(), + value: Number::from(1.0_f64), + }; + let string = PathComponent::String("1".to_string()); + let boolean = PathComponent::Raw("true".to_string()); + let boolean_string = PathComponent::String("true".to_string()); + let null = PathComponent::Raw("null".to_string()); + let null_string = PathComponent::String("null".to_string()); + + assert!(integer.matches_path_component(&decimal)); + assert!(!integer.matches_path_component(&string)); + assert!(!boolean.matches_path_component(&boolean_string)); + assert!(!null.matches_path_component(&null_string)); + } +} + pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { fn collect_simple(refr: &Expr, components: &mut Vec) -> Result<()> { match refr { diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 9c272ac97..c5c90d746 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -4,6 +4,216 @@ use anyhow::{bail, Result}; use regorus::*; +fn with_string_key_override_engine() -> Result { + let mut engine = Engine::new(); + engine.add_policy( + "policy.rego".to_string(), + r#" + package policy + items[1].blocked := true + default allow := true + allow := false if { + data.policy.items[1].blocked with data.policy.items["1"] as {"blocked": false} + } + "# + .to_string(), + )?; + Ok(engine) +} + +#[test] +fn with_string_key_override_does_not_suppress_numeric_rule() -> Result<()> { + let mut engine = with_string_key_override_engine()?; + + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(false) + ); + assert_eq!( + engine + .eval_query("data.policy.items[1].blocked".to_string(), false)? + .result[0] + .expressions[0] + .value, + Value::Bool(true), + "the with override must be restored before the next Engine query" + ); + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(false), + "the Engine remains reusable after evaluating the with modifier" + ); + + Ok(()) +} + +#[test] +fn compiled_interpreter_with_string_key_does_not_suppress_numeric_rule() -> Result<()> { + let mut engine = with_string_key_override_engine()?; + let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; + + for _ in 0..2 { + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::Bool(false), + "the compiled policy remains reusable after a data override" + ); + } + + Ok(()) +} + +#[test] +fn with_string_overrides_do_not_suppress_boolean_or_null_rules() -> Result<()> { + for key in ["true", "null"] { + let mut engine = Engine::new(); + engine.add_policy( + "policy.rego".to_string(), + format!( + r#" + package policy + items[{key}].blocked := true + default allow := true + allow := false if {{ + data.policy.items[{key}].blocked with data.policy.items["{key}"] as {{"blocked": false}} + }} + "# + ), + )?; + let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; + + for _ in 0..2 { + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(false), + "the string override must not suppress the {key} rule" + ); + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::Bool(false), + "the compiled policy must preserve {key} rule identity" + ); + } + assert_eq!( + engine + .eval_query(format!("data.policy.items[{key}].blocked"), false)? + .result[0] + .expressions[0] + .value, + Value::Bool(true), + "the original {key} rule is restored before the next query" + ); + } + + Ok(()) +} + +#[test] +fn mixed_scalar_rule_buckets_keep_override_identity_in_both_orders() -> Result<()> { + for key in ["true", "null"] { + for non_string_first in [true, false] { + let non_string_rule = format!("items[{key}].blocked := true"); + let string_rule = format!("items[\"{key}\"].blocked := false"); + let (first_rule, second_rule) = if non_string_first { + (non_string_rule, string_rule) + } else { + (string_rule, non_string_rule) + }; + let policy = format!( + r#" + package policy + {first_rule} + {second_rule} + default allow := true + allow := false if {{ + data.policy.items[{key}].blocked with data.policy.items["{key}"] as {{"blocked": false}} + }} + string_override := true if {{ + data.policy.items["{key}"].blocked with data.policy.items["{key}"] as {{"blocked": true}} + }} + "# + ); + let mut engine = Engine::new(); + engine.add_policy("policy.rego".to_string(), policy)?; + let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; + let compiled_string_override = + engine.compile_with_entrypoint(&"data.policy.string_override".into())?; + let string_path = format!("data.policy.items[\"{key}\"].blocked"); + let string_entrypoint: Rc = string_path.clone().into(); + let compiled_string_path = engine.compile_with_entrypoint(&string_entrypoint)?; + + for _ in 0..2 { + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(false), + "the string override must not suppress the {key} rule; non-string first={non_string_first}" + ); + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::Bool(false), + "compiled result for {key}; non-string first={non_string_first}" + ); + assert_eq!( + engine.eval_rule("data.policy.string_override".to_string())?, + Value::Bool(true), + "the string-key rule remains overridable for {key}; non-string first={non_string_first}" + ); + assert_eq!( + compiled_string_override.eval_with_input(Value::new_object())?, + Value::Bool(true), + "compiled string-key override for {key}; non-string first={non_string_first}" + ); + assert_eq!( + compiled_string_path.eval_with_input(Value::new_object())?, + Value::Bool(false), + "compiled string-key value is restored for {key}; non-string first={non_string_first}" + ); + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(false), + "the numeric or boolean rule remains visible after the string override for {key}; non-string first={non_string_first}" + ); + } + assert_eq!( + engine.eval_rule(string_path)?, + Value::Bool(false), + "the original string-key rule is restored for {key}; non-string first={non_string_first}" + ); + } + } + + Ok(()) +} + +#[test] +fn numeric_rule_paths_keep_integer_decimal_equivalence() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "policy.rego".to_string(), + r#" + package policy + items[1].blocked := true + allow := data.policy.items[1.0].blocked + "# + .to_string(), + )?; + let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; + + assert_eq!( + engine.eval_rule("data.policy.allow".to_string())?, + Value::Bool(true) + ); + for _ in 0..2 { + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::Bool(true), + "the compiled policy preserves 1 == 1.0 numeric path lookup" + ); + } + + Ok(()) +} + #[test] fn namespace_literal_dot_rule_path_uses_bracketed_component() -> Result<()> { let mut engine = Engine::new(); @@ -1181,6 +1391,56 @@ fn rule_path_component_limit_preserves_engine_behavior() -> Result<()> { Ok(()) } +#[test] +fn bracketed_entrypoints_match_dotted_paths_at_supported_depths() -> Result<()> { + for (package_component_count, rule_component_count) in [(31, 1), (32, 1), (32, 32)] { + let package_components = (0..package_component_count) + .map(|index| format!("p{index}")) + .collect::>(); + let rule_components = (0..rule_component_count) + .map(|index| format!("r{index}")) + .collect::>(); + let package_path = package_components.join("."); + let rule_path = rule_components.join("."); + let dotted_path = format!("data.{package_path}.{rule_path}"); + let mut bracketed_path = String::from("data"); + for component in package_components.iter().chain(&rule_components) { + bracketed_path.push_str(&format!("[\"{component}\"]")); + } + + let mut engine = Engine::new(); + engine.add_policy( + "deep.rego".to_string(), + format!("package {package_path}\n{rule_path} := 7"), + )?; + + let dotted_entrypoint: Rc = dotted_path.clone().into(); + let compiled = engine.compile_with_entrypoint(&dotted_entrypoint)?; + assert_eq!(compiled.eval_with_input(Value::Null)?, Value::from(7)); + assert_eq!( + engine.eval_rule(dotted_path.clone())?, + Value::from(7), + "dotted path with {package_component_count} package and {rule_component_count} rule components" + ); + + let bracketed_entrypoint: Rc = bracketed_path.clone().into(); + let bracketed_compiled = engine.compile_with_entrypoint(&bracketed_entrypoint)?; + assert_eq!( + bracketed_compiled.eval_with_input(Value::Null)?, + Value::from(7) + ); + for _ in 0..2 { + assert_eq!( + engine.eval_rule(bracketed_path.clone())?, + Value::from(7), + "bracketed path with {package_component_count} package and {rule_component_count} rule components" + ); + } + } + + Ok(()) +} + #[test] fn engine_rejects_huge_dotted_and_bracketed_rule_paths() -> Result<()> { let mut engine = Engine::new(); diff --git a/tests/memory_limits.rs b/tests/memory_limits.rs index 7c0dd758a..7c04cf828 100644 --- a/tests/memory_limits.rs +++ b/tests/memory_limits.rs @@ -50,6 +50,10 @@ impl LimitGuard { set_global_memory_limit(Some(limit)); } + fn disable_limit(&mut self) { + set_global_memory_limit(None); + } + fn set_with_usage_limit(&mut self, calc: F) where F: FnOnce(u64) -> u64, @@ -135,6 +139,162 @@ fn new_engine_with_module(module: &str) -> Engine { engine } +const RULE_CLAUSE_COUNT: usize = 64; +const DEEP_PACKAGE_COMPONENT_COUNT: usize = 32; + +fn repeated_clause_policy() -> (String, String) { + let package = (0..DEEP_PACKAGE_COMPONENT_COUNT) + .map(|index| format!("package_component_{index:02}_long")) + .collect::>() + .join("."); + let mut module = format!("package {package}\n\n"); + for value in 0..RULE_CLAUSE_COUNT { + module.push_str(&format!("items contains {value} if {{ true }}\n")); + } + + (module, format!("data.{package}.items")) +} + +fn assert_repeated_clause_set(value: Value) { + match value { + Value::Set(values) => assert_eq!(values.len(), RULE_CLAUSE_COUNT), + other => panic!("expected set with {RULE_CLAUSE_COUNT} values, got {other:?}"), + } +} + +#[test] +fn interpreter_rule_registration_checks_memory_and_can_retry() { + let mut guard = LimitGuard::lock(); + let (module, entrypoint) = repeated_clause_policy(); + let mut engine = Engine::new(); + engine + .add_policy("many-clauses.rego".to_string(), module) + .expect("add many-clause policy"); + let entrypoint: regorus::Rc = entrypoint.into(); + + for _ in 0..2 { + guard.set_with_additional_budget(16 * 1024); + let err = match engine.compile_with_entrypoint(&entrypoint) { + Ok(_) => panic!("expected rule registration to hit the memory limit"), + Err(err) => err, + }; + assert_memory_limit_error(&err); + } + + guard.disable_limit(); + let compiled = engine + .compile_with_entrypoint(&entrypoint) + .expect("retry compilation with a relaxed memory limit"); + let registered_rules = compiled + .get_rules() + .get(entrypoint.as_ref()) + .expect("deep entrypoint rules are registered after retry"); + assert_eq!(registered_rules.len(), RULE_CLAUSE_COUNT); + assert_repeated_clause_set( + compiled + .eval_with_input(Value::Undefined) + .expect("evaluate compiled policy after retry"), + ); + assert_repeated_clause_set( + engine + .eval_rule(entrypoint.to_string()) + .expect("reuse engine after registration retry"), + ); +} + +#[test] +fn interpreter_indexed_default_registration_can_retry_after_memory_limit() { + let mut guard = LimitGuard::lock(); + let (mut module, entrypoint) = repeated_clause_policy(); + let package_end = module.find("\n\n").expect("package declaration") + 2; + module.insert_str(package_end, "default indexed[true] = \"bool_true\"\n\n"); + let default_query = format!( + "{}.indexed[true]", + entrypoint + .strip_suffix(".items") + .expect("entrypoint ends with items") + ); + let mut engine = Engine::new(); + engine + .add_policy("many-clauses.rego".to_string(), module) + .expect("add many-clause policy with indexed default"); + let entrypoint: regorus::Rc = entrypoint.into(); + let retained = engine + .compile_with_entrypoint(&entrypoint) + .expect("compile policy before constrained re-preparation"); + engine.clear_data(); + + for _ in 0..2 { + guard.set_with_additional_budget(16 * 1024); + let err = match engine.compile_with_entrypoint(&entrypoint) { + Ok(_) => panic!("expected indexed default registration to hit the memory limit"), + Err(err) => err, + }; + assert_memory_limit_error(&err); + } + + guard.disable_limit(); + let compiled = engine + .compile_with_entrypoint(&entrypoint) + .expect("retry indexed default registration with a relaxed memory limit"); + let registered_rules = compiled + .get_rules() + .get(entrypoint.as_ref()) + .expect("deep entrypoint rules are registered after retry"); + assert_eq!(registered_rules.len(), RULE_CLAUSE_COUNT); + assert_repeated_clause_set( + retained + .eval_with_input(Value::Undefined) + .expect("retained compiled policy remains usable after retry"), + ); + assert_repeated_clause_set( + compiled + .eval_with_input(Value::Undefined) + .expect("evaluate compiled policy after retry"), + ); + let default_result = engine + .eval_query(default_query, false) + .expect("evaluate indexed default after retry"); + assert_eq!( + default_result.result[0].expressions[0].value, + Value::from("bool_true") + ); +} + +#[test] +fn interpreter_compile_checks_memory_before_publishing_cow_clone() { + let mut guard = LimitGuard::lock(); + let (module, entrypoint) = repeated_clause_policy(); + let mut engine = Engine::new(); + engine + .add_policy("many-clauses.rego".to_string(), module) + .expect("add many-clause policy"); + let entrypoint: regorus::Rc = entrypoint.into(); + let first_compiled = engine + .compile_with_entrypoint(&entrypoint) + .expect("compile first policy"); + + let baseline = global_allocation_stats_snapshot().allocated as u64; + guard.set_absolute_limit(baseline.saturating_add(16 * 1024)); + let err = match engine.compile_with_entrypoint(&entrypoint) { + Ok(_) => panic!("expected recompilation clone to exceed the memory limit"), + Err(err) => err, + }; + assert_memory_limit_error(&err); + + guard.disable_limit(); + assert_repeated_clause_set( + first_compiled + .eval_with_input(Value::Undefined) + .expect("first compiled policy remains usable"), + ); + assert_repeated_clause_set( + engine + .eval_rule(entrypoint.to_string()) + .expect("engine remains usable after failed recompilation"), + ); +} + #[cfg(feature = "rvm")] fn memory_budget(limit: u64) -> MemoryBudgetConfig { MemoryBudgetConfig { diff --git a/tests/rvm/rego/mod.rs b/tests/rvm/rego/mod.rs index 29ef13963..c47c6eef2 100644 --- a/tests/rvm/rego/mod.rs +++ b/tests/rvm/rego/mod.rs @@ -1275,6 +1275,69 @@ fn bracketed_identifier_entrypoint_roundtrips_and_executes_by_name_and_index() - Ok(()) } +#[test] +fn bracketed_entrypoints_match_dotted_paths_at_supported_depths_and_reuse_vm() -> Result<()> { + for (package_component_count, rule_component_count) in [(31, 1), (32, 1), (32, 32)] { + let package_components = (0..package_component_count) + .map(|index| format!("p{index}")) + .collect::>(); + let rule_components = (0..rule_component_count) + .map(|index| format!("r{index}")) + .collect::>(); + let package_path = package_components.join("."); + let rule_path = rule_components.join("."); + let dotted_path = format!("data.{package_path}.{rule_path}"); + let mut bracketed_path = String::from("data"); + for component in package_components.iter().chain(&rule_components) { + bracketed_path.push_str(&format!("[\"{component}\"]")); + } + + let mut engine = Engine::new(); + engine.add_policy( + "deep.rego".to_string(), + format!("package {package_path}\n{rule_path} := 7"), + )?; + let dotted_entrypoint: Rc = dotted_path.into(); + let compiled = engine.compile_with_entrypoint(&dotted_entrypoint)?; + let program = Compiler::compile_from_policy(&compiled, &[bracketed_path.as_str()])?; + + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let json_program = Program::deserialize_json(&json).map_err(anyhow::Error::msg)?; + let binary = program.serialize_binary().map_err(anyhow::Error::msg)?; + let binary_program = + match Program::deserialize_binary(&binary).map_err(anyhow::Error::msg)? { + DeserializationResult::Complete(program) => program, + DeserializationResult::Partial(_) => { + anyhow::bail!("deep namespace program unexpectedly needs recompilation") + } + }; + + for (program, mode) in [ + (json_program, ExecutionMode::RunToCompletion), + (binary_program, ExecutionMode::Suspendable), + ] { + let mut vm = RegoVM::new(); + vm.load_program(Arc::new(program)); + vm.set_data(Value::new_object())?; + vm.set_execution_mode(mode); + for _ in 0..2 { + assert_eq!( + vm.execute_entry_point_by_name(&bracketed_path)?, + Value::from(7), + "{mode:?} named path with {package_component_count} package and {rule_component_count} rule components" + ); + assert_eq!( + vm.execute_entry_point_by_index(0)?, + Value::from(7), + "{mode:?} indexed path with {package_component_count} package and {rule_component_count} rule components" + ); + } + } + } + + Ok(()) +} + #[test_resources("tests/rvm/rego/cases/*.yaml")] fn run_rego_compiler_yaml(file: &str) { yaml_test_impl(file).unwrap(); From d60a5053cddf2a39c0cc87cc335cde5d753860c8 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Wed, 7 Oct 2026 19:01:32 +0200 Subject: [PATCH 6/7] fix: defer scalar-key override identity Restore legacy scalar-key suppression for with overrides while preserving literal-dot components, registered-depth lookup and resource cleanup. Leave numeric/string ambiguity for a separate follow-up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/compiled_policy.rs | 4 +- src/interpreter.rs | 97 +++++++----- src/interpreter/target/resolve.rs | 10 +- src/languages/rego/compiler/references.rs | 4 +- src/languages/rego/compiler/rules.rs | 31 ++-- src/utils.rs | 55 +------ tests/engine/mod.rs | 181 ---------------------- tests/memory_limits.rs | 2 +- 8 files changed, 72 insertions(+), 312 deletions(-) diff --git a/src/compiled_policy.rs b/src/compiled_policy.rs index 90bfe70d6..f401c5545 100644 --- a/src/compiled_policy.rs +++ b/src/compiled_policy.rs @@ -215,9 +215,7 @@ pub(crate) struct CompiledPolicyData { pub(crate) schedule: Option>, pub(crate) rules: Map>>, pub(crate) default_rules: Map>, - pub(crate) rule_path_components: Map>, - // Canonical string paths can coalesce rules with distinct typed selectors. - pub(crate) rule_path_components_by_rule: BTreeMap, Vec>, + pub(crate) rule_path_components: Map>, pub(crate) imports: BTreeMap>, pub(crate) functions: FunctionTable, pub(crate) rule_paths: MapSet, diff --git a/src/interpreter.rs b/src/interpreter.rs index 6ea3dc2a6..be39a1963 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -1293,7 +1293,7 @@ impl Interpreter { &mut self, paths: &Value, path: &mut Vec, - overridden_paths: &mut Vec>, + overridden_paths: &mut Vec>, ) -> Result { let mut inherited_paths = Value::new_object(); let Value::Object(entries) = paths else { @@ -1307,8 +1307,8 @@ impl Interpreter { inherited_paths .as_object_mut()? .insert(key.clone(), value.clone()); - let mut overridden_path = vec![PathComponent::String("data".to_string())]; - overridden_path.extend(path.iter().cloned().map(PathComponent::String)); + let mut overridden_path = vec!["data".to_string()]; + overridden_path.extend(path.iter().cloned()); overridden_paths.push(overridden_path); self.memory_check()?; } else if let Value::String(component) = key { @@ -1326,26 +1326,38 @@ impl Interpreter { Ok(inherited_paths) } - fn suppress_rules_under_data_override( - &mut self, - overridden_path: &[PathComponent], - ) -> Result<()> { - fn path_starts_with(path: &[PathComponent], prefix: &[PathComponent]) -> bool { - path.len() >= prefix.len() - && path - .iter() - .zip(prefix) - .all(|(path_component, prefix_component)| { - path_component.matches_path_component(prefix_component) - }) - } - + fn suppress_rules_under_data_override(&mut self, overridden_path: &[String]) -> Result<()> { let compiled_policy = self.compiled_policy.clone(); - for (rule, rule_components) in &compiled_policy.rule_path_components_by_rule { + for (rule_path, rules) in &compiled_policy.rules { self.check_execution_time()?; - if path_starts_with(rule_components, overridden_path) { - self.processed.insert(rule.clone()); - self.memory_check()?; + let rule_components = compiled_policy + .rule_path_components + .get(rule_path) + .ok_or_else(|| { + anyhow!("missing components for registered rule path {rule_path}") + })?; + if rule_components.starts_with(overridden_path) { + for rule in rules { + self.check_execution_time()?; + self.processed.insert(rule.clone()); + self.memory_check()?; + } + } + } + for (rule_path, rules) in &compiled_policy.default_rules { + self.check_execution_time()?; + let rule_components = compiled_policy + .rule_path_components + .get(rule_path) + .ok_or_else(|| { + anyhow!("missing components for registered default rule path {rule_path}") + })?; + if rule_components.starts_with(overridden_path) { + for (rule, _) in rules { + self.check_execution_time()?; + self.processed.insert(rule.clone()); + self.memory_check()?; + } } } Ok(()) @@ -1363,7 +1375,7 @@ impl Interpreter { let processed_paths = core::mem::replace(&mut self.processed_paths, Value::new_object()); - let mut inherited_overrides: Vec> = Vec::new(); + let mut inherited_overrides: Vec> = Vec::new(); let inherited_paths = match self.copy_explicit_data_with_override_markers( &processed_paths, &mut Vec::new(), @@ -1494,14 +1506,17 @@ impl Interpreter { *obj = value; if first.value() == "data" { - let overridden_path = path.clone(); + let overridden_path: Vec = path + .iter() + .map(|component| component.value().to_string()) + .collect(); let (_, overridden_subtree) = overridden_path .split_first() .ok_or_else(|| anyhow!("empty path in with modifier"))?; self.mark_data_with_override( &overridden_subtree .iter() - .map(PathComponent::value) + .map(String::as_str) .collect::>(), )?; self.suppress_rules_under_data_override(&overridden_path)?; @@ -3245,7 +3260,7 @@ impl Interpreter { .get(&path) .cloned() .ok_or_else(|| anyhow!("missing components for registered rule path {path}"))?; - let comps: Vec<&str> = components.iter().map(PathComponent::value).collect(); + let comps: Vec<&str> = components.iter().map(String::as_str).collect(); let (_, tail) = comps .split_first() .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; @@ -4013,11 +4028,10 @@ impl Interpreter { registered_components.split_first().ok_or_else(|| { anyhow!("internal error: expected rule path components") })?; - if root.value() != "data" { + if root != "data" { bail!("internal error: rule path must start with data"); } - let value_path: Vec<&str> = - tail.iter().map(PathComponent::value).collect(); + let value_path: Vec<&str> = tail.iter().map(String::as_str).collect(); let value = Self::get_value_chained(self.data.clone(), &value_path); if value != Value::Undefined { @@ -4455,18 +4469,17 @@ impl Interpreter { fn record_rule(&mut self, refr: &Ref, rule: Ref) -> Result<()> { let rule_components = get_rule_path_components(refr)?; let mut path_components = self.current_module_data_path_components()?; - let mut full_path_components = path_components.clone(); - full_path_components.extend(rule_components.iter().cloned()); - self.compiled_policy_mut() - .rule_path_components_by_rule - .insert(rule.clone(), full_path_components); for (index, component) in rule_components.iter().enumerate() { path_components.push(component.clone()); let path = format_path_components(&path_components)?; + let string_components = path_components + .iter() + .map(|path_component| path_component.value().to_string()) + .collect(); let compiled_policy = self.compiled_policy_mut(); compiled_policy .rule_path_components - .insert(path.clone(), path_components.clone()); + .insert(path.clone(), string_components); if index == rule_components.len().saturating_sub(1) { compiled_policy.rule_paths.insert(path.clone()); } @@ -4492,18 +4505,17 @@ impl Interpreter { ) -> Result<()> { let rule_components = get_rule_path_components(refr)?; let mut path_components = self.current_module_data_path_components()?; - let mut full_path_components = path_components.clone(); - full_path_components.extend(rule_components.iter().cloned()); - self.compiled_policy_mut() - .rule_path_components_by_rule - .insert(rule.clone(), full_path_components); for (idx, component) in rule_components.iter().enumerate() { path_components.push(component.clone()); let path = format_path_components(&path_components)?; + let string_components = path_components + .iter() + .map(|path_component| path_component.value().to_string()) + .collect(); let compiled_policy = self.compiled_policy_mut(); compiled_policy .rule_path_components - .insert(path.clone(), path_components.clone()); + .insert(path.clone(), string_components); if idx == rule_components.len().saturating_sub(1) { compiled_policy.rule_paths.insert(path.clone()); } @@ -4538,7 +4550,6 @@ impl Interpreter { compiled_policy.default_rules.clear(); compiled_policy.rule_paths.clear(); compiled_policy.rule_path_components.clear(); - compiled_policy.rule_path_components_by_rule.clear(); } pub fn process_imports(&mut self) -> Result<()> { @@ -4837,10 +4848,10 @@ impl Interpreter { let (root, tail) = registered_components .split_first() .ok_or_else(|| anyhow!("internal error: expected rule path components"))?; - if root.value() != "data" { + if root != "data" { bail!("internal error: rule path must start with data"); } - let tail: Vec<&str> = tail.iter().map(PathComponent::value).collect(); + let tail: Vec<&str> = tail.iter().map(String::as_str).collect(); let value = Self::get_value_chained(self.data.clone(), &tail); #[cfg(feature = "azure_policy")] diff --git a/src/interpreter/target/resolve.rs b/src/interpreter/target/resolve.rs index ff879f26d..1aa5cafa7 100644 --- a/src/interpreter/target/resolve.rs +++ b/src/interpreter/target/resolve.rs @@ -154,10 +154,7 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile .get(rule_path) .is_some_and(|components| { components.len() > expected_components.len() - && components - .iter() - .zip(&expected_components) - .all(|(component, expected)| component.value() == expected) + && components.starts_with(&expected_components) }); if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only @@ -181,10 +178,7 @@ pub fn resolve_effect(interpreter: &mut Interpreter) -> Result<(), TargetCompile .get(rule_path) .is_some_and(|components| { components.len() > expected_components.len() - && components - .iter() - .zip(&expected_components) - .all(|(component, expected)| component.value() == expected) + && components.starts_with(&expected_components) }); if is_sub_path { // Sub-paths are not allowed for effects - they must be exact matches only diff --git a/src/languages/rego/compiler/references.rs b/src/languages/rego/compiler/references.rs index 6643583aa..ca8d8c5bd 100644 --- a/src/languages/rego/compiler/references.rs +++ b/src/languages/rego/compiler/references.rs @@ -258,10 +258,10 @@ impl<'a> Compiler<'a> { let pattern_part = &pattern[i]; if pattern_part.is_none() { // Wildcard in pattern matches any non-empty rule component exactly - if rule_part.value().is_empty() { + if rule_part.is_empty() { return false; } - } else if pattern_part.as_deref() != Some(rule_part.value()) { + } else if pattern_part.as_ref() != Some(rule_part) { return false; } } diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index 614c18d4c..abd6b0641 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -237,10 +237,7 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_refs: Vec<&str> = package_parts - .iter() - .map(crate::utils::PathComponent::value) - .collect(); + let package_refs: Vec<&str> = package_parts.iter().map(String::as_str).collect(); let package = format_string_path(&package_refs).map_err(|error| CompilerError::General { message: format!("failed to format package path for '{rule_path}': {error}"), @@ -754,15 +751,10 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_path: Vec = package_parts - .iter() - .map(|component| component.value().to_string()) - .collect(); - let _ = self.program.add_rule_to_tree( - &package_path, - rule_name.value(), - rule_index as usize, - ); + let package_path = package_parts.to_vec(); + let _ = + self.program + .add_rule_to_tree(&package_path, rule_name, rule_index as usize); } self.register_counter = saved_register_counter; @@ -793,15 +785,10 @@ impl<'a> Compiler<'a> { .ok_or_else(|| CompilerError::General { message: format!("invalid registered rule path '{rule_path}'"), })?; - let package_path: Vec = package_parts - .iter() - .map(|component| component.value().to_string()) - .collect(); - let _ = self.program.add_rule_to_tree( - &package_path, - rule_name.value(), - rule_index as usize, - ); + let package_path = package_parts.to_vec(); + let _ = + self.program + .add_rule_to_tree(&package_path, rule_name, rule_index as usize); } self.register_counter = saved_register_counter; diff --git a/src/utils.rs b/src/utils.rs index 555f752ee..e0c2bebad 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -13,7 +13,6 @@ pub mod limits; use crate::ast::*; use crate::builtins::*; use crate::lexer::SourceStr; -use crate::number::Number; use crate::parser::Parser; use crate::*; @@ -24,23 +23,12 @@ use anyhow::{bail, Result}; pub(crate) enum PathComponent { String(String), Raw(String), - Number { lexeme: String, value: Number }, } impl PathComponent { pub(crate) fn value(&self) -> &str { match self { Self::String(value) | Self::Raw(value) => value, - Self::Number { lexeme, .. } => lexeme, - } - } - - pub(crate) fn matches_path_component(&self, other: &Self) -> bool { - match (self, other) { - (Self::String(left), Self::String(right)) => left == right, - (Self::Raw(left), Self::Raw(right)) => left == right, - (Self::Number { value: left, .. }, Self::Number { value: right, .. }) => left == right, - _ => false, } } } @@ -98,9 +86,6 @@ pub(crate) fn format_path_components(components: &[PathComponent]) -> Result append_path_component_to(&mut path, value)?, PathComponent::Raw(value) => append_raw_path_component_to(&mut path, value), - PathComponent::Number { lexeme, .. } => { - append_raw_path_component_to(&mut path, lexeme); - } } } Ok(path) @@ -138,16 +123,9 @@ pub(crate) fn get_rule_path_components(refr: &Expr) -> Result Expr::String { value, .. } => components.push(PathComponent::String( value.as_string()?.as_ref().to_string(), )), - Expr::Number { span, value, .. } => { - let Value::Number(number) = value else { - bail!("internal error: number expression has non-numeric value"); - }; - components.push(PathComponent::Number { - lexeme: span.text().to_string(), - value: number.clone(), - }); - } - Expr::Bool { span, .. } | Expr::Null { span, .. } => { + Expr::Number { span, .. } + | Expr::Bool { span, .. } + | Expr::Null { span, .. } => { components.push(PathComponent::Raw(span.text().to_string())); } _ => { @@ -170,33 +148,6 @@ pub(crate) fn get_rule_path_components(refr: &Expr) -> Result Ok(components) } -#[cfg(test)] -mod path_component_tests { - use super::*; - - #[test] - fn path_component_matching_preserves_scalar_identity_and_numeric_equality() { - let integer = PathComponent::Number { - lexeme: "1".to_string(), - value: Number::from(1_i64), - }; - let decimal = PathComponent::Number { - lexeme: "1.0".to_string(), - value: Number::from(1.0_f64), - }; - let string = PathComponent::String("1".to_string()); - let boolean = PathComponent::Raw("true".to_string()); - let boolean_string = PathComponent::String("true".to_string()); - let null = PathComponent::Raw("null".to_string()); - let null_string = PathComponent::String("null".to_string()); - - assert!(integer.matches_path_component(&decimal)); - assert!(!integer.matches_path_component(&string)); - assert!(!boolean.matches_path_component(&boolean_string)); - assert!(!null.matches_path_component(&null_string)); - } -} - pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { fn collect_simple(refr: &Expr, components: &mut Vec) -> Result<()> { match refr { diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index c5c90d746..36cbee0ab 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -4,187 +4,6 @@ use anyhow::{bail, Result}; use regorus::*; -fn with_string_key_override_engine() -> Result { - let mut engine = Engine::new(); - engine.add_policy( - "policy.rego".to_string(), - r#" - package policy - items[1].blocked := true - default allow := true - allow := false if { - data.policy.items[1].blocked with data.policy.items["1"] as {"blocked": false} - } - "# - .to_string(), - )?; - Ok(engine) -} - -#[test] -fn with_string_key_override_does_not_suppress_numeric_rule() -> Result<()> { - let mut engine = with_string_key_override_engine()?; - - assert_eq!( - engine.eval_rule("data.policy.allow".to_string())?, - Value::Bool(false) - ); - assert_eq!( - engine - .eval_query("data.policy.items[1].blocked".to_string(), false)? - .result[0] - .expressions[0] - .value, - Value::Bool(true), - "the with override must be restored before the next Engine query" - ); - assert_eq!( - engine.eval_rule("data.policy.allow".to_string())?, - Value::Bool(false), - "the Engine remains reusable after evaluating the with modifier" - ); - - Ok(()) -} - -#[test] -fn compiled_interpreter_with_string_key_does_not_suppress_numeric_rule() -> Result<()> { - let mut engine = with_string_key_override_engine()?; - let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; - - for _ in 0..2 { - assert_eq!( - compiled.eval_with_input(Value::new_object())?, - Value::Bool(false), - "the compiled policy remains reusable after a data override" - ); - } - - Ok(()) -} - -#[test] -fn with_string_overrides_do_not_suppress_boolean_or_null_rules() -> Result<()> { - for key in ["true", "null"] { - let mut engine = Engine::new(); - engine.add_policy( - "policy.rego".to_string(), - format!( - r#" - package policy - items[{key}].blocked := true - default allow := true - allow := false if {{ - data.policy.items[{key}].blocked with data.policy.items["{key}"] as {{"blocked": false}} - }} - "# - ), - )?; - let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; - - for _ in 0..2 { - assert_eq!( - engine.eval_rule("data.policy.allow".to_string())?, - Value::Bool(false), - "the string override must not suppress the {key} rule" - ); - assert_eq!( - compiled.eval_with_input(Value::new_object())?, - Value::Bool(false), - "the compiled policy must preserve {key} rule identity" - ); - } - assert_eq!( - engine - .eval_query(format!("data.policy.items[{key}].blocked"), false)? - .result[0] - .expressions[0] - .value, - Value::Bool(true), - "the original {key} rule is restored before the next query" - ); - } - - Ok(()) -} - -#[test] -fn mixed_scalar_rule_buckets_keep_override_identity_in_both_orders() -> Result<()> { - for key in ["true", "null"] { - for non_string_first in [true, false] { - let non_string_rule = format!("items[{key}].blocked := true"); - let string_rule = format!("items[\"{key}\"].blocked := false"); - let (first_rule, second_rule) = if non_string_first { - (non_string_rule, string_rule) - } else { - (string_rule, non_string_rule) - }; - let policy = format!( - r#" - package policy - {first_rule} - {second_rule} - default allow := true - allow := false if {{ - data.policy.items[{key}].blocked with data.policy.items["{key}"] as {{"blocked": false}} - }} - string_override := true if {{ - data.policy.items["{key}"].blocked with data.policy.items["{key}"] as {{"blocked": true}} - }} - "# - ); - let mut engine = Engine::new(); - engine.add_policy("policy.rego".to_string(), policy)?; - let compiled = engine.compile_with_entrypoint(&"data.policy.allow".into())?; - let compiled_string_override = - engine.compile_with_entrypoint(&"data.policy.string_override".into())?; - let string_path = format!("data.policy.items[\"{key}\"].blocked"); - let string_entrypoint: Rc = string_path.clone().into(); - let compiled_string_path = engine.compile_with_entrypoint(&string_entrypoint)?; - - for _ in 0..2 { - assert_eq!( - engine.eval_rule("data.policy.allow".to_string())?, - Value::Bool(false), - "the string override must not suppress the {key} rule; non-string first={non_string_first}" - ); - assert_eq!( - compiled.eval_with_input(Value::new_object())?, - Value::Bool(false), - "compiled result for {key}; non-string first={non_string_first}" - ); - assert_eq!( - engine.eval_rule("data.policy.string_override".to_string())?, - Value::Bool(true), - "the string-key rule remains overridable for {key}; non-string first={non_string_first}" - ); - assert_eq!( - compiled_string_override.eval_with_input(Value::new_object())?, - Value::Bool(true), - "compiled string-key override for {key}; non-string first={non_string_first}" - ); - assert_eq!( - compiled_string_path.eval_with_input(Value::new_object())?, - Value::Bool(false), - "compiled string-key value is restored for {key}; non-string first={non_string_first}" - ); - assert_eq!( - engine.eval_rule("data.policy.allow".to_string())?, - Value::Bool(false), - "the numeric or boolean rule remains visible after the string override for {key}; non-string first={non_string_first}" - ); - } - assert_eq!( - engine.eval_rule(string_path)?, - Value::Bool(false), - "the original string-key rule is restored for {key}; non-string first={non_string_first}" - ); - } - } - - Ok(()) -} - #[test] fn numeric_rule_paths_keep_integer_decimal_equivalence() -> Result<()> { let mut engine = Engine::new(); diff --git a/tests/memory_limits.rs b/tests/memory_limits.rs index 7c04cf828..9e376df18 100644 --- a/tests/memory_limits.rs +++ b/tests/memory_limits.rs @@ -173,7 +173,7 @@ fn interpreter_rule_registration_checks_memory_and_can_retry() { let entrypoint: regorus::Rc = entrypoint.into(); for _ in 0..2 { - guard.set_with_additional_budget(16 * 1024); + guard.set_with_additional_budget(8 * 1024); let err = match engine.compile_with_entrypoint(&entrypoint) { Ok(_) => panic!("expected rule registration to hit the memory limit"), Err(err) => err, From 441e838ccff4c21bfa2b2ff785b813884a6c0c18 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Thu, 8 Oct 2026 23:14:59 +0200 Subject: [PATCH 7/7] fix: preserve legacy public package metadata Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/csharp/Regorus.Tests/RegorusTests.cs | 12 +- bindings/csharp/Regorus/Engine.cs | 16 +++ bindings/ffi/src/engine.rs | 51 +++++++- src/engine.rs | 83 ++++++++++--- .../fixtures/legacy_package_projection.rego | 3 + tests/engine/mod.rs | 114 ++++++++++++++++-- 6 files changed, 246 insertions(+), 33 deletions(-) create mode 100644 tests/engine/fixtures/legacy_package_projection.rego diff --git a/bindings/csharp/Regorus.Tests/RegorusTests.cs b/bindings/csharp/Regorus.Tests/RegorusTests.cs index e4b46c808..663b44919 100644 --- a/bindings/csharp/Regorus.Tests/RegorusTests.cs +++ b/bindings/csharp/Regorus.Tests/RegorusTests.cs @@ -52,7 +52,7 @@ public void Rule_conflict_preserves_error_status_and_reports_previous_location() } [TestMethod] - public void Dotted_package_metadata_and_rule_paths_remain_canonical() + public void Package_metadata_preserves_legacy_projection_while_rule_paths_remain_canonical() { using var engine = new Engine(); var package = engine.AddPolicy( @@ -64,7 +64,15 @@ package graph.defUniqueName["1.0.0"] deny := true if { input.blocked == true } """); - Assert.AreEqual("data.graph.defUniqueName[\"1.0.0\"]", package); + Assert.AreEqual("data.graph.defUniqueName.1.0.0", package); + var packageNames = JsonNode.Parse(engine.GetPolicyPackageNames()!); + Assert.AreEqual( + "graph.defUniqueName.1.0.0", + packageNames![0]!["package_name"]!.GetValue()); + Assert.AreEqual( + "namespace.rego", + packageNames[0]!["source_file"]!.GetValue()); + engine.SetInputJson("""{"blocked":false}"""); Assert.AreEqual( "false", diff --git a/bindings/csharp/Regorus/Engine.cs b/bindings/csharp/Regorus/Engine.cs index 852be9783..76e1934e8 100644 --- a/bindings/csharp/Regorus/Engine.cs +++ b/bindings/csharp/Regorus/Engine.cs @@ -111,6 +111,12 @@ public void ClearPolicyLengthConfig() }); } + /// + /// Adds a policy and returns its legacy dot-joined package metadata. + /// Quoted string contents omit their delimiter quotes. This value is + /// not a canonical rule path; use bracket notation for string + /// components when evaluating a rule. + /// public string? AddPolicy(string path, string rego) { return Utf8Marshaller.WithUtf8(path, pathPtr => @@ -128,6 +134,11 @@ public void SetRegoV0(bool enable) }); } + /// + /// Adds a policy file and returns its legacy dot-joined package + /// metadata, not a canonical rule path. Quoted string contents omit + /// their delimiter quotes. + /// public string? AddPolicyFromFile(string path) { return Utf8Marshaller.WithUtf8(path, pathPtr => @@ -261,6 +272,11 @@ public void SetGatherPrints(bool enable) }); } + /// + /// Returns policy package metadata with its legacy dot-joined text and + /// no data prefix. Quoted string contents omit their delimiter + /// quotes; use canonical bracketed paths for rule lookup. + /// public string? GetPolicyPackageNames() { return UseHandle(enginePtr => diff --git a/bindings/ffi/src/engine.rs b/bindings/ffi/src/engine.rs index f4cbe4537..af6be0f3d 100644 --- a/bindings/ffi/src/engine.rs +++ b/bindings/ffi/src/engine.rs @@ -250,6 +250,9 @@ pub extern "C" fn regorus_engine_drop(engine: *mut RegorusEngine) { /// /// The policy is parsed into AST. /// See https://docs.rs/regorus/latest/regorus/struct.Engine.html#method.add_policy +/// The returned package string is the legacy metadata projection: component +/// text is dot-joined, and quoted strings omit their delimiter quotes. It is +/// not a canonical rule lookup path. /// /// * `path`: A filename to be associated with the policy. /// * `rego`: Rego policy. @@ -304,6 +307,9 @@ pub extern "C" fn regorus_engine_add_data_json( /// Get list of loaded Rego packages as JSON. /// /// See https://docs.rs/regorus/latest/regorus/struct.Engine.html#method.get_packages +/// Package strings use the legacy dot-joined metadata projection; quoted string +/// contents omit their delimiter quotes. Canonical rule lookups continue to +/// use bracket notation for string keys. #[no_mangle] pub extern "C" fn regorus_engine_get_packages(engine: *mut RegorusEngine) -> RegorusResult { with_unwind_guard(|| { @@ -661,6 +667,9 @@ pub extern "C" fn regorus_engine_get_ast_as_json(engine: *mut RegorusEngine) -> /// Gets the package names defined in each policy added to the engine. /// /// See https://docs.rs/regorus/latest/regorus/coverage/struct.Engine.html#method.get_policy_package_names +/// Each `package_name` uses the legacy dot-joined metadata projection without +/// a `data` prefix; quoted string contents omit their delimiter quotes. It is +/// not a canonical rule lookup path. #[no_mangle] #[cfg(feature = "azure_policy")] pub extern "C" fn regorus_engine_get_policy_package_names( @@ -866,16 +875,19 @@ pub extern "C" fn regorus_engine_compile_program_with_entrypoints( #[cfg(all(test, feature = "std"))] mod namespace_tests { + #[cfg(feature = "azure_policy")] + use super::regorus_engine_get_policy_package_names; use super::{ regorus_engine_add_policy, regorus_engine_compile_program_with_entrypoints, - regorus_engine_drop, regorus_engine_eval_rule, regorus_engine_new, + regorus_engine_drop, regorus_engine_eval_rule, regorus_engine_get_packages, + regorus_engine_new, }; use crate::common::{regorus_result_drop, RegorusStatus}; use core::ffi::CStr; use std::ffi::CString; #[test] - fn canonical_dotted_namespace_paths_cross_the_ffi_boundary() { + fn legacy_package_metadata_and_canonical_rule_paths_cross_the_ffi_boundary() { let engine = regorus_engine_new(); assert!(!engine.is_null()); @@ -892,10 +904,43 @@ mod namespace_tests { let package = CStr::from_ptr(added.output) .to_str() .expect("UTF-8 package"); - assert_eq!(package, "data.graph.defUniqueName[\"1.0.0\"]"); + assert_eq!(package, "data.graph.defUniqueName.1.0.0"); } regorus_result_drop(added); + let packages = regorus_engine_get_packages(engine); + assert!(matches!(packages.status, RegorusStatus::Ok)); + unsafe { + let packages_json = CStr::from_ptr(packages.output) + .to_str() + .expect("UTF-8 package JSON"); + assert_eq!( + serde_json::from_str::>(packages_json).expect("valid package JSON"), + vec!["data.graph.defUniqueName.1.0.0"] + ); + } + regorus_result_drop(packages); + + #[cfg(feature = "azure_policy")] + { + let names = regorus_engine_get_policy_package_names(engine); + assert!(matches!(names.status, RegorusStatus::Ok)); + unsafe { + let names_json = CStr::from_ptr(names.output) + .to_str() + .expect("UTF-8 package-name JSON"); + assert_eq!( + serde_json::from_str::(names_json) + .expect("valid package-name JSON"), + serde_json::json!([{ + "source_file": "namespace.rego", + "package_name": "graph.defUniqueName.1.0.0" + }]) + ); + } + regorus_result_drop(names); + } + let rule = CString::new("data.graph.defUniqueName[\"1.0.0\"].deny").expect("valid rule path"); let result = regorus_engine_eval_rule(engine, rule.as_ptr()); diff --git a/src/engine.rs b/src/engine.rs index 1ba70bf07..ffb64e123 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -8,9 +8,9 @@ use crate::interpreter::*; use crate::lexer::*; use crate::parser::*; use crate::scheduler::*; +use crate::utils::gather_functions; use crate::utils::limits::PolicyLengthConfig; use crate::utils::limits::{self, fallback_execution_timer_config, ExecutionTimerConfig}; -use crate::utils::{gather_functions, get_path_string}; use crate::value::*; use crate::*; use crate::{Extension, QueryResults}; @@ -59,6 +59,49 @@ pub struct PolicyParameters { pub modifiers: Vec, } +fn legacy_package_metadata_projection<'a>( + reference: &'a Expr, + document: Option<&'a str>, +) -> Result { + let mut components = Vec::new(); + let mut expr = Some(reference); + + while let Some(node) = expr { + match *node { + Expr::RefDot { + ref refr, + ref field, + .. + } => { + components.push(field.0.text()); + expr = Some(refr); + } + Expr::RefBrack { + ref refr, + ref index, + .. + } => match *index.as_ref() { + Expr::String { ref span, .. } => { + components.push(span.text()); + expr = Some(refr); + } + _ => bail!(node.span().error("invalid ref expression")), + }, + Expr::Var { ref span, .. } => { + components.push(span.text()); + expr = None; + } + _ => bail!(node.span().error("invalid ref expression")), + } + } + + if let Some(document) = document { + components.push(document); + } + components.reverse(); + Ok(components.join(".")) +} + /// Create a default engine. impl Default for Engine { fn default() -> Self { @@ -217,9 +260,12 @@ impl Engine { /// /// The policy file will be parsed and converted to AST representation. /// Multiple policy files may be added to the engine. - /// Returns the Rego package name declared in the policy, formatted as a - /// canonical Rego path. String components that are not identifiers use - /// bracket notation, for example `data.graph["1.0.0"]`. + /// Returns the legacy public package-metadata projection, which joins + /// package component text with dots and prefixes `data`. String components + /// retain their source content without their delimiter quotes: for example, + /// `package graph["1.0.0"]` returns `data.graph.1.0.0`. This metadata + /// projection is not a canonical rule path; use + /// `data.graph["1.0.0"]` to look up a rule in that package. /// /// * `path`: A filename to be associated with the policy. /// * `rego`: The rego policy code. @@ -254,16 +300,18 @@ impl Engine { Rc::make_mut(&mut self.modules).push(module.clone()); // if policies change, interpreter needs to be prepared again self.prepared = false; - get_path_string(&module.package.refr, Some("data")) + legacy_package_metadata_projection(&module.package.refr, Some("data")) } /// Add a policy from a given file. /// /// The policy file will be parsed and converted to AST representation. /// Multiple policy files may be added to the engine. - /// Returns the Rego package name declared in the policy, formatted as a - /// canonical Rego path (using bracket notation for non-identifier string - /// components). + /// Returns the legacy public package-metadata projection, joining package + /// component text with dots and prefixing `data`. String components retain + /// their source content without delimiter quotes. This is not a canonical + /// rule path: use bracket notation for literal string components when + /// looking up rules. /// /// * `path`: Path to the policy file (.rego). /// @@ -294,15 +342,16 @@ impl Engine { Rc::make_mut(&mut self.modules).push(module.clone()); // if policies change, interpreter needs to be prepared again self.prepared = false; - get_path_string(&module.package.refr, Some("data")) + legacy_package_metadata_projection(&module.package.refr, Some("data")) } /// Get the list of packages defined by loaded policies. /// - /// Each package is returned as a canonical Rego path. String components - /// that are not identifiers use JSON-escaped bracket notation, so a - /// literal component such as `"1.0.0"` is not confused with three nested - /// identifier components. + /// Each package uses the legacy public metadata projection: component text + /// is joined with dots and prefixed by `data`. Quoted string components + /// retain their source content without delimiter quotes, so + /// `["1.0.0"]` is reported as `.1.0.0`. This differs from canonical rule + /// lookup paths, which use bracket notation for string components. /// /// ``` /// # use regorus::*; @@ -323,7 +372,7 @@ impl Engine { pub fn get_packages(&self) -> Result> { self.modules .iter() - .map(|m| get_path_string(&m.package.refr, Some("data"))) + .map(|m| legacy_package_metadata_projection(&m.package.refr, Some("data"))) .collect() } @@ -1486,6 +1535,10 @@ impl Engine { /// Get the package names of each policy added to the engine. /// + /// `package_name` preserves the legacy source-content metadata projection + /// without a `data` prefix. String delimiters are omitted and components + /// are dot-joined. It is not a canonical rule lookup path; use bracket + /// notation for quoted string components when evaluating rules. /// /// ```rust /// # use regorus::*; @@ -1507,7 +1560,7 @@ impl Engine { pub fn get_policy_package_names(&self) -> Result> { let mut package_names = vec![]; for m in self.modules.iter() { - let package_name = get_path_string(&m.package.refr, None)?; + let package_name = legacy_package_metadata_projection(&m.package.refr, None)?; package_names.push(PolicyPackageNameDefinition { source_file: m.package.span.source.file().to_string(), package_name, diff --git a/tests/engine/fixtures/legacy_package_projection.rego b/tests/engine/fixtures/legacy_package_projection.rego new file mode 100644 index 000000000..47ba5a201 --- /dev/null +++ b/tests/engine/fixtures/legacy_package_projection.rego @@ -0,0 +1,3 @@ +package graph.defUniqueName["1.0.0"] + +value := 23 diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 36cbee0ab..c4ad02f95 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -79,24 +79,40 @@ fn namespace_literal_dot_rule_path_uses_bracketed_component() -> Result<()> { } #[test] -fn namespace_literal_dot_package_metadata_is_unambiguous() -> Result<()> { +fn namespace_literal_dot_package_metadata_preserves_legacy_dot_join() -> Result<()> { let mut engine = Engine::new(); let package = engine.add_policy( "namespace.rego".to_string(), - r#"package graph.defUniqueName["1.0.0"]"#.to_string(), + r#"package graph.defUniqueName["1.0.0"] + value := 1"# + .to_string(), + )?; + let second_package = engine.add_policy( + "second.rego".to_string(), + "package second\nvalue := 2".to_string(), )?; - assert_eq!(package, "data.graph.defUniqueName[\"1.0.0\"]"); + assert_eq!(package, "data.graph.defUniqueName.1.0.0"); + assert_eq!(second_package, "data.second"); assert_eq!( engine.get_packages()?, - vec!["data.graph.defUniqueName[\"1.0.0\"]"] + vec!["data.graph.defUniqueName.1.0.0", "data.second"] ); + let path = r#"data.graph.defUniqueName["1.0.0"].value"#; + let compiled = engine.compile_with_entrypoint(&path.into())?; + for _ in 0..2 { + assert_eq!(engine.eval_rule(path.to_string())?, Value::from(1)); + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::from(1) + ); + } Ok(()) } #[test] -fn namespace_escaped_literal_component_uses_json_path_escaping() -> Result<()> { +fn namespace_escaped_literal_component_metadata_preserves_escape_spelling() -> Result<()> { let mut engine = Engine::new(); let package = engine.add_policy( "escaped.rego".to_string(), @@ -105,11 +121,16 @@ fn namespace_escaped_literal_component_uses_json_path_escaping() -> Result<()> { .to_string(), )?; - assert_eq!(package, r#"data.graph["a\".b"]"#); - assert_eq!( - engine.eval_rule(r#"data.graph["a\".b"].value"#.to_string())?, - Value::from(1) - ); + assert_eq!(package, r#"data.graph.a\".b"#); + let path = r#"data.graph["a\".b"].value"#; + let compiled = engine.compile_with_entrypoint(&path.into())?; + for _ in 0..2 { + assert_eq!(engine.eval_rule(path.to_string())?, Value::from(1)); + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::from(1) + ); + } Ok(()) } @@ -129,6 +150,72 @@ fn namespace_bracketed_identifier_path_is_equivalent_to_dotted_path() -> Result< Ok(()) } +#[test] +fn namespace_bracketed_and_dotted_identifiers_share_legacy_metadata_projection() -> Result<()> { + let mut bracketed = Engine::new(); + let package = bracketed.add_policy( + "bracketed-identifier.rego".to_string(), + "package graph[\"defUniqueName\"]\nvalue := true".to_string(), + )?; + assert_eq!(package, "data.graph.defUniqueName"); + assert_eq!(bracketed.get_packages()?, vec!["data.graph.defUniqueName"]); + assert_eq!( + bracketed.eval_rule(r#"data.graph["defUniqueName"].value"#.to_string())?, + Value::Bool(true) + ); + + let mut dotted = Engine::new(); + let package = dotted.add_policy( + "dotted-identifier.rego".to_string(), + "package graph.defUniqueName\nvalue := true".to_string(), + )?; + assert_eq!(package, "data.graph.defUniqueName"); + assert_eq!(bracketed.get_packages()?, dotted.get_packages()?); + assert_eq!(dotted.get_packages()?, vec!["data.graph.defUniqueName"]); + assert_eq!( + dotted.eval_rule(r#"data.graph["defUniqueName"].value"#.to_string())?, + Value::Bool(true) + ); + + Ok(()) +} + +#[test] +#[cfg(feature = "std")] +fn namespace_file_package_metadata_preserves_legacy_source_spelling() -> Result<()> { + let mut engine = Engine::new(); + let package = + engine.add_policy_from_file("tests/engine/fixtures/legacy_package_projection.rego")?; + + assert_eq!(package, "data.graph.defUniqueName.1.0.0"); + assert_eq!( + engine.get_packages()?, + vec!["data.graph.defUniqueName.1.0.0"] + ); + #[cfg(feature = "azure_policy")] + { + let package_names = engine.get_policy_package_names()?; + assert_eq!(package_names.len(), 1); + assert_eq!(package_names[0].package_name, "graph.defUniqueName.1.0.0"); + assert_eq!( + package_names[0].source_file, + "tests/engine/fixtures/legacy_package_projection.rego" + ); + } + + let path = r#"data.graph.defUniqueName["1.0.0"].value"#; + let compiled = engine.compile_with_entrypoint(&path.into())?; + for _ in 0..2 { + assert_eq!(engine.eval_rule(path.to_string())?, Value::from(23)); + assert_eq!( + compiled.eval_with_input(Value::new_object())?, + Value::from(23) + ); + } + + Ok(()) +} + #[test] fn namespace_escaped_package_materializes_only_the_decoded_key() -> Result<()> { let mut engine = Engine::new(); @@ -1388,9 +1475,9 @@ fn extension_with_state() -> Result<()> { #[cfg_attr(docsrs, doc(cfg(feature = "azure_policy")))] fn get_policy_package_names() -> Result<()> { let mut engine = Engine::new(); - engine.add_policy( + let package = engine.add_policy( "testPolicy1".to_string(), - r#"package test + r#"package test["1.0.0"] deny if { 1 == 2 @@ -1398,6 +1485,7 @@ fn get_policy_package_names() -> Result<()> { "# .to_string(), )?; + assert_eq!(package, "data.test.1.0.0"); engine.add_policy( "testPolicy2".to_string(), @@ -1412,7 +1500,7 @@ fn get_policy_package_names() -> Result<()> { let package_names = engine.get_policy_package_names()?; assert_eq!(2, package_names.len()); - assert_eq!("test", package_names[0].package_name); + assert_eq!("test.1.0.0", package_names[0].package_name); assert_eq!("testPolicy1", package_names[0].source_file); assert_eq!("test.nested.name", package_names[1].package_name);