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 writer can change the authenticated bytes; 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. No new package. Issue #359. PR #1071.
+
 - **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.

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

@@ -10,6 +10,8 @@ mod read;
 mod remove;
 #[cfg(all(feature = "test-support", unix))]
 mod remove_unix;
+#[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");
@@ -180,26 +168,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)
-    );
-}

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

@@ -0,0 +1,212 @@
+//! Windows removal: while the authenticated handle is held the name cannot be rebound and the bytes
+//! cannot be written, a competing handle makes the removal fail closed, and the delete is a disposition
+//! of the authenticated handle. The counter-proof shows what a share mode that allowed writing would
+//! have let through.
+
+use std::ffi::OsStr;
+use std::fs::{self, File, OpenOptions};
+use std::io::Read;
+use std::mem::{align_of, size_of};
+
+use windows_sys::Win32::Foundation::ERROR_SHARING_VIOLATION;
+use windows_sys::Win32::Storage::FileSystem::{
+    DELETE, FILE_DISPOSITION_INFO_EX, FILE_GENERIC_READ, FILE_SHARE_READ, FILE_SHARE_WRITE,
+};
+
+use super::remove::{holds, names, removed};
+use super::Scratch;
+use crate::pinned::windows::nt;
+use crate::pinned::{PinnedDir, PinnedError, RemovalOutcome, RemovalStage};
+
+/// Whether opening `path` for writing is refused because another handle holds the file.
+fn writer_is_refused(path: &std::path::Path) -> bool {
+    OpenOptions::new()
+        .write(true)
+        .open(path)
+        .is_err_and(|error| error.raw_os_error() == Some(ERROR_SHARING_VIOLATION as i32))
+}
+
+#[test]
+fn while_the_authenticated_handle_is_held_the_name_cannot_be_rebound() {
+    let scratch = Scratch::new("win-rebind");
+    let (root, aside) = (scratch.root(), scratch.aside());
+    fs::write(root.join("a"), b"alpha").unwrap();
+    fs::write(aside.join("other"), b"other").unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let mut attempts = None;
+    let outcome = pinned.remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| {
+        if stage == RemovalStage::Checked {
+            attempts = Some((
+                fs::rename(root.join("a"), aside.join("original")).is_err(),
+                fs::remove_file(root.join("a")).is_err(),
+                fs::rename(aside.join("other"), root.join("a")).is_err(),
+            ));
+        }
+        Ok(())
+    });
+    assert_eq!(
+        (
+            outcome,
+            attempts,
+            root.join("a").exists(),
+            fs::read(aside.join("other")).unwrap()
+        ),
+        (
+            removed(),
+            Some((true, true, true)),
+            false,
+            b"other".to_vec()
+        )
+    );
+}
+
+#[test]
+fn nobody_can_write_to_the_file_between_the_authentication_and_the_delete() {
+    let scratch = Scratch::new("win-no-writer");
+    let root = scratch.root();
+    let path = root.join("a");
+    fs::write(&path, b"alpha").unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let (mut during_authentication, mut at_the_check) = (None, None);
+    let outcome = pinned.remove_authenticated_file_observed(
+        "a",
+        |file| {
+            during_authentication = Some(writer_is_refused(&path));
+            Ok(file.read_at_most(8)? == b"alpha")
+        },
+        &mut |stage| {
+            if stage == RemovalStage::Checked {
+                at_the_check = Some(writer_is_refused(&path));
+            }
+            Ok(())
+        },
+    );
+    assert_eq!(
+        (outcome, during_authentication, at_the_check, path.exists()),
+        (removed(), Some(true), Some(true), false)
+    );
+}
+
+#[test]
+fn counter_proof_a_handle_that_shared_write_would_let_a_writer_change_the_bytes() {
+    let scratch = Scratch::new("win-shared-write");
+    let root = scratch.root();
+    fs::write(root.join("a"), b"alpha").unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let mut held = nt::open_relative(
+        &pinned.handle,
+        OsStr::new("a"),
+        FILE_GENERIC_READ | DELETE,
+        FILE_SHARE_READ | FILE_SHARE_WRITE,
+    )
+    .unwrap();
+    // The write succeeds although the delete handle is held: with this share mode the bytes that were
+    // authenticated could be replaced before the disposition.
+    fs::write(root.join("a"), b"changed").unwrap();
+    let mut seen = Vec::new();
+    held.read_to_end(&mut seen).unwrap();
+    assert_eq!(seen, b"changed".to_vec());
+}
+
+#[test]
+fn a_writer_that_already_has_the_file_open_makes_the_removal_busy_and_changes_nothing() {
+    let scratch = Scratch::new("win-busy");
+    let root = scratch.root();
+    fs::write(root.join("a"), b"alpha").unwrap();
+    // std shares read, write and delete; what matters is that this handle can write.
+    let writer = OpenOptions::new().write(true).open(root.join("a")).unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let outcome = pinned.remove_authenticated_file("a", |_| Ok(true));
+    drop(writer);
+    assert_eq!(
+        (outcome, fs::read(root.join("a")).unwrap()),
+        (Err(PinnedError::Busy), b"alpha".to_vec())
+    );
+}
+
+#[test]
+fn a_reader_that_shares_delete_does_not_stop_the_removal_and_the_name_is_free_at_once() {
+    let scratch = Scratch::new("win-share-delete");
+    let root = scratch.root();
+    fs::write(root.join("a"), b"alpha").unwrap();
+    let reader = File::open(root.join("a")).unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let outcome = pinned.remove_authenticated_file("a", holds(b"alpha"));
+    let name_free = File::create_new(root.join("a")).is_ok();
+    drop(reader);
+    assert_eq!((outcome, name_free), (removed(), true));
+}
+
+#[test]
+fn a_directory_is_removed_only_while_empty_and_its_name_cannot_be_rebound_meanwhile() {
+    let scratch = Scratch::new("win-dir");
+    let (root, aside) = (scratch.root(), scratch.aside());
+    fs::create_dir(root.join("d")).unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let mut rebound = None;
+    let outcome = pinned.remove_empty_dir_observed("d", &mut |stage| {
+        if stage == RemovalStage::Checked {
+            rebound = Some(fs::rename(root.join("d"), aside.join("moved")).is_err());
+        }
+        Ok(())
+    });
+    assert_eq!(
+        (outcome, rebound, root.join("d").exists()),
+        (removed(), Some(true), false)
+    );
+}
+
+#[test]
+fn a_read_only_file_is_not_deleted_and_the_refusal_is_reported() {
+    let scratch = Scratch::new("win-readonly");
+    let root = scratch.root();
+    let path = root.join("a");
+    fs::write(&path, b"alpha").unwrap();
+    let mut permissions = fs::metadata(&path).unwrap().permissions();
+    permissions.set_readonly(true);
+    fs::set_permissions(&path, permissions.clone()).unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let outcome = pinned.remove_authenticated_file("a", holds(b"alpha"));
+    // The attribute is cleared with the permissions of a file that is not read-only.
+    fs::set_permissions(&path, fs::metadata(&root).unwrap().permissions()).unwrap();
+    assert_eq!(
+        (outcome.is_err(), fs::read(&path).unwrap()),
+        (true, b"alpha".to_vec())
+    );
+}
+
+#[test]
+fn a_leftover_removal_is_unsupported_because_windows_creates_no_quarantine() {
+    let scratch = Scratch::new("win-leftover");
+    let pinned = PinnedDir::open_root(&scratch.root()).unwrap();
+    assert_eq!(
+        pinned.remove_leftover_file(".h1q-0", |_| Ok(true)),
+        Err(PinnedError::Unsupported)
+    );
+}
+
+#[test]
+fn the_disposition_structure_has_the_layout_the_system_reads() {
+    assert_eq!(
+        (
+            size_of::<FILE_DISPOSITION_INFO_EX>(),
+            align_of::<FILE_DISPOSITION_INFO_EX>()
+        ),
+        (4, 4)
+    );
+}
+
+#[test]
+fn a_name_is_listed_only_if_nothing_else_was_changed() {
+    let scratch = Scratch::new("win-names");
+    let root = scratch.root();
+    fs::write(root.join("a"), b"alpha").unwrap();
+    fs::write(root.join("b"), b"beta").unwrap();
+    let pinned = PinnedDir::open_root(&root).unwrap();
+    let outcome: Result<RemovalOutcome, PinnedError> =
+        pinned.remove_authenticated_file("a", holds(b"alpha"));
+    assert_eq!(
+        (outcome, names(&pinned)),
+        (removed(), vec!["b".to_string()])
+    );
+}