From 09f2168ad2b114c207eb3da68dca4fcc5e4daacd Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:38:15 -0500 Subject: [PATCH 01/12] docs: add a staging setup and a Try it section for each PR The development guide gains a short block that aliases the staging wrapper, builds, and logs in, for checking a pull request's branch by hand. AGENTS.md and the pull request template ask each pull request for a "Try it on staging" section: the exact commands to paste after that setup, every changed help and docs example, and what to expect. --- .github/pull_request_template.md | 15 +++++++++++++++ AGENTS.md | 25 +++++++++++++++++++++++++ docs/development.md | 14 ++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index c98162c..7dfab98 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,22 @@ +## Try it on staging + + + +Setup: [Try a pull request on staging](docs/development.md#try-a-pull-request-on-staging) + +```sh +voltage ... +``` + +Expect: ... + +## Checks + - [ ] User-visible change: a guide or reference under `docs/` and `CHANGELOG.md` (**Unreleased**) are updated, or the PR has the `no-changelog` label. - [ ] The verification gate in `docs/development.md` passes. - [ ] The diff was reviewed against its base with the `voltage-pr-review` skill, and every new error and early-return path has a test. - [ ] CRAP report, when `src/` or `tests/` changed: the highest score and each function at or above 20. - [ ] Checks not run, and why: +- [ ] **Try it on staging** lists every changed `--help` and docs example, with what to expect. diff --git a/AGENTS.md b/AGENTS.md index e231d44..fc89d0f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,31 @@ so a pull request is not ready until each one is done and reported. 5. In the pull request description, give the checks you ran and the CRAP result: the highest score and each function at or above 20. Name any check you could not run, and why. +6. In the pull request description, add a **Try it on staging** section that a reviewer can + paste after the [staging setup](docs/development.md#try-a-pull-request-on-staging). + See [Try it on staging](#try-it-on-staging). + +## Try it on staging + +The section tells a reviewer exactly what to run and what to look for. Write it for a +reviewer who is on the branch and has run the setup, and nothing else: + +- Start with one line: `Setup: [Try a pull request on staging](docs/development.md#try-a-pull-request-on-staging)`. +- Give the commands in `sh` blocks, ready to paste. Use `voltage` (the staging alias) and + `--account me`, never a profile the reviewer may not have. Use a placeholder only for a + value each reviewer has to supply, such as `WALLET_ID`, and say how to find it, such as + `voltage wallets list --account me`. +- After each command or group, say what to expect in one line: the text, symbol, color, or + exit code that shows the change works. +- List every `--help` and docs example that the pull request adds or changes, adapted to the + setup. To find them, run + `git diff origin/master...HEAD -- README.md docs src | grep '^+.*voltage '`. +- Cover the variants the change affects, such as piped output (`2>&1 | cat`), `--json`, + `--quiet`, `--color never`, or `--no-input`. +- Mark each command that sends a change. It needs `--execute` and a test-network wallet; + say so. +- For a change with nothing to see, such as a refactor or a test-only change, write + `Nothing to try on staging` and the reason. A reviewer runs the CRAP report on the pull request's head and reports each violation as a finding, together with the review from step 1. diff --git a/docs/development.md b/docs/development.md index d7aca10..1aec35d 100644 --- a/docs/development.md +++ b/docs/development.md @@ -203,6 +203,20 @@ command line. A profile ignores `VOLTAGE_WALLET_ID` and the other ambient variab `--wallet` explicitly. Confirm a wallet's reported network is a test network before creating a payment. Browser login needs the device-login release deployed on the staging auth service. +### Try a pull request on staging + +To check a pull request's change by hand, check out its branch, with `.env` filled in as in +step 1 above, and paste this from the repository root: + +```sh +alias voltage="$PWD/scripts/voltage-staging" +voltage --build +voltage login --account me --credential-store file +``` + +Then run the commands in the pull request's **Try it on staging** section; they assume this +setup. After pulling new commits, run `voltage --build` again. + ## Staging acceptance Complete these steps against the deployed staging services before publishing a release, and From c6318f41268679cba5e17b7206c5f93ef3948aab Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:04:21 -0500 Subject: [PATCH 02/12] fix(cli): accept --all on environments, organizations, and profiles list These local lists return every item at once but rejected --all, unlike the API lists fixed in #94. They now accept it and ignore it, through the same argument. A test walks the command tree so a new list cannot miss --all. Closes #119. --- CHANGELOG.md | 1 + docs/guides/requests.md | 2 ++ src/cli.rs | 80 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..b21f9b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- `organizations list`, `environments list`, and `profiles list` accept `--all` and ignore it, like the other lists that return every item at once. Before, they rejected it. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/guides/requests.md b/docs/guides/requests.md index 29364c6..fafe639 100644 --- a/docs/guides/requests.md +++ b/docs/guides/requests.md @@ -36,4 +36,6 @@ voltage payments list --profile prod --statuses completed --statuses failed \ `--all` follows pagination by cursor, which the API recommends and the CLI requests by default; endpoints that only page by offset are followed by offset and count. The API has deprecated explicit offset paging (`--offset`, `--pagination offset`) where cursors exist, and the CLI warns when you use it. `--timeout SECONDS` (default 60) bounds the whole request, including paging and waits. +Every `list` command accepts `--all`. A list without `--limit`, such as `wallets list`, `organizations list`, `environments list`, or `profiles list`, returns every item at once, so it ignores `--all`. + Related: [safety model](../concepts/safety-model.md), [output reference](../reference/output.md). diff --git a/src/cli.rs b/src/cli.rs index d3899d4..c825101 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -292,12 +292,14 @@ pub struct ImportKeyFlags { #[derive(Debug, Subcommand)] pub enum ListCommand { + #[command(arg = all_at_once_arg())] List, } #[derive(Debug, Subcommand, Eq, PartialEq)] pub enum ProfileCommand { /// Show every saved profile + #[command(arg = all_at_once_arg())] List, /// Show one profile Get { name: String }, @@ -1027,18 +1029,21 @@ fn list_args(cmd: ClapCommand, operation: &Operation) -> ClapCommand { if operation.has_parameter("limit") { PaginationFlags::augment_args(cmd) } else if operation.lists_everything_at_once() { - cmd.arg( - Arg::new("all_at_once") - .long("all") - .action(ArgAction::SetTrue) - .help("No effect: this list returns every item in one response") - .help_heading("Query"), - ) + cmd.arg(all_at_once_arg()) } else { cmd } } +/// `--all` on a list without paging, such as `environments list`: accepted and ignored. +fn all_at_once_arg() -> Arg { + Arg::new("all_at_once") + .long("all") + .action(ArgAction::SetTrue) + .help("No effect: this list shows every item at once") + .help_heading("Query") +} + /// The resource ID argument and `--id`, one of which is required. A wallet command can /// name its wallet with `--wallet` instead, so `target_id` checks that one was given. fn target_args(cmd: ClapCommand, target: ResourceTarget, wallet: bool) -> ClapCommand { @@ -1556,6 +1561,67 @@ mod tests { ); } + #[test] + fn all_is_accepted_by_every_list_command() { + fn lists(cmd: &ClapCommand, path: &mut Vec, found: &mut Vec<(String, bool)>) { + path.push(cmd.get_name().into()); + if cmd.get_name() == "list" { + let all = cmd.get_arguments().any(|arg| arg.get_long() == Some("all")); + found.push((path.join(" "), all)); + } + for sub in cmd.get_subcommands() { + lists(sub, path, found); + } + path.pop(); + } + let mut found = Vec::new(); + lists(&command(), &mut Vec::new(), &mut found); + assert!( + found + .iter() + .any(|(path, _)| path == "voltage environments list"), + "{found:?}" + ); + let missing: Vec<_> = found.iter().filter(|(_, all)| !all).collect(); + assert!(missing.is_empty(), "lists without --all: {missing:?}"); + } + + #[test] + fn all_is_ignored_by_local_lists_and_rejected_by_other_local_commands() { + let parse = |args: &[&str]| { + Invocation::try_parse_from(["voltage"].iter().chain(args)).map(|i| i.command) + }; + assert!(matches!( + parse(&["environments", "list", "--all"]), + Ok(Command::Local(LocalCommand::Environments { + command: ListCommand::List + })) + )); + assert!(matches!( + parse(&["organizations", "list", "--all"]), + Ok(Command::Local(LocalCommand::Organizations { + command: ListCommand::List + })) + )); + assert!(matches!( + parse(&["profiles", "list", "--all"]), + Ok(Command::Local(LocalCommand::Profiles { + command: ProfileCommand::List + })) + )); + for args in [ + &["profiles", "get", "stage", "--all"][..], + &["context", "--all"], + ] { + let error = parse(args).unwrap_err(); + assert!( + error.message.contains("unexpected argument '--all'"), + "{args:?}: {}", + error.message + ); + } + } + #[test] fn wallet_flag_names_the_wallet_a_wallet_command_acts_on() { const OTHER: &str = "44444444-4444-4444-8444-444444444444"; 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 03/12] 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() { From 39666d1a45f59f68506194dfdee38a0a0e7702a8 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:39:17 -0500 Subject: [PATCH 04/12] fix(cli): warn when a profile or flag overrides a set VOLTAGE_* variable Under --profile, or with a flag such as --wallet, commands ignored set VOLTAGE_* scope, key, and directory variables without saying so; only `voltage context` listed them. The voltage-api skill's helper lets the same variables win, so the two tools could reach different wallets in one shell. Every command that reads the configuration now prints one stderr warning per ignored variable before it prompts or sends a request, from the same list `voltage context` reports. --quiet keeps it, as it keeps other warnings; stdout is unchanged. --- CHANGELOG.md | 1 + docs/guides/login-and-profiles.md | 7 ++- docs/reference/environment.md | 2 +- src/auth.rs | 20 ++++-- src/config.rs | 47 +++++++++++--- src/context.rs | 100 ++++++++++++++++++++++++++++-- src/startup.rs | 8 ++- tests/cli_integration.rs | 40 +++++++++++- 8 files changed, 201 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..2851189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index cfeeeb0..cebe279 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -62,7 +62,12 @@ An API key cannot list organizations or environments, so with a key the CLI offe ## Profiles -A profile bundles an organization, an environment, and a credential, and ignores the scope variables above and `VOLTAGE_API_KEY`; flags still override its scope for one command. +A profile bundles an organization, an environment, and a credential, and ignores the scope variables above and `VOLTAGE_API_KEY`; flags still override its scope for one command. Other tools, such as the `voltage-api` agent skill's helper, read the same variables and let them win, so each command names on stderr every variable it ignores, even with `--quiet`: + +```console +$ VOLTAGE_WALLET_ID=WALLET_ID voltage wallets list --profile staging +warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected +``` ```sh voltage profiles create staging --org ORG_ID --env ENV_ID --account work diff --git a/docs/reference/environment.md b/docs/reference/environment.md index 375d3a4..a491ff7 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -1,6 +1,6 @@ # Environment variables -A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. `voltage context` lists every variable that is set but ignored, and where each value it uses came from. +A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `voltage context` lists the same variables, and where each value it uses came from. | Variable | Used for | Flag that wins | |---|---|---| diff --git a/src/auth.rs b/src/auth.rs index b053228..468514c 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -10,8 +10,8 @@ use crate::{ cli::{GlobalFlags, ImportKeyFlags, LoginFlags}, config::{ AUTH_URL, Account, AccountKind, Credential, CredentialProblem, CredentialStore, - InputSource, LOCK_TIMEOUT, Login, Scope, ScopeSources, Settings, Source, name_in_use, - read_secret, shell_word, unreadable_name, + IgnoredVariable, InputSource, LOCK_TIMEOUT, Login, Precedence, Scope, ScopeSources, + Settings, Source, name_in_use, read_secret, shell_word, unreadable_name, }, error::{CredentialRejection, CredentialSources, ErrorDetail, ResponseLimit}, secret::Secret, @@ -576,9 +576,19 @@ pub(crate) fn environment_key_sources( }) } -/// `VOLTAGE_API_KEY` is set, but a profile or account selection takes precedence over it. -pub(crate) fn ignores_ambient_api_key(flags: &GlobalFlags) -> bool { - std::env::var_os("VOLTAGE_API_KEY").is_some() && ambient_api_key(flags).is_none() +/// `VOLTAGE_API_KEY` when it is set but a profile or account selection takes precedence. +pub(crate) fn ignored_api_key(flags: &GlobalFlags) -> Option { + if std::env::var_os("VOLTAGE_API_KEY").is_none() || ambient_api_key(flags).is_some() { + return None; + } + Some(IgnoredVariable { + name: "VOLTAGE_API_KEY", + precedence: if flags.profile.is_some() { + Precedence::Profile + } else { + Precedence::Flag("--account") + }, + }) } /// The credential a command would use, chosen without reading a saved secret. diff --git a/src/config.rs b/src/config.rs index 1887d2e..b49cf0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -162,18 +162,47 @@ impl CredentialProblem { } } -/// `VOLTAGE_*` variables that are set but supply nothing, because a flag or the selected -/// profile takes precedence. Kept beside the precedence rules that decide it. -pub(crate) fn ignored_variables(sources: &ScopeSources, config_dir: Source) -> Vec<&'static str> { +/// What takes precedence over a `VOLTAGE_*` variable that is set. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum Precedence { + /// The named flag supplies the value. + Flag(&'static str), + /// The selected profile is a complete configuration, so ambient variables supply nothing, + /// even a value the profile lacks. + Profile, +} + +/// A `VOLTAGE_*` variable that is set but supplies nothing. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) struct IgnoredVariable { + pub(crate) name: &'static str, + pub(crate) precedence: Precedence, +} + +/// `VOLTAGE_*` scope and directory variables that are set but supply nothing, because a flag +/// or the selected profile takes precedence. Kept beside the precedence rules that decide it. +pub(crate) fn ignored_variables( + sources: &ScopeSources, + config_dir: Source, +) -> Vec { [ - ("VOLTAGE_ORGANIZATION_ID", sources.org), - ("VOLTAGE_ENVIRONMENT_ID", sources.envs), - ("VOLTAGE_WALLET_ID", sources.wallet), - ("VOLTAGE_CONFIG_DIR", config_dir), + ("VOLTAGE_ORGANIZATION_ID", "--org", sources.org), + ("VOLTAGE_ENVIRONMENT_ID", "--env", sources.envs), + ("VOLTAGE_WALLET_ID", "--wallet", sources.wallet), + ("VOLTAGE_CONFIG_DIR", "--config-dir", config_dir), ] .into_iter() - .filter(|(name, source)| std::env::var_os(name).is_some() && *source != Source::Environment) - .map(|(name, _)| name) + .filter(|(name, _, source)| std::env::var_os(name).is_some() && *source != Source::Environment) + .map(|(name, flag, source)| IgnoredVariable { + name, + // A set variable loses only to its flag or to a profile, which leaves it `Profile` + // or, for a value the profile lacks, `Unset`. + precedence: if source == Source::Flag { + Precedence::Flag(flag) + } else { + Precedence::Profile + }, + }) .collect() } diff --git a/src/context.rs b/src/context.rs index ff95348..d8244db 100644 --- a/src/context.rs +++ b/src/context.rs @@ -9,8 +9,10 @@ use crate::{ auth::{self, Rejected, Selected}, cli::GlobalFlags, config::{ - self, API_URL, AUTH_URL, AccountKind, CredentialProblem, CredentialStore, Settings, Source, + self, API_URL, AUTH_URL, AccountKind, CredentialProblem, CredentialStore, IgnoredVariable, + Precedence, ScopeSources, Settings, Source, }, + style::{Line, Role}, }; use serde::Serialize; use uuid::Uuid; @@ -98,14 +100,52 @@ pub struct Context { ignored_variables: Vec<&'static str>, } +/// Every `VOLTAGE_*` variable that is set but supplies nothing, because a flag or the selected +/// profile takes precedence. Commands warn with this list and `voltage context` reports it, so +/// the two cannot disagree. +pub(crate) fn ignored_variables( + global: &GlobalFlags, + sources: &ScopeSources, + config_dir: Source, +) -> Vec { + let mut ignored = config::ignored_variables(sources, config_dir); + ignored.extend(auth::ignored_api_key(global)); + ignored +} + +/// Warn on stderr about each ignored variable. Another tool can read the same variables with +/// the opposite precedence, so a silent override can send two tools in one shell to different +/// wallets. `--quiet` keeps the warning for that reason; stdout is unchanged. +pub(crate) fn warn_ignored(global: &GlobalFlags, ignored: &[IgnoredVariable]) { + let terminal = global.terminal(); + for variable in ignored { + terminal.important_line(ignored_warning(variable, global.profile.as_deref())); + } +} + +/// `warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected`. +fn ignored_warning(variable: &IgnoredVariable, profile: Option<&str>) -> Line { + let line = Line::default() + .paint(Role::Caution, "warning:") + .text(&format!(" {} is set but ignored because ", variable.name)); + match (variable.precedence, profile) { + (Precedence::Profile, Some(profile)) => { + line.text("--profile ").text(profile).text(" is selected") + } + // Only a selected profile yields `Profile`; this keeps the sentence whole regardless. + (Precedence::Profile, None) => line.text("a profile is selected"), + (Precedence::Flag(flag), _) => line.text(flag).text(" is given"), + } +} + /// Resolve the context with the same precedence every command uses. pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { let (scope, sources) = settings.sourced_scope(global.scope_selection())?; let (_, config_dir_source) = config::sourced_directory(global.config_dir.clone())?; - let mut ignored_variables = config::ignored_variables(&sources, config_dir_source); - if auth::ignores_ambient_api_key(global) { - ignored_variables.push("VOLTAGE_API_KEY"); - } + let ignored_variables = ignored_variables(global, &sources, config_dir_source) + .into_iter() + .map(|ignored| ignored.name) + .collect(); let (selected, credential_problem) = match auth::select(settings, &scope, global) { Ok(selected) => (Some(selected), None), Err(Rejected { problem, selected }) => (selected, Some(problem)), @@ -208,3 +248,53 @@ pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { ignored_variables, }) } + +#[cfg(test)] +mod tests { + use super::*; + + fn shown(name: &'static str, precedence: Precedence, profile: Option<&str>) -> String { + let ignored = IgnoredVariable { name, precedence }; + anstream::adapter::strip_str(ignored_warning(&ignored, profile).as_str()).to_string() + } + + #[test] + fn a_warning_names_the_variable_and_what_took_its_place() { + assert_eq!( + shown("VOLTAGE_WALLET_ID", Precedence::Profile, Some("staging")), + "warning: VOLTAGE_WALLET_ID is set but ignored because --profile staging is selected" + ); + assert_eq!( + shown("VOLTAGE_WALLET_ID", Precedence::Flag("--wallet"), None), + "warning: VOLTAGE_WALLET_ID is set but ignored because --wallet is given" + ); + assert_eq!( + shown("VOLTAGE_API_KEY", Precedence::Flag("--account"), None), + "warning: VOLTAGE_API_KEY is set but ignored because --account is given" + ); + } + + #[test] + fn a_profile_warning_without_a_profile_name_stays_a_sentence() { + assert_eq!( + shown("VOLTAGE_API_KEY", Precedence::Profile, None), + "warning: VOLTAGE_API_KEY is set but ignored because a profile is selected" + ); + } + + #[test] + fn a_profile_name_cannot_write_control_characters_to_the_terminal() { + let ignored = IgnoredVariable { + name: "VOLTAGE_WALLET_ID", + precedence: Precedence::Profile, + }; + // Unstripped, so an escape the name smuggled in would still be here. + let line = ignored_warning(&ignored, Some("evil\u{1b}]0;x\u{7}")); + assert!( + line.as_str() + .ends_with(" is set but ignored because --profile evil ]0;x is selected"), + "{:?}", + line.as_str() + ); + } +} diff --git a/src/startup.rs b/src/startup.rs index ab3a316..7dcd094 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -177,8 +177,14 @@ impl<'a> Session<'a> { global: &'a GlobalFlags, submission: &'a SubmissionState, ) -> Result { - let settings = Settings::open(config::directory(global.config_dir.clone())?)?; + let (dir, dir_source) = config::sourced_directory(global.config_dir.clone())?; + let settings = Settings::open(dir)?; let (mut scope, sources) = settings.sourced_scope(global.scope_selection())?; + // Before any prompt or request, so the warning precedes what the command does. + context::warn_ignored( + global, + &context::ignored_variables(global, &sources, dir_source), + ); let credential_sources = auth::environment_key_sources(global, &sources); let resolver = Resolver { settings: &settings, diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..42a24af 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -934,12 +934,20 @@ async fn profile_credentials_override_ambient_key_and_scope() { ok(json!([])), ) .await; - cli(dir.path(), &server) + let profiled = cli(dir.path(), &server) .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) - .args(["wallets", "list", "--profile", "stage"]) + .args(["wallets", "list", "--profile", "stage", "--quiet"]) .assert() .success(); + // Each ignored variable is named on stderr, even under --quiet; stdout stays the result. + assert_eq!( + stderr(&profiled), + "warning: VOLTAGE_ORGANIZATION_ID is set but ignored because --profile stage is selected\n\ + warning: VOLTAGE_ENVIRONMENT_ID is set but ignored because --profile stage is selected\n\ + warning: VOLTAGE_API_KEY is set but ignored because --profile stage is selected\n" + ); + assert_eq!(json_stdout(&profiled)["data"], json!([])); cli(dir.path(), &server) .args(["wallets", "list", "--profile", "stage", "--env", RESOURCE]) .assert() @@ -947,6 +955,34 @@ async fn profile_credentials_override_ambient_key_and_scope() { server.verify().await; } +#[tokio::test] +async fn a_flag_over_a_set_variable_warns_and_a_used_variable_does_not() { + let (server, dir) = fixture().await; + respond( + &server, + route("GET", wallets_path(ORG)).and(header("x-api-key", ACCOUNT_KEY)), + ok(json!([])), + 2, + ) + .await; + let flagged = cli(dir.path(), &server) + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .args(["wallets", "list", "--org", ORG]) + .assert() + .success(); + assert_eq!( + stderr(&flagged), + "warning: VOLTAGE_ORGANIZATION_ID is set but ignored because --org is given\n" + ); + let ambient = cli(dir.path(), &server) + .env("VOLTAGE_ORGANIZATION_ID", ORG) + .args(["wallets", "list"]) + .assert() + .success(); + assert_eq!(stderr(&ambient), ""); + server.verify().await; +} + #[tokio::test(flavor = "multi_thread")] async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { let (server, dir) = fixture().await; From f736a45c5f5ae690abe157a3a47269d6749eab42 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:39:22 -0500 Subject: [PATCH 05/12] feat(cli): print a profile's scope as dotenv lines with context --format env Other Voltage tools, such as the voltage-api skill's helper, take scope only from VOLTAGE_* variables or a dotenv file, so handing them a profile's scope meant copying IDs by hand or parsing `context --json`. `voltage context --format env` prints the effective organization, environment, and wallet under those names. It leaves out unset values, never prints a credential, refuses more than one environment with exit code 2, and refuses --json, --output, and --output-file before or after the command. --- CHANGELOG.md | 1 + docs/guides/login-and-profiles.md | 16 +++++ docs/guides/scripting.md | 2 + src/cli.rs | 97 ++++++++++++++++++++++++++++++- src/context.rs | 31 +++++++++- src/pick.rs | 2 +- src/startup.rs | 20 +++++-- tests/cli_integration.rs | 45 ++++++++++++++ 8 files changed, 204 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..e231098 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- `voltage context --format env` prints the effective organization, environment, and wallet as `VOLTAGE_*` dotenv lines, the names the `voltage-api` agent skill's helper reads, so a profile's scope can be handed to it. It leaves out unset values, never prints a credential, and fails with exit code 2 when more than one environment is selected. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index cfeeeb0..f809433 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -80,6 +80,22 @@ voltage context --profile staging voltage context --json | jq -r '.data.organization_id.value' ``` +### Hand the scope to another tool + +`voltage context --format env` prints the effective organization, environment, and wallet as dotenv lines, named as the `voltage-api` agent skill's helper and the [environment variables](../reference/environment.md) read them. Redirect it into the helper's dotenv file, or pass it to `env`: + +```console +$ voltage context --profile staging --format env +VOLTAGE_ORGANIZATION_ID=ORG_ID +VOLTAGE_ENVIRONMENT_ID=ENV_ID +VOLTAGE_WALLET_ID=WALLET_ID +$ voltage context --profile staging --format env > scope.env +``` + +A value that is not set is left out rather than printed empty, because a tool can treat an empty variable as a value. More than one environment, as from repeated `--env`, fails with exit code 2 instead of choosing one. It cannot be combined with `--json`, `--output`, or `--output-file`; redirect stdout instead. + +It never prints a credential: like the rest of `voltage context`, it reads no saved secret. Give the other tool an API key some other way, and use a key for the printed organization and environment, or its requests go elsewhere or fail. + ## Configuration directory Configuration lives in `$XDG_CONFIG_HOME/voltage` or `~/.config/voltage`; override the directory with `VOLTAGE_CONFIG_DIR` or `--config-dir`. The directory and everything in it are owner-only. diff --git a/docs/guides/scripting.md b/docs/guides/scripting.md index 9bace58..6d36d20 100644 --- a/docs/guides/scripting.md +++ b/docs/guides/scripting.md @@ -7,6 +7,8 @@ voltage payments list --profile prod --json | jq '.data.items[].status' voltage context --json | jq -r '.data.organization_id.value' ``` +To hand a profile's organization, environment, and wallet to a tool that reads `VOLTAGE_*` variables, use [`voltage context --format env`](login-and-profiles.md#hand-the-scope-to-another-tool). + ## Send changes from a script Commands that change something only describe their request unless you pass `--execute`, then exit with code 6 so a script that forgot it fails loudly. To send every change in a script or chain of commands, set `VOLTAGE_EXECUTE=1` (or `true`) instead: diff --git a/src/cli.rs b/src/cli.rs index d3899d4..d599491 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -18,6 +18,7 @@ use crate::{ }; use clap::{ Arg, ArgAction, ArgMatches, Args, Command as ClapCommand, FromArgMatches, Subcommand, + ValueEnum, builder::{PossibleValuesParser, TypedValueParser}, error::ErrorKind, }; @@ -211,7 +212,7 @@ pub enum LocalCommand { command: ProfileCommand, }, /// Show the effective profile, scope, credential, and config path without secrets - Context, + Context(ContextFlags), /// Show the BTC/USD price and the sats one dollar buys Price(PriceFlags), /// Convert an amount between BTC and USD at the current or a past price @@ -223,6 +224,44 @@ pub enum LocalCommand { }, } +#[derive(Debug, Args)] +pub struct ContextFlags { + /// Print the organization, environment, and wallet as VOLTAGE_* dotenv lines instead + #[arg(long, value_enum, value_name = "FORMAT")] + pub format: Option, +} + +impl ContextFlags { + /// `--format` writes its own lines to stdout, so it takes no result format or file. Checked + /// on the typed flags because clap does not compare a global flag given before the + /// subcommand with the subcommand's own flags. + fn check(&self, global: &GlobalFlags) -> Result<()> { + let result_flag = if global.json { + Some("--json") + } else if global.output.is_some() { + Some("--output") + } else if global.output_file.is_some() { + Some("--output-file") + } else { + None + }; + match (self.format, result_flag) { + (Some(_), Some(flag)) => Err(Error::usage(format!( + "--format cannot be combined with {flag}" + )) + .with_hint("Redirect stdout to save the lines, as in `voltage context --format env > scope.env`.")), + _ => Ok(()), + } + } +} + +/// Another tool's format for `voltage context`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] +pub enum ContextFormat { + /// `NAME=VALUE` lines, the names the `voltage-api` skill's helper reads + Env, +} + #[derive(Debug, Args)] pub struct PriceFlags { /// Price at this UTC time, such as 2026-09-18T17:30:00Z, instead of now @@ -752,6 +791,9 @@ impl Invocation { } Err(error) => return Err(Error::usage(error.to_string())), }; + if let Command::Local(LocalCommand::Context(flags)) = &command { + flags.check(&global)?; + } Ok(Self { global, command }) } } @@ -1345,7 +1387,9 @@ fn local_examples(path: &[&str]) -> Option<&'static str> { } ["context"] => { " # Show what a command with this profile would use, and where each value came from - voltage context --profile prod" + voltage context --profile prod + # Hand the profile's organization, environment, and wallet to another tool as dotenv lines + voltage context --profile prod --format env > scope.env" } ["convert"] => { " # What $10 buys in bitcoin now, and 21,000 sats in dollars on a past date @@ -1444,6 +1488,45 @@ mod tests { } } + #[test] + fn context_format_refuses_a_result_format_or_file_before_or_after_the_command() { + for (args, flag) in [ + ( + &["voltage", "--json", "context", "--format", "env"][..], + "--json", + ), + ( + &["voltage", "context", "--format", "env", "--json"], + "--json", + ), + ( + &["voltage", "-o", "table", "context", "--format", "env"], + "--output", + ), + ( + &[ + "voltage", + "context", + "--format", + "env", + "--output-file", + "scope.env", + ], + "--output-file", + ), + ] { + let error = Invocation::try_parse_from(args).err().unwrap(); + assert_eq!(error.kind, Kind::Usage, "{args:?}"); + assert_eq!( + error.message, + format!("--format cannot be combined with {flag}"), + "{args:?}" + ); + } + assert!(Invocation::try_parse_from(["voltage", "context", "--json"]).is_ok()); + assert!(Invocation::try_parse_from(["voltage", "context", "--format", "shell"]).is_err()); + } + #[test] fn command_tree_is_valid() { command().debug_assert(); @@ -1881,7 +1964,15 @@ mod tests { Invocation::try_parse_from(["voltage", "context"]) .unwrap() .command, - Command::Local(LocalCommand::Context) + Command::Local(LocalCommand::Context(ContextFlags { format: None })) + )); + assert!(matches!( + Invocation::try_parse_from(["voltage", "context", "--format", "env"]) + .unwrap() + .command, + Command::Local(LocalCommand::Context(ContextFlags { + format: Some(ContextFormat::Env) + })) )); assert!(matches!( Invocation::try_parse_from(["voltage", "price"]) diff --git a/src/context.rs b/src/context.rs index ff95348..786fb6f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -5,7 +5,7 @@ //! makes a network request, or creates the configuration directory. use crate::{ - Result, + Error, Result, auth::{self, Rejected, Selected}, cli::GlobalFlags, config::{ @@ -208,3 +208,32 @@ pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { ignored_variables, }) } + +impl Context { + /// The organization, environment, and wallet as dotenv lines, named as the `voltage-api` + /// skill's helper reads them, for `voltage context --format env`. An unset value is left + /// out, because the helper treats an empty variable as an override. A credential is never + /// printed: inspection reads no secret. A tool takes one environment, so more than one is + /// refused rather than choosing. + pub fn dotenv(&self) -> Result { + let environment = match self.environment_ids.value.as_slice() { + [] => None, + [environment] => Some(*environment), + environments => { + return Err(Error::usage(format!( + "--format env takes one environment, but {} are selected", + environments.len() + )) + .with_hint("Select one with --env ENV_ID.")); + } + }; + Ok([ + ("VOLTAGE_ORGANIZATION_ID", self.organization_id.value), + ("VOLTAGE_ENVIRONMENT_ID", environment), + ("VOLTAGE_WALLET_ID", self.wallet_id.value), + ] + .into_iter() + .filter_map(|(name, value)| value.map(|id| format!("{name}={id}\n"))) + .collect()) + } +} diff --git a/src/pick.rs b/src/pick.rs index 2d1efe3..6262f9d 100644 --- a/src/pick.rs +++ b/src/pick.rs @@ -71,7 +71,7 @@ fn needs(command: &Command) -> Needs { command: ProfileCommand::List | ProfileCommand::Get { .. } | ProfileCommand::Delete { .. }, } - | LocalCommand::Context + | LocalCommand::Context(_) | LocalCommand::Price(_) | LocalCommand::Convert(_) | LocalCommand::Completions { .. }, diff --git a/src/startup.rs b/src/startup.rs index ab3a316..2274245 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -5,8 +5,8 @@ use crate::{ api::{self, Execution, SubmissionState}, auth::{self, Discovery, Resolver}, cli::{ - self, AuthCommand, Command, ConvertFlags, GlobalFlags, Invocation, LocalCommand, - ParseFailure, PriceFlags, ProfileCommand, + self, AuthCommand, Command, ContextFlags, ContextFormat, ConvertFlags, GlobalFlags, + Invocation, LocalCommand, ParseFailure, PriceFlags, ProfileCommand, }, config::{self, Profile, Scope, Settings}, context, @@ -150,6 +150,14 @@ async fn execute(invocation: Invocation, submission: &SubmissionState) -> Result let mut out = output(!execution.describes())?; api::execute(&api, execution, &global, resolver, &session.scope, &mut out).await } + // Dotenv lines for another tool, not a result envelope. + Command::Local(LocalCommand::Context(ContextFlags { + format: Some(ContextFormat::Env), + })) => write_stdout( + context::inspect(&session.settings, &global)? + .dotenv()? + .as_bytes(), + ), Command::Local(local) => { let mut out = output(true)?; let envelope = session.local(local).await?; @@ -245,8 +253,10 @@ impl<'a> Session<'a> { .discover(scope, Discovery::Environments) .await?, ), - LocalCommand::Context => Envelope::settings(context::inspect(&self.settings, global)?) - .map(|context| context.titled("voltage context")), + LocalCommand::Context(_) => { + Envelope::settings(context::inspect(&self.settings, global)?) + .map(|context| context.titled("voltage context")) + } LocalCommand::Profiles { command } => { profiles(command, &mut self.settings, scope, global).await } @@ -410,7 +420,7 @@ mod tests { // Any other command comes back unchanged, so `execute` can go on with it. assert!(matches!( PriceQuery::try_from(command(&["voltage", "context"])), - Err(Command::Local(LocalCommand::Context)) + Err(Command::Local(LocalCommand::Context(_))) )); assert!(matches!( PriceQuery::try_from(command(&["voltage", "wallets", "list"])), diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..2019e63 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -1006,6 +1006,51 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { assert!(reloaded.config.accounts.contains_key(LOGIN_NAME)); } +#[tokio::test] +async fn context_env_format_prints_only_set_scope_and_never_a_credential() { + let (server, dir) = fixture().await; + let mut settings = seeded_api_key(dir.path(), "stage", "stage-key"); + settings.config.profiles.insert( + "stage".into(), + Profile { + organization_id: uuid(ORG), + environment_id: uuid(ENV), + account: "stage".into(), + }, + ); + settings.save().unwrap(); + let env_format = |args: &[&str]| { + Command::from_std(human_process_at(dir.path(), &server.uri())) + .args(["context", "--format", "env"]) + .args(args) + .assert() + }; + + let profiled = env_format(&["--profile", "stage", "--wallet", WALLET]).success(); + assert_eq!( + stdout(&profiled), + format!( + "VOLTAGE_ORGANIZATION_ID={ORG}\nVOLTAGE_ENVIRONMENT_ID={ENV}\nVOLTAGE_WALLET_ID={WALLET}\n" + ) + ); + // Neither the profile's saved key nor the ambient key is printed. + assert!(!stdout(&profiled).contains("stage-key")); + assert!(!stdout(&profiled).contains(ACCOUNT_KEY)); + + // An unset value is left out rather than printed empty. + let partial = env_format(&["--org", ORG]).success(); + assert_eq!(stdout(&partial), format!("VOLTAGE_ORGANIZATION_ID={ORG}\n")); + assert_eq!(stdout(&env_format(&[]).success()), ""); + + let two = env_format(&["--org", ORG, "--env", ENV, "--env", RESOURCE]).code(2); + assert_eq!(stdout(&two), ""); + assert!( + stderr(&two).contains("--format env takes one environment, but 2 are selected"), + "{}", + stderr(&two) + ); +} + #[tokio::test] async fn context_reports_effective_scope_and_sources_without_reading_secrets() { let (server, dir) = fixture().await; From 174456c8dc278f6d26c4963a858ae17cc7ec02cb Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 28 Oct 2026 05:09:59 -0500 Subject: [PATCH 06/12] fix(cli): keep import-key and profiles create quiet about flags they save Both commands save the organization, environment, and account their flags name, so warning that those flags override VOLTAGE_* variables was noise. They still warn about a profile or --config-dir. The staging and local wrappers now pass their directory as VOLTAGE_CONFIG_DIR instead of --config-dir, so the hint after `profiles create` no longer names a flag the wrappers refuse. --- CHANGELOG.md | 2 +- docs/development.md | 2 +- docs/reference/environment.md | 2 +- scripts/voltage-local | 5 +++-- scripts/voltage-staging | 5 +++-- src/startup.rs | 32 +++++++++++++++++++++++++++----- tests/cli_integration.rs | 9 ++++++++- 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2851189..d03f6e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased -- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. +- A command that ignores a set `VOLTAGE_*` variable, because `--profile` or a flag such as `--wallet` takes precedence, says so on stderr with one `warning:` line per variable, even with `--quiet`. Before, only `voltage context` reported it, so the CLI and a tool that lets the variable win could act on different wallets in one shell. Stdout is unchanged. `auth import-key` and `profiles create`, which save what their flags name, do not warn about the flags they were given. - Filter `payments list` with `--check-only true|false`, and `wallets ledger list` with `--event-type adjustment`, from the refreshed API contract. Omit `--event-type` for payment ledger entries. Filters carry across pagination. - `webhooks test WEBHOOK_ID` without `--data` sends a test event, with a new `delivery_id` and a payload of type `test` and event `created`. Before, it failed with "This operation requires --data". `--data` still sends your own payload. - A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. diff --git a/docs/development.md b/docs/development.md index d7aca10..452e68e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -133,7 +133,7 @@ build when they disagree. `docs/commands.md` is generated; regenerate it with API-key runs, the optional `VOLTAGE_API_KEY`, `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, and `VOLTAGE_WALLET_ID` from the repository's private `.env` (copy `.env.example`, fill it in, `chmod 600 .env`) without evaluating it as shell code, - keeps CLI state under `.work/staging/cli`, and refuses `--config-dir`, `--api-url`, and + keeps CLI state under `.work/staging/cli` through `VOLTAGE_CONFIG_DIR`, and refuses `--config-dir`, `--api-url`, and `--auth-url` so a staging command cannot target another deployment. A leading `--build` works as it does for `voltage-local`, but checks `.env` before it builds when a command follows; `--build` alone needs no `.env`. Browser login and diff --git a/docs/reference/environment.md b/docs/reference/environment.md index a491ff7..840f2b6 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -1,6 +1,6 @@ # Environment variables -A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `voltage context` lists the same variables, and where each value it uses came from. +A flag beats a variable. A selected profile is a complete configuration, so `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, `VOLTAGE_WALLET_ID`, and `VOLTAGE_API_KEY` are ignored while one is selected; `--account` alone ignores only `VOLTAGE_API_KEY`. Every command that reads the configuration prints one `warning:` line on stderr for each variable that is set but ignored, before it prompts or sends a request, even with `--quiet`; stdout is unchanged. `auth import-key` and `profiles create` save the scope and account their flags name, so they warn only about a profile or `--config-dir`. `voltage context` lists the same variables, and where each value it uses came from. | Variable | Used for | Flag that wins | |---|---|---| diff --git a/scripts/voltage-local b/scripts/voltage-local index 6d8395a..8ab33d8 100755 --- a/scripts/voltage-local +++ b/scripts/voltage-local @@ -24,8 +24,9 @@ if build: if status or not args: sys.exit(status) binary = root / 'target/debug/voltage' +# A variable rather than --config-dir, so hints the CLI prints do not name a refused flag. +env['VOLTAGE_CONFIG_DIR'] = str(root / '.work/local-demo/cli') if not binary.is_file(): sys.exit('Build the development CLI first: ./scripts/voltage-local --build') -os.execve(binary, [str(binary), '--config-dir', str(root / '.work/local-demo/cli'), - '--auth-url', 'http://127.0.0.1:8081/api/v1', +os.execve(binary, [str(binary), '--auth-url', 'http://127.0.0.1:8081/api/v1', '--api-url', 'http://127.0.0.1:1/disabled-api', *args], env) diff --git a/scripts/voltage-staging b/scripts/voltage-staging index f2ae052..d0387a5 100755 --- a/scripts/voltage-staging +++ b/scripts/voltage-staging @@ -100,6 +100,9 @@ if build: env = {key: value for key, value in os.environ.items() if not key.startswith("VOLTAGE_")} env.update({key: value for key, value in loaded.items() if key in CLI_ENV}) +# The directory is a variable rather than --config-dir, so hints the CLI prints, such as +# the one after `profiles create`, do not name a flag this wrapper refuses. +env["VOLTAGE_CONFIG_DIR"] = str(root / ".work/staging/cli") binary = root / "target/debug/voltage" if not binary.is_file(): sys.exit("Build the development CLI first: ./scripts/voltage-staging --build") @@ -108,8 +111,6 @@ os.execve( binary, [ str(binary), - "--config-dir", - str(root / ".work/staging/cli"), "--auth-url", loaded["VOLTAGE_STAGING_AUTH_URL"], "--api-url", diff --git a/src/startup.rs b/src/startup.rs index 7dcd094..8ba36b3 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -8,7 +8,7 @@ use crate::{ self, AuthCommand, Command, ConvertFlags, GlobalFlags, Invocation, LocalCommand, ParseFailure, PriceFlags, ProfileCommand, }, - config::{self, Profile, Scope, Settings}, + config::{self, Precedence, Profile, Scope, Settings}, context, error::{CredentialSources, ErrorKind}, greeting, @@ -158,6 +158,22 @@ async fn execute(invocation: Invocation, submission: &SubmissionState) -> Result } } +/// Commands that save the organization, environment, and account their flags name. A flag +/// over a set variable is what such a command is for, so only the configuration directory, +/// which decides where it saves, still warns. +fn saves_selection(command: &Command) -> bool { + matches!( + command, + Command::Local( + LocalCommand::Auth { + command: AuthCommand::ImportKey(_), + } | LocalCommand::Profiles { + command: ProfileCommand::Create { .. }, + } + ) + ) +} + /// One command once its configuration is read and its scope is complete: what the flags /// asked for, the saved settings, the scope, and what the command may have changed. /// Commands run from here; what they call receives only the parts it uses. @@ -180,11 +196,17 @@ impl<'a> Session<'a> { let (dir, dir_source) = config::sourced_directory(global.config_dir.clone())?; let settings = Settings::open(dir)?; let (mut scope, sources) = settings.sourced_scope(global.scope_selection())?; + let mut ignored = context::ignored_variables(global, &sources, dir_source); + if saves_selection(command) { + ignored.retain(|variable| { + matches!( + variable.precedence, + Precedence::Profile | Precedence::Flag("--config-dir") + ) + }); + } // Before any prompt or request, so the warning precedes what the command does. - context::warn_ignored( - global, - &context::ignored_variables(global, &sources, dir_source), - ); + context::warn_ignored(global, &ignored); let credential_sources = auth::environment_key_sources(global, &sources); let resolver = Resolver { settings: &settings, diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 42a24af..0cfcbd9 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -1000,6 +1000,8 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { ]; let created = cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) .args(create) .assert() .success(); @@ -1007,6 +1009,8 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { json_stdout(&created)["data"], json!({"profile":"stage","created":true}) ); + // The flags name what the profile saves, so the variables they override are no surprise. + assert_eq!(stderr(&created), ""); cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") .args(create) @@ -1338,7 +1342,8 @@ async fn imported_keys_bind_their_scope_and_report_status_without_secrets() { ENV, ]; let imported = cli(dir.path(), &server) - .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_ENVIRONMENT_ID", RESOURCE) .args(import) .args(["--credential-store", "file"]) .write_stdin(" imported-secret \n") @@ -1348,6 +1353,8 @@ async fn imported_keys_bind_their_scope_and_report_status_without_secrets() { json_stdout(&imported)["data"], json!({"account":"staging-key","imported":true}) ); + // The flags name what the key is bound to, so the variables they override are no surprise. + assert_eq!(stderr(&imported), ""); let settings = Settings::open(dir.path().to_path_buf()).unwrap(); let account = &settings.config.accounts["staging-key"]; assert_eq!(account.kind, AccountKind::ApiKey); From e43d367939074fd149031ffdfa19a3163cbb3b5a Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 28 Oct 2026 05:10:00 -0500 Subject: [PATCH 07/12] fix(scripts): pass the wrapper config directory as VOLTAGE_CONFIG_DIR The hint after `profiles create` repeats --config-dir, which the staging and local wrappers refuse. Passing the directory as a variable keeps the hint runnable through the wrappers. --- docs/development.md | 2 +- scripts/voltage-local | 5 +++-- scripts/voltage-staging | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/development.md b/docs/development.md index d7aca10..452e68e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -133,7 +133,7 @@ build when they disagree. `docs/commands.md` is generated; regenerate it with API-key runs, the optional `VOLTAGE_API_KEY`, `VOLTAGE_ORGANIZATION_ID`, `VOLTAGE_ENVIRONMENT_ID`, and `VOLTAGE_WALLET_ID` from the repository's private `.env` (copy `.env.example`, fill it in, `chmod 600 .env`) without evaluating it as shell code, - keeps CLI state under `.work/staging/cli`, and refuses `--config-dir`, `--api-url`, and + keeps CLI state under `.work/staging/cli` through `VOLTAGE_CONFIG_DIR`, and refuses `--config-dir`, `--api-url`, and `--auth-url` so a staging command cannot target another deployment. A leading `--build` works as it does for `voltage-local`, but checks `.env` before it builds when a command follows; `--build` alone needs no `.env`. Browser login and diff --git a/scripts/voltage-local b/scripts/voltage-local index 6d8395a..8ab33d8 100755 --- a/scripts/voltage-local +++ b/scripts/voltage-local @@ -24,8 +24,9 @@ if build: if status or not args: sys.exit(status) binary = root / 'target/debug/voltage' +# A variable rather than --config-dir, so hints the CLI prints do not name a refused flag. +env['VOLTAGE_CONFIG_DIR'] = str(root / '.work/local-demo/cli') if not binary.is_file(): sys.exit('Build the development CLI first: ./scripts/voltage-local --build') -os.execve(binary, [str(binary), '--config-dir', str(root / '.work/local-demo/cli'), - '--auth-url', 'http://127.0.0.1:8081/api/v1', +os.execve(binary, [str(binary), '--auth-url', 'http://127.0.0.1:8081/api/v1', '--api-url', 'http://127.0.0.1:1/disabled-api', *args], env) diff --git a/scripts/voltage-staging b/scripts/voltage-staging index f2ae052..d0387a5 100755 --- a/scripts/voltage-staging +++ b/scripts/voltage-staging @@ -100,6 +100,9 @@ if build: env = {key: value for key, value in os.environ.items() if not key.startswith("VOLTAGE_")} env.update({key: value for key, value in loaded.items() if key in CLI_ENV}) +# The directory is a variable rather than --config-dir, so hints the CLI prints, such as +# the one after `profiles create`, do not name a flag this wrapper refuses. +env["VOLTAGE_CONFIG_DIR"] = str(root / ".work/staging/cli") binary = root / "target/debug/voltage" if not binary.is_file(): sys.exit("Build the development CLI first: ./scripts/voltage-staging --build") @@ -108,8 +111,6 @@ os.execve( binary, [ str(binary), - "--config-dir", - str(root / ".work/staging/cli"), "--auth-url", loaded["VOLTAGE_STAGING_AUTH_URL"], "--api-url", From b32e028b0d23308276db682c6eaa0153121bea87 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:47:27 -0600 Subject: [PATCH 08/12] docs: keep the reviewer CRAP note under Before you request review --- AGENTS.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fc89d0f..0afd070 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,9 @@ so a pull request is not ready until each one is done and reported. paste after the [staging setup](docs/development.md#try-a-pull-request-on-staging). See [Try it on staging](#try-it-on-staging). +A reviewer runs the CRAP report on the pull request's head and reports each violation as a +finding, together with the review from step 1. + ## Try it on staging The section tells a reviewer exactly what to run and what to look for. Write it for a @@ -59,9 +62,6 @@ reviewer who is on the branch and has run the setup, and nothing else: - For a change with nothing to see, such as a refactor or a test-only change, write `Nothing to try on staging` and the reason. -A reviewer runs the CRAP report on the pull request's head and reports each violation as a -finding, together with the review from step 1. - ## Work plans Agent work plans live in `docs/plans//` as Markdown files. This directory is From 3f75e3861be6289a426bd0ec1d7fd3805dbd5a67 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:51:51 -0600 Subject: [PATCH 09/12] fix(terminal): bound the hidden key prompt and keep its buffer from reallocating --- CHANGELOG.md | 2 +- src/terminal.rs | 68 +++++++++++++++++++++++++++++----------- tests/cli_integration.rs | 37 ++++++++++++++++++++++ 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e68ff26..23f68c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - `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. - A command reads a saved login's or API key's secret from the OS credential store once, instead of once for each lookup, so macOS asks for keychain access at most once per command. A login refresh still reads it again under the credential lock, to see a refresh that another command saved. -- 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. +- 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. A key longer than 8 KiB is refused with exit 2 instead of being read without limit. ## 0.2.0 - 2026-10-22 diff --git a/src/terminal.rs b/src/terminal.rs index 85fabd4..318c8e4 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -197,29 +197,37 @@ impl Terminal { } } -/// 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 { +/// The most a hidden prompt accepts, in bytes. Its buffer is allocated at this size, so typing +/// never reallocates it and leaves no copy of the secret behind for zeroize to miss. +const MAX_HIDDEN_BYTES: usize = 8 * 1024; + +/// What a hidden prompt's read means: the secret on Enter, Ctrl-C as an interruption, a +/// secret over [`MAX_HIDDEN_BYTES`] as a usage error, and a terminal that cannot be put in +/// raw mode or read as a transport error. +fn hidden(read: std::io::Result<(Hidden, Zeroizing)>) -> Result { match read { - Ok(Some(secret)) => Ok(secret), - Ok(None) => Err(Error::interrupted( + Ok((Hidden::Entered, typed)) => Ok(Secret::from(typed)), + Ok((Hidden::Interrupted, _)) => Err(Error::interrupted( "Interrupted before any resource change was submitted", )), - Err(_) => Err(Error::transport( + Ok((Hidden::TooLong, _)) => Err(Error::usage(format!( + "The secret is longer than {MAX_HIDDEN_BYTES} bytes" + ))), + // `read_hidden_keys` returns only once typing ends; treat anything else as unread. + Ok((Hidden::Typing, _)) | 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()); +/// Keys typed at a hidden prompt, until Enter, Ctrl-C, or too long a secret. +fn read_hidden_keys() -> std::io::Result<(Hidden, Zeroizing)> { + let mut typed = Zeroizing::new(String::with_capacity(MAX_HIDDEN_BYTES)); 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), + ended => return Ok((ended, typed)), } } } @@ -231,11 +239,13 @@ enum Hidden { Typing, Entered, Interrupted, + TooLong, } /// 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. +/// keys change nothing. A character that would take the secret past [`MAX_HIDDEN_BYTES`] +/// ends the read, rather than saving a key cut short. fn type_hidden(typed: &mut String, key: KeyEvent) -> Hidden { if key.kind == KeyEventKind::Release { return Hidden::Typing; @@ -249,7 +259,12 @@ fn type_hidden(typed: &mut String, key: KeyEvent) -> Hidden { KeyCode::Backspace => { typed.pop(); } - KeyCode::Char(c) if !control && !key.modifiers.contains(KeyModifiers::ALT) => typed.push(c), + KeyCode::Char(c) if !control && !key.modifiers.contains(KeyModifiers::ALT) => { + if typed.len() + c.len_utf8() > MAX_HIDDEN_BYTES { + return Hidden::TooLong; + } + typed.push(c); + } _ => {} } Hidden::Typing @@ -565,18 +580,35 @@ mod tests { assert_eq!(typed, "aBc"); } + #[test] + fn a_hidden_prompt_stops_at_its_limit_without_reallocating() { + let key = |c| KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE); + let mut typed = String::with_capacity(MAX_HIDDEN_BYTES); + let buffer = typed.as_ptr(); + for _ in 0..MAX_HIDDEN_BYTES - 1 { + assert_eq!(type_hidden(&mut typed, key('a')), Hidden::Typing); + } + // A two-byte character no longer fits; the last single byte does. + assert_eq!(type_hidden(&mut typed, key('é')), Hidden::TooLong); + assert_eq!(type_hidden(&mut typed, key('a')), Hidden::Typing); + assert_eq!(type_hidden(&mut typed, key('a')), Hidden::TooLong); + assert_eq!(typed.len(), MAX_HIDDEN_BYTES); + assert_eq!(typed.as_ptr(), buffer); + } + #[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(); + let read = |ended| Ok((ended, Zeroizing::new("k".to_owned()))); + assert_eq!(hidden(read(Hidden::Entered)).unwrap().expose(), "k"); + let interrupted = hidden(read(Hidden::Interrupted)).unwrap_err(); assert_eq!(interrupted.kind, Exit::Interrupted); assert_eq!( interrupted.message, "Interrupted before any resource change was submitted" ); + let too_long = hidden(read(Hidden::TooLong)).unwrap_err(); + assert_eq!(too_long.kind, Exit::Usage); + assert_eq!(too_long.message, "The secret is longer than 8192 bytes"); // 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(); diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index dbf959b..9678a3d 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2338,6 +2338,43 @@ async fn the_hidden_key_prompt_saves_the_edited_key_without_showing_it() { assert_eq!(key.expose(), "hidden-secret"); } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn the_hidden_key_prompt_refuses_a_key_over_its_limit_and_saves_nothing() { + // One byte more than the prompt accepts; the read stops there, before any Enter. + static TOO_LONG: [u8; 8 * 1024 + 1] = [b'k'; 8 * 1024 + 1]; + let (server, dir) = fixture().await; + let import = [ + "auth", + "import-key", + "--account", + "k", + "--org", + ORG, + "--env", + ENV, + "--credential-store", + "file", + ]; + let run = on_a_terminal( + dir.path(), + &server, + &import, + &[("Environment API key:", &TOO_LONG)], + ) + .await; + assert_eq!(run.code, Some(2), "{}", run.shown); + assert!( + run.shown.contains("The secret is longer than 8192 bytes"), + "{}", + run.shown + ); + assert_eq!(run.modes.0, run.modes.1); + assert!(!run.shown.contains("kkkk"), "{}", run.shown); + let settings = Settings::open(dir.path().to_path_buf()).unwrap(); + assert!(settings.read_credential("k").is_err()); +} + #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn interrupt_during_a_refresh_says_the_login_may_need_renewal() { From 9c7bc297c2b6941b0c542e42b6fd8bc35a88828b Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:53:58 -0600 Subject: [PATCH 10/12] test(cli): check profiles create still warns about an overridden config directory --- tests/cli_integration.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 2f56a07..f78db8a 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -1012,11 +1012,23 @@ async fn profiles_are_created_from_scope_and_listed_without_ambient_state() { ); // The flags name what the profile saves, so the variables they override are no surprise. assert_eq!(stderr(&created), ""); - cli(dir.path(), &server) + // The directory decides where it saves, so a --config-dir over the variable still warns. + let duplicate = cli(dir.path(), &server) .env_remove("VOLTAGE_API_KEY") + .env("VOLTAGE_ORGANIZATION_ID", RESOURCE) + .env("VOLTAGE_CONFIG_DIR", dir.path().join("unused")) .args(create) .assert() .code(2); + let warnings: Vec<_> = stderr(&duplicate) + .lines() + .filter(|line| line.starts_with("warning:")) + .map(str::to_owned) + .collect(); + assert_eq!( + warnings, + ["warning: VOLTAGE_CONFIG_DIR is set but ignored because --config-dir is given"] + ); let listed = cli(dir.path(), &server) .args(["profiles", "list"]) .assert() From 161b3d30a84ff9b57cebe6276b4993af1874aaf0 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 10:25:06 -0600 Subject: [PATCH 11/12] fix(terminal): show the hidden prompt only once raw mode is on On Linux, keys typed before raw mode reach the line editor, which echoes them and drops a line past 4,095 bytes, so an over-long key never reached the limit check. --- src/terminal.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/terminal.rs b/src/terminal.rs index 318c8e4..f49022b 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -181,8 +181,6 @@ impl Terminal { /// 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 { - 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. @@ -190,6 +188,10 @@ impl Terminal { Ok(raw) => raw, Err(error) => return hidden(Err(error)), }; + // The prompt follows raw mode, so nothing typed after it reaches the terminal's line + // editor, which on Linux echoes it and drops input past 4,095 bytes. + eprint!("{prompt}"); + let _ = std::io::stderr().flush(); let read = read_detached(read_hidden_keys).await?; drop(raw); eprintln!(); From 08ed1054249841a7c584bc4aab791fd8de57c3be Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Wed, 4 Nov 2026 10:41:03 -0600 Subject: [PATCH 12/12] fix(terminal): keep the hidden prompt's limit within one terminal read crossterm reads at most 1,024 bytes per readiness event, and on Linux it reads the rest of a longer burst only when more input arrives. A 1,000-byte limit, still far above an API key, lets a paste one byte over the limit reach the check. --- CHANGELOG.md | 2 +- src/terminal.rs | 11 +++++++---- tests/cli_integration.rs | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23f68c0..36e7795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - `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. - A command reads a saved login's or API key's secret from the OS credential store once, instead of once for each lookup, so macOS asks for keychain access at most once per command. A login refresh still reads it again under the credential lock, to see a refresh that another command saved. -- 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. A key longer than 8 KiB is refused with exit 2 instead of being read without limit. +- 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. A key longer than 1,000 bytes is refused with exit 2 instead of being read without limit. ## 0.2.0 - 2026-10-22 diff --git a/src/terminal.rs b/src/terminal.rs index f49022b..59fb3e1 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -199,9 +199,12 @@ impl Terminal { } } -/// The most a hidden prompt accepts, in bytes. Its buffer is allocated at this size, so typing -/// never reallocates it and leaves no copy of the secret behind for zeroize to miss. -const MAX_HIDDEN_BYTES: usize = 8 * 1024; +/// The most a hidden prompt accepts, in bytes; an API key is far shorter. Its buffer is +/// allocated at this size, so typing never reallocates it and leaves no copy of the secret +/// behind for zeroize to miss. The limit stays below crossterm's 1,024-byte terminal read: on +/// Linux, crossterm reads the rest of a longer paste only when more input arrives, so a paste +/// one byte over the limit still reaches the check in a single read. +const MAX_HIDDEN_BYTES: usize = 1000; /// What a hidden prompt's read means: the secret on Enter, Ctrl-C as an interruption, a /// secret over [`MAX_HIDDEN_BYTES`] as a usage error, and a terminal that cannot be put in @@ -610,7 +613,7 @@ mod tests { ); let too_long = hidden(read(Hidden::TooLong)).unwrap_err(); assert_eq!(too_long.kind, Exit::Usage); - assert_eq!(too_long.message, "The secret is longer than 8192 bytes"); + assert_eq!(too_long.message, "The secret is longer than 1000 bytes"); // 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(); diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 9678a3d..4086f0e 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -2342,7 +2342,7 @@ async fn the_hidden_key_prompt_saves_the_edited_key_without_showing_it() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn the_hidden_key_prompt_refuses_a_key_over_its_limit_and_saves_nothing() { // One byte more than the prompt accepts; the read stops there, before any Enter. - static TOO_LONG: [u8; 8 * 1024 + 1] = [b'k'; 8 * 1024 + 1]; + static TOO_LONG: [u8; 1001] = [b'k'; 1001]; let (server, dir) = fixture().await; let import = [ "auth", @@ -2365,7 +2365,7 @@ async fn the_hidden_key_prompt_refuses_a_key_over_its_limit_and_saves_nothing() .await; assert_eq!(run.code, Some(2), "{}", run.shown); assert!( - run.shown.contains("The secret is longer than 8192 bytes"), + run.shown.contains("The secret is longer than 1000 bytes"), "{}", run.shown );