diff --git a/Cargo.lock b/Cargo.lock index db6818b..da70ba6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -132,6 +132,7 @@ dependencies = [ "signal-hook 0.4.4", "thiserror 2.0.18", "url", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 05bb13e..e577641 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,5 +37,14 @@ url = "2.5.8" rustix = { version = "1.1.4", features = ["event"] } signal-hook = "0.4.4" +# Console VT input/output modes for ConPTY forwarding and clean stdin-reader +# shutdown after recording; see src/recorder/windows.rs. +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = [ + "Win32_Foundation", + "Win32_System_Console", + "Win32_System_Threading", +] } + [dev-dependencies] httpmock = "0.8.3" diff --git a/README.md b/README.md index e1911de..b7cd38c 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ The config file follows the XDG standard and lives at | Config file key | CLI flag | Meaning | | ---------------- | ------------------------- | ----------------------------------------------------------------- | | `outputDir` | | Where to store recording files (defaults to the XDG data dir) | -| `recordingShell` | `-s`, `--shell` | Shell to launch (defaults to `$SHELL`) | +| `recordingShell` | `-s`, `--shell` | Shell to launch (defaults to `$SHELL` on Unix; on Windows, `pwsh` if installed, else `powershell.exe`) | | `operationSlug` | `--operation` | Operation to upload to (can also be selected before recording) | | `apiURL` | | Where the ASHIRT backend service is located | | `accessKey` | | Access Key for the backend (created in the frontend) | diff --git a/src/config.rs b/src/config.rs index a8564cf..e4ed24b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -157,13 +157,14 @@ struct ConfigFile { impl Config { /// Returns the built-in defaults — the lowest-precedence source. /// - /// Mirrors the Go `TermRecorderConfigWithDefaults`: schema version `1`, the - /// recording shell taken from the `SHELL` environment variable, and the + /// Mirrors the Go `TermRecorderConfigWithDefaults`: schema version `1`, a + /// platform-appropriate recording shell (see [`default_shell`] — `$SHELL` on + /// Unix, PowerShell on Windows, preferring `pwsh` when installed), and the /// output base defaulting to aterm's per-user XDG data directory. pub fn with_defaults() -> Self { Config { config_version: 1, - recording_shell: std::env::var("SHELL").unwrap_or_default(), + recording_shell: default_shell(), // Recordings are *data*, so base them under aterm's per-user XDG // data directory by default (`~/.local/share/aterm` on Linux, // honouring `$XDG_DATA_HOME`; the platform data dir on Windows; @@ -459,6 +460,54 @@ fn default_output_dir() -> String { } } +/// Returns the platform-appropriate default recording shell. +/// +/// * Unix (Linux/macOS): the user's login shell from `$SHELL`, falling back to +/// `/bin/sh` when it is unset or empty. This is what "pull whatever the +/// `SHELL` environment variable is" means at configuration time. +/// * Windows: PowerShell 7 (`pwsh.exe`) when it is on `PATH`, otherwise Windows +/// PowerShell (`powershell.exe`). Windows has no `$SHELL`, and `%COMSPEC%` +/// points at the legacy `cmd.exe`, so PowerShell is the saner modern default. +/// `pwsh` is preferred because aterm forwards its environment to the child: +/// when launched from `pwsh`, the inherited `PSModulePath` points at +/// PowerShell 7's modules, which breaks module loading (PSReadLine included) +/// in a Windows PowerShell 5.1 child. Launching `pwsh` keeps the shell and its +/// `PSModulePath` consistent; `powershell.exe` is the always-present fallback. +/// +/// Always returns a non-empty value, so it doubles as the last-resort fallback +/// when no shell is configured (see [`crate::menu`]). +pub fn default_shell() -> String { + #[cfg(windows)] + { + if executable_on_path("pwsh.exe") { + "pwsh.exe".to_string() + } else { + "powershell.exe".to_string() + } + } + #[cfg(not(windows))] + { + match std::env::var("SHELL") { + Ok(shell) if !shell.trim().is_empty() => shell, + _ => "/bin/sh".to_string(), + } + } +} + +/// Returns whether `exe` is found in any directory on `PATH`. +/// +/// A minimal `which`-style lookup (no extra dependency) used to prefer +/// `pwsh.exe` over `powershell.exe` only when PowerShell 7 is actually +/// installed. `PATH` is split with the platform separator via +/// [`std::env::split_paths`]. +#[cfg(windows)] +fn executable_on_path(exe: &str) -> bool { + let Some(path) = std::env::var_os("PATH") else { + return false; + }; + std::env::split_paths(&path).any(|dir| dir.join(exe).is_file()) +} + /// Returns the platform configuration directory for aterm. /// /// A plain `aterm` directory under the per-user config base: @@ -803,6 +852,38 @@ mod tests { fs::remove_dir(&dir).ok(); } + /// The default shell is always a non-empty, platform-appropriate value, so + /// `with_defaults` never seeds an empty `recording_shell` and the menu + /// fallback always has something to launch. + #[test] + fn default_shell_is_non_empty_and_platform_appropriate() { + let shell = default_shell(); + assert!(!shell.trim().is_empty(), "default shell must be non-empty"); + + // Windows prefers `pwsh.exe` when on PATH, else `powershell.exe`. + #[cfg(windows)] + assert!( + shell == "pwsh.exe" || shell == "powershell.exe", + "Windows default shell must be pwsh.exe or powershell.exe, got {shell}" + ); + + // On Unix the default is `$SHELL` when set, else `/bin/sh`. + #[cfg(not(windows))] + match std::env::var("SHELL") { + Ok(s) if !s.trim().is_empty() => assert_eq!( + shell, s, + "Unix default shell must come from $SHELL when set" + ), + _ => assert_eq!( + shell, "/bin/sh", + "Unix default shell must fall back to /bin/sh" + ), + } + + // `with_defaults` seeds the same value. + assert_eq!(Config::with_defaults().recording_shell, shell); + } + #[test] fn config_path_ends_with_expected_segments() { // Don't assert the platform prefix, just the trailing structure. diff --git a/src/lib.rs b/src/lib.rs index c4c4422..e0d1e2b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,9 +12,13 @@ //! `anyhow::Result` is used ONLY at the command boundary ([`app::run`] and //! `main`). See [`config`] for a worked thiserror example and [`app`] for the //! anyhow boundary. -//! * No `unsafe`: the crate is `#![forbid(unsafe_code)]`. Platform syscalls go -//! through safe wrappers (e.g. `rustix` on Unix). -#![forbid(unsafe_code)] +//! * No `unsafe` by default: the crate is `#![deny(unsafe_code)]`. Platform +//! syscalls go through safe wrappers (e.g. `rustix` on Unix). The one +//! exception is the Windows console FFI in [`recorder::windows`] — VT +//! console-mode setup and cancelling the blocking stdin read — which has no +//! safe-wrapper equivalent and opts in via a scoped, documented +//! `#[allow(unsafe_code)]` on that module alone. +#![deny(unsafe_code)] pub mod app; pub mod asciicast; diff --git a/src/menu.rs b/src/menu.rs index 8822a5a..74b02b3 100644 --- a/src/menu.rs +++ b/src/menu.rs @@ -666,17 +666,16 @@ fn start_recording( Ok(()) } -/// Picks the shell to record: the configured shell, falling back to `$SHELL`, -/// then `/bin/sh`. The configured value is normally already seeded from `$SHELL` -/// by [`Config::with_defaults`]; this keeps a sensible last resort. +/// Picks the shell to record: the configured shell, falling back to the +/// platform default ([`crate::config::default_shell`] — `$SHELL` on Unix, +/// PowerShell on Windows). The configured value is normally already seeded with +/// that same default by [`Config::with_defaults`]; this keeps a sensible last +/// resort when the config carries an empty shell. fn recording_shell(config: &Config) -> String { if !config.recording_shell.trim().is_empty() { return config.recording_shell.clone(); } - match std::env::var("SHELL") { - Ok(shell) if !shell.trim().is_empty() => shell, - _ => "/bin/sh".to_string(), - } + crate::config::default_shell() } #[cfg(test)] diff --git a/src/recorder/mod.rs b/src/recorder/mod.rs index 2f73eee..25939f4 100644 --- a/src/recorder/mod.rs +++ b/src/recorder/mod.rs @@ -131,6 +131,10 @@ impl PtySession { .map_err(|e| RecorderError::OpenPty(e.to_string()))?; let mut cmd = CommandBuilder::new(shell); + // Forward the current environment verbatim. `std::env::vars()` only yields + // variables that are actually set, so `TERM`/`SHELL` are passed through + // when present (e.g. a Git Bash / MSYS / WSL-interop launch) and simply + // absent otherwise — native Windows consoles define neither. for (key, value) in std::env::vars() { cmd.env(key, value); } @@ -454,6 +458,11 @@ pub fn record_session( // Raw mode is entered only after the PTY is up so an early failure leaves the // terminal untouched. The guard restores cooked mode on any exit path. let _raw = RawModeGuard::enable()?; + // Platform terminal-mode setup, restored on drop: on Windows this enables the + // console's virtual-terminal input/output so host key presses reach the + // ConPTY child as VT sequences and its VT output renders; on Unix it is a + // no-op (the PTY already does this). Dropped after teardown, before the menu. + let _term_mode = platform::TerminalModeGuard::install()?; let mut resize_watcher = platform::ResizeWatcher::install()?; let mut stdin_poller = platform::StdinPoller::new()?; diff --git a/src/recorder/unix.rs b/src/recorder/unix.rs index 59b032a..5dae8b5 100644 --- a/src/recorder/unix.rs +++ b/src/recorder/unix.rs @@ -18,9 +18,21 @@ use rustix::io::Errno; use super::StdinRead; -/// Returns the user's preferred shell (`$SHELL`), falling back to `/bin/sh`. -pub fn default_shell() -> String { - std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()) +/// No-op terminal-mode guard on Unix. +/// +/// Unix PTYs already deliver raw VT input and render VT output without any extra +/// console-mode setup, so the cooked-mode clearing done by the shared +/// [`RawModeGuard`](super::RawModeGuard) is sufficient. This type exists only so +/// the shared recorder can install platform terminal-mode setup uniformly; its +/// Windows counterpart configures the console's virtual-terminal modes. +#[must_use = "kept symmetric with the Windows guard; bind it to a variable"] +pub struct TerminalModeGuard; + +impl TerminalModeGuard { + /// Installs nothing; succeeds unconditionally. + pub fn install() -> io::Result { + Ok(Self) + } } /// Detects terminal resizes via the `SIGWINCH` signal. diff --git a/src/recorder/windows.rs b/src/recorder/windows.rs index 3b3157c..95608ac 100644 --- a/src/recorder/windows.rs +++ b/src/recorder/windows.rs @@ -4,23 +4,134 @@ //! shared in [`super`] and runs over ConPTY (selected by `portable-pty`). On //! Windows there is no `SIGWINCH`, so: //! +//! * console VT modes are configured for the recording so host key presses are +//! forwarded to the ConPTY child as VT escape sequences and the child's VT +//! output renders correctly ([`TerminalModeGuard`]); //! * resize detection polls the console size and emits a resize whenever it //! changes ([`ResizeWatcher`]); the shared session then drives the ConPTY //! resize; //! * stdin is read on a dedicated thread feeding a channel ([`StdinPoller`]), //! so the input-forwarding loop can poll with a timeout and react to the //! child exiting rather than blocking on a console read. +//! +//! # `unsafe` +//! +//! The crate is otherwise `#![deny(unsafe_code)]`; this module is the single +//! opt-out. The Windows console APIs used here — `GetStdHandle` / +//! `GetConsoleMode` / `SetConsoleMode` for VT mode setup, and +//! `WaitForSingleObject` to wait on the console input handle with a timeout so +//! the stdin reader stays interruptible — have no safe-wrapper equivalent (the +//! way `rustix` covers the Unix syscalls), so each call is a small, individually +//! justified `unsafe` block with a `SAFETY` comment. +#![allow(unsafe_code)] use std::io::{self, Read}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{self, Receiver, RecvTimeoutError}; -use std::thread; +use std::thread::{self, JoinHandle}; use std::time::Duration; +use windows_sys::Win32::Foundation::{HANDLE, WAIT_OBJECT_0, WAIT_TIMEOUT}; +use windows_sys::Win32::System::Console::{ + CONSOLE_MODE, DISABLE_NEWLINE_AUTO_RETURN, ENABLE_ECHO_INPUT, ENABLE_LINE_INPUT, + ENABLE_PROCESSED_INPUT, ENABLE_VIRTUAL_TERMINAL_INPUT, ENABLE_VIRTUAL_TERMINAL_PROCESSING, + GetConsoleMode, GetStdHandle, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, SetConsoleMode, +}; +use windows_sys::Win32::System::Threading::WaitForSingleObject; + use super::StdinRead; -/// Returns the user's shell. Honors `%COMSPEC%`, falling back to `cmd.exe`. -pub fn default_shell() -> String { - std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string()) +/// Reads the console mode for `handle`, returning `None` when the handle is not +/// a console (e.g. redirected stdin/stdout) so callers can no-op gracefully. +fn console_mode(handle: HANDLE) -> Option { + let mut mode: CONSOLE_MODE = 0; + // SAFETY: `handle` comes from `GetStdHandle`; `GetConsoleMode` writes the + // current mode through the out-pointer and returns nonzero on success. + let ok = unsafe { GetConsoleMode(handle, &mut mode) }; + (ok != 0).then_some(mode) +} + +/// Best-effort `SetConsoleMode`; failures (e.g. redirected handles) are ignored. +fn set_console_mode(handle: HANDLE, mode: CONSOLE_MODE) { + // SAFETY: `handle` comes from `GetStdHandle`; the call only reads `mode`. + unsafe { + let _ = SetConsoleMode(handle, mode); + } +} + +/// Configures the host console's VT modes for the duration of a recording and +/// restores the exact previous modes on drop. +/// +/// Unlike crossterm's raw mode — which only *clears* the cooked-input flags and +/// never touches the virtual-terminal flags — a ConPTY input forwarder must: +/// +/// * **input:** enable `ENABLE_VIRTUAL_TERMINAL_INPUT` so the console encodes +/// key presses (arrows, Ctrl-R, function keys, …) as the VT escape sequences +/// the child shell expects. PowerShell/PSReadLine's line editor and +/// reverse-search only work when those sequences arrive; without the flag +/// the console delivers only typed characters and special keys are lost. The +/// cooked-input flags are also cleared so the child — not our console — owns +/// line editing and echo. +/// * **output:** enable `ENABLE_VIRTUAL_TERMINAL_PROCESSING` so the child's VT +/// output (colours, cursor motion) renders instead of printing escape bytes +/// literally, plus `DISABLE_NEWLINE_AUTO_RETURN` to stop the console adding a +/// carriage return at the right margin. +/// +/// Restoring the captured modes on drop matters as much as setting them: the +/// post-recording menu uses crossterm/inquire, which reads native key-event +/// records rather than VT input, so leaving `ENABLE_VIRTUAL_TERMINAL_INPUT` +/// enabled is what makes the menu's arrow keys misbehave afterwards. +#[must_use = "console modes are restored when the guard is dropped; bind it to a variable"] +pub struct TerminalModeGuard { + stdin: HANDLE, + stdout: HANDLE, + original_stdin: Option, + original_stdout: Option, +} + +impl TerminalModeGuard { + /// Captures the current console modes and switches them into VT mode for + /// recording. Non-console handles are left untouched (and not restored). + pub fn install() -> io::Result { + // SAFETY: `GetStdHandle` returns this process's standard handles. + let stdin = unsafe { GetStdHandle(STD_INPUT_HANDLE) }; + let stdout = unsafe { GetStdHandle(STD_OUTPUT_HANDLE) }; + + let original_stdin = console_mode(stdin); + let original_stdout = console_mode(stdout); + + if let Some(mode) = original_stdin { + let new = (mode & !(ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_PROCESSED_INPUT)) + | ENABLE_VIRTUAL_TERMINAL_INPUT; + set_console_mode(stdin, new); + } + + if let Some(mode) = original_stdout { + let new = mode | ENABLE_VIRTUAL_TERMINAL_PROCESSING | DISABLE_NEWLINE_AUTO_RETURN; + set_console_mode(stdout, new); + } + + Ok(Self { + stdin, + stdout, + original_stdin, + original_stdout, + }) + } +} + +impl Drop for TerminalModeGuard { + fn drop(&mut self) { + // Best-effort restore of the exact modes captured at install; nothing is + // actionable if it fails and a Drop impl must not panic. + if let Some(mode) = self.original_stdin { + set_console_mode(self.stdin, mode); + } + if let Some(mode) = self.original_stdout { + set_console_mode(self.stdout, mode); + } + } } /// Detects terminal resizes by polling the console size. @@ -56,25 +167,66 @@ impl ResizeWatcher { /// Console reads block until input arrives, so a background thread performs the /// blocking reads and forwards chunks over a channel; [`poll`](StdinPoller::poll) /// drains the channel with a timeout. +/// +/// The thread is stopped and joined on drop (see [`Drop`]). This matters because +/// a blocking console read left running past the end of a recording would steal +/// the first keystroke aimed at the post-recording menu — and a join on a thread +/// stuck in an un-interruptible read would hang until the user pressed a key. To +/// avoid both, the reader gates each read behind a short timed wait so it can +/// observe the stop flag and exit on its own. pub struct StdinPoller { rx: Receiver>, leftover: Vec, eof: bool, + /// The reader thread, taken and joined in [`Drop`]. + reader: Option>, + /// Signals the reader thread to stop between waits. + stop: Arc, } impl StdinPoller { /// Spawns the background stdin reader. pub fn new() -> io::Result { let (tx, rx) = mpsc::channel(); - thread::spawn(move || { + let stop = Arc::new(AtomicBool::new(false)); + let reader_stop = Arc::clone(&stop); + let reader = thread::spawn(move || { + // SAFETY: returns this process's standard input handle. + let handle: HANDLE = unsafe { GetStdHandle(STD_INPUT_HANDLE) }; let stdin = io::stdin(); let mut lock = stdin.lock(); let mut buf = [0u8; 8192]; loop { + if reader_stop.load(Ordering::Relaxed) { + break; + } + // A blocking console read cannot be cancelled, so it would pin the + // thread until the next keystroke — making teardown hang and the + // post-recording menu fail to appear. Instead wait on the console + // handle with a 50ms timeout: on timeout the loop re-checks `stop` + // and can exit on its own (the shell-exit case, where no input is + // pending), and only reads when input is actually available. + // SAFETY: `handle` is the console input handle from GetStdHandle. + match unsafe { WaitForSingleObject(handle, 50) } { + WAIT_TIMEOUT => continue, + WAIT_OBJECT_0 => {} + // WAIT_FAILED / WAIT_ABANDONED: stop forwarding rather than + // spin on a handle we can no longer wait on. + _ => break, + } + // Input is pending, so this read returns promptly. (A lone + // non-character event could still make `read` block until the next + // character, but at shell-exit the thread is parked in the wait + // above, not here, so teardown is not held up.) match lock.read(&mut buf) { Ok(0) => break, Ok(n) => { - if tx.send(buf[..n].to_vec()).is_err() { + // If we were asked to stop, or the receiver is gone, drop + // this chunk and exit rather than forward (and thereby + // consume) input meant for the next phase. + if reader_stop.load(Ordering::Relaxed) + || tx.send(buf[..n].to_vec()).is_err() + { break; } } @@ -86,6 +238,8 @@ impl StdinPoller { rx, leftover: Vec::new(), eof: false, + reader: Some(reader), + stop, }) } @@ -112,3 +266,16 @@ impl StdinPoller { Ok(StdinRead::Data(n)) } } + +impl Drop for StdinPoller { + fn drop(&mut self) { + // Signal the reader to stop. It is parked in a 50ms-bounded wait rather + // than an un-interruptible console read, so it observes this and exits + // within one wait interval — the join completes promptly without needing + // a keystroke, letting the post-recording menu appear immediately. + self.stop.store(true, Ordering::Relaxed); + if let Some(reader) = self.reader.take() { + let _ = reader.join(); + } + } +}