From d8cc74fc0ee4845a070b48fbf3d30706f0dbdfde Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Fri, 21 Aug 2026 04:16:12 +0000 Subject: [PATCH] cli: remove recipe secret discovery (#11435) --- crates/goose-cli/src/recipes/mod.rs | 1 - crates/goose-cli/src/recipes/recipe.rs | 83 +--- .../goose-cli/src/recipes/secret_discovery.rs | 413 ------------------ .../docs/guides/recipes/recipe-reference.md | 16 +- .../docs/guides/recipes/session-recipes.md | 3 +- 5 files changed, 6 insertions(+), 510 deletions(-) delete mode 100644 crates/goose-cli/src/recipes/secret_discovery.rs diff --git a/crates/goose-cli/src/recipes/mod.rs b/crates/goose-cli/src/recipes/mod.rs index 8e4d56847..8dd270e20 100644 --- a/crates/goose-cli/src/recipes/mod.rs +++ b/crates/goose-cli/src/recipes/mod.rs @@ -3,4 +3,3 @@ pub mod github_recipe; pub mod print_recipe; pub mod recipe; pub mod search_recipe; -pub mod secret_discovery; diff --git a/crates/goose-cli/src/recipes/recipe.rs b/crates/goose-cli/src/recipes/recipe.rs index a25d3335d..943025ed3 100644 --- a/crates/goose-cli/src/recipes/recipe.rs +++ b/crates/goose-cli/src/recipes/recipe.rs @@ -3,9 +3,7 @@ use crate::recipes::print_recipe::{ print_required_parameters_for_template, }; use crate::recipes::search_recipe::load_recipe_file; -use crate::recipes::secret_discovery::{discover_recipe_secrets, SecretRequirement}; use anyhow::Result; -use goose::config::Config; use goose::recipe::build_recipe::{ apply_values_to_parameters_without_file_expansion, build_recipe_from_template, RecipeError, }; @@ -30,16 +28,7 @@ pub fn load_recipe(recipe_name: &str, params: Vec<(String, String)>) -> Result { - let secret_requirements = discover_recipe_secrets(&recipe); - if let Err(e) = collect_missing_secrets(&secret_requirements) { - eprintln!( - "Warning: Failed to collect some secrets: {}. Recipe will continue to run.", - e - ); - } - Ok(recipe) - } + Ok(recipe) => Ok(recipe), Err(RecipeError::MissingParams { parameters }) => Err(anyhow::anyhow!( "Please provide the following parameters in the command line: {}", missing_parameters_command_line(parameters) @@ -48,76 +37,6 @@ pub fn load_recipe(recipe_name: &str, params: Vec<(String, String)>) -> Result Result<()> { - if requirements.is_empty() { - return Ok(()); - } - - let config = Config::global(); - let mut missing_secrets = Vec::new(); - - for req in requirements { - match config.get_secret::(&req.key) { - Ok(_) => continue, // Secret exists - Err(_) => missing_secrets.push(req), - } - } - - if missing_secrets.is_empty() { - return Ok(()); - } - - println!( - "šŸ” This recipe uses {} secret(s) that are not yet configured (press ESC to skip any that are optional):", - missing_secrets.len() - ); - - for req in &missing_secrets { - println!("\nšŸ“‹ Extension: {}", req.extension_name); - println!("šŸ”‘ Secret: {}", req.key); - - let value = cliclack::password(format!( - "Enter {} ({}) - press ESC to skip", - req.key, - req.description() - )) - .mask('ā–Ŗ') - .interact() - .unwrap_or_else(|_| String::new()); - - if !value.trim().is_empty() { - if let Err(e) = config.set_secret(&req.key, &value) { - println!("āš ļø Failed to store secret in secure storage: {}. Secret available for this session only.", e); - println!( - " Consider setting {} as an environment variable for future use.", - req.key - ); - } else { - println!("āœ… Secret stored securely for {}", req.extension_name); - } - } else { - println!("ā­ļø Skipped {} for {}", req.key, req.extension_name); - } - } - - if !missing_secrets.is_empty() { - println!("\nšŸŽ‰ Secret collection complete! Recipe execution will now continue."); - } - - Ok(()) -} - pub fn render_recipe_as_yaml(recipe_name: &str, params: Vec<(String, String)>) -> Result<()> { let recipe = load_recipe(recipe_name, params)?; match serde_yaml::to_string(&recipe) { diff --git a/crates/goose-cli/src/recipes/secret_discovery.rs b/crates/goose-cli/src/recipes/secret_discovery.rs deleted file mode 100644 index 56f20a47a..000000000 --- a/crates/goose-cli/src/recipes/secret_discovery.rs +++ /dev/null @@ -1,413 +0,0 @@ -use crate::recipes::search_recipe::load_recipe_file; -use goose::agents::extension::ExtensionConfig; -use goose::recipe::Recipe; -use regex::{NoExpand, Regex}; -use std::collections::HashSet; - -/// Represents a secret requirement discovered from a recipe extension -#[derive(Debug, Clone, PartialEq)] -pub struct SecretRequirement { - /// The environment variable name (e.g., "GITHUB_TOKEN") - pub key: String, - /// The name of the extension that requires this secret - pub extension_name: String, -} - -impl SecretRequirement { - pub fn new(extension_name: String, key: String) -> Self { - Self { - key, - extension_name, - } - } - - /// Returns a human-readable description of what this secret is for - pub fn description(&self) -> String { - format!("Required by {} extension", self.extension_name) - } -} - -/// Discovers all secrets required by MCP extensions in a recipe and its sub-recipes -/// -/// This function recursively scans the recipe and all its sub-recipes for extensions -/// and collects their declared env_keys, creating SecretRequirement structs for each -/// unique environment variable. -/// -/// # Arguments -/// * `recipe` - The recipe to analyze for secret requirements -/// -/// # Returns -/// A vector of SecretRequirement objects, deduplicated by key name -pub fn discover_recipe_secrets(recipe: &Recipe) -> Vec { - let mut visited_recipes = HashSet::new(); - discover_recipe_secrets_recursive(recipe, &mut visited_recipes) -} - -/// Extract secrets from a list of extensions -fn extract_secrets_from_extensions( - extensions: &[ExtensionConfig], - seen_keys: &mut HashSet, -) -> Vec { - let mut secrets = Vec::new(); - - for ext in extensions { - let (extension_name, env_keys, client_secret_key) = match ext { - ExtensionConfig::Stdio { name, env_keys, .. } => (name, env_keys, None), - ExtensionConfig::StreamableHttp { - name, - env_keys, - client_secret_key, - .. - } => (name, env_keys, client_secret_key.as_ref()), - ExtensionConfig::Builtin { name, .. } => (name, &Vec::new(), None), - ExtensionConfig::Platform { name, .. } => (name, &Vec::new(), None), - ExtensionConfig::Frontend { name, .. } => (name, &Vec::new(), None), - ExtensionConfig::InlinePython { name, .. } => (name, &Vec::new(), None), - // SSE is unsupported - skip - ExtensionConfig::Sse { name, .. } => { - tracing::warn!(name = %name, "SSE is unsupported, skipping"); - continue; - } - }; - - for key in env_keys.iter().chain(client_secret_key) { - if seen_keys.insert(key.clone()) { - let secret_req = SecretRequirement::new(extension_name.clone(), key.clone()); - secrets.push(secret_req); - } - } - } - - secrets -} - -/// Internal recursive function (depth-first search) to discover secrets nested in sub-recipes -/// This is future-proofing for a time when we have more than one-level of sub-recipe nesting -fn discover_recipe_secrets_recursive( - recipe: &Recipe, - visited_recipes: &mut HashSet, -) -> Vec { - let mut secrets: Vec = Vec::new(); - let mut seen_keys = HashSet::new(); - - if let Some(extensions) = &recipe.extensions { - secrets.extend(extract_secrets_from_extensions(extensions, &mut seen_keys)); - } - - if let Some(sub_recipes) = &recipe.sub_recipes { - for sub_recipe in sub_recipes { - if visited_recipes.contains(&sub_recipe.path) { - continue; - } - visited_recipes.insert(sub_recipe.path.clone()); - - match load_sub_recipe(&sub_recipe.path) { - Ok((loaded_recipe, parent_dir)) => { - let sub_secrets = - discover_sub_recipe_secrets(&loaded_recipe, &parent_dir, visited_recipes); - for sub_secret in sub_secrets { - if seen_keys.insert(sub_secret.key.clone()) { - secrets.push(sub_secret); - } - } - } - Err(_) => { - continue; - } - } - } - } - - secrets -} - -/// Discovers secrets from a loaded sub-recipe, resolving `{{ recipe_dir }}` in nested -/// sub-recipe paths so they can be loaded without triggering confusing lookup failures. -fn discover_sub_recipe_secrets( - recipe: &Recipe, - parent_dir: &str, - visited_recipes: &mut HashSet, -) -> Vec { - let re = Regex::new(r"\{\{\s*recipe_dir\s*\}\}").expect("valid regex"); - let mut resolved = recipe.clone(); - if let Some(ref mut sub_recipes) = resolved.sub_recipes { - for sr in sub_recipes.iter_mut() { - sr.path = re.replace_all(&sr.path, NoExpand(parent_dir)).into_owned(); - } - } - discover_recipe_secrets_recursive(&resolved, visited_recipes) -} - -/// Loads a recipe from a file path for sub-recipe secret discovery. -/// -/// Returns the parsed recipe along with its parent directory path, which is -/// needed to resolve `{{ recipe_dir }}` in any nested sub-recipe paths. -fn load_sub_recipe(recipe_path: &str) -> Result<(Recipe, String), Box> { - let recipe_file = load_recipe_file(recipe_path)?; - let recipe: Recipe = serde_yaml::from_str(&recipe_file.content)?; - let parent_dir = recipe_file.parent_dir.display().to_string(); - Ok((recipe, parent_dir)) -} - -#[cfg(test)] -mod tests { - use super::*; - use goose::agents::extension::{Envs, ExtensionConfig}; - use goose::recipe::Recipe; - use std::collections::HashMap; - - fn create_test_recipe_with_extensions() -> Recipe { - Recipe { - version: "1.0.0".to_string(), - title: "Test Recipe".to_string(), - description: "A test recipe with MCP extensions".to_string(), - instructions: Some("Test instructions".to_string()), - prompt: None, - extensions: Some(vec![ - ExtensionConfig::StreamableHttp { - name: "github-mcp".to_string(), - uri: "http://localhost:8080/mcp".to_string(), - envs: Envs::new(HashMap::new()), - env_keys: vec!["GITHUB_TOKEN".to_string(), "GITHUB_API_URL".to_string()], - description: "github-mcp".to_string(), - timeout: None, - socket: None, - client_id: None, - client_secret_key: None, - scopes: vec![], - bundled: None, - available_tools: Vec::new(), - headers: HashMap::new(), - }, - ExtensionConfig::Stdio { - name: "slack-mcp".to_string(), - cmd: "slack-mcp".to_string(), - args: vec![], - envs: Envs::new(HashMap::new()), - env_keys: vec!["SLACK_TOKEN".to_string()], - timeout: None, - cwd: None, - description: "slack-mcp".to_string(), - bundled: None, - available_tools: Vec::new(), - }, - ExtensionConfig::Builtin { - name: "builtin-ext".to_string(), - display_name: None, - description: "builtin-ext".to_string(), - timeout: None, - bundled: None, - available_tools: Vec::new(), - }, - ]), - settings: None, - activities: None, - author: None, - parameters: None, - response: None, - sub_recipes: None, - retry: None, - } - } - - #[test] - fn test_discover_recipe_secrets() { - let recipe = create_test_recipe_with_extensions(); - let secrets = discover_recipe_secrets(&recipe); - - assert_eq!(secrets.len(), 3); - - let github_token = secrets.iter().find(|s| s.key == "GITHUB_TOKEN").unwrap(); - assert_eq!(github_token.key, "GITHUB_TOKEN"); - assert_eq!(github_token.extension_name, "github-mcp"); - assert_eq!( - github_token.description(), - "Required by github-mcp extension" - ); - - let github_api = secrets.iter().find(|s| s.key == "GITHUB_API_URL").unwrap(); - assert_eq!(github_api.key, "GITHUB_API_URL"); - assert_eq!(github_api.extension_name, "github-mcp"); - - let slack_token = secrets.iter().find(|s| s.key == "SLACK_TOKEN").unwrap(); - assert_eq!(slack_token.key, "SLACK_TOKEN"); - assert_eq!(slack_token.extension_name, "slack-mcp"); - } - - #[test] - fn test_discover_recipe_secrets_empty_recipe() { - let recipe = Recipe { - version: "1.0.0".to_string(), - title: "Empty Recipe".to_string(), - description: "A recipe with no extensions".to_string(), - instructions: Some("Test instructions".to_string()), - prompt: None, - extensions: None, - settings: None, - activities: None, - author: None, - parameters: None, - response: None, - sub_recipes: None, - retry: None, - }; - - let secrets = discover_recipe_secrets(&recipe); - assert_eq!(secrets.len(), 0); - } - - #[test] - fn test_discover_recipe_secrets_deduplication() { - let recipe = Recipe { - version: "1.0.0".to_string(), - title: "Test Recipe".to_string(), - description: "A test recipe with duplicate secrets".to_string(), - instructions: Some("Test instructions".to_string()), - prompt: None, - extensions: Some(vec![ - ExtensionConfig::StreamableHttp { - name: "service-a".to_string(), - uri: "http://localhost:8080/mcp".to_string(), - envs: Envs::new(HashMap::new()), - env_keys: vec!["API_KEY".to_string()], - description: "service-a".to_string(), - timeout: None, - socket: None, - client_id: None, - client_secret_key: None, - scopes: vec![], - bundled: None, - available_tools: Vec::new(), - headers: HashMap::new(), - }, - ExtensionConfig::Stdio { - name: "service-b".to_string(), - cmd: "service-b".to_string(), - args: vec![], - envs: Envs::new(HashMap::new()), - env_keys: vec!["API_KEY".to_string()], // Same original key, different extension - timeout: None, - cwd: None, - description: "service-b".to_string(), - bundled: None, - available_tools: Vec::new(), - }, - ]), - settings: None, - activities: None, - author: None, - parameters: None, - response: None, - sub_recipes: None, - retry: None, - }; - - let secrets = discover_recipe_secrets(&recipe); - assert_eq!(secrets.len(), 1); - - let api_key = secrets.iter().find(|s| s.key == "API_KEY").unwrap(); - assert_eq!(api_key.key, "API_KEY"); - assert!(api_key.extension_name == "service-a" || api_key.extension_name == "service-b"); - } - - #[test] - fn test_discover_recipe_secrets_includes_client_secret_key() { - let recipe = Recipe { - version: "1.0.0".to_string(), - title: "OAuth Recipe".to_string(), - description: "A recipe with a pre-registered OAuth client".to_string(), - instructions: Some("Test instructions".to_string()), - prompt: None, - extensions: Some(vec![ExtensionConfig::StreamableHttp { - name: "oauth-ext".to_string(), - uri: "http://localhost:8080/mcp".to_string(), - envs: Envs::new(HashMap::new()), - env_keys: vec!["API_TOKEN".to_string()], - description: "oauth-ext".to_string(), - timeout: None, - socket: None, - client_id: Some("registered-client".to_string()), - client_secret_key: Some("OAUTH_CLIENT_SECRET".to_string()), - scopes: vec![], - bundled: None, - available_tools: Vec::new(), - headers: HashMap::new(), - }]), - sub_recipes: None, - settings: None, - activities: None, - author: None, - parameters: None, - response: None, - retry: None, - }; - - let secrets = discover_recipe_secrets(&recipe); - let keys: Vec<&str> = secrets.iter().map(|s| s.key.as_str()).collect(); - - assert!(keys.contains(&"API_TOKEN")); - assert!(keys.contains(&"OAUTH_CLIENT_SECRET")); - let client_secret = secrets - .iter() - .find(|s| s.key == "OAUTH_CLIENT_SECRET") - .unwrap(); - assert_eq!(client_secret.extension_name, "oauth-ext"); - } - - #[test] - fn test_secret_requirement_creation() { - let req = SecretRequirement::new("test-ext".to_string(), "API_TOKEN".to_string()); - - assert_eq!(req.key, "API_TOKEN"); - assert_eq!(req.extension_name, "test-ext"); - assert_eq!(req.description(), "Required by test-ext extension"); - } - - #[test] - fn test_discover_recipe_secrets_with_sub_recipes() { - use goose::recipe::SubRecipe; - - let recipe = Recipe { - version: "1.0.0".to_string(), - title: "Parent Recipe".to_string(), - description: "A recipe with sub-recipes".to_string(), - instructions: Some("Test instructions".to_string()), - prompt: None, - extensions: Some(vec![ExtensionConfig::StreamableHttp { - name: "parent-ext".to_string(), - uri: "http://localhost:8080/mcp".to_string(), - envs: Envs::new(HashMap::new()), - env_keys: vec!["PARENT_TOKEN".to_string()], - description: "parent-ext".to_string(), - timeout: None, - socket: None, - client_id: None, - client_secret_key: None, - scopes: vec![], - bundled: None, - available_tools: Vec::new(), - headers: HashMap::new(), - }]), - sub_recipes: Some(vec![SubRecipe { - name: "child-recipe".to_string(), - path: "path/to/child.yaml".to_string(), - values: None, - sequential_when_repeated: false, - description: None, - }]), - settings: None, - activities: None, - author: None, - parameters: None, - response: None, - retry: None, - }; - - let secrets = discover_recipe_secrets(&recipe); - - assert_eq!(secrets.len(), 1); - - let parent_secret = secrets.iter().find(|s| s.key == "PARENT_TOKEN").unwrap(); - assert_eq!(parent_secret.extension_name, "parent-ext"); - } -} diff --git a/documentation/docs/guides/recipes/recipe-reference.md b/documentation/docs/guides/recipes/recipe-reference.md index 48bbf8158..85a202e61 100644 --- a/documentation/docs/guides/recipes/recipe-reference.md +++ b/documentation/docs/guides/recipes/recipe-reference.md @@ -281,22 +281,14 @@ extensions: -#### Extension Secrets +#### Extension environment variables -This feature is only available through the CLI. +Extensions can declare the names of required environment variables in `env_keys`. goose resolves these values when the extension starts, using an environment variable first and then goose secret storage (the system keyring, or `secrets.yaml` when the keyring is disabled). -If a recipe uses an extension that requires a secret, goose can prompt users to provide the secret when running the recipe: - -1. When a recipe is loaded, goose scans all extensions (including those in subrecipes) for `env_keys` fields -2. If any required environment variables are missing from the secure keyring, goose prompts the user to enter them -3. Values are stored securely in the system keyring and reused for subsequent runs - -To update a stored secret, remove it from the system keyring and run the recipe again to be re-prompted. +Recipe loading does not prompt for missing values. Configure them before starting the recipe; if a required value is unavailable, the extension reports an initialization error. :::info -This feature is designed to prompt for and securely store secrets (such as API keys), but `env_keys` can include any environment variable needed by the extension (such as API endpoints, configuration values, etc.). - -Users can press `ESC` to skip entering a variable if it's optional for the extension. +`env_keys` can include secrets such as API keys as well as non-secret configuration such as API endpoints. ::: ### Parameters diff --git a/documentation/docs/guides/recipes/session-recipes.md b/documentation/docs/guides/recipes/session-recipes.md index 8a3e7e004..4ecf31499 100644 --- a/documentation/docs/guides/recipes/session-recipes.md +++ b/documentation/docs/guides/recipes/session-recipes.md @@ -397,11 +397,10 @@ You can customize how goose generates recipes by editing the `recipe.md` [prompt - :::info Privacy, Isolation, & Secrets + :::info Privacy & Isolation - Each person gets their own private session - No data is shared between users - Your session won't affect the original recipe creator's session - - The CLI can prompt users for required [extension secrets](/docs/guides/recipes/recipe-reference#extension-secrets) :::