From 45b88ddffeb07955d51df19ac36af65bda0eb942 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Wed, 23 Sep 2026 13:50:03 +0200 Subject: [PATCH 1/3] feat: support effective package override during policy loading Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/csharp/API.md | 11 + bindings/csharp/Regorus.Tests/RegorusTests.cs | 30 ++ .../csharp/Regorus.Tests/RvmProgramTests.cs | 17 ++ bindings/csharp/Regorus/Engine.cs | 20 ++ bindings/csharp/Regorus/NativeMethods.cs | 6 + bindings/ffi/src/engine.rs | 164 +++++++++++ src/ast.rs | 4 + src/engine.rs | 73 ++++- src/interpreter.rs | 47 ++-- src/interpreter/target/resolve.rs | 2 +- src/languages/rego/compiler/rules.rs | 25 +- src/parser.rs | 1 + src/scheduler.rs | 8 +- src/utils.rs | 45 ++- tests/engine/mod.rs | 262 ++++++++++++++++++ tests/rvm/compiler.rs | 30 ++ 16 files changed, 697 insertions(+), 48 deletions(-) diff --git a/bindings/csharp/API.md b/bindings/csharp/API.md index ae1cd7e1b..a5b8fe51e 100644 --- a/bindings/csharp/API.md +++ b/bindings/csharp/API.md @@ -578,3 +578,14 @@ var result = policy.EvalWithInput(inputJson); ``` The compiled approach provides better performance for repeated evaluations and clearer resource management. + +## Effective Package Override + +For engine-based loading, `Engine.AddPolicyWithPackage(path, rego, effectivePackage)` +loads the original policy text under a bare dotted package path such as +`tenant.authz` (not `data.tenant.authz`). This changes the policy's effective +package for evaluation and compilation without changing its source. Absolute +`data.*` references and imports are left unchanged; callers must update those +references themselves if they should point to the overridden package. +For RVM compilation, use `Program.CompileFromEngine(engine, new[] { "data.tenant.authz.allow" })`; +`Program.CompileFromModules` has no package-override option. diff --git a/bindings/csharp/Regorus.Tests/RegorusTests.cs b/bindings/csharp/Regorus.Tests/RegorusTests.cs index c4aa7ca4f..0da32d840 100644 --- a/bindings/csharp/Regorus.Tests/RegorusTests.cs +++ b/bindings/csharp/Regorus.Tests/RegorusTests.cs @@ -27,6 +27,36 @@ public void Basic_evaluation_succeeds() Assert.AreEqual("\"Hello\"", result); } + [TestMethod] + public void Add_policy_with_package_evaluates_under_effective_package() + { + using var engine = new Engine(); + var package = engine.AddPolicyWithPackage( + "source.rego", + "package original\nallow := input.user == \"alice\"", + "tenant.authz"); + engine.SetInputJson("{\"user\":\"alice\"}"); + + Assert.AreEqual("data.tenant.authz", package); + Assert.AreEqual("true", engine.EvalRule("data.tenant.authz.allow")); + } + + [TestMethod] + public void Add_policy_with_package_rejects_embedded_nul_without_adding_policy() + { + using var engine = new Engine(); + engine.AddPolicy("existing.rego", "package existing\nallow := true"); + var packagesBefore = engine.GetPolicyPackageNames(); + + Assert.ThrowsException( + () => engine.AddPolicyWithPackage( + "source.rego", + "package original\nallow := true", + "tenant.authz\0evil")); + + Assert.AreEqual(packagesBefore, engine.GetPolicyPackageNames()); + } + [TestMethod] public void Evaluation_using_file_policies_succeeds() { diff --git a/bindings/csharp/Regorus.Tests/RvmProgramTests.cs b/bindings/csharp/Regorus.Tests/RvmProgramTests.cs index d5b197272..4d3c599df 100644 --- a/bindings/csharp/Regorus.Tests/RvmProgramTests.cs +++ b/bindings/csharp/Regorus.Tests/RvmProgramTests.cs @@ -192,6 +192,23 @@ public void Program_compile_from_engine_succeeds() Assert.AreEqual("true", result, "expected allow=true"); } + [TestMethod] + public void Program_compile_from_engine_uses_effective_package_override() + { + using var engine = new Engine(); + engine.AddPolicyWithPackage( + "source.rego", + "package original\nis_admin(user) := user == \"alice\"\nallow if is_admin(input.user)", + "tenant.authz"); + + var program = Program.CompileFromEngine(engine, new[] { "data.tenant.authz.allow" }); + using var vm = new Rvm(); + vm.LoadProgram(program); + vm.SetInputJson("{\"user\":\"alice\"}"); + + Assert.AreEqual("true", vm.Execute(), "expected the overridden package entrypoint to evaluate"); + } + [TestMethod] public void Program_host_await_suspend_and_resume_succeeds() { diff --git a/bindings/csharp/Regorus/Engine.cs b/bindings/csharp/Regorus/Engine.cs index 852be9783..cce31a267 100644 --- a/bindings/csharp/Regorus/Engine.cs +++ b/bindings/csharp/Regorus/Engine.cs @@ -120,6 +120,26 @@ public void ClearPolicyLengthConfig() ))); } + /// + /// Adds a policy using a bare dotted effective package path, such as + /// tenant.authz. The original policy text is preserved. Absolute + /// data.* references are not rewritten. + /// + public string? AddPolicyWithPackage(string path, string rego, string effectivePackage) + { + Utf8Marshaller.ThrowIfContainsNul(effectivePackage, nameof(effectivePackage)); + return Utf8Marshaller.WithUtf8(path, pathPtr => + Utf8Marshaller.WithUtf8(rego, regoPtr => + Utf8Marshaller.WithUtf8(effectivePackage, packagePtr => + UseHandle(enginePtr => + CheckAndDropResult(Regorus.Internal.API.regorus_engine_add_policy_with_package( + (Regorus.Internal.RegorusEngine*)enginePtr, + (byte*)pathPtr, + (byte*)regoPtr, + (byte*)packagePtr)) + )))); + } + public void SetRegoV0(bool enable) { UseHandle(enginePtr => diff --git a/bindings/csharp/Regorus/NativeMethods.cs b/bindings/csharp/Regorus/NativeMethods.cs index 6859081d4..49850f1e9 100644 --- a/bindings/csharp/Regorus/NativeMethods.cs +++ b/bindings/csharp/Regorus/NativeMethods.cs @@ -284,6 +284,12 @@ internal static unsafe partial class API [DllImport(LibraryName, EntryPoint = "regorus_engine_add_policy", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)] internal static extern RegorusResult regorus_engine_add_policy(RegorusEngine* engine, byte* path, byte* rego); + /// + /// Add a policy with an effective package path override. + /// + [DllImport(LibraryName, EntryPoint = "regorus_engine_add_policy_with_package", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)] + internal static extern RegorusResult regorus_engine_add_policy_with_package(RegorusEngine* engine, byte* path, byte* rego, byte* package); + /// /// Add a policy from file. /// diff --git a/bindings/ffi/src/engine.rs b/bindings/ffi/src/engine.rs index 52016cde5..0199f22d1 100644 --- a/bindings/ffi/src/engine.rs +++ b/bindings/ffi/src/engine.rs @@ -58,6 +58,146 @@ impl Clone for RegorusEngine { } } +#[cfg(all(test, feature = "std"))] +mod package_override_tests { + #[cfg(feature = "azure_policy")] + use super::regorus_engine_get_policy_package_names; + use super::{ + regorus_engine_add_policy, regorus_engine_add_policy_with_package, regorus_engine_drop, + regorus_engine_get_packages, regorus_engine_get_policies, regorus_engine_new, + }; + use crate::common::{regorus_result_drop, RegorusDataType, RegorusStatus}; + use core::ffi::c_char; + use core::ptr; + use std::ffi::{CStr, CString}; + + fn result_text(result: &crate::common::RegorusResult) -> String { + assert!(matches!(&result.status, RegorusStatus::Ok)); + assert!(matches!(&result.data_type, RegorusDataType::String)); + assert!(!result.output.is_null()); + unsafe { + CStr::from_ptr(result.output) + .to_str() + .expect("FFI output should be UTF-8") + .to_owned() + } + } + + fn assert_rejected(result: crate::common::RegorusResult) { + assert!(matches!(&result.status, RegorusStatus::Error)); + assert!(!result.error_message.is_null()); + unsafe { + assert!(!CStr::from_ptr(result.error_message) + .to_str() + .expect("FFI error should be UTF-8") + .is_empty()); + } + regorus_result_drop(result); + } + + #[test] + fn package_override_ffi_reports_effective_package_and_preserves_source() { + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let legacy_path = CString::new("legacy.rego").unwrap(); + let legacy_policy = CString::new("package legacy\nvalue := 1").unwrap(); + let legacy_result = + regorus_engine_add_policy(engine, legacy_path.as_ptr(), legacy_policy.as_ptr()); + assert_eq!( + "data.legacy", + result_text(&legacy_result), + "the existing two-argument symbol keeps declared-package behavior" + ); + regorus_result_drop(legacy_result); + + let path = CString::new("source.rego").unwrap(); + let rego = CString::new("package original.authz\nallowed := true").unwrap(); + let package = CString::new("tenant.authz").unwrap(); + let added = regorus_engine_add_policy_with_package( + engine, + path.as_ptr(), + rego.as_ptr(), + package.as_ptr(), + ); + assert_eq!("data.tenant.authz", result_text(&added)); + regorus_result_drop(added); + + let packages = regorus_engine_get_packages(engine); + let package_json: serde_json::Value = + serde_json::from_str(&result_text(&packages)).expect("valid package JSON"); + assert_eq!( + serde_json::json!(["data.legacy", "data.tenant.authz"]), + package_json + ); + regorus_result_drop(packages); + + let policies = regorus_engine_get_policies(engine); + let policy_json: serde_json::Value = + serde_json::from_str(&result_text(&policies)).expect("valid policy JSON"); + assert_eq!("source.rego", policy_json[1]["path"]); + assert_eq!( + "package original.authz\nallowed := true", + policy_json[1]["contents"] + ); + regorus_result_drop(policies); + + #[cfg(feature = "azure_policy")] + { + let names = regorus_engine_get_policy_package_names(engine); + let name_json: serde_json::Value = + serde_json::from_str(&result_text(&names)).expect("valid package-name JSON"); + assert_eq!("tenant.authz", name_json[1]["package_name"]); + regorus_result_drop(names); + } + + regorus_engine_drop(engine); + } + + #[test] + fn package_override_ffi_rejects_null_and_malformed_without_adding_modules() { + let engine = regorus_engine_new(); + assert!(!engine.is_null()); + + let existing_path = CString::new("existing.rego").unwrap(); + let existing_policy = CString::new("package existing\nvalue := true").unwrap(); + let existing = + regorus_engine_add_policy(engine, existing_path.as_ptr(), existing_policy.as_ptr()); + assert_eq!("data.existing", result_text(&existing)); + regorus_result_drop(existing); + + let path = CString::new("invalid.rego").unwrap(); + let rego = CString::new("package invalid\nvalue := 1").unwrap(); + assert_rejected(regorus_engine_add_policy_with_package( + engine, + path.as_ptr(), + rego.as_ptr(), + ptr::null::(), + )); + + let malformed_package = CString::new("tenant .authz").unwrap(); + assert_rejected(regorus_engine_add_policy_with_package( + engine, + path.as_ptr(), + rego.as_ptr(), + malformed_package.as_ptr(), + )); + + let packages = regorus_engine_get_packages(engine); + let package_json: serde_json::Value = + serde_json::from_str(&result_text(&packages)).expect("valid package JSON"); + assert_eq!(serde_json::json!(["data.existing"]), package_json); + regorus_result_drop(packages); + + let policies = regorus_engine_get_policies(engine); + let policy_json: serde_json::Value = + serde_json::from_str(&result_text(&policies)).expect("valid policy JSON"); + assert_eq!(1, policy_json.as_array().expect("array").len()); + regorus_result_drop(policies); + regorus_engine_drop(engine); + } +} + #[cfg(all(test, feature = "contention_checks", feature = "std"))] mod tests { use super::RegorusEngine; @@ -232,6 +372,30 @@ pub extern "C" fn regorus_engine_add_policy( }) } +/// Add a policy with an effective package path override. +/// +/// `package` is a bare dotted Rego package path (for example `tenant.authz`), +/// without the `data.` prefix or `package` keyword. +#[no_mangle] +pub extern "C" fn regorus_engine_add_policy_with_package( + engine: *mut RegorusEngine, + path: *const c_char, + rego: *const c_char, + package: *const c_char, +) -> RegorusResult { + with_unwind_guard(|| { + to_regorus_string_result(|| -> Result { + let engine = to_shared_ref(engine as *const RegorusEngine)?; + let mut guard = engine.try_write()?; + guard.add_policy_with_package( + from_c_str(path)?, + from_c_str(rego)?, + from_c_str(package)?, + ) + }()) + }) +} + #[cfg(feature = "std")] #[no_mangle] pub extern "C" fn regorus_engine_add_policy_from_file( diff --git a/src/ast.rs b/src/ast.rs index 5cd61a310..13af5b9e8 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -462,6 +462,10 @@ pub struct Import { #[cfg_attr(feature = "ast", derive(serde::Serialize))] pub struct Module { pub package: Package, + /// Effective package path used for evaluation, when overridden by the caller. + /// The original package AST is retained so source spans and policy text stay intact. + #[cfg_attr(feature = "ast", serde(skip_serializing_if = "Option::is_none"))] + pub effective_package: Option, pub imports: Vec, #[cfg_attr(feature = "ast", serde(rename(serialize = "rules")))] pub policy: Vec>, diff --git a/src/engine.rs b/src/engine.rs index 97aa6ef97..b0dfa0666 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -240,6 +240,34 @@ impl Engine { /// ``` /// pub fn add_policy(&mut self, path: String, rego: String) -> Result { + self.add_policy_internal(path, rego, None) + } + + /// Add a policy while overriding its effective package path. + /// + /// `effective_package` must be a bare dotted Rego package path, such as + /// `tenant.authz` (without the `data.` prefix or `package` keyword). + /// The original policy text is retained. References written as absolute + /// `data.*` paths are not rewritten and must be updated by the caller when + /// needed. + /// + /// Returns the effective package name including the `data.` prefix. + pub fn add_policy_with_package( + &mut self, + path: String, + rego: String, + effective_package: String, + ) -> Result { + self.validate_effective_package(&effective_package)?; + self.add_policy_internal(path, rego, Some(effective_package)) + } + + fn add_policy_internal( + &mut self, + path: String, + rego: String, + effective_package: Option, + ) -> Result { let source = Source::from_contents_with_limits( path, rego, @@ -247,12 +275,41 @@ impl Engine { self.policy_length_config.max_lines, )?; let mut parser = self.make_parser(&source)?; - let module = Ref::new(parser.parse()?); + let mut parsed_module = parser.parse()?; + parsed_module.effective_package = effective_package; + let module = Ref::new(parsed_module); limits::enforce_memory_limit().map_err(|err| anyhow!(err))?; 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")) + crate::utils::get_module_package_path(&module, Some("data")) + } + + fn validate_effective_package(&self, effective_package: &str) -> Result<()> { + if effective_package.is_empty() || effective_package.starts_with("data.") { + bail!("effective package must be a non-empty bare dotted Rego package path (for example `tenant.authz`)"); + } + + let source = Source::from_contents_with_limits( + "".to_string(), + format!("package {effective_package}"), + self.policy_length_config.max_file_bytes, + self.policy_length_config.max_lines, + )?; + let mut parser = self.make_parser(&source)?; + let parsed = parser + .parse() + .map_err(|error| anyhow!("invalid effective package `{effective_package}`: {error}"))?; + let components = Parser::get_path_ref_components(&parsed.package.refr)?; + let parsed_package = components + .iter() + .map(|component| component.text()) + .collect::>() + .join("."); + if parsed_package != effective_package || !parsed.policy.is_empty() { + bail!("effective package must be a bare dotted Rego package path without whitespace, comments, or policy statements"); + } + Ok(()) } /// Add a policy from a given file. @@ -290,7 +347,7 @@ 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")) + crate::utils::get_module_package_path(&module, Some("data")) } /// Get the list of packages defined by loaded policies. @@ -314,7 +371,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| crate::utils::get_module_package_path(m, Some("data"))) .collect() } @@ -1203,8 +1260,7 @@ 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 = crate::utils::get_module_package_components(m)?; let vref = Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?; if *vref == Value::Undefined { @@ -1229,8 +1285,7 @@ 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 = crate::utils::get_module_package_components(m)?; let vref = Interpreter::make_or_get_value_mut(self.interpreter.get_data_mut(), &path[..])?; if *vref == Value::Undefined { @@ -1490,7 +1545,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 = crate::utils::get_module_package_path(m, 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 a4b8d565c..443fe1469 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -19,7 +19,10 @@ 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::{ + get_extra_arg, get_module_package_components, get_module_package_path, get_path_string, + get_root_var, FunctionTable, +}; use crate::value::*; use crate::*; use crate::{Expression, Extension, Location, QueryResult, QueryResults}; @@ -3010,7 +3013,7 @@ impl Interpreter { // Prevent cyclic evaluation. continue; } - let module_path_str = get_path_string(&module.package.refr, Some("data"))?; + let module_path_str = get_module_package_path(&module, Some("data"))?; let has_dot_after_prefix = module_path_str .get(path.len()..) .is_some_and(|suffix| suffix.starts_with('.')); @@ -3019,9 +3022,7 @@ impl Interpreter { && (module_path_str.len() == path.len() || has_dot_after_prefix) { // 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 module_path_components = get_module_package_components(&module)?; let vref = Self::make_or_get_value_mut(&mut self.data, &module_path_components)?; if *vref == Value::Undefined { *vref = Value::new_object(); @@ -3167,8 +3168,8 @@ 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(); + let parsed_path = get_module_package_components(&module)?; + let mut module_var_path = parsed_path; module_var_path.push(name.text()); if self.is_processed(&module_var_path)? { @@ -3632,8 +3633,7 @@ impl Interpreter { ) -> 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_module_package_path(new_module, Some("data"))?; self.current_module_index = self.find_module_index(new_module); } self.module = module; @@ -3753,7 +3753,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_module_package_components(module.as_ref())?; let (refr, index) = match refr.as_ref() { Expr::RefBrack { refr, index, .. } => (refr, Some(index.clone())), @@ -3764,8 +3764,9 @@ 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(); + let rule_path = Parser::get_path_ref_components(refr)?; + path.extend(rule_path.iter().map(|component| component.text())); + let paths = path; Self::check_default_value(value)?; let value = self.eval_expr(value)?; @@ -3913,7 +3914,10 @@ impl Interpreter { let is_object = ctx.key_expr.is_some() && !is_set; let value = self.eval_rule_bodies(ctx, span, rule_body)?; - let package_components = self.eval_rule_ref(&module.package.refr)?; + let package_components = get_module_package_components(module)? + .into_iter() + .map(Value::from) + .collect::>(); if value != Value::Undefined { for (path, value_in_map) in value.as_object()? { @@ -3953,11 +3957,11 @@ impl Interpreter { RuleHead::Func { refr, args, assign, .. } => { - 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(); + let current_module = self.current_module()?; + let package_path = get_module_package_components(current_module.as_ref())?; + let mut path = package_path; + let rule_path = Parser::get_path_ref_components(refr)?; + path.extend(rule_path.iter().map(|component| component.text())); // Ensure that for functions with a nesting level (e.g: a.foo), // `a` is created as an empty object. @@ -4202,7 +4206,10 @@ impl Interpreter { pub fn create_rule_prefixes(&mut self) -> Result<()> { for module in self.compiled_policy.modules.clone().iter() { - let module_path = Self::get_rule_path_components(&module.package.refr)?; + let module_path: Vec> = get_module_package_components(module)? + .into_iter() + .map(Rc::from) + .collect(); for rule in &module.policy { let rule_refr = Self::get_rule_refr(rule); @@ -4323,7 +4330,7 @@ impl Interpreter { 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"))?; + let module_path = get_module_package_path(module, Some("data"))?; for import in &module.imports { let target = match &import.r#as { Some(s) => s.text(), diff --git a/src/interpreter/target/resolve.rs b/src/interpreter/target/resolve.rs index fc3750c8b..0bd0272a1 100644 --- a/src/interpreter/target/resolve.rs +++ b/src/interpreter/target/resolve.rs @@ -32,7 +32,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 = get_module_package_path(module, None) .map_err(|_| TargetCompileError::TargetNotFound(module_target.clone().into()))?; match &target_name { diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index e18f4b720..5b19b50e3 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -16,7 +16,7 @@ use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; use crate::rvm::program::{Program, RuleType}; use crate::rvm::Instruction; -use crate::utils::get_path_string; +use crate::utils::get_module_package_path; use crate::Map; use crate::{CompiledPolicy, Value}; use alloc::collections::BTreeSet; @@ -202,19 +202,18 @@ impl<'a> Compiler<'a> { for (module_index, module) in self.policy.get_modules().iter().enumerate() { for policy_rule in &module.policy { if core::ptr::eq(policy_rule.as_ref(), rule) { - let package_path = - match get_path_string(&module.package.refr, Some("data")) { - Ok(path) => path, - Err(e) => { - return Err(CompilerError::General { - message: format!( - "Failed to get package path for module: {}", - e - ), - } - .into()); + let package_path = match get_module_package_path(module, Some("data")) { + Ok(path) => path, + Err(e) => { + return Err(CompilerError::General { + message: format!( + "Failed to get package path for module: {}", + e + ), } - }; + .into()); + } + }; return Ok((package_path, module_index as u32)); } } diff --git a/src/parser.rs b/src/parser.rs index 8f7f73ee7..1f11037ec 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2153,6 +2153,7 @@ impl<'source> Parser<'source> { let m = Module { package, + effective_package: None, imports, target, policy, diff --git a/src/scheduler.rs b/src/scheduler.rs index a54774d60..87bb0ff49 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -320,7 +320,7 @@ impl Analyzer { fn add_rules_and_aliases(&mut self, modules: &[Ref]) -> Result<()> { for m in modules { - let path = get_path_string(&m.package.refr, Some("data"))?; + let path = get_module_package_path(m, Some("data"))?; let scope: &mut Scope = self.packages.entry(path).or_default(); for r in &m.policy { let var = match r.as_ref() { @@ -348,7 +348,7 @@ impl Analyzer { } fn analyze_module(&mut self, m: &Module) -> Result<()> { - let path = get_path_string(&m.package.refr, Some("data"))?; + let path = get_module_package_path(m, Some("data"))?; let scope = match self.packages.get(&path) { Some(s) => s, _ => bail!("internal error: package scope missing"), @@ -1143,7 +1143,7 @@ pub fn compute_module_globals( // First pass: collect all rule names by package for m in modules { - let path = get_path_string(&m.package.refr, Some("data"))?; + let path = get_module_package_path(m, Some("data"))?; let package_globals: &mut crate::Rc> = packages.entry(path).or_default(); for r in &m.policy { @@ -1163,7 +1163,7 @@ pub fn compute_module_globals( // Second pass: for each module, combine package globals with module-specific imports for (module_idx, m) in modules.iter().enumerate() { - let path = get_path_string(&m.package.refr, Some("data"))?; + let path = get_module_package_path(m, Some("data"))?; let mut module_globals = packages.get(&path).cloned().unwrap_or_default(); // Add import aliases specific to this module diff --git a/src/utils.rs b/src/utils.rs index 37193140a..7d64d68eb 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -12,6 +12,7 @@ pub mod limits; use crate::ast::*; use crate::builtins::*; +use crate::interpreter::Interpreter; use crate::lexer::*; use crate::*; @@ -47,6 +48,48 @@ pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { Ok(comps.join(".")) } +pub fn get_module_package_path(module: &Module, document: Option<&str>) -> Result { + if let Some(package) = &module.effective_package { + return Ok(document.map_or_else( + || package.clone(), + |document| format!("{document}.{package}"), + )); + } + Interpreter::get_path_string(&module.package.refr, document) +} + +pub fn get_module_package_components(module: &Module) -> Result> { + if let Some(package) = &module.effective_package { + return Ok(package.split('.').collect()); + } + + let mut components = vec![]; + let mut expr = module.package.refr.as_ref(); + loop { + match expr { + Expr::RefDot { refr, field, .. } => { + components.push(field.0.text()); + expr = refr.as_ref(); + } + Expr::RefBrack { refr, index, .. } => { + if let Expr::String { span, .. } = index.as_ref() { + components.push(span.text()); + } else { + bail!("internal error: not a package path {expr:?}"); + } + expr = refr.as_ref(); + } + Expr::Var { span, .. } => { + components.push(span.text()); + break; + } + _ => bail!("internal error: not a package path {expr:?}"), + } + } + components.reverse(); + Ok(components) +} + pub type FunctionTable = BTreeMap>, u8, Ref)>; fn get_extra_arg_impl( @@ -87,7 +130,7 @@ pub fn gather_functions(modules: &[Ref]) -> Result { let mut table = FunctionTable::new(); for module in modules { - let module_path = get_path_string(&module.package.refr, Some("data"))?; + let module_path = get_module_package_path(module, Some("data"))?; for rule in &module.policy { if let Rule::Spec { span, diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 292bcb9b9..33e9d8603 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -102,6 +102,250 @@ fn extension_with_state() -> Result<()> { Ok(()) } +#[test] +fn policy_package_override_is_used_for_evaluation() -> Result<()> { + let mut engine = Engine::new(); + let package = engine.add_policy_with_package( + "source.rego".to_string(), + r#"package original.authz + + default allowed = false + is_admin(user) := user == "admin" + allowed if is_admin(input.user) + "# + .to_string(), + "tenant.authz".to_string(), + )?; + + assert_eq!("data.tenant.authz", package); + assert_eq!(vec!["data.tenant.authz"], engine.get_packages()?); + let source = engine.get_policies()?; + assert_eq!("source.rego", source[0].get_path()); + assert!(source[0] + .get_contents() + .starts_with("package original.authz")); + let sources_json = engine.get_policies_as_json()?; + assert!(sources_json.contains("package original.authz")); + assert!(!sources_json.contains("tenant.authz")); + + engine.set_input(Value::from_json_str(r#"{"user":"admin"}"#)?); + let result = engine.eval_query("data.tenant.authz.allowed".to_string(), false)?; + assert_eq!(Value::from(true), result.result[0].expressions[0].value); + + Ok(()) +} + +#[test] +fn legacy_bracketed_package_segments_remain_evaluable() -> Result<()> { + let mut engine = Engine::new(); + let package = engine.add_policy( + "bracketed.rego".to_string(), + "package team[\"authz\"]\nallow := true".to_string(), + )?; + + assert_eq!("data.team.authz", package); + assert_eq!( + Value::from(true), + engine.eval_rule("data.team.authz.allow".into())? + ); + Ok(()) +} + +#[test] +fn invalid_package_overrides_leave_existing_engine_state_unchanged() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "original.rego".to_string(), + "package original\ndefined := true".to_string(), + )?; + assert_eq!( + Value::from(true), + engine + .eval_query("data.original.defined".to_string(), false)? + .result[0] + .expressions[0] + .value + ); + + for invalid_package in [ + "", + "data.tenant.authz", + "tenant .authz", + "tenant.authz # comment", + "tenant.authz\nallow := true", + "package tenant.authz", + "tenant..authz", + "tenant\0authz", + ] { + assert!( + engine + .add_policy_with_package( + "invalid.rego".to_string(), + "package invalid\nvalue := 1".to_string(), + invalid_package.to_string(), + ) + .is_err(), + "accepted malformed package {invalid_package:?}" + ); + } + + assert!(engine + .add_policy_with_package( + "broken.rego".to_string(), + "package broken\nvalue := {".to_string(), + "tenant.authz".to_string(), + ) + .is_err()); + + assert_eq!(vec!["data.original"], engine.get_packages()?); + assert_eq!(1, engine.get_policies()?.len()); + assert_eq!( + Value::from(true), + engine + .eval_query("data.original.defined".to_string(), false)? + .result[0] + .expressions[0] + .value + ); + Ok(()) +} + +#[test] +fn adding_overridden_policy_after_evaluation_updates_only_that_engine() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy( + "first.rego".to_string(), + "package first\nvalue := 1".to_string(), + )?; + assert_eq!( + Value::from(1), + engine + .eval_query("data.first.value".to_string(), false)? + .result[0] + .expressions[0] + .value + ); + let mut clone = engine.clone(); + + engine.add_policy_with_package( + "source.rego".to_string(), + "package original\nanswer := 2".to_string(), + "tenant.authz".to_string(), + )?; + + assert_eq!( + Value::from(2), + engine + .eval_query("data.tenant.authz.answer".to_string(), false)? + .result[0] + .expressions[0] + .value + ); + assert_eq!( + vec!["data.first", "data.tenant.authz"], + engine.get_packages()? + ); + assert_eq!(vec!["data.first"], clone.get_packages()?); + assert!(clone + .eval_query("data.tenant.authz.answer".to_string(), false)? + .result + .is_empty()); + + Ok(()) +} + +#[test] +fn modules_with_the_same_effective_package_merge_distinct_rules() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy_with_package( + "first.rego".to_string(), + "package first\none := 1".to_string(), + "tenant.authz".to_string(), + )?; + engine.add_policy_with_package( + "second.rego".to_string(), + "package second\ntwo := 2".to_string(), + "tenant.authz".to_string(), + )?; + + assert_eq!( + Value::from(1), + engine.eval_rule("data.tenant.authz.one".into())? + ); + assert_eq!( + Value::from(2), + engine.eval_rule("data.tenant.authz.two".into())? + ); + Ok(()) +} + +#[test] +fn colliding_rules_in_overridden_packages_keep_existing_conflict_behavior() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy_with_package( + "first.rego".to_string(), + "package first\nvalue := 1".to_string(), + "tenant.authz".to_string(), + )?; + engine.add_policy_with_package( + "second.rego".to_string(), + "package second\nvalue := 2".to_string(), + "tenant.authz".to_string(), + )?; + + let error = engine + .eval_rule("data.tenant.authz.value".into()) + .expect_err("different complete-rule values must conflict"); + assert!(error + .to_string() + .contains("rule conflicts with the following rule")); + Ok(()) +} + +#[test] +fn absolute_references_and_imports_are_not_rewritten() -> Result<()> { + let mut engine = Engine::new(); + engine.add_data(Value::from_json_str(r#"{"original":{"base":100}}"#)?)?; + engine.add_policy_with_package( + "moved.rego".to_string(), + "package original\nbase := 1\nabsolute := data.original.base".to_string(), + "tenant.authz".to_string(), + )?; + engine.add_policy_with_package( + "helpers.rego".to_string(), + "package original.helpers\nvalue := 7".to_string(), + "tenant.authz.helpers".to_string(), + )?; + engine.add_policy( + "consumer.rego".to_string(), + "package consumer\nimport data.original.helpers\nresult := helpers.value".to_string(), + )?; + + assert_eq!( + Value::from(100), + engine.eval_rule("data.tenant.authz.absolute".into())? + ); + assert_eq!( + Value::Undefined, + engine.eval_rule("data.consumer.result".into())? + ); + Ok(()) +} + +#[test] +#[cfg(feature = "ast")] +fn ast_json_omits_unset_effective_package_for_legacy_output() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy("legacy.rego".to_string(), "package legacy".to_string())?; + + let ast = engine.get_ast_as_json()?; + assert!( + !ast.contains("\"effective_package\""), + "legacy AST output should omit unset override metadata" + ); + Ok(()) +} + #[test] #[cfg(feature = "azure_policy")] #[cfg_attr(docsrs, doc(cfg(feature = "azure_policy")))] @@ -139,6 +383,24 @@ fn get_policy_package_names() -> Result<()> { Ok(()) } +#[test] +#[cfg(feature = "azure_policy")] +#[cfg_attr(docsrs, doc(cfg(feature = "azure_policy")))] +fn get_policy_package_names_uses_effective_package() -> Result<()> { + let mut engine = Engine::new(); + engine.add_policy_with_package( + "source.rego".to_string(), + "package original\nallow := true".to_string(), + "tenant.authz".to_string(), + )?; + + let package_names = engine.get_policy_package_names()?; + assert_eq!(1, package_names.len()); + assert_eq!("tenant.authz", package_names[0].package_name); + assert_eq!("source.rego", package_names[0].source_file); + Ok(()) +} + #[test] #[cfg(feature = "azure_policy")] #[cfg_attr(docsrs, doc(cfg(feature = "azure_policy")))] diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index 47bc39831..8b3a22f38 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -50,6 +50,36 @@ fn assert_literal_exists(program: ®orus::rvm::program::Program, expected: &Va ); } +#[test] +fn package_override_is_used_for_rvm_entrypoint_and_original_source() { + let source = + "package original\nis_admin(user) := user == \"alice\"\nallow if is_admin(input.user)"; + let mut engine = Engine::new(); + assert_eq!( + "data.tenant.authz", + engine + .add_policy_with_package( + "source.rego".to_string(), + source.to_string(), + "tenant.authz".to_string(), + ) + .expect("failed to add package-overridden policy") + ); + + let compiled = engine + .compile_with_entrypoint(&Rc::from("data.tenant.authz.allow")) + .expect("failed to compile overridden entrypoint"); + let program = Compiler::compile_from_policy(&compiled, &["data.tenant.authz.allow"]) + .expect("failed to compile RVM program"); + + assert!( + program.get_entry_point("data.tenant.authz.allow").is_some(), + "RVM should expose the effective-package entrypoint" + ); + assert_eq!("source.rego", program.sources[0].name); + assert_eq!(source, program.sources[0].content); +} + #[test] fn constant_array_is_hoisted() { let program = compile_rule( From 4630a68da68a694b905c09a125ef79463ddc3375 Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Wed, 23 Sep 2026 16:28:55 +0200 Subject: [PATCH 2/3] fix: validate package override inputs and source limits Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bindings/csharp/Regorus.Tests/RegorusTests.cs | 34 ++++++++++++++++++ bindings/csharp/Regorus/Engine.cs | 2 ++ src/engine.rs | 34 +++++++++++++++--- tests/engine/mod.rs | 36 +++++++++++++++++++ 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/bindings/csharp/Regorus.Tests/RegorusTests.cs b/bindings/csharp/Regorus.Tests/RegorusTests.cs index 0da32d840..9640f5f64 100644 --- a/bindings/csharp/Regorus.Tests/RegorusTests.cs +++ b/bindings/csharp/Regorus.Tests/RegorusTests.cs @@ -57,6 +57,40 @@ public void Add_policy_with_package_rejects_embedded_nul_without_adding_policy() Assert.AreEqual(packagesBefore, engine.GetPolicyPackageNames()); } + [TestMethod] + public void Add_policy_with_package_rejects_embedded_nul_in_rego_without_adding_policy() + { + using var engine = new Engine(); + engine.AddPolicy("existing.rego", "package existing\nallow := true"); + var packagesBefore = engine.GetPolicyPackageNames(); + + Assert.ThrowsException( + () => engine.AddPolicyWithPackage( + "source.rego", + "package original\nallow := true\0\nallow := false", + "tenant.authz")); + + Assert.AreEqual(packagesBefore, engine.GetPolicyPackageNames()); + Assert.AreEqual("true", engine.EvalRule("data.existing.allow")); + } + + [TestMethod] + public void Add_policy_with_package_rejects_embedded_nul_in_path_without_adding_policy() + { + using var engine = new Engine(); + engine.AddPolicy("existing.rego", "package existing\nallow := true"); + var packagesBefore = engine.GetPolicyPackageNames(); + + Assert.ThrowsException( + () => engine.AddPolicyWithPackage( + "source.rego\0suffix", + "package original\nallow := true", + "tenant.authz")); + + Assert.AreEqual(packagesBefore, engine.GetPolicyPackageNames()); + Assert.AreEqual("true", engine.EvalRule("data.existing.allow")); + } + [TestMethod] public void Evaluation_using_file_policies_succeeds() { diff --git a/bindings/csharp/Regorus/Engine.cs b/bindings/csharp/Regorus/Engine.cs index cce31a267..bb4c19940 100644 --- a/bindings/csharp/Regorus/Engine.cs +++ b/bindings/csharp/Regorus/Engine.cs @@ -127,6 +127,8 @@ public void ClearPolicyLengthConfig() /// public string? AddPolicyWithPackage(string path, string rego, string effectivePackage) { + Utf8Marshaller.ThrowIfContainsNul(path, nameof(path)); + Utf8Marshaller.ThrowIfContainsNul(rego, nameof(rego)); Utf8Marshaller.ThrowIfContainsNul(effectivePackage, nameof(effectivePackage)); return Utf8Marshaller.WithUtf8(path, pathPtr => Utf8Marshaller.WithUtf8(rego, regoPtr => diff --git a/src/engine.rs b/src/engine.rs index b0dfa0666..823893209 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -17,6 +17,7 @@ use crate::{Extension, QueryResults}; use crate::Rc; use anyhow::{anyhow, bail, Result}; +use core::num::{NonZeroU32, NonZeroUsize}; /// The Rego evaluation engine. /// @@ -290,13 +291,30 @@ impl Engine { bail!("effective package must be a non-empty bare dotted Rego package path (for example `tenant.authz`)"); } + const PACKAGE_PREFIX: &str = "package "; + // The configured limits apply to caller input, not this generated prefix. + let package_prefix_columns = u32::try_from(PACKAGE_PREFIX.len()).unwrap_or(u32::MAX); + let max_file_bytes = NonZeroUsize::new( + self.policy_length_config + .max_file_bytes + .get() + .saturating_add(PACKAGE_PREFIX.len()), + ) + .unwrap_or(self.policy_length_config.max_file_bytes); + let max_col = NonZeroU32::new( + self.policy_length_config + .max_col + .get() + .saturating_add(package_prefix_columns), + ) + .unwrap_or(self.policy_length_config.max_col); let source = Source::from_contents_with_limits( "".to_string(), - format!("package {effective_package}"), - self.policy_length_config.max_file_bytes, + format!("{PACKAGE_PREFIX}{effective_package}"), + max_file_bytes, self.policy_length_config.max_lines, )?; - let mut parser = self.make_parser(&source)?; + let mut parser = self.make_parser_with_max_col(&source, max_col)?; let parsed = parser .parse() .map_err(|error| anyhow!("invalid effective package `{effective_package}`: {error}"))?; @@ -1660,8 +1678,16 @@ impl Engine { } fn make_parser<'a>(&self, source: &'a Source) -> Result> { + self.make_parser_with_max_col(source, self.policy_length_config.max_col) + } + + fn make_parser_with_max_col<'a>( + &self, + source: &'a Source, + max_col: NonZeroU32, + ) -> Result> { let mut parser = Parser::new(source)?; - parser.set_max_col(self.policy_length_config.max_col); + parser.set_max_col(max_col); if self.rego_v1 { parser.enable_rego_v1()?; } diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index 33e9d8603..b354effbb 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -135,6 +135,42 @@ fn policy_package_override_is_used_for_evaluation() -> Result<()> { Ok(()) } +#[test] +fn policy_package_override_uses_limits_for_policy_source_not_synthetic_package() -> Result<()> { + let mut engine = Engine::new(); + engine.set_policy_length_config(PolicyLengthConfig { + max_col: core::num::NonZeroU32::new(24) + .ok_or_else(|| anyhow::anyhow!("invalid test limit"))?, + max_file_bytes: core::num::NonZeroUsize::new(24) + .ok_or_else(|| anyhow::anyhow!("invalid test limit"))?, + max_lines: core::num::NonZeroUsize::new(20) + .ok_or_else(|| anyhow::anyhow!("invalid test limit"))?, + }); + + let package = engine.add_policy_with_package( + "source.rego".to_string(), + "package a\nallow := true".to_string(), + "tenant.authorization".to_string(), + )?; + + assert_eq!("data.tenant.authorization", package); + assert_eq!( + Value::from(true), + engine.eval_rule("data.tenant.authorization.allow".into())? + ); + + assert!(engine + .add_policy_with_package( + "over-limit.rego".to_string(), + "package a\nallow := true\n\n".to_string(), + "tenant.other".to_string(), + ) + .is_err()); + assert_eq!(vec!["data.tenant.authorization"], engine.get_packages()?); + + Ok(()) +} + #[test] fn legacy_bracketed_package_segments_remain_evaluable() -> Result<()> { let mut engine = Engine::new(); From d945a6f1a376b31eb28a3a3c4861962aec56ccfe Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Fri, 25 Sep 2026 17:16:14 +0200 Subject: [PATCH 3/3] test: align package collision diagnostic with main Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ddf258-ed55-43f9-a073-ae0c2ea911cf --- tests/engine/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/engine/mod.rs b/tests/engine/mod.rs index b354effbb..99639aa00 100644 --- a/tests/engine/mod.rs +++ b/tests/engine/mod.rs @@ -334,7 +334,7 @@ fn colliding_rules_in_overridden_packages_keep_existing_conflict_behavior() -> R .expect_err("different complete-rule values must conflict"); assert!(error .to_string() - .contains("rule conflicts with the following rule")); + .contains("rule conflicts with rule at first.rego:2:1")); Ok(()) }