Repository navigation
nvx-rs: retire the egress policy fixture and read policies as strict JSON - #828
Merged
Pedro Henrique Penna (ppenna) merged 8 commits intoOct 25, 2026
Merged
Conversation
The tests that are to replace the egress policy fixture (#729) check that an action is refused exactly when it needs more than the 256 native rules that OpenVMM accepts. A refused action emits nothing, so a test has to learn how many rules the action needs some other way: by lowering its rules under a higher limit. Split the two steps that need it out of the functions that run them, without changing what they do: - lowering::lower_within lowers the rules of an action within a limit of native rules that its caller chooses. lower keeps OpenVMM's limit, MAX_RULES_PER_ACTION, and the message of a refusal still names it. - policy::parse_category parses the rules of an action, which compile_category then lowers, so that a test can parse an action and lower it under another limit. Every policy compiles to the same rules or fails with the same message as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The egress policy compiler is checked against a fixture of what the retired Python compiler did with 784 inputs (#722). Its expectations are recorded output, which a deliberate change of behavior can only edit by hand. Add tests whose expectations come from the format that doc/run.md documents instead (#729), without changing the compiler: - A reference model, in policy/tests/model.rs, which shares no code with the compiler. It reads a policy file as the format describes it, with serde_json and the standard library's address parsers, and refuses what the format refuses. It decides whether a rule matches a packet of a family, address, protocol (TCP, UDP, ICMP, or another), and port, and parses native rules, CIDR[:PROTOCOL[:PORT[-LAST]]], which it matches as doc/run.md describes OpenVMM's rules. - An exact comparison: for each family and protocol, the boundaries of the source and native networks, exceptions, and port ranges partition the packets into cells within which every rule matches all packets or none, and the model evaluates one point of each cell, ports 0 and 65535 included. The model also finds redundant native rules, and checks that native rules are canonical: written as it writes them, without host bits, sorted without duplicates, with no network that holds or merges with another of the same selector, and no overlapping or adjacent port ranges of one network. - A proptest generator, in policy/tests/generator.rs, ported with its probabilities from Corpus, which drew the fixture's seed-359 policies in the retired scripts/nvx_rs_fixtures.py (36c5bf7^): overlapping networks in dense IPv4 and IPv6 neighborhoods, exceptions, ports at range boundaries, the MXC and flat forms, every spelling of a network, crowds of rules near the limit, broken policies, and files written as json.dumps writes them and sometimes broken. A crowd of address-only and protocol-wide rules, which Corpus did not draw, takes the first steps of the lowering to the limit too. - Properties, in policy/tests/properties.rs: each action lowers to canonical native rules, none redundant, that match exactly what the model matches; an action is refused exactly when it needs more than 256 native rules, which the tests learn by lowering it within a lifted limit of 1024, and then under either name, alone or beside the other action, so allow and deny lower independently; the compiler refuses what the model refuses, and compiles what it reads unless an action needs too many rules; reordering rules, repeating them, and splitting a network into its halves change no lowering; a policy's text, its value, and the NetworkPolicy rules of openvmm_arguments compile alike and deterministically, and with the cli feature compiled rules read back as native rules lower to themselves; and arbitrary bytes and broken policies compile or fail with an error. - networks_at_the_ends_of_each_family_have_the_properties checks the properties of networks at the ends of each family's addresses, such as ::/0 less its last address, without ports and with them, which the generator draws only by chance. Without it, a summary that stops one network short when the next address is ffff:...:fffe passes every other test, the fixture's included. - every_fixture_case_has_the_properties_that_replace_the_fixture runs the properties once over the fixture's 784 inputs, where the model agrees with the compiler on every one, before a later change deletes the fixture. Checks of the model itself pin how it reads CIDRs and native rules, that it finds what it looks for, and that it reads every spelling of the generator as the network that it spells. Applied one at a time to the lowering and the address parsing, mutants that write a range of one port as a range, extend a swept range one port too far, leave the rules unsorted, keep networks that other rules cover, leave adjacent intervals unmerged, ignore exceptions, lower `any` with ports to TCP alone, forget what protocol-wide rules cover, stop summaries at /16, accept ports for ICMP, or let the families or the address-only step overrun the limit each fail several of the properties. The properties take about 12 seconds in a debug build. The README describes the tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fixture of the retired Python compiler holds hand-written cases that reach every message of the policy format, and checks each message byte for byte (#722). Messages may change once the compiler is Rust-native, but what a refusal names and its error code may not, so add a test for each rule of the format, in policy/tests/grammar.rs, that checks them instead (#729): - the root is an object of allow and deny arrays, and rules are objects; - MXC and flat fields do not mix, fields of no form are refused, case included, and a field appears once in an object; - a destination needs one CIDR string, which is an IPv4 or IPv6 network, in every spelling that the format reads, whose host bits are cleared, and which names no IPv6 scope, with or without host bits (#462); exceptions lie within their network and family; - protocols are tcp, udp, icmp, or any, a null protocol included; ports need tcp, udp, or any, and a flat rule's port needs a protocol; ports are integers from 1 to 65535, which have at most 64 digits; endPort needs a port and does not precede it; - to and ports are nonempty arrays, and each action lowers to at most 256 native rules, which both families share; - files are at most 1 MiB of UTF-8 JSON, and deep nesting is refused without exhausting the stack. Each refusal must have the error code unsupported (#724), ErrorCode::Unsupported, name the field path, or the file or the repeated field where messages name those, and give a fragment of its reason that tells apart the rules that refusals at one path break. Without the fragment, a mutant that refuses a rule that mixes forms as one with unknown fields, at the same path and with the same code, would pass. Each test also checks what the rule accepts. Applied one at a time, mutants that accept IPv6 scopes, unknown or repeated fields, mixed forms, port 0, ports below their first port, IPv4 addresses with leading zeros, or a file of one byte over the limit, refuse hostmasks, or move the limits of integers and native rules by one each fail one of these tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage measurement of #726 selects the egress policy family of golden tests by name, so that each step of #722 can show what only the family's tests reach: the lines and branches that all tests reach and the tests outside the family don't. The family named three tests, but two more replay the fixture's policies: compiled_rules_read_back_as_native_rules_lower_to_themselves, with the cli feature that the measurement enables, and every_fixture_case_has_the_properties_that_replace_the_fixture, which runs the properties of #729 over its inputs. Outside the family, they would reach whatever only the fixture's inputs reach, so the measurement would understate what deleting the fixture loses. Name all five tests in the family, and in its table in doc/ci.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tests/fixtures/egress-policy.json recorded what the retired Python compiler did with 784 inputs, and its tests replayed them byte for byte (#722). The reference model, properties, and grammar tests of the previous change check the compiler from the format instead, and ran once over the fixture's inputs, so retire the fixture without changing the compiler (#729): - Delete the fixture, NVX_EGRESS_POLICY_FIXTURE, and the tests that read it: every_fixture_case_compiles_as_python_compiled_it, unreadable_files_fail_as_in_python, paths_print_as_pathlib_prints_them, the run of the properties over its inputs, and compiled_rules_read_back_as_native_rules_lower_to_themselves, whose property now reads back the rules of generated policies. - Map the fixture's other sections to tests: a grammar test checks that a missing file and a directory fail with the error code io and a message that names them, and the unit test of python::path checks the fixture's 30 paths that it lacked, with their posix and windows forms. The `\\?\` device paths among them reach a branch of the Windows parser, which PurePath shares, that only the fixture reached. - Drop the fixture's family from scripts/nvx_rs_coverage.py and doc/ci.md, and its mentions from the README, .gitattributes, and the documentation of python::json. The coverage measurement of the previous change (run 40018023086) shows what only the fixture's tests reached, on Linux and Windows alike: the branch of python/path.rs that the paths now reach, and the saturation of integers beyond an i128 in policy::port_value, which reading policies as strict JSON replaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Static review of the four scoped commits found no blocking defects in strict parsing, fixture retirement, or downstream diagnostic updates.
No open issues 🎉
What changed in this PR
Completes the egress-policy fixture retirement from #729, building on #824’s specification-based tests. The four scoped commits make NVX’s policy reader Rust-native while preserving the compiled rules of previously accepted policies.
Changes:
- Retires the recorded policy fixture and its replay tests, retaining grammar and property-based coverage.
- Adds strict JSON parsing with duplicate-field rejection and a 32-level nesting limit.
- Uses Rust-style quoting and paths as supplied, updating diagnostics, expectations, and documentation.
| File | Description |
|---|---|
scripts/nvx_rs_coverage.py |
Removes the retired egress fixture coverage family. |
nvx-rs/tests/fixtures/cli.json |
Updates recorded JSON, quoting, and path diagnostics. |
nvx-rs/src/host/sandbox/plan/tests.rs |
Updates the quoted CIDR error expectation. |
nvx-rs/src/host/python/path.rs |
Removes policy-only path emulation and preserves path cases in unit tests. |
nvx-rs/src/host/python/json.rs |
Removes the unused value adapter and updates commentary. |
nvx-rs/src/host/network/policy/tests/properties.rs |
Checks native-rule read-back using generated policies. |
nvx-rs/src/host/network/policy/tests/model.rs |
Aligns the reference reader with strict JSON. |
nvx-rs/src/host/network/policy/tests/grammar.rs |
Covers strict parsing, diagnostics, and unreadable files. |
nvx-rs/src/host/network/policy/tests/generator.rs |
Updates commentary to identify the retired fixture. |
nvx-rs/src/host/network/policy/tests.rs |
Removes fixture replay tests and updates error expectations. |
nvx-rs/src/host/network/policy/mod.rs |
Compiles JSON values directly and opens paths as supplied. |
nvx-rs/src/host/network/policy/json.rs |
Adds strict parsing with duplicate and depth checks. |
nvx-rs/src/host/network/mod.rs |
Removes Python text conversion and updates diagnostics tests. |
nvx-rs/src/host/network/lowering.rs |
Accepts string slices and quotes invalid CIDRs. |
nvx-rs/src/host/network/address.rs |
Replaces Python text parameters with string slices. |
nvx-rs/README.md |
Documents strict policies and specification-based testing. |
doc/run.md |
Documents policy parsing limits and rejection messages. |
doc/ci.md |
Removes the retired coverage family listing. |
aci_edge_sandboxes/src/openvmm/mod.rs |
Updates the quoted IPv6-scope error expectation. |
aci_edge_sandboxes/README.md |
Updates documented CIDR diagnostic wording. |
.gitattributes |
Removes the retired fixture’s generated-file designation. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/issue-729-retire-egress-fixture
branch
from
October 23, 2026 22:13
926ff50 to
bad2ef8
Compare
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/issue-729-retire-egress-fixture
branch
from
October 23, 2026 22:54
bad2ef8 to
07c85bb
Compare
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The security-sensitive JSON reader relies on serde_json’s private arbitrary-precision representation, warranting human review of feature-dependent parsing and depth enforcement.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
The egress policy compiler read files as Python's json.loads reads them, through the CPython emulation of nvx::host::python::json, so that it failed with Python's messages: NaN and Infinity were numbers, an integer of more than 64 digits failed as such, a repeated field failed without saying where, and every document that json.loads refused failed as `failed to read egress policy file: PATH`, without a reason. With the fixture retired, read policy files as RFC 8259 JSON instead, as - policy/json.rs reads a file with serde_json, which refuses NaN, Infinity, numbers beyond the range of a double, unpaired surrogates, a byte-order mark, and trailing text, through a seed that refuses an object that repeats a field, naming the object's path, and arrays and objects nested more than MAX_JSON_DEPTH, 32, deep, which a stack would never reach. A policy needs six. Another crate of a build can enable serde_json's arbitrary_precision feature, with which serde_json hands the seed each number that is not a 64-bit integer as a map of one field, and -0 as the integer 0; the seed then reads those numbers as serde_json reads them without the feature, so that what a file holds does not depend on the crates beside nvx. It tells such a map from an object of the document with that field, which serde_json's own Value takes for a number, by the field's value: serde_json hands the text of a number as an owned string, and never a string of a document so. - The compiler checks and lowers the serde_json::Value that it reads, which compile_policy already took, with the checks, order, and messages of before. It no longer parses a Python document or text, so lowering, address, and openvmm_arguments take &str instead of PyText, and python::json loses Document::from_value, which only the compiler used. - A file that is not UTF-8 or not JSON fails as `egress policy file PATH is not UTF-8: REASON` or `... is not valid JSON: REASON`, with serde_json's reason and position, one nested too deep as `egress policy file PATH nests arrays and objects more than 32 deep at line L column C`, and a repeated field as `OBJECT has duplicate field 'NAME'`, where OBJECT is the path of the object, or `egress policy` for the root. Missing and unreadable files and oversized ones fail as before. - serde_json reads -0, and integers beyond 64 bits, as doubles. A port is an integer written without a fraction or an exponent, so -0 fails as not an integer, while a double beyond 64 bits, which only an integer that large or an exponent writes, is out of range. Every policy that compiled compiles to the same rules: the properties and grammar tests, which the fixture's 784 inputs all satisfied, check the lowering. The grammar tests check the new refusals: repeated fields by their object's path, NaN, Infinity, numbers beyond a double, and unpaired surrogates as invalid JSON, -0 and integers beyond 64 bits as ports, and nesting up to and beyond the limit; the model reads files within the limit too. Unit tests of policy/json.rs check that it reads values as serde_json reads them, and numbers alike when serde_json hands them as with arbitrary_precision, but not an object with the field that holds them, refuses what RFC 8259 refuses, names the object of a repeated field, and limits nesting. Two ported tests now expect `duplicate field`, and an integer beyond a double as invalid JSON. The CLI golden scenarios of a policy file that is not JSON, two for each system, give the reason, as #722 lets a change edit them by hand. The README and doc/run.md describe the format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Messages of the egress policy compiler quoted values as Python's did: unknown fields in single quotes, as `egress policy has unknown field 'other'`, and CIDRs not at all, as in `allow[0].to[1].cidr is not a valid IPv4 or IPv6 CIDR: 10.0.0.256/8`, so that a value with a space, a quote, or nothing at all read ambiguously. #729 asks for Rust's quoting, so quote these values as `{:?}` does: - unknown fields, as in `egress policy has unknown field "other"`; - repeated fields, as in `allow[0].ports[0] has duplicate field "port"`; and - CIDRs that are invalid or name an IPv6 scope, as in `allow[0].to[1].cidr is not a valid IPv4 or IPv6 CIDR: "10.0.0.256/8"`. Field paths, networks that the compiler wrote, and numbers stay unquoted. openvmm_arguments shares the messages, so the rules of NetworkPolicy, and of aci_edge_sandboxes, quote these values too. The grammar tests check that each of these messages ends with the quoted value, and the tests that compare whole messages, of the compiler, of NetworkPolicy, of persisted sandbox plans, and of aci_edge_sandboxes, expect the quotes. The CLI golden scenarios of an unknown field, one for each system, quote it, as #722 lets a change edit them by hand. The nvx-rs README and the breaking changes of the aci_edge_sandboxes README quote the values of their examples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The compiler named and opened a policy file as Python's
str(pathlib.Path(path)) normalized it, through python::path's
python_path: it dropped empty and `.` components, wrote `\` for `/` on
Windows, and opened `policy.json` for `policy.json/`. With the
fixture's paths test retired, nothing asks for pathlib's spelling, and
the compiler was python_path's only user, so open the path as given and
name it as Path::display shows it: a policy file at `{root}/./x.json`
is now named so, instead of `{root}/x.json` (#729).
The compiler no longer uses the CPython emulation of nvx::host::python.
python_path and PythonPath go, and so do render_utf16 and
render_surrogateescape, which printed its paths, and their tests;
posix_str and windows_str stay as helpers of the tests of the parser
that PurePath shares with them.
The CLI golden scenarios of a missing policy file named with `./`, one
for each system, keep its `./`, as #722 lets a change edit them by
hand, and the README says that messages name files as they were given.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/issue-729-retire-egress-fixture
branch
from
October 23, 2026 23:17
07c85bb to
8d08b87
Compare
Pedro Henrique Penna (ppenna)
deleted the
agents/issue-729-retire-egress-fixture
branch
October 25, 2026 02:14
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
tests/fixtures/cli.json held the 315 scenarios that scripts recorded from the Python implementations of nvx.py sandbox and nvx.py run, and nvx_rs_reproduces_the_recorded_scenarios replayed them byte for byte once placeholders replaced the root and volatile values. The scenario tests of tests/cli/ check what callers rely on in their stead (#728): every scenario, and every regression that #722 traces, maps to them; on Linux and Windows, the replay reaches no line or branch of production code beyond the tests outside its family; and cargo-mutants finds no mutant of src/host/cli that the replay catches and the other tests of the CLI miss. The fixture goes, with the replay, its normalization and population helpers, and the mapping of tests/cli/scenarios.rs, whose tables of cases are private again, as is Root::empty(), which only Root::new() calls now. .gitattributes no longer marks the fixture as generated, and the test of the guest binaries' source identity names another file of tests/fixtures, root-tokens.json, which holds the cases of a root's boot tokens rather than behavior recorded from Python, so that no retirement of #722 removes it. The README describes the scenario tests in a section of their own, which replaces Golden scenarios and records how they replaced the transcripts, and its Python parity and Testing sections point to it. #828, which merged after #825 added the scenario tests, changed two messages that they check, and recorded the change in the transcripts alone: an unknown field of an egress policy is quoted as Rust quotes it, "other" where Python wrote 'other', and a policy file that cannot be read is named as it was given, ./ included, where Python normalized its path. The tests of egress compile and of the refusals of run expect these messages, which the transcripts no longer check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Build on the behavior-preserving specifications of #823 for the retirement half of #730. Remove the six Python-oracle tests of src/host/sandbox/golden.rs and their fixtures, tests/fixtures/sandbox.json and tests/fixtures/sandbox_options.json. Preserve all four accepted launch shapes, eight POSIX and Windows records with identical contracts, as explicit argument, token, and exact-budget regressions in the specification module rather than as fixtures. Remove the launch-profile family from the coverage inventory of scripts/nvx_rs_coverage.py, and update its inventory test to recognize the specification module. #828 and #830 retired the egress-policy and CLI-scenario families on dev, so the inventory and its table in doc/ci.md keep only the Python-semantics family, and doc/ci.md notes this retirement beside that of the CLI family. Update the coverage and mutation documentation, remove the temporary section mapping that #823 kept in the README, and drop the sandbox fixtures from the README's list of Python records. The native input and API changes follow in separate commits. Validation on Windows, rebased on dev: 701 library tests and 51 CLI tests pass with all features, and the coverage-script tests, Ruff, and the Ruff format check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Retire the Python-backed scalar compatibility layer of #730 in favor of native sandbox values while preserving the existing wire contracts. - Remove the public `Int` and `IntError` exports; use `Option<u64>` for the memory limit and `Option<u32>` for the process limit, including the bound that leaves room for the guest's init process. - Share ASCII-digit-guarded `FromStr` parsing for UID and GID, the CLI resource limits, and the limits that sandbox configurations persist, which they keep writing as JSON integers. - Use the canonical native `Uuid` parser while preserving tokens and all valid plan and configuration wire encodings. - Reject malformed limits at the CLI usage stage with clear native errors: `--memory-max` and `--pids-max` fail with `argument-invalid` and exit status 2, naming the accepted ASCII decimal range, instead of Python's `invalid int value` and the later `--pids-max must be positive` refusal with exit status 1. - Quote layer and persisted-plan values with Rust debug formatting. - Remove the now-unused Python integer arithmetic, keep only the test-only historical Python UUID oracle, and narrow the visibility of the remaining private integer helpers. - Update the scenario tests that replaced the CLI transcripts (#825, #830) to match: the usage table expects the double-quoted layer UUID and roles and the native `--memory-max` message, and the `--pids-max 0` case moves from the validation table, whose cases exit with status 1, to the usage table. - Document the input contract and the API change in the README, whose Python parity section no longer lists the sandbox, nor UUIDs among the values that it checks as Python does; #828 already removed the egress policy compiler from that section. Validation on Windows, rebased on dev: 702 library tests and 51 CLI tests pass with all features, and `cargo fmt --all --check` passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Complete the kernel-token part of #730 with the existing protocol::cmdline Linux lexer rather than Python's whitespace splitting. Refuse control characters, Unicode whitespace, quotes, and the bytes that Linux treats as whitespace, including the 0xa0 inside a UTF-8 sequence, in entrypoints, workload arguments, and guest targets, including the target of the single directory share of run. Check reserved names after Linux's quote splitting and its aliasing of hyphens and underscores, and also in the view of the guest's shell bootstrap, which splits the expanded command line on IFS whitespace without interpreting quotes, so that a quoted value such as extra="ignored nvx_memory_max=max ignored" cannot override a configured entrypoint, memory limit, process limit, or share; ordinary quoted caller values remain accepted. Trim the caller's command line once, before both reservation checks and rendering, so that the checks see exactly the text that is rendered and leading Unicode whitespace, such as U+2003 EM SPACE, cannot hide a reserved name. Confirm that Linux can still see every generated root parameter, so that -- and unmatched quotes cannot hide the appended boot settings. Keep the guest root parser and the exact 1024-byte budgets for both share-delivery modes and adapter plans. Broaden the properties to quoted and dashed reserved inputs, leading Unicode whitespace, and the known separator bytes, add explicit hidden-root and quoted-override regressions for both share-delivery modes, and make the host-path unit tests native-only. Update the documentation, and the validation table of the scenario tests that replaced the CLI transcripts (#825, #830): a sandbox argument with a space is now refused for its whitespace rather than its spaces. Build the Python compatibility code that only the CLI and the historical tests use for them alone, so that the default-feature library has no dead code, and add `cargo clippy --lib -- -D warnings` to check-nvx-rs so that it stays so. Gate the Python split and strip oracles to tests, and the integer, codec, and Unicode-digit helpers to the cli feature and tests. Since #828 moved the egress policy compiler to serde_json, and the previous commit the sandbox metadata readers, only the CLI's state files use the Python JSON reader and Python strings, so gate the json and text modules whole, as their dumps, shell, and value siblings are. The README's Python parity section now credits the nvx-rs CLI, their only remaining user, with the whitespace, int, and json.loads semantics, and the module documentation of the JSON reader no longer cites the retired egress and distro-manifest readers. Validation on Windows, rebased on dev: 706 library tests and 51 CLI tests pass with all features, and 630 library tests with default features; `cargo fmt --all --check`, Clippy with warnings denied for all targets with all features, for the default-feature library, and for the library without default features, rustdoc with warnings denied, and the Markdown link check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Build on the behavior-preserving specifications of #823 for the retirement half of #730. Remove the six Python-oracle tests of src/host/sandbox/golden.rs and their fixtures, tests/fixtures/sandbox.json and tests/fixtures/sandbox_options.json. Preserve all four accepted launch shapes, eight POSIX and Windows records with identical contracts, as explicit argument, token, and exact-budget regressions in the specification module rather than as fixtures. Remove the launch-profile family from the coverage inventory of scripts/nvx_rs_coverage.py, and update its inventory test to recognize the specification module. #828 and #830 retired the egress-policy and CLI-scenario families on dev, so the inventory and its table in doc/ci.md keep only the Python-semantics family, and doc/ci.md notes this retirement beside that of the CLI family. Update the coverage and mutation documentation, remove the temporary section mapping that #823 kept in the README, and drop the sandbox fixtures from the README's list of Python records. The native input and API changes follow in separate commits. Validation on Windows, rebased on dev: 701 library tests and 51 CLI tests pass with all features, and the coverage-script tests, Ruff, and the Ruff format check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Retire the Python-backed scalar compatibility layer of #730 in favor of native sandbox values while preserving the existing wire contracts. - Remove the public `Int` and `IntError` exports; use `Option<u64>` for the memory limit and `Option<u32>` for the process limit, including the bound that leaves room for the guest's init process. - Share ASCII-digit-guarded `FromStr` parsing for UID and GID, the CLI resource limits, and the limits that sandbox configurations persist, which they keep writing as JSON integers. - Use the canonical native `Uuid` parser while preserving tokens and all valid plan and configuration wire encodings. - Reject malformed limits at the CLI usage stage with clear native errors: `--memory-max` and `--pids-max` fail with `argument-invalid` and exit status 2, naming the accepted ASCII decimal range, instead of Python's `invalid int value` and the later `--pids-max must be positive` refusal with exit status 1. - Quote layer and persisted-plan values with Rust debug formatting. - Remove the now-unused Python integer arithmetic, keep only the test-only historical Python UUID oracle, and narrow the visibility of the remaining private integer helpers. - Update the scenario tests that replaced the CLI transcripts (#825, #830) to match: the usage table expects the double-quoted layer UUID and roles and the native `--memory-max` message, and the `--pids-max 0` case moves from the validation table, whose cases exit with status 1, to the usage table. - Document the input contract and the API change in the README, whose Python parity section no longer lists the sandbox, nor UUIDs among the values that it checks as Python does; #828 already removed the egress policy compiler from that section. Validation on Windows, rebased on dev: 702 library tests and 51 CLI tests pass with all features, and `cargo fmt --all --check` passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Retire the Python JSON and surrogate behavior from the sandbox metadata checks for #730. Parse bounded environment files directly as Vec<String> with serde_json and validate ordinary UTF-8 KEY=VALUE strings. Read distro manifests as strict JSON, require the integer format 1, and refuse non-finite numbers and unpaired surrogate escapes, even in unused fields. Preserve the file, entry, and byte limits, the binding of the artifact name and digest, the file seals, and the independent refusal of systemd. Read distro manifests with the strict JSON reader of egress policy files that #828 added, which host::network::policy::json now shares as read_document, with the name that its errors give the root of the document. serde_json's own Value depends on the features of serde_json, which another crate of a build may enable: with arbitrary_precision, it reads an object that holds a number as that number, so that a format of {"$serde_json::private::Number": "1"} would pass the integer check, and it accepts numbers beyond the range of a double in unused fields. The shared reader reads both as it does without the feature. It also refuses an object that repeats a field, as `manifest has duplicate field "format"` or `packages[0] has duplicate field "name"`, and arrays and objects nested more than MAX_JSON_DEPTH, 32, deep, and a manifest that is not UTF-8 now fails as such. Quote sandbox values and paths with Rust debug formatting, and keep the native JSON and I/O causes in diagnostics. Move the now-unused Python JSON lookup and UTF-8 encode-oracle helpers out of production code; the unrelated historical Python family keeps its test-only oracles. Update the focused checks of tests/cli.rs that name these values and paths; the transcripts of tests/fixtures/cli.json, which #830 retired, no longer record them. Document the accepted-input and diagnostic changes. Update the scenario tests that replaced the transcripts (#825, #830) to match. The validation table expects the double-quoted --mount modes and targets, and the serde_json causes of environment files that hold an object or a non-string entry. Its environment-file cases name the file as nvx-rs now quotes it: Root::expand, in the shared harness, gains the {root:?} and {root/RELATIVE:?} placeholders, which quote a path as Rust's Debug quotes it, so that the fragments match the escaped backslashes of Windows paths as well as POSIX paths. The manifest tests refuse that object-valued format as a mismatch, and refuse numbers beyond a double, repeated fields at the root and in a package, nesting beyond the limit, and a manifest that is not UTF-8. A test of the shared reader simulates arbitrary_precision, as its other tests do, and checks that the object stays an object there, and another that read_document names the root of a document in its errors. Validation on Windows, rebased on dev: 705 library tests and 51 CLI tests pass with all features, `cargo fmt --all --check` passes, Clippy passes with warnings denied for all targets with all features, and the Markdown link check passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 25, 2026
Complete the kernel-token part of #730 with the existing protocol::cmdline Linux lexer rather than Python's whitespace splitting. Refuse control characters, Unicode whitespace, quotes, and the bytes that Linux treats as whitespace, including the 0xa0 inside a UTF-8 sequence, in entrypoints, workload arguments, and guest targets, including the target of the single directory share of run. Check reserved names after Linux's quote splitting and its aliasing of hyphens and underscores, and also in the view of the guest's shell bootstrap, which splits the expanded command line on IFS whitespace without interpreting quotes, so that a quoted value such as extra="ignored nvx_memory_max=max ignored" cannot override a configured entrypoint, memory limit, process limit, or share; ordinary quoted caller values remain accepted. Trim the caller's command line once, before both reservation checks and rendering, so that the checks see exactly the text that is rendered and leading Unicode whitespace, such as U+2003 EM SPACE, cannot hide a reserved name. Confirm that Linux can still see every generated root parameter, so that -- and unmatched quotes cannot hide the appended boot settings. Keep the guest root parser and the exact 1024-byte budgets for both share-delivery modes and adapter plans. Broaden the properties to quoted and dashed reserved inputs, leading Unicode whitespace, and the known separator bytes, add explicit hidden-root and quoted-override regressions for both share-delivery modes, and make the host-path unit tests native-only. Update the documentation, and the validation table of the scenario tests that replaced the CLI transcripts (#825, #830): a sandbox argument with a space is now refused for its whitespace rather than its spaces. Build the Python compatibility code that only the CLI and the historical tests use for them alone, so that the default-feature library has no dead code, and add `cargo clippy --lib -- -D warnings` to check-nvx-rs so that it stays so. Gate the Python split and strip oracles to tests, and the integer, codec, and Unicode-digit helpers to the cli feature and tests. Since #828 moved the egress policy compiler to serde_json, and the previous commit the sandbox metadata readers, only the CLI's state files use the Python JSON reader and Python strings, so gate the json and text modules whole, as their dumps, shell, and value siblings are. The README's Python parity section now credits the nvx-rs CLI, their only remaining user, with the whitespace, int, and json.loads semantics, and the module documentation of the JSON reader no longer cites the retired egress and distro-manifest readers. Validation on Windows, rebased on dev: 708 library tests and 51 CLI tests pass with all features, and 632 library tests with default features; `cargo fmt --all --check`, Clippy with warnings denied for all targets with all features, for the default-feature library, and for the library without default features, rustdoc with warnings denied, and the Markdown link check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Oct 25, 2026
Pedro Henrique Penna (ppenna)
added a commit
that referenced
this pull request
Oct 30, 2026
dev requires Required status check, but neither a merge queue nor branches that are up to date with it, so each pull request is tested on dev as dev was when its run started, and two pull requests that pass on their own can break dev together, as #825 and #828 did on October 25 (#895). A merge queue tests each queued pull request on the latest dev and the pull requests ahead of it, in a merge group whose run reports the required check on the commit that the queue then merges. The two commands through which CI selects and checks its jobs reject the merge_group event, though, so a merge group's run could neither classify its change nor pass its required check. ci.py now names the events that CI distinguishes: - CI_PRE_MERGE_EVENTS, pull_request and merge_group, whose runs check a change before it merges; - CI_CLASSIFIED_EVENTS, those and push, whose changes the detector classifies; and - CI_EVENTS, those and the full runs of schedule and workflow_dispatch. classify_ci_changes accepts a merge group and classifies its change as a pull request's: each suite runs only when one of its own inputs changed, the platform jobs included, since a merge group publishes no development release and records no performance history. Only a push runs the platform jobs for every change but documentation. required_ci_expected_results expects of a merge group the jobs of a same-repository pull request, with the regression gate after the platform jobs and the debug kernel on KVM alone, but not the rehearsal of the guest artifact reuse, which the pull request's own run made. A merge group runs in the repository whatever repository its pull requests come from, so the policy expects the repository's jobs whatever --same-repository says, as it does for a push. The --event-name options of classify-ci-changes and check-required-ci take their choices from these tuples. The CLI tests parse every event and reject the others, check that a merge group classifies every case of the classification tests as a pull request does and explains it the same way, run the detector's script and the artifact invariants for all three classified events, and extend the required-result policy cases with merge groups. doc/usage.md documents the new choices. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #824. This branch is #824's head (
ed3d733e9) plus four commits. Until #824 merges, the diff here includes its four commits, so review only the last four:306a863c2,d79dff947,df0c7da14, and8d08b871f.Fixes #729, as the second of its two pull requests. #824 added tests whose expectations come from the policy format instead of from
tests/fixtures/egress-policy.json, the recording of what the retired Python compiler did. This retires the fixture and makes the compiler Rust-native: it reads policy files as strict RFC 8259 JSON, quotes values in its messages as Rust does, and names files as they are given. Every policy that compiled compiles to the same rules.What changed
Retire the fixture (
306a863c2). Deletesegress-policy.json,NVX_EGRESS_POLICY_FIXTURE, and the tests that read it:every_fixture_case_compiles_as_python_compiled_it,unreadable_files_fail_as_in_python,paths_print_as_pathlib_prints_them, the run of the properties over its inputs, andcompiled_rules_read_back_as_native_rules_lower_to_themselves, whose property now reads back the rules of generated policies. The fixture's other sections become tests: a grammar test checks that a missing file and a directory fail with the error codeioand name the path, and the unit test ofpython::pathgains the fixture's 30 paths that it lacked. The "Egress policies" coverage family leavesscripts/nvx_rs_coverage.pyanddoc/ci.md.Strict JSON (
d79dff947). The newpolicy/json.rsreads a file withserde_json, through a seed that refuses a repeated field, naming its object's path, and arrays and objects nested more thanMAX_JSON_DEPTH, 32, deep (a policy needs six). The compiler checks theserde_json::Valuewith the checks, order, and messages of before, and no longer goes through the CPython emulation ofpython::json, solowering,address, andopenvmm_argumentstake&strinstead ofPyText, andDocument::from_valuegoes. Another crate of a build can enableserde_json'sarbitrary_precisionfeature for every crate that uses it, with whichserde_jsonhands a visitor each number that isn't a 64-bit integer as a map of one field, and-0as the integer 0; the seed then reads those numbers asserde_jsonreads them without the feature, so what a file holds doesn't depend on the crates besidenvx. It tells such a map from an object of the document with that field, whichserde_json's ownValuetakes for a number, by the field's value:serde_jsonhands the text of a number as an owned string, and never a string of a document so.NaN,Infinity, numbers beyond a double, unpaired surrogates, a byte-order mark, and trailing text. No policy that compiled used them.egress policy file PATH is not UTF-8: REASONand… is not valid JSON: REASON, withserde_json's reason and position, where every refused document wasfailed to read egress policy file: PATH;… nests arrays and objects more than 32 deep at line L column C; andOBJECT has duplicate field …, whereOBJECTis the object's path, oregress policyfor the root.serde_jsonreads-0, and integers beyond 64 bits, as doubles:-0is not an integer, and an integer beyond 64 bits is out of range, where it was "more than 64 digits".Rust's quoting (
df0c7da14). Unknown and repeated fields, and CIDRs that are invalid or name an IPv6 scope, are quoted as{:?}quotes them:egress policy has unknown field "other"andallow[0].to[1].cidr is not a valid IPv4 or IPv6 CIDR: "10.0.0.256/8".openvmm_argumentsshares these messages, so those ofNetworkPolicyandaci_edge_sandboxeschange too, and the breaking changes of theaci_edge_sandboxesREADME quote them.Paths as given (
8d08b871f). The compiler opens the path that it's given and names it asPath::displayshows it, instead of aspathlibnormalized it:{root}/./missing.jsonstays so.python_path,PythonPath,render_utf16, andrender_surrogateescapego, since the compiler was their only user.The golden scenarios of
tests/fixtures/cli.jsonchange by hand, as #722 allows, one or two records of each system for each change: a policy file that isn't JSON, an unknown field, and a missing file named with./. The README's "Egress policy files" anddoc/run.mddescribe the strict format.The gate of #729
python/path.rs(\\?\device paths) and the saturation of integers beyond ani128in the oldpolicy::port_value. On this branch (run), the ported paths take that branch on both systems, and strict JSON replaced the saturation:policy/mod.rshas every line and branch covered, andpolicy/json.rsevery one butVisitor::expecting, whichserde_jsoncalls only for a type that the seed doesn't take, and it takes every type.lowering.rs,policy/mod.rs, andaddress.rsthat the fixture's tests catch (see nvx-rs: check egress policies against a reference model and property tests #824). On this branch, thehost::networktests catch 73 of the 80 viable mutants ofpolicy/json.rsandpolicy/mod.rs(run). Three are equivalent: the twoexpectingmethods are never called, andvisit_unitalready returnsValue::Null. The other four drop the sign of-0, or misplace the value of a$serde_json::private::Numberfield witharbitrary_precision; tests ofpolicy/json.rsadded since fail with each of them, applied by hand.nvx-rs egress compileof this branch: the 272 that compiled compile to the same rules, and the 512 that were refused are refused.Validation
nvx-rs:cargo fmt --check;cargo clippy -D warningswith--all-targets --all-featuresat each commit,--lib --no-default-features, and forx86_64-unknown-linux-gnu;cargo testwith all, default, and no features, the CLI's golden scenarios included;cargo docwith-D warnings;cargo +1.89 check --all-targets --all-features; andcargo check --target aarch64-apple-darwin.aci_edge_sandboxes:cargo fmt --check,cargo clippy, andcargo testwith all features.--features serde_json/arbitrary_precision, the tests ofpolicy/json.rsand the grammar tests pass, and{"port": {"$serde_json::private::Number": "1e400"}}reads as an object, as without it. The reference model of the properties reads JSON with a visitor of its own, which assumesserde_json's default features, as the tests do.scripts/test_nvx_tools.pyandscripts/test_microvm_tests.py, withNVX_RSset to this build;scripts/test_nvx_rs_coverage.py,scripts/test_markdown_links.py,scripts/check_markdown_links.py, andruff.endPortmatches their error code and field path, which don't change, soscripts/nvx_tools/microvm_tests.pyneeds no change. The hypervisor scenarios are left to CI.Interactions
#825, the first pull request of #728, adds CLI scenario tests, two of which expect forms that this changes: the "unknown field" case of
tests/cli/egress.rsexpects'other', andtests/cli/validation.rsexpects{root/missing.json}for a file given as{root/./missing.json}. Whichever of the two lands second updates them.