fix: enforce ACP global config directory invariant

Build Config::init_global from the same config path stack as Config::default and return an error when an existing global config points at a different writable path. Validate that invariant from GooseAcpAgent::new so direct ACP callers cannot silently use a stale global config.

Update ACP fixtures to use one process-global test config directory and add a regression test for mismatched config directories.

Signed-off-by: Matt Toohey <contact@matttoohey.com>

Author: matt2e

crates/goose/src/acp/server.rs

@@ -951,6 +951,8 @@ impl GooseAcpAgent {
         disable_session_naming: bool,
         goose_platform: GoosePlatform,
     ) -> Result<Self> {
+        Config::init_global(config_dir.clone())?;
+
         let session_manager = Arc::new(SessionManager::new(data_dir));
 
         // Eagerly initialize the SQLite pool so it's ready when providers/sessions need it.

crates/goose/src/acp/server_factory.rs

@@ -21,7 +21,7 @@ impl AcpServer {
     }
 
     pub async fn create_agent(&self) -> Result<Arc<GooseAcpAgent>> {
-        let config = crate::config::Config::init_global(self.config.config_dir.clone());
+        let config = crate::config::Config::init_global(self.config.config_dir.clone())?;
 
         let goose_mode = config
             .get_goose_mode()

crates/goose/src/config/base.rs

@@ -52,6 +52,10 @@ pub enum ConfigError {
     KeyringError(String),
     #[error("Failed to lock config file: {0}")]
     LockError(String),
+    #[error(
+        "Global config already initialized with writable path {existing}; cannot use requested writable path {requested}"
+    )]
+    GlobalConfigPathMismatch { existing: String, requested: String },
     #[error("Secret stored using file-based fallback")]
     FallbackToFileStorage,
 }
