From 960a199cd2e1cf8d01a0674de84a991899ce1d9d Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Tue, 2 Jun 2026 14:16:45 -0600 Subject: [PATCH] feat: refactored gman usage to be generic and work with various vault providers and use the SupportedProvider enum directly for configurations --- src/config/app_config.rs | 80 ++++++++++++++++++++++++++ src/config/install_remote.rs | 2 +- src/config/mod.rs | 103 +++++++++++++++++++++++++++++++++- src/config/request_context.rs | 48 ++++++++++++++-- src/mcp/mod.rs | 2 +- src/vault/mod.rs | 22 +++----- src/vault/utils.rs | 33 ++++++++--- 7 files changed, 261 insertions(+), 29 deletions(-) diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 057f82f..2eb9b37 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -4,6 +4,7 @@ use crate::utils::{IS_STDOUT_TERMINAL, NO_COLOR, decode_bin, get_env_name}; use super::paths; use anyhow::{Context, Result, anyhow}; +use gman::providers::SupportedProvider; use indexmap::IndexMap; use serde::Deserialize; use std::collections::HashMap; @@ -29,6 +30,7 @@ pub struct AppConfig { pub wrap: Option, pub wrap_code: bool, pub(crate) vault_password_file: Option, + pub(crate) secrets_provider: SupportedProvider, pub function_calling_support: bool, pub mapping_tools: IndexMap, @@ -94,6 +96,7 @@ impl Default for AppConfig { wrap: None, wrap_code: false, vault_password_file: None, + secrets_provider: SupportedProvider::default(), function_calling_support: true, mapping_tools: Default::default(), @@ -160,6 +163,7 @@ impl AppConfig { wrap: config.wrap, wrap_code: config.wrap_code, vault_password_file: config.vault_password_file, + secrets_provider: config.secrets_provider, function_calling_support: config.function_calling_support, mapping_tools: config.mapping_tools, @@ -208,6 +212,7 @@ impl AppConfig { clients: config.clients, }; + app_config.migrate_legacy_password_file(); app_config.load_envs(); if let Some(wrap) = app_config.wrap.clone() { app_config.set_wrap(&wrap)?; @@ -218,6 +223,17 @@ impl AppConfig { Ok(app_config) } + fn migrate_legacy_password_file(&mut self) { + let Some(legacy) = self.vault_password_file.take() else { + return; + }; + if let SupportedProvider::Local { provider_def } = &mut self.secrets_provider + && provider_def.password_file.is_none() + { + provider_def.password_file = Some(legacy); + } + } + pub fn resolve_model(&mut self) -> Result<()> { if self.model_id.is_empty() { let models = list_models(self, crate::client::ModelType::Chat); @@ -772,4 +788,68 @@ mod tests { app.resolve_model().unwrap(); assert_eq!(app.model_id, "provider:explicit"); } + + #[test] + fn migrate_legacy_copies_into_empty_local_provider() { + let mut app = AppConfig { + vault_password_file: Some(PathBuf::from("/tmp/test-coyote-password")), + ..AppConfig::default() + }; + + app.migrate_legacy_password_file(); + + match &app.secrets_provider { + SupportedProvider::Local { provider_def } => { + assert_eq!( + provider_def.password_file, + Some(PathBuf::from("/tmp/test-coyote-password")) + ); + } + _ => panic!("expected Local provider"), + } + assert!(app.vault_password_file.is_none()); + } + + #[test] + fn migrate_legacy_does_not_overwrite_existing_password_file() { + let mut app = AppConfig { + vault_password_file: Some(PathBuf::from("/tmp/legacy")), + ..AppConfig::default() + }; + if let SupportedProvider::Local { provider_def } = &mut app.secrets_provider { + provider_def.password_file = Some(PathBuf::from("/tmp/explicit")); + } + + app.migrate_legacy_password_file(); + + match &app.secrets_provider { + SupportedProvider::Local { provider_def } => { + assert_eq!( + provider_def.password_file, + Some(PathBuf::from("/tmp/explicit")) + ); + } + _ => panic!("expected Local provider"), + } + assert!(app.vault_password_file.is_none()); + } + + #[test] + fn migrate_legacy_noop_for_non_local_provider() { + let mut app = AppConfig { + vault_password_file: Some(PathBuf::from("/tmp/orphaned")), + secrets_provider: SupportedProvider::Gopass { + provider_def: Default::default(), + }, + ..AppConfig::default() + }; + + app.migrate_legacy_password_file(); + + assert!(app.vault_password_file.is_none()); + assert!(matches!( + app.secrets_provider, + SupportedProvider::Gopass { .. } + )); + } } diff --git a/src/config/install_remote.rs b/src/config/install_remote.rs index 50a071b..f07f04e 100644 --- a/src/config/install_remote.rs +++ b/src/config/install_remote.rs @@ -732,7 +732,7 @@ fn merge_mcp_json( write_atomically(&final_path, &serialized)?; let vault = Vault::init_bare(); - let (_parsed, missing) = interpolate_secrets(&serialized, &vault); + let (_parsed, missing) = interpolate_secrets(&serialized, &vault)?; let mut deduped: Vec = Vec::new(); for s in missing { if !deduped.contains(&s) { diff --git a/src/config/mod.rs b/src/config/mod.rs index 1d603a5..db734e2 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -53,6 +53,7 @@ use crate::config::macros::Macro; use crate::vault::{GlobalVault, Vault, create_vault_password_file, interpolate_secrets}; use anyhow::{Context, Result, anyhow, bail}; use fancy_regex::Regex; +use gman::providers::SupportedProvider; use indexmap::IndexMap; use indoc::formatdoc; use inquire::{Confirm, Select}; @@ -76,6 +77,45 @@ pub const TEMP_SESSION_NAME: &str = "temp"; static PASSWORD_FILE_SECRET_RE: LazyLock = LazyLock::new(|| Regex::new(r#"vault_password_file:.*['|"]?\{\{(.+)}}['|"]?"#).unwrap()); +fn validate_no_template_in_secrets_provider(content: &str) -> Result<()> { + let mut in_block = false; + + for (line_num, line) in content.lines().enumerate() { + if line.starts_with("secrets_provider:") { + if line.contains("{{") { + bail!( + "secret injection cannot be done on the secrets_provider property (line {}): the secrets_provider config is loaded before the vault is initialized", + line_num + 1 + ); + } + in_block = true; + continue; + } + + if in_block { + let trimmed = line.trim_start(); + + if trimmed.is_empty() || trimmed.starts_with('#') { + continue; + } + + if !line.starts_with(char::is_whitespace) { + in_block = false; + continue; + } + + if line.contains("{{") { + bail!( + "secret injection cannot be done within the secrets_provider block (line {}): the secrets_provider config is loaded before the vault is initialized", + line_num + 1 + ); + } + } + } + + Ok(()) +} + /// Monokai Extended const DARK_THEME: &[u8] = include_bytes!("../../assets/monokai-extended.theme.bin"); const LIGHT_THEME: &[u8] = include_bytes!("../../assets/monokai-extended-light.theme.bin"); @@ -149,6 +189,9 @@ pub struct Config { pub wrap_code: bool, pub(super) vault_password_file: Option, + #[serde(default)] + pub(super) secrets_provider: SupportedProvider, + pub function_calling_support: bool, pub mapping_tools: IndexMap, pub enabled_tools: Option, @@ -213,6 +256,7 @@ impl Default for Config { wrap: None, wrap_code: false, vault_password_file: None, + secrets_provider: SupportedProvider::default(), function_calling_support: true, mapping_tools: Default::default(), @@ -441,7 +485,7 @@ impl Config { ..AppConfig::default() }; let vault = Vault::init(&bootstrap_app); - let (parsed_config, missing_secrets) = interpolate_secrets(&content, &vault); + let (parsed_config, missing_secrets) = interpolate_secrets(&content, &vault)?; if !missing_secrets.is_empty() && !info_flag { debug!( "Global config references secrets that are missing from the vault: {missing_secrets:?}" @@ -480,6 +524,7 @@ impl Config { if PASSWORD_FILE_SECRET_RE.is_match(content)? { bail!("secret injection cannot be done on the vault_password_file property"); } + validate_no_template_in_secrets_provider(content)?; let config: Self = serde_yaml::from_str(content) .map_err(|err| { @@ -753,6 +798,62 @@ where mod tests { use super::*; + #[test] + fn validate_secrets_provider_rejects_template_in_field() { + let yaml = "\ +secrets_provider: + type: aws_secrets_manager + aws_profile: '{{AWS_PROFILE}}' + aws_region: us-east-1 +"; + assert!(validate_no_template_in_secrets_provider(yaml).is_err()); + } + + #[test] + fn validate_secrets_provider_rejects_template_in_local_password_file() { + let yaml = "\ +secrets_provider: + type: local + password_file: '{{COYOTE_PASSWORD}}' +"; + assert!(validate_no_template_in_secrets_provider(yaml).is_err()); + } + + #[test] + fn validate_secrets_provider_accepts_clean_yaml() { + let yaml = "\ +secrets_provider: + type: aws_secrets_manager + aws_profile: default + aws_region: us-east-1 +"; + assert!(validate_no_template_in_secrets_provider(yaml).is_ok()); + } + + #[test] + fn validate_secrets_provider_allows_templates_outside_block() { + let yaml = "\ +secrets_provider: + type: local + password_file: ~/.coyote_password +clients: + - type: openai + api_key: '{{OPENAI_KEY}}' +"; + assert!(validate_no_template_in_secrets_provider(yaml).is_ok()); + } + + #[test] + fn validate_secrets_provider_handles_missing_block() { + let yaml = "\ +model: openai:gpt-4 +clients: + - type: openai + api_key: '{{OPENAI_KEY}}' +"; + assert!(validate_no_template_in_secrets_provider(yaml).is_ok()); + } + #[test] fn config_defaults_match_expected() { let cfg = Config::default(); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 2f28402..dab3939 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -32,6 +32,7 @@ use crate::utils::{ use crate::graph; use anyhow::{Context, Error, Result, bail}; +use gman::providers::SupportedProvider; #[cfg(test)] use indexmap::IndexMap; use indoc::formatdoc; @@ -904,11 +905,50 @@ impl RequestContext { ("macros_dir", display_path(&paths::macros_dir())), ("functions_dir", display_path(&paths::functions_dir())), ("messages_file", display_path(&self.messages_file())), - ( - "vault_password_file", - display_path(&app.vault_password_file()), - ), ]; + + items.push(("secrets_provider", app.secrets_provider.to_string())); + match &app.secrets_provider { + SupportedProvider::Local { provider_def } => { + let path = provider_def + .password_file + .clone() + .unwrap_or_else(gman::config::Config::local_provider_password_file); + items.push(("vault_password_file", display_path(&path))); + } + SupportedProvider::AwsSecretsManager { provider_def } => { + if let Some(p) = &provider_def.aws_profile { + items.push(("aws_profile", p.clone())); + } + if let Some(r) = &provider_def.aws_region { + items.push(("aws_region", r.clone())); + } + } + SupportedProvider::GcpSecretManager { provider_def } => { + if let Some(id) = &provider_def.gcp_project_id { + items.push(("gcp_project_id", id.clone())); + } + } + SupportedProvider::AzureKeyVault { provider_def } => { + if let Some(n) = &provider_def.vault_name { + items.push(("azure_vault_name", n.clone())); + } + } + SupportedProvider::Gopass { provider_def } => { + if let Some(s) = &provider_def.store { + items.push(("gopass_store", s.clone())); + } + } + SupportedProvider::OnePassword { provider_def } => { + if let Some(v) = &provider_def.vault { + items.push(("op_vault", v.clone())); + } + if let Some(a) = &provider_def.account { + items.push(("op_account", a.clone())); + } + } + } + if let Ok((_, Some(log_path))) = paths::log_config() { items.push(("log_path", display_path(&log_path))); } diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 61e956a..2528a52 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -182,7 +182,7 @@ impl McpRegistry { return Ok(registry); } - let (parsed_content, missing_secrets) = interpolate_secrets(&content, vault); + let (parsed_content, missing_secrets) = interpolate_secrets(&content, vault)?; if !missing_secrets.is_empty() { return Err(anyhow!(formatdoc!( diff --git a/src/vault/mod.rs b/src/vault/mod.rs index 7a0c041..f8a4864 100644 --- a/src/vault/mod.rs +++ b/src/vault/mod.rs @@ -42,21 +42,17 @@ impl Vault { } pub fn init(config: &AppConfig) -> Self { - let vault_password_file = config.vault_password_file(); - let mut local_provider = LocalProvider { - password_file: Some(vault_password_file), - git_branch: None, - ..LocalProvider::default() - }; + let mut provider = config.secrets_provider.clone(); - ensure_password_file_initialized(&mut local_provider) - .expect("Failed to initialize password file"); - - Self { - provider: SupportedProvider::Local { - provider_def: local_provider, - }, + if let SupportedProvider::Local { provider_def } = &mut provider { + if provider_def.password_file.is_none() { + provider_def.password_file = Some(config.vault_password_file()); + } + ensure_password_file_initialized(provider_def) + .expect("Failed to initialize password file"); } + + Self { provider } } pub fn local_password_file(&self) -> Result { diff --git a/src/vault/utils.rs b/src/vault/utils.rs index 48626f1..ad2fdf3 100644 --- a/src/vault/utils.rs +++ b/src/vault/utils.rs @@ -8,6 +8,7 @@ use indoc::formatdoc; use inquire::validator::Validation; use inquire::{Confirm, Password, PasswordDisplayMode, Text, min_length, required}; use std::path::PathBuf; +use gman::SecretError; pub fn ensure_password_file_initialized(local_provider: &mut LocalProvider) -> Result<()> { let vault_password_file = local_provider @@ -172,24 +173,34 @@ pub fn create_vault_password_file(vault: &mut Vault) -> Result<()> { Ok(()) } -pub fn interpolate_secrets(content: &str, vault: &Vault) -> (String, Vec) { +pub fn interpolate_secrets(content: &str, vault: &Vault) -> Result<(String, Vec)> { let mut missing_secrets = vec![]; + let mut fatal_error: Option = None; + let parsed_content: String = content .lines() .map(|line| { - if line.trim_start().starts_with('#') { + if line.trim_start().starts_with('#') || fatal_error.is_some() { return line.to_string(); } SECRET_RE .replace_all(line, |caps: &fancy_regex::Captures<'_>| { - let secret = vault.get_secret(caps[1].trim(), false); - match secret { + let name = caps[1].trim(); + match vault.get_secret(name, false) { Ok(s) => s, - Err(_) => { - missing_secrets.push(caps[1].to_string()); - "".to_string() - } + Err(e) => match e.downcast_ref::() { + Some(SecretError::NotFound { .. }) => { + missing_secrets.push(name.to_string()); + String::new() + } + _ => { + fatal_error = Some(anyhow!( + "Failed to fetch secret '{name}' from vault: {e}" + )); + String::new() + } + }, } }) .to_string() @@ -197,5 +208,9 @@ pub fn interpolate_secrets(content: &str, vault: &Vault) -> (String, Vec .collect::>() .join("\n"); - (parsed_content, missing_secrets) + if let Some(err) = fatal_error { + return Err(err); + } + + Ok((parsed_content, missing_secrets)) }