Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/goose/src/config/declarative_providers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,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));
Expand Down
2 changes: 1 addition & 1 deletion crates/goose/src/providers/declarative/llama_swap.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
2 changes: 1 addition & 1 deletion crates/goose/src/providers/declarative/lmstudio.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
2 changes: 1 addition & 1 deletion crates/goose/src/providers/declarative/omlx.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
2 changes: 1 addition & 1 deletion crates/goose/src/providers/inventory/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
160 changes: 144 additions & 16 deletions crates/goose/src/providers/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,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<String, crate::config::ConfigError>,
) -> Result<Option<String>> {
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,
Expand All @@ -318,22 +365,7 @@ impl OpenAiProvider {
}

let global_config = crate::config::Config::global();

let api_key: Option<String> = if config.requires_auth && !config.api_key_env.is_empty() {
Some(global_config.get_secret::<String>(&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))?;
Expand Down Expand Up @@ -1213,4 +1245,100 @@ 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 {
DeclarativeProviderConfig {
name: "custom_test".to_string(),
engine: ProviderEngine::OpenAI,
display_name: "Custom Test".to_string(),
description: None,
api_key_env: api_key_env.to_string(),
base_url: "http://localhost:1".to_string(),
models: vec![],
headers: None,
timeout_seconds: None,
supports_streaming: Some(true),
requires_auth,
catalog_provider_id: None,
base_path: None,
env_vars: None,
dynamic_models: None,
skip_canonical_filtering: false,
model_doc_link: None,
setup_steps: vec![],
fast_model: None,
}
}

#[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
);
}
}
10 changes: 8 additions & 2 deletions crates/goose/src/providers/provider_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,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()
}
Expand Down