Harden timed value traversal and serialization resource errors by kusha · Pull Request #920 · microsoft/regorus

Mark Birger (kusha) · 2026-11-06T10:57:34Z

Summary

Value traversal/drop hardening targeting main. Includes the iterative
Value/container comparison, formatting, serialization budget, and final-owner
destruction from anakrish/hardening-value-safety at
5cd97dc7de316269de127f3500c47b148c682ce9, plus the focused supplement below.

Integrate timer-aware iterative RVM comparisons and membership, including
same-type values, string checkpoints, resumed timeout cleanup, and recovery.
Preserve generic Serde nesting exhaustion as a typed fatal resource error
through strict/non-strict builtin calls, rules, function alternatives, and
default-function comprehensions. Ordinary type-error behavior is unchanged.
Bound strict comparison and shared builtin diagnostics with short structural
summaries while preserving public Value comparison, Debug, and successful
JSON semantics.
Add isolated regressions requesting exactly 1,048,576-byte stacks for deep
equality, ordering, Debug, JSON/Display/native returned-value formatting,
destruction, ownership/COW, deadline exhaustion, and process recovery.

The supplement does not change the inherited production destructor. Its tests
exercise the final-owner drain and preserve early exhausted Object iterator
release.

After retargeting the PR, main was merged into the feature branch. Its
comparison deadline checks are integrated with the iterative traversal and
interruptible set membership, and its comparison regression coverage is retained.
The original hardening commits are preserved without rebasing.

Validation

Validated on Rust 1.95.0, with that toolchain inherited by xtask children:

cargo +1.95.0 xtask ci-debug
All 10 RVM comparator tests, combining main's deadline regressions with
extended iterative traversal, interruption, and native-equivalence cases.
Targeted value_drop_safety, value_serialization_limits, and
value_walk_safety suites: all features (5/10/1 tests), Rc/std (5/7/1),
and Rc with allocator memory limits (4/7/1).
cargo +1.95.0 clippy --all-targets --all-features -- -D warnings
cargo +1.95.0 xtask test-no-std for thumbv7m-none-eabi
cargo +1.95.0 test --manifest-path bindings/ffi/Cargo.toml -- --test-threads=1:
75 passed, including the exact 1 MiB native worker.
Formatting and whitespace checks.

The default-function comprehension regression reproduced lost LimitError
identity before the correction. It now checks exact typed identity for both
strict settings and exact healthy output after reusing the same engine.

Review

Two full deep-review pipelines were completed. Confirmed findings were fixed,
with failing regressions demonstrated before their corrections and validation
rerun afterward. The second review found the default-function identity gap and
its missing regression; both are included in the separate follow-up commit.
A third full deep-review pipeline has not been run after that focused fix or
the target-branch merge. The merged tree was validated again on Rust 1.95.0.

The review skill's named models were unavailable; reviewers used runtime-default
models with distinct methodologies and a fresh adversarial verifier.

Compatibility and limitations

Timed set membership deliberately uses an O(n) sorted scan; untimed membership
keeps native B-tree lookup. Other implicit B-tree comparisons remain
infallible, and scalar number comparison is an indivisible checkpoint unit.
Generic Serde callbacks retain the upstream 64-container budget; this is not
an input admission limit. Iterative Display/JSON output is not subject to
that callback budget.
Allocation-free evidence applies to single-child array/set chains, not
arbitrary branching/object shapes. Cooperative limits do not guarantee
successful cleanup under physical allocator exhaustion.
Tests request an exact 1 MiB thread stack; actual reservation/rounding and
frame sizes remain platform-dependent.
Miri is unavailable for the installed Windows MSVC toolchain. CMake is
absent, so separate C/C++ and all-nine-binding smoke gates were not run.
Existing unsupported Rc+rvm and no_std+arc+rvm combinations fail in untouched
compiler/indexmap code and are not claimed to pass.
Native FFI tests run serially because unrelated tests share a process-global
memory limit.

Use checked template and alias prefix slices, consume complete UTF-8 layout characters, handle empty array subsets, and reserve numeric ranges fallibly with checked counts. Add focused interpreter, RVM, and alias-loading regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2665dfb6-f2d7-48ed-bdd8-ca120a5df099

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2665dfb6-f2d7-48ed-bdd8-ca120a5df099

Reuse iterative comparisons for timer-aware RVM execution, preserve serialization limits through engine errors, and bound strict diagnostics. Add isolated 1 MiB traversal, formatting, recovery, and ownership regressions without changing upstream destruction. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Restore caller state before propagating typed resource errors from default functions. Add strict/non-strict identity and recovery coverage and document the guarantee. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Comparison traversal can grow exponentially and bypass per-execution memory budgets, while additional limit-error propagation and cleanup defects can mask failures or retain execution state.

