diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 6097c275a..a62eff31f 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -150,7 +150,15 @@ enum SecretStorage { // Global instance static GLOBAL_CONFIG: OnceCell = OnceCell::new(); +#[cfg(test)] +pub(crate) const TEST_SYSTEM_CONFIG_PATH_ENV: &str = "GOOSE_TEST_SYSTEM_CONFIG_PATH"; + fn system_config_path() -> PathBuf { + #[cfg(test)] + if let Some(path) = env::var_os(TEST_SYSTEM_CONFIG_PATH_ENV) { + return path.into(); + } + #[cfg(unix)] { PathBuf::from("/etc/goose/config.yaml") @@ -169,6 +177,25 @@ fn additional_config_paths_from_env() -> Vec { .unwrap_or_default() } +fn metadata_is_symlink_or_reparse_point(metadata: &std::fs::Metadata) -> bool { + if metadata.file_type().is_symlink() { + return true; + } + + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + + const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x400; + metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 + } + + #[cfg(not(windows))] + { + false + } +} + impl Default for Config { fn default() -> Self { let config_dir = Paths::config_dir(); @@ -519,6 +546,37 @@ impl Config { Ok(merged) } + fn load_strict(&self) -> Result { + let mut merged = Mapping::new(); + + for path in &self.config_paths { + match std::fs::symlink_metadata(path) { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + for ancestor in path.ancestors().skip(1) { + match std::fs::symlink_metadata(ancestor) { + Ok(metadata) if metadata_is_symlink_or_reparse_point(&metadata) => { + std::fs::metadata(ancestor)?; + } + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + } + continue; + } + Err(error) => return Err(error.into()), + } + let content = std::fs::read_to_string(path)?; + let layer = parse_yaml_content(&content)?; + merge_config_values(&mut merged, layer); + } + + crate::config::migrations::run_read_migrations(&mut merged); + + Ok(merged) + } + pub fn all_values(&self) -> Result, ConfigError> { let config_values = self.load()?; let mut map = HashMap::from_iter(config_values.into_iter().filter_map(|(k, v)| { @@ -1107,6 +1165,26 @@ config_value!(CHATGPT_CODEX_REASONING_EFFORT, String, "medium"); config_value!(GOOSE_SEARCH_PATHS, Vec); config_value!(GOOSE_MODE, GooseMode); +impl Config { + pub(crate) fn get_goose_mode_strict(&self) -> Result { + match env::var("GOOSE_MODE") { + Ok(value) => { + let value = Self::parse_env_value(&value)?; + Ok(serde_json::from_value(value)?) + } + Err(env::VarError::NotPresent) => { + let values = self.load_strict()?; + let value = values + .get("GOOSE_MODE") + .ok_or_else(|| ConfigError::NotFound("GOOSE_MODE".to_string()))?; + Ok(serde_yaml::from_value(value.clone())?) + } + Err(env::VarError::NotUnicode(_)) => Err(ConfigError::DeserializeError( + "GOOSE_MODE contains non-Unicode data".to_string(), + )), + } + } +} // GOOSE_PROVIDER and GOOSE_MODEL are handled by crate::config::providers // which checks the structured `providers:` block first and falls back to // the legacy flat keys. The accessors below delegate to that module. diff --git a/crates/goose/src/providers/codex_acp.rs b/crates/goose/src/providers/codex_acp.rs index 29d618af5..bb00df192 100644 --- a/crates/goose/src/providers/codex_acp.rs +++ b/crates/goose/src/providers/codex_acp.rs @@ -7,7 +7,7 @@ use crate::acp::{ extension_configs_to_mcp_servers, AcpProvider, AcpProviderConfig, ACP_CURRENT_MODEL, }; use crate::config::search_path::SearchPaths; -use crate::config::{Config, GooseMode}; +use crate::config::{Config, ConfigError, GooseMode}; use crate::providers::base::{ current_working_dir, ProviderDef, ProviderDescriptor, ProviderMetadata, }; @@ -18,6 +18,16 @@ const CODEX_ACP_DOC_URL: &str = "https://github.com/agentclientprotocol/codex-ac pub struct CodexAcpProvider; +fn resolve_goose_mode( + configured_mode: Result, +) -> Result { + match configured_mode { + Ok(mode) => Ok(mode), + Err(ConfigError::NotFound(_)) => Ok(GooseMode::Auto), + Err(error) => Err(error), + } +} + impl goose_providers::base::ProviderDescriptor for CodexAcpProvider { fn metadata() -> ProviderMetadata { ProviderMetadata::new( @@ -65,7 +75,7 @@ impl ProviderDef for CodexAcpProvider { let resolved_command = SearchPaths::builder() .with_npm() .resolve(CODEX_ACP_PROVIDER_NAME)?; - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = resolve_goose_mode(config.get_goose_mode_strict())?; let mcp_servers = extension_configs_to_mcp_servers(&extensions); let mode_mapping = HashMap::from([ @@ -94,3 +104,191 @@ impl ProviderDef for CodexAcpProvider { }) } } + +#[cfg(test)] +mod tests { + use super::*; + #[cfg(unix)] + use std::fs; + #[cfg(unix)] + use std::os::unix::fs::{symlink, PermissionsExt}; + #[cfg(unix)] + use std::process::Command; + + #[cfg(unix)] + const CHILD_ENV: &str = "GOOSE_CODEX_ACP_MODE_TEST_CHILD"; + #[cfg(unix)] + const EXPECT_CONFIG_ERROR_ENV: &str = "GOOSE_CODEX_ACP_EXPECT_CONFIG_ERROR"; + + #[cfg(unix)] + #[derive(Clone, Copy)] + enum ConfigFixture { + Absent, + File(&'static str), + Directory, + DanglingSymlink, + DanglingParentSymlink, + } + + #[test] + fn missing_goose_mode_defaults_to_auto() { + assert_eq!( + resolve_goose_mode(Err(ConfigError::NotFound("GOOSE_MODE".to_string()))).unwrap(), + GooseMode::Auto + ); + } + + #[test] + fn configured_goose_modes_are_preserved() { + for mode in [ + GooseMode::Auto, + GooseMode::SmartApprove, + GooseMode::Approve, + GooseMode::Chat, + ] { + assert_eq!(resolve_goose_mode(Ok(mode)).unwrap(), mode); + } + } + + #[test] + fn invalid_goose_mode_errors_are_preserved() { + assert!(matches!( + resolve_goose_mode(Err(ConfigError::DeserializeError("invalid".to_string()))), + Err(ConfigError::DeserializeError(_)) + )); + assert!(matches!( + resolve_goose_mode(Err(ConfigError::FileError(std::io::Error::other( + "unreadable" + )))), + Err(ConfigError::FileError(_)) + )); + } + + #[cfg(unix)] + #[tokio::test] + async fn goose_mode_validation_precedes_codex_acp_launch() { + if std::env::var_os(CHILD_ENV).is_some() { + let error = CodexAcpProvider::from_env(vec![], None) + .await + .expect_err("marker executable should fail ACP initialization"); + let is_config_error = matches!( + error.downcast_ref::(), + Some(ConfigError::DeserializeError(_) | ConfigError::FileError(_)) + ); + assert_eq!( + is_config_error, + std::env::var_os(EXPECT_CONFIG_ERROR_ENV).is_some(), + "unexpected provider error: {error:#}" + ); + return; + } + + let fixture = tempfile::tempdir().unwrap(); + let executable = fixture.path().join(CODEX_ACP_PROVIDER_NAME); + fs::write( + &executable, + "#!/bin/sh\n: > \"$GOOSE_CODEX_ACP_MARKER\"\nexit 1\n", + ) + .unwrap(); + fs::set_permissions(&executable, fs::Permissions::from_mode(0o700)).unwrap(); + + let search_paths = serde_json::to_string(&vec![fixture.path()]).unwrap(); + for (name, mode, config_fixture, should_launch) in [ + ("unset", None, ConfigFixture::Absent, true), + ("auto", Some("auto"), ConfigFixture::Absent, true), + ( + "smart_approve", + Some("smart_approve"), + ConfigFixture::Absent, + true, + ), + ("approve", Some("approve"), ConfigFixture::Absent, true), + ("chat", Some("chat"), ConfigFixture::Absent, true), + ("invalid", Some("invalid"), ConfigFixture::Absent, false), + ( + "configured_file", + None, + ConfigFixture::File("GOOSE_MODE: approve\n"), + true, + ), + ( + "malformed_file", + None, + ConfigFixture::File("GOOSE_MODE: ["), + false, + ), + ("unreadable_file", None, ConfigFixture::Directory, false), + ( + "dangling_symlink", + None, + ConfigFixture::DanglingSymlink, + false, + ), + ( + "dangling_parent_symlink", + None, + ConfigFixture::DanglingParentSymlink, + false, + ), + ] { + let marker = fixture.path().join(format!("launched-{name}")); + let path_root = fixture.path().join(format!("config-{name}")); + let config_dir = path_root.join("config"); + fs::create_dir_all(&config_dir).unwrap(); + let config_path = config_dir.join("config.yaml"); + match config_fixture { + ConfigFixture::Absent => {} + ConfigFixture::File(content) => fs::write(&config_path, content).unwrap(), + ConfigFixture::Directory => fs::create_dir(&config_path).unwrap(), + ConfigFixture::DanglingSymlink => { + symlink(config_dir.join("missing.yaml"), &config_path).unwrap() + } + ConfigFixture::DanglingParentSymlink => { + fs::remove_dir(&config_dir).unwrap(); + symlink(path_root.join("missing-config"), &config_dir).unwrap(); + } + } + let mut command = Command::new(std::env::current_exe().unwrap()); + command + .arg("--exact") + .arg("providers::codex_acp::tests::goose_mode_validation_precedes_codex_acp_launch") + .arg("--nocapture") + .env(CHILD_ENV, "1") + .env("GOOSE_SEARCH_PATHS", &search_paths) + .env("GOOSE_CODEX_ACP_MARKER", &marker) + .env("GOOSE_PATH_ROOT", &path_root) + .env( + crate::config::base::TEST_SYSTEM_CONFIG_PATH_ENV, + path_root.join("system-config.yaml"), + ) + .env_remove("GOOSE_ADDITIONAL_CONFIG_FILES") + .env("GOOSE_DISABLE_KEYRING", "1"); + if should_launch { + command.env_remove(EXPECT_CONFIG_ERROR_ENV); + } else { + command.env(EXPECT_CONFIG_ERROR_ENV, "1"); + } + match mode { + Some(mode) => { + command.env("GOOSE_MODE", mode); + } + None => { + command.env_remove("GOOSE_MODE"); + } + } + let output = command.output().unwrap(); + + assert!( + output.status.success(), + "{name} child test failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + marker.exists(), + should_launch, + "unexpected codex-acp launch result for {name} GOOSE_MODE" + ); + } + } +}