From 760cb7aeca0f8ba91b1544474f604f3899e749e5 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:34:48 -0500 Subject: [PATCH] refactor(cli): replace hand-rolled terminal code with crossterm The hidden API-key prompt reads keys in crossterm's raw mode instead of rpassword, and a guard turns raw mode off through crossterm, which restores the saved terminal modes when Ctrl-C cancels a read that cannot return. This removes the unsafe tcgetattr/tcsetattr exception, the rpassword dependency, and the Unix-only restore: pickers and the hidden prompt now restore the terminal on every platform. A typed Ctrl-C at the hidden prompt exits 130 with the same interruption message as before. The picker, live-watch, and status-line escape sequences come from crossterm commands instead of literal bytes; the bytes written are the same, except that home is now ESC[1;1H. --- CHANGELOG.md | 1 + Cargo.lock | 22 ---- Cargo.toml | 9 +- docs/quality.md | 4 - src/output.rs | 25 ++++- src/progress.rs | 10 +- src/terminal.rs | 232 ++++++++++++++++++++++++++++++--------- tests/cli_integration.rs | 35 ++++++ 8 files changed, 249 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..799dd54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - A payment that settles on a terminal closes with `⚡ Payment ID completed` instead of a check mark, printed after the payment's record so it stays on screen. - `voltage payments list --watch` follows payments until Ctrl-C: a live table that marks new and changed payments on a terminal, or one `snapshot`, `new`, or `updated` event per payment when piped or with `--json`. `--interval` sets the refresh (default 5 seconds). - `voltage` with no command, on a terminal, greets you with the wordmark, who you are signed in as, your saved scope, and commands to try; off a terminal it is still a usage error. An empty wallet, payment, or webhook list, and a new profile, end with a `›` line naming the next command. +- The hidden API-key prompt of `voltage auth import-key` is shown on stderr, accepts Ctrl-U to clear what was typed, and restores the terminal after Ctrl-C on every platform. Ctrl-C there exits 130 with `Interrupted before any resource change was submitted`, as it did on Unix. ## 0.2.0 - 2026-10-22 diff --git a/Cargo.lock b/Cargo.lock index 7ea7934..3f2a666 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1805,27 +1805,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "rpassword" -version = "7.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2da316a15f47e3d053de9cb2c439650bd8fa4aaeb9365f2e5f27f492ff73c196" -dependencies = [ - "libc", - "rtoolbox", - "windows-sys 0.61.2", -] - -[[package]] -name = "rtoolbox" -version = "0.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a1efe12a1469752d0e6ff5ebec0b6ef4924cc5c4c71046b0ec730040535819d" -dependencies = [ - "libc", - "windows-sys 0.61.2", -] - [[package]] name = "rustc_version" version = "0.4.1" @@ -2557,7 +2536,6 @@ dependencies = [ "libc", "qrcode", "reqwest", - "rpassword", "rustls", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index cba699b..997e383 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,6 @@ rustls = { version = "=0.23.46", default-features = false, features = ["ring", " uuid = { version = "=1.28.0", features = ["v4", "serde"] } sha2 = "=0.11.1" hex = "=0.4.3" -rpassword = "=7.5.4" webbrowser = "=1.2.4" futures-util = "=0.3.34" url = "=2.5.8" @@ -48,9 +47,11 @@ comfy-table = { version = "=8.0.1", default-features = false } # Display widths, to find a table column after layout and color it; comfy-table already # depends on it, so it adds no crate. unicode-width = "=0.2.2" -# Reads the terminal width on every platform. Without its `windows` feature, crossterm -# refuses to compile on Windows. -crossterm = { version = "=0.29.0", default-features = false, features = ["windows"] } +# The terminal backend: width, raw mode for hidden prompts (it saves and restores the +# terminal's modes), key events, and the screen and cursor commands. Without its `windows` +# feature, crossterm refuses to compile on Windows. `events` and `bracketed-paste` are +# already on through inquire. +crossterm = { version = "=0.29.0", default-features = false, features = ["windows", "events", "bracketed-paste"] } # Pickers for a missing organization, environment, or wallet. Its default filter matches a # substring of a name or ID. It draws through crossterm, so the CLI keeps one terminal backend. inquire = { version = "=0.9.4", default-features = false, features = ["crossterm"] } diff --git a/docs/quality.md b/docs/quality.md index ae4c92a..f8a99cd 100644 --- a/docs/quality.md +++ b/docs/quality.md @@ -211,10 +211,6 @@ an existing simpler pattern cannot satisfy it. - `check_owner` in `config.rs` calls `libc::geteuid` in an `unsafe` block because std has no safe effective-UID accessor; the block is one call with a `SAFETY:` comment. -- `TerminalModes` in `terminal.rs` calls `libc::tcgetattr` and `libc::tcsetattr` - in `unsafe` blocks because std cannot save or restore terminal attributes. It - restores echo when Ctrl-C cancels a hidden prompt whose read cannot return; - each block is one call with a `SAFETY:` comment. - `config`, `registry`, and `secret` are public modules. The integration suite seeds saved credentials through the real persistence boundary and walks the operation contract to exercise every route; no other item is diff --git a/src/output.rs b/src/output.rs index 74bdfb3..df4a1c1 100644 --- a/src/output.rs +++ b/src/output.rs @@ -15,6 +15,10 @@ use crate::{ terminal::{self, scrub_line}, }; use clap::ValueEnum; +use crossterm::{ + cursor::{Hide, MoveTo, Show}, + terminal::{Clear, ClearType, EnterAlternateScreen, LeaveAlternateScreen}, +}; use serde::Serialize; use serde_json::{Value, json}; use std::{ @@ -347,7 +351,12 @@ impl Output { frame.push_str(&format!(" … {more} more, below the terminal's height\n")); } // Home and clear are screen control, not color, so they skip the color stream. - finish_stdout(std::io::stdout().lock(), b"\x1b[H\x1b[2J")?; + let home = format!( + "{}{}", + terminal::ansi(MoveTo(0, 0)), + terminal::ansi(Clear(ClearType::All)) + ); + finish_stdout(std::io::stdout().lock(), home.as_bytes())?; write_stdout_styled(frame.as_bytes(), self.color) } @@ -404,14 +413,24 @@ pub struct LiveScreen; impl LiveScreen { pub fn enter() -> Self { - let _ = finish_stdout(std::io::stdout().lock(), b"\x1b[?1049h\x1b[?25l"); + let enter = format!( + "{}{}", + terminal::ansi(EnterAlternateScreen), + terminal::ansi(Hide) + ); + let _ = finish_stdout(std::io::stdout().lock(), enter.as_bytes()); Self } } impl Drop for LiveScreen { fn drop(&mut self) { - let _ = finish_stdout(std::io::stdout().lock(), b"\x1b[?25h\x1b[?1049l"); + let leave = format!( + "{}{}", + terminal::ansi(Show), + terminal::ansi(LeaveAlternateScreen) + ); + let _ = finish_stdout(std::io::stdout().lock(), leave.as_bytes()); } } diff --git a/src/progress.rs b/src/progress.rs index 19bb5a6..2b89829 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -5,6 +5,7 @@ use crate::{ style::{Glyphs, Role, paint}, terminal, }; +use crossterm::terminal::{Clear, ClearType}; use std::{ io::Write, sync::{Arc, Mutex, PoisonError}, @@ -16,6 +17,11 @@ const FRAME: Duration = Duration::from_millis(100); /// A quick wait shows no time; one the caller times shows it from the start. const SHOW_WAITED_AFTER: Duration = Duration::from_secs(2); +/// A carriage return to the line's start, then the line cleared. +fn clear_line() -> String { + format!("\r{}", terminal::ansi(Clear(ClearType::CurrentLine))) +} + /// How the status line is drawn on this stderr. #[derive(Clone, Copy, Debug)] pub struct Look { @@ -130,13 +136,13 @@ impl StatusLine { .expires .map(|expires| expires.saturating_duration_since(Instant::now())); let text = self.text(frame, self.since.elapsed(), left, self.width); - (self.write)(format!("\r\x1b[2K{text}").as_bytes()); + (self.write)(format!("{}{text}", clear_line()).as_bytes()); } fn clear(&self) { let mut cleared = self.cleared.lock().unwrap_or_else(PoisonError::into_inner); *cleared = true; - (self.write)(b"\r\x1b[2K"); + (self.write)(clear_line().as_bytes()); } /// The line for one frame, cut to fit `width` so clearing never leaves a wrapped remnant. diff --git a/src/terminal.rs b/src/terminal.rs index 82fb568..85fabd4 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -7,14 +7,20 @@ use crate::{ secret::Secret, style::{self, ColorChoice, Glyphs, Line}, }; +use crossterm::{ + Command, cursor, + event::{self, DisableBracketedPaste, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}, + terminal, +}; use inquire::{InquireError, Select, Text}; use std::{ fmt::{self, Display}, - io::{ErrorKind, IsTerminal, Write}, + io::{IsTerminal, Write}, time::Duration, }; use tokio::{sync::oneshot, time::Instant}; use uuid::Uuid; +use zeroize::Zeroizing; /// What the process may show to and ask of the person at the terminal, from `--quiet`, /// `--no-input`, and `--color`. @@ -153,8 +159,7 @@ impl Terminal { /// `None` means they pressed Esc. Raw mode turns a typed Ctrl-C into a key, so the picker /// reports Ctrl-C as an interruption. pub async fn pick(self, question: &'static str, choices: Vec) -> Result> { - #[cfg(unix)] - let _modes = TerminalModes::save(); + let _raw = RawMode; let _screen = PickerScreen; picked( read_detached(move || Select::new(question, choices).with_page_size(12).prompt()) @@ -168,28 +173,88 @@ impl Terminal { question: &'static str, default: &'static str, ) -> Result> { - #[cfg(unix)] - let _modes = TerminalModes::save(); + let _raw = RawMode; let _screen = PickerScreen; answered(read_detached(move || Text::new(question).with_default(default).prompt()).await?) } - /// Read a secret without echo. + /// Read a secret without echo. The prompt goes to stderr, and raw mode hides the typed + /// characters and turns a typed Ctrl-C into a key. pub async fn read_hidden(self, prompt: &'static str) -> Result { - // The detached read restores echo only when it returns, and Ctrl-C does not wait for - // it. This guard restores the terminal when the read completes or is cancelled. - #[cfg(unix)] - let _modes = TerminalModes::save(); - match read_detached(move || rpassword::prompt_password(prompt).map(Secret::new)).await? { - // rpassword turns off terminal signals, so a typed Ctrl-C reaches it as a - // character. It raises SIGINT itself and then returns this error; waiting here - // lets the interrupt handler report the interruption instead of an I/O failure. - Err(error) if error.kind() == ErrorKind::Interrupted => std::future::pending().await, - read => Ok(read?), + eprint!("{prompt}"); + let _ = std::io::stderr().flush(); + // The detached read cannot be cancelled, and Ctrl-C does not wait for it. Raw mode is + // turned on here, not on the reading thread, so this guard restores the terminal + // whether the read completes or is cancelled. + let raw = match RawMode::enable() { + Ok(raw) => raw, + Err(error) => return hidden(Err(error)), + }; + let read = read_detached(read_hidden_keys).await?; + drop(raw); + eprintln!(); + hidden(read) + } +} + +/// What a hidden prompt's read means: the secret, Ctrl-C (`None`) as an interruption, and a +/// terminal that cannot be put in raw mode or read as a transport error. +fn hidden(read: std::io::Result>) -> Result { + match read { + Ok(Some(secret)) => Ok(secret), + Ok(None) => Err(Error::interrupted( + "Interrupted before any resource change was submitted", + )), + Err(_) => Err(Error::transport( + "Could not read the secret from the terminal", + )), + } +} + +/// Keys typed at a hidden prompt, until Enter. `None` means Ctrl-C. +fn read_hidden_keys() -> std::io::Result> { + let mut typed = Zeroizing::new(String::new()); + loop { + if let Event::Key(key) = event::read()? { + match type_hidden(&mut typed, key) { + Hidden::Typing => {} + Hidden::Entered => return Ok(Some(Secret::from(typed))), + Hidden::Interrupted => return Ok(None), + } } } } +/// Where a hidden prompt stands after a key. +#[derive(Debug, PartialEq)] +enum Hidden { + Typing, + Entered, + Interrupted, +} + +/// Apply one key to the secret typed so far. Enter, or Ctrl-J for a pasted newline, ends the +/// read; Backspace removes a character; Ctrl-U clears the line. Releases and other control +/// keys change nothing. +fn type_hidden(typed: &mut String, key: KeyEvent) -> Hidden { + if key.kind == KeyEventKind::Release { + return Hidden::Typing; + } + let control = key.modifiers.contains(KeyModifiers::CONTROL); + match key.code { + KeyCode::Char('c') if control => return Hidden::Interrupted, + KeyCode::Enter => return Hidden::Entered, + KeyCode::Char('j') if control => return Hidden::Entered, + KeyCode::Char('u') if control => typed.clear(), + KeyCode::Backspace => { + typed.pop(); + } + KeyCode::Char(c) if !control && !key.modifiers.contains(KeyModifiers::ALT) => typed.push(c), + _ => {} + } + Hidden::Typing +} + fn picked(answer: std::result::Result) -> Result> { Ok(answered(answer)?.map(|choice| choice.id)) } @@ -215,12 +280,40 @@ struct PickerScreen; impl Drop for PickerScreen { fn drop(&mut self) { + let undo = format!("{}{}", ansi(DisableBracketedPaste), ansi(cursor::Show)); let mut stderr = std::io::stderr(); - let _ = stderr.write_all(b"\x1b[?2004l\x1b[?25h"); + let _ = stderr.write_all(undo.as_bytes()); let _ = stderr.flush(); } } +/// Raw mode, which turns off echo and terminal signals. crossterm saves the terminal's modes +/// when it turns raw mode on and restores them when it turns it off. A prompt's read restores +/// them only when it returns; a signal that ends the command first leaves that to this guard. +/// Dropping it when raw mode is already off does nothing. +struct RawMode; + +impl RawMode { + fn enable() -> std::io::Result { + terminal::enable_raw_mode()?; + Ok(Self) + } +} + +impl Drop for RawMode { + fn drop(&mut self) { + let _ = terminal::disable_raw_mode(); + } +} + +/// A crossterm command's ANSI sequence, for output written as ordered bytes. crossterm's +/// `execute!` would instead call the Windows console API, out of order with buffered text. +pub fn ansi(command: impl Command) -> String { + let mut sequence = String::new(); + let _ = command.write_ansi(&mut sequence); + sequence +} + /// One resource a person can pick: its name, scrubbed because the API supplied it, and its ID. pub struct Choice { id: Uuid, @@ -356,42 +449,6 @@ pub async fn read_detached( .map_err(|_| Error::transport("Terminal input ended unexpectedly")) } -/// The controlling terminal's attributes, put back when dropped. `rpassword` turns off echo -/// on `/dev/tty` and turns it back on only when its read returns. -#[cfg(unix)] -struct TerminalModes { - tty: std::fs::File, - saved: libc::termios, -} - -#[cfg(unix)] -impl TerminalModes { - /// `None` without a controlling terminal, where there is nothing to restore. - fn save() -> Option { - use std::os::fd::AsRawFd; - let tty = std::fs::File::open("/dev/tty").ok()?; - let mut saved = std::mem::MaybeUninit::::uninit(); - // SAFETY: the descriptor is open for the duration of the call, and tcgetattr fully - // initializes `saved` when it returns 0. - if unsafe { libc::tcgetattr(tty.as_raw_fd(), saved.as_mut_ptr()) } != 0 { - return None; - } - // SAFETY: tcgetattr returned 0 above. - let saved = unsafe { saved.assume_init() }; - Some(Self { tty, saved }) - } -} - -#[cfg(unix)] -impl Drop for TerminalModes { - fn drop(&mut self) { - use std::os::fd::AsRawFd; - // SAFETY: the descriptor is still owned by `tty`, and `saved` holds attributes read - // from the same terminal. - unsafe { libc::tcsetattr(self.tty.as_raw_fd(), libc::TCSANOW, &self.saved) }; - } -} - /// How long a wait has run, for a status line that would otherwise look stuck: `42s`, or /// `3m 05s` from a minute on. pub fn elapsed(duration: Duration) -> String { @@ -459,7 +516,7 @@ mod tests { // A terminal that goes away or is not one fails the read; no choice is assumed. for failure in [ InquireError::NotTTY, - InquireError::IO(std::io::Error::from(ErrorKind::BrokenPipe)), + InquireError::IO(std::io::Error::from(std::io::ErrorKind::BrokenPipe)), ] { let error = picked(Err(failure)).unwrap_err(); assert_eq!(error.kind, Exit::Transport); @@ -470,6 +527,73 @@ mod tests { } } + #[test] + fn a_hidden_prompt_edits_the_secret_until_enter_or_ctrl_c() { + let press = |code, modifiers| KeyEvent::new(code, modifiers); + let none = KeyModifiers::NONE; + let mut typed = String::new(); + for key in [ + press(KeyCode::Char('x'), none), + press(KeyCode::Char('u'), KeyModifiers::CONTROL), + press(KeyCode::Char('a'), none), + press(KeyCode::Char('B'), KeyModifiers::SHIFT), + press(KeyCode::Char('z'), none), + press(KeyCode::Backspace, none), + press(KeyCode::Char('c'), none), + // Alt and control chords, other keys, and releases type nothing. + press(KeyCode::Char('d'), KeyModifiers::ALT), + press(KeyCode::Char('e'), KeyModifiers::CONTROL), + press(KeyCode::Left, none), + KeyEvent::new_with_kind(KeyCode::Char('f'), none, KeyEventKind::Release), + ] { + assert_eq!(type_hidden(&mut typed, key), Hidden::Typing, "{key:?}"); + } + assert_eq!(typed, "aBc"); + assert_eq!( + type_hidden(&mut typed, press(KeyCode::Enter, none)), + Hidden::Entered + ); + // A pasted newline reaches a raw terminal as Ctrl-J. + assert_eq!( + type_hidden(&mut typed, press(KeyCode::Char('j'), KeyModifiers::CONTROL)), + Hidden::Entered + ); + assert_eq!( + type_hidden(&mut typed, press(KeyCode::Char('c'), KeyModifiers::CONTROL)), + Hidden::Interrupted + ); + assert_eq!(typed, "aBc"); + } + + #[test] + fn a_hidden_read_becomes_the_secret_an_interruption_or_a_transport_error() { + assert_eq!( + hidden(Ok(Some(Secret::new("k".into())))).unwrap().expose(), + "k" + ); + let interrupted = hidden(Ok(None)).unwrap_err(); + assert_eq!(interrupted.kind, Exit::Interrupted); + assert_eq!( + interrupted.message, + "Interrupted before any resource change was submitted" + ); + // No terminal to put in raw mode, or one that goes away mid-read; no secret is assumed. + let error = + hidden(Err(std::io::Error::from(std::io::ErrorKind::NotConnected))).unwrap_err(); + assert_eq!(error.kind, Exit::Transport); + assert_eq!(error.message, "Could not read the secret from the terminal"); + } + + #[test] + fn crossterm_commands_become_their_ansi_sequences() { + assert_eq!(ansi(DisableBracketedPaste), "\x1b[?2004l"); + assert_eq!(ansi(cursor::Show), "\x1b[?25h"); + assert_eq!( + ansi(terminal::Clear(terminal::ClearType::CurrentLine)), + "\x1b[2K" + ); + } + #[test] fn elapsed_time_reads_in_seconds_then_minutes() { assert_eq!(elapsed(Duration::from_millis(900)), "0s"); diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..c936232 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2302,6 +2302,41 @@ async fn typed_ctrl_c_at_the_hidden_key_prompt_reports_an_interruption() { assert!(echo); } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn the_hidden_key_prompt_saves_the_edited_key_without_showing_it() { + let (server, dir) = fixture().await; + let import = [ + "auth", + "import-key", + "--account", + "k", + "--org", + ORG, + "--env", + ENV, + "--credential-store", + "file", + ]; + // Ctrl-U clears the first attempt, and Backspace removes the X. + let keys = b"wrong\x15hidden-secX\x7fret\r"; + let run = on_a_terminal( + dir.path(), + &server, + &import, + &[("Environment API key:", keys)], + ) + .await; + assert_eq!(run.code, Some(0), "{}", run.shown); + assert_eq!(run.modes.0, run.modes.1); + assert!(!run.shown.contains("hidden"), "{}", run.shown); + let settings = Settings::open(dir.path().to_path_buf()).unwrap(); + let Credential::ApiKey(key) = settings.read_credential("k").unwrap() else { + panic!("expected an API key"); + }; + assert_eq!(key.expose(), "hidden-secret"); +} + #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn interrupt_during_a_refresh_says_the_login_may_need_renewal() {