diff --git a/src/languages/rego/compiler/loops.rs b/src/languages/rego/compiler/loops.rs index 327669c..3c1118c 100644 --- a/src/languages/rego/compiler/loops.rs +++ b/src/languages/rego/compiler/loops.rs @@ -13,7 +13,7 @@ use crate::ast::{self, ExprRef, LiteralStmt, Query}; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::compiler::hoist::{HoistedLoop, LoopType}; use crate::lexer::Span; -use crate::rvm::instructions::{LoopMode, LoopStartParams}; +use crate::rvm::instructions::{GuardMode, LoopMode, LoopStartParams}; use crate::rvm::Instruction; use crate::Value; use alloc::format; @@ -197,6 +197,19 @@ impl<'a> Compiler<'a> { *end = loop_end; } + // The loop writes its overall pass/fail into `result_reg` + // (`success_count == total_iterations` for `Every`). The enclosing query + // must fail (evaluate to undefined) when the quantifier does not hold, so + // guard on `result_reg` here. Without this the `every` result is computed + // but discarded, leaving the surrounding rule to always succeed. + self.emit_instruction( + Instruction::Guard { + register: result_reg, + mode: GuardMode::Condition, + }, + span, + ); + Ok(()) } @@ -312,6 +325,25 @@ impl<'a> Compiler<'a> { *end = loop_end; } + // A hoisted index-iteration loop inside an `every` body acts as a + // condition on the current iteration: if the indexed reference matches + // nothing the iteration must fail. The `every` body emits no context + // yield, so the loop result register is otherwise discarded (same + // situation as `some ... in`). Guard on it so a non-matching indexed + // reference fails the enclosing `every` iteration. + if matches!( + self.context_stack.last().map(|c| &c.context_type), + Some(ContextType::Every) + ) { + self.emit_instruction( + Instruction::Guard { + register: result_reg, + mode: GuardMode::Condition, + }, + collection.span(), + ); + } + Ok(()) } diff --git a/src/languages/rego/compiler/queries.rs b/src/languages/rego/compiler/queries.rs index f25c855..8ba78e2 100644 --- a/src/languages/rego/compiler/queries.rs +++ b/src/languages/rego/compiler/queries.rs @@ -70,12 +70,31 @@ impl<'a> Compiler<'a> { .. } = &stmt.literal { - self.compile_some_in_loop_with_remaining_statements( + let some_result_reg = self.compile_some_in_loop_with_remaining_statements( key, value, collection, &stmts[idx..], )?; + // Inside an `every` body a `some ... in` acts as a condition + // on the current iteration: if it matches nothing the + // iteration must fail. Unlike a top-level rule body (where + // per-iteration context yields produce the results), the + // `every` body has no yield, so the loop result register is + // otherwise discarded. Guard on it so a `some` that matches + // nothing fails the enclosing `every` iteration. + if matches!( + self.context_stack.last().map(|c| &c.context_type), + Some(ContextType::Every) + ) { + self.emit_instruction( + Instruction::Guard { + register: some_result_reg, + mode: GuardMode::Condition, + }, + &stmt.span, + ); + } return Ok(()); } } diff --git a/src/rvm/vm/loops.rs b/src/rvm/vm/loops.rs index f6ecf11..fa21ad1 100644 --- a/src/rvm/vm/loops.rs +++ b/src/rvm/vm/loops.rs @@ -495,8 +495,15 @@ impl RegoVM { // over a virtual null element. Ok(Some(IterationState::Single { consumed: false })) } else { - // Standard Rego or count/forEach: non-collection → immediate result. - let result = non_collection_result(mode); + // Standard Rego: iterating a non-collection scalar (number, + // string, bool, null, undefined) yields no iterations. For + // `every` this makes the quantifier undefined (it fails) — it + // is NOT vacuously true, which only applies to a genuinely + // empty collection. `any`/`forEach` remain false. + let result = match *mode { + LoopMode::Every => Value::Undefined, + LoopMode::Any | LoopMode::ForEach => Value::Bool(false), + }; self.set_register(params.result_reg, result)?; self.pc = usize::from(params.loop_end).saturating_sub(1); Ok(None) diff --git a/tests/opa.rs b/tests/opa.rs index 5392b64..79be338 100644 --- a/tests/opa.rs +++ b/tests/opa.rs @@ -26,7 +26,6 @@ const OPA_TODO_FOLDERS: &[&str] = &[ "baseandvirtualdocs", "dataderef", "defaultkeyword", - "every", "fix1863", "functions", "partialdocconstants", @@ -102,6 +101,20 @@ fn log_rvm_skip(case_note: &str, folder_name: Option<&str>) { } } +/// Allows temporarily enabling RVM verification for folders otherwise listed in +/// `OPA_TODO_FOLDERS`, without editing the source. Set `OPA_UNSKIP_FOLDERS` to a +/// comma-separated list of folder names (e.g. `every,functions`) or `all`. +/// Intended for auditing latent RVM bugs in currently-skipped constructs. +fn folder_rvm_unskipped(folder: &str) -> bool { + match std::env::var("OPA_UNSKIP_FOLDERS") { + Ok(list) => { + let list = list.trim(); + list.eq_ignore_ascii_case("all") || list.split(',').any(|f| f.trim() == folder) + } + Err(_) => false, + } +} + fn setup_engine_for_case(case: &TestCase, is_rego_v0_test: bool) -> Result { let mut engine = Engine::new(); @@ -388,7 +401,7 @@ fn run_opa_tests(opa_tests_dir: String, folders: &[String]) -> Result<()> { let folder_name = folder_name_from_path(path_dir); let skip_rvm_for_folder = folder_name .as_deref() - .map(|folder| OPA_TODO_FOLDERS.contains(&folder)) + .map(|folder| OPA_TODO_FOLDERS.contains(&folder) && !folder_rvm_unskipped(folder)) .unwrap_or(false); if path.is_dir() { diff --git a/tests/rvm/rego/cases/loops_and_quantifiers.yaml b/tests/rvm/rego/cases/loops_and_quantifiers.yaml index 1449e41..ac2c2ec 100644 --- a/tests/rvm/rego/cases/loops_and_quantifiers.yaml +++ b/tests/rvm/rego/cases/loops_and_quantifiers.yaml @@ -44,6 +44,295 @@ cases: query: data.test.main want_result: true + - note: every_body_fails_for_one_element + data: {} + modules: + - | + package test + main := result if { + every x in [1, 2, 3] { + x > 1 + } + result := true + } + query: data.test.main + want_result: "#undefined" + + - note: every_body_fails_for_all_elements + data: {} + modules: + - | + package test + main := result if { + every x in [1, 2, 3] { + x > 100 + } + result := true + } + query: data.test.main + want_result: "#undefined" + + - note: every_used_directly_as_condition_false + data: {} + modules: + - | + package test + allowed if { + every x in [1, 2, 3] { + x > 1 + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_used_directly_as_condition_true + data: {} + modules: + - | + package test + allowed if { + every x in [1, 2, 3] { + x > 0 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_over_non_iterable_number_is_undefined + data: {} + modules: + - | + package test + allowed if { + every x in 42 { + x > 1 + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_over_non_iterable_string_is_undefined + data: {} + modules: + - | + package test + allowed if { + every x in "hello" { + x == x + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_over_empty_array_is_vacuously_true + data: {} + modules: + - | + package test + allowed if { + every x in [] { + x > 1 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_with_inner_some_matching_nothing_fails + data: {} + modules: + - | + package test + allowed if { + every c in [1, 2] { + some x in [] + c == x + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_with_inner_some_matching_succeeds + data: {} + modules: + - | + package test + allowed if { + every c in [1, 2] { + some x in [1, 2, 3] + c == x + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_with_hoisted_index_matching_nothing_fails + data: {} + modules: + - | + package test + allowed if { + every c in [1] { + some i + [2, 3][i] == c + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_with_hoisted_index_matching_succeeds + data: {} + modules: + - | + package test + allowed if { + every c in [2, 3] { + some i + [1, 2, 3][i] == c + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_keyval_over_object_succeeds + data: {} + modules: + - | + package test + allowed if { + every k, v in {"a": 1, "b": 2} { + v > 0 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_keyval_over_object_fails + data: {} + modules: + - | + package test + allowed if { + every k, v in {"a": 1, "b": 2} { + v > 1 + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_over_set_domain_succeeds + data: {} + modules: + - | + package test + allowed if { + every x in {1, 2, 3} { + x > 0 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_over_empty_object_is_vacuously_true + data: {} + modules: + - | + package test + allowed if { + every k, v in {} { + v > 1 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_over_empty_set_is_vacuously_true + data: {} + modules: + - | + package test + allowed if { + every x in set() { + x > 1 + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_over_undefined_domain_is_undefined + data: {} + modules: + - | + package test + allowed if { + every x in input.missing { + x > 0 + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_nested_inside_every_succeeds + data: {} + modules: + - | + package test + allowed if { + every row in [[1, 2], [3, 4]] { + every c in row { + c > 0 + } + } + } + main := allowed + query: data.test.main + want_result: true + + - note: every_nested_inside_every_fails + data: {} + modules: + - | + package test + allowed if { + every row in [[1, 2], [3, 0]] { + every c in row { + c > 0 + } + } + } + main := allowed + query: data.test.main + want_result: "#undefined" + + - note: every_with_outer_binding + data: {} + modules: + - | + package test + allowed if { + threshold := 5 + every x in [6, 7, 8] { + x > threshold + } + } + main := allowed + query: data.test.main + want_result: true + - note: simple_loop_test data: {} modules: