mirror of
https://github.com/microsoft/regorus.git
synced 2026-08-05 02:16:11 +00:00
Fix RVM evaluation of default-only rules (#664)
Default-only rules (e.g., `default deny := true` with no conditional body) returned Undefined in the RVM instead of the default value. Compiler: - compute_rule_type: return Complete when rule exists only in default_rules map - compile_worklist_rule: emit register slots and data-tree entries for default-only rules (else branch) VM: - execute_call_rule_common + execute_call_rule_suspendable: check default_literal_index before returning Undefined when definitions is empty Tests: - 3 new RVM cases (default_rules.yaml): bool, object, entry-point - 3 new interpreter cases (default/basic.yaml): matching coverage Co-authored-by: Mark Birger <markbirger@microsoft.com>
This commit is contained in:
@@ -38,6 +38,11 @@ impl<'a> Compiler<'a> {
|
||||
|
||||
pub(super) fn compute_rule_type(&self, rule_path: &str) -> Result<RuleType> {
|
||||
let Some(definitions) = self.policy.inner.rules.get(rule_path) else {
|
||||
// Default-only rules (e.g., `default deny := true`) have no regular definitions
|
||||
// in the `rules` map — they only exist in `default_rules`. Treat them as Complete.
|
||||
if self.policy.inner.default_rules.contains_key(rule_path) {
|
||||
return Ok(RuleType::Complete);
|
||||
}
|
||||
return Err(CompilerError::General {
|
||||
message: format!("no definitions found for rule path '{}'", rule_path),
|
||||
}
|
||||
@@ -614,6 +619,31 @@ impl<'a> Compiler<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
self.register_counter = saved_register_counter;
|
||||
self.current_package = saved_package;
|
||||
self.current_module_index = saved_module_index;
|
||||
} else {
|
||||
// Default-only rule — no body definitions to compile.
|
||||
// Ensure rule_num_registers is sized so finish() won't panic.
|
||||
if let Some(&rule_index) = self.rule_index_map.get(rule_path) {
|
||||
while self.rule_num_registers.len() <= rule_index as usize {
|
||||
self.rule_num_registers.push(0);
|
||||
}
|
||||
|
||||
// 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<String> =
|
||||
package_parts.iter().map(|s| s.to_string()).collect();
|
||||
|
||||
let _ = self.program.add_rule_to_tree(
|
||||
&package_path,
|
||||
rule_name,
|
||||
rule_index as usize,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
self.register_counter = saved_register_counter;
|
||||
self.current_package = saved_package;
|
||||
self.current_module_index = saved_module_index;
|
||||
|
||||
@@ -191,7 +191,13 @@ impl RegoVM {
|
||||
let rule_definitions = rule_info.definitions.clone();
|
||||
|
||||
if rule_definitions.is_empty() {
|
||||
let result = Value::Undefined;
|
||||
// No compiled definitions — check for a default value before returning Undefined.
|
||||
// Default-only rules (e.g., `default deny := true`) have no body definitions
|
||||
// but their default value was evaluated at compile time and stored as a literal.
|
||||
let result = rule_info
|
||||
.default_literal_index
|
||||
.and_then(|idx| self.program.literals.get(usize::from(idx)).cloned())
|
||||
.unwrap_or(Value::Undefined);
|
||||
if !is_function_rule {
|
||||
let available = self.rule_cache.len();
|
||||
let entry =
|
||||
@@ -336,7 +342,11 @@ impl RegoVM {
|
||||
}
|
||||
|
||||
if rule_info.definitions.is_empty() {
|
||||
let result = Value::Undefined;
|
||||
// No compiled definitions — check for a default value before returning Undefined.
|
||||
let result = rule_info
|
||||
.default_literal_index
|
||||
.and_then(|idx| self.program.literals.get(usize::from(idx)).cloned())
|
||||
.unwrap_or(Value::Undefined);
|
||||
if !is_function_rule {
|
||||
let available = self.rule_cache.len();
|
||||
let entry =
|
||||
|
||||
@@ -321,6 +321,37 @@ cases:
|
||||
skip: true
|
||||
want_result: [5]
|
||||
|
||||
- note: default_only_rule_bool
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package test
|
||||
default deny := true
|
||||
query: data.test.deny
|
||||
want_result: true
|
||||
|
||||
- note: default_only_rule_object
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package test
|
||||
default deny := {"result": false, "reasons": []}
|
||||
query: data.test.deny
|
||||
want_result:
|
||||
result: false
|
||||
reasons: []
|
||||
|
||||
- note: default_only_rule_with_package_query
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package graph.mypolicy
|
||||
default deny := {"result": false, "reasons": []}
|
||||
query: data.graph.mypolicy.deny
|
||||
want_result:
|
||||
result: false
|
||||
reasons: []
|
||||
|
||||
- note: valid-var-in-some-in-value
|
||||
data: {}
|
||||
modules:
|
||||
|
||||
@@ -154,6 +154,39 @@ cases:
|
||||
query: data.test.auth.allow
|
||||
want_result: false
|
||||
|
||||
- note: default_only_rule_bool
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package test
|
||||
default deny := true
|
||||
query: data.test.deny
|
||||
want_result: true
|
||||
|
||||
- note: default_only_rule_object
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package test
|
||||
default deny := {"result": false, "reasons": []}
|
||||
query: data.test.deny
|
||||
want_result:
|
||||
result: false
|
||||
reasons: []
|
||||
|
||||
- note: default_only_rule_with_entry_point
|
||||
data: {}
|
||||
modules:
|
||||
- |
|
||||
package graph.mypolicy
|
||||
default deny := {"result": false, "reasons": []}
|
||||
entry_points:
|
||||
- data.graph.mypolicy.deny
|
||||
query: data.graph.mypolicy.deny
|
||||
want_result:
|
||||
result: false
|
||||
reasons: []
|
||||
|
||||
- note: multiple_default_rules_different_names
|
||||
skip: true
|
||||
data: {}
|
||||
|
||||
Reference in New Issue
Block a user