From 11b430db23400ecd05878af8bdff067c5c8f3180 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 10 Nov 2026 01:05:39 -0800 Subject: [PATCH 1/5] host: reconcile verified stop outcomes onto feature Port the preserved SL2.10 implementation and coupled callers, API snapshots, docs and real-process tests onto feature c52c7101 without changing legacy lifecycle policy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c8e42ad-4102-451f-96f0-0a4ae0483d1a --- doc/usage.md | 1 + nvx-rs/README.md | 24 ++- nvx-rs/public-api/linux.txt | 8 + nvx-rs/public-api/windows.txt | 8 + nvx-rs/src/error.rs | 9 + nvx-rs/src/error/tests.rs | 1 + nvx-rs/src/host/instance/locked.rs | 3 +- nvx-rs/src/host/platform/windows/tests.rs | 15 +- nvx-rs/src/host/process.rs | 18 +- nvx-rs/src/host/process/tests.rs | 3 +- nvx-rs/src/host/process/tests/linux.rs | 5 +- nvx-rs/src/host/report.rs | 25 ++- nvx-rs/src/host/report/tests.rs | 55 ++++- nvx-rs/src/host/template/clone/records.rs | 4 +- .../src/host/template/clone/tests/launch.rs | 17 +- .../src/host/template/clone/tests/restore.rs | 2 +- .../src/host/template/store/leases/tests.rs | 8 +- nvx-rs/src/host/vm.rs | 74 +++++++ nvx-rs/src/host/vm/tests.rs | 189 +++++++++++++++++- nvx-rs/src/host/vm/tests/reattach.rs | 2 +- nvx-rs/tests/checked_handoff.rs | 2 + nvx-rs/tests/cli.rs | 9 +- nvx-rs/tests/cli/download.rs | 16 ++ nvx-rs/tests/cli/lifecycle.rs | 15 +- nvx-rs/tests/fake_openvmm.rs | 7 +- nvx-rs/tests/store.rs | 11 +- nvx-rs/tests/supervisor.rs | 42 ++-- .../src/bin/runtime-adapter-host.rs | 16 +- tests/runtime-adapter/tests/vm_clone.rs | 14 +- 29 files changed, 530 insertions(+), 73 deletions(-) diff --git a/doc/usage.md b/doc/usage.md index a0e64ec5d..12eb06dd9 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -110,6 +110,7 @@ each code for callers of the `nvx` library. | `template-ineligible` | 1 | The restore window of a template's generation, which the time contract of its capture sets, excludes the host's time, so no clone may restore it now. | | `template-tombstoned` | 1 | A tombstone quarantines the template's generation, which takes no new clones. | | `exited` | 1 | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one. | +| `still-running` | 1 | A stop could not confirm the VM process's exit, so the caller keeps its verified process. | | `exec-refused` | 1 | The guest agent refused the workload, as for a working directory that it cannot use. | | `endpoint-closed` | 1 | OpenVMM closed the control endpoint, as it does when the VM shuts down. | | `protocol` | 1 | The control session failed otherwise, as when the guest breaks the protocol or the session is reset. | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index cd9c6f7ca..3019c6ebc 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -124,7 +124,7 @@ changes only when the module's purpose does. | [`nvx::Hypervisor`](#nvxhypervisor) | KVM, MSHV, or WHP, with its serde form behind the `serde` feature | | [`nvx::exec`](#nvxexec) | The execution engine: the streams, credit, and outcome of an execution's two sides, without I/O, for any framing | | [`nvx::host::runtime`](#nvxhostruntime) | `Runtime`: what every launch shares, the runtime's files and their identity, the hypervisor and its probe, and the launch options | -| [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, and caller-saved reattachment | +| [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, confirmed stop, and caller-saved reattachment | | [`nvx::host::snapshot`](#nvxhostsnapshot) | Snapshot compatibility keys, strict manifests, restore-time eligibility, and the opaque input of a future VM restore | | [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: pure configuration, cold-boot kernel/OpenVMM arguments, portable length checks, and capture identity | | [`nvx::host::restore`](#nvxhostrestore) | A caller-driven restore to Ready without I/O or a clock, and its peer-verified restore-ready listener | @@ -500,6 +500,17 @@ or reading runtime files. `Reattached` holds both the process and authenticated connection. Failures retain their original kinds and sources and leave the saved values unchanged. +`stop(process, outputs, graceful, grace, deadline)` waits through the verified +process, first after the caller's graceful request, then after forced termination +if needed. It returns `Stopped` only after confirmed exit. `self_exited` records +confirmation before a forced attempt, and `forced` records that attempt, not +proof from kill success. An unconfirmed exit is `StillRunning`, and the caller +keeps the process. The callback receives the bounded grace deadline and must +respect it; its boolean result is never exit proof. + +`Stopped::report` retains the outcome-report read's result separately, so report +failure never hides confirmed exit. + `vm::create(&runtime, &config, None, &dir)` creates an inspectable cold `Prepared` launch without starting a process or binding a listener. It requires `config.boot`, refuses a snapshot input, rechecks the directory's privacy, and @@ -976,6 +987,10 @@ On Windows, the duplicate retains the original handle's access rights. Duplication failures have kind `Io` and retain their original I/O source. No watcher thread, future or callback is created by the library. +`Process::wait(deadline)` returns `Some(Exited)` only after verified exit, or +`None` when the deadline passes without confirmation. `Exited` cannot be made +by a caller. + `Process::bytes_read` reports how many bytes the process has read through the operating system's read calls, whether the host's cache served them or not: `rchar` of `/proc//io` on Linux, which it reads only while the process @@ -1422,6 +1437,11 @@ The authenticated control session, which carries NVX's control protocol, `NVXC`, one frame per DATA record of a `HostConnection`; Linux and Windows primitives; and OpenVMM's outcome report. +`report::read` returns `Ok(None)` for a missing report, and an `Io` error for an +unreadable or malformed report, retaining the underlying error. It keeps today's +summary of the outcome category, status code and teardown steps, not free-form +fields. + A `Transport` to an OpenVMM control endpoint reads and writes with deadlines, and tells, through `Transport::unread`, what the endpoint has not read yet of what it wrote, where the platform tells: on Linux, what `SIOCOUTQ` reports of @@ -3055,6 +3075,7 @@ optional cause. | `Unavailable` | A missing or unusable artifact, hypervisor, or release, a guest image without a required control feature, or an unsupported host | | `Unsupported` | A request that NVX refuses: a value that it does not accept, such as a malformed or out-of-range one, options that it cannot combine, or a request that it cannot represent or enforce, such as a policy beyond a limit of OpenVMM or the guest agent | | `Exited` | The VM's OpenVMM process exited | +| `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `Io` | A host I/O failure, or state that NVX recorded on the host and cannot read | | `NotFound` | State that an operation names and that does not exist on the host, such as the store of a sandbox that was never created or that a removal deleted | | `Protocol` | A control session that failed, or a request that the guest agent refused | @@ -3087,6 +3108,7 @@ usage errors, which no library error has. | `unavailable` | `Unavailable` | A dependency is missing or unusable, other than as a finer code says: install metadata, the hypervisor, a release, a distro layer or scratch image that the request names, or a host that NVX does not support | | `unsupported` | `Unsupported` | NVX refuses the request: options that it cannot combine, a value that it does not accept, or a policy that it cannot represent or enforce. The message names the option or field | | `exited` | `Exited` | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one | +| `still-running` | `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `io` | `Io` | A host I/O operation failed, or state that NVX recorded on the host is unreadable or malformed | | `not-found` | `NotFound` | Other state that the operation names does not exist on the host | | `protocol` | `Protocol` | The control session failed otherwise: the peer violated the protocol, reset the session, or failed authentication | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 6a422c570..2d90a5759 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -2445,6 +2447,7 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -3768,6 +3771,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -3780,6 +3787,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 9aa7bf0f8..c1e2bc8cf 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -1031,6 +1033,7 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -2353,6 +2356,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -2365,6 +2372,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index 989538bab..e98ae8830 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -19,6 +19,8 @@ pub enum ErrorKind { Unsupported, /// The OpenVMM process of a VM exited. Exited, + /// The exit of a VM's process could not be confirmed; it may still run. + StillRunning, /// A host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -44,6 +46,7 @@ impl ErrorKind { Self::Unavailable => "unavailable", Self::Unsupported => "unsupported", Self::Exited => "exited", + Self::StillRunning => "still-running", Self::Io => "io", Self::NotFound => "not-found", Self::Protocol => "protocol", @@ -88,6 +91,8 @@ pub enum ErrorCode { Unsupported, /// `exited`: the OpenVMM process of a VM exited, as when a start fails. Exited, + /// `still-running`: the exit of a VM's process could not be confirmed. + StillRunning, /// `io`: a host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -170,6 +175,7 @@ impl ErrorCode { Self::Unavailable, Self::Unsupported, Self::Exited, + Self::StillRunning, Self::Io, Self::NotFound, Self::Protocol, @@ -205,6 +211,7 @@ impl ErrorCode { Self::Unavailable => ErrorKind::Unavailable.as_str(), Self::Unsupported => ErrorKind::Unsupported.as_str(), Self::Exited => ErrorKind::Exited.as_str(), + Self::StillRunning => ErrorKind::StillRunning.as_str(), Self::Io => ErrorKind::Io.as_str(), Self::NotFound => ErrorKind::NotFound.as_str(), Self::Protocol => ErrorKind::Protocol.as_str(), @@ -248,6 +255,7 @@ impl ErrorCode { | Self::ArgumentMissing | Self::ArgumentInvalid => ErrorKind::Unsupported, Self::Exited | Self::SandboxExited => ErrorKind::Exited, + Self::StillRunning => ErrorKind::StillRunning, Self::Io | Self::FormatUnsupported => ErrorKind::Io, Self::NotFound | Self::SandboxNotProvisioned => ErrorKind::NotFound, Self::Protocol | Self::ExecRefused | Self::EndpointClosed => ErrorKind::Protocol, @@ -278,6 +286,7 @@ impl From for ErrorCode { ErrorKind::Unavailable => Self::Unavailable, ErrorKind::Unsupported => Self::Unsupported, ErrorKind::Exited => Self::Exited, + ErrorKind::StillRunning => Self::StillRunning, ErrorKind::Io => Self::Io, ErrorKind::NotFound => Self::NotFound, ErrorKind::Protocol => Self::Protocol, diff --git a/nvx-rs/src/error/tests.rs b/nvx-rs/src/error/tests.rs index eb87d7dc0..a893e72b7 100644 --- a/nvx-rs/src/error/tests.rs +++ b/nvx-rs/src/error/tests.rs @@ -52,6 +52,7 @@ fn every_kind_has_a_code_of_its_own_spelling() { ErrorKind::Unavailable, ErrorKind::Unsupported, ErrorKind::Exited, + ErrorKind::StillRunning, ErrorKind::Io, ErrorKind::NotFound, ErrorKind::Protocol, diff --git a/nvx-rs/src/host/instance/locked.rs b/nvx-rs/src/host/instance/locked.rs index fca2a638b..2cbee7f48 100644 --- a/nvx-rs/src/host/instance/locked.rs +++ b/nvx-rs/src/host/instance/locked.rs @@ -216,7 +216,8 @@ impl LockedInstance { }; Ok(Stopped { forced, - report: report::read(&self.instance.report_path()), + // This legacy wrapper keeps its best-effort report policy. + report: report::read(&self.instance.report_path()).ok().flatten(), }) } diff --git a/nvx-rs/src/host/platform/windows/tests.rs b/nvx-rs/src/host/platform/windows/tests.rs index 44dd7359a..7da84f781 100644 --- a/nvx-rs/src/host/platform/windows/tests.rs +++ b/nvx-rs/src/host/platform/windows/tests.rs @@ -16,7 +16,8 @@ use windows_sys::Win32::Foundation::{ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::JobObjects::CreateJobObjectW; use windows_sys::Win32::System::Threading::{ - CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, TerminateProcess, + CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, + PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, TerminateProcess, }; pub(crate) use super::endpoint::tests::{accept_control, control_server}; @@ -38,6 +39,16 @@ pub(crate) fn wait_exit_handle(process: OwnedHandle, deadline: Instant) -> io::R .map(|exited| exited.expect("Windows process handles always provide an exit event")) } +impl ProcessHandle { + pub(crate) fn wait_only(pid: u32) -> Self { + Self { + process: open_handle(pid, PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE) + .unwrap(), + terminate: false, + } + } +} + thread_local! { /// What a spawn of this thread runs in its window, once its handles are inheritable and /// before it creates its process. @@ -1084,7 +1095,7 @@ fn a_launch_whose_vmm_exited_ends_while_a_process_spawned_in_its_window_holds_it let vmm = launch.record_process(&launched).unwrap(); launched.detach(); assert!( - vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap(), + vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap().is_some(), "the stand-in did not exit" ); // The VMM has exited, so the launch ends, whatever the other process holds. diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index 32047f8d6..100abe93d 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -188,6 +188,12 @@ pub struct Process { handle: platform::ProcessHandle, } +/// Proof that a verified process exited, produced only by [`Process::wait`]. +/// +/// It proves exit, not the process's exit status, and cannot be constructed by a caller. +#[derive(Debug)] +pub struct Exited(()); + impl Process { /// Finds the process that `reference` names, and returns `None` if it has exited. /// @@ -241,13 +247,17 @@ impl Process { }) } - /// Waits until the process exits or `deadline` passes, and returns whether it exited. + /// Waits until the process exits or `deadline` passes, returning exit proof or `None`. /// /// A process exits with its last thread. The wait sleeps until the exit event of the /// process, or with [`WaitMethod::Polling`], checks its identity at a short interval. An /// error means that the wait cannot determine whether the process exited. - pub fn wait(&self, deadline: Instant) -> io::Result { - self.wait_bool(deadline) + pub fn wait(&self, deadline: Instant) -> io::Result> { + Ok(if self.wait_bool(deadline)? { + Some(Exited(())) + } else { + None + }) } /// Keeps the boolean wait result for the legacy lifecycle callers. @@ -262,7 +272,7 @@ impl Process { /// Returns whether the process has exited, without waiting. pub fn has_exited(&self) -> io::Result { - self.wait(Instant::now()) + self.wait(Instant::now()).map(|exited| exited.is_some()) } /// Ends the process at once, unless it has exited or is exiting: with `SIGKILL` on Linux, and diff --git a/nvx-rs/src/host/process/tests.rs b/nvx-rs/src/host/process/tests.rs index ebc305ffa..a90c83102 100644 --- a/nvx-rs/src/host/process/tests.rs +++ b/nvx-rs/src/host/process/tests.rs @@ -275,7 +275,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { let still_running = process.has_exited(); child.kill().unwrap(); child.wait().unwrap(); - assert!(!running.unwrap()); + assert!(running.unwrap().is_none()); assert_eq!(refused.unwrap_err().kind(), io::ErrorKind::Unsupported); assert!(!still_running.unwrap()); // Once the child is reaped, its ID names no process, or another one. @@ -283,6 +283,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } diff --git a/nvx-rs/src/host/process/tests/linux.rs b/nvx-rs/src/host/process/tests/linux.rs index 4c9177b53..a2a0cf72f 100644 --- a/nvx-rs/src/host/process/tests/linux.rs +++ b/nvx-rs/src/host/process/tests/linux.rs @@ -152,13 +152,14 @@ fn a_process_runs_until_its_last_thread_exits() { .unwrap() .expect("a process whose main thread exited still runs"); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(100)) .unwrap() + .is_none() ); child.0.kill().unwrap(); child.0.wait().unwrap(); - assert!(process.wait(Instant::now()).unwrap()); + assert!(process.wait(Instant::now()).unwrap().is_some()); } #[test] diff --git a/nvx-rs/src/host/report.rs b/nvx-rs/src/host/report.rs index 48f86fec4..f07bf2ee4 100644 --- a/nvx-rs/src/host/report.rs +++ b/nvx-rs/src/host/report.rs @@ -9,6 +9,8 @@ use std::path::Path; use serde_json::Value; +use crate::{Error, Result}; + /// Summary of an outcome report. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Report { @@ -20,9 +22,24 @@ pub struct Report { pub teardown_complete: Option, } -/// Reads the report at `path`, or returns `None` if it is missing or not JSON. -pub fn read(path: &Path) -> Option { - let report: Value = serde_json::from_slice(&fs::read(path).ok()?).ok()?; +/// Reads the report at `path`, or returns `Ok(None)` if it is missing. +/// +/// An unreadable or malformed report fails with [`ErrorKind::Io`](crate::ErrorKind::Io), +/// retaining the underlying filesystem or JSON error. +pub fn read(path: &Path) -> Result> { + let bytes = match fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => { + return Err( + Error::io(format!("cannot read outcome report {}", path.display())) + .with_source(error), + ); + } + }; + let report: Value = serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse outcome report {}", path.display())).with_source(error) + })?; let mut summary = Report::default(); if let Some(outcome) = report.get("outcome") { summary.category = outcome @@ -35,7 +52,7 @@ pub fn read(path: &Path) -> Option { let complete = teardown.values().all(|step| step.as_bool() == Some(true)); summary.teardown_complete = Some(complete); } - Some(summary) + Ok(Some(summary)) } #[cfg(test)] diff --git a/nvx-rs/src/host/report/tests.rs b/nvx-rs/src/host/report/tests.rs index 127fd5b2e..961377aa4 100644 --- a/nvx-rs/src/host/report/tests.rs +++ b/nvx-rs/src/host/report/tests.rs @@ -11,7 +11,7 @@ fn reports_are_summarized_without_free_form_fields() { ) .unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { category: Some("success".to_owned()), status_code: Some(0), @@ -20,13 +20,60 @@ fn reports_are_summarized_without_free_form_fields() { ); fs::write(&path, r#"{"teardown":{"workers":true,"memory":false}}"#).unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { teardown_complete: Some(false), ..Report::default() } ); - assert!(read(&directory.path().join("missing.json")).is_none()); +} + +#[test] +fn missing_reports_are_distinct_from_malformed_or_unreadable_reports() { + let directory = tempfile::tempdir().unwrap(); + assert!( + read(&directory.path().join("missing.json")) + .unwrap() + .is_none() + ); + let path = directory.path().join("outcome.json"); fs::write(&path, b"{ truncated").unwrap(); - assert!(read(&path).is_none()); + let malformed = read(&path).unwrap_err(); + assert_eq!(malformed.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&malformed) + .unwrap() + .is::() + ); + let unreadable = read(directory.path()).unwrap_err(); + assert_eq!(unreadable.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&unreadable) + .unwrap() + .is::() + ); +} + +#[cfg(windows)] +#[test] +fn a_report_opened_without_read_sharing_returns_its_io_error() { + use std::os::windows::fs::OpenOptionsExt; + + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("outcome.json"); + fs::write(&path, b"{}").unwrap(); + let held = fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(&path) + .unwrap(); + let error = read(&path).unwrap_err(); + assert_eq!(error.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&error) + .unwrap() + .is::() + ); + drop(held); + assert_eq!(read(&path).unwrap(), Some(Report::default())); } diff --git a/nvx-rs/src/host/template/clone/records.rs b/nvx-rs/src/host/template/clone/records.rs index ce9752576..dc5fbc696 100644 --- a/nvx-rs/src/host/template/clone/records.rs +++ b/nvx-rs/src/host/template/clone/records.rs @@ -296,7 +296,7 @@ impl Recorder { /// report only once the clone's VM ended, or its teardown did. pub(super) fn observed(&self) -> Option<&Report> { self.observed - .get_or_init(|| report::read(&self.report)) + .get_or_init(|| report::read(&self.report).ok().flatten()) .as_ref() } @@ -1119,7 +1119,7 @@ impl CloneRecords { // The launch removed the report of an earlier VM of the sandbox before it wrote the first // of the clone's records, which a progress past the plan shows. let report = (progress > Progress::Planned) - .then(|| report::read(&sandbox.outcome_path())) + .then(|| report::read(&sandbox.outcome_path()).ok().flatten()) .flatten(); Ok(Some(Self { lease, diff --git a/nvx-rs/src/host/template/clone/tests/launch.rs b/nvx-rs/src/host/template/clone/tests/launch.rs index c58043a4c..7ec87eec6 100644 --- a/nvx-rs/src/host/template/clone/tests/launch.rs +++ b/nvx-rs/src/host/template/clone/tests/launch.rs @@ -365,14 +365,14 @@ impl VmmDir { .unwrap() .expect("the VMM runs until the test ends it"); self.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } /// Waits until the VMM, which exits on its own, as its [plan](Self::plan) asks, has exited. pub(super) fn wait_exit(&self) { let reference = self.reference.as_ref().expect("the test knows the VMM"); if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } @@ -413,7 +413,9 @@ impl Drop for VmmDir { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -1567,7 +1569,7 @@ fn a_clone_whose_launch_failed_before_it_began_launches_no_more_until_its_releas // clone's: an inspection takes none, and the launch removes it before it records anything. let report = acquired.locked.outcome_path(); fs::write(&report, guest_exit(195)).unwrap(); - assert!(crate::host::report::read(&report).is_some()); + assert!(crate::host::report::read(&report).unwrap().is_some()); let records = CloneRecords::read(&acquired.locked).unwrap().unwrap(); assert_eq!( (records.progress, records.report), @@ -2323,7 +2325,7 @@ fn a_launch_whose_caller_crashes_once_its_vmm_started_ends_the_vmm_as_reattach_r } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(!exists(&crashed.socket())); crashed.releases(&mut locked); } @@ -2356,7 +2358,10 @@ fn a_restore_whose_caller_crashes_before_its_commit_ends_its_vmm_as_reattach_rec } => assert_eq!(terminated, reference, "{step}"), other => panic!("unexpected reattach at {step}: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap(), "{step}"); + assert!( + process.wait(Instant::now() + TIMEOUT).unwrap().is_some(), + "{step}" + ); assert!(!exists(&crashed.socket()), "{step}"); crashed.releases(&mut locked); } diff --git a/nvx-rs/src/host/template/clone/tests/restore.rs b/nvx-rs/src/host/template/clone/tests/restore.rs index 0a33cb1dd..61282c561 100644 --- a/nvx-rs/src/host/template/clone/tests/restore.rs +++ b/nvx-rs/src/host/template/clone/tests/restore.rs @@ -422,7 +422,7 @@ fn vmm_with(plan: &str) -> VmmDir { fn assert_vmm_ended(vmm: &VmmDir) { let reference = vmm.reference.as_ref().unwrap(); if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 1205e4ba3..eaeed151c 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -2460,7 +2460,9 @@ impl Drop for VmmDir { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -2639,7 +2641,7 @@ fn a_clone_that_crashes_after_its_spawn_keeps_its_lease_until_its_unrecorded_vmm crashed.refuses_release(&mut locked); // Once the VMM exits, the claim that it released proves it. crashed.vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -2669,7 +2671,7 @@ fn a_clone_that_crashes_after_recording_its_vmm_keeps_its_lease_until_reattach_e } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); crashed.releases(&mut locked); } diff --git a/nvx-rs/src/host/vm.rs b/nvx-rs/src/host/vm.rs index 1429a7a26..72e724aa6 100644 --- a/nvx-rs/src/host/vm.rs +++ b/nvx-rs/src/host/vm.rs @@ -16,6 +16,7 @@ use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; +use std::time::Instant; use crate::host::config::{ConsoleAttachment, RootConfig, UpperConfig, VmConfig}; use crate::host::filesystem; @@ -25,6 +26,7 @@ use crate::host::platform; use crate::host::platform::{check_store_dir as check_dir, create_store_dir as create_dir}; #[cfg(windows)] use crate::host::platform::{check_vm_dir as check_dir, create_vm_dir as create_dir}; +use crate::host::report::{self, Report}; use crate::host::runtime::{Runtime, RuntimeIdentity}; use crate::host::snapshot::Snapshot; use crate::host::spawn::{self, VmmCommand}; @@ -522,6 +524,78 @@ pub fn spawn(prepared: Prepared) -> Result { spawn_verified(prepared, spawn::Launched::process) } +/// A confirmed VM exit, with the outcome-report read kept separate from exit proof. +#[derive(Debug)] +pub struct Stopped { + /// Exit was confirmed before any forced termination was attempted. + pub self_exited: bool, + /// Forced termination through the verified process was attempted. + pub forced: bool, + /// The outcome summary, missing report, or report-read error after confirmed exit. + pub report: Result>, +} + +/// Stops a VM, returning only after its verified process has exited. +/// +/// Unless exit is already confirmed, calls `graceful` with `grace`, bounded by `deadline`, +/// then waits until that grace deadline. The callback must respect the supplied deadline; +/// its boolean request result is not exit proof. If exit is still unconfirmed, attempts +/// forced termination and waits until `deadline`. A failed termination is harmless only +/// when that final wait confirms exit. +/// +/// An unconfirmed exit fails with [`ErrorKind::StillRunning`](crate::ErrorKind::StillRunning), +/// retaining host errors. The caller keeps the process in that case. +/// Report-read errors have kind [`ErrorKind::Io`](crate::ErrorKind::Io) +/// inside [`Stopped::report`], never hiding confirmed exit. +pub fn stop( + process: &Process, + outputs: &VmOutputs, + graceful: impl FnOnce(Instant) -> bool, + grace: Instant, + deadline: Instant, +) -> Result { + let failure = |kill_error: Option<&io::Error>| { + let mut message = format!( + "cannot confirm exit of VM process {}", + process.reference().pid + ); + if let Some(error) = kill_error { + message.push_str(&format!("; forced termination failed: {error}")); + } + Error::new(crate::ErrorKind::StillRunning, message) + }; + let wait = |until| { + process + .wait(until) + .map_err(|error| failure(None).with_source(error)) + }; + let self_exited = if wait(Instant::now())?.is_some() { + true + } else { + let grace = grace.min(deadline); + graceful(grace); + wait(grace)?.is_some() + }; + if !self_exited { + let kill_error = process.kill().err(); + let exited = process + .wait(deadline) + .map_err(|error| failure(kill_error.as_ref()).with_source(error))?; + if exited.is_none() { + let error = failure(kill_error.as_ref()); + return Err(match kill_error { + Some(source) => error.with_source(source), + None => error, + }); + } + } + Ok(Stopped { + self_exited, + forced: !self_exited, + report: report::read(&outputs.outcome), + }) +} + /// Keeps the child owned until its process identity is verified, or its exit is confirmed. fn spawn_verified( prepared: Prepared, diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index 05af49bd0..b637a7532 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -1098,6 +1098,7 @@ fn spawn_caller_helper() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); panic!("cannot record the helper's child: {error}"); } @@ -1181,6 +1182,7 @@ fn an_unrelated_process_started_in_the_library_spawn_window_gets_no_vm_claim() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert!( platform::tests::claim_released_within(&log, identity, Duration::from_secs(10)), @@ -1282,10 +1284,193 @@ fn current_cold_spawn_boots_a_guest_accepts_shell_input_and_exits_cleanly() { ) .unwrap(); read_until(&mut *console, b"NVX-SPAWN-COLD-OK"); - assert!(launched.0.process.wait(deadline).unwrap()); + assert!(launched.0.process.wait(deadline).unwrap().is_some()); assert!(reopened.has_exited().unwrap()); - let outcome = crate::host::report::read(&launched.0.outputs.outcome).unwrap(); + let outcome = crate::host::report::read(&launched.0.outputs.outcome) + .unwrap() + .unwrap(); assert_eq!(outcome.category.as_deref(), Some("success")); assert_eq!(outcome.status_code, Some(0)); println!("NVX-LIBRARY-COLD-SPAWN-EXIT-OK: {}", reference.pid); } + +#[cfg(any(target_os = "linux", windows))] +struct StopChild { + child: std::process::Child, + process: Process, +} + +#[cfg(any(target_os = "linux", windows))] +impl StopChild { + fn start() -> Self { + use std::io::{BufRead, BufReader}; + + #[cfg(windows)] + let mut command = { + let mut command = Command::new( + PathBuf::from(std::env::var_os("SystemRoot").unwrap()).join("System32\\cmd.exe"), + ); + command.args(["/d", "/q", "/c", "echo ready & set /p wait="]); + command + }; + #[cfg(target_os = "linux")] + let mut command = { + let mut command = Command::new("sh"); + command.args(["-c", "printf 'ready\\n'; read -r wait"]); + command + }; + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + let mut child = crate::host::tests::spawn_locked(&mut command).unwrap(); + let mut ready = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut ready) + .unwrap(); + assert_eq!(ready.trim_end(), "ready"); + let reference = crate::host::process::ProcessRef { + pid: child.id(), + start_time: platform::process_start_time(child.id()).unwrap().unwrap(), + boot: crate::host::process::BootId::current().unwrap(), + }; + let process = Process::open(&reference).unwrap().unwrap(); + Self { child, process } + } +} + +#[cfg(any(target_os = "linux", windows))] +impl Drop for StopChild { + fn drop(&mut self) { + if self.child.try_wait().unwrap().is_none() { + self.child.kill().unwrap(); + } + self.child.wait().unwrap(); + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_preserves_report_results_after_a_real_self_exit() { + for report in ["missing", "valid", "malformed", "unreadable"] { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + match report { + "missing" => {} + "valid" => fs::write( + &outputs.outcome, + br#"{"outcome":{"category":"success","status_code":0},"teardown":{"workers":true}}"#, + ).unwrap(), + "malformed" => fs::write(&outputs.outcome, b"{ truncated").unwrap(), + "unreadable" => fs::create_dir(&outputs.outcome).unwrap(), + _ => unreachable!(), + } + let mut child = StopChild::start(); + let deadline = Instant::now() + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, deadline); + drop(child.child.stdin.take()); + false + }, + deadline, + deadline, + ) + .unwrap(); + assert!(stopped.self_exited); + assert!(!stopped.forced); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + let already_exited = stop( + &child.process, + &outputs, + |_| panic!("an exited process needs no graceful request"), + Instant::now(), + Instant::now(), + ) + .unwrap(); + assert!(already_exited.self_exited); + assert!(!already_exited.forced); + match report { + "missing" => assert!(stopped.report.unwrap().is_none()), + "valid" => assert_eq!( + stopped.report.unwrap(), + Some(Report { + category: Some("success".to_owned()), + status_code: Some(0), + teardown_complete: Some(true), + }) + ), + _ => { + let error = stopped.report.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(std::error::Error::source(&error).is_some()); + } + } + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_forces_only_its_verified_process_after_grace() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + let mut child = StopChild::start(); + let mut other = StopChild::start(); + let grace = Instant::now() + Duration::from_millis(80); + let deadline = grace + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, grace); + true + }, + grace, + deadline, + ) + .unwrap(); + assert!(!stopped.self_exited); + assert!(stopped.forced); + assert!(stopped.report.unwrap().is_none()); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + assert!(other.process.wait(Instant::now()).unwrap().is_none()); + assert!(other.child.try_wait().unwrap().is_none()); +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_returns_still_running_without_exit_confirmation_at_its_deadline() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + fs::write(&outputs.outcome, b"{ truncated").unwrap(); + let mut child = StopChild::start(); + #[cfg(target_os = "linux")] + let handle = platform::ProcessHandle::without_pidfd(); + #[cfg(windows)] + let handle = platform::ProcessHandle::wait_only(child.child.id()); + let process = Process::held(child.process.reference().clone(), handle); + let grace = Instant::now(); + let deadline = grace + Duration::from_millis(80); + let error = stop(&process, &outputs, |_| false, grace, deadline).unwrap_err(); + assert!(Instant::now() >= deadline); + assert_eq!(error.kind(), ErrorKind::StillRunning); + assert_eq!(error.code().as_str(), "still-running"); + let cause = std::error::Error::source(&error) + .unwrap() + .downcast_ref::() + .unwrap(); + assert_eq!( + cause.kind(), + if cfg!(windows) { + io::ErrorKind::PermissionDenied + } else { + io::ErrorKind::Unsupported + } + ); + assert!(process.wait(Instant::now()).unwrap().is_none()); + assert!(child.child.try_wait().unwrap().is_none()); +} diff --git a/nvx-rs/src/host/vm/tests/reattach.rs b/nvx-rs/src/host/vm/tests/reattach.rs index 88a95d005..0611675b6 100644 --- a/nvx-rs/src/host/vm/tests/reattach.rs +++ b/nvx-rs/src/host/vm/tests/reattach.rs @@ -244,7 +244,7 @@ fn reattaching_returns_the_held_owned_child_and_authenticated_connection() { b"response" ); owner.wait(); - assert!(attached.process.wait(deadline).unwrap()); + assert!(attached.process.wait(deadline).unwrap().is_some()); } #[test] diff --git a/nvx-rs/tests/checked_handoff.rs b/nvx-rs/tests/checked_handoff.rs index b07a9e340..8f1c7b07b 100644 --- a/nvx-rs/tests/checked_handoff.rs +++ b/nvx-rs/tests/checked_handoff.rs @@ -703,6 +703,7 @@ fn a_prepared_launch_hands_the_vmm_its_checked_images_alone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); let scratch = fs::read_to_string(staged.scratch()).unwrap(); @@ -788,6 +789,7 @@ fn a_vmm_keeps_the_scratch_images_claim_once_its_launcher_is_gone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); drop(check_written().unwrap()); diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index f3b64de5b..01191d000 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -1007,9 +1007,10 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n ); assert_eq!(runtime_record(&root, "state"), None); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "{field}" ); process.kill().unwrap(); @@ -1017,6 +1018,7 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } assert_eq!(transition(&root, "deprovision", "state", &[]), succeeded()); @@ -1153,6 +1155,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "deprovision", "unanswered", &[]), @@ -1173,6 +1176,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(30)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "stop", "slow", &[]), @@ -1194,6 +1198,7 @@ fn a_sandbox_whose_openvmm_exited_reports_it_once_and_starts_again() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); }; crash(); diff --git a/nvx-rs/tests/cli/download.rs b/nvx-rs/tests/cli/download.rs index be959f532..f9d758cea 100644 --- a/nvx-rs/tests/cli/download.rs +++ b/nvx-rs/tests/cli/download.rs @@ -171,6 +171,22 @@ fn provision_and_deprovision_need_no_artifact_root() { let executable = elsewhere .path() .join(Path::new(NVX_RS).file_name().unwrap()); + #[cfg(unix)] + { + // Concurrent forks of this test process must inherit no writable executable. + let copied = Command::new("cp") + .arg("-p") + .arg(NVX_RS) + .arg(&executable) + .output() + .unwrap(); + assert!( + copied.status.success(), + "cannot copy the executable: {}", + String::from_utf8_lossy(&copied.stderr) + ); + } + #[cfg(windows)] fs::copy(NVX_RS, &executable).unwrap(); let nvx_rs = |arguments: &[String]| { let output = Command::new(&executable) diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index 7078aba9e..d82a0f548 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -604,7 +604,8 @@ fn check_lifecycle(case: &Configuration) { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -694,7 +695,8 @@ fn lifecycle_of_a_sandbox_whose_upper_is_in_ram() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -737,7 +739,8 @@ fn stops_keep_an_outcome_report_that_the_caller_reads_whatever_its_umask() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -1437,9 +1440,10 @@ fn recycled_process_id(root: &Root) { assert_refused(root, &outcome, "sandbox-exited", &["stale", "not running"]); assert!(!path.exists()); assert!( - !openvmm + openvmm .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "the stale record's process was signaled" ); openvmm.kill().unwrap(); @@ -1447,6 +1451,7 @@ fn recycled_process_id(root: &Root) { openvmm .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); let outcome = operate(root, "stop", "state", &[]); assert_refused(root, &outcome, "sandbox-not-running", &["not running"]); diff --git a/nvx-rs/tests/fake_openvmm.rs b/nvx-rs/tests/fake_openvmm.rs index c667dbe84..b112f951b 100644 --- a/nvx-rs/tests/fake_openvmm.rs +++ b/nvx-rs/tests/fake_openvmm.rs @@ -478,7 +478,7 @@ fn outcome_reports_of_another_runtime_are_never_read() { .unwrap(); if exits_first { process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } let stopped = locked.terminate(mismatched, TIMEOUT).unwrap(); assert_eq!( @@ -528,11 +528,12 @@ fn records_that_name_another_process_never_end_it() { drop(locked); assert!(records_nothing(&instance)); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) .unwrap() + .is_none() ); process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } diff --git a/nvx-rs/tests/store.rs b/nvx-rs/tests/store.rs index 39d55d2e4..9602cce68 100644 --- a/nvx-rs/tests/store.rs +++ b/nvx-rs/tests/store.rs @@ -183,7 +183,9 @@ fn kill(pid: u32, start_time: u64) -> io::Result { return Ok(true); }; process.kill()?; - process.wait(Instant::now() + RECOVERY_TIMEOUT) + process + .wait(Instant::now() + RECOVERY_TIMEOUT) + .map(|exited| exited.is_some()) } /// A VMM that a crashed orchestrator left behind, which is killed when the test ends if it still @@ -533,7 +535,12 @@ fn a_committed_launch_names_its_vm_for_another_build() { .expect("the recorded VMM runs"); assert!(!reattached.has_exited().unwrap()); drop(vmm); - assert!(reattached.wait(Instant::now() + RECOVERY_TIMEOUT).unwrap()); + assert!( + reattached + .wait(Instant::now() + RECOVERY_TIMEOUT) + .unwrap() + .is_some() + ); other.lock(None).unwrap().clear_runtime().unwrap(); assert!(other.runtime().unwrap().is_none()); } diff --git a/nvx-rs/tests/supervisor.rs b/nvx-rs/tests/supervisor.rs index be2fb01a0..45b2058ab 100644 --- a/nvx-rs/tests/supervisor.rs +++ b/nvx-rs/tests/supervisor.rs @@ -450,7 +450,9 @@ impl Drop for VmmDirectory { let _ = File::create(self.path().join(EXIT_MARKER)); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -613,7 +615,13 @@ fn long_stores_launch_with_a_short_working_directory() { let mut vmm = VmmDirectory::new(); let mut locked = store.lock(None).unwrap(); let running = launch_vmm(&mut locked, &mut vmm); - assert!(!running.process().wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!( + running + .process() + .wait(Instant::now() + SHORT_WAIT) + .unwrap() + .is_none() + ); let stopped = running .stop(&mut locked, Instant::now() + TIMEOUT, None, |_, _| { vmm.end(); @@ -671,15 +679,15 @@ fn a_child_that_exits_wakes_its_waiter() { assert_eq!(process.reference().boot, BootId::current().unwrap()); assert_eq!(process.wait_method(), WaitMethod::Event); assert!(!process.has_exited().unwrap()); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); // The reference finds the same process through a handle of its own. let found = open_running(process.reference()); assert_eq!(found.reference(), process.reference()); let started = Instant::now(); vmm.end(); - assert!(process.wait(started + TIMEOUT).unwrap()); - assert!(found.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); + assert!(found.wait(started + TIMEOUT).unwrap().is_some()); assert!(started.elapsed() < TIMEOUT); assert_ended(&process); // Ending a process that exited does nothing. @@ -698,12 +706,12 @@ fn a_process_that_is_not_a_child_wakes_its_waiters() { launcher.finish(); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); let waiters: Vec = thread::scope(|scope| { let waiters: Vec<_> = (0..2) - .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap())) + .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap().is_some())) .collect(); vmm.end(); waiters @@ -740,10 +748,10 @@ fn a_restarted_launcher_finds_its_vmm_again_and_ends_it() { vmm.record(&reference); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); process.kill().unwrap(); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); } @@ -783,7 +791,7 @@ fn a_reused_process_id_is_never_signaled() { assert!(!process.has_exited().unwrap()); open_running(&reference); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -814,7 +822,7 @@ fn references_that_name_no_process_are_never_opened() { let launched = spawn_vmm(vmm.path(), false); let process = launched.process().unwrap(); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); assert!(Process::open(process.reference()).unwrap().is_none()); assert!(process.has_exited().unwrap()); @@ -877,7 +885,7 @@ fn an_unrecorded_vmm_that_serves_no_endpoint_is_never_signaled() { assert!(store.journal().unwrap().is_some()); // Once the VMM exits, the claim that it released proves it. vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -1264,7 +1272,7 @@ fn check_vmm(reader: PipeReader, writer: &PipeWriter, others: &Mutex> } } vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -1493,7 +1501,7 @@ fn a_vmm_in_its_orchestrators_job_ends_with_it() { // The VMM did not break away, so closing the job ends it with its orchestrator. drop(job); orchestrator.wait(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); match supervisor::reattach(&mut locked, &runtime()).unwrap() { Reattached::Exited { @@ -1517,10 +1525,10 @@ fn a_job_that_ends_its_vmm_wakes_the_waiters() { vmm.record(&reference); // The VMM did not break away, so the job holds it too, and closing the job ends it. let process = open_running(&reference); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); drop(job); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); launcher.wait(); } @@ -1587,7 +1595,7 @@ fn a_process_that_this_user_may_not_terminate_is_waited_for_but_never_signaled() // Its creator ends it, which wakes the waiter, and ending a process that exited does nothing. child.0.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); process.kill().unwrap(); assert_ended(&process); } diff --git a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs index 449ceb1f0..844c9a4f0 100644 --- a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs +++ b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs @@ -2053,6 +2053,7 @@ fn launch_failed( let exited = starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?; starting.abandon().map_err(|error| describe(&error))?; println!("stopped={}", if exited { "graceful" } else { "forced" }); @@ -2078,6 +2079,7 @@ fn table_failed( Ok(_) => starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?, Err(_) => false, }; @@ -3625,12 +3627,14 @@ fn hold(locked: &mut LockedStore, runtime: &Runtime) -> Result Result<(), String> { - let outcome = report::read(&store.outcome_path()).ok_or_else(|| { - format!( - "the VM left no outcome report {}", - store.outcome_path().display() - ) - })?; + let outcome = report::read(&store.outcome_path()) + .map_err(|error| error.to_string())? + .ok_or_else(|| { + format!( + "the VM left no outcome report {}", + store.outcome_path().display() + ) + })?; print_outcome(&outcome); if ( outcome.category.as_deref(), diff --git a/tests/runtime-adapter/tests/vm_clone.rs b/tests/runtime-adapter/tests/vm_clone.rs index 654fad486..cbc20e665 100644 --- a/tests/runtime-adapter/tests/vm_clone.rs +++ b/tests/runtime-adapter/tests/vm_clone.rs @@ -2321,7 +2321,7 @@ impl CloneControl for Early { .send(&self.early, Some(deadline)) .map_err(io::Error::other)?; let answer = session.receive(Some(deadline)); - self.exited = self.vmm.wait(deadline)?; + self.exited = self.vmm.wait(deadline)?.is_some(); Err(match answer { Ok(message) => io::Error::other(format!("the clone answered {message:?}")), Err(error) => io::Error::other(error), @@ -3910,7 +3910,9 @@ fn a_host_program_of_another_runtime_ends_the_clone_through_its_verified_identit assert!(!ended.contains_key("epoch"), "{ended:?}"); assert!(sandbox.stderr().contains("differs in initramfs")); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); assert!(sandbox.record().is_none()); @@ -3948,7 +3950,9 @@ fn a_clone_whose_vm_exits_once_a_stop_found_it_fails_before_its_lease_is_release assert_eq!(sandbox.paused(&mut stopping, &pause), "found"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); fs::remove_file(&pause).unwrap(); @@ -5313,7 +5317,9 @@ fn a_run_whose_clone_s_vm_exits_during_its_execution_ends_with_the_clone_s_failu let vmm = sandbox.vmm().expect("the clone's VMM runs"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); From 99d004572b8ffad55c48bd7885278457297cbda6 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 10 Nov 2026 01:11:03 -0800 Subject: [PATCH 2/5] host: journal CLI launches and persist saved VM records Port the scoped SL3.2b donor cf485b72 onto verified feature c52c7101 without its historical producer history. Persist prepared capability and outputs before spawn, the matching process before commit, and every SavedVm field in the private runtime record. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eb3cc7e0-f51e-45ed-b911-c7b214740e05 --- nvx-rs/src/host/cli/store/files.rs | 51 +++ nvx-rs/src/host/cli/store/launch.rs | 80 +++++ nvx-rs/src/host/cli/store/mod.rs | 7 + nvx-rs/src/host/cli/store/record.rs | 159 +++++++++ nvx-rs/src/host/cli/store/tests.rs | 2 + nvx-rs/src/host/cli/store/tests/launch.rs | 382 ++++++++++++++++++++++ 6 files changed, 681 insertions(+) create mode 100644 nvx-rs/src/host/cli/store/files.rs create mode 100644 nvx-rs/src/host/cli/store/launch.rs create mode 100644 nvx-rs/src/host/cli/store/record.rs create mode 100644 nvx-rs/src/host/cli/store/tests/launch.rs diff --git a/nvx-rs/src/host/cli/store/files.rs b/nvx-rs/src/host/cli/store/files.rs new file mode 100644 index 000000000..9050cf281 --- /dev/null +++ b/nvx-rs/src/host/cli/store/files.rs @@ -0,0 +1,51 @@ +//! Private, regular, non-link files, atomically replaced and synced under the state lock. + +use std::fs; +use std::io; + +use super::StateFile; +use super::locked::LockedState; +use crate::host::platform; +use crate::host::store::{files, regular}; +use crate::{Error, Result}; + +/// Bounds a fixed record's paths and identities without reading an unbounded corrupt file. +const MAX_RECORD_BYTES: u64 = 1024 * 1024; + +pub(super) fn read(state: &LockedState, name: StateFile) -> Result>> { + let path = state.path(name); + let failed = |error| Error::io(format!("cannot read {}", path.display())).with_source(error); + let file = match regular::open_regular(&path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(failed(error)), + }; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(Error::io(format!( + "the sandbox file {} {problem}", + path.display() + ))); + } + regular::read_at_most(file, MAX_RECORD_BYTES, "a sandbox record") + .map(Some) + .map_err(failed) +} + +pub(super) fn replace(state: &LockedState, name: StateFile, bytes: &[u8]) -> Result<()> { + if bytes.len() as u64 > MAX_RECORD_BYTES { + return Err(Error::io("the sandbox record exceeds its size bound")); + } + // Refuse an existing unsafe entry rather than replacing it or following its target. + read(state, name)?; + files::write_private(&state.path(name), bytes)?; + files::sync_dir(state.dir()) +} + +pub(super) fn remove(state: &LockedState, name: StateFile) -> Result<()> { + read(state, name)?; + let path = state.path(name); + fs::remove_file(&path).map_err(|error| { + Error::io(format!("cannot remove {}", path.display())).with_source(error) + })?; + files::sync_dir(state.dir()) +} diff --git a/nvx-rs/src/host/cli/store/launch.rs b/nvx-rs/src/host/cli/store/launch.rs new file mode 100644 index 000000000..d0a5c04e6 --- /dev/null +++ b/nvx-rs/src/host/cli/store/launch.rs @@ -0,0 +1,80 @@ +//! Locked launch transitions, without spawning, recovery, or process termination. + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use super::record::{LaunchJournal, RuntimeRecord}; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm}; +use crate::{Error, Result}; + +/// A launch whose lock stays borrowed until commit or until its journal is left behind. +#[derive(Debug)] +pub(crate) struct Launch<'a> { + locked: &'a mut LockedState, + journal: LaunchJournal, +} + +impl LockedState { + /// Durably publishes the capability and complete prepared journal before the caller spawns. + pub(crate) fn begin_launch( + &mut self, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result> { + if self.journal()?.is_some() || self.runtime()?.is_some() { + return Err(Error::other( + "the sandbox already has a runtime record or an interrupted launch", + )); + } + let journal = LaunchJournal::prepared(self, prepared, runtime)?; + files::replace(self, StateFile::Capability, &journal.capability)?; + self.write_record(StateFile::Journal, &journal)?; + Ok(Launch { + locked: self, + journal, + }) + } +} + +impl Launch<'_> { + /// Records only the caller's spawned process, durably, before changing the in-memory state. + pub(crate) fn record_process(&mut self, process: &ProcessRef) -> Result<()> { + if self.journal.process.is_some() { + return Err(Error::other("the launch has recorded its process already")); + } + let journal = LaunchJournal { + process: Some(process.clone()), + ..self.journal.clone() + }; + self.locked.write_record(StateFile::Journal, &journal)?; + self.journal = journal; + Ok(()) + } + + /// Publishes the exact saved VM, only after its matching process was journaled. + /// + /// The runtime record is the commit. A failed journal removal keeps that committed record + /// and reports the failure; it never rolls back the commit or signals the VM. + pub(crate) fn commit(self, saved: &SavedVm) -> Result<()> { + if self.journal.process.as_ref() != Some(&saved.process) + || self.journal.vm_dir != saved.vm_dir.path() + || self.journal.endpoint != saved.endpoint + || self.journal.capability != saved.capability + || self.journal.runtime != saved.runtime + { + return Err(Error::other( + "the saved VM does not match the launch and its journaled process", + )); + } + self.locked + .write_record(StateFile::Runtime, &RuntimeRecord::from(saved))?; + files::remove(self.locked, StateFile::Journal).map_err(|error| { + Error::io(format!( + "the VM runtime was committed but its launch journal could not be removed: {error}" + )) + .with_source(error) + }) + } +} diff --git a/nvx-rs/src/host/cli/store/mod.rs b/nvx-rs/src/host/cli/store/mod.rs index 6f0c6479f..16bcb2432 100644 --- a/nvx-rs/src/host/cli/store/mod.rs +++ b/nvx-rs/src/host/cli/store/mod.rs @@ -26,6 +26,10 @@ //! and the Administrators group, as one under the user's profile does. An existing directory that //! others may access is refused rather than restricted, since they could have placed files in it, //! or still hold them open. The store creates its files readable only by the current user. +//! +//! A locked launch writes its prepared capability and outputs before spawn, records the spawned +//! process durably, and publishes the caller's exact saved VM only at commit. Dropping an +//! uncommitted launch keeps its journal; these transitions do not recover or signal a process. #![cfg_attr( not(test), expect( @@ -34,8 +38,11 @@ ) )] +mod files; +mod launch; mod locked; mod names; +mod record; use std::io; use std::path::{Path, PathBuf}; diff --git a/nvx-rs/src/host/cli/store/record.rs b/nvx-rs/src/host/cli/store/record.rs new file mode 100644 index 000000000..d84718801 --- /dev/null +++ b/nvx-rs/src/host/cli/store/record.rs @@ -0,0 +1,159 @@ +//! The fixed launch journal and the caller-saved VM's runtime record. + +use std::fmt; +use std::path::PathBuf; + +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm, VmDir, VmOutputs}; +use crate::protocol::CAPABILITY_LEN; +use crate::{Error, Result}; + +#[derive(Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(crate) struct LaunchJournal { + pub(super) vm_dir: PathBuf, + pub(super) capability: [u8; CAPABILITY_LEN], + pub(super) log: PathBuf, + pub(super) outcome: PathBuf, + pub(super) endpoint: String, + pub(super) console: Option, + pub(super) runtime: RuntimeIdentity, + pub(super) process: Option, +} + +impl fmt::Debug for LaunchJournal { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("LaunchJournal") + .field("vm_dir", &self.vm_dir) + .field("log", &self.log) + .field("outcome", &self.outcome) + .field("endpoint", &self.endpoint) + .field("console", &self.console) + .field("runtime", &self.runtime) + .field("process", &self.process) + .finish_non_exhaustive() + } +} + +impl LaunchJournal { + pub(super) fn prepared( + state: &LockedState, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result { + runtime.check()?; + let vm_dir = VmDir::open(state.dir())?; + let VmOutputs { + log, + outcome, + endpoint, + console, + } = prepared.outputs(); + if *log != vm_dir.path().join(StateFile::Log.name()) + || *outcome != vm_dir.path().join(StateFile::Outcome.name()) + { + return Err(Error::other( + "the prepared launch belongs to another sandbox directory", + )); + } + Ok(Self { + vm_dir: vm_dir.path().to_path_buf(), + capability: *prepared.capability().as_bytes(), + log: log.clone(), + outcome: outcome.clone(), + endpoint: endpoint.clone(), + console: console.clone(), + runtime: runtime.clone(), + process: None, + }) + } + + pub(crate) fn process(&self) -> Option<&ProcessRef> { + self.process.as_ref() + } +} + +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(super) struct RuntimeRecord { + vm_dir: PathBuf, + endpoint: String, + capability: [u8; CAPABILITY_LEN], + process: ProcessRef, + runtime: RuntimeIdentity, +} + +impl From<&SavedVm> for RuntimeRecord { + fn from(saved: &SavedVm) -> Self { + Self { + vm_dir: saved.vm_dir.path().to_path_buf(), + endpoint: saved.endpoint.clone(), + capability: saved.capability, + process: saved.process.clone(), + runtime: saved.runtime.clone(), + } + } +} + +impl LockedState { + pub(crate) fn journal(&self) -> Result> { + self.read_record(StateFile::Journal) + } + + pub(crate) fn runtime(&self) -> Result> { + let Some(record) = self.read_record::(StateFile::Runtime)? else { + return Ok(None); + }; + let vm_dir = VmDir::open(self.dir())?; + if record.vm_dir != vm_dir.path() { + return Err(Error::io( + "the runtime record belongs to another sandbox directory", + )); + } + Ok(Some(SavedVm { + vm_dir, + endpoint: record.endpoint, + capability: record.capability, + process: record.process, + runtime: record.runtime, + })) + } + + pub(crate) fn capability(&self) -> Result> { + files::read(self, StateFile::Capability)? + .map(|bytes| { + bytes.try_into().map_err(|_| { + Error::io(format!( + "the sandbox capability must hold {CAPABILITY_LEN} bytes" + )) + }) + }) + .transpose() + } + + fn read_record(&self, name: StateFile) -> Result> { + files::read(self, name)? + .map(|bytes| { + serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse {}", self.path(name).display())) + .with_source(error) + }) + }) + .transpose() + } + + pub(super) fn write_record(&self, name: StateFile, record: &impl Serialize) -> Result<()> { + let bytes = serde_json::to_vec(record).map_err(|error| { + Error::io(format!("cannot serialize {}", self.path(name).display())).with_source(error) + })?; + files::replace(self, name, &bytes) + } +} diff --git a/nvx-rs/src/host/cli/store/tests.rs b/nvx-rs/src/host/cli/store/tests.rs index bb36e8418..f28a33df9 100644 --- a/nvx-rs/src/host/cli/store/tests.rs +++ b/nvx-rs/src/host/cli/store/tests.rs @@ -8,6 +8,8 @@ use super::*; use crate::host::cli::deadline::Deadline; use crate::host::store::{JOURNAL_NAME, RUNTIME_NAME, SandboxStore}; +mod launch; + /// How long a test waits for a lock that it released to be free. /// /// A process that a concurrent test spawns holds a copy of each descriptor of the test process diff --git a/nvx-rs/src/host/cli/store/tests/launch.rs b/nvx-rs/src/host/cli/store/tests/launch.rs new file mode 100644 index 000000000..aff660e14 --- /dev/null +++ b/nvx-rs/src/host/cli/store/tests/launch.rs @@ -0,0 +1,382 @@ +use std::error::Error as _; + +use super::*; +use crate::ErrorKind; +use crate::activation::{MachineIdMode, View}; +use crate::host::cli::store::{files, record}; +use crate::host::config::{Boot, ConsoleAttachment, RootConfig, VmConfig}; +use crate::host::process::{BootId, ProcessRef}; +use crate::host::runtime::{ArtifactIdentity, LaunchOptions, Runtime, RuntimeIdentity}; +use crate::host::vm::{self, Prepared, SavedVm, VmDir}; +use crate::hypervisor::Hypervisor; +use crate::protocol::CAPABILITY_LEN; + +fn prepared(state: &StateDir, console: ConsoleAttachment) -> (Prepared, RuntimeIdentity) { + let identity = |text| ArtifactIdentity::new(text).unwrap(); + let runtime = Runtime { + openvmm: "missing-openvmm".into(), + kernel: "missing-kernel".into(), + initramfs: "missing-initramfs".into(), + identity: RuntimeIdentity { + openvmm: identity("vmm-revision"), + kernel: identity("kernel-revision"), + initramfs: identity("initramfs-revision"), + boot_contract: u64::MAX, + control_contract: "saved-control-revision".to_owned(), + control_session: u64::MAX - 1, + }, + hypervisor: if cfg!(windows) { + Hypervisor::Whp + } else { + Hypervisor::Mshv + }, + cpu_profile: None, + windows_breakaway: false, + options: LaunchOptions::default(), + }; + let config = VmConfig { + processors: 1, + memory_mib: 256, + root: RootConfig::Blockless, + view: View { + hostname: b"journal-test".to_vec(), + machine_id: MachineIdMode::Image, + }, + network: None, + filesystem: None, + console, + boot: Some(Boot { + command_line: "nvx_entrypoint=/bin/sh".to_owned(), + capture: None, + }), + }; + let vm_dir = VmDir::open(state.dir()).unwrap(); + ( + vm::create(&runtime, &config, None, &vm_dir).unwrap(), + runtime.identity, + ) +} + +fn saved(state: &StateDir, prepared: &Prepared, runtime: RuntimeIdentity) -> SavedVm { + SavedVm { + vm_dir: VmDir::open(state.dir()).unwrap(), + endpoint: prepared.outputs().endpoint.clone(), + capability: *prepared.capability().as_bytes(), + process: ProcessRef { + pid: u32::MAX, + start_time: u64::MAX, + boot: BootId::from_bytes([0xa5; 16]), + }, + runtime, + } +} + +fn assert_saved(actual: &SavedVm, expected: &SavedVm) { + assert_eq!(actual.vm_dir, expected.vm_dir); + assert_eq!(actual.endpoint, expected.endpoint); + assert_eq!(actual.capability, expected.capability); + assert_eq!(actual.process, expected.process); + assert_eq!(actual.runtime, expected.runtime); +} + +fn read_file(locked: &locked::LockedState, name: StateFile) -> crate::Result { + match name { + StateFile::Journal => locked.journal().map(|record| record.is_some()), + StateFile::Runtime => locked.runtime().map(|record| record.is_some()), + StateFile::Capability => locked.capability().map(|record| record.is_some()), + _ => unreachable!(), + } +} + +#[test] +fn the_pre_spawn_barrier_and_commit_preserve_every_prepared_and_saved_value() { + for console in [ConsoleAttachment::Listener, ConsoleAttachment::Disconnected] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, console); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let spawn_hook = || { + let bytes = fs::read(state.path(StateFile::Journal)).unwrap(); + let journal: record::LaunchJournal = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(journal.vm_dir, saved.vm_dir.path()); + assert_eq!(journal.capability, saved.capability); + assert_eq!(journal.log, prepared.outputs().log); + assert_eq!(journal.outcome, prepared.outputs().outcome); + assert_eq!(journal.endpoint, prepared.outputs().endpoint); + assert_eq!(journal.console, prepared.outputs().console); + assert_eq!(journal.runtime, runtime); + assert_eq!(journal.process(), None); + assert!(!format!("{journal:?}").contains(&format!("{:?}", saved.capability))); + assert_eq!( + fs::read(state.path(StateFile::Capability)).unwrap(), + saved.capability + ); + for name in [StateFile::Journal, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + assert!(!state.path(StateFile::Runtime).exists()); + }; + spawn_hook(); + launch.record_process(&saved.process).unwrap(); + let journal: record::LaunchJournal = + serde_json::from_slice(&fs::read(state.path(StateFile::Journal)).unwrap()).unwrap(); + assert_eq!(journal.process(), Some(&saved.process)); + assert!(!state.path(StateFile::Runtime).exists()); + launch.commit(&saved).unwrap(); + assert!(!state.path(StateFile::Journal).exists()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert_eq!(locked.capability().unwrap(), Some(saved.capability)); + drop(locked); + drop(prepared); + let reopened = StateDir::open(state.dir()) + .unwrap() + .lock(Deadline::after(10.0)) + .unwrap(); + assert_saved(&reopened.runtime().unwrap().unwrap(), &saved); + for name in [StateFile::Runtime, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + assert!(!crate::host::filesystem::is_reparse_point(&metadata)); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + } +} + +#[test] +fn an_interruption_keeps_the_journal_and_capability_without_publishing_runtime() { + for spawned in [false, true] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Listener); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + { + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if spawned { + launch.record_process(&saved.process).unwrap(); + } + } + drop(locked); + let mut reopened = state.lock(Deadline::after(10.0)).unwrap(); + let journal = reopened.journal().unwrap().unwrap(); + assert_eq!(journal.process(), spawned.then_some(&saved.process)); + assert_eq!(journal.capability, saved.capability); + assert_eq!(reopened.capability().unwrap(), Some(saved.capability)); + assert!(reopened.runtime().unwrap().is_none()); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + assert!(reopened.begin_launch(&prepared, &runtime).is_err()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + } +} + +#[test] +fn a_commit_needs_the_same_saved_values_and_an_already_journaled_process() { + for mismatch in 0..6 { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if mismatch != 0 { + launch.record_process(&saved.process).unwrap(); + assert!(launch.record_process(&saved.process).is_err()); + } + let original = fs::read(state.path(StateFile::Journal)).unwrap(); + match mismatch { + 0 => {} + 1 => saved.process.start_time -= 1, + 2 => saved.endpoint.push('x'), + 3 => saved.capability[0] ^= 1, + 4 => saved.runtime.control_session -= 1, + 5 => { + let other = state.dir().join("other-vm"); + vm::create_private_dir(&other).unwrap(); + saved.vm_dir = VmDir::open(&other).unwrap(); + } + _ => unreachable!(), + } + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), original); + } +} + +#[test] +fn preparation_for_another_directory_and_failed_record_writes_do_not_publish_runtime() { + let (_directory, state) = state(); + let (_other_directory, other) = super::state(); + let (other_prepared, runtime) = prepared(&other, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + assert!(locked.begin_launch(&other_prepared, &runtime).is_err()); + assert!(!state.path(StateFile::Journal).exists()); + assert!(!state.path(StateFile::Capability).exists()); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let journal = state.path(StateFile::Journal); + let original = journal.with_extension("original"); + fs::rename(&journal, &original).unwrap(); + fs::create_dir(&journal).unwrap(); + let error = launch.record_process(&saved.process).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + fs::remove_dir(&journal).unwrap(); + fs::rename(&original, &journal).unwrap(); + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert!(locked.journal().unwrap().unwrap().process().is_none()); +} + +#[test] +fn a_failed_runtime_publication_keeps_the_spawned_journal() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Runtime)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + assert_eq!( + fs::read_dir(state.path(StateFile::Runtime)) + .unwrap() + .count(), + 0 + ); +} + +#[test] +fn a_failed_journal_removal_reports_that_runtime_already_committed() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + fs::remove_file(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Journal)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert!(error.message().contains("runtime was committed"), "{error}"); + assert!(error.source().unwrap().source().is_some()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert!(state.path(StateFile::Journal).is_dir()); +} + +#[test] +fn malformed_records_and_capabilities_keep_their_parse_errors() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [StateFile::Journal, StateFile::Runtime] { + files::replace(&locked, name, b"{malformed").unwrap(); + let error = read_file(&locked, name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!( + error + .source() + .unwrap() + .downcast_ref::() + .is_some(), + "{error}" + ); + } + files::replace(&locked, StateFile::Capability, &[0x5a; CAPABILITY_LEN - 1]).unwrap(); + assert_eq!(locked.capability().unwrap_err().kind(), ErrorKind::Io); +} + +#[test] +fn records_and_capabilities_refuse_non_regular_entries_and_links_before_read_or_replace() { + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + let (directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + let path = state.path(name); + fs::create_dir(&path).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(&path).unwrap().count(), 0); + fs::remove_dir(&path).unwrap(); + let target = directory.path().join("target"); + fs::write(&target, b"preserved target").unwrap(); + #[cfg(unix)] + let linked = std::os::unix::fs::symlink(&target, &path); + #[cfg(windows)] + let linked = std::os::windows::fs::symlink_file(&target, &path); + match linked { + Ok(()) => { + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read(&target).unwrap(), b"preserved target"); + fs::remove_file(&path).unwrap(); + } + #[cfg(windows)] + Err(error) if error.raw_os_error() == Some(1314) => { + eprintln!("skipped the symbolic link without Developer Mode or privilege: {error}"); + } + Err(error) => panic!("cannot create the test symbolic link: {error}"), + } + #[cfg(target_os = "linux")] + { + use nix::sys::stat::Mode; + nix::unistd::mkfifo(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + } + #[cfg(windows)] + { + let target = directory.path().join("junction-target"); + fs::create_dir(&target).unwrap(); + junction(&path, &target); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(target).unwrap().count(), 0); + } + } +} + +#[cfg(windows)] +#[test] +fn records_and_capabilities_that_other_users_may_access_are_refused_without_replacement() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + files::replace(&locked, name, b"preserved shared contents").unwrap(); + let path = state.path(name); + allow_everyone(&path, ""); + let before = fs::read(&path).unwrap(); + for error in [ + read_file(&locked, name).unwrap_err(), + files::replace(&locked, name, b"replacement").unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.message().contains("allows other users"), "{error}"); + } + assert_eq!(fs::read(path).unwrap(), before); + } +} From dc669fae0b56615a17650c6e46849f85be474275 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 10 Nov 2026 02:29:08 -0800 Subject: [PATCH 3/5] tests: keep adapter sandbox names short below data roots Use the existing PID and atomic create-directory retry to retain unique fixture directories without a long diagnostic prefix. Keep the scenario name in creation errors. Capture and restore control sockets now fit approved data-volume temp roots without changing production limits, assertions, timeouts or skipped tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c8e42ad-4102-451f-96f0-0a4ae0483d1a --- tests/runtime-adapter/tests/common/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/runtime-adapter/tests/common/mod.rs b/tests/runtime-adapter/tests/common/mod.rs index 3c3540923..797d3044a 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -154,11 +154,11 @@ impl Sandbox { pub(crate) fn new(name: &str) -> Self { let pid = std::process::id(); let root = (0..) - .map(|attempt| env::temp_dir().join(format!("runtime-adapter-{name}-{pid}-{attempt}"))) + .map(|attempt| env::temp_dir().join(format!("ra-{pid}-{attempt}"))) .find(|root| match fs::create_dir(root) { Ok(()) => true, Err(error) if error.kind() == io::ErrorKind::AlreadyExists => false, - Err(error) => panic!("cannot create {}: {error}", root.display()), + Err(error) => panic!("cannot create the {name} sandbox {}: {error}", root.display()), }) .expect("a directory name is free"); Self { From adeb1d4778229fd23b64c2be6cd762df352de382 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 10 Nov 2026 02:39:32 -0800 Subject: [PATCH 4/5] tests: format the compact sandbox creation diagnostic Apply the pinned rustfmt wrapping requested by the final-head review; fixture semantics and production source remain unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c8e42ad-4102-451f-96f0-0a4ae0483d1a --- tests/runtime-adapter/tests/common/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/runtime-adapter/tests/common/mod.rs b/tests/runtime-adapter/tests/common/mod.rs index 797d3044a..dc5e0f914 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -158,7 +158,10 @@ impl Sandbox { .find(|root| match fs::create_dir(root) { Ok(()) => true, Err(error) if error.kind() == io::ErrorKind::AlreadyExists => false, - Err(error) => panic!("cannot create the {name} sandbox {}: {error}", root.display()), + Err(error) => panic!( + "cannot create the {name} sandbox {}: {error}", + root.display() + ), }) .expect("a directory name is free"); Self { From 797d300557e54644f0a8e66ba729de0dcf1884fb Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 10 Nov 2026 03:43:25 -0800 Subject: [PATCH 5/5] host: open strict private snapshot directories Validate the fixed snapshot layout, private regular files, bounded metadata and key/manifest agreement without reading snapshot.json. Retain the caller immutability contract and cover Linux/Windows refusal cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6789d491-7d13-4929-bc64-4edfec93ba60 --- nvx-rs/README.md | 18 +- nvx-rs/public-api/linux.txt | 6 + nvx-rs/public-api/windows.txt | 6 + nvx-rs/src/host/snapshot.rs | 189 +++++++++++- nvx-rs/src/host/snapshot/key.rs | 13 + nvx-rs/src/host/snapshot/key/tests.rs | 23 ++ nvx-rs/src/host/snapshot/tests.rs | 408 +++++++++++++++++++++++++- nvx-rs/src/host/vm/tests.rs | 3 +- scripts/test-layout-allowlist.json | 1 + 9 files changed, 657 insertions(+), 10 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 3019c6ebc..c432b148f 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -125,7 +125,7 @@ changes only when the module's purpose does. | [`nvx::exec`](#nvxexec) | The execution engine: the streams, credit, and outcome of an execution's two sides, without I/O, for any framing | | [`nvx::host::runtime`](#nvxhostruntime) | `Runtime`: what every launch shares, the runtime's files and their identity, the hypervisor and its probe, and the launch options | | [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, confirmed stop, and caller-saved reattachment | -| [`nvx::host::snapshot`](#nvxhostsnapshot) | Snapshot compatibility keys, strict manifests, restore-time eligibility, and the opaque input of a future VM restore | +| [`nvx::host::snapshot`](#nvxhostsnapshot) | Strict private snapshot directories, verified keys and manifests, time eligibility, and the input of a future VM restore | | [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: pure configuration, cold-boot kernel/OpenVMM arguments, portable length checks, and capture identity | | [`nvx::host::restore`](#nvxhostrestore) | A caller-driven restore to Ready without I/O or a clock, and its peer-verified restore-ready listener | | [`nvx::host::artifacts`](#nvxhostartifacts) | Locating the OpenVMM executable and the guest artifacts that a VM runs, and their runtime identity | @@ -579,9 +579,19 @@ The key's canonical encoding and identity use the snapshot names and omit and no reader or migration of the prior encoding is supported. `CaptureClass` still records the legacy capture inputs, including its guest build. -`Snapshot` is an opaque input of a future restore. `vm::create` currently -refuses `Some(snapshot)` with `ErrorKind::Unsupported`; only cold boots are -supported. +`Snapshot::open` checks exactly `key.bin`, `snapshot.json` and +`openvmm/{manifest.bin,memory.bin,state.bin}`. Directories are private, files +are regular and private to their owner, artifact lengths equal the strict +manifest's, and the canonical key is consistent with that manifest. +`KEY_FILE`, `DESCRIPTION_FILE` and `OPENVMM_DIR` name the root entries. +The description must exist, but its contents are never read, parsed or +trusted. `Snapshot::key` and `manifest` expose the verified metadata. + +The caller keeps the directory and every entry private and immutable while +using the snapshot; opening checks them once and retains no file handles +or lock. `Snapshot` remains an opaque input of a future restore: +`vm::create` still refuses `Some(snapshot)` with `ErrorKind::Unsupported`; +only cold boots are supported. ### `nvx::host::config` diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 2d90a5759..f134e992e 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -3035,6 +3035,7 @@ method nvx::host::snapshot::CaptureClass::to_bytes struct nvx::host::snapshot::ClassIdentity method nvx::host::snapshot::ClassIdentity::as_bytes method nvx::host::snapshot::ClassIdentity::from_bytes +const nvx::host::snapshot::DESCRIPTION_FILE struct nvx::host::snapshot::Downtime field nvx::host::snapshot::Downtime::nanos field nvx::host::snapshot::Downtime::source @@ -3072,13 +3073,18 @@ method nvx::host::snapshot::Invariants::iter method nvx::host::snapshot::Invariants::len method nvx::host::snapshot::Invariants::new method nvx::host::snapshot::Invariants::with +const nvx::host::snapshot::KEY_FILE struct nvx::host::snapshot::KeyIdentity method nvx::host::snapshot::KeyIdentity::as_bytes method nvx::host::snapshot::KeyIdentity::from_bytes const nvx::host::snapshot::MANIFEST_NAME const nvx::host::snapshot::MAX_DOWNTIME_NS const nvx::host::snapshot::MAX_MANIFEST_LEN +const nvx::host::snapshot::OPENVMM_DIR struct nvx::host::snapshot::Snapshot +method nvx::host::snapshot::Snapshot::key +method nvx::host::snapshot::Snapshot::manifest +method nvx::host::snapshot::Snapshot::open struct nvx::host::snapshot::SnapshotKey const nvx::host::snapshot::SnapshotKey::MAX_LEN const nvx::host::snapshot::SnapshotKey::VERSION diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index c1e2bc8cf..375201d56 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -1621,6 +1621,7 @@ method nvx::host::snapshot::CaptureClass::to_bytes struct nvx::host::snapshot::ClassIdentity method nvx::host::snapshot::ClassIdentity::as_bytes method nvx::host::snapshot::ClassIdentity::from_bytes +const nvx::host::snapshot::DESCRIPTION_FILE struct nvx::host::snapshot::Downtime field nvx::host::snapshot::Downtime::nanos field nvx::host::snapshot::Downtime::source @@ -1658,13 +1659,18 @@ method nvx::host::snapshot::Invariants::iter method nvx::host::snapshot::Invariants::len method nvx::host::snapshot::Invariants::new method nvx::host::snapshot::Invariants::with +const nvx::host::snapshot::KEY_FILE struct nvx::host::snapshot::KeyIdentity method nvx::host::snapshot::KeyIdentity::as_bytes method nvx::host::snapshot::KeyIdentity::from_bytes const nvx::host::snapshot::MANIFEST_NAME const nvx::host::snapshot::MAX_DOWNTIME_NS const nvx::host::snapshot::MAX_MANIFEST_LEN +const nvx::host::snapshot::OPENVMM_DIR struct nvx::host::snapshot::Snapshot +method nvx::host::snapshot::Snapshot::key +method nvx::host::snapshot::Snapshot::manifest +method nvx::host::snapshot::Snapshot::open struct nvx::host::snapshot::SnapshotKey const nvx::host::snapshot::SnapshotKey::MAX_LEN const nvx::host::snapshot::SnapshotKey::VERSION diff --git a/nvx-rs/src/host/snapshot.rs b/nvx-rs/src/host/snapshot.rs index 599a0fb53..7fcb88f6e 100644 --- a/nvx-rs/src/host/snapshot.rs +++ b/nvx-rs/src/host/snapshot.rs @@ -1,4 +1,8 @@ -//! Snapshot metadata and the opaque input of a future VM restore. +//! Strict snapshot directories, compatibility keys, manifests and restore-time eligibility. +//! +//! [`Snapshot::open`] checks the one snapshot format without reading its informational +//! description. The caller keeps the directory and its entries private and immutable while +//! using the opened snapshot. //! //! [`SnapshotKey`] derives a compatibility key from a capture's class and manifest. //! [`SnapshotManifest::read`] reads the manifest that OpenVMM wrote, and @@ -31,15 +35,192 @@ pub(super) mod manifest; pub(crate) mod tests; mod wire; +use std::ffi::OsString; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +use crate::host::paths::{absolute, normalized}; +use crate::host::store::regular; +use crate::host::{filesystem, platform}; +use crate::{Error, Result}; + pub use self::eligibility::{Downtime, DowntimeSource, Eligibility, HostTime, MAX_DOWNTIME_NS}; pub use self::key::{CaptureClass, ClassIdentity, Invariants, KeyIdentity, SnapshotKey}; pub use self::manifest::{ BlockGeometry, HostClock, MANIFEST_NAME, MAX_MANIFEST_LEN, SnapshotManifest, TimeContract, }; -/// An opaque snapshot input. Creating a VM from a snapshot is not supported yet. +/// The canonical snapshot key. +pub const KEY_FILE: &str = "key.bin"; +/// The informational description, whose contents [`Snapshot::open`] never reads. +pub const DESCRIPTION_FILE: &str = "snapshot.json"; +/// The directory of OpenVMM's manifest, RAM and saved state. +pub const OPENVMM_DIR: &str = "openvmm"; + +/// A checked snapshot directory and its verified metadata, for a future VM restore. +/// +/// The caller must keep the directory and its entries private and immutable while using this +/// value. Opening checks them once; it neither watches them nor holds their files open. #[derive(Debug)] pub struct Snapshot { - /// Reserved for snapshot metadata. - pub(crate) _private: (), + /// The absolute directory path, with its components preserved rather than its links resolved. + #[cfg_attr( + not(test), + expect(dead_code, reason = "SL2.15 uses the snapshot directory (#1103)") + )] + pub(crate) dir: PathBuf, + /// The bounded canonical key checked against the manifest. + key: SnapshotKey, + /// The strictly parsed metadata of OpenVMM's snapshot. + manifest: SnapshotManifest, +} + +impl Snapshot { + /// Opens the one strict snapshot format without changing any entry. + /// + /// `dir` must be a private directory containing exactly [`KEY_FILE`], [`DESCRIPTION_FILE`] + /// and [`OPENVMM_DIR`]. The latter must be a private directory containing exactly + /// [`MANIFEST_NAME`], `memory.bin` and `state.bin`. Every file must be regular, not a link or + /// reparse point, and private to its owner. The manifest is read by + /// [`SnapshotManifest::read`], its artifact lengths must match, and the bounded canonical + /// key must be consistent with it. The description is required but its contents are never + /// read, parsed or trusted. + /// + /// The caller must keep the directory and its entries private and immutable while using + /// the returned snapshot. This check retains no open file handles or lock. + /// + /// Fails with [`ErrorKind::Io`](crate::ErrorKind::Io) for an unreadable or invalid directory, + /// entry, length or key relationship, and otherwise as [`SnapshotManifest::read`] and + /// [`SnapshotKey::from_bytes`] do. + pub fn open(dir: &Path) -> Result { + let dir = absolute(&normalized(dir)).map_err(|error| entry_error(dir, error))?; + check_directory(&dir, &[KEY_FILE, DESCRIPTION_FILE, OPENVMM_DIR])?; + let openvmm = dir.join(OPENVMM_DIR); + check_directory(&openvmm, &[MANIFEST_NAME, "memory.bin", "state.bin"])?; + for path in [ + dir.join(KEY_FILE), + dir.join(DESCRIPTION_FILE), + openvmm.join(MANIFEST_NAME), + ] { + check_file(&path)?; + } + let manifest = SnapshotManifest::read(&openvmm)?; + for (name, expected) in [ + ("memory.bin", manifest.memory_len()), + ("state.bin", manifest.state_len()), + ] { + let path = openvmm.join(name); + let found = check_file(&path)?.len(); + if found != expected { + return Err(Error::io(format!( + "the snapshot artifact {} is {found} bytes long, not the {expected} bytes \ + that its manifest records", + path.display() + ))); + } + } + let path = dir.join(KEY_FILE); + let bytes = regular::open_regular(&path) + .and_then(|file| { + regular::read_at_most(file, SnapshotKey::MAX_LEN as u64, "a snapshot key") + }) + .map_err(|error| entry_error(&path, error))?; + let key = SnapshotKey::from_bytes(&bytes)?; + key.check_manifest(&manifest)?; + Ok(Self { dir, key, manifest }) + } + + /// Returns the verified snapshot key. + pub fn key(&self) -> &SnapshotKey { + &self.key + } + + /// Returns the strictly parsed OpenVMM manifest. + pub fn manifest(&self) -> &SnapshotManifest { + &self.manifest + } +} + +/// Names the failing snapshot entry while retaining its I/O error. +fn entry_error(path: &Path, error: io::Error) -> Error { + Error::io(format!("cannot examine snapshot entry {}", path.display())).with_source(error) +} + +/// Checks one private directory and its exact fixed entry-name inventory. +fn check_directory(path: &Path, expected: &[&str]) -> Result<()> { + if let Some(problem) = + platform::check_snapshot_dir(path).map_err(|error| entry_error(path, error))? + { + return Err(Error::io(format!( + "the snapshot directory {} {problem}", + path.display() + ))); + } + let mut found = fs::read_dir(path) + .and_then(|entries| { + entries + .map(|entry| Ok(entry?.file_name())) + .collect::>>() + }) + .map_err(|error| entry_error(path, error))?; + let mut expected: Vec = expected.iter().map(|name| OsString::from(*name)).collect(); + found.sort(); + expected.sort(); + if found != expected { + return Err(Error::io(format!( + "the snapshot directory {} holds {found:?}, not exactly {expected:?}", + path.display() + ))); + } + Ok(()) +} + +/// Checks a regular private file from metadata alone, without reading its contents. +fn check_file(path: &Path) -> Result { + #[cfg(windows)] + let file = { + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_FLAG_OPEN_REPARSE_POINT, FILE_READ_ATTRIBUTES, READ_CONTROL, + }; + + fs::OpenOptions::new() + .access_mode(READ_CONTROL | FILE_READ_ATTRIBUTES) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(path) + .map_err(|error| entry_error(path, error))? + }; + #[cfg(windows)] + let metadata = file.metadata().map_err(|error| entry_error(path, error))?; + #[cfg(not(windows))] + let metadata = fs::symlink_metadata(path).map_err(|error| entry_error(path, error))?; + if !metadata.is_file() || filesystem::is_reparse_point(&metadata) { + return Err(Error::io(format!( + "the snapshot entry {} is not a regular file, or is a link or reparse point", + path.display() + ))); + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + + if metadata.uid() != nix::unistd::Uid::effective().as_raw() || metadata.mode() & 0o077 != 0 + { + return Err(Error::io(format!( + "the snapshot file {} is not private to the current user", + path.display() + ))); + } + } + #[cfg(windows)] + if let Some(problem) = + platform::private_file_problem(&file).map_err(|error| entry_error(path, error))? + { + return Err(Error::io(format!( + "the snapshot file {} {problem}", + path.display() + ))); + } + Ok(metadata) } diff --git a/nvx-rs/src/host/snapshot/key.rs b/nvx-rs/src/host/snapshot/key.rs index 9f0c77169..8ab84cc1d 100644 --- a/nvx-rs/src/host/snapshot/key.rs +++ b/nvx-rs/src/host/snapshot/key.rs @@ -634,6 +634,19 @@ impl SnapshotKey { self.contracts == Contracts::CURRENT } + /// Checks the manifest's complete machine, the current key contracts and the filesystem tag. + pub(super) fn check_manifest(&self, manifest: &SnapshotManifest) -> Result<()> { + if self.machine != manifest.machine + || !self.speaks_current_contracts() + || self.filesystem_tag != MOUNT_TAG + { + return Err(Error::io( + "the snapshot key is not consistent with its manifest and this format's contracts", + )); + } + Ok(()) + } + /// Returns the backend. pub fn hypervisor(&self) -> Hypervisor { self.machine.hypervisor diff --git a/nvx-rs/src/host/snapshot/key/tests.rs b/nvx-rs/src/host/snapshot/key/tests.rs index c7fce6252..0ee47de27 100644 --- a/nvx-rs/src/host/snapshot/key/tests.rs +++ b/nvx-rs/src/host/snapshot/key/tests.rs @@ -709,6 +709,29 @@ fn key_refusal(bytes: &[u8], code: ErrorCode) -> String { error.message().to_owned() } +#[cfg(any(target_os = "linux", windows))] +#[test] +fn snapshot_open_refuses_other_key_contracts_and_filesystem_tags() { + use crate::host::snapshot::{KEY_FILE, Snapshot}; + use crate::host::store::files::write_private; + + let changes: [fn(&mut SnapshotKey); 2] = [ + |key| key.contracts.snapshot += 1, + |key| key.filesystem_tag.push_str("-other"), + ]; + for change in changes { + let fixture = support::SnapshotFixture::new(); + let mut key = Snapshot::open(&fixture.dir).unwrap().key().clone(); + change(&mut key); + let bytes = key.to_bytes(); + assert!(SnapshotKey::from_bytes(&bytes).is_ok()); + write_private(&fixture.dir.join(KEY_FILE), &bytes).unwrap(); + let error = Snapshot::open(&fixture.dir).unwrap_err(); + assert_eq!(error.code(), ErrorCode::Io, "{error}"); + assert!(error.message().contains("not consistent"), "{error}"); + } +} + #[test] fn other_encodings_of_a_key_are_refused() { let key = network_key(); diff --git a/nvx-rs/src/host/snapshot/tests.rs b/nvx-rs/src/host/snapshot/tests.rs index 3772e3ac5..c73fe2710 100644 --- a/nvx-rs/src/host/snapshot/tests.rs +++ b/nvx-rs/src/host/snapshot/tests.rs @@ -4,9 +4,16 @@ //! that the tests check, and [`ManifestSpec`] holds each field of a manifest, so that a test //! changes one field and encodes the others as they were. +use std::fs; +use std::path::{Path, PathBuf}; + use sha2::{Digest, Sha256}; -use super::{BlockGeometry, CaptureClass, Invariants}; +use super::{ + BlockGeometry, CaptureClass, DESCRIPTION_FILE, Invariants, KEY_FILE, MANIFEST_NAME, + OPENVMM_DIR, Snapshot, SnapshotKey, SnapshotManifest, +}; +use crate::ErrorKind; use crate::host::network::{Access, NetworkPolicy}; use crate::host::sandbox::SandboxNetwork; use crate::host::store::{ArtifactIdentity, RuntimeIdentity}; @@ -986,3 +993,402 @@ pub(in crate::host) fn network_class() -> CaptureClass { ..class() } } + +pub(in crate::host) struct SnapshotFixture { + root: tempfile::TempDir, + pub(in crate::host) dir: PathBuf, + spec: ManifestSpec, + key: SnapshotKey, +} + +impl SnapshotFixture { + pub(in crate::host) fn new() -> Self { + use crate::host::platform; + use crate::host::store::files::{create_new, write_private}; + + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("snapshot"); + assert_eq!(platform::create_owner_only_dir(&dir).unwrap(), None); + let openvmm = dir.join(OPENVMM_DIR); + assert_eq!(platform::create_owner_only_dir(&openvmm).unwrap(), None); + let spec = ManifestSpec::template(); + let manifest = SnapshotManifest::parse(&spec.encode()).unwrap(); + let key = SnapshotKey::new(&class(), &manifest).unwrap(); + write_private(&dir.join(KEY_FILE), &key.to_bytes()).unwrap(); + write_private(&dir.join(DESCRIPTION_FILE), b"not JSON").unwrap(); + write_private(&openvmm.join(MANIFEST_NAME), &spec.encode()).unwrap(); + for (name, length) in [ + ("memory.bin", spec.memory_size), + ("state.bin", spec.state_size), + ] { + let file = create_new(&openvmm.join(name)).unwrap(); + platform::make_sparse(&file).unwrap(); + file.set_len(length).unwrap(); + } + Self { + root, + dir, + spec, + key, + } + } + + fn files(&self) -> [PathBuf; 5] { + let openvmm = self.dir.join(OPENVMM_DIR); + [ + self.dir.join(KEY_FILE), + self.dir.join(DESCRIPTION_FILE), + openvmm.join(MANIFEST_NAME), + openvmm.join("memory.bin"), + openvmm.join("state.bin"), + ] + } +} + +fn refused(path: &Path) { + let error = Snapshot::open(path).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io, "{error}"); +} + +#[test] +fn snapshot_open_checks_the_complete_directory_and_preserves_its_metadata() { + let fixture = SnapshotFixture::new(); + let snapshot = Snapshot::open(&fixture.dir).unwrap(); + assert_eq!(snapshot.dir, fixture.dir); + assert_eq!(snapshot.key(), &fixture.key); + assert_eq!( + snapshot.manifest(), + &SnapshotManifest::parse(&fixture.spec.encode()).unwrap() + ); + for suffix in ["", std::path::MAIN_SEPARATOR_STR, "."] { + let path = if suffix == "." { + fixture.dir.join(".") + } else { + let mut path = fixture.dir.as_os_str().to_os_string(); + path.push(suffix); + PathBuf::from(path) + }; + assert_eq!(Snapshot::open(&path).unwrap().key(), &fixture.key); + } +} + +#[test] +fn snapshot_open_refuses_every_missing_entry() { + for index in 0..6 { + let fixture = SnapshotFixture::new(); + let missing = if index == 5 { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.files()[index].clone() + }; + fs::rename(&missing, fixture.root.path().join("missing")).unwrap(); + refused(&fixture.dir); + } + let fixture = SnapshotFixture::new(); + refused(&fixture.root.path().join("absent")); +} + +#[test] +fn snapshot_open_refuses_extra_entries_and_alternate_names() { + use crate::host::store::files::write_private; + + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let dir = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + write_private(&dir.join("extra"), b"extra").unwrap(); + refused(&fixture.dir); + } + let fixture = SnapshotFixture::new(); + fs::rename(fixture.dir.join(KEY_FILE), fixture.dir.join("KEY.BIN")).unwrap(); + refused(&fixture.dir); +} + +#[test] +fn snapshot_open_refuses_every_directory_in_place_of_a_regular_file() { + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::remove_file(path).unwrap(); + fs::create_dir(path).unwrap(); + refused(&fixture.dir); + } +} + +#[test] +fn snapshot_open_refuses_files_in_place_of_its_directories() { + use crate::host::store::files::write_private; + + let fixture = SnapshotFixture::new(); + let file = fixture.root.path().join("file"); + write_private(&file, b"file").unwrap(); + refused(&file); + fs::rename( + fixture.dir.join(OPENVMM_DIR), + fixture.root.path().join("saved"), + ) + .unwrap(); + write_private(&fixture.dir.join(OPENVMM_DIR), b"file").unwrap(); + refused(&fixture.dir); +} + +#[test] +fn snapshot_open_refuses_both_artifact_length_mismatches() { + for name in ["memory.bin", "state.bin"] { + for difference in [-1i64, 1] { + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(OPENVMM_DIR).join(name); + let file = fs::OpenOptions::new().write(true).open(path).unwrap(); + let length = file.metadata().unwrap().len(); + file.set_len(length.checked_add_signed(difference).unwrap()) + .unwrap(); + refused(&fixture.dir); + } + } +} + +#[test] +fn snapshot_open_uses_the_strict_manifest_and_bounded_key_decoders() { + use crate::host::store::files::write_private; + + for bytes in [vec![0x80], vec![0; super::MAX_MANIFEST_LEN as usize + 1]] { + let fixture = SnapshotFixture::new(); + write_private(&fixture.dir.join(OPENVMM_DIR).join(MANIFEST_NAME), &bytes).unwrap(); + assert!(Snapshot::open(&fixture.dir).is_err()); + } + let fixture = SnapshotFixture::new(); + let mut trailing = fixture.key.to_bytes(); + trailing.push(0); + let mut version = fixture.key.to_bytes(); + version[0] += 1; + for bytes in [ + vec![0], + trailing, + version, + vec![0; SnapshotKey::MAX_LEN + 1], + ] { + write_private(&fixture.dir.join(KEY_FILE), &bytes).unwrap(); + assert!(Snapshot::open(&fixture.dir).is_err()); + } +} + +#[test] +fn snapshot_open_refuses_a_key_from_another_manifest_machine() { + use crate::host::store::files::write_private; + + let mutations: [fn(&mut ManifestSpec); 4] = [ + |spec| spec.cpu().digest[0] ^= 1, + |spec| spec.contract().command_line.push_str(" extra"), + |spec| spec.contract().devices[0].state_unit.push_str("-other"), + |spec| spec.time().tsc_hz += 1, + ]; + for mutate in mutations { + let fixture = SnapshotFixture::new(); + let mut spec = fixture.spec.clone(); + mutate(&mut spec); + let manifest = SnapshotManifest::parse(&spec.encode()).unwrap(); + let key = SnapshotKey::new(&class(), &manifest).unwrap(); + write_private(&fixture.dir.join(KEY_FILE), &key.to_bytes()).unwrap(); + refused(&fixture.dir); + } +} + +#[test] +fn snapshot_description_contents_do_not_affect_opening() { + use crate::host::store::files::write_private; + + let fixture = SnapshotFixture::new(); + for contents in [b"".as_slice(), b"{broken JSON", &[0xff, 0, 0x80]] { + write_private(&fixture.dir.join(DESCRIPTION_FILE), contents).unwrap(); + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_every_shared_file_and_both_shared_directories() { + use std::os::unix::fs::PermissionsExt; + + for index in 0..5 { + for mode in [0o640, 0o602, 0o610] { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::set_permissions(path, fs::Permissions::from_mode(mode)).unwrap(); + refused(&fixture.dir); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + mode + ); + } + } + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let path = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + fs::set_permissions(&path, fs::Permissions::from_mode(0o750)).unwrap(); + refused(&fixture.dir); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o750 + ); + } +} + +#[cfg(target_os = "linux")] +#[test] +#[ignore = "requires root to change only the owned snapshot fixture's entry owners"] +fn snapshot_open_refuses_entries_owned_by_another_user() { + use nix::unistd::{Uid, chown}; + + assert!(Uid::effective().is_root()); + for index in 0..7 { + let fixture = SnapshotFixture::new(); + let path = match index { + 5 => fixture.dir.clone(), + 6 => fixture.dir.join(OPENVMM_DIR), + _ => fixture.files()[index].clone(), + }; + chown(&path, Some(Uid::from_raw(1)), None).unwrap(); + refused(&fixture.dir); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_file_links_fifos_and_sockets_without_reading_them() { + use std::os::unix::fs::symlink; + use std::os::unix::net::UnixListener; + + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + let saved = fixture.root.path().join("saved"); + fs::rename(path, &saved).unwrap(); + symlink(&saved, path).unwrap(); + refused(&fixture.dir); + fs::remove_file(path).unwrap(); + nix::unistd::mkfifo(path, nix::sys::stat::Mode::S_IRUSR).unwrap(); + refused(&fixture.dir); + fs::remove_file(path).unwrap(); + let _socket = UnixListener::bind(path).unwrap(); + refused(&fixture.dir); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_directory_links_even_with_trailing_separators() { + use std::os::unix::fs::symlink; + + let fixture = SnapshotFixture::new(); + let link = fixture.root.path().join("link"); + symlink(&fixture.dir, &link).unwrap(); + for suffix in ["", "/", "/.", "/./"] { + let mut path = link.as_os_str().to_os_string(); + path.push(suffix); + refused(Path::new(&path)); + } + let saved = fixture.root.path().join("saved"); + let openvmm = fixture.dir.join(OPENVMM_DIR); + fs::rename(&openvmm, &saved).unwrap(); + symlink(&saved, &openvmm).unwrap(); + refused(&fixture.dir); +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_description_needs_no_read_permission() { + use std::os::unix::fs::PermissionsExt; + + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(DESCRIPTION_FILE); + fs::set_permissions(&path, fs::Permissions::from_mode(0o200)).unwrap(); + if !nix::unistd::Uid::effective().is_root() { + assert_eq!( + fs::read(&path).unwrap_err().kind(), + std::io::ErrorKind::PermissionDenied + ); + } + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o200 + ); +} + +#[cfg(windows)] +#[test] +fn snapshot_open_refuses_every_shared_file_and_both_shared_directory_acls() { + use crate::host::platform::tests::{access, set_access, user_sid}; + + let sid = user_sid(); + let shared = format!("D:P(A;;FA;;;{sid})(A;;FR;;;WD)"); + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + set_access(path, &shared).unwrap(); + let before = access(path).unwrap(); + refused(&fixture.dir); + assert_eq!(access(path).unwrap(), before); + } + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let path = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + set_access(&path, &shared).unwrap(); + let before = access(&path).unwrap(); + refused(&fixture.dir); + assert_eq!(access(&path).unwrap(), before); + } +} + +#[cfg(windows)] +#[test] +fn snapshot_open_refuses_directory_junctions_and_every_reparse_file_entry() { + use crate::host::tests::junction; + + let fixture = SnapshotFixture::new(); + let link = fixture.root.path().join("link"); + junction(&link, &fixture.dir); + for path in [link.clone(), link.join("."), link.join("")] { + refused(&path); + } + let saved = fixture.root.path().join("saved"); + let openvmm = fixture.dir.join(OPENVMM_DIR); + fs::rename(&openvmm, &saved).unwrap(); + junction(&openvmm, &saved); + refused(&fixture.dir); + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::remove_file(path).unwrap(); + junction(path, fixture.root.path()); + refused(&fixture.dir); + fs::remove_dir(path).unwrap(); + } +} + +#[cfg(windows)] +#[test] +fn snapshot_description_needs_no_data_read_access() { + use crate::host::platform::tests::{set_access, user_sid}; + + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(DESCRIPTION_FILE); + let sid = user_sid(); + set_access(&path, &format!("D:P(D;;0x1;;;{sid})(A;;FA;;;{sid})")).unwrap(); + assert_eq!( + fs::read(&path).unwrap_err().kind(), + std::io::ErrorKind::PermissionDenied + ); + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); +} diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index b637a7532..be5e3c799 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -494,7 +494,8 @@ fn a_missing_boot_and_a_snapshot_are_refused_before_any_output_is_created() { let runtime = cold_runtime(); let mut config = cold_config(); config.boot = None; - let snapshot = Snapshot { _private: () }; + let fixture = crate::host::snapshot::tests::SnapshotFixture::new(); + let snapshot = Snapshot::open(&fixture.dir).unwrap(); for (from, config, expected) in [ (None, &config, "Boot"), (Some(&snapshot), &cold_config(), "Snapshot"), diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 5b7e6aa10..adf7c96c6 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -17,6 +17,7 @@ "nvx-rs/src/host/restore.rs": 3, "nvx-rs/src/host/sandbox/plan.rs": 5, "nvx-rs/src/host/session.rs": 1, + "nvx-rs/src/host/snapshot.rs": 1, "nvx-rs/src/host/vm.rs": 5, "nvx-rs/src/protocol/app.rs": 6 }