From 091863db8882f937ff4b4765ea7fe1bd551372a2 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 5 Aug 2026 23:59:18 -0700 Subject: [PATCH 1/2] fix: harden process matching during updates Resolve running processes by executable launch name and installation directory before waiting or terminating them. Use cross-bitness-safe path queries, held process handles, and reliable termination checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Cargo.toml | 4 +- src/process.rs | 683 +++++++++++++++++++++++++++++++++++++------------ 2 files changed, 518 insertions(+), 169 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5d08650..b8581c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,8 +24,8 @@ features = [ "Win32_System_Diagnostics_Debug", "Win32_Storage_FileSystem", "Win32_Security", - "Win32_System_ProcessStatus", - "Win32_System_Diagnostics_ToolHelp" + "Win32_System_Diagnostics_ToolHelp", + "Win32_Globalization" ] [profile.release] diff --git a/src/process.rs b/src/process.rs index db6c3f1..7563de0 100644 --- a/src/process.rs +++ b/src/process.rs @@ -3,12 +3,44 @@ * Licensed under the MIT License. See LICENSE in the project root for license information. *----------------------------------------------------------------------------------------*/ -use std::ffi::c_void; -use std::path::{Path, PathBuf}; -use std::{error, io, mem, ptr, thread, time}; -use crate::strings::from_utf16; -use slog; +use crate::strings::{from_utf16, to_u16s}; use crate::util; +use std::ffi::OsString; +use std::os::windows::ffi::OsStringExt; +use std::path::{Path, PathBuf}; +use std::{error, io, mem, thread, time}; +use windows_sys::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE}; + +const MAX_PROCESS_PATH_LENGTH: usize = 32_768; + +struct OwnedHandle(HANDLE); + +impl OwnedHandle { + fn new(handle: HANDLE) -> io::Result { + if handle == 0 || handle == INVALID_HANDLE_VALUE { + Err(io::Error::last_os_error()) + } else { + Ok(Self(handle)) + } + } + + fn get(&self) -> HANDLE { + self.0 + } +} + +impl Drop for OwnedHandle { + fn drop(&mut self) { + unsafe { + CloseHandle(self.0); + } + } +} + +struct MatchingProcess { + process: RunningProcess, + handle: OwnedHandle, +} pub struct RunningProcess { pub name: String, @@ -16,21 +48,20 @@ pub struct RunningProcess { } pub fn get_running_processes() -> Result, io::Error> { - use windows_sys::Win32::Foundation::{CloseHandle, INVALID_HANDLE_VALUE}; + use windows_sys::Win32::Foundation::ERROR_NO_MORE_FILES; use windows_sys::Win32::System::Diagnostics::ToolHelp::{ - CreateToolhelp32Snapshot, Process32FirstW, Process32NextW, PROCESSENTRY32W, + CreateToolhelp32Snapshot, PROCESSENTRY32W, Process32FirstW, Process32NextW, TH32CS_SNAPPROCESS, }; unsafe { - let handle = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); - - if handle == INVALID_HANDLE_VALUE { - return Err(io::Error::new( - io::ErrorKind::Other, - "Could not create process snapshot", - )); - } + let handle = + OwnedHandle::new(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0)).map_err(|err| { + io::Error::new( + err.kind(), + format!("Could not create process snapshot: {}", err), + ) + })?; let mut pe32 = PROCESSENTRY32W { dwSize: 0, @@ -47,28 +78,31 @@ pub fn get_running_processes() -> Result, io::Error> { pe32.dwSize = mem::size_of::() as u32; - if Process32FirstW(handle, &mut pe32) == 0 { - CloseHandle(handle); - - return Err(io::Error::new( - io::ErrorKind::Other, - "Could not get first process data", - )); + if Process32FirstW(handle.get(), &mut pe32) == 0 { + return Err(io::Error::other(format!( + "Could not get first process data: {}", + io::Error::last_os_error() + ))); } let mut result: Vec = vec![]; loop { result.push(RunningProcess { - name: from_utf16(&pe32.szExeFile).inspect_err(|_| { - CloseHandle(handle); - })?, + name: from_utf16(&pe32.szExeFile)?, id: pe32.th32ProcessID, }); - if Process32NextW(handle, &mut pe32) == 0 { - CloseHandle(handle); - break; + if Process32NextW(handle.get(), &mut pe32) == 0 { + let err = io::Error::last_os_error(); + if err.raw_os_error() == Some(ERROR_NO_MORE_FILES as i32) { + break; + } + + return Err(io::Error::new( + err.kind(), + format!("Could not get next process data: {}", err), + )); } } @@ -76,155 +110,311 @@ pub fn get_running_processes() -> Result, io::Error> { } } +fn open_process(process_id: u32, access: u32) -> io::Result { + use windows_sys::Win32::System::Threading::OpenProcess; + + unsafe { OwnedHandle::new(OpenProcess(access, 0, process_id)) } +} + +fn get_process_path_from_handle(handle: HANDLE) -> io::Result { + use windows_sys::Win32::System::Threading::QueryFullProcessImageNameW; + + unsafe { + let mut raw_path = vec![0u16; MAX_PROCESS_PATH_LENGTH]; + let mut len = raw_path.len() as u32; + if QueryFullProcessImageNameW(handle, 0, raw_path.as_mut_ptr(), &mut len) == 0 { + return Err(io::Error::last_os_error()); + } + + raw_path.truncate(len as usize); + Ok(PathBuf::from(OsString::from_wide(&raw_path))) + } +} + +#[cfg(test)] +fn get_process_path(process_id: u32) -> io::Result { + use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; + + let handle = open_process(process_id, PROCESS_QUERY_LIMITED_INFORMATION)?; + get_process_path_from_handle(handle.get()) +} + +fn paths_equal(left: &Path, right: &Path) -> bool { + use windows_sys::Win32::Globalization::{CSTR_EQUAL, CompareStringOrdinal}; + + let left = to_u16s(left.as_os_str()); + let right = to_u16s(right.as_os_str()); + unsafe { CompareStringOrdinal(left.as_ptr(), -1, right.as_ptr(), -1, 1) == CSTR_EQUAL as i32 } +} + +fn process_matches_target( + process: &RunningProcess, + process_path: &Path, + target_path: &Path, +) -> bool { + let Some(target_name) = target_path.file_name() else { + return false; + }; + let Some(target_parent) = target_path.parent() else { + return false; + }; + let Some(process_parent) = process_path.parent() else { + return false; + }; + + // The image path follows an on-disk rename, while Toolhelp preserves the launch name. + // The updater keeps old_Code.exe beside Code.exe, so match the launch name and directory. + paths_equal(Path::new(&process.name), Path::new(target_name)) + && paths_equal(process_parent, target_parent) +} + +#[cfg(test)] +fn process_has_path(process_id: u32, path: &Path) -> io::Result { + use windows_sys::Win32::Storage::FileSystem::SYNCHRONIZE; + use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; + + let handle = open_process(process_id, PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE)?; + get_active_process_path(handle.get()).map(|process_path| { + process_path.is_some_and(|process_path| paths_equal(&process_path, path)) + }) +} + +fn process_has_exited(handle: HANDLE) -> io::Result { + use windows_sys::Win32::Foundation::{WAIT_FAILED, WAIT_OBJECT_0, WAIT_TIMEOUT}; + use windows_sys::Win32::System::Threading::WaitForSingleObject; + + unsafe { + match WaitForSingleObject(handle, 0) { + WAIT_OBJECT_0 => Ok(true), + WAIT_TIMEOUT => Ok(false), + WAIT_FAILED => Err(io::Error::last_os_error()), + result => Err(io::Error::other(format!( + "Unexpected process wait result: {}", + result + ))), + } + } +} + +fn get_active_process_path(handle: HANDLE) -> io::Result> { + if process_has_exited(handle)? { + return Ok(None); + } + + match get_process_path_from_handle(handle) { + Ok(path) => Ok(Some(path)), + Err(_) if process_has_exited(handle)? => Ok(None), + Err(err) => Err(err), + } +} + +fn wait_for_process_exit(handle: HANDLE, timeout: time::Duration) -> io::Result<()> { + use windows_sys::Win32::Foundation::{WAIT_FAILED, WAIT_OBJECT_0, WAIT_TIMEOUT}; + use windows_sys::Win32::System::Threading::WaitForSingleObject; + + let timeout_ms = timeout.as_millis().min((u32::MAX - 1) as u128) as u32; + unsafe { + match WaitForSingleObject(handle, timeout_ms) { + WAIT_OBJECT_0 => Ok(()), + WAIT_TIMEOUT => Err(io::Error::new( + io::ErrorKind::TimedOut, + "Timed out waiting for process to exit", + )), + WAIT_FAILED => Err(io::Error::last_os_error()), + result => Err(io::Error::other(format!( + "Unexpected process wait result: {}", + result + ))), + } + } +} + /** - * Kills a running process, if its path is the same as the provided one. + * Kills a running process if its path still matches the provided path. */ fn kill_process_if( log: &slog::Logger, - process: &RunningProcess, + matching_process: &MatchingProcess, path: &Path, + exit_timeout: time::Duration, ) -> Result<(), Box> { - use windows_sys::Win32::Foundation::{CloseHandle, MAX_PATH}; - use windows_sys::Win32::System::ProcessStatus::K32GetModuleFileNameExW; + use windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER; + use windows_sys::Win32::Storage::FileSystem::SYNCHRONIZE; use windows_sys::Win32::System::Threading::{ - OpenProcess, TerminateProcess, PROCESS_QUERY_INFORMATION, PROCESS_TERMINATE, - PROCESS_VM_READ, + PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_TERMINATE, TerminateProcess, }; + let process = &matching_process.process; info!( log, - "Kill process if found: {}, {}", process.id, process.name + "Verifying process before termination: pid={}, name={}", process.id, process.name ); - unsafe { - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms684320(v=vs.85).aspx - let handle = OpenProcess( - PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | PROCESS_TERMINATE, - 0, - process.id, - ); + if process_has_exited(matching_process.handle.get())? { + info!(log, "Process {} has already exited", process.id); + return Ok(()); + } - if ptr::eq(handle as *mut c_void, ptr::null()) { - return Err(io::Error::new( - io::ErrorKind::Other, - format!( - "Failed to open process: {}", - util::get_last_error_message()? - ), - ) - .into()); + let handle = match open_process( + process.id, + PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE | SYNCHRONIZE, + ) { + Ok(handle) => handle, + Err(err) if err.raw_os_error() == Some(ERROR_INVALID_PARAMETER as i32) => { + info!(log, "Process {} has already exited", process.id); + return Ok(()); } - - let mut raw_path = [0u16; MAX_PATH as usize]; - let len = K32GetModuleFileNameExW(handle, mem::zeroed(), raw_path.as_mut_ptr(), MAX_PATH) - as usize; - - if len == 0 { - CloseHandle(handle); - + Err(err) => { return Err(io::Error::new( - io::ErrorKind::Other, + err.kind(), format!( - "Failed to get process file name: {}", - util::get_last_error_message()? + "Failed to open process {} for termination: {}", + process.id, err ), ) .into()); } + }; - let process_path = PathBuf::from(from_utf16(&raw_path[0..len])?); - - info!( - log, - "Found {} running {}, attempting to kill...", process_path.display(), path.display() - ); - - if process_path != path { - CloseHandle(handle); - return Ok(()); - } - + let Some(process_path) = get_active_process_path(handle.get()).map_err(|err| { + io::Error::new( + err.kind(), + format!( + "Failed to inspect process {} before termination: {}", + process.id, err + ), + ) + })? + else { + info!(log, "Process {} has already exited", process.id); + return Ok(()); + }; + if !process_matches_target(process, &process_path, path) { info!( log, - "Found {} running, pid {}, attempting to kill...", process.name, process.id + "Skipping pid {} because its path changed to {}", + process.id, + process_path.display() ); + return Ok(()); + } - if TerminateProcess(handle, 0).is_negative() { - return Err(io::Error::new(io::ErrorKind::Other, "Failed to kill process").into()); + info!( + log, + "Terminating {}, pid {}", + process_path.display(), + process.id + ); + unsafe { + if TerminateProcess(handle.get(), 1) == 0 { + let err = io::Error::last_os_error(); + return Err(io::Error::new( + err.kind(), + format!("Failed to terminate process {}: {}", process.id, err), + ) + .into()); } - - info!( - log, - "Successfully killed {}, pid {}", process.name, process.id - ); - - CloseHandle(handle); - Ok(()) } + + wait_for_process_exit(handle.get(), exit_timeout).map_err(|err| { + io::Error::new( + err.kind(), + format!("Failed waiting for process {} to exit: {}", process.id, err), + ) + })?; + info!( + log, + "Successfully terminated {}, pid {}", process.name, process.id + ); + Ok(()) } -/** - * Checks if a process with the given PID is still running. - */ -fn is_process_running(pid: u32) -> bool { - use std::ffi::c_void; - use windows_sys::Win32::Foundation::CloseHandle; - use windows_sys::Win32::System::Threading::{ - GetExitCodeProcess, OpenProcess, PROCESS_QUERY_INFORMATION, - }; +fn get_matching_processes( + log: &slog::Logger, + path: &Path, +) -> Result, Box> { + use windows_sys::Win32::Storage::FileSystem::SYNCHRONIZE; + use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; - const STILL_ACTIVE: u32 = 259; + let file_name = path + .file_name() + .ok_or_else(|| io::Error::other("Could not get process file name"))?; - unsafe { - let handle = OpenProcess(PROCESS_QUERY_INFORMATION, 0, pid); + let file_name = file_name.to_string_lossy(); + let mut target_processes = Vec::new(); + for process in get_running_processes()? + .into_iter() + .filter(|process| process.name.eq_ignore_ascii_case(&file_name)) + { + let handle = match open_process(process.id, PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE) + { + Ok(handle) => handle, + Err(err) => { + warn!( + log, + "Unable to inspect pid {} with the same name; skipping it: {}", process.id, err + ); + continue; + } + }; - if ptr::eq(handle as *mut c_void, ptr::null()) { - return false; + match get_active_process_path(handle.get()) { + Ok(Some(process_path)) if process_matches_target(&process, &process_path, path) => { + target_processes.push(MatchingProcess { process, handle }); + } + Ok(Some(process_path)) => info!( + log, + "Ignoring pid {} with the same name at {}", + process.id, + process_path.display() + ), + Ok(None) => {} + Err(err) => warn!( + log, + "Unable to inspect pid {} with the same name; skipping it: {}", process.id, err + ), } + } - let mut exit_code = 0u32; - let result = GetExitCodeProcess(handle, &mut exit_code); - CloseHandle(handle); - - result != 0 && exit_code == STILL_ACTIVE + if target_processes.is_empty() { + info!(log, "{} is not running", file_name); } + + Ok(target_processes) } -pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box> { +fn wait_or_kill_with_options( + log: &slog::Logger, + path: &Path, + max_wait_attempts: u32, + wait_interval: time::Duration, + exit_timeout: time::Duration, +) -> Result<(), Box> { let file_name = path .file_name() - .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "Could not get process file name"))?; - - let file_name = file_name.to_str().ok_or_else(|| { - io::Error::new( - io::ErrorKind::Other, - "Could not get convert file name to str", - ) - })?; - - // Get the initial list of processes that match our target - let target_processes: Vec = get_running_processes()? - .into_iter() - .filter(|p| p.name == file_name) - .collect(); - + .ok_or_else(|| io::Error::other("Could not get process file name"))? + .to_string_lossy(); + let target_processes = get_matching_processes(log, path)?; if target_processes.is_empty() { - info!(log, "{} is not running", file_name); return Ok(()); } info!( log, - "Found {} running {} processes: {:?}", + "Found {} running {} processes at {}: {:?}", target_processes.len(), file_name, - target_processes.iter().map(|p| p.id).collect::>() + path.display(), + target_processes + .iter() + .map(|process| process.process.id) + .collect::>() ); let mut attempt: u32 = 0; - let mut still_running: Vec<&RunningProcess>; + let mut still_running: Vec<&MatchingProcess>; - // wait for up to 30 seconds until all target processes are dead + // Wait for the matching processes to exit naturally. loop { attempt += 1; @@ -233,24 +423,33 @@ pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box= max_wait_attempts { info!( log, "Gave up waiting for {} to exit, {} processes still running: {:?}", file_name, still_running.len(), - still_running.iter().map(|p| p.id).collect::>() + still_running + .iter() + .map(|process| process.process.id) + .collect::>() ); break; } @@ -259,12 +458,14 @@ pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box>() + still_running + .iter() + .map(|process| process.process.id) + .collect::>() ); - thread::sleep(time::Duration::from_millis(500)); + thread::sleep(wait_interval); } - // try to kill any running target processes util::retry( "attempting to kill any running processes", |attempt| { @@ -275,7 +476,7 @@ pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box = still_running .iter() - .filter_map(|p| kill_process_if(log, p, path).err()) + .filter_map(|process| kill_process_if(log, process, path, exit_timeout).err()) .collect(); for err in &kill_errors { @@ -284,23 +485,35 @@ pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box Ok(()), - _ => Err(kill_errors.into_iter().nth(0).unwrap()), + _ => Err(kill_errors.into_iter().next().unwrap()), } }, None, ) } +pub fn wait_or_kill(log: &slog::Logger, path: &Path) -> Result<(), Box> { + wait_or_kill_with_options( + log, + path, + 60, + time::Duration::from_millis(500), + time::Duration::from_secs(2), + ) +} + #[cfg(test)] mod tests { use super::*; - use std::path::PathBuf; - use std::process::{Command, Child}; + use slog::{Drain, Logger, o}; + use slog_async::Async; + use slog_term::{FullFormat, TermDecorator}; + use std::process::{Child, Command}; + use std::sync::Mutex; use std::thread; use std::time::Duration; - use slog::{Logger, o, Drain}; - use slog_term::{TermDecorator, FullFormat}; - use slog_async::Async; + + static PROCESS_TEST_MUTEX: Mutex<()> = Mutex::new(()); fn setup_test_logger() -> Logger { let decorator = TermDecorator::new().build(); @@ -312,9 +525,7 @@ mod tests { fn get_test_helper_path() -> PathBuf { let profile = std::env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); let target_dir = std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_string()); - let target = std::env::var("TARGET").unwrap_or_else(|_| { - "i686-pc-windows-msvc".to_string() - }); + let target = std::env::var("TARGET").unwrap_or_else(|_| "i686-pc-windows-msvc".to_string()); // Resolve target_dir to absolute path relative to project root let project_root = std::env::current_dir().expect("Failed to get current directory"); @@ -327,41 +538,90 @@ mod tests { fn start_test_process(args: &[&str]) -> Result { let test_helper = get_test_helper_path(); - Command::new(&test_helper) - .args(args) - .spawn() + start_test_process_at(&test_helper, args) + } + + fn start_test_process_at(path: &Path, args: &[&str]) -> Result { + Command::new(path).args(args).spawn() } - fn wait_for_process_start(expected_name: &str, timeout_ms: u64) -> bool { + fn wait_for_process_path(process_id: u32, path: &Path, timeout_ms: u64) -> bool { let start = std::time::Instant::now(); while start.elapsed().as_millis() < timeout_ms as u128 { - if let Ok(processes) = get_running_processes() { - if processes.iter().any(|p| p.name == expected_name) { - return true; - } + if process_has_path(process_id, path).unwrap_or(false) { + return true; } thread::sleep(Duration::from_millis(10)); } false } + fn wait_or_kill_for_test(log: &Logger, path: &Path) -> Result<(), Box> { + wait_or_kill_with_options( + log, + path, + 1, + Duration::from_millis(10), + Duration::from_secs(2), + ) + } + + #[test] + fn test_get_current_process_path() { + let actual = get_process_path(std::process::id()).expect("Should get current process path"); + let expected = std::env::current_exe().expect("Should get current executable path"); + assert!( + paths_equal(&actual, &expected), + "Expected {:?}, got {:?}", + expected, + actual + ); + } + + #[test] + fn test_paths_equal_ignores_case() { + assert!(paths_equal( + Path::new("C:\\Program Files\\Microsoft VS Code\\Code.exe"), + Path::new("c:\\program files\\microsoft vs code\\CODE.EXE"), + )); + } + #[test] fn test_wait_or_kill_no_processes_running() { let log = setup_test_logger(); let fake_path = PathBuf::from("C:\\nonexistent\\fake_process.exe"); let result = wait_or_kill(&log, &fake_path); - assert!(result.is_ok(), "Should succeed when no processes are running"); + assert!( + result.is_ok(), + "Should succeed when no processes are running" + ); } #[test] fn test_wait_or_kill_process_exits_naturally() { + let _guard = PROCESS_TEST_MUTEX + .lock() + .unwrap_or_else(|err| err.into_inner()); let log = setup_test_logger(); let test_helper_path = get_test_helper_path(); - let mut child = start_test_process(&["run-for-duration", "5"]).expect("Failed to start test process"); - assert!(wait_for_process_start("test_helper.exe", 1000), "Test process should start and be visible"); - let result = wait_or_kill(&log, &test_helper_path); + let mut child = + start_test_process(&["run-for-duration", "1"]).expect("Failed to start test process"); + assert!( + wait_for_process_path(child.id(), &test_helper_path, 1000), + "Test process should start and be visible" + ); + let result = wait_or_kill_with_options( + &log, + &test_helper_path, + 200, + Duration::from_millis(10), + Duration::from_secs(2), + ); let _ = child.wait(); - assert!(result.is_ok(), "Should succeed when process exits naturally"); + assert!( + result.is_ok(), + "Should succeed when process exits naturally" + ); } #[test] @@ -370,22 +630,111 @@ mod tests { let path = PathBuf::from(""); let result = wait_or_kill(&log, &path); assert!(result.is_err(), "Should fail with invalid path"); - assert!(result.unwrap_err().to_string().contains("Could not get process file name")); + assert!( + result + .unwrap_err() + .to_string() + .contains("Could not get process file name") + ); } #[test] fn test_wait_or_kill_multiple_processes() { + let _guard = PROCESS_TEST_MUTEX + .lock() + .unwrap_or_else(|err| err.into_inner()); let log = setup_test_logger(); let test_helper = get_test_helper_path(); - let mut child1 = start_test_process(&["run-forever"]).expect("Failed to start test process 1"); - let mut child2 = start_test_process(&["run-forever"]).expect("Failed to start test process 2"); - assert!(wait_for_process_start("test_helper.exe", 2000), "Test process should start and be visible"); - let processes = get_running_processes().unwrap(); - let test_helper_count = processes.iter().filter(|p| p.name == "test_helper.exe").count(); - assert!(test_helper_count >= 2, "Should have at least 2 test helper processes running"); - let result = wait_or_kill(&log, &test_helper); + let mut child1 = + start_test_process(&["run-forever"]).expect("Failed to start test process 1"); + let mut child2 = + start_test_process(&["run-forever"]).expect("Failed to start test process 2"); + assert!( + wait_for_process_path(child1.id(), &test_helper, 2000) + && wait_for_process_path(child2.id(), &test_helper, 2000), + "Test processes should start and be visible" + ); + let processes = get_matching_processes(&log, &test_helper).unwrap(); + assert!( + processes.len() >= 2, + "Should have at least 2 matching test helper processes" + ); + let result = wait_or_kill_for_test(&log, &test_helper); let _ = child1.wait(); let _ = child2.wait(); - assert!(result.is_ok(), "Should succeed when killing multiple processes"); + assert!( + result.is_ok(), + "Should succeed when killing multiple processes" + ); + } + + #[test] + fn test_wait_or_kill_ignores_same_name_at_different_path() { + let _guard = PROCESS_TEST_MUTEX + .lock() + .unwrap_or_else(|err| err.into_inner()); + let log = setup_test_logger(); + let test_helper = get_test_helper_path(); + let temp_dir = tempfile::tempdir().expect("Failed to create temporary directory"); + let other_test_helper = temp_dir.path().join("test_helper.exe"); + std::fs::copy(&test_helper, &other_test_helper).expect("Failed to copy test helper"); + let mut other_child = start_test_process_at(&other_test_helper, &["run-forever"]) + .expect("Failed to start test process"); + assert!( + wait_for_process_path(other_child.id(), &other_test_helper, 1000), + "Test process should start and be visible" + ); + + let result = wait_or_kill_for_test(&log, &test_helper); + let other_status = other_child + .try_wait() + .expect("Failed to query test process"); + let _ = other_child.kill(); + let _ = other_child.wait(); + + assert!( + result.is_ok(), + "Unrelated same-name process should not block the update" + ); + assert!( + other_status.is_none(), + "Unrelated same-name process should not be terminated" + ); + } + + #[test] + fn test_wait_or_kill_matches_process_after_executable_is_renamed() { + let _guard = PROCESS_TEST_MUTEX + .lock() + .unwrap_or_else(|err| err.into_inner()); + let log = setup_test_logger(); + let test_helper = get_test_helper_path(); + let temp_dir = tempfile::tempdir().expect("Failed to create temporary directory"); + let current_path = temp_dir.path().join("test_helper.exe"); + let old_path = temp_dir.path().join("old_test_helper.exe"); + std::fs::copy(&test_helper, ¤t_path).expect("Failed to copy test helper"); + let mut child = start_test_process_at(¤t_path, &["run-forever"]) + .expect("Failed to start process"); + assert!( + wait_for_process_path(child.id(), ¤t_path, 1000), + "Test process should start and be visible" + ); + std::fs::rename(¤t_path, &old_path).expect("Failed to rename running executable"); + + let result = wait_or_kill_for_test(&log, ¤t_path); + let child_status = child.try_wait().expect("Failed to query test process"); + if child_status.is_none() { + let _ = child.kill(); + let _ = child.wait(); + } + + assert!( + result.is_ok(), + "Renaming the executable should not prevent terminating its process" + ); + assert!( + child_status.is_some(), + "Process running from the renamed executable should be terminated" + ); } } From 274b8e5d752b5d5fbb46ea0b07a2a404ec44f162 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 12 Aug 2026 00:41:36 -0700 Subject: [PATCH 2/2] test: strengthen process path matching Use Unicode-aware Windows path comparison for candidate names and normalize path separators component-wise. Add direct and integration coverage for different installations and the executable replacement flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/process.rs | 70 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/src/process.rs b/src/process.rs index 7563de0..3431e12 100644 --- a/src/process.rs +++ b/src/process.rs @@ -142,9 +142,24 @@ fn get_process_path(process_id: u32) -> io::Result { fn paths_equal(left: &Path, right: &Path) -> bool { use windows_sys::Win32::Globalization::{CSTR_EQUAL, CompareStringOrdinal}; - let left = to_u16s(left.as_os_str()); - let right = to_u16s(right.as_os_str()); - unsafe { CompareStringOrdinal(left.as_ptr(), -1, right.as_ptr(), -1, 1) == CSTR_EQUAL as i32 } + let mut left = left.components(); + let mut right = right.components(); + loop { + match (left.next(), right.next()) { + (Some(left), Some(right)) => { + let left = to_u16s(left.as_os_str()); + let right = to_u16s(right.as_os_str()); + if unsafe { + CompareStringOrdinal(left.as_ptr(), -1, right.as_ptr(), -1, 1) + != CSTR_EQUAL as i32 + } { + return false; + } + } + (None, None) => return true, + _ => return false, + } + } } fn process_matches_target( @@ -344,7 +359,7 @@ fn get_matching_processes( let mut target_processes = Vec::new(); for process in get_running_processes()? .into_iter() - .filter(|process| process.name.eq_ignore_ascii_case(&file_name)) + .filter(|process| paths_equal(Path::new(&process.name), Path::new(file_name.as_ref()))) { let handle = match open_process(process.id, PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE) { @@ -579,10 +594,38 @@ mod tests { } #[test] - fn test_paths_equal_ignores_case() { + fn test_paths_equal_uses_windows_path_semantics() { assert!(paths_equal( + Path::new("C:\\Program Files\\Microsoft VS Code\\\\Code.exe"), + Path::new("c:/program files/microsoft vs code/CODE.EXE"), + )); + assert!(!paths_equal( Path::new("C:\\Program Files\\Microsoft VS Code\\Code.exe"), - Path::new("c:\\program files\\microsoft vs code\\CODE.EXE"), + Path::new("C:\\Program Files\\Microsoft VS Code Insiders\\Code.exe"), + )); + } + + #[test] + fn test_process_matches_target() { + let process = RunningProcess { + name: "Code.exe".to_string(), + id: 1, + }; + + assert!(process_matches_target( + &process, + Path::new("C:\\Program Files\\Microsoft VS Code\\old_Code.exe"), + Path::new("c:/program files/microsoft vs code/Code.exe"), + )); + assert!(!process_matches_target( + &process, + Path::new("C:\\Program Files\\Other VS Code\\Code.exe"), + Path::new("C:\\Program Files\\Microsoft VS Code\\Code.exe"), + )); + assert!(!process_matches_target( + &process, + Path::new("C:\\Program Files\\Microsoft VS Code\\Code.exe"), + Path::new("C:\\Program Files\\Microsoft VS Code"), )); } @@ -684,6 +727,19 @@ mod tests { wait_for_process_path(other_child.id(), &other_test_helper, 1000), "Test process should start and be visible" ); + assert!( + get_matching_processes(&log, &test_helper) + .expect("Failed to find target processes") + .is_empty(), + "Different installation should not match the target" + ); + assert!( + get_matching_processes(&log, &other_test_helper) + .expect("Failed to find copied test process") + .iter() + .any(|process| process.process.id == other_child.id()), + "Copied test process should match its own installation" + ); let result = wait_or_kill_for_test(&log, &test_helper); let other_status = other_child @@ -720,6 +776,8 @@ mod tests { "Test process should start and be visible" ); std::fs::rename(¤t_path, &old_path).expect("Failed to rename running executable"); + std::fs::copy(&test_helper, ¤t_path) + .expect("Failed to install replacement executable"); let result = wait_or_kill_for_test(&log, ¤t_path); let child_status = child.try_wait().expect("Failed to query test process");