diff --git a/Cargo.lock b/Cargo.lock index cadd579d67ff..8265010cb828 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3556,6 +3556,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "serial_test", "shlex", "tar", "temp-env", diff --git a/crates/goose-cli/Cargo.toml b/crates/goose-cli/Cargo.toml index 0bcfc5506b0c..ed3e4409351a 100644 --- a/crates/goose-cli/Cargo.toml +++ b/crates/goose-cli/Cargo.toml @@ -81,3 +81,4 @@ tempfile = "3" temp-env = { version = "0.3.6", features = ["async_closure"] } test-case = "3.3" tokio = { version = "1.43", features = ["rt", "macros"] } +serial_test = "3.0.0" diff --git a/crates/goose-cli/src/recipes/mod.rs b/crates/goose-cli/src/recipes/mod.rs index a1ceef614213..570386916136 100644 --- a/crates/goose-cli/src/recipes/mod.rs +++ b/crates/goose-cli/src/recipes/mod.rs @@ -3,4 +3,6 @@ pub mod github_recipe; pub mod print_recipe; pub mod recipe; pub mod search_recipe; +pub mod secret_collector; +pub mod secret_discovery; pub mod template_recipe; diff --git a/crates/goose-cli/src/recipes/secret_collector.rs b/crates/goose-cli/src/recipes/secret_collector.rs new file mode 100644 index 000000000000..398ecd1f683c --- /dev/null +++ b/crates/goose-cli/src/recipes/secret_collector.rs @@ -0,0 +1,233 @@ +use crate::recipes::secret_discovery::SecretRequirement; +use anyhow::{Context, Result}; +use console::style; +use goose::config::Config; +use serde_json::Value; +use std::collections::HashMap; + +/// Interactive secret collector for gathering missing credentials +pub struct SecretCollector { + config: &'static Config, +} + +impl SecretCollector { + /// Create a new SecretCollector instance + pub fn new() -> Self { + Self { + config: Config::global(), + } + } + + /// Collect missing secrets through interactive prompts + pub async fn collect_missing_secrets( + &self, + requirements: Vec, + ) -> Result<()> { + let missing_requirements: Vec<_> = requirements + .into_iter() + .filter(|req| !req.is_available) + .collect(); + + if missing_requirements.is_empty() { + return Ok(()); + } + + // Group secrets by extension for better UX + let mut grouped_secrets: HashMap> = HashMap::new(); + for requirement in missing_requirements { + grouped_secrets + .entry(requirement.extension_name.clone()) + .or_default() + .push(requirement); + } + + // Present intro + cliclack::intro(style(" extension-setup ").on_blue().black()) + .context("Failed to show intro")?; + + // Show overview of what's needed + let total_secrets: usize = grouped_secrets.values().map(|v| v.len()).sum(); + let extension_count = grouped_secrets.len(); + + println!( + "{}", + style(format!( + "Extension credentials required ({} {} across {} {}):", + total_secrets, + if total_secrets == 1 { + "secret" + } else { + "secrets" + }, + extension_count, + if extension_count == 1 { + "extension" + } else { + "extensions" + } + )) + .dim() + ); + println!(); + + // Show grouped overview + for (extension_name, secrets) in &grouped_secrets { + println!(" {} extension needs:", style(extension_name).cyan().bold()); + for secret in secrets { + println!(" • {}", style(&secret.key).yellow()); + } + println!(); + } + + // Collect secrets for each extension + for (extension_name, secrets) in grouped_secrets { + self.collect_extension_secrets(&extension_name, &secrets) + .await?; + } + + cliclack::outro("Extension credentials configured successfully") + .context("Failed to show outro")?; + Ok(()) + } + + /// Present extension-specific secret collection dialog + async fn collect_extension_secrets( + &self, + extension_name: &str, + missing_secrets: &[SecretRequirement], + ) -> Result<()> { + if missing_secrets.len() > 1 { + println!( + "{}", + style(format!("Configuring {} extension:", extension_name)) + .green() + .bold() + ); + println!(); + } + + for secret in missing_secrets { + self.collect_single_secret(secret, extension_name).await?; + } + + Ok(()) + } + + /// Collect a single secret with appropriate prompting + async fn collect_single_secret( + &self, + secret: &SecretRequirement, + extension_name: &str, + ) -> Result<()> { + let prompt = format!("Enter {} for {} extension", secret.key, extension_name); + + // Use password input for sensitive data + let value = cliclack::password(prompt) + .mask('▪') + .validate(|input: &String| { + if input.trim().is_empty() { + Err("Value cannot be empty") + } else { + Ok(()) + } + }) + .interact() + .context("Failed to collect secret input")?; + + // Store the secret + self.store_secret(&secret.key, &value)?; + + Ok(()) + } + + /// Store a secret securely + fn store_secret(&self, key: &str, value: &str) -> Result<()> { + self.config + .set_secret(key, Value::String(value.to_string())) + .with_context(|| format!("Failed to store secret '{}'", key))?; + Ok(()) + } +} + +impl Default for SecretCollector { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::recipes::secret_discovery::{SecretRequirement, SecretSource}; + + fn create_test_requirement( + key: &str, + extension: &str, + description: Option<&str>, + ) -> SecretRequirement { + SecretRequirement { + key: key.to_string(), + extension_name: extension.to_string(), + description: description.map(|d| d.to_string()), + is_available: false, + source: SecretSource::Missing, + } + } + + #[test] + fn test_secret_collection_grouping() { + let requirements = vec![ + create_test_requirement("GITHUB_TOKEN", "github-mcp", Some("GitHub API token")), + create_test_requirement("GITHUB_REPO", "github-mcp", Some("GitHub repository")), + create_test_requirement("OPENAI_API_KEY", "openai-provider", Some("OpenAI API key")), + ]; + + let mut grouped_secrets: HashMap> = HashMap::new(); + for requirement in requirements { + grouped_secrets + .entry(requirement.extension_name.clone()) + .or_default() + .push(requirement); + } + + assert_eq!(grouped_secrets.len(), 2); + assert_eq!(grouped_secrets["github-mcp"].len(), 2); + assert_eq!(grouped_secrets["openai-provider"].len(), 1); + } + + #[test] + fn test_empty_requirements() { + let empty_requirements: Vec = vec![]; + + // This should not panic and should handle empty case gracefully + // In a real test, we'd use tokio::test, but for now we just test the logic + let missing: Vec = empty_requirements + .into_iter() + .filter(|req| !req.is_available) + .collect(); + + assert!(missing.is_empty()); + } + + #[test] + fn test_available_requirements_filtered() { + let requirements = vec![ + SecretRequirement { + key: "AVAILABLE_KEY".to_string(), + extension_name: "test-extension".to_string(), + description: None, + is_available: true, + source: SecretSource::Environment, + }, + create_test_requirement("MISSING_KEY", "test-extension", None), + ]; + + let missing: Vec = requirements + .into_iter() + .filter(|req| !req.is_available) + .collect(); + + assert_eq!(missing.len(), 1); + assert_eq!(missing[0].key, "MISSING_KEY"); + } +} diff --git a/crates/goose-cli/src/recipes/secret_discovery.rs b/crates/goose-cli/src/recipes/secret_discovery.rs new file mode 100644 index 000000000000..203c7b87c5f2 --- /dev/null +++ b/crates/goose-cli/src/recipes/secret_discovery.rs @@ -0,0 +1,276 @@ +use goose::{ + agents::extension::ExtensionConfig, + config::Config, + keyring::{KeyringBackend, SystemKeyringBackend}, + recipe::Recipe, +}; +use std::sync::Arc; + +/// Represents a required secret for an extension +#[derive(Debug, Clone)] +pub struct SecretRequirement { + pub key: String, + pub extension_name: String, + pub description: Option, + pub is_available: bool, + pub source: SecretSource, +} + +/// Indicates where a secret value is found or missing +#[derive(Debug, Clone)] +pub enum SecretSource { + Missing, + Environment, + Keyring, +} + +/// Engine for discovering required secrets from recipes and extensions +pub struct SecretDiscovery { + #[allow(dead_code)] + keyring_backend: Arc, +} + +impl SecretDiscovery { + /// Create a new SecretDiscovery instance + pub fn new() -> Self { + Self { + keyring_backend: Arc::new(SystemKeyringBackend), + } + } + + /// Create a new SecretDiscovery instance with a custom keyring backend + pub fn with_keyring_backend(keyring_backend: Arc) -> Self { + Self { keyring_backend } + } + + /// Analyze a recipe and collect all required secrets from its extensions + pub fn discover_required_secrets(&self, recipe: &Recipe) -> Vec { + let mut requirements = Vec::new(); + + if let Some(extensions) = &recipe.extensions { + for extension in extensions { + let extension_requirements = self.discover_extension_secrets(extension); + requirements.extend(extension_requirements); + } + } + + requirements + } + + /// Discover required secrets for a specific extension configuration + pub fn discover_extension_secrets(&self, config: &ExtensionConfig) -> Vec { + let extension_name = config.name(); + let env_keys = self.get_env_keys_from_config(config); + + env_keys + .into_iter() + .map(|key| { + let (is_available, source) = self.check_secret_availability(&key); + SecretRequirement { + key: key.clone(), + extension_name: extension_name.clone(), + description: None, + is_available, + source, + } + }) + .collect() + } + + /// Check if a specific secret is available in environment or keyring + pub fn is_secret_available(&self, key: &str) -> bool { + let (is_available, _) = self.check_secret_availability(key); + is_available + } + + /// Get the list of env_keys from an ExtensionConfig + fn get_env_keys_from_config(&self, config: &ExtensionConfig) -> Vec { + match config { + ExtensionConfig::Sse { env_keys, .. } => env_keys.clone(), + ExtensionConfig::Stdio { env_keys, .. } => env_keys.clone(), + ExtensionConfig::StreamableHttp { env_keys, .. } => env_keys.clone(), + ExtensionConfig::Builtin { .. } => Vec::new(), + ExtensionConfig::Frontend { .. } => Vec::new(), + } + } + + /// Check availability of a secret and determine its source + fn check_secret_availability(&self, key: &str) -> (bool, SecretSource) { + // First check environment variables + if std::env::var(key).is_ok() { + return (true, SecretSource::Environment); + } + + // Then check keyring/config system + let config = Config::global(); + match config.get(key, true) { + Ok(value) => { + if value.is_null() { + (false, SecretSource::Missing) + } else if value.is_string() { + (true, SecretSource::Keyring) + } else { + (false, SecretSource::Missing) + } + } + Err(_) => (false, SecretSource::Missing), + } + } +} + +impl Default for SecretDiscovery { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use goose::{keyring::MockKeyringBackend, recipe::Recipe}; + + fn create_test_discovery() -> SecretDiscovery { + let mock_keyring = Arc::new(MockKeyringBackend::new()); + SecretDiscovery::with_keyring_backend(mock_keyring) + } + + #[test] + fn test_discover_extension_secrets_stdio() { + let discovery = create_test_discovery(); + + let config = ExtensionConfig::Stdio { + name: "github-mcp".to_string(), + cmd: "github-mcp".to_string(), + args: vec![], + envs: Default::default(), + env_keys: vec!["GITHUB_TOKEN".to_string(), "GITHUB_REPO".to_string()], + timeout: Some(300), + description: Some("GitHub MCP extension".to_string()), + bundled: Some(false), + }; + + let requirements = discovery.discover_extension_secrets(&config); + + assert_eq!(requirements.len(), 2); + assert_eq!(requirements[0].key, "GITHUB_TOKEN"); + assert_eq!(requirements[0].extension_name, "github-mcp"); + assert!(!requirements[0].is_available); + assert!(matches!(requirements[0].source, SecretSource::Missing)); + + assert_eq!(requirements[1].key, "GITHUB_REPO"); + assert_eq!(requirements[1].extension_name, "github-mcp"); + assert!(!requirements[1].is_available); + } + + #[test] + fn test_discover_extension_secrets_sse() { + let discovery = create_test_discovery(); + + let config = ExtensionConfig::Sse { + name: "openai-provider".to_string(), + uri: "http://example.com/sse".to_string(), + envs: Default::default(), + env_keys: vec!["OPENAI_API_KEY".to_string()], + description: Some("OpenAI provider".to_string()), + timeout: Some(300), + bundled: Some(false), + }; + + let requirements = discovery.discover_extension_secrets(&config); + + assert_eq!(requirements.len(), 1); + assert_eq!(requirements[0].key, "OPENAI_API_KEY"); + assert_eq!(requirements[0].extension_name, "openai-provider"); + assert!(!requirements[0].is_available); + } + + #[test] + fn test_discover_extension_secrets_builtin() { + let discovery = create_test_discovery(); + + let config = ExtensionConfig::Builtin { + name: "developer".to_string(), + display_name: Some("Developer Tools".to_string()), + timeout: Some(300), + bundled: Some(true), + }; + + let requirements = discovery.discover_extension_secrets(&config); + + // Builtin extensions don't have env_keys + assert_eq!(requirements.len(), 0); + } + + #[test] + fn test_discover_required_secrets_from_recipe() { + let discovery = create_test_discovery(); + + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "Test Recipe".to_string(), + description: "A test recipe".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(vec![ + ExtensionConfig::Stdio { + name: "github-mcp".to_string(), + cmd: "github-mcp".to_string(), + args: vec![], + envs: Default::default(), + env_keys: vec!["GITHUB_TOKEN".to_string()], + timeout: Some(300), + description: Some("GitHub MCP extension".to_string()), + bundled: Some(false), + }, + ExtensionConfig::Sse { + name: "openai-provider".to_string(), + uri: "http://example.com/sse".to_string(), + envs: Default::default(), + env_keys: vec!["OPENAI_API_KEY".to_string()], + description: Some("OpenAI provider".to_string()), + timeout: Some(300), + bundled: Some(false), + }, + ]), + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let requirements = discovery.discover_required_secrets(&recipe); + + assert_eq!(requirements.len(), 2); + assert_eq!(requirements[0].key, "GITHUB_TOKEN"); + assert_eq!(requirements[0].extension_name, "github-mcp"); + assert_eq!(requirements[1].key, "OPENAI_API_KEY"); + assert_eq!(requirements[1].extension_name, "openai-provider"); + } + + #[test] + fn test_check_secret_availability_with_env_var() { + let discovery = create_test_discovery(); + + // Set an environment variable for testing + std::env::set_var("TEST_SECRET", "test_value"); + + let (is_available, source) = discovery.check_secret_availability("TEST_SECRET"); + assert!(is_available); + assert!(matches!(source, SecretSource::Environment)); + + // Clean up + std::env::remove_var("TEST_SECRET"); + } + + #[test] + fn test_check_secret_availability_missing() { + let discovery = create_test_discovery(); + + let (is_available, source) = discovery.check_secret_availability("NONEXISTENT_SECRET"); + assert!(!is_available); + assert!(matches!(source, SecretSource::Missing)); + } +} diff --git a/crates/goose-cli/src/session/builder.rs b/crates/goose-cli/src/session/builder.rs index b5d3da1d7f45..de15db27a153 100644 --- a/crates/goose-cli/src/session/builder.rs +++ b/crates/goose-cli/src/session/builder.rs @@ -12,6 +12,8 @@ use std::sync::Arc; use super::output; use super::Session; +use crate::recipes::secret_collector::SecretCollector; +use crate::recipes::secret_discovery::SecretDiscovery; /// Configuration for building a new Goose session /// @@ -162,6 +164,24 @@ pub struct SessionSettings { pub temperature: Option, } +/// Collect missing secrets for extensions +async fn collect_extension_secrets(extensions: &[ExtensionConfig]) -> anyhow::Result<()> { + let discovery = SecretDiscovery::new(); + let mut all_requirements = Vec::new(); + + for extension in extensions { + let requirements = discovery.discover_extension_secrets(extension); + all_requirements.extend(requirements); + } + + if !all_requirements.is_empty() { + let collector = SecretCollector::new(); + collector.collect_missing_secrets(all_requirements).await?; + } + + Ok(()) +} + pub async fn build_session(session_config: SessionBuilderConfig) -> Session { // Load config and get provider/model let config = Config::global(); @@ -331,6 +351,19 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session { .collect() }; + if session_config.interactive && !extensions_to_run.is_empty() { + if let Err(e) = collect_extension_secrets(&extensions_to_run).await { + eprintln!( + "{}", + style(format!( + "Warning: Failed to collect extension secrets: {}", + e + )) + .yellow() + ); + } + } + for extension in extensions_to_run { if let Err(e) = agent.add_extension(extension.clone()).await { let err = match e { @@ -510,6 +543,8 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session { #[cfg(test)] mod tests { use super::*; + use goose::agents::extension::Envs; + use std::collections::HashMap; #[test] fn test_session_builder_config_creation() { @@ -579,4 +614,77 @@ mod tests { assert_eq!(extension_name, "test-extension"); assert_eq!(error_message, "test error"); } + + #[tokio::test] + async fn test_collect_extension_secrets_with_empty_extensions() { + let extensions: Vec = vec![]; + let result = collect_extension_secrets(&extensions).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_collect_extension_secrets_with_builtin_extensions() { + let extensions = vec![ExtensionConfig::Builtin { + name: "developer".to_string(), + display_name: Some("Developer Tools".to_string()), + timeout: Some(300), + bundled: Some(true), + }]; + + let result = collect_extension_secrets(&extensions).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_collect_extension_secrets_with_stdio_extensions_logic() { + let extensions = vec![ExtensionConfig::Stdio { + name: "test-extension".to_string(), + cmd: "test-cmd".to_string(), + args: vec![], + envs: Envs::new(HashMap::new()), + env_keys: vec!["TEST_API_KEY".to_string()], + timeout: Some(300), + description: Some("Test extension".to_string()), + bundled: Some(false), + }]; + + // Test the discovery part of the logic (without interactive collection) + let discovery = SecretDiscovery::new(); + let mut all_requirements = Vec::new(); + + for extension in &extensions { + let requirements = discovery.discover_extension_secrets(extension); + all_requirements.extend(requirements); + } + + // Should discover that TEST_API_KEY is missing + assert_eq!(all_requirements.len(), 1); + assert_eq!(all_requirements[0].key, "TEST_API_KEY"); + assert_eq!(all_requirements[0].extension_name, "test-extension"); + assert!(!all_requirements[0].is_available); + } + + #[tokio::test] + async fn test_collect_extension_secrets_with_available_secrets() { + // Set environment variable to simulate available secret + std::env::set_var("TEST_AVAILABLE_KEY", "test_value"); + + let extensions = vec![ExtensionConfig::Stdio { + name: "test-extension".to_string(), + cmd: "test-cmd".to_string(), + args: vec![], + envs: Envs::new(HashMap::new()), + env_keys: vec!["TEST_AVAILABLE_KEY".to_string()], + timeout: Some(300), + description: Some("Test extension".to_string()), + bundled: Some(false), + }]; + + let result = collect_extension_secrets(&extensions).await; + // Should succeed because the secret is already available + assert!(result.is_ok()); + + // Clean up + std::env::remove_var("TEST_AVAILABLE_KEY"); + } } diff --git a/crates/goose-cli/tests/secret_collection_integration_test.rs b/crates/goose-cli/tests/secret_collection_integration_test.rs new file mode 100644 index 000000000000..b5eb384946e1 --- /dev/null +++ b/crates/goose-cli/tests/secret_collection_integration_test.rs @@ -0,0 +1,350 @@ +use anyhow::Result; +use goose::agents::extension::{Envs, ExtensionConfig}; +use goose::config::Config; +use goose::recipe::Recipe; +use goose_cli::recipes::secret_discovery::SecretDiscovery; +use goose_cli::session::{build_session, SessionBuilderConfig}; +use serde_json::{json, Value}; +use serial_test::serial; +use std::env; + +/// Integration tests for the secret collection flow +/// +/// These tests cover core functionality including: +/// - First-time user with no configured secrets +/// - Existing user with partial secrets configured +/// - Recipe with no extension requirements +/// - Recipe with multiple extensions requiring same secrets + +fn setup_test_environment() -> Result<()> { + // Ensure we're using mock keyring for tests + env::set_var("GOOSE_DISABLE_KEYRING", "1"); + Ok(()) +} + +#[cfg(test)] +mod integration_tests { + use super::*; + + #[tokio::test] + async fn test_recipe_with_no_extension_requirements() -> Result<()> { + setup_test_environment()?; + + // Create a recipe with no extensions + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "test-recipe".to_string(), + description: "Test recipe with no extensions".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: None, + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let discovery = SecretDiscovery::new(); + let requirements = discovery.discover_required_secrets(&recipe); + + // Should have no secret requirements + assert!(requirements.is_empty()); + Ok(()) + } + + #[tokio::test] + async fn test_recipe_with_multiple_extensions_same_secrets() -> Result<()> { + setup_test_environment()?; + + // Create a recipe with multiple extensions requiring the same secret + let extensions = vec![ + ExtensionConfig::Stdio { + name: "github-extension".to_string(), + cmd: "github-mcp".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec!["GITHUB_TOKEN".to_string()], + timeout: None, + description: None, + bundled: None, + }, + ExtensionConfig::Stdio { + name: "github-analyzer".to_string(), + cmd: "github-analyzer".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec!["GITHUB_TOKEN".to_string()], + timeout: None, + description: None, + bundled: None, + }, + ]; + + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "multi-github-recipe".to_string(), + description: "Recipe with multiple extensions using same secret".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(extensions), + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let discovery = SecretDiscovery::new(); + let requirements = discovery.discover_required_secrets(&recipe); + + // Should have 2 requirements for the same secret (one per extension) + assert_eq!(requirements.len(), 2); + assert_eq!(requirements[0].key, "GITHUB_TOKEN"); + assert_eq!(requirements[1].key, "GITHUB_TOKEN"); + assert_ne!( + requirements[0].extension_name, + requirements[1].extension_name + ); + Ok(()) + } + + #[tokio::test] + async fn test_first_time_user_scenario() -> Result<()> { + setup_test_environment()?; + + // Create a recipe with extensions requiring secrets + let extensions = vec![ExtensionConfig::Stdio { + name: "openai-provider".to_string(), + cmd: "openai-mcp".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec!["OPENAI_API_KEY".to_string()], + timeout: None, + description: None, + bundled: None, + }]; + + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "first-time-user-recipe".to_string(), + description: "Recipe for first-time user".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(extensions), + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let discovery = SecretDiscovery::new(); + let requirements = discovery.discover_required_secrets(&recipe); + + // Should have 1 missing requirement + assert_eq!(requirements.len(), 1); + assert_eq!(requirements[0].key, "OPENAI_API_KEY"); + assert!(!requirements[0].is_available); // Should be missing for first-time user + Ok(()) + } + + #[tokio::test] + #[serial] + async fn test_existing_user_with_partial_secrets() -> Result<()> { + setup_test_environment()?; + + // Set one secret in environment + env::set_var("GITHUB_TOKEN", "test-github-token"); + + // Create a recipe with multiple extensions + let extensions = vec![ + ExtensionConfig::Stdio { + name: "github-extension".to_string(), + cmd: "github-mcp".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec!["GITHUB_TOKEN".to_string()], + timeout: None, + description: None, + bundled: None, + }, + ExtensionConfig::Stdio { + name: "openai-extension".to_string(), + cmd: "openai-mcp".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec!["OPENAI_API_KEY".to_string()], + timeout: None, + description: None, + bundled: None, + }, + ]; + + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "partial-secrets-recipe".to_string(), + description: "Recipe for user with partial secrets".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(extensions), + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let discovery = SecretDiscovery::new(); + let requirements = discovery.discover_required_secrets(&recipe); + + // Should have 2 requirements: 1 available, 1 missing + assert_eq!(requirements.len(), 2); + + let github_req = requirements + .iter() + .find(|r| r.key == "GITHUB_TOKEN") + .unwrap(); + let openai_req = requirements + .iter() + .find(|r| r.key == "OPENAI_API_KEY") + .unwrap(); + + assert!(github_req.is_available); // Should be available from env + assert!(!openai_req.is_available); // Should be missing + + // Clean up + env::remove_var("GITHUB_TOKEN"); + Ok(()) + } + + #[tokio::test] + #[serial] + async fn test_session_builder_integration() -> Result<()> { + setup_test_environment()?; + + // Create session config with non-interactive mode + let session_config = SessionBuilderConfig { + interactive: false, // Non-interactive mode + extensions: vec!["stdio://github-mcp".to_string()], + ..Default::default() + }; + + // This should not trigger secret collection in non-interactive mode + // The session building should succeed without prompting for input + let _session = build_session(session_config).await; + + // If we reach here, the session was built successfully without hanging + // This validates that non-interactive mode works correctly + + Ok(()) + } + + #[tokio::test] + async fn test_secret_collection_empty_extensions() -> Result<()> { + setup_test_environment()?; + + // Test extension with no env_keys + let extensions = vec![ExtensionConfig::Stdio { + name: "no-secrets-extension".to_string(), + cmd: "basic-mcp".to_string(), + args: vec![], + envs: Envs::default(), + env_keys: vec![], + timeout: None, + description: None, + bundled: None, + }]; + + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "no-secrets-recipe".to_string(), + description: "Recipe with extension that needs no secrets".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(extensions), + context: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + sub_recipes: None, + }; + + let discovery = SecretDiscovery::new(); + let requirements = discovery.discover_required_secrets(&recipe); + + // Should have no requirements + assert_eq!(requirements.len(), 0); + + Ok(()) + } +} + +/// Security-focused tests for secret handling +#[cfg(test)] +mod security_tests { + use super::*; + + #[tokio::test] + #[serial] + async fn test_secret_isolation() -> Result<()> { + setup_test_environment()?; + + // Test that secrets are properly isolated between different services + let config = Config::global(); + + // Store secrets for different "services" + config.set_secret("SERVICE_A_KEY", json!("secret-a"))?; + config.set_secret("SERVICE_B_KEY", json!("secret-b"))?; + + // Verify each service only gets its own secret + let secret_a: Value = config.get_secret("SERVICE_A_KEY")?; + let secret_b: Value = config.get_secret("SERVICE_B_KEY")?; + assert_eq!(secret_a.as_str().unwrap(), "secret-a"); + assert_eq!(secret_b.as_str().unwrap(), "secret-b"); + + // Verify non-existent secret returns error + let result = config.get_secret::("NON_EXISTENT_KEY"); + assert!(result.is_err()); + + // Clean up + config.delete_secret("SERVICE_A_KEY")?; + config.delete_secret("SERVICE_B_KEY")?; + + Ok(()) + } + + #[tokio::test] + #[serial] + async fn test_secret_overwrite_protection() -> Result<()> { + setup_test_environment()?; + + let config = Config::global(); + let key = "OVERWRITE_TEST_KEY"; + + // Store initial secret + config.set_secret(key, json!("initial-value"))?; + let initial: Value = config.get_secret(key)?; + assert_eq!(initial.as_str().unwrap(), "initial-value"); + + // Overwrite with new value + config.set_secret(key, json!("new-value"))?; + let updated: Value = config.get_secret(key)?; + assert_eq!(updated.as_str().unwrap(), "new-value"); + + // Clean up + config.delete_secret(key)?; + + Ok(()) + } +} diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index a418fe112a9e..c25d2928ac8a 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -15,7 +15,7 @@ use tracing::{error, warn}; use super::extension::{ExtensionConfig, ExtensionError, ExtensionInfo, ExtensionResult, ToolInfo}; use super::tool_execution::ToolCallResult; use crate::agents::extension::Envs; -use crate::config::{Config, ExtensionConfigManager}; +use crate::config::{Config, ConfigError, ExtensionConfigManager}; use crate::prompt_template; use mcp_client::client::{ClientCapabilities, ClientInfo, McpClient, McpClientTrait}; use mcp_client::transport::{SseTransport, StdioTransport, StreamableHttpTransport, Transport}; @@ -156,6 +156,13 @@ impl ExtensionManager { ); } } + Err(ConfigError::NotFound(_)) => { + return Err(ExtensionError::SetupError(format!( + "Missing required secret '{}' for extension '{}'. {}", + key, ext_name, + "Run 'goose configure' to set up credentials, set via environment variable, or run a recipe interactively to be prompted for credentials." + ))); + } Err(e) => { error!( key = %key,