diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 9e1156d7..0a942435 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -13,7 +13,9 @@ use deepseek_agent::ModelRegistry; use deepseek_app_server::{ AppServerOptions, run as run_app_server, run_stdio as run_app_server_stdio, }; -use deepseek_config::{CliRuntimeOverrides, ConfigStore, ProviderKind, ResolvedRuntimeOptions}; +use deepseek_config::{ + CliRuntimeOverrides, ConfigStore, ProviderKind, ResolvedRuntimeOptions, RuntimeApiKeySource, +}; use deepseek_execpolicy::{AskForApproval, ExecPolicyContext, ExecPolicyEngine}; use deepseek_mcp::{McpServerDefinition, run_stdio_server}; use deepseek_secrets::Secrets; @@ -232,7 +234,7 @@ struct AuthArgs { #[derive(Debug, Subcommand)] enum AuthCommand { - /// Show current provider, env vars, and config-file presence. + /// Show current provider and credential source state. Status, /// Save an API key to the shared user config file. Reads from /// `--api-key`, `--api-key-stdin`, or prompts on stdin when @@ -253,7 +255,7 @@ enum AuthCommand { #[arg(long, value_enum)] provider: ProviderArg, }, - /// Delete a provider's key from the shared user config file. + /// Delete a provider's key from config and keyring storage. Clear { #[arg(long, value_enum)] provider: ProviderArg, @@ -423,53 +425,71 @@ fn run() -> Result<()> { approval_policy: cli.approval_policy.clone(), sandbox_mode: cli.sandbox_mode.clone(), }; - let resolved_runtime = store.config.resolve_runtime_options(&runtime_overrides); - let command = cli.command.take(); match command { - Some(Commands::Run(args)) => delegate_to_tui(&cli, &resolved_runtime, args.args), + Some(Commands::Run(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); + delegate_to_tui(&cli, &resolved_runtime, args.args) + } Some(Commands::Doctor(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("doctor", args)) } Some(Commands::Models(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("models", args)) } Some(Commands::Sessions(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("sessions", args)) } - Some(Commands::Resume(args)) => run_resume_command(&cli, &resolved_runtime, args), + Some(Commands::Resume(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); + run_resume_command(&cli, &resolved_runtime, args) + } Some(Commands::Fork(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("fork", args)) } Some(Commands::Init(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("init", args)) } Some(Commands::Setup(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("setup", args)) } Some(Commands::Exec(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("exec", args)) } Some(Commands::Review(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("review", args)) } Some(Commands::Apply(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("apply", args)) } Some(Commands::Eval(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("eval", args)) } Some(Commands::Mcp(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("mcp", args)) } Some(Commands::Features(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("features", args)) } Some(Commands::Serve(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("serve", args)) } Some(Commands::Completions(args)) => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); delegate_to_tui(&cli, &resolved_runtime, tui_args("completions", args)) } Some(Commands::Login(args)) => run_login_command(&mut store, args), @@ -489,6 +509,7 @@ fn run() -> Result<()> { Some(Commands::Metrics(args)) => run_metrics_command(args), Some(Commands::Update) => update::run_update(), None => { + let resolved_runtime = resolve_runtime_for_dispatch(&mut store, &runtime_overrides); let mut forwarded = Vec::new(); if let Some(prompt) = cli.prompt_flag.clone().or_else(|| cli.prompt.clone()) { forwarded.push("--prompt".to_string()); @@ -499,6 +520,48 @@ fn run() -> Result<()> { } } +fn resolve_runtime_for_dispatch( + store: &mut ConfigStore, + runtime_overrides: &CliRuntimeOverrides, +) -> ResolvedRuntimeOptions { + let runtime_secrets = Secrets::auto_detect(); + resolve_runtime_for_dispatch_with_secrets(store, runtime_overrides, &runtime_secrets) +} + +fn resolve_runtime_for_dispatch_with_secrets( + store: &mut ConfigStore, + runtime_overrides: &CliRuntimeOverrides, + secrets: &Secrets, +) -> ResolvedRuntimeOptions { + let mut resolved = store + .config + .resolve_runtime_options_with_secrets(runtime_overrides, secrets); + + if resolved.api_key_source == Some(RuntimeApiKeySource::Keyring) + && !provider_config_set(store, resolved.provider) + && let Some(api_key) = resolved.api_key.clone() + { + write_provider_api_key_to_config(store, resolved.provider, &api_key); + match store.save() { + Ok(()) => { + eprintln!( + "info: recovered API key from OS keyring 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}", + store.path().display() + ); + } + } + } + + resolved +} + fn tui_args(command: &str, args: TuiPassthroughArgs) -> Vec { let mut forwarded = Vec::with_capacity(args.args.len() + 1); forwarded.push(command.to_string()); @@ -507,13 +570,13 @@ fn tui_args(command: &str, args: TuiPassthroughArgs) -> Vec { } fn run_login_command(store: &mut ConfigStore, args: LoginArgs) -> Result<()> { - run_login_command_with_secrets(store, args, &no_keyring_secrets()) + run_login_command_with_secrets(store, args, &Secrets::auto_detect()) } fn run_login_command_with_secrets( store: &mut ConfigStore, args: LoginArgs, - _secrets: &Secrets, + secrets: &Secrets, ) -> Result<()> { let provider: ProviderKind = args.provider.into(); store.config.provider = provider; @@ -552,31 +615,35 @@ fn run_login_command_with_secrets( None => read_api_key_from_stdin()?, }; write_provider_api_key_to_config(store, provider, &api_key); + let keyring_saved = write_provider_api_key_to_keyring(secrets, provider, &api_key); store.save()?; + let destination = if keyring_saved { + format!("{} and {}", store.path().display(), secrets.backend_name()) + } else { + store.path().display().to_string() + }; if provider == ProviderKind::Deepseek { - println!( - "logged in using API key mode (deepseek); saved key to {}", - store.path().display() - ); + println!("logged in using API key mode (deepseek); saved key to {destination}"); } else { println!( - "logged in using API key mode ({}); saved key to {}", + "logged in using API key mode ({}); saved key to {destination}", provider.as_str(), - store.path().display() ); } Ok(()) } fn run_logout_command(store: &mut ConfigStore) -> Result<()> { - run_logout_command_with_secrets(store, &no_keyring_secrets()) + run_logout_command_with_secrets(store, &Secrets::auto_detect()) } -fn run_logout_command_with_secrets(store: &mut ConfigStore, _secrets: &Secrets) -> Result<()> { +fn run_logout_command_with_secrets(store: &mut ConfigStore, secrets: &Secrets) -> Result<()> { + let active_provider = store.config.provider; store.config.api_key = None; for provider in PROVIDER_LIST { clear_provider_api_key_from_config(store, provider); } + clear_provider_api_key_from_keyring(secrets, active_provider); store.config.auth_mode = None; store.config.chatgpt_access_token = None; store.config.device_code_session = None; @@ -611,6 +678,7 @@ const PROVIDER_LIST: [ProviderKind; 8] = [ ProviderKind::Openai, ]; +#[cfg(test)] fn no_keyring_secrets() -> Secrets { Secrets::new(std::sync::Arc::new( deepseek_secrets::InMemoryKeyringStore::new(), @@ -665,15 +733,28 @@ fn provider_config_set(store: &ConfigStore, provider: ProviderKind) -> bool { slot.or(root).is_some_and(|v| !v.trim().is_empty()) } +fn provider_keyring_set(secrets: &Secrets, provider: ProviderKind) -> bool { + secrets + .get(provider_slot(provider)) + .ok() + .flatten() + .is_some_and(|v| !v.trim().is_empty()) +} + +fn write_provider_api_key_to_keyring( + secrets: &Secrets, + provider: ProviderKind, + api_key: &str, +) -> bool { + secrets.set(provider_slot(provider), api_key).is_ok() +} + +fn clear_provider_api_key_from_keyring(secrets: &Secrets, provider: ProviderKind) { + let _ = secrets.delete(provider_slot(provider)); +} + fn run_auth_command(store: &mut ConfigStore, command: AuthCommand) -> Result<()> { - match command { - AuthCommand::Migrate { dry_run } => run_auth_command_with_secrets( - store, - AuthCommand::Migrate { dry_run }, - &Secrets::auto_detect(), - ), - other => run_auth_command_with_secrets(store, other, &no_keyring_secrets()), - } + run_auth_command_with_secrets(store, command, &Secrets::auto_detect()) } fn run_auth_command_with_secrets( @@ -683,13 +764,28 @@ fn run_auth_command_with_secrets( ) -> Result<()> { match command { AuthCommand::Status => { - println!("provider: {}", store.config.provider.as_str()); - for provider in PROVIDER_LIST { - let slot = provider_slot(provider); - let env_set = provider_env_set(provider); - let file_set = provider_config_set(store, provider); - println!("{slot} auth: env={}, config={}", env_set, file_set); - } + let provider = store.config.provider; + println!("provider: {}", provider.as_str()); + println!("credential precedence: config -> keyring -> env"); + let slot = provider_slot(provider); + let file_set = provider_config_set(store, provider); + let keyring_set = (!file_set).then(|| provider_keyring_set(secrets, provider)); + let env_set = provider_env_set(provider); + let active = if file_set { + "config" + } else if keyring_set == Some(true) { + "keyring" + } else if env_set { + "env" + } else { + "missing" + }; + println!( + "{slot} auth: config={}, keyring={}, env={}, active={active}", + file_set, + keyring_status_short(keyring_set), + env_set + ); Ok(()) } AuthCommand::Set { @@ -705,23 +801,39 @@ fn run_auth_command_with_secrets( (None, false) => prompt_api_key(slot)?, }; write_provider_api_key_to_config(store, provider, &api_key); + let keyring_saved = write_provider_api_key_to_keyring(secrets, provider, &api_key); store.save()?; // Don't print the key. Don't echo length. - println!("saved API key for {slot} to {}", store.path().display()); + if keyring_saved { + println!( + "saved API key for {slot} to {} and {}", + store.path().display(), + secrets.backend_name() + ); + } else { + println!("saved API key for {slot} to {}", store.path().display()); + } Ok(()) } AuthCommand::Get { provider } => { let provider: ProviderKind = provider.into(); let slot = provider_slot(provider); - let in_env = provider_env_set(provider); let in_file = provider_config_set(store, provider); + let in_keyring = !in_file && provider_keyring_set(secrets, provider); + let in_env = provider_env_set(provider); // Report the highest-priority source that has it. - let resolved = in_env || in_file; - if resolved { - let source = if in_file { "config-file" } else { "env" }; - println!("{slot}: set (source: {source})"); + let source = if in_file { + Some("config-file") + } else if in_keyring { + Some("keyring") + } else if in_env { + Some("env") } else { - println!("{slot}: not set"); + None + }; + match source { + Some(source) => println!("{slot}: set (source: {source})"), + None => println!("{slot}: not set"), } Ok(()) } @@ -729,17 +841,35 @@ fn run_auth_command_with_secrets( let provider: ProviderKind = provider.into(); let slot = provider_slot(provider); clear_provider_api_key_from_config(store, provider); + clear_provider_api_key_from_keyring(secrets, provider); store.save()?; - println!("cleared API key for {slot}"); + println!("cleared API key for {slot} from config and keyring"); Ok(()) } AuthCommand::List => { - println!("provider env config"); + println!("provider config keyring env active"); + let active_provider = store.config.provider; for provider in PROVIDER_LIST { let slot = provider_slot(provider); - let env = provider_env_set(provider); let file = provider_config_set(store, provider); - println!("{slot:<12} {} {}", yes_no(env), yes_no(file)); + let keyring = (provider == active_provider && !file) + .then(|| provider_keyring_set(secrets, provider)); + let env = provider_env_set(provider); + let active = if file { + "config" + } else if keyring == Some(true) { + "keyring" + } else if env { + "env" + } else { + "missing" + }; + println!( + "{slot:<12} {} {} {} {active}", + yes_no(file), + keyring_status_short(keyring), + yes_no(env) + ); } Ok(()) } @@ -751,6 +881,14 @@ fn yes_no(b: bool) -> &'static str { if b { "yes" } else { "no " } } +fn keyring_status_short(state: Option) -> &'static str { + match state { + Some(true) => "yes", + Some(false) => "no ", + None => "n/a", + } +} + fn prompt_api_key(slot: &str) -> Result { use std::io::{IsTerminal, Write}; eprint!("Enter API key for {slot}: "); @@ -1158,7 +1296,11 @@ fn build_tui_command( cmd.env("DEEPSEEK_PROVIDER", resolved_runtime.provider.as_str()); if let Some(api_key) = resolved_runtime.api_key.as_ref() { cmd.env("DEEPSEEK_API_KEY", api_key); - cmd.env("DEEPSEEK_API_KEY_SOURCE", "dispatcher"); + let source = resolved_runtime + .api_key_source + .unwrap_or(RuntimeApiKeySource::Env) + .as_env_value(); + cmd.env("DEEPSEEK_API_KEY_SOURCE", source); } if let Some(model) = cli.model.as_ref() { @@ -1768,13 +1910,17 @@ mod tests { #[test] fn auth_set_writes_to_shared_config_file() { + use deepseek_secrets::{InMemoryKeyringStore, KeyringStore}; + use std::sync::Arc; + let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default(); let path = std::env::temp_dir().join(format!( "deepseek-cli-auth-set-test-{}-{nanos}.toml", std::process::id() )); let mut store = ConfigStore::load(Some(path.clone())).expect("store should load"); - let secrets = no_keyring_secrets(); + let inner = Arc::new(InMemoryKeyringStore::new()); + let secrets = Secrets::new(inner.clone()); run_auth_command_with_secrets( &mut store, @@ -1794,12 +1940,19 @@ mod tests { ); let saved = std::fs::read_to_string(&path).unwrap_or_default(); assert!(saved.contains("api_key = \"sk-keyring\"")); + assert_eq!( + inner.get("deepseek").unwrap().as_deref(), + Some("sk-keyring") + ); let _ = std::fs::remove_file(path); } #[test] fn auth_clear_removes_from_config() { + use deepseek_secrets::{InMemoryKeyringStore, KeyringStore}; + use std::sync::Arc; + let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default(); let path = std::env::temp_dir().join(format!( "deepseek-cli-auth-clear-test-{}-{nanos}.toml", @@ -1810,7 +1963,9 @@ mod tests { store.config.providers.deepseek.api_key = Some("sk-stale".to_string()); store.save().unwrap(); - let secrets = no_keyring_secrets(); + let inner = Arc::new(InMemoryKeyringStore::new()); + inner.set("deepseek", "sk-stale").unwrap(); + let secrets = Secrets::new(inner.clone()); run_auth_command_with_secrets( &mut store, @@ -1823,6 +1978,108 @@ mod tests { assert!(store.config.api_key.is_none()); assert!(store.config.providers.deepseek.api_key.is_none()); + assert_eq!(inner.get("deepseek").unwrap(), None); + + let _ = std::fs::remove_file(path); + } + + #[test] + fn auth_status_and_list_only_probe_active_provider_keyring() { + use deepseek_secrets::{KeyringStore, SecretsError}; + use std::sync::{Arc, Mutex}; + + #[derive(Default)] + struct RecordingStore { + gets: Mutex>, + } + + impl KeyringStore for RecordingStore { + fn get(&self, key: &str) -> Result, SecretsError> { + self.gets.lock().unwrap().push(key.to_string()); + Ok(None) + } + + fn set(&self, _key: &str, _value: &str) -> Result<(), SecretsError> { + Ok(()) + } + + fn delete(&self, _key: &str) -> Result<(), SecretsError> { + Ok(()) + } + + fn backend_name(&self) -> &'static str { + "recording" + } + } + + let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default(); + let path = std::env::temp_dir().join(format!( + "deepseek-cli-auth-active-keyring-test-{}-{nanos}.toml", + std::process::id() + )); + let mut store = ConfigStore::load(Some(path.clone())).expect("store should load"); + store.config.provider = ProviderKind::Deepseek; + let inner = Arc::new(RecordingStore::default()); + let secrets = Secrets::new(inner.clone()); + + run_auth_command_with_secrets(&mut store, AuthCommand::Status, &secrets) + .expect("status should succeed"); + run_auth_command_with_secrets(&mut store, AuthCommand::List, &secrets) + .expect("list should succeed"); + + assert_eq!( + inner.gets.lock().unwrap().as_slice(), + ["deepseek", "deepseek"] + ); + + let _ = std::fs::remove_file(path); + } + + #[test] + fn dispatch_keyring_recovery_self_heals_into_config_file() { + use deepseek_secrets::{InMemoryKeyringStore, KeyringStore}; + use std::sync::Arc; + + let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default(); + let path = std::env::temp_dir().join(format!( + "deepseek-cli-dispatch-keyring-heal-test-{}-{nanos}.toml", + std::process::id() + )); + let mut store = ConfigStore::load(Some(path.clone())).expect("store should load"); + let inner = Arc::new(InMemoryKeyringStore::new()); + inner.set("deepseek", "ring-key").unwrap(); + let secrets = Secrets::new(inner); + + let resolved = resolve_runtime_for_dispatch_with_secrets( + &mut store, + &CliRuntimeOverrides::default(), + &secrets, + ); + + assert_eq!(resolved.api_key.as_deref(), Some("ring-key")); + assert_eq!( + resolved.api_key_source, + Some(RuntimeApiKeySource::ConfigFile) + ); + assert_eq!(store.config.api_key.as_deref(), Some("ring-key")); + assert_eq!( + store.config.providers.deepseek.api_key.as_deref(), + Some("ring-key") + ); + + let saved = std::fs::read_to_string(&path).expect("config should be written"); + assert!(saved.contains("api_key = \"ring-key\"")); + + let resolved_again = resolve_runtime_for_dispatch_with_secrets( + &mut store, + &CliRuntimeOverrides::default(), + &no_keyring_secrets(), + ); + assert_eq!(resolved_again.api_key.as_deref(), Some("ring-key")); + assert_eq!( + resolved_again.api_key_source, + Some(RuntimeApiKeySource::ConfigFile) + ); let _ = std::fs::remove_file(path); } diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 0369f94f..cd0b5ab7 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use std::sync::OnceLock; use anyhow::{Context, Result, bail}; +use deepseek_secrets::SecretSource; pub use deepseek_secrets::Secrets; use serde::{Deserialize, Serialize}; @@ -673,10 +674,9 @@ impl ConfigToml { /// Resolve runtime options without touching platform credential stores. /// - /// v0.8.8 keeps the default auth path deliberately boring: - /// CLI flag → config file → environment. Explicit keyring migration - /// remains available through auth commands, but normal startup and - /// diagnostics must not trigger platform credential prompts. + /// 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. #[must_use] pub fn resolve_runtime_options(&self, cli: &CliRuntimeOverrides) -> ResolvedRuntimeOptions { let no_keyring = Secrets::new(std::sync::Arc::new( @@ -687,9 +687,7 @@ impl ConfigToml { /// Resolve runtime options using an explicit secrets façade. /// - /// API-key precedence is **CLI flag → config-file → environment**. - /// If a caller explicitly injects a secrets façade with a populated - /// credential store, that store is used only when config/env are empty. + /// API-key precedence is **CLI flag → config-file → keyring → environment**. #[must_use] pub fn resolve_runtime_options_with_secrets( &self, @@ -711,15 +709,23 @@ impl ConfigToml { .flatten(); // 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. The default caller injects an empty - // in-memory store, so this path does not touch platform credential - // stores during ordinary startup. + // 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. let from_file = provider_cfg.api_key.clone().or(root_deepseek_api_key); - let api_key = cli - .api_key - .clone() - .or_else(|| from_file.clone()) - .or_else(|| secrets.resolve(provider.as_str())); + let (api_key, api_key_source) = if let Some(value) = cli.api_key.clone() { + (Some(value), Some(RuntimeApiKeySource::Cli)) + } else if let Some(value) = from_file.clone().filter(|v| !v.trim().is_empty()) { + (Some(value), Some(RuntimeApiKeySource::ConfigFile)) + } else if let Some((value, source)) = secrets.resolve_with_source(provider.as_str()) { + let source = match source { + SecretSource::Keyring => RuntimeApiKeySource::Keyring, + SecretSource::Env => RuntimeApiKeySource::Env, + }; + (Some(value), Some(source)) + } else { + (None, None) + }; let base_url = cli .base_url @@ -792,6 +798,7 @@ impl ConfigToml { provider, model, api_key, + api_key_source, base_url, auth_mode, output_mode, @@ -890,11 +897,32 @@ pub struct CliRuntimeOverrides { pub sandbox_mode: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RuntimeApiKeySource { + Cli, + ConfigFile, + Keyring, + Env, +} + +impl RuntimeApiKeySource { + #[must_use] + pub fn as_env_value(self) -> &'static str { + match self { + Self::Cli => "cli", + Self::ConfigFile => "config", + Self::Keyring => "keyring", + Self::Env => "env", + } + } +} + #[derive(Debug, Clone)] pub struct ResolvedRuntimeOptions { pub provider: ProviderKind, pub model: String, pub api_key: Option, + pub api_key_source: Option, pub base_url: String, pub auth_mode: Option, pub output_mode: Option, @@ -1687,6 +1715,10 @@ mod tests { let resolved = config.resolve_runtime_options_with_secrets(&CliRuntimeOverrides::default(), &secrets); assert_eq!(resolved.api_key.as_deref(), Some("file-key")); + assert_eq!( + resolved.api_key_source, + Some(RuntimeApiKeySource::ConfigFile) + ); // Safety: env mutation guarded by env_lock(). unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }; @@ -1707,6 +1739,7 @@ mod tests { let resolved = config.resolve_runtime_options_with_secrets(&CliRuntimeOverrides::default(), &secrets); assert_eq!(resolved.api_key.as_deref(), Some("env-key")); + assert_eq!(resolved.api_key_source, Some(RuntimeApiKeySource::Env)); // Safety: env mutation guarded by env_lock(). unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }; @@ -1726,6 +1759,31 @@ mod tests { let resolved = config.resolve_runtime_options_with_secrets(&CliRuntimeOverrides::default(), &secrets); assert_eq!(resolved.api_key.as_deref(), Some("file-key")); + assert_eq!( + resolved.api_key_source, + Some(RuntimeApiKeySource::ConfigFile) + ); + } + + #[test] + fn keyring_resolves_when_config_file_empty_even_if_env_is_set() { + use deepseek_secrets::KeyringStore; + let _lock = env_lock(); + let _env = EnvGuard::without_deepseek_runtime_overrides(); + // Safety: env mutation guarded by env_lock(). + unsafe { std::env::set_var("DEEPSEEK_API_KEY", "stale-env-key") }; + + let store = std::sync::Arc::new(deepseek_secrets::InMemoryKeyringStore::new()); + store.set("deepseek", "ring-key").unwrap(); + let secrets = Secrets::new(store); + + let resolved = ConfigToml::default() + .resolve_runtime_options_with_secrets(&CliRuntimeOverrides::default(), &secrets); + assert_eq!(resolved.api_key.as_deref(), Some("ring-key")); + assert_eq!(resolved.api_key_source, Some(RuntimeApiKeySource::Keyring)); + + // Safety: env mutation guarded by env_lock(). + unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }; } #[test] @@ -1744,5 +1802,6 @@ mod tests { }; let resolved = ConfigToml::default().resolve_runtime_options_with_secrets(&cli, &secrets); assert_eq!(resolved.api_key.as_deref(), Some("cli-key")); + assert_eq!(resolved.api_key_source, Some(RuntimeApiKeySource::Cli)); } } diff --git a/crates/secrets/src/lib.rs b/crates/secrets/src/lib.rs index c500bf77..911f769e 100644 --- a/crates/secrets/src/lib.rs +++ b/crates/secrets/src/lib.rs @@ -328,6 +328,15 @@ pub struct Secrets { service: String, } +/// Source layer that provided a resolved secret. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SecretSource { + /// The configured keyring backend returned the secret. + Keyring, + /// A process environment variable returned the secret. + Env, +} + impl std::fmt::Debug for Secrets { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Secrets") @@ -379,12 +388,18 @@ impl Secrets { /// Empty strings on either layer are treated as "not set". #[must_use] pub fn resolve(&self, name: &str) -> Option { + self.resolve_with_source(name).map(|(value, _)| value) + } + + /// Resolve a secret and report which layer supplied it. + #[must_use] + pub fn resolve_with_source(&self, name: &str) -> Option<(String, SecretSource)> { if let Ok(Some(v)) = self.store.get(name) && !v.trim().is_empty() { - return Some(v); + return Some((v, SecretSource::Keyring)); } - env_for(name) + env_for(name).map(|value| (value, SecretSource::Env)) } /// Convenience: write a secret through the underlying store. @@ -494,6 +509,10 @@ mod tests { let secrets = Secrets::new(store); assert_eq!(secrets.resolve("deepseek").as_deref(), Some("ring-key")); + assert_eq!( + secrets.resolve_with_source("deepseek"), + Some(("ring-key".to_string(), SecretSource::Keyring)) + ); // Safety: env mutation guarded by env_lock(). unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }; } @@ -507,6 +526,10 @@ mod tests { let secrets = Secrets::new(Arc::new(InMemoryKeyringStore::new())); assert_eq!(secrets.resolve("deepseek").as_deref(), Some("env-fallback")); + assert_eq!( + secrets.resolve_with_source("deepseek"), + Some(("env-fallback".to_string(), SecretSource::Env)) + ); // Safety: env mutation guarded by env_lock(). unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }; } diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index c8d6c1ce..d68e31a4 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -1245,10 +1245,23 @@ fn report_write_status(label: &str, path: &Path, status: WriteStatus) { enum ApiKeySource { Env, Config, + Keyring, Missing, } fn resolve_api_key_source(config: &Config) -> ApiKeySource { + if std::env::var("DEEPSEEK_API_KEY") + .ok() + .filter(|k| !k.trim().is_empty()) + .is_some() + { + match std::env::var("DEEPSEEK_API_KEY_SOURCE").ok().as_deref() { + Some("config") => return ApiKeySource::Config, + Some("keyring") => return ApiKeySource::Keyring, + _ => {} + } + } + if config .api_key .as_ref() @@ -1303,6 +1316,10 @@ fn run_setup_status(config: &Config, workspace: &Path) -> Result<()> { " {} api_key: set via DEEPSEEK_API_KEY", "✓".truecolor(aqua_r, aqua_g, aqua_b) ), + ApiKeySource::Keyring => println!( + " {} api_key: set via OS keyring", + "✓".truecolor(aqua_r, aqua_g, aqua_b) + ), ApiKeySource::Config => println!( " {} api_key: set via config", "✓".truecolor(aqua_r, aqua_g, aqua_b) @@ -1546,6 +1563,7 @@ async fn run_doctor(config: &Config, workspace: &Path, config_path_override: Opt // Per-provider state: env + config file only (no values printed). // Keep doctor/status prompt-free even for unsigned rebuilt binaries. + let dispatcher_api_key_source = std::env::var("DEEPSEEK_API_KEY_SOURCE").ok(); for (provider, slot, env_names) in [ ( crate::config::ApiProvider::Deepseek, @@ -1589,11 +1607,16 @@ async fn run_doctor(config: &Config, workspace: &Path, config_path_override: Opt .filter(|v| !v.trim().is_empty()) .is_some() }); + let injected_runtime_key = matches!( + dispatcher_api_key_source.as_deref(), + Some("keyring" | "env" | "cli") + ); let in_config = config .provider_config_for(provider) .and_then(|entry| entry.api_key.as_ref()) .is_some_and(|v| !v.trim().is_empty()) || (matches!(provider, crate::config::ApiProvider::Deepseek) + && !injected_runtime_key && config .api_key .as_ref() @@ -1610,12 +1633,13 @@ async fn run_doctor(config: &Config, workspace: &Path, config_path_override: Opt if in_config { "yes" } else { "no" } ); } - println!(" · credential precedence: ~/.deepseek/config.toml, then env"); + println!(" · credential precedence: ~/.deepseek/config.toml, OS keyring, then env"); let api_key_source = resolve_api_key_source(config); let has_api_key = if config.deepseek_api_key().is_ok() { let source_label = match api_key_source { ApiKeySource::Config => "config.toml", + ApiKeySource::Keyring => "OS keyring", ApiKeySource::Env => "environment", ApiKeySource::Missing => "unknown source", }; @@ -1662,8 +1686,17 @@ async fn run_doctor(config: &Config, workspace: &Path, config_path_override: Opt "✗".truecolor(red_r, red_g, red_b) ); if error_msg.contains("401") || error_msg.contains("Unauthorized") { - println!(" Invalid API key. Check your DEEPSEEK_API_KEY or config.toml"); - if matches!(api_key_source, ApiKeySource::Env) { + println!( + " Invalid API key. Check `deepseek auth status`, DEEPSEEK_API_KEY, or config.toml" + ); + if matches!(api_key_source, ApiKeySource::Keyring) { + println!( + " The rejected key came from the OS keyring via the dispatcher." + ); + println!( + " Run `deepseek auth status` to inspect config/keyring/env sources." + ); + } else if matches!(api_key_source, ApiKeySource::Env) { println!( " The rejected key came from DEEPSEEK_API_KEY; no saved config key is present." ); @@ -2041,6 +2074,7 @@ fn run_doctor_json( let api_key_state = match resolve_api_key_source(config) { ApiKeySource::Env => "env", ApiKeySource::Config => "config", + ApiKeySource::Keyring => "keyring", ApiKeySource::Missing => "missing", }; @@ -5032,8 +5066,10 @@ mod setup_helper_tests { fn resolve_api_key_source_reports_env_when_set() { let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); let prev = std::env::var("DEEPSEEK_API_KEY").ok(); + let prev_source = std::env::var("DEEPSEEK_API_KEY_SOURCE").ok(); unsafe { std::env::set_var("DEEPSEEK_API_KEY", "test-helper-value"); + std::env::remove_var("DEEPSEEK_API_KEY_SOURCE"); } let cfg = Config::default(); let source = resolve_api_key_source(&cfg); @@ -5041,15 +5077,43 @@ mod setup_helper_tests { Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY", value) }, None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }, } + match prev_source { + Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY_SOURCE", value) }, + None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY_SOURCE") }, + } assert_eq!(source, ApiKeySource::Env); } + #[test] + fn resolve_api_key_source_reports_dispatcher_keyring() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + let prev = std::env::var("DEEPSEEK_API_KEY").ok(); + let prev_source = std::env::var("DEEPSEEK_API_KEY_SOURCE").ok(); + unsafe { + std::env::set_var("DEEPSEEK_API_KEY", "test-helper-value"); + std::env::set_var("DEEPSEEK_API_KEY_SOURCE", "keyring"); + } + let cfg = Config::default(); + let source = resolve_api_key_source(&cfg); + match prev { + Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY", value) }, + None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }, + } + match prev_source { + Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY_SOURCE", value) }, + None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY_SOURCE") }, + } + assert_eq!(source, ApiKeySource::Keyring); + } + #[test] fn resolve_api_key_source_prefers_config_over_env() { let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); let prev = std::env::var("DEEPSEEK_API_KEY").ok(); + let prev_source = std::env::var("DEEPSEEK_API_KEY_SOURCE").ok(); unsafe { std::env::set_var("DEEPSEEK_API_KEY", "stale-env-key"); + std::env::remove_var("DEEPSEEK_API_KEY_SOURCE"); } let cfg = Config { api_key: Some("fresh-config-key".to_string()), @@ -5060,6 +5124,10 @@ mod setup_helper_tests { Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY", value) }, None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY") }, } + match prev_source { + Some(value) => unsafe { std::env::set_var("DEEPSEEK_API_KEY_SOURCE", value) }, + None => unsafe { std::env::remove_var("DEEPSEEK_API_KEY_SOURCE") }, + } assert_eq!(source, ApiKeySource::Config); }