From 8abb8f7f03191fd590e40e1ccb9c8142c180c7c8 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 05:06:05 -0800 Subject: [PATCH] host: delete the sandbox accessors that only tests use 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> --- nvx-rs/public-api/linux.txt | 21 --- nvx-rs/public-api/windows.txt | 21 --- nvx-rs/src/host/sandbox/mount/aggregate.rs | 4 +- nvx-rs/src/host/sandbox/mount/mod.rs | 51 ------- nvx-rs/src/host/sandbox/mount/request.rs | 51 ++----- nvx-rs/src/host/sandbox/mount/tests.rs | 88 +++++++----- nvx-rs/src/host/sandbox/options.rs | 23 ---- nvx-rs/src/host/sandbox/options/tests.rs | 125 +++++++++++++----- nvx-rs/src/host/sandbox/tests.rs | 4 - .../src/host/sandbox/tests/specification.rs | 10 +- 10 files changed, 168 insertions(+), 230 deletions(-) diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 2b17bbd4e..2331d4fb9 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -1275,17 +1275,14 @@ enum nvx::host::sandbox::MountAccess variant nvx::host::sandbox::MountAccess::ReadOnly variant nvx::host::sandbox::MountAccess::ReadWrite method nvx::host::sandbox::MountAccess::as_str -method nvx::host::sandbox::MountAccess::from_name enum nvx::host::sandbox::MountKind variant nvx::host::sandbox::MountKind::Directory variant nvx::host::sandbox::MountKind::File method nvx::host::sandbox::MountKind::as_str -method nvx::host::sandbox::MountKind::from_name enum nvx::host::sandbox::MountOwner variant nvx::host::sandbox::MountOwner::Caller variant nvx::host::sandbox::MountOwner::Vmm method nvx::host::sandbox::MountOwner::as_str -method nvx::host::sandbox::MountOwner::from_name struct nvx::host::sandbox::MountPolicy field nvx::host::sandbox::MountPolicy::allowed field nvx::host::sandbox::MountPolicy::denied @@ -1299,16 +1296,9 @@ variant nvx::host::sandbox::PolicyKind::Writable method nvx::host::sandbox::PolicyKind::as_str method nvx::host::sandbox::PolicyKind::option struct nvx::host::sandbox::SandboxMount -method nvx::host::sandbox::SandboxMount::absolute -method nvx::host::sandbox::SandboxMount::absolute_policy_paths -method nvx::host::sandbox::SandboxMount::access -method nvx::host::sandbox::SandboxMount::guest_target method nvx::host::sandbox::SandboxMount::host_path -method nvx::host::sandbox::SandboxMount::kind method nvx::host::sandbox::SandboxMount::new -method nvx::host::sandbox::SandboxMount::owner method nvx::host::sandbox::SandboxMount::parse -method nvx::host::sandbox::SandboxMount::policy method nvx::host::sandbox::SandboxMount::validated method nvx::host::sandbox::SandboxMount::with_kind mod nvx::host::sandbox::mount @@ -1320,17 +1310,14 @@ enum nvx::host::sandbox::mount::MountAccess variant nvx::host::sandbox::mount::MountAccess::ReadOnly variant nvx::host::sandbox::mount::MountAccess::ReadWrite method nvx::host::sandbox::mount::MountAccess::as_str -method nvx::host::sandbox::mount::MountAccess::from_name enum nvx::host::sandbox::mount::MountKind variant nvx::host::sandbox::mount::MountKind::Directory variant nvx::host::sandbox::mount::MountKind::File method nvx::host::sandbox::mount::MountKind::as_str -method nvx::host::sandbox::mount::MountKind::from_name enum nvx::host::sandbox::mount::MountOwner variant nvx::host::sandbox::mount::MountOwner::Caller variant nvx::host::sandbox::mount::MountOwner::Vmm method nvx::host::sandbox::mount::MountOwner::as_str -method nvx::host::sandbox::mount::MountOwner::from_name struct nvx::host::sandbox::mount::MountPolicy field nvx::host::sandbox::mount::MountPolicy::allowed field nvx::host::sandbox::mount::MountPolicy::denied @@ -1347,16 +1334,9 @@ method nvx::host::sandbox::mount::PolicyKind::option const nvx::host::sandbox::mount::RESERVED_EXACT_MOUNT_TARGETS const nvx::host::sandbox::mount::RESERVED_MOUNT_TARGETS struct nvx::host::sandbox::mount::SandboxMount -method nvx::host::sandbox::mount::SandboxMount::absolute -method nvx::host::sandbox::mount::SandboxMount::absolute_policy_paths -method nvx::host::sandbox::mount::SandboxMount::access -method nvx::host::sandbox::mount::SandboxMount::guest_target method nvx::host::sandbox::mount::SandboxMount::host_path -method nvx::host::sandbox::mount::SandboxMount::kind method nvx::host::sandbox::mount::SandboxMount::new -method nvx::host::sandbox::mount::SandboxMount::owner method nvx::host::sandbox::mount::SandboxMount::parse -method nvx::host::sandbox::mount::SandboxMount::policy method nvx::host::sandbox::mount::SandboxMount::validated method nvx::host::sandbox::mount::SandboxMount::with_kind fn nvx::host::sandbox::mount::mount_kind @@ -1397,7 +1377,6 @@ field nvx::host::sandbox::options::SandboxOptions::mounts field nvx::host::sandbox::options::SandboxOptions::net field nvx::host::sandbox::options::SandboxOptions::network_options field nvx::host::sandbox::options::SandboxOptions::network_profile -method nvx::host::sandbox::options::SandboxOptions::new field nvx::host::sandbox::options::SandboxOptions::operation field nvx::host::sandbox::options::SandboxOptions::outcome_report field nvx::host::sandbox::options::SandboxOptions::policy_paths diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 6548939a6..7f7a9be67 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -733,17 +733,14 @@ enum nvx::host::sandbox::MountAccess variant nvx::host::sandbox::MountAccess::ReadOnly variant nvx::host::sandbox::MountAccess::ReadWrite method nvx::host::sandbox::MountAccess::as_str -method nvx::host::sandbox::MountAccess::from_name enum nvx::host::sandbox::MountKind variant nvx::host::sandbox::MountKind::Directory variant nvx::host::sandbox::MountKind::File method nvx::host::sandbox::MountKind::as_str -method nvx::host::sandbox::MountKind::from_name enum nvx::host::sandbox::MountOwner variant nvx::host::sandbox::MountOwner::Caller variant nvx::host::sandbox::MountOwner::Vmm method nvx::host::sandbox::MountOwner::as_str -method nvx::host::sandbox::MountOwner::from_name struct nvx::host::sandbox::MountPolicy field nvx::host::sandbox::MountPolicy::allowed field nvx::host::sandbox::MountPolicy::denied @@ -757,16 +754,9 @@ variant nvx::host::sandbox::PolicyKind::Writable method nvx::host::sandbox::PolicyKind::as_str method nvx::host::sandbox::PolicyKind::option struct nvx::host::sandbox::SandboxMount -method nvx::host::sandbox::SandboxMount::absolute -method nvx::host::sandbox::SandboxMount::absolute_policy_paths -method nvx::host::sandbox::SandboxMount::access -method nvx::host::sandbox::SandboxMount::guest_target method nvx::host::sandbox::SandboxMount::host_path -method nvx::host::sandbox::SandboxMount::kind method nvx::host::sandbox::SandboxMount::new -method nvx::host::sandbox::SandboxMount::owner method nvx::host::sandbox::SandboxMount::parse -method nvx::host::sandbox::SandboxMount::policy method nvx::host::sandbox::SandboxMount::validated method nvx::host::sandbox::SandboxMount::with_kind mod nvx::host::sandbox::mount @@ -778,17 +768,14 @@ enum nvx::host::sandbox::mount::MountAccess variant nvx::host::sandbox::mount::MountAccess::ReadOnly variant nvx::host::sandbox::mount::MountAccess::ReadWrite method nvx::host::sandbox::mount::MountAccess::as_str -method nvx::host::sandbox::mount::MountAccess::from_name enum nvx::host::sandbox::mount::MountKind variant nvx::host::sandbox::mount::MountKind::Directory variant nvx::host::sandbox::mount::MountKind::File method nvx::host::sandbox::mount::MountKind::as_str -method nvx::host::sandbox::mount::MountKind::from_name enum nvx::host::sandbox::mount::MountOwner variant nvx::host::sandbox::mount::MountOwner::Caller variant nvx::host::sandbox::mount::MountOwner::Vmm method nvx::host::sandbox::mount::MountOwner::as_str -method nvx::host::sandbox::mount::MountOwner::from_name struct nvx::host::sandbox::mount::MountPolicy field nvx::host::sandbox::mount::MountPolicy::allowed field nvx::host::sandbox::mount::MountPolicy::denied @@ -805,16 +792,9 @@ method nvx::host::sandbox::mount::PolicyKind::option const nvx::host::sandbox::mount::RESERVED_EXACT_MOUNT_TARGETS const nvx::host::sandbox::mount::RESERVED_MOUNT_TARGETS struct nvx::host::sandbox::mount::SandboxMount -method nvx::host::sandbox::mount::SandboxMount::absolute -method nvx::host::sandbox::mount::SandboxMount::absolute_policy_paths -method nvx::host::sandbox::mount::SandboxMount::access -method nvx::host::sandbox::mount::SandboxMount::guest_target method nvx::host::sandbox::mount::SandboxMount::host_path -method nvx::host::sandbox::mount::SandboxMount::kind method nvx::host::sandbox::mount::SandboxMount::new -method nvx::host::sandbox::mount::SandboxMount::owner method nvx::host::sandbox::mount::SandboxMount::parse -method nvx::host::sandbox::mount::SandboxMount::policy method nvx::host::sandbox::mount::SandboxMount::validated method nvx::host::sandbox::mount::SandboxMount::with_kind fn nvx::host::sandbox::mount::mount_kind @@ -855,7 +835,6 @@ field nvx::host::sandbox::options::SandboxOptions::mounts field nvx::host::sandbox::options::SandboxOptions::net field nvx::host::sandbox::options::SandboxOptions::network_options field nvx::host::sandbox::options::SandboxOptions::network_profile -method nvx::host::sandbox::options::SandboxOptions::new field nvx::host::sandbox::options::SandboxOptions::operation field nvx::host::sandbox::options::SandboxOptions::outcome_report field nvx::host::sandbox::options::SandboxOptions::policy_paths diff --git a/nvx-rs/src/host/sandbox/mount/aggregate.rs b/nvx-rs/src/host/sandbox/mount/aggregate.rs index b730b64e3..fd06b3b15 100644 --- a/nvx-rs/src/host/sandbox/mount/aggregate.rs +++ b/nvx-rs/src/host/sandbox/mount/aggregate.rs @@ -26,7 +26,9 @@ use crate::host::sandbox::refused; /// finds to be of the other kind is refused, as [`SandboxMount::validated`] refuses it. Returns /// `None` without shares. /// -/// Fails as those checks, [`SandboxMount::absolute`], and [`filesystem::plan`] do. +/// Fails as those checks and [`filesystem::plan`] do, and with +/// [`ErrorKind::Io`](crate::ErrorKind::Io) if a relative host path meets an unreadable current +/// directory, or Windows cannot qualify a drive-relative path. pub fn plan_mounts(mounts: &[SandboxMount]) -> Result> { validate_mounts(mounts)?; for mount in mounts { diff --git a/nvx-rs/src/host/sandbox/mount/mod.rs b/nvx-rs/src/host/sandbox/mount/mod.rs index a679bc790..213438df7 100644 --- a/nvx-rs/src/host/sandbox/mount/mod.rs +++ b/nvx-rs/src/host/sandbox/mount/mod.rs @@ -25,9 +25,6 @@ mod request; use std::fmt; -use super::refused; -use crate::error::Result; - pub use crate::protocol::share::{RESERVED_EXACT_MOUNT_TARGETS, RESERVED_MOUNT_TARGETS}; pub use self::aggregate::plan_mounts; @@ -74,18 +71,6 @@ impl MountAccess { Self::ReadWrite => "rw", } } - - /// Returns the mode that `name` names, as a recorded share gives it. - /// - /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for another name. - pub fn from_name(name: &str) -> Result { - setting_from_name( - "mode", - [Self::ReadOnly, Self::ReadWrite], - Self::as_str, - name, - ) - } } impl fmt::Display for MountAccess { @@ -112,13 +97,6 @@ impl MountOwner { Self::Caller => "caller", } } - - /// Returns the owner that `name` names, as a recorded share gives it. - /// - /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for another name. - pub fn from_name(name: &str) -> Result { - setting_from_name("owner", [Self::Vmm, Self::Caller], Self::as_str, name) - } } impl fmt::Display for MountOwner { @@ -145,13 +123,6 @@ impl MountKind { Self::File => "file", } } - - /// Returns the kind that `name` names, as a recorded share gives it. - /// - /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for another name. - pub fn from_name(name: &str) -> Result { - setting_from_name("kind", [Self::Directory, Self::File], Self::as_str, name) - } } impl fmt::Display for MountKind { @@ -160,28 +131,6 @@ impl fmt::Display for MountKind { } } -/// Returns the one of `values`, the two values of a share's `setting`, whose name, as `as_str` -/// gives it, is `name`, as a recorded share gives it. -/// -/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for another name, with a -/// message that names the setting and its two values. -fn setting_from_name( - setting: &str, - values: [T; 2], - as_str: fn(T) -> &'static str, - name: &str, -) -> Result { - values - .into_iter() - .find(|&value| as_str(value) == name) - .ok_or_else(|| { - let [first, second] = values.map(as_str); - refused(format!( - "unsupported sandbox mount {setting} {name:?}; choose {first} or {second}", - )) - }) -} - /// A kind of access-policy path. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum PolicyKind { diff --git a/nvx-rs/src/host/sandbox/mount/request.rs b/nvx-rs/src/host/sandbox/mount/request.rs index 9d940d5fc..258aa82d1 100644 --- a/nvx-rs/src/host/sandbox/mount/request.rs +++ b/nvx-rs/src/host/sandbox/mount/request.rs @@ -272,36 +272,11 @@ impl SandboxMount { }) } - /// Returns the guest target. - pub fn guest_target(&self) -> &str { - &self.guest_target - } - /// Returns the host directory or file. pub fn host_path(&self) -> &Path { &self.host_path } - /// Returns the access mode. - pub fn access(&self) -> MountAccess { - self.access - } - - /// Returns the access-policy paths. - pub fn policy(&self) -> &MountPolicy { - &self.policy - } - - /// Returns the identity of the share's file operations. - pub fn owner(&self) -> MountOwner { - self.owner - } - - /// Returns whether the share binds a directory or a file. - pub fn kind(&self) -> MountKind { - self.kind - } - /// Checks the share against the host: its owner must be one that the host supports, its /// host path must be a regular file or a plain directory, as its kind requires, without /// being a symbolic link, which on Windows includes a junction, as [`Path::is_symlink`] tells @@ -352,18 +327,14 @@ impl SandboxMount { } /// Returns the share with its host path made absolute, as the one function of NVX that makes - /// host paths absolute makes it: joined to the current directory if it is relative, without - /// resolving it, so OpenVMM still sees, and refuses, every symbolic-link component of the - /// path that was requested. On Windows, a path with another drive and no root, such as - /// `D:relative`, is below the current directory of its drive. + /// host paths absolute makes it, with `cwd` as the current directory: joined to the current + /// directory if it is relative, without resolving it, so OpenVMM still sees, and refuses, + /// every symbolic-link component of the path that was requested. On Windows, a path with + /// another drive and no root, such as `D:relative`, is below the current directory of its + /// drive. /// /// Fails with [`ErrorKind::Io`](crate::ErrorKind::Io) if a relative host path meets an /// unreadable current directory, or Windows cannot qualify a drive-relative path. - pub fn absolute(&self) -> Result { - self.absolute_with(&paths::current_dir) - } - - /// Implements [`absolute`](Self::absolute) with `cwd` as the current directory. pub(crate) fn absolute_with(&self, cwd: &dyn Fn() -> io::Result) -> Result { if self.host_path.is_absolute() { return Ok(self.clone()); @@ -376,16 +347,10 @@ impl SandboxMount { } /// Returns each kind of access-policy path with its requested paths, relative paths joined - /// to the absolute host path of the share, as OpenVMM attributes them to the children of an - /// aggregate. + /// to the absolute host path of the share, with `cwd` as the current directory, as OpenVMM + /// attributes them to the children of an aggregate. /// - /// Fails as [`absolute`](Self::absolute) does. - pub fn absolute_policy_paths(&self) -> Result)>> { - self.absolute_policy_paths_with(&paths::current_dir) - } - - /// Implements [`absolute_policy_paths`](Self::absolute_policy_paths) with `cwd` as the - /// current directory. + /// Fails as [`absolute_with`](Self::absolute_with) does. pub(crate) fn absolute_policy_paths_with( &self, cwd: &dyn Fn() -> io::Result, diff --git a/nvx-rs/src/host/sandbox/mount/tests.rs b/nvx-rs/src/host/sandbox/mount/tests.rs index ab845ecf7..ac11002fd 100644 --- a/nvx-rs/src/host/sandbox/mount/tests.rs +++ b/nvx-rs/src/host/sandbox/mount/tests.rs @@ -8,6 +8,7 @@ pub(crate) use super::host_paths::validate_mount_host_paths_with; use super::request::require_mount_owner_supported_on; use super::*; use crate::ErrorKind; +use crate::error::Result; use crate::host::filesystem::{Owner, ReservedTargets}; use crate::host::paths; #[cfg(windows)] @@ -18,6 +19,20 @@ fn parse(value: &str, policy: MountPolicy) -> Result { SandboxMount::parse(value, policy, MountOwner::Vmm, MountKind::Directory) } +/// Returns the share of the directory `host_path` at `target`, with `access` and `policy`, whose +/// file operations run as OpenVMM, which [`parse`] returns for a value that names them. +fn share(target: &str, host_path: &str, access: MountAccess, policy: MountPolicy) -> SandboxMount { + SandboxMount::new( + target, + host_path, + access, + policy, + MountOwner::Vmm, + MountKind::Directory, + ) + .unwrap() +} + fn message(result: Result) -> String { let error = result.unwrap_err(); assert_eq!(error.kind(), ErrorKind::Unsupported); @@ -27,18 +42,28 @@ fn message(result: Result) -> String { #[test] fn mount_parser_defaults_to_read_only_and_accepts_rw() { let default = parse("/workspace,host-dir", MountPolicy::default()).unwrap(); - assert_eq!(default.guest_target(), "/workspace"); assert_eq!(default.host_path(), Path::new("host-dir")); - assert_eq!(default.access(), MountAccess::ReadOnly); - assert!(default.policy().denied.is_empty()); + assert_eq!( + default, + share( + "/workspace", + "host-dir", + MountAccess::ReadOnly, + MountPolicy::default() + ) + ); - let writable = parse( - "/opt/hostedtoolcache,host-dir,rw", - policy(&["logs", "secrets"], &[], &[]), - ) - .unwrap(); - assert_eq!(writable.access(), MountAccess::ReadWrite); - assert_eq!(writable.policy().denied, ["logs", "secrets"]); + let denied = policy(&["logs", "secrets"], &[], &[]); + let writable = parse("/opt/hostedtoolcache,host-dir,rw", denied.clone()).unwrap(); + assert_eq!( + writable, + share( + "/opt/hostedtoolcache", + "host-dir", + MountAccess::ReadWrite, + denied + ) + ); } #[test] @@ -139,14 +164,14 @@ fn mount_parser_accepts_host_paths_with_commas() { ); let mount = parse(&value, policy(&["secret"], &[], &[])).unwrap(); assert_eq!(mount.host_path(), Path::new(&native(host_path))); - assert_eq!(mount.access(), access); + assert_eq!(mount.access, access); } } #[test] fn mount_owner_defaults_to_vmm_and_forwards_caller() { let default = parse("/workspace,host-dir,rw", MountPolicy::default()).unwrap(); - assert_eq!(default.owner(), MountOwner::Vmm); + assert_eq!(default.owner, MountOwner::Vmm); let caller = SandboxMount::parse( "/workspace,host-dir,rw", policy(&["secrets"], &[], &[]), @@ -154,27 +179,27 @@ fn mount_owner_defaults_to_vmm_and_forwards_caller() { MountKind::Directory, ) .unwrap(); - assert_eq!(caller.absolute().unwrap().owner(), MountOwner::Caller); assert_eq!( - message(MountOwner::from_name("root")), - "unsupported sandbox mount owner \"root\"; choose vmm or caller" - ); - assert_eq!( - message(MountAccess::from_name("rx")), - "unsupported sandbox mount mode \"rx\"; choose ro or rw" + caller.absolute_with(&paths::current_dir).unwrap().owner, + MountOwner::Caller ); } #[test] fn mount_access_policy_is_validated_and_forwarded() { - let mount = parse( - "/tmp/mcp-runtime,host-dir,rw", - policy(&["mcp-logs"], &["mcp-logs/payloads"], &["agent", "cache"]), - ) - .unwrap(); - let absolute = mount.absolute().unwrap(); - assert_eq!(absolute.policy().allowed, ["mcp-logs/payloads"]); - assert_eq!(absolute.policy().writable, ["agent", "cache"]); + let requested = policy(&["mcp-logs"], &["mcp-logs/payloads"], &["agent", "cache"]); + let mount = parse("/tmp/mcp-runtime,host-dir,rw", requested.clone()).unwrap(); + let absolute = mount.absolute_with(&paths::current_dir).unwrap(); + let host_path = paths::current_dir().unwrap().join("host-dir"); + assert_eq!( + absolute, + share( + "/tmp/mcp-runtime", + host_path.to_str().unwrap(), + MountAccess::ReadWrite, + requested + ) + ); let paths: Vec = (0..129).map(|index| format!("path-{index}")).collect(); let many: Vec<&str> = paths.iter().map(String::as_str).collect(); for (value, policy, expected) in [ @@ -270,11 +295,14 @@ fn caller_owner_requires_linux_and_a_directory_that_root_does_not_own() { #[test] fn mount_absolute_path_keeps_symbolic_link_components() { let mount = parse("/workspace,link/share,rw", MountPolicy::default()).unwrap(); - let absolute = mount.absolute().unwrap(); + let absolute = mount.absolute_with(&paths::current_dir).unwrap(); let expected = paths::current_dir().unwrap().join("link").join("share"); assert_eq!(absolute.host_path(), expected.as_path()); assert!(absolute.host_path.iter().any(|part| part == "link")); - assert_eq!(absolute.absolute().unwrap(), absolute); + assert_eq!( + absolute.absolute_with(&paths::current_dir).unwrap(), + absolute + ); let fixed = host("/fixture/cwd"); let relative = SandboxMount::parse( "/workspace,link/share,rw", @@ -366,7 +394,7 @@ fn mount_absolute_path_keeps_a_link_on_the_host() { MountPolicy::default(), ) .unwrap(); - let absolute = mount.absolute().unwrap(); + let absolute = mount.absolute_with(&paths::current_dir).unwrap(); assert!(!absolute.host_path.iter().any(|part| part == "link-target")); } diff --git a/nvx-rs/src/host/sandbox/options.rs b/nvx-rs/src/host/sandbox/options.rs index a35fa871f..cfe39c6a2 100644 --- a/nvx-rs/src/host/sandbox/options.rs +++ b/nvx-rs/src/host/sandbox/options.rs @@ -136,29 +136,6 @@ pub struct SandboxOptions { } impl SandboxOptions { - /// Returns the options of an operation with the defaults of `nvx-rs sandbox`. - pub fn new(operation: SandboxOperation) -> Self { - Self { - operation, - dry_run: false, - cwd: None, - environment: Vec::new(), - environment_file: None, - inherit_default_environment: false, - network_options: false, - net: false, - network_profile: false, - entrypoint: super::DEFAULT_ENTRYPOINT.to_owned(), - outcome_report: false, - mounts: Vec::new(), - policy_paths: Vec::new(), - mount_owner: None, - layers: false, - scratch: false, - upper: None, - } - } - /// Applies the checks of `nvx-rs sandbox` that precede the launch, in its order, and /// returns the managed exec environment of a `run` or `exec`, if one was given. /// diff --git a/nvx-rs/src/host/sandbox/options/tests.rs b/nvx-rs/src/host/sandbox/options/tests.rs index 6c3b58ae7..6a155985a 100644 --- a/nvx-rs/src/host/sandbox/options/tests.rs +++ b/nvx-rs/src/host/sandbox/options/tests.rs @@ -1,7 +1,7 @@ //! The `nvx.py sandbox` and `nvx.py run` option tests of `scripts/test_nvx_tools.py`, ported. use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use proptest::prelude::*; #[cfg(feature = "cli")] @@ -12,11 +12,13 @@ use sha2::{Digest, Sha256}; use super::*; use crate::ErrorKind; use crate::host::paths::{self, resolve}; +use crate::host::sandbox::DEFAULT_ENTRYPOINT; #[cfg(feature = "cli")] use crate::host::sandbox::distro::{DISTRO_POLICY_REVISION, DistroBinding, DistroFormat}; #[cfg(feature = "cli")] use crate::host::sandbox::layer::{LayerRole, SandboxLayer}; -use crate::host::sandbox::mount::{plan_mounts, validate_mounts}; +use crate::host::sandbox::mount::{MountAccess, plan_mounts, validate_mounts}; +use crate::host::sandbox::tests::policy as mount_policy; fn message(result: Result) -> String { let error = result.unwrap_err(); @@ -24,11 +26,34 @@ fn message(result: Result) -> String { error.to_string() } +/// Returns the options of `operation` with the defaults of `nvx-rs sandbox`. +fn defaults(operation: SandboxOperation) -> SandboxOptions { + SandboxOptions { + operation, + dry_run: false, + cwd: None, + environment: Vec::new(), + environment_file: None, + inherit_default_environment: false, + network_options: false, + net: false, + network_profile: false, + entrypoint: DEFAULT_ENTRYPOINT.to_owned(), + outcome_report: false, + mounts: Vec::new(), + policy_paths: Vec::new(), + mount_owner: None, + layers: false, + scratch: false, + upper: None, + } +} + fn launching(operation: SandboxOperation) -> SandboxOptions { SandboxOptions { layers: true, scratch: true, - ..SandboxOptions::new(operation) + ..defaults(operation) } } @@ -51,7 +76,7 @@ fn sandbox_rejects_systemd_entrypoint() { } let stop = SandboxOptions { entrypoint: entrypoint.to_owned(), - ..SandboxOptions::new(SandboxOperation::Stop) + ..defaults(SandboxOperation::Stop) }; assert_eq!(stop.check().unwrap(), None); } @@ -290,7 +315,11 @@ fn policy_paths_need_a_preceding_mount_only_with_several_shares() { path: "secret".to_owned(), }; let mounts = sandbox_mounts(std::slice::from_ref(&first), &[option(None)], None).unwrap(); - assert_eq!(mounts[0].policy().denied, ["secret"]); + let denied = mount_policy(&["secret"], &[], &[]); + assert_eq!( + mounts, + [SandboxMount::parse(&first, denied, MountOwner::Vmm, MountKind::Directory).unwrap()] + ); for policy in [option(None), option(Some(2))] { assert_eq!( sandbox_mounts(&[first.clone(), second.clone()], &[policy], None) @@ -340,10 +369,11 @@ proptest! { let actual = sandbox_mounts(&mounts, &options, None).unwrap(); prop_assert_eq!(actual.len(), expected.len()); for (index, (mount, policy)) in actual.iter().zip(expected).enumerate() { - prop_assert_eq!(mount.guest_target(), format!("/share-{index}")); - prop_assert_eq!(mount.policy(), &policy); - prop_assert_eq!(mount.owner(), MountOwner::Vmm); - prop_assert_eq!(mount.kind(), MountKind::Directory); + let share = SandboxMount::new( + &format!("/share-{index}"), &format!("host-{index}"), MountAccess::ReadWrite, + policy, MountOwner::Vmm, MountKind::Directory, + ).unwrap(); + prop_assert_eq!(mount, &share); } } @@ -563,7 +593,7 @@ fn sandbox_managed_operations_reject_dry_run_before_state_access() { ] { let options = SandboxOptions { dry_run: true, - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!( message(options.check()), @@ -585,7 +615,7 @@ fn sandbox_exec_forwards_explicit_empty_environment_and_cwd() { let options = SandboxOptions { cwd: Some("/work".to_owned()), environment_file: Some(file.to_str().unwrap().to_owned()), - ..SandboxOptions::new(SandboxOperation::Exec) + ..defaults(SandboxOperation::Exec) }; assert_eq!(options.check().unwrap(), Some(Vec::new())); } @@ -601,7 +631,7 @@ fn sandbox_exec_forwards_environment_inheritance() { let options = SandboxOptions { environment: environment.iter().map(|&entry| entry.to_owned()).collect(), inherit_default_environment: true, - ..SandboxOptions::new(SandboxOperation::Exec) + ..defaults(SandboxOperation::Exec) }; let expected = expected.map(|entries| entries.iter().map(|&entry| entry.to_owned()).collect()); @@ -614,7 +644,7 @@ fn sandbox_run_and_exec_alone_take_the_execution_options() { let both = SandboxOptions { environment: vec!["A=1".to_owned()], environment_file: Some("environment.json".to_owned()), - ..SandboxOptions::new(SandboxOperation::Exec) + ..defaults(SandboxOperation::Exec) }; assert_eq!( message(both.check()), @@ -637,17 +667,17 @@ fn sandbox_run_and_exec_alone_take_the_execution_options() { ] { let environment = SandboxOptions { environment: vec!["A=1".to_owned()], - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!(message(environment.check()), exec_only); let inheriting = SandboxOptions { inherit_default_environment: true, - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!(message(inheriting.check()), exec_only); let cwd = SandboxOptions { cwd: Some("/".to_owned()), - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!(message(cwd.check()), exec_only); } @@ -693,7 +723,7 @@ fn sandbox_environment_is_validated_before_state_access() { ] { let options = SandboxOptions { environment, - ..SandboxOptions::new(SandboxOperation::Exec) + ..defaults(SandboxOperation::Exec) }; assert_eq!(message(options.check()), expected); } @@ -760,7 +790,7 @@ fn managed_non_launch_operations_reject_network_policy_options() { ] { let options = SandboxOptions { network_options: true, - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!( message(options.check()), @@ -797,7 +827,7 @@ fn managed_non_launch_operations_reject_network_policy_options() { } let report = SandboxOptions { outcome_report: true, - ..SandboxOptions::new(SandboxOperation::Start) + ..defaults(SandboxOperation::Start) }; assert_eq!( message(report.check()), @@ -859,7 +889,7 @@ fn sandbox_launches_take_the_scratch_image_that_their_upper_needs() { for upper in [RootUpper::Scratch, RootUpper::Ram] { let options = SandboxOptions { upper: Some(upper), - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!( message(options.check()), @@ -898,7 +928,7 @@ fn sandbox_command_rejects_misplaced_mount_options() { ] { let options = SandboxOptions { mounts: vec!["/workspace,share,rw".to_owned()], - ..SandboxOptions::new(operation) + ..defaults(operation) }; assert_eq!( message(options.check()), @@ -1155,11 +1185,19 @@ fn sandbox_command_attributes_policy_paths_to_the_preceding_mount() { }) .collect(); let parsed = sandbox_mounts(&mounts, &options, None).unwrap(); - assert_eq!(parsed[0].policy().denied, ["logs"]); - assert_eq!(parsed[0].policy().allowed, ["logs/payloads"]); - assert_eq!(parsed[0].policy().writable, ["out"]); - assert_eq!(parsed[1].policy().denied, ["credentials"]); - assert!(parsed[1].policy().allowed.is_empty() && parsed[1].policy().writable.is_empty()); + let share = |value: &str, requested: MountPolicy| { + SandboxMount::parse(value, requested, MountOwner::Vmm, MountKind::Directory).unwrap() + }; + assert_eq!( + parsed, + [ + share( + &mounts[0], + mount_policy(&["logs"], &["logs/payloads"], &["out"]) + ), + share(&mounts[1], mount_policy(&["credentials"], &[], &[])), + ] + ); let (arguments, _) = shares.run(&mounts, &policy, None).unwrap(); // With several shares, OpenVMM attributes absolute paths. assert_eq!( @@ -1318,7 +1356,16 @@ fn sandbox_command_attributes_mount_deny_to_the_preceding_mount() { None, ) .unwrap(); - assert_eq!(single[0].policy().denied, ["secrets"]); + assert_eq!( + single, + [SandboxMount::parse( + &format!("/w,{work},rw"), + mount_policy(&["secrets"], &[], &[]), + MountOwner::Vmm, + MountKind::Directory, + ) + .unwrap()] + ); let error = shares .run( &[ @@ -1354,17 +1401,31 @@ fn sandbox_command_attributes_mount_deny_to_the_preceding_mount() { fn sandbox_mounts_take_the_kind_of_each_host_path() { let mounts = ["/a,a".to_owned(), "/b,b.json,rw".to_owned()]; let kind_of = |mount: &SandboxMount| { - if mount.guest_target() == "/b" { + if mount.host_path() == Path::new("b.json") { MountKind::File } else { MountKind::Directory } }; let parsed = sandbox_mounts_with(&mounts, &[], Some(MountOwner::Caller), &kind_of).unwrap(); - assert_eq!(parsed[0].kind(), MountKind::Directory); - assert_eq!(parsed[1].kind(), MountKind::File); - assert_eq!(parsed[0].owner(), MountOwner::Caller); - assert_eq!(parsed[1].owner(), MountOwner::Caller); + let share = |target, host_path, access, kind| { + SandboxMount::new( + target, + host_path, + access, + MountPolicy::default(), + MountOwner::Caller, + kind, + ) + .unwrap() + }; + assert_eq!( + parsed, + [ + share("/a", "a", MountAccess::ReadOnly, MountKind::Directory), + share("/b", "b.json", MountAccess::ReadWrite, MountKind::File), + ] + ); } #[test] diff --git a/nvx-rs/src/host/sandbox/tests.rs b/nvx-rs/src/host/sandbox/tests.rs index 18165773e..4e756fd23 100644 --- a/nvx-rs/src/host/sandbox/tests.rs +++ b/nvx-rs/src/host/sandbox/tests.rs @@ -161,10 +161,6 @@ fn file_shares_take_no_policy_paths_or_reserved_targets() { .unwrap_err(); assert!(error.to_string().contains("shares a file"), "{error}"); } - assert_eq!( - message(MountKind::from_name("socket")), - "unsupported sandbox mount kind \"socket\"; choose directory or file" - ); // The runtime replaces /etc/machine-id with a file, binds the guest's resolver file over // /etc/resolv.conf, and binds its time state over /run/nvx/time, so nothing at or below them // may be shared. diff --git a/nvx-rs/src/host/sandbox/tests/specification.rs b/nvx-rs/src/host/sandbox/tests/specification.rs index 08b6b3f6f..8e990c8f5 100644 --- a/nvx-rs/src/host/sandbox/tests/specification.rs +++ b/nvx-rs/src/host/sandbox/tests/specification.rs @@ -213,7 +213,7 @@ fn windows_drive_relative_and_unc_paths() { MountKind::Directory, ) .unwrap() - .absolute() + .absolute_with(&paths::current_dir) .unwrap(); assert_eq!(mount.host_path(), expected); assert!(mount.host_path().is_absolute()); @@ -243,7 +243,9 @@ fn windows_drive_relative_policy_paths_become_absolute() { MountKind::Directory, ) .unwrap(); - let paths = mount.absolute_policy_paths().unwrap(); + let paths = mount + .absolute_policy_paths_with(&paths::current_dir) + .unwrap(); assert_eq!( Path::new(&paths[0].1[0]), std::path::absolute(relative).unwrap() @@ -339,11 +341,11 @@ proptest! { "/workspace", relative.to_str().unwrap(), MountAccess::ReadOnly, MountPolicy::default(), MountOwner::Vmm, MountKind::Directory, ).unwrap(); - let absolute = mount.absolute().unwrap(); + let absolute = mount.absolute_with(&paths::current_dir).unwrap(); prop_assert_eq!(absolute.host_path(), &expected); prop_assert!(absolute.host_path().is_absolute()); prop_assert!(absolute.host_path().is_dir()); - prop_assert_eq!(absolute.absolute().unwrap(), absolute); + prop_assert_eq!(absolute.absolute_with(&paths::current_dir).unwrap(), absolute); } /// A share's target is one value for the guest: a target in which the kernel would end a