6 open issues

What changed in this PR

Hardens value handling across Regorus’s interpreter, Rego Virtual Machine (RVM), and native FFI, building on the existing iterative traversal and destruction implementations.

Changes:

Add timer-aware comparisons and membership checks, with cleanup after resource failures.
Preserve typed serialization-depth errors and bound builtin diagnostic output.
Add isolated 1 MiB-stack regressions for traversal, serialization, destruction, ownership, and recovery.

File	Description
`tests/value_walk_safety.rs`	Tests deep traversal, deadlines, formatting, and recovery.
`tests/value_serialization_limits.rs`	Tests serialization budgets and error propagation across execution paths.
`tests/value_drop_safety.rs`	Tests destruction, shared ownership, and allocation behavior.
`src/value/tests.rs`	Updates diagnostic assertions and tests serialization-error identity.
`src/value/set/serde.rs`	Propagates serialization-budget exhaustion tracking through sets.
`src/value/object/serde.rs`	Propagates serialization-budget exhaustion tracking through objects.
`src/value/mod.rs`	Adds checked traversal, bounded diagnostics, and typed serialization handling.
`src/value/array/serde.rs`	Propagates serialization-budget exhaustion tracking through arrays.
`src/utils/limits/error.rs`	Adds the typed serialization-depth resource error.
`src/rvm/vm/state.rs`	Extends cleanup for deadline and serialization-depth failures.
`src/rvm/vm/rules.rs`	Classifies serialization-depth failures as fatal.
`src/rvm/vm/mod.rs`	Registers the comparison module.
`src/rvm/vm/machine.rs`	Extracts reusable timer ticking and maps serialization-depth errors.
`src/rvm/vm/errors.rs`	Adds the VM serialization-depth error variant and conversion.
`src/rvm/vm/dispatch.rs`	Routes comparisons and membership through checked helpers.
`src/rvm/vm/comparison.rs`	Implements timer-aware comparison and membership helpers with tests.
`src/interpreter.rs`	Preserves resource errors through function alternatives and defaults.
`src/builtins/utils.rs`	Bounds shared builtin argument diagnostics.
`src/builtins/encoding.rs`	Preserves resource-error identity during JSON and YAML serialization.
`docs/rvm/vm-runtime.md`	Documents traversal checkpoints, serialization budgets, and cleanup behavior.
`bindings/ffi/src/value_safety_tests.rs`	Tests native formatting, serialization limits, destruction, and recovery.
`bindings/ffi/src/rvm.rs`	Registers the native value-safety tests.

_{Review effort: Max}

