Repository navigation
nvx-rs: parse legacy egress rules with the MXC form's helpers - #682
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoOct 21, 2026
Merged
Conversation
parse_legacy_rule, which parses the legacy flat rules of an egress policy file, repeated two steps that the helpers of the MXC form already take. Like parse_peer, it required the cidr field with a copy of the "cidr is required" check before it passed cidr and except to parse_peer_fields. And it wrote out the selector of a rule without a port, which lowering::port_selectors builds for an MXC port selector, calling port_protocol_selectors, the rest of port_selectors, only for a rule with a port. parse_peer_fields now reads the required cidr and the optional except fields itself, so parse_peer and parse_legacy_rule each call it once. parse_legacy_rule passes its port fields to port_selectors, as parse_mxc_port and the lowering of openvmm_arguments do, and port_protocol_selectors, which had no other caller, folds into port_selectors. Each rule form now has one path from its fields to its destinations and selectors, and the lowering module has one function fewer. Behavior is unchanged: the checks run in the same order, with the same messages. A legacy rule is still checked for unknown fields, then for cidr, its network and exceptions, endPort without port, port without protocol, and its protocol. port_selectors refuses endPort without port too, but parse_legacy_rule refuses it before port_selectors can see it, so a legacy rule without a port still gets the one selector of its protocol, and a rule with a port the ICMP check and port range of port_protocol_selectors. Reading the port fields before the call only converts their nodes, which cannot fail. The policy tests cover both forms, among them the 784 cases of tests/fixtures/egress-policy.json, the Python compiler's recorded outcomes, whose legacy edge cases include endPort without port, port without protocol, and ICMP with a port, an endPort, or neither. The network tests cover openvmm_arguments, and the egress scenarios of test-microvm compile legacy and MXC rules with nvx-rs egress compile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow refactor preserves behavior, has existing regression coverage, and has no unresolved findings.
No open issues 🎉
What changed in this PR
Consolidates legacy and MXC egress-policy parsing around shared helpers while preserving validation order, error messages, and native rule output.
Changes:
- Centralizes required
cidrlookup and exception parsing. - Routes legacy port fields through
port_selectors, removing the redundant helper.
| File | Description |
|---|---|
| nvx-rs/src/host/network/policy/mod.rs | Reuses shared destination and port-selector parsing. |
| nvx-rs/src/host/network/lowering.rs | Folds port handling into port_selectors and updates its documentation. |
Review effort: Balanced
This was referenced Oct 21, 2026
Pedro Henrique Penna (ppenna)
deleted the
nvx-rs-quality/legacy-egress-rules
branch
October 21, 2026 19:43
This was referenced Oct 21, 2026
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.
Summary
parse_legacy_ruleinnvx-rs/src/host/network/policy/mod.rsparses the legacy flat rules of an egress policy file,{"cidr": CIDR, "except": [...], "protocol": PROTOCOL, "port": FIRST, "endPort": LAST}. It repeated two steps that the helpers of the MXC form already take:parse_peer, it looked up the requiredcidrfield with its own copy of the{description}.cidr is requiredcheck, and then passedcidrandexcepttoparse_peer_fields.vec![(protocol.native(), None)], whichlowering::port_selectorsbuilds for an MXC port selector, and calledport_protocol_selectors, the rest ofport_selectors, only for a rule with a port.Both rule forms now go through the same helpers, so each has one path from its fields to its destinations and selectors, and the lowering module has one function fewer. Earlier runs listed this as a follow-up in #635, #636, #637, and #678.
Changes
nvx-rs/src/host/network/policy/mod.rs:parse_peer_fields(object, description)reads the requiredcidrand the optionalexceptfields itself, failing with the same{description}.cidr is required; it took them as arguments. The two sites that now share that lookup areparse_peer, for each destination of an MXC rule, andparse_legacy_rule, for a legacy rule.parse_legacy_rulepasses itsportandendPortfields tolowering::port_selectors.nvx-rs/src/host/network/lowering.rs:port_protocol_selectors, whose only other caller wasport_selectors, folds into it. Its doc comment now names both rule forms and keeps the old function's note that ICMP takes no port.port_selectors:parse_mxc_port(an MXC rule's port selectors),parse_legacy_rule(a legacy rule's port fields), andparse_ruleinhost/network/mod.rs(theEgressRules thatopenvmm_argumentslowers).2 files, 17 insertions and 35 deletions.
Behavior
Unchanged: every policy compiles to the same native rules, or fails at the same check with the same message and
ErrorKind::Unsupported.parse_peerstill checks that a destination is an object with known fields before it requirescidr.parse_legacy_rulestill checks its fields, thencidr, the network and its exceptions,endPortwithoutport,portwithoutprotocol, and the protocol, in that order.port_selectorsrefusesendPortwithoutporttoo, butparse_legacy_rulerefuses that combination first, before it reads the protocol, so a legacy rule never reachesport_selectorswith it. A legacy rule withoutportstill gets the one selector of its protocol, and a rule withportthe ICMP check and port range thatport_protocol_selectorsapplied.matcharm, butport_valueonly converts a JSON node and cannot fail, and withoutportthere is noendPortto convert.Tests that pin this:
host::network::policy::tests, among themevery_fixture_case_compiles_as_python_compiled_it, which replays the 784 outcomes oftests/fixtures/egress-policy.jsonthat the Python compiler recorded: its test vectors, a seeded random corpus of 200 policies, and edge cases such aslegacy endPort without port,legacy port without protocol, andlegacy icmpwith a port, anendPort, or neither.openvmm_arguments, inhost::network::tests, and the goldenegress compilescenarios oftests/cli.rs.test-microvm, which compile legacy and MXC rules with annvx-rs egress compilebuilt from this tree and run guests under them.Follow-ups
Not in this pull request, to keep it to one change. I found no suspected bug.
host/sandbox/mount.rs:MountAccess,MountOwner, andMountKindrepeatas_str,from_namewith one message shape,unsupported sandbox mount NOUN ...; choose A or B, andDisplay, andparse_mount_valuerepeatsMountAccess's match with a message of its own.host/python/golden.rsandhost/sandbox/golden.rsopen and close 19 sections withMismatches::new(SECTION),cases(&fixture, SECTION), andfinish(); one helper that runs a section would name each section once and makefinishimpossible to forget.guest/nvxs.rs:GuestConnection::attach,acknowledge, andcreditrepeat the same check, clear, encode, and write around one session call.protocol/nvxs/guest.rsandhost.rs:receiverepeats the same nine lines after its state check in both roles.host/python/fs.rs's Windowsstatandlstatbuild the sameStat, andhost/python/dumps.rs'swrite_valuerepeats its bracket, comma, and indentation loop for lists and dicts.write_privateinhost/instance.rsand inhost/store/files.rsshare the temporary file's name, write, sync, rename, and cleanup, and differ in how they create the file; nvx-rs: add the VMM supervisor with a spawn allowlist, stop, and reattach #656 and nvx-rs: refuse read-only paths inside read-write directories on every host #672 changeinstance.rs.Validation
BASE:
916a61f0297900db13d7d9d04d170f01ef9ed71f,origin/devwhen the run started and still when this pull request was opened. No open pull request todevchanges the files of this one.TREE:
2f7b0791b5763dabf3db541939d349d8fcc19ff2, the tree of this commit. Every host applied the patch to BASE and checked thatgit write-treeequals TREE.Release on every host:
v0.1.0-dev.dad3e41d7f05, the nearest first-parent ancestor of BASE that had packages, two commits earlier. Its OpenVMM,d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision, as each host checked inbuild/openvmm.provenance.json.Each host replaced the release's
nvx-rswith one built from TREE by the Cargo command thatnvx.pyuses, recorded its provenance withrecord-nvx-rs-provenance, and checked that the record names TREE's sources (source_sha2567a8a4abf7d77cb206a00a390261b27dff8df3315e116e04fdab790c06e2d494d) and the executable that it built. Each host built themanaged_lifecycleandopenvmm_e2etest binaries before its hypervisor tests, whose logs show no compilation.Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts (they are denied on prometheus32):
build-guest-binaries --output, from guest sources with digestb2e9b0bb21ce7435221e02af32eed4ba188e633bb1e6a7db15bf7f6808d34495;test-guest-binaries --binaries, where 34 tests passed;build-initramfswithNVX_GUEST_BINARIES.initramfs.cpio.gz(SHA-2562d12a797f23e00d7363ba9290f7efdb86b0b401227a380f6bc13c8e58f690f4b), its packages manifest, and its provenance went through this machine to the other two hosts, where the SHA-256 digests matched.Local checks with Rust 1.95.0 on Windows: rustfmt; Clippy with
-D warnings, with all targets and features and on the library without default features; and the tests with all features, the 45 policy tests among them, and with the default ones. This machine also ran the steps that no host could run, because the hosts lack the toolchain or target: the Windows MSRV (1.89) checks ofnvx,aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations, on Windows; and, on WSL Ubuntu, the macOS (aarch64-apple-darwin) checks ofnvxandaci_edge_sandboxesand the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with itsrustup target add, which gave the local 1.89 the musl target. I removed that target again afterwards.The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.
A harness note: the first run of prometheus32's gates was started in the background with
&from a non-interactive shell, which left SIGINT, SIGQUIT, and SIGHUP ignored, so the five Ctrl-C tests oftests/cli.rsfailed: thenvx-rsthat they interrupt inherited the ignored SIGINT. I reran that job, and prometheus30's guest build, undersetsid -fwith the default signal dispositions. Every step passed, the five Ctrl-C tests among them, and the rebuilt initramfs was byte-identical. The table reports the reruns.test-aci-edge-sandboxestest-microvm, default suitenvxtests with all features (the 45 policy tests and the 13 oftests/cli.rsamong them) and 362 with the default ones, artifact bundling (15),check-guest-binaries(397 tests), the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests), 158 and 85aci_edge_sandboxestests over the two feature sets, and agent bundling (4). The macOS checks and the guest-only MSRV check ran locallynvxtests with all features (the 45 policy tests among them) and 358 with the default ones, artifact bundling (15), the runtime adapter's protocol-only and host-only configurations (13 and 14 tests), 158 and 85aci_edge_sandboxestests over the two feature sets, and agent bundling (4). The MSRV checks ran locallyLeft to CI: the
test-microvmscenarios that run only when named (smp-lapic,managed-exec-config, andsandbox-lifecycle), which need artifacts that packages omit; the Ubuntu and Azure Linux guests;test-openvmm-unitandtest-openvmm, since OpenVMM is unchanged; the Docker guest build andverify-guest-determinism; the benchmarks and performance gates; and check-quality and validate-nvx, which no Python, shell, or Markdown change touches.