feat(core): remove on Windows through the authenticated handle (#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:** removal on Windows, `test-support` only (slice H1a-2, part 4 of 4). The object is opened through the handle-relative lookup with delete access and **read-only sharing** (no handle that opens the file can write to it while the removal holds it, with the exception of a writable mapped view that existed before, see below; a writer that already has it open makes the open fail `Busy`), authenticated on that handle, and then given the POSIX delete disposition on the same handle: no `FILE_FLAG_DELETE_ON_CLOSE`, no fallback, a read-only file is refused. One new `unsafe` call (the disposition, one fixed structure, layout pinned by a test); a counter-proof shows what write sharing would have let through. The cases of `remove.rs` now run on Windows too, every code that means "no POSIX delete" (including `ERROR_INVALID_FUNCTION`) is `Unsupported`, and a directory is looked at again right before the disposition so that one converted into a junction meanwhile is left alone (`Redirected`; narrows the window, does not close it). A writable memory-mapped view that another process created earlier and keeps after closing its handle is not stopped by the sharing rule (same-user residual, stated, not closed); on Unix, which has no sharing mode, any same-user writer can change the bytes between the authentication and the removal, which was not stated before: the removal binds the object, not its bytes. No new package. Issue #359. PR #1075.
+
 - **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.

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

@@ -4,11 +4,12 @@
 //! **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 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.
+//! **Status of this slice.** The API and its isolation; on Unix the removal of files and of the leftovers
+//! of a crash, on Linux the removal of directories, and on Windows the removal of files and directories
+//! through the authenticated handle. The removal of a directory on any other Unix system, and the
+//! leftover recovery on Windows (which creates no quarantine directory), answer
+//! [`PinnedError::Unsupported`] (see [`PinnedDir::remove_empty_dir`]), so nothing is removed there and
+//! nothing claims a safety it does not have.
 //!
 //! **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
@@ -36,6 +37,13 @@
 //!
 //! A hard link is one name: removing it leaves the other names. [`PinnedFile::link_count`] lets a caller
 //! refuse a file with more than one.
+//!
+//! **What is bound is the object, not its bytes.** `authenticate` reads the open file, and the removal
+//! then removes that object. On Unix there is no sharing mode, so a process of the same user that has a
+//! writable descriptor or a writable shared mapping of the file can change its bytes between the two. On
+//! Windows the read-only share mode stops every writer that opens the file, but not a writable
+//! memory-mapped view that another process created earlier and keeps after closing its handle. Neither is
+//! closed here; both belong to the same-user residual that is decided before any production use.
 
 use std::ffi::OsStr;
 use std::path::Path;

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

@@ -18,6 +18,8 @@ mod remove_unix_cleanup;
 mod remove_unix_dir;
 #[cfg(all(feature = "test-support", unix))]
 mod remove_unix_sync;
+#[cfg(all(feature = "test-support", windows))]
+mod remove_windows;
 #[cfg(windows)]
 mod windows_redirect;
 

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

@@ -1,27 +1,21 @@
-//! Removal on every platform that implements it: only the authenticated object goes, and a refusal
-//! changes nothing. Windows has no implementation in this part yet and keeps the refusal test.
+//! Removal on every platform: only the authenticated object goes, and a refusal changes nothing.
 
 use std::fs;
 
-#[cfg(unix)]
 use super::link_to_dir;
 use super::Scratch;
-#[cfg(unix)]
 use crate::durable::DirectoryDurability;
-#[cfg(unix)]
 use crate::pinned::PinnedFile;
 use crate::pinned::{PinnedDir, PinnedError, RemovalOutcome};
 
 /// Accepts a file whose whole content is `expected`.
-#[cfg(unix)]
 pub(super) fn holds(
     expected: &'static [u8],
 ) -> impl FnOnce(&mut PinnedFile) -> Result<bool, PinnedError> {
     move |file| Ok(file.read_at_most(64)? == expected)
 }
 
 /// What a completed removal reports on this platform.
-#[cfg(unix)]
 pub(super) fn removed() -> Result<RemovalOutcome, PinnedError> {
     let durability = if cfg!(unix) {
         DirectoryDurability::Confirmed
@@ -31,7 +25,6 @@ pub(super) fn removed() -> Result<RemovalOutcome, PinnedError> {
     Ok(RemovalOutcome::Removed(durability))
 }
 
-#[cfg(unix)]
 pub(super) fn names(pinned: &PinnedDir) -> Vec<String> {
     let mut listed: Vec<String> = pinned
         .entries(32)
@@ -43,7 +36,6 @@ pub(super) fn names(pinned: &PinnedDir) -> Vec<String> {
     listed
 }
 
-#[cfg(unix)]
 #[test]
 fn an_authenticated_file_is_removed_and_nothing_else_is() {
     let scratch = Scratch::new("remove");
@@ -58,7 +50,6 @@ fn an_authenticated_file_is_removed_and_nothing_else_is() {
     );
 }
 
-#[cfg(unix)]
 #[test]
 fn a_refused_authentication_a_failing_one_and_a_wrong_identity_remove_nothing() {
     let scratch = Scratch::new("refuse");
@@ -89,7 +80,6 @@ fn a_refused_authentication_a_failing_one_and_a_wrong_identity_remove_nothing()
     );
 }
 
-#[cfg(unix)]
 #[test]
 fn a_link_a_directory_and_a_missing_name_are_not_removed_as_files() {
     let scratch = Scratch::new("not-files");
@@ -119,7 +109,6 @@ fn a_link_a_directory_and_a_missing_name_are_not_removed_as_files() {
     );
 }
 
-#[cfg(unix)]
 #[test]
 fn removing_a_hard_linked_name_leaves_the_other_names() {
     let scratch = Scratch::new("hard-link");
@@ -147,7 +136,6 @@ 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");
@@ -159,9 +147,10 @@ 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") {
+    // Linux (a link count) and Windows (the delete is the disposition of the open handle) can tell that
+    // the directory they removed is the one they opened; on any other Unix system 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!(any(target_os = "linux", windows)) {
         (removed(), vec!["filled".to_string(), "link".to_string()])
     } else {
         (
@@ -194,26 +183,3 @@ fn an_empty_directory_is_removed_and_a_filled_one_or_a_link_is_not() {
         )
     );
 }
-
-#[cfg(windows)]
-#[test]
-fn every_removal_refuses_until_windows_implements_it_and_removes_nothing() {
-    let scratch = Scratch::new("remove-stub");
-    let root = scratch.root();
-    fs::write(root.join("a"), b"alpha").unwrap();
-    fs::create_dir(root.join("d")).unwrap();
-    let pinned = PinnedDir::open_root(&root).unwrap();
-    let refusals: [Result<RemovalOutcome, PinnedError>; 3] = [
-        pinned.remove_authenticated_file("a", |_| Ok(true)),
-        pinned.remove_empty_dir("d"),
-        pinned.remove_leftover_file(".h1q-00000000000000000000000000000000", |_| Ok(true)),
-    ];
-    assert_eq!(
-        (
-            refusals,
-            fs::read(root.join("a")).unwrap(),
-            root.join("d").is_dir()
-        ),
-        ([Err(PinnedError::Unsupported); 3], b"alpha".to_vec(), true)
-    );
-}