dgf1988/codewhale
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(auth): default to file-backed secret store

Browse Source
This commit is contained in:
Hunter Bown
2026-05-08 17:20:07 -05:00
parent 9e6924c749
commit 07410521d7
3 changed files with 154 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/cli/src/lib.rs
+18 -16
View File
@@ -256,7 +256,7 @@ enum AuthCommand {
#[arg(long, value_enum)]
provider: ProviderArg,
},
/// Delete a provider's key from config and keyring storage.
/// Delete a provider's key from config and secret-store storage.
Clear {
#[arg(long, value_enum)]
provider: ProviderArg,
@@ -556,14 +556,14 @@ fn resolve_runtime_for_dispatch_with_secrets(
match store.save() {
Ok(()) => {
eprintln!(
"info: recovered API key from OS keyring and saved it to {}",
"info: recovered API key from secret store and saved it to {}",
store.path().display()
);
resolved.api_key_source = Some(RuntimeApiKeySource::ConfigFile);
}
Err(err) => {
eprintln!(
"warning: recovered API key from OS keyring but failed to save {}: {err}",
"warning: recovered API key from secret store but failed to save {}: {err}",
store.path().display()
);
}
@@ -806,7 +806,7 @@ fn auth_status_lines(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
let active_source = if config_key.is_some() {
"config"
} else if keyring_key.is_some() {
"keyring"
"secret store"
} else if env_key.is_some() {
"env"
} else {
@@ -832,14 +832,14 @@ fn auth_status_lines(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
vec![
format!("provider: {}", provider.as_str()),
format!("active source: {active_label}"),
"lookup order: config -> keyring -> env".to_string(),
"lookup order: config -> secret store -> env".to_string(),
format!(
"config file: {} ({})",
store.path().display(),
source_status(config_key, "missing")
),
format!(
"keyring: {} ({})",
"secret store: {} ({})",
secrets.backend_name(),
source_status(keyring_key.as_deref(), "missing")
),
@@ -929,7 +929,7 @@ fn run_auth_command_with_secrets(
let source = if in_file {
Some("config-file")
} else if in_keyring {
Some("keyring")
Some("secret-store")
} else if in_env {
Some("env")
} else {
@@ -947,11 +947,11 @@ fn run_auth_command_with_secrets(
clear_provider_api_key_from_config(store, provider);
clear_provider_api_key_from_keyring(secrets, provider);
store.save()?;
println!("cleared API key for {slot} from config and keyring");
println!("cleared API key for {slot} from config and secret store");
Ok(())
}
AuthCommand::List => {
println!("provider config keyring env active");
println!("provider config store env active");
let active_provider = store.config.provider;
for provider in PROVIDER_LIST {
let slot = provider_slot(provider);
@@ -962,7 +962,7 @@ fn run_auth_command_with_secrets(
let active = if file {
"config"
} else if keyring == Some(true) {
"keyring"
"store"
} else if env {
"env"
} else {
@@ -1012,8 +1012,8 @@ fn prompt_api_key(slot: &str) -> Result<String> {
Ok(key)
}
/// Move plaintext keys from config.toml into an explicit platform credential
/// store. Hidden in v0.8.8 because the normal setup path is config/env only.
/// Move plaintext keys from config.toml into the configured secret store.
/// Hidden in v0.8.8 because the normal setup path is config/env only.
fn run_auth_migrate(store: &mut ConfigStore, secrets: &Secrets, dry_run: bool) -> Result<()> {
let mut migrated: Vec<(ProviderKind, &'static str)> = Vec::new();
let mut warnings: Vec<String> = Vec::new();
@@ -1042,7 +1042,9 @@ fn run_auth_migrate(store: &mut ConfigStore, secrets: &Secrets, dry_run: bool) -
migrated.push((provider, slot));
continue;
} else if let Err(err) = secrets.set(slot, &value) {
warnings.push(format!("skipped {slot}: failed to write to keyring: {err}"));
warnings.push(format!(
"skipped {slot}: failed to write to secret store: {err}"
));
continue;
}
if !dry_run {
@@ -1060,7 +1062,7 @@ fn run_auth_migrate(store: &mut ConfigStore, secrets: &Secrets, dry_run: bool) -
.context("failed to write updated config.toml")?;
}
println!("keyring backend: {}", secrets.backend_name());
println!("secret store backend: {}", secrets.backend_name());
if migrated.is_empty() {
println!("nothing to migrate (config.toml has no plaintext api_key entries)");
} else {
@@ -2273,10 +2275,10 @@ mod tests {
assert!(output.contains("provider: deepseek"));
assert!(output.contains("active source: config (last4: ...3333)"));
assert!(output.contains("lookup order: config -> keyring -> env"));
assert!(output.contains("lookup order: config -> secret store -> env"));
assert!(output.contains("config file: "));
assert!(output.contains("set, last4: ...3333"));
assert!(output.contains("keyring: in-memory (test) (set, last4: ...2222)"));
assert!(output.contains("secret store: in-memory (test) (set, last4: ...2222)"));
assert!(output.contains("env var: DEEPSEEK_API_KEY (set, last4: ...1111)"));
assert!(!output.contains("sk-config-3333"));
assert!(!output.contains("sk-keyring-2222"));
crates/config/src/lib.rs
+5 -4
View File
@@ -835,7 +835,8 @@ impl ConfigToml {
///
/// This method keeps library callers prompt-free: CLI flag → config file
/// → environment. Call `resolve_runtime_options_with_secrets` when a
/// user-facing dispatcher should recover OS-keyring credentials.
/// user-facing dispatcher should recover credentials from the configured
/// secret store.
#[must_use]
pub fn resolve_runtime_options(&self, cli: &CliRuntimeOverrides) -> ResolvedRuntimeOptions {
let no_keyring = Secrets::new(std::sync::Arc::new(
@@ -846,7 +847,7 @@ impl ConfigToml {
/// Resolve runtime options using an explicit secrets façade.
///
/// API-key precedence is **CLI flag → config-file → keyring → environment**.
/// API-key precedence is **CLI flag → config-file → secret store → environment**.
#[must_use]
pub fn resolve_runtime_options_with_secrets(
&self,
@@ -869,8 +870,8 @@ impl ConfigToml {
// CLI flag wins outright. Otherwise: config-file → injected secrets/env.
// This makes `deepseek auth set` a reliable fix even when the user's
// shell still exports an old key. When the file is empty, the injected
// secrets façade recovers older OS-keyring credentials before falling
// back to ambient env.
// secrets façade recovers configured secret-store credentials before
// falling back to ambient env.
let from_file = provider_cfg.api_key.clone().or(root_deepseek_api_key);
let (api_key, api_key_source) = if let Some(value) = cli.api_key.clone() {
(Some(value), Some(RuntimeApiKeySource::Cli))
crates/secrets/src/lib.rs
+131 -19
View File
@@ -1,14 +1,13 @@
//! Secret storage for DeepSeek API keys.
//!
//! Provides a small abstraction (`KeyringStore`) plus a default
//! implementation backed by the OS keyring (`DefaultKeyringStore`),
//! a file-based fallback for headless or unsupported platforms
//! (`FileKeyringStore`), and an in-memory store for tests
//! file-based implementation (`FileKeyringStore`), an opt-in OS keyring
//! implementation (`DefaultKeyringStore`), and an in-memory store for tests
//! (`InMemoryKeyringStore`).
//!
//! Higher-level lookup through [`Secrets::resolve`] checks the keyring first
//! Higher-level lookup through [`Secrets::resolve`] checks the secret store first
//! and falls back to environment variables. Config-file precedence lives in the
//! config crate so user-facing commands can keep `config -> keyring -> env`
//! config crate so user-facing commands can keep `config -> secret store -> env`
//! explicit at the call site.
#![deny(missing_docs)]
@@ -23,6 +22,9 @@ use thiserror::Error;
/// Default OS keychain service name. macOS users can verify entries with
/// `security find-generic-password -s deepseek -a <provider>`.
pub const DEFAULT_SERVICE: &str = "deepseek";
/// Select the secret storage backend. Supported values are `file` (default)
/// and `system`/`keyring` for the OS credential store.
pub const SECRET_BACKEND_ENV: &str = "DEEPSEEK_SECRET_BACKEND";
/// Errors that may arise from a [`KeyringStore`] backend.
#[derive(Debug, Error)]
@@ -61,9 +63,10 @@ pub trait KeyringStore: Send + Sync {
}
/// OS keyring backend (macOS Keychain, Windows Credential Manager,
/// Linux Secret Service / kwallet). On platforms without a configured
/// native keyring dependency, probing this backend returns an unsupported
/// error so [`Secrets::auto_detect`] can fall back to [`FileKeyringStore`].
/// Linux Secret Service / kwallet). This backend is opt-in through
/// [`SECRET_BACKEND_ENV`]. On platforms without a configured native
/// keyring dependency, probing this backend returns an unsupported error so
/// [`Secrets::auto_detect`] can fall back to [`FileKeyringStore`].
#[derive(Debug, Clone)]
pub struct DefaultKeyringStore {
/// Keyring service name (defaults to [`DEFAULT_SERVICE`]).
@@ -349,17 +352,35 @@ impl KeyringStore for FileKeyringStore {
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum SecretBackendSelection {
File,
System,
Unknown,
}
fn secret_backend_selection(value: Option<&str>) -> SecretBackendSelection {
match value.map(str::trim).filter(|value| !value.is_empty()) {
None => SecretBackendSelection::File,
Some(value) => match value.to_ascii_lowercase().as_str() {
"file" | "local" | "json" => SecretBackendSelection::File,
"system" | "keyring" | "os" | "os-keyring" => SecretBackendSelection::System,
_ => SecretBackendSelection::Unknown,
},
}
}
/// High-level façade combining a [`KeyringStore`] with environment
/// variable fallbacks.
///
/// Lookup precedence: **keyring → env → none**. Callers that also have
/// Lookup precedence: **secret store → env → none**. Callers that also have
/// a TOML config layer must wire that themselves at the very end of
/// the chain.
#[derive(Clone)]
pub struct Secrets {
/// Underlying secret store.
pub store: Arc<dyn KeyringStore>,
/// Owner identifier within the keyring (typically "deepseek"); the
/// Owner identifier within the secret store (typically "deepseek"); the
/// `key` parameter passed to `resolve` is mapped to a slot in the
/// store as-is, while envs are looked up by canonical name.
service: String,
@@ -368,7 +389,7 @@ pub struct Secrets {
/// Source layer that provided a resolved secret.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SecretSource {
/// The configured keyring backend returned the secret.
/// The configured secret-store backend returned the secret.
Keyring,
/// A process environment variable returned the secret.
Env,
@@ -393,11 +414,50 @@ impl Secrets {
}
}
/// Construct the platform-appropriate default backend. On platforms
/// where an OS keyring backend is reachable this returns
/// [`DefaultKeyringStore`]; otherwise it falls back to
/// [`FileKeyringStore`] under `~/.deepseek/secrets/`.
/// Construct the default backend. The prompt-free default is
/// [`FileKeyringStore`] under `~/.deepseek/secrets/`. Set
/// [`SECRET_BACKEND_ENV`] to `system` or `keyring` to opt into the OS
/// credential store.
pub fn auto_detect() -> Self {
match secret_backend_selection(std::env::var(SECRET_BACKEND_ENV).ok().as_deref()) {
SecretBackendSelection::File => Self::file_backed_default(),
SecretBackendSelection::Unknown => {
tracing::warn!(
"{SECRET_BACKEND_ENV} has an unsupported value; using file-backed secret store"
);
Self::file_backed_default()
}
SecretBackendSelection::System => {
let default_store = DefaultKeyringStore::default();
match default_store.probe() {
Ok(()) => Self::new(Arc::new(default_store)),
Err(err) => {
tracing::warn!(
"OS keyring unavailable ({err}); falling back to file-backed secret store"
);
Self::file_backed_default()
}
}
}
}
}
fn file_backed_default() -> Self {
let path = FileKeyringStore::default_path()
.unwrap_or_else(|_| PathBuf::from(".deepseek-secrets.json"));
Self::new(Arc::new(FileKeyringStore::new(path)))
}
/// Construct the file-backed default backend directly.
#[must_use]
pub fn file_backed() -> Self {
Self::file_backed_default()
}
/// Construct the opt-in OS credential backend, falling back to the
/// file-backed store when the platform backend is unavailable.
#[must_use]
pub fn system_keyring() -> Self {
let default_store = DefaultKeyringStore::default();
match default_store.probe() {
Ok(()) => Self::new(Arc::new(default_store)),
@@ -405,9 +465,7 @@ impl Secrets {
tracing::warn!(
"OS keyring unavailable ({err}); falling back to file-backed secret store"
);
let path = FileKeyringStore::default_path()
.unwrap_or_else(|_| PathBuf::from(".deepseek-secrets.json"));
Self::new(Arc::new(FileKeyringStore::new(path)))
Self::file_backed_default()
}
}
}
@@ -418,7 +476,7 @@ impl Secrets {
self.store.backend_name()
}
/// Resolve a secret with `keyring → env → none` precedence.
/// Resolve a secret with `secret store → env → none` precedence.
///
/// `name` is the canonical provider name (`"deepseek"`,
/// `"openrouter"`, `"novita"`, `"nvidia"`/`"nvidia-nim"`, `"openai"`).
@@ -512,6 +570,7 @@ mod tests {
"VLLM_API_KEY",
"OLLAMA_API_KEY",
"OPENAI_API_KEY",
SECRET_BACKEND_ENV,
] {
// Safety: tests serialise on env_lock(); the broader
// workspace has the same pattern in `crates/config`.
@@ -519,6 +578,59 @@ mod tests {
}
}
#[test]
fn backend_selection_defaults_to_file() {
assert_eq!(secret_backend_selection(None), SecretBackendSelection::File);
assert_eq!(
secret_backend_selection(Some("")),
SecretBackendSelection::File
);
assert_eq!(
secret_backend_selection(Some(" file ")),
SecretBackendSelection::File
);
}
#[test]
fn backend_selection_accepts_explicit_system_keyring() {
assert_eq!(
secret_backend_selection(Some("system")),
SecretBackendSelection::System
);
assert_eq!(
secret_backend_selection(Some("keyring")),
SecretBackendSelection::System
);
assert_eq!(
secret_backend_selection(Some("os-keyring")),
SecretBackendSelection::System
);
}
#[test]
fn auto_detect_is_file_backed_by_default() {
let _lock = env_lock();
clear_known_envs();
let secrets = Secrets::auto_detect();
assert_eq!(secrets.backend_name(), "file-based (~/.deepseek/secrets/)");
}
#[test]
fn auto_detect_honors_explicit_file_backend() {
let _lock = env_lock();
clear_known_envs();
// Safety: env mutation guarded by env_lock().
unsafe { std::env::set_var(SECRET_BACKEND_ENV, "local") };
let secrets = Secrets::auto_detect();
assert_eq!(secrets.backend_name(), "file-based (~/.deepseek/secrets/)");
// Safety: env mutation guarded by env_lock().
unsafe { std::env::remove_var(SECRET_BACKEND_ENV) };
}
#[test]
fn in_memory_store_round_trips() {
let store = InMemoryKeyringStore::new();