From 24683a83a1ab7ae341384bae6c7c76a264ee7afa Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:58:41 +0200 Subject: [PATCH] feat(core): remove a directory on Unix and finish the leftovers of a crash (#445) A directory is moved through the same quarantine directory, identified by the open directory and removed with AT_REMOVEDIR; one that fills after the check goes back. The link count of the held directory is checked afterwards, as for a file. A quarantine directory left behind by a crash or by an entry that could not be put back is taken by its generated name only, the file in it is authenticated again, and only an accepted file is removed. Names outside the generated grammar are refused and left alone. --- CHANGELOG.md | 2 + .../src/pinned/tests/remove.rs | 34 ++++ .../src/pinned/tests/remove_unix.rs | 190 +++++++++++++++++- .../src/pinned/unix/removal.rs | 111 ++++++++-- .../r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md | 27 +++ 5 files changed, 342 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a72b160b..4aa4cb234 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 of 4). `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 lost permission and a foreign object left by a refused put-back are covered. No new `unsafe`, no new package. Issue #359. PR #1070. + - **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. - **R-15 Gate 4:** the API and the isolation of the bound removal, `test-support` only (slice H1a-2, part 1 of 4). `PinnedDir::remove_authenticated_file`, `remove_empty_dir` and `remove_leftover_file` with `RemovalOutcome`, `RemovalStage` and `Observer` exist only with the test-only feature, have no caller and, in this part, answer `Unsupported` and remove nothing; the platform implementations follow in three stacked parts. A scan of the sources and a compile of a dependent with and without the feature keep the primitives out of every other build. `PinnedDir::sync` (no deletion) is added. The contract states what no Unix primitive can do (unlink "this inode": a same-user actor in the quarantine directory can win one race, which is detected after the fact as `Disturbed`, not prevented) and leaves the decision about that residual to the maintainer before any sweep uses it. No new `unsafe`, no new dependency feature, no new package. Issue #359. PR #1068. 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 02a315151..736ffed2f 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs @@ -1,4 +1,4 @@ -//! Unix removal of a file under the swap the design exists for: another actor replaces the last +//! Unix removal of a file, a directory and of a leftover quarantine directory under the swap the design exists for: another actor replaces the last //! component of the name, 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 (by identity where the content could be mistaken). The counter-proofs show that the swap @@ -7,6 +7,7 @@ use std::fs; use std::os::fd::AsFd; use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; +use std::panic::{catch_unwind, AssertUnwindSafe}; use std::path::{Path, PathBuf}; use rustix::fs::{unlinkat, AtFlags}; @@ -555,3 +556,190 @@ fn an_error_after_the_move_that_cannot_be_rolled_back_is_reported_as_stranded() ) ); } + +#[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_is_taken_by_its_generated_name_only_and_an_empty_one_is_removed() { + 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)); + assert_eq!( + ( + taken("a"), + taken("not-a-quarantine"), + taken(".h1q-empty"), + taken(&format!("{QUARANTINE_PREFIX}{}", "G".repeat(32))), + taken(&generated), + names(&f.pinned) + ), + ( + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + 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() { + assert!( + !rustix::process::geteuid().is_root(), + "the permission checks of this case do not apply to root; run the tests as a normal user" + ); + let f = fixture("stranded-recovery"); + let stranded = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, lock_the_quarantine), + ); + let left = quarantines(&f.root); + fs::set_permissions(&left[0], fs::Permissions::from_mode(0o700)).unwrap(); + let name = left[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.is_ok(), refused, finished, names(&f.pinned)), + ( + true, + Ok(RemovalOutcome::Refused), + removed(), + Vec::::new() + ) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 88333ca43..58848fbbc 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}; @@ -25,6 +25,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 { @@ -53,6 +56,17 @@ enum Step { Unlinked, } +/// 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]; @@ -84,6 +98,21 @@ impl Quarantine { } } + /// An existing quarantine directory of `parent`, found by its generated name. + 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); + } + Ok(Quarantine { + fd, + name: name.to_os_string(), + }) + } + /// The directory `name`, if it is a directory of this process's user (one of another user that was /// swapped in is refused; the same user is not kept out, which the removal does not rely on). fn open_owned(parent: &DirHandle, name: &OsStr) -> Result { @@ -239,6 +268,65 @@ 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) +} + +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), + })?; + Ok(unlinked(unlink_verified( + held, + &pinned.file, + EntryKind::File, + )?)) +} + /// Removes the entry of `target` from `dir` if, once moved aside, it is the object it must be. fn remove_moved( dir: &PinnedDir, @@ -338,22 +426,3 @@ 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) -} diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index c20255dc8..a04939052 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1780,3 +1780,30 @@ failed inspection fails the `Stranded` case. **Not claimed.** Directories and the leftover recovery are part 3 (they answer `Unsupported` here); 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`, non-hexadecimal digits, other directories, plain files) are refused; a foreign object left in a quarantine +directory by a refused put-back is never removed by it; an entry stranded by a quarantine directory that lost its search +permission is finished after the permission is back and the file authenticates again. + +**Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed.