Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 125 additions & 9 deletions crates/atuin/src/command/client/account/login.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::{io, path::PathBuf};
use std::{
io::{self, Read},
path::PathBuf,
};

use clap::Parser;
use eyre::{Context, Result, bail};
Expand All @@ -14,15 +17,31 @@ use atuin_client::{
};
use rpassword::prompt_password;

const PASSWORD_ENV: &str = "ATUIN_PASSWORD";
const KEY_ENV: &str = "ATUIN_ENCRYPTION_KEY";

#[derive(Parser, Debug)]
pub struct Cmd {
#[clap(long, short)]
pub username: Option<String>,

#[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<String>,

/// 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_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<String>,

Expand Down Expand Up @@ -81,7 +100,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 {
Expand Down Expand Up @@ -136,7 +155,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?;

Expand Down Expand Up @@ -178,6 +197,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<String> {
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);
Expand All @@ -190,10 +229,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]"));
Comment on lines +232 to +236

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Avoid ATUIN_KEY collision

ATUIN_KEY is already used by Atuin's config environment loader as the key_path override, and doctor.rs reports that same variable as the key path. This new fallback now treats the same value as raw encryption key material. When a user has ATUIN_KEY=/custom/path/to/key to point Atuin at a key file, atuin login will try to parse /custom/path/to/key as the encryption key and fail instead of using the existing key file.


// if provided, the key may be EITHER base64, or a bip mnemonic
// try to normalize on base64
Expand Down Expand Up @@ -328,9 +368,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<String> {
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<String> {
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() {
Expand Down
55 changes: 47 additions & 8 deletions crates/atuin/src/command/client/account/register.rs
Original file line number Diff line number Diff line change
@@ -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<String>,

#[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<String>,

/// 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<String>,
}

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<Option<String>> {
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 {
Expand All @@ -45,12 +72,26 @@ impl Cmd {
SyncAuth::NotLoggedIn { .. } => {}
}

// 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;
let provided = [
self.username.is_some(),
self.email.is_some(),
self.password.is_some(),
resolved_password.is_some(),
]
.iter()
.filter(|&b| *b)
Expand All @@ -62,7 +103,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;
Expand Down Expand Up @@ -112,6 +153,7 @@ impl Cmd {
super::login::Cmd {
username: None,
password: None,
password_stdin: false,
key: None,
totp_code: None,
from_registration: true,
Expand All @@ -125,10 +167,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");
Expand Down
Loading