From 9442f4c68127713d7ebae59424bca0ff684351f7 Mon Sep 17 00:00:00 2001 From: Zious Date: Tue, 21 Apr 2026 08:06:32 -0500 Subject: [PATCH 1/4] feat(auth): add non-interactive flags to login and refresh (#211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add --email, --token, --client-id, --client-secret to both `jr auth login` and `jr auth refresh`. Each value resolves via flag > env var > TTY prompt. Under --no-input, missing credentials produce JrError::UserError (exit 64) naming the specific flag + env var so agents and CI scripts can recover. Env vars: JR_EMAIL (api_token flow) JR_API_TOKEN (api_token flow; prefer over --token to avoid process-list leakage) JR_OAUTH_CLIENT_ID (OAuth flow) JR_OAUTH_CLIENT_SECRET (OAuth flow; same preference) OAuth errors include a dev-console URL hint so first-time agents learn they must create an app at developer.atlassian.com before passing credentials — not just *how* to pass them. refresh_credentials keeps its clear-then-login ordering; the existing "Credentials were cleared" recovery line is now pinned by an integration test assertion so a future refactor can't drop the hint silently. Tests: - 5 new unit tests on resolve_credential (flag/env/empty/no_input/hint) - RAII EnvGuard for test env vars so a panic can't leak state across parallel tests in the same process - Tightened tests/auth_refresh.rs to assert exit 64, specific flag/env message, and recovery hint (replaces pre-#211 "fails without panic") Closes #211 --- src/cli/auth.rs | 280 ++++++++++++++++++++++++++++++++++++++---- src/cli/init.rs | 9 +- src/cli/mod.rs | 28 +++++ src/main.rs | 31 ++++- tests/auth_refresh.rs | 41 +++++-- 5 files changed, 345 insertions(+), 44 deletions(-) diff --git a/src/cli/auth.rs b/src/cli/auth.rs index aa60e5c..4ab587e 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -3,8 +3,77 @@ use dialoguer::{Input, Password}; use crate::api::auth; use crate::config::Config; +use crate::error::JrError; use crate::output; +/// Environment variable names for the four auth credentials. +/// +/// Flag > env > prompt precedence is implemented by [`resolve_credential`]. +/// Callers pass the matching `flag_name` so error messages can cite both +/// names verbatim. +pub(crate) const ENV_EMAIL: &str = "JR_EMAIL"; +pub(crate) const ENV_API_TOKEN: &str = "JR_API_TOKEN"; +pub(crate) const ENV_OAUTH_CLIENT_ID: &str = "JR_OAUTH_CLIENT_ID"; +pub(crate) const ENV_OAUTH_CLIENT_SECRET: &str = "JR_OAUTH_CLIENT_SECRET"; + +/// Resolve a credential value via flag → env → TTY prompt, or error under +/// `--no-input`. +/// +/// Order of precedence: +/// 1. `flag_value` (explicit CLI arg wins). +/// 2. `env::var(env_name)` if non-empty. +/// 3. If `no_input` is true, return a `JrError::UserError` naming the flag +/// and env var so scripts/agents can recover. `hint` — if supplied — +/// is appended to the error so first-time agents learn *where to obtain* +/// the credential, not just how to pass it (relevant for OAuth where +/// users must first create an app at developer.atlassian.com). +/// 4. Otherwise, prompt interactively. `is_password` chooses between +/// `dialoguer::Password` (masked) and `Input` (visible). +/// +/// Empty env values are ignored so an accidentally-exported-but-unset var +/// doesn't silently substitute for real input. +pub(crate) fn resolve_credential( + flag_value: Option, + env_name: &str, + flag_name: &str, + prompt_label: &str, + is_password: bool, + no_input: bool, + hint: Option<&str>, +) -> Result { + if let Some(v) = flag_value.filter(|v| !v.is_empty()) { + return Ok(v); + } + if let Ok(v) = std::env::var(env_name) + && !v.is_empty() + { + return Ok(v); + } + if no_input { + let base = format!("{prompt_label} is required. Provide {flag_name} or set ${env_name}."); + let msg = match hint { + Some(h) => format!("{base} {h}"), + None => base, + }; + return Err(JrError::UserError(msg).into()); + } + if is_password { + Password::new() + .with_prompt(prompt_label) + .interact() + .with_context(|| format!("failed to read {prompt_label}")) + } else { + Input::new() + .with_prompt(prompt_label) + .interact_text() + .with_context(|| format!("failed to read {prompt_label}")) + } +} + +/// Hint for OAuth client_id / client_secret errors so first-time agents +/// discover they must create an OAuth app before passing credentials. +const OAUTH_APP_HINT: &str = "Create one at https://developer.atlassian.com/console/myapps/."; + /// Which auth flow `jr auth refresh` should dispatch to. /// /// Internal detail of the `refresh` command; kept module-private so it @@ -44,17 +113,30 @@ fn chosen_flow(config: &Config, oauth_override: bool) -> AuthFlow { } } -/// Prompt for email and API token, then store in keychain. -pub async fn login_token() -> Result<()> { - let email: String = dialoguer::Input::new() - .with_prompt("Jira email") - .interact_text() - .context("failed to read Jira email")?; - - let token: String = dialoguer::Password::new() - .with_prompt("API token") - .interact() - .context("failed to read API token")?; +/// Resolve email and API token (flag → env → prompt), then store in keychain. +pub async fn login_token( + email: Option, + token: Option, + no_input: bool, +) -> Result<()> { + let email = resolve_credential( + email, + ENV_EMAIL, + "--email", + "Jira email", + false, + no_input, + None, + )?; + let token = resolve_credential( + token, + ENV_API_TOKEN, + "--token", + "API token", + true, + no_input, + None, + )?; auth::store_api_token(&email, &token)?; eprintln!("Credentials stored in keychain."); @@ -62,20 +144,37 @@ pub async fn login_token() -> Result<()> { } /// Run the OAuth 2.0 (3LO) login flow and persist site configuration. -/// Prompts the user for their own OAuth app credentials. -pub async fn login_oauth() -> Result<()> { - eprintln!("OAuth 2.0 requires your own Atlassian OAuth app."); - eprintln!("Create one at: https://developer.atlassian.com/console/myapps/\n"); - - let client_id: String = Input::new() - .with_prompt("OAuth Client ID") - .interact_text() - .context("failed to read OAuth client ID")?; +/// +/// Credentials resolved via flag → env → prompt, so CI/agent workflows can +/// pipe them in without a TTY. +pub async fn login_oauth( + client_id: Option, + client_secret: Option, + no_input: bool, +) -> Result<()> { + if !no_input { + eprintln!("OAuth 2.0 requires your own Atlassian OAuth app."); + eprintln!("Create one at: https://developer.atlassian.com/console/myapps/\n"); + } - let client_secret: String = Password::new() - .with_prompt("OAuth Client Secret") - .interact() - .context("failed to read OAuth client secret")?; + let client_id = resolve_credential( + client_id, + ENV_OAUTH_CLIENT_ID, + "--client-id", + "OAuth Client ID", + false, + no_input, + Some(OAUTH_APP_HINT), + )?; + let client_secret = resolve_credential( + client_secret, + ENV_OAUTH_CLIENT_SECRET, + "--client-secret", + "OAuth Client Secret", + true, + no_input, + Some(OAUTH_APP_HINT), + )?; // Store OAuth app credentials in keychain crate::api::auth::store_oauth_app_credentials(&client_id, &client_secret)?; @@ -152,6 +251,11 @@ fn refresh_success_payload(flow: AuthFlow) -> serde_json::Value { /// before the error is propagated. pub async fn refresh_credentials( oauth_override: bool, + email: Option, + token: Option, + client_id: Option, + client_secret: Option, + no_input: bool, output: &crate::cli::OutputFormat, ) -> Result<()> { let config = Config::load()?; @@ -162,8 +266,8 @@ pub async fn refresh_credentials( )?; let login_result = match flow { - AuthFlow::Token => login_token().await, - AuthFlow::OAuth => login_oauth().await, + AuthFlow::Token => login_token(email, token, no_input).await, + AuthFlow::OAuth => login_oauth(client_id, client_secret, no_input).await, }; if let Err(err) = login_result { @@ -263,4 +367,128 @@ mod tests { assert_eq!(payload["status"], "refreshed"); assert_eq!(payload["auth_method"], "oauth"); } + + // ── resolve_credential ─────────────────────────────────────────── + // + // Env-reading tests use per-test env var names to avoid races with + // parallel test threads. `EnvGuard` removes the var on drop so a panic + // mid-test doesn't leak state to later tests in the same process. + + struct EnvGuard(&'static str); + + impl EnvGuard { + fn set(key: &'static str, value: &str) -> Self { + // SAFETY: test-local keys (all prefixed `_JR_TEST_`), never read + // by production code. The Drop impl unsets the same key. + unsafe { + std::env::set_var(key, value); + } + EnvGuard(key) + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: matches the test-local key set in `EnvGuard::set`. + unsafe { + std::env::remove_var(self.0); + } + } + } + + #[test] + fn resolve_credential_prefers_flag_over_env() { + let _guard = EnvGuard::set("_JR_TEST_PREFERS_FLAG", "from-env"); + let got = resolve_credential( + Some("from-flag".into()), + "_JR_TEST_PREFERS_FLAG", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap(); + assert_eq!(got, "from-flag"); + } + + #[test] + fn resolve_credential_falls_back_to_env_when_flag_absent() { + let _guard = EnvGuard::set("_JR_TEST_FALLS_BACK", "from-env"); + let got = resolve_credential( + None, + "_JR_TEST_FALLS_BACK", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap(); + assert_eq!(got, "from-env"); + } + + #[test] + fn resolve_credential_ignores_empty_flag_and_env() { + // Empty values should fall through to the no_input error path. + let _guard = EnvGuard::set("_JR_TEST_EMPTY", ""); + let err = resolve_credential( + Some(String::new()), + "_JR_TEST_EMPTY", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap_err(); + assert!( + err.downcast_ref::() + .is_some_and(|e| matches!(e, JrError::UserError(_))), + "Expected JrError::UserError for empty inputs, got: {err}" + ); + } + + #[test] + fn resolve_credential_no_input_errors_when_missing() { + let err = resolve_credential( + None, + "_JR_TEST_UNSET_MISSING", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap_err(); + let msg = err.to_string(); + assert!( + err.downcast_ref::() + .is_some_and(|e| matches!(e, JrError::UserError(_))), + "Expected JrError::UserError, got: {err}" + ); + assert!( + msg.contains("--email") && msg.contains("$_JR_TEST_UNSET_MISSING"), + "Error should cite both flag and env var: {msg}" + ); + } + + #[test] + fn resolve_credential_oauth_hint_appears_in_error() { + let err = resolve_credential( + None, + "_JR_TEST_UNSET_OAUTH", + "--client-id", + "OAuth Client ID", + false, + true, + Some(OAUTH_APP_HINT), + ) + .unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("developer.atlassian.com/console/myapps"), + "OAuth error should cite dev console URL: {msg}" + ); + } } diff --git a/src/cli/init.rs b/src/cli/init.rs index 62abbae..17f2865 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -43,11 +43,14 @@ pub async fn handle() -> Result<()> { }; config.save_global()?; - // Step 3: Authenticate + // Step 3: Authenticate. `jr init` is inherently interactive (Select + // prompts above), so pass no_input=false and let dialoguer handle each + // credential prompt. Flags aren't plumbed through init — users who want + // a non-interactive setup should run `jr auth login` directly. if auth_choice == 0 { - crate::cli::auth::login_oauth().await?; + crate::cli::auth::login_oauth(None, None, false).await?; } else { - crate::cli::auth::login_token().await?; + crate::cli::auth::login_token(None, None, false).await?; let mut config = Config::load()?; config.global.instance.auth_method = Some("api_token".into()); config.save_global()?; diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 20aef62..4636e91 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -188,6 +188,20 @@ pub enum AuthCommand { /// Use OAuth 2.0 instead of API token (requires your own OAuth app) #[arg(long)] oauth: bool, + /// Jira email (API token flow). Prefer $JR_EMAIL over this flag. + #[arg(long)] + email: Option, + /// API token (API token flow). Prefer $JR_API_TOKEN over this flag — bare + /// CLI args can leak via process lists (`ps`, audit logs). + #[arg(long)] + token: Option, + /// OAuth Client ID (OAuth flow). Prefer $JR_OAUTH_CLIENT_ID over this flag. + #[arg(long)] + client_id: Option, + /// OAuth Client Secret (OAuth flow). Prefer $JR_OAUTH_CLIENT_SECRET over + /// this flag — bare CLI args can leak via process lists. + #[arg(long)] + client_secret: Option, }, /// Show authentication status Status, @@ -202,6 +216,20 @@ pub enum AuthCommand { /// Use OAuth 2.0 instead of API token (matches `jr auth login --oauth`) #[arg(long)] oauth: bool, + /// Jira email (API token flow). Prefer $JR_EMAIL over this flag. + #[arg(long)] + email: Option, + /// API token (API token flow). Prefer $JR_API_TOKEN over this flag — + /// bare CLI args can leak via process lists. + #[arg(long)] + token: Option, + /// OAuth Client ID (OAuth flow). Prefer $JR_OAUTH_CLIENT_ID over this flag. + #[arg(long)] + client_id: Option, + /// OAuth Client Secret (OAuth flow). Prefer $JR_OAUTH_CLIENT_SECRET over + /// this flag — bare CLI args can leak via process lists. + #[arg(long)] + client_secret: Option, }, } diff --git a/src/main.rs b/src/main.rs index b74689f..e27092b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,16 +70,37 @@ async fn run(cli: Cli) -> anyhow::Result<()> { cli::assets::handle(command, &cli.output, &client).await } cli::Command::Auth { command } => match command { - cli::AuthCommand::Login { oauth } => { + cli::AuthCommand::Login { + oauth, + email, + token, + client_id, + client_secret, + } => { if oauth { - cli::auth::login_oauth().await + cli::auth::login_oauth(client_id, client_secret, cli.no_input).await } else { - cli::auth::login_token().await + cli::auth::login_token(email, token, cli.no_input).await } } cli::AuthCommand::Status => cli::auth::status().await, - cli::AuthCommand::Refresh { oauth } => { - cli::auth::refresh_credentials(oauth, &cli.output).await + cli::AuthCommand::Refresh { + oauth, + email, + token, + client_id, + client_secret, + } => { + cli::auth::refresh_credentials( + oauth, + email, + token, + client_id, + client_secret, + cli.no_input, + &cli.output, + ) + .await } }, cli::Command::Me => { diff --git a/tests/auth_refresh.rs b/tests/auth_refresh.rs index e0bc533..cabb3f8 100644 --- a/tests/auth_refresh.rs +++ b/tests/auth_refresh.rs @@ -40,13 +40,15 @@ fn auth_refresh_oauth_help_is_accepted() { } #[test] -fn auth_refresh_non_interactive_fails_without_panic() { - // Pin: with stdin closed, `jr auth refresh` must exit non-zero without - // panicking. The dialoguer prompts inside login_token hit EOF and return - // io::UnexpectedEof, which should propagate as a normal error rather - // than a panic or abort. If login gains non-interactive flag - // equivalents later, tighten this to assert the specific exit code and - // the "Credentials were cleared" recovery message. +fn auth_refresh_no_input_fails_with_clear_message() { + // Pin: `jr auth refresh --no-input` without any credential flags must + // fail with a UserError (exit 64) that names the missing flag and env + // var. Enabled by #211 — login flows now resolve credentials via + // flag → env → prompt and error explicitly under --no-input. + // + // Replaces the pre-#211 test that asserted "fails without panic" when + // stdin was closed; the new contract is stronger (specific exit code + + // actionable message) so scripts/agents can recover. // // `JR_SERVICE_NAME` scopes the keychain service so `auth::clear_credentials()` // inside the subprocess never touches the developer's real `jr-jira-cli` @@ -59,8 +61,9 @@ fn auth_refresh_non_interactive_fails_without_panic() { .env("XDG_CACHE_HOME", cache_dir.path()) .env("XDG_CONFIG_HOME", config_dir.path()) .env("JR_SERVICE_NAME", "jr-jira-cli-test") - .args(["auth", "refresh"]) - .write_stdin("") + .env_remove("JR_EMAIL") + .env_remove("JR_API_TOKEN") + .args(["--no-input", "auth", "refresh"]) .output() .unwrap(); @@ -68,8 +71,26 @@ fn auth_refresh_non_interactive_fails_without_panic() { assert!( !output.status.success(), - "auth refresh with closed stdin should fail, got stdout: {}", + "auth refresh --no-input without flags should fail, got stdout: {}", String::from_utf8_lossy(&output.stdout) ); + assert_eq!( + output.status.code(), + Some(64), + "Missing credentials under --no-input should exit 64 (UserError), got: {:?}", + output.status.code() + ); + assert!( + stderr.contains("--email") && stderr.contains("$JR_EMAIL"), + "Error should cite --email flag and $JR_EMAIL env var: {stderr}" + ); + // The clear-then-login ordering means credentials *are* cleared before + // the login failure bubbles up. The recovery hint tells users exactly + // how to get back to a working state — pinning it here so a future + // refactor can't silently drop the guidance. + assert!( + stderr.contains("Credentials were cleared"), + "Error should include recovery hint after cleared credentials: {stderr}" + ); assert!(!stderr.contains("panic"), "stderr leaked a panic: {stderr}"); } From b0284971a9a09e71231c9083a5e750552a9d99bf Mon Sep 17 00:00:00 2001 From: Zious Date: Tue, 21 Apr 2026 08:11:43 -0500 Subject: [PATCH 2/4] docs: README guidance for non-interactive auth flags (#211) Document the new flag/env-var surface in three places: - Quick Start code block: shell examples for both api_token and OAuth flows under --no-input - macOS upgrade section: CI/headless recovery path - Scripting & AI Agents section: bullet referencing all 4 flags + env vars - Commands table: expand auth login entry, add auth refresh entry Paired with the feature commit (9442f4c) on this branch. --- README.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index bb6a23c..ec456c3 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,10 @@ new binary becomes the creator of fresh Keychain entries. **Click "Always Allow"** on the two prompts macOS shows during re-store — otherwise future commands will prompt again. +For CI or headless boxes, pass credentials via env vars or flags +(e.g. `JR_EMAIL="..." JR_API_TOKEN="$TOKEN" jr --no-input auth refresh`) +so the flow completes without a TTY. + Tracked in [#207](https://github.com/Zious11/jira-cli/issues/207). A longer-term fix (Developer ID signing) is tracked as a separate issue. @@ -86,6 +90,12 @@ jr auth login # Or authenticate with OAuth 2.0 (requires your own OAuth app) jr auth login --oauth +# Non-interactive (CI / agents): flags or env vars, no TTY required. +# Prefer env vars for secrets — bare CLI args can leak via process lists. +JR_EMAIL="you@example.com" JR_API_TOKEN="$TOKEN" jr --no-input auth login +JR_OAUTH_CLIENT_ID="$ID" JR_OAUTH_CLIENT_SECRET="$SECRET" \ + jr --no-input auth login --oauth + # View your current sprint/board issues jr issue list --project FOO @@ -134,7 +144,8 @@ jr issue comment KEY-123 "Deployed to staging" | Command | Description | |---------|-------------| | `jr init` | Configure Jira instance and authenticate | -| `jr auth login` | Authenticate with API token (default) or `--oauth` for OAuth 2.0 | +| `jr auth login` | Authenticate with API token (default) or `--oauth` for OAuth 2.0. Non-interactive: `--email`/`--token` or `JR_EMAIL`/`JR_API_TOKEN`; `--client-id`/`--client-secret` or `JR_OAUTH_CLIENT_ID`/`JR_OAUTH_CLIENT_SECRET` for OAuth | +| `jr auth refresh` | Clear stored credentials and re-run login (same flags/env vars as `auth login`) | | `jr auth status` | Show authentication status | | `jr me` | Show current user info | | `jr issue list` | List issues (`--assignee`, `--reporter`, `--recent`, `--status`, `--open`, `--team`, `--asset KEY`, `--jql`, `--limit`/`--all`, `--points`, `--assets`) | @@ -227,6 +238,7 @@ board_id = 42 - `--url-only` prints URLs instead of opening a browser - State-changing commands are idempotent (exit 0 if already in target state) - Structured exit codes (see [Exit Codes](#exit-codes) table) +- `auth login` / `auth refresh` accept credentials via flags (`--email`, `--token`, `--client-id`, `--client-secret`) or env vars (`JR_EMAIL`, `JR_API_TOKEN`, `JR_OAUTH_CLIENT_ID`, `JR_OAUTH_CLIENT_SECRET`) — no TTY required. Prefer env vars for secrets. ```bash # AI agent workflow example From 8bd718b7b13f33236b12945cc306bb368162b8d0 Mon Sep 17 00:00:00 2001 From: Zious Date: Tue, 21 Apr 2026 08:15:21 -0500 Subject: [PATCH 3/4] fix(tests): pin api_token flow in auth_refresh test (#211) Config::load() merges JR_* env vars via figment's Env::prefixed at src/config.rs:65. A developer with JR_INSTANCE_AUTH_METHOD=oauth exported in their shell would take the OAuth path under `auth refresh`, and the test's email/JR_EMAIL stderr assertions would fail. Explicitly env_remove("JR_INSTANCE_AUTH_METHOD") so the test is hermetic to parent-env configuration. Flagged by Copilot on PR #243. --- tests/auth_refresh.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/auth_refresh.rs b/tests/auth_refresh.rs index cabb3f8..49c72d5 100644 --- a/tests/auth_refresh.rs +++ b/tests/auth_refresh.rs @@ -63,6 +63,12 @@ fn auth_refresh_no_input_fails_with_clear_message() { .env("JR_SERVICE_NAME", "jr-jira-cli-test") .env_remove("JR_EMAIL") .env_remove("JR_API_TOKEN") + // Config::load() merges JR_* via figment's Env::prefixed at + // src/config.rs:65 — JR_INSTANCE_AUTH_METHOD=oauth in the parent + // shell would flip refresh to the OAuth path and our email/JR_EMAIL + // stderr assertions would fail. Explicitly clear it to pin the + // api_token flow for this test. + .env_remove("JR_INSTANCE_AUTH_METHOD") .args(["--no-input", "auth", "refresh"]) .output() .unwrap(); From b25f4b480c61b38e0eacbcb9b68bc477ec727284 Mon Sep 17 00:00:00 2001 From: Zious Date: Tue, 21 Apr 2026 08:25:45 -0500 Subject: [PATCH 4/4] fix(tests): serialize env mutation with static Mutex (#211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Copilot review on PR #243: `EnvGuard` did RAII cleanup but didn't serialize env-var mutation across parallel tests. Since `std::env::set_var` / `remove_var` are unsafe in Rust 2024 (concurrent env access is UB — C's getenv/setenv aren't thread-safe), tests could still race even with distinct per-test keys. Add `static ENV_LOCK: Mutex<()>` and hold the lock for EnvGuard's full lifetime. Also lock reader-only tests that call `resolve_credential` (which reads env via `std::env::var`), since reads must also be serialized against concurrent writes. Matches the established pattern in src/config.rs::ENV_MUTEX and src/cache.rs::ENV_MUTEX. --- src/cli/auth.rs | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/cli/auth.rs b/src/cli/auth.rs index 4ab587e..7fb2175 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -370,28 +370,40 @@ mod tests { // ── resolve_credential ─────────────────────────────────────────── // - // Env-reading tests use per-test env var names to avoid races with - // parallel test threads. `EnvGuard` removes the var on drop so a panic - // mid-test doesn't leak state to later tests in the same process. - - struct EnvGuard(&'static str); + // Env-reading tests must serialize process-environment mutation across + // parallel test threads. `std::env::set_var` / `remove_var` are unsafe + // in Rust 2024 because concurrent env access (even on different keys) + // is UB — C's getenv/setenv aren't thread-safe. `EnvGuard` holds + // `ENV_LOCK` for its full lifetime and removes the var on drop so a + // panic mid-test doesn't leak state to later tests in the same + // process. Matches the pattern in src/config.rs::ENV_MUTEX. + + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + struct EnvGuard { + key: &'static str, + _lock: std::sync::MutexGuard<'static, ()>, + } impl EnvGuard { fn set(key: &'static str, value: &str) -> Self { - // SAFETY: test-local keys (all prefixed `_JR_TEST_`), never read - // by production code. The Drop impl unsets the same key. + let lock = ENV_LOCK.lock().unwrap(); + // SAFETY: test env mutation is serialized by ENV_LOCK, held for + // this guard's lifetime. The Drop impl unsets the same + // test-local key before releasing the lock. unsafe { std::env::set_var(key, value); } - EnvGuard(key) + EnvGuard { key, _lock: lock } } } impl Drop for EnvGuard { fn drop(&mut self) { - // SAFETY: matches the test-local key set in `EnvGuard::set`. + // SAFETY: matches the test-local key set in `EnvGuard::set` + // while `_lock` is still held by this `EnvGuard`. unsafe { - std::env::remove_var(self.0); + std::env::remove_var(self.key); } } } @@ -451,6 +463,9 @@ mod tests { #[test] fn resolve_credential_no_input_errors_when_missing() { + // resolve_credential reads env via std::env::var — hold ENV_LOCK to + // serialize against set/remove calls in sibling tests. + let _lock = ENV_LOCK.lock().unwrap(); let err = resolve_credential( None, "_JR_TEST_UNSET_MISSING", @@ -475,6 +490,8 @@ mod tests { #[test] fn resolve_credential_oauth_hint_appears_in_error() { + // Same env-read serialization as the test above. + let _lock = ENV_LOCK.lock().unwrap(); let err = resolve_credential( None, "_JR_TEST_UNSET_OAUTH",