From 8fe10d61eae845110a436419688c83062cd0208e Mon Sep 17 00:00:00 2001 From: fufesou <13586388+fufesou@users.noreply.github.com> Date: Wed, 7 Jan 2026 16:07:14 +0800 Subject: [PATCH] fix(terminal): linux, macOS, win as the controlled (#13930) 1. `TERM` on linux terminal. 2. `htop` command not found on macOS. 3. `vim` and `claude code cli` hung up on windows. Signed-off-by: fufesou --- Cargo.toml | 11 +- src/core_main.rs | 11 + src/platform/linux.rs | 150 ++++- src/server.rs | 2 + src/server/connection.rs | 13 +- src/server/terminal_helper.rs | 1062 ++++++++++++++++++++++++++++++++ src/server/terminal_service.rs | 430 +++++++++++-- 7 files changed, 1608 insertions(+), 71 deletions(-) create mode 100644 src/server/terminal_helper.rs diff --git a/Cargo.toml b/Cargo.toml index 0b63a8167..71894b660 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -123,10 +123,19 @@ winapi = { version = "0.3", features = [ ] } windows = { version = "0.61", features = [ "Win32", + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", "Win32_System", "Win32_System_Diagnostics", - "Win32_System_Threading", "Win32_System_Diagnostics_ToolHelp", + "Win32_System_Environment", + "Win32_System_IO", + "Win32_System_Memory", + "Win32_System_Pipes", + "Win32_System_Threading", + "Win32_UI_Shell", ] } winreg = "0.11" windows-service = "0.6" diff --git a/src/core_main.rs b/src/core_main.rs index 59adf3aff..ad8154dc6 100644 --- a/src/core_main.rs +++ b/src/core_main.rs @@ -615,6 +615,17 @@ pub fn core_main() -> Option> { #[cfg(feature = "hwcodec")] crate::ipc::hwcodec_process(); return None; + } else if args[0] == "--terminal-helper" { + // Terminal helper process - runs as user to create ConPTY + // This is needed because ConPTY has compatibility issues with CreateProcessAsUserW + #[cfg(target_os = "windows")] + { + let helper_args: Vec = args[1..].to_vec(); + if let Err(e) = crate::server::terminal_helper::run_terminal_helper(&helper_args) { + log::error!("Terminal helper failed: {}", e); + } + } + return None; } else if args[0] == "--cm" { // call connection manager to establish connections // meanwhile, return true to call flutter window to show control panel diff --git a/src/platform/linux.rs b/src/platform/linux.rs index d5a5edac0..5e608aa08 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -35,13 +35,20 @@ static mut UNMODIFIED: bool = true; const INVALID_TERM_VALUES: [&str; 3] = ["", "unknown", "dumb"]; const SHELL_PROCESSES: [&str; 4] = ["bash", "zsh", "fish", "sh"]; +// Terminal type constants +const TERM_XTERM_256COLOR: &str = "xterm-256color"; +const TERM_SCREEN_256COLOR: &str = "screen-256color"; +const TERM_XTERM: &str = "xterm"; + lazy_static::lazy_static! { pub static ref IS_X11: bool = hbb_common::platform::linux::is_x11_or_headless(); + // Cache for TERM value - once TERM_XTERM_256COLOR is found, reuse it directly + static ref CACHED_TERM: std::sync::Mutex> = std::sync::Mutex::new(None); static ref DATABASE_XTERM_256COLOR: Option = { - match Database::from_name("xterm-256color") { + match Database::from_name(TERM_XTERM_256COLOR) { Ok(database) => Some(database), Err(err) => { - log::error!("Failed to initialize xterm-256color database: {}", err); + log::error!("Failed to initialize {} database: {}", TERM_XTERM_256COLOR, err); None } } @@ -310,12 +317,12 @@ fn start_uinput_service() { /// modern features required by many applications. fn suggest_best_term() -> String { if is_running_in_tmux() || is_running_in_screen() { - return "screen-256color".to_string(); + return TERM_SCREEN_256COLOR.to_string(); } - if term_supports_256_colors("xterm-256color") { - return "xterm-256color".to_string(); + if term_supports_256_colors(TERM_XTERM_256COLOR) { + return TERM_XTERM_256COLOR.to_string(); } - "xterm".to_string() + TERM_XTERM.to_string() } fn is_running_in_tmux() -> bool { @@ -332,7 +339,7 @@ fn supports_256_colors(db: &Database) -> bool { fn term_supports_256_colors(term: &str) -> bool { match term { - "xterm-256color" => DATABASE_XTERM_256COLOR + TERM_XTERM_256COLOR => DATABASE_XTERM_256COLOR .as_ref() .map_or(false, |db| supports_256_colors(db)), _ => Database::from_name(term).map_or(false, |db| supports_256_colors(&db)), @@ -340,25 +347,140 @@ fn term_supports_256_colors(term: &str) -> bool { } fn get_cur_term(uid: &str) -> Option { + // Check cache first - if TERM_XTERM_256COLOR was found before, reuse it + if let Ok(cache) = CACHED_TERM.lock() { + if let Some(ref cached) = *cache { + if cached == TERM_XTERM_256COLOR { + return Some(cached.clone()); + } + } + } + if uid.is_empty() { return None; } + // Check current process environment if let Ok(term) = std::env::var("TERM") { - if !INVALID_TERM_VALUES.contains(&term.as_str()) { + if term == TERM_XTERM_256COLOR { + if let Ok(mut cache) = CACHED_TERM.lock() { + *cache = Some(term.clone()); + } return Some(term); } } - for proc in SHELL_PROCESSES { - // Construct a regex pattern to match either the process name followed by '$' or 'bin/' followed by the process name. - let term = get_env("TERM", uid, &format!("{}$|bin/{}", proc, proc)); - if !INVALID_TERM_VALUES.contains(&term.as_str()) { - return Some(term); + // Collect all TERM values from shell processes, looking for TERM_XTERM_256COLOR + let terms = get_all_term_values(uid); + + // Prefer TERM_XTERM_256COLOR + if terms.iter().any(|t| t == TERM_XTERM_256COLOR) { + if let Ok(mut cache) = CACHED_TERM.lock() { + *cache = Some(TERM_XTERM_256COLOR.to_string()); + } + return Some(TERM_XTERM_256COLOR.to_string()); + } + + // Return first valid TERM if no TERM_XTERM_256COLOR found + let fallback = terms.into_iter().next(); + if let Some(ref term) = fallback { + log::debug!( + "TERM_XTERM_256COLOR not found, using fallback TERM: {}", + term + ); + } + fallback +} + +/// Get all TERM values from shell processes (bash, zsh, fish, sh). +/// Returns a Vec of unique, valid TERM values. +fn get_all_term_values(uid: &str) -> Vec { + let Ok(uid_num) = uid.parse::() else { + return Vec::new(); + }; + + // Build regex pattern to match shell processes using only argv[0] (the executable path) + // Pattern: match process name at start or after '/', followed by space or end + // e.g., "bash", "/bin/bash", "/usr/bin/zsh" + let shell_pattern = SHELL_PROCESSES + .iter() + .map(|p| format!(r"(^|/){p}(\s|$)")) + .collect::>() + .join("|"); + let Ok(re) = Regex::new(&shell_pattern) else { + return Vec::new(); + }; + + let Ok(entries) = std::fs::read_dir("/proc") else { + return Vec::new(); + }; + + let mut terms = Vec::new(); + + for entry in entries.flatten() { + let file_name = entry.file_name(); + let Some(pid_str) = file_name.to_str() else { + continue; + }; + if !pid_str.chars().all(|c| c.is_ascii_digit()) { + continue; + } + + let proc_path = entry.path(); + + // Check if process belongs to the specified uid + if let Ok(meta) = std::fs::metadata(&proc_path) { + use std::os::unix::fs::MetadataExt; + if meta.uid() != uid_num { + continue; + } + } else { + continue; + } + + // Check cmdline matches process pattern + // /proc//cmdline is a sequence of null-terminated strings; the first + // one (argv[0]) is the executable path. Match the regex only against that + // to avoid false positives from arguments (e.g., "python /path/to/bash-script.py"). + let cmdline_path = proc_path.join("cmdline"); + let Ok(cmdline) = std::fs::read(&cmdline_path) else { + continue; + }; + let exe_end = cmdline.iter().position(|&b| b == 0).unwrap_or(cmdline.len()); + let exe_str = String::from_utf8_lossy(&cmdline[..exe_end]); + if !re.is_match(&exe_str) { + continue; + } + + // Read environ and extract TERM + let environ_path = proc_path.join("environ"); + let Ok(environ) = std::fs::read(&environ_path) else { + continue; + }; + + for part in environ.split(|&b| b == 0) { + if part.is_empty() { + continue; + } + if let Some(eq) = part.iter().position(|&b| b == b'=') { + let key_bytes = &part[..eq]; + if key_bytes == b"TERM" { + let val_bytes = &part[eq + 1..]; + let term = String::from_utf8_lossy(val_bytes).into_owned(); + if !INVALID_TERM_VALUES.contains(&term.as_str()) && !terms.contains(&term) { + // Early return if we found the preferred term + if term == TERM_XTERM_256COLOR { + return vec![term]; + } + terms.push(term); + } + break; + } + } } } - None + terms } #[inline] diff --git a/src/server.rs b/src/server.rs index bdf43e36e..9d2e4b804 100644 --- a/src/server.rs +++ b/src/server.rs @@ -33,6 +33,8 @@ use video_service::VideoSource; use crate::ipc::Data; pub mod audio_service; +#[cfg(target_os = "windows")] +pub mod terminal_helper; #[cfg(not(any(target_os = "android", target_os = "ios")))] pub mod terminal_service; cfg_if::cfg_if! { diff --git a/src/server/connection.rs b/src/server/connection.rs index 3670fb7cf..ee8cad591 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -3231,12 +3231,15 @@ impl Connection { if !token.is_null() { match crate::platform::ensure_primary_token(token) { Ok(t) => { - self.terminal_user_token = Some(TerminalUserToken::CurrentLogonUser(t as _)); + self.terminal_user_token = Some(TerminalUserToken::CurrentLogonUser( + crate::terminal_service::UserToken::new(t as usize), + )); } Err(e) => { log::error!("Failed to ensure primary token: {}", e); - self.terminal_user_token = - Some(TerminalUserToken::CurrentLogonUser(token as _)); + self.terminal_user_token = Some(TerminalUserToken::CurrentLogonUser( + crate::terminal_service::UserToken::new(token as usize), + )); } } None @@ -5049,9 +5052,9 @@ impl Drop for Connection { #[cfg(target_os = "windows")] if let Some(TerminalUserToken::CurrentLogonUser(token)) = self.terminal_user_token.take() { - if token != 0 { + if token.as_raw() != 0 { unsafe { - hbb_common::allow_err!(CloseHandle(HANDLE(token as _))); + hbb_common::allow_err!(CloseHandle(HANDLE(token.as_raw() as _))); }; } } diff --git a/src/server/terminal_helper.rs b/src/server/terminal_helper.rs new file mode 100644 index 000000000..8edf4621b --- /dev/null +++ b/src/server/terminal_helper.rs @@ -0,0 +1,1062 @@ +//! Terminal Helper Process +//! +//! This module implements a helper process that runs as the logged-in user and creates +//! the ConPTY + Shell. This is necessary because ConPTY has compatibility issues with +//! CreateProcessAsUserW when the ConPTY is created by a different user (SYSTEM service). +//! +//! Architecture: +//! ``` +//! SYSTEM Service (terminal_service.rs) +//! | +//! +-- CreateProcessAsUserW --> Terminal Helper (this module, runs as user) +//! | | +//! | +-- CreateProcessW + ConPTY --> Shell +//! | | +//! +-- Named Pipes <----------------+ +//! ``` +//! +//! This module also contains Windows-specific utility functions used by terminal_service.rs: +//! - Named pipe creation and connection +//! - User token and SID handling +//! - Helper process launching + +use hbb_common::{ + anyhow::{anyhow, Context, Result}, + log, +}; +use portable_pty::{CommandBuilder, MasterPty, PtySize}; +use std::{ + ffi::{c_void, OsStr}, + fs::File, + io::{Read, Write}, + os::windows::{ffi::OsStrExt, io::FromRawHandle, raw::HANDLE as RawHandle}, + ptr, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, + }, + thread, + time::Duration, +}; + +use windows::{ + core::{PCWSTR, PWSTR}, + Win32::{ + Foundation::{ + CloseHandle, LocalFree, ERROR_IO_PENDING, ERROR_PIPE_CONNECTED, HANDLE, HLOCAL, + INVALID_HANDLE_VALUE, WAIT_OBJECT_0, + }, + Security::{ + Authorization::{ + SetEntriesInAclW, EXPLICIT_ACCESS_W, SET_ACCESS, TRUSTEE_IS_SID, TRUSTEE_IS_USER, + TRUSTEE_W, + }, + CreateWellKnownSid, GetLengthSid, GetTokenInformation, InitializeSecurityDescriptor, + SetSecurityDescriptorDacl, TokenUser, WinLocalSystemSid, ACE_FLAGS, ACL, + PSECURITY_DESCRIPTOR, PSID, SECURITY_ATTRIBUTES, TOKEN_USER, + }, + Storage::FileSystem::{ + CreateFileW, FILE_ALL_ACCESS, FILE_FLAGS_AND_ATTRIBUTES, FILE_FLAG_OVERLAPPED, + FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_SHARE_READ, FILE_SHARE_WRITE, + OPEN_EXISTING, + }, + System::{ + Environment::{CreateEnvironmentBlock, DestroyEnvironmentBlock}, + Pipes::{ + ConnectNamedPipe, CreateNamedPipeW, PIPE_READMODE_BYTE, PIPE_TYPE_BYTE, PIPE_WAIT, + }, + Threading::{ + CreateEventW, CreateProcessAsUserW, WaitForSingleObject, CREATE_NO_WINDOW, + CREATE_UNICODE_ENVIRONMENT, PROCESS_CREATION_FLAGS, PROCESS_INFORMATION, + STARTUPINFOW, + }, + IO::{GetOverlappedResult, OVERLAPPED}, + }, + }, +}; + +// Re-export types needed by terminal_service.rs +pub use windows::Win32::{ + Foundation::{ + CloseHandle as WinCloseHandle, HANDLE as WinHANDLE, WAIT_OBJECT_0 as WIN_WAIT_OBJECT_0, + }, + System::Threading::{ + GetExitCodeProcess as WinGetExitCodeProcess, TerminateProcess as WinTerminateProcess, + WaitForSingleObject as WinWaitForSingleObject, + }, +}; + +/// User token wrapper for cross-module use. +/// +/// Using newtype pattern for type safety. The inner value is `usize` to match +/// platform pointer size (32-bit on x86, 64-bit on x64). +/// Windows HANDLE is defined as `*mut c_void`, which has the same size as `usize`. +/// +/// # Design Note +/// This type is defined here (terminal_helper.rs) for Windows and in +/// terminal_service.rs for non-Windows platforms. This avoids circular +/// dependencies while keeping the API consistent across platforms. +/// Both definitions MUST have identical public API (new, as_raw methods). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct UserToken(pub usize); + +impl UserToken { + /// Create a new UserToken from a raw handle value. + pub fn new(handle: usize) -> Self { + Self(handle) + } + + /// Get the raw handle value. + pub fn as_raw(&self) -> usize { + self.0 + } +} + +// Windows pipe access mode constants (not exported by windows crate) +const PIPE_ACCESS_INBOUND: u32 = 0x00000001; +const PIPE_ACCESS_OUTBOUND: u32 = 0x00000002; + +// Named pipe configuration constants +const PIPE_BUFFER_SIZE: u32 = 65536; // 64KB for better throughput with large terminal output +const PIPE_DEFAULT_TIMEOUT_MS: u32 = 5000; +/// Timeout for waiting for helper process to connect to pipes +pub const PIPE_CONNECTION_TIMEOUT_MS: u32 = 10000; + +/// Message type constants for helper protocol. +/// Used to distinguish between terminal data and control commands. +/// Note: Using non-zero values to make debugging easier (0x00 could indicate uninitialized memory). +pub const MSG_TYPE_DATA: u8 = 0x01; +pub const MSG_TYPE_RESIZE: u8 = 0x02; + +/// Message header size: 1 byte type + 4 bytes length +pub const MSG_HEADER_SIZE: usize = 5; + +/// Maximum payload size to prevent denial of service from malicious messages. +/// 16MB should be more than enough for any legitimate terminal data. +const MAX_PAYLOAD_SIZE: usize = 16 * 1024 * 1024; + +/// Timeout in milliseconds to wait for helper process to exit gracefully before force termination. +/// Using 500ms to allow helper process enough time to clean up, especially under high system load. +pub const HELPER_GRACEFUL_EXIT_TIMEOUT_MS: u64 = 500; + +/// Information about a launched helper process. +/// Contains both the process handle and PID for tracking and status checks. +#[derive(Debug)] +pub struct HelperProcessInfo { + /// Process handle for termination and waiting + pub handle: HANDLE, + /// Process ID for logging and status display + pub pid: u32, +} + +/// Wrapper for Windows HANDLE that implements Send. +/// This is safe because Windows HANDLEs are valid across threads. +/// Note: We only implement Send, not Sync. The handle is protected by +/// Mutex in TerminalSession, so concurrent access is controlled there. +/// +/// # Ownership and Cleanup +/// This type intentionally does NOT implement Drop. The handle is owned by +/// `TerminalSession` and explicitly closed in `TerminalSession::close_internal()` +/// after graceful shutdown logic (waiting for helper to exit, force termination if needed). +/// Implementing Drop here would interfere with that cleanup sequence. +#[derive(Debug)] +pub struct SendableHandle(HANDLE); + +impl SendableHandle { + /// Create a new SendableHandle from a raw HANDLE. + pub fn new(handle: HANDLE) -> Self { + Self(handle) + } + + /// Get the raw HANDLE value. + pub fn as_raw(&self) -> HANDLE { + self.0 + } +} + +unsafe impl Send for SendableHandle {} + +/// RAII wrapper for Windows HANDLE that automatically closes the handle on drop. +/// This ensures proper resource cleanup even when errors occur or code paths diverge. +pub struct OwnedHandle(HANDLE); + +impl OwnedHandle { + /// Create a new OwnedHandle from a raw HANDLE. + /// The handle will be closed when this OwnedHandle is dropped. + pub fn new(handle: HANDLE) -> Self { + Self(handle) + } + + /// Consume the OwnedHandle and return the raw HANDLE without closing it. + /// Use this when transferring ownership to another resource (e.g., File). + pub fn into_raw(self) -> HANDLE { + let handle = self.0; + std::mem::forget(self); // Prevent Drop from closing the handle + handle + } + + /// Get the raw HANDLE value. + pub fn as_raw(&self) -> HANDLE { + self.0 + } +} + +impl Drop for OwnedHandle { + fn drop(&mut self) { + if self.0 != INVALID_HANDLE_VALUE && !self.0.is_invalid() { + unsafe { + let _ = CloseHandle(self.0); + } + } + } +} + +/// RAII guard for helper process that terminates the process on drop. +/// This prevents helper process leaks when pipe connection fails or other errors occur. +/// +/// Unlike OwnedHandle (which only closes the handle), this guard: +/// 1. Terminates the process using TerminateProcess +/// 2. Then closes the handle +/// +/// Use `disarm()` to prevent termination when the helper is successfully handed off +/// to the terminal session for proper lifecycle management. +pub struct HelperProcessGuard { + handle: HANDLE, + pid: u32, + armed: bool, +} + +impl HelperProcessGuard { + /// Create a new guard for a helper process. + pub fn new(handle: HANDLE, pid: u32) -> Self { + Self { + handle, + pid, + armed: true, + } + } + + /// Get the raw process HANDLE. + pub fn as_raw(&self) -> HANDLE { + self.handle + } + + /// Get the process ID. + pub fn pid(&self) -> u32 { + self.pid + } + + /// Disarm the guard and return the raw HANDLE. + /// After calling this, the guard will NOT terminate the process on drop. + /// Use this when successfully handing off the helper to session management. + pub fn disarm(self) -> HANDLE { + let handle = self.handle; + std::mem::forget(self); // Prevent Drop from running + handle + } +} + +impl Drop for HelperProcessGuard { + fn drop(&mut self) { + if self.armed && self.handle != INVALID_HANDLE_VALUE && !self.handle.is_invalid() { + log::warn!( + "HelperProcessGuard: terminating leaked helper process (PID {})", + self.pid + ); + unsafe { + // Terminate the process first + let _ = WinTerminateProcess(self.handle, 1); + // Then close the handle + let _ = CloseHandle(self.handle); + } + } + } +} + +/// Encode a message for the helper protocol. +/// Format: [type: u8][length: u32 LE][payload: bytes] +pub fn encode_helper_message(msg_type: u8, payload: &[u8]) -> Vec { + let mut msg = Vec::with_capacity(MSG_HEADER_SIZE + payload.len()); + msg.push(msg_type); + msg.extend_from_slice(&(payload.len() as u32).to_le_bytes()); + msg.extend_from_slice(payload); + msg +} + +/// Encode a resize message for the helper protocol. +/// Payload: rows (u16 LE) + cols (u16 LE) +pub fn encode_resize_message(rows: u16, cols: u16) -> Vec { + let mut payload = Vec::with_capacity(4); + payload.extend_from_slice(&rows.to_le_bytes()); + payload.extend_from_slice(&cols.to_le_bytes()); + encode_helper_message(MSG_TYPE_RESIZE, &payload) +} + +/// Get the default shell for Windows. +pub fn get_default_shell() -> String { + // Try PowerShell Core first (absolute paths only) + let pwsh_paths = [ + "pwsh.exe", + r"C:\Program Files\PowerShell\7\pwsh.exe", + r"C:\Program Files\PowerShell\6\pwsh.exe", + ]; + + for path in &pwsh_paths { + if std::path::Path::new(path).exists() { + log::debug!("Found PowerShell Core: {}", path); + return path.to_string(); + } + } + + // Try Windows PowerShell + let powershell_path = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"; + if std::path::Path::new(powershell_path).exists() { + return powershell_path.to_string(); + } + + // Fallback to cmd.exe + std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string()) +} + +/// Get the SID of the user from a token. +/// Returns a Vec containing the SID bytes. +pub fn get_user_sid_from_token(user_token: UserToken) -> Result> { + let token_handle = HANDLE(user_token.as_raw() as _); + + // First call to get required buffer size + let mut return_length = 0u32; + let _ = unsafe { GetTokenInformation(token_handle, TokenUser, None, 0, &mut return_length) }; + + if return_length == 0 { + return Err(anyhow!( + "Failed to get token information size: {}", + std::io::Error::last_os_error() + )); + } + + // Allocate buffer and get token information + let mut buffer = vec![0u8; return_length as usize]; + unsafe { + GetTokenInformation( + token_handle, + TokenUser, + Some(buffer.as_mut_ptr() as *mut c_void), + return_length, + &mut return_length, + ) + .map_err(|e| anyhow!("Failed to get token information: {}", e))?; + } + + // Extract SID from TOKEN_USER structure + let token_user = unsafe { &*(buffer.as_ptr() as *const TOKEN_USER) }; + let sid_ptr = token_user.User.Sid; + + // Get SID length and copy to owned buffer + let sid_length = unsafe { GetLengthSid(sid_ptr) }; + + if sid_length == 0 { + return Err(anyhow!("Invalid SID length")); + } + + let mut sid_buffer = vec![0u8; sid_length as usize]; + unsafe { + ptr::copy_nonoverlapping( + sid_ptr.0 as *const u8, + sid_buffer.as_mut_ptr(), + sid_length as usize, + ); + } + + Ok(sid_buffer) +} + +/// Create a restricted DACL that only allows SYSTEM and a specific user. +/// Returns a pointer to the ACL that must be freed with LocalFree. +/// +/// # Safety +/// +/// This function is safe to call, but contains internal unsafe code that relies on +/// pointer lifetime guarantees: +/// +/// - The `user_sid` slice must contain valid SID binary data. +/// - Internally, raw pointers to `system_sid_buffer` (stack-allocated) and `user_sid` +/// are stored in `TRUSTEE_W.ptstrName` fields. These pointers are only used during +/// the `SetEntriesInAclW` call, which occurs before either buffer goes out of scope. +/// - The returned ACL pointer is allocated by Windows and must be freed with `LocalFree`. +pub fn create_restricted_dacl(user_sid: &[u8]) -> Result<*mut c_void> { + // Create SYSTEM SID (well-known SID: S-1-5-18) + // SAFETY: This buffer must outlive the TRUSTEE_W structures that reference it + let mut system_sid_buffer = vec![0u8; 64]; // Max SID size + let mut system_sid_size = system_sid_buffer.len() as u32; + unsafe { + CreateWellKnownSid( + WinLocalSystemSid, + None, // No domain SID + Some(PSID(system_sid_buffer.as_mut_ptr() as *mut c_void)), + &mut system_sid_size, + ) + .map_err(|e| anyhow!("Failed to create SYSTEM SID: {}", e))?; + } + + // Build EXPLICIT_ACCESS entries for SYSTEM and user + // SAFETY: The ptstrName pointers below reference system_sid_buffer and user_sid. + // These buffers must remain valid until SetEntriesInAclW returns. + let mut explicit_access: [EXPLICIT_ACCESS_W; 2] = unsafe { std::mem::zeroed() }; + + // Entry 0: SYSTEM - full access + explicit_access[0].grfAccessPermissions = FILE_ALL_ACCESS.0; + explicit_access[0].grfAccessMode = SET_ACCESS; + explicit_access[0].grfInheritance = ACE_FLAGS(0); // No inheritance for pipes + explicit_access[0].Trustee = TRUSTEE_W { + pMultipleTrustee: ptr::null_mut(), + MultipleTrusteeOperation: Default::default(), + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_USER, + ptstrName: PWSTR::from_raw(system_sid_buffer.as_ptr() as *mut u16), + }; + + // Entry 1: User - full access + explicit_access[1].grfAccessPermissions = FILE_ALL_ACCESS.0; + explicit_access[1].grfAccessMode = SET_ACCESS; + explicit_access[1].grfInheritance = ACE_FLAGS(0); // No inheritance for pipes + // SAFETY: When TrusteeForm is TRUSTEE_IS_SID, ptstrName is interpreted as a PSID + // pointer, not a string pointer. The Windows API reuses this field for different + // purposes based on TrusteeForm. The SID binary data in user_sid is valid for + // the duration of this function call (until SetEntriesInAclW returns). + explicit_access[1].Trustee = TRUSTEE_W { + pMultipleTrustee: ptr::null_mut(), + MultipleTrusteeOperation: Default::default(), + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_USER, + ptstrName: PWSTR::from_raw(user_sid.as_ptr() as *mut u16), + }; + + // Create ACL from explicit access entries + // After this call returns, system_sid_buffer and user_sid are no longer needed + let mut new_acl: *mut ACL = ptr::null_mut(); + let result = unsafe { + SetEntriesInAclW( + Some(&explicit_access), + None, // No existing ACL + &mut new_acl, + ) + }; + + if result.0 != 0 { + return Err(anyhow!( + "SetEntriesInAclW failed with error code: {}", + result.0 + )); + } + + if new_acl.is_null() { + return Err(anyhow!("SetEntriesInAclW returned null ACL")); + } + + Ok(new_acl as *mut c_void) +} + +/// Create a named pipe with a restricted DACL. +/// Only SYSTEM and the specified user can access the pipe. +/// +/// # Arguments +/// * `pipe_name` - The name of the pipe to create +/// * `for_input` - True if service writes to this pipe (helper reads), false otherwise +/// * `user_token` - Required user token for creating restricted DACL +/// +/// # Security +/// +/// The restricted DACL limits pipe access to: +/// - SYSTEM account (the service) +/// - The specific user whose token was provided (the helper process) +/// +/// This function requires a valid user_token and will fail if DACL creation fails, +/// rather than falling back to a less secure NULL DACL. +pub fn create_named_pipe_server( + pipe_name: &str, + for_input: bool, + user_token: UserToken, +) -> Result { + // SECURITY_DESCRIPTOR minimum length is 40 bytes on x64. + const SD_BUFFER_SIZE: usize = 64; + const _: () = assert!( + SD_BUFFER_SIZE >= 40, + "SD_BUFFER_SIZE must be at least 40 bytes for SECURITY_DESCRIPTOR" + ); + + let mut sd_buffer = [0u8; SD_BUFFER_SIZE]; + let sd_ptr = PSECURITY_DESCRIPTOR(sd_buffer.as_mut_ptr() as *mut c_void); + + // Initialize security descriptor + unsafe { + InitializeSecurityDescriptor(sd_ptr, 1) + .map_err(|e| anyhow!("Failed to initialize security descriptor: {}", e))?; + } + + // Create restricted DACL - fail if this doesn't work (no NULL DACL fallback) + let user_sid = get_user_sid_from_token(user_token) + .context("Failed to get user SID from token for pipe DACL")?; + let acl_ptr = + create_restricted_dacl(&user_sid).context("Failed to create restricted DACL for pipe")?; + + log::debug!("Created restricted DACL for pipe: {}", pipe_name); + + // Set DACL on security descriptor + unsafe { + SetSecurityDescriptorDacl(sd_ptr, true, Some(acl_ptr as *const _ as *const _), false) + .map_err(|e| { + // Clean up ACL on error (ignore result - cleanup is best-effort, original error takes precedence) + let _ = LocalFree(Some(HLOCAL(acl_ptr))); + anyhow!("Failed to set restricted DACL: {}", e) + })?; + } + + let sa = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: sd_buffer.as_mut_ptr() as *mut c_void, + bInheritHandle: false.into(), + }; + + let wide_name: Vec = OsStr::new(pipe_name) + .encode_wide() + .chain(std::iter::once(0)) + .collect(); + + let access_mode = if for_input { + FILE_FLAGS_AND_ATTRIBUTES(PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED.0) + } else { + FILE_FLAGS_AND_ATTRIBUTES(PIPE_ACCESS_OUTBOUND | FILE_FLAG_OVERLAPPED.0) + }; + + log::debug!( + "Creating named pipe: {} (for_input={}, restricted_dacl=true)", + pipe_name, + for_input + ); + + let handle = unsafe { + CreateNamedPipeW( + PCWSTR::from_raw(wide_name.as_ptr()), + access_mode, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + 1, // max instances + PIPE_BUFFER_SIZE, + PIPE_BUFFER_SIZE, + PIPE_DEFAULT_TIMEOUT_MS, + Some(&sa), + ) + }; + + // Clean up ACL after pipe creation (security descriptor has been applied) + // Ignore result: LocalFree failure is non-critical since the pipe is already created + unsafe { + let _ = LocalFree(Some(HLOCAL(acl_ptr))); + } + + if handle == INVALID_HANDLE_VALUE { + return Err(anyhow!( + "Failed to create named pipe {}: {}", + pipe_name, + std::io::Error::last_os_error() + )); + } + + log::debug!("Named pipe created: {}", pipe_name); + Ok(handle) +} + +/// Wait for client to connect to named pipe with timeout. +/// +/// # Ownership +/// This function **takes ownership** of the `pipe_handle` via OwnedHandle: +/// - On success: the handle is extracted and wrapped in a `File`. +/// - On failure: the handle is automatically closed when OwnedHandle drops. +pub fn wait_for_pipe_connection( + pipe_handle: OwnedHandle, + pipe_name: &str, + timeout_ms: u32, +) -> Result { + log::debug!("Waiting for pipe connection: {}", pipe_name); + + // Create an event for overlapped I/O (also wrapped in OwnedHandle for RAII) + let event = unsafe { CreateEventW(None, true, false, PCWSTR::null()) } + .map_err(|e| anyhow!("Failed to create event for pipe connection: {}", e))?; + let event_handle = OwnedHandle::new(event); + + let mut overlapped: OVERLAPPED = unsafe { std::mem::zeroed() }; + overlapped.hEvent = event_handle.as_raw(); + + let result = unsafe { ConnectNamedPipe(pipe_handle.as_raw(), Some(&mut overlapped)) }; + if result.is_err() { + let err = std::io::Error::last_os_error(); + let err_code = err.raw_os_error().unwrap_or(0); + + // ERROR_PIPE_CONNECTED means client already connected, which is OK + if err_code == ERROR_PIPE_CONNECTED.0 as i32 { + log::debug!("Pipe already connected: {}", pipe_name); + return Ok(unsafe { File::from_raw_handle(pipe_handle.into_raw().0 as RawHandle) }); + } + + // ERROR_IO_PENDING means we need to wait + if err_code == ERROR_IO_PENDING.0 as i32 { + log::debug!("Pipe connection pending, waiting with timeout..."); + let wait_result = unsafe { WaitForSingleObject(event_handle.as_raw(), timeout_ms) }; + + if wait_result != WAIT_OBJECT_0 { + log::error!("Timeout waiting for pipe connection: {}", pipe_name); + return Err(anyhow!( + "Timeout waiting for pipe connection: {}", + pipe_name + )); + } + + // Check if connection was successful + let mut bytes_transferred = 0u32; + let overlapped_result = unsafe { + GetOverlappedResult( + pipe_handle.as_raw(), + &overlapped, + &mut bytes_transferred, + false, + ) + }; + if overlapped_result.is_err() { + let err = std::io::Error::last_os_error(); + log::error!("Failed to complete pipe connection {}: {}", pipe_name, err); + return Err(anyhow!( + "Failed to complete pipe connection {}: {}", + pipe_name, + err + )); + } + + log::debug!("Pipe connected: {}", pipe_name); + } else { + log::error!("Failed to connect named pipe {}: {}", pipe_name, err); + return Err(anyhow!( + "Failed to connect named pipe {}: {}", + pipe_name, + err + )); + } + } else { + log::debug!("Pipe connected immediately: {}", pipe_name); + } + + // Success: transfer pipe ownership to File, event_handle drops + Ok(unsafe { File::from_raw_handle(pipe_handle.into_raw().0 as RawHandle) }) +} + +/// Launch terminal helper process as the logged-in user using the provided token. +/// The helper process creates ConPTY and shell, communicating via named pipes. +/// This uses CreateProcessAsUserW directly with the user token, which works because +/// the helper process itself doesn't need ConPTY - it creates ConPTY internally. +/// +/// Returns HelperProcessInfo containing the process handle and PID. + +/// RAII guard for environment block cleanup. +/// Ensures DestroyEnvironmentBlock is called even if an error occurs. +struct EnvironmentBlockGuard { + ptr: *mut c_void, +} + +impl Drop for EnvironmentBlockGuard { + fn drop(&mut self) { + if !self.ptr.is_null() { + unsafe { + // Ignore result: DestroyEnvironmentBlock failure is non-critical during cleanup + let _ = DestroyEnvironmentBlock(self.ptr); + } + } + } +} + +pub fn launch_terminal_helper_with_token( + user_token: UserToken, + input_pipe_name: &str, + output_pipe_name: &str, + terminal_id: i32, + rows: u16, + cols: u16, +) -> Result { + let exe_path = + std::env::current_exe().map_err(|e| anyhow!("Failed to get current exe path: {}", e))?; + + // Build command line arguments (without exe path to avoid escaping issues) + // lpApplicationName will contain the exe path separately + let cmd_args = format!( + "--terminal-helper {} {} {} {} {}", + input_pipe_name, output_pipe_name, rows, cols, terminal_id + ); + + log::debug!("Launching terminal helper for terminal {}", terminal_id); + + // Convert exe path to wide string for lpApplicationName + let exe_path_wide: Vec = OsStr::new(exe_path.as_os_str()) + .encode_wide() + .chain(std::iter::once(0)) + .collect(); + + // Command line must include exe name as first argument per Windows convention + let cmd_line = format!("\"{}\" {}", exe_path.display(), cmd_args); + let mut cmd_wide: Vec = OsStr::new(&cmd_line) + .encode_wide() + .chain(std::iter::once(0)) + .collect(); + + let mut si: STARTUPINFOW = unsafe { std::mem::zeroed() }; + si.cb = std::mem::size_of::() as u32; + + let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + + // Create environment block for the user with RAII cleanup + let mut environment: *mut c_void = ptr::null_mut(); + let env_ok = unsafe { + CreateEnvironmentBlock( + &mut environment, + Some(HANDLE(user_token.as_raw() as _)), + true, + ) + } + .is_ok(); + + // Use RAII guard to ensure cleanup even on error paths + let _env_guard = if env_ok && !environment.is_null() { + Some(EnvironmentBlockGuard { ptr: environment }) + } else { + if !env_ok { + log::warn!("Failed to create environment block, using default"); + } + None + }; + + let creation_flags = CREATE_NO_WINDOW + | if env_ok { + CREATE_UNICODE_ENVIRONMENT + } else { + PROCESS_CREATION_FLAGS(0) + }; + + // Use lpApplicationName to pass exe path separately from command line + // This avoids potential issues with special characters in the exe path + let result = unsafe { + CreateProcessAsUserW( + Some(HANDLE(user_token.as_raw() as _)), + PCWSTR::from_raw(exe_path_wide.as_ptr()), // lpApplicationName: exe path + Some(PWSTR::from_raw(cmd_wide.as_mut_ptr())), // lpCommandLine: full command + None, + None, + false, // Don't inherit handles + creation_flags, + if env_ok { Some(environment) } else { None }, + PCWSTR::null(), // Use default current directory + &si, + &mut pi, + ) + }; + + // Environment block cleanup is handled by _env_guard's Drop + + if let Err(e) = result { + log::error!("CreateProcessAsUserW failed: {}", e); + return Err(anyhow!("Failed to launch terminal helper: {}", e)); + } + + // Close thread handle - we only need the process handle for tracking + // Ignore result: CloseHandle failure here is non-critical since process is already launched + unsafe { + let _ = CloseHandle(pi.hThread); + } + + log::info!("Terminal helper launched with PID {}", pi.dwProcessId); + // Return process info for tracking + Ok(HelperProcessInfo { + handle: pi.hProcess, + pid: pi.dwProcessId, + }) +} + +/// Check if a helper process is still running. +/// Returns true if the process is running, false if it has exited. +pub fn is_helper_process_running(handle: HANDLE) -> bool { + let wait_result = unsafe { WaitForSingleObject(handle, 0) }; + // WAIT_TIMEOUT (258) means process is still running + // WAIT_OBJECT_0 (0) means process has exited + wait_result != WAIT_OBJECT_0 +} + +/// Run terminal helper process +/// Args: --terminal-helper +pub fn run_terminal_helper(args: &[String]) -> Result<()> { + if args.len() < 5 { + return Err(anyhow!( + "Usage: --terminal-helper " + )); + } + + let input_pipe_name = &args[0]; + let output_pipe_name = &args[1]; + let rows: u16 = args[2] + .parse() + .map_err(|e| anyhow!("Failed to parse rows '{}': {}", args[2], e))?; + let cols: u16 = args[3] + .parse() + .map_err(|e| anyhow!("Failed to parse cols '{}': {}", args[3], e))?; + let terminal_id: i32 = args[4] + .parse() + .map_err(|e| anyhow!("Failed to parse terminal_id '{}': {}", args[4], e))?; + + log::debug!( + "Terminal helper starting: terminal_id={}, size={}x{}", + terminal_id, + cols, + rows + ); + + // Open named pipes (created by the service) + let mut input_pipe = open_pipe(input_pipe_name, true)?; + let mut output_pipe = open_pipe(output_pipe_name, false)?; + + // Create ConPTY and shell + let pty_size = PtySize { + rows, + cols, + pixel_width: 0, + pixel_height: 0, + }; + + let pty_system = portable_pty::native_pty_system(); + let pty_pair = pty_system.openpty(pty_size).context("Failed to open PTY")?; + + let shell = get_default_shell(); + log::debug!("Using shell: {}", shell); + + let cmd = CommandBuilder::new(&shell); + let mut child = pty_pair + .slave + .spawn_command(cmd) + .context("Failed to spawn shell")?; + + // Explicitly drop slave after spawning to release resources + drop(pty_pair.slave); + + let pid = child.process_id().unwrap_or(0); + log::debug!("Shell started with PID: {}", pid); + + let mut pty_writer = pty_pair + .master + .take_writer() + .context("Failed to get PTY writer")?; + + let mut pty_reader = pty_pair + .master + .try_clone_reader() + .context("Failed to get PTY reader")?; + + // Wrap pty_pair.master in Arc for sharing with input thread (for resize). + let pty_master: Arc>> = Arc::new(Mutex::new(pty_pair.master)); + + let exiting = Arc::new(AtomicBool::new(false)); + + // Thread: Read from input pipe, parse messages, write data to PTY or handle control commands + let exiting_clone = exiting.clone(); + let pty_master_clone = pty_master.clone(); + let input_thread = thread::spawn(move || { + let mut input_pipe = input_pipe; + let mut header_buf = [0u8; MSG_HEADER_SIZE]; + let mut payload_buf = vec![0u8; 4096]; + + loop { + if exiting_clone.load(Ordering::SeqCst) { + break; + } + + // Read message header + match read_exact_or_eof(&mut input_pipe, &mut header_buf) { + Ok(false) => { + log::debug!("Input pipe EOF"); + break; + } + Ok(true) => {} + Err(e) => { + log::error!("Input pipe header read error: {}", e); + break; + } + } + + let msg_type = header_buf[0]; + let payload_len = + u32::from_le_bytes([header_buf[1], header_buf[2], header_buf[3], header_buf[4]]) + as usize; + + // Validate payload length to prevent denial of service + if payload_len > MAX_PAYLOAD_SIZE { + log::error!( + "Payload too large: {} bytes (max {})", + payload_len, + MAX_PAYLOAD_SIZE + ); + break; + } + + // Ensure payload buffer is large enough + if payload_buf.len() < payload_len { + payload_buf.resize(payload_len, 0); + } + + // Read payload + if payload_len > 0 { + match read_exact_or_eof(&mut input_pipe, &mut payload_buf[..payload_len]) { + Ok(false) => { + log::debug!("Input pipe EOF during payload read"); + break; + } + Ok(true) => {} + Err(e) => { + log::error!("Input pipe payload read error: {}", e); + break; + } + } + } + + match msg_type { + MSG_TYPE_DATA => { + // Write terminal data to PTY + if let Err(e) = pty_writer.write_all(&payload_buf[..payload_len]) { + log::error!("PTY write error: {}", e); + break; + } + if let Err(e) = pty_writer.flush() { + log::error!("PTY flush error: {}", e); + break; + } + } + MSG_TYPE_RESIZE => { + if payload_len >= 4 { + let rows = u16::from_le_bytes([payload_buf[0], payload_buf[1]]); + let cols = u16::from_le_bytes([payload_buf[2], payload_buf[3]]); + log::debug!("Resize: {}x{}", cols, rows); + if let Ok(master) = pty_master_clone.lock() { + let _ = master.resize(PtySize { + rows, + cols, + pixel_width: 0, + pixel_height: 0, + }); + } + } + } + _ => { + // Unknown type may indicate data corruption - stop to avoid parse errors + log::error!("Unknown message type: {}, terminating", msg_type); + break; + } + } + } + log::debug!("Input thread exiting"); + }); + + // Thread: Read from PTY, write to output pipe + let exiting_clone = exiting.clone(); + let output_thread = thread::spawn(move || { + let mut output_pipe = output_pipe; + let mut buf = vec![0u8; 4096]; + loop { + if exiting_clone.load(Ordering::SeqCst) { + break; + } + match pty_reader.read(&mut buf) { + Ok(0) => { + log::debug!("PTY EOF"); + break; + } + Ok(n) => { + if let Err(e) = output_pipe.write_all(&buf[..n]) { + log::error!("Output pipe write error: {}", e); + break; + } + if let Err(e) = output_pipe.flush() { + log::error!("Output pipe flush error: {}", e); + break; + } + } + Err(e) => { + if e.kind() != std::io::ErrorKind::WouldBlock { + log::error!("PTY read error: {}", e); + break; + } + thread::sleep(Duration::from_millis(10)); + } + } + } + log::debug!("Output thread exiting"); + }); + + // Wait for child process to exit + let exit_status = child.wait(); + log::info!("Shell exited: {:?}", exit_status); + + exiting.store(true, Ordering::SeqCst); + + // Wait for threads + let _ = input_thread.join(); + let _ = output_thread.join(); + + // pty_master will be dropped here, releasing PTY resources + drop(pty_master); + + log::info!("Terminal helper exiting"); + Ok(()) +} + +/// Read exactly `buf.len()` bytes from reader. +/// Returns Ok(true) if successful, Ok(false) on EOF, Err on error. +fn read_exact_or_eof(reader: &mut R, buf: &mut [u8]) -> std::io::Result { + let mut pos = 0; + while pos < buf.len() { + match reader.read(&mut buf[pos..]) { + Ok(0) => return Ok(false), // EOF + Ok(n) => pos += n, + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => return Err(e), + } + } + Ok(true) +} + +/// Open a named pipe as a client. +/// `for_read`: true for reading (input pipe), false for writing (output pipe). +fn open_pipe(pipe_name: &str, for_read: bool) -> Result { + let wide_name: Vec = OsStr::new(pipe_name) + .encode_wide() + .chain(std::iter::once(0)) + .collect(); + + let access = if for_read { + FILE_GENERIC_READ.0 + } else { + FILE_GENERIC_WRITE.0 + }; + + let handle = unsafe { + CreateFileW( + PCWSTR::from_raw(wide_name.as_ptr()), + access, + FILE_SHARE_READ | FILE_SHARE_WRITE, + None, + OPEN_EXISTING, + FILE_FLAGS_AND_ATTRIBUTES(0), + None, + ) + }; + + match handle { + Ok(h) => Ok(unsafe { File::from_raw_handle(h.0 as _) }), + Err(e) => Err(anyhow!( + "Failed to open {} pipe '{}': {}", + if for_read { "input" } else { "output" }, + pipe_name, + e + )), + } +} diff --git a/src/server/terminal_service.rs b/src/server/terminal_service.rs index 194e41ef1..743f849c4 100644 --- a/src/server/terminal_service.rs +++ b/src/server/terminal_service.rs @@ -17,6 +17,15 @@ use std::{ time::{Duration, Instant}, }; +// Windows-specific imports from terminal_helper module +#[cfg(target_os = "windows")] +use super::terminal_helper::{ + create_named_pipe_server, encode_helper_message, encode_resize_message, + is_helper_process_running, launch_terminal_helper_with_token, wait_for_pipe_connection, + HelperProcessGuard, OwnedHandle, SendableHandle, WinCloseHandle, WinTerminateProcess, + WinWaitForSingleObject, MSG_TYPE_DATA, PIPE_CONNECTION_TIMEOUT_MS, WIN_WAIT_OBJECT_0, +}; + const MAX_OUTPUT_BUFFER_SIZE: usize = 1024 * 1024; // 1MB per terminal const MAX_BUFFER_LINES: usize = 10000; const MAX_SERVICES: usize = 100; // Maximum number of persistent terminal services @@ -53,28 +62,8 @@ pub fn generate_service_id() -> String { fn get_default_shell() -> String { #[cfg(target_os = "windows")] { - // Try PowerShell Core first (cross-platform version) - // Common installation paths for PowerShell Core - let pwsh_paths = [ - "pwsh.exe", - r"C:\Program Files\PowerShell\7\pwsh.exe", - r"C:\Program Files\PowerShell\6\pwsh.exe", - ]; - - for path in &pwsh_paths { - if std::path::Path::new(path).exists() { - return path.to_string(); - } - } - - // Try Windows PowerShell (should be available on all Windows systems) - let powershell_path = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"; - if std::path::Path::new(powershell_path).exists() { - return powershell_path.to_string(); - } - - // Final fallback to cmd.exe - std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string()) + // Use shared implementation from terminal_helper + super::terminal_helper::get_default_shell() } #[cfg(not(target_os = "windows"))] { @@ -280,7 +269,30 @@ pub fn get_terminal_session_count(include_zombie_tasks: bool) -> usize { c } -pub type UserToken = u64; +/// User token wrapper for cross-module use. +/// +/// # Design Note +/// On Windows, this type is defined in terminal_helper.rs and re-exported here. +/// On non-Windows platforms, it's defined here directly. +/// This design avoids circular dependencies while keeping the API consistent. +/// Both definitions MUST have identical public API (new, as_raw methods). +#[cfg(not(target_os = "windows"))] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct UserToken(pub usize); + +#[cfg(not(target_os = "windows"))] +impl UserToken { + pub fn new(handle: usize) -> Self { + Self(handle) + } + + pub fn as_raw(&self) -> usize { + self.0 + } +} + +#[cfg(target_os = "windows")] +pub use super::terminal_helper::UserToken; #[derive(Clone)] pub struct TerminalService { @@ -458,6 +470,12 @@ pub struct TerminalSession { // Track if we've already sent the closed message closed_message_sent: bool, is_opened: bool, + // Helper mode: PTY is managed by helper process, communication via message protocol + #[cfg(target_os = "windows")] + is_helper_mode: bool, + // Handle to helper process for termination when session closes + #[cfg(target_os = "windows")] + helper_process_handle: Option, } impl TerminalSession { @@ -479,6 +497,10 @@ impl TerminalSession { cols, closed_message_sent: false, is_opened: false, + #[cfg(target_os = "windows")] + is_helper_mode: false, + #[cfg(target_os = "windows")] + helper_process_handle: None, } } @@ -497,14 +519,58 @@ impl TerminalSession { // Send a final newline to ensure the reader can read some data, and then exit. // This is required on Windows and Linux. // Although `self.pty_pair = None;` is called below, we can still send a final newline here. - if let Err(e) = input_tx.send(b"\r\n".to_vec()) { + #[cfg(target_os = "windows")] + let final_msg = if self.is_helper_mode { + encode_helper_message(MSG_TYPE_DATA, b"\r\n") + } else { + b"\r\n".to_vec() + }; + #[cfg(not(target_os = "windows"))] + let final_msg = b"\r\n".to_vec(); + + if let Err(e) = input_tx.send(final_msg) { log::warn!("Failed to send final newline to the terminal: {}", e); } drop(input_tx); } self.output_rx = None; - // 1. Windows + // CRITICAL: In helper mode, we must terminate the helper process BEFORE joining threads! + // The reader thread is blocking on output_pipe.read(), which only returns EOF when + // the helper process exits. If we try to join the reader thread first, we deadlock. + // + // Sequence for helper mode: + // 1. Signal exiting and close input channel (done above) + // 2. Terminate helper process (causes output pipe EOF) + // 3. Join reader thread (now unblocked due to EOF) + // 4. Join writer thread + #[cfg(target_os = "windows")] + if self.is_helper_mode { + if let Some(helper_handle) = self.helper_process_handle.take() { + let handle = helper_handle.as_raw(); + log::debug!("Helper mode: terminating helper process before joining threads..."); + + // Give helper a very short time to exit gracefully (it should detect pipe close) + // But don't wait too long - we need to unblock the reader thread + let wait_result = unsafe { WinWaitForSingleObject(handle, 100) }; + + if wait_result == WIN_WAIT_OBJECT_0 { + log::debug!("Helper process exited gracefully"); + } else { + // Force terminate to unblock reader thread + log::debug!("Force terminating helper process to unblock reader thread"); + unsafe { + let _ = WinTerminateProcess(handle, 0); + } + } + + unsafe { + let _ = WinCloseHandle(handle); + } + } + } + + // 1. Windows (non-helper mode) // `pty_pair` uses pipe. https://github.com/rustdesk-org/wezterm/blob/80174f8009f41565f0fa8c66dab90d4f9211ae16/pty/src/win/conpty.rs#L16 // `read()` may stuck at https://github.com/rustdesk-org/wezterm/blob/80174f8009f41565f0fa8c66dab90d4f9211ae16/filedescriptor/src/windows.rs#L345 // We can close the pipe to signal the reader thread to exit. @@ -747,6 +813,15 @@ impl TerminalServiceProxy { return Ok(Some(response)); } + // Windows with user_token: use helper process to run shell as the logged-in user + // This solves the ConPTY + CreateProcessAsUserW incompatibility issue where + // vim, Claude Code, and other TUI applications hang when ConPTY is created + // by SYSTEM service but shell runs as user via CreateProcessAsUserW. + #[cfg(target_os = "windows")] + if self.user_token.is_some() { + return self.handle_open_with_helper(service, open); + } + // Create new terminal session log::info!( "Creating new terminal {} for service: {}", @@ -774,12 +849,19 @@ impl TerminalServiceProxy { #[allow(unused_mut)] let mut cmd = CommandBuilder::new(&shell); - // Set `TERM` environment variable for macOS to ensure proper terminal behavior - // This fixes issues with control sequences (e.g., Delete/Backspace keys) - // macOS terminfo uses hex naming: '78' = 'x' for xterm entries + // macOS-specific terminal configuration + // 1. Use login shell (-l) to load user's shell profile (~/.zprofile, ~/.bash_profile) + // This ensures PATH includes Homebrew paths (/opt/homebrew/bin, /usr/local/bin) + // 2. Set TERM environment variable for proper terminal behavior + // This fixes issues with control sequences (e.g., Delete/Backspace keys) + // macOS terminfo uses hex naming: '78' = 'x' for xterm entries // Note: For Linux, `TERM` is set in src/platform/linux.rs try_start_server_() #[cfg(target_os = "macos")] { + // Start as login shell to load user environment (PATH, etc.) + cmd.arg("-l"); + log::debug!("Added -l flag for macOS login shell"); + let term = if std::path::Path::new("/usr/share/terminfo/78/xterm-256color").exists() { "xterm-256color" } else { @@ -789,10 +871,9 @@ impl TerminalServiceProxy { log::debug!("Set TERM={} for macOS PTY", term); } - #[cfg(target_os = "windows")] - if let Some(token) = &self.user_token { - cmd.set_user_token(*token as _); - } + // Note: On Windows with user_token, we use helper mode (handle_open_with_helper) + // which is dispatched earlier in this function. This code path is only reached + // when user_token is None (e.g., running directly as user, not as SYSTEM service). log::debug!("Spawning shell process..."); let child = pty_pair @@ -820,17 +901,6 @@ impl TerminalServiceProxy { let terminal_id = open.terminal_id; let writer_thread = thread::spawn(move || { let mut writer = writer; - // Write initial carriage return: - // 1. Windows requires at least one carriage return for `drop()` to work properly. - // Without this, the reader may fail to read the buffer after `input_tx.send(b"\r\n".to_vec()).ok();`. - // 2. This also refreshes the terminal interface on the controlling side (workaround for blank content on connect). - if let Err(e) = writer.write_all(b"\r") { - log::error!("Terminal {} initial write error: {}", terminal_id, e); - } else { - if let Err(e) = writer.flush() { - log::error!("Terminal {} initial flush error: {}", terminal_id, e); - } - } while let Ok(data) = input_rx.recv() { if let Err(e) = writer.write_all(&data) { log::error!("Terminal {} write error: {}", terminal_id, e); @@ -930,6 +1000,222 @@ impl TerminalServiceProxy { Ok(Some(response)) } + /// Windows-only: Open terminal using helper process pattern + /// This solves the ConPTY + CreateProcessAsUserW incompatibility issue. + /// The helper process runs as the logged-in user and creates ConPTY + shell, + /// communicating with this service via named pipes. + #[cfg(target_os = "windows")] + fn handle_open_with_helper( + &self, + service: &mut PersistentTerminalService, + open: &OpenTerminal, + ) -> Result> { + let mut response = TerminalResponse::new(); + + log::info!( + "Creating new terminal {} using helper process for service: {}", + open.terminal_id, + service.service_id + ); + + let mut session = + TerminalSession::new(open.terminal_id, open.rows as u16, open.cols as u16); + + // Generate unique pipe names for this terminal + let pipe_id = uuid::Uuid::new_v4(); + let input_pipe_name = format!(r"\\.\pipe\rustdesk_term_in_{}", pipe_id); + let output_pipe_name = format!(r"\\.\pipe\rustdesk_term_out_{}", pipe_id); + + log::debug!( + "Creating pipes: input={}, output={}", + input_pipe_name, + output_pipe_name + ); + + // Get user_token early - needed for both DACL creation and helper launch + let user_token = self + .user_token + .ok_or_else(|| anyhow!("user_token is required for helper mode"))?; + + // Create pipes (server side, don't wait for connection yet) + // input_pipe: service WRITES to this, helper READS from this + // output_pipe: service READS from this, helper WRITES to this + // Using OwnedHandle for RAII - handles are automatically closed on error + // Pass user_token to create restricted DACL (only SYSTEM + user can access) + let input_pipe_handle = OwnedHandle::new(create_named_pipe_server( + &input_pipe_name, + false, + user_token, + )?); + let output_pipe_handle = OwnedHandle::new(create_named_pipe_server( + &output_pipe_name, + true, + user_token, + )?); + + let helper_process_info = launch_terminal_helper_with_token( + user_token, + &input_pipe_name, + &output_pipe_name, + open.terminal_id, + open.rows as u16, + open.cols as u16, + )?; + + // Use HelperProcessGuard for RAII cleanup - terminates process on error + // Unlike OwnedHandle which only closes the handle, this guard ensures + // the helper process is terminated if pipe connection fails or other errors occur. + let helper_process_guard = + HelperProcessGuard::new(helper_process_info.handle, helper_process_info.pid); + let helper_pid = helper_process_guard.pid(); + + // Wait for helper to connect to pipes + // If this fails, HelperProcessGuard will terminate the helper process + let mut input_pipe = wait_for_pipe_connection( + input_pipe_handle, + &input_pipe_name, + PIPE_CONNECTION_TIMEOUT_MS, + )?; + let mut output_pipe = wait_for_pipe_connection( + output_pipe_handle, + &output_pipe_name, + PIPE_CONNECTION_TIMEOUT_MS, + )?; + + // Check if helper process is still running after pipe connection + // This provides early detection if helper crashed during startup + if !is_helper_process_running(helper_process_guard.as_raw()) { + return Err(anyhow!( + "Helper process (PID {}) exited unexpectedly after pipe connection", + helper_pid + )); + } + + // Disarm the guard and transfer ownership to session + // From this point, the session is responsible for terminating the helper + let helper_raw_handle = helper_process_guard.disarm(); + + // Use helper process PID for session tracking + // Note: This is the helper process PID, not the actual shell PID. + // The real shell runs inside the helper process but its PID is not exposed here. + // For process management (termination, status), the helper PID is what we need. + session.pid = helper_pid; + + // Create channels for input/output (same as direct PTY mode) + let (input_tx, input_rx) = mpsc::sync_channel::>(CHANNEL_BUFFER_SIZE); + let (output_tx, output_rx) = mpsc::sync_channel::>(CHANNEL_BUFFER_SIZE); + + // Spawn writer thread: reads from channel, writes to input pipe + let terminal_id = open.terminal_id; + let writer_thread = thread::spawn(move || { + while let Ok(data) = input_rx.recv() { + if let Err(e) = input_pipe.write_all(&data) { + log::error!("Terminal {} pipe write error: {}", terminal_id, e); + break; + } + if let Err(e) = input_pipe.flush() { + log::error!("Terminal {} pipe flush error: {}", terminal_id, e); + } + } + log::debug!( + "Terminal {} writer thread (helper mode) exiting", + terminal_id + ); + }); + + // Spawn reader thread: reads from output pipe, sends to channel + // Note: The output pipe was created with FILE_FLAG_OVERLAPPED for timeout support + // during ConnectNamedPipe. However, once converted to a File handle, reads are + // performed synchronously. The WouldBlock handling below is defensive but may + // not be triggered in practice since File::read() blocks until data is available. + let exiting = session.exiting.clone(); + let terminal_id = open.terminal_id; + let reader_thread = thread::spawn(move || { + let mut buf = vec![0u8; 4096]; + loop { + match output_pipe.read(&mut buf) { + Ok(0) => { + // EOF - helper process exited + log::debug!("Terminal {} helper output EOF", terminal_id); + break; + } + Ok(n) => { + if exiting.load(Ordering::SeqCst) { + break; + } + let data = buf[..n].to_vec(); + match output_tx.try_send(data) { + Ok(_) => {} + Err(mpsc::TrySendError::Full(_)) => { + log::debug!( + "Terminal {} output channel full, dropping data", + terminal_id + ); + } + Err(mpsc::TrySendError::Disconnected(_)) => { + log::debug!("Terminal {} output channel disconnected", terminal_id); + break; + } + } + } + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => { + // Defensive: WouldBlock is unlikely with synchronous File::read(), + // but handle it gracefully just in case. + if exiting.load(Ordering::SeqCst) { + break; + } + thread::sleep(Duration::from_millis(10)); + } + Err(e) => { + log::error!("Terminal {} pipe read error: {}", terminal_id, e); + break; + } + } + } + log::debug!( + "Terminal {} reader thread (helper mode) exiting", + terminal_id + ); + }); + + // In helper mode, we don't have pty_pair or child - helper manages those + session.pty_pair = None; + session.child = None; + session.input_tx = Some(input_tx); + session.output_rx = Some(output_rx); + session.reader_thread = Some(reader_thread); + session.writer_thread = Some(writer_thread); + session.is_opened = true; + session.is_helper_mode = true; + session.helper_process_handle = Some(SendableHandle::new(helper_raw_handle)); + + let mut opened = TerminalOpened::new(); + opened.terminal_id = open.terminal_id; + opened.success = true; + opened.message = "Terminal opened (helper mode)".to_string(); + opened.pid = session.pid; + opened.service_id = service.service_id.clone(); + if service.needs_session_sync { + if !service.sessions.is_empty() { + opened.persistent_sessions = service.sessions.keys().cloned().collect(); + } + service.needs_session_sync = false; + } + response.set_opened(opened); + + log::info!( + "Terminal {} opened successfully using helper process (PID {})", + open.terminal_id, + session.pid + ); + + service + .sessions + .insert(open.terminal_id, Arc::new(Mutex::new(session))); + + Ok(Some(response)) + } + fn handle_resize( &self, session: Option>>, @@ -941,18 +1227,50 @@ impl TerminalServiceProxy { session.rows = resize.rows as u16; session.cols = resize.cols as u16; - if let Some(pty_pair) = &session.pty_pair { - pty_pair.master.resize(PtySize { - rows: resize.rows as u16, - cols: resize.cols as u16, - pixel_width: 0, - pixel_height: 0, - })?; + // Windows: handle helper mode vs direct PTY mode + #[cfg(target_os = "windows")] + { + if session.is_helper_mode { + // Helper mode: send resize command via message protocol + if let Some(input_tx) = &session.input_tx { + let msg = encode_resize_message(resize.rows as u16, resize.cols as u16); + if let Err(e) = input_tx.send(msg) { + log::error!("Failed to send resize to helper: {}", e); + } + } else { + log::warn!( + "Terminal {} is in helper mode but input_tx is None, cannot send resize", + resize.terminal_id + ); + } + } else { + // Direct PTY mode + Self::resize_pty(&session, resize)?; + } + } + + // Non-Windows: always direct PTY mode + #[cfg(not(target_os = "windows"))] + { + Self::resize_pty(&session, resize)?; } } Ok(None) } + /// Resize PTY directly (used for non-helper mode) + fn resize_pty(session: &TerminalSession, resize: &ResizeTerminal) -> Result<()> { + if let Some(pty_pair) = &session.pty_pair { + pty_pair.master.resize(PtySize { + rows: resize.rows as u16, + cols: resize.cols as u16, + pixel_width: 0, + pixel_height: 0, + })?; + } + Ok(()) + } + fn handle_data( &self, session: Option>>, @@ -962,8 +1280,18 @@ impl TerminalServiceProxy { let mut session = session_arc.lock().unwrap(); session.update_activity(); if let Some(input_tx) = &session.input_tx { + // Encode data for helper mode or send raw for direct PTY mode + #[cfg(target_os = "windows")] + let msg = if session.is_helper_mode { + encode_helper_message(MSG_TYPE_DATA, &data.data) + } else { + data.data.to_vec() + }; + #[cfg(not(target_os = "windows"))] + let msg = data.data.to_vec(); + // Send data to writer thread - if let Err(e) = input_tx.send(data.data.to_vec()) { + if let Err(e) = input_tx.send(msg) { log::error!( "Failed to send data to terminal {}: {}", data.terminal_id,