From 41761c39571ef4862b2c3f96807cc42841f3aab6 Mon Sep 17 00:00:00 2001 From: martian56 Date: Tue, 23 Jun 2026 08:40:03 +0400 Subject: [PATCH] fix(tauri): identify foreign process on core port + consent-gated force-quit (#3331) When port 7788 is held by a non-OpenHuman process (or a stale instance the automatic reap can't terminate), startup recovery did nothing and the user was stuck on a generic "couldn't start" screen with no way to act. Recovery now identifies the owning process (pid + name), surfaces it in the boot-check screen, and offers an explicit, consent-gated "Force-quit " button that re-validates port ownership immediately before terminating and refuses protected/self pids. - core_process: PortOwner + RecoveryOutcome.foreign_owner; identify_port_owner, process_name (tasklist/ps, pure parsers), validate_kill_target, force_quit_port_owner (term -> revalidate -> kill -> restart). - lib: force_quit_port_owner Tauri command. - BootCheckGate + bootCheck orchestrator + service: surface the owner and wire the consent-gated force-quit; re-run the boot check on success. - i18n: new keys translated across all locales. Closes #3331 --- app/src-tauri/src/core_process.rs | 209 +++++++++++++++++- app/src-tauri/src/core_process_tests.rs | 91 +++++++- app/src-tauri/src/lib.rs | 19 ++ .../BootCheckGate/BootCheckGate.tsx | 91 +++++++- .../__tests__/BootCheckGate.test.tsx | 112 ++++++++++ app/src/lib/bootCheck/index.test.ts | 21 ++ app/src/lib/bootCheck/index.ts | 24 +- app/src/lib/i18n/ar.ts | 7 + app/src/lib/i18n/bn.ts | 8 + app/src/lib/i18n/de.ts | 8 + app/src/lib/i18n/en.ts | 7 + app/src/lib/i18n/es.ts | 8 + app/src/lib/i18n/fr.ts | 8 + app/src/lib/i18n/hi.ts | 8 + app/src/lib/i18n/id.ts | 8 + app/src/lib/i18n/it.ts | 8 + app/src/lib/i18n/ko.ts | 8 + app/src/lib/i18n/pl.ts | 7 + app/src/lib/i18n/pt.ts | 8 + app/src/lib/i18n/ru.ts | 7 + app/src/lib/i18n/zh-CN.ts | 5 + app/src/services/bootCheckService.test.ts | 20 ++ app/src/services/bootCheckService.ts | 27 ++- 23 files changed, 697 insertions(+), 22 deletions(-) diff --git a/app/src-tauri/src/core_process.rs b/app/src-tauri/src/core_process.rs index 5471df0bb87..3f42aa6fddd 100644 --- a/app/src-tauri/src/core_process.rs +++ b/app/src-tauri/src/core_process.rs @@ -666,12 +666,25 @@ impl CoreProcessHandle { } } +/// A non-OpenHuman process holding the core RPC port. Surfaced to the frontend +/// so the user can see what to free, and so the consent-gated force-quit knows +/// which pid the user agreed to terminate. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct PortOwner { + pub pid: u32, + pub name: String, +} + /// Result returned to the frontend after a port-conflict auto-recovery attempt. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct RecoveryOutcome { pub success: bool, pub message: String, pub new_port: Option, + /// Set when recovery failed because a foreign process still holds the port. + /// `None` on success or when the owner can't be identified (e.g. a kernel + /// reservation), in which case the frontend offers guidance, not a kill. + pub foreign_owner: Option, } impl CoreProcessHandle { @@ -707,16 +720,112 @@ impl CoreProcessHandle { success: true, message: format!("Core recovered on port {new_port}"), new_port: Some(new_port), + foreign_owner: None, } } Err(err) => { - log::warn!("[core_process] recover_port_conflict: recovery failed: {err}"); + // Reaping stale OpenHuman processes didn't free the port, so a + // foreign process is holding it. Identify it (name + pid) so the + // frontend can surface it and offer a consent-gated force-quit. + let port = self.preferred_port; + let foreign_owner = tokio::task::spawn_blocking(move || identify_port_owner(port)) + .await + .unwrap_or(None); + log::warn!( + "[core_process] recover_port_conflict: recovery failed: {err}; foreign_owner={foreign_owner:?}" + ); RecoveryOutcome { success: false, message: format!("Recovery failed: {err}"), new_port: None, + foreign_owner, + } + } + } + } + + /// Terminate the foreign process the user consented to (the pid surfaced in + /// the prior [`RecoveryOutcome::foreign_owner`]), then restart the core. + /// + /// Re-validates that `expected_pid` still owns the port immediately before + /// killing — closing the PID-reuse window and refusing if the owner changed, + /// the port is already free, or the owner is OpenHuman's own process. + pub async fn force_quit_port_owner(&self, expected_pid: u32) -> RecoveryOutcome { + let port = self.preferred_port; + let self_pid = std::process::id(); + log::info!("[core_process] force_quit_port_owner: port {port} expected_pid {expected_pid}"); + + let current = tokio::task::spawn_blocking(move || find_pid_on_port(port)) + .await + .unwrap_or(None); + let pid = match validate_kill_target(current, expected_pid, self_pid) { + Ok(pid) => pid, + Err(reason) => { + log::warn!("[core_process] force_quit_port_owner: refused: {reason}"); + return RecoveryOutcome { + success: false, + message: reason, + new_port: None, + foreign_owner: None, + }; + } + }; + + // Best-effort: a TERM failure is non-fatal because the escalation below + // re-validates ownership and force-kills if the process is still there. + if let Err(err) = + tokio::task::spawn_blocking(move || crate::process_kill::kill_pid_term(pid)) + .await + .unwrap_or_else(|e| Err(format!("SIGTERM task panicked: {e}"))) + { + log::debug!( + "[core_process] force_quit_port_owner: SIGTERM pid {pid} (best-effort): {err}" + ); + } + tokio::time::sleep(Duration::from_millis(500)).await; + + // Re-validate before escalating: a bare `== Some(pid)` would SIGKILL a + // different process that reused the pid and rebound the port within the + // grace window, and would skip the self/protected-pid guards. + let still = tokio::task::spawn_blocking(move || find_pid_on_port(port)) + .await + .unwrap_or(None); + if validate_kill_target(still, pid, self_pid).is_ok() { + if let Err(err) = + tokio::task::spawn_blocking(move || crate::process_kill::kill_pid_force(pid)) + .await + .unwrap_or_else(|e| Err(format!("force-kill task panicked: {e}"))) + { + log::warn!("[core_process] force_quit_port_owner: force-kill pid {pid}: {err}"); + return RecoveryOutcome { + success: false, + message: format!("Could not terminate pid {pid}: {err}"), + new_port: None, + foreign_owner: None, + }; + } + tokio::time::sleep(Duration::from_millis(500)).await; + } + + match self.ensure_running().await { + Ok(()) => { + let new_port = self.port(); + log::info!( + "[core_process] force_quit_port_owner: core recovered on port {new_port}" + ); + RecoveryOutcome { + success: true, + message: format!("Core recovered on port {new_port}"), + new_port: Some(new_port), + foreign_owner: None, } } + Err(err) => RecoveryOutcome { + success: false, + message: format!("Recovery failed after terminating pid {pid}: {err}"), + new_port: None, + foreign_owner: None, + }, } } } @@ -836,8 +945,11 @@ fn is_expected_port_clash(reason: &str) -> bool { #[cfg(unix)] fn find_pid_on_port(port: u16) -> Option { + // Single port-qualified selector: `-iTCP:` ANDs protocol+port. Two + // separate `-i` flags (`-iTCP -i:`) are ORed by lsof and would match + // any TCP listener — risking returning an unrelated process as the owner. let output = std::process::Command::new("lsof") - .args(["-nP", "-iTCP", &format!("-i:{port}"), "-sTCP:LISTEN", "-t"]) + .args(["-nP", &format!("-iTCP:{port}"), "-sTCP:LISTEN", "-t"]) .output() .ok()?; if !output.status.success() { @@ -903,6 +1015,99 @@ fn parse_netstat_pid(stdout: &str, port: u16) -> Option { None } +/// Identify the process currently holding `port`, resolving its pid (via +/// [`find_pid_on_port`]) to a human-readable name. Returns `None` when the +/// port is free or owned by an unidentifiable (e.g. kernel-reserved) holder. +fn identify_port_owner(port: u16) -> Option { + let pid = find_pid_on_port(port)?; + // Empty when the name can't be resolved — the frontend already shows the + // pid, so a `"PID "` fallback here would render as "PID n (PID n)". + let name = process_name(pid).unwrap_or_default(); + Some(PortOwner { pid, name }) +} + +#[cfg(windows)] +fn process_name(pid: u32) -> Option { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + let output = std::process::Command::new("tasklist") + .args(["/FI", &format!("PID eq {pid}"), "/FO", "CSV", "/NH"]) + .creation_flags(CREATE_NO_WINDOW) + .output() + .ok()?; + if !output.status.success() { + return None; + } + parse_tasklist_name(&String::from_utf8_lossy(&output.stdout)) +} + +#[cfg(unix)] +fn process_name(pid: u32) -> Option { + let output = std::process::Command::new("ps") + .args(["-p", &pid.to_string(), "-o", "comm="]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + parse_ps_comm(&String::from_utf8_lossy(&output.stdout)) +} + +/// Pure parse of `tasklist /FO CSV /NH`: the image name is the first quoted +/// CSV field (`"chrome.exe","1234",...`). The "no tasks" sentinel and any +/// non-CSV line yield `None`. +#[allow(dead_code)] // exercised only on windows builds +fn parse_tasklist_name(stdout: &str) -> Option { + let line = stdout.lines().map(str::trim).find(|l| !l.is_empty())?; + let name = line.strip_prefix('"')?.split('"').next()?.trim(); + if name.is_empty() { + None + } else { + Some(name.to_string()) + } +} + +/// Pure parse of `ps -p -o comm=`: a single line, reduced to its +/// trailing path component so a full executable path collapses to a file name. +#[allow(dead_code)] // exercised only on unix builds +fn parse_ps_comm(stdout: &str) -> Option { + let line = stdout.lines().map(str::trim).find(|l| !l.is_empty())?; + let name = line.rsplit(['/', '\\']).next().unwrap_or(line).trim(); + if name.is_empty() { + None + } else { + Some(name.to_string()) + } +} + +/// Decide whether the consent-gated force-quit may proceed. `current_owner` is +/// the pid `find_pid_on_port` reports right before the kill; `expected_pid` is +/// the owner the user was shown and agreed to terminate. Returns the pid to +/// kill, or the reason the kill must be refused. +fn validate_kill_target( + current_owner: Option, + expected_pid: u32, + self_pid: u32, +) -> Result { + match current_owner { + None => Err("the port is no longer held by any process".to_string()), + Some(pid) if pid == self_pid => Err(format!( + "refusing to terminate OpenHuman's own process (pid {pid})" + )), + // Defense-in-depth: low/kernel pids are never legitimate port owners and + // must never be signalled, even if one slipped past `find_pid_on_port`. + // 0 — `kill(0, sig)` broadcasts to the caller's whole process group on + // Unix; 1 — init/launchd; 4 — Windows NT kernel. + Some(pid) if pid <= 1 || pid == 4 => { + Err(format!("refusing to terminate protected pid {pid}")) + } + Some(pid) if pid != expected_pid => Err(format!( + "port owner changed (was pid {expected_pid}, now pid {pid}); not terminating" + )), + Some(pid) => Ok(pid), + } +} + #[cfg(test)] #[path = "core_process_tests.rs"] mod tests; diff --git a/app/src-tauri/src/core_process_tests.rs b/app/src-tauri/src/core_process_tests.rs index 207ea78f221..954a3f84c1f 100644 --- a/app/src-tauri/src/core_process_tests.rs +++ b/app/src-tauri/src/core_process_tests.rs @@ -1,6 +1,7 @@ use super::{ current_rpc_token, default_core_port, generate_rpc_token, is_expected_port_clash, - is_openhuman_root_body, parse_lsof_pid, parse_netstat_pid, CoreProcessHandle, RecoveryOutcome, + is_openhuman_root_body, parse_lsof_pid, parse_netstat_pid, parse_ps_comm, parse_tasklist_name, + validate_kill_target, CoreProcessHandle, PortOwner, RecoveryOutcome, }; use std::sync::{Mutex, MutexGuard, OnceLock}; @@ -644,6 +645,7 @@ fn recovery_outcome_serializes_correctly() { success: true, message: "Core recovered on port 7789".to_string(), new_port: Some(7789), + foreign_owner: None, }; let json = serde_json::to_value(&outcome).expect("serialize"); assert_eq!(json["success"], serde_json::json!(true)); @@ -660,6 +662,7 @@ fn recovery_outcome_failure_serializes_with_null_port() { success: false, message: "Recovery failed: port still busy".to_string(), new_port: None, + foreign_owner: None, }; let json = serde_json::to_value(&outcome).expect("serialize"); assert_eq!(json["success"], serde_json::json!(false)); @@ -667,6 +670,92 @@ fn recovery_outcome_failure_serializes_with_null_port() { json["new_port"].is_null(), "new_port should be null when None" ); + assert!( + json["foreign_owner"].is_null(), + "foreign_owner should be null when None" + ); +} + +#[test] +fn recovery_outcome_serializes_foreign_owner() { + let outcome = RecoveryOutcome { + success: false, + message: "Recovery failed: port still busy".to_string(), + new_port: None, + foreign_owner: Some(PortOwner { + pid: 4242, + name: "Skype.exe".to_string(), + }), + }; + let json = serde_json::to_value(&outcome).expect("serialize"); + assert_eq!(json["foreign_owner"]["pid"], serde_json::json!(4242)); + assert_eq!( + json["foreign_owner"]["name"], + serde_json::json!("Skype.exe") + ); +} + +#[test] +fn parse_tasklist_name_extracts_image_name() { + assert_eq!( + parse_tasklist_name("\"chrome.exe\",\"1234\",\"Console\",\"1\",\"123,456 K\"\r\n"), + Some("chrome.exe".to_string()) + ); + // Leading blank lines are skipped. + assert_eq!( + parse_tasklist_name("\n\"node.exe\",\"42\",\"Console\",\"1\",\"9,000 K\""), + Some("node.exe".to_string()) + ); +} + +#[test] +fn parse_tasklist_name_rejects_no_tasks_sentinel_and_blanks() { + assert_eq!( + parse_tasklist_name("INFO: No tasks are running which match the specified criteria.\r\n"), + None + ); + assert_eq!(parse_tasklist_name(""), None); + assert_eq!(parse_tasklist_name("\"\",\"1\""), None); +} + +#[test] +fn parse_ps_comm_takes_basename() { + assert_eq!(parse_ps_comm("nginx\n"), Some("nginx".to_string())); + assert_eq!( + parse_ps_comm("/usr/lib/postgresql/16/bin/postgres\n"), + Some("postgres".to_string()) + ); + assert_eq!(parse_ps_comm(" \n"), None); + assert_eq!(parse_ps_comm(""), None); +} + +#[test] +fn validate_kill_target_accepts_unchanged_owner() { + assert_eq!(validate_kill_target(Some(1234), 1234, 999), Ok(1234)); +} + +#[test] +fn validate_kill_target_refuses_when_owner_gone_or_changed() { + // Port freed between surfacing the owner and confirming. + assert!(validate_kill_target(None, 1234, 999).is_err()); + // A different process now holds the port — never kill what the user did + // not consent to (PID-reuse / race guard). + assert!(validate_kill_target(Some(5678), 1234, 999).is_err()); +} + +#[test] +fn validate_kill_target_refuses_self_pid() { + let err = validate_kill_target(Some(999), 999, 999).unwrap_err(); + assert!(err.contains("own process"), "got: {err}"); +} + +#[test] +fn validate_kill_target_refuses_protected_pids() { + // Low/kernel pids must never be signalled even if the user "consented": + // 0 (kill(0) hits the process group on Unix), 1 (init/launchd), 4 (NT kernel). + assert!(validate_kill_target(Some(0), 0, 999).is_err()); + assert!(validate_kill_target(Some(1), 1, 999).is_err()); + assert!(validate_kill_target(Some(4), 4, 999).is_err()); } #[test] diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 55256e2582b..09ed27a3011 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -307,6 +307,24 @@ async fn recover_port_conflict( Ok(outcome) } +/// Terminate the foreign process holding the core RPC port, after the user +/// consented to the specific pid surfaced by `recover_port_conflict`. +#[tauri::command] +async fn force_quit_port_owner( + pid: u32, + state: tauri::State<'_, core_process::CoreProcessHandle>, +) -> Result { + log::info!("[core] force_quit_port_owner: command invoked for pid {pid}"); + let _guard = state.inner().restart_lock().await; + let outcome = state.inner().force_quit_port_owner(pid).await; + log::debug!( + "[core] force_quit_port_owner: result success={} message={}", + outcome.success, + outcome.message + ); + Ok(outcome) +} + /// Start the embedded core process on demand. /// /// Called by the BootCheckGate (Local mode) before the version check. The @@ -3479,6 +3497,7 @@ pub fn run() { install_app_update, restart_core_process, recover_port_conflict, + force_quit_port_owner, start_core_process, local_data_reset::reset_local_data, app_quit, diff --git a/app/src/components/BootCheckGate/BootCheckGate.tsx b/app/src/components/BootCheckGate/BootCheckGate.tsx index b7e057f0346..cac5254b54f 100644 --- a/app/src/components/BootCheckGate/BootCheckGate.tsx +++ b/app/src/components/BootCheckGate/BootCheckGate.tsx @@ -14,7 +14,11 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { type BootCheckResult, runBootCheck } from '../../lib/bootCheck'; import { useT } from '../../lib/i18n/I18nContext'; -import { bootCheckTransport, recoverPortConflict } from '../../services/bootCheckService'; +import { + bootCheckTransport, + forceQuitPortOwner, + recoverPortConflict, +} from '../../services/bootCheckService'; import { clearCoreRpcTokenCache, clearCoreRpcUrlCache, @@ -417,6 +421,7 @@ interface ResultScreenProps { actionBusy: boolean; actionError: string | null; onAction: () => void; + onForceQuit: (pid: number) => void; } function ResultScreen({ @@ -427,12 +432,14 @@ function ResultScreen({ actionBusy, actionError, onAction, + onForceQuit, }: ResultScreenProps) { const { t } = useT(); if (result.kind === 'match') return null; if (result.kind === 'unreachable') { const isPortConflict = result.portConflict === true; + const foreignOwner = result.foreignOwner; return (

@@ -440,22 +447,48 @@ function ResultScreen({

{isPortConflict - ? t('bootCheck.portConflictBody') + ? foreignOwner + ? t('bootCheck.portConflictOwner') + .replace('{name}', foreignOwner.name) + .replace('{pid}', String(foreignOwner.pid)) + .trim() + : t('bootCheck.portConflictBody') : result.reason || t('bootCheck.cannotReachDesc')}

+ {isPortConflict && foreignOwner && ( +

+ {t('bootCheck.portConflictGuidance')} +

+ )} {actionError &&

{actionError}

}
- {isPortConflict && ( + {isPortConflict && foreignOwner ? ( + ) : ( + isPortConflict && ( + + ) )}