Skip to content

host: delete the sandbox accessors that only tests use (SL6.6c) - #1538

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
feature/nvx-refactorfrom
agents/slim-sl6-6c-sandbox-test-only
Nov 12, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
feature/nvx-refactorfrom
agents/slim-sl6-6c-sandbox-test-only

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Leaving code only. This PR changes only code that this effort removes from NVX: twelve items of host::sandbox that no NVX program calls, MountAccess::from_name, MountOwner::from_name and MountKind::from_name with their helper setting_from_name, SandboxMount::{guest_target, access, policy, owner, kind, absolute, absolute_policy_paths}, and SandboxOptions::new, with the edits that remove references to them. Nothing that stays changes, and no later PR uses what it changes, so the size limits for kept code don't apply.

Part of #1122 (SL6.6c, "the sandbox accessors that only tests use are gone", recorded on #1122). Parent #1087, umbrella #1076.

It needs SL6.4 (#1533), which has merged; it starts from 55f44aa4607, the tip after SL4.4d (#1545). SL6.7b (#1124) stacks on this one and makes host::sandbox and host::network::policy private to the CLI, which would leave these twelve items dead.

What and why

SL6.7b makes host::sandbox crate-private. Twelve of its items have no caller in any NVX program, only in the public API and in the module's own tests, so the warning-denied builds of every feature set would find them dead once private, on Windows, Linux and the macOS check. As SL6.6b did for SL6.4, this PR deletes them first, so that SL6.7b stays visibility-only. One commit, host: delete the sandbox accessors that only tests use (eed421f9):

  • MountAccess::from_name, MountOwner::from_name, MountKind::from_name and setting_from_name, which read a share's settings by name, "as a recorded share gives it". The CLI maps the choices of --mount-owner itself, and records a sandbox's export plan, not its shares' settings. The imports that only they used, super::refused and crate::error::Result in mount/mod.rs, go with them.
  • SandboxMount::{guest_target, access, policy, owner, kind}, which nothing outside the tests read: plan_mounts, validate_mounts and the checks of host paths read the fields within the mount module, and host_path, which options::sandbox_mounts calls, stays. SandboxMount::{absolute, absolute_policy_paths} called the crate-private absolute_with and absolute_policy_paths_with with paths::current_dir; plan_mounts and the checks of host paths call those variants, whose docs now say what the wrappers' said.
  • SandboxOptions::new, the options of an operation with the defaults of nvx-rs sandbox. The CLI builds its SandboxOptions field by field from what it parsed.
  • plan_mounts' docs no longer link to the deleted SandboxMount::absolute, and say instead how making a host path absolute fails, Io for an unreadable current directory or a drive-relative path that Windows cannot qualify.

The public API snapshots lose 21 lines on each platform: the eleven public methods at both of their paths, nvx::host::sandbox:: and nvx::host::sandbox::mount::, and nvx::host::sandbox::options::SandboxOptions::new. Nothing is added. README.md, doc/ and the case matrix name none of the items.

Size

Measured against 55f44aa4607, the tip after SL4.4d (#1545), by #1076's rules: leaving code only, no limit. The measuring script counts 0 added or changed code lines: production code only loses the items and their two imports (105 non-blank lines). Counted files: 3, src/host/sandbox/mount/{mod, request}.rs and src/host/sandbox/options.rs; src/host/sandbox/mount/aggregate.rs changes only its docs. 10 files change: +168 and −230 lines, tests and snapshots included.

Old tests of the area

  • Retired, with what they checked: the three assertions of from_name's refusals, "unsupported sandbox mount owner \"root\"; choose vmm or caller" and "unsupported sandbox mount mode \"rx\"; choose ro or rw" in host::sandbox::mount::tests::mount_owner_defaults_to_vmm_and_forwards_caller, and "unsupported sandbox mount kind \"socket\"; choose directory or file" in host::sandbox::tests::file_shares_take_no_policy_paths_or_reserved_targets. The rest of both tests stays.
  • Rewritten, checking what they checked of the code that stays: the 24 tests in Test changes, below.
  • Kept: every other test of host::sandbox, the CLI's tests, and the integration and doc tests.

Test changes

python scripts/check_test_changes.py --base 55f44aa4607 lists 25 changes to existing tests and 2 added helpers, hence tests-modified:

  • nvx-rs/src/host/sandbox/mount/tests.rs (6), with a new helper share, the directory share that parse returns for a value with its target, host path, access and policy:
    • mount_parser_defaults_to_read_only_and_accepts_rw compares the parsed shares with whole shares, so it still checks the target, the read-only and read-write access and the policy, and now also the owner and kind that parse took, instead of reading the accessors.
    • mount_parser_accepts_host_paths_with_commas and mount_owner_defaults_to_vmm_and_forwards_caller read the access and owner fields, which the mount module's tests see; the second also loses the two from_name assertions (Retired, above).
    • mount_access_policy_is_validated_and_forwarded compares the share that absolute_with(&paths::current_dir) returns with the whole share, its absolute host path and its whole policy, instead of reading its allowed and writable paths; its refusals don't change.
    • mount_absolute_path_keeps_symbolic_link_components and mount_absolute_path_keeps_a_link_on_the_host call absolute_with(&paths::current_dir), what absolute called.
  • nvx-rs/src/host/sandbox/options/tests.rs (15), with a new helper defaults, which holds what SandboxOptions::new did:
    • the helper launching and the 9 tests that built options with SandboxOptions::new build them with defaults: sandbox_rejects_systemd_entrypoint, sandbox_managed_operations_reject_dry_run_before_state_access, sandbox_exec_forwards_explicit_empty_environment_and_cwd, sandbox_exec_forwards_environment_inheritance, sandbox_run_and_exec_alone_take_the_execution_options, sandbox_environment_is_validated_before_state_access, managed_non_launch_operations_reject_network_policy_options, sandbox_launches_take_the_scratch_image_that_their_upper_needs and sandbox_command_rejects_misplaced_mount_options;
    • policy_paths_need_a_preceding_mount_only_with_several_shares, policy_paths_attach_to_their_shares, sandbox_command_attributes_policy_paths_to_the_preceding_mount and sandbox_command_attributes_mount_deny_to_the_preceding_mount compare the shares of sandbox_mounts with whole shares, their policies included, instead of reading policy(), guest_target(), owner() and kind();
    • sandbox_mounts_take_the_kind_of_each_host_path tells the shared file by its host path instead of its target, and compares both shares, their kinds and the caller owner included, with whole shares.
  • nvx-rs/src/host/sandbox/tests.rs (1): file_shares_take_no_policy_paths_or_reserved_targets (Retired, above).
  • nvx-rs/src/host/sandbox/tests/specification.rs (3): windows_drive_relative_and_unc_paths, windows_drive_relative_policy_paths_become_absolute and the proptest host_paths_become_absolute call absolute_with and absolute_policy_paths_with with paths::current_dir, what the deleted wrappers called.

doc/acceptance-case-matrix.md names none of these tests, so it doesn't change.

Validation

local_gates.py --files (from dev) selects test-changes, markdown-links, test-layout, the check-nvx-rs steps and nvx-cli; I ran them from this branch's action definitions, with Rust 1.95.0, on Windows and on nvx-mshv-test (Azure Linux 3, kernel 6.18, linux-mshv, virtual machine), in a temporary worktree. Nothing here changes what a VM shows: no NVX program called these items. Before this merge turn, the branch was rebased onto 5715bb6d709, after SL7.8c, SL6.14, SL4.4a to SL4.4c, SL6.11b, SL6.11d and SL6.13b, with no conflicts, and the checks below ran on that rebase, a63fbd07b56. At this turn it was rebased onto 55f44aa4607, after SL4.4d (#1545), with no conflicts: public_api.py --update leaves the snapshots as they are, and git range-diff shows = against the reviewed e757a1872ed, so this head, 8abb8f7f031, carries the same change. On a63fbd07b56:

Check Windows (local) Linux (MSHV, kernel 6.18)
cargo fmt --all --check passed passed
Clippy -D warnings: --all-targets --all-features, --lib, --lib --no-default-features, serde, cli and testing alone passed, natively and for --target x86_64-unknown-linux-gnu, and so did --all-targets with default features passed
Guest Clippy, --target x86_64-unknown-linux-musl --no-default-features --features guest -D warnings passed passed (check-guest-binaries)
cargo check --all-targets --all-features --target aarch64-apple-darwin passed; its one warning, in host/snapshot/tests.rs, is the base's passed, the same warning
cargo test --locked --all-features passed: lib 1067 passed, 18 ignored; every integration test (cli 114 passed, 2 ignored) and the doc tests passed: lib 1776 passed, 59 ignored; every integration test (cli 124 passed) and the doc tests
cargo test --locked, cargo build --locked --lib --no-default-features, cargo test --locked --lib --no-default-features passed (lib 686 passed, 10 ignored; 190 passed without default features) passed (lib 681 passed, 10 ignored; 190 passed without default features)
RUSTDOCFLAGS=-D warnings cargo doc --locked --no-deps --all-features passed passed
MSRV: cargo +1.89 check --locked --all-targets --all-features passed passed
python scripts/public_api.py passed passed
check-guest-binaries, build-guest-binaries, test-guest-binaries passed (33 tests)
check_test_layout.py, check_markdown_links.py, check_matrix.py <worktree> 99d2022085f passed

A code-review subagent reviewed the whole diff and found no issue. The validate-nvx unittest list is unaffected: this PR changes no script.

Left to CI: the microVM scenarios, as for any change, and openvmm-ci, for the unchanged pin.

@ppenna Pedro Henrique Penna (ppenna) added tests-modified Changes existing tests, test data, or test wiring feat-nvx-refactor labels Nov 12, 2026
@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

This pull request changes existing tests

It modifies or removes 25 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests.

Test code

Change What Before After
modified helper fn launching in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:27 nvx-rs/src/host/sandbox/options/tests.rs:52
modified test fn mount_parser_defaults_to_read_only_and_accepts_rw in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:28 nvx-rs/src/host/sandbox/mount/tests.rs:43
modified test fn mount_parser_accepts_host_paths_with_commas in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:92 nvx-rs/src/host/sandbox/mount/tests.rs:117
modified test fn mount_owner_defaults_to_vmm_and_forwards_caller in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:147 nvx-rs/src/host/sandbox/mount/tests.rs:172
modified test fn mount_access_policy_is_validated_and_forwarded in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:169 nvx-rs/src/host/sandbox/mount/tests.rs:189
modified test fn mount_absolute_path_keeps_symbolic_link_components in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:271 nvx-rs/src/host/sandbox/mount/tests.rs:296
modified test fn mount_absolute_path_keeps_a_link_on_the_host in host::sandbox::mount::tests nvx-rs/src/host/sandbox/mount/tests.rs:349 nvx-rs/src/host/sandbox/mount/tests.rs:377
modified test fn sandbox_rejects_systemd_entrypoint in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:36 nvx-rs/src/host/sandbox/options/tests.rs:61
modified test fn policy_paths_need_a_preceding_mount_only_with_several_shares in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:284 nvx-rs/src/host/sandbox/options/tests.rs:309
modified test fn policy_paths_attach_to_their_shares in host::sandbox::options::tests, proptest!`` nvx-rs/src/host/sandbox/options/tests.rs:312 nvx-rs/src/host/sandbox/options/tests.rs:341
modified test fn sandbox_managed_operations_reject_dry_run_before_state_access in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:556 nvx-rs/src/host/sandbox/options/tests.rs:586
modified test fn sandbox_exec_forwards_explicit_empty_environment_and_cwd in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:581 nvx-rs/src/host/sandbox/options/tests.rs:611
modified test fn sandbox_exec_forwards_environment_inheritance in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:594 nvx-rs/src/host/sandbox/options/tests.rs:624
modified test fn sandbox_run_and_exec_alone_take_the_execution_options in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:613 nvx-rs/src/host/sandbox/options/tests.rs:643
modified test fn sandbox_environment_is_validated_before_state_access in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:673 nvx-rs/src/host/sandbox/options/tests.rs:703
modified test fn managed_non_launch_operations_reject_network_policy_options in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:754 nvx-rs/src/host/sandbox/options/tests.rs:784
modified test fn sandbox_launches_take_the_scratch_image_that_their_upper_needs in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:809 nvx-rs/src/host/sandbox/options/tests.rs:839
modified test fn sandbox_command_rejects_misplaced_mount_options in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:873 nvx-rs/src/host/sandbox/options/tests.rs:903
modified test fn sandbox_command_attributes_policy_paths_to_the_preceding_mount in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:1132 nvx-rs/src/host/sandbox/options/tests.rs:1162
modified test fn sandbox_command_attributes_mount_deny_to_the_preceding_mount in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:1267 nvx-rs/src/host/sandbox/options/tests.rs:1305
modified test fn sandbox_mounts_take_the_kind_of_each_host_path in host::sandbox::options::tests nvx-rs/src/host/sandbox/options/tests.rs:1354 nvx-rs/src/host/sandbox/options/tests.rs:1401
modified test fn file_shares_take_no_policy_paths_or_reserved_targets in host::sandbox::tests nvx-rs/src/host/sandbox/tests.rs:146 nvx-rs/src/host/sandbox/tests.rs:146
modified test fn windows_drive_relative_and_unc_paths in host::sandbox::tests::specification nvx-rs/src/host/sandbox/tests/specification.rs:174 nvx-rs/src/host/sandbox/tests/specification.rs:174
modified test fn windows_drive_relative_policy_paths_become_absolute in host::sandbox::tests::specification nvx-rs/src/host/sandbox/tests/specification.rs:225 nvx-rs/src/host/sandbox/tests/specification.rs:225
modified test fn host_paths_become_absolute in host::sandbox::tests::specification, proptest!`` nvx-rs/src/host/sandbox/tests/specification.rs:331 nvx-rs/src/host/sandbox/tests/specification.rs:333

Relocated unchanged: 0; added: 2. Compared the merge base 55f44aa46073 with 8abb8f7f0319.

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

No active review issues block the sandbox helper cleanup.

No open issues 🎉

What changed in this PR

This PR removes sandbox helpers used only by tests, preparing host::sandbox to become private to the CLI in SL6.7b as part of the NVX library-slimming workstream. Retained runtime behavior is unchanged.

Changes:

  • Deletes mount setting parsers, unused accessors, path-conversion wrappers, and SandboxOptions::new.
  • Adapts tests to retained methods, whole-value comparisons, and test-local defaults; retires assertions for deleted parsers.
  • Updates documentation and removes 21 entries from each platform’s public API snapshot.
File Description
nvx-rs/​src/​host/​sandbox/​tests/​specification.rs Calls retained path-conversion methods directly.
nvx-rs/​src/​host/​sandbox/​tests.rs Removes the deleted kind parser’s rejection assertion.
nvx-rs/​src/​host/​sandbox/​options/​tests.rs Adds test-local defaults and compares complete mounts.
nvx-rs/​src/​host/​sandbox/​options.rs Deletes SandboxOptions::new.
nvx-rs/​src/​host/​sandbox/​mount/​tests.rs Reworks assertions and calls without deleted helpers.
nvx-rs/​src/​host/​sandbox/​mount/​request.rs Removes unused accessors and wrappers; preserves implementation docs.
nvx-rs/​src/​host/​sandbox/​mount/​mod.rs Deletes name parsers, their shared helper, and unused imports.
nvx-rs/​src/​host/​sandbox/​mount/​aggregate.rs Replaces a deleted-method documentation link with explicit error conditions.
nvx-rs/​public-api/​windows.txt Removes deleted methods from the Windows API snapshot.
nvx-rs/​public-api/​linux.txt Removes deleted methods from the Linux API snapshot.

Review effort: Balanced

SL6.7b (#1124) makes host::sandbox private to the CLI. Twelve of its
items have no caller in any NVX program, only in the public API and in
the module's own tests, so they would be dead once private. This change
deletes them first, as SL6.6c (#1122) records:

- MountAccess::from_name, MountOwner::from_name and
  MountKind::from_name, with their helper setting_from_name, which read
  a share's settings by name, as a recorded share gave them. The CLI
  maps the choices of --mount-owner itself and records a sandbox's
  export plan, not its shares' settings.
- SandboxMount's accessors guest_target, access, policy, owner and
  kind, and absolute and absolute_policy_paths, which called
  absolute_with and absolute_policy_paths_with with the current
  directory. plan_mounts and the checks of host paths call those
  variants, whose docs now say what the wrappers' said, and read the
  fields; host_path stays for sandbox_mounts. plan_mounts' docs no
  longer name absolute, and say how making a host path absolute fails.
- SandboxOptions::new, the options of an operation with the defaults of
  nvx-rs sandbox, which the CLI builds field by field instead.

The tests keep what they check of the code that stays:

- The share tests compare whole shares with the shares that
  SandboxMount::new makes, or read the fields that the mount module
  shows them, and make shares absolute with absolute_with and
  absolute_policy_paths_with and the current directory, as the deleted
  wrappers did.
- The option tests build their options with a helper of their own,
  defaults, which holds what SandboxOptions::new did, and compare the
  shares of sandbox_mounts with whole shares, policies included.
- The checks of from_name's refusals go with it: they were the only
  callers of what they checked.

The public API snapshots lose 21 lines on each platform, the eleven
public methods at both of their paths and SandboxOptions::new, and gain
none.

Part of #1122 (SL6.6c).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ppenna
Pedro Henrique Penna (ppenna) force-pushed the agents/slim-sl6-6c-sandbox-test-only branch from e757a18 to 8abb8f7 Compare November 12, 2026 20:09
@ppenna
Pedro Henrique Penna (ppenna) marked this pull request as ready for review November 12, 2026 20:10
@ppenna
Pedro Henrique Penna (ppenna) merged commit 27c6134 into feature/nvx-refactor Nov 12, 2026
85 checks passed
@ppenna
Pedro Henrique Penna (ppenna) deleted the agents/slim-sl6-6c-sandbox-test-only branch November 12, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat-nvx-refactor tests-modified Changes existing tests, test data, or test wiring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants