diff --git a/crates/goose/src/config/declarative_providers.rs b/crates/goose/src/config/declarative_providers.rs index a4bb1d35a7..b02da9f20b 100644 --- a/crates/goose/src/config/declarative_providers.rs +++ b/crates/goose/src/config/declarative_providers.rs @@ -674,7 +674,7 @@ mod tests { assert_eq!(config.name, "llama_swap"); assert_eq!(config.display_name, "Llama Swap"); assert!(matches!(config.engine, ProviderEngine::OpenAI)); - assert_eq!(config.api_key_env, ""); + assert_eq!(config.api_key_env, "LLAMA_SWAP_API_KEY"); assert!(!config.requires_auth); assert!(config.skip_canonical_filtering); assert_eq!(config.dynamic_models, Some(true)); diff --git a/crates/goose/src/providers/declarative/llama_swap.json b/crates/goose/src/providers/declarative/llama_swap.json index 3e66a01603..770d2c6a7e 100644 --- a/crates/goose/src/providers/declarative/llama_swap.json +++ b/crates/goose/src/providers/declarative/llama_swap.json @@ -3,7 +3,7 @@ "engine": "openai", "display_name": "Llama Swap", "description": "Local proxy that hot-swaps llama.cpp (and other) inference backends on demand via an OpenAI-compatible API.", - "api_key_env": "", + "api_key_env": "LLAMA_SWAP_API_KEY", "base_url": "${LLAMA_SWAP_HOST}/v1/chat/completions", "env_vars": [ { diff --git a/crates/goose/src/providers/declarative/lmstudio.json b/crates/goose/src/providers/declarative/lmstudio.json index 0ecbeef865..71dc4b8039 100644 --- a/crates/goose/src/providers/declarative/lmstudio.json +++ b/crates/goose/src/providers/declarative/lmstudio.json @@ -3,7 +3,7 @@ "engine": "openai", "display_name": "LM Studio", "description": "Run local models with LM Studio", - "api_key_env": "", + "api_key_env": "LMSTUDIO_API_KEY", "base_url": "${LMSTUDIO_HOST}/v1/chat/completions", "env_vars": [ { diff --git a/crates/goose/src/providers/declarative/omlx.json b/crates/goose/src/providers/declarative/omlx.json index ec4cc14d20..020b2f4ce7 100644 --- a/crates/goose/src/providers/declarative/omlx.json +++ b/crates/goose/src/providers/declarative/omlx.json @@ -3,7 +3,7 @@ "engine": "openai", "display_name": "oMLX", "description": "Run local models with oMLX", - "api_key_env": "", + "api_key_env": "OMLX_API_KEY", "base_url": "${OMLX_HOST}/v1/chat/completions", "env_vars": [ { diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index 67d496e5bd..3b76a59c4f 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -810,7 +810,7 @@ pub fn declarative_inventory_identity( .public_inputs .insert("headers".to_string(), serialize_string_map(headers)?); } - if config.requires_auth && !config.api_key_env.is_empty() { + if !config.api_key_env.is_empty() { if let Some(value) = config_secret_value(global, &config.api_key_env) { identity .secret_inputs diff --git a/crates/goose/src/providers/openai.rs b/crates/goose/src/providers/openai.rs index fe12aaa8e4..b2914ec554 100644 --- a/crates/goose/src/providers/openai.rs +++ b/crates/goose/src/providers/openai.rs @@ -298,6 +298,53 @@ impl OpenAiProvider { } } + /// Resolve the API key from a declarative provider config. + /// + /// Returns `Some(key)` if a key is found, `None` if the key is optional/missing, + /// or an error if the key is required but missing/unreadable. + /// + /// The `get_secret` closure is used to look up the secret by key name. This allows + /// testing without depending on `Config::global()`. + pub fn resolve_api_key( + config: &DeclarativeProviderConfig, + get_secret: &dyn Fn(&str) -> Result, + ) -> Result> { + if config.api_key_env.is_empty() { + return Ok(None); + } + + match get_secret(&config.api_key_env) { + Ok(key) => Ok(Some(key)), + Err(e) => { + use crate::config::ConfigError; + match e { + ConfigError::NotFound(_) => { + if config.requires_auth { + anyhow::bail!( + "Required API key {} is not set. Configure it via `goose configure` or set the {} environment variable.", + config.api_key_env, + config.api_key_env + ); + } + Ok(None) + } + other => { + if config.requires_auth { + anyhow::bail!("Failed to read {}: {}", config.api_key_env, other); + } else { + tracing::warn!( + "Failed to read optional API key {}: {}. Proceeding without authentication.", + config.api_key_env, + other + ); + Ok(None) + } + } + } + } + } + } + pub fn from_custom_config( model: ModelConfig, config: DeclarativeProviderConfig, @@ -323,22 +370,7 @@ impl OpenAiProvider { } let global_config = crate::config::Config::global(); - - let api_key: Option = if config.requires_auth && !config.api_key_env.is_empty() { - Some(global_config.get_secret::(&config.api_key_env).map_err(|e| { - use crate::config::ConfigError; - match e { - ConfigError::NotFound(_) => anyhow::anyhow!( - "Required API key {} is not set. Configure it via `goose configure` or set the {} environment variable.", - config.api_key_env, - config.api_key_env - ), - other => anyhow::anyhow!("Failed to read {}: {}", config.api_key_env, other), - } - })?) - } else { - None - }; + let api_key = Self::resolve_api_key(&config, &|key| global_config.get_secret(key))?; let url = url::Url::parse(&config.base_url) .map_err(|e| anyhow::anyhow!("Invalid base URL '{}': {}", config.base_url, e))?; @@ -1225,4 +1257,83 @@ mod tests { "error message should mention dynamic_models: false; got: {msg}" ); } + + // ── resolve_api_key tests ────────────────────────────────────────────── + + fn config_with_key(api_key_env: &str, requires_auth: bool) -> DeclarativeProviderConfig { + let mut config = base_declarative_config(vec![], None); + config.api_key_env = api_key_env.to_string(); + config.requires_auth = requires_auth; + config + } + + #[test] + fn resolve_api_key_empty_env_returns_none() { + let config = config_with_key("", true); + assert_eq!( + OpenAiProvider::resolve_api_key(&config, &|_| unreachable!()).unwrap(), + None + ); + } + + #[test] + fn resolve_api_key_missing_with_requires_auth_bails() { + let config = config_with_key("MY_KEY", true); + let err = OpenAiProvider::resolve_api_key(&config, &|_| { + Err(crate::config::ConfigError::NotFound("x".into())) + }) + .unwrap_err() + .to_string(); + assert!( + err.contains("MY_KEY"), + "error should mention the key name; got: {err}" + ); + } + + #[test] + fn resolve_api_key_missing_without_requires_auth_returns_none() { + let config = config_with_key("MY_KEY", false); + assert_eq!( + OpenAiProvider::resolve_api_key(&config, &|_| Err( + crate::config::ConfigError::NotFound("x".into()) + )) + .unwrap(), + None + ); + } + + #[test] + fn resolve_api_key_present_returns_value() { + let config = config_with_key("MY_KEY", true); + assert_eq!( + OpenAiProvider::resolve_api_key(&config, &|_| Ok("secret".into())).unwrap(), + Some("secret".to_string()) + ); + } + + #[test] + fn resolve_api_key_other_error_bails_when_required() { + let config = config_with_key("MY_KEY", true); + let err = OpenAiProvider::resolve_api_key(&config, &|_| { + Err(crate::config::ConfigError::KeyringError("ring fail".into())) + }) + .unwrap_err() + .to_string(); + assert!( + err.contains("MY_KEY"), + "error should mention the key name; got: {err}" + ); + } + + #[test] + fn resolve_api_key_other_error_warns_and_returns_none_when_optional() { + let config = config_with_key("MY_KEY", false); + assert_eq!( + OpenAiProvider::resolve_api_key(&config, &|_| Err( + crate::config::ConfigError::KeyringError("ring fail".into()) + )) + .unwrap(), + None + ); + } } diff --git a/crates/goose/src/providers/provider_registry.rs b/crates/goose/src/providers/provider_registry.rs index a534057e2f..7e2c14ce1f 100644 --- a/crates/goose/src/providers/provider_registry.rs +++ b/crates/goose/src/providers/provider_registry.rs @@ -186,8 +186,14 @@ impl ProviderRegistry { .collect(); let mut config_keys = if provider_type == ProviderType::Declarative { - if config.requires_auth && !config.api_key_env.is_empty() { - vec![ConfigKey::new(&config.api_key_env, true, true, None, true)] + if !config.api_key_env.is_empty() { + vec![ConfigKey::new( + &config.api_key_env, + config.requires_auth, + true, + None, + true, + )] } else { Vec::new() }