fix: avoid reaping unrelated processes from stale PID files

src-tauri/src/aether/orphan.rs

@@ -1,5 +1,6 @@
 use std::fs;
 use std::path::{Path, PathBuf};
+use std::process::Command;
 
 fn pid_file(data_dir: &Path) -> PathBuf {
     data_dir.join("aether.pid")
@@ -13,52 +14,115 @@ pub fn clear_pid(data_dir: &Path) {
     let _ = fs::remove_file(pid_file(data_dir));
 }
 
-/// On startup, if a pid file survives from a prior crash and that process is
-/// still alive, kill it before the user can click Connect — otherwise a
-/// leftover Aether would just fail to bind the SOCKS port for the new one.
-/// This is a defensive backstop; `connect()`'s own port-in-use check (see
-/// aether/mod.rs) covers the case where this file is missing or stale.
-pub fn reap_orphan(data_dir: &Path) {
-    let path = pid_file(data_dir);
-    let Ok(contents) = fs::read_to_string(&path) else {
-        return;
-    };
-    if let Ok(pid) = contents.trim().parse::<u32>() {
-        if is_alive(pid) {
-            kill_pid(pid);
-        }
+fn expected_aether_name(name: &str) -> bool {
+    if cfg!(windows) {
+        name.eq_ignore_ascii_case("aether.exe")
+    } else {
+        name == "aether"
     }
-    let _ = fs::remove_file(&path);
 }
 
-#[cfg(unix)]
-fn is_alive(pid: u32) -> bool {
-    std::process::Command::new("kill")
-        .args(["-0", &pid.to_string()])
-        .status()
-        .map(|s| s.success())
+#[cfg(windows)]
+fn no_window(command: &mut Command) {
+    use std::os::windows::process::CommandExt;
+    const CREATE_NO_WINDOW: u32 = 0x0800_0000;
+    command.creation_flags(CREATE_NO_WINDOW);
+}
+
+#[cfg(windows)]
+fn is_expected_process(pid: u32) -> bool {
+    let mut command = Command::new("tasklist");
+    command.args(["/FI", &format!("PID eq {pid}"), "/FO", "CSV", "/NH"]);
+    no_window(&mut command);
+    command
+        .output()
+        .map(|output| {
+            String::from_utf8_lossy(&output.stdout).lines().any(|line| {
+                line.split(',')
+                    .next()
+                    .map(|name| expected_aether_name(name.trim_matches('"')))
+                    .unwrap_or(false)
+            })
+        })
         .unwrap_or(false)
 }
 
 #[cfg(unix)]
-fn kill_pid(pid: u32) {
-    let _ = std::process::Command::new("kill")
-        .args(["-9", &pid.to_string()])
-        .status();
+fn is_expected_process(pid: u32) -> bool {
+    Command::new("ps")
+        .args(["-p", &pid.to_string(), "-o", "comm="])
+        .output()
+        .map(|output| {
+            String::from_utf8_lossy(&output.stdout).lines().any(|line| {
+                Path::new(line.trim())
+                    .file_name()
+                    .map(|name| expected_aether_name(&name.to_string_lossy()))
+                    .unwrap_or(false)
+            })
+        })
+        .unwrap_or(false)
 }
 
 #[cfg(windows)]
-fn is_alive(pid: u32) -> bool {
-    std::process::Command::new("tasklist")
-        .args(["/FI", &format!("PID eq {pid}")])
+fn kill_pid(pid: u32) -> bool {
+    let mut command = Command::new("taskkill");
+    command.args(["/PID", &pid.to_string(), "/F"]);
+    no_window(&mut command);
+    command
         .output()
-        .map(|o| String::from_utf8_lossy(&o.stdout).contains(&pid.to_string()))
+        .map(|output| output.status.success())
         .unwrap_or(false)
 }
 
-#[cfg(windows)]
-fn kill_pid(pid: u32) {
-    let _ = std::process::Command::new("taskkill")
-        .args(["/PID", &pid.to_string(), "/F"])
-        .status();
+#[cfg(unix)]
+fn kill_pid(pid: u32) -> bool {
+    Command::new("kill")
+        .args(["-9", &pid.to_string()])
+        .status()
+        .map(|status| status.success())
+        .unwrap_or(false)
+}
+
+/// On startup, clean up a surviving Aether process from a prior crash.
+///
+/// A PID file alone is not proof of ownership: operating systems can reuse a
+/// PID after the original process exits. Verify that the PID still belongs to
+/// the expected Aether executable before terminating it, otherwise a stale PID
+/// file could cause the GUI to kill an unrelated process.
+pub fn reap_orphan(data_dir: &Path) {
+    let path = pid_file(data_dir);
+    let Ok(contents) = fs::read_to_string(&path) else {
+        return;
+    };
+    let Ok(pid) = contents.trim().parse::<u32>() else {
+        let _ = fs::remove_file(&path);
+        return;
+    };
+
+    if !is_expected_process(pid) {
+        let _ = fs::remove_file(&path);
+        return;
+    }
+
+    // Keep the PID file when termination fails so a later startup can retry;
+    // deleting it would lose the only record of a still-running owned process.
+    if kill_pid(pid) {
+        let _ = fs::remove_file(&path);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn accepts_only_the_aether_executable_name() {
+        assert!(expected_aether_name(if cfg!(windows) {
+            "aether.exe"
+        } else {
+            "aether"
+        }));
+        assert!(!expected_aether_name("not-aether.exe"));
+        assert!(!expected_aether_name("aether-helper.exe"));
+    }
 }