From 96fa25bce88613b0ed018612688d73328a912e8b Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 28 Apr 2026 07:57:32 +1000 Subject: [PATCH] fix: remove defaults yaml (#8854) --- crates/goose-server/src/commands/agent.rs | 2 + crates/goose/src/config/base.rs | 142 +--------------------- crates/goose/src/security/mod.rs | 24 ++++ 3 files changed, 28 insertions(+), 140 deletions(-) diff --git a/crates/goose-server/src/commands/agent.rs b/crates/goose-server/src/commands/agent.rs index 03f9ac1d5d..8e0dbb1336 100644 --- a/crates/goose-server/src/commands/agent.rs +++ b/crates/goose-server/src/commands/agent.rs @@ -42,6 +42,8 @@ pub async fn run() -> Result<()> { boot_marker("main entered"); crate::logging::setup_logging(Some("goosed"))?; + goose::security::set_security_defaults(); + let settings = configuration::Settings::new()?; let secret_key = std::env::var("GOOSE_SERVER__SECRET_KEY") diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 93b562ca9c..56b2764323 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -122,7 +122,6 @@ impl From for ConfigError { /// For goose-specific configuration, consider prefixing with "goose_" to avoid conflicts. pub struct Config { config_path: PathBuf, - defaults_path: Option, secrets: SecretStorage, guard: Mutex<()>, secrets_cache: Arc>>>, @@ -142,16 +141,6 @@ impl Default for Config { let config_path = config_dir.join(CONFIG_YAML_NAME); - let defaults_path = find_workspace_or_exe_root().and_then(|root| { - let path = root.join("defaults.yaml"); - if path.exists() { - tracing::info!("Found bundled defaults.yaml at: {:?}", path); - Some(path) - } else { - None - } - }); - let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok() || keyring_disabled_in_config(&config_path) { @@ -165,7 +154,6 @@ impl Default for Config { }; Config { config_path, - defaults_path, secrets, guard: Mutex::new(()), secrets_cache: Arc::new(Mutex::new(None)), @@ -284,7 +272,6 @@ impl Config { pub fn new>(config_path: P, service: &str) -> Result { Ok(Config { config_path: config_path.as_ref().to_path_buf(), - defaults_path: None, secrets: SecretStorage::Keyring { service: service.to_string(), }, @@ -303,23 +290,6 @@ impl Config { ) -> Result { Ok(Config { config_path: config_path.as_ref().to_path_buf(), - defaults_path: None, - secrets: SecretStorage::File { - path: secrets_path.as_ref().to_path_buf(), - }, - guard: Mutex::new(()), - secrets_cache: Arc::new(Mutex::new(None)), - }) - } - - pub fn new_with_defaults, P2: AsRef, P3: AsRef>( - config_path: P1, - secrets_path: P2, - defaults_path: P3, - ) -> Result { - Ok(Config { - config_path: config_path.as_ref().to_path_buf(), - defaults_path: Some(defaults_path.as_ref().to_path_buf()), secrets: SecretStorage::File { path: secrets_path.as_ref().to_path_buf(), }, @@ -369,9 +339,7 @@ impl Config { } fn load(&self) -> Result { - let mut values = self.load_raw()?; - self.merge_missing_defaults(&mut values); - Ok(values) + self.load_raw() } pub fn all_values(&self) -> Result, ConfigError> { @@ -712,7 +680,6 @@ impl Config { /// This will attempt to get the value from (in order): /// 1. Environment variable with the uppercase key name /// 2. Configuration file (~/.config/goose/config.yaml) - /// 3. Bundled defaults file (defaults.yaml in workspace root or executable directory) /// /// The value will be deserialized into the requested type. This works with /// both simple types (String, i32, etc.) and complex types that implement @@ -738,24 +705,6 @@ impl Config { .and_then(|v| Ok(serde_yaml::from_value(v.clone())?)) } - fn load_defaults(&self) -> Option { - let path = self.defaults_path.as_ref()?; - let content = std::fs::read_to_string(path).ok()?; - parse_yaml_content(&content).ok() - } - - fn merge_missing_defaults(&self, values: &mut Mapping) { - let Some(defaults) = self.load_defaults() else { - return; - }; - - for (key, default_value) in defaults { - if !values.contains_key(&key) { - values.insert(key, default_value); - } - } - } - /// Set a configuration value in the config file (non-secret). /// /// This will immediately write the value to the config file. The value @@ -1889,101 +1838,14 @@ mod tests { Config::new_with_file_secrets(config_file.path(), secrets_file.path()).unwrap() } - fn new_test_config_with_defaults(defaults_content: &str) -> (Config, NamedTempFile) { - let config_file = NamedTempFile::new().unwrap(); - let secrets_file = NamedTempFile::new().unwrap(); - let defaults_file = NamedTempFile::new().unwrap(); - std::fs::write(defaults_file.path(), defaults_content).unwrap(); - let config = Config::new_with_defaults( - config_file.path(), - secrets_file.path(), - defaults_file.path(), - ) - .unwrap(); - (config, defaults_file) - } - #[test] - fn test_defaults_fallback_when_key_not_in_config() -> Result<(), ConfigError> { - let (config, _defaults) = - new_test_config_with_defaults("SECURITY_PROMPT_ENABLED: true\nsome_key: default_val"); - - // Key only in defaults → returns defaults value - let value: bool = config.get_param("SECURITY_PROMPT_ENABLED")?; - assert!(value); - - let value: String = config.get_param("some_key")?; - assert_eq!(value, "default_val"); - - Ok(()) - } - - #[test] - #[serial] - fn test_full_precedence_env_over_config_over_defaults() -> Result<(), ConfigError> { - let (config, _defaults) = new_test_config_with_defaults("my_key: from_defaults"); - - // Only defaults → returns defaults - let value: String = config.get_param("my_key")?; - assert_eq!(value, "from_defaults"); - - // Config file overrides defaults - config.set_param("my_key", "from_config")?; - let value: String = config.get_param("my_key")?; - assert_eq!(value, "from_config"); - - // Env var overrides config file (and defaults) - std::env::set_var("MY_KEY", "from_env"); - let value: String = config.get_param("my_key")?; - assert_eq!(value, "from_env"); - std::env::remove_var("MY_KEY"); - - // After removing env var, config file value is back - let value: String = config.get_param("my_key")?; - assert_eq!(value, "from_config"); - - Ok(()) - } - - #[test] - fn test_no_defaults_file_behaves_as_before() { - // Config without defaults (the normal open-source case) + fn test_missing_key_returns_not_found() { let config = new_test_config(); let result: Result = config.get_param("nonexistent_key"); assert!(matches!(result, Err(ConfigError::NotFound(_)))); } - #[test] - fn test_defaults_not_persisted_on_write() -> Result<(), ConfigError> { - let (config, _defaults) = new_test_config_with_defaults("default_key: default_value"); - - // Read a default value (should work) - let value: String = config.get_param("default_key")?; - assert_eq!(value, "default_value"); - - // Write a different key - config.set_param("user_key", "user_value")?; - - // Read config file directly - should NOT contain default_key - let config_path = PathBuf::from(config.path()); - let file_content = std::fs::read_to_string(&config_path)?; - assert!( - !file_content.contains("default_key"), - "Defaults should not be persisted to config file on write" - ); - assert!( - file_content.contains("user_key"), - "User's key should be in config file" - ); - - // But reading via get_param should still return the default - let value: String = config.get_param("default_key")?; - assert_eq!(value, "default_value"); - - Ok(()) - } - #[test] #[cfg(unix)] fn test_secrets_file_created_with_restricted_permissions() -> Result<(), ConfigError> { diff --git a/crates/goose/src/security/mod.rs b/crates/goose/src/security/mod.rs index 12a667073f..c6938f2ceb 100644 --- a/crates/goose/src/security/mod.rs +++ b/crates/goose/src/security/mod.rs @@ -13,6 +13,29 @@ use scanner::PromptInjectionScanner; use std::sync::OnceLock; use uuid::Uuid; +fn set_default_if_not_exist(config: &Config, key: &str, default_env: &str) { + if config.get_param::(key).is_ok() { + return; + } + if let Ok(parsed) = config.get_param::(default_env) { + let _ = config.set_param(key, parsed); + } +} + +pub fn set_security_defaults() { + let config = Config::global(); + set_default_if_not_exist( + config, + "SECURITY_PROMPT_ENABLED", + "DEFAULT_SECURITY_PROMPT_ENABLED", + ); + set_default_if_not_exist( + config, + "SECURITY_COMMAND_CLASSIFIER_ENABLED", + "DEFAULT_SECURITY_COMMAND_CLASSIFIER_ENABLED", + ); +} + pub struct SecurityManager { scanner: OnceLock, } @@ -29,6 +52,7 @@ pub struct SecurityResult { impl SecurityManager { pub fn new() -> Self { + set_security_defaults(); Self { scanner: OnceLock::new(), }