diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index 95d82b55..8c33c136 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -81,6 +81,206 @@ fn dir_size(dir: &Path) -> u64 { total } +/// File name of the marker a preserved branch leaves in its storage dir. Lives +/// next to `upper/`, never inside it, so it is not part of the change set. +const PRESERVED_MARKER: &str = "PRESERVED"; + +/// Why a branch's private storage was preserved instead of reclaimed. +/// +/// Every preserved branch is storage that nothing in this process will free +/// again — see [`SeccompCowBranch::preserve`]. What it holds is the upper plus +/// the marker's deletions, together: see [`PreservedBranch`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PreserveReason { + /// A merge into the workdir was started and did not finish. The workdir may + /// be partially modified — the marker is written before the first + /// destructive step, so this is also what a merge still in flight looks + /// like — and the storage holds the part that had not landed. + MergeInterrupted, + /// The changes were complete and mergeable, but the merge never started — + /// the commit could not take the workdir lock in time. The workdir is + /// untouched and the storage holds the whole change set. + CommitDeferred, + /// The caller asked for the changes to be kept for inspection rather than + /// merged or discarded ([`crate::sandbox::BranchAction::Keep`]). + Kept, +} + +impl PreserveReason { + /// Stable token for this reason, as written into the on-disk marker. + fn as_token(self) -> &'static str { + match self { + PreserveReason::MergeInterrupted => "merge-interrupted", + PreserveReason::CommitDeferred => "commit-deferred", + PreserveReason::Kept => "kept", + } + } + + fn from_token(token: &[u8]) -> Option { + match token { + b"merge-interrupted" => Some(PreserveReason::MergeInterrupted), + b"commit-deferred" => Some(PreserveReason::CommitDeferred), + b"kept" => Some(PreserveReason::Kept), + _ => None, + } + } +} + +/// A branch whose storage was preserved, as read back off disk. +/// +/// This is what an out-of-band recovery works from: the process that created +/// the branch is gone, so the only thing tying an upper on disk to the workdir +/// it belongs to is the marker this was parsed from. +/// +/// A change set is the upper **and** [`deleted`](Self::deleted) together. +/// Deletions are tracked in RAM while the branch is live (there are no whiteout +/// entries in the upper), so recovering by copying the upper over the workdir +/// and nothing else would resurrect every file the run deleted; the marker +/// records them for exactly that reason. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PreservedBranch { + /// The branch's private storage dir, i.e. what to remove once recovered. + pub branch_dir: PathBuf, + /// The upper holding the preserved additions and modifications. + pub upper: PathBuf, + /// The workdir the changes belong to, canonicalized when the branch was + /// created. + pub workdir: PathBuf, + /// Paths the run deleted, relative to `workdir`, in sorted order. The other + /// half of the change set: nothing in `upper` represents them. + pub deleted: Vec, + /// Why it was preserved, which says what state the workdir is in. + pub reason: PreserveReason, + /// The process that preserved it. + /// + /// Load-bearing for one thing: a `MergeInterrupted` marker is written + /// *before* the merge, so a live merge and an interrupted one are the same + /// record and this pid is what tells them apart (see [`list_preserved`]). + /// Beyond that it is triage only — the process may have exited and the pid + /// may since have been reused. + pub pid: u32, +} + +/// Escape a path's raw bytes for the line-based marker format: a path may +/// legally contain a newline, and it need not be UTF-8, so the bytes go through +/// verbatim with `\` and `\n` escaped. +fn marker_escape(raw: &[u8]) -> Vec { + let mut out = Vec::with_capacity(raw.len()); + for &b in raw { + match b { + b'\\' => out.extend_from_slice(b"\\\\"), + b'\n' => out.extend_from_slice(b"\\n"), + _ => out.push(b), + } + } + out +} + +fn marker_unescape(raw: &[u8]) -> Vec { + let mut out = Vec::with_capacity(raw.len()); + let mut it = raw.iter().copied(); + while let Some(b) = it.next() { + if b != b'\\' { + out.push(b); + continue; + } + match it.next() { + Some(b'n') => out.push(b'\n'), + Some(other) => out.push(other), + None => out.push(b'\\'), + } + } + out +} + +/// Read the preservation marker of one branch storage dir, if it has one. +/// +/// `None` means the dir is not a preserved branch: either it is live storage of +/// a running process, or it was orphaned by something that never marked it. +pub fn read_preserved(branch_dir: &Path) -> Option { + use std::os::unix::ffi::OsStringExt; + + let body = fs::read(branch_dir.join(PRESERVED_MARKER)).ok()?; + let mut reason = None; + let mut workdir = None; + let mut upper = None; + let mut pid = None; + let mut deleted = Vec::new(); + for line in body.split(|&b| b == b'\n') { + let sep = match line.iter().position(|&b| b == b'=') { + Some(i) => i, + None => continue, + }; + let (key, value) = (&line[..sep], &line[sep + 1..]); + let path = || PathBuf::from(std::ffi::OsString::from_vec(marker_unescape(value))); + match key { + b"reason" => reason = PreserveReason::from_token(value), + b"workdir" => workdir = Some(path()), + b"upper" => upper = Some(path()), + // Repeated, one per deleted path — the only multi-valued key. + b"deleted" => deleted.push(path()), + b"pid" => pid = std::str::from_utf8(value).ok().and_then(|s| s.parse().ok()), + _ => {} + } + } + Some(PreservedBranch { + branch_dir: branch_dir.to_path_buf(), + upper: upper?, + workdir: workdir?, + deleted, + reason: reason?, + pid: pid?, + }) +} + +/// Enumerate every preserved branch directly under `storage_base` — the sweep +/// primitive for recovering work this process (or a previous one) could not +/// publish. +/// +/// `storage_base` is one `fs_storage` dir. With the default storage the base is +/// per-process (`$TMPDIR/sandlock-cow-`), so a sweep across process +/// lifetimes has to enumerate those bases itself; pass an explicit `fs_storage` +/// to keep every branch under one root. +/// +/// Unreadable entries are skipped rather than failing the sweep: one broken +/// branch dir must not hide the rest. +/// +/// **A merge that is still running looks exactly like one that was +/// interrupted.** `commit()` writes the [`PreserveReason::MergeInterrupted`] +/// marker before its first destructive step — it has to, or a crash mid-merge +/// would leave nothing to find — so for the duration of the merge the live +/// branch is listed here. The marker's `pid` is the only thing that separates +/// the two: a sweep that acts on a branch, rather than only reporting it, must +/// check that pid is not a live process first. +pub fn list_preserved(storage_base: &Path) -> Vec { + let mut found = Vec::new(); + if let Ok(rd) = fs::read_dir(storage_base) { + for entry in rd.flatten() { + if let Some(p) = read_preserved(&entry.path()) { + found.push(p); + } + } + } + found +} + +/// Disposition of a branch's private storage, which decides what `Drop` does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BranchState { + /// No disposition yet. The upper holds nothing the caller has asked to keep, + /// so dropping the branch reclaims it. + Open, + /// The upper holds changes that must outlive this branch, for the reason + /// carried here. The storage MUST survive `Drop`: it is the only copy of + /// those changes, and the only thing a retry (in this process) or a sweep + /// over [`list_preserved`] (after it is gone) can work from. Nothing frees + /// it automatically — see [`SeccompCowBranch::preserve`]. + Preserved(PreserveReason), + /// `commit()` or `abort()` completed. Nothing is left to reclaim — both + /// already removed the storage. + Finished, +} + /// Seccomp-based COW branch. Redirects writes to an upper directory /// and tracks deletions in memory. pub struct SeccompCowBranch { @@ -90,7 +290,11 @@ pub struct SeccompCowBranch { storage_dir: PathBuf, deleted: HashSet, has_changes: bool, - finished: bool, + state: BranchState, + /// What `Drop` does with a branch that was never disposed of: reclaim it + /// (the default) or preserve it. Set from `BranchAction::Keep`, whose + /// holder may never reach a disposition at all — see `Drop`. + keep_if_abandoned: bool, max_disk_bytes: u64, disk_used: u64, } @@ -107,12 +311,15 @@ impl SeccompCowBranch { let branch_dir = storage_base.join(&branch_id); let upper = branch_dir.join("upper"); - fs::create_dir_all(&upper) - .map_err(|e| BranchError::Operation(format!("create upper: {}", e)))?; - + // Canonicalize the workdir BEFORE creating the branch dir, so a failure + // here (e.g. the workdir was removed between validation and now) can't + // orphan an empty branch/upper dir on disk. let workdir = workdir.canonicalize() .map_err(|e| BranchError::Operation(format!("canonicalize workdir: {}", e)))?; + fs::create_dir_all(&upper) + .map_err(|e| BranchError::Operation(format!("create upper: {}", e)))?; + Ok(Self { workdir_str: workdir.to_string_lossy().into_owned(), workdir, @@ -120,7 +327,8 @@ impl SeccompCowBranch { storage_dir: branch_dir, deleted: HashSet::new(), has_changes: false, - finished: false, + state: BranchState::Open, + keep_if_abandoned: false, max_disk_bytes, disk_used: 0, }) @@ -511,13 +719,25 @@ impl SeccompCowBranch { /// Handle unlink/rmdir. /// /// Returns `Ok(true)` on success, `Ok(false)` if the path doesn't match, - /// or `Err(errno)` for filesystem errors (e.g. ENOTDIR when rmdir is - /// called on a non-directory, EISDIR when unlink is called on a directory). + /// or `Err(errno)` for filesystem errors: `ENOTDIR` when rmdir is called on + /// a non-directory, `EISDIR` when unlink is called on a directory, `EBUSY` + /// for the workdir root itself. + /// + /// Diverges from `rmdir(2)` in one destructive direction: `rmdir` on a + /// NON-EMPTY directory is accepted here and becomes a recursive delete at + /// commit time, where `rmdir(2)` would return `ENOTEMPTY`. pub fn handle_unlink(&mut self, path: &str, is_dir: bool) -> Result { let rel = match self.safe_rel(path) { Some(r) => r, None => return Ok(false), }; + // The workdir root is not a deletable entry. Recording it would put an + // empty relative path in `deleted`, and `commit()` would then empty the + // whole workdir through `remove_dir_all_in_root(wd, "")` and fail + // `EINVAL` on the root itself — permanently, on every retry. + if rel.is_empty() || rel == "." { + return Err(libc::EBUSY); + } let upper_file = self.upper.join(&rel); let lower_file = self.workdir.join(&rel); @@ -812,32 +1032,159 @@ impl SeccompCowBranch { } /// Commit: merge upper into workdir. + /// + /// The merge is file-by-file and not crash-atomic. If it fails + /// (`ENOSPC`, `EACCES`, an obstructing symlink, ...) the workdir may be left + /// partially merged and this returns `Err` — but the upper is **preserved**, + /// holding exactly what did not make it across: each change is dropped from + /// the upper as it lands, so after a failure `changes()` reports the + /// REMAINDER and not the whole run. Call `commit()` again to retry it once + /// the cause is cleared, or `abort()` to discard the remainder. Dropping the + /// branch after a failed commit does NOT reclaim it. + /// + /// Deletions are applied first, one at a time, and each is dropped from the + /// set as it lands; if any is still outstanding when they have all been + /// tried the commit fails there, before a single addition is copied. So a + /// failure on this side is not "the workdir is untouched" — every deletion + /// that could be applied already has been — it is "no addition was + /// published, and what is left in `deleted` is what still has to happen". + /// + /// `Ok(())` from a merge means every recorded change landed: the successful + /// tail removes the storage, so a change reported as merged but left behind + /// would have no copy anywhere. Two things the merge cannot carry across, + /// and so fail rather than claim: an entry whose name is not UTF-8, and a + /// workdir entry of the wrong type where the upper holds a directory. + /// + /// The short-circuit below is the exception, and it is not a merge. On a + /// [`BranchState::Finished`] branch there is nothing left to merge — the + /// storage is already gone — so `Ok(())` is an idempotent no-op. On a + /// [`PreserveReason::Kept`] branch it is a **caller error reported as + /// success**: the upper still holds the change set, `Ok(())` comes back, and + /// nothing lands. `Kept` means the holder deliberately took the storage over + /// for later inspection, so committing it afterwards is a contradiction the + /// caller has to resolve; today this code answers it by doing nothing + /// quietly, which is a wart worth fixing before the surface is public + /// (either exclude `Kept` from the short-circuit, or return an error). + /// Guarded by `a_kept_branch_reports_a_commit_it_did_not_perform`. + /// + /// The mode of each merged file is the upper's, not the destination's. + /// + /// A change is dropped from the upper only after its workdir side is in + /// place, so the failure mode of that bookkeeping is a change reported (and + /// re-merged) twice, never one silently lost. Re-merging is idempotent: the + /// copy truncates and the symlink is recreated. + /// + /// Entries are merged in sorted order, so a partial merge is a prefix of a + /// deterministic sequence rather than an arbitrary subset. pub fn commit(&mut self) -> Result<(), BranchError> { - if self.finished { return Ok(()); } - - // Apply deletions - for rel_path in &self.deleted { - let dest = self.workdir.join(rel_path); - if dest.is_dir() { - let _ = crate::sys::fs::remove_dir_all_in_root(&self.workdir, rel_path); - } else if dest.exists() || dest.is_symlink() { - let _ = crate::sys::fs::unlinkat_in_root(&self.workdir, rel_path, false); + if self.is_disposed() { return Ok(()); } + + // Enter the interrupted state BEFORE the first destructive operation, + // which also puts the marker on disk before the workdir is touched, so a + // crash mid-merge still leaves a sweep something to find. Every `?` + // below returns with the state still set, which is what keeps `Drop` + // from reclaiming an upper that holds unmerged data. Both are cleared + // only by the successful tail of this function, which removes the whole + // storage dir. + // + // The cost is that a merge in flight is indistinguishable on disk from + // one that was interrupted, for as long as it runs. That is the right + // way round — the alternative loses the crash — and the marker's pid is + // what a sweep uses to tell them apart (see `list_preserved`). + self.preserve(PreserveReason::MergeInterrupted); + + // Apply deletions, forgetting each one that is no longer outstanding so + // a retry (and `changes()`) sees only what is left to do. Whether the + // removal call succeeded is not the test — the entry being gone is — + // because a deletion of something the workdir no longer has is already + // applied. + let pending_deletions: Vec = self.deleted.iter().cloned().collect(); + let mut deletion_failure: Option = None; + for rel_path in pending_deletions { + let dest = self.workdir.join(&rel_path); + // Classify without dereferencing: `is_dir()` follows a symlink, so + // a symlink pointing at a directory was dispatched to the recursive + // remove, which then refused it with `ENOTDIR`. The guard below + // turned that into a permanent failure of the whole merge — the + // same errno on every retry, with no way past it. + let dest_kind = dest.symlink_metadata(); + let removal = if dest_kind.as_ref().map(|m| m.is_dir()).unwrap_or(false) { + crate::sys::fs::remove_dir_all_in_root(&self.workdir, &rel_path) + } else if dest_kind.is_ok() { + crate::sys::fs::unlinkat_in_root(&self.workdir, &rel_path, false) + } else { + Ok(()) + }; + if !dest.exists() && !dest.is_symlink() { + self.deleted.remove(&rel_path); + } else if deletion_failure.is_none() { + deletion_failure = Some(match removal { + Err(e) => format!("{}: errno {}", rel_path, e), + Ok(()) => format!("{}: still present after removal", rel_path), + }); } } + // A deletion left outstanding is a merge that did not happen. Stopping + // here — before a single entry is copied — is what keeps the ADDITIONS + // all-or-nothing: running on would publish them, and the successful + // tail would then remove the storage and destroy the record of the + // deletion that never landed. + // + // The deletions themselves are NOT all-or-nothing. The loop above + // applies each one in turn, so by the time this fires every deletion + // that could be applied already has been, and the workdir is not what + // it was before the commit. Only the ones still listed in `deleted` + // (and reported by `changes()`) are outstanding. + if !self.deleted.is_empty() { + let detail = deletion_failure.unwrap_or_else(|| "unknown".to_string()); + return Err(BranchError::Operation(format!( + "delete: {} deletion(s) could not be applied to the workdir, first: {}", + self.deleted.len(), + detail + ))); + } + + // Collect the entries before merging: the loop unlinks from the upper as + // it goes, and mutating a tree while walking it is not something walkdir + // promises to survive. + let walk = walkdir::WalkDir::new(&self.upper) + .min_depth(1) + .sort_by_file_name(); + let mut entries = Vec::new(); + for entry in walk { + entries.push(entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?); + } // Copy upper to workdir let mut synced_dirs = HashSet::new(); - for entry in walkdir::WalkDir::new(&self.upper).min_depth(1) { - let entry = entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?; + for entry in entries { let rel = entry.path().strip_prefix(&self.upper).unwrap(); let rel_str = match rel.to_str() { Some(s) => s, - None => continue, + // The confined merge helpers take a `&str`, so this entry + // cannot be merged. Skipping it and running on would reach the + // successful tail, which removes the storage — reporting + // `Ok(())` while destroying the only copy of the change. + None => { + return Err(BranchError::Operation(format!( + "copy: {} is not valid UTF-8 and cannot be merged", + rel.display() + ))) + } }; let dest = self.workdir.join(rel); if entry.file_type().is_dir() { crate::sys::fs::mkdirp_in_root(&self.workdir, rel_str, 0o755) .map_err(|e| BranchError::Operation(format!("mkdir: {}", e)))?; + // `mkdirp_in_root` reports `EEXIST` as success, so without this + // an entry of another type already at that path would swallow + // the whole subdirectory and still return `Ok(())`. + if !dest.symlink_metadata().map(|m| m.is_dir()).unwrap_or(false) { + return Err(BranchError::Operation(format!( + "mkdir: {} exists in the workdir and is not a directory", + rel_str + ))); + } } else if entry.file_type().is_symlink() { // Recreate the symlink verbatim. fs::copy would follow it and // read the target outside any root or Landlock (issue #112), @@ -854,6 +1201,7 @@ impl SeccompCowBranch { &target.to_string_lossy(), ) .map_err(|e| BranchError::Operation(format!("symlink: {}", e)))?; + self.drop_merged_entry(entry.path()); synced_dirs.insert(dest.parent().unwrap().to_path_buf()); } else { if let Some(p) = parent_rel(rel_str) { @@ -862,6 +1210,15 @@ impl SeccompCowBranch { // Source is the upper entry (supervisor-owned real path, safe to read directly). let mut src = fs::File::open(entry.path()) .map_err(|e| BranchError::Operation(format!("copy: {}", e)))?; + // The upper's mode is the run's intent: a copy-up carries the + // lower file's mode across, and a file the child created carries + // the mode the child asked for. The create mode below only + // applies to a destination that does not exist yet, and never to + // one being truncated, so the mode has to be set explicitly. + let src_mode = src + .metadata() + .ok() + .map(|m| std::os::unix::fs::PermissionsExt::mode(&m.permissions()) & 0o7777); let dst_fd = crate::sys::fs::openat2_in_root( &self.workdir, rel_str, @@ -872,6 +1229,14 @@ impl SeccompCowBranch { let mut dst = unsafe { fs::File::from_raw_fd(dst_fd) }; std::io::copy(&mut src, &mut dst) .map_err(|e| BranchError::Operation(format!("copy: {}", e)))?; + if let Some(mode) = src_mode { + // fchmod on the fd this merge opened — no second path + // resolution, so nothing can be swapped in underneath it. + dst.set_permissions(std::os::unix::fs::PermissionsExt::from_mode(mode)) + .map_err(|e| BranchError::Operation(format!("chmod: {}", e)))?; + } + drop((src, dst)); + self.drop_merged_entry(entry.path()); synced_dirs.insert(dest.parent().unwrap().to_path_buf()); } } @@ -884,134 +1249,897 @@ impl SeccompCowBranch { } self.cleanup(); - self.finished = true; + self.state = BranchState::Finished; Ok(()) } + /// Forget an upper entry that is now in the workdir, so what is left in the + /// upper is the unmerged remainder. + /// + /// Best-effort: if the unlink fails the entry stays and is merged again on a + /// retry, which is harmless — the alternative (assuming it is gone) would + /// drop a change that never landed. Directories are left in place; they are + /// not changes (`changes()` skips them) and removing them here would have to + /// wait for their contents anyway. + fn drop_merged_entry(&mut self, upper_path: &Path) { + if let Ok(meta) = upper_path.symlink_metadata() { + if fs::remove_file(upper_path).is_ok() { + self.disk_used = self.disk_used.saturating_sub(meta.len()); + } + } + } + /// Abort: discard all changes. + /// + /// After a failed `commit()` this is a deliberate request to throw the + /// unmerged remainder away; the workdir stays as the partial merge left it. pub fn abort(&mut self) -> Result<(), BranchError> { - if self.finished { return Ok(()); } + if self.is_disposed() { return Ok(()); } self.cleanup(); - self.finished = true; + self.state = BranchState::Finished; Ok(()) } + /// Mark the branch as intentionally kept: its upper is left on disk and the + /// `Drop` backstop below will not clean it up. Used for `BranchAction::Keep`, + /// which preserves the changes for later inspection rather than merging or + /// discarding them. + pub(crate) fn keep(&mut self) { + self.preserve(PreserveReason::Kept); + } + + /// Record that this branch's holder asked for `BranchAction::Keep`, so an + /// abandoned branch (never committed, aborted or kept) is preserved by + /// `Drop` instead of reclaimed. + /// + /// The holder that configured `Keep` may never run a disposition at all: a + /// `Sandbox` only moves its branch into its own `Drop` handler after a + /// completed `wait()`, and a sandbox abandoned before that is exactly the + /// case `Keep` exists for. Without this the branch's `Drop` would silently + /// override the request and delete the upper. + pub(crate) fn set_keep_if_abandoned(&mut self, keep: bool) { + self.keep_if_abandoned = keep; + } + + /// Hand the branch's private storage over to whoever recovers it: `Drop` + /// will not reclaim it and no other code path frees it either. + /// + /// This is a **deliberate leak** — the caller is asserting that the storage + /// holds the only copy of changes that must survive this process. Reclaiming + /// it is out-of-band work, so a marker naming the workdir, the upper, the + /// reason and this pid is written alongside the upper: without it a + /// preserved upper is indistinguishable from any orphaned one and a sweep + /// cannot tell which workdir it belongs to. Read it back with + /// [`read_preserved`] / [`list_preserved`]. + /// + /// The marker also carries the deletions. They live only in this struct's + /// `deleted` set while the branch is live — nothing in the upper represents + /// them — so without writing them down a preserved branch would be an upper + /// that resurrects every file the run deleted when it is recovered. + /// + /// Writing the marker is best-effort. If it fails the upper is still + /// preserved in this process — losing the data would be worse than losing + /// the record — but an out-of-band sweep will not find it. + pub(crate) fn preserve(&mut self, reason: PreserveReason) { + self.state = BranchState::Preserved(reason); + let _ = self.write_preserved_marker(reason); + } + + fn write_preserved_marker(&self, reason: PreserveReason) -> std::io::Result<()> { + use std::os::unix::ffi::OsStrExt; + + let mut body = Vec::new(); + body.extend_from_slice(b"reason="); + body.extend_from_slice(reason.as_token().as_bytes()); + body.extend_from_slice(b"\nworkdir="); + body.extend_from_slice(&marker_escape(self.workdir.as_os_str().as_bytes())); + body.extend_from_slice(b"\nupper="); + body.extend_from_slice(&marker_escape(self.upper.as_os_str().as_bytes())); + // One line per deletion, sorted so the marker is byte-stable for the + // same change set. This is the set as it stood when `preserve` was + // called — `commit()` calls it before applying any of them — so a + // recovery may re-apply a deletion that has since landed. That is a + // no-op, and it is the safe direction: the other one loses a deletion. + let mut deletions: Vec<&String> = self.deleted.iter().collect(); + deletions.sort(); + for rel in deletions { + body.extend_from_slice(b"\ndeleted="); + body.extend_from_slice(&marker_escape(rel.as_bytes())); + } + body.extend_from_slice(format!("\npid={}\n", std::process::id()).as_bytes()); + fs::write(self.storage_dir.join(PRESERVED_MARKER), body) + } + + /// Whether a further `commit()`/`abort()` would be a no-op: the storage is + /// either already gone or deliberately handed over to the caller. + fn is_disposed(&self) -> bool { + matches!( + self.state, + BranchState::Finished | BranchState::Preserved(PreserveReason::Kept) + ) + } + fn cleanup(&self) { let _ = fs::remove_dir_all(&self.storage_dir); } } +impl Drop for SeccompCowBranch { + /// Reclaims the branch's private storage when it was never disposed of. + /// + /// **Blast radius**: this applies to *every* holder of a `SeccompCowBranch`, + /// not only transactions. A `Sandbox` whose branch is abandoned without + /// `wait()` (or that panicked before its `Drop` ran a disposition) no longer + /// leaves its upper behind; scratch branches in tests likewise vanish at end + /// of scope. That is a behavior change, not a pure leak fix. The one thing it + /// deliberately does not override is an explicit `BranchAction::Keep`, which + /// the holder records with [`Self::set_keep_if_abandoned`] — "keep for later + /// inspection" has to survive the abandoned case, which *is* the forensic + /// case. + /// + /// It is deliberately **not** a "clean up on error" hook: anything the code + /// marked [`BranchState::Preserved`] holds changes that must outlive the + /// failure (see [`PreserveReason`]) and is kept. Only [`BranchState::Open`], + /// i.e. no disposition was ever attempted, reclaims here. + /// + /// `remove_dir_all` is idempotent and scoped to this branch's own uuid dir, + /// never to a caller-supplied `fs_storage` base. + fn drop(&mut self) { + if self.state == BranchState::Open { + if self.keep_if_abandoned { + self.preserve(PreserveReason::Kept); + } else { + self.cleanup(); + } + } + } +} + #[cfg(test)] mod tests { use super::*; use std::fs; - fn setup_workdir() -> (tempfile::TempDir, tempfile::TempDir) { + #[test] + fn drop_cleans_undisposed_branch_but_keep_preserves() { let workdir = tempfile::tempdir().unwrap(); - let storage = tempfile::tempdir().unwrap(); - // Create a test file in workdir - fs::write(workdir.path().join("existing.txt"), "hello").unwrap(); - fs::create_dir(workdir.path().join("subdir")).unwrap(); - fs::write(workdir.path().join("subdir/nested.txt"), "nested").unwrap(); - (workdir, storage) + + // A branch dropped without commit/abort/keep must remove its private + // storage dir — otherwise a failed/aborted-by-error transaction orphans + // the upper on disk (the leak this Drop backstop closes). + let leaked = { + let branch = SeccompCowBranch::create(workdir.path(), None, 0).unwrap(); + let dir = branch.storage_dir.clone(); + assert!(dir.exists()); + dir + }; + assert!(!leaked.exists(), "an undisposed branch must clean its storage on drop"); + + // keep() marks the branch finished, so Drop preserves the upper. + let kept = { + let mut branch = SeccompCowBranch::create(workdir.path(), None, 0).unwrap(); + let dir = branch.storage_dir.clone(); + branch.keep(); + dir + }; + assert!(kept.exists(), "a kept branch must survive drop"); + let _ = fs::remove_dir_all(&kept); } - #[test] - fn test_create_branch() { - let (workdir, storage) = setup_workdir(); - let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - assert!(branch.upper_dir().exists()); - assert!(!branch.has_changes()); + /// A deletion that could not be applied must FAIL the commit. Reporting + /// `Ok(())` here is worse than any other merge failure: it claims an + /// all-or-nothing merge that did not happen, and the successful tail then + /// removes the storage, so the record of the missing deletion is destroyed + /// along with the change set. + /// + /// The failure is injected the way it actually happens in the field, with no + /// permission games (so it fails as intended when the suite runs as root): a + /// symlinked parent component in the workdir. The child unlinked + /// `link/x.txt`, which the COW layer recorded as a deletion; applying it goes + /// through the confined `unlinkat`, which resolves `link` inside the workdir + /// root (issue #112) and so does not reach the file the host path names. + #[test] + fn commit_fails_when_a_deletion_could_not_be_applied() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + fs::write(outside.path().join("x.txt"), "survives").unwrap(); + std::os::unix::fs::symlink(outside.path(), workdir.path().join("link")).unwrap(); + + let storage_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("link/x.txt"); + + let err = branch + .commit() + .expect_err("a deletion that was not applied must fail the commit"); + assert!( + matches!(err, BranchError::Operation(ref m) if m.starts_with("delete:")), + "expected the deletion step to fail, got: {err:?}" + ); + assert!( + branch.is_deleted("link/x.txt"), + "an unapplied deletion must stay outstanding so a retry still sees it" + ); + } + + // All-or-nothing: the merge stopped before copying anything across. + assert!( + !workdir.path().join("added.txt").exists(), + "a commit that failed on a deletion must not have merged the additions" + ); + // ...and the change set survives the drop, marked for recovery. + assert!( + storage_dir.join("upper").join("added.txt").exists(), + "the unmerged change set must be preserved, not destroyed by a bogus success" + ); + assert!( + read_preserved(&storage_dir).is_some(), + "the preserved branch must be findable by an out-of-band sweep" + ); } + /// A commit that fails partway must PRESERVE the upper: the workdir is + /// already partially merged, so the unmerged remainder in the upper is the + /// only copy of the outstanding data and the only thing a retry or an + /// out-of-band recovery can work from. Dropping the branch after such a + /// failure must not reclaim it either. + /// + /// The failure is injected the way it actually happens in the field: the + /// workdir holds a symlink where the upper holds a regular file, so the + /// merge's `openat2(O_NOFOLLOW)` fails with `ELOOP`. No permission games, so + /// this also fails as intended when the suite runs as root. #[test] - fn test_matches() { - let (workdir, storage) = setup_workdir(); - let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let wdstr = workdir.path().canonicalize().unwrap(); - let wdstr = wdstr.to_str().unwrap(); - assert!(branch.matches(&format!("{}/foo.txt", wdstr))); - assert!(branch.matches(wdstr)); - assert!(!branch.matches("/tmp/other")); + fn failed_commit_preserves_the_unmerged_upper() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + // The obstruction: a symlink in the workdir at the path the merge will + // try to write a regular file to. + std::os::unix::fs::symlink("/dev/null", workdir.path().join("blocked.txt")).unwrap(); + + let storage_dir; + let upper_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + storage_dir = branch.storage_dir.clone(); + upper_dir = branch.upper.clone(); + fs::write(upper_dir.join("blocked.txt"), "unmerged payload").unwrap(); + + let err = branch.commit().expect_err("the obstructed merge must fail"); + assert!( + matches!(err, BranchError::Operation(ref m) if m.starts_with("copy:")), + "expected the copy step to fail, got: {err:?}" + ); + // Still on disk WHILE the branch is alive... + assert!(upper_dir.join("blocked.txt").exists()); + } + // ...and still on disk AFTER the drop. This is the regression that + // matters: reclaiming here destroys the only copy of the remainder. + assert!( + storage_dir.exists(), + "a branch whose commit failed must keep its storage after drop" + ); + assert_eq!( + fs::read_to_string(upper_dir.join("blocked.txt")).unwrap(), + "unmerged payload", + "the unmerged remainder must survive intact" + ); } + /// Because a failed commit does not latch the branch as finished, clearing + /// the cause and calling `commit()` again completes the merge. A guard that + /// simply marked the branch finished on entry would turn the retry into a + /// silent no-op that reports success. #[test] - fn test_ensure_cow_copy() { - let (workdir, storage) = setup_workdir(); + fn commit_is_retryable_after_a_failed_merge() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/dev/null", workdir.path().join("blocked.txt")).unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let upper = branch.ensure_cow_copy("existing.txt").unwrap(); - assert!(upper.exists()); - assert_eq!(fs::read_to_string(&upper).unwrap(), "hello"); - assert!(branch.has_changes()); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("blocked.txt"), "payload").unwrap(); + branch.commit().expect_err("the obstructed merge must fail"); + + // Clear the obstruction and retry. + fs::remove_file(workdir.path().join("blocked.txt")).unwrap(); + branch.commit().expect("the retry must complete the merge"); + + assert_eq!( + fs::read_to_string(workdir.path().join("blocked.txt")).unwrap(), + "payload", + "the retried commit must actually merge the remainder" + ); + assert!(!storage_dir.exists(), "a completed commit reclaims its storage"); } + /// After a partial merge the upper must hold the REMAINDER, not the whole + /// run: `changes()` is what an operator recovering a half-merged workdir + /// reads to find out what is still outstanding, and it walks the upper. So + /// each change has to leave the upper as it lands — otherwise a 2-of-3 merge + /// reports the same three changes as a 0-of-3 merge and the answer is + /// useless. #[test] - fn test_resolve_read_prefers_upper() { - let (workdir, storage) = setup_workdir(); + fn a_partial_merge_leaves_only_the_remainder_in_the_upper() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + // Merged in sorted order: a.txt lands, b.txt hits the obstruction + // (symlink vs regular file → ELOOP under O_NOFOLLOW), c.txt is never + // reached. + std::os::unix::fs::symlink("/dev/null", workdir.path().join("b.txt")).unwrap(); + fs::write(workdir.path().join("gone.txt"), "delete me").unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let upper = branch.ensure_cow_copy("existing.txt").unwrap(); - fs::write(&upper, "modified").unwrap(); - let resolved = branch.resolve_read("existing.txt"); - assert_eq!(fs::read_to_string(&resolved).unwrap(), "modified"); + for name in ["a.txt", "b.txt", "c.txt"] { + fs::write(branch.upper.join(name), name).unwrap(); + } + branch.mark_deleted("gone.txt"); + + branch.commit().expect_err("the obstructed merge must fail"); + + assert_eq!( + fs::read_to_string(workdir.path().join("a.txt")).unwrap(), + "a.txt", + "a.txt was merged before the failure", + ); + assert!(!branch.upper.join("a.txt").exists(), "a merged change must leave the upper"); + assert!( + !workdir.path().join("gone.txt").exists(), + "the deletion was applied before the failure", + ); + + let mut outstanding: Vec<(crate::dry_run::ChangeKind, String)> = branch + .changes() + .unwrap() + .into_iter() + .map(|c| (c.kind, c.path.display().to_string())) + .collect(); + outstanding.sort_by(|a, b| a.1.cmp(&b.1)); + assert_eq!( + outstanding, + vec![ + // b.txt is "modified" because the obstructing symlink is still + // there in the workdir; c.txt was never reached. + (crate::dry_run::ChangeKind::Modified, "b.txt".to_string()), + (crate::dry_run::ChangeKind::Added, "c.txt".to_string()), + ], + "changes() after a partial merge must report the remainder only", + ); + + // And the retry finishes exactly that remainder. + fs::remove_file(workdir.path().join("b.txt")).unwrap(); + branch.commit().expect("the retry must complete the merge"); + assert_eq!(fs::read_to_string(workdir.path().join("b.txt")).unwrap(), "b.txt"); + assert_eq!(fs::read_to_string(workdir.path().join("c.txt")).unwrap(), "c.txt"); } + /// Preserving is only half a guarantee if it lives in RAM: once the process + /// is gone a preserved upper is indistinguishable from any orphaned one and + /// nothing says which workdir it belongs to. A sweep must be able to find it + /// on disk, with the workdir, the reason and the payload. #[test] - fn test_is_deleted() { - let (workdir, storage) = setup_workdir(); - let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - assert!(!branch.is_deleted("existing.txt")); - branch.mark_deleted("existing.txt"); - assert!(branch.is_deleted("existing.txt")); + fn a_preserved_branch_is_findable_on_disk_after_its_process_forgets_it() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/dev/null", workdir.path().join("blocked.txt")).unwrap(); + + assert!( + list_preserved(storage.path()).is_empty(), + "nothing is preserved before anything has run" + ); + + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("blocked.txt"), "unmerged payload").unwrap(); + branch.commit().expect_err("the obstructed merge must fail"); + // Dropped here: everything the process knew about this branch is gone. + } + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1, "the sweep must find the preserved branch, got {found:?}"); + let b = &found[0]; + assert_eq!( + b.reason, + PreserveReason::MergeInterrupted, + "the marker must say what state the workdir is in", + ); + assert_eq!( + b.workdir, + workdir.path().canonicalize().unwrap(), + "the marker must name the workdir the changes belong to", + ); + assert_eq!(b.pid, std::process::id()); + assert_eq!( + fs::read_to_string(b.upper.join("blocked.txt")).unwrap(), + "unmerged payload", + "the sweep must reach the preserved payload through the marker", + ); + + // A branch that WAS disposed of leaves nothing behind for the sweep. + let mut clean = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(clean.upper.join("fine.txt"), "merged").unwrap(); + clean.abort().unwrap(); + assert_eq!( + list_preserved(storage.path()).len(), + 1, + "an aborted branch must not show up as work awaiting recovery", + ); } - #[test] - fn test_commit_merges_upper() { - let (workdir, storage) = setup_workdir(); + /// A preserved branch must carry the DELETIONS as well as the upper. + /// + /// Deletions live only in the branch's in-RAM `deleted` set — there are no + /// whiteout entries in the upper — so a recovery that reads the upper alone + /// resurrects every file the run deleted. That is the worst case of all, + /// `commit()` on a `Kept` branch answers `Ok(())` without merging anything. + /// + /// This pins a WART, not a guarantee: `Ok(())` from `commit()` otherwise + /// means the whole change set landed, and here it means the opposite — the + /// upper still holds every byte. `is_disposed()` covers + /// `Preserved(Kept)`, so the short-circuit at the top of `commit()` fires + /// before any merge work. The test exists so the wart cannot be lost: + /// whichever way it is resolved (excluding `Kept` from the short-circuit, or + /// returning an error), this test must be updated deliberately rather than + /// keep passing by accident. + #[test] + fn a_kept_branch_reports_a_commit_it_did_not_perform() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - // Write a new file via COW - let upper = branch.ensure_cow_copy("new.txt").unwrap(); - fs::write(&upper, "new content").unwrap(); - branch.commit().unwrap(); - assert_eq!(fs::read_to_string(workdir.path().join("new.txt")).unwrap(), "new content"); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + + branch.keep(); + assert_eq!(branch.state, BranchState::Preserved(PreserveReason::Kept)); + + // Reported as a successful commit... + branch + .commit() + .expect("the short-circuit reports success on a Kept branch"); + + // ...while nothing was merged and the change set is still in the upper. + assert!( + !workdir.path().join("added.txt").exists(), + "commit() on a Kept branch must not be believed: it published nothing" + ); + assert!( + branch.upper.join("added.txt").exists(), + "the Kept branch still holds the whole change set" + ); + assert_eq!( + branch.state, + BranchState::Preserved(PreserveReason::Kept), + "the short-circuit must not move a Kept branch to Finished" + ); } + /// because `TxnError::Merge` tells the operator that recovering the + /// preserved storage IS how the transaction gets finished. #[test] - fn test_commit_applies_deletions() { - let (workdir, storage) = setup_workdir(); - let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - branch.mark_deleted("existing.txt"); - branch.commit().unwrap(); - assert!(!workdir.path().join("existing.txt").exists()); + fn a_preserved_branch_carries_its_deletions_not_only_its_upper() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("keep.txt"), "still here").unwrap(); + + // The commit-lock path: the whole change set is complete and NONE of it + // has been applied, so a recovery that only copies the upper over the + // workdir leaves keep.txt behind — a file the run deleted. + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("keep.txt"); + branch.mark_deleted("sub/also gone.txt"); + branch.preserve(PreserveReason::CommitDeferred); + } + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1, "the sweep must find the preserved branch, got {found:?}"); + assert!( + found[0].upper.join("added.txt").exists(), + "the additions are the half that lives in the upper", + ); + assert_eq!( + found[0].deleted, + vec![PathBuf::from("keep.txt"), PathBuf::from("sub/also gone.txt")], + "recovering the preserved branch must not resurrect what the run deleted", + ); } + /// The marker is a line-based format holding paths, and a path may contain a + /// newline (and need not be UTF-8). Round-trip one so the format cannot be + /// silently broken by a legal workdir name — including a DELETED path, which + /// is a name the child chose and so is even less constrained. #[test] - fn test_abort_discards_changes() { - let (workdir, storage) = setup_workdir(); + fn the_preserved_marker_round_trips_a_deleted_path_with_a_newline() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/dev/null", workdir.path().join("blocked.txt")).unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let upper = branch.ensure_cow_copy("new.txt").unwrap(); - fs::write(&upper, "should be discarded").unwrap(); - branch.abort().unwrap(); - assert!(!workdir.path().join("new.txt").exists()); + fs::write(branch.upper.join("blocked.txt"), "payload").unwrap(); + branch.mark_deleted("we\nird\\name.txt"); + branch.commit().expect_err("the obstructed merge must fail"); + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1); + assert_eq!( + found[0].deleted, + vec![PathBuf::from("we\nird\\name.txt")], + "a deleted path with a newline and a backslash must survive the round-trip", + ); } + /// The marker is a line-based format holding paths, and a path may contain a + /// newline (and need not be UTF-8). Round-trip one so the format cannot be + /// silently broken by a legal workdir name. #[test] - fn test_changes_added_file() { - let (workdir, storage) = setup_workdir(); - let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let upper = branch.ensure_cow_copy("brand_new.txt").unwrap(); - fs::write(&upper, "new content").unwrap(); - let changes = branch.changes().unwrap(); - assert_eq!(changes.len(), 1); - assert_eq!(changes[0].kind, crate::dry_run::ChangeKind::Added); - assert_eq!(changes[0].path, std::path::PathBuf::from("brand_new.txt")); + fn the_preserved_marker_round_trips_a_path_with_a_newline() { + let root = tempfile::tempdir().unwrap(); + let workdir = root.path().join("we\nird dir"); + fs::create_dir(&workdir).unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/dev/null", workdir.join("blocked.txt")).unwrap(); + + let mut branch = SeccompCowBranch::create(&workdir, Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("blocked.txt"), "payload").unwrap(); + branch.commit().expect_err("the obstructed merge must fail"); + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1); + assert_eq!( + found[0].workdir, + workdir.canonicalize().unwrap(), + "a workdir path with a newline must survive the marker round-trip", + ); } + /// `matches()` is the gate on every interception (`cow::dispatch` returns + /// `Continue` when it says no), and it must say no to the branch's own + /// storage. With `fs_storage` inside the workdir the upper is itself under + /// the workdir prefix, so without the storage exclusion an access to + /// `/f` would be treated as a workdir path and copied up again into + /// `/.cow//upper/f`. #[test] - fn test_changes_modified_file() { - let (workdir, storage) = setup_workdir(); - let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let upper = branch.ensure_cow_copy("existing.txt").unwrap(); - fs::write(&upper, "modified content").unwrap(); + fn matches_excludes_the_branch_storage_that_lives_under_the_workdir() { + let workdir = tempfile::tempdir().unwrap(); + let wd = workdir.path().canonicalize().unwrap(); + let storage = wd.join(".cow"); + fs::create_dir(&storage).unwrap(); + + let branch = SeccompCowBranch::create(&wd, Some(&storage), 0).unwrap(); + + assert!( + branch.matches(&abs(&branch, "existing.txt")), + "a plain workdir path is what the branch is there to intercept", + ); + let upper_file = branch.upper_dir().join("existing.txt"); + assert!( + !branch.matches(upper_file.to_str().unwrap()), + "the branch's own upper must not be intercepted as a workdir path", + ); + assert!( + !branch.matches(branch.storage_dir.to_str().unwrap()), + "the branch's own storage dir must not be intercepted either", + ); + } + + /// Nothing outside the workdir may be mapped into the upper. `safe_rel` is + /// the only thing standing between a host path and a COW copy of it, so it + /// must reject both an ordinary escape and the string-prefix trap — a + /// sibling directory whose name merely extends the workdir's, which a + /// non-component-wise prefix test would swallow whole. + #[test] + fn a_path_outside_the_workdir_is_neither_mapped_nor_intercepted() { + let root = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let workdir = root.path().join("wd"); + let sibling = root.path().join("wd-extra"); + fs::create_dir(&workdir).unwrap(); + fs::create_dir(&sibling).unwrap(); + fs::write(sibling.join("secret.txt"), "host bytes").unwrap(); + + let mut branch = SeccompCowBranch::create(&workdir, Some(storage.path()), 0).unwrap(); + let escape = sibling.canonicalize().unwrap().join("secret.txt"); + let escape = escape.to_str().unwrap(); + + assert_eq!( + branch.safe_rel(escape), + None, + "a sibling that merely shares the workdir's name prefix is outside the workdir", + ); + assert!( + !branch.matches(escape), + "a path outside the workdir must not be intercepted at all", + ); + + // ...and a write open of it is left to the kernel: no relative path, no + // copy-up, and the branch does not claim to hold a change. + assert!( + branch.handle_open(escape, O_WRONLY).unwrap().is_none(), + "a write outside the workdir must not be redirected into the upper", + ); + assert!(!branch.has_changes(), "nothing in the workdir was changed"); + assert!( + !branch.upper_dir().join("secret.txt").exists() + && !branch.upper_dir().join("../wd-extra/secret.txt").exists(), + "the outside file must not have been copied up", + ); + } + + /// One branch dir a sweep cannot parse must not hide the rest: a marker is + /// written by a process that may be killed mid-write, and a storage base + /// also holds the live storage of running branches, which have no marker at + /// all. Either one aborting the sweep would strand every preserved change + /// set beside it. + #[test] + fn a_branch_dir_the_sweep_cannot_parse_does_not_hide_the_ones_beside_it() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + // Live storage of a running branch: no marker. + fs::create_dir_all(storage.path().join("live/upper")).unwrap(); + // A marker cut short before the `upper=` line was written. + fs::create_dir(storage.path().join("truncated")).unwrap(); + fs::write( + storage.path().join("truncated").join(PRESERVED_MARKER), + b"reason=kept\nworkdir=/some/workdir\npid=1\n".as_slice(), + ) + .unwrap(); + // Not a branch dir at all. + fs::write(storage.path().join("stray-file"), "junk").unwrap(); + + let preserved_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + preserved_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.preserve(PreserveReason::CommitDeferred); + } + + assert!( + read_preserved(&storage.path().join("truncated")).is_none(), + "a marker missing the upper it points at is not a recoverable branch", + ); + assert!( + read_preserved(&storage.path().join("live")).is_none(), + "a branch that never marked itself is not awaiting recovery", + ); + + let found: Vec = list_preserved(storage.path()) + .into_iter() + .map(|p| p.branch_dir) + .collect(); + assert_eq!( + found, + vec![preserved_dir], + "the sweep must report the parseable branch and only that one", + ); + } + + /// A branch kept for inspection must read back as [`PreserveReason::Kept`]: + /// the reason is what tells a recovery what state the workdir is in, and + /// `Kept` is the one that says the workdir was never touched and nothing is + /// owed to it. Reading it back as an interrupted merge would send an + /// operator looking for a half-merged workdir that does not exist. + #[test] + fn a_kept_branch_reads_back_as_kept_with_its_whole_change_set() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("gone.txt"), "still here").unwrap(); + + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("gone.txt"); + branch.keep(); + } + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1, "the kept branch must be findable, got {found:?}"); + assert_eq!( + found[0].reason, + PreserveReason::Kept, + "the marker must say the changes were kept, not that a merge was interrupted", + ); + assert_eq!( + fs::read_to_string(found[0].upper.join("added.txt")).unwrap(), + "payload", + "the additions must be reachable through the marker", + ); + assert_eq!( + found[0].deleted, + vec![PathBuf::from("gone.txt")], + "the deletions are the half of the change set that lives only in the marker", + ); + // Keep merges nothing: the workdir is exactly as the run found it. + assert!(workdir.path().join("gone.txt").exists()); + assert!(!workdir.path().join("added.txt").exists()); + } + + /// `keep()` hands the storage to the caller, so a later `abort()` must not + /// throw it away. `abort()` normally means "discard the changes", and a + /// holder that runs one after the other — a disposition followed by a + /// blanket cleanup — would otherwise destroy the only copy of the change + /// set that was explicitly kept for inspection. + #[test] + fn abort_after_keep_does_not_destroy_the_kept_change_set() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.keep(); + + branch.abort().unwrap(); + + assert!( + branch.upper.join("added.txt").exists(), + "abort must not discard a change set that was already kept", + ); + assert_eq!( + fs::read_to_string(branch.upper.join("added.txt")).unwrap(), + "payload", + "the kept change set must survive intact", + ); + assert_eq!( + list_preserved(storage.path()).len(), + 1, + "the kept branch must still be findable by a sweep after the abort", + ); + } + + /// The marker's reason describes the state of the WORKDIR, so a second + /// `preserve()` has to overwrite the first record rather than leave the + /// stale one on disk. + /// + /// A commit that could not take the workdir lock preserves as + /// `CommitDeferred` — the workdir is untouched — and the branch stays + /// committable. If the retry then merges part way and fails, the workdir is + /// half merged; a sweep still reading `commit-deferred` would recover it as + /// though nothing had landed. + #[test] + fn a_second_preserve_replaces_the_reason_recorded_on_disk() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + // The obstruction that fails the merge: a symlink in the workdir where + // the upper holds a regular file (ELOOP under O_NOFOLLOW). + std::os::unix::fs::symlink("/dev/null", workdir.path().join("blocked.txt")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("blocked.txt"), "payload").unwrap(); + + branch.preserve(PreserveReason::CommitDeferred); + assert_eq!( + read_preserved(&storage_dir).unwrap().reason, + PreserveReason::CommitDeferred, + "the deferred commit left the workdir untouched", + ); + + branch.commit().expect_err("the obstructed merge must fail"); + + assert_eq!( + read_preserved(&storage_dir).unwrap().reason, + PreserveReason::MergeInterrupted, + "once the merge has run, the marker must say the workdir may be partial", + ); + } + + fn setup_workdir() -> (tempfile::TempDir, tempfile::TempDir) { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + // Create a test file in workdir + fs::write(workdir.path().join("existing.txt"), "hello").unwrap(); + fs::create_dir(workdir.path().join("subdir")).unwrap(); + fs::write(workdir.path().join("subdir/nested.txt"), "nested").unwrap(); + (workdir, storage) + } + + #[test] + fn test_create_branch() { + let (workdir, storage) = setup_workdir(); + let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + assert!(branch.upper_dir().exists()); + assert!(!branch.has_changes()); + } + + #[test] + fn test_matches() { + let (workdir, storage) = setup_workdir(); + let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wdstr = workdir.path().canonicalize().unwrap(); + let wdstr = wdstr.to_str().unwrap(); + assert!(branch.matches(&format!("{}/foo.txt", wdstr))); + assert!(branch.matches(wdstr)); + assert!(!branch.matches("/tmp/other")); + } + + #[test] + fn test_ensure_cow_copy() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.ensure_cow_copy("existing.txt").unwrap(); + assert!(upper.exists()); + assert_eq!(fs::read_to_string(&upper).unwrap(), "hello"); + assert!(branch.has_changes()); + } + + #[test] + fn test_resolve_read_prefers_upper() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.ensure_cow_copy("existing.txt").unwrap(); + fs::write(&upper, "modified").unwrap(); + let resolved = branch.resolve_read("existing.txt"); + assert_eq!(fs::read_to_string(&resolved).unwrap(), "modified"); + } + + #[test] + fn test_is_deleted() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + assert!(!branch.is_deleted("existing.txt")); + branch.mark_deleted("existing.txt"); + assert!(branch.is_deleted("existing.txt")); + } + + #[test] + fn test_commit_merges_upper() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + // Write a new file via COW + let upper = branch.ensure_cow_copy("new.txt").unwrap(); + fs::write(&upper, "new content").unwrap(); + branch.commit().unwrap(); + assert_eq!(fs::read_to_string(workdir.path().join("new.txt")).unwrap(), "new content"); + } + + #[test] + fn test_commit_applies_deletions() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("existing.txt"); + branch.commit().unwrap(); + assert!(!workdir.path().join("existing.txt").exists()); + } + + #[test] + fn test_abort_discards_changes() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.ensure_cow_copy("new.txt").unwrap(); + fs::write(&upper, "should be discarded").unwrap(); + branch.abort().unwrap(); + assert!(!workdir.path().join("new.txt").exists()); + } + + #[test] + fn test_changes_added_file() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.ensure_cow_copy("brand_new.txt").unwrap(); + fs::write(&upper, "new content").unwrap(); + let changes = branch.changes().unwrap(); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, crate::dry_run::ChangeKind::Added); + assert_eq!(changes[0].path, std::path::PathBuf::from("brand_new.txt")); + } + + #[test] + fn test_changes_modified_file() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.ensure_cow_copy("existing.txt").unwrap(); + fs::write(&upper, "modified content").unwrap(); let changes = branch.changes().unwrap(); assert_eq!(changes.len(), 1); assert_eq!(changes[0].kind, crate::dry_run::ChangeKind::Modified); @@ -1705,4 +2833,920 @@ mod tests { "upper write escaped through symlinked parent into the outside dir" ); } + + // ---- Deletions: what the merge does with each shape of workdir entry ---- + + /// A deletion of a symlink that points at a directory must unlink the LINK + /// and leave the directory alone. + /// + /// `is_dir()` follows the link, so classifying the deletion with it sent a + /// symlink-to-a-directory down the recursive-remove path, which refused it + /// with `ENOTDIR`. That is not a transient failure: the deletion stays + /// outstanding, the guard fails the whole merge, and every retry produces + /// the identical errno. `mv ld renamed` on a workdir holding such a link is + /// enough to reach it. + #[test] + fn a_deletion_of_a_symlink_to_a_directory_unlinks_the_link_not_the_directory() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::create_dir(workdir.path().join("d")).unwrap(); + fs::write(workdir.path().join("d/inner.txt"), "must survive").unwrap(); + std::os::unix::fs::symlink("d", workdir.path().join("ld")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("ld"); + branch + .commit() + .expect("deleting a symlink to a directory must not fail the merge"); + + assert!( + !workdir.path().join("ld").is_symlink(), + "the symlink itself must be gone", + ); + assert!( + workdir.path().join("d").is_dir() && workdir.path().join("d/inner.txt").exists(), + "the directory the link pointed at must be untouched, with its contents", + ); + } + + /// A deletion of a dangling symlink, and of a symlink to a regular file, + /// must remove the LINK without touching (or needing) the target. + /// + /// Both are decided by the `symlink_metadata` classification: dereferencing + /// anywhere in that chain either makes a dangling link permanently + /// unappliable, or deletes the target instead of the link. + #[test] + fn a_deletion_of_a_symlink_removes_the_link_and_never_its_target() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("target.txt"), "must survive").unwrap(); + std::os::unix::fs::symlink("target.txt", workdir.path().join("to_file")).unwrap(); + std::os::unix::fs::symlink("nowhere", workdir.path().join("dangling")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("to_file"); + branch.mark_deleted("dangling"); + branch.commit().expect("both symlink deletions must apply"); + + assert!(!workdir.path().join("to_file").is_symlink(), "the link to a file must be gone"); + assert!(!workdir.path().join("dangling").is_symlink(), "the dangling link must be gone"); + assert_eq!( + fs::read_to_string(workdir.path().join("target.txt")).unwrap(), + "must survive", + "the link's target is not part of the deletion", + ); + } + + /// A deletion at a nested path removes that entry in place and leaves every + /// parent directory standing. Only the root-level shape was exercised + /// before, so a merge that removed the parent instead would have gone + /// unnoticed. + #[test] + fn a_nested_deletion_removes_only_the_entry_not_its_parents() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::create_dir_all(workdir.path().join("sub/dir")).unwrap(); + fs::write(workdir.path().join("sub/dir/file.txt"), "doomed").unwrap(); + fs::write(workdir.path().join("sub/sibling.txt"), "survives").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("sub/dir/file.txt"); + branch.commit().expect("a nested deletion must apply"); + + assert!(!workdir.path().join("sub/dir/file.txt").exists(), "the entry must be gone"); + assert!(workdir.path().join("sub/dir").is_dir(), "its parent must stay"); + assert!( + workdir.path().join("sub/sibling.txt").exists(), + "a sibling under the same parent must be untouched", + ); + } + + /// A deletion of something the workdir no longer has is ALREADY APPLIED: it + /// is dropped from the set and the commit succeeds. + /// + /// The test is "is the entry gone", not "did the removal call succeed" — + /// which is what makes a retry after a partly-applied merge converge + /// instead of failing forever on the deletions that landed the first time. + #[test] + fn a_deletion_of_something_already_absent_counts_as_applied() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("never-existed.txt"); + branch + .commit() + .expect("a deletion of an absent path is already applied"); + assert!( + !branch.is_deleted("never-existed.txt"), + "an applied deletion must be dropped from the set, or a retry re-runs it forever", + ); + } + + /// A directory and everything recorded under it must all apply, in whatever + /// order they come out of the set. + /// + /// `deleted` is a `HashSet`, so the iteration order genuinely varies per + /// run; only order-INDEPENDENCE is assertable. It holds because each entry + /// is tested for absence rather than for a successful removal — the + /// recursive delete of the parent takes the children with it, and they are + /// then already applied whichever way round they are visited. + #[test] + fn deletions_of_a_directory_and_its_children_apply_whatever_the_order() { + for _ in 0..8 { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::create_dir_all(workdir.path().join("d/e")).unwrap(); + fs::write(workdir.path().join("d/e/f.txt"), "doomed").unwrap(); + + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("d"); + branch.mark_deleted("d/e"); + branch.mark_deleted("d/e/f.txt"); + branch + .commit() + .expect("overlapping deletions must apply in any order"); + assert!(!workdir.path().join("d").exists(), "the whole subtree must be gone"); + } + } + + /// Deletions run before additions, so `rm -rf d` followed by writing + /// `d/new.txt` in the same run publishes `new.txt` into a directory that no + /// longer holds the stale contents. + /// + /// This ordering is the only thing that makes the sequence work; run the + /// additions first and the recursive delete takes the new file with it. + #[test] + fn deletions_are_applied_before_additions_at_the_same_path() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::create_dir(workdir.path().join("d")).unwrap(); + fs::write(workdir.path().join("d/stale.txt"), "from a previous run").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + branch.mark_deleted("d"); + fs::create_dir_all(branch.upper.join("d")).unwrap(); + fs::write(branch.upper.join("d/new.txt"), "fresh").unwrap(); + branch.commit().expect("delete-then-recreate must merge"); + + assert!( + !workdir.path().join("d/stale.txt").exists(), + "the deletion must have run before the addition re-created the directory", + ); + assert_eq!( + fs::read_to_string(workdir.path().join("d/new.txt")).unwrap(), + "fresh", + "the addition must survive the deletion of its parent", + ); + } + + /// When one deletion cannot be applied, the ones that CAN already have + /// been: deletions are applied one at a time, not as a group. + /// + /// The doc above `commit()` says the additions are all-or-nothing, and they + /// are — nothing is copied. It does NOT say the deletions are, because they + /// are not, and a caller that reads a `delete:` failure as "the workdir is + /// as I left it" is wrong. The single-deletion test cannot see this: one + /// deletion is a degenerate group. + #[test] + fn a_failed_deletion_does_not_undo_the_deletions_that_already_landed() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + // The obstruction, injected the way it happens in the field and with no + // permission games: a symlinked parent component, which the confined + // unlinkat resolves inside the workdir root and so cannot reach. + let outside = tempfile::tempdir().unwrap(); + fs::write(outside.path().join("x.txt"), "outside the root").unwrap(); + std::os::unix::fs::symlink(outside.path(), workdir.path().join("link")).unwrap(); + for i in 0..6 { + fs::write(workdir.path().join(format!("f{i}.txt")), "doomed").unwrap(); + } + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + for i in 0..6 { + branch.mark_deleted(&format!("f{i}.txt")); + } + branch.mark_deleted("link/x.txt"); + + let err = branch.commit().expect_err("the unappliable deletion must fail the merge"); + assert!( + matches!(err, BranchError::Operation(ref m) if m.starts_with("delete:")), + "expected the deletion step to fail, got: {err:?}" + ); + + for i in 0..6 { + assert!( + !workdir.path().join(format!("f{i}.txt")).exists(), + "f{i}.txt was removable, so it was removed before the failure", + ); + } + assert_eq!( + branch + .changes() + .unwrap() + .into_iter() + .filter(|c| c.kind == crate::dry_run::ChangeKind::Deleted) + .map(|c| c.path) + .collect::>(), + vec![PathBuf::from("link/x.txt")], + "only the deletion that did not land may still be reported outstanding", + ); + assert!( + !workdir.path().join("added.txt").exists(), + "the additions are the half that IS all-or-nothing", + ); + } + + /// A commit that failed on a deletion must complete once the obstruction is + /// cleared: the guard is a stopping point, not a latch. + /// + /// Nothing else proves a branch can get past that guard at all. If it could + /// not, `TxnError::Merge`'s promise that recovering the preserved storage + /// finishes the transaction would be unreachable by the one route the crate + /// does provide — calling `commit()` again. + #[test] + fn a_commit_that_failed_on_a_deletion_completes_after_the_obstruction_is_cleared() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + fs::write(outside.path().join("x.txt"), "outside the root").unwrap(); + std::os::unix::fs::symlink(outside.path(), workdir.path().join("link")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("link/x.txt"); + branch.commit().expect_err("the unappliable deletion must fail the merge"); + + // Clear it the way an operator would: the deletion's target is gone, so + // it is now already applied. + fs::remove_file(outside.path().join("x.txt")).unwrap(); + + branch.commit().expect("the retry must get past the deletion guard"); + assert_eq!( + fs::read_to_string(workdir.path().join("added.txt")).unwrap(), + "payload", + "the additions held back by the guard must publish on the retry", + ); + assert!( + !storage_dir.exists(), + "a completed merge must reclaim the storage it was preserving", + ); + assert!( + list_preserved(storage.path()).is_empty(), + "and it must no longer look like work awaiting recovery", + ); + } + + /// The workdir root itself is not a deletable entry. + /// + /// `safe_rel` maps it to the empty relative path, which `commit()` would + /// hand to the recursive remove as "everything under the root" — emptying + /// the workdir — and then fail `EINVAL` trying to remove the root from + /// inside itself, permanently, on every retry. An `rmdir` of its own cwd is + /// all it takes. + #[test] + fn unlinking_the_workdir_root_is_refused_before_anything_is_recorded() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("a.txt"), "must survive").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = workdir.path().canonicalize().unwrap(); + + assert_eq!( + branch.handle_unlink(wd.to_str().unwrap(), true), + Err(libc::EBUSY), + "rmdir of the workdir root must be refused", + ); + assert_eq!( + branch.handle_unlink(&format!("{}/.", wd.display()), true), + Err(libc::EBUSY), + "and so must the same root spelled with a trailing dot", + ); + assert!(!branch.is_deleted(""), "nothing may have been recorded"); + assert!(!branch.has_changes(), "a refused unlink is not a change"); + + branch.commit().expect("a branch with nothing recorded must commit cleanly"); + assert_eq!( + fs::read_to_string(workdir.path().join("a.txt")).unwrap(), + "must survive", + "the workdir contents must not have been swept away", + ); + } + + // ---- What `Ok(())` is allowed to mean ---- + + /// A chmod with no content change must land in the workdir. + /// + /// `handle_chmod` copies the file up and chmods the upper, and `changes()` + /// reports it Modified — so the run's whole visible contract says the mode + /// change is a recorded change. The merge opens the destination with a + /// create mode, which does nothing to a file that already exists, so + /// without propagating the upper's mode the commit returned `Ok(())` having + /// published nothing at all. + #[test] + fn a_chmod_only_change_lands_in_the_workdir() { + use std::os::unix::fs::PermissionsExt; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let f = workdir.path().join("f.txt"); + fs::write(&f, "content").unwrap(); + fs::set_permissions(&f, fs::Permissions::from_mode(0o644)).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = workdir.path().canonicalize().unwrap(); + assert!(branch.handle_chmod(&format!("{}/f.txt", wd.display()), 0o600).unwrap()); + assert_eq!( + branch + .changes() + .unwrap() + .into_iter() + .map(|c| (c.kind, c.path)) + .collect::>(), + vec![(crate::dry_run::ChangeKind::Modified, PathBuf::from("f.txt"))], + "precondition: the run reports the chmod as a recorded change", + ); + + branch.commit().unwrap(); + + assert_eq!( + fs::metadata(&f).unwrap().permissions().mode() & 0o777, + 0o600, + "a change reported as merged must actually be in the workdir", + ); + assert_eq!( + fs::read_to_string(&f).unwrap(), + "content", + "and the content must be intact", + ); + } + + /// The mode of a file created in the upper survives the merge. + /// + /// The merge used to hardcode the destination mode, so a script or binary + /// the run produced arrived in the workdir un-executable — a committed + /// result that cannot be run. `execute_copy` already carries the mode on + /// the way down into the upper; this is the same property on the way back. + #[test] + fn the_mode_of_a_file_created_in_the_upper_survives_the_merge() { + use std::os::unix::fs::PermissionsExt; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + for (name, mode) in [("script.sh", 0o755u32), ("secret", 0o600), ("plain", 0o644)] { + let up = branch.upper.join(name); + fs::write(&up, "payload").unwrap(); + fs::set_permissions(&up, fs::Permissions::from_mode(mode)).unwrap(); + } + branch.commit().unwrap(); + + for (name, mode) in [("script.sh", 0o755u32), ("secret", 0o600), ("plain", 0o644)] { + assert_eq!( + fs::metadata(workdir.path().join(name)).unwrap().permissions().mode() & 0o777, + mode, + "{name} must be committed with the mode the run gave it", + ); + } + } + + /// An upper entry whose name is not valid UTF-8 must FAIL the merge. + /// + /// The confined merge helpers take a `&str`, so such an entry cannot be + /// carried across. Skipping it and running on reached the successful tail, + /// which removes the whole storage dir — reporting `Ok(())` while + /// destroying the only copy of that change, with nothing left on disk for a + /// sweep to find. Failing instead preserves the branch, which is what + /// `Ok(())` is documented to exclude. + #[test] + fn a_non_utf8_upper_entry_fails_the_merge_instead_of_being_destroyed() { + use std::os::unix::ffi::OsStrExt; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let name = std::ffi::OsStr::from_bytes(b"bad-\xff.bin"); + + let storage_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join(name), "payload").unwrap(); + let err = branch + .commit() + .expect_err("an entry the merge cannot carry across must not report success"); + assert!( + matches!(err, BranchError::Operation(ref m) if m.contains("not valid UTF-8")), + "expected the UTF-8 refusal, got: {err:?}" + ); + } + + assert_eq!( + fs::read(storage_dir.join("upper").join(name)).unwrap(), + b"payload", + "the unmergeable change must survive on disk, not be reclaimed", + ); + assert_eq!( + list_preserved(storage.path()).len(), + 1, + "and it must be findable by an out-of-band sweep", + ); + } + + /// An upper DIRECTORY over a workdir entry of another type must fail the + /// merge. + /// + /// `mkdirp_in_root` reports `EEXIST` as success and does not check the + /// type, so the directory silently never landed while the commit returned + /// `Ok(())` and then destroyed the storage — the same "reported merged, + /// no copy anywhere" class as the non-UTF-8 entry, reached by an ordinary + /// `mkdir` over a stale file. + #[test] + fn an_upper_directory_over_a_workdir_file_fails_the_merge() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("x"), "a file is in the way").unwrap(); + + let storage_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + storage_dir = branch.storage_dir.clone(); + fs::create_dir(branch.upper.join("x")).unwrap(); + let err = branch + .commit() + .expect_err("a directory that cannot be created must not report success"); + assert!( + matches!(err, BranchError::Operation(ref m) if m.starts_with("mkdir:")), + "expected the mkdir step to fail, got: {err:?}" + ); + } + + assert!( + workdir.path().join("x").is_file(), + "the workdir entry that blocked the merge is left as it was", + ); + assert!( + storage_dir.join("upper").join("x").is_dir(), + "the unmerged directory must survive for a retry", + ); + } + + /// A commit that already succeeded is a no-op, so a workdir the caller + /// edited afterwards is not silently overwritten by a second call. + /// + /// Deliberately asymmetric with a FAILED commit, which must NOT latch — + /// see the retry test above. Both halves are the `is_disposed` / + /// `BranchState::Finished` split, and collapsing them either way breaks one + /// of the two. + #[test] + fn a_successful_commit_does_not_republish_on_a_second_call() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("f.txt"), "from the run").unwrap(); + branch.commit().unwrap(); + assert_eq!(fs::read_to_string(workdir.path().join("f.txt")).unwrap(), "from the run"); + + fs::write(workdir.path().join("f.txt"), "edited afterwards").unwrap(); + branch.commit().expect("a second commit is a no-op, not an error"); + + assert_eq!( + fs::read_to_string(workdir.path().join("f.txt")).unwrap(), + "edited afterwards", + "a committed branch must not re-merge over a workdir that has moved on", + ); + } + + /// `changes()` labels an entry Added or Modified by looking at the LIVE + /// workdir, not at a snapshot taken when the branch was created. + /// + /// The same branch reports the same upper entry differently depending on + /// what the workdir holds at the moment of the call, which is what a caller + /// reading a dry run or a recovery report is actually being told. + #[test] + fn changes_labels_an_entry_against_the_workdir_as_it_stands_now() { + use crate::dry_run::ChangeKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("f.txt"), "from the run").unwrap(); + assert_eq!( + branch.changes().unwrap()[0].kind, + ChangeKind::Added, + "nothing in the workdir yet, so the entry is an addition", + ); + + fs::write(workdir.path().join("f.txt"), "appeared underneath").unwrap(); + assert_eq!( + branch.changes().unwrap()[0].kind, + ChangeKind::Modified, + "the label follows the live workdir: the commit will now overwrite a file", + ); + } + + // ---- Names, symlinks and the confined path helpers ---- + + /// `safe_rel` normalises the spellings that name the same entry, rejects an + /// escape out of the workdir, and passes an INTERIOR `..` through verbatim + /// — confinement for that lives downstream in `openat2(RESOLVE_IN_ROOT)`, + /// not here. + /// + /// That split is what the whole COW layer rests on and it is written down + /// nowhere else; a "hardening" that rejected interior `..` here, or a + /// simplification that accepted a leading one, would both change it + /// silently. + #[test] + fn safe_rel_normalises_spellings_and_rejects_only_an_escaping_prefix() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = workdir.path().canonicalize().unwrap(); + let rel = |suffix: &str| branch.safe_rel(&format!("{}{}", wd.display(), suffix)); + + assert_eq!(rel("/a.txt").as_deref(), Some("a.txt")); + assert_eq!(rel("/./a.txt").as_deref(), Some("a.txt"), "a `.` component normalises away"); + assert_eq!(rel("//a.txt").as_deref(), Some("a.txt"), "a doubled separator normalises away"); + assert_eq!(rel("/sub/").as_deref(), Some("sub"), "a trailing separator normalises away"); + assert_eq!( + branch.safe_rel(wd.to_str().unwrap()).as_deref(), + Some(""), + "the workdir root maps to the empty relative path", + ); + + assert_eq!(rel("/.."), None, "a leading escape is refused outright"); + assert_eq!( + rel("/sub/../x").as_deref(), + Some("sub/../x"), + "an interior `..` is passed through, to be clamped by the confined syscall", + ); + assert_eq!( + rel("/sub/../../outside.txt").as_deref(), + Some("sub/../../outside.txt"), + "even one that lexically escapes: `safe_rel` is not the confinement boundary", + ); + } + + /// `handle_symlink` refuses to record a link whose target is absolute or + /// walks up out of the tree, and records an ordinary in-tree one. + /// + /// This is a deliberate security decision with an asymmetry worth pinning: + /// a link the run CREATES with such a target is refused, while a + /// pre-existing workdir link with the very same target is copied up + /// verbatim by `prepare_copy` and merged back. + #[test] + fn handle_symlink_refuses_an_absolute_or_escaping_target_but_copies_up_a_lower_one() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/etc/passwd", workdir.path().join("preexisting")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = workdir.path().canonicalize().unwrap(); + let link = |n: &str| format!("{}/{}", wd.display(), n); + + assert!( + !branch.handle_symlink("/etc/passwd", &link("abs")).unwrap(), + "an absolute target must be refused", + ); + assert!( + !branch.handle_symlink("../outside", &link("up")).unwrap(), + "a target walking out of the tree must be refused", + ); + assert!( + !branch.upper_dir().join("abs").is_symlink() && !branch.upper_dir().join("up").is_symlink(), + "a refused symlink must not be recorded in the upper", + ); + assert!( + branch.handle_symlink("inside.txt", &link("ok")).unwrap(), + "an ordinary relative in-tree target is recorded", + ); + assert_eq!(fs::read_link(branch.upper_dir().join("ok")).unwrap(), PathBuf::from("inside.txt")); + + // The asymmetry: the same target, already on disk, IS copied up. + branch.ensure_cow_copy("preexisting").unwrap(); + assert_eq!( + fs::read_link(branch.upper_dir().join("preexisting")).unwrap(), + PathBuf::from("/etc/passwd"), + "a pre-existing absolute link is copied up verbatim, unlike a newly created one", + ); + } + + // ---- The on-disk marker: what a sweep in another binary can rely on ---- + + /// A marker written before `deleted=` existed must still parse, with an + /// empty deletion list. + /// + /// Preserved branches outlive the binary that wrote them by construction — + /// that is what preserving them is for — so a rolling upgrade meets old + /// markers. Making the key required would make every one of them invisible + /// to the sweep that is supposed to recover them. + #[test] + fn a_marker_written_before_the_deleted_key_existed_still_parses() { + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join(PRESERVED_MARKER), + b"reason=commit-deferred\nworkdir=/w\nupper=/s/upper\npid=41\n".as_slice(), + ) + .unwrap(); + + let p = read_preserved(dir.path()).expect("an older marker must still be recoverable"); + assert_eq!(p.reason, PreserveReason::CommitDeferred); + assert_eq!(p.workdir, PathBuf::from("/w")); + assert_eq!(p.upper, PathBuf::from("/s/upper")); + assert_eq!(p.pid, 41); + assert!(p.deleted.is_empty(), "no deletions were recorded, so there are none"); + } + + /// Forward compatibility has two halves that must stay apart: an UNKNOWN + /// KEY is ignored, but an UNKNOWN REASON makes the whole branch vanish from + /// the sweep. + /// + /// The second half is a trap — a newer writer plus an older sweeper reports + /// "nothing to recover" over a complete change set — and it is the reason + /// the first half must keep working: extending the format by adding keys is + /// safe, extending it by adding reasons is not. + #[test] + fn an_unknown_marker_key_is_ignored_but_an_unknown_reason_hides_the_branch() { + let with_key = tempfile::tempdir().unwrap(); + fs::write( + with_key.path().join(PRESERVED_MARKER), + b"reason=kept\nworkdir=/w\nupper=/u\npid=1\nsomething-new=x\n".as_slice(), + ) + .unwrap(); + assert_eq!( + read_preserved(with_key.path()).expect("an unknown key must not break parsing").reason, + PreserveReason::Kept, + ); + + let with_reason = tempfile::tempdir().unwrap(); + fs::write( + with_reason.path().join(PRESERVED_MARKER), + b"reason=written-by-a-newer-build\nworkdir=/w\nupper=/u\npid=1\n".as_slice(), + ) + .unwrap(); + assert_eq!( + read_preserved(with_reason.path()), + None, + "an unrecognised reason must not be guessed at", + ); + assert!( + list_preserved(with_reason.path().parent().unwrap()) + .iter() + .all(|p| p.branch_dir != with_reason.path()), + "and the branch is then invisible to the sweep — the cost of that choice", + ); + } + + /// Every required key is required: a marker missing any one of them is not + /// a preserved branch. + /// + /// The alternative is worse than dropping it. A record defaulting `workdir` + /// to empty joins to the process CWD, and a recovery acting on it would + /// merge a stranger's upper into whatever directory it happened to be in. + #[test] + fn a_marker_missing_any_required_key_is_not_a_preserved_branch() { + let full = "reason=kept\nworkdir=/w\nupper=/u\npid=1\n"; + for dropped in ["reason=", "workdir=", "upper=", "pid="] { + let dir = tempfile::tempdir().unwrap(); + let body: String = full + .lines() + .filter(|l| !l.starts_with(dropped)) + .map(|l| format!("{l}\n")) + .collect(); + fs::write(dir.path().join(PRESERVED_MARKER), &body).unwrap(); + assert_eq!( + read_preserved(dir.path()), + None, + "a marker without {dropped} must not parse, got a record from: {body:?}", + ); + } + // ...and the same body with nothing dropped does parse, so the loop is + // not passing for some unrelated reason. + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join(PRESERVED_MARKER), full).unwrap(); + assert!(read_preserved(dir.path()).is_some(), "the control must parse"); + } + + /// A marker truncated at any byte offset must read back as "not a preserved + /// branch", never as a half-populated record. + /// + /// `write_preserved_marker` is a plain `fs::write` — create, truncate, + /// write, no temp file and no rename — and it runs immediately before the + /// merge's first destructive step, so a crash there leaves exactly these + /// bytes. The dangerous shape is a prefix cut inside the `deleted=` lines: + /// it parses as a complete record whose change set is missing deletions, + /// and recovering from that resurrects the files the run deleted. + #[test] + fn a_marker_truncated_at_any_offset_never_reads_back_as_a_partial_record() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let storage_dir; + { + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + storage_dir = branch.storage_dir.clone(); + branch.mark_deleted("gone-a.txt"); + branch.mark_deleted("gone-b.txt"); + branch.preserve(PreserveReason::CommitDeferred); + } + let marker = storage_dir.join(PRESERVED_MARKER); + let full = fs::read(&marker).unwrap(); + let complete = read_preserved(&storage_dir).expect("the complete marker must parse"); + assert_eq!(complete.deleted.len(), 2, "precondition: both deletions are recorded"); + + for cut in 0..full.len() { + fs::write(&marker, &full[..cut]).unwrap(); + if let Some(partial) = read_preserved(&storage_dir) { + assert_eq!( + partial.deleted, complete.deleted, + "a marker truncated at byte {cut} parsed with a SHORTER deletion list; \ + recovering from it would resurrect the files the run deleted", + ); + } + } + fs::write(&marker, &full).unwrap(); + } + + /// A non-UTF-8 workdir path round-trips through the marker byte-exactly. + /// + /// The escaping is documented to be byte-based rather than string-based; + /// nothing tested it, and the failure mode is a recovery that merges into a + /// path with `U+FFFD` where the real bytes were — a directory that does not + /// exist, or worse, a different one that does. + #[test] + fn the_marker_round_trips_a_workdir_path_that_is_not_utf8() { + use std::os::unix::ffi::OsStrExt; + let root = tempfile::tempdir().unwrap(); + let workdir = root.path().join(std::ffi::OsStr::from_bytes(b"dir-\xff-name")); + fs::create_dir(&workdir).unwrap(); + let storage = tempfile::tempdir().unwrap(); + + { + let mut branch = SeccompCowBranch::create(&workdir, Some(storage.path()), 0).unwrap(); + branch.preserve(PreserveReason::CommitDeferred); + } + + let found = list_preserved(storage.path()); + assert_eq!(found.len(), 1); + assert_eq!( + found[0].workdir, + workdir.canonicalize().unwrap(), + "the raw bytes of the workdir path must survive the marker round-trip", + ); + assert!( + found[0].workdir.as_os_str().as_bytes().ends_with(b"dir-\xff-name"), + "the 0xff byte must come back as itself; a lossy conversion anywhere in the \ + round-trip would have replaced it with the three bytes of U+FFFD", + ); + } + + /// The marker is the trust anchor and the child is untrusted, so a workdir + /// file called `PRESERVED` must not be able to become one. + /// + /// The child's file goes through the COW layer into `upper/PRESERVED`; the + /// marker lives one level up, beside the upper. Flatten that layout and a + /// child could forge or clobber the record that says which workdir a + /// preserved change set belongs to. + #[test] + fn a_child_created_preserved_file_lands_inside_the_upper_not_beside_it() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + let wd = workdir.path().canonicalize().unwrap(); + + let upper = branch + .handle_open(&format!("{}/{}", wd.display(), PRESERVED_MARKER), O_WRONLY | O_CREAT) + .unwrap() + .expect("the child's write must be redirected into the upper"); + assert_eq!(upper, branch.upper_dir().join(PRESERVED_MARKER)); + fs::write(&upper, b"reason=kept\nworkdir=/etc\nupper=/etc\npid=1\n".as_slice()).unwrap(); + + assert_eq!( + read_preserved(&storage_dir), + None, + "a file the child wrote must not be readable as this branch's marker", + ); + + branch.preserve(PreserveReason::CommitDeferred); + let p = read_preserved(&storage_dir).expect("the real marker must be there"); + assert_eq!(p.workdir, wd, "the real marker names the real workdir, not the forged one"); + + branch.abort().unwrap(); + // ...and the child's file was an ordinary change all along. + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper_dir().join(PRESERVED_MARKER), "child bytes").unwrap(); + branch.commit().unwrap(); + assert_eq!( + fs::read_to_string(workdir.path().join(PRESERVED_MARKER)).unwrap(), + "child bytes", + "and it merges into the workdir like any other file", + ); + } + + /// A branch preserved as `CommitDeferred` is deliberately NOT disposed: the + /// commit it deferred can still be run, and running it merges and reclaims. + /// + /// This is the documented recovery for a transaction that lost the race for + /// the workdir lock. If `is_disposed()` were ever widened to cover it — it + /// already covers `Kept`, which looks similar from the outside — every + /// conflicted retry would return `Ok(())` having merged nothing. + #[test] + fn a_commit_deferred_branch_can_still_run_the_commit_it_deferred() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("gone.txt"), "still here").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("gone.txt"); + branch.preserve(PreserveReason::CommitDeferred); + assert_eq!(list_preserved(storage.path()).len(), 1, "precondition: it is preserved"); + + branch.commit().expect("a deferred commit must still be runnable"); + + assert_eq!( + fs::read_to_string(workdir.path().join("added.txt")).unwrap(), + "payload", + "the deferred change set must publish", + ); + assert!(!workdir.path().join("gone.txt").exists(), "including its deletions"); + assert!(!storage_dir.exists(), "and the storage is reclaimed once it has landed"); + assert!(list_preserved(storage.path()).is_empty(), "so no sweep still sees work here"); + } + + /// A preserved branch listed by the sweep can be destroyed by `abort()` — + /// for `MergeInterrupted` and `CommitDeferred`, but not for `Kept`. + /// + /// `abort()` is documented unconditionally as "discard all changes" and + /// this is where that is literally true, including for storage another + /// process may already have listed. The `Kept` exception is the only one, + /// and it is what the whole `is_disposed` split exists for. + #[test] + fn abort_destroys_preserved_storage_except_when_it_was_kept() { + for reason in [PreserveReason::MergeInterrupted, PreserveReason::CommitDeferred] { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.preserve(reason); + assert_eq!(list_preserved(storage.path()).len(), 1, "precondition for {reason:?}"); + + branch.abort().unwrap(); + + assert!(!storage_dir.exists(), "abort must destroy {reason:?} storage"); + assert!( + list_preserved(storage.path()).is_empty(), + "and it must disappear from the sweep, mid-flight or not", + ); + } + + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let storage_dir = branch.storage_dir.clone(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.keep(); + branch.abort().unwrap(); + assert!(storage_dir.exists(), "abort must not destroy storage that was kept"); + } + + /// `changes()` on a branch kept for inspection still reports the whole + /// change set, additions and deletions. + /// + /// Inspecting the change set is the entire point of `Keep`, and the + /// deletions are the half that exists nowhere but in RAM and in the marker + /// — so a `Keep` that reported only the upper would answer "what did this + /// run do" with half the truth. + #[test] + fn changes_on_a_kept_branch_still_reports_the_whole_change_set() { + use crate::dry_run::ChangeKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("gone.txt"), "still here").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + fs::write(branch.upper.join("added.txt"), "payload").unwrap(); + branch.mark_deleted("gone.txt"); + branch.keep(); + + let mut reported: Vec<(ChangeKind, PathBuf)> = + branch.changes().unwrap().into_iter().map(|c| (c.kind, c.path)).collect(); + reported.sort_by(|a, b| a.1.cmp(&b.1)); + assert_eq!( + reported, + vec![ + (ChangeKind::Added, PathBuf::from("added.txt")), + (ChangeKind::Deleted, PathBuf::from("gone.txt")), + ], + "a kept branch must still describe what the run did", + ); + } } diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 2cfd68db..a0954666 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -24,6 +24,7 @@ pub mod netlink; pub(crate) mod procfs; pub(crate) mod port_remap; pub mod pipeline; +pub mod transaction; pub mod policy_fn; pub mod image; pub mod fork; @@ -41,6 +42,10 @@ pub use sandbox::{ }; pub use result::{RunResult, ExitStatus}; pub use pipeline::{Stage, Pipeline, Gather}; +pub use transaction::{AbortReason, Transaction, TxnError, TxnOutcome}; +// Recovery of COW branch storage that was preserved rather than reclaimed. The +// rest of `cow` is internal; these are what an out-of-band sweep needs. +pub use cow::seccomp::{list_preserved, read_preserved, PreserveReason, PreservedBranch}; pub use dry_run::{Change, ChangeKind, DryRunResult}; // Sectioned-profile parsing types: ProfileInput is the top-level deserialization // target; ProgramSpec carries [program].exec/args (not a Sandbox field). diff --git a/crates/sandlock-core/src/pipeline.rs b/crates/sandlock-core/src/pipeline.rs index 0f30fc2a..9f10b413 100644 --- a/crates/sandlock-core/src/pipeline.rs +++ b/crates/sandlock-core/src/pipeline.rs @@ -4,6 +4,10 @@ //! Data flows through kernel pipe buffers between stages; the parent process //! never reads inter-stage data. //! +//! For stages that instead share a workspace and commit all-or-nothing, see +//! [`Transaction`](crate::transaction::Transaction): different execution model, +//! different type. +//! //! ```ignore //! let result = ( //! Stage::new(&policy_a, &["echo", "hello"]) @@ -76,8 +80,12 @@ impl std::ops::BitOr for Stage { /// /// Minimum 2 stages. Each stage runs concurrently in its own sandbox. /// Only the last stage's stdout and stderr are captured. +/// +/// The stages are private: "these are connected by pipes" is the whole meaning +/// of a `Pipeline`, and a bare `Vec` has lost it. Taking them out is +/// [`into_stages`](Self::into_stages), which a caller has to name. pub struct Pipeline { - pub stages: Vec, + stages: Vec, } impl Pipeline { @@ -91,6 +99,28 @@ impl Pipeline { Ok(Self { stages }) } + /// How many stages the chain has. + pub fn len(&self) -> usize { + self.stages.len() + } + + /// Always false — a `Pipeline` has at least 2 stages by construction. + pub fn is_empty(&self) -> bool { + self.stages.is_empty() + } + + /// Take the stages out of the chain, dropping the pipes between them. + /// + /// The pipes are not a property of a `Stage`; they exist only while the + /// stages are held as a `Pipeline`. Handing the result to something that + /// runs stages another way — [`Transaction`](crate::transaction::Transaction) + /// runs them sequentially over a shared workdir — therefore runs commands + /// that were written to stream into each other, without the stream. That is + /// a decision, so it has a name rather than a public field. + pub fn into_stages(self) -> Vec { + self.stages + } + /// Run the pipeline. Returns the last stage's exit status and captured output. /// /// `timeout` applies to the entire pipeline. diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 5f46a33a..a7e710eb 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -271,6 +271,28 @@ struct Runtime { on_bind: Option) + Send + Sync>>, handlers: Vec<(i64, Arc)>, ready_w: Option, + /// Set when this sandbox is a stage of a [`Transaction`](crate::transaction::Transaction) + /// that shares one COW upper across all stages. When present, `do_create_stdio` + /// reuses this `CowState` (instead of building its own branch), and neither + /// `wait()` nor `Drop` take/commit/abort the branch — the transaction + /// coordinator owns the single commit/abort. See [`SharedCow`]. + shared_cow: Option, +} + +/// A COW branch (one `upper` over the workdir) shared by every stage of a +/// [`Transaction`](crate::transaction::Transaction). Cloned into each stage's +/// `Runtime` so sequential stages accumulate writes in the same upper +/// (read-committed), while the coordinator retains the original to commit/abort +/// once at the end. +#[derive(Clone)] +pub(crate) struct SharedCow { + /// The shared supervisor COW state (holds the single `SeccompCowBranch`). + pub(crate) state: Arc>, + /// The branch's upper dir. Granted to each stage's Landlock ruleset for the + /// same reason a single sandbox grants its own upper: Landlock checks + /// EXECUTE against a file's real path, which for anything written inside the + /// workdir is the upper. Cached here to avoid locking `state` to read it. + pub(crate) upper_dir: PathBuf, } /// Lifecycle state for the runtime. @@ -782,9 +804,15 @@ impl Sandbox { if let Some(h) = rt.throttle_handle.take() { h.abort(); } if let Some(h) = rt.loadavg_handle.take() { h.abort(); } - if let Some(ref cow_state) = self.rt().supervisor_cow.clone() { - let mut cow = cow_state.lock().await; - self.rt_mut().seccomp_cow = cow.branch.take(); + // A transactional-pipeline stage leaves the branch in the shared COW + // state for the next stage / the coordinator's single commit — don't + // take it out (that would strip the upper from later stages) and don't + // let Drop commit/abort it (`seccomp_cow` stays None). + if self.rt().shared_cow.is_none() { + if let Some(ref cow_state) = self.rt().supervisor_cow.clone() { + let mut cow = cow_state.lock().await; + self.rt_mut().seccomp_cow = cow.branch.take(); + } } let stdout = self.rt_mut()._stdout_read.take().map(sandbox_read_fd_to_end); @@ -1162,12 +1190,18 @@ impl Sandbox { } /// Dry-run: create, start, wait, collect filesystem changes, then abort. + /// + /// The branch action is forced to `Abort`, not `Keep`: a dry run must never + /// merge, and must not leave its upper on disk either — the changes are read + /// out of the branch here and returned, so nothing needs preserving. `Keep` + /// would additionally ask the branch to survive an abandoned run (`?` on + /// create/wait below), which for a dry run is a pure leak. pub async fn dry_run( &mut self, cmd: &[&str], ) -> Result { - self.on_exit = BranchAction::Keep; - self.on_error = BranchAction::Keep; + self.on_exit = BranchAction::Abort; + self.on_error = BranchAction::Abort; self.do_create(cmd, true).await?; self.do_start()?; let run_result = self.wait().await?; @@ -1176,13 +1210,13 @@ impl Sandbox { Ok(crate::dry_run::DryRunResult { run_result, changes }) } - /// Dry-run with inherited stdio. + /// Dry-run with inherited stdio. Same branch handling as [`Self::dry_run`]. pub async fn dry_run_interactive( &mut self, cmd: &[&str], ) -> Result { - self.on_exit = BranchAction::Keep; - self.on_error = BranchAction::Keep; + self.on_exit = BranchAction::Abort; + self.on_error = BranchAction::Abort; self.do_create(cmd, false).await?; self.do_start()?; let run_result = self.wait().await?; @@ -1328,6 +1362,7 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + shared_cow: None, })); clones.push(clone_sb); } @@ -1422,10 +1457,21 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + shared_cow: None, })); Ok(()) } + /// Attach a transaction's shared COW branch to this sandbox before `create`. + /// The stage reuses the shared upper instead of building its own, and leaves + /// commit/abort to the transaction coordinator. Internal — used only by + /// [`Transaction`](crate::transaction::Transaction). + pub(crate) fn set_shared_cow(&mut self, shared: SharedCow) -> Result<(), crate::error::SandlockError> { + self.ensure_runtime()?; + self.rt_mut().shared_cow = Some(shared); + Ok(()) + } + // ================================================================ // Internal: collect_changes / do_abort // ================================================================ @@ -1575,12 +1621,32 @@ impl Sandbox { // workdir live in the upper dir, and Landlock checks EXECUTE on the // file's real path at execve time — so the upper dir must be granted // read+execute (READ_ACCESS) or `./created-binary` fails with EACCES. - let seccomp_cow_branch = if !no_supervisor && self.workdir.is_some() { + // A transactional-pipeline stage reuses one shared upper across stages; + // it must not build its own branch. Grant Landlock read+exec on the + // shared upper so a binary created in the workdir stays executable. + let shared_cow = self.rt().shared_cow.clone(); + let seccomp_cow_branch = if let Some(ref shared) = shared_cow { + self.fs_readable.push(shared.upper_dir.clone()); + None + } else if !no_supervisor && self.workdir.is_some() { let workdir = self.workdir.as_ref().unwrap().clone(); let storage = self.fs_storage.clone(); let max_disk = self.max_disk.map(|b| b.0).unwrap_or(0); match crate::cow::seccomp::SeccompCowBranch::create(&workdir, storage.as_deref(), max_disk) { - Ok(branch) => { + Ok(mut branch) => { + // `Keep` must survive a sandbox that is never `wait()`ed: + // the branch only reaches `Sandbox`'s own disposition after + // a completed `wait()`, and the branch's `Drop` would + // otherwise reclaim the upper the caller asked to keep. + // Commit and Abort are NOT carried over that way — an + // abandoned run has no exit status and merging its writes + // is not something it can ask for, so those keep the + // reclaiming default. With no exit status there is also no + // choice between the two actions, so either one asking for + // `Keep` preserves. + branch.set_keep_if_abandoned( + self.on_exit == BranchAction::Keep || self.on_error == BranchAction::Keep, + ); self.fs_readable.push(branch.upper_dir().to_path_buf()); Some(branch) } @@ -1847,6 +1913,10 @@ impl Sandbox { let mut cow_state = CowState::new(); cow_state.branch = seccomp_cow_branch; + // A shared transactional-pipeline branch overrides the per-stage one: + // reuse the coordinator's single COW state so every stage writes into + // (and reads through) the same upper. + let shared_cow_state = shared_cow.as_ref().map(|s| Arc::clone(&s.state)); let mut policy_fn_state = PolicyFnState::new(); @@ -1895,7 +1965,10 @@ impl Sandbox { let res_state = Arc::new(tokio::sync::Mutex::new(res_state)); self.rt_mut().supervisor_resource = Some(Arc::clone(&res_state)); - let cow_state = Arc::new(tokio::sync::Mutex::new(cow_state)); + let cow_state = match shared_cow_state { + Some(shared) => shared, + None => Arc::new(tokio::sync::Mutex::new(cow_state)), + }; self.rt_mut().supervisor_cow = Some(Arc::clone(&cow_state)); let net_state = Arc::new(tokio::sync::Mutex::new(net_state)); @@ -2091,7 +2164,9 @@ impl Drop for Sandbox { match action { BranchAction::Commit => { let _ = cow.commit(); } BranchAction::Abort => { let _ = cow.abort(); } - BranchAction::Keep => {} + // Mark kept so the branch's Drop backstop preserves the upper + // instead of cleaning it as an undisposed leak. + BranchAction::Keep => cow.keep(), } } } diff --git a/crates/sandlock-core/src/transaction.rs b/crates/sandlock-core/src/transaction.rs new file mode 100644 index 00000000..a5ad2c57 --- /dev/null +++ b/crates/sandlock-core/src/transaction.rs @@ -0,0 +1,1857 @@ +//! Filesystem transactions — sequential sandboxed stages over one shared COW +//! workdir, committed all-or-nothing (RFC #65). +//! +//! A transaction is **not** a pipeline. [`Pipeline`](crate::pipeline::Pipeline) +//! is the `|` operator: N stages running *concurrently*, each stage's stdout +//! wired to the next stage's stdin through a kernel pipe. A [`Transaction`] +//! runs its stages *sequentially* with **no inter-stage pipes and all stdio +//! inherited from the parent**; stages exchange data by reading and writing a +//! shared workspace, not by streaming bytes. The two are separate types +//! precisely so a `|`-built chain cannot be handed to the sequential runner and +//! silently lose its pipes: there is no `From` and no `BitOr` for +//! `Transaction`, and a `Pipeline`'s stages are private. Taking them out is +//! [`Pipeline::into_stages`](crate::pipeline::Pipeline::into_stages) — still +//! possible, and deliberately so, but a caller has to name it. +//! +//! ```ignore +//! let outcome = Transaction::new([ +//! Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), +//! Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), +//! ]).run(None).await?; +//! assert!(outcome.committed); +//! ``` + +use std::os::unix::io::AsRawFd; +use std::time::Duration; + +use crate::error::{SandboxError, SandboxRuntimeError, SandlockError}; +use crate::pipeline::Stage; +use crate::result::{ExitStatus, RunResult}; + +/// Default for [`Transaction::commit_lock_wait`]: how long to wait for another +/// transaction's commit merge to finish before giving up. Merges are short (a +/// file-by-file copy of one upper), so a wait this long only expires when +/// something is genuinely wrong. +const COMMIT_LOCK_WAIT: Duration = Duration::from_secs(30); + +/// Poll interval while waiting for the commit lock. `flock` has no timed +/// variant, so the wait is a bounded retry over the non-blocking form. +const COMMIT_LOCK_POLL: Duration = Duration::from_millis(20); + +// ============================================================ +// Transaction +// ============================================================ + +/// A set of stages run sequentially over one shared COW workdir, committed +/// all-or-nothing. +/// +/// Stages run **in declaration order, one at a time**, sharing a single COW +/// upper layered over their common workdir: stage N+1 sees stage N's writes +/// (read-committed) while the real workdir stays untouched for the duration of +/// the run. If every stage exits 0 the shared upper is merged into the workdir +/// in one step; if any stage fails, or the transaction times out, the upper is +/// discarded and the workdir is byte-identical to before the run. The two cases +/// where the upper is neither merged nor discarded — the commit could not be +/// performed — are the `Err` contract of [`run`](Self::run). +/// +/// **Stages are not connected by pipes.** Each stage inherits the parent's +/// stdin, stdout and stderr; data moves between stages through the shared +/// workspace. This is why a `Transaction` cannot be built with `|` — see the +/// [module docs](self). +/// +/// Every stage must set the same `workdir`, run with the supervisor +/// (`no_supervisor == false`), leave `on_exit`/`on_error` at their defaults, set +/// no `chroot`, and set the same `fs_storage`/`max_disk`. The transaction owns +/// the single shared upper and its commit/abort, so a per-stage override would +/// conflict and is rejected before anything runs. +pub struct Transaction { + stages: Vec, + commit_lock_wait: Duration, +} + +impl Transaction { + /// Build a transaction from an explicit list of stages. + /// + /// This accepts any list and validates nothing. The two-stage minimum and + /// every cross-stage rule (see [`Transaction`]) are checked by + /// [`run`](Self::run)/[`dry_run`](Self::dry_run) and reported as + /// [`TxnError::Invalid`], so a caller has one place to handle a + /// misconfigured transaction. That is deliberately unlike + /// [`Pipeline::new`](crate::pipeline::Pipeline::new), which returns + /// `Result` for the stage count alone. + /// + /// There is deliberately no `From` and no `BitOr` impl: a + /// `|`-built chain means "connect these by pipes", which a transaction does + /// not do. + pub fn new(stages: impl IntoIterator) -> Self { + Self { stages: stages.into_iter().collect(), commit_lock_wait: COMMIT_LOCK_WAIT } + } + + /// How long the commit may wait for another transaction to release the + /// workdir lock before giving up. Defaults to 30s. + /// + /// Giving up does not discard the run: the shared upper is preserved and + /// named in the error (see [`Transaction::run`]). Neither does dropping the + /// `run()` future during the wait — the commit phase runs on a blocking + /// thread that a cancellation detaches rather than stops, so this wait is + /// also the bound on how long that thread outlives an abandoned `run()`. + pub fn commit_lock_wait(mut self, wait: Duration) -> Self { + self.commit_lock_wait = wait; + self + } + + /// Run every stage, then commit the shared upper if all of them exited 0. + /// + /// `timeout` applies to the stage phase as a whole; on expiry the + /// transaction aborts and the workdir is untouched. + /// + /// Returns `Err` when the transaction could not be carried out at all: an + /// invalid stage configuration, a failure to start a stage, or a commit that + /// could not be performed. + /// + /// Once every stage has succeeded the run's work is never thrown away + /// silently. If the commit cannot take the workdir lock, or the merge itself + /// fails partway, the shared upper is **preserved** and its path is named in + /// the error. + /// + /// That holds when this future is **cancelled** too, which is the case a + /// caller reaches by wrapping `run()` in a `tokio::time::timeout` (the + /// `timeout` argument bounds the stage phase only) or by racing it in a + /// `select!`. Cancelling during the stage phase aborts and leaves the workdir + /// untouched; cancelling once the commit phase has begun does not stop it — + /// it runs on a blocking thread that a dropped future detaches — so the + /// change set still lands in the workdir or is preserved. What a cancelled + /// run gives up is the *report*: no `TxnOutcome`, no `TxnError`, and the + /// preserved upper has to be found through + /// [`list_preserved`](crate::cow::seccomp::list_preserved). + pub async fn run(self, timeout: Option) -> Result { + validate_txn_stages(&self.stages)?; + run_txn(self.stages, timeout, Disposition::Commit, self.commit_lock_wait).await + } + + /// Run every stage and report what the transaction *would* change, then + /// discard it. Nothing is ever merged into the workdir. + /// + /// The stages really execute — this predicts the filesystem effect on the + /// workdir, not the effect of running the commands. Same contract as + /// [`Sandbox::dry_run`](crate::sandbox::Sandbox::dry_run) for one sandbox. + /// The outcome always has `committed == false` and + /// `abort_reason == Some(`[`AbortReason::DryRun`]`)` unless a stage failed + /// or the transaction timed out first. + pub async fn dry_run(self, timeout: Option) -> Result { + validate_txn_stages(&self.stages)?; + run_txn(self.stages, timeout, Disposition::DryRun, self.commit_lock_wait).await + } +} + +/// What to do with the shared upper once every stage has succeeded. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Disposition { + Commit, + DryRun, +} + +// ============================================================ +// Errors +// ============================================================ + +/// Why a transaction could not be carried out at all. +/// +/// A transaction has two failure channels and this type is the second one. The +/// **abort** channel — a stage exited non-zero, the run timed out — is an +/// `Ok(TxnOutcome)` with an [`AbortReason`]: the transaction did its job and +/// the workdir is untouched. The **commit** channel is this type: the +/// transaction could not be carried out, and each way that can happen is its +/// own variant so a caller never has to match on a message. +/// +/// `#[non_exhaustive]`: match with a `_` arm. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum TxnError { + /// The stage set is not a valid transaction. Checked before anything runs, + /// so nothing was executed and no branch was created. + #[error("invalid transaction: {0}")] + Invalid(String), + + /// The shared COW branch could not be created. No stage ran. + #[error("transaction: failed to create the shared COW branch over {workdir}: {source}")] + Branch { + workdir: std::path::PathBuf, + #[source] + source: crate::error::BranchError, + }, + + /// Stage `index` could not be started or driven to completion. This is not a + /// stage that *failed* — that is [`AbortReason::StageFailed`] — it is a + /// stage that never produced an exit status. + #[error("transaction: stage {index} could not be run: {source}")] + Stage { + index: usize, + #[source] + source: SandlockError, + }, + + /// Conflict: another transaction held the workdir commit lock for longer + /// than [`Transaction::commit_lock_wait`]. Every stage had succeeded, so the + /// workdir is untouched and the whole change set is preserved — additions + /// and modifications under `preserved_upper`, deletions in the `PRESERVED` + /// marker beside it (see + /// [`read_preserved`](crate::cow::seccomp::read_preserved)). Retrying is the + /// expected response. + #[error( + "transaction: gave up after {waited:?} waiting for another commit to release the workdir \ + lock on {workdir}. The workdir is untouched and this transaction's changes were \ + preserved at {preserved_upper}" + )] + Conflict { + workdir: std::path::PathBuf, + waited: Duration, + preserved_upper: std::path::PathBuf, + }, + + /// The workdir commit lock could not be taken for a reason other than + /// contention (the workdir could not be opened, or `flock` failed). As with + /// [`Self::Conflict`] the workdir is untouched and the change set is + /// preserved. + #[error( + "transaction: could not take the commit lock on {workdir}: {source}. The workdir is \ + untouched and this transaction's changes were preserved at {preserved_upper}" + )] + CommitLock { + workdir: std::path::PathBuf, + preserved_upper: std::path::PathBuf, + #[source] + source: std::io::Error, + }, + + /// The commit merge failed. The change set that did not land is preserved — + /// additions and modifications under `preserved_upper`, deletions in the + /// `PRESERVED` marker beside it (see + /// [`read_preserved`](crate::cow::seccomp::read_preserved)). + /// + /// The merge is not rolled back, so the workdir may be partially merged and + /// re-running the stages is not the same thing as finishing this + /// transaction; recovering that storage is what completes it. How much + /// landed is not carried here — it is the difference between the workdir and + /// what the preserved storage still holds. + #[error( + "transaction: the commit merge into {workdir} failed: {source}. The workdir may be \ + partially merged; what did not land was preserved at {preserved_upper}, with any \ + outstanding deletions listed in the PRESERVED marker beside it" + )] + Merge { + workdir: std::path::PathBuf, + preserved_upper: std::path::PathBuf, + #[source] + source: crate::error::BranchError, + }, + + /// The commit phase never ran to completion because the runtime was shut + /// down under it. Unlike every other variant this one cannot say what state + /// the workdir and the upper are in: the phase may not have started at all, + /// in which case the change set was reclaimed with the branch. + #[error("transaction: the commit did not run to completion: {0}")] + CommitAbandoned(String), +} + +impl TxnError { + /// Process exit code for this failure, keeping the RFC's channels apart: + /// configuration (78), stage driver (70), commit (74), conflict (75). + /// + /// The values are the conventional `sysexits.h` names — `EX_CONFIG`, + /// `EX_SOFTWARE`, `EX_IOERR`, `EX_TEMPFAIL`. `EX_TEMPFAIL` for a conflict is + /// the one that carries meaning beyond "distinct": it says retry. + /// + /// These do NOT distinguish themselves from a stage's own exit code — a + /// child is free to exit 75 — so a caller that needs to tell a commit + /// failure from a stage failure takes it from the `Result`, not the number. + pub fn exit_code(&self) -> i32 { + match self { + TxnError::Invalid(_) => 78, + TxnError::Stage { .. } => 70, + TxnError::Branch { .. } + | TxnError::CommitLock { .. } + | TxnError::Merge { .. } + | TxnError::CommitAbandoned(_) => 74, + TxnError::Conflict { .. } => 75, + } + } +} + +impl From for SandlockError { + /// Flatten into the crate-wide error for callers that do not care about the + /// channel — but not into one that says something false. A misconfigured + /// transaction stays a `Sandbox` error; a stage that could not be driven + /// keeps its own `SandlockError` verbatim; and the commit channel becomes a + /// `Branch` error, because that is what it is — a disposition of the shared + /// COW branch that could not be carried out. `Child` would claim a child + /// process failed, which is the abort channel and never reaches here. + /// + /// The commit channel keeps its rendered message rather than only its + /// `BranchError` source (which `Conflict` does not even have): the message + /// names the preserved upper, and that path is the one thing a caller cannot + /// reconstruct from anything else. + fn from(e: TxnError) -> Self { + match e { + TxnError::Invalid(m) => SandlockError::Sandbox(SandboxError::Invalid(m)), + TxnError::Stage { source, .. } => source, + other => SandlockError::Runtime(SandboxRuntimeError::Branch( + crate::error::BranchError::Operation(other.to_string()), + )), + } + } +} + +/// Why [`acquire_commit_lock`] gave up, so the caller can tell contention (a +/// conflict, worth retrying) from a broken workdir. +#[derive(Debug)] +enum LockFailure { + /// The lock was held by someone else for the whole wait. + Contended(Duration), + /// The workdir could not be opened, or `flock` failed for a reason other + /// than contention. + Io(std::io::Error), +} + +// ============================================================ +// Outcome +// ============================================================ + +/// Why a transaction did not commit, having run and left the workdir untouched. +/// A transaction that could not be carried out at all is a [`TxnError`] instead. +/// +/// `#[non_exhaustive]`: these are the reasons RFC #65 Phase 1 can produce and +/// later phases add to them. Match with a `_` arm. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum AbortReason { + /// Stage `index` (0-based, in declaration order) did not exit 0. Later + /// stages were not run. + StageFailed { index: usize, status: ExitStatus }, + /// The stage phase exceeded the transaction's timeout. The in-flight stage + /// was killed; `TxnOutcome::stages` holds the stages that had completed. + TimedOut, + /// [`Transaction::dry_run`] completed: every stage succeeded and the upper + /// was discarded on purpose rather than merged. + DryRun, +} + +impl std::fmt::Display for AbortReason { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AbortReason::StageFailed { index, status } => { + write!(f, "stage {index} did not exit cleanly: {status:?}") + } + AbortReason::TimedOut => write!(f, "the transaction timed out"), + AbortReason::DryRun => write!(f, "dry run: changes were reported, not committed"), + } + } +} + +/// Outcome of [`Transaction::run`] / [`Transaction::dry_run`]. +#[derive(Debug, Clone)] +#[must_use = "a transaction can finish without committing; inspect `committed`"] +pub struct TxnOutcome { + /// True if every stage exited 0 and the shared upper was merged into the + /// workdir. False if any stage failed, the transaction timed out, or this + /// was a dry run — in all of which the workdir is byte-identical to before. + pub committed: bool, + /// Per-stage results in execution order, holding every stage that ran — + /// including on an abort, and including the partial run recorded before a + /// timeout cancelled the in-flight stage. + /// + /// **`stdout` and `stderr` are always `None` here, by construction.** Stages + /// inherit the parent's stdio (see the [module docs](self)), so their output + /// has already gone to the parent's own fd 1/2 and there is nothing to + /// capture. This is asymmetric with `Pipeline::run`, which pipes the last + /// stage and returns its bytes. A caller that needs a failing stage's output + /// has to arrange for it — redirect inside the stage command, or read the + /// parent's stream — because [`AbortReason::StageFailed`] carries an index + /// and a status and nothing else. + pub stages: Vec, + /// The filesystem changes the shared upper held at the end of the run, i.e. + /// what the commit merged (or, when not committed, what was discarded). + /// Captured from the branch before it is disposed of. + pub changes: Vec, + /// Why the transaction did not commit; `None` when it did. + pub abort_reason: Option, +} + +impl TxnOutcome { + /// Process exit code for this outcome, for a caller that fronts a + /// transaction with a command-line tool. + /// + /// A committed transaction and a completed dry run are 0 — both did what + /// was asked. A stage failure reports that stage's own status, under the + /// shell convention (a signalled child is `128 + signal`), so the code a + /// caller sees is the code the failing command produced. A timeout is 124, + /// as `timeout(1)` reports it. + /// + /// The commit channel is not represented here at all: it is + /// [`TxnError::exit_code`]. + pub fn exit_code(&self) -> i32 { + match &self.abort_reason { + None | Some(AbortReason::DryRun) => 0, + Some(AbortReason::TimedOut) => 124, + Some(AbortReason::StageFailed { status, .. }) => match status { + ExitStatus::Code(c) => *c, + ExitStatus::Signal(s) => 128 + *s, + ExitStatus::Killed => 128 + libc::SIGKILL, + ExitStatus::Timeout => 124, + }, + } + } +} + +// ============================================================ +// Validation +// ============================================================ + +/// Reject stage configurations that can't participate in a transaction. The +/// transaction owns the single shared upper and the single commit/abort, so each +/// stage must set the same workdir, keep the supervisor, and not carry its own +/// branch action. +/// +/// These are configuration errors, reported as [`TxnError::Invalid`] so a +/// misconfigured transaction is distinguishable from a child-process failure. +fn validate_txn_stages(stages: &[Stage]) -> Result<(), TxnError> { + fn reject(msg: impl Into) -> TxnError { + TxnError::Invalid(msg.into()) + } + + if stages.len() < 2 { + return Err(reject("transaction requires at least 2 stages")); + } + + let base = &stages[0].sandbox; + let base_wd = base.workdir.as_ref().ok_or_else(|| { + reject("transaction: stage 0 has no workdir; every stage must set the shared transaction workdir") + })?; + let base_max_disk = base.max_disk.map(|b| b.0).unwrap_or(0); + + for (i, stage) in stages.iter().enumerate() { + let sb = &stage.sandbox; + let wd = sb.workdir.as_ref().ok_or_else(|| { + reject(format!("transaction: stage {i} has no workdir; every stage must set the shared transaction workdir")) + })?; + if wd != base_wd { + return Err(reject(format!( + "transaction: stages must share one workdir (stage 0 = {}, stage {i} = {})", + base_wd.display(), + wd.display() + ))); + } + if sb.no_supervisor { + return Err(reject(format!( + "transaction: stage {i} has no_supervisor=true; transactions require the COW supervisor" + ))); + } + // All stages overlay ONE shared upper, so per-stage COW storage/quota and + // chroot can't each take effect — reject a stage that sets them differently + // (or at all, for chroot) rather than silently using only stage 0's. + if sb.chroot.is_some() { + return Err(reject(format!( + "transaction: stage {i} sets chroot, which is unsupported with a shared COW workdir" + ))); + } + if sb.fs_storage != base.fs_storage || sb.max_disk.map(|b| b.0).unwrap_or(0) != base_max_disk { + return Err(reject(format!( + "transaction: stage {i} sets a different fs_storage/max_disk; all stages share one COW upper, so these must match stage 0" + ))); + } + // The builder leaves both actions at `BranchAction::Commit` by default + // (`unwrap_or_default()`); anything else is an explicit per-stage choice + // that conflicts with the transaction owning commit/abort. + if sb.on_exit != crate::sandbox::BranchAction::Commit + || sb.on_error != crate::sandbox::BranchAction::Commit + { + return Err(reject(format!( + "transaction: stage {i} sets on_exit/on_error, which conflicts with a transaction (the transaction owns commit/abort) — leave them at their defaults" + ))); + } + } + Ok(()) +} + +// ============================================================ +// Coordinator +// ============================================================ + +/// Create the shared COW branch, drive the stages sequentially over it, then +/// commit-all or abort-all. The branch lives here (outside the driven future) +/// so a timeout that cancels the stage loop can still abort cleanly. +async fn run_txn( + stages: Vec, + timeout: Option, + disposition: Disposition, + commit_lock_wait: Duration, +) -> Result { + // All stages share the validated workdir; take COW storage/quota from the + // first stage (they overlay the same lower). + let base = &stages[0].sandbox; + let workdir = base.workdir.clone().expect("validated: stage 0 has a workdir"); + let storage = base.fs_storage.clone(); + let max_disk = base.max_disk.map(|b| b.0).unwrap_or(0); + + let branch = crate::cow::seccomp::SeccompCowBranch::create(&workdir, storage.as_deref(), max_disk) + .map_err(|source| TxnError::Branch { workdir: workdir.clone(), source })?; + let upper_dir = branch.upper_dir().to_path_buf(); + + let mut cow_state = crate::seccomp::state::CowState::new(); + cow_state.branch = Some(branch); + let state = std::sync::Arc::new(tokio::sync::Mutex::new(cow_state)); + let shared = crate::sandbox::SharedCow { state: std::sync::Arc::clone(&state), upper_dir: upper_dir.clone() }; + + // Stage results are accumulated through a handle the coordinator also holds, + // so a timeout that cancels the driver future does not take the completed + // stages' results down with it. + let results = std::sync::Arc::new(std::sync::Mutex::new(Vec::::new())); + let drive = drive_txn_stages(stages, shared, std::sync::Arc::clone(&results)); + let driven: Result, TxnError> = match timeout { + Some(dur) => match tokio::time::timeout(dur, drive).await { + Ok(r) => r, + Err(_) => Ok(Some(AbortReason::TimedOut)), + }, + None => drive.await, + }; + + // Finalize the shared upper in EVERY case — commit only on a clean full run, + // otherwise discard — before propagating any driver error, so a mid-loop + // failure never leaves the upper dangling. (`SeccompCowBranch`'s Drop is a + // further backstop for a panic between here and the disposition.) + // Take the branch out from under the async mutex first, then move the owned + // value onto a blocking thread: the merge must not run while holding the + // guard, must not run on an executor worker, and must not be droppable by a + // cancellation (see `finish_branch`). + let taken = { state.lock().await.branch.take() }; + let (mut reason, drive_err) = match driven { + Ok(rsn) => (rsn, None), + Err(e) => (None, Some(e)), + }; + let all_ok = reason.is_none() && drive_err.is_none(); + + let mut changes = Vec::new(); + let committed = match taken { + Some(branch) => { + let want_commit = all_ok && disposition == Disposition::Commit; + let wd = workdir.clone(); + let handle = tokio::task::spawn_blocking(move || { + finish_branch(branch, &wd, commit_lock_wait, want_commit) + }); + let finished = match handle.await { + Ok(f) => f, + Err(join) if join.is_panic() => std::panic::resume_unwind(join.into_panic()), + Err(join) => return Err(TxnError::CommitAbandoned(join.to_string())), + }; + changes = finished.changes; + match finished.commit { + Some(Ok(())) => true, + Some(Err(CommitFailure::Lock(LockFailure::Contended(waited)))) => { + return Err(TxnError::Conflict { workdir, waited, preserved_upper: upper_dir }) + } + Some(Err(CommitFailure::Lock(LockFailure::Io(source)))) => { + return Err(TxnError::CommitLock { + workdir, + preserved_upper: upper_dir, + source, + }) + } + Some(Err(CommitFailure::Merge(source))) => { + return Err(TxnError::Merge { workdir, preserved_upper: upper_dir, source }) + } + // The upper was discarded rather than merged: a dry run, or a + // run that aborted. + None => { + if all_ok { + reason = Some(AbortReason::DryRun); + } + false + } + } + } + // Unreachable: `create` above always yields a branch and a transactional + // stage never takes it out of the shared state. Treat a future violation + // of that invariant as "nothing was committed" rather than reporting a + // commit that did not happen. + None => false, + }; + if let Some(e) = drive_err { + return Err(e); + } + + let stages = std::sync::Arc::try_unwrap(results) + .map(|m| m.into_inner().unwrap_or_default()) + .unwrap_or_else(|arc| arc.lock().map(|g| g.clone()).unwrap_or_default()); + + Ok(TxnOutcome { + committed, + stages, + changes, + abort_reason: if committed { None } else { reason }, + }) +} + +/// Why the commit phase could not publish the change set. Turned into a +/// [`TxnError`] by the caller, which holds the workdir and upper paths the +/// message names. +enum CommitFailure { + Lock(LockFailure), + Merge(crate::error::BranchError), +} + +/// What the commit phase did with the shared upper. +struct Finished { + /// The change set the upper held, read before it was disposed of. + changes: Vec, + /// `None` when the upper was discarded rather than merged (a dry run, or an + /// aborted run); otherwise the result of the commit. + commit: Option>, +} + +/// Read the shared upper's change set and dispose of it — the entire blocking +/// half of a transaction, run on a blocking thread. +/// +/// Both halves are unbounded in the caller's data: `changes()` walks the whole +/// upper, and `commit()` walks it again copying file-by-file and fsyncing +/// directories. Neither may run on an executor worker, and the commit half holds +/// a cross-process `flock` while it does. +/// +/// The branch is **moved in**, and that is what makes the commit phase +/// uncancellable. Dropping the `run()` future — a `tokio::time::timeout` around +/// it, a `select!` on shutdown — drops the `JoinHandle`, which detaches this task +/// rather than stopping it, so the owned branch is never dropped in +/// `BranchState::Open` by a cancellation and the change set of a run whose stages +/// all succeeded is always either published or preserved. Dropping the branch at +/// the end of this function is right in every case: `commit()` leaves it +/// `Finished` on success and `Preserved` on failure, and the lock path preserves +/// it explicitly. +fn finish_branch( + mut branch: crate::cow::seccomp::SeccompCowBranch, + workdir: &std::path::Path, + commit_lock_wait: Duration, + commit: bool, +) -> Finished { + // The branch is about to be disposed of, so read the change set first: after + // a commit or an abort the upper is gone and there is nothing left to report. + let changes = branch.changes().unwrap_or_default(); + if !commit { + let _ = branch.abort(); + return Finished { changes, commit: None }; + } + + // Serialize the merge against any other transaction committing into this + // workdir. commit() rewrites the workdir file-by-file, so two merges + // interleaving would tear it. This is mutual exclusion between merges, NOT + // serializable isolation: a transaction that snapshotted before another one + // committed still merges over that result (last writer wins per file). The + // lock is also scoped to transactions — a plain Sandbox committing its own + // branch into the same workdir does not take it. + let lock = match acquire_commit_lock(workdir, commit_lock_wait) { + Ok(l) => l, + Err(f) => { + // Every stage exited 0, so the upper holds a full, mergeable change + // set that only failed to be published. Returning here would + // otherwise drop an `Open` branch and reclaim it — losing the work of + // a run that did nothing wrong. Hand the storage over instead, for + // the caller to name in the error. + branch.preserve(crate::cow::seccomp::PreserveReason::CommitDeferred); + return Finished { changes, commit: Some(Err(CommitFailure::Lock(f))) }; + } + }; + // `commit()` preserves the upper itself when it fails partway — it marks the + // branch before touching the workdir. + let merged = branch.commit().map_err(CommitFailure::Merge); + drop(lock); // release the workdir lock after the merge + Finished { changes, commit: Some(merged) } +} + +/// Take an exclusive lock on the workdir, waiting up to `deadline_after` for a +/// concurrent commit merge to release it. `flock` has no timed variant, so this +/// is a bounded poll over the non-blocking form. +/// +/// Waiting rather than failing fast is deliberate: a transaction that has run +/// every stage successfully should publish its work, not discard it because +/// another merge happened to be in flight for a few milliseconds. Expiring the +/// wait does not discard it either — the caller preserves the upper. +/// +/// Blocking, and only ever called from [`finish_branch`] on a blocking thread: +/// the merge it guards is blocking too, so making the wait cancellable would buy +/// nothing and would put the branch back on a droppable await. +fn acquire_commit_lock( + workdir: &std::path::Path, + deadline_after: Duration, +) -> Result { + acquire_commit_lock_polling(workdir, deadline_after, std::thread::sleep) +} + +/// [`acquire_commit_lock`] with the poll sleep injected, so a test can observe +/// how many times — if at all — the loop actually waited. +fn acquire_commit_lock_polling( + workdir: &std::path::Path, + deadline_after: Duration, + mut sleep: impl FnMut(Duration), +) -> Result { + let lock = std::fs::File::open(workdir).map_err(LockFailure::Io)?; + let deadline = std::time::Instant::now() + deadline_after; + loop { + if unsafe { libc::flock(lock.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) } == 0 { + return Ok(lock); + } + let err = std::io::Error::last_os_error(); + // EWOULDBLOCK (== EAGAIN on Linux) means another commit holds the lock. + // Any other errno is a real failure and must not be retried. + if err.raw_os_error() != Some(libc::EWOULDBLOCK) { + return Err(LockFailure::Io(err)); + } + if std::time::Instant::now() >= deadline { + return Err(LockFailure::Contended(deadline_after)); + } + sleep(COMMIT_LOCK_POLL); + } +} + +/// Run each stage to completion in order over the shared upper, with no +/// inter-stage pipes (all stdio inherited). Stops at the first non-zero exit: +/// under a sequential shared-workspace model stage N+1's inputs do not exist if +/// stage N failed, and the transaction is going to abort regardless. +/// +/// Each result is published to `results` as soon as its stage finishes, so the +/// coordinator still has them if this future is cancelled by a timeout. +async fn drive_txn_stages( + stages: Vec, + shared: crate::sandbox::SharedCow, + results: std::sync::Arc>>, +) -> Result, TxnError> { + for (i, stage) in stages.into_iter().enumerate() { + let at = |source: SandlockError| TxnError::Stage { index: i, source }; + let cmd_refs: Vec<&str> = stage.args.iter().map(|s| s.as_str()).collect(); + let mut sb = stage.sandbox.with_name(format!("txn-stage-{i}")); + sb.set_shared_cow(shared.clone()).map_err(at)?; + sb.create_with_io(&cmd_refs, None, None, None).await.map_err(at)?; + sb.start().map_err(at)?; + let result = sb.wait().await.map_err(at)?; + + let status = result.exit_status.clone(); + if let Ok(mut guard) = results.lock() { + guard.push(result); + } + if !matches!(status, ExitStatus::Code(0)) { + return Ok(Some(AbortReason::StageFailed { index: i, status })); + } + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::sandbox::{BranchAction, ByteSize, Sandbox}; + use std::os::unix::io::AsRawFd; + + // ------------------------------------------------------------ + // Validation + // ------------------------------------------------------------ + + /// A stage that satisfies every cross-stage rule: it sets the shared + /// workdir and nothing else the transaction owns. The command is never run — + /// every validation test below is rejected (or accepted) before `run_txn` + /// touches a cage. + fn ok_stage(workdir: &std::path::Path) -> Stage { + Stage::new(&Sandbox::builder().workdir(workdir).build().unwrap(), &["true"]) + } + + /// The rejection message of a stage set, or `None` if it validates. + fn rejection(stages: &[Stage]) -> Option { + match validate_txn_stages(stages) { + Ok(()) => None, + Err(TxnError::Invalid(m)) => Some(m), + Err(other) => panic!("validation must only ever produce Invalid, got: {other:?}"), + } + } + + /// Both per-stage branch actions must be left at `Commit`, and the check is + /// on the VALUE, not on whether the builder was called: a stage that spells + /// out `on_exit = commit` — what a TOML profile writing the default looks + /// like — is accepted, while every other combination of the two fields is + /// rejected. + /// + /// This walks all nine cells because the two halves of the check are + /// separately deletable. Narrowing it to `on_exit` alone leaves the + /// pre-existing `test_txn_rejects_branch_action` green, so nothing has been + /// guarding `on_error` — and `on_error(Keep)` is exactly what a caller who + /// wants forensics on a failed stage reaches for, which would preserve the + /// SHARED upper behind the coordinator's back. + /// + /// The rejection message is pinned too. It says "leave them at their + /// defaults" without naming the default, which is `Commit`. + #[test] + fn per_stage_branch_actions_are_rejected_unless_both_are_the_default() { + let wd = tempfile::tempdir().unwrap(); + let actions = [BranchAction::Commit, BranchAction::Abort, BranchAction::Keep]; + + for on_exit in &actions { + for on_error in &actions { + let sb = Sandbox::builder() + .workdir(wd.path()) + .on_exit(on_exit.clone()) + .on_error(on_error.clone()) + .build() + .unwrap(); + let stages = vec![Stage::new(&sb, &["true"]), ok_stage(wd.path())]; + let got = rejection(&stages); + + if *on_exit == BranchAction::Commit && *on_error == BranchAction::Commit { + assert_eq!( + got, None, + "the default pair must be accepted even when set explicitly, \ + but on_exit={on_exit:?}/on_error={on_error:?} was rejected" + ); + continue; + } + let msg = got.unwrap_or_else(|| { + panic!( + "on_exit={on_exit:?}/on_error={on_error:?} is a per-stage branch action \ + and must be rejected, but the transaction validated" + ) + }); + assert!( + msg.contains("stage 0 sets on_exit/on_error") + && msg.contains("leave them at their defaults"), + "on_exit={on_exit:?}/on_error={on_error:?} must be rejected as a branch-action \ + conflict naming the stage, got: {msg}" + ); + } + } + } + + /// Every guardrail that can fire at all must fire when the offending stage + /// is stage 0. + /// + /// Stage 0 is the baseline the other stages are compared against, so it is + /// the index a refactor is most likely to special-case. Making the loop + /// `continue` on `i == 0` leaves every pre-existing `test_txn_rejects_*` + /// integration test green, because each one puts its violation on the + /// second stage. + /// + /// The workdir and fs_storage/max_disk rules are absent here on purpose: + /// they compare a stage against stage 0, so they cannot fire AT stage 0. + #[test] + fn every_guardrail_that_can_fire_at_stage_zero_does() { + let wd = tempfile::tempdir().unwrap(); + let chroot = tempfile::tempdir().unwrap(); + let cases: [(&str, Sandbox); 4] = [ + ( + "has no_supervisor=true", + Sandbox::builder().workdir(wd.path()).no_supervisor(true).build().unwrap(), + ), + ( + "sets chroot", + Sandbox::builder().workdir(wd.path()).chroot(chroot.path()).build().unwrap(), + ), + ( + "sets on_exit/on_error", + Sandbox::builder() + .workdir(wd.path()) + .on_exit(BranchAction::Abort) + .build() + .unwrap(), + ), + ( + "sets on_exit/on_error", + Sandbox::builder() + .workdir(wd.path()) + .on_error(BranchAction::Keep) + .build() + .unwrap(), + ), + ]; + + for (fragment, offender) in cases { + let stages = vec![Stage::new(&offender, &["true"]), ok_stage(wd.path())]; + let msg = rejection(&stages).unwrap_or_else(|| { + panic!("a stage 0 that {fragment} must be rejected, but the transaction validated") + }); + assert!( + msg.contains(&format!("stage 0 {fragment}")), + "the violation is at stage 0 and must be reported there, got: {msg}" + ); + } + } + + /// A transaction whose stages set no workdir at all is an `Invalid` error, + /// not a panic — through both entry points. + /// + /// `run_txn` reads stage 0's workdir with `.expect("validated: stage 0 has a + /// workdir")`, so the validator is the only thing standing between a + /// misconfigured caller and a panic inside a supervisor. Both `run` and + /// `dry_run` must validate before anything runs. + #[tokio::test] + async fn a_transaction_with_no_workdir_is_an_error_from_both_entry_points_not_a_panic() { + let stageless = || { + let sb = Sandbox::builder().build().unwrap(); + vec![Stage::new(&sb, &["true"]), Stage::new(&sb, &["true"])] + }; + + for (entry, err) in [ + ("run", Transaction::new(stageless()).run(None).await.unwrap_err()), + ("dry_run", Transaction::new(stageless()).dry_run(None).await.unwrap_err()), + ] { + assert!( + matches!(&err, TxnError::Invalid(m) if m.contains("stage 0 has no workdir")), + "{entry} must reject a workdir-less transaction as a configuration error, got: {err:?}" + ); + } + } + + /// An unset `max_disk` and `max_disk = 0` are the SAME quota (both mean + /// unlimited), so mixing them across stages is accepted — while a stage that + /// really asks for a different quota is rejected. + /// + /// The whole quota half of that check is deletable on its own: dropping the + /// `max_disk` comparison and keeping only `fs_storage` leaves every other + /// test green, and the transaction would then silently run every stage under + /// stage 0's quota while a stage's own limit was ignored. + #[test] + fn an_unset_max_disk_equals_a_zero_max_disk_but_a_real_one_must_match() { + let wd = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let with = |disk: Option| { + let mut b = Sandbox::builder().workdir(wd.path()).fs_storage(storage.path()); + if let Some(d) = disk { + b = b.max_disk(ByteSize(d)); + } + Stage::new(&b.build().unwrap(), &["true"]) + }; + + assert_eq!( + rejection(&[with(None), with(Some(0))]), + None, + "an unset quota and a zero quota both mean unlimited and must be interchangeable" + ); + let msg = rejection(&[with(Some(4096)), with(Some(8192))]) + .expect("two stages asking for different quotas over one shared upper must be rejected"); + assert!( + msg.contains("stage 1 sets a different fs_storage/max_disk"), + "a differing quota must be reported as the fs_storage/max_disk conflict, got: {msg}" + ); + let msg = rejection(&[with(None), with(Some(4096))]) + .expect("a real quota does not match an unset one and must be rejected"); + assert!( + msg.contains("stage 1 sets a different fs_storage/max_disk"), + "a quota against an unset one must be reported the same way, got: {msg}" + ); + } + + /// The shared workdir is compared LEXICALLY, by path components: spellings + /// that differ only in redundant separators or a `.` component are the same + /// workdir, while a `..` component or a symlink naming the same real + /// directory are not. + /// + /// The symlink case is the one that matters, and it is an asymmetry rather + /// than a bug to fix here: `SeccompCowBranch::create` canonicalizes, so the + /// branch would treat the two spellings as one workdir while the validator + /// does not. Rejecting is the safe side of that disagreement — the stages + /// would otherwise disagree with each other about which paths the COW layer + /// intercepts. + #[test] + fn the_shared_workdir_is_compared_by_path_components_not_by_target() { + let base = tempfile::tempdir().unwrap(); + let real = base.path().join("wd"); + std::fs::create_dir(&real).unwrap(); + let alias = base.path().join("alias"); + std::os::unix::fs::symlink(&real, &alias).unwrap(); + + let named = |p: std::path::PathBuf| { + Stage::new(&Sandbox::builder().workdir(p).build().unwrap(), &["true"]) + }; + let same = [ + std::path::PathBuf::from(format!("{}/", real.display())), + real.join("."), + ]; + for spelling in same { + assert_eq!( + rejection(&[named(real.clone()), named(spelling.clone())]), + None, + "{} names the same workdir as {} and must be accepted", + spelling.display(), + real.display() + ); + } + for spelling in [alias.clone(), real.join("..").join("wd")] { + let msg = rejection(&[named(real.clone()), named(spelling.clone())]).unwrap_or_else( + || { + panic!( + "{} is not the same path as {} and must be rejected", + spelling.display(), + real.display() + ) + }, + ); + assert!( + msg.contains("stages must share one workdir"), + "a differing workdir spelling must be reported as the shared-workdir conflict, \ + got: {msg}" + ); + } + } + + /// `chroot` and `no_supervisor` are rejected ABSOLUTELY — even when every + /// stage sets them identically — while `workdir` and `fs_storage`/`max_disk` + /// are only required to AGREE, so a value shared by every stage is accepted. + /// + /// The two kinds of rule sit three lines apart in the same loop and either + /// could be "simplified" into the other without a test noticing: comparing + /// `chroot` against stage 0's would let a fully-chrooted transaction run, + /// and rejecting any `fs_storage` at all would break the ordinary case of + /// pointing every stage at one storage dir. + #[test] + fn chroot_and_no_supervisor_are_absolute_while_workdir_and_storage_only_have_to_agree() { + let wd = tempfile::tempdir().unwrap(); + let chroot = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + let chrooted = + Sandbox::builder().workdir(wd.path()).chroot(chroot.path()).build().unwrap(); + let msg = rejection(&[ + Stage::new(&chrooted, &["true"]), + Stage::new(&chrooted, &["true"]), + ]) + .expect("chroot is unsupported with a shared COW workdir even when every stage sets it"); + assert!( + msg.contains("stage 0 sets chroot"), + "a chroot every stage sets must still be rejected, at the first stage, got: {msg}" + ); + + let unsupervised = + Sandbox::builder().workdir(wd.path()).no_supervisor(true).build().unwrap(); + let msg = rejection(&[ + Stage::new(&unsupervised, &["true"]), + Stage::new(&unsupervised, &["true"]), + ]) + .expect("a transaction cannot run without the COW supervisor, however many stages agree"); + assert!( + msg.contains("stage 0 has no_supervisor=true"), + "no_supervisor must be rejected even when shared, at the first stage, got: {msg}" + ); + + let shared = Sandbox::builder() + .workdir(wd.path()) + .fs_storage(storage.path()) + .max_disk(ByteSize(4096)) + .build() + .unwrap(); + assert_eq!( + rejection(&[Stage::new(&shared, &["true"]), Stage::new(&shared, &["true"])]), + None, + "one storage dir and one quota named by every stage is the ordinary configuration \ + and must be accepted" + ); + } + + /// Which problem a caller is told about is fixed: the LOWEST offending stage + /// wins across stages, and within one stage the order is workdir → + /// no_supervisor → chroot → fs_storage/max_disk → on_exit/on_error. + /// + /// This is user-visible: the message is what a caller shows, so reordering + /// the checks silently changes which of several real problems gets reported + /// and which stays hidden until the first one is fixed. + #[test] + fn the_first_offending_stage_and_the_first_broken_rule_within_it_are_what_get_reported() { + let wd = tempfile::tempdir().unwrap(); + let other = tempfile::tempdir().unwrap(); + let chroot = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + + // Across stages: stage 1 breaks chroot, stage 2 breaks no_supervisor. + // Stage 1 is reported even though the rule it breaks is checked later. + let s1 = Sandbox::builder().workdir(wd.path()).chroot(chroot.path()).build().unwrap(); + let s2 = Sandbox::builder().workdir(wd.path()).no_supervisor(true).build().unwrap(); + let msg = rejection(&[ok_stage(wd.path()), Stage::new(&s1, &["true"]), Stage::new(&s2, &["true"])]) + .expect("two offending stages must still be rejected"); + assert!( + msg.contains("stage 1 sets chroot"), + "the lowest offending stage index wins, got: {msg}" + ); + + // Within one stage: peel the violations off in check order and assert + // each successive rule is the one reported. + let all = Sandbox::builder() + .workdir(other.path()) + .no_supervisor(true) + .chroot(chroot.path()) + .fs_storage(storage.path()) + .on_exit(BranchAction::Keep) + .build() + .unwrap(); + let peeled = [ + ("stages must share one workdir", all.clone()), + ("stage 1 has no_supervisor=true", { + let mut s = all.clone(); + s.workdir = Some(wd.path().to_path_buf()); + s + }), + ("stage 1 sets chroot", { + let mut s = all.clone(); + s.workdir = Some(wd.path().to_path_buf()); + s.no_supervisor = false; + s + }), + ("stage 1 sets a different fs_storage/max_disk", { + let mut s = all.clone(); + s.workdir = Some(wd.path().to_path_buf()); + s.no_supervisor = false; + s.chroot = None; + s + }), + ("stage 1 sets on_exit/on_error", { + let mut s = all.clone(); + s.workdir = Some(wd.path().to_path_buf()); + s.no_supervisor = false; + s.chroot = None; + s.fs_storage = None; + s + }), + ]; + for (expected, offender) in peeled { + let msg = rejection(&[ok_stage(wd.path()), Stage::new(&offender, &["true"])]) + .unwrap_or_else(|| panic!("expected a rejection naming {expected:?}")); + assert!( + msg.contains(expected), + "checks must run in a fixed order; expected {expected:?}, got: {msg}" + ); + } + } + + // ------------------------------------------------------------ + // Error channels + // ------------------------------------------------------------ + + /// Which of the RFC's channels a failure belongs to. + #[derive(Debug, PartialEq, Eq)] + enum Channel { + Config, + StageDriver, + Commit, + Conflict, + } + + /// Classify a failure by matching EXHAUSTIVELY, so that the table-driven + /// tests below cannot drift into asserting over a subset of the type. + /// + /// `#[non_exhaustive]` has no effect inside the defining crate, so adding a + /// `TxnError` variant stops this file compiling. `TxnError::exit_code` is + /// exhaustive too and would already have caught that; what this adds is the + /// channel itself, which `From for SandlockError` decides in a + /// catch-all `other` arm that would absorb a new variant silently. + fn channel_of(e: &TxnError) -> Channel { + match e { + TxnError::Invalid(_) => Channel::Config, + TxnError::Stage { .. } => Channel::StageDriver, + TxnError::Branch { .. } + | TxnError::CommitLock { .. } + | TxnError::Merge { .. } + | TxnError::CommitAbandoned(_) => Channel::Commit, + TxnError::Conflict { .. } => Channel::Conflict, + } + } + + /// One of every `TxnError` variant, for table-driven tests over the whole + /// error type. + fn one_of_every_txn_error() -> Vec { + vec![ + TxnError::Invalid("bad stages".into()), + TxnError::Stage { + index: 0, + source: SandlockError::Runtime(SandboxRuntimeError::NotRunning), + }, + TxnError::Branch { + workdir: "/wd".into(), + source: crate::error::BranchError::Operation("create upper".into()), + }, + TxnError::Conflict { + workdir: "/wd".into(), + waited: Duration::from_secs(1), + preserved_upper: "/st/upper".into(), + }, + TxnError::CommitLock { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: std::io::Error::other("flock"), + }, + TxnError::Merge { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: crate::error::BranchError::Operation("copy".into()), + }, + TxnError::CommitAbandoned("runtime shutting down".into()), + ] + } + + /// Every variant reports the EXACT `sysexits.h` code its channel is + /// documented to use: 78 config, 70 stage driver, 74 commit, 75 conflict. + /// + /// The sibling test pins the four codes apart and this one pins their + /// values, which is a different property: renumbering config from 78 to 64 + /// keeps every code distinct and every existing assertion green, while a + /// caller's `case 78)` stops matching. + #[test] + fn every_txn_error_variant_reports_the_exact_exit_code_of_its_channel() { + for e in one_of_every_txn_error() { + let want = match channel_of(&e) { + Channel::Config => 78, + Channel::StageDriver => 70, + Channel::Commit => 74, + Channel::Conflict => 75, + }; + assert_eq!( + e.exit_code(), + want, + "{e:?} is in channel {:?} and must report {want}", + channel_of(&e) + ); + } + } + + /// Flattening into `SandlockError` keeps, for every commit-channel variant, + /// the phrase that says what state the workdir and the change set are in. + /// + /// The rendered message is the ONLY thing that survives the flatten — the + /// typed fields do not — so a variant whose text is dropped or replaced + /// leaves a caller unable to tell "nothing ran" from "the workdir may be + /// half merged". The sibling flatten test asserts the preserved upper is + /// kept, which the two variants that have no preserved upper cannot carry. + #[test] + fn flattening_a_commit_channel_failure_keeps_the_phrase_that_names_the_workdir_state() { + let tokens = [ + ("failed to create the shared COW branch over /wd", "nothing ran"), + ("gave up after", "contended, retry"), + ("could not take the commit lock on /wd", "lock broken"), + ("may be partially merged", "torn workdir"), + ("did not run to completion", "unknown"), + ]; + let commit_channel: Vec = one_of_every_txn_error() + .into_iter() + .filter(|e| matches!(channel_of(e), Channel::Commit | Channel::Conflict)) + .collect(); + assert_eq!( + commit_channel.len(), + tokens.len(), + "every commit-channel variant needs a phrase pinned here" + ); + + for (e, (token, meaning)) in commit_channel.into_iter().zip(tokens) { + let debug = format!("{e:?}"); + let flat: SandlockError = e.into(); + let rendered = flat.to_string(); + assert!( + rendered.contains(token), + "{debug} means {meaning:?}, so flattening must keep {token:?}, got: {rendered}" + ); + } + } + + /// A stage that exits with one of the transaction channel's own numbers has + /// that number reported back verbatim. + /// + /// The numbers deliberately collide — a child is free to exit 75 — and the + /// documented way to tell the channels apart is the `Result`, not the code. + /// "Fixing" the collision by remapping a child's code would make the outcome + /// report a number the command never produced. + #[test] + fn a_stage_exit_code_that_collides_with_the_commit_channel_is_reported_verbatim() { + for code in [70, 74, 75, 78] { + let outcome = aborted(AbortReason::StageFailed { + index: 0, + status: ExitStatus::Code(code), + }); + assert_eq!( + outcome.exit_code(), + code, + "a stage that exited {code} must report {code}, not a remapped transaction code" + ); + } + } + + /// Each `AbortReason` renders its own sentence, and a stage failure names + /// the stage and its status. + /// + /// `Display` is the only human-readable form of the abort channel and + /// nothing else in the workspace calls it, so its arms can be reordered, + /// swapped or deleted without a single test noticing. + #[test] + fn abort_reason_renders_each_of_its_arms_distinctly() { + let stage_failed = AbortReason::StageFailed { + index: 2, + status: ExitStatus::Code(3), + } + .to_string(); + assert!( + stage_failed.contains("stage 2") && stage_failed.contains("Code(3)"), + "a stage failure must name the stage and its status, got: {stage_failed}" + ); + assert!( + AbortReason::TimedOut.to_string().contains("timed out"), + "a timeout must say so, got: {}", + AbortReason::TimedOut + ); + let dry = AbortReason::DryRun.to_string(); + assert!( + dry.contains("dry run") && dry.contains("not committed"), + "a dry run must say the changes were reported rather than committed, got: {dry}" + ); + + let rendered: std::collections::HashSet = + [stage_failed, AbortReason::TimedOut.to_string(), dry].into_iter().collect(); + assert_eq!(rendered.len(), 3, "each abort reason must render differently: {rendered:?}"); + } + + // ------------------------------------------------------------ + // Commit lock + // ------------------------------------------------------------ + + /// A zero wait still ACQUIRES an uncontended lock. The loop attempts the + /// lock first and only then checks the deadline, so "no budget to wait" is + /// not "no budget to succeed". + /// + /// The sibling zero-wait test covers the contended half. Together they pin + /// the loop order: hoisting the deadline check above the `flock` attempt + /// keeps the contended case correct — it still reports `Contended` without + /// sleeping — while making every uncontended zero-wait commit fail on a + /// workdir nobody is touching. + #[test] + fn commit_lock_with_a_zero_wait_still_takes_an_uncontended_lock() { + let dir = tempfile::tempdir().unwrap(); + let mut polls = 0usize; + let lock = acquire_commit_lock_polling(dir.path(), Duration::ZERO, |_| polls += 1) + .expect("a lock nobody holds must be taken even with no budget to wait for it"); + assert_eq!(polls, 0, "taking a free lock must not sleep"); + drop(lock); + } + + /// A commit lock that fails for a reason OTHER than contention preserves the + /// upper too. + /// + /// Every stage exited 0 either way, so the change set is complete and + /// mergeable whether the lock was busy or the workdir had vanished — and it + /// is `CommitDeferred` in both cases, because in neither was the workdir + /// touched. The sibling test covers the contended route only, so a change + /// that preserved just that one — "a broken workdir means these changes are + /// useless" — would silently reclaim a complete change set. + #[test] + fn finish_branch_preserves_the_upper_when_the_workdir_cannot_be_locked_at_all() { + let (workdir, _storage, branch) = branch_holding_one_added_file(); + let branch_dir = branch.upper_dir().parent().unwrap().to_path_buf(); + let gone = workdir.path().join("no-such-workdir"); + + let finished = finish_branch(branch, &gone, Duration::from_millis(50), true); + + assert!( + matches!(finished.commit, Some(Err(CommitFailure::Lock(LockFailure::Io(_))))), + "a workdir that cannot be opened is an I/O failure, not contention" + ); + let survived = std::fs::read_to_string(branch_dir.join("upper").join("a.txt")) + .unwrap_or_else(|e| { + panic!("the unpublished change set must survive on disk, but reading it gave {e}") + }); + assert_eq!(survived, "plan\n", "the preserved upper must still hold the stage's bytes"); + let preserved = crate::cow::seccomp::read_preserved(&branch_dir) + .expect("a preserved upper must be findable through its marker"); + assert_eq!( + preserved.reason, + crate::cow::seccomp::PreserveReason::CommitDeferred, + "the lock was never taken, so the workdir is untouched: that is CommitDeferred" + ); + } + + // ------------------------------------------------------------ + // The type a transaction is deliberately NOT + // ------------------------------------------------------------ + + /// `Pipeline::is_empty` is documented "always false"; this is the constructor + /// invariant that makes that true. + /// + /// It lives here because the transaction module's docs lean on `Pipeline` + /// being a distinct, always-populated type — `into_stages` is the only way + /// to move stages between the two runners, and it is documented as handing + /// over a chain, not a possibly-empty list. + #[test] + fn a_pipeline_can_never_be_empty() { + crate::pipeline::Pipeline::new(Vec::new()) + .err() + .expect("a pipeline of no stages must be rejected at construction"); + let sb = Sandbox::builder().build().unwrap(); + let chain = crate::pipeline::Pipeline::new(vec![ + Stage::new(&sb, &["true"]), + Stage::new(&sb, &["true"]), + ]) + .expect("two stages are a valid chain"); + assert!(!chain.is_empty(), "a constructed pipeline always has stages"); + assert_eq!(chain.len(), 2, "and reports how many"); + } + + /// The commit lock WAITS for a concurrent merge instead of failing fast: + /// a transaction that ran every stage successfully must publish its work, + /// not lose it because another merge was in flight for a moment. + #[test] + fn commit_lock_waits_for_a_held_lock_to_be_released() { + let dir = tempfile::tempdir().unwrap(); + let held = std::fs::File::open(dir.path()).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0 + ); + + let releaser = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(250)); + drop(held); + }); + + let mut polls = 0usize; + let lock = acquire_commit_lock_polling(dir.path(), Duration::from_secs(10), |d| { + polls += 1; + std::thread::sleep(d); + }) + .expect("must acquire the lock once the holder releases it"); + releaser.join().unwrap(); + drop(lock); + + assert!( + polls > 0, + "must actually have gone round the poll loop waiting for the holder" + ); + } + + /// The wait is bounded, and expiring it is reported as CONTENTION rather + /// than as a broken workdir: that distinction is what makes the failure a + /// retryable `TxnError::Conflict` instead of an I/O error. + #[test] + fn commit_lock_wait_is_bounded_and_reports_contention() { + let dir = tempfile::tempdir().unwrap(); + let held = std::fs::File::open(dir.path()).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0 + ); + + let started = std::time::Instant::now(); + let err = acquire_commit_lock(dir.path(), Duration::from_millis(200)) + .expect_err("a lock that is never released must time out"); + let waited = started.elapsed(); + drop(held); + + assert!( + matches!(err, LockFailure::Contended(d) if d == Duration::from_millis(200)), + "a held lock must be reported as contention carrying the wait, got: {err:?}" + ); + assert!( + waited >= Duration::from_millis(200), + "must have waited out the whole deadline, waited {waited:?}" + ); + } + + /// A workdir that cannot be opened at all is NOT contention: it must not be + /// reported as a retryable conflict. + #[test] + fn commit_lock_reports_a_missing_workdir_as_io() { + let dir = tempfile::tempdir().unwrap(); + let gone = dir.path().join("no-such-workdir"); + let err = acquire_commit_lock(&gone, Duration::from_millis(200)) + .expect_err("a workdir that does not exist cannot be locked"); + assert!( + matches!(err, LockFailure::Io(_)), + "a missing workdir is an I/O failure, not contention, got: {err:?}" + ); + } + + /// The three failure channels stay apart, and stay honest, when flattened + /// into the crate-wide error. Each arm is pinned to its exact variant: a + /// commit failure reported as `Runtime(Child(..))` would claim a child + /// process failed, which is the abort channel and cannot reach here, and + /// `Runtime(_)` alone does not notice that. + #[test] + fn txn_errors_flatten_without_losing_the_channel() { + let invalid: SandlockError = TxnError::Invalid("bad stages".into()).into(); + assert!( + matches!(invalid, SandlockError::Sandbox(SandboxError::Invalid(_))), + "a configuration error must stay a sandbox error, got: {invalid:?}" + ); + + // A stage that could not be driven keeps its own error verbatim. + let stage: SandlockError = TxnError::Stage { + index: 0, + source: SandlockError::Runtime(SandboxRuntimeError::NotRunning), + } + .into(); + assert!( + matches!(stage, SandlockError::Runtime(SandboxRuntimeError::NotRunning)), + "a stage driver failure must flatten to the stage's own error, got: {stage:?}" + ); + + for original in [ + TxnError::Conflict { + workdir: "/wd".into(), + waited: Duration::from_secs(1), + preserved_upper: "/st/upper".into(), + }, + TxnError::Merge { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: crate::error::BranchError::Operation("copy".into()), + }, + TxnError::CommitLock { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: std::io::Error::other("flock"), + }, + ] { + let rendered = original.to_string(); + let flat: SandlockError = original.into(); + assert!( + matches!(flat, SandlockError::Runtime(SandboxRuntimeError::Branch(_))), + "a commit-channel failure is a branch failure, not a child failure, got: {flat:?}" + ); + assert!( + flat.to_string().contains("/st/upper"), + "flattening must not lose the preserved upper of {rendered:?}, got: {flat}" + ); + } + } + + /// Each commit-channel failure the RFC names gets its own exit code, and a + /// conflict specifically reports EX_TEMPFAIL: "retry me". + #[test] + fn commit_channel_failures_have_distinct_exit_codes() { + let stage = TxnError::Stage { + index: 0, + source: SandlockError::Runtime(SandboxRuntimeError::NotRunning), + }; + let merge = TxnError::Merge { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: crate::error::BranchError::Operation("copy".into()), + }; + let conflict = TxnError::Conflict { + workdir: "/wd".into(), + waited: Duration::from_secs(1), + preserved_upper: "/st/upper".into(), + }; + let invalid = TxnError::Invalid("bad stages".into()); + + let codes = [stage.exit_code(), merge.exit_code(), conflict.exit_code(), invalid.exit_code()]; + let unique: std::collections::HashSet = codes.iter().copied().collect(); + assert_eq!( + unique.len(), + codes.len(), + "stage/commit/conflict/config must not share an exit code, got {codes:?}" + ); + assert_eq!(conflict.exit_code(), 75, "a conflict is EX_TEMPFAIL: retry it"); + } + + /// A zero wait must give up on a held lock immediately, without sleeping + /// once: the deadline is checked before the poll sleeps, not after it. + /// + /// Asserted by counting sleeps. Timing it cannot carry this — the whole + /// difference between checking the deadline before and after the sleep is + /// one `COMMIT_LOCK_POLL`, and it still returns the same `Contended`. + #[test] + fn commit_lock_with_a_zero_wait_gives_up_without_sleeping() { + let dir = tempfile::tempdir().unwrap(); + let held = std::fs::File::open(dir.path()).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0 + ); + + let mut polls = 0usize; + let err = acquire_commit_lock_polling(dir.path(), Duration::ZERO, |_| polls += 1) + .expect_err("a held lock with no wait budget cannot be taken"); + drop(held); + + assert_eq!(polls, 0, "a zero wait must not sleep before giving up"); + assert!( + matches!(err, LockFailure::Contended(d) if d == Duration::ZERO), + "giving up on a held lock is contention even with no wait, got: {err:?}" + ); + } + + /// An uncontended lock must not go through the poll loop's wait at all — + /// the common case is every commit that is not racing another one. + /// + /// Asserted by counting the poll loop's sleeps, not by timing it. A + /// wall-clock bound cannot carry this property: a 100ms budget does not + /// notice a 20ms regression, and it flakes on a loaded runner. + #[test] + fn commit_lock_uncontended_does_not_wait_at_all() { + let dir = tempfile::tempdir().unwrap(); + let mut polls = 0usize; + let lock = acquire_commit_lock_polling(dir.path(), Duration::from_secs(10), |d| { + polls += 1; + std::thread::sleep(d); + }) + .unwrap(); + assert_eq!(polls, 0, "taking an uncontended lock must not sleep"); + drop(lock); + } + + /// The whole commit channel — every way the shared upper's disposition can + /// fail other than contention — reports one exit code, EX_IOERR. + /// + /// The sibling test above pins the channels apart; this one pins them + /// together: a caller that keys "the change set is on disk, go recover it" + /// off 74 must get it from the merge failure, the lock failure, the branch + /// that could never be created and the commit that was abandoned alike. + #[test] + fn every_commit_channel_failure_reports_ex_ioerr() { + let failures = [ + TxnError::Branch { + workdir: "/wd".into(), + source: crate::error::BranchError::Operation("create upper".into()), + }, + TxnError::CommitLock { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: std::io::Error::other("flock"), + }, + TxnError::Merge { + workdir: "/wd".into(), + preserved_upper: "/st/upper".into(), + source: crate::error::BranchError::Operation("copy".into()), + }, + TxnError::CommitAbandoned("runtime shutting down".into()), + ]; + for failure in failures { + assert_eq!( + failure.exit_code(), + 74, + "the commit channel is EX_IOERR, but {failure:?} reported {}", + failure.exit_code() + ); + } + } + + fn aborted(reason: AbortReason) -> TxnOutcome { + TxnOutcome { + committed: false, + stages: Vec::new(), + changes: Vec::new(), + abort_reason: Some(reason), + } + } + + /// An aborted transaction reports the number a caller fronting it with a + /// command-line tool would have got from the command itself: the failing + /// stage's own code, `128 + signal` for a signalled one, and 124 for a + /// timeout as `timeout(1)` reports it. + /// + /// Only `Code` is exercised end to end (a stage exiting 1); the signalled + /// and killed stages are what this pins, because collapsing them onto the + /// signal number — 9 instead of 137 — is both easy to write and + /// indistinguishable from a command that really exited 9. + #[test] + fn an_aborted_outcome_reports_the_failing_stage_status_as_a_shell_would() { + for (status, want) in [ + (ExitStatus::Code(3), 3), + (ExitStatus::Signal(libc::SIGTERM), 128 + libc::SIGTERM), + (ExitStatus::Killed, 128 + libc::SIGKILL), + (ExitStatus::Timeout, 124), + ] { + let outcome = aborted(AbortReason::StageFailed { index: 1, status: status.clone() }); + assert_eq!( + outcome.exit_code(), + want, + "a stage that ended as {status:?} must report {want}" + ); + } + assert_eq!( + aborted(AbortReason::TimedOut).exit_code(), + 124, + "a transaction timeout reports 124, as timeout(1) does" + ); + } + + /// A transaction that did what was asked reports success — including a dry + /// run, which does not commit but did not fail either. + #[test] + fn a_committed_transaction_and_a_completed_dry_run_both_report_success() { + let committed = TxnOutcome { + committed: true, + stages: Vec::new(), + changes: Vec::new(), + abort_reason: None, + }; + assert_eq!(committed.exit_code(), 0, "a committed transaction succeeded"); + assert_eq!( + aborted(AbortReason::DryRun).exit_code(), + 0, + "a dry run reported its changes as asked; not committing is not a failure" + ); + } + + /// A branch over a fresh workdir whose upper already holds `a.txt`, as a + /// run whose stages all succeeded would have left it. The tempdirs are + /// returned because dropping them removes the workdir and the storage. + fn branch_holding_one_added_file( + ) -> (tempfile::TempDir, tempfile::TempDir, crate::cow::seccomp::SeccompCowBranch) { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let branch = crate::cow::seccomp::SeccompCowBranch::create( + workdir.path(), + Some(storage.path()), + 0, + ) + .unwrap(); + std::fs::write(branch.upper_dir().join("a.txt"), "plan\n").unwrap(); + (workdir, storage, branch) + } + + /// Discarding the upper still reports what it held: the change set is read + /// off the branch BEFORE it is disposed of, because after the abort there + /// is nothing left on disk to read. This is what fills `TxnOutcome::changes` + /// on the abort and dry-run paths. + #[test] + fn finish_branch_discarding_the_upper_still_reports_what_it_held() { + let (workdir, _storage, branch) = branch_holding_one_added_file(); + let branch_dir = branch.upper_dir().parent().unwrap().to_path_buf(); + + let finished = finish_branch(branch, workdir.path(), Duration::from_secs(30), false); + + assert!( + finished.commit.is_none(), + "an upper that was discarded rather than merged reports no commit result" + ); + let paths: Vec<_> = finished + .changes + .iter() + .map(|c| (c.kind.clone(), c.path.clone())) + .collect(); + assert_eq!( + paths, + vec![(crate::dry_run::ChangeKind::Added, std::path::PathBuf::from("a.txt"))], + "the discarded change set must still be reported" + ); + assert!( + !workdir.path().join("a.txt").exists(), + "a discarded upper must not reach the workdir" + ); + assert!(!branch_dir.exists(), "a discarded upper must be reclaimed from disk"); + } + + /// The commit merges the upper into the workdir and does not keep the + /// workdir lock afterwards: the lock is mutual exclusion between merges, so + /// holding it past the merge would stall every later transaction for the + /// life of this process. + #[test] + fn finish_branch_merges_the_upper_and_leaves_the_workdir_unlocked() { + let (workdir, _storage, branch) = branch_holding_one_added_file(); + let branch_dir = branch.upper_dir().parent().unwrap().to_path_buf(); + + let finished = finish_branch(branch, workdir.path(), Duration::from_secs(30), true); + + assert!( + matches!(finished.commit, Some(Ok(()))), + "an uncontended commit of a mergeable upper must succeed" + ); + assert_eq!( + std::fs::read_to_string(workdir.path().join("a.txt")).unwrap(), + "plan\n", + "the committed change set must be in the workdir, with its bytes" + ); + assert!(!branch_dir.exists(), "a merged upper must be reclaimed from disk"); + + let after = std::fs::File::open(workdir.path()).unwrap(); + assert_eq!( + unsafe { libc::flock(after.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "the commit must release the workdir lock, not hold it past the merge" + ); + } + + /// Giving up on the commit lock hands the storage over instead of dropping + /// it: every stage exited 0, so the upper holds a complete change set that + /// only failed to be published. + /// + /// The marker says `CommitDeferred`, which is the part a recovery acts on — + /// it means the workdir was never touched and the whole change set is here, + /// unlike `MergeInterrupted`, where the workdir may already be half merged. + #[test] + fn finish_branch_preserves_the_upper_as_commit_deferred_when_the_lock_is_contended() { + let (workdir, _storage, branch) = branch_holding_one_added_file(); + let branch_dir = branch.upper_dir().parent().unwrap().to_path_buf(); + + // Stand in for another transaction whose merge never finishes. + let held = std::fs::File::open(workdir.path()).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + let finished = finish_branch(branch, workdir.path(), Duration::from_millis(50), true); + drop(held); + + assert!( + matches!( + finished.commit, + Some(Err(CommitFailure::Lock(LockFailure::Contended(d)))) if d == Duration::from_millis(50) + ), + "a lock held for the whole wait must be reported as contention" + ); + assert!( + !workdir.path().join("a.txt").exists(), + "nothing may be merged when the lock was never taken" + ); + let survived = std::fs::read_to_string(branch_dir.join("upper").join("a.txt")) + .unwrap_or_else(|e| { + panic!("the unpublished change set must survive on disk, but reading it gave {e}") + }); + assert_eq!(survived, "plan\n", "the preserved upper must still hold the stage's bytes"); + let preserved = crate::cow::seccomp::read_preserved(&branch_dir) + .expect("a preserved upper must be findable through its marker"); + assert_eq!( + preserved.reason, + crate::cow::seccomp::PreserveReason::CommitDeferred, + "the workdir is untouched and the whole change set is here: that is CommitDeferred" + ); + } + + /// A merge that fails partway leaves the remainder on disk rather than + /// reclaiming it, and marks it as an interrupted merge — the workdir may + /// hold part of the change set, so recovery cannot assume it is untouched. + /// + /// The merge is made to fail without any privilege trick: the upper holds a + /// regular file where the workdir already has a directory of the same name, + /// so the merge's `open(O_WRONLY|O_CREAT)` on it fails with `EISDIR`. + #[test] + fn finish_branch_preserves_the_remainder_when_the_merge_fails() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::fs::create_dir(workdir.path().join("x")).unwrap(); + let branch = crate::cow::seccomp::SeccompCowBranch::create( + workdir.path(), + Some(storage.path()), + 0, + ) + .unwrap(); + let branch_dir = branch.upper_dir().parent().unwrap().to_path_buf(); + std::fs::write(branch.upper_dir().join("x"), "built\n").unwrap(); + + let finished = finish_branch(branch, workdir.path(), Duration::from_secs(30), true); + + assert!( + matches!(finished.commit, Some(Err(CommitFailure::Merge(_)))), + "a merge that cannot write an entry into the workdir must report a merge failure" + ); + let remainder = std::fs::read_to_string(branch_dir.join("upper").join("x")) + .unwrap_or_else(|e| { + panic!("the change that did not land must survive on disk, but reading it gave {e}") + }); + assert_eq!(remainder, "built\n", "the preserved remainder must still hold its bytes"); + let preserved = crate::cow::seccomp::read_preserved(&branch_dir) + .expect("a preserved remainder must be findable through its marker"); + assert_eq!( + preserved.reason, + crate::cow::seccomp::PreserveReason::MergeInterrupted, + "the workdir may be partially merged, so this is not CommitDeferred" + ); + } +} diff --git a/crates/sandlock-core/tests/integration.rs b/crates/sandlock-core/tests/integration.rs index bb93f880..883d28ad 100644 --- a/crates/sandlock-core/tests/integration.rs +++ b/crates/sandlock-core/tests/integration.rs @@ -31,6 +31,9 @@ mod test_landlock; #[path = "integration/test_pipeline.rs"] mod test_pipeline; +#[path = "integration/test_transaction.rs"] +mod test_transaction; + #[path = "integration/test_network.rs"] mod test_network; diff --git a/crates/sandlock-core/tests/integration/test_cow.rs b/crates/sandlock-core/tests/integration/test_cow.rs index f683514a..98c30f76 100644 --- a/crates/sandlock-core/tests/integration/test_cow.rs +++ b/crates/sandlock-core/tests/integration/test_cow.rs @@ -1,4 +1,4 @@ -use sandlock_core::{Sandbox}; +use sandlock_core::{Sandbox, SandboxBuilder}; use sandlock_core::sandbox::BranchAction; use std::fs; use std::path::PathBuf; @@ -611,3 +611,477 @@ async fn test_seccomp_cow_exec_packed_argv_relocation() { let _ = fs::remove_dir_all(&workdir); } + +// ============================================================ +// Branch disposition on an abandoned sandbox +// ============================================================ + +/// Number of branch subdirectories under a `fs_storage` dir. +fn branch_count(storage: &std::path::Path) -> usize { + fs::read_dir(storage).map(|rd| rd.count()).unwrap_or(0) +} + +/// `BranchAction::Keep` means "preserve the changes for later inspection", and a +/// sandbox that is abandoned without `wait()` IS the case worth inspecting. +/// +/// A branch only reaches `Sandbox`'s own disposition after a completed `wait()`, +/// so on this path the branch's `Drop` backstop is what decides. It must honour +/// an explicit `Keep` — and must still reclaim under the default action, which +/// is the leak the backstop exists to close. +/// +/// `Keep` is honoured from EITHER action. An abandoned run has no exit status, +/// so there is no choosing between `on_exit` and `on_error`: a caller who asked +/// to keep the changes in either case asked to keep them here. +#[tokio::test] +async fn test_abandoned_sandbox_honours_keep_and_still_reclaims_by_default() { + let workdir = temp_dir("abandon-wd"); + let keep_store = temp_dir("abandon-keep-st"); + let on_error_store = temp_dir("abandon-onerror-st"); + let default_store = temp_dir("abandon-default-st"); + for d in [&keep_store, &on_error_store, &default_store] { + let _ = fs::remove_dir_all(d); + let _ = fs::create_dir_all(d); + } + + let base = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir); + + { + let mut sb = base.clone() + .fs_storage(&keep_store) + .on_exit(BranchAction::Keep) + .build() + .unwrap(); + sb.create(&["sh", "-c", "echo kept > k.txt"]).await.unwrap(); + sb.start().unwrap(); + // Dropped here, with no wait(): the caller walked away from the run. + } + { + let mut sb = base.clone() + .fs_storage(&on_error_store) + .on_error(BranchAction::Keep) + .build() + .unwrap(); + sb.create(&["sh", "-c", "echo kept > k.txt"]).await.unwrap(); + sb.start().unwrap(); + } + { + let mut sb = base.clone() + .fs_storage(&default_store) + .build() + .unwrap(); + sb.create(&["sh", "-c", "echo gone > g.txt"]).await.unwrap(); + sb.start().unwrap(); + } + + // Dropping the sandbox does not itself drop the branch: the shared COW state + // is also held by the aborted supervisor task, so the branch is dropped when + // the runtime reaps it. Wait for the default store to empty — that reclaim IS + // the proof that the branch drops have run, without which the Keep assertion + // below would pass on a branch that simply had not been dropped yet. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while branch_count(&default_store) != 0 && std::time::Instant::now() < deadline { + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + assert_eq!( + branch_count(&default_store), 0, + "an abandoned sandbox with the default action must still reclaim its upper", + ); + assert_eq!( + branch_count(&keep_store), 1, + "an abandoned sandbox configured on_exit(Keep) must still preserve its upper", + ); + assert_eq!( + branch_count(&on_error_store), 1, + "an abandoned sandbox configured on_error(Keep) must still preserve its upper", + ); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&keep_store); + let _ = fs::remove_dir_all(&on_error_store); + let _ = fs::remove_dir_all(&default_store); +} + +// ============================================================ +// Branch disposition on a completed run +// ============================================================ + +/// Build a COW sandbox over `workdir` with the two dispositions under test. +fn disposition_sandbox( + workdir: &std::path::Path, + on_exit: BranchAction, + on_error: BranchAction, +) -> Sandbox { + Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(workdir).workdir(workdir).cwd(workdir) + .on_exit(on_exit) + .on_error(on_error) + .build() + .unwrap() +} + +/// Run `script` in a fresh COW workdir under the two dispositions and report +/// the child's exit code together with whether its write reached the workdir. +async fn run_and_report_landing( + tag: &str, + script: &str, + on_exit: BranchAction, + on_error: BranchAction, +) -> (Option, bool) { + let workdir = temp_dir(tag); + let _ = fs::remove_dir_all(&workdir); + let _ = fs::create_dir_all(&workdir); + + let code = { + let mut sb = disposition_sandbox(&workdir, on_exit, on_error); + let result = sb.run(&["sh", "-c", script]).await.unwrap(); + result.code() + // Dropped here: the disposition runs in `Sandbox`'s Drop. + }; + let landed = workdir.join("f.txt").exists(); + let _ = fs::remove_dir_all(&workdir); + (code, landed) +} + +/// Which branch action a completed run applies is selected by the child's exit +/// status: `on_exit` for exit code 0, `on_error` for a non-zero one. +/// +/// The two failing runs differ only in which way round the actions are wired, +/// so an implementation that always took `on_exit` and one that always took +/// `on_error` each contradict one of them; the succeeding run pins which of the +/// two names goes with which status. +#[tokio::test] +async fn test_branch_action_is_selected_by_the_child_exit_status() { + let script_fail = "echo written > f.txt; exit 3"; + let script_ok = "echo written > f.txt"; + + let (code, landed) = run_and_report_landing( + "disp-fail-abort", script_fail, BranchAction::Commit, BranchAction::Abort, + ).await; + assert_eq!(code, Some(3), "the failing script must actually have failed"); + assert!( + !landed, + "a non-zero exit must apply on_error (Abort), so the write must not reach the workdir", + ); + + let (code, landed) = run_and_report_landing( + "disp-fail-commit", script_fail, BranchAction::Abort, BranchAction::Commit, + ).await; + assert_eq!(code, Some(3), "the failing script must actually have failed"); + assert!( + landed, + "a non-zero exit must apply on_error (Commit), so the write must reach the workdir", + ); + + let (code, landed) = run_and_report_landing( + "disp-ok-commit", script_ok, BranchAction::Commit, BranchAction::Abort, + ).await; + assert_eq!(code, Some(0), "the succeeding script must actually have succeeded"); + assert!( + landed, + "exit code 0 must apply on_exit (Commit), so the write must reach the workdir", + ); + + let (code, landed) = run_and_report_landing( + "disp-ok-abort", script_ok, BranchAction::Abort, BranchAction::Commit, + ).await; + assert_eq!(code, Some(0), "the succeeding script must actually have succeeded"); + assert!( + !landed, + "exit code 0 must apply on_exit (Abort), so the write must not reach the workdir", + ); +} + +/// A run kept with `BranchAction::Keep` leaves a marker that records the +/// deletions alongside the upper, so an out-of-band recovery restores the +/// change set instead of resurrecting the files the run deleted. +/// +/// The deletion here is the child's own `rm`, so this covers the marker written +/// from the real supervisor path: the COW layer holds that deletion in RAM only +/// and nothing in the upper represents it, which is why copying the upper back +/// over the workdir is not by itself a recovery. +#[tokio::test] +async fn test_kept_branch_marker_records_the_runs_deletion_not_only_its_upper() { + let workdir = temp_dir("keep-marker-wd"); + let storage = temp_dir("keep-marker-st"); + for d in [&workdir, &storage] { + let _ = fs::remove_dir_all(d); + let _ = fs::create_dir_all(d); + } + fs::write(workdir.join("victim.txt"), "ORIGINAL").unwrap(); + + { + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .on_exit(BranchAction::Keep) + .build() + .unwrap(); + let result = sb.run(&["sh", "-c", "rm victim.txt && echo NEW > added.txt"]).await.unwrap(); + assert!( + result.success(), + "the child must have deleted and written, exit={:?}, stderr={}", + result.code(), result.stderr_str().unwrap_or(""), + ); + } + + let preserved = sandlock_core::list_preserved(&storage); + assert_eq!(preserved.len(), 1, "the kept branch must be findable by a sweep"); + let branch = &preserved[0]; + assert_eq!(branch.reason, sandlock_core::PreserveReason::Kept); + assert_eq!( + branch.workdir, + workdir.canonicalize().unwrap(), + "the marker must name the workdir the change set belongs to", + ); + assert_eq!( + branch.deleted, + vec![PathBuf::from("victim.txt")], + "the marker must record the file the run deleted", + ); + assert_eq!( + fs::read_to_string(branch.upper.join("added.txt")).unwrap(), + "NEW\n", + "the upper must hold the file the run added", + ); + + // Keep does not merge: the workdir still holds the pre-run state, so the + // marker's deletion is the only record that the file was removed. + assert_eq!(fs::read_to_string(workdir.join("victim.txt")).unwrap(), "ORIGINAL"); + assert!(!workdir.join("added.txt").exists(), "Keep must not merge the upper"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +// ============================================================ +// Merge failure, storage layout, and the syscall error contract +// ============================================================ + +/// Build a COW sandbox over `workdir` that reads enough of the host to run a +/// shell or python, and writes only inside the workdir. +fn cow_sandbox(workdir: &std::path::Path, on_exit: BranchAction) -> SandboxBuilder { + Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc").fs_read("/dev") + .fs_write(workdir).workdir(workdir).cwd(workdir) + .on_exit(on_exit) +} + +/// A merge that fails leaves the change set on disk under a `MergeInterrupted` +/// marker, which on the plain-`Sandbox` path is the ONLY record that the +/// workdir was never updated: the disposition runs in `Drop` and discards the +/// `commit()` error, so the caller — who already has its `RunResult` in hand, +/// reporting a successful run — is never told. +/// +/// The merge is made to fail by removing the workdir between the run and the +/// disposition, so the first copy has no root to open under. +#[tokio::test] +async fn test_failed_merge_on_the_drop_path_leaves_the_upper_recoverable() { + let workdir = temp_dir("merge-fail-wd"); + let storage = temp_dir("merge-fail-st"); + for d in [&workdir, &storage] { + let _ = fs::remove_dir_all(d); + let _ = fs::create_dir_all(d); + } + + { + let mut sb = cow_sandbox(&workdir, BranchAction::Commit) + .fs_storage(&storage) + .build() + .unwrap(); + let result = sb.run(&["sh", "-c", "echo NEW > added.txt"]).await.unwrap(); + assert!( + result.success(), + "the child must have written, exit={:?}, stderr={}", + result.code(), result.stderr_str().unwrap_or(""), + ); + // Out of band, as an unmount or another process would: the workdir the + // merge is about to copy into is gone. + fs::remove_dir_all(&workdir).unwrap(); + // Dropped here: `Drop` commits, the merge fails, and the error is lost. + } + + let preserved = sandlock_core::list_preserved(&storage); + assert_eq!( + preserved.len(), 1, + "a merge that could not run must leave exactly one branch for a sweep, got {:?}", + preserved, + ); + let branch = &preserved[0]; + assert_eq!( + branch.reason, + sandlock_core::PreserveReason::MergeInterrupted, + "the reason must say the workdir may have been touched, not that the caller kept the branch", + ); + assert_eq!( + fs::read_to_string(branch.upper.join("added.txt")).unwrap(), + "NEW\n", + "the unpublished change must still be readable from the preserved upper", + ); + + let _ = fs::remove_dir_all(&storage); + let _ = fs::remove_dir_all(&workdir); +} + +/// With no `fs_storage`, branch storage goes to the per-process default base +/// `$TMPDIR/sandlock-cow-`, and `list_preserved` reads exactly one level: +/// a sweep of `$TMPDIR` itself does not reach a branch under that base. +/// +/// The two halves are the documented limitation of the default layout — a sweep +/// across process lifetimes has to enumerate the per-process bases itself — and +/// the reason a caller who wants one recoverable root must pass `fs_storage`. +#[tokio::test] +async fn test_default_storage_base_is_per_process_and_the_sweep_does_not_recurse() { + let workdir = temp_dir("default-base-wd"); + let _ = fs::remove_dir_all(&workdir); + let _ = fs::create_dir_all(&workdir); + + { + let mut sb = cow_sandbox(&workdir, BranchAction::Keep).build().unwrap(); + let result = sb + .run(&["sh", "-c", "echo x > default-base-marker.txt"]) + .await + .unwrap(); + assert!( + result.success(), + "the child must have written, exit={:?}, stderr={}", + result.code(), result.stderr_str().unwrap_or(""), + ); + } + + let base = std::env::temp_dir().join(format!("sandlock-cow-{}", std::process::id())); + let ours: Vec<_> = sandlock_core::list_preserved(&base) + .into_iter() + .filter(|p| p.upper.join("default-base-marker.txt").exists()) + .collect(); + assert_eq!( + ours.len(), 1, + "the kept branch must be under the per-process default base {}", + base.display(), + ); + assert_eq!( + ours[0].branch_dir.parent(), + Some(base.as_path()), + "the branch must sit directly under the base, one level down", + ); + + let from_tmp = sandlock_core::list_preserved(&std::env::temp_dir()); + assert!( + !from_tmp.iter().any(|p| p.branch_dir == ours[0].branch_dir), + "the sweep must not descend from $TMPDIR into the per-process base, but it reached {}", + ours[0].branch_dir.display(), + ); + + let _ = fs::remove_dir_all(&ours[0].branch_dir); + let _ = fs::remove_dir_all(&workdir); +} + +/// `O_CREAT|O_EXCL` reports EEXIST for a file that exists only in the workdir +/// as well as for one this run created in the upper, and still creates a file +/// whose name neither layer holds. +/// +/// The existence check has to span both layers: against the upper alone a +/// lock-file idiom would silently clobber a pre-existing workdir file, and +/// against the workdir alone it would clobber one the same run had just made. +#[tokio::test] +async fn test_o_excl_sees_both_the_workdir_and_the_upper() { + let workdir = temp_dir("excl-layers"); + let _ = fs::remove_dir_all(&workdir); + let _ = fs::create_dir_all(&workdir); + fs::write(workdir.join("lower.txt"), "LOWER").unwrap(); + + let script = concat!( + "import os, errno\n", + "def e(*a):\n", + " try:\n", + " os.close(os.open(*a)); return 'CREATED'\n", + " except OSError as ex: return errno.errorcode.get(ex.errno, str(ex.errno))\n", + "flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY\n", + "open('upper.txt', 'w').write('UPPER')\n", + "print('lower', e('lower.txt', flags))\n", + "print('upper', e('upper.txt', flags))\n", + "print('fresh', e('fresh.txt', flags))\n", + ); + + let mut sb = cow_sandbox(&workdir, BranchAction::Abort).build().unwrap(); + let result = sb.run(&["python3", "-c", script]).await.unwrap(); + assert!( + result.success(), + "the probe must have run, exit={:?}, stderr={}", + result.code(), result.stderr_str().unwrap_or(""), + ); + let out = result.stdout_str().unwrap_or("").to_string(); + assert!(out.contains("lower EEXIST"), "O_EXCL must see the workdir file, stdout={:?}", out); + assert!(out.contains("upper EEXIST"), "O_EXCL must see the file this run created, stdout={:?}", out); + assert!( + out.contains("fresh CREATED"), + "O_EXCL must still create a name neither layer holds, stdout={:?}", out, + ); + + // Abort: the pre-existing file must be untouched by the probe. + drop(sb); + assert_eq!(fs::read_to_string(workdir.join("lower.txt")).unwrap(), "LOWER"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// `unlink` on a directory reaches the child as EISDIR and `rmdir` on a regular +/// file as ENOTDIR, whether the path exists only in the workdir or was created +/// by this run in the upper. +/// +/// The COW layer answers both from the merged view rather than letting the +/// kernel decide, so each layer is a separate arm of the type check and each +/// needs its own case. +#[tokio::test] +async fn test_unlink_type_mismatches_reach_the_child_as_eisdir_and_enotdir() { + let workdir = temp_dir("unlink-errno"); + let _ = fs::remove_dir_all(&workdir); + let _ = fs::create_dir_all(&workdir); + fs::create_dir_all(workdir.join("lower_dir")).unwrap(); + fs::write(workdir.join("lower_file.txt"), "x").unwrap(); + + let script = concat!( + "import os, errno\n", + "def e(fn, *a):\n", + " try:\n", + " fn(*a); return 'OK'\n", + " except OSError as ex: return errno.errorcode.get(ex.errno, str(ex.errno))\n", + "os.mkdir('upper_dir')\n", + "open('upper_file.txt', 'w').write('x')\n", + "print('unlink_lower_dir', e(os.unlink, 'lower_dir'))\n", + "print('rmdir_lower_file', e(os.rmdir, 'lower_file.txt'))\n", + "print('unlink_upper_dir', e(os.unlink, 'upper_dir'))\n", + "print('rmdir_upper_file', e(os.rmdir, 'upper_file.txt'))\n", + ); + + let mut sb = cow_sandbox(&workdir, BranchAction::Abort).build().unwrap(); + let result = sb.run(&["python3", "-c", script]).await.unwrap(); + assert!( + result.success(), + "the probe must have run, exit={:?}, stderr={}", + result.code(), result.stderr_str().unwrap_or(""), + ); + let out = result.stdout_str().unwrap_or("").to_string(); + for expected in [ + "unlink_lower_dir EISDIR", + "rmdir_lower_file ENOTDIR", + "unlink_upper_dir EISDIR", + "rmdir_upper_file ENOTDIR", + ] { + assert!(out.contains(expected), "expected {:?} in stdout={:?}", expected, out); + } + + // Abort: nothing the probe touched may reach the workdir. + drop(sb); + assert!(workdir.join("lower_dir").is_dir(), "a refused unlink must not have deleted the directory"); + assert!(!workdir.join("upper_dir").exists(), "an aborted run must not publish its directory"); + + let _ = fs::remove_dir_all(&workdir); +} diff --git a/crates/sandlock-core/tests/integration/test_pipeline.rs b/crates/sandlock-core/tests/integration/test_pipeline.rs index fe580a92..a038201a 100644 --- a/crates/sandlock-core/tests/integration/test_pipeline.rs +++ b/crates/sandlock-core/tests/integration/test_pipeline.rs @@ -27,7 +27,7 @@ async fn test_stage_or_stage_returns_pipeline() { let policy = base_policy(); let pipeline = Stage::new(&policy, &["echo", "hello"]) | Stage::new(&policy, &["cat"]); - assert_eq!(pipeline.stages.len(), 2); + assert_eq!(pipeline.len(), 2); } #[tokio::test] @@ -36,7 +36,7 @@ async fn test_stage_chaining() { let pipeline = Stage::new(&policy, &["echo", "a"]) | Stage::new(&policy, &["cat"]) | Stage::new(&policy, &["cat"]); - assert_eq!(pipeline.stages.len(), 3); + assert_eq!(pipeline.len(), 3); } // ============================================================ diff --git a/crates/sandlock-core/tests/integration/test_transaction.rs b/crates/sandlock-core/tests/integration/test_transaction.rs new file mode 100644 index 00000000..282d9a34 --- /dev/null +++ b/crates/sandlock-core/tests/integration/test_transaction.rs @@ -0,0 +1,1780 @@ +//! Transaction tests (RFC #65 Phase 1a). +//! +//! Sequential stages share one COW upper over a common workdir: a later stage +//! sees an earlier stage's writes (read-committed), and the whole transaction +//! commits all-or-nothing. Data is exchanged through the shared workspace, not +//! inter-stage pipes. + +use sandlock_core::sandbox::BranchAction; +use sandlock_core::{AbortReason, ChangeKind, Sandbox, Stage, Transaction, TxnError}; +use std::fs; +use std::os::unix::io::AsRawFd; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +fn temp_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("sandlock-test-txn-{}-{}", name, std::process::id())); + let _ = fs::remove_dir_all(&dir); + let _ = fs::create_dir_all(&dir); + dir +} + +/// Base policy shared by every stage: read the system, write+COW the workdir, +/// and run with the workdir as cwd so relative paths resolve into the upper. +/// `on_exit`/`on_error` are left at their defaults (the transaction owns commit). +fn stage_policy(workdir: &Path) -> Sandbox { + Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(workdir) + .workdir(workdir) + .cwd(workdir) + .build() + .unwrap() +} + +/// Whether this environment can actually run a sandbox (Landlock + seccomp). Used +/// to skip the behavioral tests EXPLICITLY, so a real regression in the +/// transaction logic hard-fails instead of hiding behind a tolerated error. +async fn sandbox_available() -> bool { + let mut sb = Sandbox::builder().fs_read("/usr").fs_read("/bin").build().unwrap(); + matches!(sb.run(&["true"]).await, Ok(r) if r.success()) +} + +/// Number of branch subdirectories under a `fs_storage` dir. Zero means every COW +/// branch's upper has been reclaimed (committed or aborted or dropped). +fn branch_count(storage: &Path) -> usize { + fs::read_dir(storage).map(|rd| rd.count()).unwrap_or(0) +} + +/// Full success: stage 1 writes `a.txt`, stage 2 reads it (proving read-committed) +/// and writes `b.txt`, stage 3 reads both. On commit both files land in workdir. +#[tokio::test] +async fn test_txn_commits_on_success() { + if !sandbox_available().await { + eprintln!("commit test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("commit"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt b.txt"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert!(outcome.committed, "transaction should commit; abort_reason: {:?}", outcome.abort_reason); + assert!(outcome.abort_reason.is_none(), "a committed transaction has no abort reason"); + assert_eq!(outcome.stages.len(), 3, "all three stages should have run"); + assert!(workdir.join("a.txt").exists(), "a.txt must be committed to workdir"); + assert!(workdir.join("b.txt").exists(), "b.txt must be committed to workdir"); + assert_eq!(fs::read_to_string(workdir.join("a.txt")).unwrap(), "plan\n"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// A later stage must be able to EXECUTE a file an earlier stage created. +/// +/// Landlock checks EXECUTE against the file's real path at `execve` time, and a +/// file created in the workdir really lives in the shared upper — so unless the +/// stage's ruleset grants read+execute on that upper, `./x.sh` fails with +/// `EACCES` and the shell reports 126. Nothing else in the suite covers it: the +/// shared-COW path builds no branch of its own, so it does not go through the +/// grant that `Sandbox`'s own branch gets, and +/// `test_cow::test_seccomp_cow_exec_packed_argv_relocation` only covers the +/// plain-`Sandbox` half of that pair. +#[tokio::test] +async fn test_txn_stage_can_exec_what_an_earlier_stage_created() { + if !sandbox_available().await { + eprintln!("shared-upper exec test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("shared-upper-exec"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "printf '#!/bin/sh\\nexit 0\\n' > x.sh && chmod 755 x.sh"]), + // `exec` so the stage's own status IS the exec's outcome: 126 when the + // kernel refuses it, not a shell error swallowed into 1. + Stage::new(&policy, &["sh", "-c", "exec ./x.sh"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert_eq!( + outcome.abort_reason, None, + "a stage must be able to exec a file an earlier stage created in the shared upper", + ); + assert!(outcome.committed, "the transaction should have committed"); + assert!(workdir.join("x.sh").exists(), "x.sh must be committed to the workdir"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Whether any branch under `storage` has `name` in its upper. Used to observe +/// how far the stages have got from outside the transaction. +fn upper_holds(storage: &Path, name: &str) -> bool { + fs::read_dir(storage) + .map(|rd| rd.flatten().any(|e| e.path().join("upper").join(name).exists())) + .unwrap_or(false) +} + +/// The commit merge is serialized against another transaction's merge, and a +/// transaction that finds the workdir locked WAITS for it rather than +/// discarding a full run's work. +/// +/// The lock is held until the transaction is DEMONSTRABLY blocked on it: the +/// test drives the transaction concurrently and fails the moment it finishes +/// while the lock is still held, so it cannot pass by having the holder let go +/// before the commit was ever reached. Only then is the lock released, and the +/// transaction must still commit. +#[tokio::test] +async fn test_txn_waits_for_a_concurrent_commit_lock() { + if !sandbox_available().await { + eprintln!("commit-lock-wait test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("commit-lock-wait-wd"); + let storage = temp_dir("commit-lock-wait-st"); + let policy = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + + // Stand in for another transaction mid-merge by holding the workdir lock. + let held = std::fs::File::open(&workdir).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + let txn = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + ]) + .run(None); + tokio::pin!(txn); + + // Drive the transaction while holding the lock. `b.txt` landing in the + // shared upper is the last stage's write, so from that point the only thing + // left for the transaction to do is the commit — and it must not get past + // it. Completing here at all, committed or not, is the failure. + let mut stages_done_at = None; + loop { + tokio::select! { + early = &mut txn => panic!( + "the transaction finished while the commit lock was held — it never waited: {early:?}" + ), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + match stages_done_at { + None if upper_holds(&storage, "b.txt") => { + stages_done_at = Some(std::time::Instant::now()) + } + // Keep holding for a grace period after the stages are done, so the + // transaction has had time to reach the commit and block on it. + Some(t) if t.elapsed() >= Duration::from_millis(500) => break, + _ => {} + } + } + drop(held); + + let outcome = txn.await.expect("transaction should run"); + assert!( + outcome.committed, + "a transaction must wait out a concurrent commit, not lose its work; abort_reason: {:?}", + outcome.abort_reason + ); + assert_eq!(fs::read_to_string(workdir.join("a.txt")).unwrap(), "plan\n"); + assert_eq!(fs::read_to_string(workdir.join("b.txt")).unwrap(), "built\n"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// The wait is bounded, and expiring it must NOT throw the run away. Every stage +/// exited 0, so the upper holds a complete, mergeable change set; giving up on +/// the lock leaves the workdir untouched and PRESERVES that upper with its +/// content, naming it in the error. +#[tokio::test] +async fn test_txn_preserves_the_upper_when_the_commit_lock_never_frees() { + if !sandbox_available().await { + eprintln!("commit-lock-preserve test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("lock-preserve-wd"); + let storage = temp_dir("lock-preserve-st"); + let policy = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + + // Stand in for another transaction whose merge never finishes: the lock is + // held for the whole test, well past this transaction's wait. + let held = std::fs::File::open(&workdir).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + let err = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + ]) + .commit_lock_wait(Duration::from_millis(300)) + .run(None) + .await + .expect_err("a commit lock that never frees must fail the commit"); + drop(held); + + // All-or-nothing still holds on the workdir side: nothing was merged. + assert!(!workdir.join("a.txt").exists(), "nothing may be merged when the lock was never taken"); + assert!(!workdir.join("b.txt").exists(), "nothing may be merged when the lock was never taken"); + + // ...and the work itself survives, with its bytes, so it can still be + // published out of band. + let branches: Vec = + fs::read_dir(&storage).unwrap().map(|e| e.unwrap().path()).collect(); + assert_eq!( + branches.len(), + 1, + "the upper of a fully successful run must be preserved, not reclaimed; found {branches:?}", + ); + let upper = branches[0].join("upper"); + assert_eq!( + fs::read_to_string(upper.join("a.txt")).unwrap(), + "plan\n", + "the preserved upper must still hold stage 0's write", + ); + assert_eq!( + fs::read_to_string(upper.join("b.txt")).unwrap(), + "built\n", + "the preserved upper must still hold stage 1's write", + ); + // The failure is typed as a CONFLICT — the retryable channel — not as a + // stage or configuration failure, and it names the preserved upper. + match &err { + TxnError::Conflict { workdir: wd, preserved_upper, .. } => { + assert_eq!(wd, &workdir, "the conflict must name the contended workdir"); + assert_eq!(preserved_upper, &upper, "the conflict must name the preserved upper"); + } + other => panic!("expected a commit-lock conflict, got: {other:?}"), + } + assert_eq!(err.exit_code(), 75, "a conflict reports EX_TEMPFAIL: retry it"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// CANCELLING the run once the commit phase has begun must not destroy the +/// change set. +/// +/// This is the case a caller reaches by construction, not an exotic one: +/// `run(timeout)` bounds the stage phase only, so anything wanting a wall-clock +/// bound on the whole thing wraps `run()` in `tokio::time::timeout`, and a +/// shutdown `select!` does the same. Dropping the future while an owned, +/// undisposed branch is alive across an await would reclaim its storage — every +/// stage exited 0 and the whole run would be gone, with no error, no marker and +/// nothing in the workdir. +/// +/// The window is held open with the workdir lock, exactly as a concurrent merge +/// would: the transaction is dropped while it is waiting for it. +#[tokio::test] +async fn test_txn_cancelled_during_the_commit_lock_wait_keeps_the_change_set() { + if !sandbox_available().await { + eprintln!("commit-cancel test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("cancel-commit-wd"); + let storage = temp_dir("cancel-commit-st"); + let policy = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + + // Stand in for another transaction mid-merge, so the commit blocks on the + // lock instead of racing through it. + let held = std::fs::File::open(&workdir).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + { + let txn = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + ]) + .commit_lock_wait(Duration::from_secs(5)) + .run(None); + tokio::pin!(txn); + + // Drive it until the last stage's write is in the shared upper: from + // there the only thing left is the commit, which cannot get past the + // held lock. Finishing at all would mean the window never opened. + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while !upper_holds(&storage, "b.txt") { + tokio::select! { + early = &mut txn => panic!("the transaction finished while the lock was held: {early:?}"), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + assert!(std::time::Instant::now() < deadline, "stages never reached the commit"); + } + // Grace for the last stage to be reaped and the commit to be entered. + tokio::select! { + early = &mut txn => panic!("the transaction finished while the lock was held: {early:?}"), + _ = tokio::time::sleep(Duration::from_millis(300)) => {} + } + } // <-- the run future is dropped here, mid-wait: the cancellation. + + // The lock is still held, so the commit gives up on it and preserves. Poll + // for the marker rather than sleeping a fixed time. + let deadline = std::time::Instant::now() + Duration::from_secs(20); + let preserved = loop { + let found = sandlock_core::list_preserved(&storage); + if !found.is_empty() { + break found; + } + assert!( + std::time::Instant::now() < deadline, + "a cancelled run's change set was never preserved (storage now holds {} branches)", + branch_count(&storage), + ); + tokio::time::sleep(Duration::from_millis(50)).await; + }; + drop(held); + + assert_eq!(preserved.len(), 1, "expected exactly one preserved branch, got {preserved:?}"); + let p = &preserved[0]; + assert_eq!(p.workdir, workdir.canonicalize().unwrap(), "the marker must name the workdir"); + assert_eq!( + p.reason, + sandlock_core::PreserveReason::CommitDeferred, + "the changes were complete and never merged: the workdir is untouched", + ); + // The bytes, not just the directory: a preserved upper that lost its + // content is the same data loss with a marker on top. + assert_eq!( + fs::read_to_string(p.upper.join("a.txt")).unwrap(), + "plan\n", + "the cancelled run's stage 0 write must survive", + ); + assert_eq!( + fs::read_to_string(p.upper.join("b.txt")).unwrap(), + "built\n", + "the cancelled run's stage 1 write must survive", + ); + // Nothing was published: all-or-nothing still holds on the workdir side. + assert!(!workdir.join("a.txt").exists(), "the lock was never taken, so nothing may be merged"); + assert!(!workdir.join("b.txt").exists(), "the lock was never taken, so nothing may be merged"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// Any stage failing aborts the whole transaction: earlier stages' writes are +/// discarded and the workdir is byte-identical to before the run. +#[tokio::test] +async fn test_txn_aborts_on_stage_failure() { + if !sandbox_available().await { + eprintln!("abort test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("abort"); + fs::write(workdir.join("existing.txt"), "original\n").unwrap(); + let policy = stage_policy(&workdir); + + // Stage 2 writes b.txt but the final stage exits non-zero → abort all. + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + Stage::new(&policy, &["sh", "-c", "exit 1"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert!(!outcome.committed, "a failing stage must abort the transaction"); + // The reason is typed: a caller can tell WHICH stage failed and how, with no + // string matching. + assert_eq!( + outcome.abort_reason, + Some(AbortReason::StageFailed { + index: 2, + status: sandlock_core::ExitStatus::Code(1), + }), + "abort must name the failing stage and its status", + ); + assert_eq!( + outcome.exit_code(), 1, + "the outcome reports the failing stage's own exit code", + ); + assert!(!workdir.join("a.txt").exists(), "a.txt must NOT leak after abort"); + assert!(!workdir.join("b.txt").exists(), "b.txt must NOT leak after abort"); + assert_eq!(fs::read_to_string(workdir.join("existing.txt")).unwrap(), "original\n"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// The shared upper is reclaimed from disk after BOTH abort and commit — the +/// end-to-end check that a failed/completed transaction never orphans its upper. +#[tokio::test] +async fn test_txn_reclaims_upper() { + if !sandbox_available().await { + eprintln!("reclaim test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("reclaim-wd"); + let storage = temp_dir("reclaim-st"); + let policy = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + + // Abort path: the upper must be gone from the storage dir. + let aborted = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo x > a.txt"]), + Stage::new(&policy, &["sh", "-c", "exit 1"]), + ]) + .run(None).await.expect("transaction should run"); + assert!(!aborted.committed); + assert_eq!(branch_count(&storage), 0, "aborted transaction must reclaim its upper from the storage dir"); + + // Commit path: also reclaimed after the merge. + let committed = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo y > b.txt"]), + Stage::new(&policy, &["sh", "-c", "cat b.txt"]), + ]) + .run(None).await.expect("transaction should run"); + assert!(committed.committed); + assert_eq!(branch_count(&storage), 0, "committed transaction must reclaim its upper from the storage dir"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// A timeout aborts the whole transaction without leaking earlier stages' +/// writes into the workdir — and the outcome still carries the results of the +/// stages that DID complete before the deadline, so the caller can see how far +/// it got without a second call. +#[tokio::test] +async fn test_txn_timeout_aborts_and_keeps_completed_stage_results() { + if !sandbox_available().await { + eprintln!("timeout test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("timeout"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "sleep 30"]), + ]) + .run(Some(Duration::from_millis(600))) + .await + .expect("transaction should run"); + + assert!(!outcome.committed, "a timed-out transaction must abort"); + assert_eq!(outcome.abort_reason, Some(AbortReason::TimedOut)); + assert_eq!( + outcome.stages.len(), + 1, + "the first stage completed before the deadline; its result must survive the cancellation", + ); + assert!(outcome.stages[0].success(), "the completed stage exited 0"); + assert!(!workdir.join("a.txt").exists(), "a.txt must NOT leak after a timeout abort"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// The outcome reports the filesystem changes the transaction made, on both the +/// commit and the abort path. They are read off the shared upper before it is +/// disposed of — after a commit or an abort there is nothing left to read. +#[tokio::test] +async fn test_txn_reports_changes_on_commit_and_abort() { + if !sandbox_available().await { + eprintln!("changes test skipped: sandbox unavailable"); + return; + } + // Commit path: an added file and a modified pre-existing one. + let wd_c = temp_dir("changes-commit"); + fs::write(wd_c.join("existing.txt"), "before\n").unwrap(); + let p_c = stage_policy(&wd_c); + let committed = Transaction::new([ + Stage::new(&p_c, &["sh", "-c", "echo new > added.txt"]), + Stage::new(&p_c, &["sh", "-c", "echo after > existing.txt"]), + ]) + .run(None).await.expect("transaction should run"); + assert!(committed.committed, "abort_reason: {:?}", committed.abort_reason); + + let mut got: Vec<(ChangeKind, String)> = committed + .changes + .iter() + .map(|c| (c.kind.clone(), c.path.display().to_string())) + .collect(); + got.sort_by(|a, b| a.1.cmp(&b.1)); + assert_eq!( + got, + vec![ + (ChangeKind::Added, "added.txt".to_string()), + (ChangeKind::Modified, "existing.txt".to_string()), + ], + "a committed transaction must report exactly what it merged", + ); + + // Abort path: the changes are still reported, even though nothing landed. + let wd_a = temp_dir("changes-abort"); + let p_a = stage_policy(&wd_a); + let aborted = Transaction::new([ + Stage::new(&p_a, &["sh", "-c", "echo new > added.txt"]), + Stage::new(&p_a, &["sh", "-c", "exit 1"]), + ]) + .run(None).await.expect("transaction should run"); + assert!(!aborted.committed); + assert_eq!( + aborted.changes.iter().map(|c| c.path.display().to_string()).collect::>(), + vec!["added.txt".to_string()], + "an aborted transaction must still report what it discarded", + ); + assert!(!wd_a.join("added.txt").exists(), "nothing may be merged on abort"); + + let _ = fs::remove_dir_all(&wd_c); + let _ = fs::remove_dir_all(&wd_a); +} + +/// `dry_run` runs every stage and reports the changes, then discards them: the +/// workdir is byte-identical afterwards. +#[tokio::test] +async fn test_txn_dry_run_reports_without_committing() { + if !sandbox_available().await { + eprintln!("dry-run test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("dry-run"); + fs::write(workdir.join("existing.txt"), "before\n").unwrap(); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo after > existing.txt"]), + ]) + .dry_run(None) + .await + .expect("dry run should run"); + + assert!(!outcome.committed, "a dry run must never commit"); + assert_eq!(outcome.abort_reason, Some(AbortReason::DryRun)); + assert_eq!(outcome.stages.len(), 2, "a dry run still runs every stage"); + let mut paths: Vec = outcome.changes.iter().map(|c| c.path.display().to_string()).collect(); + paths.sort(); + assert_eq!(paths, vec!["a.txt".to_string(), "existing.txt".to_string()]); + + assert!(!workdir.join("a.txt").exists(), "a dry run must not create a.txt in the workdir"); + assert_eq!( + fs::read_to_string(workdir.join("existing.txt")).unwrap(), + "before\n", + "a dry run must leave the workdir byte-identical", + ); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Guardrail: a non-default `on_exit`/`on_error` conflicts with the transaction +/// owning commit/abort, and is rejected before anything runs. (No sandbox needed.) +/// +/// Misconfiguration is its own typed variant, NOT a stage or commit failure, so +/// a caller can tell a bad transaction from a failed command without reading the +/// message. +#[tokio::test] +async fn test_txn_rejects_branch_action() { + let workdir = temp_dir("guard-action"); + let plain = stage_policy(&workdir); + let with_action = Sandbox::builder() + .fs_read("/usr").fs_write(&workdir).workdir(&workdir) + .on_exit(BranchAction::Keep) + .build() + .unwrap(); + + let err = Transaction::new([ + Stage::new(&plain, &["true"]), + Stage::new(&with_action, &["true"]), + ]) + .run(None).await.unwrap_err(); + assert!( + matches!(err, TxnError::Invalid(_)), + "a misconfigured transaction must not masquerade as a stage or commit failure, got: {err:?}" + ); + assert_eq!(err.exit_code(), 78, "a configuration error reports EX_CONFIG"); + assert!(err.to_string().contains("on_exit/on_error"), "expected the on_exit guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Guardrail: every stage must set a workdir (the shared transaction dir). +#[tokio::test] +async fn test_txn_rejects_missing_workdir() { + let workdir = temp_dir("guard-workdir"); + let with_wd = stage_policy(&workdir); + let no_wd = Sandbox::builder().fs_read("/usr").build().unwrap(); + + let err = Transaction::new([Stage::new(&with_wd, &["true"]), Stage::new(&no_wd, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("no workdir"), "expected the workdir guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Guardrail: fewer than two stages is rejected, not a panic. `Transaction::new` +/// accepts any stage list, so the check in `run` is the only one there is. A +/// one-stage transaction would index fine but is not a transaction; the ZERO +/// stage case is the one that would panic, because the coordinator reads +/// `stages[0]` for the shared workdir. +#[tokio::test] +async fn test_txn_rejects_too_few_stages() { + let workdir = temp_dir("guard-count"); + let policy = stage_policy(&workdir); + let err = Transaction::new([Stage::new(&policy, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("at least 2 stages"), "expected the stage-count guardrail, got: {err}"); + + // An empty transaction must be rejected by the same check, not indexed into. + let err = Transaction::new([]).run(None).await.unwrap_err().to_string(); + assert!(err.contains("at least 2 stages"), "expected the stage-count guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Guardrail: stages that each set a workdir but a DIFFERENT one are rejected — +/// distinct from the missing-workdir case (they share one COW upper). +#[tokio::test] +async fn test_txn_rejects_mismatched_workdir() { + let wd1 = temp_dir("guard-wd-a"); + let wd2 = temp_dir("guard-wd-b"); + let s0 = stage_policy(&wd1); + let s1 = stage_policy(&wd2); // valid workdir, but not the same one + let err = Transaction::new([Stage::new(&s0, &["true"]), Stage::new(&s1, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("share one workdir"), "expected the shared-workdir guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&wd1); + let _ = fs::remove_dir_all(&wd2); +} + +/// Guardrail: a stage running without the supervisor cannot participate in a COW +/// transaction (no notif path to build/commit the shared upper). +#[tokio::test] +async fn test_txn_rejects_no_supervisor() { + let workdir = temp_dir("guard-nosup"); + let ok = stage_policy(&workdir); + // Same workdir (so the workdir guardrail doesn't fire first) but no supervisor. + let nosup = Sandbox::builder() + .fs_read("/usr").fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .no_supervisor(true) + .build() + .unwrap(); + let err = Transaction::new([Stage::new(&ok, &["true"]), Stage::new(&nosup, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("no_supervisor"), "expected the no_supervisor guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Guardrail: chroot is unsupported with a shared COW workdir (the workdir path +/// can't resolve the same across differing roots). +#[tokio::test] +async fn test_txn_rejects_chroot() { + let workdir = temp_dir("guard-chroot"); + let rootfs = temp_dir("guard-chroot-root"); + let ok = stage_policy(&workdir); + let with_chroot = Sandbox::builder() + .fs_read("/usr").fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .chroot(&rootfs) + .build() + .unwrap(); + let err = Transaction::new([Stage::new(&ok, &["true"]), Stage::new(&with_chroot, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("chroot"), "expected the chroot guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&rootfs); +} + +/// Guardrail: stages must share one COW upper, so differing fs_storage/max_disk +/// (here stage 1 sets fs_storage while stage 0 does not) is rejected. +#[tokio::test] +async fn test_txn_rejects_mismatched_fs_storage() { + let workdir = temp_dir("guard-store-wd"); + let storage = temp_dir("guard-store-st"); + let s0 = stage_policy(&workdir); // no fs_storage + let s1 = Sandbox::builder() + .fs_read("/usr").fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + let err = Transaction::new([Stage::new(&s0, &["true"]), Stage::new(&s1, &["true"])]) + .run(None).await.unwrap_err().to_string(); + assert!(err.contains("fs_storage/max_disk"), "expected the fs_storage guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// The guardrails also gate `dry_run`, which runs the same stages over the same +/// shared upper — it must not be a way around them. +#[tokio::test] +async fn test_txn_dry_run_enforces_the_same_guardrails() { + let workdir = temp_dir("guard-dryrun"); + let with_wd = stage_policy(&workdir); + let no_wd = Sandbox::builder().fs_read("/usr").build().unwrap(); + + let err = Transaction::new([Stage::new(&with_wd, &["true"]), Stage::new(&no_wd, &["true"])]) + .dry_run(None).await.unwrap_err().to_string(); + assert!(err.contains("no workdir"), "dry_run must enforce the workdir guardrail, got: {err}"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Boundary: the FIRST stage failing aborts, and the transaction STOPS — later +/// stages must not run (outcome.stages holds only the failed stage). Distinct +/// from the last-stage-failure case. +#[tokio::test] +async fn test_txn_aborts_on_first_stage_failure() { + if !sandbox_available().await { + eprintln!("first-fail test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("first-fail"); + let policy = stage_policy(&workdir); + // Stage 0 writes a.txt then exits non-zero; stages 1 and 2 must NOT run. + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo a > a.txt; exit 1"]), + Stage::new(&policy, &["sh", "-c", "echo b > b.txt"]), + Stage::new(&policy, &["sh", "-c", "echo c > c.txt"]), + ]) + .run(None).await.expect("transaction should run"); + + assert!(!outcome.committed, "first-stage failure must abort"); + assert_eq!( + outcome.abort_reason, + Some(AbortReason::StageFailed { index: 0, status: sandlock_core::ExitStatus::Code(1) }), + ); + assert_eq!(outcome.stages.len(), 1, "transaction must stop at the failed stage — later stages must not run"); + assert!(!workdir.join("a.txt").exists(), "a.txt must NOT leak after abort"); + assert!(!workdir.join("b.txt").exists(), "stage 2 must not have run"); + assert!(!workdir.join("c.txt").exists(), "stage 3 must not have run"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Combination: a timeout aborts AND reclaims the shared upper (no orphan on the +/// timeout path — the reclaim test only covered clean abort/commit). +#[tokio::test] +async fn test_txn_timeout_reclaims_upper() { + if !sandbox_available().await { + eprintln!("timeout-reclaim test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("to-reclaim-wd"); + let storage = temp_dir("to-reclaim-st"); + let policy = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo x > a.txt"]), + Stage::new(&policy, &["sh", "-c", "sleep 30"]), + ]) + .run(Some(Duration::from_millis(600))) + .await + .expect("transaction should run"); + assert!(!outcome.committed, "timed-out transaction must abort"); + assert_eq!(branch_count(&storage), 0, "timed-out transaction must reclaim its upper from the storage dir"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// COW deletion (whiteout) semantics through commit AND abort: +/// - commit: a stage deleting a pre-existing workdir file removes it from the +/// workdir, and a later stage sees the deletion (read-committed); +/// - abort: the deletion is discarded — the file survives byte-identical. +#[tokio::test] +async fn test_txn_deletion_commit_applies_abort_preserves() { + if !sandbox_available().await { + eprintln!("deletion test skipped: sandbox unavailable"); + return; + } + // Commit path. + let wd_c = temp_dir("del-commit"); + fs::write(wd_c.join("keep.txt"), "orig\n").unwrap(); + let p_c = stage_policy(&wd_c); + let committed = Transaction::new([ + Stage::new(&p_c, &["sh", "-c", "rm keep.txt"]), + Stage::new(&p_c, &["sh", "-c", "test ! -e keep.txt"]), // stage 2 must SEE the deletion + ]) + .run(None) + .await + .expect("transaction should run"); + assert!(committed.committed, "commit expected; abort_reason: {:?}", committed.abort_reason); + assert!(!wd_c.join("keep.txt").exists(), "committed deletion must remove keep.txt from the workdir"); + assert_eq!( + committed.changes.iter().map(|c| (c.kind.clone(), c.path.display().to_string())).collect::>(), + vec![(ChangeKind::Deleted, "keep.txt".to_string())], + "a deletion must be reported as a change", + ); + + // Abort path: same deletion, but the transaction aborts → deletion discarded. + let wd_a = temp_dir("del-abort"); + fs::write(wd_a.join("keep.txt"), "orig\n").unwrap(); + let p_a = stage_policy(&wd_a); + let aborted = Transaction::new([ + Stage::new(&p_a, &["sh", "-c", "rm keep.txt"]), + Stage::new(&p_a, &["sh", "-c", "exit 1"]), + ]) + .run(None) + .await + .expect("transaction should run"); + assert!(!aborted.committed, "abort expected"); + assert_eq!( + fs::read_to_string(wd_a.join("keep.txt")).unwrap(), "orig\n", + "aborted deletion must leave keep.txt intact in the workdir", + ); + + let _ = fs::remove_dir_all(&wd_c); + let _ = fs::remove_dir_all(&wd_a); +} + +/// Policy for a transaction that keeps its COW branches in `storage`, so a test +/// can look at the upper from outside the run. +fn stage_policy_in(workdir: &Path, storage: &Path) -> Sandbox { + Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(workdir) + .workdir(workdir) + .cwd(workdir) + .fs_storage(storage) + .build() + .unwrap() +} + +/// Restore write permission on a directory, so a failed assertion cannot leave a +/// read-only tree behind for the next run to trip over. +fn make_writable(dir: &Path) { + use std::os::unix::fs::PermissionsExt; + let mut perm = match fs::metadata(dir) { + Ok(m) => m.permissions(), + Err(_) => return, + }; + perm.set_mode(0o755); + let _ = fs::set_permissions(dir, perm); +} + +fn make_read_only(dir: &Path) { + use std::os::unix::fs::PermissionsExt; + let mut perm = fs::metadata(dir).unwrap().permissions(); + perm.set_mode(0o555); + fs::set_permissions(dir, perm).unwrap(); +} + +/// A merge that cannot be applied does not throw the change set away: the run is +/// reported as `TxnError::Merge`, the storage is preserved as +/// `MergeInterrupted`, and the deletion that did not land is recorded in the +/// marker — the upper alone does not represent it, so recovering from the upper +/// on its own would resurrect the deleted file. +/// +/// The merge is made to fail on its deletion half, which runs before any entry +/// is copied: the workdir's `sub/` is read-only, so `sub/f.txt` cannot be +/// unlinked. The stages themselves are unaffected — their unlink is intercepted +/// by the COW supervisor and only recorded — so this is a failure of the commit, +/// not of the transaction's stages. +#[tokio::test] +async fn test_txn_merge_failure_preserves_the_unmerged_change_set() { + if !sandbox_available().await { + eprintln!("merge-failure test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("merge-fail-wd"); + let storage = temp_dir("merge-fail-st"); + make_writable(&workdir.join("sub")); + fs::create_dir_all(workdir.join("sub")).unwrap(); + fs::write(workdir.join("sub/f.txt"), "orig\n").unwrap(); + let policy = stage_policy_in(&workdir, &storage); + + let txn = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "rm -f sub/f.txt"]), + Stage::new(&policy, &["sh", "-c", "echo new > added.txt"]), + ]); + make_read_only(&workdir.join("sub")); + let err = txn.run(None).await.expect_err("a merge that cannot be applied must fail the commit"); + make_writable(&workdir.join("sub")); + + match &err { + TxnError::Merge { workdir: wd, preserved_upper, .. } => { + assert_eq!(wd, &workdir, "the merge failure must name the workdir it was merging into"); + assert!( + preserved_upper.starts_with(&storage), + "the merge failure must name the preserved upper, got {preserved_upper:?}", + ); + } + other => panic!("expected a merge failure, got: {other:?}"), + } + assert_eq!(err.exit_code(), 74, "a failed commit reports EX_IOERR"); + + // The deletion did not land, and neither did the addition: the merge stops on + // its deletion half before copying a single entry. + assert_eq!( + fs::read_to_string(workdir.join("sub/f.txt")).unwrap(), + "orig\n", + "the deletion could not be applied, so the file must still be there", + ); + assert!( + !workdir.join("added.txt").exists(), + "no entry may be published once a deletion could not be applied", + ); + + // The whole change set survives — the upper for the addition, the marker for + // the deletion the upper cannot represent. + let preserved = sandlock_core::list_preserved(&storage); + assert_eq!(preserved.len(), 1, "the unmerged change set must be preserved, got {preserved:?}"); + let p = &preserved[0]; + assert_eq!( + p.reason, + sandlock_core::PreserveReason::MergeInterrupted, + "a merge that started and did not finish must say so: the workdir may be partial", + ); + assert_eq!(p.workdir, workdir, "the marker must name the workdir the changes belong to"); + assert_eq!( + p.deleted, + vec![PathBuf::from("sub/f.txt")], + "the deletion that did not land must be in the marker; nothing in the upper represents it", + ); + assert_eq!( + fs::read_to_string(p.upper.join("added.txt")).unwrap(), + "new\n", + "the preserved upper must still hold the addition that was not merged", + ); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// A workdir that cannot be branched fails the transaction as `TxnError::Branch` +/// before any stage runs, and leaves nothing behind in the storage dir: the +/// branch dir is only created once the workdir has been resolved, so a failure +/// here cannot orphan an empty branch for a sweep to trip over. +#[tokio::test] +async fn test_txn_branch_failure_runs_nothing_and_orphans_no_storage() { + let workdir = temp_dir("branch-fail-wd"); + let storage = temp_dir("branch-fail-st"); + let policy = stage_policy_in(&workdir, &storage); + // Valid at build time, gone by the time the branch is created — the case the + // guardrails cannot catch, because they only check that a workdir is set. + fs::remove_dir_all(&workdir).unwrap(); + + let err = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo a > a.txt"]), + Stage::new(&policy, &["sh", "-c", "echo b > b.txt"]), + ]) + .run(None) + .await + .expect_err("a workdir that does not exist cannot be branched"); + + match &err { + TxnError::Branch { workdir: wd, .. } => { + assert_eq!(wd, &workdir, "the branch failure must name the workdir it could not branch"); + } + other => panic!("expected a branch-creation failure, got: {other:?}"), + } + assert_eq!(err.exit_code(), 74, "a branch that could not be created is a commit-channel failure"); + assert_eq!( + branch_count(&storage), + 0, + "a branch that was never created must not leave storage behind", + ); + + let _ = fs::remove_dir_all(&storage); +} + +/// A stage that cannot be driven at all is `TxnError::Stage` naming its index — +/// and the shared upper is still disposed of as an abort, so the earlier stages' +/// writes are discarded rather than published. +/// +/// Distinct from a stage that *fails*: that one produced an exit status and is +/// `Ok(TxnOutcome)` with an `AbortReason`. Here stage 1 has an empty command, so +/// it never starts and there is no status to report. +#[tokio::test] +async fn test_txn_undrivable_stage_errors_and_discards_the_shared_upper() { + if !sandbox_available().await { + eprintln!("undrivable-stage test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("undrivable-wd"); + let storage = temp_dir("undrivable-st"); + let policy = stage_policy_in(&workdir, &storage); + + let err = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &[]), + ]) + .run(None) + .await + .expect_err("a stage that cannot be started must fail the transaction"); + + assert!( + matches!(err, TxnError::Stage { index: 1, .. }), + "the failure must name the stage that could not be run, got: {err:?}", + ); + assert_eq!(err.exit_code(), 70, "a stage that could not be driven reports EX_SOFTWARE"); + assert!( + !workdir.join("a.txt").exists(), + "the completed stage's write must be discarded, not committed, when a later stage cannot run", + ); + assert_eq!(branch_count(&storage), 0, "the shared upper must not be left behind"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// Read fd `n` of the parent test process, as a `/proc` link target. +fn parent_fd(n: i32) -> PathBuf { + fs::read_link(format!("/proc/self/fd/{n}")) + .unwrap_or_else(|e| panic!("test setup: the parent has no fd {n}: {e}")) +} + +/// Stages inherit the parent's stdin, stdout and stderr — they are not connected +/// to each other by pipes, and nothing is captured into their results. +/// +/// Each stage records where its own fd 0/1/2 point (fd 1 through a dup, so the +/// recording redirect does not hide it) and every one must be the very same +/// open file as the parent's. A stage wired to a pipe — to the next stage or to +/// a capture buffer — would name a different one. +#[tokio::test] +async fn test_txn_stages_inherit_parent_stdio_and_capture_nothing() { + if !sandbox_available().await { + eprintln!("stdio test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("stdio"); + let policy = stage_policy(&workdir); + let record = |tag: &str| { + format!( + "exec 3>&1; readlink /proc/self/fd/0 > {tag}0; readlink /proc/self/fd/3 > {tag}1; \ + readlink /proc/self/fd/2 > {tag}2; echo on-stdout; echo on-stderr 1>&2" + ) + }; + let s0 = record("s"); + let s1 = record("t"); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", &s0]), + Stage::new(&policy, &["sh", "-c", &s1]), + ]) + .run(None) + .await + .expect("transaction should run"); + assert!(outcome.committed, "abort_reason: {:?}", outcome.abort_reason); + + for (stage, tag) in [(0usize, "s"), (1, "t")] { + for fd in 0..3i32 { + let seen = fs::read_to_string(workdir.join(format!("{tag}{fd}"))).unwrap(); + assert_eq!( + Path::new(seen.trim_end_matches('\n')), + parent_fd(fd), + "stage {stage} fd {fd} must be the parent's own, not a pipe", + ); + } + } + for (i, r) in outcome.stages.iter().enumerate() { + assert!( + r.stdout.is_none() && r.stderr.is_none(), + "stage {i} wrote to both streams, but an inherited stage has nothing to capture: {r:?}", + ); + } + + let _ = fs::remove_dir_all(&workdir); +} + +/// A stage killed by a signal aborts the transaction with that signal in the +/// reason, and the outcome's exit code is the shell's `128 + signal` for it — +/// not the raw signal number, and not a generic failure code. +#[tokio::test] +async fn test_txn_signalled_stage_reports_the_signal_and_shell_exit_code() { + if !sandbox_available().await { + eprintln!("signal test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("signal"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "kill -TERM $$; sleep 5"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert!(!outcome.committed, "a signalled stage must abort the transaction"); + assert_eq!( + outcome.abort_reason, + Some(AbortReason::StageFailed { + index: 1, + status: sandlock_core::ExitStatus::Signal(libc::SIGTERM), + }), + "the abort must carry the signal the stage died from", + ); + assert_eq!( + outcome.exit_code(), + 128 + libc::SIGTERM, + "a signalled stage reports 128 + signal, as a shell does", + ); + assert!(!workdir.join("a.txt").exists(), "nothing may be committed after a signalled stage"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// The outcome's exit code separates "did what was asked" from "aborted": a dry +/// run that completed is 0 even though it committed nothing, while a timeout is +/// 124, the code `timeout(1)` reports. +#[tokio::test] +async fn test_txn_exit_code_separates_a_completed_dry_run_from_a_timeout() { + if !sandbox_available().await { + eprintln!("outcome exit-code test skipped: sandbox unavailable"); + return; + } + let wd_d = temp_dir("code-dry"); + let p_d = stage_policy(&wd_d); + let dry = Transaction::new([ + Stage::new(&p_d, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&p_d, &["sh", "-c", "cat a.txt"]), + ]) + .dry_run(None) + .await + .expect("dry run should run"); + assert_eq!(dry.abort_reason, Some(AbortReason::DryRun)); + assert_eq!( + dry.exit_code(), + 0, + "a dry run that ran every stage did what was asked, even though it committed nothing", + ); + + let wd_t = temp_dir("code-timeout"); + let p_t = stage_policy(&wd_t); + let timed_out = Transaction::new([ + Stage::new(&p_t, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&p_t, &["sh", "-c", "sleep 30"]), + ]) + .run(Some(Duration::from_millis(600))) + .await + .expect("transaction should run"); + assert_eq!(timed_out.abort_reason, Some(AbortReason::TimedOut)); + assert_eq!(timed_out.exit_code(), 124, "a timed-out transaction reports 124, as timeout(1) does"); + + let _ = fs::remove_dir_all(&wd_d); + let _ = fs::remove_dir_all(&wd_t); +} + +/// A dry run must not take the workdir commit lock: it merges nothing, so +/// serializing it against a concurrent merge would only make it block. +/// +/// The lock is held for the whole run, and the dry run is given a wait budget it +/// would visibly exceed if it ever asked for the lock — so hoisting the +/// acquisition above the discard path turns this into a `TxnError::Conflict` +/// instead of an outcome. The upper is reclaimed either way: a dry run that +/// preserved its storage would leak one branch per invocation. +#[tokio::test] +async fn test_txn_dry_run_neither_takes_the_commit_lock_nor_keeps_its_upper() { + if !sandbox_available().await { + eprintln!("dry-run lock test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("dry-lock-wd"); + let storage = temp_dir("dry-lock-st"); + let policy = stage_policy_in(&workdir, &storage); + + // Stand in for another transaction mid-merge, for the whole dry run. + let held = std::fs::File::open(&workdir).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt"]), + ]) + .commit_lock_wait(Duration::from_millis(50)) + .dry_run(None) + .await + .expect("a dry run does not merge, so a held commit lock cannot fail it"); + drop(held); + + assert_eq!( + outcome.abort_reason, + Some(AbortReason::DryRun), + "the dry run completed; the held lock is none of its business", + ); + assert!(!outcome.committed, "a dry run must never commit"); + assert!(!workdir.join("a.txt").exists(), "a dry run must not publish anything"); + assert_eq!( + branch_count(&storage), + 0, + "a dry run must reclaim its upper, not leave a branch behind per invocation", + ); + assert!( + sandlock_core::list_preserved(&storage).is_empty(), + "a dry run has nothing to preserve: its change set was reported and discarded", + ); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// A dry run that did NOT get through its stages reports why it stopped, not +/// `DryRun`. +/// +/// `DryRun` means "every stage ran and the change set was discarded on purpose". +/// A caller reads this field to tell "here is the diff" from "could not get that +/// far", so a stage failure must keep its index and status and a timeout must +/// stay `TimedOut` — with the outcome's exit code following the same split. +#[tokio::test] +async fn test_txn_dry_run_reports_the_failure_that_stopped_it_not_dry_run() { + if !sandbox_available().await { + eprintln!("dry-run failure test skipped: sandbox unavailable"); + return; + } + let wd_f = temp_dir("dry-fail"); + let p_f = stage_policy(&wd_f); + let failed = Transaction::new([ + Stage::new(&p_f, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&p_f, &["sh", "-c", "exit 3"]), + ]) + .dry_run(None) + .await + .expect("a failing stage is an outcome, not a dry-run error"); + assert_eq!( + failed.abort_reason, + Some(AbortReason::StageFailed { + index: 1, + status: sandlock_core::ExitStatus::Code(3), + }), + "a dry run that stopped at a failing stage must say so, not claim it reported a diff", + ); + assert_eq!(failed.exit_code(), 3, "the failing stage's own code, not the dry run's 0"); + assert!(!wd_f.join("a.txt").exists(), "a dry run publishes nothing either way"); + + let wd_t = temp_dir("dry-timeout"); + let p_t = stage_policy(&wd_t); + let timed_out = Transaction::new([ + Stage::new(&p_t, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&p_t, &["sh", "-c", "sleep 30"]), + ]) + .dry_run(Some(Duration::from_millis(600))) + .await + .expect("a timeout is an outcome, not a dry-run error"); + assert_eq!( + timed_out.abort_reason, + Some(AbortReason::TimedOut), + "a dry run cut short by its timeout never reported a full diff", + ); + assert_eq!(timed_out.exit_code(), 124, "a timed-out dry run reports 124, as timeout(1) does"); + + let _ = fs::remove_dir_all(&wd_f); + let _ = fs::remove_dir_all(&wd_t); +} + +/// `Sandbox::dry_run` overrides the caller's `on_exit`/`on_error`, so a dry run +/// cannot leave its upper on disk even when it is ABANDONED. +/// +/// `Keep` is the case the override exists for. On the ordinary path `dry_run` +/// aborts the branch itself, so the setting makes no difference; what it changes +/// is `keep_if_abandoned`, which asks the branch to survive a run that never +/// reaches its disposition. For a dry run that is a pure leak — the change set +/// is read out and returned, so nothing is left to recover — and the storage +/// would grow by one branch, with a `PRESERVED` marker inviting a sweep to merge +/// it, every time a dry run is cancelled. +/// +/// The abandonment is the shape a caller reaches by construction: `dry_run` +/// takes no timeout, so bounding it means wrapping it in one. +#[tokio::test] +async fn test_sandbox_dry_run_overrides_keep_so_an_abandoned_dry_run_leaves_no_upper() { + if !sandbox_available().await { + eprintln!("dry-run override test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("dry-keep-wd"); + let storage = temp_dir("dry-keep-st"); + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .on_exit(BranchAction::Keep) + .build() + .unwrap(); + + // Abandon the dry run once its write is in the upper, so there really is a + // change set that `Keep` could have asked to survive. + { + let dry = sb.dry_run(&["sh", "-c", "echo plan > a.txt && sleep 30"]); + tokio::pin!(dry); + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while !upper_holds(&storage, "a.txt") { + tokio::select! { + early = &mut dry => panic!("the dry run finished before it could be abandoned: {early:?}"), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + assert!(std::time::Instant::now() < deadline, "the dry run never wrote into its upper"); + } + } // <-- the dry-run future is dropped here: the run is abandoned. + drop(sb); + + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while branch_count(&storage) != 0 { + assert!( + std::time::Instant::now() < deadline, + "an abandoned dry run must not keep its upper, but {} branch(es) remain in {}", + branch_count(&storage), + storage.display(), + ); + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert!( + sandlock_core::list_preserved(&storage).is_empty(), + "a dry run must not leave a marker inviting a sweep to merge its discarded changes", + ); + assert!(!workdir.join("a.txt").exists(), "a dry run must never merge"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// The dry-run override is a mutation of the sandbox, not a per-call setting: a +/// `Sandbox` that has been dry-run carries `on_exit`/`on_error == Abort` +/// afterwards, and is therefore REJECTED as a transaction stage. +/// +/// This pins a sharp edge rather than approving of it. The caller never set a +/// branch action, so the rejection message — "stage 1 sets on_exit/on_error ... +/// leave them at their defaults" — accuses them of something they did not do. +#[tokio::test] +async fn test_txn_rejects_a_stage_policy_that_was_previously_dry_run() { + if !sandbox_available().await { + eprintln!("dry-run reuse test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("dry-reuse"); + let mut policy = stage_policy(&workdir); + // A transaction built from this policy is valid before the dry run... + assert!( + Transaction::new([Stage::new(&policy, &["true"]), Stage::new(&policy, &["true"])]) + .run(None) + .await + .is_ok(), + "the policy must be a valid transaction stage to begin with", + ); + + policy.dry_run(&["true"]).await.expect("dry run should run"); + + // ...and rejected after it, with nothing about the stage list having changed. + let err = Transaction::new([Stage::new(&policy, &["true"]), Stage::new(&policy, &["true"])]) + .run(None) + .await + .expect_err("a dry-run sandbox carries on_exit=Abort and is no longer a valid stage"); + assert!( + matches!(err, TxnError::Invalid(_)), + "the rejection is a configuration error, got: {err:?}", + ); + assert!( + err.to_string().contains("on_exit/on_error"), + "the branch-action guardrail is what fires, got: {err}", + ); + + let _ = fs::remove_dir_all(&workdir); +} + +/// `run`'s `timeout` bounds the STAGE phase only. A commit that has to wait out +/// another transaction's merge is not cut short by it — the stages have all +/// succeeded and the work is mergeable, so the wait is governed by +/// `commit_lock_wait` alone. +/// +/// The lock is held past the timeout after the last stage's write is visible in +/// the shared upper, so the transaction is demonstrably inside the commit with +/// the deadline already expired. Finishing during that window at all is the +/// failure. +#[tokio::test] +async fn test_txn_timeout_bounds_the_stage_phase_not_the_commit() { + if !sandbox_available().await { + eprintln!("timeout-scope test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("to-scope-wd"); + let storage = temp_dir("to-scope-st"); + let policy = stage_policy_in(&workdir, &storage); + + let held = std::fs::File::open(&workdir).unwrap(); + assert_eq!( + unsafe { libc::flock(held.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }, + 0, + "test setup: could not take the workdir lock" + ); + + let stage_timeout = Duration::from_millis(800); + let txn = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "cat a.txt && echo built > b.txt"]), + ]) + .commit_lock_wait(Duration::from_secs(30)) + .run(Some(stage_timeout)); + tokio::pin!(txn); + + // Drive until the last stage's write is in the shared upper: the stage phase + // is over and the only thing left is the commit. + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while !upper_holds(&storage, "b.txt") { + tokio::select! { + early = &mut txn => panic!("the transaction finished while the lock was held: {early:?}"), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + assert!(std::time::Instant::now() < deadline, "stages never reached the commit"); + } + // Hold the lock well past the stage timeout. A transaction whose commit was + // bounded by it would give up in here. + let hold_until = std::time::Instant::now() + stage_timeout * 2; + while std::time::Instant::now() < hold_until { + tokio::select! { + early = &mut txn => panic!( + "the commit was cut short by the stage timeout instead of waiting for the lock: {early:?}" + ), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + } + drop(held); + + let outcome = txn.await.expect("the commit is not bounded by the stage timeout"); + assert!( + outcome.committed, + "every stage succeeded within the timeout, so the transaction must commit; abort_reason: {:?}", + outcome.abort_reason, + ); + assert_eq!(fs::read_to_string(workdir.join("b.txt")).unwrap(), "built\n"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// A timeout that expires before the first stage can start aborts with NO stage +/// results at all — the one outcome whose `stages` vector is empty. +/// +/// `TxnOutcome::stages` is otherwise "every stage that ran", which invites +/// indexing `stages[0]` after an abort. It also pins that an expired deadline is +/// an abort and not a clean run: treating the cancelled driver as "nothing went +/// wrong" would commit an upper no stage ever wrote to. +#[tokio::test] +async fn test_txn_a_deadline_that_expires_immediately_aborts_with_no_stage_results() { + if !sandbox_available().await { + eprintln!("zero-timeout test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("zero-to-wd"); + let storage = temp_dir("zero-to-st"); + fs::write(workdir.join("existing.txt"), "before\n").unwrap(); + let policy = stage_policy_in(&workdir, &storage); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["sh", "-c", "echo built > b.txt"]), + ]) + .run(Some(Duration::ZERO)) + .await + .expect("an expired deadline is an outcome, not an error"); + + assert!(!outcome.committed, "a transaction that never ran a stage must not commit"); + assert_eq!(outcome.abort_reason, Some(AbortReason::TimedOut)); + assert!( + outcome.stages.is_empty(), + "no stage completed, so there is nothing to report; got {:?}", + outcome.stages, + ); + assert_eq!(outcome.exit_code(), 124, "an expired deadline reports 124, as timeout(1) does"); + assert!(!workdir.join("a.txt").exists(), "no stage ran, so nothing may be published"); + assert_eq!(fs::read_to_string(workdir.join("existing.txt")).unwrap(), "before\n"); + assert_eq!(branch_count(&storage), 0, "the untouched upper must still be reclaimed"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// A transaction rejected by a guardrail runs NOTHING — the validation happens +/// before the shared branch is created and before the first stage is started. +/// +/// Asserted with a side effect the COW layer cannot hide: stage 0 writes a +/// sentinel OUTSIDE the workdir, straight to the real filesystem, so a validator +/// that ran after the stages would leave it behind. The shared upper must not be +/// created either, or every rejected transaction orphans one branch dir. +#[tokio::test] +async fn test_txn_a_rejected_transaction_creates_no_branch_and_starts_no_stage() { + if !sandbox_available().await { + eprintln!("validation-ordering test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("reject-order-wd"); + let storage = temp_dir("reject-order-st"); + let outside = temp_dir("reject-order-out"); + let sentinel = outside.join("ran.txt"); + + // Stage 0 is valid and, if it ever ran, would leave `sentinel` on the real + // filesystem: `outside` is not the workdir, so it is not COW-intercepted. + let base = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .fs_write(&workdir).fs_write(&outside).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .build() + .unwrap(); + // Stage 1 is the violation: a per-stage branch action the transaction owns. + let bad = Sandbox::builder() + .fs_read("/usr").fs_write(&workdir).workdir(&workdir).cwd(&workdir) + .fs_storage(&storage) + .on_error(BranchAction::Keep) + .build() + .unwrap(); + + let cmd = format!("echo ran > {}", sentinel.display()); + let err = Transaction::new([ + Stage::new(&base, &["sh", "-c", &cmd]), + Stage::new(&bad, &["true"]), + ]) + .run(None) + .await + .expect_err("a per-stage branch action must be rejected"); + + assert!(matches!(err, TxnError::Invalid(_)), "expected a configuration error, got: {err:?}"); + assert!( + !sentinel.exists(), + "a rejected transaction must not have started stage 0: {} exists", + sentinel.display(), + ); + assert_eq!( + branch_count(&storage), + 0, + "a rejected transaction must not create the shared branch", + ); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); + let _ = fs::remove_dir_all(&outside); +} + +/// `TxnOutcome::stages` holds every stage that RAN, in execution order, and the +/// failing stage's own result is one of them. +/// +/// Both halves matter to a caller reporting on a failed transaction: dropping +/// the failing stage's result loses the only place its status is recorded +/// alongside the successful ones, and a reordered vector silently misattributes +/// every status. Pinned with a distinct exit code at a known index, so neither +/// can pass. +#[tokio::test] +async fn test_txn_stage_results_are_in_execution_order_and_include_the_failed_stage() { + if !sandbox_available().await { + eprintln!("stage-order test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("stage-order"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "exit 0"]), + Stage::new(&policy, &["sh", "-c", "exit 0"]), + Stage::new(&policy, &["sh", "-c", "exit 7"]), + Stage::new(&policy, &["sh", "-c", "exit 0"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert!(!outcome.committed); + assert_eq!( + outcome.stages.iter().map(|r| r.exit_status.clone()).collect::>(), + vec![ + sandlock_core::ExitStatus::Code(0), + sandlock_core::ExitStatus::Code(0), + sandlock_core::ExitStatus::Code(7), + ], + "the three stages that ran must be reported in order, the failing one included", + ); + + let _ = fs::remove_dir_all(&workdir); +} + +/// A command that cannot be exec'd is a stage FAILURE, not a driver error: the +/// stage really started, the exec failed inside the child, and the shell +/// convention 127 comes back as the abort reason. +/// +/// This is the boundary of `TxnError::Stage`, which is reserved for a stage that +/// never produced an exit status at all. A caller that expects `Err` for a +/// missing binary — the natural reading of "could not be run" — would never see +/// it. +#[tokio::test] +async fn test_txn_a_command_that_cannot_be_execed_is_a_stage_failure_not_a_driver_error() { + if !sandbox_available().await { + eprintln!("missing-binary test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("no-such-binary"); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), + Stage::new(&policy, &["/no/such/binary"]), + ]) + .run(None) + .await + .expect("a binary that cannot be exec'd is an abort, not a commit-channel error"); + + assert_eq!( + outcome.abort_reason, + Some(AbortReason::StageFailed { + index: 1, + status: sandlock_core::ExitStatus::Code(127), + }), + "an exec failure surfaces as the stage's own 127, as a shell reports it", + ); + assert_eq!(outcome.exit_code(), 127); + assert_eq!(outcome.stages.len(), 2, "the stage ran and produced a status, so it is reported"); + assert!(!workdir.join("a.txt").exists(), "the earlier stage's write must be discarded"); + + let _ = fs::remove_dir_all(&workdir); +} + +/// Cancelling during the STAGE phase RECLAIMS the change set — the mirror image +/// of cancelling during the commit phase, which preserves it. +/// +/// The distinction is the whole point: a run whose stages all succeeded holds a +/// complete, mergeable change set that must outlive the cancellation, while a +/// run abandoned mid-stage holds a partial one that no recovery could safely +/// publish. So this path must leave no storage and no `PRESERVED` marker for a +/// sweep to find — and the workdir must be byte-identical, since nothing was +/// ever merged. +/// +/// The reclaim is asynchronous (the notification supervisor holds the branch +/// until its own abort lands), so it is polled for rather than assumed. +#[tokio::test] +async fn test_txn_cancelled_during_the_stage_phase_reclaims_the_change_set() { + if !sandbox_available().await { + eprintln!("stage-cancel test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("cancel-stage-wd"); + let storage = temp_dir("cancel-stage-st"); + fs::write(workdir.join("existing.txt"), "before\n").unwrap(); + let policy = stage_policy_in(&workdir, &storage); + + { + let txn = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo plan > a.txt && sleep 30"]), + Stage::new(&policy, &["sh", "-c", "echo built > b.txt"]), + ]) + .run(None); + tokio::pin!(txn); + + // Drive until stage 0's write is in the shared upper: the run is + // demonstrably mid-stage, with a change set on disk to lose. + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while !upper_holds(&storage, "a.txt") { + tokio::select! { + early = &mut txn => panic!("the transaction finished before it could be cancelled: {early:?}"), + _ = tokio::time::sleep(Duration::from_millis(20)) => {} + } + assert!(std::time::Instant::now() < deadline, "stage 0 never wrote into the shared upper"); + } + } // <-- the run future is dropped here, mid-stage: the cancellation. + + let deadline = std::time::Instant::now() + Duration::from_secs(20); + while branch_count(&storage) != 0 { + assert!( + std::time::Instant::now() < deadline, + "a run cancelled mid-stage must reclaim its upper, but {} branch(es) remain in {}", + branch_count(&storage), + storage.display(), + ); + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert!( + sandlock_core::list_preserved(&storage).is_empty(), + "a partial change set must not be preserved: no recovery could safely publish it", + ); + assert!(!workdir.join("a.txt").exists(), "nothing was merged, so nothing may be in the workdir"); + assert_eq!( + fs::read_to_string(workdir.join("existing.txt")).unwrap(), + "before\n", + "the workdir must be byte-identical to before the cancelled run", + ); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_dir_all(&storage); +} + +/// Read-committed for a MODIFIED pre-existing file: a later stage reading a +/// workdir file an earlier stage rewrote must see the NEW bytes. +/// +/// This is the half of read-committed that fails silently. A stage reading a +/// file that was ADDED by an earlier stage gets a loud `ENOENT` if the read is +/// not resolved into the shared upper; a stage reading a file that was MODIFIED +/// gets the stale lower content and carries on, so the transaction commits a +/// result computed from data that was already superseded. +#[tokio::test] +async fn test_txn_stage_reads_an_earlier_stage_s_modification_not_the_workdir_copy() { + if !sandbox_available().await { + eprintln!("read-committed test skipped: sandbox unavailable"); + return; + } + let workdir = temp_dir("read-modified"); + fs::write(workdir.join("in.txt"), "before\n").unwrap(); + let policy = stage_policy(&workdir); + + let outcome = Transaction::new([ + Stage::new(&policy, &["sh", "-c", "echo after > in.txt"]), + // `cat` (not `test -e`) so the assertion is on the CONTENT: a stale read + // succeeds and would otherwise look identical to a fresh one. + Stage::new(&policy, &["sh", "-c", "cat in.txt > seen.txt"]), + ]) + .run(None) + .await + .expect("transaction should run"); + + assert!(outcome.committed, "abort_reason: {:?}", outcome.abort_reason); + assert_eq!( + fs::read_to_string(workdir.join("seen.txt")).unwrap(), + "after\n", + "stage 1 must read what stage 0 wrote, not the pre-transaction workdir copy", + ); + assert_eq!(fs::read_to_string(workdir.join("in.txt")).unwrap(), "after\n"); + + let _ = fs::remove_dir_all(&workdir); +}