In Number::modulo, it calls Number::ints_to_bigint, which could panic. The reason is that calling .to_integer() isn't enough to guarantee that .to_bigint_owned() will return Some, but Number::ints_to_bigint assumes it will and calls unwrap(). In particular, it might be that it's a float corresponding to an integer that's larger than F64_SAFE_INTEGER. The fix is to not call Number::ints_to_bigint (and indeed to delete that entire function, which is only used in this one place), and instead only call unwrap when Some is returned.
* Deep-merge nested data documents in Engine::add_data
add_data previously performed a shallow merge: adding a nested object under a key that already existed either replaced the whole subtree or errored on a spurious conflict, instead of merging the trees. This makes Engine::add_data (and the shared Value::merge) recurse into nested objects so keys from both sides are preserved, matching OPA's data-document merge semantics. Nested sets are unioned as a regorus extension (OPA data is JSON and has no sets). Genuine leaf conflicts (same path, two different scalar values) still error; equal values remain a no-op, which the shared rule-evaluation path relies on. Adds tests for object deep-merge, set union, leaf/type conflicts, and interaction with the 'with data.x' modifier.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* docs(value): clarify Value::merge conflict wording
Copilot review on #760 noted the doc comment called non-mergeable variants 'non-container values', which is misleading since arrays are containers yet still conflict unless equal. Reword to describe a conflict as any differing pair that is not both objects or both sets (e.g. unequal scalars or arrays).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* perf(value): avoid deep-cloning RHS set during merge union
When unioning sets in Value::merge, the RHS set is often shared: the object arm recurses via existing.merge(v.clone()), which bumps the incoming set's Rc refcount. The old Rc::make_mut(new) then structurally deep-cloned the entire RHS BTreeSet just to drain it via append and immediately discard the copy.
Move the elements out when the RHS set is uniquely owned, and otherwise clone only the per-element Rc handles into the destination. The union result is identical (BTreeSet dedups), but no throwaway set is allocated on the nested-merge path exercised by add_data deep-merge.
Addresses a Copilot review comment on #760.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* fix(engine): make add_data atomic on merge conflict
Now that Value::merge recurses, a conflict in a later nested key was reported only after earlier keys of the same document had already been written into the live init_data, leaving the engine partially mutated on a rejected add_data.
Add a read-only Value::check_mergeable that mirrors merge's conflict rule (objects deep-merge, sets union, equal values no-op, anything else conflicts) and run it in add_data before merging. On conflict nothing is mutated, so add_data is all-or-nothing. The check allocates nothing and never copies the data spine, preserving merge's in-place uniquely-owned fast path (no candidate copy of the data document).
Adds regression tests for a partial object-leaf conflict and a partial set-union conflict. Reported by a maintainer on #760.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* test(engine): add array atomicity regression for add_data
Arrays are atomic leaves, so a differing array at a shared path is a
conflict. The new key sorts before the conflicting array key, so a naive
in-place merge would leak the new key before hitting the conflict. This
test locks in that add_data rejects the whole call and leaves data
untouched.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* fix: make add_data atomic under allocator memory limits
On llocator-memory-limits builds, Value::merge runs the limit check
*after* inserting each key, so an add_data whose merge trips the limit
mid-way left the data document partially mutated. check_mergeable only
models semantic conflicts, not limit failures, so the validate-then-merge
precheck couldn't cover this failure mode.
Use a build-split strategy in dd_data:
- default builds: keep the zero-copy validate-then-merge fast path
(a conflict is the only way the merge can fail).
- allocator-memory-limits builds: merge into a candidate copy and commit
only on success, making both conflict and limit failures transactional.
Value is Rc/copy-on-write, so only touched subtrees are cloned.
check_mergeable is now cfg-gated to the default build to avoid dead code.
Tests (allocator-memory-limits build): add a partial-merge atomicity test
(limit trips mid-merge, data must be untouched) and a candidate-copy
conflict-atomicity test.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* fix: separate strict rule-output merge from data-document deep-merge
#760 made Value::merge recursive so Engine::add_data deep-merges nested
data documents. But that same method also backs rule materialization,
where recursion is wrong: two rule definitions producing different
outputs for one path must conflict (OPA complete-rule semantics), not
silently combine.
Split the two behaviors:
- Value::merge is strict and shallow again (as pre-#760): a key on both
sides must be equal or it conflicts; used for rule outputs.
- Value::deep_merge is the recursive data-document merge behind add_data;
check_mergeable validates it up front without allocating, so the
default build merges in place instead of cloning a candidate.
Also fix zero-arg functions (f() := ...): route their materialization
through strict equality via a new RuleValueMerge selector, so disjoint
outputs ({a:1} vs {b:2}) conflict as OPA does while prefix scaffolding
(a.foo + a.bar) still combines.
Add a 14-case interpreter conformance matrix (multiple_outputs.yaml)
covering functions, static/dynamic partial objects, and ref-heads,
matched against OPA v1.2.0.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* perf(value): make deep_merge acquire mutable access lazily
deep_merge's object arm called Rc::make_mut on the target map up front,
cloning a shared map's spine even when the merge changed nothing (a
no-op subset re-add) or conflicted before any mutation. Decide each
incoming key from a read-only probe (skip / insert / recurse / conflict)
and take Rc::make_mut only when a key actually mutates, so no-op and
conflict merges leave shared maps untouched.
Behavior is unchanged: the equality short-circuit that previously ran
inside the recursive call now runs in the probe, and conflicts bail with
the same message. Add value tests asserting Rc::ptr_eq is preserved
across no-op subset, equal-nested-object, and first-key-conflict merges.
OPA conformance unchanged (3021 pass / 651 fail, byte-identical).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* feat(value): bound deep_merge recursion depth to prevent stack-overflow DoS
deep_merge and check_mergeable recursed unbounded on object/set nesting.
A Value built without serde_json's parse-time recursion limit (the Python
and Ruby native bindings, or programmatic construction) could therefore
drive add_data into a stack overflow -- an uncatchable abort that poisons
every engine in an FFI process.
Thread a depth counter through both functions and bail past MAX_MERGE_DEPTH
(128, matching serde_json's default) so over-deep data fails with a clean
Err. In the default build check_mergeable trips first, keeping add_data
atomic; the guard in deep_merge covers the allocator-memory-limits build
and any disjoint-then-overlapping merge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* docs(changelog): note strict zero-arg function conflict and add_data depth limit
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Mark Birger <markbirger@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Initial plan
* Add keywords_in_refs: allow reserved keywords as dot-notation field names
* Address review feedback: improve parse_ref_field doc comment and clean up test comment
* Add complex keyword-in-ref test cases
* Polish keyword-ref test expectations and validate coverage
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Partial object rules with dynamic keys (e.g. `violations[k] if { ... }`)
only produced a single entry instead of collecting all bindings. Two
independent bugs caused this:
1. Interpreter: the early-return optimization in eval_output_expr_in_loop
checked whether the rule_ref was constant but never verified whether
the key expression was also constant. A variable key like `k` was
treated as constant output, causing the loop to exit after the first
iteration. Fixed by gating early-return on key_expr constness.
2. RVM: compute_rule_type incorrectly classified `p[k] if { ... }` as
PartialSet instead of PartialObject. OPA v1 semantics define this
form as a partial object (key -> true). Fixed the classification and
added compiler error guards for patterns the RVM codegen cannot yet
handle (constant keys, nested bracket keys), ensuring graceful
fallback to the interpreter.
The OPA test harness now skips RVM validation per-case when partial
object compiler errors are raised, rather than blanket-skipping entire
folders. This preserves RVM coverage for unrelated tests in the same
folders.
Closes#712
Co-authored-by: Anand Krishnamoorthi <anakrish@microsoft.com>
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>
- Introduce ExecutionTimer/ExecutionTimerConfig to allow limiting evaluating time.
- To amortize time checking costs, checking interval can be configured via the notion of work units
- A global fallback time limit can be set to universally limit all evaluation in addition to engine level limit setting.
- Implement limnits in interpreter and RVM. In RVM, also handle suspend/resume so that time during pause is not counted.
- Add engine-level APIs to set/clear per-engine timer configuration and apply global fallback defaults.
- Surface execution-time limits through FFI and C# bindings
- Add C# tests and example usage to validate engine overrides, global fallback behavior, and compiled policy enforcement.
- Expand docs for execution-time limit
- Add interpreter YAML cases and VM unit tests for time-limit behavior and deterministic time sources.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Handle imports that don't use the `as` clause to create a binding.
These imports are bound to the last identifier in the imported path.
Fix both interpreter and compiler.
Add tests.
fixes#541
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
- Supply chain: Use the popular num-bigint crate for handling large integers
- Optimization: Handle f64, i64, u64 directly. These will be the most common instances of a number.
OPA number semantics isn't clear.
https://github.com/open-policy-agent/opa/issues/6281
As part of this change, we update the following failing tests:
- A local test that relies on what 15.3/3 evaluates to.
With our current change, we round in a different direction than what OPA does, but consistent
with Rust. We produce 5.1000000000000005 where as the OPA test expects 5.1.
There is no clear definition in Rego of what the right answer is. Moreover, policies should not
rely on exact floating point value comparison. Therefore this deviations is justified.
The test is patched to pass.
- Another local vm test that exercised 1.1 + 2.2
- Another local vm test that exercises 5.5 - 2.2
- An OPA test that expects that a large integer number say 10e308 is printed in exponent notation.
num-bigint does not print using scientific notation and instead prints all the digits.
The benefit of preserving this compatibility is not clear. We skip this test.
- Doc tests that exercised handling floating point numbers with more than 15 (what f64 supports)
digits of precision. There is no usecase for this scenario. The tests are updated to reflect
the behavior.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
- add a dedicated `compiler/destructuring_planner` feature that precomputes binding plans for assignments, parameters, and `some in` expressions
- enrich `ScopeContext` with same-scope tracking, local scheduling hints, and module globals so the planner enforces := shadowing rules without blocking parent scopes
- wire the planner through compiler, hoist, interpreter, and engine paths while updating binding plan variants and adding query traversal helpers for dependency analysis
- document the new planner architecture and ship interpreter regressions that exercise nested destructuring, shadowing, and error reporting
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Cryptographic builtins are removed due to various reasons like FIPS
compliance. Users needing crypto builtins are encouraged to use
extensions.
Deprecated functions are also removed.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Add `or` operator to Rego languages. Available via `rego-extensions`
Cargo feature.
If the evaluated lhs value is not false, null or undefined it is returned.
Otherwise rhs is evaluated and returned.
or operator has least precedence, and is left-associative.
closes#314
A block with a single or expression needs to be treated as a comprehension instead of a
set/array with 1 item. e.g.: {1 | 1 }, [2 | foo]
Allow successfully parsing object comprehensions as rule body
x if { 1:2 | 1 }
fixes#306, fixes#307
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Handle undefined values correctly in ordered-else. Previously an undefined value
in one of the blocks could cause the entire rule to evaluate to undefined.
Handle undefined values correctly in generic rule refs to prevent them from
propagating to output.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
In case of empty delimiter, Rust's split returns leading and trailing
empty strings whereas Golang's doesn't.
Change behavior to match Golang/OPA.
fixes#291
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Earlier scheduler only recognized rules and would raise an
`unsafe var` error on alias.
Register alias var names to fix this.
fixes#284
Also fix clippy warning treated as error
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
If a rule is written to produce a constant value, then not all iterations of loops
within it need to be executed. Execution can stop via early return once the first iteration
that produces a value has been executed.
This brings forth the question : What if one of the subsequent iterations would have resulted
in an error?
e.g:
x {
[1, "hello"][_] + 1
}
Such errors are not raised; consistent with OPA.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Second lookup of an object rule without fully qualified path, resulted
in returning the object instead of the requested field.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Previously we used to rely on whether there was a value in the
data document for a given rule path. This approach cannot handle
the case of evaluating a.b when a.b.c has been evaluated but
a.b.d has not been evaluated. Upon evaluating a.b.c, the data document
will already have a value of a.b even though a.b.d has not yet
been evaluated.
Hence we need to keep track of evaluated rules separately.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Allow `import input` instead of erroring out.
This import is redundant and has no effect.
Emit `print` messages to stderr onstead of stdout.
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Implement `import rego.v1`
https://www.openpolicyagent.org/docs/latest/policy-language/#the-regov1-import
- `if` required before rule body
- import rego.v1 automatically imports future.keywords
- handle import shadowing
- data, input cannot be shadowed
- deprecated functions as disallowed
- rules must have assignment or body
- `contains` required for parital set
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
- ignore worktrees
- feature guard time module
- Apply with modifiers before evaluating loop expressions
- Support value modifier for functions
- stubs for http.send and io.jwt.decode_verify
- Initialize with-document after initializing init data
- In case of conflict, with modifier override init-data values.
- In case of conflict, subsequent with modifier overrides earlier ones.
- Ensure that zero parameter functions are evaluated and added to document
- opa.runtime builtin
returns:
- git commit hash
- environment vars
- regorus features enabled
- builtins available
- deprecated builtins available
- If `sort_bindings` is specified, sort the bindings in OPA tests
- gather inputs, used vars and comprehensions in with modifiers
- For refs starting with `data`, ensure that modules are evaluated before looking up
value of the expression. Thie ensures that modules that have only been partly populated
(E.g via with mods) are completely evaluated before the value is looked up
- Mark rules overridden using with modifiers are evaluated.
- Exclude env vars in opa.runtime.
- Include regorus version in OPA runtime
- update to opa v0.60.0
- scheduler: Handle function refs in with modifers. Error out only if
a truly undefined ref.
- Handle undefined params, parameter expression evaluation errors before
applying with modifiers.
- When applying with modifiers, first determine whether the target is a
function. If so, handle cleanly.
- concat: raise error only in strict mode
- In strict mode, propagate errors raised by function rule execution
in case of multiple function definitions for same rule
- skip "withkeyword/builtin-builtin: arity 0" test which can never pass.
- When a mock has is being applied, clear with_function so that
other mocks won't be applied during the evaluation of the mock.
- Ability to specify strictness in tests
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
- object.union
- object.union_n
- treat negative integers as two separate tokens (Sub and Number)
when seen in arithmetic expressions
- Ensure that fully query string is parsed
- Evaluate queries in a separate module instead of the last read module.
This correctly handles queries of the form `x = data.test.y` where x is
already a ref in `data.test`
- Handle queries producing multiple outputs in test infrastructure
- Add tests for engine
- Add tests locking down valid queries
- Update opa.passing
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
- Remove unnecessary memory allocations
- Add --non-strict flag
- Ensure that only empty modules (ones without rules) are initialzed prior to evaluating rules.
- Record rule as entry for each of its prefixes.
For example, for a rule a.b.c =... in package test, record it in
rules["data.test.a"], rules["data.test.a.b"] and rules["data.test.a.b.c"]
This allows evaluating the correct list of rules based on expessions
a.b.c, a.b, a, data.test.a.b.c, data.test.a.b, data.test.a
Closes#69Closes#70
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
Number is implemented using rust_decimal::Decimal which uses a 96 bit mantissa.
TODO:
a) Support u64, i64 variants
b) Determine desired semantics for floating-point
c) Determine desired big integer length
d) Explore other big int/big float crates
Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
