From b0fbf2f7a0ac06538b0cc4996d84e52ae911b849 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 29 Oct 2026 09:26:52 -0700 Subject: [PATCH 1/2] nvx-rs: pin the messages with which the sandbox store refuses its files The sandbox store reads its runtime record, launch journal, and capability, and takes its lock, through free functions of store/files.rs, to which each caller passes the file's path and its description again (#645). The tests check that a record or the capability fails to read when it is not a regular file, is malformed, or is not private, but mostly through fragments of the messages, such as "is malformed" or "allows other users", and no test checks the messages of a missing or an invalid capability. A change that gathers those paths and descriptions in one type could then change what a message calls a file, or which file it names, without a test noticing. The new test pins, on every platform, the messages with which the store refuses a missing capability, one of the wrong length or all zero, a runtime record or a journal that does not parse, and a directory in the place of either record or of the capability, which Unix opens and refuses as no regular file, and Windows refuses to open. The Windows test of the store's privacy now pins the whole message with which the store refuses a journal, a capability, and a lock whose access control list allows other users, rather than its end. The tests pass with the store as it is. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/store/tests.rs | 94 ++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 4 deletions(-) diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 12ddc19ea..6a34581c1 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -495,6 +495,77 @@ fn records_and_capabilities_that_are_not_regular_files_fail_at_once() { } } +#[test] +fn failures_to_read_the_records_and_the_capability_name_them() { + let (_directory, store) = store(); + let read = |name: &str| match name { + RUNTIME_NAME => store.runtime().map(drop), + JOURNAL_NAME => store.journal().map(drop), + _ => store.read_capability().map(drop), + }; + let files = [ + (RUNTIME_NAME, "sandbox runtime record"), + (JOURNAL_NAME, "sandbox launch journal"), + (CAPABILITY_NAME, "sandbox control capability"), + ]; + // A missing record is none, but a missing capability fails, as its launch needs it. + assert!(store.runtime().unwrap().is_none()); + assert!(store.journal().unwrap().is_none()); + let path = store.path(CAPABILITY_NAME); + let error = store.read_capability().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!( + "cannot read the sandbox control capability {}", + path.display() + ) + ); + let cause = std::error::Error::source(&error) + .and_then(|source| source.downcast_ref::()) + .map(io::Error::kind); + assert_eq!(cause, Some(io::ErrorKind::NotFound), "{error}"); + // A capability of another length, or an all-zero one, is invalid. + for invalid in [&CAPABILITY[1..], &[0; CAPABILITY_LEN][..]] { + fs::write(&path, invalid).unwrap(); + let error = store.read_capability().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!( + "the sandbox control capability {} is invalid", + path.display() + ) + ); + } + fs::remove_file(&path).unwrap(); + // A record that does not parse is malformed. + for (name, description) in &files[..2] { + let path = store.path(name); + fs::write(&path, b"{").unwrap(); + let error = read(name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!("the {description} {} is malformed", path.display()) + ); + fs::remove_file(&path).unwrap(); + } + // A directory in the place of a record or of the capability cannot be read as one: Unix opens + // it, and refuses it as no regular file, and Windows refuses to open it. + for (name, description) in files { + let path = store.path(name); + fs::create_dir(&path).unwrap(); + let error = read(name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!("cannot read the {description} {}", path.display()) + ); + fs::remove_dir(&path).unwrap(); + } +} + #[cfg(target_os = "linux")] #[test] fn the_store_is_private_to_the_current_user() { @@ -652,17 +723,32 @@ fn the_store_is_private_to_the_current_user() { "{name}" ); } + let refused = |description: &str, name: &str| { + format!( + "the {description} {} allows other users", + store.path(name).display() + ) + }; let error = store.journal().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox launch journal", JOURNAL_NAME) + ); let error = locked.recover().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox launch journal", JOURNAL_NAME) + ); let error = store.read_capability().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox control capability", CAPABILITY_NAME) + ); // So is a lock file whose own list allows others, who could hold its lock. drop(locked); platform::set_access(&store.path(LOCK_NAME), "D:P(A;;FA;;;WD)").unwrap(); let error = store.lock(None).unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!(error.message(), refused("sandbox lock", LOCK_NAME)); } #[test] From b59f94f272c80b0307affa9d25b94b0e05db5107 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 29 Oct 2026 09:29:04 -0700 Subject: [PATCH 2/2] nvx-rs: wrap the file operations of the sandbox store in a type store/files.rs offered the sandbox store's file operations as free functions: create_new, open_lock, write_private, read_private, and remove_if_present. Every caller passed a file's path again, with the description that messages give the file, and had to remember to check the privacy of what it opened: read_private refused a record or the capability that others may open, but take_lock checked its lock file with platform::private_file_problem itself, and read_capability built the message of a missing capability from a description of its own. A new record or caller could skip a check, or name a file differently (#645). A StoreFile now holds one file of the store, its path with what messages call it, and gives it the operations that the store uses: create_new, open_lock, replace, read, and remove_if_present. SandboxStore builds one for each file that it creates and reads, its lock, launch journal, runtime record, capability, and log, and opens, creates, replaces, and reads those files only through it: - StoreFile::read is read_private, which is gone, so the store reads a file only as a StoreFile, which refuses it if it is not a regular file or not private. - StoreFile::open_lock opens the lock and refuses it if others may open it, the check that take_lock made, and maps the failures with the caller's mapper, so that "cannot lock" still names them all. - record::read and record::write take the StoreFile of the record, whose description is the record's Record::DESCRIPTION, and name it in their messages. - The messages of the capability and of the log, and the refusal of a lock, name the file by its description: "sandbox control capability", "log", and "sandbox lock", as they did. private_options, which only files.rs uses, is private to it now. The free functions that the template store uses, create_new, open_lock, write_private, remove_if_present, and sync_dir, stay pub(crate). The store's lifecycle still removes its lock file, and reaches the entries of its directory, by name: it removes the lock file only while the lock's handle still names it, and reports a RemovalError, and it removes the other entries through a handle of the directory, neither of which a StoreFile models. The on-disk layout, the record format, the error kinds, and the messages are unchanged. The previous commit's test, which pins the messages with which the store refuses its records and its capability on every platform, and on Windows those that refuse a journal, a capability, or a lock that others may open, passes unchanged, as do the FIFO and link refusals on Unix, the rest of the store's unit tests, which now write records through their StoreFiles, and tests/store.rs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/store/files.rs | 178 ++++++++++++++++++++++++-------- nvx-rs/src/host/store/mod.rs | 108 ++++++++++++------- nvx-rs/src/host/store/record.rs | 25 +++-- nvx-rs/src/host/store/tests.rs | 6 +- 4 files changed, 217 insertions(+), 100 deletions(-) diff --git a/nvx-rs/src/host/store/files.rs b/nvx-rs/src/host/store/files.rs index 63fde8e50..d20664270 100644 --- a/nvx-rs/src/host/store/files.rs +++ b/nvx-rs/src/host/store/files.rs @@ -1,12 +1,17 @@ //! The private files of the store: their creation, atomic replacement, reading, and removal, and //! the syncs of their directories. //! +//! The sandbox's store opens, creates, replaces, and reads its lock, its records, its capability, +//! and its log only as [`StoreFile`]s, each of which holds a file's path with what messages call +//! it, so that no read or lock of one of them skips the checks that refuse a file that others may +//! open. +//! //! The template store, [`template::Store`](crate::host::template::Store), keeps its own records //! with the same lock files, atomic replacements, removals, and syncs. use std::fs::{self, File, OpenOptions}; use std::io::{self, Read, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::error::{Error, Result}; use crate::host::{filesystem, platform}; @@ -27,7 +32,7 @@ const PRIVATE_FILE_MODE: u32 = 0o600; /// Returns options that create a file readable only by the current user (mode 0600 on Unix) /// and that open no link that its path names on Unix. -pub(super) fn private_options() -> OpenOptions { +fn private_options() -> OpenOptions { let mut options = OpenOptions::new(); options.write(true); #[cfg(unix)] @@ -159,48 +164,6 @@ pub(crate) fn write_private(path: &Path, contents: &[u8]) -> Result<()> { written.map_err(io_error(format!("cannot write {}", path.display()))) } -/// Reads the file at `path`, the store's `description`, or returns `None` if it does not exist. -/// -/// The open does not wait, as it would on Unix for a FIFO without a writer, and a file that is -/// not a regular file is refused before anything is read from it, as a FIFO whose writer keeps -/// it open would keep a read waiting. A file that is not private to the current user is refused, -/// since others could have written it: on Windows, a file's own access control list decides who -/// may open it, whatever the store's directory allows. -pub(super) fn read_private(path: &Path, description: &str) -> Result>> { - let failed = || io_error(format!("cannot read the {description} {}", path.display())); - let mut options = OpenOptions::new(); - options.read(true); - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - - // Reads of a regular file do not depend on the flag. - options.custom_flags(nix::fcntl::OFlag::O_NONBLOCK.bits()); - } - let mut file = match options.open(path) { - Ok(file) => file, - Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), - // A socket refuses an open. - #[cfg(unix)] - Err(error) if error.raw_os_error() == Some(nix::errno::Errno::ENXIO as i32) => { - return Err(failed()(not_regular())); - } - Err(error) => return Err(failed()(error)), - }; - if !file.metadata().map_err(failed())?.file_type().is_file() { - return Err(failed()(not_regular())); - } - if let Some(problem) = platform::private_file_problem(&file).map_err(failed())? { - return Err(Error::io(format!( - "the {description} {} {problem}", - path.display() - ))); - } - let mut contents = Vec::new(); - file.read_to_end(&mut contents).map_err(failed())?; - Ok(Some(contents)) -} - /// Makes the changes to the entries of the directory `dir` so far durable, such as files that /// were renamed into it or removed from it: a crash that keeps a later change keeps them too, even /// one on another filesystem. @@ -234,3 +197,130 @@ pub(crate) fn remove_if_present(path: &Path) -> Result<()> { _ => Ok(()), } } + +/// One file of a sandbox's store: its path, and what messages call it. +/// +/// [`SandboxStore`](super::SandboxStore) builds one for each file that it creates and reads, its +/// lock, its launch journal, its runtime record, its capability, and its log, and opens, creates, +/// replaces, and reads those files only through it, so that each of them is created readable only +/// by the current user, replaced atomically, read only if it is a regular file that others may not +/// open, and locked only if others may not open it, and that every message about it calls it by +/// its description. +#[derive(Debug)] +pub(super) struct StoreFile { + /// The file's path. + path: PathBuf, + /// What messages call the file, such as `sandbox runtime record`. + description: &'static str, +} + +impl StoreFile { + /// Returns the file at `path`, which messages call `description`. + pub(super) fn new(path: PathBuf, description: &'static str) -> Self { + Self { path, description } + } + + /// Returns the file's path. + pub(super) fn path(&self) -> &Path { + &self.path + } + + /// Returns what messages call the file. + pub(super) fn description(&self) -> &'static str { + self.description + } + + /// Creates the file, which must not exist, readable only by the current user, as + /// [`create_new`] does. + pub(super) fn create_new(&self) -> io::Result { + create_new(&self.path) + } + + /// Opens the file as a lock, creating it readable only by the current user if it does not + /// exist, as [`open_lock`] does, and refuses it if it is not private to the current user, since + /// others could hold its lock. + /// + /// `failed` maps the failures to open the file and to check it, as the caller maps those of + /// the lock that it then takes. + pub(super) fn open_lock(&self, failed: impl Fn(io::Error) -> Error) -> Result { + let file = open_lock(&self.path).map_err(&failed)?; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(self.not_private(&problem)); + } + Ok(file) + } + + /// Atomically replaces the file with `contents`, readable only by the current user, as + /// [`write_private`] does. + pub(super) fn replace(&self, contents: &[u8]) -> Result<()> { + write_private(&self.path, contents) + } + + /// Reads the file, or returns `None` if it does not exist. + /// + /// The open does not wait, as it would on Unix for a FIFO without a writer, and a file that is + /// not a regular file is refused before anything is read from it, as a FIFO whose writer keeps + /// it open would keep a read waiting. A file that is not private to the current user is + /// refused, since others could have written it: on Windows, a file's own access control list + /// decides who may open it, whatever the store's directory allows. + pub(super) fn read(&self) -> Result>> { + let mut options = OpenOptions::new(); + options.read(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + + // Reads of a regular file do not depend on the flag. + options.custom_flags(nix::fcntl::OFlag::O_NONBLOCK.bits()); + } + let mut file = match options.open(&self.path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + // A socket refuses an open. + #[cfg(unix)] + Err(error) if error.raw_os_error() == Some(nix::errno::Errno::ENXIO as i32) => { + return Err(self.read_failed()(not_regular())); + } + Err(error) => return Err(self.read_failed()(error)), + }; + if !file + .metadata() + .map_err(self.read_failed())? + .file_type() + .is_file() + { + return Err(self.read_failed()(not_regular())); + } + if let Some(problem) = platform::private_file_problem(&file).map_err(self.read_failed())? { + return Err(self.not_private(&problem)); + } + let mut contents = Vec::new(); + file.read_to_end(&mut contents) + .map_err(self.read_failed())?; + Ok(Some(contents)) + } + + /// Removes the file if it exists, as [`remove_if_present`] does. + pub(super) fn remove_if_present(&self) -> Result<()> { + remove_if_present(&self.path) + } + + /// Builds an error mapper that keeps the I/O source of a failure to read the file. + pub(super) fn read_failed(&self) -> impl FnOnce(io::Error) -> Error { + io_error(format!( + "cannot read the {} {}", + self.description, + self.path.display() + )) + } + + /// Returns the error of the file, which is not private to the current user for the reason + /// that `problem` gives. + fn not_private(&self, problem: &str) -> Error { + Error::io(format!( + "the {} {} {problem}", + self.description, + self.path.display() + )) + } +} diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 45833869c..89ec87ac1 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -249,12 +249,13 @@ use std::time::{Duration, Instant}; pub use self::caller::CallerFiles; use self::caller::{is_report_staging, is_store_temporary}; -use self::files::{io_error, remove_if_present}; +use self::files::{StoreFile, io_error, remove_if_present}; pub use self::identity::{ ArtifactIdentity, IdentityCheck, IdentityPart, MAX_IDENTITY_LEN, RuntimeIdentity, }; use self::lease::{Lease, clone_socket_name, is_clone_socket, open_payload}; pub use self::lifecycle::{Lingering, RemovalError}; +use self::record::Record; pub use self::record::{ LaunchId, LaunchJournal, LaunchState, LogClaim, MAX_ENDPOINT_LEN, RuntimeRecord, }; @@ -520,6 +521,36 @@ impl SandboxStore { self.dir.join(name) } + /// Returns the store's file `name`, one of [`FILE_NAMES`], which messages call `description`. + fn file(&self, name: &str, description: &'static str) -> StoreFile { + StoreFile::new(self.path(name), description) + } + + /// Returns the sandbox's lock file, [`LOCK_NAME`]. + fn lock_file(&self) -> StoreFile { + self.file(LOCK_NAME, "sandbox lock") + } + + /// Returns the launch journal, [`JOURNAL_NAME`]. + fn journal_file(&self) -> StoreFile { + self.file(JOURNAL_NAME, LaunchJournal::DESCRIPTION) + } + + /// Returns the runtime record, [`RUNTIME_NAME`]. + fn runtime_file(&self) -> StoreFile { + self.file(RUNTIME_NAME, RuntimeRecord::DESCRIPTION) + } + + /// Returns the file that holds the launch capability, [`CAPABILITY_NAME`]. + fn capability_file(&self) -> StoreFile { + self.file(CAPABILITY_NAME, "sandbox control capability") + } + + /// Returns the VMM's log, [`LOG_NAME`]. + fn log_file(&self) -> StoreFile { + self.file(LOG_NAME, "log") + } + /// Returns the path of the VMM's log. pub fn log_path(&self) -> PathBuf { self.path(LOG_NAME) @@ -582,29 +613,26 @@ impl SandboxStore { /// Reads the runtime record, which names the sandbox's VM from the commit of its launch until /// its runtime is cleared. pub fn runtime(&self) -> Result> { - record::read(&self.path(RUNTIME_NAME)) + record::read(&self.runtime_file()) } /// Reads the journal of a launch in progress, or of one that was interrupted. pub fn journal(&self) -> Result> { - record::read(&self.path(JOURNAL_NAME)) + record::read(&self.journal_file()) } /// Reads the launch capability of the sandbox's VM. pub fn read_capability(&self) -> Result<[u8; CAPABILITY_LEN]> { - let path = self.capability_path(); - let description = "sandbox control capability"; - let Some(bytes) = files::read_private(&path, description)? else { - return Err(io_error(format!( - "cannot read the {description} {}", - path.display() - ))(io::Error::from(io::ErrorKind::NotFound))); + let file = self.capability_file(); + let Some(bytes) = file.read()? else { + return Err(file.read_failed()(io::Error::from(io::ErrorKind::NotFound))); }; match <[u8; CAPABILITY_LEN]>::try_from(bytes.as_slice()) { Ok(capability) if capability != [0; CAPABILITY_LEN] => Ok(capability), _ => Err(Error::io(format!( - "the sandbox control capability {} is invalid", - path.display() + "the {} {} is invalid", + file.description(), + file.path().display() ))), } } @@ -643,15 +671,10 @@ impl SandboxStore { /// then: the open then fails with an I/O error whose cause is the denied access, while a /// removed lock file that the lock's own handle keeps is taken for a replaced one. fn take_lock(&self, deadline: Option) -> Result { - let path = self.path(LOCK_NAME); + let lock = self.lock_file(); + let path = lock.path(); let failed = |error| store_error(format!("cannot lock {}", path.display()))(error); - let file = files::open_lock(&path).map_err(failed)?; - if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { - return Err(Error::io(format!( - "the sandbox lock {} {problem}", - path.display() - ))); - } + let file = lock.open_lock(failed)?; match deadline { None => file.lock().map_err(failed)?, Some(deadline) => loop { @@ -669,7 +692,7 @@ impl SandboxStore { }, } let locked = platform::opened_file_id(&file).map_err(failed)?; - match platform::file_id(&path) { + match platform::file_id(path) { Ok(current) if current == locked => Ok(Taken::Locked(file)), Ok(_) => Ok(Taken::Replaced), Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(Taken::Replaced), @@ -945,10 +968,11 @@ impl LockedStore { let endpoint = platform::control_endpoint(listening) .map_err(|error| Error::io("cannot choose a control endpoint").with_source(error))?; let (log, claim) = self.create_log()?; - files::write_private(&self.capability_path(), capability)?; + let capability_file = self.capability_file(); + capability_file.replace(capability)?; let journal = LaunchJournal::prepared(launch.clone(), endpoint, runtime.clone(), claim); - if let Err(error) = record::write(&self.path(JOURNAL_NAME), &journal) { - let _ = remove_if_present(&self.capability_path()); + if let Err(error) = record::write(&self.journal_file(), &journal) { + let _ = capability_file.remove_if_present(); return Err(error); } // A lease reserves one launch, which has begun. @@ -962,10 +986,16 @@ impl LockedStore { /// Replaces the log of the previous launch with a fresh file, and claims it. fn create_log(&self) -> Result<(File, LogClaim)> { - let path = self.log_path(); - let failed = |error| io_error(format!("cannot create the log {}", path.display()))(error); - remove_if_present(&path)?; - let log = files::create_new(&path).map_err(failed)?; + let file = self.log_file(); + let failed = |error| { + io_error(format!( + "cannot create the {} {}", + file.description(), + file.path().display() + ))(error) + }; + file.remove_if_present()?; + let log = file.create_new().map_err(failed)?; platform::claim_log(&log).map_err(failed)?; let (device, index) = platform::opened_file_identity(&log).map_err(failed)?; let claim = LogClaim { @@ -992,7 +1022,7 @@ impl LockedStore { match self.runtime()? { None => self.end_launch(&journal), Some(runtime) if runtime.launch() == journal.launch() => { - remove_if_present(&self.path(JOURNAL_NAME))?; + self.journal_file().remove_if_present()?; Ok(Recovery::Clean) } Some(runtime) => Err(Error::io(format!( @@ -1012,14 +1042,13 @@ impl LockedStore { None => self.end_unidentified(journal), }; if !matches!(recovery, Recovery::Indeterminate(_)) { - for name in [CAPABILITY_NAME, SOCKET_NAME] { - remove_if_present(&self.path(name))?; - } + self.capability_file().remove_if_present()?; + remove_if_present(&self.socket_path())?; self.remove_clone_socket(journal.endpoint())?; self.remove_report_staging()?; // The journal goes last, so that an interrupted removal leaves it for a later // recovery to finish. - remove_if_present(&self.path(JOURNAL_NAME))?; + self.journal_file().remove_if_present()?; } Ok(recovery) } @@ -1143,14 +1172,13 @@ impl LockedStore { /// does, given `record`, the runtime record that the caller read under this lock, if any, whose /// control endpoint is the VM's. pub(crate) fn clear_runtime_of(&mut self, record: Option<&RuntimeRecord>) -> Result<()> { - for name in [CAPABILITY_NAME, SOCKET_NAME] { - remove_if_present(&self.path(name))?; - } + self.capability_file().remove_if_present()?; + remove_if_present(&self.socket_path())?; if let Some(record) = record { self.remove_clone_socket(record.endpoint())?; } self.remove_report_staging()?; - remove_if_present(&self.path(RUNTIME_NAME)) + self.runtime_file().remove_if_present() } /// Removes the control socket at `endpoint`, the control endpoint that a launch of the @@ -1276,7 +1304,7 @@ impl Launch<'_> { .process() .map_err(|error| Error::io("cannot identify the VMM process").with_source(error))?; let journal = self.journal.spawned(process.reference().clone()); - record::write(&self.locked.path(JOURNAL_NAME), &journal)?; + record::write(&self.locked.journal_file(), &journal)?; self.journal = journal; Ok(process) } @@ -1295,8 +1323,8 @@ impl Launch<'_> { )); }; let runtime = RuntimeRecord::committed(&self.journal, process.clone()); - record::write(&self.locked.path(RUNTIME_NAME), &runtime)?; - let _ = remove_if_present(&self.locked.path(JOURNAL_NAME)); + record::write(&self.locked.runtime_file(), &runtime)?; + let _ = self.locked.journal_file().remove_if_present(); Ok(runtime) } diff --git a/nvx-rs/src/host/store/record.rs b/nvx-rs/src/host/store/record.rs index 748c3e3b7..fe86eac4f 100644 --- a/nvx-rs/src/host/store/record.rs +++ b/nvx-rs/src/host/store/record.rs @@ -2,13 +2,12 @@ //! the identities that they hold, and their reading and writing in format [`FORMAT`]. use std::fmt; -use std::path::Path; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use super::FORMAT; -use super::files::{read_private, write_private}; +use super::files::StoreFile; use super::identity::{IdentityCheck, RuntimeIdentity}; use crate::error::{Error, ErrorCode, Result}; use crate::host::filesystem::FileIdentity; @@ -285,7 +284,7 @@ impl LaunchJournal { /// A record of the store, in format [`FORMAT`]. pub(super) trait Record: Serialize + DeserializeOwned { - /// What messages call the record. + /// What messages call the record, and the [`StoreFile`] that holds it. const DESCRIPTION: &'static str; /// Explains why the record, which parsed, is invalid, if it is. @@ -314,17 +313,17 @@ impl Record for LaunchJournal { } } -/// Reads the record at `path`, or returns `None` if the file does not exist. +/// Reads the record that `file` holds, or returns `None` if the file does not exist. /// /// A record of another format fails before its fields are read, with a message that names its /// format, and the code [`ErrorCode::FormatUnsupported`]: the store reads format [`FORMAT`] only, /// and migrates no other. A record file that is not private to the current user fails as well, /// since others could have written it, and so does one that is not a regular file, without waiting -/// on it. Every failure to read or check the record is an [`ErrorKind::Io`](crate::ErrorKind::Io) -/// error. -pub(super) fn read(path: &Path) -> Result> { - let description = T::DESCRIPTION; - let Some(text) = read_private(path, description)? else { +/// on it, as [`StoreFile::read`] reads it. Every failure to read or check the record is an +/// [`ErrorKind::Io`](crate::ErrorKind::Io) error. +pub(super) fn read(file: &StoreFile) -> Result> { + let (path, description) = (file.path(), file.description()); + let Some(text) = file.read()? else { return Ok(None); }; let malformed = || format!("the {description} {} is malformed", path.display()); @@ -355,11 +354,11 @@ pub(super) fn read(path: &Path) -> Result> { } } -/// Atomically replaces the record at `path` with `record`, as pretty JSON and a newline. -pub(super) fn write(path: &Path, record: &T) -> Result<()> { +/// Atomically replaces the record that `file` holds with `record`, as pretty JSON and a newline. +pub(super) fn write(file: &StoreFile, record: &T) -> Result<()> { let mut text = serde_json::to_vec_pretty(record).map_err(|error| { - Error::other(format!("cannot encode the {}", T::DESCRIPTION)).with_source(error) + Error::other(format!("cannot encode the {}", file.description())).with_source(error) })?; text.push(b'\n'); - write_private(path, &text) + file.replace(&text) } diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 6a34581c1..7b810d298 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -982,8 +982,8 @@ fn a_journal_that_its_runtime_record_supersedes_is_removed() { drop(launch); // The launch wrote its runtime record and crashed before it removed its journal. let committed = RuntimeRecord::committed(&journal, journal.process().unwrap().clone()); - record::write(&store.path(RUNTIME_NAME), &committed).unwrap(); - record::write(&store.path(JOURNAL_NAME), &journal).unwrap(); + record::write(&store.runtime_file(), &committed).unwrap(); + record::write(&store.journal_file(), &journal).unwrap(); assert_eq!(locked.recover().unwrap(), Recovery::Clean); assert!(store.journal().unwrap().is_none()); assert_eq!(store.runtime().unwrap(), Some(committed)); @@ -994,7 +994,7 @@ fn a_journal_that_its_runtime_record_supersedes_is_removed() { runtime(), *journal.log(), ); - record::write(&store.path(JOURNAL_NAME), &other).unwrap(); + record::write(&store.journal_file(), &other).unwrap(); let error = locked.recover().unwrap_err(); assert_eq!(error.kind(), ErrorKind::Io); assert!(error.message().contains("names launch"), "{error}");