+                    if e.downcast_ref::<crate::LimitError>().is_some() {
+                        if let Some(with_functions) = with_functions_saved {
+                            self.with_functions = with_functions;
+                        }
+                        return Err(e);


+                                    if e.downcast_ref::<crate::LimitError>().is_some() {
+                                        self.scopes = scopes;


Keep iterative timer-aware comparisons and interruptible set membership, retain main comparison regressions, and reconcile resource-error mapping and runtime documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Comparison and JSON-output paths can bypass memory limits or exhaust the process, while serializing valid deeply nested programs can panic.

13 open issues

_{Review effort: Max}

+
+impl PartialEq for Value {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other).is_eq()


+        SerializeWithBudget {
+            value: self,
+            remaining: MAX_SERIALIZATION_CONTAINERS,
+            limit_exceeded: None,
+        }
+        .serialize(serializer)


Address High AI review feedback on comparison scratch accounting, aliased ordering, iterative JSON output, fallible program serialization, and Miri subprocess tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Unresolved critical findings allow exponential equality traversal on compact DAGs and swallowed resource-limit errors that can let evaluation succeed after exhaustion.

9 open issues

9 resolved since last review

_{Review effort: Max}

-                left.cmp(right)
-            });
+        if !self.needs_checked_comparison() {
+            return Ok(self.get_register(left)?.cmp(self.get_register(right)?));



+impl PartialEq for Array {
+    fn eq(&self, other: &Self) -> bool {
+        self.len() == other.len() && self.cmp(other).is_eq()


+    fn scalar<T: Serialize + ?Sized>(&mut self, value: &T) -> core::result::Result<(), E> {
+        #[cfg(feature = "std")]
+        {
+            let result = value.serialize(&mut serde_json::Serializer::new(&mut *self));


+        match self.json_string(false) {
            Ok(s) => write!(f, "{s}"),
-            Err(_e) => Err(fmt::Error),
+            Err(e) => write!(f, "<value cannot be serialized: {e}>"),


-                    let key_str = serde_json::to_string(k).map_err(Error::custom)?;
-                    map.serialize_entry(&key_str, v)?;
+                    // Non-string keys retain their compact JSON string encoding.
+                    let key_str = k.json_string(false).map_err(Error::custom)?;


Resolve the numeric range conflict by retaining main allocation checks and the existing value hardening. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Critical issues can silently turn memory-limit failures into successful policy results and cause exponential work in equality comparisons.

13 open issues

_{Review effort: Max}


+impl PartialEq for Set {
+    fn eq(&self, other: &Self) -> bool {
+        self.len() == other.len() && self.cmp(other).is_eq()


+            Value::Set(ref items) => {
+                for item in items.iter_sorted() {


+                    if iter.len() != 0 {
+                        pending.push(ReleaseFrame::Object(iter));
+                    }
+                    pending.push(ReleaseFrame::Value(value));


+        // serde_json does not expose its writer-based Serializer without std.
+        #[cfg(not(feature = "std"))]
+        {
+            self.append(&serde_json::to_vec(value).map_err(anyhow::Error::new)?)


Anand Krishnamoorthi (anakrish)

LGTM. The following must be fixed in subequent PRs

• Restore nested equality shortcuts across Value , arrays, sets, objects, and unchecked RVM equality to prevent newly exponential comparisons.
• Preserve typed resource errors through non-string-key serialization and function-valued with replacements so exhaustion cannot be swallowed.
• Propagate serialization failures at semantic string-conversion sites instead of returning diagnostic text as successful policy data.
• Restore FFI query-result and host-argument JSON compatibility for accepted values deeper than 64 containers.
• Preserve efficient scalar set membership under memory limits instead of replacing logarithmic lookups with linear scans.

Anand Krishnamoorthi (anakrish) and others added 4 commits November 5, 2026 16:03

fix: use iterative value processing

5cd97dc

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2665dfb6-f2d7-48ed-bdd8-ca120a5df099

Anand Krishnamoorthi (anakrish) requested a max review from Copilot November 6, 2026 12:25

Copilot started reviewing on behalf of Anand Krishnamoorthi (anakrish) November 6, 2026 12:26

View session

Mark Birger (kusha) changed the base branch from anakrish/hardening-value-safety to main November 6, 2026 12:37

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Anand Krishnamoorthi (anakrish) requested a max review from Copilot November 6, 2026 13:37

Copilot started reviewing on behalf of Anand Krishnamoorthi (anakrish) November 6, 2026 13:38

View session

Mark Birger (kusha) mentioned this pull request Nov 6, 2026

Harden core boundaries and avoid repeated comprehension parsing #924

Merged

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Anand Krishnamoorthi (anakrish) requested a max review from Copilot November 6, 2026 15:48

Copilot started reviewing on behalf of Anand Krishnamoorthi (anakrish) November 6, 2026 15:48

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Merge main and preserve range allocation preflight

32e498e

Resolve the numeric range conflict by retaining main allocation checks and the existing value hardening. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Anand Krishnamoorthi (anakrish) requested a max review from Copilot November 6, 2026 17:09

Copilot started reviewing on behalf of Anand Krishnamoorthi (anakrish) November 6, 2026 17:10

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Anand Krishnamoorthi (anakrish) approved these changes Nov 6, 2026

View reviewed changes

Anand Krishnamoorthi (anakrish) merged commit 76cdb52 into microsoft:main Nov 6, 2026
60 checks passed

This was referenced Nov 6, 2026

Restore value equality shortcuts and preserve fallible serialization errors kusha/regorus#6

Closed

Restore value equality shortcuts and preserve fallible serialization errors #927

Merged

Navigation Menu

Repository navigation

Harden timed value traversal and serialization resource errors - #920

Harden timed value traversal and serialization resource errors#920
Anand Krishnamoorthi (anakrish) merged 7 commits into
microsoft:mainfrom
kusha:kusha-value-walk-drop-improvements

Mark Birger (kusha) commented Nov 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Anand Krishnamoorthi (anakrish) left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if e.downcast_ref::<crate::LimitError>().is_some() {
		self.scopes = scopes;

Conversation

Mark Birger (kusha) commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Review

Compatibility and limitations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Anand Krishnamoorthi (anakrish) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mark Birger (kusha) commented Nov 6, 2026 •

edited

Loading