@@ -163,43 +167,7 @@ fn bundled_defaults_path() -> Option<PathBuf> {
 
 impl Default for Config {
     fn default() -> Self {
-        let config_dir = Paths::config_dir();
-        let user_config_path = config_dir.join(CONFIG_YAML_NAME);
-
-        let mut config_paths = vec![system_config_path()];
-        if let Some(defaults) = bundled_defaults_path() {
-            config_paths.insert(0, defaults);
-        }
-        config_paths.push(user_config_path.clone());
-
-        let no_secrets_config = Self {
-            config_paths: config_paths.clone(),
-            secrets: SecretStorage::File {
-                path: Default::default(),
-            },
-            guard: Mutex::new(()),
-            secrets_cache: Arc::new(Mutex::new(None)),
-        };
-
-        let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
-            || no_secrets_config
-                .get_param::<serde_yaml::Value>("GOOSE_DISABLE_KEYRING")
-                .is_ok_and(|v| keyring_disabled_value(&v))
-        {
-            SecretStorage::File {
-                path: config_dir.join("secrets.yaml"),
-            }
-        } else {
-            SecretStorage::Keyring {
-                service: KEYRING_SERVICE.to_string(),
-            }
-        };
-        Self {
-            config_paths,
-            secrets,
-            guard: Mutex::new(()),
-            secrets_cache: Arc::new(Mutex::new(None)),
-        }
+        Self::with_config_dir(Paths::config_dir())
     }
 }
 
@@ -352,37 +320,77 @@ impl Config {
         GLOBAL_CONFIG.get_or_init(Config::default)
     }
 
-    /// Initialize the global configuration with a custom config directory.
-    ///
-    /// If the global instance has already been initialized (e.g. by a prior call
-    /// to `global()` or `init_global()`), this returns the existing instance and
-    /// the provided path is ignored. This is safe because Goose runs one ACP
-    /// server per process.
-    pub fn init_global(config_dir: PathBuf) -> &'static Config {
-        GLOBAL_CONFIG.get_or_init(|| {
-            let config_path = config_dir.join(CONFIG_YAML_NAME);
-
-            let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
-                || keyring_disabled_in_config(&config_path)
-            {
-                SecretStorage::File {
-                    path: config_dir.join("secrets.yaml"),
-                }
-            } else {
-                SecretStorage::Keyring {
-                    service: KEYRING_SERVICE.to_string(),
-                }
-            };
+    fn config_paths_for_dir(config_dir: &Path) -> Vec<PathBuf> {
+        let mut config_paths = vec![system_config_path()];
+        if let Some(defaults) = bundled_defaults_path() {
+            config_paths.insert(0, defaults);
+        }
+        config_paths.push(config_dir.join(CONFIG_YAML_NAME));
+        config_paths
+    }
+
+    fn with_config_dir(config_dir: PathBuf) -> Self {
+        let config_paths = Self::config_paths_for_dir(&config_dir);
+
+        let no_secrets_config = Self {
+            config_paths: config_paths.clone(),
+            secrets: SecretStorage::File {
+                path: Default::default(),
+            },
+            guard: Mutex::new(()),
+            secrets_cache: Arc::new(Mutex::new(None)),
+        };
 
-            Config {
-                config_path,
-                secrets,
-                guard: Mutex::new(()),
-                secrets_cache: Arc::new(Mutex::new(None)),
+        let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
+            || no_secrets_config
+                .get_param::<serde_yaml::Value>("GOOSE_DISABLE_KEYRING")
+                .is_ok_and(|v| keyring_disabled_value(&v))
+        {
+            SecretStorage::File {
+                path: config_dir.join("secrets.yaml"),
             }
+        } else {
+            SecretStorage::Keyring {
+                service: KEYRING_SERVICE.to_string(),
+            }
+        };
+
+        Self {
+            config_paths,
+            secrets,
+            guard: Mutex::new(()),
+            secrets_cache: Arc::new(Mutex::new(None)),
+        }
+    }
+
+    fn ensure_writable_path(&self, requested: &Path) -> Result<(), ConfigError> {
+        if self.write_path() == requested {
+            return Ok(());
+        }
+
+        Err(ConfigError::GlobalConfigPathMismatch {
+            existing: self.write_path().display().to_string(),
+            requested: requested.display().to_string(),
         })
     }
 
+    /// Initialize the global configuration with a custom config directory.
+    ///
+    /// If the global instance has already been initialized, this validates that
+    /// the requested config directory resolves to the same writable config path.
+    pub fn init_global(config_dir: PathBuf) -> Result<&'static Config, ConfigError> {
+        let requested = config_dir.join(CONFIG_YAML_NAME);
+
+        if let Some(config) = GLOBAL_CONFIG.get() {
+            config.ensure_writable_path(&requested)?;
+            return Ok(config);
+        }
+
+        let config = GLOBAL_CONFIG.get_or_init(|| Self::with_config_dir(config_dir));
+        config.ensure_writable_path(&requested)?;
+        Ok(config)
+    }
+
     /// Create a new configuration instance with custom paths
     ///
     /// This is primarily useful for testing or for applications that need

crates/goose/tests/acp_common_tests/mod.rs

@@ -948,18 +948,19 @@ pub async fn run_permission_persistence<C: Connection>() {
     };
 
     let mut conn = C::new(config, openai).await;
+    let permission_path = conn.permission_config_path();
     let SessionData { mut session, .. } = conn.new_session().await.unwrap();
     expected_session_id.set(&session.session_id().0);
 
     for (decision, expected_status, expected_yaml) in cases {
         conn.reset_openai();
         conn.reset_permissions();
-        let _ = fs::remove_file(temp_dir.path().join("permission.yaml"));
+        let _ = fs::remove_file(&permission_path);
         let output = session.prompt(prompt, decision).await.unwrap();
 
         assert_eq!(output.tool_status.unwrap(), expected_status);
         assert_eq!(
-            fs::read_to_string(temp_dir.path().join("permission.yaml")).unwrap_or_default(),
+            fs::read_to_string(&permission_path).unwrap_or_default(),
             expected_yaml,
         );
     }

crates/goose/tests/acp_fixtures/mod.rs

@@ -23,8 +23,8 @@ use sacp::schema::{
 };
 use std::collections::VecDeque;
 use std::future::Future;
-use std::path::PathBuf;
-use std::sync::{Arc, Mutex};
+use std::path::{Path, PathBuf};
+use std::sync::{Arc, Mutex, OnceLock};
 use tokio::task::JoinHandle;
 use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
 use wiremock::matchers::{method, path};
@@ -130,6 +130,41 @@ pub type DuplexTransport = sacp::ByteStreams<
     tokio_util::compat::Compat<tokio::io::DuplexStream>,
 >;
 
+static ACP_TEST_CONFIG_DIR: OnceLock<tempfile::TempDir> = OnceLock::new();
+static ACP_TEST_LOCK: Mutex<()> = Mutex::new(());
+
+fn acp_test_config_dir() -> PathBuf {
+    if std::env::var_os("GOOSE_PATH_ROOT").is_some() {
+        return Paths::config_dir();
+    }
+
+    ACP_TEST_CONFIG_DIR
+        .get_or_init(|| tempfile::tempdir().unwrap())
+        .path()
+        .to_path_buf()
+}
+
+fn prepare_acp_config_dir(data_root: &Path, config_dir: &Path, current_model: &str) {
+    fs::create_dir_all(config_dir).unwrap();
+
+    let source_config = data_root.join(goose::config::base::CONFIG_YAML_NAME);
+    let target_config = config_dir.join(goose::config::base::CONFIG_YAML_NAME);
+
+    if source_config.exists() {
+        if source_config != target_config {
+            fs::copy(source_config, target_config).unwrap();
+        }
+    } else {
+        fs::write(
+            target_config,
+            format!("GOOSE_MODEL: {current_model}\nGOOSE_PROVIDER: openai\n"),
+        )
+        .unwrap();
+    }
+
+    let _ = fs::remove_file(config_dir.join("permission.yaml"));
+}
+
 /// Wires up duplex streams, spawns `serve` for the given agent, and returns
 /// a ready-to-use sacp transport plus the server handle.
 #[allow(dead_code)]
@@ -161,14 +196,8 @@ pub async fn spawn_acp_server_in_process(
     fs::create_dir_all(data_root).unwrap();
     // TODO: Paths::in_state_dir is global, ignoring per-test data_root
     fs::create_dir_all(Paths::in_state_dir("logs")).unwrap();
-    let config_path = data_root.join(goose::config::base::CONFIG_YAML_NAME);
-    if !config_path.exists() {
-        fs::write(
-            &config_path,
-            format!("GOOSE_MODEL: {current_model}\nGOOSE_PROVIDER: openai\n"),
-        )
-        .unwrap();
-    }
+    let config_dir = acp_test_config_dir();
+    prepare_acp_config_dir(data_root, &config_dir, current_model);
     let provider_factory = provider_factory.unwrap_or_else(|| {
         let base_url = openai_base_url.to_string();
         Arc::new(move |_provider_name, model_config, _extensions| {
@@ -188,7 +217,7 @@ pub async fn spawn_acp_server_in_process(
         provider_factory,
         builtins.to_vec(),
         data_root.to_path_buf(),
-        data_root.to_path_buf(),
+        config_dir,
         goose_mode,
         true,
         GoosePlatform::GooseCli,
@@ -526,6 +555,7 @@ pub trait Connection: Sized {
         value: &str,
     ) -> anyhow::Result<()>;
     fn data_root(&self) -> std::path::PathBuf;
+    fn permission_config_path(&self) -> std::path::PathBuf;
     fn reset_openai(&self);
     fn reset_permissions(&self);
 }
@@ -556,6 +586,8 @@ where
 {
     register_builtin_extensions(goose_mcp::BUILTIN_EXTENSIONS.clone());
 
+    let _guard = ACP_TEST_LOCK.lock().unwrap_or_else(|err| err.into_inner());
+
     let handle = std::thread::Builder::new()
         .name("acp-test".to_string())
         .stack_size(8 * 1024 * 1024)

crates/goose/tests/acp_fixtures/provider.rs

@@ -287,6 +287,10 @@ impl Connection for AcpProviderConnection {
         self.data_root.clone()
     }
 
+    fn permission_config_path(&self) -> std::path::PathBuf {
+        self.permission_manager.get_config_path().to_path_buf()
+    }
+
     async fn set_mode(&self, _session_id: &str, _mode_id: &str) -> anyhow::Result<()> {
         Err(anyhow::anyhow!("not implemented for AcpProviderConnection"))
     }

crates/goose/tests/acp_fixtures/server.rs

@@ -439,6 +439,10 @@ impl Connection for AcpServerConnection {
         self.data_root.clone()
     }
 
+    fn permission_config_path(&self) -> std::path::PathBuf {
+        self.permission_manager.get_config_path().to_path_buf()
+    }
+
     fn reset_openai(&self) {
         self._openai.reset();
     }

crates/goose/tests/acp_global_config_test.rs

@@ -0,0 +1,52 @@
+use goose::acp::server::{AcpProviderFactory, GooseAcpAgent};
+use goose::agents::GoosePlatform;
+use goose::config::base::CONFIG_YAML_NAME;
+use goose::config::{Config, GooseMode};
+use goose::providers::base::Provider;
+use std::sync::Arc;
+
+#[tokio::test]
+async fn acp_agent_rejects_different_config_dir_after_global_initialization() {
+    let first = tempfile::tempdir().unwrap();
+    let second = tempfile::tempdir().unwrap();
+    let data = tempfile::tempdir().unwrap();
+
+    let first_config = first.path().join(CONFIG_YAML_NAME);
+    let second_config = second.path().join(CONFIG_YAML_NAME);
+
+    let config = Config::init_global(first.path().to_path_buf()).unwrap();
+    assert_eq!(config.path(), first_config.display().to_string());
+
+    let provider_factory: AcpProviderFactory = Arc::new(|_, _, _| {
+        Box::pin(async { Err::<Arc<dyn Provider>, _>(anyhow::anyhow!("provider not used")) })
+    });
+
+    let err = match GooseAcpAgent::new(
+        provider_factory,
+        Vec::new(),
+        data.path().to_path_buf(),
+        second.path().to_path_buf(),
+        GooseMode::Auto,
+        true,
+        GoosePlatform::GooseCli,
+    )
+    .await
+    {
+        Ok(_) => panic!("expected config directory mismatch"),
+        Err(err) => err,
+    };
+
+    let message = err.to_string();
+    assert!(
+        message.contains("Global config already initialized with writable path"),
+        "{message}"
+    );
+    assert!(
+        message.contains(&first_config.display().to_string()),
+        "{message}"
+    );
+    assert!(
+        message.contains(&second_config.display().to_string()),
+        "{message}"
+    );
+}