fix(auth): show credential source table
## Summary - show config, keyring, and env credential sources in deepseek auth status - point env-only auth failures at auth status and auth set recovery commands - document auth status and provider key precedence ## Test plan - cargo test -p deepseek-tui-cli auth_ --locked - cargo test -p deepseek-tui-cli --locked - cargo test -p deepseek-tui env_only_auth_error_gets_recovery_hint --locked - cargo test -p deepseek-config api_key --locked - cargo test -p deepseek-config config_file_resolves_above_env_and_keyring --locked - cargo test -p deepseek-config keyring_resolves_when_config_file_empty_even_if_env_is_set --locked - cargo test -p deepseek-secrets --locked - cargo fmt --all -- --check - git diff --check - cargo clippy -p deepseek-tui-cli -p deepseek-tui -p deepseek-secrets --all-targets --all-features --locked -- -D warnings Closes #907
This commit is contained in:
@@ -95,6 +95,7 @@ You can also set it ahead of time:
|
||||
|
||||
```bash
|
||||
deepseek auth set --provider deepseek # saves to ~/.deepseek/config.toml
|
||||
deepseek auth status # shows the active credential source
|
||||
|
||||
export DEEPSEEK_API_KEY="YOUR_KEY" # env var alternative; use ~/.zshenv for non-interactive shells
|
||||
deepseek
|
||||
@@ -103,9 +104,10 @@ deepseek doctor # verify setup
|
||||
```
|
||||
|
||||
If `deepseek doctor` says the rejected key came from `DEEPSEEK_API_KEY`, remove
|
||||
the stale export from your shell startup file, open a fresh shell, then run
|
||||
`deepseek auth set --provider deepseek`. Saved config keys take precedence over
|
||||
the environment and are easier to rotate.
|
||||
the stale export from your shell startup file, open a fresh shell, or run
|
||||
`deepseek auth set --provider deepseek`. Use `deepseek auth status` to see the
|
||||
config, keyring, and env-var source state without printing the key. Saved config
|
||||
keys take precedence over the keyring and environment and are easier to rotate.
|
||||
|
||||
> To rotate or remove a saved key: `deepseek auth clear --provider deepseek`.
|
||||
|
||||
|
||||
+182
-29
@@ -721,28 +721,59 @@ fn clear_provider_api_key_from_config(store: &mut ConfigStore, provider: Provide
|
||||
}
|
||||
|
||||
fn provider_env_set(provider: ProviderKind) -> bool {
|
||||
deepseek_secrets::env_for(provider_slot(provider)).is_some()
|
||||
provider_env_value(provider).is_some()
|
||||
}
|
||||
|
||||
fn provider_config_set(store: &ConfigStore, provider: ProviderKind) -> bool {
|
||||
fn provider_env_vars(provider: ProviderKind) -> &'static [&'static str] {
|
||||
match provider {
|
||||
ProviderKind::Deepseek => &["DEEPSEEK_API_KEY"],
|
||||
ProviderKind::Openrouter => &["OPENROUTER_API_KEY"],
|
||||
ProviderKind::Novita => &["NOVITA_API_KEY"],
|
||||
ProviderKind::NvidiaNim => &["NVIDIA_API_KEY", "NVIDIA_NIM_API_KEY", "DEEPSEEK_API_KEY"],
|
||||
ProviderKind::Fireworks => &["FIREWORKS_API_KEY"],
|
||||
ProviderKind::Sglang => &["SGLANG_API_KEY"],
|
||||
ProviderKind::Vllm => &["VLLM_API_KEY"],
|
||||
ProviderKind::Ollama => &["OLLAMA_API_KEY"],
|
||||
ProviderKind::Openai => &["OPENAI_API_KEY"],
|
||||
}
|
||||
}
|
||||
|
||||
fn provider_env_value(provider: ProviderKind) -> Option<(&'static str, String)> {
|
||||
provider_env_vars(provider).iter().find_map(|var| {
|
||||
std::env::var(var)
|
||||
.ok()
|
||||
.filter(|value| !value.trim().is_empty())
|
||||
.map(|value| (*var, value))
|
||||
})
|
||||
}
|
||||
|
||||
fn provider_config_api_key(store: &ConfigStore, provider: ProviderKind) -> Option<&str> {
|
||||
let slot = store
|
||||
.config
|
||||
.providers
|
||||
.for_provider(provider)
|
||||
.api_key
|
||||
.as_ref();
|
||||
.as_deref();
|
||||
let root = (provider == ProviderKind::Deepseek)
|
||||
.then_some(store.config.api_key.as_ref())
|
||||
.then_some(store.config.api_key.as_deref())
|
||||
.flatten();
|
||||
slot.or(root).is_some_and(|v| !v.trim().is_empty())
|
||||
slot.or(root).filter(|v| !v.trim().is_empty())
|
||||
}
|
||||
|
||||
fn provider_keyring_set(secrets: &Secrets, provider: ProviderKind) -> bool {
|
||||
fn provider_config_set(store: &ConfigStore, provider: ProviderKind) -> bool {
|
||||
provider_config_api_key(store, provider).is_some()
|
||||
}
|
||||
|
||||
fn provider_keyring_api_key(secrets: &Secrets, provider: ProviderKind) -> Option<String> {
|
||||
secrets
|
||||
.get(provider_slot(provider))
|
||||
.ok()
|
||||
.flatten()
|
||||
.is_some_and(|v| !v.trim().is_empty())
|
||||
.filter(|v| !v.trim().is_empty())
|
||||
}
|
||||
|
||||
fn provider_keyring_set(secrets: &Secrets, provider: ProviderKind) -> bool {
|
||||
provider_keyring_api_key(secrets, provider).is_some()
|
||||
}
|
||||
|
||||
fn write_provider_api_key_to_keyring(
|
||||
@@ -757,6 +788,72 @@ fn clear_provider_api_key_from_keyring(secrets: &Secrets, provider: ProviderKind
|
||||
let _ = secrets.delete(provider_slot(provider));
|
||||
}
|
||||
|
||||
fn auth_status_lines(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
|
||||
let provider = store.config.provider;
|
||||
let config_key = provider_config_api_key(store, provider);
|
||||
let keyring_key = provider_keyring_api_key(secrets, provider);
|
||||
let env_key = provider_env_value(provider);
|
||||
|
||||
let active_source = if config_key.is_some() {
|
||||
"config"
|
||||
} else if keyring_key.is_some() {
|
||||
"keyring"
|
||||
} else if env_key.is_some() {
|
||||
"env"
|
||||
} else {
|
||||
"missing"
|
||||
};
|
||||
let active_last4 = config_key
|
||||
.map(last4_label)
|
||||
.or_else(|| keyring_key.as_deref().map(last4_label))
|
||||
.or_else(|| env_key.as_ref().map(|(_, value)| last4_label(value)));
|
||||
let active_label = active_last4
|
||||
.map(|last4| format!("{active_source} (last4: {last4})"))
|
||||
.unwrap_or_else(|| active_source.to_string());
|
||||
|
||||
let env_var_label = env_key
|
||||
.as_ref()
|
||||
.map(|(name, _)| (*name).to_string())
|
||||
.unwrap_or_else(|| provider_env_vars(provider).join("/"));
|
||||
let env_status = env_key
|
||||
.as_ref()
|
||||
.map(|(_, value)| format!("set, last4: {}", last4_label(value)))
|
||||
.unwrap_or_else(|| "unset".to_string());
|
||||
|
||||
vec![
|
||||
format!("provider: {}", provider.as_str()),
|
||||
format!("active source: {active_label}"),
|
||||
"lookup order: config -> keyring -> env".to_string(),
|
||||
format!(
|
||||
"config file: {} ({})",
|
||||
store.path().display(),
|
||||
source_status(config_key, "missing")
|
||||
),
|
||||
format!(
|
||||
"keyring: {} ({})",
|
||||
secrets.backend_name(),
|
||||
source_status(keyring_key.as_deref(), "missing")
|
||||
),
|
||||
format!("env var: {env_var_label} ({env_status})"),
|
||||
]
|
||||
}
|
||||
|
||||
fn source_status(value: Option<&str>, missing_label: &str) -> String {
|
||||
value
|
||||
.map(|v| format!("set, last4: {}", last4_label(v)))
|
||||
.unwrap_or_else(|| missing_label.to_string())
|
||||
}
|
||||
|
||||
fn last4_label(value: &str) -> String {
|
||||
let trimmed = value.trim();
|
||||
let chars: Vec<char> = trimmed.chars().collect();
|
||||
if chars.len() <= 4 {
|
||||
return "<redacted>".to_string();
|
||||
}
|
||||
let last4: String = chars[chars.len() - 4..].iter().collect();
|
||||
format!("...{last4}")
|
||||
}
|
||||
|
||||
fn run_auth_command(store: &mut ConfigStore, command: AuthCommand) -> Result<()> {
|
||||
run_auth_command_with_secrets(store, command, &Secrets::auto_detect())
|
||||
}
|
||||
@@ -768,28 +865,9 @@ fn run_auth_command_with_secrets(
|
||||
) -> Result<()> {
|
||||
match command {
|
||||
AuthCommand::Status => {
|
||||
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
|
||||
);
|
||||
for line in auth_status_lines(store, secrets) {
|
||||
println!("{line}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
AuthCommand::Set {
|
||||
@@ -1491,6 +1569,8 @@ fn read_api_key_from_stdin() -> Result<String> {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use clap::error::ErrorKind;
|
||||
use std::ffi::OsString;
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
|
||||
fn parse_ok(argv: &[&str]) -> Cli {
|
||||
Cli::try_parse_from(argv).unwrap_or_else(|err| panic!("parse failed for {argv:?}: {err}"))
|
||||
@@ -1502,6 +1582,41 @@ mod tests {
|
||||
err.to_string()
|
||||
}
|
||||
|
||||
fn env_lock() -> std::sync::MutexGuard<'static, ()> {
|
||||
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
|
||||
LOCK.get_or_init(|| Mutex::new(()))
|
||||
.lock()
|
||||
.unwrap_or_else(|p| p.into_inner())
|
||||
}
|
||||
|
||||
struct ScopedEnvVar {
|
||||
name: &'static str,
|
||||
previous: Option<OsString>,
|
||||
}
|
||||
|
||||
impl ScopedEnvVar {
|
||||
fn set(name: &'static str, value: &str) -> Self {
|
||||
let previous = std::env::var_os(name);
|
||||
// Safety: tests using this helper serialize with env_lock() and
|
||||
// restore the original value in Drop.
|
||||
unsafe { std::env::set_var(name, value) };
|
||||
Self { name, previous }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ScopedEnvVar {
|
||||
fn drop(&mut self) {
|
||||
// Safety: tests using this helper serialize with env_lock().
|
||||
unsafe {
|
||||
if let Some(previous) = self.previous.take() {
|
||||
std::env::set_var(self.name, previous);
|
||||
} else {
|
||||
std::env::remove_var(self.name);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clap_command_definition_is_consistent() {
|
||||
Cli::command().debug_assert();
|
||||
@@ -2105,6 +2220,44 @@ mod tests {
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auth_status_reports_all_active_provider_sources_with_last4() {
|
||||
use deepseek_secrets::{InMemoryKeyringStore, KeyringStore};
|
||||
use std::sync::Arc;
|
||||
|
||||
let _lock = env_lock();
|
||||
let _env = ScopedEnvVar::set("DEEPSEEK_API_KEY", "sk-env-1111");
|
||||
|
||||
let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default();
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"deepseek-cli-auth-status-table-test-{}-{nanos}.toml",
|
||||
std::process::id()
|
||||
));
|
||||
let mut store = ConfigStore::load(Some(path.clone())).expect("store should load");
|
||||
store.config.provider = ProviderKind::Deepseek;
|
||||
store.config.api_key = Some("sk-config-3333".to_string());
|
||||
store.config.providers.deepseek.api_key = Some("sk-config-3333".to_string());
|
||||
|
||||
let inner = Arc::new(InMemoryKeyringStore::new());
|
||||
inner.set("deepseek", "sk-keyring-2222").unwrap();
|
||||
let secrets = Secrets::new(inner);
|
||||
|
||||
let output = auth_status_lines(&store, &secrets).join("\n");
|
||||
|
||||
assert!(output.contains("provider: deepseek"));
|
||||
assert!(output.contains("active source: config (last4: ...3333)"));
|
||||
assert!(output.contains("lookup order: config -> keyring -> 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("env var: DEEPSEEK_API_KEY (set, last4: ...1111)"));
|
||||
assert!(!output.contains("sk-config-3333"));
|
||||
assert!(!output.contains("sk-keyring-2222"));
|
||||
assert!(!output.contains("sk-env-1111"));
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dispatch_keyring_recovery_self_heals_into_config_file() {
|
||||
use deepseek_secrets::{InMemoryKeyringStore, KeyringStore};
|
||||
|
||||
@@ -5,13 +5,10 @@
|
||||
//! a file-based fallback for headless Linux (`FileKeyringStore`), and
|
||||
//! an in-memory store for tests (`InMemoryKeyringStore`).
|
||||
//!
|
||||
//! Higher-level lookup goes through [`Secrets::resolve`], which checks
|
||||
//! the keyring first and falls back to environment variables. The
|
||||
//! caller (typically the config crate) then falls back to plaintext
|
||||
//! TOML if both are empty — that final layer lives outside this crate
|
||||
//! so the precedence is explicit at the call site.
|
||||
//!
|
||||
//! Hard rule: **keyring → env → config-file**. Never swap.
|
||||
//! Higher-level lookup through [`Secrets::resolve`] checks the keyring first
|
||||
//! and falls back to environment variables. Config-file precedence lives in the
|
||||
//! config crate so user-facing commands can keep `config -> keyring -> env`
|
||||
//! explicit at the call site.
|
||||
#![deny(missing_docs)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -380,7 +380,8 @@ impl Engine {
|
||||
|
||||
Some(format!(
|
||||
"The rejected key came from {env_var}; no saved config key is present.\n\
|
||||
Run `deepseek auth set --provider {provider}` to save a valid key in ~/.deepseek/config.toml, \
|
||||
Run `deepseek auth status` to inspect credential sources, then \
|
||||
`deepseek auth set --provider {provider}` to save a valid key in ~/.deepseek/config.toml, \
|
||||
or remove the stale export and open a fresh shell.",
|
||||
provider = provider.as_str()
|
||||
))
|
||||
|
||||
@@ -93,6 +93,7 @@ fn env_only_auth_error_gets_recovery_hint() {
|
||||
|
||||
assert!(message.contains("DEEPSEEK_API_KEY"));
|
||||
assert!(message.contains("no saved config key is present"));
|
||||
assert!(message.contains("deepseek auth status"));
|
||||
assert!(message.contains("deepseek auth set --provider deepseek"));
|
||||
}
|
||||
|
||||
|
||||
@@ -55,6 +55,12 @@ the legacy `deepseek login --api-key ...` alias) saves the key to
|
||||
`~/.deepseek/config.toml`, and `deepseek --model deepseek-v4-flash` is forwarded
|
||||
to the TUI as `DEEPSEEK_MODEL`.
|
||||
|
||||
Credential lookup uses `config -> keyring -> env` after any explicit CLI
|
||||
`--api-key`. Run `deepseek auth status` to inspect the active provider's config
|
||||
file, OS keyring backend, environment variable, winning source, and last-four
|
||||
label without printing the key itself. The command only probes the active
|
||||
provider's keyring entry.
|
||||
|
||||
For hosted or self-hosted providers, set `provider = "nvidia-nim"`,
|
||||
`"fireworks"`, `"sglang"`, `"vllm"`, or `"ollama"` or pass `deepseek --provider <name>`. The facade
|
||||
saves provider credentials to the shared user config and forwards the resolved
|
||||
@@ -131,7 +137,8 @@ If a profile is selected but missing, DeepSeek TUI exits with an error listing a
|
||||
|
||||
## Environment Variables
|
||||
|
||||
These override config values:
|
||||
Most runtime environment variables override config values. API-key variables are
|
||||
fallbacks after saved config and keyring credentials:
|
||||
|
||||
- `DEEPSEEK_API_KEY`
|
||||
- `DEEPSEEK_BASE_URL`
|
||||
|
||||
Reference in New Issue
Block a user