chore(fspy): cleanup unused code on Windows (#37)

### TL;DR

Improved file access tracking for Windows process creation and fixed path access detection in tests.

### What changed?

- Renamed `subprocess` test to `read_in_subprocess` and added a new `read_program` test to verify program file access detection
- Enhanced `assert_contains` in test utilities to properly handle access mode comparison
- Added Windows-specific tests for `CreateProcessA` and `CreateProcessW` functions
- Removed unused `stack_once.rs` module and simplified process creation hooks
- Fixed a re-entrance detection issue in process creation hooks by using thread-local state
- Simplified `NativeStr` by removing the unused ANSI string support, as Windows paths are primarily handled as wide strings

### How to test?

- Run the existing and new tests on Windows to verify process creation hooks work correctly
- Verify that file access tracking properly detects program file access when creating processes

### Why make this change?

The previous implementation had issues with detecting program file access during process creation on Windows. This change improves the reliability of file access tracking by simplifying the hook implementation and removing unnecessary complexity. The tests now properly verify that the system correctly tracks file access when creating processes, which is essential for accurate dependency tracking.

Author: branchseer

Cargo.lock

@@ -47,7 +47,7 @@ async fn readdir() -> anyhow::Result<()> {
 }
 
 #[test(tokio::test)]
-async fn subprocess() -> anyhow::Result<()> {
+async fn read_in_subprocess() -> anyhow::Result<()> {
     let accesses = track_child!((), |(): ()| {
         let mut command = if cfg!(windows) {
             let mut command = std::process::Command::new("cmd");
@@ -72,3 +72,18 @@ async fn subprocess() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[test(tokio::test)]
+async fn read_program() -> anyhow::Result<()> {
+    let accesses = track_child!((), |(): ()| {
+        let _ = std::process::Command::new("./not_exist.exe").spawn();
+    })
+    .await?;
+    assert_contains(
+        &accesses,
+        current_dir().unwrap().join("not_exist.exe").as_path(),
+        AccessMode::READ,
+    );
+
+    Ok(())
+}

crates/fspy/tests/rust_std.rs

@@ -11,25 +11,34 @@ pub fn assert_contains(
     expected_path: &Path,
     expected_mode: AccessMode,
 ) {
-    let found = accesses.iter().any(|access| {
+    let mut actual_mode: AccessMode = AccessMode::empty();
+    for access in accesses.iter() {
         let Ok(stripped) =
             access.path.strip_path_prefix::<_, Result<PathBuf, StripPrefixError>, _>(
                 expected_path,
                 |strip_result| strip_result.map(Path::to_path_buf),
             )
         else {
-            return false;
+            continue;
         };
-        stripped.as_os_str().is_empty() && access.mode == expected_mode
-    });
-    if !found {
-        panic!(
-            "Expected to find access to path {:?} with mode {:?}, but it was not found in: {:?}",
-            expected_path,
-            expected_mode,
-            accesses.iter().collect::<Vec<_>>()
-        );
+        if stripped.as_os_str().is_empty() {
+            actual_mode.insert(access.mode);
+        }
     }
+
+    if actual_mode.contains(AccessMode::READ_DIR) {
+        // READ_DIR already implies READ.
+        actual_mode.remove(AccessMode::READ);
+    }
+
+    assert_eq!(
+        expected_mode,
+        actual_mode,
+        "Expected to find access to path {:?} with mode {:?}, but it was not found in: {:?}",
+        expected_path,
+        expected_mode,
+        accesses.iter().collect::<Vec<_>>()
+    );
 }
 
 #[macro_export]

crates/fspy/tests/test_utils.rs

@@ -0,0 +1,66 @@
+#![cfg(windows)]
+
+mod test_utils;
+
+use std::{ffi::OsStr, os::windows::ffi::OsStrExt, path::Path, ptr::null_mut};
+
+use fspy::AccessMode;
+use test_log::test;
+use test_utils::assert_contains;
+use winapi::um::processthreadsapi::{
+    CreateProcessA, CreateProcessW, PROCESS_INFORMATION, STARTUPINFOA, STARTUPINFOW,
+};
+
+#[test(tokio::test)]
+async fn create_process_a() -> anyhow::Result<()> {
+    let accesses = track_child!((), |(): ()| {
+        let mut si: STARTUPINFOA = unsafe { std::mem::zeroed() };
+        let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() };
+        unsafe {
+            CreateProcessA(
+                c"C:\\fspy_test_not_exist_program.exe".as_ptr().cast(),
+                null_mut(),
+                null_mut(),
+                null_mut(),
+                0,
+                0,
+                null_mut(),
+                null_mut(),
+                &mut si,
+                &mut pi,
+            )
+        };
+    })
+    .await?;
+    assert_contains(&accesses, Path::new("C:\\fspy_test_not_exist_program.exe"), AccessMode::READ);
+
+    Ok(())
+}
+
+#[test(tokio::test)]
+async fn create_process_w() -> anyhow::Result<()> {
+    let accesses = track_child!((), |(): ()| {
+        let mut si: STARTUPINFOW = unsafe { std::mem::zeroed() };
+        let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() };
+        let program =
+            OsStr::new("C:\\fspy_test_not_exist_program.exe\0").encode_wide().collect::<Vec<u16>>();
+        unsafe {
+            CreateProcessW(
+                program.as_ptr(),
+                null_mut(),
+                null_mut(),
+                null_mut(),
+                0,
+                0,
+                null_mut(),
+                null_mut(),
+                &mut si,
+                &mut pi,
+            )
+        };
+    })
+    .await?;
+    assert_contains(&accesses, Path::new("C:\\fspy_test_not_exist_program.exe"), AccessMode::READ);
+
+    Ok(())
+}

crates/fspy/tests/winapi.rs

@@ -1,5 +1,4 @@
 #![cfg(windows)]
 #![feature(sync_unsafe_cell)]
 
-mod stack_once;
 pub mod windows;

crates/fspy_preload_windows/src/lib.rs

@@ -8,29 +8,11 @@ use fspy_shared::{
 };
 use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE};
 
-use crate::stack_once::{StackOnceGuard, stack_once_token};
-
 pub struct Client<'a> {
     payload: Payload<'a>,
     ipc_sender: Option<Sender>,
 }
 
-stack_once_token!(PATH_ACCESS_ONCE);
-
-pub struct PathAccessSender<'a> {
-    ipc_sender: &'a Option<Sender>,
-    _once_guard: StackOnceGuard,
-}
-
-impl<'a> PathAccessSender<'a> {
-    pub fn send(&self, access: PathAccess<'_>) {
-        let Some(sender) = &self.ipc_sender else {
-            return;
-        };
-        sender.write_encoded(&access, BINCODE_CONFIG).expect("failed to send path access");
-    }
-}
-
 impl<'a> Client<'a> {
     pub fn from_payload_bytes(payload_bytes: &'a [u8]) -> Self {
         let (payload, decoded_len) =
@@ -58,11 +40,6 @@ impl<'a> Client<'a> {
         sender.write_encoded(&access, BINCODE_CONFIG).expect("failed to send path access");
     }
 
-    pub fn sender(&self) -> Option<PathAccessSender<'_>> {
-        let guard = PATH_ACCESS_ONCE.try_enter()?;
-        Some(PathAccessSender { ipc_sender: &self.ipc_sender, _once_guard: guard })
-    }
-
     pub unsafe fn prepare_child_process(&self, child_handle: HANDLE) -> BOOL {
         let payload_bytes = encode_to_vec(&self.payload, BINCODE_CONFIG).unwrap();
         unsafe {

crates/fspy_preload_windows/src/stack_once.rs

@@ -1,8 +1,4 @@
-use std::ffi::CStr;
-
 use fspy_detours_sys::{DetourCreateProcessWithDllExA, DetourCreateProcessWithDllExW};
-use fspy_shared::ipc::{AccessMode, NativeStr, PathAccess};
-use widestring::U16CStr;
 use winapi::{
     shared::{
         minwindef::{BOOL, DWORD, LPVOID},
@@ -24,6 +20,29 @@ use crate::windows::{
     detour::{Detour, DetourAny},
 };
 
+thread_local! {
+    static IS_HOOKING_CREATE_PROCESS: std::cell::Cell<bool> = std::cell::Cell::new(false);
+}
+
+struct HookGuard;
+impl HookGuard {
+    pub fn new() -> Option<Self> {
+        let already_hooking = IS_HOOKING_CREATE_PROCESS.with(|c| {
+            let v = c.get();
+            c.set(true);
+            v
+        });
+        if already_hooking { None } else { Some(Self) }
+    }
+}
+impl Drop for HookGuard {
+    fn drop(&mut self) {
+        IS_HOOKING_CREATE_PROCESS.with(|c| {
+            c.set(false);
+        });
+    }
+}
+
 static DETOUR_CREATE_PROCESS_W: Detour<
     unsafe extern "system" fn(
         LPCWSTR,
@@ -51,8 +70,7 @@ static DETOUR_CREATE_PROCESS_W: Detour<
             lp_startup_info: LPSTARTUPINFOW,
             lp_process_information: LPPROCESS_INFORMATION,
         ) -> BOOL {
-            let client = unsafe { global_client() };
-            let Some(sender) = client.sender() else {
+            let Some(_hook_guard) = HookGuard::new() else {
                 // Detect re-entrance and avoid double hooking
                 return unsafe {
                     (DETOUR_CREATE_PROCESS_W.real())(
@@ -61,7 +79,7 @@ static DETOUR_CREATE_PROCESS_W: Detour<
                         lp_process_attributes,
                         lp_thread_attributes,
                         b_inherit_handles,
-                        dw_creation_flags | CREATE_SUSPENDED,
+                        dw_creation_flags,
                         lp_environment,
                         lp_current_directory,
                         lp_startup_info,
@@ -70,16 +88,7 @@ static DETOUR_CREATE_PROCESS_W: Detour<
                 };
             };
 
-            if !lp_application_name.is_null() {
-                unsafe {
-                    sender.send(PathAccess {
-                        mode: AccessMode::READ,
-                        path: NativeStr::from_wide(
-                            U16CStr::from_ptr_str(lp_application_name).as_slice(),
-                        ),
-                    });
-                }
-            }
+            let client = unsafe { global_client() };
 
             unsafe extern "system" fn create_process_with_payload_w(
                 lp_application_name: LPCWSTR,
@@ -175,8 +184,7 @@ static DETOUR_CREATE_PROCESS_A: Detour<
             lp_startup_info: LPSTARTUPINFOA,
             lp_process_information: LPPROCESS_INFORMATION,
         ) -> BOOL {
-            let client = unsafe { global_client() };
-            let Some(sender) = client.sender() else {
+            let Some(_hook_guard) = HookGuard::new() else {
                 // Detect re-entrance and avoid double hooking
                 return unsafe {
                     (DETOUR_CREATE_PROCESS_A.real())(
@@ -185,23 +193,15 @@ static DETOUR_CREATE_PROCESS_A: Detour<
                         lp_process_attributes,
                         lp_thread_attributes,
                         b_inherit_handles,
-                        dw_creation_flags | CREATE_SUSPENDED,
+                        dw_creation_flags,
                         lp_environment,
                         lp_current_directory,
                         lp_startup_info,
                         lp_process_information,
                     )
                 };
             };
-
-            if !lp_application_name.is_null() {
-                unsafe {
-                    sender.send(PathAccess {
-                        mode: AccessMode::READ,
-                        path: NativeStr::from_ansi(CStr::from_ptr(lp_application_name).to_bytes()),
-                    });
-                }
-            }
+            let client = unsafe { global_client() };
 
             unsafe extern "system" fn create_process_with_payload_a(
                 lp_application_name: LPCSTR,

crates/fspy_preload_windows/src/windows/client.rs

@@ -19,7 +19,6 @@ uuid = { workspace = true, features = ["v4"] }
 bytemuck = { workspace = true }
 os_str_bytes = { workspace = true }
 winapi = { workspace = true, features = ["std"] }
-winsafe = { workspace = true }
 
 [dev-dependencies]
 assert2 = { workspace = true }