From 01917c1c652ec588a9f0749cc892a7276f66552b Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:53:45 +0200 Subject: [PATCH 1/9] feat(core): remove a directory on Unix and finish the leftovers of a crash (#445) Fresh re-cut of #1070 on the hardened main (1f37cd83), not a retarget: that PR was built on the history of the squash-merged #1069 and its code predates the quarantine sync, the identity checks and the fail-closed look that main has now. Its one own change is re-applied on that code, with its tests. test-support only, no caller, no new unsafe, no new package. Windows still answers Unsupported. - remove_empty_dir opens the child directory (no link followed), refuses one that has entries and moves it through the same quarantine directory; it is identified by the open directory and unlinked with AT_REMOVEDIR. A directory that fills after the check goes back (NotEmpty), through the step that syncs the quarantine directory like any put-back. - remove_leftover_file takes a .h1q-* directory by its generated name only (the prefix and exactly 32 lowercase hexadecimal digits, otherwise InvalidName before anything is opened), requires a directory of the effective user without access for group or others, authenticates the file in it again and unlinks it only if accepted. A file that does not authenticate and anything that is not a regular file (a link, a directory) stay and give Refused; an empty generated directory is removed. - On the hardened code the leftover removal syncs the quarantine directory it unlinks in and says Removed(NotConfirmed) if that fails; an entry that vanished before the unlink is reported like one that vanished before. Tests (own file): a directory swapped or filled after the check, an empty directory that wins the last window (Disturbed), a simulated crash and its recovery, the name grammar (short, long, upper case, not hexadecimal), a foreign object and a link left in the quarantine directory, a stranded entry (put-back blocked, proved to be there) finished by the recovery, and the failed sync of the held directory. The earlier permission-based stranded case is now Disturbed on main (a failed look proves nothing) and is not asserted here. Mutation-checked: no sync in the leftover removal, no refusal of a foreign entry, a loosened name grammar and an ignored authentication each fail a case. The PR number in the changelog entry is to be set to the number of this PR when it is opened. Issue #359. --- CHANGELOG.md | 2 + .../src/pinned/removal.rs | 7 +- .../src/pinned/tests/mod.rs | 2 + .../src/pinned/tests/remove.rs | 34 +++ .../src/pinned/tests/remove_unix.rs | 4 +- .../src/pinned/tests/remove_unix_dir.rs | 259 ++++++++++++++++++ .../src/pinned/unix/removal.rs | 111 ++++++-- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 34 ++- 8 files changed, 426 insertions(+), 27 deletions(-) create mode 100644 crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f6b928237..e899155ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal. No new `unsafe`, no new package. Issue #359. PR #1070. + - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. - **R-15 Gate 4:** removal of a file on Unix, `test-support` only (slice H1a-2, part 2 of 4). `PinnedDir::remove_authenticated_file` moves the entry into a quarantine directory with `RENAME_NOREPLACE`, identifies the open object there and only then unlinks it; anything else is put back, a failure after the move rolls back or reports `Stranded`, and the parent is synced. The one window no Unix primitive closes (a same-user actor in the quarantine directory after the last identification) is forced in a test and reported as `Disturbed`, detected after the fact and not prevented. The rustix `process` feature is added for `geteuid`; no new package, no new `unsafe`. Directories, leftovers and Windows follow. Issue #359. PR #1069. diff --git a/crates/worldscript-secure-storage/src/pinned/removal.rs b/crates/worldscript-secure-storage/src/pinned/removal.rs index a7a37536a..d83284d8e 100644 --- a/crates/worldscript-secure-storage/src/pinned/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/removal.rs @@ -4,9 +4,10 @@ //! **Availability.** Everything in this module exists only with the test-only `test-support` feature, //! has no caller, and is not part of any production build. Reclamation of a real store is not admitted. //! -//! **Status of this slice.** The API and its isolation. Every call answers [`PinnedError::Unsupported`] -//! until the platform implementations land in the successor slices (Unix files; Unix directories and -//! leftovers; Windows), so nothing is removed and nothing claims a safety it does not have yet. +//! **Status of this slice.** The API and its isolation, and on Unix the removal of files, of +//! directories and of the leftovers of a crash. A call on Windows answers [`PinnedError::Unsupported`] +//! until the Windows implementation lands in its own slice, so nothing is removed there and nothing +//! claims a safety it does not have yet. //! //! **The two designs the platform slices do not use.** Removing "the entry that was checked" with //! `unlinkat(parent, name)` after a check on the name is exposed to a swap of the last component: POSIX diff --git a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs index 7479f89d5..984b5f2b1 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs @@ -13,6 +13,8 @@ mod remove_unix; #[cfg(all(feature = "test-support", unix))] mod remove_unix_cleanup; #[cfg(all(feature = "test-support", unix))] +mod remove_unix_dir; +#[cfg(all(feature = "test-support", unix))] mod remove_unix_sync; #[cfg(windows)] mod windows_redirect; diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs index 03bfa29e7..75039ba93 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs @@ -147,6 +147,40 @@ fn removing_a_hard_linked_name_leaves_the_other_names() { ); } +#[cfg(unix)] +#[test] +fn an_empty_directory_is_removed_and_a_filled_one_or_a_link_is_not() { + let scratch = Scratch::new("dirs"); + let root = scratch.root(); + let aside = scratch.aside(); + fs::create_dir(root.join("empty")).unwrap(); + fs::create_dir(root.join("filled")).unwrap(); + fs::write(root.join("filled").join("inside"), b"inside").unwrap(); + fs::create_dir(aside.join("target")).unwrap(); + link_to_dir(&aside.join("target"), &root.join("link")); + let pinned = PinnedDir::open_root(&root).unwrap(); + assert_eq!( + ( + pinned.remove_empty_dir("empty"), + pinned.remove_empty_dir("filled"), + pinned.remove_empty_dir("link"), + pinned.remove_empty_dir("missing"), + fs::read(root.join("filled").join("inside")).unwrap(), + aside.join("target").is_dir(), + names(&pinned) + ), + ( + removed(), + Ok(RemovalOutcome::NotEmpty), + Err(PinnedError::Redirected), + Err(PinnedError::NotFound), + b"inside".to_vec(), + true, + vec!["filled".to_string(), "link".to_string()] + ) + ); +} + #[cfg(windows)] #[test] fn every_removal_refuses_until_windows_implements_it_and_removes_nothing() { diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs index 16bb21f68..f9a00299f 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs @@ -60,7 +60,7 @@ pub(super) fn at(stage: RemovalStage<'_>) -> (At, Option<&Path>) { } /// An observer that runs `action` at the stage `here` and otherwise does nothing. -fn on( +pub(super) fn on( here: At, mut action: impl FnMut(Option<&Path>), ) -> impl FnMut(RemovalStage<'_>) -> Result<(), PinnedError> { @@ -94,7 +94,7 @@ pub(super) fn swap_in_a_live_file(root: &Path, aside: &Path) { } /// The inode of the object at `path` (not following a link): who is who, whatever the content says. -fn ino(path: &Path) -> u64 { +pub(super) fn ino(path: &Path) -> u64 { fs::symlink_metadata(path).unwrap().ino() } diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs new file mode 100644 index 000000000..df932b0ea --- /dev/null +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs @@ -0,0 +1,259 @@ +//! Unix removal of a directory, and of what a crash or a refused put-back left in a quarantine +//! directory, under the swaps the design exists for: another actor replaces the directory, or the entry +//! inside the quarantine directory, between the steps of the removal. Each case forces the swap through +//! the observer at a chosen stage and pins what happens to every object. + +use std::fs; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use super::remove::{holds, names, removed}; +use super::remove_unix::{ + at, denied, fixture, held_in_quarantine, ino, on, quarantines, swap_in_a_live_file, At, Fixture, +}; +use crate::durable::DirectoryDurability::NotConfirmed; +use crate::pinned::unix::removal::sync_fault; +use crate::pinned::{PinnedError, RemovalOutcome, QUARANTINE_PREFIX}; + +#[test] +fn a_directory_swapped_or_filled_after_the_check_is_put_back_and_not_removed() { + let f = fixture("dir-swap"); + fs::create_dir(f.root.join("d")).unwrap(); + fs::create_dir(f.root.join("e")).unwrap(); + let swapped = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Checked, |_| { + fs::rename(f.root.join("d"), f.aside.join("original-d")).unwrap(); + fs::create_dir(f.root.join("d")).unwrap(); + fs::write(f.root.join("d").join("live"), b"live").unwrap(); + }), + ); + let filled = f.pinned.remove_empty_dir_observed( + "e", + &mut on(At::Checked, |_| { + fs::write(f.root.join("e").join("late"), b"late").unwrap() + }), + ); + assert_eq!( + ( + swapped, + fs::read(f.root.join("d").join("live")).unwrap(), + f.aside.join("original-d").is_dir(), + filled, + fs::read(f.root.join("e").join("late")).unwrap(), + names(&f.pinned) + ), + ( + Ok(RemovalOutcome::Swapped { restored: true }), + b"live".to_vec(), + true, + Ok(RemovalOutcome::NotEmpty), + b"late".to_vec(), + vec!["a".to_string(), "d".to_string(), "e".to_string()] + ) + ); +} + +#[test] +fn an_empty_directory_swapped_in_the_last_window_is_reported_as_disturbed() { + let f = fixture("window-empty-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let original = ino(&f.root.join("d")); + let outcome = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Identified, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("original-d")).unwrap(); + fs::create_dir(quarantined).unwrap(); + }), + ); + assert_eq!( + ( + outcome, + f.root.join("d").exists(), + ino(&f.aside.join("original-d")) == original, + quarantines(&f.root).len() + ), + (Ok(RemovalOutcome::Disturbed), false, true, 0) + ); +} + +/// A removal that "crashes" (the observer panics) once the object is in the quarantine directory. +fn crash_after_the_move(f: &Fixture) { + let crashed = catch_unwind(AssertUnwindSafe(|| { + let _ = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, |_| panic!("simulated crash")), + ); + })); + assert!(crashed.is_err()); +} + +#[test] +fn a_crash_leaves_the_object_in_quarantine_and_a_leftover_removal_finishes_it_after_authenticating() +{ + let f = fixture("crash"); + crash_after_the_move(&f); + let left = quarantines(&f.root); + let name = left[0].file_name().unwrap().to_owned(); + let before = (names(&f.pinned).len(), held_in_quarantine(&f.root)); + let refused = f + .pinned + .remove_leftover_file(&name, holds(b"something else")); + let finished = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (before, refused, finished, names(&f.pinned)), + ( + (1, Some(b"alpha".to_vec())), + Ok(RemovalOutcome::Refused), + removed(), + Vec::::new() + ) + ); +} + +#[test] +fn a_leftover_whose_quarantine_directory_could_not_be_synced_is_not_confirmed() { + // The leftover removal changes the quarantine directory it unlinks in, like any removal, and says + // `NotConfirmed` if that directory could not be synced (modelled through the seam of the removal). + let f = fixture("leftover-sync"); + crash_after_the_move(&f); + let name = quarantines(&f.root)[0].file_name().unwrap().to_owned(); + let (outcome, syncs) = + sync_fault::during(Some(PinnedError::Io(std::io::ErrorKind::Other)), || { + f.pinned.remove_leftover_file(&name, holds(b"alpha")) + }); + assert_eq!( + (outcome, syncs, names(&f.pinned), quarantines(&f.root).len()), + ( + Ok(RemovalOutcome::Removed(NotConfirmed)), + 1, + Vec::::new(), + 0 + ) + ); +} + +#[test] +fn a_leftover_is_taken_by_its_generated_name_only_and_an_empty_one_is_removed() { + use std::os::unix::fs::PermissionsExt; + let f = fixture("leftover-names"); + let generated = format!("{QUARANTINE_PREFIX}{}", "0".repeat(32)); + for name in [generated.as_str(), ".h1q-empty", "not-a-quarantine"] { + fs::create_dir(f.root.join(name)).unwrap(); + fs::set_permissions(f.root.join(name), fs::Permissions::from_mode(0o700)).unwrap(); + } + let taken = |name: &str| f.pinned.remove_leftover_file(name, |_| Ok(true)); + // Right prefix, wrong digits: too short, too long, upper case, not hexadecimal. Refused by the name + // alone, before anything is opened. + let wrong_digits = [ + "0".repeat(31), + "0".repeat(33), + "A".repeat(32), + "G".repeat(32), + ] + .map(|digits| taken(&format!("{QUARANTINE_PREFIX}{digits}"))); + assert_eq!( + ( + taken("a"), + taken("not-a-quarantine"), + taken(".h1q-empty"), + wrong_digits, + taken(&generated), + names(&f.pinned) + ), + ( + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + [Err(PinnedError::InvalidName); 4], + Ok(RemovalOutcome::Vanished), + vec![ + ".h1q-empty".to_string(), + "a".to_string(), + "not-a-quarantine".to_string() + ] + ) + ); +} + +#[test] +fn a_foreign_object_left_in_quarantine_is_never_removed_by_a_leftover_removal() { + let f = fixture("leftover-foreign"); + let mut at_checked = on(At::Checked, |_| swap_in_a_live_file(&f.root, &f.aside)); + let mut at_moved = on(At::MovedAside, |_| { + fs::write(f.root.join("a"), b"retaken").unwrap() + }); + let first = f + .pinned + .remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| { + at_checked(stage)?; + at_moved(stage) + }); + let name = quarantines(&f.root)[0].file_name().unwrap().to_owned(); + // The object in the quarantine directory is the live file, not an object of the sweep. + let refused = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (first, refused, held_in_quarantine(&f.root)), + ( + Ok(RemovalOutcome::Swapped { restored: false }), + Ok(RemovalOutcome::Refused), + Some(b"live".to_vec()) + ) + ); +} + +#[test] +fn a_stranded_entry_is_finished_by_the_leftover_removal_after_authenticating_it() { + // The put-back is blocked (the name is taken again) and the observer fails: the entry is proved to + // be in the quarantine directory, which is what `Stranded` says, and runs the same for root. + let f = fixture("stranded-recovery"); + let stranded = + f.pinned + .remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| { + if at(stage).0 == At::MovedAside { + fs::write(f.root.join("a"), b"taken").unwrap(); + return Err(denied()); + } + Ok(()) + }); + let name = quarantines(&f.root)[0].file_name().unwrap().to_owned(); + let refused = f + .pinned + .remove_leftover_file(&name, holds(b"something else")); + let finished = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (stranded, refused, finished, names(&f.pinned)), + ( + Ok(RemovalOutcome::Stranded { cause: denied() }), + Ok(RemovalOutcome::Refused), + removed(), + vec!["a".to_string()] + ) + ); +} + +#[test] +fn a_link_left_in_a_quarantine_directory_is_refused_and_not_followed() { + use std::os::unix::fs::PermissionsExt; + let f = fixture("leftover-link"); + let name = format!("{QUARANTINE_PREFIX}{}", "1".repeat(32)); + fs::create_dir(f.root.join(&name)).unwrap(); + fs::set_permissions(f.root.join(&name), fs::Permissions::from_mode(0o700)).unwrap(); + fs::write(f.aside.join("target"), b"target").unwrap(); + std::os::unix::fs::symlink(f.aside.join("target"), f.root.join(&name).join("x")).unwrap(); + // Whatever the authentication would say, an entry that is not a regular file is never handed to it. + let outcome = f.pinned.remove_leftover_file(&name, |_| Ok(true)); + assert_eq!( + ( + outcome, + fs::read_link(f.root.join(&name).join("x")).ok(), + fs::read(f.aside.join("target")).unwrap() + ), + ( + Ok(RemovalOutcome::Refused), + Some(f.aside.join("target")), + b"target".to_vec() + ) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 408ffac73..ea4889776 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -1,10 +1,10 @@ //! Unix removal, test-support only: move the entry into a quarantine directory, identify the open //! object there, and only then unlink it. See the parent `removal` module for the reasoning and for the -//! one window that no Unix primitive closes. This part removes files; directories and the leftover -//! recovery follow and refuse with `Unsupported` until then. +//! one window that no Unix primitive closes. use std::ffi::{OsStr, OsString}; use std::fs::File; +use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; @@ -27,6 +27,9 @@ use crate::random::{OsRandom, RandomSource}; /// The name an entry has inside the quarantine directory. pub(in crate::pinned) const INSIDE: &str = "x"; +/// How many hexadecimal digits follow the prefix in the name of a quarantine directory. +const SUFFIX_DIGITS: usize = 32; + /// A quarantine directory inside the pinned parent, held by descriptor. Whatever it holds is outside /// every name grammar of the store and is never removed by a sweep. struct Quarantine { @@ -52,6 +55,8 @@ enum Inside { /// How a removal ended before the parent directory was synced. enum Step { + /// Nothing changed (a leftover that was refused); the outcome is final and nothing is synced. + Unchanged(RemovalOutcome), /// The directory changed (a quarantine directory came and went, the name led nowhere, or tampering /// was found); the outcome is final. Changed(RemovalOutcome), @@ -68,6 +73,17 @@ enum Step { Stranded(PinnedError), } +/// Whether `name` is exactly what [`Quarantine::create`] produces. +fn is_generated_name(name: &OsStr) -> bool { + let bytes = name.as_bytes(); + let prefix = QUARANTINE_PREFIX.as_bytes(); + bytes.len() == prefix.len() + SUFFIX_DIGITS + && bytes.starts_with(prefix) + && bytes[prefix.len()..] + .iter() + .all(|digit| matches!(digit, b'0'..=b'9' | b'a'..=b'f')) +} + impl Quarantine { fn create(parent: &DirHandle) -> Result { let mut bits = [0u8; 16]; @@ -98,6 +114,19 @@ impl Quarantine { Ok(Quarantine { fd, name, identity }) } + /// An existing quarantine directory of `parent`, found by its generated name: a directory of this + /// process's user that no one else can enter. Its identity is the one seen now. + fn open_existing(parent: &DirHandle, name: &OsStr) -> Result { + if !is_generated_name(name) { + return Err(PinnedError::InvalidName); + } + let fd = Self::open_owned(parent, name)?; + if Self::mode_of(&fd)? & 0o077 != 0 { + return Err(PinnedError::Unsupported); + } + Self::held(fd, name.to_os_string()) + } + /// The directory just created, with the owner's access set explicitly (a restrictive umask would /// otherwise leave a directory that nothing can be moved into) and checked. fn made_usable(parent: &DirHandle, name: &OsStr) -> Result { @@ -341,6 +370,64 @@ pub(in crate::pinned) fn remove_file( remove_moved(dir, target, observer) } +pub(in crate::pinned) fn remove_dir( + dir: &PinnedDir, + name: &OsStr, + observer: Observer<'_>, +) -> Result { + let child = dir.open_dir(name)?; + match child.entries(0) { + Ok(_) => {} + Err(PinnedError::TooManyEntries) => return Ok(RemovalOutcome::NotEmpty), + Err(error) => return Err(error), + } + observer(RemovalStage::Checked)?; + let target = Target { + name, + expected: child.identity(), + kind: EntryKind::Directory, + }; + remove_moved(dir, target, observer) +} + +/// Removes the leftover file of the quarantine directory `quarantine` after authenticating it again. +pub(in crate::pinned) fn remove_leftover( + dir: &PinnedDir, + quarantine: &OsStr, + authenticate: impl FnOnce(&mut PinnedFile) -> Result, + observer: Observer<'_>, +) -> Result { + let held = Quarantine::open_existing(&dir.handle, quarantine)?; + let step = leftover_step(&held, dir, authenticate, observer); + held.discard(&dir.handle); + settle(dir, step?, observer) +} + +/// Looks at what a crash or a refused put-back left in `held`, authenticates it again and unlinks it +/// like any other removal: the quarantine directory is synced for what the unlink changed in it. +fn leftover_step( + held: &Quarantine, + dir: &PinnedDir, + authenticate: impl FnOnce(&mut PinnedFile) -> Result, + observer: Observer<'_>, +) -> Result { + let (file, identity) = match held.inspect(EntryKind::File)? { + // An empty quarantine directory is junk and goes with `discard`. + Inside::Absent => return Ok(Step::Changed(RemovalOutcome::Vanished)), + Inside::Foreign(_) => return Ok(Step::Unchanged(RemovalOutcome::Refused)), + Inside::Object(file, identity) => (file, identity), + }; + let mut pinned = PinnedFile { file, identity }; + if !authenticate(&mut pinned)? { + return Ok(Step::Unchanged(RemovalOutcome::Refused)); + } + observer(RemovalStage::Identified { + quarantined: &held.path_inside(&dir.path), + })?; + let outcome = unlink_verified(held, &pinned.file, EntryKind::File)?; + Ok(unlinked(outcome, held.sync())) +} + /// Removes the entry of `target` from `dir` if, once moved aside, it is the object it must be. fn remove_moved( dir: &PinnedDir, @@ -482,6 +569,7 @@ fn settle( observer: Observer<'_>, ) -> Result { match step { + Step::Unchanged(outcome) => Ok(outcome), Step::RolledBack(cause) => { sync_after_failure(dir, observer); Err(cause) @@ -507,25 +595,6 @@ fn settle( } } -/// Removing a directory follows in the next part. -pub(in crate::pinned) fn remove_dir( - _dir: &PinnedDir, - _name: &OsStr, - _observer: Observer<'_>, -) -> Result { - Err(PinnedError::Unsupported) -} - -/// Finishing a leftover quarantine directory follows in the next part. -pub(in crate::pinned) fn remove_leftover( - _dir: &PinnedDir, - _quarantine: &OsStr, - _authenticate: impl FnOnce(&mut PinnedFile) -> Result, - _observer: Observer<'_>, -) -> Result { - Err(PinnedError::Unsupported) -} - /// After a failure that moved an entry aside and back (or left it aside) the directory has changed too: /// the sync is attempted, and its own failure does not replace the failure that is being reported. fn sync_after_failure(dir: &PinnedDir, observer: Observer<'_>) { diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index b1cd3fb89..b963a803f 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1828,6 +1828,38 @@ dropping a failed sync of it, a `sync_all()?` inside the rename step, confirming sync for an entry found gone at the inspection, dropping it for one gone at the unlink and passing no identity for a foreign entry each fail at least one of the sync cases, and counting a failed look as present fails the permission case. -**Not claimed.** Directories and the leftover recovery are part 3 (they answer `Unsupported` here); Windows is part 4; the +**Not claimed.** Directories and the leftover recovery are part 3 (below); Windows is part 4; the Disturbed window is bounded and detected, not closed; a device node swapped in is refused before it is opened and cannot be created by an unprivileged actor, which is argued, not tested. + +#### Part 3: removal of a directory on Unix, leftovers and crashes + +Status words: `H1A_2_UNIX_DIRECTORY_AND_LEFTOVERS=IMPLEMENTED_TEST_SUPPORT_ONLY`, `H1_PROVED=NO`. `remove_empty_dir` opens the child +directory (no link followed), refuses one that has entries (`NotEmpty`) and moves it through the same quarantine directory: +identified by the open directory, then `unlinkat(AT_REMOVEDIR)`; a directory that fills after the check goes back +(`NotEmpty`). The same window exists and is detected the same way: the link count of the held directory must have dropped +(`Disturbed` otherwise; for a directory it falls to zero). An empty foreign directory that wins the race in the last window +is removed instead of the authenticated one, and the removal says so; it holds no data. + +**Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, +`Stranded`), leaves a `.h1q-*` directory holding the entry as `x`. A sweep never removes it by itself. +`remove_leftover_file(name, authenticate)` takes it by its **generated name only** (`.h1q-` and exactly 32 lowercase +hexadecimal digits; any other name, including an unrelated private directory, is `InvalidName` and untouched), requires a +directory of the effective user without access for group or others, authenticates the file in it again and, only if +accepted, unlinks it with the same link-count check. A file that does not authenticate (a foreign object left by a refused +put-back) stays; an empty generated quarantine directory is removed. + +**Proof.** A directory swapped in the first window and one filled after the check are put back and not removed; an empty +directory that wins the last window gives `Disturbed` with the original directory intact; a simulated crash (the observer +panics once the entry is in the quarantine directory) leaves the entry there under no name of the store, a leftover removal +with a failing authentication refuses and leaves it, and with the right one finishes it; names without the generated grammar +(`.h1q-empty`, too few or too many digits, upper case, non-hexadecimal digits, other directories, plain files) are refused +before anything is opened; a foreign object left in a quarantine directory by a refused put-back, and a link left there, are +never removed by it; an entry stranded by a blocked put-back (the name taken again, so it is proved to be in the quarantine +directory) is finished after the file authenticates again. The leftover removal syncs the quarantine directory it unlinks +in, like any removal, and says `NotConfirmed` if that fails (modelled through the same test-only seam). Mutation-checked: +syncing nothing in the leftover removal, not refusing a foreign entry, loosening the name grammar to any length, and +ignoring the authentication result each fail at least one of these cases. An entry stranded by a lost search permission is +`Disturbed`, not `Stranded`: a failed look proves nothing (see part 2). + +**Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed. From 26c9c62a2bc5548f1522a54a822305816f7c5fa5 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:57:06 +0200 Subject: [PATCH 2/9] docs(changelog): set the PR number of the Unix directory and leftover entry (#445) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e899155ae..eb7d65499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal. No new `unsafe`, no new package. Issue #359. PR #1070. +- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. From 5d62328e5b1ca43b456e96dbf35d25c2da451d0e Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:03:06 +0200 Subject: [PATCH 3/9] fix(core): report a swapped-in non-empty directory as disturbed, state the limits of the leftover recovery (#445) Carries the findings that were open on the superseded #1070 into this PR, checked against the code of main (most were already closed by the hardening it has: the rollback sync, the sync of the held directory, the identity-checked cleanup, the root-safe test). A directory that cannot be removed because it has entries goes back where it was and was reported as NotEmpty. If an actor won the last window by moving the authenticated empty directory away and putting a non-empty one in its place, the refusal was about that directory and said nothing about ours (Codex P2 on #1070). The unlink refusal now checks that what is inside is still the authenticated directory (held open, so its identity is unique); if not, it goes back under the name all the same and the outcome is Disturbed. A put-back that fails stays a failure that ends in Stranded or Disturbed as before. Stated, not changed: an empty foreign directory that wins the same window is still removed instead of the authenticated one (no Unix primitive unlinks "this inode"; it holds no data, and the removal reports Disturbed), and the leftover recovery reclaims files, so an empty directory left by a crashed directory removal stays in its .h1q-* directory (Sourcery on #1070). Both are in the changelog, the evidence document and the API documentation. Test: a non-empty directory swapped in at the last window gives Disturbed with the original directory intact and the swapped-in one back under the name. Mutation-checked: reporting NotEmpty for any directory fails it. test-support only, no caller, no new unsafe, no new package. Issue #359. --- CHANGELOG.md | 2 +- .../src/pinned/removal.rs | 6 ++-- .../src/pinned/tests/remove_unix_dir.rs | 33 +++++++++++++++++++ .../src/pinned/unix/removal.rs | 24 +++++++++----- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 17 ++++++---- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb7d65499..4d30dedea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal. No new `unsafe`, no new package. Issue #359. PR #1074. +- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. diff --git a/crates/worldscript-secure-storage/src/pinned/removal.rs b/crates/worldscript-secure-storage/src/pinned/removal.rs index d83284d8e..50ea3e85a 100644 --- a/crates/worldscript-secure-storage/src/pinned/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/removal.rs @@ -152,8 +152,10 @@ impl PinnedDir { /// Finishes what a crash or a refused put-back left in the quarantine directory `quarantine` (a /// `.h1q-*` entry of this directory): the file in it is authenticated again and, if `authenticate` - /// accepts it, removed like any other. A file that is not accepted stays; an empty quarantine - /// directory is removed. Unix only: Windows creates none (`Unsupported`). + /// accepts it, removed like any other. A file that is not accepted stays, and so does anything that + /// is not a regular file (a link; an empty directory that a crashed directory removal left is not + /// reclaimed); an empty quarantine directory is removed. Unix only: Windows creates none + /// (`Unsupported`). pub fn remove_leftover_file( &self, quarantine: impl AsRef, diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs index df932b0ea..261eed609 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs @@ -77,6 +77,39 @@ fn an_empty_directory_swapped_in_the_last_window_is_reported_as_disturbed() { ); } +#[test] +fn a_non_empty_directory_swapped_in_the_last_window_is_not_reported_as_not_empty() { + // The refusal to remove a directory with entries is about the directory an actor put there, not about + // the authenticated one, which the actor moved away. What is there goes back under the name, and + // the outcome is tampering, not `NotEmpty`. + let f = fixture("window-filled-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let original = ino(&f.root.join("d")); + let outcome = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Identified, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("original-d")).unwrap(); + fs::create_dir(quarantined).unwrap(); + fs::write(quarantined.join("late"), b"late").unwrap(); + }), + ); + assert_eq!( + ( + outcome, + fs::read(f.root.join("d").join("late")).ok(), + ino(&f.aside.join("original-d")) == original, + quarantines(&f.root).len() + ), + ( + Ok(RemovalOutcome::Disturbed), + Some(b"late".to_vec()), + true, + 0 + ) + ); +} + /// A removal that "crashes" (the observer panics) once the object is in the quarantine directory. fn crash_after_the_move(f: &Fixture) { let crashed = catch_unwind(AssertUnwindSafe(|| { diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index ea4889776..1b9eb4e10 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -511,14 +511,7 @@ impl Removal<'_> { quarantined: &inside, })?; match unlink_verified(&self.quarantine, object, self.target.kind)? { - // A directory that filled after the check goes back where it was. - RemovalOutcome::NotEmpty => { - self.restore()?; - Ok(Step::Moved( - RemovalOutcome::NotEmpty, - self.quarantine.sync(), - )) - } + RemovalOutcome::NotEmpty => self.filled(), outcome => { // The unlink changed the directory that is held, and that directory can outlive the // removal (an actor renamed it away, or put an entry into it so that it cannot be @@ -529,6 +522,21 @@ impl Removal<'_> { } } + /// The unlink was refused because the directory has entries. What is inside goes back where it was; + /// it is the authenticated directory that filled after the check only if it still is that directory + /// (the open one keeps its identity unique). If an actor swapped a non-empty directory in, the refusal + /// was about that one and says nothing about ours: the outcome is tampering, not `NotEmpty`. + fn filled(&self) -> Result { + let authenticated = self.quarantine.holds(self.target.expected); + self.restore()?; + let held = self.quarantine.sync(); + Ok(if authenticated { + Step::Moved(RemovalOutcome::NotEmpty, held) + } else { + Step::Changed(RemovalOutcome::Disturbed) + }) + } + fn restore(&self) -> Result<(), PinnedError> { self.quarantine.restore(&self.dir.handle, self.target.name) } diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index b963a803f..08a2b41b9 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1837,9 +1837,11 @@ created by an unprivileged actor, which is argued, not tested. Status words: `H1A_2_UNIX_DIRECTORY_AND_LEFTOVERS=IMPLEMENTED_TEST_SUPPORT_ONLY`, `H1_PROVED=NO`. `remove_empty_dir` opens the child directory (no link followed), refuses one that has entries (`NotEmpty`) and moves it through the same quarantine directory: identified by the open directory, then `unlinkat(AT_REMOVEDIR)`; a directory that fills after the check goes back -(`NotEmpty`). The same window exists and is detected the same way: the link count of the held directory must have dropped -(`Disturbed` otherwise; for a directory it falls to zero). An empty foreign directory that wins the race in the last window -is removed instead of the authenticated one, and the removal says so; it holds no data. +(`NotEmpty`), but only if what is inside is still the authenticated directory: if an actor swapped a non-empty directory in, +the refusal was about that one, it goes back under the name all the same and the outcome is `Disturbed`. The same window exists +and is detected the same way: the link count of the held directory must have dropped (`Disturbed` otherwise; for a directory +it falls to zero). An empty foreign directory that wins the race in the last window is removed instead of the authenticated +one, and the removal says so; it holds no data. **Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, `Stranded`), leaves a `.h1q-*` directory holding the entry as `x`. A sweep never removes it by itself. @@ -1850,7 +1852,8 @@ accepted, unlinks it with the same link-count check. A file that does not authen put-back) stays; an empty generated quarantine directory is removed. **Proof.** A directory swapped in the first window and one filled after the check are put back and not removed; an empty -directory that wins the last window gives `Disturbed` with the original directory intact; a simulated crash (the observer +directory that wins the last window gives `Disturbed` with the original directory intact, and so does a non-empty one (not +`NotEmpty`); a simulated crash (the observer panics once the entry is in the quarantine directory) leaves the entry there under no name of the store, a leftover removal with a failing authentication refuses and leaves it, and with the right one finishes it; names without the generated grammar (`.h1q-empty`, too few or too many digits, upper case, non-hexadecimal digits, other directories, plain files) are refused @@ -1859,7 +1862,9 @@ never removed by it; an entry stranded by a blocked put-back (the name taken aga directory) is finished after the file authenticates again. The leftover removal syncs the quarantine directory it unlinks in, like any removal, and says `NotConfirmed` if that fails (modelled through the same test-only seam). Mutation-checked: syncing nothing in the leftover removal, not refusing a foreign entry, loosening the name grammar to any length, and -ignoring the authentication result each fail at least one of these cases. An entry stranded by a lost search permission is -`Disturbed`, not `Stranded`: a failed look proves nothing (see part 2). +ignoring the authentication result, and reporting `NotEmpty` for any directory fail at least one of these cases. An entry +stranded by a lost search permission is `Disturbed`, not `Stranded`: a failed look proves nothing (see part 2). **Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed. +The leftover recovery reclaims **files**: an empty directory that a crashed directory removal left in a `.h1q-*` directory +(no data, but no recovery for it) stays there and is never touched by a sweep; a recovery for it is not part of this slice. From 1acf85f2d18c2af8be9a6a110669fd8d5ddc891e Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:15:17 +0200 Subject: [PATCH 4/9] fix(core): tell a removed directory from a moved one on macOS by its path, not its link count (#445) The hosted macOS evidence of this PR failed the removal of an empty directory: the outcome was Disturbed instead of Removed(Confirmed). The check after the unlink asked the open directory whether its link count had dropped, which holds on Linux, where a removed directory that is still open reports zero. macOS keeps reporting the old count (or cannot be asked), so the check could never confirm a removal there. The superseded #1070 never ran on a hosted OS (a stacked PR gets no hosted evidence), so this was the first time that path met macOS; the other directory cases passed there only because they expect Disturbed. For a directory on Apple targets the signal is now that the open descriptor no longer has a path: fcntl(F_GETPATH), rustix::fs::getpath, fails with "no such file" for a removed directory and still answers for one that an actor moved away and that is therefore not the one that was removed. Anything else, such as a path that is still there or another error, stays Disturbed (a removal that cannot be checked afterwards says nothing). Linux and files are unchanged. No new package and no new feature: getpath needs rustix's alloc, which std already enables. Whether macOS answers as described is not known from here; the hosted macOS run of this commit is the evidence, and the changelog and the evidence document state the signal as the one that run showed. test-support only, no caller, no new unsafe. Issue #359. --- CHANGELOG.md | 2 +- .../src/pinned/unix/removal.rs | 36 +++++++++++++++---- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 4 ++- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d30dedea..aba86288b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. +- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks afterwards that the held directory lost its name (its link count on Linux; on macOS, which keeps the old count for a removed directory that is still open, that the descriptor no longer has a path); `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 1b9eb4e10..6d505a21e 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -8,6 +8,8 @@ use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; +#[cfg(target_vendor = "apple")] +use rustix::fs::getpath; use rustix::fs::{ fchmod, fstat, mkdirat, openat, renameat_with, unlinkat, AtFlags, Mode, RenameFlags, }; @@ -303,10 +305,28 @@ pub(in crate::pinned) fn both_confirmed( } } +/// Whether the open `object`, a directory that was just removed, was the one that lost its name. Where a +/// removed directory that is still open reports a link count that dropped (Linux), that is the signal. macOS +/// keeps reporting the old count for it (the hosted run showed a removed empty directory as untouched), so +/// there the signal is that the descriptor no longer has a path: `F_GETPATH` fails with "no such file" +/// for a removed directory and still answers for one that was moved away and is merely not the one removed. +#[cfg(target_vendor = "apple")] +fn directory_lost_its_name(object: &File, _before: u64) -> bool { + matches!(getpath(object), Err(Errno::NOENT)) +} + +#[cfg(not(target_vendor = "apple"))] +fn directory_lost_its_name(object: &File, before: u64) -> bool { + object + .metadata() + .is_ok_and(|metadata| metadata.nlink() < before) +} + /// Unlinks the entry that was identified in the quarantine directory and asks the open object whether -/// it was the one that lost a name: its link count must have dropped. That is a check after the fact -/// and a heuristic, not a proof of identity (see the residual in the parent module): an actor that also -/// changes the link count of the authenticated object in the same window is not noticed. +/// it was the one that lost a name: its link count must have dropped (for a directory, see +/// [`directory_lost_its_name`]). That is a check after the fact and a heuristic, not a proof of identity +/// (see the residual in the parent module): an actor that also changes the link count of the +/// authenticated object in the same window is not noticed. fn unlink_verified( quarantine: &Quarantine, object: &File, @@ -327,10 +347,14 @@ fn unlink_verified( Err(errno) => return Err(io_error(errno)), } // Nothing can be said about a removal that cannot be checked afterwards. - let Ok(after) = object.metadata().map(|metadata| metadata.nlink()) else { - return Ok(RemovalOutcome::Disturbed); + let lost_its_name = if kind == EntryKind::Directory { + directory_lost_its_name(object, before) + } else { + object + .metadata() + .is_ok_and(|metadata| metadata.nlink() < before) }; - if after < before { + if lost_its_name { Ok(RemovalOutcome::Removed(DirectoryDurability::Confirmed)) } else { Ok(RemovalOutcome::Disturbed) diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index 08a2b41b9..2955654a7 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1840,7 +1840,9 @@ identified by the open directory, then `unlinkat(AT_REMOVEDIR)`; a directory tha (`NotEmpty`), but only if what is inside is still the authenticated directory: if an actor swapped a non-empty directory in, the refusal was about that one, it goes back under the name all the same and the outcome is `Disturbed`. The same window exists and is detected the same way: the link count of the held directory must have dropped (`Disturbed` otherwise; for a directory -it falls to zero). An empty foreign directory that wins the race in the last window is removed instead of the authenticated +it falls to zero on Linux). macOS keeps reporting the old count for a removed directory that is still open (the first hosted +macOS run showed a removed empty directory as untouched), so there the signal is that the open descriptor no longer has a path: +`F_GETPATH` fails with "no such file" for a removed directory and still answers for one that was only moved away. An empty foreign directory that wins the race in the last window is removed instead of the authenticated one, and the removal says so; it holds no data. **Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, From b59fe385d80fc39255d503a94d20ff011db18198 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:24:16 +0200 Subject: [PATCH 5/9] test(core): TEMPORARY probe of what macOS reports for a removed, a moved and a live open directory (#445) Always fails on purpose so the hosted macOS log prints the answer; removed again in the next commit. Not part of the change. --- .../src/pinned/tests/macos_probe.rs | 51 +++++++++++++++++++ .../src/pinned/tests/mod.rs | 2 + 2 files changed, 53 insertions(+) create mode 100644 crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs diff --git a/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs b/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs new file mode 100644 index 000000000..d64076327 --- /dev/null +++ b/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs @@ -0,0 +1,51 @@ +//! TEMPORARY probe, removed again in the next commit: what does macOS report for an open directory that was +//! removed, compared with one that was only moved and one that is untouched? The test always panics so that +//! the answer is printed in the hosted log. + +use std::fs::{self, File}; + +use rustix::fs::{fstat, getpath, openat, statat, AtFlags, Mode, OFlags}; + +use super::Scratch; + +fn describe(label: &str, dir: &File) -> String { + let nlink = fstat(dir).map(|stat| stat.st_nlink as u64); + let path = getpath(dir); + let dotdot = openat(dir, "..", OFlags::RDONLY | OFlags::DIRECTORY, Mode::empty()).map(|_| ()); + let dot = statat(dir, ".", AtFlags::empty()).map(|stat| stat.st_nlink as u64); + let create = openat( + dir, + "probe-file", + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL, + Mode::RUSR | Mode::WUSR, + ) + .map(|_| ()); + format!( + "{label}: fstat_nlink={nlink:?} getpath={path:?} openat_dotdot={dotdot:?} statat_dot_nlink={dot:?} create_in_dir={create:?}" + ) +} + +#[test] +fn probe() { + let scratch = Scratch::new("probe"); + let root = scratch.root(); + let live = root.join("live"); + let moved = root.join("moved-from"); + let removed = root.join("removed"); + for dir in [&live, &moved, &removed] { + fs::create_dir(dir).unwrap(); + } + let live_fd = File::open(&live).unwrap(); + let moved_fd = File::open(&moved).unwrap(); + let removed_fd = File::open(&removed).unwrap(); + let before = describe("before-removal", &removed_fd); + fs::rename(&moved, root.join("moved-to")).unwrap(); + fs::remove_dir(&removed).unwrap(); + let lines = [ + before, + describe("live", &live_fd), + describe("moved", &moved_fd), + describe("removed", &removed_fd), + ]; + panic!("PROBE\n{}", lines.join("\n")); +} diff --git a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs index 984b5f2b1..c38ff7bf7 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs @@ -5,6 +5,8 @@ use std::fs; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU32, Ordering}; +#[cfg(all(feature = "test-support", target_vendor = "apple"))] +mod macos_probe; mod read; #[cfg(feature = "test-support")] mod remove; From 8e294a2cf4704eef3a6a656f8127711b45fffc59 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:30:02 +0200 Subject: [PATCH 6/9] test(core): TEMPORARY macOS probe, do not create the probe file before the directory is removed (#445) Still temporary and removed again in the next commit. --- .../src/pinned/tests/macos_probe.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs b/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs index d64076327..32601598a 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs @@ -8,18 +8,24 @@ use rustix::fs::{fstat, getpath, openat, statat, AtFlags, Mode, OFlags}; use super::Scratch; -fn describe(label: &str, dir: &File) -> String { +fn describe(label: &str, dir: &File, try_create: bool) -> String { let nlink = fstat(dir).map(|stat| stat.st_nlink as u64); let path = getpath(dir); let dotdot = openat(dir, "..", OFlags::RDONLY | OFlags::DIRECTORY, Mode::empty()).map(|_| ()); let dot = statat(dir, ".", AtFlags::empty()).map(|stat| stat.st_nlink as u64); - let create = openat( - dir, - "probe-file", - OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL, - Mode::RUSR | Mode::WUSR, - ) - .map(|_| ()); + let create = if try_create { + Some( + openat( + dir, + "probe-file", + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL, + Mode::RUSR | Mode::WUSR, + ) + .map(|_| ()), + ) + } else { + None + }; format!( "{label}: fstat_nlink={nlink:?} getpath={path:?} openat_dotdot={dotdot:?} statat_dot_nlink={dot:?} create_in_dir={create:?}" ) @@ -38,14 +44,14 @@ fn probe() { let live_fd = File::open(&live).unwrap(); let moved_fd = File::open(&moved).unwrap(); let removed_fd = File::open(&removed).unwrap(); - let before = describe("before-removal", &removed_fd); + let before = describe("before-removal", &removed_fd, false); fs::rename(&moved, root.join("moved-to")).unwrap(); fs::remove_dir(&removed).unwrap(); let lines = [ before, - describe("live", &live_fd), - describe("moved", &moved_fd), - describe("removed", &removed_fd), + describe("live", &live_fd, true), + describe("moved", &moved_fd, true), + describe("removed", &removed_fd, true), ]; panic!("PROBE\n{}", lines.join("\n")); } From 2989ba80d47040fb9183d3a3f05727909e07fcec Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 02:41:41 +0200 Subject: [PATCH 7/9] fix(core): remove a directory on Linux only, answer Unsupported where the system cannot tell afterwards (#445) The hosted macOS evidence of this PR failed the removal of an empty directory (Disturbed instead of Removed). A first attempt to read the removal from fcntl(F_GETPATH) was also refused by macOS, so the answer was measured instead of guessed: a temporary probe on the hosted runner (commits b59fe385 and 8e294a2c, removed here) compared an open directory that was removed, one that was only moved and one that was untouched. link count 2 / 2 / 2 (unchanged for the removed one) F_GETPATH the old path for the removed one (stale) openat(".."), statat(".") succeed for all three create an entry in it fails with ENOENT only for the removed one The removal of a directory counts only if the open directory can be asked afterwards whether it was the one that lost its name. Linux answers (a removed directory that is still open reports a link count of zero). On macOS the only signal is a write into a directory that, in the attack case, an actor moved away and that belongs to another path, and it fails for lack of permission on a read-only directory. That is not a check to build a success on. remove_empty_dir therefore answers Unsupported on any Unix system but Linux, after the checks that need no such signal (a link, a missing name, a directory that has entries) and before anything is moved. The directory cases move to a Linux-only test file; on the other systems the same call is pinned to Unsupported with nothing moved, and the generic directory test expects it. The leftover recovery (files) is unchanged and runs on both. Whether and how macOS gets directory removal is left to the decision about the production use. The changelog, the evidence document and the API documentation state the limit and the measured behaviour. test-support only, no caller, no new unsafe, no new package. Issue #359. --- CHANGELOG.md | 2 +- .../src/pinned/removal.rs | 15 ++- .../src/pinned/tests/macos_probe.rs | 57 --------- .../src/pinned/tests/mod.rs | 4 +- .../src/pinned/tests/remove.rs | 18 ++- .../src/pinned/tests/remove_linux_dir.rs | 107 +++++++++++++++++ .../src/pinned/tests/remove_unix_dir.rs | 112 ++++-------------- .../src/pinned/unix/removal.rs | 47 +++----- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 20 ++-- 9 files changed, 188 insertions(+), 194 deletions(-) delete mode 100644 crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs create mode 100644 crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index aba86288b..9acaa7d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks afterwards that the held directory lost its name (its link count on Linux; on macOS, which keeps the old count for a removed directory that is still open, that the descriptor no longer has a path); `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. +- **R-15 Gate 4:** removal of a directory on Linux, the recovery of leftovers and the crash cases on Unix, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; macOS reports no usable signal for a removed directory that is still open (hosted probe: link count and `F_GETPATH` unchanged), so there an empty directory answers `Unsupported` before anything is moved; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. diff --git a/crates/worldscript-secure-storage/src/pinned/removal.rs b/crates/worldscript-secure-storage/src/pinned/removal.rs index 50ea3e85a..af78a7740 100644 --- a/crates/worldscript-secure-storage/src/pinned/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/removal.rs @@ -4,10 +4,11 @@ //! **Availability.** Everything in this module exists only with the test-only `test-support` feature, //! has no caller, and is not part of any production build. Reclamation of a real store is not admitted. //! -//! **Status of this slice.** The API and its isolation, and on Unix the removal of files, of -//! directories and of the leftovers of a crash. A call on Windows answers [`PinnedError::Unsupported`] -//! until the Windows implementation lands in its own slice, so nothing is removed there and nothing -//! claims a safety it does not have yet. +//! **Status of this slice.** The API and its isolation, and on Unix the removal of files and of the +//! leftovers of a crash, and on Linux the removal of directories. A call on Windows, and the removal of +//! a directory on any other system, answers [`PinnedError::Unsupported`] (see +//! [`PinnedDir::remove_empty_dir`]), so nothing is removed there and nothing claims a safety it does not +//! have yet. //! //! **The two designs the platform slices do not use.** Removing "the entry that was checked" with //! `unlinkat(parent, name)` after a check on the name is exposed to a swap of the last component: POSIX @@ -135,7 +136,11 @@ impl PinnedDir { } /// Removes the child directory `name` if it has no entries, and exactly the directory that was - /// opened. + /// opened. On Unix this is available on Linux only: the removal counts only if the open directory can + /// be asked afterwards whether it was the one removed, and no other Unix system answers that without + /// writing into the directory (macOS keeps the old link count and the old path of a removed directory + /// that is still open). There an empty directory answers [`PinnedError::Unsupported`] before anything is + /// moved; a link, a missing name and a directory with entries are told the same everywhere. pub fn remove_empty_dir(&self, name: impl AsRef) -> Result { self.remove_empty_dir_observed(name, &mut |_| Ok(())) } diff --git a/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs b/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs deleted file mode 100644 index 32601598a..000000000 --- a/crates/worldscript-secure-storage/src/pinned/tests/macos_probe.rs +++ /dev/null @@ -1,57 +0,0 @@ -//! TEMPORARY probe, removed again in the next commit: what does macOS report for an open directory that was -//! removed, compared with one that was only moved and one that is untouched? The test always panics so that -//! the answer is printed in the hosted log. - -use std::fs::{self, File}; - -use rustix::fs::{fstat, getpath, openat, statat, AtFlags, Mode, OFlags}; - -use super::Scratch; - -fn describe(label: &str, dir: &File, try_create: bool) -> String { - let nlink = fstat(dir).map(|stat| stat.st_nlink as u64); - let path = getpath(dir); - let dotdot = openat(dir, "..", OFlags::RDONLY | OFlags::DIRECTORY, Mode::empty()).map(|_| ()); - let dot = statat(dir, ".", AtFlags::empty()).map(|stat| stat.st_nlink as u64); - let create = if try_create { - Some( - openat( - dir, - "probe-file", - OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL, - Mode::RUSR | Mode::WUSR, - ) - .map(|_| ()), - ) - } else { - None - }; - format!( - "{label}: fstat_nlink={nlink:?} getpath={path:?} openat_dotdot={dotdot:?} statat_dot_nlink={dot:?} create_in_dir={create:?}" - ) -} - -#[test] -fn probe() { - let scratch = Scratch::new("probe"); - let root = scratch.root(); - let live = root.join("live"); - let moved = root.join("moved-from"); - let removed = root.join("removed"); - for dir in [&live, &moved, &removed] { - fs::create_dir(dir).unwrap(); - } - let live_fd = File::open(&live).unwrap(); - let moved_fd = File::open(&moved).unwrap(); - let removed_fd = File::open(&removed).unwrap(); - let before = describe("before-removal", &removed_fd, false); - fs::rename(&moved, root.join("moved-to")).unwrap(); - fs::remove_dir(&removed).unwrap(); - let lines = [ - before, - describe("live", &live_fd, true), - describe("moved", &moved_fd, true), - describe("removed", &removed_fd, true), - ]; - panic!("PROBE\n{}", lines.join("\n")); -} diff --git a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs index c38ff7bf7..f77c34734 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs @@ -5,11 +5,11 @@ use std::fs; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU32, Ordering}; -#[cfg(all(feature = "test-support", target_vendor = "apple"))] -mod macos_probe; mod read; #[cfg(feature = "test-support")] mod remove; +#[cfg(all(feature = "test-support", target_os = "linux"))] +mod remove_linux_dir; #[cfg(all(feature = "test-support", unix))] mod remove_unix; #[cfg(all(feature = "test-support", unix))] diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs index 75039ba93..539376f0a 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs @@ -159,6 +159,20 @@ fn an_empty_directory_is_removed_and_a_filled_one_or_a_link_is_not() { fs::create_dir(aside.join("target")).unwrap(); link_to_dir(&aside.join("target"), &root.join("link")); let pinned = PinnedDir::open_root(&root).unwrap(); + // Only Linux can tell afterwards that the open directory was the one removed; elsewhere the removal + // of an empty directory is refused before anything is moved, and the other cases are the same. + let (empty_removal, left) = if cfg!(target_os = "linux") { + (removed(), vec!["filled".to_string(), "link".to_string()]) + } else { + ( + Err(PinnedError::Unsupported), + vec![ + "empty".to_string(), + "filled".to_string(), + "link".to_string(), + ], + ) + }; assert_eq!( ( pinned.remove_empty_dir("empty"), @@ -170,13 +184,13 @@ fn an_empty_directory_is_removed_and_a_filled_one_or_a_link_is_not() { names(&pinned) ), ( - removed(), + empty_removal, Ok(RemovalOutcome::NotEmpty), Err(PinnedError::Redirected), Err(PinnedError::NotFound), b"inside".to_vec(), true, - vec!["filled".to_string(), "link".to_string()] + left ) ); } diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs new file mode 100644 index 000000000..8b0e0de8c --- /dev/null +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs @@ -0,0 +1,107 @@ +//! Linux removal of a directory under the swaps the design exists for: another actor replaces the +//! directory, or the entry inside the quarantine directory, between the steps of the removal. Each case +//! forces the swap through the observer at a chosen stage and pins what happens to every object. Other +//! Unix systems answer `Unsupported` (see `remove_unix_dir`), because they cannot tell afterwards, without +//! writing into the directory, that the open one is the directory that was removed. + +use std::fs; + +use super::remove::names; +use super::remove_unix::{fixture, ino, on, quarantines, At}; +use crate::pinned::RemovalOutcome; + +#[test] +fn a_directory_swapped_or_filled_after_the_check_is_put_back_and_not_removed() { + let f = fixture("dir-swap"); + fs::create_dir(f.root.join("d")).unwrap(); + fs::create_dir(f.root.join("e")).unwrap(); + let swapped = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Checked, |_| { + fs::rename(f.root.join("d"), f.aside.join("original-d")).unwrap(); + fs::create_dir(f.root.join("d")).unwrap(); + fs::write(f.root.join("d").join("live"), b"live").unwrap(); + }), + ); + let filled = f.pinned.remove_empty_dir_observed( + "e", + &mut on(At::Checked, |_| { + fs::write(f.root.join("e").join("late"), b"late").unwrap() + }), + ); + assert_eq!( + ( + swapped, + fs::read(f.root.join("d").join("live")).unwrap(), + f.aside.join("original-d").is_dir(), + filled, + fs::read(f.root.join("e").join("late")).unwrap(), + names(&f.pinned) + ), + ( + Ok(RemovalOutcome::Swapped { restored: true }), + b"live".to_vec(), + true, + Ok(RemovalOutcome::NotEmpty), + b"late".to_vec(), + vec!["a".to_string(), "d".to_string(), "e".to_string()] + ) + ); +} + +#[test] +fn an_empty_directory_swapped_in_the_last_window_is_reported_as_disturbed() { + let f = fixture("window-empty-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let original = ino(&f.root.join("d")); + let outcome = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Identified, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("original-d")).unwrap(); + fs::create_dir(quarantined).unwrap(); + }), + ); + assert_eq!( + ( + outcome, + f.root.join("d").exists(), + ino(&f.aside.join("original-d")) == original, + quarantines(&f.root).len() + ), + (Ok(RemovalOutcome::Disturbed), false, true, 0) + ); +} + +#[test] +fn a_non_empty_directory_swapped_in_the_last_window_is_not_reported_as_not_empty() { + // The refusal to remove a directory with entries is about the directory an actor put there, not about + // the authenticated one, which the actor moved away. What is there goes back under the name, and + // the outcome is tampering, not `NotEmpty`. + let f = fixture("window-filled-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let original = ino(&f.root.join("d")); + let outcome = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Identified, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("original-d")).unwrap(); + fs::create_dir(quarantined).unwrap(); + fs::write(quarantined.join("late"), b"late").unwrap(); + }), + ); + assert_eq!( + ( + outcome, + fs::read(f.root.join("d").join("late")).ok(), + ino(&f.aside.join("original-d")) == original, + quarantines(&f.root).len() + ), + ( + Ok(RemovalOutcome::Disturbed), + Some(b"late".to_vec()), + true, + 0 + ) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs index 261eed609..b3072d393 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs @@ -1,110 +1,42 @@ -//! Unix removal of a directory, and of what a crash or a refused put-back left in a quarantine -//! directory, under the swaps the design exists for: another actor replaces the directory, or the entry -//! inside the quarantine directory, between the steps of the removal. Each case forces the swap through -//! the observer at a chosen stage and pins what happens to every object. +//! Unix recovery of what a crash or a refused put-back left in a quarantine directory, under the swaps +//! the design exists for: another actor replaces the entry inside the quarantine directory between the +//! steps of the removal. Each case forces the swap through the observer at a chosen stage and pins what +//! happens to every object. The removal of a directory is in `remove_linux_dir`; where it is not +//! available it answers `Unsupported`, which is pinned here. use std::fs; use std::panic::{catch_unwind, AssertUnwindSafe}; use super::remove::{holds, names, removed}; use super::remove_unix::{ - at, denied, fixture, held_in_quarantine, ino, on, quarantines, swap_in_a_live_file, At, Fixture, + at, denied, fixture, held_in_quarantine, on, quarantines, swap_in_a_live_file, At, Fixture, }; use crate::durable::DirectoryDurability::NotConfirmed; use crate::pinned::unix::removal::sync_fault; use crate::pinned::{PinnedError, RemovalOutcome, QUARANTINE_PREFIX}; +#[cfg(not(target_os = "linux"))] #[test] -fn a_directory_swapped_or_filled_after_the_check_is_put_back_and_not_removed() { - let f = fixture("dir-swap"); - fs::create_dir(f.root.join("d")).unwrap(); - fs::create_dir(f.root.join("e")).unwrap(); - let swapped = f.pinned.remove_empty_dir_observed( - "d", - &mut on(At::Checked, |_| { - fs::rename(f.root.join("d"), f.aside.join("original-d")).unwrap(); - fs::create_dir(f.root.join("d")).unwrap(); - fs::write(f.root.join("d").join("live"), b"live").unwrap(); - }), - ); - let filled = f.pinned.remove_empty_dir_observed( - "e", - &mut on(At::Checked, |_| { - fs::write(f.root.join("e").join("late"), b"late").unwrap() - }), - ); - assert_eq!( - ( - swapped, - fs::read(f.root.join("d").join("live")).unwrap(), - f.aside.join("original-d").is_dir(), - filled, - fs::read(f.root.join("e").join("late")).unwrap(), - names(&f.pinned) - ), - ( - Ok(RemovalOutcome::Swapped { restored: true }), - b"live".to_vec(), - true, - Ok(RemovalOutcome::NotEmpty), - b"late".to_vec(), - vec!["a".to_string(), "d".to_string(), "e".to_string()] - ) - ); -} - -#[test] -fn an_empty_directory_swapped_in_the_last_window_is_reported_as_disturbed() { - let f = fixture("window-empty-dir"); - fs::create_dir(f.root.join("d")).unwrap(); - let original = ino(&f.root.join("d")); - let outcome = f.pinned.remove_empty_dir_observed( - "d", - &mut on(At::Identified, |quarantined| { - let quarantined = quarantined.unwrap(); - fs::rename(quarantined, f.aside.join("original-d")).unwrap(); - fs::create_dir(quarantined).unwrap(); - }), - ); +fn removing_a_directory_is_unsupported_where_the_system_cannot_tell_afterwards_and_moves_nothing() { + // The checks that need no such signal still hold: a directory with entries is `NotEmpty`, a link is + // refused, a missing name is not found; an empty directory is refused as unsupported before it is moved. + let f = fixture("dir-unsupported"); + fs::create_dir(f.root.join("empty")).unwrap(); + fs::create_dir(f.root.join("filled")).unwrap(); + fs::write(f.root.join("filled").join("inside"), b"inside").unwrap(); assert_eq!( ( - outcome, - f.root.join("d").exists(), - ino(&f.aside.join("original-d")) == original, - quarantines(&f.root).len() - ), - (Ok(RemovalOutcome::Disturbed), false, true, 0) - ); -} - -#[test] -fn a_non_empty_directory_swapped_in_the_last_window_is_not_reported_as_not_empty() { - // The refusal to remove a directory with entries is about the directory an actor put there, not about - // the authenticated one, which the actor moved away. What is there goes back under the name, and - // the outcome is tampering, not `NotEmpty`. - let f = fixture("window-filled-dir"); - fs::create_dir(f.root.join("d")).unwrap(); - let original = ino(&f.root.join("d")); - let outcome = f.pinned.remove_empty_dir_observed( - "d", - &mut on(At::Identified, |quarantined| { - let quarantined = quarantined.unwrap(); - fs::rename(quarantined, f.aside.join("original-d")).unwrap(); - fs::create_dir(quarantined).unwrap(); - fs::write(quarantined.join("late"), b"late").unwrap(); - }), - ); - assert_eq!( - ( - outcome, - fs::read(f.root.join("d").join("late")).ok(), - ino(&f.aside.join("original-d")) == original, + f.pinned.remove_empty_dir("empty"), + f.pinned.remove_empty_dir("filled"), + f.pinned.remove_empty_dir("missing"), + names(&f.pinned), quarantines(&f.root).len() ), ( - Ok(RemovalOutcome::Disturbed), - Some(b"late".to_vec()), - true, + Err(PinnedError::Unsupported), + Ok(RemovalOutcome::NotEmpty), + Err(PinnedError::NotFound), + vec!["a".to_string(), "empty".to_string(), "filled".to_string()], 0 ) ); diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 6d505a21e..670695b73 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -8,8 +8,6 @@ use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; -#[cfg(target_vendor = "apple")] -use rustix::fs::getpath; use rustix::fs::{ fchmod, fstat, mkdirat, openat, renameat_with, unlinkat, AtFlags, Mode, RenameFlags, }; @@ -305,28 +303,10 @@ pub(in crate::pinned) fn both_confirmed( } } -/// Whether the open `object`, a directory that was just removed, was the one that lost its name. Where a -/// removed directory that is still open reports a link count that dropped (Linux), that is the signal. macOS -/// keeps reporting the old count for it (the hosted run showed a removed empty directory as untouched), so -/// there the signal is that the descriptor no longer has a path: `F_GETPATH` fails with "no such file" -/// for a removed directory and still answers for one that was moved away and is merely not the one removed. -#[cfg(target_vendor = "apple")] -fn directory_lost_its_name(object: &File, _before: u64) -> bool { - matches!(getpath(object), Err(Errno::NOENT)) -} - -#[cfg(not(target_vendor = "apple"))] -fn directory_lost_its_name(object: &File, before: u64) -> bool { - object - .metadata() - .is_ok_and(|metadata| metadata.nlink() < before) -} - /// Unlinks the entry that was identified in the quarantine directory and asks the open object whether -/// it was the one that lost a name: its link count must have dropped (for a directory, see -/// [`directory_lost_its_name`]). That is a check after the fact and a heuristic, not a proof of identity -/// (see the residual in the parent module): an actor that also changes the link count of the -/// authenticated object in the same window is not noticed. +/// it was the one that lost a name: its link count must have dropped. That is a check after the fact +/// and a heuristic, not a proof of identity (see the residual in the parent module): an actor that also +/// changes the link count of the authenticated object in the same window is not noticed. fn unlink_verified( quarantine: &Quarantine, object: &File, @@ -347,14 +327,10 @@ fn unlink_verified( Err(errno) => return Err(io_error(errno)), } // Nothing can be said about a removal that cannot be checked afterwards. - let lost_its_name = if kind == EntryKind::Directory { - directory_lost_its_name(object, before) - } else { - object - .metadata() - .is_ok_and(|metadata| metadata.nlink() < before) + let Ok(after) = object.metadata().map(|metadata| metadata.nlink()) else { + return Ok(RemovalOutcome::Disturbed); }; - if lost_its_name { + if after < before { Ok(RemovalOutcome::Removed(DirectoryDurability::Confirmed)) } else { Ok(RemovalOutcome::Disturbed) @@ -394,6 +370,14 @@ pub(in crate::pinned) fn remove_file( remove_moved(dir, target, observer) } +/// Removes the empty child directory `name`. Linux only: the removal is only counted if the open +/// directory can be asked afterwards whether it was the one that lost its name, and only Linux answers +/// that without writing into the directory (a removed directory that is still open reports a link count of +/// zero). On macOS the count stays at 2, `F_GETPATH` keeps answering with the old path, and only creating +/// an entry in it fails (`ENOENT`), which would write into a directory an actor may have moved away and +/// fails for lack of permission on a read-only one (hosted macOS probe). Elsewhere the call answers +/// `Unsupported` after the checks that need no such signal (a link, a missing name, a directory that has +/// entries) and before anything is moved. pub(in crate::pinned) fn remove_dir( dir: &PinnedDir, name: &OsStr, @@ -405,6 +389,9 @@ pub(in crate::pinned) fn remove_dir( Err(PinnedError::TooManyEntries) => return Ok(RemovalOutcome::NotEmpty), Err(error) => return Err(error), } + if !cfg!(target_os = "linux") { + return Err(PinnedError::Unsupported); + } observer(RemovalStage::Checked)?; let target = Target { name, diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index 2955654a7..a71d13d6b 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1832,17 +1832,23 @@ entry each fail at least one of the sync cases, and counting a failed look as pr Disturbed window is bounded and detected, not closed; a device node swapped in is refused before it is opened and cannot be created by an unprivileged actor, which is argued, not tested. -#### Part 3: removal of a directory on Unix, leftovers and crashes - -Status words: `H1A_2_UNIX_DIRECTORY_AND_LEFTOVERS=IMPLEMENTED_TEST_SUPPORT_ONLY`, `H1_PROVED=NO`. `remove_empty_dir` opens the child +#### Part 3: removal of a directory on Linux, leftovers and crashes on Unix + +Status words: `H1A_2_UNIX_DIRECTORY_AND_LEFTOVERS=IMPLEMENTED_TEST_SUPPORT_ONLY` (directories: Linux only), `H1_PROVED=NO`. +**Directories are Linux only.** The removal counts only if the open directory can be asked afterwards whether it was the one that +lost its name. A hosted macOS probe of an open directory that was removed, one that was only moved and one that was untouched +showed: the link count stays 2 for all three; `F_GETPATH` keeps answering with the old path for the removed one; `openat("..")` and +`statat(".")` succeed for all three; only creating an entry in the removed one fails (`ENOENT`). The only signal is therefore a write +into a directory that, in the attack case, an actor moved away and that belongs to another path, and it would fail for lack of +permission on a read-only directory, so it is not used. Where the answer is not available an empty directory gives `Unsupported` +before anything is moved (`NotEmpty`, a link and a missing name are told the same everywhere); whether and how macOS +gets directory removal is left to the decision about the production use (H1b). `remove_empty_dir` opens the child directory (no link followed), refuses one that has entries (`NotEmpty`) and moves it through the same quarantine directory: identified by the open directory, then `unlinkat(AT_REMOVEDIR)`; a directory that fills after the check goes back (`NotEmpty`), but only if what is inside is still the authenticated directory: if an actor swapped a non-empty directory in, the refusal was about that one, it goes back under the name all the same and the outcome is `Disturbed`. The same window exists and is detected the same way: the link count of the held directory must have dropped (`Disturbed` otherwise; for a directory -it falls to zero on Linux). macOS keeps reporting the old count for a removed directory that is still open (the first hosted -macOS run showed a removed empty directory as untouched), so there the signal is that the open descriptor no longer has a path: -`F_GETPATH` fails with "no such file" for a removed directory and still answers for one that was only moved away. An empty foreign directory that wins the race in the last window is removed instead of the authenticated +it falls to zero). An empty foreign directory that wins the race in the last window is removed instead of the authenticated one, and the removal says so; it holds no data. **Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, @@ -1853,7 +1859,7 @@ directory of the effective user without access for group or others, authenticate accepted, unlinks it with the same link-count check. A file that does not authenticate (a foreign object left by a refused put-back) stays; an empty generated quarantine directory is removed. -**Proof.** A directory swapped in the first window and one filled after the check are put back and not removed; an empty +**Proof.** The directory cases run on Linux; on any other Unix system the same call is pinned to `Unsupported` with nothing moved. A directory swapped in the first window and one filled after the check are put back and not removed; an empty directory that wins the last window gives `Disturbed` with the original directory intact, and so does a non-empty one (not `NotEmpty`); a simulated crash (the observer panics once the entry is in the quarantine directory) leaves the entry there under no name of the store, a leftover removal From ec7ba9317b67a4e48fcf0b1cafb4779cacef095a Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:15:48 +0200 Subject: [PATCH 8/9] fix(core): sync the parent when a leftover's quarantine directory goes, look at a restored directory again, recover an empty directory leftover (#445) Three findings on 2989ba80, all valid. A leftover removal removes the emptied quarantine directory with discard, which changes the parent. When the removal itself changed nothing (a refusal) or failed, that change was never synced: Unchanged skipped the sync, and an error returned before the settle. A crash could then bring the empty directory back although the caller had been told the result (Sourcery, Codex P2 and a CodeAnt nitpick, independently). discard now reports whether it removed the directory; the parent is synced in that case for a refusal and for an error, and the result of the removal is kept. A directory that cannot be removed because it has entries goes back under its name, and was looked at before the rename. An actor that swaps another directory in between the look and the rename had it restored and reported as NotEmpty (Codex P2). It is now looked at under its name after it went back, against the identity of the open authenticated directory, and anything else is Disturbed. The earlier look is gone: it was the same check at a worse place. A crashed directory removal leaves an empty directory as x in its quarantine directory, and the leftover recovery only took files (Sourcery, on #1070 and again here). An empty directory holds nothing, so it needs no authentication and goes with the same unlink and the same check as the removal of a directory, on Linux; elsewhere it is refused, a directory with entries stays (NotEmpty), and a link or a FIFO stays (Refused). The temporary macOS probe thread (Codex P1) is obsolete: the probe was removed in the previous commit and the hosted macOS run is green. Tests: the parent sync for a refusal and an error with the entry taken, and not when the entry stays; the recovery of an empty directory after a crash during a directory removal, with the callback never asked; a directory with entries stays; Refused for an empty directory where the removal cannot be checked. Each of the three changes is mutation-checked. test-support only, no caller, no new unsafe, no new package. Issue #359. --- CHANGELOG.md | 2 +- .../src/pinned/removal.rs | 5 +- .../src/pinned/tests/remove_linux_dir.rs | 51 ++++++++++++- .../src/pinned/tests/remove_unix_dir.rs | 71 ++++++++++++++++++- .../src/pinned/unix/removal.rs | 66 +++++++++++++---- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 14 ++-- 6 files changed, 186 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9acaa7d83..d005de794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Linux, the recovery of leftovers and the crash cases on Unix, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; macOS reports no usable signal for a removed directory that is still open (hosted probe: link count and `F_GETPATH` unchanged), so there an empty directory answers `Unsupported` before anything is moved; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery reclaims files: an empty directory left by a crashed directory removal is not reclaimed. No new `unsafe`, no new package. Issue #359. PR #1074. +- **R-15 Gate 4:** removal of a directory on Linux, the recovery of leftovers and the crash cases on Unix, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; macOS reports no usable signal for a removed directory that is still open (hosted probe: link count and `F_GETPATH` unchanged), so there an empty directory answers `Unsupported` before anything is moved; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery also removes an empty directory left by a crashed directory removal (Linux; it holds nothing, so it is not authenticated), and syncs the parent when it removes the emptied quarantine directory, whatever the removal came to. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. diff --git a/crates/worldscript-secure-storage/src/pinned/removal.rs b/crates/worldscript-secure-storage/src/pinned/removal.rs index af78a7740..241d7a0ca 100644 --- a/crates/worldscript-secure-storage/src/pinned/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/removal.rs @@ -158,8 +158,9 @@ impl PinnedDir { /// Finishes what a crash or a refused put-back left in the quarantine directory `quarantine` (a /// `.h1q-*` entry of this directory): the file in it is authenticated again and, if `authenticate` /// accepts it, removed like any other. A file that is not accepted stays, and so does anything that - /// is not a regular file (a link; an empty directory that a crashed directory removal left is not - /// reclaimed); an empty quarantine directory is removed. Unix only: Windows creates none + /// is neither a regular file nor an empty directory (a link, a directory with entries); an empty + /// directory that a crashed directory removal left holds nothing, needs no authentication and is + /// removed on Linux; an empty quarantine directory is removed. Unix only: Windows creates none /// (`Unsupported`). pub fn remove_leftover_file( &self, diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs index 8b0e0de8c..169766bb9 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_linux_dir.rs @@ -5,10 +5,13 @@ //! writing into the directory, that the open one is the directory that was removed. use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::panic::{catch_unwind, AssertUnwindSafe}; -use super::remove::names; +use super::remove::{names, removed}; use super::remove_unix::{fixture, ino, on, quarantines, At}; -use crate::pinned::RemovalOutcome; +use crate::pinned::unix::removal::INSIDE; +use crate::pinned::{RemovalOutcome, QUARANTINE_PREFIX}; #[test] fn a_directory_swapped_or_filled_after_the_check_is_put_back_and_not_removed() { @@ -105,3 +108,47 @@ fn a_non_empty_directory_swapped_in_the_last_window_is_not_reported_as_not_empty ) ); } + +#[test] +fn an_empty_directory_left_by_a_crashed_directory_removal_is_finished_by_the_leftover_recovery() { + let f = fixture("crash-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let crashed = catch_unwind(AssertUnwindSafe(|| { + let _ = f + .pinned + .remove_empty_dir_observed("d", &mut on(At::MovedAside, |_| panic!("simulated crash"))); + })); + assert!(crashed.is_err()); + let left = quarantines(&f.root); + let name = left[0].file_name().unwrap().to_owned(); + let before = (left[0].join(INSIDE).is_dir(), f.root.join("d").exists()); + // An empty directory holds nothing to authenticate: the callback is never asked. + let finished = f.pinned.remove_leftover_file(&name, |_| panic!("asked")); + assert_eq!( + ( + before, + finished, + names(&f.pinned), + quarantines(&f.root).len() + ), + ((true, false), removed(), vec!["a".to_string()], 0) + ); +} + +#[test] +fn a_directory_with_entries_left_in_a_quarantine_directory_is_never_removed_by_the_recovery() { + let f = fixture("leftover-filled-dir"); + let name = format!("{QUARANTINE_PREFIX}{}", "3".repeat(32)); + fs::create_dir(f.root.join(&name)).unwrap(); + fs::set_permissions(f.root.join(&name), fs::Permissions::from_mode(0o700)).unwrap(); + fs::create_dir(f.root.join(&name).join(INSIDE)).unwrap(); + fs::write(f.root.join(&name).join(INSIDE).join("inside"), b"inside").unwrap(); + let outcome = f.pinned.remove_leftover_file(&name, |_| Ok(true)); + assert_eq!( + ( + outcome, + fs::read(f.root.join(&name).join(INSIDE).join("inside")).unwrap() + ), + (Ok(RemovalOutcome::NotEmpty), b"inside".to_vec()) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs index b3072d393..ff4ec1c59 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix_dir.rs @@ -12,7 +12,7 @@ use super::remove_unix::{ at, denied, fixture, held_in_quarantine, on, quarantines, swap_in_a_live_file, At, Fixture, }; use crate::durable::DirectoryDurability::NotConfirmed; -use crate::pinned::unix::removal::sync_fault; +use crate::pinned::unix::removal::{remove_leftover, sync_fault, INSIDE}; use crate::pinned::{PinnedError, RemovalOutcome, QUARANTINE_PREFIX}; #[cfg(not(target_os = "linux"))] @@ -222,3 +222,72 @@ fn a_link_left_in_a_quarantine_directory_is_refused_and_not_followed() { ) ); } + +/// One leftover removal after a crash, with the entry taken away by an actor while it is authenticated +/// (or not), and what the authentication says: the outcome, whether the parent was about to be synced, +/// and how many quarantine directories are left. +fn leftover_run( + label: &str, + take_entry: bool, + verdict: Result, +) -> (Result, bool, usize) { + let f = fixture(label); + crash_after_the_move(&f); + let q = quarantines(&f.root)[0].clone(); + let name = q.file_name().unwrap().to_owned(); + let mut synced = false; + let outcome = remove_leftover( + &f.pinned, + &name, + |_file| { + if take_entry { + fs::remove_file(q.join(INSIDE)).unwrap(); + } + verdict + }, + &mut |stage| { + if at(stage).0 == At::BeforeSync { + synced = true; + } + Ok(()) + }, + ); + (outcome, synced, quarantines(&f.root).len()) +} + +#[test] +fn a_leftover_whose_empty_quarantine_directory_goes_with_the_discard_syncs_the_parent_whatever_the_result( +) { + // When the entry stays, nothing changed and nothing is synced. When an actor takes it away while it + // is looked at, the quarantine directory is empty and is removed, which changes the parent: the + // parent is synced, for a refusal and for an error, and the result is kept. + assert_eq!( + [ + leftover_run("kept", false, Ok(false)), + leftover_run("taken-refused", true, Ok(false)), + leftover_run("taken-error", true, Err(denied())), + ], + [ + (Ok(RemovalOutcome::Refused), false, 1), + (Ok(RemovalOutcome::Refused), true, 0), + (Err(denied()), true, 0), + ] + ); +} + +#[cfg(not(target_os = "linux"))] +#[test] +fn an_empty_directory_left_in_a_quarantine_directory_is_refused_where_its_removal_cannot_be_checked( +) { + use std::os::unix::fs::PermissionsExt; + let f = fixture("leftover-dir-unsupported"); + let name = format!("{QUARANTINE_PREFIX}{}", "2".repeat(32)); + fs::create_dir(f.root.join(&name)).unwrap(); + fs::set_permissions(f.root.join(&name), fs::Permissions::from_mode(0o700)).unwrap(); + fs::create_dir(f.root.join(&name).join(INSIDE)).unwrap(); + let outcome = f.pinned.remove_leftover_file(&name, |_| Ok(true)); + assert_eq!( + (outcome, f.root.join(&name).join(INSIDE).is_dir()), + (Ok(RemovalOutcome::Refused), true) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 670695b73..b5c2528bc 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -239,11 +239,11 @@ impl Quarantine { parent_path.join(&self.name).join(INSIDE) } - /// Removes this directory if it is empty. A directory that still holds an entry is left, and so is - /// whatever the name leads to if that is no longer the directory that was created (see - /// [`remove_if_named`]). - fn discard(&self, parent: &DirHandle) { - remove_if_named(parent, &self.name, Some(self.identity)); + /// Removes this directory if it is empty; whether it was removed, which changed the parent. A + /// directory that still holds an entry is left, and so is whatever the name leads to if that is no + /// longer the directory that was created (see [`remove_if_named`]). + fn discard(&self, parent: &DirHandle) -> bool { + remove_if_named(parent, &self.name, Some(self.identity)) } /// Whether the entry is inside, as far as a look at the name can prove it: it was looked at and is @@ -410,8 +410,20 @@ pub(in crate::pinned) fn remove_leftover( ) -> Result { let held = Quarantine::open_existing(&dir.handle, quarantine)?; let step = leftover_step(&held, dir, authenticate, observer); - held.discard(&dir.handle); - settle(dir, step?, observer) + // Removing the empty quarantine directory changes the parent whatever the removal came to (an actor + // can take the entry away while it is being looked at), so the parent is synced then too, and the + // result of the removal is kept. + let discarded = held.discard(&dir.handle); + match step { + Ok(Step::Unchanged(outcome)) if discarded => settle(dir, Step::Changed(outcome), observer), + Ok(step) => settle(dir, step, observer), + Err(error) => { + if discarded { + sync_after_failure(dir, observer); + } + Err(error) + } + } } /// Looks at what a crash or a refused put-back left in `held`, authenticates it again and unlinks it @@ -425,7 +437,7 @@ fn leftover_step( let (file, identity) = match held.inspect(EntryKind::File)? { // An empty quarantine directory is junk and goes with `discard`. Inside::Absent => return Ok(Step::Changed(RemovalOutcome::Vanished)), - Inside::Foreign(_) => return Ok(Step::Unchanged(RemovalOutcome::Refused)), + Inside::Foreign(_) => return leftover_directory(held, dir, observer), Inside::Object(file, identity) => (file, identity), }; let mut pinned = PinnedFile { file, identity }; @@ -439,6 +451,32 @@ fn leftover_step( Ok(unlinked(outcome, held.sync())) } +/// A leftover that is not a file. An empty directory, which a crashed directory removal leaves, holds +/// nothing, so it needs no authentication and goes with the same unlink and the same check as the removal +/// of a directory (Linux only, see [`remove_dir`]). Anything else stays and is refused: a link, a FIFO, a +/// directory on a system that cannot check the removal, and a directory that has entries (`NotEmpty`). +fn leftover_directory( + held: &Quarantine, + dir: &PinnedDir, + observer: Observer<'_>, +) -> Result { + let Inside::Object(directory, _) = held.inspect(EntryKind::Directory)? else { + return Ok(Step::Unchanged(RemovalOutcome::Refused)); + }; + if !cfg!(target_os = "linux") { + return Ok(Step::Unchanged(RemovalOutcome::Refused)); + } + observer(RemovalStage::Identified { + quarantined: &held.path_inside(&dir.path), + })?; + Ok( + match unlink_verified(held, &directory, EntryKind::Directory)? { + RemovalOutcome::NotEmpty => Step::Unchanged(RemovalOutcome::NotEmpty), + outcome => unlinked(outcome, held.sync()), + }, + ) +} + /// Removes the entry of `target` from `dir` if, once moved aside, it is the object it must be. fn remove_moved( dir: &PinnedDir, @@ -533,13 +571,15 @@ impl Removal<'_> { } } - /// The unlink was refused because the directory has entries. What is inside goes back where it was; - /// it is the authenticated directory that filled after the check only if it still is that directory - /// (the open one keeps its identity unique). If an actor swapped a non-empty directory in, the refusal - /// was about that one and says nothing about ours: the outcome is tampering, not `NotEmpty`. + /// The unlink was refused because the directory has entries. What is inside goes back where it was, + /// and is looked at under its name: it is the authenticated directory that filled after the check only + /// if it is that directory (the open one keeps its identity unique). If an actor swapped another + /// directory in, before it went back or while it did, the refusal was about that one and says nothing + /// about ours: the outcome is tampering, not `NotEmpty`. fn filled(&self) -> Result { - let authenticated = self.quarantine.holds(self.target.expected); self.restore()?; + let authenticated = + identity_at(&self.dir.handle, self.target.name) == Some(self.target.expected); let held = self.quarantine.sync(); Ok(if authenticated { Step::Moved(RemovalOutcome::NotEmpty, held) diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index a71d13d6b..1473434a0 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1857,7 +1857,11 @@ one, and the removal says so; it holds no data. hexadecimal digits; any other name, including an unrelated private directory, is `InvalidName` and untouched), requires a directory of the effective user without access for group or others, authenticates the file in it again and, only if accepted, unlinks it with the same link-count check. A file that does not authenticate (a foreign object left by a refused -put-back) stays; an empty generated quarantine directory is removed. +put-back) stays; an empty generated quarantine directory is removed. An empty directory that a crashed directory removal left as +`x` holds nothing, so it needs no authentication (the callback is not asked) and goes with the same unlink and the same check as +the removal of a directory (Linux; elsewhere it is refused); a directory with entries stays (`NotEmpty`). Removing the empty +quarantine directory changes the parent whatever the leftover removal came to (an actor can take the entry away while it is +looked at), so the parent is synced then too, for a refusal and for an error, and the result is kept. **Proof.** The directory cases run on Linux; on any other Unix system the same call is pinned to `Unsupported` with nothing moved. A directory swapped in the first window and one filled after the check are put back and not removed; an empty directory that wins the last window gives `Disturbed` with the original directory intact, and so does a non-empty one (not @@ -1870,9 +1874,11 @@ never removed by it; an entry stranded by a blocked put-back (the name taken aga directory) is finished after the file authenticates again. The leftover removal syncs the quarantine directory it unlinks in, like any removal, and says `NotConfirmed` if that fails (modelled through the same test-only seam). Mutation-checked: syncing nothing in the leftover removal, not refusing a foreign entry, loosening the name grammar to any length, and -ignoring the authentication result, and reporting `NotEmpty` for any directory fail at least one of these cases. An entry +ignoring the authentication result, reporting `NotEmpty` for any directory, not looking at a directory again once it went back, +not recovering an empty directory leftover, and ignoring that the discard removed the quarantine directory each fail at least one +of these cases. An entry stranded by a lost search permission is `Disturbed`, not `Stranded`: a failed look proves nothing (see part 2). **Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed. -The leftover recovery reclaims **files**: an empty directory that a crashed directory removal left in a `.h1q-*` directory -(no data, but no recovery for it) stays there and is never touched by a sweep; a recovery for it is not part of this slice. +A foreign entry that is neither a file nor an empty directory (a link, a FIFO) stays in its `.h1q-*` directory and is never +touched by a sweep. From 3a0835089e38a422f431381c730e4d3b0a8dfa7e Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:42:09 +0200 Subject: [PATCH 9/9] docs(core): say that the recovery refuses a link or a FIFO and that only an explicit call takes a leftover (#445) A leftover quarantine directory is not removed by a sweep; the explicit recovery takes a file or an empty directory and refuses anything else, which stays. Wording only (CodeRabbit on ec7ba931), plus one spelling nit in the changelog. No code change. --- CHANGELOG.md | 2 +- docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d005de794..7de359c01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -- **R-15 Gate 4:** removal of a directory on Linux, the recovery of leftovers and the crash cases on Unix, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; macOS reports no usable signal for a removed directory that is still open (hosted probe: link count and `F_GETPATH` unchanged), so there an empty directory answers `Unsupported` before anything is moved; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery also removes an empty directory left by a crashed directory removal (Linux; it holds nothing, so it is not authenticated), and syncs the parent when it removes the emptied quarantine directory, whatever the removal came to. No new `unsafe`, no new package. Issue #359. PR #1074. +- **R-15 Gate 4:** removal of a directory on Linux, the recovery of leftovers and the crash cases on Unix, `test-support` only (slice H1a-2, part 3). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterward; macOS reports no usable signal for a removed directory that is still open (hosted probe: link count and `F_GETPATH` unchanged), so there an empty directory answers `Unsupported` before anything is moved; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a blocked put-back and a foreign object left by a refused put-back are covered; the held directory is synced for what the unlink changed in it, like any removal; a non-empty directory swapped in the last window is `Disturbed`, not `NotEmpty`. The recovery also removes an empty directory left by a crashed directory removal (Linux; it holds nothing, so it is not authenticated), and syncs the parent when it removes the emptied quarantine directory, whatever the removal came to. No new `unsafe`, no new package. Issue #359. PR #1074. - **R-15 Gate 4:** hardening of the Unix file removal against a same-user actor in the quarantine directory, `test-support` only (slice H1a-2, part 2b of 5). The parent is synced after a rollback, a stranded entry, a put-back and a vanished name, and the quarantine directory an entry moved through is synced too (a sync of it that fails is an error after a put-back, a kept entry or an entry an actor removed again, and makes a removal `NotConfirmed`, never another outcome); the quarantine directory, also when its setup fails, is removed by its name only after a check that the name still leads to the directory that was created (the check narrows the race and does not close it, since `rmdir` takes a name: a lost race removes at most an empty directory); an entry, a foreign one included, is reported as retained only if the recorded name still leads to its directory and the entry in it was looked at and is the one that was seen, otherwise `Disturbed` (a look at a name, so it detects after the fact and does not prevent; a foreign entry is not opened, so nothing holds its identity unique and a reused identity is not noticed); a quarantine directory that cannot be opened is refused. Tests force each case, including the quarantine renamed away and the put-back blocked. No new `unsafe`, no new package. Issue #359. PR #1073. diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index 1473434a0..37a8a22db 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1852,7 +1852,8 @@ it falls to zero). An empty foreign directory that wins the race in the last win one, and the removal says so; it holds no data. **Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, -`Stranded`), leaves a `.h1q-*` directory holding the entry as `x`. A sweep never removes it by itself. +`Stranded`), leaves a `.h1q-*` directory holding the entry as `x`. No sweep removes it by itself (a sweep skips the +`.h1q-` prefix); only an explicit call of the recovery below takes it. `remove_leftover_file(name, authenticate)` takes it by its **generated name only** (`.h1q-` and exactly 32 lowercase hexadecimal digits; any other name, including an unrelated private directory, is `InvalidName` and untouched), requires a directory of the effective user without access for group or others, authenticates the file in it again and, only if @@ -1880,5 +1881,5 @@ of these cases. An entry stranded by a lost search permission is `Disturbed`, not `Stranded`: a failed look proves nothing (see part 2). **Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed. -A foreign entry that is neither a file nor an empty directory (a link, a FIFO) stays in its `.h1q-*` directory and is never -touched by a sweep. +A foreign entry that is neither a file nor an empty directory (a link, a FIFO) stays in its `.h1q-*` directory: the recovery +refuses it (`Refused`) and no sweep touches it, so nothing removes it until a person decides.