From bba094086d311e128502572b476b8ccab9e68c76 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Tue, 2 Jun 2026 14:52:36 -0600 Subject: [PATCH] feat: vault_password_file or nothing at all is shorthand for just using the local gman provider for secret management --- src/config/app_config.rs | 94 +++++++++++------------------------ src/config/mod.rs | 4 +- src/config/request_context.rs | 80 +++++++++++++++-------------- src/vault/mod.rs | 13 +++-- 4 files changed, 83 insertions(+), 108 deletions(-) diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 2eb9b37..07b5669 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -30,7 +30,7 @@ pub struct AppConfig { pub wrap: Option, pub wrap_code: bool, pub(crate) vault_password_file: Option, - pub(crate) secrets_provider: SupportedProvider, + pub(crate) secrets_provider: Option, pub function_calling_support: bool, pub mapping_tools: IndexMap, @@ -96,7 +96,7 @@ impl Default for AppConfig { wrap: None, wrap_code: false, vault_password_file: None, - secrets_provider: SupportedProvider::default(), + secrets_provider: None, function_calling_support: true, mapping_tools: Default::default(), @@ -212,7 +212,6 @@ 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)?; @@ -223,17 +222,6 @@ 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); @@ -790,66 +778,40 @@ mod tests { } #[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()); + fn default_secrets_provider_is_none() { + let app = AppConfig::default(); + assert!(app.secrets_provider.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 { + fn secrets_provider_can_hold_non_local_variant() { + let app = AppConfig { + secrets_provider: Some(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 { .. } + Some(SupportedProvider::Gopass { .. }) + )); + } + + #[test] + fn from_config_copies_secrets_provider() { + let cfg = Config { + model_id: "test-model".to_string(), + clients: vec![ClientConfig::default()], + secrets_provider: Some(SupportedProvider::Gopass { + provider_def: Default::default(), + }), + ..Config::default() + }; + + let app = AppConfig::from_config(cfg).unwrap(); + assert!(matches!( + app.secrets_provider, + Some(SupportedProvider::Gopass { .. }) )); } } diff --git a/src/config/mod.rs b/src/config/mod.rs index db734e2..ca01f65 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -190,7 +190,7 @@ pub struct Config { pub(super) vault_password_file: Option, #[serde(default)] - pub(super) secrets_provider: SupportedProvider, + pub(super) secrets_provider: Option, pub function_calling_support: bool, pub mapping_tools: IndexMap, @@ -256,7 +256,7 @@ impl Default for Config { wrap: None, wrap_code: false, vault_password_file: None, - secrets_provider: SupportedProvider::default(), + secrets_provider: None, function_calling_support: true, mapping_tools: Default::default(), diff --git a/src/config/request_context.rs b/src/config/request_context.rs index dab3939..cf1f68b 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -907,44 +907,52 @@ impl RequestContext { ("messages_file", display_path(&self.messages_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))); + None => { + items.push(("secrets_provider", "local".to_string())); + items.push(("vault_password_file", display_path(&app.vault_password_file()))); } - 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())); + Some(provider) => { + items.push(("secrets_provider", provider.to_string())); + match 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())); + } + } } } } diff --git a/src/vault/mod.rs b/src/vault/mod.rs index f8a4864..dc29134 100644 --- a/src/vault/mod.rs +++ b/src/vault/mod.rs @@ -42,12 +42,17 @@ impl Vault { } pub fn init(config: &AppConfig) -> Self { - let mut provider = config.secrets_provider.clone(); + let mut provider = match &config.secrets_provider { + Some(p) => p.clone(), + None => SupportedProvider::Local { + provider_def: LocalProvider { + password_file: Some(config.vault_password_file()), + ..LocalProvider::default() + }, + }, + }; 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"); }