From 7b645e6442ced1a194c687b530f07f009e5f4247 Mon Sep 17 00:00:00 2001 From: Brad Frank Date: Sun, 24 May 2026 17:37:37 -0700 Subject: [PATCH 1/2] feat: read login password and key from env or stdin Adds ATUIN_PASSWORD and ATUIN_KEY environment variable fallbacks and a new --password-stdin flag to `atuin login`, with the same env+stdin treatment for `atuin register`. Addresses the unattended-login gap from #183: --password is visible in the host's process list, which is unsafe in shared environments and impractical for CI / config management (Ansible, etc.) workflows. The 2021 fix (#185) only added an interactive prompt, which doesn't help non-interactive callers. Precedence: --password flag > --password-stdin > env var > interactive prompt. --password and --password-stdin are mutually exclusive (enforced by clap). Existing flag-only and interactive flows are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../atuin/src/command/client/account/login.rs | 131 ++++++++++++++++-- .../src/command/client/account/register.rs | 43 ++++-- 2 files changed, 157 insertions(+), 17 deletions(-) diff --git a/crates/atuin/src/command/client/account/login.rs b/crates/atuin/src/command/client/account/login.rs index 072f08151c3..14efc91c2b2 100644 --- a/crates/atuin/src/command/client/account/login.rs +++ b/crates/atuin/src/command/client/account/login.rs @@ -1,4 +1,7 @@ -use std::{io, path::PathBuf}; +use std::{ + io::{self, Read}, + path::PathBuf, +}; use clap::Parser; use eyre::{Context, Result, bail}; @@ -14,15 +17,28 @@ use atuin_client::{ }; use rpassword::prompt_password; +const PASSWORD_ENV: &str = "ATUIN_PASSWORD"; +const KEY_ENV: &str = "ATUIN_KEY"; + #[derive(Parser, Debug)] pub struct Cmd { #[clap(long, short)] pub username: Option, - #[clap(long, short)] + /// Account password. Falls back to the `ATUIN_PASSWORD` environment + /// variable, or `--password-stdin`, before prompting interactively. + /// Avoid `--password` in shared environments: it is visible in the + /// process list. + #[clap(long, short, conflicts_with = "password_stdin")] pub password: Option, - /// The encryption key for your account + /// Read the account password from standard input. Mutually exclusive + /// with `--password`. + #[clap(long, conflicts_with = "password")] + pub password_stdin: bool, + + /// The encryption key for your account. Falls back to the `ATUIN_KEY` + /// environment variable before prompting interactively. #[clap(long, short)] pub key: Option, @@ -81,7 +97,7 @@ impl Cmd { self.prompt_and_store_key(settings, store).await?; - let password = self.password.clone().unwrap_or_else(read_user_password); + let password = self.resolve_password()?; let mut totp_code = self.totp_code.clone(); let (session, auth_type) = loop { @@ -136,7 +152,7 @@ impl Cmd { /// (or accept them via flags). async fn run_legacy_login(&self, settings: &Settings, store: &SqliteStore) -> Result<()> { let username = or_user_input(self.username.clone(), "username"); - let password = self.password.clone().unwrap_or_else(read_user_password); + let password = self.resolve_password()?; self.prompt_and_store_key(settings, store).await?; @@ -178,6 +194,26 @@ impl Cmd { Ok(()) } + /// Resolve the account password from, in order: the `--password` flag, + /// `--password-stdin`, the `ATUIN_PASSWORD` environment variable, or an + /// interactive prompt. + /// + /// # Errors + /// Returns an error if `--password-stdin` was set and stdin could not be + /// read. + fn resolve_password(&self) -> Result { + if let Some(p) = &self.password { + return Ok(p.clone()); + } + if self.password_stdin { + return read_secret_from_stdin(); + } + if let Some(p) = env_secret(PASSWORD_ENV) { + return Ok(p); + } + Ok(read_user_password()) + } + async fn prompt_and_store_key(&self, settings: &Settings, store: &SqliteStore) -> Result<()> { let key_path = settings.key_path.as_str(); let key_path = PathBuf::from(key_path); @@ -190,10 +226,11 @@ impl Cmd { println!("Do not share this key with anyone."); println!("\nRead more here: https://docs.atuin.sh/guide/sync/#login \n"); - let key = or_user_input( - self.key.clone(), - "encryption key [blank to use existing key file]", - ); + let key = self + .key + .clone() + .or_else(|| env_secret(KEY_ENV)) + .unwrap_or_else(|| read_user_input("encryption key [blank to use existing key file]")); // if provided, the key may be EITHER base64, or a bip mnemonic // try to normalize on base64 @@ -328,9 +365,85 @@ fn read_user_input(name: &'static str) -> String { get_input().expect("Failed to read from input") } +/// Return the value of `var` if it is set and non-empty. +pub(super) fn env_secret(var: &str) -> Option { + std::env::var(var).ok().filter(|s| !s.is_empty()) +} + +/// Read a secret from stdin, stripping a single trailing newline (CR/LF). +/// +/// # Errors +/// Returns an error if stdin cannot be read or does not contain valid UTF-8. +pub(super) fn read_secret_from_stdin() -> Result { + let mut buf = String::new(); + io::stdin() + .read_to_string(&mut buf) + .context("failed to read secret from stdin")?; + Ok(buf.trim_end_matches(&['\r', '\n'][..]).to_string()) +} + #[cfg(test)] mod tests { + use super::*; use atuin_client::encryption::Key; + use clap::Parser; + + #[test] + fn password_and_password_stdin_are_mutually_exclusive() { + let result = Cmd::try_parse_from(["login", "--password", "x", "--password-stdin"]); + assert!(result.is_err(), "clap should reject both flags together"); + } + + #[test] + fn password_stdin_parses_without_password_flag() { + let cmd = Cmd::try_parse_from(["login", "--password-stdin"]).unwrap(); + assert!(cmd.password_stdin); + assert!(cmd.password.is_none()); + } + + #[test] + fn defaults_leave_stdin_flag_false() { + let cmd = Cmd::try_parse_from(["login"]).unwrap(); + assert!(!cmd.password_stdin); + assert!(cmd.password.is_none()); + } + + // The env var names in these tests carry a `_XYZZY` suffix so that no + // other code in the test binary (now or in the future) is expected to + // read them. That isolation is what makes the `unsafe` mutations sound + // under edition 2024's stricter env-mutation contract: no concurrent + // reader can observe torn state, because there is no concurrent reader. + + #[test] + fn env_secret_returns_none_when_unset() { + let name = "ATUIN_TEST_ENV_SECRET_UNSET_XYZZY"; + // SAFETY: no other test or production code reads this uniquely-named + // env var, so a parallel test thread cannot observe this mutation. + unsafe { std::env::remove_var(name) }; + assert_eq!(env_secret(name), None); + } + + #[test] + fn env_secret_returns_none_when_empty() { + let name = "ATUIN_TEST_ENV_SECRET_EMPTY_XYZZY"; + // SAFETY: no other test or production code reads this uniquely-named + // env var, so a parallel test thread cannot observe this mutation. + unsafe { std::env::set_var(name, "") }; + assert_eq!(env_secret(name), None); + // SAFETY: same as above. + unsafe { std::env::remove_var(name) }; + } + + #[test] + fn env_secret_returns_value_when_set() { + let name = "ATUIN_TEST_ENV_SECRET_SET_XYZZY"; + // SAFETY: no other test or production code reads this uniquely-named + // env var, so a parallel test thread cannot observe this mutation. + unsafe { std::env::set_var(name, "hunter2") }; + assert_eq!(env_secret(name), Some("hunter2".to_string())); + // SAFETY: same as above. + unsafe { std::env::remove_var(name) }; + } #[test] fn mnemonic_round_trip() { diff --git a/crates/atuin/src/command/client/account/register.rs b/crates/atuin/src/command/client/account/register.rs index f01427c09e8..e30e378a420 100644 --- a/crates/atuin/src/command/client/account/register.rs +++ b/crates/atuin/src/command/client/account/register.rs @@ -1,26 +1,53 @@ use clap::Parser; use eyre::{Result, bail}; -use super::login::or_user_input; +use super::login::{env_secret, or_user_input, read_secret_from_stdin}; use atuin_client::{ auth::{self, AuthResponse}, record::sqlite_store::SqliteStore, settings::{Settings, SyncAuth}, }; +const PASSWORD_ENV: &str = "ATUIN_PASSWORD"; + #[derive(Parser, Debug)] pub struct Cmd { #[clap(long, short)] pub username: Option, - #[clap(long, short)] + /// Account password. Falls back to the `ATUIN_PASSWORD` environment + /// variable, or `--password-stdin`, before prompting interactively. + #[clap(long, short, conflicts_with = "password_stdin")] pub password: Option, + /// Read the account password from standard input. Mutually exclusive + /// with `--password`. + #[clap(long, conflicts_with = "password")] + pub password_stdin: bool, + #[clap(long, short)] pub email: Option, } impl Cmd { + /// Resolve the account password from, in order: the `--password` flag, + /// `--password-stdin`, or the `ATUIN_PASSWORD` environment variable. + /// Returns `None` if no source was provided — callers decide whether + /// to prompt interactively or fall through to a different flow. + /// + /// # Errors + /// Returns an error if `--password-stdin` was set and stdin could not be + /// read. + fn resolve_password(&self) -> Result> { + if let Some(p) = &self.password { + return Ok(Some(p.clone())); + } + if self.password_stdin { + return Ok(Some(read_secret_from_stdin()?)); + } + Ok(env_secret(PASSWORD_ENV)) + } + #[allow(clippy::too_many_lines)] pub async fn run(&self, settings: &Settings, store: &SqliteStore) -> Result<()> { match settings.resolve_sync_auth().await { @@ -45,12 +72,14 @@ impl Cmd { SyncAuth::NotLoggedIn { .. } => {} } + let resolved_password = self.resolve_password()?; + if settings.is_hub_sync() { let required_for_headless = 3; let provided = [ self.username.is_some(), self.email.is_some(), - self.password.is_some(), + resolved_password.is_some(), ] .iter() .filter(|&b| *b) @@ -62,7 +91,7 @@ impl Cmd { } if let (Some(username), Some(email), Some(password)) = - (&self.username, &self.email, &self.password) + (&self.username, &self.email, &resolved_password) { // Headless registration via v0 API (for CI / scripting). let client = auth::auth_client(settings).await; @@ -112,6 +141,7 @@ impl Cmd { super::login::Cmd { username: None, password: None, + password_stdin: false, key: None, totp_code: None, from_registration: true, @@ -125,10 +155,7 @@ impl Cmd { let username = or_user_input(self.username.clone(), "username"); let email = or_user_input(self.email.clone(), "email"); - let password = self - .password - .clone() - .unwrap_or_else(super::login::read_user_password); + let password = resolved_password.unwrap_or_else(super::login::read_user_password); if password.is_empty() { bail!("please provide a password"); From c40b229031319d41af410e28d078e8523a518d1f Mon Sep 17 00:00:00 2001 From: Brad Frank Date: Sun, 24 May 2026 17:45:17 -0700 Subject: [PATCH 2/2] fixup: address review feedback * Rename ATUIN_KEY -> ATUIN_ENCRYPTION_KEY. The `atuin doctor` output already labels the `key_path` setting with "ATUIN_KEY", so reusing that name for a different concept (raw key contents) would have been user-facing confusion even though figment's prefix wiring does not actually collide (the real path override would be ATUIN_KEY_PATH). * Defer register's stdin/env password read until the headless path is reachable, so `printf %s "$PASSWORD" | atuin register --password-stdin --email a@b.com` (no username) no longer consumes the piped secret before falling through to OAuth. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/atuin/src/command/client/account/login.rs | 9 ++++++--- .../atuin/src/command/client/account/register.rs | 14 +++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/atuin/src/command/client/account/login.rs b/crates/atuin/src/command/client/account/login.rs index 14efc91c2b2..47029b19e46 100644 --- a/crates/atuin/src/command/client/account/login.rs +++ b/crates/atuin/src/command/client/account/login.rs @@ -18,7 +18,7 @@ use atuin_client::{ use rpassword::prompt_password; const PASSWORD_ENV: &str = "ATUIN_PASSWORD"; -const KEY_ENV: &str = "ATUIN_KEY"; +const KEY_ENV: &str = "ATUIN_ENCRYPTION_KEY"; #[derive(Parser, Debug)] pub struct Cmd { @@ -37,8 +37,11 @@ pub struct Cmd { #[clap(long, conflicts_with = "password")] pub password_stdin: bool, - /// The encryption key for your account. Falls back to the `ATUIN_KEY` - /// environment variable before prompting interactively. + /// The encryption key for your account. Falls back to the + /// `ATUIN_ENCRYPTION_KEY` environment variable before prompting + /// interactively. (Distinct from the existing `key_path`/`ATUIN_KEY` + /// concept, which is a path to a key file — this variable holds the + /// key contents.) #[clap(long, short)] pub key: Option, diff --git a/crates/atuin/src/command/client/account/register.rs b/crates/atuin/src/command/client/account/register.rs index e30e378a420..9d174c23ceb 100644 --- a/crates/atuin/src/command/client/account/register.rs +++ b/crates/atuin/src/command/client/account/register.rs @@ -72,7 +72,19 @@ impl Cmd { SyncAuth::NotLoggedIn { .. } => {} } - let resolved_password = self.resolve_password()?; + // For Hub sync, only resolve the password (which may read stdin) once + // we know the headless path is reachable; otherwise a piped secret + // would be silently consumed before falling through to OAuth. + let resolved_password = if settings.is_hub_sync() { + if self.username.is_some() && self.email.is_some() { + self.resolve_password()? + } else { + None + } + } else { + // Legacy registration always needs the password. + self.resolve_password()? + }; if settings.is_hub_sync() { let required_for_headless = 3;