kata-containers/regorus
1
0
Fork 0
mirror of https://github.com/microsoft/regorus.git synced 2026-08-05 02:16:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: refine constant-key bracket rule handling

Browse Source 
Agent-Logs-Url: https://github.com/microsoft/regorus/sessions/272a971a-ae52-45ae-8cb3-714e747599c4

Co-authored-by: anakrish <35780660+anakrish@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-05-06 22:19:56 +00:00
committed by GitHub
parent bec159a580
commit 78f226f957
9 changed files with 190 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
src/interpreter.rs
View File

@@ -4,9 +4,9 @@
use crate::ast::*;
use crate::builtins::{self, BuiltinFcn};
use crate::compiled_policy::CompiledPolicyData;
#[cfg(feature = "azure_policy")]
use crate::compiled_policy::TargetInfo;
use crate::compiled_policy::{CompiledPolicyData, DefaultRuleInfo};
use crate::compiler::destructuring_planner::{
AssignmentPlan, BindingPlan, DestructuringPlan, WildcardSide,
};
@@ -1724,6 +1724,9 @@ impl Interpreter {
// For now, we restrict constant refs to those that contain only simple literals.
fn is_constant_ref(mut expr: &Ref<Expr>) -> Result<bool> {
loop {
if Self::is_simple_literal(expr)? {
return Ok(true);
}
match expr.as_ref() {
Expr::Var { .. } => break,
Expr::RefDot { refr, .. } => expr = refr,
@@ -1747,6 +1750,27 @@ impl Interpreter {
))
}
fn is_constant_key_expr(&self, expr: &Ref<Expr>) -> Result<bool> {
if Self::is_simple_literal(expr)? {
return Ok(true);
}
match expr.as_ref() {
Expr::Var { span, .. } => {
let scope = self
.scopes
.last()
.ok_or_else(|| anyhow!("internal error: no current scope"))?;
Ok(!scope.contains_key(&span.source_str()))
}
Expr::RefDot { refr, .. } => self.is_constant_key_expr(refr),
Expr::RefBrack { refr, index, .. } => {
Ok(self.is_constant_key_expr(refr)? && self.is_constant_key_expr(index)?)
}
_ => Ok(false),
}
}
// A rule's output expression is constant if it does not contain local variables.
// For now, we restrict output expressions to those that contain only simple literals.
fn is_constant_output(key_expr: &Option<Ref<Expr>>, output_expr: &Ref<Expr>) -> Result<bool> {
@@ -1798,8 +1822,10 @@ impl Interpreter {
output
} else {
// Implicit-true partial object rules can vary with each successful key binding.
if key_expr.is_some() && !is_old_style_set {
is_const_rule = false;
if let Some(ke) = &key_expr {
if !is_old_style_set && !self.is_constant_key_expr(ke)? {
is_const_rule = false;
}
}
Value::Bool(true)
};
@@ -2945,6 +2971,41 @@ impl Interpreter {
Ok(())
}
fn default_rules_for_path(&self, path: &str) -> Option<Vec<DefaultRuleInfo>> {
if let Some(rules) = self.compiled_policy.default_rules.get(path) {
return Some(rules.clone());
}
let (parent_path, index) = path.rsplit_once('.')?;
let rules = self.compiled_policy.default_rules.get(parent_path)?;
let matches = rules
.iter()
.filter(|(_, rule_index)| Self::default_rule_index_matches(rule_index, index))
.cloned()
.collect::<Vec<_>>();
if matches.is_empty() {
None
} else {
Some(matches)
}
}
fn has_default_rules_for_path(&self, path: &str) -> bool {
self.default_rules_for_path(path).is_some()
}
fn default_rule_index_matches(index: &Option<String>, path_component: &str) -> bool {
match index.as_deref() {
Some(index) if index == path_component => true,
Some(index) => index
.strip_prefix('"')
.and_then(|index| index.strip_suffix('"'))
.is_some_and(|index| index == path_component),
None => false,
}
}
fn ensure_rule_evaluated(&mut self, path: String) -> Result<()> {
self.check_execution_time()?;
let mut matched = false;
@@ -2959,9 +3020,9 @@ impl Interpreter {
}
// Evaluate the associated default rules after non-default rules
if let Some(rules) = self.compiled_policy.default_rules.get(&path) {
if let Some(rules) = self.default_rules_for_path(&path) {
matched = true;
for (r, _) in rules.clone() {
for (r, _) in rules {
if !self.processed.contains(&r) {
let module = self.get_rule_module(&r)?;
let prev_module = self.set_current_module(Some(module))?;
@@ -3052,10 +3113,7 @@ impl Interpreter {
let prefix = fields.iter().take(i).copied().collect::<Vec<_>>();
let prefix_path = format!("data.{}", prefix.join("."));
if self.compiled_policy.rules.contains_key(&prefix_path)
|| self
.compiled_policy
.default_rules
.contains_key(&prefix_path)
|| self.has_default_rules_for_path(&prefix_path)
{
self.ensure_rule_evaluated(prefix_path)?;
break;
@@ -3079,7 +3137,7 @@ impl Interpreter {
if !no_error
&& !self.compiled_policy.rules.contains_key(&rule_path)
&& !self.compiled_policy.default_rules.contains_key(&rule_path)
&& !self.has_default_rules_for_path(&rule_path)
&& !self.compiled_policy.imports.contains_key(&rule_path)
{
bail!(span.error(&format!(
@@ -3102,7 +3160,7 @@ impl Interpreter {
};
if self.compiled_policy.rules.contains_key(&path)
|| self.compiled_policy.default_rules.contains_key(&path)
|| self.has_default_rules_for_path(&path)
{
self.ensure_rule_evaluated(path)?;
found = true;
@@ -3649,7 +3707,7 @@ impl Interpreter {
self.data = Value::Undefined;
self.ensure_loop_var_values_capacity();
let default_rules = self.compiled_policy.default_rules.get(rule_path).cloned();
let default_rules = self.default_rules_for_path(rule_path);
if let Some(rules) = default_rules {
for (rule, _) in rules {

4
src/languages/rego/compiler/program.rs
View File

@@ -181,10 +181,6 @@ impl<'a> Compiler<'a> {
}
fn evaluate_default_rule(&mut self, rule_path: &str) -> Option<u16> {
if !self.policy.inner.default_rules.contains_key(rule_path) {
return None;
}
let mut interpreter = Interpreter::new_from_compiled_policy(self.policy.inner.clone());
match interpreter.eval_default_rule_for_compiler(rule_path) {

7
src/languages/rego/compiler/rules.rs
View File

@@ -56,7 +56,12 @@ impl<'a> Compiler<'a> {
match head {
RuleHead::Set { .. } => RuleType::PartialSet,
RuleHead::Compr { refr, .. } => match refr.as_ref() {
crate::ast::Expr::RefBrack { .. } => RuleType::PartialObject,
crate::ast::Expr::RefBrack { index, .. }
if super::expressions::try_eval_const(index.as_ref()).is_none() =>
{
RuleType::PartialObject
}
crate::ast::Expr::RefBrack { .. } => RuleType::Complete,
_ => RuleType::Complete,
},
_ => RuleType::Complete,

5
src/rvm/vm/rules.rs
View File

@@ -17,8 +17,9 @@ use super::execution_model::{
use super::machine::RegoVM;
impl RegoVM {
/// Returns true if the error represents a resource-limit violation that
/// must never be silently absorbed by rule evaluation.
/// Returns true if the error must never be silently absorbed by rule
/// evaluation backtracking, including resource-limit failures and semantic
/// rule consistency errors.
pub(super) const fn is_fatal_vm_error(err: &VmError) -> bool {
matches!(
err,

29
tests/interpreter/cases/default/basic.yaml
View File

@@ -341,6 +341,35 @@ cases:
result: false
reasons: []
- note: default_rule_with_object_key
data: {}
input: {}
modules:
- |
package test
import rego.v1
default config["timeout"] := 30
config["timeout"] := input.val if {
val := input.val
}
query: data.test.config.timeout
want_result: 30
- note: default_rule_with_object_key_override
data: {}
input:
val: 60
modules:
- |
package test
import rego.v1
default config["timeout"] := 30
config["timeout"] := input.val if {
val := input.val
}
query: data.test.config.timeout
want_result: 60
- note: default_only_rule_with_package_query
data: {}
modules:

20
tests/interpreter/cases/rule/partial_object_iteration.yaml
View File

@@ -114,6 +114,26 @@ cases:
BAZ: true
FOO: true
- note: constant_key_implicit_true_rule_is_complete_v1
data: {}
input:
enabled: true
other: false
modules:
- |
package test
import rego.v1
p["x"] if {
input.enabled
}
p["x"] if {
input.other
}
query: data.test.p.x
want_result: true
- note: partial_object_duplicate_keys_same_value_are_ok_v1
data: {}
input:

22
tests/rvm/rego/cases/default_rules.yaml
View File

@@ -48,18 +48,34 @@ cases:
want_result: true
- note: default_rule_with_object_key
skip: true # TODO: Fix rule type classification for config["timeout"] - should be Complete, not PartialObject
data: {}
input: {}
modules:
- |
package test
import rego.v1
default config["timeout"] := 30
config["timeout"] := 60 if {
false # This will fail
config["timeout"] := input.val if {
val := input.val
}
query: data.test.config.timeout
want_result: 30
- note: default_rule_with_object_key_override
data: {}
input:
val: 60
modules:
- |
package test
import rego.v1
default config["timeout"] := 30
config["timeout"] := input.val if {
val := input.val
}
query: data.test.config.timeout
want_result: 60
- note: default_rule_complex_value
data: {}
modules:

20
tests/rvm/rego/cases/partial_object_rules.yaml
View File

@@ -109,6 +109,26 @@ cases:
BAZ: true
FOO: true
- note: constant_key_implicit_true_rule_is_complete_v1
data: {}
input:
enabled: true
other: false
modules:
- |
package test
import rego.v1
p["x"] if {
input.enabled
}
p["x"] if {
input.other
}
query: data.test.p.x
want_result: true
- note: partial_object_duplicate_keys_same_value_are_ok_v1
data: {}
input:

24
tests/rvm/vm/suites/object_operations.yaml
View File

@@ -8,7 +8,7 @@
cases:
- note: object_key_collision_conflict
description: Setting same key twice with different values should raise a rule output conflict
example_rego: "p[key] := value if { value := [1, 2][_] }"
example_rego: "p[\"key\"] := value if { value := [1, 2][_] }"
literals:
- {}
- "key"
@@ -30,6 +30,28 @@ cases:
- "Return { value: 0 }"
want_error: "multiple outputs"
- note: object_key_duplicate_same_value
description: Setting same key twice with the same value should succeed
example_rego: "p[\"key\"] := 1 if { some _ in [0, 1] }"
literals:
- {}
- "key"
- 1
instruction_params:
object_create_params:
- dest: 0
template_literal_idx: 0
literal_key_fields: []
fields: []
instructions:
- "ObjectCreate { params_index: 0 }"
- "Load { dest: 1, literal_idx: 1 }" # key
- "Load { dest: 2, literal_idx: 2 }" # value
- "ObjectSet { obj: 0, key: 1, value: 2 }"
- "ObjectSet { obj: 0, key: 1, value: 2 }"
- "Return { value: 0 }"
want_result: {"key": 1}
- note: object_dynamic_key_generation
description: Generate object keys dynamically from loop iteration
example_rego: "{sprintf(\"key_%d\", [i]): i | i := [1, 2][_]}"