codewhale-hq/Codewhale
1
0 Fork 0
mirror of https://github.com/codewhale-hq/Codewhale.git synced 2026-11-11 01:51:34 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

feat(tui): resolve provider credentials through one sourced resolver

Browse Source 
`has_api_key_for` was 150 lines of provider special cases with no stated
precedence rule, returning a bare bool. Callers could learn that a
provider had no usable credential; they could not learn where it looked,
which is why the provider picker's "missing key" was unactionable.

The cascade moves verbatim, in order, into config::credential_resolve and
each outcome gains a `CredentialSource`: the env var that won, the config
table, the durable slot, the consented external CLI file, the OAuth flow,
or `Missing` carrying the ordered list of places actually probed and the
one command that fixes each. The precedence rule is now stated once, in a
doc comment beside the resolver — including the two places CodeWhale
deliberately departs from pi's "stored credential owns the provider"
(a route-named api_key_env and official-endpoint ambient variables both
outrank the durable store), which were previously true but unwritten.

No precedence decision changed. `has_api_key_for` is now a wrapper, and a
test asserts the two agree for every provider.

Two credential write paths also stop interleaving. Both the root and
per-provider saves snapshot the durable slot, write it, mutate the config
document, and roll back on failure, and logout probes each slot before
deleting it — none of it under a lock, so a save racing a logout on one
slot could leave the store and the config file disagreeing. All three now
hold that provider's credential write lock across the whole
read-modify-write, and logout enumerates through the store's own `list`.

A new regression test pins the contradiction that started this lane: with
a key in the durable DeepSeek slot but no api-key marker in the config
table, `Config::deepseek_api_key` resolves it while `has_api_key_for`
reports nothing — #5033's marker gate applies to readiness but not to the
request path. The gate is left alone; what changes is that the resolver
now reports the slot as "not read" with the reason, instead of letting it
look empty.

Implemented with agent assistance.

