diff --git a/.github/workflows/_ci-rust.yml b/.github/workflows/_ci-rust.yml index f49cc96eb2e..e25639d3ac7 100644 --- a/.github/workflows/_ci-rust.yml +++ b/.github/workflows/_ci-rust.yml @@ -151,6 +151,9 @@ jobs: # Serial: windows_resolver_tests mutate process-global env # (BUZZ_SHELL/GIT_BASH/SystemRoot) that SharedState::new reads. run: cargo test -p buzz-dev-mcp --target $env:TARGET -- --test-threads=1 + - name: Test (ACP launch-prefix platform contract) + # Execute the non-Unix refusal branch; a cross-compile cannot test it. + run: cargo test -p buzz-acp --target $env:TARGET --lib acp::launch::platform_tests - name: Test (buzz-agent auth coordinator) # The auth coordinator single-flights on an OS advisory lock, which is # LockFileEx on Windows; this integration suite drives real second diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index 7e7b3e5ff00..efcfd3cd972 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -197,6 +197,34 @@ buzz-acp --respond-to anyone buzz-acp --respond-to nobody --heartbeat-interval 300 ``` +### Host-controlled launch wrapper + +On Unix, a trusted host can set `BUZZ_ACP_LAUNCH_PREFIX` to a JSON argument array, for +example `["/absolute/launcher", "--policy", "/absolute/policy.json", "--"]`. +Keep `BUZZ_ACP_AGENT_COMMAND` set to the real worker (such as Goose). The harness +launches `prefix... worker normalized-args...` without a shell, preserving worker +identity for argument defaults, environment setup, managed skills and ACP handling. + +Configured prefixes are rejected on non-Unix platforms, including Windows, because +per-worker process-tree cleanup is not available there. Unset direct launches +remain available on every platform. + +Hosts must reserve this key against user-supplied environment settings and apply +the trusted prefix after merging user environment. Desktop's shared reserved-key +filter covers saved persona/agent settings and local/remote launch environments. + +The first prefix element must be an absolute executable path. Unset means direct +launch; empty, malformed or unusable prefixes fail launch with no direct fallback. +The shared spawn path applies the prefix to every worker creation, including pool +startup, wake, crash replacement, local tasks, model discovery and authentication. +The variable is removed from the child's environment to avoid recursive wrapping. + +The wrapper must preserve ACP stdin/stdout and either exec the worker or supervise +it within the inherited process group. This hook does not enforce a sandbox or +verify executable contents. The host owns policy, immutable launcher staging and +its lifetime across delayed launches/restarts. Hosts requiring this hook must pin +a supporting runtime; older binaries do not understand the environment setting. + ### Configuration Examples **Single agent, no heartbeat (default):** diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index eea500f6697..0efcea5cad4 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -8,6 +8,8 @@ //! 4. [`AcpClient::session_prompt_with_idle_timeout`] — send prompt with idle/hard deadline, return stop reason //! 5. [`AcpClient::session_cancel`] / [`AcpClient::cancel_with_cleanup`] — cancel in-flight turn +mod launch; + use futures_util::StreamExt; use tokio::io::AsyncWriteExt; use tokio::process::{Child, ChildStdin, ChildStdout}; @@ -474,8 +476,7 @@ impl AcpClient { ) -> Result { use std::process::Stdio; - let mut cmd = tokio::process::Command::new(command); - cmd.args(args); + let mut cmd = launch::command(command, args)?; if crate::config::normalize_agent_command_identity(command) == BUZZ_PI_ACP_NAME { if !args.iter().any(|arg| arg == "--") { cmd.arg("--"); @@ -585,7 +586,13 @@ impl AcpClient { _ => None, }; cmd.envs(launch_env.iter().cloned()); - let mut child = cmd.spawn()?; + cmd.env_remove(launch::PREFIX_ENV); + let mut child = cmd.spawn().map_err(|error| { + std::io::Error::new( + error.kind(), + format!("failed to spawn {:?}: {error}", cmd.as_std().get_program()), + ) + })?; let stdin = child .stdin diff --git a/crates/buzz-acp/src/acp/launch.rs b/crates/buzz-acp/src/acp/launch.rs new file mode 100644 index 00000000000..2f4f8ee1c3d --- /dev/null +++ b/crates/buzz-acp/src/acp/launch.rs @@ -0,0 +1,83 @@ +//! Host-controlled wrapping at the shared ACP subprocess boundary. +use super::AcpError; +use tokio::process::Command; + +pub(super) const PREFIX_ENV: &str = "BUZZ_ACP_LAUNCH_PREFIX"; + +pub(super) fn command(worker: &str, args: &[String]) -> Result { + command_with_prefix(worker, args, std::env::var_os(PREFIX_ENV)) +} + +fn command_with_prefix( + worker: &str, + args: &[String], + prefix: Option, +) -> Result { + // Non-Unix cleanup only owns the immediate child, not a supervised worker. + if prefix.is_some() && !cfg!(unix) { + return Err(AcpError::Protocol(format!( + "{PREFIX_ENV} is supported only on Unix; refusing to launch an uncontained worker" + ))); + } + let mut command = match prefix { + None => Command::new(worker), + Some(value) => { + let invalid = || { + AcpError::Protocol(format!( + "{PREFIX_ENV} must be a nonempty JSON string array with an absolute executable path" + )) + }; + let prefix: Vec = + serde_json::from_str(&value.into_string().map_err(|_| invalid())?) + .map_err(|_| invalid())?; + let executable = prefix + .first() + .filter(|path| std::path::Path::new(path).is_absolute()) + .ok_or_else(invalid)?; + if prefix.iter().any(|arg| arg.contains('\0')) { + return Err(invalid()); + } + let mut command = Command::new(executable); + command.args(&prefix[1..]).arg(worker); + command + } + }; + command.args(args); + Ok(command) +} + +#[cfg(all(test, unix))] +mod tests; + +#[cfg(test)] +mod platform_tests { + use super::*; + + #[test] + fn unset_prefix_keeps_direct_launch_on_every_platform() { + let command = command_with_prefix("worker", &["arg".into()], None).unwrap(); + assert_eq!(command.as_std().get_program(), "worker"); + assert_eq!(command.as_std().get_args().collect::>(), ["arg"]); + } + + #[test] + fn configured_prefix_requires_unix_process_containment() { + // A real absolute executable prevents a bad path from hiding the guard. + let executable = std::env::current_exe().unwrap(); + let prefix = serde_json::to_string(&[&executable]).unwrap(); + let result = command_with_prefix("worker", &[], Some(prefix.into())); + if cfg!(unix) { + assert_eq!( + result.unwrap().as_std().get_program(), + executable.as_os_str() + ); + } else { + assert!(result + .unwrap_err() + .to_string() + .contains("supported only on Unix")); + // Even an empty setting must refuse rather than downgrade to direct launch. + assert!(command_with_prefix("worker", &[], Some("".into())).is_err()); + } + } +} diff --git a/crates/buzz-acp/src/acp/launch/tests.rs b/crates/buzz-acp/src/acp/launch/tests.rs new file mode 100644 index 00000000000..be2a08e85af --- /dev/null +++ b/crates/buzz-acp/src/acp/launch/tests.rs @@ -0,0 +1,225 @@ +//! Subprocess isolation keeps the harness environment immutable in parallel tests. +use super::PREFIX_ENV; +use crate::{ + acp::AcpClient, + config::{CliArgs, Config}, + usage::StandardAdapterKind, +}; +use clap::Parser; +use serde_json::{json, Value}; +use std::{fs, os::unix::fs::PermissionsExt, path::PathBuf, process::Command}; + +const CHILD: &str = "BUZZ_LAUNCH_TEST_CHILD"; +const LITERAL: &str = "policy with spaces, ; $(not-a-shell)"; + +struct Fixture(PathBuf); +impl Fixture { + fn new() -> Self { + let dir = std::env::temp_dir().join(format!("buzz launch {}", uuid::Uuid::new_v4())); + fs::create_dir(&dir).unwrap(); + let wrapper = dir.join("wrapper.py"); + fs::write( + &wrapper, + r#"import json, os, sys +with open('wrapper.jsonl', 'a') as log: + log.write(json.dumps(sys.argv[1:]) + '\n') +os.execv(sys.argv[2], sys.argv[2:]) +"#, + ) + .unwrap(); + for name in [ + "goose", + "codex-acp", + "claude-agent-acp", + "hermes-acp", + "buzz-pi-acp", + ] { + let path = dir.join(name); + fs::write( + &path, + r#"#!/usr/bin/env python3 +import json, os, sys +with open('worker-started', 'a') as log: + log.write('started\n') +for line in sys.stdin: + request = json.loads(line) + result = {'protocolVersion': 1, 'argv': sys.argv[1:], + 'hermes': os.environ.get('HERMES_ACP_SKIP_CONFIGURED_MCP'), + 'codex': os.environ.get('CODEX_CONFIG'), + 'prefix': os.environ.get('BUZZ_ACP_LAUNCH_PREFIX')} + print(json.dumps({'jsonrpc': '2.0', 'id': request['id'], 'result': result}), flush=True) +"#, + ) + .unwrap(); + fs::set_permissions(path, fs::Permissions::from_mode(0o700)).unwrap(); + } + Self(dir) + } + fn prefix(&self) -> String { + json!([ + "/usr/bin/env", + "python3", + self.0.join("wrapper.py"), + LITERAL + ]) + .to_string() + } + fn run(&self, mode: &str, prefix: Option<&str>) { + let mut cmd = Command::new(std::env::current_exe().unwrap()); + for (name, _) in std::env::vars_os() { + if name.to_string_lossy().starts_with("BUZZ_") { + cmd.env_remove(name); + } + } + cmd.args(["--exact", "acp::launch::tests::spawn_child", "--nocapture"]) + .env(CHILD, mode) + .env_remove("CODEX_CONFIG") + .env_remove("HERMES_ACP_SKIP_CONFIGURED_MCP") + .current_dir(&self.0); + if let Some(prefix) = prefix { + cmd.env(PREFIX_ENV, prefix); + } + let output = cmd.output().unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } +} +impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +#[test] +fn wrapper_preserves_worker_identity_and_runs_on_each_spawn() { + for wrapped in [false, true] { + let f = Fixture::new(); + let prefix = f.prefix(); + f.run("success", wrapped.then_some(prefix.as_str())); + assert_eq!( + fs::read_to_string(f.0.join("worker-started")) + .unwrap() + .lines() + .count(), + 10 + ); + if wrapped { + let calls: Vec = fs::read_to_string(f.0.join("wrapper.jsonl")) + .unwrap() + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!(calls.len(), 10); + for call in calls { + assert_eq!(call[0], LITERAL); + } + } else { + assert!(!f.0.join("wrapper.jsonl").exists()); + } + } +} + +#[test] +fn invalid_or_unavailable_wrapper_never_falls_back_to_worker() { + let f = Fixture::new(); + for prefix in [ + "", + "[]", + "null", + "{}", + "[1]", + "[\"\"]", + "[\"relative\"]", + "[\"/tmp/\\u0000\"]", + "[\"/missing-buzz-launcher\"]", + ] { + f.run("failure", Some(prefix)); + assert!(!f.0.join("worker-started").exists()); + } +} + +#[tokio::test] +async fn spawn_child() { + let Ok(mode) = std::env::var(CHILD) else { + return; + }; + // Bound a broken wrapper/stdio regression; no arbitrary timing gates. + tokio::time::timeout(std::time::Duration::from_secs(10), async { + for name in [ + "goose", + "codex-acp", + "claude-agent-acp", + "hermes-acp", + "buzz-pi-acp", + ] { + let worker = std::env::current_dir().unwrap().join(name); + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + "0000000000000000000000000000000000000000000000000000000000000001", + "--agent-command", + worker.to_str().unwrap(), + "--agent-args", + "", + ]) + .unwrap(); + let config = Config::from_args(args).unwrap(); + // A second spawn uses the same host configuration after shutdown, + // just as a replacement worker does. No prefix is consumed once. + for _ in 0..2 { + let spawned = AcpClient::spawn( + &config.agent_command, + &config.agent_args, + &config.persona_env_vars, + config.has_generated_codex_config, + ) + .await; + if mode == "failure" { + let error = spawned.err().expect("invalid wrapper launched a worker"); + if std::env::var(PREFIX_ENV) + .unwrap() + .contains("/missing-buzz-launcher") + { + assert!(error.to_string().contains("/missing-buzz-launcher")); + } + return; + } + let mut client = spawned.unwrap(); + let probe = client.initialize().await.unwrap(); + let expected = match name { + "goose" => json!(["acp"]), + "hermes-acp" => json!([]), + "buzz-pi-acp" => json!([ + "--", + "--skill", + std::env::current_dir().unwrap().join(".agents/skills") + ]), + _ => json!([]), + }; + assert_eq!(probe["argv"], expected, "{name}"); + assert!(probe["prefix"].is_null()); + if name == "hermes-acp" { + assert_eq!(probe["hermes"], "1"); + } + let adapter = match name { + "codex-acp" => { + let config: Value = + serde_json::from_str(probe["codex"].as_str().unwrap()).unwrap(); + assert_eq!(config["sandbox_workspace_write"]["network_access"], true); + Some(StandardAdapterKind::Codex) + } + "claude-agent-acp" => Some(StandardAdapterKind::Claude), + _ => None, + }; + assert_eq!(client.standard_adapter, adapter); + client.shutdown().await; + } + } + }) + .await + .expect("wrapped ACP handshake exceeded test deadline"); +} diff --git a/crates/buzz-acp/tests/run_task.rs b/crates/buzz-acp/tests/run_task.rs index 4bf2b60540c..fec3d786917 100644 --- a/crates/buzz-acp/tests/run_task.rs +++ b/crates/buzz-acp/tests/run_task.rs @@ -577,3 +577,37 @@ async fn memory_is_loaded_before_session_and_opt_out_makes_no_request() { } } } + +#[test] +fn launch_prefix_wraps_the_shipped_local_task_entrypoint() { + let f = Fixture::new(); + let mut command = f.command(); + command + .env( + "BUZZ_ACP_LAUNCH_PREFIX", + json!([ + "/bin/sh", + "-c", + "printf invoked > wrapper-called; exec \"$@\"", + "fixture-launcher" + ]) + .to_string(), + ) + .args(["--no-memory", "--task", "-"]); + let mut child = command.spawn().unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(f.task().to_string().as_bytes()) + .unwrap(); + terminal(&wait(child), 0, "completed"); + assert_eq!( + fs::read_to_string(f.dir.join("wrapper-called")).unwrap(), + "invoked" + ); + assert!(f + .wire() + .iter() + .any(|message| message["method"] == "session/prompt")); +} diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index dc38c3d126f..8081f132c61 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -190,6 +190,7 @@ fn reserved_keys_include_code_execution_surface() { "BUZZ_ACP_AGENT_COMMAND", "BUZZ_ACP_AGENT_ARGS", "BUZZ_ACP_MCP_COMMAND", + "BUZZ_ACP_LAUNCH_PREFIX", ] { assert!(is_reserved_env_key(key), "{key} should be reserved"); } @@ -512,3 +513,19 @@ fn deploy_model_precedence_none_when_both_absent() { let effective = persona_model.clone().or(record_model.clone()); assert_eq!(effective, None); } + +#[test] +fn launch_prefix_cannot_be_saved_or_merged_from_user_environment() { + for key in [ + "BUZZ_ACP_LAUNCH_PREFIX", + "buzz_acp_launch_prefix", + "Buzz_Acp_Launch_Prefix", + ] { + let env = map(&[(key, r#"["/usr/bin/env"]"#)]); + assert!(validate_user_env_keys(&env) + .unwrap_err() + .contains("reserved")); + assert!(merged_user_env(&env, &BTreeMap::new()).is_empty()); + assert!(merged_user_env(&BTreeMap::new(), &env).is_empty()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs index 07fcf8d2592..52a6ae53e3f 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -41,6 +41,7 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_AGENT_COMMAND", "BUZZ_ACP_AGENT_ARGS", "BUZZ_ACP_MCP_COMMAND", + "BUZZ_ACP_LAUNCH_PREFIX", // Control-plane parallelism: the Desktop resolves the effective // worker-pool size (applying any per-harness cap) and writes it into // launch.policy_env. A user-supplied BUZZ_ACP_AGENTS would bypass the