feat(core): harden the Unix file removal against a same-user actor in the quarantine directory (#445)

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:** 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 if the name still leads to the directory that was created; an entry is reported as retained only if the recorded name still leads to its directory, otherwise `Disturbed`; 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 #1072.
+
 - **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.

crates/worldscript-secure-storage/src/pinned/removal.rs

@@ -51,27 +51,37 @@ pub const QUARANTINE_PREFIX: &str = ".h1q-";
 pub enum RemovalOutcome {
     /// The authenticated object lost this name (on Unix as far as the check after the unlink can tell,
     /// see [`Disturbed`](Self::Disturbed)); the durability of the directory entry change as far as the
-    /// platform can confirm it.
+    /// platform can confirm it. On Unix that covers the directory and the quarantine directory the
+    /// object was unlinked from: if the latter could not be synced the removal is `NotConfirmed`.
     Removed(DirectoryDurability),
     /// The authentication said no; nothing was changed.
     Refused,
-    /// The name no longer led to an object (someone else removed it); nothing was removed.
+    /// The name no longer led to an object (someone else removed it); nothing was removed. If the entry
+    /// had been moved aside and was gone from there, the quarantine directory was synced as for
+    /// [`Swapped`](Self::Swapped), and a sync that fails is an error.
     Vanished,
     /// The directory still has entries; nothing was removed.
     NotEmpty,
     /// Something other than the authenticated object was bound to the name. It was not removed: it is
     /// back under its name (`restored`) or, if the name was taken meanwhile, kept in the quarantine
-    /// directory (`.h1q-*`, never removed by a sweep).
+    /// directory (`.h1q-*`, never removed by a sweep). Reported only after the directory and the
+    /// quarantine directory the entry moved through were synced. A sync that fails is an error and not
+    /// another outcome: the rename decided `restored`, and the entry is back or aside whatever the sync
+    /// did.
     Swapped { restored: bool },
-    /// Unix only: after the unlink the authenticated object had not lost a name, so the name was bound
-    /// to another object when it was unlinked (see the window in the module documentation). That
-    /// object is gone and nothing can be undone: the caller must treat the directory as tampered with,
-    /// and never count this as a success.
+    /// Unix only: the quarantine directory was tampered with. Either the authenticated object had not
+    /// lost a name after the unlink, so the name was bound to another object when it was unlinked (see
+    /// the window in the module documentation), and that object is gone; or the entry could not be put
+    /// back and is not where the outcome would have said: an actor renamed the quarantine directory
+    /// away, removed the entry, or replaced it with another object, so the authenticated object cannot
+    /// be found by the name that was recorded. Nothing can be undone: the caller must treat the
+    /// directory as tampered with, and never count this as a success.
     Disturbed,
     /// Unix only: an error happened after the entry was moved into the quarantine directory and it
     /// could not be put back (`cause` is the error that started it). The entry is in the quarantine
     /// directory (`.h1q-*`, as `x`), found with a listing; [`PinnedDir::remove_leftover_file`] takes it
-    /// from there after authenticating it again. Nothing was removed.
+    /// from there after authenticating it again. Nothing was removed. This reports a failure by its
+    /// cause, so both directories are synced best effort and a crash may undo where the entry is.
     Stranded { cause: PinnedError },
 }
 

crates/worldscript-secure-storage/src/pinned/tests/mod.rs

@@ -10,6 +10,10 @@ mod read;
 mod remove;
 #[cfg(all(feature = "test-support", unix))]
 mod remove_unix;
+#[cfg(all(feature = "test-support", unix))]
+mod remove_unix_cleanup;
+#[cfg(all(feature = "test-support", unix))]
+mod remove_unix_sync;
 #[cfg(windows)]
 mod windows_redirect;