Signed-off-by: Hunter Bown <[email protected]>
This commit is contained in:
Hunter Bown committed 2026-08-20 15:09:18 -07:00
1 parent 7bc5ccf032
commit b6160739fc
3 files changed
+721 -263
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/tui/src/config.rs
+163 -263
View File
@@ -14,6 +14,7 @@ use serde_json::json;
use std::os::unix::fs::PermissionsExt;
use crate::audit::log_sensitive_event;
use crate::credentials::CredentialStore;
use crate::features::{Feature, Features, FeaturesToml, is_known_feature_key};
use crate::hooks::HooksConfig;
@@ -24,6 +25,11 @@ use crate::hooks::HooksConfig;
// (#3311).
#[cfg(test)]
mod scope_tests;
// The single place provider credential precedence is decided. Lives inside
// `config` so it can walk the private probe helpers without widening their
// visibility; `has_api_key_for` is a thin wrapper over it (#pi-auth-port).
mod credential_resolve;
pub(crate) use credential_resolve::resolve_credential_source;
mod subagent_limits;
pub use subagent_limits::*;
use subagent_limits::{resolve_subagent_api_timeout_secs, resolve_subagent_heartbeat_timeout_secs};
@@ -10716,21 +10722,24 @@ fn save_root_api_key_for_secret_slot(
let path = credential_config_path().context("Failed to resolve config path for API key.")?;
if let Some(secrets) = credential_secret_store() {
let prior_secret = secrets.get(secret_slot);
match prior_secret.as_ref() {
Ok(prior) => match secrets.set(secret_slot, trimmed) {
Ok(()) => {
if let Err(error) = save_root_api_key_metadata_without_plaintext(
&path,
clear_deepseek_provider_slot,
) {
let current = secrets.get(secret_slot).map_err(|rollback| {
// Same read-modify-write as the per-provider save below; hold the slot's
// write lock across snapshot, store write, config write, and rollback.
return crate::credentials::store::with_provider_write_lock(secret_slot, || {
let prior_secret = secrets.get(secret_slot);
match prior_secret.as_ref() {
Ok(prior) => match secrets.set(secret_slot, trimmed) {
Ok(()) => {
if let Err(error) = save_root_api_key_metadata_without_plaintext(
&path,
clear_deepseek_provider_slot,
) {
let current = secrets.get(secret_slot).map_err(|rollback| {
anyhow::anyhow!(
"{error}; additionally could not verify secret-store rollback for {secret_slot}: {rollback}"
)
})?;
if current.as_deref() == Some(trimmed) {
match prior {
if current.as_deref() == Some(trimmed) {
match prior {
Some(previous) => secrets.set(secret_slot, previous),
None => secrets.delete(secret_slot),
}
@@ -10739,31 +10748,32 @@ fn save_root_api_key_for_secret_slot(
"{error}; additionally failed to restore prior secret-store state for {secret_slot}: {rollback}"
)
})?;
}
return Err(error);
}
return Err(error);
codewhale_config::scrub_plaintext_api_keys_from_config_backup(&path)?;
let backend = secrets.backend_name().to_string();
log_sensitive_event(
"credential.save",
json!({
"backend": backend.clone(),
"config_path": path.display().to_string(),
"plaintext_config_fallback": false,
}),
);
return Ok(SavedCredential::KeyringAndConfigFile { backend, path });
}
codewhale_config::scrub_plaintext_api_keys_from_config_backup(&path)?;
let backend = secrets.backend_name().to_string();
log_sensitive_event(
"credential.save",
json!({
"backend": backend.clone(),
"config_path": path.display().to_string(),
"plaintext_config_fallback": false,
}),
);
return Ok(SavedCredential::KeyringAndConfigFile { backend, path });
Err(err) => {
return Err(plaintext_credential_fallback_refused("write", &path, &err));
}
},
Err(error) => {
return Err(plaintext_credential_fallback_refused(
"snapshot", &path, &error,
));
}
Err(err) => {
return Err(plaintext_credential_fallback_refused("write", &path, &err));
}
},
Err(error) => {
return Err(plaintext_credential_fallback_refused(
"snapshot", &path, &error,
));
}
}
});
}
let path = save_api_key_to_config_file(trimmed)?;
@@ -11088,154 +11098,7 @@ fn user_global_config_api_key(provider: ApiProvider) -> Option<String> {
/// prompt for a key inline.
#[must_use]
pub fn has_api_key_for(config: &Config, provider: ApiProvider) -> bool {
let auth_mode = config.auth_mode_for_provider(provider);
if auth_mode_disables_api_key(auth_mode.as_deref()) {
return true;
}
if provider == config.api_provider()
&& !provider_uses_oauth_credentials(config, provider)
&& explicit_cli_api_key_override().is_some()
{
return true;
}
if provider_config_env_api_key(config, provider).is_some() {
return true;
}
if !config.should_skip_secret_store_for_provider(provider)
&& provider
.env_vars()
.iter()
.any(|var| std::env::var(var).is_ok_and(|k| !k.trim().is_empty()))
{
return true;
}
if provider == ApiProvider::Moonshot && provider_uses_oauth_credentials(config, provider) {
return false;
}
if provider == ApiProvider::OpenaiCodex && !config.provider_uses_custom_endpoint(provider) {
// Token env overrides are checked above. An external Codex login is
// considered only after exact read-only consent has been validated.
let path = crate::oauth::auth_file_path();
return config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::CodexCli,
&path,
)
.is_ok_and(|grant| crate::oauth::stored_credentials_present(&grant));
}
if provider == ApiProvider::Xai
&& !config.provider_uses_custom_endpoint(provider)
&& crate::xai_oauth::credentials_present(config)
{
// xAI supports both API keys and OAuth. A Grok-compatible token file is
// sufficient, but its absence must fall through to the ordinary API-key
// checks below instead of masking a configured key.
return true;
}
if provider == ApiProvider::Antigravity && !config.provider_uses_custom_endpoint(provider) {
let path = codewhale_config::default_agy_credentials_path();
if config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::AgyCli,
&path,
)
.is_ok_and(|grant| {
crate::agy_credentials::antigravity_oauth_token_from_grant(&grant)
.ok()
.flatten()
.is_some()
})
{
return true;
}
}
if matches!(
provider,
ApiProvider::Deepseek | ApiProvider::DeepseekAnthropic
) && !config.provider_uses_custom_endpoint(provider)
{
let path = codewhale_config::default_dsh_credentials_path();
if config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::DshCli,
&path,
)
.is_ok_and(|grant| {
crate::dsh_credentials::deepseek_api_key_from_grant(&grant)
.ok()
.flatten()
.is_some()
})
{
return true;
}
}
if !auth_mode_requires_api_key(auth_mode.as_deref())
&& (provider_route_is_keyless_self_hosted(provider, &config.base_url_for_route(provider))
|| (provider == config.api_provider()
&& base_url_uses_local_host(&config.deepseek_base_url())))
{
return true;
}
if config.config_credentials_are_bound_to_provider_endpoint(provider)
&& config
.provider_config_string_with_runtime_fallback(provider, |entry| entry.api_key.clone())
.is_some_and(|key| {
classify_config_api_key_value(&key) == ConfigApiKeyValueKind::Literal
})
{
return true;
}
// Probe the active provider, plus any provider whose persisted
// `[providers.<name>]` table carries the marker the secret-store save
// path itself writes (an api-key auth mode with no config literal). A
// configured-but-inactive provider must not render as unconfigured just
// because the operator switched providers after saving its key (#5033).
// Shared-slot families (one account, several provider variants — e.g.
// Model Studio Token/Coding Plan × OpenAI/Anthropic dialects) honor the
// marker written by ANY sibling variant, since the save path stores one
// key under the family's canonical slot. The probe stays bounded to
// explicitly configured providers, and the non-active case is strictly
// read-only so rendering the catalog never migrates a legacy store or
// opens a write-capable backend.
if !config.should_skip_secret_store_for_provider(provider) {
if provider == config.api_provider() {
if provider_secret_store_api_key(config, provider).is_some() {
return true;
}
} else if secret_slot_save_marker_on_shared_slot(config, provider)
&& provider_secret_store_api_key_with_mode(config, provider, true).is_some()
{
return true;
}
}
if (matches!(provider, ApiProvider::Deepseek | ApiProvider::DeepseekCN)
|| (provider == ApiProvider::Custom && config.uses_legacy_literal_custom_route()))
&& config.config_credentials_are_bound_to_provider_endpoint(provider)
&& config
.api_key
.as_ref()
.is_some_and(|key| classify_config_api_key_value(key) == ConfigApiKeyValueKind::Literal)
{
return true;
}
// Last resort: the user-global config file. A key saved there must not
// disappear just because this process loaded a workspace config.
if user_global_config_api_key(provider).is_some() {
return true;
}
false
credential_resolve::resolve_credential_source(config, provider).is_present()
}
impl Config {
@@ -11458,51 +11321,60 @@ fn save_api_key_for_identity_unlocked(
&& let Some(secrets) = credential_secret_store()
{
let secret_slot = provider_secret_store_slot(provider);
let prior_secret = secrets.get(secret_slot);
match prior_secret.as_ref() {
Ok(prior) => match secrets.set(secret_slot, api_key) {
Ok(()) => {
let config_result =
crate::config_persistence::mutate_config_document(&config_path, |doc| {
if pin_kimi_code_base_url {
// Snapshot -> write -> config-write -> rollback is a read-modify-write.
// Hold this provider's credential write lock across the whole sequence
// so a concurrent save or logout on the same slot cannot interleave and
// leave the secret store and the config document disagreeing. This is
// the `modify`-is-the-only-write-path rule ported from pi-mono; see
// `crate::credentials::store`.
return crate::credentials::store::with_provider_write_lock(secret_slot, || {
let prior_secret = secrets.get(secret_slot);
match prior_secret.as_ref() {
Ok(prior) => match secrets.set(secret_slot, api_key) {
Ok(()) => {
let config_result = crate::config_persistence::mutate_config_document(
&config_path,
|doc| {
if pin_kimi_code_base_url {
crate::config_persistence::set_document_value(
doc,
&["providers", key_inside, "base_url"],
DEFAULT_KIMI_CODE_BASE_URL,
)?;
}
crate::config_persistence::set_document_value(
doc,
&["providers", key_inside, "base_url"],
DEFAULT_KIMI_CODE_BASE_URL,
&["providers", key_inside, "auth_mode"],
"api_key",
)?;
}
crate::config_persistence::set_document_value(
doc,
&["providers", key_inside, "auth_mode"],
"api_key",
)?;
crate::config_persistence::unset_document_value(
doc,
&["providers", key_inside, "external_credentials"],
)?;
if provider == ApiProvider::Xai {
crate::config_persistence::unset_document_value(
doc,
&["providers", key_inside, "oauth_credential_generation"],
&["providers", key_inside, "external_credentials"],
)?;
}
crate::config_persistence::unset_document_value(
doc,
&["providers", key_inside, "api_key"],
)?;
Ok(())
})
if provider == ApiProvider::Xai {
crate::config_persistence::unset_document_value(
doc,
&["providers", key_inside, "oauth_credential_generation"],
)?;
}
crate::config_persistence::unset_document_value(
doc,
&["providers", key_inside, "api_key"],
)?;
Ok(())
},
)
.with_context(|| {
format!("Failed to write config to {}", config_path.display())
});
if let Err(error) = config_result {
let current = secrets.get(secret_slot).map_err(|rollback| {
if let Err(error) = config_result {
let current = secrets.get(secret_slot).map_err(|rollback| {
anyhow::anyhow!(
"{error}; additionally could not verify secret-store rollback for {secret_slot}: {rollback}"
)
})?;
if current.as_deref() == Some(api_key) {
match prior {
if current.as_deref() == Some(api_key) {
match prior {
Some(previous) => secrets.set(secret_slot, previous),
None => secrets.delete(secret_slot),
}
@@ -11511,41 +11383,44 @@ fn save_api_key_for_identity_unlocked(
"{error}; additionally failed to restore prior secret-store state for {secret_slot}: {rollback}"
)
})?;
}
return Err(error);
}
return Err(error);
codewhale_config::scrub_plaintext_api_keys_from_config_backup(
&config_path,
)?;
let backend = secrets.backend_name().to_string();
log_sensitive_event(
"credential.save",
json!({
"backend": backend.clone(),
"provider": identity.key,
"config_path": config_path.display().to_string(),
"plaintext_config_fallback": false,
}),
);
return Ok(SavedCredential::KeyringAndConfigFile {
backend,
path: config_path,
});
}
codewhale_config::scrub_plaintext_api_keys_from_config_backup(&config_path)?;
let backend = secrets.backend_name().to_string();
log_sensitive_event(
"credential.save",
json!({
"backend": backend.clone(),
"provider": identity.key,
"config_path": config_path.display().to_string(),
"plaintext_config_fallback": false,
}),
);
return Ok(SavedCredential::KeyringAndConfigFile {
backend,
path: config_path,
});
}
Err(err) => {
Err(err) => {
return Err(plaintext_credential_fallback_refused(
"write",
&config_path,
&err,
));
}
},
Err(error) => {
return Err(plaintext_credential_fallback_refused(
"write",
"snapshot",
&config_path,
&err,
&error,
));
}
},
Err(error) => {
return Err(plaintext_credential_fallback_refused(
"snapshot",
&config_path,
&error,
));
}
}
});
}
// Edit the `[providers.<name>]` table in place so unrelated sections,
@@ -11939,11 +11814,17 @@ fn provider_secret_store_api_key_with_mode(
} else {
codewhale_secrets::Secrets::auto_detect()
};
let primary = secrets
.get(provider_secret_store_slot(provider))
// Read through the credential-store trait so every read of a durable slot
// goes through one adapter (`crate::credentials::store`), and the value is
// carried as a type-tagged `Credential` rather than a bare String that can
// drift into a log line.
let store =
crate::credentials::store::SecretStoreCredentials::new(secrets, known_secret_store_slots());
let primary = store
.read(provider_secret_store_slot(provider))
.ok()
.flatten()
.filter(|value| !value.trim().is_empty());
.map(|credential| credential.expose_secret().to_string());
if primary.is_some() {
return primary;
}
@@ -11954,15 +11835,33 @@ fn provider_secret_store_api_key_with_mode(
// selection, and never write/copy/delete either slot while resolving.
(provider == ApiProvider::OllamaCloud && config.selects_legacy_ollama_cloud_route())
.then(|| {
secrets
.get(ApiProvider::Ollama.as_str())
store
.read(ApiProvider::Ollama.as_str())
.ok()
.flatten()
.filter(|value| !value.trim().is_empty())
.map(|credential| credential.expose_secret().to_string())
})
.flatten()
}
/// Every durable credential slot CodeWhale knows how to write.
///
/// The backing keyring exposes no key enumeration, so
/// [`crate::credentials::store::SecretStoreCredentials::list`] is given the
/// slot names to probe. Deduplicated because shared-account families collapse
/// several providers onto one slot.
fn known_secret_store_slots() -> Vec<String> {
let mut slots: Vec<String> = ApiProvider::all()
.iter()
.copied()
.chain(std::iter::once(ApiProvider::DeepseekCN))
.map(|provider| provider_secret_store_slot(provider).to_string())
.collect();
slots.sort();
slots.dedup();
slots
}
/// The shadowing warning for a config-file `api_key` that wins over a live
/// secret-store credential, if both exist (#5194).
///
@@ -12134,7 +12033,7 @@ fn clear_api_key_unlocked() -> Result<()> {
// even when the config file is absent: the slot survives independently
// of the file.
if let Some(secrets) = credential_secret_store() {
let failures = clear_all_provider_api_keys_from_secret_store(&secrets);
let failures = clear_all_provider_api_keys_from_secret_store(secrets);
if !failures.is_empty() {
anyhow::bail!(
"failed to delete stored credentials for: {}",
@@ -12155,25 +12054,26 @@ fn clear_api_key_unlocked() -> Result<()> {
/// the caller can fail loudly instead of claiming a clean logout while
/// credentials linger in the store (#5196).
fn clear_all_provider_api_keys_from_secret_store(
secrets: &codewhale_secrets::Secrets,
secrets: codewhale_secrets::Secrets,
) -> Vec<String> {
let mut failures = Vec::new();
let mut cleared_slots = std::collections::HashSet::new();
for provider in ApiProvider::all() {
let slot = provider_secret_store_slot(*provider);
if !cleared_slots.insert(slot) {
continue;
let store =
crate::credentials::store::SecretStoreCredentials::new(secrets, known_secret_store_slots());
// `list` enumerates the slots that actually hold something, without
// exposing any value — the deduplication that used to live here is now the
// slot table's job.
let stored: Vec<crate::credentials::CredentialInfo> = match store.list() {
Ok(stored) => stored,
Err(error) => {
failures.push(format!("secret store enumeration: {error}"));
return failures;
}
let has_value = secrets
.get(slot)
.ok()
.flatten()
.is_some_and(|value| !value.trim().is_empty());
if !has_value {
continue;
}
if let Err(error) = secrets.delete(slot) {
failures.push(format!("{slot}: {error}"));
};
for entry in stored {
// Delete goes through the store's own per-provider write lock, so a
// logout racing a save cannot delete the key that save just wrote.
if let Err(error) = store.delete(&entry.provider_id) {
failures.push(format!("{}: {error}", entry.provider_id));
}
}
failures
crates/tui/src/config/credential_resolve.rs
+478
View File
@@ -0,0 +1,478 @@
//! The one place provider credential precedence is decided.
//!
//! Ported from pi-mono `packages/ai/src/auth/resolve.ts` (MIT, Copyright (c)
//! 2025 Mario Zechner; full notice in `crate::credentials`). The idea taken is
//! pi's: a single resolver, one precedence rule stated in a doc comment beside
//! it, and a result that names the place it resolved from. The walk itself is
//! CodeWhale's — it is the former body of `has_api_key_for`, moved here
//! unchanged in order so no existing decision changes, with a
//! [`CredentialSource`] attached to each outcome.
//!
//! # Precedence rule
//!
//! **A stored credential owns the provider: ambient/env is consulted only when
//! nothing is stored. No silent env fallback after a failed refresh.**
//!
//! CodeWhale's order below is that rule instantiated over the stores it
//! actually has. Reading top to bottom:
//!
//! 1. `auth_mode = "none"` — the route sends no credential at all.
//! 2. An explicit `--api-key` on the active, non-OAuth provider.
//! 3. `[providers.<name>] api_key_env` — a credential the route *names*.
//! 4. An ambient provider environment variable (official endpoints only).
//! 5. Provider-owned login state: an explicitly consented external CLI
//! credential file (Codex, DeepSeek Harness, Antigravity) or CodeWhale's own
//! xAI OAuth storage.
//! 6. A keyless self-hosted / loopback route.
//! 7. `[providers.<name>] api_key` in the config file.
//! 8. CodeWhale's durable secret store.
//! 9. The root `api_key` compatibility slot.
//! 10. The user-global `~/.codewhale/config.toml`.
//!
//! Two departures from pi are deliberate and load-bearing here:
//!
//! * Ambient env outranks the secret store for a *named* binding (step 3) and
//! for official-endpoint provider variables (step 4). That is CodeWhale's
//! existing, documented behavior and users depend on it; changing it is not
//! in this lane's scope. It is stated here so it is at least *visible*.
//! * External CLI credential files are only ever consulted through
//! [`Config::external_credential_read_grant`], which enforces the read-only
//! consent model (exact path, explicit consent, never refreshed, never
//! rewritten). This resolver adds no new way to reach them.
//!
//! # Redaction
//!
//! This module never returns, logs, or renders secret material. It returns
//! only a [`CredentialSource`] label. Every probe that needs a value calls an
//! existing helper and discards the value with `.is_some()`.
use super::*;
use crate::credentials::{
AuthContext, CredentialProbe, CredentialResolution, CredentialSource,
context::ProcessAuthContext,
};
/// Resolve which place holds a credential for `provider`, using the real
/// process environment.
pub(crate) fn resolve_credential_source(
config: &Config,
provider: ApiProvider,
) -> CredentialResolution {
resolve_credential_source_with(config, provider, &ProcessAuthContext)
}
/// Resolve with an injected [`AuthContext`].
///
/// Only the ambient reads this function performs *itself* go through `ctx`.
/// The provider-specific helpers it delegates to (secret store, external
/// grants, xAI OAuth) still read the real environment and filesystem; making
/// those injectable means threading a context through config.rs and is not in
/// this lane.
pub(crate) fn resolve_credential_source_with(
config: &Config,
provider: ApiProvider,
ctx: &dyn AuthContext,
) -> CredentialResolution {
let mut probed: Vec<CredentialProbe> = Vec::new();
let auth_mode = config.auth_mode_for_provider(provider);
if auth_mode_disables_api_key(auth_mode.as_deref()) {
return CredentialResolution::found(CredentialSource::AuthModeNone);
}
if provider == config.api_provider()
&& !provider_uses_oauth_credentials(config, provider)
&& explicit_cli_api_key_override().is_some()
{
return CredentialResolution::found(CredentialSource::CliOverride);
}
if let Some(var) = bound_provider_api_key_env_name(config, provider) {
if provider_config_env_api_key(config, provider).is_some() {
return CredentialResolution::found(CredentialSource::ProviderConfigEnv { var });
}
probed.push(CredentialProbe::with_fix(
format!("env {var} (bound by api_key_env)"),
format!("export {var}=<key>"),
));
}
let skip_secret_store = config.should_skip_secret_store_for_provider(provider);
if !skip_secret_store {
if let Some(var) = provider
.env_vars()
.iter()
.find(|var| ctx.env(var).is_some())
{
return CredentialResolution::found(CredentialSource::AmbientEnv {
var: (*var).to_string(),
});
}
if let Some(var) = provider.env_vars().first() {
probed.push(CredentialProbe::with_fix(
format!("env {}", provider.env_vars_label()),
format!("export {var}=<key>"),
));
}
}
if provider == ApiProvider::Moonshot && provider_uses_oauth_credentials(config, provider) {
// Kimi CLI credentials are never imported; the route needs its own key.
probed.push(CredentialProbe::with_fix(
"Kimi CLI credentials (never imported)",
"codewhale auth set --provider moonshot",
));
return CredentialResolution::missing(probed);
}
if provider == ApiProvider::OpenaiCodex && !config.provider_uses_custom_endpoint(provider) {
// Token env overrides are checked above. An external Codex login is
// considered only after exact read-only consent has been validated.
let path = crate::oauth::auth_file_path();
let granted = config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::CodexCli,
&path,
)
.is_ok_and(|grant| crate::oauth::stored_credentials_present(&grant));
return if granted {
CredentialResolution::found(CredentialSource::ExternalGrant {
cli: "Codex CLI".to_string(),
path: path.display().to_string(),
})
} else {
probed.push(external_grant_probe(
"Codex CLI",
&path,
"codewhale auth external-consent --provider openai-codex --mode read-only",
ctx,
));
CredentialResolution::missing(probed)
};
}
if provider == ApiProvider::Xai
&& !config.provider_uses_custom_endpoint(provider)
&& crate::xai_oauth::credentials_present(config)
{
// xAI supports both API keys and OAuth. A Grok-compatible token file is
// sufficient, but its absence must fall through to the ordinary API-key
// checks below instead of masking a configured key.
return CredentialResolution::found(CredentialSource::OAuth {
flow: "xAI".to_string(),
});
}
if provider == ApiProvider::Antigravity && !config.provider_uses_custom_endpoint(provider) {
let path = codewhale_config::default_agy_credentials_path();
if config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::AgyCli,
&path,
)
.is_ok_and(|grant| {
crate::agy_credentials::antigravity_oauth_token_from_grant(&grant)
.ok()
.flatten()
.is_some()
})
{
return CredentialResolution::found(CredentialSource::ExternalGrant {
cli: "Antigravity CLI".to_string(),
path: path.display().to_string(),
});
}
probed.push(external_grant_probe(
"Antigravity CLI",
&path,
"codewhale auth external-consent --provider antigravity --mode read-only",
ctx,
));
}
if matches!(
provider,
ApiProvider::Deepseek | ApiProvider::DeepseekAnthropic
) && !config.provider_uses_custom_endpoint(provider)
{
let path = codewhale_config::default_dsh_credentials_path();
if config
.external_credential_read_grant(
provider,
codewhale_config::ExternalCredentialSource::DshCli,
&path,
)
.is_ok_and(|grant| {
crate::dsh_credentials::deepseek_api_key_from_grant(&grant)
.ok()
.flatten()
.is_some()
})
{
return CredentialResolution::found(CredentialSource::ExternalGrant {
cli: "DeepSeek Harness".to_string(),
path: path.display().to_string(),
});
}
probed.push(external_grant_probe(
"DeepSeek Harness",
&path,
"codewhale auth external-consent --provider deepseek --mode read-only",
ctx,
));
}
if !auth_mode_requires_api_key(auth_mode.as_deref())
&& (provider_route_is_keyless_self_hosted(provider, &config.base_url_for_route(provider))
|| (provider == config.api_provider()
&& base_url_uses_local_host(&config.deepseek_base_url())))
{
return CredentialResolution::found(CredentialSource::KeylessRoute {
base_url: config.base_url_for_route(provider),
});
}
if config.config_credentials_are_bound_to_provider_endpoint(provider) {
if config
.provider_config_string_with_runtime_fallback(provider, |entry| entry.api_key.clone())
.is_some_and(|key| {
classify_config_api_key_value(&key) == ConfigApiKeyValueKind::Literal
})
{
return CredentialResolution::found(CredentialSource::ProviderConfigApiKey {
table: provider_config_table_name(provider)
.unwrap_or_else(|_| format!("providers.{}", provider.as_str())),
});
}
if let Ok(table) = provider_config_table_name(provider) {
probed.push(CredentialProbe::with_fix(
format!("[{table}] api_key"),
format!("add api_key to [{table}] in ~/.codewhale/config.toml"),
));
}
}
// Probe the active provider, plus any provider whose persisted
// `[providers.<name>]` table carries the marker the secret-store save
// path itself writes (an api-key auth mode with no config literal). A
// configured-but-inactive provider must not render as unconfigured just
// because the operator switched providers after saving its key (#5033).
// Shared-slot families (one account, several provider variants — e.g.
// Model Studio Token/Coding Plan × OpenAI/Anthropic dialects) honor the
// marker written by ANY sibling variant, since the save path stores one
// key under the family's canonical slot. The probe stays bounded to
// explicitly configured providers, and the non-active case is strictly
// read-only so rendering the catalog never migrates a legacy store or
// opens a write-capable backend.
if !skip_secret_store {
let slot = provider_secret_store_slot(provider).to_string();
if provider == config.api_provider() {
if provider_secret_store_api_key(config, provider).is_some() {
return CredentialResolution::found(CredentialSource::SecretStore { slot });
}
probed.push(secret_store_probe(&slot, provider));
} else if secret_slot_save_marker_on_shared_slot(config, provider) {
if provider_secret_store_api_key_with_mode(config, provider, true).is_some() {
return CredentialResolution::found(CredentialSource::SecretStore { slot });
}
probed.push(secret_store_probe(&slot, provider));
} else {
// #5033's marker gate: without a `[providers.<name>]` api-key
// auth-mode marker the store is not read at all for an inactive
// provider. Say so, because the row is otherwise indistinguishable
// from a genuinely empty slot — and the request path *would* read
// it once this provider became active.
probed.push(CredentialProbe::with_fix(
format!(
"secret store \"{slot}\" (not read: inactive provider, no api-key marker)"
),
format!(
"codewhale auth set --provider {} writes the marker that makes this slot readable while inactive",
provider.as_str()
),
));
}
}
if (matches!(provider, ApiProvider::Deepseek | ApiProvider::DeepseekCN)
|| (provider == ApiProvider::Custom && config.uses_legacy_literal_custom_route()))
&& config.config_credentials_are_bound_to_provider_endpoint(provider)
&& config
.api_key
.as_ref()
.is_some_and(|key| classify_config_api_key_value(key) == ConfigApiKeyValueKind::Literal)
{
return CredentialResolution::found(CredentialSource::RootConfigApiKey);
}
// Last resort: the user-global config file. A key saved there must not
// disappear just because this process loaded a workspace config.
if user_global_config_api_key(provider).is_some() {
return CredentialResolution::found(CredentialSource::UserGlobalConfig);
}
probed.push(CredentialProbe::with_fix(
"~/.codewhale/config.toml",
format!("codewhale auth set --provider {}", provider.as_str()),
));
CredentialResolution::missing(probed)
}
fn secret_store_probe(slot: &str, provider: ApiProvider) -> CredentialProbe {
CredentialProbe::with_fix(
format!("secret store \"{slot}\""),
format!("codewhale auth set --provider {}", provider.as_str()),
)
}
fn external_grant_probe(
cli: &str,
path: &std::path::Path,
consent_command: &str,
ctx: &dyn AuthContext,
) -> CredentialProbe {
let exists = ctx.file_exists(path);
let place = if exists {
format!(
"{cli} credentials at {} (present, not consented)",
path.display()
)
} else {
format!("{cli} credentials at {} (absent)", path.display())
};
if exists {
CredentialProbe::with_fix(place, consent_command.to_string())
} else {
CredentialProbe::new(place)
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::credentials::context::MapAuthContext;
use crate::test_support::{EnvVarGuard, lock_test_env};
fn deepseek_config() -> Config {
let mut config = Config::default();
config.provider = Some("deepseek".to_string());
config
}
/// The precedence rule has to be enforced somewhere a test can see it.
#[test]
fn a_named_env_binding_resolves_and_names_itself() {
let _lock = lock_test_env();
let _key = EnvVarGuard::set("CW_TEST_BOUND_KEY", "bound-value");
let mut config = Config::default();
config.provider = Some("openrouter".to_string());
config.providers = Some(
toml::from_str("[openrouter]\napi_key_env = \"CW_TEST_BOUND_KEY\"\n")
.expect("provider table"),
);
let resolution = resolve_credential_source(&config, ApiProvider::Openrouter);
assert_eq!(
resolution.source,
CredentialSource::ProviderConfigEnv {
var: "CW_TEST_BOUND_KEY".to_string()
},
"a route that names its variable must resolve from it and say so"
);
assert_eq!(resolution.source.label(), "api_key_env CW_TEST_BOUND_KEY");
}
/// An ambient export must name the exact variable that won, not just
/// "configured" — this is pi's `source: "ANTHROPIC_API_KEY"`.
#[test]
fn ambient_env_names_the_variable_that_won() {
let _lock = lock_test_env();
let ctx = MapAuthContext::new().with_env("OPENROUTER_API_KEY", "value");
let config = Config::default();
let resolution = resolve_credential_source_with(&config, ApiProvider::Openrouter, &ctx);
assert_eq!(
resolution.source,
CredentialSource::AmbientEnv {
var: "OPENROUTER_API_KEY".to_string()
}
);
assert_eq!(resolution.source.label(), "OPENROUTER_API_KEY");
}
/// The regression this whole lane exists for: a provider with no
/// credential anywhere used to report a bare boolean. It must now name
/// every place that was probed, in precedence order, and offer a fix.
#[test]
fn a_missing_credential_names_every_place_that_was_checked() {
let _lock = lock_test_env();
let ctx = MapAuthContext::new();
let config = Config::default();
let resolution = resolve_credential_source_with(&config, ApiProvider::Openrouter, &ctx);
assert!(!resolution.is_present());
let checked = resolution.checked_places();
assert!(
checked.contains("OPENROUTER_API_KEY"),
"the ambient variable must be named: {checked}"
);
assert!(
checked.contains("secret store \"openrouter\""),
"the durable slot must be named: {checked}"
);
assert!(
checked.contains("~/.codewhale/config.toml"),
"the user-global config must be named: {checked}"
);
assert_eq!(
resolution.first_fix(),
Some("export OPENROUTER_API_KEY=<key>"),
"the first probed place must carry the command that fixes it"
);
}
/// #5033's marker gate is a real asymmetry between what the picker
/// reports and what the request path would find: for a provider that is
/// not active and whose config table carries no api-key marker, the
/// durable slot is *not read at all*. That is defensible, but it must be
/// visible — a user staring at "missing key" has to be told the slot was
/// skipped rather than found empty.
#[test]
fn an_unread_secret_slot_says_it_was_not_read_and_why() {
let _lock = lock_test_env();
let ctx = MapAuthContext::new();
let config = deepseek_config();
let resolution = resolve_credential_source_with(&config, ApiProvider::Openrouter, &ctx);
let checked = resolution.checked_places();
assert!(
checked.contains("(not read: inactive provider, no api-key marker)"),
"an unread slot must not look like an empty one: {checked}"
);
}
/// `auth_mode = "none"` is a resolution, not an absence.
#[test]
fn no_auth_routes_resolve_to_the_auth_mode_itself() {
let _lock = lock_test_env();
let mut config = Config::default();
config.providers =
Some(toml::from_str("[openrouter]\nauth_mode = \"none\"\n").expect("provider table"));
let resolution = resolve_credential_source(&config, ApiProvider::Openrouter);
assert_eq!(resolution.source, CredentialSource::AuthModeNone);
assert!(resolution.is_present());
assert!(resolution.checked_places().is_empty());
}
/// The resolver is the sole authority; `has_api_key_for` must agree with
/// it for every provider, or two surfaces can disagree again.
#[test]
fn has_api_key_for_agrees_with_the_resolver_for_every_provider() {
let _lock = lock_test_env();
let config = Config::default();
for provider in ApiProvider::all() {
let resolution = resolve_credential_source(&config, *provider);
assert_eq!(
has_api_key_for(&config, *provider),
resolution.is_present(),
"{provider:?} disagreed: {:?}",
resolution.source
);
}
}
}
crates/tui/src/config/tests.rs
+80
View File
@@ -12925,3 +12925,83 @@ fn native_memory_path_honours_an_already_native_setting() {
std::path::PathBuf::from("/tmp/cw-test/memory/global/MEMORY.md")
);
}
/// Reproduces the report that started the credential-resolution lane: a home
/// whose secret store holds a working DeepSeek key, where the provider picker
/// reads "missing key" while a real turn from the same home resolves that key.
///
/// The asymmetry is #5033's marker gate. For a provider that is not currently
/// active, `has_api_key_for` reads the durable slot only when some sibling's
/// `[providers.<name>]` table carries the api-key auth-mode marker. The request
/// path has no such gate — it reads the slot for whatever provider is active.
/// Any home where the marker is absent but the slot is populated (a config
/// written by an older CodeWhale, a workspace config loaded in place of the
/// user-global one, a key written straight into the store) makes the two
/// surfaces contradict each other, and nothing in the old picker output let a
/// user tell which one was lying.
///
/// This test does not change that policy — the gate is what keeps catalog
/// rendering from opening a write-capable keyring for 40 providers. It pins
/// the contradiction, and pins the part that is now fixed: the row says the
/// slot was skipped, and why.
#[test]
fn picker_and_request_path_disagree_when_the_secret_slot_marker_is_missing() -> Result<()> {
use crate::credentials::CredentialSource;
let _lock = lock_test_env();
let temp_root = tempfile::tempdir()?;
let temp_root = temp_root.path().canonicalize()?;
let _guard = EnvGuard::new(&temp_root);
let codewhale_home = temp_root.join("codewhale-home");
let config_path = codewhale_home.join("config.toml");
let _home = EnvVarGuard::set("CODEWHALE_HOME", codewhale_home.as_os_str());
let _path = EnvVarGuard::set("CODEWHALE_CONFIG_PATH", config_path.as_os_str());
let _backend = EnvVarGuard::set("CODEWHALE_SECRET_BACKEND", "file");
save_api_key("deepseek-working-key")?;
assert_eq!(
codewhale_secrets::Secrets::auto_detect().get("deepseek")?,
Some("deepseek-working-key".to_string()),
"precondition: the durable slot holds a usable key"
);
// Drop the marker the save wrote, standing in for a home whose config was
// written by an older CodeWhale or replaced by a workspace-scoped file.
fs::write(&config_path, "provider = \"openrouter\"\n")?;
let mut active_deepseek = Config::default();
active_deepseek.provider = Some("deepseek".to_string());
assert_eq!(
active_deepseek.deepseek_api_key().ok(),
Some("deepseek-working-key".to_string()),
"the request path still resolves the stored key"
);
let mut viewing_config = Config::default();
viewing_config.provider = Some("openrouter".to_string());
assert!(
!has_api_key_for(&viewing_config, ApiProvider::Deepseek),
"the reported contradiction: readiness reports no key for the same home"
);
let resolution = resolve_credential_source(&viewing_config, ApiProvider::Deepseek);
assert!(matches!(
resolution.source,
CredentialSource::Missing { .. }
));
let checked = resolution.checked_places();
assert!(
checked
.contains("secret store \"deepseek\" (not read: inactive provider, no api-key marker)"),
"the row must explain the skip instead of implying an empty slot: {checked}"
);
assert!(
resolution.source.probed().iter().any(|probe| probe
.fix
.as_deref()
.is_some_and(|fix| fix.contains("codewhale auth set"))),
"the row must offer the command that resolves it: {resolution:?}"
);
println!("CAPTURED checked-places: {checked}");
println!("CAPTURED first-fix: {:?}", resolution.first_fix());
Ok(())
}