Skip to content

Specify sandbox launch and option contracts before fixture retirement - #823

Merged
Pedro Henrique Penna (ppenna) merged 2 commits into
devfrom
agents/issue-730-fix-logical-commits
Oct 24, 2026
Merged

Pedro Henrique Penna (ppenna) merged 2 commits into
devfrom
agents/issue-730-fix-logical-commits

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Part 1 of #730 adds independently derived table/property specifications without runtime behavior changes. Both fixtures and all six golden tests remain unchanged.

The specifications summarize UID:GID, layers, mounts, and native paths; SHA-256 restore child names; OpenVMM arguments and independently reconstructed 1024-byte kernel budgets with exact boundary tests; sandbox/run policy ordering; exec environment and file limits; distro bindings and unconditional systemd refusal; and the temporary 12-section mapping table.

The two logical commits are:

  • 7e2ca440e — nvx-rs: specify sandbox launch and restore contracts
  • 96a7c0d0f — nvx-rs: specify sandbox option attribution and metadata

Windows verification and the all-feature/default nvx tests passed (768/646 passing). No-default builds/tests, fmt, Clippy, rustdoc, MSRV1.89, runtime adapters, dependent workspace tests/docs/MSRV, and agent bundling passed; Linux/hypervisor coverage is via CI.

Fresh Windows coverage comparison found zero golden-only sandbox branches and one Python JSON-hook line reserved for deletion in the follow-up.

Related #730; intentionally not closing it until retirement PR.

Copilot AI balanced review requested due to automatic review settings October 23, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The native-path property can generate reserved Windows device names, causing intermittent failures before the intended behavior is tested.

1 open issue
What changed in this PR

Adds Rust specification tests for sandbox launch and option contracts as the first step toward fixture retirement in #730. Runtime behavior, both fixtures, and all six golden tests remain unchanged.

Changes:

  • Adds table-driven and property tests for launch arguments, kernel limits, restore names, and native paths.
  • Specifies policy ordering, environment limits, distro bindings, and systemd refusal.
  • Documents the transition with a fixture-to-specification mapping.
File Description
nvx-rs/​src/​host/​sandbox/​specification.rs Adds launch, restore, grammar, and path specifications.
nvx-rs/​src/​host/​sandbox/​options/​tests.rs Adds policy, environment, and distro contract tests.
nvx-rs/​src/​host/​sandbox/​mod.rs Registers the sandbox specification module.
nvx-rs/​src/​host/​cli/​sandbox/​tests.rs Tests preceding-mount attribution of policy options.
nvx-rs/​src/​host/​cli/​run/​specification.rs Tests order-independent policy forwarding and absolute-path requirements.
nvx-rs/​src/​host/​cli/​run.rs Registers the run specification module.
nvx-rs/​README.md Documents specifications and maps fixture sections to tests.

Review effort: Balanced

Comment thread nvx-rs/src/host/sandbox/specification.rs Outdated
#730 stage one specifies the canonical UID:GID, layer, and mount grammars; independent SHA-256 restore child names, including the /workspace known answer; native filesystem paths plus Windows drive-relative and UNC paths; exact OpenVMM block and share arguments; and guest resource tokens.

The specifications independently reconstruct every generated kernel token and exercise both delivery modes' 1024-byte budgets, including VMM fragments and the terminating NUL, with exact-fit and one-byte-over boundaries. Golden tests and runtime behavior are retained for comparison.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add specification coverage for the options half of #730 without changing runtime behavior or removing any of the six golden tests.

Pin the 256-entry and 4096-UTF-8-byte exec environment limits, exact 1 MiB JSON file boundary, artifact filename and digest binding, package metadata, and the unconditional refusal of systemd entrypoints and images. Exercise sandbox preceding-mount parsing and policy attribution with generated option sequences. Check that run forwards absolute policy paths independently of option order for OpenVMM to attribute by containment. Map all 12 semantic fixture sections to replacement tests in a temporary README table.

Windows validation passes for all-feature/default nvx tests, Clippy, rustdoc, Rust 1.89, the protocol/host runtime adapters, and aci_edge_sandboxes. Fresh branch coverage loses no golden-only sandbox branch; its sole uncovered line is a Python JSON-hook error path that the retirement PR removes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 19:49
@ppenna
Pedro Henrique Penna (ppenna) force-pushed the agents/issue-730-fix-logical-commits branch from 96a7c0d to 878ac04 Compare October 23, 2026 19:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The additive tests and documentation preserve runtime behavior and existing golden coverage, with no identified blocking issues.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Pedro Henrique Penna (ppenna) added a commit that referenced this pull request Oct 23, 2026
Build on the behavior-preserving specifications in #823 for the retirement half of #730. Remove the six Python-oracle tests, sandbox.json, and sandbox_options.json. Preserve all four accepted launch shapes (eight POSIX/Windows records with identical contracts) as explicit argument, token, and exact-budget regressions rather than fixtures.

Remove only the launch-profile family from the coverage inventory and update its inventory test to recognize the specification module. Keep the remaining Python semantics, network policy, and CLI golden families. Update coverage/mutation documentation and remove the temporary section mapping preserved in #823. Native input/API changes follow in separate commits.

Validation: 125 sandbox tests and the coverage-script tests pass on Windows; remaining formatting and documentation checks pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ppenna
Pedro Henrique Penna (ppenna) merged commit f4ef375 into dev Oct 24, 2026
42 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/issue-730-fix-logical-commits branch October 24, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants