From 67af0069b3367d7f9725e8a46e2dd68c14c76fad Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 13:44:58 -0700 Subject: [PATCH 01/12] result: describe a change by its before and after entries A Change carried a kind and a path, and the kind was the only thing it knew about either side. A caller reviewing a deferred run then had to re-read the workdir and the upper to learn what changed, and could not tell a chmod from a rewrite or pair a rename. The change now carries an Entry for each side (kind, mode, size, digest for files, target for links) and derives Added, Modified, or Deleted from which sides exist. Helpers cover the questions a reviewer asks: content unchanged, type changed, and a digest join that pairs renames. Both sides are still read from the live trees; the digests stay empty until the branch records what it saw at first touch. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/seccomp.rs | 82 ++++++--- crates/sandlock-core/src/lib.rs | 2 +- crates/sandlock-core/src/result.rs | 173 +++++++++++++++++- crates/sandlock-core/src/transaction.rs | 2 +- .../tests/integration/test_branch_action.rs | 8 +- .../tests/integration/test_transaction.rs | 4 +- crates/sandlock-ffi/src/lib.rs | 2 +- 7 files changed, 225 insertions(+), 48 deletions(-) diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index 42c1ead8..500db2ea 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -9,6 +9,7 @@ use std::fs; use std::os::unix::ffi::OsStringExt; use std::os::unix::fs::{FileTypeExt, MetadataExt}; use std::os::unix::io::FromRawFd; +use crate::result::{Entry, EntryKind}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -633,6 +634,34 @@ pub struct SeccompCowBranch { disk_used: u64, } +/// The entry at `path` without following symlinks; `None` when absent. +pub(crate) fn lstat_entry(path: &Path) -> Option { + use std::os::unix::fs::{FileTypeExt, PermissionsExt}; + let meta = fs::symlink_metadata(path).ok()?; + let ft = meta.file_type(); + let kind = if ft.is_dir() { + EntryKind::Dir + } else if ft.is_symlink() { + EntryKind::Symlink + } else if ft.is_file() { + EntryKind::File + } else if ft.is_fifo() || ft.is_socket() { + EntryKind::Other + } else { + return None; + }; + let target = (kind == EntryKind::Symlink) + .then(|| fs::read_link(path).ok().map(|t| t.to_string_lossy().into_owned())) + .flatten(); + Some(Entry { + kind, + mode: meta.permissions().mode() & 0o7777, + size: if kind == EntryKind::File { meta.len() } else { 0 }, + digest: None, + target, + }) +} + impl SeccompCowBranch { /// Create a new seccomp COW branch. /// @@ -1592,24 +1621,21 @@ impl SeccompCowBranch { /// List all filesystem changes in the COW layer. pub fn changes(&self) -> Result, BranchError> { - use crate::result::{Change, ChangeKind}; + use crate::result::Change; let mut result = Vec::new(); - // The kind compares the two trees as they stand, not the branch's - // history: a whiteouted-then-recreated path still has its old bytes - // in the workdir, and that is what a caller diffing the sides needs. for entry in walkdir::WalkDir::new(&self.upper).min_depth(1) { let entry = entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?; let rel = entry.path().strip_prefix(&self.upper).unwrap(); - let lower = self.workdir.join(rel).symlink_metadata().ok(); + let before = lstat_entry(&self.workdir.join(rel)); + let after = lstat_entry(entry.path()); // Copy-up recreates a modified file's parents in the upper; a // directory the workdir already has is scaffolding, not a change. - if entry.file_type().is_dir() && lower.as_ref().is_some_and(|m| m.is_dir()) { + if entry.file_type().is_dir() && before.as_ref().is_some_and(|b| b.kind == EntryKind::Dir) { continue; } - let kind = if lower.is_some() { ChangeKind::Modified } else { ChangeKind::Added }; - result.push(Change { kind, path: rel.to_path_buf() }); + result.push(Change { path: rel.to_path_buf(), before, after }); } // Deletions from the whiteout set; an entry re-created in the upper @@ -1623,10 +1649,8 @@ impl SeccompCowBranch { if self.upper_has(rel_path) { continue; } - result.push(Change { - kind: ChangeKind::Deleted, - path: std::path::PathBuf::from(rel_path), - }); + let Some(before) = lstat_entry(&self.workdir.join(rel_path)) else { continue }; + result.push(Change { path: std::path::PathBuf::from(rel_path), before: Some(before), after: None }); } Ok(result) @@ -3117,7 +3141,7 @@ mod tests { .changes() .unwrap() .into_iter() - .map(|c| (c.kind, c.path.display().to_string())) + .map(|c| (c.kind(), c.path.display().to_string())) .collect(); outstanding.sort_by(|a, b| a.1.cmp(&b.1)); assert_eq!( @@ -3668,7 +3692,7 @@ mod tests { fs::write(&upper, "new content").unwrap(); let changes = branch.changes().unwrap(); assert_eq!(changes.len(), 1); - assert_eq!(changes[0].kind, crate::result::ChangeKind::Added); + assert_eq!(changes[0].kind(), crate::result::ChangeKind::Added); assert_eq!(changes[0].path, std::path::PathBuf::from("brand_new.txt")); } @@ -3680,7 +3704,7 @@ mod tests { fs::write(&upper, "modified content").unwrap(); let changes = branch.changes().unwrap(); assert_eq!(changes.len(), 1); - assert_eq!(changes[0].kind, crate::result::ChangeKind::Modified); + assert_eq!(changes[0].kind(), crate::result::ChangeKind::Modified); assert_eq!(changes[0].path, std::path::PathBuf::from("existing.txt")); } @@ -3691,7 +3715,7 @@ mod tests { branch.mark_deleted("existing.txt"); let changes = branch.changes().unwrap(); assert_eq!(changes.len(), 1); - assert_eq!(changes[0].kind, crate::result::ChangeKind::Deleted); + assert_eq!(changes[0].kind(), crate::result::ChangeKind::Deleted); assert_eq!(changes[0].path, std::path::PathBuf::from("existing.txt")); } @@ -3716,11 +3740,11 @@ mod tests { let mut changes = branch.changes().unwrap(); changes.sort_by(|a, b| a.path.cmp(&b.path)); assert_eq!(changes.len(), 3); - assert_eq!(changes[0].kind, crate::result::ChangeKind::Modified); + assert_eq!(changes[0].kind(), crate::result::ChangeKind::Modified); assert_eq!(changes[0].path, std::path::PathBuf::from("existing.txt")); - assert_eq!(changes[1].kind, crate::result::ChangeKind::Added); + assert_eq!(changes[1].kind(), crate::result::ChangeKind::Added); assert_eq!(changes[1].path, std::path::PathBuf::from("new.txt")); - assert_eq!(changes[2].kind, crate::result::ChangeKind::Deleted); + assert_eq!(changes[2].kind(), crate::result::ChangeKind::Deleted); assert_eq!(changes[2].path, std::path::PathBuf::from("subdir/nested.txt")); } @@ -4525,7 +4549,7 @@ mod tests { .filter(|c| c.path == std::path::Path::new("existing.txt")) .collect(); assert_eq!(for_path.len(), 1); - assert_eq!(for_path[0].kind, crate::result::ChangeKind::Modified); + assert_eq!(for_path[0].kind(), crate::result::ChangeKind::Modified); } #[test] @@ -5425,7 +5449,7 @@ mod tests { .changes() .unwrap() .iter() - .all(|c| c.kind != crate::result::ChangeKind::Deleted), + .all(|c| c.kind() != crate::result::ChangeKind::Deleted), "a whiteout the upper re-created must not be reported as a deletion", ); @@ -5545,7 +5569,7 @@ mod tests { .changes() .unwrap() .into_iter() - .filter(|c| c.kind == crate::result::ChangeKind::Deleted) + .filter(|c| c.kind() == crate::result::ChangeKind::Deleted) .map(|c| c.path) .collect::>(), vec![PathBuf::from("link/x.txt")], @@ -5662,7 +5686,7 @@ mod tests { .changes() .unwrap() .into_iter() - .map(|c| (c.kind, c.path)) + .map(|c| (c.kind(), c.path)) .collect::>(), vec![(crate::result::ChangeKind::Modified, PathBuf::from("f.txt"))], "precondition: the run reports the chmod as a recorded change", @@ -5834,14 +5858,14 @@ mod tests { 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, + 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, + branch.changes().unwrap()[0].kind(), ChangeKind::Modified, "the label follows the live workdir: the commit will now overwrite a file", ); @@ -5866,7 +5890,7 @@ mod tests { .changes() .unwrap() .into_iter() - .map(|c| (c.kind, c.path.display().to_string())) + .map(|c| (c.kind(), c.path.display().to_string())) .collect(); assert_eq!(changes, vec![(ChangeKind::Modified, "f.txt".to_string())]); } @@ -5886,7 +5910,7 @@ mod tests { .changes() .unwrap() .into_iter() - .map(|c| (c.kind, c.path.display().to_string())) + .map(|c| (c.kind(), c.path.display().to_string())) .collect(); assert_eq!(changes, vec![(ChangeKind::Added, "newdir".to_string())]); } @@ -5908,7 +5932,7 @@ mod tests { .changes() .unwrap() .into_iter() - .map(|c| (c.kind, c.path.display().to_string())) + .map(|c| (c.kind(), c.path.display().to_string())) .collect(); assert_eq!(changes, vec![(ChangeKind::Added, "sub/a.txt".to_string())]); } @@ -6315,7 +6339,7 @@ mod tests { branch.keep(); let mut reported: Vec<(ChangeKind, PathBuf)> = - branch.changes().unwrap().into_iter().map(|c| (c.kind, c.path)).collect(); + 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, diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 9c6220e5..2fa3cbfd 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -42,7 +42,7 @@ pub use protection::{Protection, ProtectionState, ProtectionPolicy, ProtectionSt pub use sandbox::{ BindPorts, Confinement, ConfinementBuilder, Process, Sandbox, SandboxBuilder, StdioMode, }; -pub use result::{Change, ChangeKind, ExitStatus, RunResult}; +pub use result::{renames, Change, ChangeKind, Entry, EntryKind, ExitStatus, RunResult}; pub use pipeline::{Stage, Pipeline, Gather}; pub use transaction::{AbortReason, Transaction, TxnDisposition, TxnError, TxnOutcome}; // Recovery of COW branch storage that was preserved rather than reclaimed. The diff --git a/crates/sandlock-core/src/result.rs b/crates/sandlock-core/src/result.rs index 24eb502a..119eadc1 100644 --- a/crates/sandlock-core/src/result.rs +++ b/crates/sandlock-core/src/result.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// The result of running a sandboxed process. #[derive(Debug, Clone)] @@ -55,14 +55,13 @@ pub enum ExitStatus { Timeout, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ChangeKind { - /// Exists in the branch but not in the workdir. + /// No entry at the path when the run first touched it. Added, - /// Exists on both sides; the bytes are not compared, so a rewrite with - /// identical contents, a mode change, or a rename over the path all count. + /// An entry on both sides; compare the entries to see what differs. Modified, - /// Exists in the workdir but not in the branch. + /// The run removed the entry. Deleted, } @@ -76,16 +75,170 @@ impl fmt::Display for ChangeKind { } } -/// One filesystem change a run made to its COW branch. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EntryKind { + File, + Dir, + Symlink, + /// A fifo or socket: no bytes, only a mode. + Other, +} + +/// One side of a change. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Entry { + pub kind: EntryKind, + /// Permission bits only. + pub mode: u32, + /// Byte length for a file; 0 otherwise. + pub size: u64, + /// SHA-256 of the bytes. Files only. + pub digest: Option<[u8; 32]>, + /// Link target, verbatim. Symlinks only. + pub target: Option, +} + +impl Entry { + fn same_content(&self, other: &Entry) -> bool { + self.kind == other.kind && self.digest == other.digest && self.target == other.target + } +} + +/// One filesystem change a run made to its COW branch. At least one side +/// is always present. +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Change { - pub kind: ChangeKind, /// Relative to the workdir. pub path: PathBuf, + /// The workdir entry when the run first touched the path. + pub before: Option, + /// The branch entry when the change set was read. + pub after: Option, +} + +impl Change { + pub fn kind(&self) -> ChangeKind { + match (&self.before, &self.after) { + (None, _) => ChangeKind::Added, + (Some(_), None) => ChangeKind::Deleted, + (Some(_), Some(_)) => ChangeKind::Modified, + } + } + + /// Both sides present with the same kind and bytes or target: a touch, + /// a mode change, or a rewrite with identical contents. + pub fn content_unchanged(&self) -> bool { + matches!((&self.before, &self.after), (Some(b), Some(a)) if b.same_content(a)) + } + + pub fn type_changed(&self) -> bool { + matches!((&self.before, &self.after), (Some(b), Some(a)) if b.kind != a.kind) + } } impl fmt::Display for Change { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} {}", self.kind, self.path.display()) + write!(f, "{} {}", self.kind(), self.path.display()) + } +} + +/// Pair each deleted file with the added file carrying the same digest. A +/// digest seen more than once on either side is ambiguous and left unpaired. +pub fn renames(changes: &[Change]) -> Vec<(&Path, &Path)> { + use std::collections::HashMap; + fn unique_by_digest<'a>( + entries: impl Iterator, + ) -> HashMap<[u8; 32], Option<&'a Path>> { + let mut by_digest: HashMap<[u8; 32], Option<&Path>> = HashMap::new(); + for (path, entry) in entries { + if let Some(d) = entry.digest { + by_digest.entry(d).and_modify(|slot| *slot = None).or_insert(Some(path)); + } + } + by_digest + } + let deleted = unique_by_digest(changes.iter().filter_map(|c| match (&c.before, &c.after) { + (Some(b), None) => Some((c.path.as_path(), b)), + _ => None, + })); + let added = unique_by_digest(changes.iter().filter_map(|c| match (&c.before, &c.after) { + (None, Some(a)) => Some((c.path.as_path(), a)), + _ => None, + })); + let mut pairs: Vec<(&Path, &Path)> = deleted + .iter() + .filter_map(|(d, old)| Some(((*old)?, (*added.get(d)?)?))) + .collect(); + pairs.sort(); + pairs +} + +#[cfg(test)] +mod change_tests { + use super::*; + + fn file(mode: u32, size: u64, digest: u8) -> Entry { + Entry { kind: EntryKind::File, mode, size, digest: Some([digest; 32]), target: None } + } + + fn link(target: &str) -> Entry { + Entry { kind: EntryKind::Symlink, mode: 0o777, size: 0, digest: None, target: Some(target.into()) } + } + + fn change(path: &str, before: Option, after: Option) -> Change { + Change { path: PathBuf::from(path), before, after } + } + + #[test] + fn kind_is_derived_from_which_sides_are_present() { + assert_eq!(change("a", None, Some(file(0o644, 1, 1))).kind(), ChangeKind::Added); + assert_eq!(change("m", Some(file(0o644, 1, 1)), Some(file(0o644, 2, 2))).kind(), ChangeKind::Modified); + assert_eq!(change("d", Some(file(0o644, 1, 1)), None).kind(), ChangeKind::Deleted); + } + + #[test] + fn content_unchanged_means_same_kind_and_same_digest_or_target() { + assert!(change("touch", Some(file(0o644, 1, 1)), Some(file(0o644, 1, 1))).content_unchanged()); + assert!(change("chmod", Some(file(0o644, 1, 1)), Some(file(0o755, 1, 1))).content_unchanged()); + assert!(!change("edit", Some(file(0o644, 1, 1)), Some(file(0o644, 1, 2))).content_unchanged()); + assert!(change("same-link", Some(link("t")), Some(link("t"))).content_unchanged()); + assert!(!change("retarget", Some(link("t")), Some(link("u"))).content_unchanged()); + assert!(!change("added", None, Some(file(0o644, 1, 1))).content_unchanged()); + } + + #[test] + fn type_changed_needs_both_sides_with_different_kinds() { + assert!(change("f2l", Some(file(0o644, 1, 1)), Some(link("t"))).type_changed()); + assert!(!change("edit", Some(file(0o644, 1, 1)), Some(file(0o644, 1, 2))).type_changed()); + assert!(!change("added", None, Some(link("t"))).type_changed()); + } + + #[test] + fn renames_pairs_a_deleted_file_with_the_added_file_of_equal_digest() { + let changes = vec![ + change("old.txt", Some(file(0o644, 5, 7)), None), + change("new.txt", None, Some(file(0o644, 5, 7))), + change("other.txt", None, Some(file(0o644, 5, 8))), + ]; + assert_eq!( + renames(&changes), + vec![(Path::new("old.txt"), Path::new("new.txt"))], + ); + } + + #[test] + fn renames_leaves_ambiguous_digests_unpaired() { + let changes = vec![ + change("a", Some(file(0o644, 5, 7)), None), + change("b", Some(file(0o644, 5, 7)), None), + change("c", None, Some(file(0o644, 5, 7))), + ]; + assert!(renames(&changes).is_empty()); + } + + #[test] + fn display_keeps_the_kind_and_path_form() { + let c = change("dir/f.txt", None, Some(file(0o644, 1, 1))); + assert_eq!(c.to_string(), "A dir/f.txt"); } } diff --git a/crates/sandlock-core/src/transaction.rs b/crates/sandlock-core/src/transaction.rs index cd296d9e..d05f6d87 100644 --- a/crates/sandlock-core/src/transaction.rs +++ b/crates/sandlock-core/src/transaction.rs @@ -2065,7 +2065,7 @@ mod tests { let paths: Vec<_> = finished .changes .iter() - .map(|c| (c.kind.clone(), c.path.clone())) + .map(|c| (c.kind(), c.path.clone())) .collect(); assert_eq!( paths, diff --git a/crates/sandlock-core/tests/integration/test_branch_action.rs b/crates/sandlock-core/tests/integration/test_branch_action.rs index 66c19cae..6dae271f 100644 --- a/crates/sandlock-core/tests/integration/test_branch_action.rs +++ b/crates/sandlock-core/tests/integration/test_branch_action.rs @@ -32,7 +32,7 @@ async fn abort_reports_added_file_without_creating_it() { drop(sb); assert!(!workdir.join("new.txt").exists(), "aborted run must not write the workdir"); - assert!(result.changes.iter().any(|c| c.kind == ChangeKind::Added && c.path == Path::new("new.txt"))); + assert!(result.changes.iter().any(|c| c.kind() == ChangeKind::Added && c.path == Path::new("new.txt"))); let _ = fs::remove_dir_all(&workdir); let _ = fs::remove_dir_all(&storage); } @@ -49,7 +49,7 @@ async fn abort_reports_modified_file_without_changing_it() { drop(sb); assert_eq!(fs::read_to_string(workdir.join("data.txt")).unwrap(), "original"); - assert!(result.changes.iter().any(|c| c.kind == ChangeKind::Modified && c.path == Path::new("data.txt"))); + assert!(result.changes.iter().any(|c| c.kind() == ChangeKind::Modified && c.path == Path::new("data.txt"))); let _ = fs::remove_dir_all(&workdir); let _ = fs::remove_dir_all(&storage); } @@ -66,7 +66,7 @@ async fn abort_reports_deleted_file_without_removing_it() { drop(sb); assert!(workdir.join("victim.txt").exists()); - assert!(result.changes.iter().any(|c| c.kind == ChangeKind::Deleted && c.path == Path::new("victim.txt"))); + assert!(result.changes.iter().any(|c| c.kind() == ChangeKind::Deleted && c.path == Path::new("victim.txt"))); let _ = fs::remove_dir_all(&workdir); let _ = fs::remove_dir_all(&storage); } @@ -82,7 +82,7 @@ async fn commit_reports_the_changes_it_merged() { drop(sb); assert_eq!(fs::read_to_string(workdir.join("out.txt")).unwrap(), "hi\n"); - assert!(result.changes.iter().any(|c| c.kind == ChangeKind::Added && c.path == Path::new("out.txt"))); + assert!(result.changes.iter().any(|c| c.kind() == ChangeKind::Added && c.path == Path::new("out.txt"))); let _ = fs::remove_dir_all(&workdir); let _ = fs::remove_dir_all(&storage); } diff --git a/crates/sandlock-core/tests/integration/test_transaction.rs b/crates/sandlock-core/tests/integration/test_transaction.rs index 43c63a71..ca67e38f 100644 --- a/crates/sandlock-core/tests/integration/test_transaction.rs +++ b/crates/sandlock-core/tests/integration/test_transaction.rs @@ -615,7 +615,7 @@ async fn test_txn_reports_changes_on_commit_and_abort() { let mut got: Vec<(ChangeKind, String)> = committed .changes .iter() - .map(|c| (c.kind.clone(), c.path.display().to_string())) + .map(|c| (c.kind(), c.path.display().to_string())) .collect(); got.sort_by(|a, b| a.1.cmp(&b.1)); assert_eq!( @@ -927,7 +927,7 @@ async fn test_txn_deletion_commit_applies_abort_preserves() { assert!(committed.committed(), "commit expected; disposition: {:?}", committed.disposition); 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::>(), + committed.changes.iter().map(|c| (c.kind(), c.path.display().to_string())).collect::>(), vec![(ChangeKind::Deleted, "keep.txt".to_string())], "a deletion must be reported as a change", ); diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index f0c296a6..ac291042 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1765,7 +1765,7 @@ pub unsafe extern "C" fn sandlock_result_change_kind(r: *const sandlock_result_t return 0; } let changes = &(*r)._private.changes; - match changes.get(i).map(|c| &c.kind) { + match changes.get(i).map(|c| c.kind()) { Some(sandlock_core::ChangeKind::Added) => b'A' as c_char, Some(sandlock_core::ChangeKind::Modified) => b'M' as c_char, Some(sandlock_core::ChangeKind::Deleted) => b'D' as c_char, From 6ce6404992a0f54664cfc4801c1e1b9a418954bd Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 13:46:11 -0700 Subject: [PATCH 02/12] cow: add the origin map The change set can only say what the run replaced if something remembered the workdir entry before the branch touched it. This map holds that, keyed by relative path, first record wins, nothing on disk: losing it degrades a report from Modified to Added and nothing else, which does not justify an fsync in the syscall path the way a lost whiteout does. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/mod.rs | 1 + crates/sandlock-core/src/cow/origins.rs | 80 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 crates/sandlock-core/src/cow/origins.rs diff --git a/crates/sandlock-core/src/cow/mod.rs b/crates/sandlock-core/src/cow/mod.rs index 3b40b2a1..d5b3acbd 100644 --- a/crates/sandlock-core/src/cow/mod.rs +++ b/crates/sandlock-core/src/cow/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod deletions; +pub(crate) mod origins; pub(crate) mod seccomp; pub(crate) mod dispatch; pub(crate) mod result; diff --git a/crates/sandlock-core/src/cow/origins.rs b/crates/sandlock-core/src/cow/origins.rs new file mode 100644 index 00000000..624b4244 --- /dev/null +++ b/crates/sandlock-core/src/cow/origins.rs @@ -0,0 +1,80 @@ +//! What the workdir held for each path the run touched, recorded at first +//! touch so the change set can report the side the branch replaced. +//! +//! Not a source of correctness: the upper and the whiteout set are the +//! change set. Losing this only turns a Modified into an Added in the +//! report, so nothing here is written to disk during the run. + +use std::collections::BTreeMap; + +use crate::result::Entry; + +#[derive(Debug, Default)] +pub(crate) struct Origins { + entries: BTreeMap>, +} + +impl Origins { + /// Record what the workdir held at `rel`; `None` for no entry. Only the + /// first record for a path is kept. Returns whether this was the first. + pub fn record(&mut self, rel: &str, before: Option) -> bool { + if self.entries.contains_key(rel) { + return false; + } + self.entries.insert(rel.to_string(), before); + true + } + + pub fn get(&self, rel: &str) -> Option<&Option> { + self.entries.get(rel) + } + + /// Every recorded path strictly beneath `prefix`, by path component: + /// "d" yields "d/x" but never "d2". + pub fn under<'a>(&'a self, prefix: &str) -> impl Iterator)> { + let start = format!("{prefix}/"); + self.entries + .range(start.clone()..) + .take_while(move |(k, _)| k.starts_with(&start)) + .map(|(k, v)| (k.as_str(), v)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::result::{Entry, EntryKind}; + + fn file(digest: u8) -> Entry { + Entry { kind: EntryKind::File, mode: 0o644, size: 1, digest: Some([digest; 32]), target: None } + } + + #[test] + fn first_record_wins() { + let mut o = Origins::default(); + assert!(o.record("f", Some(file(1)))); + assert!(!o.record("f", Some(file(2)))); + assert_eq!(o.get("f"), Some(&Some(file(1)))); + assert_eq!(o.get("missing"), None); + } + + #[test] + fn an_absent_lower_is_recorded_as_none_and_still_counts_as_recorded() { + let mut o = Origins::default(); + assert!(o.record("new", None)); + assert!(!o.record("new", Some(file(1)))); + assert_eq!(o.get("new"), Some(&None)); + } + + #[test] + fn under_matches_path_components_not_string_prefixes() { + let mut o = Origins::default(); + o.record("d", Some(file(0))); + o.record("d/x", Some(file(1))); + o.record("d/y/z", Some(file(2))); + o.record("d2", Some(file(3))); + o.record("d2/x", Some(file(4))); + let got: Vec<&str> = o.under("d").map(|(p, _)| p).collect(); + assert_eq!(got, vec!["d/x", "d/y/z"]); + } +} From 75ebe02c6a275f68c78b56c852246d7e6206e114 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 13:55:03 -0700 Subject: [PATCH 03/12] cow: record what the run first saw at each path it touches The change set labelled a path by looking at the live workdir when the report was read, so a file that appeared underneath after the run turned a creation into an overwrite, and a report could never carry the old digest. Every handler that mutates a path now records the workdir entry at first touch, and the report is computed from that record and the upper alone. A copied-up file's digest is taken inside the copy stream, off the COW lock, and handed back once the copy lands: hashing under the lock would reintroduce the stall the two-phase copy exists to avoid. A file unlinked without a copy-up is hashed at capture from the workdir, which the commit has not touched yet, guarded by its recorded size. A whiteout over an entry the confined lstat could not reach keeps being reported, with an unknown before side, so an obstructed deletion still shows as outstanding after a failed merge. Signed-off-by: Cong Wang --- Cargo.lock | 1 + crates/sandlock-core/Cargo.toml | 1 + crates/sandlock-core/src/cow/dispatch.rs | 21 +- crates/sandlock-core/src/cow/origins.rs | 27 +- crates/sandlock-core/src/cow/seccomp.rs | 397 +++++++++++++++++++---- crates/sandlock-core/src/result.rs | 12 +- 6 files changed, 390 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a67705e6..a6464763 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1400,6 +1400,7 @@ dependencies = [ "rand", "rand_chacha", "rcgen", + "ring", "rustls", "rustls-pemfile", "serde", diff --git a/crates/sandlock-core/Cargo.toml b/crates/sandlock-core/Cargo.toml index 3bc16f48..08c1dfda 100644 --- a/crates/sandlock-core/Cargo.toml +++ b/crates/sandlock-core/Cargo.toml @@ -22,6 +22,7 @@ uuid = { version = "1", features = ["v4"] } bincode = "1" serde_json = "1" walkdir = "2" +ring = "0.17" toml = "0.8" jiff = "0.2" pathdiff = "0.2" diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index 9b91e40a..2555e85e 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -266,7 +266,15 @@ pub(crate) async fn handle_cow_open( }).await; match copy_result { - Ok(Ok(())) => upper, + Ok(Ok(digest)) => { + if let Some(d) = digest { + let mut st = cow_state.lock().await; + if let Some(cow) = st.branch.as_mut() { + cow.record_digest(&rel_path, d); + } + } + upper + } Ok(Err(_)) | Err(_) => { // Copy failed — roll back quota and let kernel handle it let mut st = cow_state.lock().await; @@ -541,11 +549,20 @@ async fn execute_deferred_copy( upper: std::path::PathBuf, file_size: u64, ) -> Option { + let rel_path = rel.clone(); let copy_result = tokio::task::spawn_blocking(move || { crate::cow::seccomp::SeccompCowBranch::execute_copy(&workdir_root, &upper_root, &rel) }).await; match copy_result { - Ok(Ok(())) => Some(upper), + Ok(Ok(digest)) => { + if let Some(d) = digest { + let mut st = cow_state.lock().await; + if let Some(cow) = st.branch.as_mut() { + cow.record_digest(&rel_path, d); + } + } + Some(upper) + } _ => { let mut st = cow_state.lock().await; if let Some(cow) = st.branch.as_mut() { diff --git a/crates/sandlock-core/src/cow/origins.rs b/crates/sandlock-core/src/cow/origins.rs index 624b4244..ead412ee 100644 --- a/crates/sandlock-core/src/cow/origins.rs +++ b/crates/sandlock-core/src/cow/origins.rs @@ -7,7 +7,7 @@ use std::collections::BTreeMap; -use crate::result::Entry; +use crate::result::{Entry, EntryKind}; #[derive(Debug, Default)] pub(crate) struct Origins { @@ -25,6 +25,15 @@ impl Origins { true } + /// Fill in the digest of a recorded file whose bytes were streamed later. + pub fn set_digest(&mut self, rel: &str, digest: [u8; 32]) { + if let Some(Some(e)) = self.entries.get_mut(rel) { + if e.kind == EntryKind::File && e.digest.is_none() { + e.digest = Some(digest); + } + } + } + pub fn get(&self, rel: &str) -> Option<&Option> { self.entries.get(rel) } @@ -66,6 +75,22 @@ mod tests { assert_eq!(o.get("new"), Some(&None)); } + #[test] + fn set_digest_only_fills_an_empty_file_digest() { + let mut o = Origins::default(); + let mut undigested = file(0); + undigested.digest = None; + o.record("f", Some(undigested)); + o.record("keep", Some(file(1))); + o.record("gone", None); + o.set_digest("f", [9; 32]); + o.set_digest("keep", [9; 32]); + o.set_digest("gone", [9; 32]); + assert_eq!(o.get("f").unwrap().as_ref().unwrap().digest, Some([9; 32])); + assert_eq!(o.get("keep").unwrap().as_ref().unwrap().digest, Some([1; 32])); + assert_eq!(o.get("gone"), Some(&None)); + } + #[test] fn under_matches_path_components_not_string_prefixes() { let mut o = Origins::default(); diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index 500db2ea..cf0ee562 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -624,6 +624,8 @@ pub struct SeccompCowBranch { /// created after the branch's own deletion landed. Callers that need the /// workdir quiescent across retries have to hold it quiescent themselves. applied_deletions: HashSet, + /// What the workdir held at each path when the run first touched it. + origins: crate::cow::origins::Origins, has_changes: bool, state: BranchState, /// What `Drop` does with a branch that was never disposed of: reclaim it @@ -634,32 +636,82 @@ pub struct SeccompCowBranch { disk_used: u64, } -/// The entry at `path` without following symlinks; `None` when absent. -pub(crate) fn lstat_entry(path: &Path) -> Option { - use std::os::unix::fs::{FileTypeExt, PermissionsExt}; - let meta = fs::symlink_metadata(path).ok()?; - let ft = meta.file_type(); - let kind = if ft.is_dir() { - EntryKind::Dir - } else if ft.is_symlink() { - EntryKind::Symlink - } else if ft.is_file() { - EntryKind::File - } else if ft.is_fifo() || ft.is_socket() { - EntryKind::Other - } else { - return None; +/// The entry at `rel` under `root`, confined and without following +/// symlinks: `Ok(None)` when absent, `Err` when it cannot be inspected. +/// Digests are filled by the caller. +pub(crate) fn lstat_entry_in_root(root: &Path, rel: &str) -> Result, i32> { + let st = match crate::sys::fs::statat_in_root(root, rel, false) { + Ok(st) => st, + Err(libc::ENOENT) => return Ok(None), + Err(e) => return Err(e), + }; + let kind = match st.st_mode & libc::S_IFMT { + libc::S_IFREG => EntryKind::File, + libc::S_IFDIR => EntryKind::Dir, + libc::S_IFLNK => EntryKind::Symlink, + libc::S_IFIFO | libc::S_IFSOCK => EntryKind::Other, + _ => return Ok(None), }; let target = (kind == EntryKind::Symlink) - .then(|| fs::read_link(path).ok().map(|t| t.to_string_lossy().into_owned())) - .flatten(); - Some(Entry { + .then(|| crate::sys::fs::readlink_in_root(root, rel).ok()) + .flatten() + .map(|t| String::from_utf8_lossy(&t).into_owned()); + Ok(Some(Entry { kind, - mode: meta.permissions().mode() & 0o7777, - size: if kind == EntryKind::File { meta.len() } else { 0 }, + mode: st.st_mode & 0o7777, + size: if kind == EntryKind::File { st.st_size as u64 } else { 0 }, digest: None, target, - }) + })) +} + +/// SHA-256 of the regular file at `rel` under `root`, read confined; +/// `None` when it cannot be read or is not a regular file. +pub(crate) fn sha256_in_root(root: &Path, rel: &str) -> Option<[u8; 32]> { + let fd = crate::sys::fs::openat2_in_root( + root, + rel, + libc::O_RDONLY | libc::O_NONBLOCK | libc::O_NOFOLLOW | libc::O_CLOEXEC, + 0, + ) + .ok()?; + let mut f = unsafe { fs::File::from_raw_fd(fd) }; + if !f.metadata().ok()?.file_type().is_file() { + return None; + } + let mut hasher = Sha256Writer::new(std::io::sink()); + std::io::copy(&mut f, &mut hasher).ok()?; + Some(hasher.finish()) +} + +/// Hashes what passes through on the way to `inner`. +struct Sha256Writer { + inner: W, + ctx: ring::digest::Context, +} + +impl Sha256Writer { + fn new(inner: W) -> Self { + Self { inner, ctx: ring::digest::Context::new(&ring::digest::SHA256) } + } + + fn finish(self) -> [u8; 32] { + let mut out = [0u8; 32]; + out.copy_from_slice(self.ctx.finish().as_ref()); + out + } +} + +impl std::io::Write for Sha256Writer { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let n = self.inner.write(buf)?; + self.ctx.update(&buf[..n]); + Ok(n) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } } impl SeccompCowBranch { @@ -713,6 +765,7 @@ impl SeccompCowBranch { storage_dir: branch_dir, deleted, applied_deletions: HashSet::new(), + origins: Default::default(), has_changes: false, state: BranchState::Open, keep_if_abandoned: false, @@ -800,12 +853,42 @@ impl SeccompCowBranch { self.deleted.covers(rel_path) && !self.upper_has(rel_path) } + /// Remember what the workdir holds at `rel` the first time the run + /// touches it. Cheap: an lstat, no bytes read. File digests arrive from + /// the copy stream or at capture. + fn touch(&mut self, rel: &str) { + if self.origins.get(rel).is_some() { + return; + } + // Unreadable is not absent: leave it unrecorded so a later deletion + // reports an unknown origin rather than a creation. + if let Ok(before) = lstat_entry_in_root(&self.workdir, rel) { + self.origins.record(rel, before); + } + } + + /// Record every ancestor of `rel`, so the directories a copy-up recreates + /// in the upper are known to be the workdir's own and not reported. + fn touch_parents(&mut self, rel: &str) { + let mut end = rel.len(); + while let Some(i) = rel[..end].rfind('/') { + self.touch(&rel[..i]); + end = i; + } + } + + /// The digest of a file the copy stream just read out of the workdir. + pub(crate) fn record_digest(&mut self, rel: &str, digest: [u8; 32]) { + self.origins.set_digest(rel, digest); + } + /// Mark a relative path as deleted (whiteout over it and its subtree). /// /// Deliberately does not touch `applied_deletions`: re-marking a path this /// branch already removed from the workdir leaves it non-outstanding, which /// is correct — the workdir entry is already gone. pub fn mark_deleted(&mut self, rel_path: &str) { + self.touch(rel_path); self.deleted.insert(rel_path); self.has_changes = true; } @@ -847,6 +930,8 @@ impl SeccompCowBranch { /// `ensure_cow_copy` (synchronous) and the async two-phase dispatch. pub fn prepare_copy(&mut self, rel_path: &str) -> Result { self.has_changes = true; + self.touch_parents(rel_path); + self.touch(rel_path); let upper_file = self.upper.join(rel_path); let lower_file = self.workdir.join(rel_path); @@ -934,7 +1019,7 @@ impl SeccompCowBranch { workdir_root: &Path, upper_root: &Path, rel: &str, - ) -> Result<(), std::io::Error> { + ) -> Result, std::io::Error> { let create_dest = || -> Result { let fd = crate::sys::fs::openat2_in_root( upper_root, @@ -960,7 +1045,7 @@ impl SeccompCowBranch { // escape target. Err(libc::EACCES) | Err(libc::ENOENT) => { create_dest()?; - return Ok(()); + return Ok(None); } // On a kernel without openat2 (ENOSYS) the copy fails and the caller // rolls back / returns Continue; the child then hits Landlock, which @@ -976,14 +1061,14 @@ impl SeccompCowBranch { // of waiting for a writer; on a regular file it is a no-op for reads. if !src.metadata()?.file_type().is_file() { create_dest()?; - return Ok(()); + return Ok(None); } - let mut dst = create_dest()?; + let mut dst = Sha256Writer::new(create_dest()?); std::io::copy(&mut src, &mut dst)?; if let Ok(meta) = src.metadata() { - let _ = dst.set_permissions(meta.permissions()); + let _ = dst.inner.set_permissions(meta.permissions()); } - Ok(()) + Ok(Some(dst.finish())) } /// Ensure a COW copy exists in upper (synchronous). Returns the upper path. @@ -993,7 +1078,12 @@ impl SeccompCowBranch { CowCopyPlan::Ready(upper) => Ok(upper), CowCopyPlan::NeedsCopy { upper, lower: _lower, file_size } => { match Self::execute_copy(&self.workdir, &self.upper, rel_path) { - Ok(()) => Ok(upper), + Ok(digest) => { + if let Some(d) = digest { + self.record_digest(rel_path, d); + } + Ok(upper) + } Err(e) => { self.rollback_copy(file_size); Err(BranchError::Operation(format!("copy: {}", e))) @@ -1339,6 +1429,8 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; + self.touch_parents(&rel); + self.touch(&rel); self.check_quota(4096)?; // directory metadata self.has_changes = true; let ok = crate::sys::fs::mkdirp_in_root(&self.upper, &rel, 0o755).is_ok(); @@ -1368,6 +1460,8 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; + self.touch_parents(&rel); + self.touch(&rel); self.check_quota(256)?; self.has_changes = true; // Ensure the parent directory exists in the upper layer before creating @@ -1415,6 +1509,8 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; + self.touch_parents(&new_rel); + self.touch(&new_rel); let src_is_dir = match self.merged_entry_is_dir(&old_rel) { Some(d) => d, None => return Err(libc::ENOENT), @@ -1476,6 +1572,8 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; + self.touch_parents(&rel); + self.touch(&rel); if std::path::Path::new(target).is_absolute() || target.split('/').any(|c| c == "..") { return Ok(false); } @@ -1503,6 +1601,8 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; + self.touch_parents(&new_rel); + self.touch(&new_rel); if self.is_deleted(&old_rel) { return Err(BranchError::Deleted); } @@ -1619,22 +1719,32 @@ impl SeccompCowBranch { None } - /// List all filesystem changes in the COW layer. + /// List all filesystem changes in the COW layer: the upper against what + /// the run first saw at each path. pub fn changes(&self) -> Result, BranchError> { use crate::result::Change; let mut result = Vec::new(); + let mut reported: HashSet = HashSet::new(); for entry in walkdir::WalkDir::new(&self.upper).min_depth(1) { let entry = entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?; let rel = entry.path().strip_prefix(&self.upper).unwrap(); - let before = lstat_entry(&self.workdir.join(rel)); - let after = lstat_entry(entry.path()); - // Copy-up recreates a modified file's parents in the upper; a - // directory the workdir already has is scaffolding, not a change. - if entry.file_type().is_dir() && before.as_ref().is_some_and(|b| b.kind == EntryKind::Dir) { - continue; + let rel_str = rel.to_string_lossy().into_owned(); + let mut after = lstat_entry_in_root(&self.upper, &rel_str).ok().flatten(); + if let Some(a) = after.as_mut().filter(|a| a.kind == EntryKind::File) { + a.digest = sha256_in_root(&self.upper, &rel_str); + } + let before = self.origins.get(&rel_str).cloned().flatten(); + // Copy-up recreates a modified file's parents in the upper. The + // commit only mkdirs, so a directory the workdir already had is + // scaffolding whatever mode the upper copy carries. + if let (Some(b), Some(a)) = (&before, &after) { + if a.kind == EntryKind::Dir && b.kind == EntryKind::Dir { + continue; + } } + reported.insert(rel_str); result.push(Change { path: rel.to_path_buf(), before, after }); } @@ -1645,17 +1755,54 @@ impl SeccompCowBranch { if self.applied_deletions.contains(rel_path) { continue; } - // Re-created in the upper: the upper walk reports it instead. - if self.upper_has(rel_path) { + if self.upper_has(rel_path) || reported.contains(rel_path) { + continue; + } + // A whiteout is only written over a lower entry, so an origin of + // absent here means the confined lstat could not reach what an + // unconfined one saw (a symlinked parent): report it with an + // unknown before side rather than drop an outstanding deletion. + let before = self.deleted_origin(rel_path); + let is_dir = before.as_ref().is_some_and(|b| b.kind == EntryKind::Dir); + reported.insert(rel_path.to_string()); + result.push(Change { path: PathBuf::from(rel_path), before, after: None }); + if !is_dir { continue; } - let Some(before) = lstat_entry(&self.workdir.join(rel_path)) else { continue }; - result.push(Change { path: std::path::PathBuf::from(rel_path), before: Some(before), after: None }); + // A whiteout hides the whole subtree; report what the run had + // seen beneath it so a moved tree pairs up entry by entry. + let children: Vec = self + .origins + .under(rel_path) + .filter(|(child, b)| b.is_some() && !self.upper_has(child) && !reported.contains(*child)) + .map(|(child, _)| child.to_string()) + .collect(); + for child in children { + let Some(before) = self.deleted_origin(&child) else { continue }; + reported.insert(child.clone()); + result.push(Change { path: PathBuf::from(child), before: Some(before), after: None }); + } } Ok(result) } + /// The recorded origin of a deleted path. A file unlinked without a + /// copy-up was never streamed, so its digest is read now from the + /// workdir, which the commit has not touched yet; a size mismatch means + /// someone else rewrote it and the digest stays unknown. + fn deleted_origin(&self, rel: &str) -> Option { + let mut before = self.origins.get(rel).cloned().flatten()?; + if before.kind == EntryKind::File && before.digest.is_none() { + let unchanged = crate::sys::fs::statat_in_root(&self.workdir, rel, false) + .is_ok_and(|st| st.st_mode & libc::S_IFMT == libc::S_IFREG && st.st_size as u64 == before.size); + if unchanged { + before.digest = sha256_in_root(&self.workdir, rel); + } + } + Some(before) + } + /// List merged directory entries (upper + lower - deleted). pub fn list_merged_dir(&self, rel_path: &str) -> Vec { let lower_dir = self.workdir.join(rel_path); @@ -3147,9 +3294,9 @@ mod tests { assert_eq!( outstanding, vec![ - // b.txt is "modified" because the obstructing symlink is still - // there in the workdir; c.txt was never reached. - (crate::result::ChangeKind::Modified, "b.txt".to_string()), + // Both were written straight into the upper, so neither has + // an origin; b.txt is the obstructed one, c.txt was never reached. + (crate::result::ChangeKind::Added, "b.txt".to_string()), (crate::result::ChangeKind::Added, "c.txt".to_string()), ], "changes() after a partial merge must report the remainder only", @@ -5843,31 +5990,26 @@ mod tests { ); } - /// `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. + /// `changes()` labels an entry against what the run saw when it first + /// touched the path, not against the live workdir at the time of the + /// call: a file that appears underneath afterwards does not turn the + /// run's creation into an overwrite. #[test] - fn changes_labels_an_entry_against_the_workdir_as_it_stands_now() { + fn changes_labels_an_entry_against_what_the_run_first_saw() { use crate::result::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", - ); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.handle_open(&format!("{}/f.txt", branch.workdir_str()), CREATE_WRITE).unwrap().unwrap(); + fs::write(&upper, "from the run").unwrap(); + assert_eq!(branch.changes().unwrap()[0].kind(), ChangeKind::Added); 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", + ChangeKind::Added, + "the label follows the run's first touch, not the workdir as it stands now", ); } @@ -5924,9 +6066,9 @@ mod tests { let storage = tempfile::tempdir().unwrap(); fs::create_dir(workdir.path().join("sub")).unwrap(); - let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - fs::create_dir(branch.upper.join("sub")).unwrap(); - fs::write(branch.upper.join("sub/a.txt"), "new").unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.handle_open(&format!("{}/sub/a.txt", branch.workdir_str()), CREATE_WRITE).unwrap().unwrap(); + fs::write(&upper, "new").unwrap(); let changes: Vec<_> = branch .changes() @@ -5937,6 +6079,139 @@ mod tests { assert_eq!(changes, vec![(ChangeKind::Added, "sub/a.txt".to_string())]); } + const CREATE_WRITE: u64 = (libc::O_CREAT | libc::O_WRONLY) as u64; + const SHA256_ABC: &str = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"; + + fn hex(d: &[u8; 32]) -> String { + d.iter().map(|b| format!("{b:02x}")).collect() + } + + fn by_path(branch: &SeccompCowBranch) -> std::collections::BTreeMap { + branch + .changes() + .unwrap() + .into_iter() + .map(|c| (c.path.display().to_string(), c)) + .collect() + } + + /// The before side is the workdir entry at first touch, digest included, + /// and the after side is the upper entry at capture. + #[test] + fn a_copied_up_file_reports_both_digests() { + use crate::result::EntryKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("f.txt"), "abc").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let upper = branch.handle_open(&format!("{}/f.txt", branch.workdir_str()), CREATE_WRITE).unwrap().unwrap(); + fs::write(&upper, "xyz").unwrap(); + + let c = &by_path(&branch)["f.txt"]; + let before = c.before.as_ref().unwrap(); + let after = c.after.as_ref().unwrap(); + assert_eq!(before.kind, EntryKind::File); + assert_eq!(before.size, 3); + assert_eq!(hex(&before.digest.unwrap()), SHA256_ABC); + assert_ne!(after.digest, before.digest); + assert!(!c.content_unchanged()); + } + + #[test] + fn a_deleted_file_reports_the_digest_it_had() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("g.txt"), "abc").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + assert!(branch.handle_unlink(&format!("{}/g.txt", branch.workdir_str()), false).unwrap()); + + let c = &by_path(&branch)["g.txt"]; + assert!(c.after.is_none()); + assert_eq!(hex(&c.before.as_ref().unwrap().digest.unwrap()), SHA256_ABC); + } + + /// A moved directory reports every entry on both sides, so a digest join + /// recovers the rename. + #[test] + fn a_renamed_directory_expands_to_per_entry_changes_that_pair_up() { + use crate::result::{renames, ChangeKind, EntryKind}; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::create_dir(workdir.path().join("d")).unwrap(); + fs::write(workdir.path().join("d/x"), "abc").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = branch.workdir_str().to_string(); + assert!(branch.handle_rename(&format!("{wd}/d"), &format!("{wd}/e")).unwrap()); + + let all = by_path(&branch); + assert_eq!(all["d"].kind(), ChangeKind::Deleted); + assert_eq!(all["d"].before.as_ref().unwrap().kind, EntryKind::Dir); + assert_eq!(all["d/x"].kind(), ChangeKind::Deleted); + assert_eq!(all["e"].kind(), ChangeKind::Added); + assert_eq!(all["e/x"].kind(), ChangeKind::Added); + let changes: Vec<_> = all.into_values().collect(); + assert_eq!(renames(&changes), vec![(Path::new("d/x"), Path::new("e/x"))]); + } + + #[test] + fn mkdir_reports_an_added_directory_with_no_before_side() { + use crate::result::EntryKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + assert!(branch.handle_mkdir(&format!("{}/newdir", branch.workdir_str())).unwrap()); + + let c = &by_path(&branch)["newdir"]; + assert!(c.before.is_none()); + assert_eq!(c.after.as_ref().unwrap().kind, EntryKind::Dir); + } + + #[test] + fn chmod_alone_is_modified_with_content_unchanged() { + use crate::result::ChangeKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("f.txt"), "abc").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + assert!(branch.handle_chmod(&format!("{}/f.txt", branch.workdir_str()), 0o755).unwrap()); + + let c = &by_path(&branch)["f.txt"]; + assert_eq!(c.kind(), ChangeKind::Modified); + assert!(c.content_unchanged()); + assert_ne!(c.before.as_ref().unwrap().mode, c.after.as_ref().unwrap().mode); + } + + #[test] + fn retargeting_a_symlink_changes_content_and_keeps_the_kind() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("a", workdir.path().join("link")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = branch.workdir_str().to_string(); + assert!(branch.handle_unlink(&format!("{wd}/link"), false).unwrap()); + assert!(branch.handle_symlink("b", &format!("{wd}/link")).unwrap()); + + let c = &by_path(&branch)["link"]; + assert_eq!(c.before.as_ref().unwrap().target.as_deref(), Some("a")); + assert_eq!(c.after.as_ref().unwrap().target.as_deref(), Some("b")); + assert!(!c.content_unchanged()); + assert!(!c.type_changed()); + } + + #[test] + fn execute_copy_returns_the_digest_of_what_it_streamed() { + let workdir = tempfile::tempdir().unwrap(); + let upper = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("f"), "abc").unwrap(); + let digest = SeccompCowBranch::execute_copy(workdir.path(), upper.path(), "f").unwrap().unwrap(); + assert_eq!(hex(&digest), SHA256_ABC); + } + // ---- Names, symlinks and the confined path helpers ---- /// `safe_rel` normalises the spellings that name the same entry, rejects an diff --git a/crates/sandlock-core/src/result.rs b/crates/sandlock-core/src/result.rs index 119eadc1..996482d2 100644 --- a/crates/sandlock-core/src/result.rs +++ b/crates/sandlock-core/src/result.rs @@ -104,13 +104,14 @@ impl Entry { } } -/// One filesystem change a run made to its COW branch. At least one side -/// is always present. +/// One filesystem change a run made to its COW branch. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Change { /// Relative to the workdir. pub path: PathBuf, - /// The workdir entry when the run first touched the path. + /// The workdir entry when the run first touched the path. `None` for a + /// path that did not exist, or for a deletion of one the run could not + /// inspect. pub before: Option, /// The branch entry when the change set was read. pub after: Option, @@ -119,8 +120,8 @@ pub struct Change { impl Change { pub fn kind(&self) -> ChangeKind { match (&self.before, &self.after) { - (None, _) => ChangeKind::Added, - (Some(_), None) => ChangeKind::Deleted, + (_, None) => ChangeKind::Deleted, + (None, Some(_)) => ChangeKind::Added, (Some(_), Some(_)) => ChangeKind::Modified, } } @@ -194,6 +195,7 @@ mod change_tests { assert_eq!(change("a", None, Some(file(0o644, 1, 1))).kind(), ChangeKind::Added); assert_eq!(change("m", Some(file(0o644, 1, 1)), Some(file(0o644, 2, 2))).kind(), ChangeKind::Modified); assert_eq!(change("d", Some(file(0o644, 1, 1)), None).kind(), ChangeKind::Deleted); + assert_eq!(change("unseen", None, None).kind(), ChangeKind::Deleted); } #[test] From bfb949c8eedc2c1af9435f166c395529e045205b Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 13:59:48 -0700 Subject: [PATCH 04/12] cow: carry the origin records in the preserved marker A kept or preserved branch outlives the process that ran it, and with it the origin map, so recovery tooling could only report additions and bare deletions. The marker already lists the outstanding deletions and is written once, atomically, off the syscall path; it now also holds one line per origin record, and a preserved branch reports the same change set a live run does through the same computation. Nothing is written during the run. A supervisor killed before it preserves loses the origins and the report degrades to unknown before sides, which is the trade for a run that pays no fsync per touch. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/origins.rs | 4 + crates/sandlock-core/src/cow/seccomp.rs | 337 ++++++++++++++++++------ 2 files changed, 262 insertions(+), 79 deletions(-) diff --git a/crates/sandlock-core/src/cow/origins.rs b/crates/sandlock-core/src/cow/origins.rs index ead412ee..c59af162 100644 --- a/crates/sandlock-core/src/cow/origins.rs +++ b/crates/sandlock-core/src/cow/origins.rs @@ -34,6 +34,10 @@ impl Origins { } } + pub fn iter(&self) -> impl Iterator)> { + self.entries.iter().map(|(k, v)| (k.as_str(), v)) + } + pub fn get(&self, rel: &str) -> Option<&Option> { self.entries.get(rel) } diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index cf0ee562..aa279a33 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -222,6 +222,9 @@ pub struct PreservedBranch { pub deleted: Vec, /// Why it was preserved, which says what state the workdir is in. pub reason: PreserveReason, + /// What the run first saw at each path it touched, as far as the marker + /// recorded it. Feeds [`changes`](Self::changes). + pub origins: Vec<(PathBuf, Option)>, /// The process that preserved it. /// /// Load-bearing for one thing: a `MergeInterrupted` marker is written @@ -296,6 +299,7 @@ pub fn read_preserved(branch_dir: &Path) -> Option { let mut upper = None; let mut pid = None; let mut deleted = Vec::new(); + let mut origins = Vec::new(); for line in body.split(|&b| b == b'\n') { let sep = match line.iter().position(|&b| b == b'=') { Some(i) => i, @@ -309,6 +313,7 @@ pub fn read_preserved(branch_dir: &Path) -> Option { b"upper" => upper = Some(path()), // Repeated, one per deleted path — the only multi-valued key. b"deleted" => deleted.push(path()), + b"before" => origins.extend(parse_origin_line(value)), b"pid" => pid = std::str::from_utf8(value).ok().and_then(|s| s.parse().ok()), _ => {} } @@ -318,11 +323,93 @@ pub fn read_preserved(branch_dir: &Path) -> Option { upper: upper?, workdir: workdir?, deleted, + origins, reason: reason?, pid: pid?, }) } +impl PreservedBranch { + /// The change set the branch still holds, in the same shape a live run + /// reports: the preserved upper against what the run first saw. + pub fn changes(&self) -> Result, BranchError> { + let mut origins = crate::cow::origins::Origins::default(); + for (path, before) in &self.origins { + origins.record(&path.to_string_lossy(), before.clone()); + } + let deleted: Vec = self.deleted.iter().map(|p| p.to_string_lossy().into_owned()).collect(); + compute_changes(&self.upper, &self.workdir, &origins, deleted.iter().map(String::as_str)) + } +} + +/// One marker line for an origin record: kind, mode, size, a hex column +/// holding the digest or the link target, and the path last because it is +/// the field that may contain a tab. +fn origin_line(rel: &str, before: &Option) -> Vec { + let hex = |bytes: &[u8]| bytes.iter().map(|b| format!("{b:02x}")).collect::(); + let line = match before { + None => format!("-\t0\t0\t-\t{rel}"), + Some(e) => { + let kind = match e.kind { + EntryKind::File => 'f', + EntryKind::Dir => 'd', + EntryKind::Symlink => 'l', + EntryKind::Other => 'o', + }; + let column = match (&e.digest, &e.target) { + (Some(d), _) => hex(d), + (None, Some(t)) => hex(t.as_bytes()), + (None, None) => "-".to_string(), + }; + format!("{kind}\t{:o}\t{}\t{column}\t{rel}", e.mode, e.size) + } + }; + marker_escape(line.as_bytes()) +} + +fn parse_origin_line(value: &[u8]) -> Option<(PathBuf, Option)> { + use std::os::unix::ffi::OsStringExt; + let raw = marker_unescape(value); + let mut fields = raw.splitn(5, |&b| b == b'\t'); + let kind = fields.next()?; + let mode = std::str::from_utf8(fields.next()?).ok()?; + let size = std::str::from_utf8(fields.next()?).ok()?; + let column = fields.next()?; + let path = PathBuf::from(std::ffi::OsString::from_vec(fields.next()?.to_vec())); + let unhex = |h: &[u8]| -> Option> { + if h.len() % 2 != 0 { + return None; + } + h.chunks(2) + .map(|c| u8::from_str_radix(std::str::from_utf8(c).ok()?, 16).ok()) + .collect() + }; + let kind = match kind { + b"-" => return Some((path, None)), + b"f" => EntryKind::File, + b"d" => EntryKind::Dir, + b"l" => EntryKind::Symlink, + b"o" => EntryKind::Other, + _ => return None, + }; + let mut entry = Entry { + kind, + mode: u32::from_str_radix(mode, 8).ok()?, + size: size.parse().ok()?, + digest: None, + target: None, + }; + if column != b"-" { + let bytes = unhex(column)?; + match kind { + EntryKind::File => entry.digest = Some(bytes.try_into().ok()?), + EntryKind::Symlink => entry.target = Some(String::from_utf8_lossy(&bytes).into_owned()), + _ => {} + } + } + Some((path, Some(entry))) +} + /// Enumerate every preserved branch directly under `storage_base` — the sweep /// primitive for recovering work this process (or a previous one) could not /// publish. @@ -714,6 +801,91 @@ impl std::io::Write for Sha256Writer { } } +/// The change set of `upper` against `origins`, plus the outstanding +/// `deleted` whiteouts. Shared by a live branch and a preserved one. +fn compute_changes<'a>( + upper: &Path, + workdir: &Path, + origins: &crate::cow::origins::Origins, + deleted: impl Iterator, +) -> Result, BranchError> { + use crate::result::Change; + + let upper_has = |rel: &str| crate::sys::fs::statat_in_root(upper, rel, false).is_ok(); + let mut result = Vec::new(); + let mut reported: HashSet = HashSet::new(); + + for entry in walkdir::WalkDir::new(upper).min_depth(1) { + let entry = entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?; + let rel = entry.path().strip_prefix(upper).unwrap(); + let rel_str = rel.to_string_lossy().into_owned(); + let mut after = lstat_entry_in_root(upper, &rel_str).ok().flatten(); + if let Some(a) = after.as_mut().filter(|a| a.kind == EntryKind::File) { + a.digest = sha256_in_root(upper, &rel_str); + } + let before = origins.get(&rel_str).cloned().flatten(); + // Copy-up recreates a modified file's parents in the upper. The + // commit only mkdirs, so a directory the workdir already had is + // scaffolding whatever mode the upper copy carries. + if let (Some(b), Some(a)) = (&before, &after) { + if a.kind == EntryKind::Dir && b.kind == EntryKind::Dir { + continue; + } + } + reported.insert(rel_str); + result.push(Change { path: rel.to_path_buf(), before, after }); + } + + // Deletions from the whiteout set; an entry re-created in the upper + // is reported by the upper walk instead. + for rel_path in deleted { + if upper_has(rel_path) || reported.contains(rel_path) { + continue; + } + // A whiteout is only written over a lower entry, so an origin of + // absent here means the confined lstat could not reach what an + // unconfined one saw (a symlinked parent): report it with an + // unknown before side rather than drop an outstanding deletion. + let before = deleted_origin(workdir, origins, rel_path); + let is_dir = before.as_ref().is_some_and(|b| b.kind == EntryKind::Dir); + reported.insert(rel_path.to_string()); + result.push(Change { path: PathBuf::from(rel_path), before, after: None }); + if !is_dir { + continue; + } + // A whiteout hides the whole subtree; report what the run had + // seen beneath it so a moved tree pairs up entry by entry. + let children: Vec = origins + .under(rel_path) + .filter(|(child, b)| b.is_some() && !upper_has(child) && !reported.contains(*child)) + .map(|(child, _)| child.to_string()) + .collect(); + for child in children { + let Some(before) = deleted_origin(workdir, origins, &child) else { continue }; + reported.insert(child.clone()); + result.push(Change { path: PathBuf::from(child), before: Some(before), after: None }); + } + } + + Ok(result) +} + +/// The recorded origin of a deleted path. A file unlinked without a +/// copy-up was never streamed, so its digest is read now from the +/// workdir, which the commit has not touched yet; a size mismatch means +/// someone else rewrote it and the digest stays unknown. +fn deleted_origin(workdir: &Path, origins: &crate::cow::origins::Origins, rel: &str) -> Option { + let mut before = origins.get(rel).cloned().flatten()?; + if before.kind == EntryKind::File && before.digest.is_none() { + let unchanged = crate::sys::fs::statat_in_root(workdir, rel, false) + .is_ok_and(|st| st.st_mode & libc::S_IFMT == libc::S_IFREG && st.st_size as u64 == before.size); + if unchanged { + before.digest = sha256_in_root(workdir, rel); + } + } + Some(before) +} + impl SeccompCowBranch { /// Create a new seccomp COW branch. /// @@ -1722,85 +1894,7 @@ impl SeccompCowBranch { /// List all filesystem changes in the COW layer: the upper against what /// the run first saw at each path. pub fn changes(&self) -> Result, BranchError> { - use crate::result::Change; - - let mut result = Vec::new(); - let mut reported: HashSet = HashSet::new(); - - for entry in walkdir::WalkDir::new(&self.upper).min_depth(1) { - let entry = entry.map_err(|e| BranchError::Operation(format!("walk: {}", e)))?; - let rel = entry.path().strip_prefix(&self.upper).unwrap(); - let rel_str = rel.to_string_lossy().into_owned(); - let mut after = lstat_entry_in_root(&self.upper, &rel_str).ok().flatten(); - if let Some(a) = after.as_mut().filter(|a| a.kind == EntryKind::File) { - a.digest = sha256_in_root(&self.upper, &rel_str); - } - let before = self.origins.get(&rel_str).cloned().flatten(); - // Copy-up recreates a modified file's parents in the upper. The - // commit only mkdirs, so a directory the workdir already had is - // scaffolding whatever mode the upper copy carries. - if let (Some(b), Some(a)) = (&before, &after) { - if a.kind == EntryKind::Dir && b.kind == EntryKind::Dir { - continue; - } - } - reported.insert(rel_str); - result.push(Change { path: rel.to_path_buf(), before, after }); - } - - // Deletions from the whiteout set; an entry re-created in the upper - // is reported by the upper walk instead. - for rel_path in self.deleted.iter() { - // Already landed this run: not something the next commit will do. - if self.applied_deletions.contains(rel_path) { - continue; - } - if self.upper_has(rel_path) || reported.contains(rel_path) { - continue; - } - // A whiteout is only written over a lower entry, so an origin of - // absent here means the confined lstat could not reach what an - // unconfined one saw (a symlinked parent): report it with an - // unknown before side rather than drop an outstanding deletion. - let before = self.deleted_origin(rel_path); - let is_dir = before.as_ref().is_some_and(|b| b.kind == EntryKind::Dir); - reported.insert(rel_path.to_string()); - result.push(Change { path: PathBuf::from(rel_path), before, after: None }); - if !is_dir { - continue; - } - // A whiteout hides the whole subtree; report what the run had - // seen beneath it so a moved tree pairs up entry by entry. - let children: Vec = self - .origins - .under(rel_path) - .filter(|(child, b)| b.is_some() && !self.upper_has(child) && !reported.contains(*child)) - .map(|(child, _)| child.to_string()) - .collect(); - for child in children { - let Some(before) = self.deleted_origin(&child) else { continue }; - reported.insert(child.clone()); - result.push(Change { path: PathBuf::from(child), before: Some(before), after: None }); - } - } - - Ok(result) - } - - /// The recorded origin of a deleted path. A file unlinked without a - /// copy-up was never streamed, so its digest is read now from the - /// workdir, which the commit has not touched yet; a size mismatch means - /// someone else rewrote it and the digest stays unknown. - fn deleted_origin(&self, rel: &str) -> Option { - let mut before = self.origins.get(rel).cloned().flatten()?; - if before.kind == EntryKind::File && before.digest.is_none() { - let unchanged = crate::sys::fs::statat_in_root(&self.workdir, rel, false) - .is_ok_and(|st| st.st_mode & libc::S_IFMT == libc::S_IFREG && st.st_size as u64 == before.size); - if unchanged { - before.digest = sha256_in_root(&self.workdir, rel); - } - } - Some(before) + compute_changes(&self.upper, &self.workdir, &self.origins, self.outstanding_deletions()) } /// List merged directory entries (upper + lower - deleted). @@ -2530,6 +2624,10 @@ impl SeccompCowBranch { body.extend_from_slice(b"\ndeleted="); body.extend_from_slice(&marker_escape(rel.as_bytes())); } + for (rel, before) in self.origins.iter() { + body.extend_from_slice(b"\nbefore="); + body.extend_from_slice(&origin_line(rel, before)); + } body.extend_from_slice(format!("\npid={}\n", std::process::id()).as_bytes()); let tmp = self.storage_dir.join(PRESERVED_TMP); @@ -6212,6 +6310,87 @@ mod tests { assert_eq!(hex(&digest), SHA256_ABC); } + fn sorted_changes(changes: Vec) -> Vec { + let mut v = changes; + v.sort_by(|a, b| a.path.cmp(&b.path)); + v + } + + /// The marker carries what the run first saw, so recovery tooling reads + /// the same change set the live branch reported, digests included. + #[test] + fn a_preserved_branch_reports_the_same_changes_as_the_live_one() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("mod.txt"), "abc").unwrap(); + fs::write(workdir.path().join("gone.txt"), "abc").unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = branch.workdir_str().to_string(); + let upper = branch.handle_open(&format!("{wd}/mod.txt"), CREATE_WRITE).unwrap().unwrap(); + fs::write(&upper, "xyz").unwrap(); + assert!(branch.handle_unlink(&format!("{wd}/gone.txt"), false).unwrap()); + assert!(branch.handle_mkdir(&format!("{wd}/newdir")).unwrap()); + let live = sorted_changes(branch.changes().unwrap()); + assert_eq!(live.len(), 3); + assert!(live.iter().all(|c| c.before.as_ref().is_none_or(|b| b.kind != EntryKind::File || b.digest.is_some()))); + + branch.preserve(PreserveReason::Kept); + let preserved = read_preserved(&branch.storage_dir).expect("a kept branch has a marker"); + assert_eq!(sorted_changes(preserved.changes().unwrap()), live); + } + + #[test] + fn the_marker_round_trips_a_symlink_target_with_a_tab_and_a_newline() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("we\tird\nname", workdir.path().join("link")).unwrap(); + + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let wd = branch.workdir_str().to_string(); + assert!(branch.handle_unlink(&format!("{wd}/link"), false).unwrap()); + branch.preserve(PreserveReason::Kept); + + let preserved = read_preserved(&branch.storage_dir).unwrap(); + let changes = preserved.changes().unwrap(); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].before.as_ref().unwrap().target.as_deref(), Some("we\tird\nname")); + } + + /// A marker from before origins existed, or one whose origin lines were + /// lost, still yields a change set: additions and deletions with no + /// before side. + #[test] + fn a_marker_without_origin_lines_reports_unknown_origins() { + use crate::result::ChangeKind; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + fs::write(workdir.path().join("gone.txt"), "abc").unwrap(); + let branch_dir = storage.path().join("old"); + fs::create_dir_all(branch_dir.join("upper")).unwrap(); + fs::write(branch_dir.join("upper/added.txt"), "payload").unwrap(); + fs::write( + branch_dir.join(PRESERVED_MARKER), + format!( + "reason=kept\nworkdir={}\nupper={}\ndeleted=gone.txt\npid=1\n", + workdir.path().display(), + branch_dir.join("upper").display(), + ), + ) + .unwrap(); + + let preserved = read_preserved(&branch_dir).unwrap(); + let changes = sorted_changes(preserved.changes().unwrap()); + let summary: Vec<_> = changes.iter().map(|c| (c.kind(), c.path.display().to_string(), c.before.is_some())).collect(); + assert_eq!( + summary, + vec![ + (ChangeKind::Added, "added.txt".to_string(), false), + (ChangeKind::Deleted, "gone.txt".to_string(), false), + ], + ); + } + // ---- Names, symlinks and the confined path helpers ---- /// `safe_rel` normalises the spellings that name the same entry, rejects an From 745375df6daa13a79cf6d5d0857c6c7e4b53dcac Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 14:01:57 -0700 Subject: [PATCH 05/12] ffi: expose both sides of a change The C ABI only carried a change's kind and path, so a binding could not show what a run replaced. One accessor fills a pointer-free struct with either side (kind, mode, size, digest) so every binding can hold it on the stack, and a second returns a symlink target as a string. The existing kind and path accessors keep their signatures. Signed-off-by: Cong Wang --- crates/sandlock-ffi/include/sandlock.h | 35 +++++++ crates/sandlock-ffi/src/lib.rs | 81 +++++++++++++++ crates/sandlock-ffi/tests/changes.rs | 138 +++++++++++++++++++++++++ 3 files changed, 254 insertions(+) create mode 100644 crates/sandlock-ffi/tests/changes.rs diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 0a38f53d..d8d55e2e 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -163,6 +163,19 @@ typedef struct sandlock_handle_t sandlock_handle_t; */ typedef struct sandlock_pipeline_t sandlock_pipeline_t; +/** + * One side of a change, as plain data so every binding can hold it on + * the stack. `kind`: 0 file, 1 dir, 2 symlink, 3 other. `digest` is + * SHA-256 and only meaningful when `has_digest` is 1 (files). + */ +typedef struct { + uint8_t kind; + uint32_t mode; + uint64_t size; + uint8_t has_digest; + uint8_t digest[32]; +} sandlock_entry_t; + /** * C-compatible syscall event passed to the policy callback. * @@ -995,6 +1008,28 @@ char sandlock_result_change_kind(const sandlock_result_t *r, uintptr_t i); */ char *sandlock_result_change_path(const sandlock_result_t *r, uintptr_t i); +/** + * Fill `out` with one side of the i-th change: `side` 0 is before the run + * touched the path, 1 is after. Returns 0 when filled, 1 when that side is + * absent (`out` untouched), -1 when `i` or `side` is out of range. + * + * # Safety + * `r` must be a valid result pointer and `out` a valid, writable pointer. + */ +int sandlock_result_change_entry(const sandlock_result_t *r, + uintptr_t i, + int side, + sandlock_entry_t *out); + +/** + * Symlink target of one side of the i-th change; NULL when that side is + * absent or not a symlink. Caller must free with `sandlock_string_free`. + * + * # Safety + * `r` must be a valid result pointer. + */ +char *sandlock_result_change_target(const sandlock_result_t *r, uintptr_t i, int side); + /** * # Safety * `r` must be null or a valid pointer from `sandlock_run`. diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index ac291042..1862c0a4 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1790,6 +1790,87 @@ pub unsafe extern "C" fn sandlock_result_change_path(r: *const sandlock_result_t } } +/// One side of a change, as plain data so every binding can hold it on +/// the stack. `kind`: 0 file, 1 dir, 2 symlink, 3 other. `digest` is +/// SHA-256 and only meaningful when `has_digest` is 1 (files). +#[repr(C)] +#[allow(non_camel_case_types)] +#[derive(Clone, Copy)] +pub struct sandlock_entry_t { + pub kind: u8, + pub mode: u32, + pub size: u64, + pub has_digest: u8, + pub digest: [u8; 32], +} + +unsafe fn change_side<'a>(r: *const sandlock_result_t, i: usize, side: c_int) -> Option<&'a Option> { + if r.is_null() { + return None; + } + let changes = &(*r)._private.changes; + let change = changes.get(i)?; + match side { + 0 => Some(&change.before), + 1 => Some(&change.after), + _ => None, + } +} + +/// Fill `out` with one side of the i-th change: `side` 0 is before the run +/// touched the path, 1 is after. Returns 0 when filled, 1 when that side is +/// absent (`out` untouched), -1 when `i` or `side` is out of range. +/// +/// # Safety +/// `r` must be a valid result pointer and `out` a valid, writable pointer. +#[no_mangle] +pub unsafe extern "C" fn sandlock_result_change_entry( + r: *const sandlock_result_t, + i: usize, + side: c_int, + out: *mut sandlock_entry_t, +) -> c_int { + use sandlock_core::EntryKind; + let Some(entry) = change_side(r, i, side) else { return -1 }; + let Some(e) = entry else { return 1 }; + if out.is_null() { + return -1; + } + *out = sandlock_entry_t { + kind: match e.kind { + EntryKind::File => 0, + EntryKind::Dir => 1, + EntryKind::Symlink => 2, + EntryKind::Other => 3, + }, + mode: e.mode, + size: e.size, + has_digest: e.digest.is_some() as u8, + digest: e.digest.unwrap_or([0; 32]), + }; + 0 +} + +/// Symlink target of one side of the i-th change; NULL when that side is +/// absent or not a symlink. Caller must free with `sandlock_string_free`. +/// +/// # Safety +/// `r` must be a valid result pointer. +#[no_mangle] +pub unsafe extern "C" fn sandlock_result_change_target( + r: *const sandlock_result_t, + i: usize, + side: c_int, +) -> *mut c_char { + let target = change_side(r, i, side) + .and_then(|e| e.as_ref()) + .and_then(|e| e.target.as_deref()); + match target { + Some(t) => CString::new(t).map(|s| s.into_raw()).unwrap_or(ptr::null_mut()), + None => ptr::null_mut(), + } +} + /// # Safety /// `r` must be null or a valid pointer from `sandlock_run`. #[no_mangle] diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs new file mode 100644 index 00000000..9b1f6064 --- /dev/null +++ b/crates/sandlock-ffi/tests/changes.rs @@ -0,0 +1,138 @@ +//! The C ABI side of a change set: both sides of every change are +//! readable as plain scalars, plus a symlink target string. + +use std::ffi::{CStr, CString}; +use std::os::raw::{c_char, c_int, c_uint}; +use std::path::Path; +use std::ptr; + +use sandlock_ffi::{ + sandlock_create_for_run, sandlock_entry_t, sandlock_handle_free, sandlock_handle_wait, + sandlock_result_change_entry, sandlock_result_change_kind, sandlock_result_change_path, + sandlock_result_change_target, sandlock_result_changes_len, sandlock_result_free, + sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, + sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, + sandlock_sandbox_builder_fs_write, sandlock_sandbox_builder_new, + sandlock_sandbox_builder_on_exit, sandlock_sandbox_builder_workdir, sandlock_sandbox_free, + sandlock_sandbox_t, sandlock_start, sandlock_string_free, +}; + +const ABORT: u8 = 1; +const BEFORE: c_int = 0; +const AFTER: c_int = 1; +const ENTRY_FILE: u8 = 0; +const ENTRY_SYMLINK: u8 = 2; +const SHA256_ABC: &str = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"; + +fn build_policy(workdir: &Path, storage: &Path) -> *mut sandlock_sandbox_t { + let mut b = sandlock_sandbox_builder_new(); + for p in ["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc"] { + if p == "/lib64" && !Path::new("/lib64").exists() { continue; } + let c = CString::new(p).unwrap(); + b = unsafe { sandlock_sandbox_builder_fs_read(b, c.as_ptr()) }; + } + let wd = CString::new(workdir.to_str().unwrap()).unwrap(); + let st = CString::new(storage.to_str().unwrap()).unwrap(); + unsafe { + b = sandlock_sandbox_builder_fs_write(b, wd.as_ptr()); + b = sandlock_sandbox_builder_workdir(b, wd.as_ptr()); + b = sandlock_sandbox_builder_cwd(b, wd.as_ptr()); + b = sandlock_sandbox_builder_fs_storage(b, st.as_ptr()); + b = sandlock_sandbox_builder_on_exit(b, ABORT); + } + let mut err: c_int = 0; + let policy = unsafe { sandlock_sandbox_build(b, &mut err, ptr::null_mut()) }; + assert!(!policy.is_null(), "build failed: {err}"); + policy +} + +fn argv(cmd: &[&str]) -> (Vec, Vec<*const c_char>) { + let owned: Vec = cmd.iter().map(|s| CString::new(*s).unwrap()).collect(); + let ptrs = owned.iter().map(|c| c.as_ptr()).collect(); + (owned, ptrs) +} + +fn hex(d: &[u8; 32]) -> String { + d.iter().map(|b| format!("{b:02x}")).collect() +} + +fn entry(r: *const sandlock_ffi::sandlock_result_t, i: usize, side: c_int) -> Option { + let mut out = sandlock_entry_t { kind: 0, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; + match unsafe { sandlock_result_change_entry(r, i, side, &mut out) } { + 0 => Some(out), + 1 => None, + rc => panic!("unexpected rc {rc}"), + } +} + +fn target(r: *const sandlock_ffi::sandlock_result_t, i: usize, side: c_int) -> Option { + let p = unsafe { sandlock_result_change_target(r, i, side) }; + if p.is_null() { + return None; + } + let s = unsafe { CStr::from_ptr(p) }.to_str().unwrap().to_string(); + unsafe { sandlock_string_free(p) }; + Some(s) +} + +#[test] +fn both_sides_of_every_change_are_readable() { + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + std::fs::write(workdir.path().join("mod.txt"), "abc").unwrap(); + std::fs::write(workdir.path().join("gone.txt"), "abc").unwrap(); + std::os::unix::fs::symlink("a", workdir.path().join("link")).unwrap(); + + let policy = build_policy(workdir.path(), storage.path()); + let (_owned, av) = argv(&[ + "sh", "-c", + "echo new > added.txt && echo xyz > mod.txt && rm gone.txt && rm link && ln -s b link", + ]); + let h = unsafe { sandlock_create_for_run(policy, ptr::null(), av.as_ptr(), av.len() as c_uint) }; + unsafe { sandlock_sandbox_free(policy) }; + assert!(!h.is_null()); + assert_eq!(unsafe { sandlock_start(h) }, 0); + let r = unsafe { sandlock_handle_wait(h) }; + assert!(unsafe { sandlock_result_success(r) }); + + let n = unsafe { sandlock_result_changes_len(r) }; + let mut by_path = std::collections::BTreeMap::new(); + for i in 0..n { + let p = unsafe { sandlock_result_change_path(r, i) }; + by_path.insert(unsafe { CStr::from_ptr(p) }.to_str().unwrap().to_string(), i); + unsafe { sandlock_string_free(p) }; + } + assert_eq!(by_path.keys().collect::>(), vec!["added.txt", "gone.txt", "link", "mod.txt"]); + + let i = by_path["mod.txt"]; + assert_eq!(unsafe { sandlock_result_change_kind(r, i) } as u8, b'M'); + let before = entry(r, i, BEFORE).unwrap(); + let after = entry(r, i, AFTER).unwrap(); + assert_eq!(before.kind, ENTRY_FILE); + assert_eq!(before.size, 3); + assert_eq!(before.has_digest, 1); + assert_eq!(hex(&before.digest), SHA256_ABC); + assert_eq!(after.size, 4); + assert_ne!(after.digest, before.digest); + + let i = by_path["added.txt"]; + assert!(entry(r, i, BEFORE).is_none()); + assert_eq!(entry(r, i, AFTER).unwrap().size, 4); + + let i = by_path["gone.txt"]; + assert_eq!(hex(&entry(r, i, BEFORE).unwrap().digest), SHA256_ABC); + assert!(entry(r, i, AFTER).is_none()); + + let i = by_path["link"]; + assert_eq!(entry(r, i, BEFORE).unwrap().kind, ENTRY_SYMLINK); + assert_eq!(target(r, i, BEFORE).as_deref(), Some("a")); + assert_eq!(target(r, i, AFTER).as_deref(), Some("b")); + assert!(target(r, by_path["mod.txt"], AFTER).is_none()); + + let mut out = sandlock_entry_t { kind: 0, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; + assert_eq!(unsafe { sandlock_result_change_entry(r, n, BEFORE, &mut out) }, -1); + assert_eq!(unsafe { sandlock_result_change_entry(r, 0, 2, &mut out) }, -1); + + unsafe { sandlock_result_free(r) }; + unsafe { sandlock_handle_free(h) }; +} From 2bbd4eee3c1aa30fa138107128b58ea7b43250fe Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 14:03:42 -0700 Subject: [PATCH 06/12] python: expose both sides of a change Change now carries the workdir entry the run first saw and the branch entry it left, each as an Entry with kind, mode, size, digest, and link target. kind is derived from which sides exist, so the README examples keep printing "A out.txt", and renames() pairs moved files by digest. Signed-off-by: Cong Wang --- python/README.md | 19 ++++++++- python/src/sandlock/__init__.py | 4 +- python/src/sandlock/_sdk.py | 56 +++++++++++++++++++++---- python/src/sandlock/sandbox.py | 74 +++++++++++++++++++++++++++++++-- python/tests/test_sandbox.py | 49 +++++++++++++++++++++- 5 files changed, 188 insertions(+), 14 deletions(-) diff --git a/python/README.md b/python/README.md index ce89234c..f73f9e6d 100644 --- a/python/README.md +++ b/python/README.md @@ -418,8 +418,25 @@ Returned by `sandbox.run()`. | Attribute | Type | Description | |-----------|------|-------------| -| `kind` | `str` | `"A"` (added), `"M"` (modified), or `"D"` (deleted) | | `path` | `str` | Path relative to workdir | +| `before` | `Entry \| None` | The workdir entry when the run first touched the path; `None` if absent | +| `after` | `Entry \| None` | The branch entry when the change set was read; `None` if removed | +| `kind` | `str` | Derived: `"A"` (no `before`), `"M"` (both sides), `"D"` (no `after`) | +| `content_unchanged` | `bool` | Both sides present with the same kind and digest or target | +| `type_changed` | `bool` | Both sides present with different kinds | + +`renames(changes)` pairs each deleted file with the added file carrying +the same digest, as `(old_path, new_path)` tuples. + +### Entry + +| Attribute | Type | Description | +|-----------|------|-------------| +| `kind` | `str` | `"file"`, `"dir"`, `"symlink"`, or `"other"` | +| `mode` | `int` | Permission bits | +| `size` | `int` | Byte length for a file; 0 otherwise | +| `digest` | `bytes \| None` | SHA-256 of the bytes; files only | +| `target` | `str \| None` | Link target; symlinks only | ### Stage and Pipeline diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index 557593b1..6152e3e3 100644 --- a/python/src/sandlock/__init__.py +++ b/python/src/sandlock/__init__.py @@ -15,7 +15,7 @@ from .inputs import inputs from .handler import Handler, NotifAction, HandlerCtx, ExceptionPolicy from .sandbox import ( - Sandbox, BranchAction, parse_ports, Change, StdioMode, Process, + Sandbox, BranchAction, parse_ports, Change, Entry, renames, StdioMode, Process, ) from ._profile import load_profile, list_profiles from .exceptions import ( @@ -52,6 +52,8 @@ "BranchAction", "parse_ports", "Change", + "Entry", + "renames", "StdioMode", "Process", "Protection", diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index 109d9f3b..2dc3d371 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -12,7 +12,7 @@ from pathlib import Path from typing import Any, NamedTuple, Sequence -from .sandbox import Change, Sandbox as PolicyDataclass +from .sandbox import Change, Entry, Sandbox as PolicyDataclass # ---------------------------------------------------------------- # Load the shared library @@ -376,6 +376,23 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_result_change_path.restype = ctypes.c_void_p _lib.sandlock_result_change_path.argtypes = [_c_result_p, ctypes.c_size_t] + +class _CEntry(ctypes.Structure): + _fields_ = [ + ("kind", ctypes.c_uint8), + ("mode", ctypes.c_uint32), + ("size", ctypes.c_uint64), + ("has_digest", ctypes.c_uint8), + ("digest", ctypes.c_uint8 * 32), + ] + + +_lib.sandlock_result_change_entry.restype = ctypes.c_int +_lib.sandlock_result_change_entry.argtypes = [_c_result_p, ctypes.c_size_t, ctypes.c_int, ctypes.POINTER(_CEntry)] + +_lib.sandlock_result_change_target.restype = ctypes.c_void_p +_lib.sandlock_result_change_target.argtypes = [_c_result_p, ctypes.c_size_t, ctypes.c_int] + # Pipeline _lib.sandlock_pipeline_new.restype = _c_pipeline_p _lib.sandlock_pipeline_new.argtypes = [] @@ -741,17 +758,40 @@ def _read_result_bytes(result_p, fn) -> bytes: return ctypes.string_at(ptr, length.value) +_ENTRY_KINDS = ("file", "dir", "symlink", "other") + + +def _take_string(p) -> str | None: + if not p: + return None + s = ctypes.string_at(p).decode("utf-8", "surrogateescape") + _lib.sandlock_string_free(ctypes.cast(p, ctypes.c_char_p)) + return s + + +def _read_change_side(result_p, i: int, side: int) -> Entry | None: + raw = _CEntry() + if _lib.sandlock_result_change_entry(result_p, i, side, ctypes.byref(raw)) != 0: + return None + return Entry( + kind=_ENTRY_KINDS[raw.kind], + mode=raw.mode, + size=raw.size, + digest=bytes(raw.digest) if raw.has_digest else None, + target=_take_string(_lib.sandlock_result_change_target(result_p, i, side)), + ) + + def _read_result_changes(result_p) -> list: """Read the change list from a result pointer.""" changes = [] for i in range(_lib.sandlock_result_changes_len(result_p)): - kind = _lib.sandlock_result_change_kind(result_p, i).decode("ascii") - path_p = _lib.sandlock_result_change_path(result_p, i) - path = "" - if path_p: - path = ctypes.string_at(path_p).decode("utf-8", "surrogateescape") - _lib.sandlock_string_free(ctypes.cast(path_p, ctypes.c_char_p)) - changes.append(Change(kind=kind, path=path)) + path = _take_string(_lib.sandlock_result_change_path(result_p, i)) or "" + changes.append(Change( + path=path, + before=_read_change_side(result_p, i, 0), + after=_read_change_side(result_p, i, 1), + )) return changes diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index a628e0cc..fd5e2829 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -117,15 +117,83 @@ class StdioMode(IntEnum): @dataclass(frozen=True) -class Change: - """A single filesystem change a run made to its COW branch.""" +class Entry: + """One side of a :class:`Change`.""" kind: str - """A=added, M=modified (exists on both sides, bytes not compared), D=deleted.""" + """``"file"``, ``"dir"``, ``"symlink"``, or ``"other"`` (fifo, socket).""" + + mode: int + """Permission bits.""" + + size: int + """Byte length for a file; 0 otherwise.""" + + digest: bytes | None + """SHA-256 of the bytes. Files only.""" + + target: str | None + """Link target, verbatim. Symlinks only.""" + + def _same_content(self, other: "Entry") -> bool: + return self.kind == other.kind and self.digest == other.digest and self.target == other.target + + +@dataclass(frozen=True) +class Change: + """A single filesystem change a run made to its COW branch.""" path: str """Path relative to workdir.""" + before: Entry | None + """The workdir entry when the run first touched the path. ``None`` for a + path that did not exist, or for a deletion of one the run could not inspect.""" + + after: Entry | None + """The branch entry when the change set was read. ``None`` when removed.""" + + @property + def kind(self) -> str: + """``"A"`` added, ``"M"`` modified (both sides present), ``"D"`` deleted.""" + if self.after is None: + return "D" + return "A" if self.before is None else "M" + + @property + def content_unchanged(self) -> bool: + """Both sides present with the same kind and bytes or target: a touch, + a mode change, or a rewrite with identical contents.""" + return self.before is not None and self.after is not None and self.before._same_content(self.after) + + @property + def type_changed(self) -> bool: + return self.before is not None and self.after is not None and self.before.kind != self.after.kind + + def __str__(self) -> str: + return f"{self.kind} {self.path}" + + +def renames(changes: Sequence[Change]) -> list[tuple[str, str]]: + """Pair each deleted file with the added file carrying the same digest. + A digest seen more than once on either side is ambiguous and left unpaired.""" + def unique(side: str) -> dict[bytes, str | None]: + by_digest: dict[bytes, str | None] = {} + for c in changes: + entry = c.before if side == "before" else c.after + other = c.after if side == "before" else c.before + if entry is None or other is not None or entry.digest is None: + continue + by_digest[entry.digest] = None if entry.digest in by_digest else c.path + return by_digest + + deleted, added = unique("before"), unique("after") + return sorted( + (old, added[d]) + for d, old in deleted.items() + if old is not None and added.get(d) is not None + ) + @dataclass class Sandbox: diff --git a/python/tests/test_sandbox.py b/python/tests/test_sandbox.py index b1eeab7c..6bd904ca 100644 --- a/python/tests/test_sandbox.py +++ b/python/tests/test_sandbox.py @@ -16,7 +16,7 @@ from pathlib import Path -from sandlock import Sandbox, BranchAction, Change +from sandlock import Sandbox, BranchAction, Change, Entry, renames _PYTHON_READABLE = list(dict.fromkeys([ @@ -771,6 +771,53 @@ def test_added_empty_directory_is_reported(self, tmp_path): assert [(c.kind, c.path) for c in result.changes] == [("A", "newdir")] assert not (workdir / "newdir").exists() + SHA256_ABC = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + + def test_a_modified_file_carries_both_entries(self, tmp_path): + workdir = tmp_path / "both-sides" + workdir.mkdir() + (workdir / "data.txt").write_text("abc") + p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.ABORT) + result = p.run(["sh", "-c", f"echo xyz > {workdir}/data.txt"]) + assert result.success, result + [c] = result.changes + assert c.kind == "M" and c.path == "data.txt" + assert isinstance(c.before, Entry) and isinstance(c.after, Entry) + assert c.before.kind == "file" and c.before.size == 3 + assert c.before.digest.hex() == self.SHA256_ABC + assert c.after.size == 4 and c.after.digest != c.before.digest + assert not c.content_unchanged and not c.type_changed + + def test_touch_alone_is_modified_with_content_unchanged(self, tmp_path): + workdir = tmp_path / "touch" + workdir.mkdir() + (workdir / "data.txt").write_text("abc") + p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.ABORT) + result = p.run(["touch", str(workdir / "data.txt")]) + assert result.success, result + [c] = result.changes + assert c.kind == "M" and c.content_unchanged + + def test_a_deleted_symlink_reports_its_target(self, tmp_path): + workdir = tmp_path / "symlink" + workdir.mkdir() + (workdir / "link").symlink_to("a") + p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.ABORT) + result = p.run(["rm", str(workdir / "link")]) + assert result.success, result + [c] = result.changes + assert c.kind == "D" and c.after is None + assert c.before.kind == "symlink" and c.before.target == "a" and c.before.digest is None + + def test_renames_pairs_moved_files_by_digest(self, tmp_path): + workdir = tmp_path / "renames" + workdir.mkdir() + (workdir / "old.txt").write_text("abc") + p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.ABORT) + result = p.run(["mv", str(workdir / "old.txt"), str(workdir / "new.txt")]) + assert result.success, result + assert renames(result.changes) == [("old.txt", "new.txt")] + def test_commit_reports_the_changes_it_merged(self, tmp_path): workdir = tmp_path / "commit" workdir.mkdir() From 959fdad386edb17eb6167eca9daf199160af7b9a Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 14:05:48 -0700 Subject: [PATCH 07/12] go: expose both sides of a change Change now carries the Before and After entries (kind, mode, size, digest, link target) and derives its kind from which sides exist; Renames pairs moved files by digest. The ChangeKind constants are unchanged. Signed-off-by: Cong Wang --- go/README.md | 7 ++- go/sandbox.go | 96 +++++++++++++++++++++++++++++++++++++-- go/sandlock_linux.go | 31 ++++++++++++- go/sandlock_linux_test.go | 65 ++++++++++++++++++++++++-- 4 files changed, 187 insertions(+), 12 deletions(-) diff --git a/go/README.md b/go/README.md index 9af3c541..9bce7fdb 100644 --- a/go/README.md +++ b/go/README.md @@ -126,8 +126,11 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) deadline does not preempt a running child. - **RunInteractive** inherits the caller's stdio and returns the exit code. - Every `Result` from a sandbox with `Workdir` carries `Changes`, the files and - directories the run added, modified, or deleted in its COW branch. A dry run is a run - with `OnExit: BranchActionAbort`. + directories the run added, modified, or deleted in its COW branch. Each + `Change` holds the `Before` and `After` entries (kind, mode, size, digest, + link target); `Kind()` derives A, M, or D from which sides exist, and + `Renames` pairs moved files by digest. A dry run is a run with + `OnExit: BranchActionAbort`. - **Spawn** starts a process without waiting, returning a `*Process`. - **Popen** is the streaming counterpart of Spawn: each stream set to `StdioPiped` is handed back on the `*Process` as an `*os.File` diff --git a/go/sandbox.go b/go/sandbox.go index d5dd6b8b..4db0cc8b 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -33,6 +33,8 @@ package sandlock import ( + "os" + "sort" "strings" "unsafe" ) @@ -342,10 +344,94 @@ const ( ChangeDeleted ChangeKind = 'D' ) -// Change is one filesystem change a run made to its COW branch. Modified -// means the path exists on both sides; the bytes are not compared, so a -// rename over an existing file counts. +// EntryKind classifies one side of a Change. +type EntryKind uint8 + +const ( + EntryFile EntryKind = iota + EntryDir + EntrySymlink + EntryOther // fifo or socket: no bytes, only a mode +) + +// Entry is one side of a Change. +type Entry struct { + Kind EntryKind + Mode os.FileMode // permission bits + Size int64 // byte length for a file; 0 otherwise + Digest *[32]byte // SHA-256 of the bytes; files only + Target string // link target, verbatim; symlinks only +} + +func (e *Entry) sameContent(o *Entry) bool { + if e.Kind != o.Kind || e.Target != o.Target || (e.Digest == nil) != (o.Digest == nil) { + return false + } + return e.Digest == nil || *e.Digest == *o.Digest +} + +// Change is one filesystem change a run made to its COW branch. type Change struct { - Kind ChangeKind // 'A' added, 'M' modified, 'D' deleted - Path string // path relative to the working directory + Path string // path relative to the working directory + Before *Entry // the workdir entry when the run first touched the path; nil if absent + After *Entry // the branch entry when the change set was read; nil if removed +} + +// Kind is derived from which sides are present. +func (c Change) Kind() ChangeKind { + switch { + case c.After == nil: + return ChangeDeleted + case c.Before == nil: + return ChangeAdded + default: + return ChangeModified + } +} + +// ContentUnchanged reports both sides present with the same kind and bytes +// or target: a touch, a mode change, or a rewrite with identical contents. +func (c Change) ContentUnchanged() bool { + return c.Before != nil && c.After != nil && c.Before.sameContent(c.After) +} + +// TypeChanged reports both sides present with different kinds. +func (c Change) TypeChanged() bool { + return c.Before != nil && c.After != nil && c.Before.Kind != c.After.Kind +} + +func (c Change) String() string { + return string(c.Kind()) + " " + c.Path +} + +// Renames pairs each deleted file with the added file carrying the same +// digest, as {old, new}. A digest seen more than once on either side is +// ambiguous and left unpaired. +func Renames(changes []Change) [][2]string { + unique := func(pick func(Change) (*Entry, *Entry)) map[[32]byte]*string { + out := map[[32]byte]*string{} + for _, c := range changes { + entry, other := pick(c) + if entry == nil || other != nil || entry.Digest == nil { + continue + } + if _, dup := out[*entry.Digest]; dup { + out[*entry.Digest] = nil + } else { + path := c.Path + out[*entry.Digest] = &path + } + } + return out + } + deleted := unique(func(c Change) (*Entry, *Entry) { return c.Before, c.After }) + added := unique(func(c Change) (*Entry, *Entry) { return c.After, c.Before }) + var pairs [][2]string + for d, old := range deleted { + if new, ok := added[d]; ok && old != nil && new != nil { + pairs = append(pairs, [2]string{*old, *new}) + } + } + sort.Slice(pairs, func(i, j int) bool { return pairs[i][0] < pairs[j][0] }) + return pairs } diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index 4044206b..9df57515 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -587,17 +587,44 @@ func readResult(r *C.sandlock_result_t) *Result { res.Stderr = readBytes(r, false) count := int(C.sandlock_result_changes_len(r)) for i := 0; i < count; i++ { - kind := byte(C.sandlock_result_change_kind(r, C.uintptr_t(i))) var path string if pc := C.sandlock_result_change_path(r, C.uintptr_t(i)); pc != nil { path = C.GoString(pc) C.sandlock_string_free(pc) } - res.Changes = append(res.Changes, Change{Kind: ChangeKind(kind), Path: path}) + res.Changes = append(res.Changes, Change{ + Path: path, + Before: readChangeSide(r, i, 0), + After: readChangeSide(r, i, 1), + }) } return res } +func readChangeSide(r *C.sandlock_result_t, i int, side int) *Entry { + var raw C.sandlock_entry_t + if C.sandlock_result_change_entry(r, C.uintptr_t(i), C.int(side), &raw) != 0 { + return nil + } + e := &Entry{ + Kind: EntryKind(raw.kind), + Mode: os.FileMode(raw.mode), + Size: int64(raw.size), + } + if raw.has_digest != 0 { + var d [32]byte + for j := range d { + d[j] = byte(raw.digest[j]) + } + e.Digest = &d + } + if pc := C.sandlock_result_change_target(r, C.uintptr_t(i), C.int(side)); pc != nil { + e.Target = C.GoString(pc) + C.sandlock_string_free(pc) + } + return e +} + func readBytes(r *C.sandlock_result_t, stdout bool) []byte { var n C.uintptr_t var p *C.uint8_t diff --git a/go/sandlock_linux_test.go b/go/sandlock_linux_test.go index b5cf8cab..2cd71885 100644 --- a/go/sandlock_linux_test.go +++ b/go/sandlock_linux_test.go @@ -247,9 +247,68 @@ func TestAbortReportsChangesAndWritesNothing(t *testing.T) { if _, statErr := os.Stat(dir + "/out.txt"); statErr == nil { t.Fatalf("an aborting run leaked a write to the host") } - want := sandlock.Change{Kind: sandlock.ChangeAdded, Path: "out.txt"} - if len(res.Changes) != 1 || res.Changes[0] != want { - t.Fatalf("changes = %+v, want [%+v]", res.Changes, want) + if len(res.Changes) != 1 || res.Changes[0].Kind() != sandlock.ChangeAdded || res.Changes[0].Path != "out.txt" { + t.Fatalf("changes = %+v, want [A out.txt]", res.Changes) + } +} + +const sha256ABC = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + +func TestChangesCarryBothSides(t *testing.T) { + requireLandlock(t) + dir := t.TempDir() + if err := os.WriteFile(dir+"/mod.txt", []byte("abc"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dir+"/old.txt", []byte("abc"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink("a", dir+"/link"); err != nil { + t.Fatal(err) + } + sb := &sandlock.Sandbox{ + FSReadable: rootfs, + FSWritable: []string{dir}, + Workdir: dir, + OnExit: sandlock.BranchActionAbort, + } + res, err := sb.Run(context.Background(), "sh", "-c", + "cd "+dir+" && echo xyz > mod.txt && mv old.txt new.txt && rm link && ln -s b link") + if err != nil { + t.Fatalf("Run: %v", err) + } + if !res.Success { + t.Fatalf("run failed: exit=%d stderr=%q", res.ExitCode, res.Stderr) + } + byPath := map[string]sandlock.Change{} + for _, c := range res.Changes { + byPath[c.Path] = c + } + + mod := byPath["mod.txt"] + if mod.Kind() != sandlock.ChangeModified || mod.Before == nil || mod.After == nil { + t.Fatalf("mod.txt = %+v", mod) + } + if mod.Before.Kind != sandlock.EntryFile || mod.Before.Size != 3 || mod.Before.Digest == nil { + t.Fatalf("mod.txt before = %+v", mod.Before) + } + if got := fmt.Sprintf("%x", *mod.Before.Digest); got != sha256ABC { + t.Fatalf("mod.txt before digest = %s", got) + } + if mod.After.Size != 4 || *mod.After.Digest == *mod.Before.Digest || mod.ContentUnchanged() { + t.Fatalf("mod.txt after = %+v", mod.After) + } + + link := byPath["link"] + if link.Before == nil || link.Before.Kind != sandlock.EntrySymlink || link.Before.Target != "a" || link.After.Target != "b" { + t.Fatalf("link = %+v", link) + } + if link.TypeChanged() || link.ContentUnchanged() { + t.Fatalf("a retargeted link keeps its kind and changes content: %+v", link) + } + + if got := sandlock.Renames(res.Changes); len(got) != 1 || got[0] != [2]string{"old.txt", "new.txt"} { + t.Fatalf("renames = %v", got) } } From 05292e81ab636755ba2410b7353500b7d6d77323 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 10 Sep 2026 14:06:02 -0700 Subject: [PATCH 08/12] docs: describe a change by its before and after entries Signed-off-by: Cong Wang --- docs/sandbox-reference.md | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index f98d6964..40fca4a1 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -491,14 +491,33 @@ class BranchAction(Enum): ## Result types ```python +@dataclass(frozen=True) +class Entry: + kind: str # "file", "dir", "symlink", or "other" (fifo, socket). + mode: int # Permission bits. + size: int # Byte length for a file; 0 otherwise. + digest: bytes | None # SHA-256 of the bytes; files only. + target: str | None # Link target; symlinks only. + @dataclass(frozen=True) class Change: - kind: str # "A" = added, "M" = modified, "D" = deleted. - path: str # Path relative to workdir. + path: str # Path relative to workdir. + before: Entry | None # The workdir entry when the run first touched the path. + after: Entry | None # The branch entry when the change set was read. + + kind: str # Derived: "A" (no before), "M" (both sides), "D" (no after). + content_unchanged: bool # Both sides present with the same kind and digest or target. + type_changed: bool # Both sides present with different kinds. + +def renames(changes: list[Change]) -> list[tuple[str, str]]: ... ``` Every `Result` carries `changes: list[Change]`, read from the COW branch -before the branch action is applied. Empty without a `workdir`. +before the branch action is applied. Empty without a `workdir`. `before` is +recorded when the run first touches a path, so a file that appears in the +workdir afterwards does not turn an addition into an overwrite. A `before` +of `None` on a deletion means the run removed an entry it could not inspect. +`renames` pairs each deleted file with the added file of equal digest. ## Helpers From dcf898be934f83939e00e1190abd954701ec33af Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 10:19:18 -0700 Subject: [PATCH 09/12] cow: classify device nodes as entries of kind Other lstat_entry_in_root mapped char and block devices to "absent", the same answer as ENOENT. A device node in the upper was then reported as a deletion of a path that still exists, and one the run removed from the workdir lost its before side. Other exists for entries that carry no bytes, so devices belong there alongside fifos and sockets. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/seccomp.rs | 13 ++++++++++++- crates/sandlock-core/src/result.rs | 2 +- docs/sandbox-reference.md | 2 +- go/sandbox.go | 2 +- python/src/sandlock/sandbox.py | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index aa279a33..9f8f7b83 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -736,7 +736,7 @@ pub(crate) fn lstat_entry_in_root(root: &Path, rel: &str) -> Result EntryKind::File, libc::S_IFDIR => EntryKind::Dir, libc::S_IFLNK => EntryKind::Symlink, - libc::S_IFIFO | libc::S_IFSOCK => EntryKind::Other, + libc::S_IFIFO | libc::S_IFSOCK | libc::S_IFCHR | libc::S_IFBLK => EntryKind::Other, _ => return Ok(None), }; let target = (kind == EntryKind::Symlink) @@ -6216,6 +6216,17 @@ mod tests { assert!(!c.content_unchanged()); } + /// A device node is an entry like any other; mapping it to "absent" + /// would report it as deleted while it still exists. + #[test] + fn a_device_node_is_an_entry_of_kind_other() { + use crate::result::EntryKind; + let e = lstat_entry_in_root(Path::new("/dev"), "null").unwrap().unwrap(); + assert_eq!(e.kind, EntryKind::Other); + assert_eq!(e.size, 0); + assert!(e.digest.is_none()); + } + #[test] fn a_deleted_file_reports_the_digest_it_had() { let workdir = tempfile::tempdir().unwrap(); diff --git a/crates/sandlock-core/src/result.rs b/crates/sandlock-core/src/result.rs index 996482d2..73065711 100644 --- a/crates/sandlock-core/src/result.rs +++ b/crates/sandlock-core/src/result.rs @@ -80,7 +80,7 @@ pub enum EntryKind { File, Dir, Symlink, - /// A fifo or socket: no bytes, only a mode. + /// A fifo, socket, or device node: no bytes, only a mode. Other, } diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index 40fca4a1..fb38124c 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -493,7 +493,7 @@ class BranchAction(Enum): ```python @dataclass(frozen=True) class Entry: - kind: str # "file", "dir", "symlink", or "other" (fifo, socket). + kind: str # "file", "dir", "symlink", or "other" (fifo, socket, device node). mode: int # Permission bits. size: int # Byte length for a file; 0 otherwise. digest: bytes | None # SHA-256 of the bytes; files only. diff --git a/go/sandbox.go b/go/sandbox.go index 4db0cc8b..1896ceca 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -351,7 +351,7 @@ const ( EntryFile EntryKind = iota EntryDir EntrySymlink - EntryOther // fifo or socket: no bytes, only a mode + EntryOther // fifo, socket, or device node: no bytes, only a mode ) // Entry is one side of a Change. diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index fd5e2829..decf454e 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -121,7 +121,7 @@ class Entry: """One side of a :class:`Change`.""" kind: str - """``"file"``, ``"dir"``, ``"symlink"``, or ``"other"`` (fifo, socket).""" + """``"file"``, ``"dir"``, ``"symlink"``, or ``"other"`` (fifo, socket, device node).""" mode: int """Permission bits.""" From 89a527b2631a47163faf4779ce403a2c1207d7e2 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 10:20:44 -0700 Subject: [PATCH 10/12] go: translate st_mode bits into os.FileMode The entry mode is st_mode & 0o7777, but os.FileMode keeps setuid, setgid, and sticky in its own high bits. Casting the raw value put those three bits where FileMode never looks, so Perm() silently dropped a setuid file to 0755. Signed-off-by: Cong Wang --- go/sandlock_linux.go | 18 +++++++++++++++++- go/sandlock_linux_test.go | 12 +++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index 9df57515..ff7f4563 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -608,7 +608,7 @@ func readChangeSide(r *C.sandlock_result_t, i int, side int) *Entry { } e := &Entry{ Kind: EntryKind(raw.kind), - Mode: os.FileMode(raw.mode), + Mode: fileMode(uint32(raw.mode)), Size: int64(raw.size), } if raw.has_digest != 0 { @@ -625,6 +625,22 @@ func readChangeSide(r *C.sandlock_result_t, i int, side int) *Entry { return e } +// os.FileMode keeps setuid, setgid, and sticky in its own high bits, so a +// raw st_mode cast would drop them into bits FileMode never reads. +func fileMode(raw uint32) os.FileMode { + m := os.FileMode(raw & 0o777) + if raw&syscall.S_ISUID != 0 { + m |= os.ModeSetuid + } + if raw&syscall.S_ISGID != 0 { + m |= os.ModeSetgid + } + if raw&syscall.S_ISVTX != 0 { + m |= os.ModeSticky + } + return m +} + func readBytes(r *C.sandlock_result_t, stdout bool) []byte { var n C.uintptr_t var p *C.uint8_t diff --git a/go/sandlock_linux_test.go b/go/sandlock_linux_test.go index 2cd71885..1cb5eebc 100644 --- a/go/sandlock_linux_test.go +++ b/go/sandlock_linux_test.go @@ -266,6 +266,12 @@ func TestChangesCarryBothSides(t *testing.T) { if err := os.Symlink("a", dir+"/link"); err != nil { t.Fatal(err) } + if err := os.WriteFile(dir+"/suid.txt", []byte("suid"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(dir+"/suid.txt", os.ModeSetuid|0o755); err != nil { + t.Fatal(err) + } sb := &sandlock.Sandbox{ FSReadable: rootfs, FSWritable: []string{dir}, @@ -273,7 +279,7 @@ func TestChangesCarryBothSides(t *testing.T) { OnExit: sandlock.BranchActionAbort, } res, err := sb.Run(context.Background(), "sh", "-c", - "cd "+dir+" && echo xyz > mod.txt && mv old.txt new.txt && rm link && ln -s b link") + "cd "+dir+" && echo xyz > mod.txt && mv old.txt new.txt && rm link && ln -s b link && rm suid.txt") if err != nil { t.Fatalf("Run: %v", err) } @@ -307,6 +313,10 @@ func TestChangesCarryBothSides(t *testing.T) { t.Fatalf("a retargeted link keeps its kind and changes content: %+v", link) } + if suid := byPath["suid.txt"]; suid.Before == nil || suid.Before.Mode != os.ModeSetuid|0o755 { + t.Fatalf("suid.txt before = %+v", suid.Before) + } + if got := sandlock.Renames(res.Changes); len(got) != 1 || got[0] != [2]string{"old.txt", "new.txt"} { t.Fatalf("renames = %v", got) } From 3740965d74e66a70184154444627d232b93682e5 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 10:22:24 -0700 Subject: [PATCH 11/12] ffi: name the entry kinds and change sides in the header Every other discriminant the header exposes is a named enum, but the entry kind and the side selector were bare integers documented only in a comment, so each binding mirrored them by position. sandlock_entry_kind and SANDLOCK_CHANGE_BEFORE/AFTER give C callers and cbindgen consumers the names, and widen kind to the uint32 the sibling enums use. Signed-off-by: Cong Wang --- crates/sandlock-ffi/cbindgen.toml | 1 + crates/sandlock-ffi/include/sandlock.h | 41 ++++++++++++++++++++++---- crates/sandlock-ffi/src/lib.rs | 40 ++++++++++++++++++------- crates/sandlock-ffi/tests/changes.rs | 20 ++++++------- go/sandbox.go | 4 +-- go/sandlock_linux.go | 4 +-- python/src/sandlock/_sdk.py | 9 +++--- 7 files changed, 85 insertions(+), 34 deletions(-) diff --git a/crates/sandlock-ffi/cbindgen.toml b/crates/sandlock-ffi/cbindgen.toml index 042a958a..679af7ed 100644 --- a/crates/sandlock-ffi/cbindgen.toml +++ b/crates/sandlock-ffi/cbindgen.toml @@ -74,6 +74,7 @@ exclude = [ "sandlock_exception_policy_t" = "sandlock_exception" "sandlock_action_kind_t" = "sandlock_action" "sandlock_exit_reason_t" = "sandlock_exit_reason" +"sandlock_entry_kind_t" = "sandlock_entry_kind" [enum] rename_variants = "ScreamingSnakeCase" diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index d8d55e2e..6cbe69f2 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -22,6 +22,17 @@ typedef struct sandlock_result_t sandlock_result_t; typedef struct sandlock_handler_t sandlock_handler_t; +/** + * `side` of `sandlock_result_change_entry` and `sandlock_result_change_target`: + * the workdir entry when the run first touched the path. + */ +#define SANDLOCK_CHANGE_BEFORE 0 + +/** + * The branch entry when the change set was read. + */ +#define SANDLOCK_CHANGE_AFTER 1 + /** * `flags` bit for [`sandlock_action_set_inject_bytes`]: leave the injected * memfd writable (do not seal). Default (bit clear) seals it read-only. @@ -67,6 +78,26 @@ enum sandlock_exit_reason typedef uint32_t sandlock_exit_reason; #endif // __cplusplus +/** + * What one side of a change is (`sandlock_entry_t.kind`). + */ +enum sandlock_entry_kind +#ifdef __cplusplus + : uint32_t +#endif // __cplusplus + { + SANDLOCK_ENTRY_KIND_FILE = 0, + SANDLOCK_ENTRY_KIND_DIR = 1, + SANDLOCK_ENTRY_KIND_SYMLINK = 2, + /** + * A fifo, socket, or device node: no bytes, only a mode. + */ + SANDLOCK_ENTRY_KIND_OTHER = 3, +}; +#ifndef __cplusplus +typedef uint32_t sandlock_entry_kind; +#endif // __cplusplus + /** * Tag distinguishing payload variants of `sandlock_action_out_t`. */ @@ -165,11 +196,11 @@ typedef struct sandlock_pipeline_t sandlock_pipeline_t; /** * One side of a change, as plain data so every binding can hold it on - * the stack. `kind`: 0 file, 1 dir, 2 symlink, 3 other. `digest` is - * SHA-256 and only meaningful when `has_digest` is 1 (files). + * the stack. `digest` is SHA-256 and only meaningful when `has_digest` + * is 1 (files). */ typedef struct { - uint8_t kind; + sandlock_entry_kind kind; uint32_t mode; uint64_t size; uint8_t has_digest; @@ -1009,8 +1040,8 @@ char sandlock_result_change_kind(const sandlock_result_t *r, uintptr_t i); char *sandlock_result_change_path(const sandlock_result_t *r, uintptr_t i); /** - * Fill `out` with one side of the i-th change: `side` 0 is before the run - * touched the path, 1 is after. Returns 0 when filled, 1 when that side is + * Fill `out` with one side of the i-th change, `SANDLOCK_CHANGE_BEFORE` or + * `SANDLOCK_CHANGE_AFTER`. Returns 0 when filled, 1 when that side is * absent (`out` untouched), -1 when `i` or `side` is out of range. * * # Safety diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index 1862c0a4..51ba6e0d 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1790,14 +1790,32 @@ pub unsafe extern "C" fn sandlock_result_change_path(r: *const sandlock_result_t } } +/// What one side of a change is (`sandlock_entry_t.kind`). +#[allow(non_camel_case_types)] +#[repr(u32)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum sandlock_entry_kind_t { + File = 0, + Dir = 1, + Symlink = 2, + /// A fifo, socket, or device node: no bytes, only a mode. + Other = 3, +} + +/// `side` of `sandlock_result_change_entry` and `sandlock_result_change_target`: +/// the workdir entry when the run first touched the path. +pub const SANDLOCK_CHANGE_BEFORE: c_int = 0; +/// The branch entry when the change set was read. +pub const SANDLOCK_CHANGE_AFTER: c_int = 1; + /// One side of a change, as plain data so every binding can hold it on -/// the stack. `kind`: 0 file, 1 dir, 2 symlink, 3 other. `digest` is -/// SHA-256 and only meaningful when `has_digest` is 1 (files). +/// the stack. `digest` is SHA-256 and only meaningful when `has_digest` +/// is 1 (files). #[repr(C)] #[allow(non_camel_case_types)] #[derive(Clone, Copy)] pub struct sandlock_entry_t { - pub kind: u8, + pub kind: sandlock_entry_kind_t, pub mode: u32, pub size: u64, pub has_digest: u8, @@ -1811,14 +1829,14 @@ unsafe fn change_side<'a>(r: *const sandlock_result_t, i: usize, side: c_int) -> let changes = &(*r)._private.changes; let change = changes.get(i)?; match side { - 0 => Some(&change.before), - 1 => Some(&change.after), + SANDLOCK_CHANGE_BEFORE => Some(&change.before), + SANDLOCK_CHANGE_AFTER => Some(&change.after), _ => None, } } -/// Fill `out` with one side of the i-th change: `side` 0 is before the run -/// touched the path, 1 is after. Returns 0 when filled, 1 when that side is +/// Fill `out` with one side of the i-th change, `SANDLOCK_CHANGE_BEFORE` or +/// `SANDLOCK_CHANGE_AFTER`. Returns 0 when filled, 1 when that side is /// absent (`out` untouched), -1 when `i` or `side` is out of range. /// /// # Safety @@ -1838,10 +1856,10 @@ pub unsafe extern "C" fn sandlock_result_change_entry( } *out = sandlock_entry_t { kind: match e.kind { - EntryKind::File => 0, - EntryKind::Dir => 1, - EntryKind::Symlink => 2, - EntryKind::Other => 3, + EntryKind::File => sandlock_entry_kind_t::File, + EntryKind::Dir => sandlock_entry_kind_t::Dir, + EntryKind::Symlink => sandlock_entry_kind_t::Symlink, + EntryKind::Other => sandlock_entry_kind_t::Other, }, mode: e.mode, size: e.size, diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs index 9b1f6064..109bf2fc 100644 --- a/crates/sandlock-ffi/tests/changes.rs +++ b/crates/sandlock-ffi/tests/changes.rs @@ -7,21 +7,21 @@ use std::path::Path; use std::ptr; use sandlock_ffi::{ - sandlock_create_for_run, sandlock_entry_t, sandlock_handle_free, sandlock_handle_wait, + sandlock_create_for_run, sandlock_entry_kind_t, sandlock_entry_t, sandlock_handle_free, + sandlock_handle_wait, sandlock_result_change_entry, sandlock_result_change_kind, sandlock_result_change_path, sandlock_result_change_target, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, sandlock_sandbox_builder_fs_write, sandlock_sandbox_builder_new, sandlock_sandbox_builder_on_exit, sandlock_sandbox_builder_workdir, sandlock_sandbox_free, - sandlock_sandbox_t, sandlock_start, sandlock_string_free, + sandlock_sandbox_t, sandlock_start, sandlock_string_free, SANDLOCK_CHANGE_AFTER, + SANDLOCK_CHANGE_BEFORE, }; const ABORT: u8 = 1; -const BEFORE: c_int = 0; -const AFTER: c_int = 1; -const ENTRY_FILE: u8 = 0; -const ENTRY_SYMLINK: u8 = 2; +const BEFORE: c_int = SANDLOCK_CHANGE_BEFORE; +const AFTER: c_int = SANDLOCK_CHANGE_AFTER; const SHA256_ABC: &str = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"; fn build_policy(workdir: &Path, storage: &Path) -> *mut sandlock_sandbox_t { @@ -57,7 +57,7 @@ fn hex(d: &[u8; 32]) -> String { } fn entry(r: *const sandlock_ffi::sandlock_result_t, i: usize, side: c_int) -> Option { - let mut out = sandlock_entry_t { kind: 0, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; + let mut out = sandlock_entry_t { kind: sandlock_entry_kind_t::File, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; match unsafe { sandlock_result_change_entry(r, i, side, &mut out) } { 0 => Some(out), 1 => None, @@ -108,7 +108,7 @@ fn both_sides_of_every_change_are_readable() { assert_eq!(unsafe { sandlock_result_change_kind(r, i) } as u8, b'M'); let before = entry(r, i, BEFORE).unwrap(); let after = entry(r, i, AFTER).unwrap(); - assert_eq!(before.kind, ENTRY_FILE); + assert_eq!(before.kind, sandlock_entry_kind_t::File); assert_eq!(before.size, 3); assert_eq!(before.has_digest, 1); assert_eq!(hex(&before.digest), SHA256_ABC); @@ -124,12 +124,12 @@ fn both_sides_of_every_change_are_readable() { assert!(entry(r, i, AFTER).is_none()); let i = by_path["link"]; - assert_eq!(entry(r, i, BEFORE).unwrap().kind, ENTRY_SYMLINK); + assert_eq!(entry(r, i, BEFORE).unwrap().kind, sandlock_entry_kind_t::Symlink); assert_eq!(target(r, i, BEFORE).as_deref(), Some("a")); assert_eq!(target(r, i, AFTER).as_deref(), Some("b")); assert!(target(r, by_path["mod.txt"], AFTER).is_none()); - let mut out = sandlock_entry_t { kind: 0, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; + let mut out = sandlock_entry_t { kind: sandlock_entry_kind_t::File, mode: 0, size: 0, has_digest: 0, digest: [0; 32] }; assert_eq!(unsafe { sandlock_result_change_entry(r, n, BEFORE, &mut out) }, -1); assert_eq!(unsafe { sandlock_result_change_entry(r, 0, 2, &mut out) }, -1); diff --git a/go/sandbox.go b/go/sandbox.go index 1896ceca..f82c6f0f 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -344,8 +344,8 @@ const ( ChangeDeleted ChangeKind = 'D' ) -// EntryKind classifies one side of a Change. -type EntryKind uint8 +// EntryKind classifies one side of a Change; values mirror sandlock_entry_kind. +type EntryKind uint32 const ( EntryFile EntryKind = iota diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index ff7f4563..626282e2 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -594,8 +594,8 @@ func readResult(r *C.sandlock_result_t) *Result { } res.Changes = append(res.Changes, Change{ Path: path, - Before: readChangeSide(r, i, 0), - After: readChangeSide(r, i, 1), + Before: readChangeSide(r, i, C.SANDLOCK_CHANGE_BEFORE), + After: readChangeSide(r, i, C.SANDLOCK_CHANGE_AFTER), }) } return res diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index 2dc3d371..deb41170 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -379,7 +379,7 @@ def confine(policy: "PolicyDataclass") -> None: class _CEntry(ctypes.Structure): _fields_ = [ - ("kind", ctypes.c_uint8), + ("kind", ctypes.c_uint32), ("mode", ctypes.c_uint32), ("size", ctypes.c_uint64), ("has_digest", ctypes.c_uint8), @@ -758,7 +758,8 @@ def _read_result_bytes(result_p, fn) -> bytes: return ctypes.string_at(ptr, length.value) -_ENTRY_KINDS = ("file", "dir", "symlink", "other") +_ENTRY_KINDS = ("file", "dir", "symlink", "other") # sandlock_entry_kind order +_CHANGE_BEFORE, _CHANGE_AFTER = 0, 1 def _take_string(p) -> str | None: @@ -789,8 +790,8 @@ def _read_result_changes(result_p) -> list: path = _take_string(_lib.sandlock_result_change_path(result_p, i)) or "" changes.append(Change( path=path, - before=_read_change_side(result_p, i, 0), - after=_read_change_side(result_p, i, 1), + before=_read_change_side(result_p, i, _CHANGE_BEFORE), + after=_read_change_side(result_p, i, _CHANGE_AFTER), )) return changes From 194d420d769efa09d36bef32b564553ab11f601b Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 10:25:16 -0700 Subject: [PATCH 12/12] ffi: drop sandlock_result_change_kind The kind of a change is now derived from which sides are present, and both bindings read the sides directly, so the character accessor had no callers left. C callers get the same answer from sandlock_result_change_entry's return value on each side. Signed-off-by: Cong Wang --- crates/sandlock-ffi/include/sandlock.h | 8 -------- crates/sandlock-ffi/src/lib.rs | 18 ------------------ crates/sandlock-ffi/tests/changes.rs | 3 +-- crates/sandlock-ffi/tests/defer.rs | 8 ++++++-- python/src/sandlock/_sdk.py | 3 --- 5 files changed, 7 insertions(+), 33 deletions(-) diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 6cbe69f2..73ad04f5 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -1022,14 +1022,6 @@ const uint8_t *sandlock_result_stderr_bytes(const sandlock_result_t *r, uintptr_ */ uintptr_t sandlock_result_changes_len(const sandlock_result_t *r); -/** - * Kind of the i-th change: 'A' (added), 'M' (modified), 'D' (deleted); 0 out of range. - * - * # Safety - * `r` must be a valid result pointer. - */ -char sandlock_result_change_kind(const sandlock_result_t *r, uintptr_t i); - /** * Workdir-relative path of the i-th change. Caller must free with * `sandlock_string_free`; NULL out of range. diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index 51ba6e0d..121dc5ed 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1755,24 +1755,6 @@ pub unsafe extern "C" fn sandlock_result_changes_len(r: *const sandlock_result_t (*r)._private.changes.len() } -/// Kind of the i-th change: 'A' (added), 'M' (modified), 'D' (deleted); 0 out of range. -/// -/// # Safety -/// `r` must be a valid result pointer. -#[no_mangle] -pub unsafe extern "C" fn sandlock_result_change_kind(r: *const sandlock_result_t, i: usize) -> c_char { - if r.is_null() { - return 0; - } - let changes = &(*r)._private.changes; - match changes.get(i).map(|c| c.kind()) { - Some(sandlock_core::ChangeKind::Added) => b'A' as c_char, - Some(sandlock_core::ChangeKind::Modified) => b'M' as c_char, - Some(sandlock_core::ChangeKind::Deleted) => b'D' as c_char, - None => 0, - } -} - /// Workdir-relative path of the i-th change. Caller must free with /// `sandlock_string_free`; NULL out of range. /// diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs index 109bf2fc..e8c798d8 100644 --- a/crates/sandlock-ffi/tests/changes.rs +++ b/crates/sandlock-ffi/tests/changes.rs @@ -9,7 +9,7 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_entry_kind_t, sandlock_entry_t, sandlock_handle_free, sandlock_handle_wait, - sandlock_result_change_entry, sandlock_result_change_kind, sandlock_result_change_path, + sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_change_target, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, @@ -105,7 +105,6 @@ fn both_sides_of_every_change_are_readable() { assert_eq!(by_path.keys().collect::>(), vec!["added.txt", "gone.txt", "link", "mod.txt"]); let i = by_path["mod.txt"]; - assert_eq!(unsafe { sandlock_result_change_kind(r, i) } as u8, b'M'); let before = entry(r, i, BEFORE).unwrap(); let after = entry(r, i, AFTER).unwrap(); assert_eq!(before.kind, sandlock_entry_kind_t::File); diff --git a/crates/sandlock-ffi/tests/defer.rs b/crates/sandlock-ffi/tests/defer.rs index c8ac5c9b..f33e0030 100644 --- a/crates/sandlock-ffi/tests/defer.rs +++ b/crates/sandlock-ffi/tests/defer.rs @@ -9,7 +9,7 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_handle_abort, sandlock_handle_commit, sandlock_handle_free, sandlock_handle_pending, sandlock_handle_upper_dir, sandlock_handle_wait, - sandlock_result_change_kind, sandlock_result_change_path, sandlock_result_changes_len, + sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, sandlock_sandbox_builder_fs_write, @@ -60,7 +60,11 @@ fn run_deferred(workdir: &Path, storage: &Path) -> *mut sandlock_ffi::sandlock_h assert!(unsafe { sandlock_result_success(r) }); assert_eq!(unsafe { sandlock_result_changes_len(r) }, 1); - assert_eq!(unsafe { sandlock_result_change_kind(r, 0) } as u8, b'A'); + let mut out = sandlock_ffi::sandlock_entry_t { + kind: sandlock_ffi::sandlock_entry_kind_t::File, mode: 0, size: 0, has_digest: 0, digest: [0; 32], + }; + assert_eq!(unsafe { sandlock_result_change_entry(r, 0, sandlock_ffi::SANDLOCK_CHANGE_BEFORE, &mut out) }, 1); + assert_eq!(unsafe { sandlock_result_change_entry(r, 0, sandlock_ffi::SANDLOCK_CHANGE_AFTER, &mut out) }, 0); let p = unsafe { sandlock_result_change_path(r, 0) }; assert_eq!(unsafe { CStr::from_ptr(p) }.to_str().unwrap(), "out.txt"); unsafe { sandlock_string_free(p) }; diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index deb41170..d0df05ac 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -370,9 +370,6 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_result_changes_len.restype = ctypes.c_size_t _lib.sandlock_result_changes_len.argtypes = [_c_result_p] -_lib.sandlock_result_change_kind.restype = ctypes.c_char -_lib.sandlock_result_change_kind.argtypes = [_c_result_p, ctypes.c_size_t] - _lib.sandlock_result_change_path.restype = ctypes.c_void_p _lib.sandlock_result_change_path.argtypes = [_c_result_p, ctypes.c_size_t]