From 05ea58359ffca911d176b39125269f5390dc44ca Mon Sep 17 00:00:00 2001 From: Tony Hill Date: Wed, 24 Jun 2026 10:57:19 +0100 Subject: [PATCH] fix: change default credential store from Keyring to Auto Auto tries the system keychain first and transparently falls back to an unencrypted file when the keychain backend is unavailable. This fixes auth on headless Linux and WSL2 environments where no keychain daemon is running, without any configuration change required by the user. Keyring remains available via --credential-store keyring or config. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- src/auth/storage.rs | 2 +- src/config.rs | 21 ++++++++++----------- tests/credential_store_config.rs | 25 +++++++++++++++++++------ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/auth/storage.rs b/src/auth/storage.rs index c918d9b..f306133 100644 --- a/src/auth/storage.rs +++ b/src/auth/storage.rs @@ -18,7 +18,7 @@ //! //! [`default_storage`] picks a backend from the resolved //! [`CredentialStore`](crate::config::CredentialStore) mode (CLI flag, env var, -//! config file, or the `Keyring` default); see [`crate::config`]. +//! config file, or the `Auto` default); see [`crate::config`]. use std::sync::Arc; diff --git a/src/config.rs b/src/config.rs index 16021d5..6bdb1f8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -11,7 +11,7 @@ //! [`CredentialStore`]. The effective mode is resolved with the precedence //! //! ```text -//! --credential-store flag > ${PREFIX}_CREDENTIAL_STORE env > config file > default (Keyring) +//! --credential-store flag > ${PREFIX}_CREDENTIAL_STORE env > config file > default (Auto) //! ``` //! //! where `${PREFIX}` is the app id sanitized by @@ -31,19 +31,18 @@ use crate::error::CliCoreError; /// Where an auth provider stores credentials. /// /// The variant selects a concrete storage backend -/// (see [`crate::auth::storage`]). `Keyring` is the default and preserves the -/// historical behavior (system keychain only, hard error when unavailable); -/// `File` is the escape hatch for environments without a working keychain -/// (headless Linux, WSL). +/// (see [`crate::auth::storage`]). `Auto` is the default and tries the system +/// keychain first, falling back to an unencrypted file when the keychain is +/// unavailable (headless Linux, WSL). #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] #[non_exhaustive] pub enum CredentialStore { /// Try the system keychain; transparently fall back to an unencrypted file - /// when the keychain backend is unavailable. + /// when the keychain backend is unavailable. This is the default. + #[default] Auto, /// System keychain only. A keychain failure is a hard error and no file is - /// ever written. This is the default. - #[default] + /// ever written. Keyring, /// File only: never contact the system keychain. Credentials are written as /// unencrypted JSON under the config base directory. @@ -656,10 +655,10 @@ mod tests { } #[test] - fn resolution_defaults_to_keyring() { + fn resolution_defaults_to_auto() { assert_eq!( resolve_credential_store_with(None, None, &EngineConfig::default()), - CredentialStore::Keyring + CredentialStore::Auto ); } @@ -678,7 +677,7 @@ mod tests { // invalid env with no file falls through to the default assert_eq!( resolve_credential_store_with(None, Some("garbage"), &EngineConfig::default()), - CredentialStore::Keyring + CredentialStore::Auto ); } diff --git a/tests/credential_store_config.rs b/tests/credential_store_config.rs index 31eb638..268eb02 100644 --- a/tests/credential_store_config.rs +++ b/tests/credential_store_config.rs @@ -5,8 +5,8 @@ //! These tests mutate process-global state (`XDG_CONFIG_HOME`, the //! `ITEST_CREDENTIAL_STORE` env var, and the `--credential-store` flag latch), //! so they serialize on a shared lock. The file-storage backend is the seam we -//! assert against: a credential file is read only in `file`/`auto` modes, never -//! in the default `keyring` mode — so "status shows logged in" cleanly +//! assert against: a credential file is read in `file`/`auto` modes but never +//! in explicit `keyring` mode — so "status shows logged in" cleanly //! distinguishes which backend the engine selected without needing a keychain //! daemon or a browser login. #![cfg(feature = "pkce-auth")] @@ -138,17 +138,25 @@ async fn config_file_selects_file_store() { } #[tokio::test] -async fn default_keyring_mode_ignores_credential_file() { +async fn explicit_keyring_mode_ignores_credential_file() { let _guard = lock(); let dir = tempfile::tempdir().expect("tempdir"); let _xdg = EnvGuard::set("XDG_CONFIG_HOME", Some(&dir.path().to_string_lossy())); let _env = EnvGuard::set(ENV_VAR, None); - // No config file => default Keyring mode. A credential file exists but must - // be ignored (keyring-only never reads the file). + // Explicit keyring mode: a credential file exists but must be ignored + // (keyring-only never reads the file). seed_credential_file(dir.path()); let out = build_cli() - .run(["itest", "auth", "status", "--env", "dev"]) + .run([ + "itest", + "--credential-store", + "keyring", + "auth", + "status", + "--env", + "dev", + ]) .await; assert_ne!(out.exit_code, 0, "expected not-logged-in: {}", out.rendered); @@ -157,6 +165,11 @@ async fn default_keyring_mode_ignores_credential_file() { "keyring mode must not read the credential file: {}", out.rendered ); + + // Reset the flag latch for subsequent tests. + let _reset = build_cli() + .run(["itest", "auth", "status", "--env", "dev"]) + .await; } #[tokio::test]