diff --git a/.env.example b/.env.example index b9bfcada0e..65a894cbbf 100644 --- a/.env.example +++ b/.env.example @@ -157,6 +157,10 @@ RUST_LOG=buzz_relay=debug,buzz_datastore=info,buzz_db=debug,buzz_auth=debug,buzz # Goose default: "acp". Codex/Claude default: "" (empty). # BUZZ_ACP_AGENT_ARGS=acp +# Absolute workspace root passed to ACP session/new. Defaults to the process +# working directory. Must exist, contain AGENTS.md, and must not be `/`. +# BUZZ_ACP_WORKSPACE=/absolute/path/to/agent-workspace + # Binary for an optional MCP server sidecar (e.g. buzz-dev-mcp for buzz-agent). # BUZZ_ACP_MCP_COMMAND= diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index e6164b02dd..061f57d387 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -110,6 +110,7 @@ All configuration is via environment variables (or CLI flags — every env var h | `BUZZ_RELAY_URL` | no | `ws://localhost:3000` | Relay WebSocket URL. | | `BUZZ_ACP_AGENT_COMMAND` | no | `goose` | Agent binary to spawn. | | `BUZZ_ACP_AGENT_ARGS` | no | `acp` | Agent arguments (comma-separated). | +| `BUZZ_ACP_WORKSPACE` | no | Process working directory | Absolute existing workspace root passed to ACP `session/new`. The directory must contain `AGENTS.md`; `/`, relative paths, missing paths, and non-directories fail startup. | | `BUZZ_ACP_MCP_COMMAND` | no | `""` (empty) | Path to an optional MCP server binary to provide to the agent subprocess. | | `BUZZ_ACP_IDLE_TIMEOUT` | no | `620` | Idle timeout: max seconds of silence before cancelling a turn. Resets on any agent stdout activity. | | `BUZZ_ACP_MAX_TURN_DURATION` | no | `7200` | Absolute wall-clock cap per turn (safety valve). | diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index dab61be30a..b189707bdf 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -179,6 +179,11 @@ pub struct ModelsArgs { #[command(flatten)] pub agent: AuthAgentArgs, + /// Absolute existing directory used as the ACP session workspace. + /// Defaults to the harness process working directory. + #[arg(long, env = "BUZZ_ACP_WORKSPACE")] + pub workspace_root: Option, + /// Output structured JSON instead of human-readable text. #[arg(long)] pub json: bool, @@ -258,6 +263,11 @@ pub struct CliArgs { )] pub agent_args: Vec, + /// Absolute existing directory used as the ACP session workspace. + /// Defaults to the harness process working directory. + #[arg(long, env = "BUZZ_ACP_WORKSPACE")] + pub workspace_root: Option, + #[arg(long, env = "BUZZ_ACP_MCP_COMMAND", default_value = "")] pub mcp_command: String, @@ -494,6 +504,9 @@ pub struct Config { pub relay_url: String, pub agent_command: String, pub agent_args: Vec, + /// Validated, canonical workspace passed to every ACP `session/new` request. + /// `None` only for setup-listener mode, which never starts an ACP session. + pub workspace_root: Option, pub mcp_command: String, pub idle_timeout_secs: u64, pub max_turn_duration_secs: u64, @@ -596,6 +609,73 @@ fn sanitize_session_title(raw: &str) -> Option { } } +/// Resolve the configured ACP workspace without ever falling back to `/`. +/// +/// The explicit path and inherited process working directory share the same +/// contract: absolute, existing, a directory other than the filesystem root, +/// and containing an `AGENTS.md` workspace guide. The canonical path is retained +/// so startup diagnostics and ACP `session/new` always describe the same workspace. +pub(crate) fn resolve_workspace_root( + configured: Option, + current_dir: std::io::Result, +) -> Result { + let workspace = match configured { + Some(path) => path, + None => current_dir.map_err(|error| { + ConfigError::ConfigFile(format!( + "cannot resolve current working directory for ACP workspace: {error}; \ + set --workspace-root / BUZZ_ACP_WORKSPACE" + )) + })?, + }; + + if !workspace.is_absolute() { + return Err(ConfigError::ConfigFile(format!( + "ACP workspace must be an absolute path, got {}", + workspace.display() + ))); + } + if !workspace.exists() { + return Err(ConfigError::ConfigFile(format!( + "ACP workspace does not exist: {}", + workspace.display() + ))); + } + if !workspace.is_dir() { + return Err(ConfigError::ConfigFile(format!( + "ACP workspace is not a directory: {}", + workspace.display() + ))); + } + + let workspace = std::fs::canonicalize(&workspace).map_err(|error| { + ConfigError::ConfigFile(format!( + "failed to canonicalize ACP workspace {}: {error}", + workspace.display() + )) + })?; + if workspace.parent().is_none() { + return Err(ConfigError::ConfigFile(format!( + "ACP workspace must not be the filesystem root: {}", + workspace.display() + ))); + } + let agent_guide = workspace.join("AGENTS.md"); + if !agent_guide.is_file() { + return Err(ConfigError::ConfigFile(format!( + "ACP workspace must contain an AGENTS.md file: {}", + workspace.display() + ))); + } + + workspace.into_os_string().into_string().map_err(|path| { + ConfigError::ConfigFile(format!( + "ACP workspace is not valid UTF-8 and cannot be sent to session/new: {}", + PathBuf::from(path).display() + )) + }) +} + /// Separator between the agent name and the channel in a composed title. /// U+00B7 MIDDLE DOT, spaces on both sides. const SESSION_TITLE_SEPARATOR: &str = " · "; @@ -821,18 +901,26 @@ pub fn propagate_legacy_env_vars() { } impl Config { - pub fn from_cli() -> Result { + pub fn from_cli(workspace_required: bool) -> Result { // Legacy env-var propagation is intentionally NOT done here. // Call `propagate_legacy_env_vars()` before the tokio runtime starts // (in the sync `fn main()` wrapper) — see Rust 2024 edition safety. let args = CliArgs::parse(); - Self::from_args(args) + Self::from_args_with_workspace_requirement(args, workspace_required) } /// Build a `Config` from already-parsed `CliArgs`. Separated from `from_cli()` so /// tests can construct `CliArgs` via `CliArgs::try_parse_from` and exercise the full /// validation path without going through process args. - pub fn from_args(mut args: CliArgs) -> Result { + #[cfg(test)] + pub fn from_args(args: CliArgs) -> Result { + Self::from_args_with_workspace_requirement(args, true) + } + + fn from_args_with_workspace_requirement( + mut args: CliArgs, + workspace_required: bool, + ) -> Result { let keys = Keys::parse(&args.private_key)?; // Best-effort zeroize: overwrite the raw private key string to reduce // exposure via core dumps or heap inspection (#41). Without the `zeroize` @@ -906,6 +994,9 @@ impl Config { } let agent_args = normalize_agent_args(&agent_command, args.agent_args); + let workspace_root = workspace_required + .then(|| resolve_workspace_root(args.workspace_root, std::env::current_dir())) + .transpose()?; if let Some(ref channels) = args.channels { for ch in channels { @@ -1058,6 +1149,7 @@ impl Config { relay_url: args.relay_url, agent_command, agent_args, + workspace_root, mcp_command: args.mcp_command, idle_timeout_secs, max_turn_duration_secs, @@ -1123,11 +1215,12 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} workspace={} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, self.agent_args.join(" "), + self.workspace_root.as_deref().unwrap_or(""), self.mcp_command, self.idle_timeout_secs, self.max_turn_duration_secs, @@ -1429,6 +1522,163 @@ mod tests { use crate::filter::{ChannelScope, SubscriptionRule}; use clap::{Parser, ValueEnum}; + #[test] + fn workspace_root_accepts_existing_absolute_directory() { + let dir = + std::env::temp_dir().join(format!("buzz-acp-workspace-valid-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("AGENTS.md"), b"# Test workspace\n").unwrap(); + + let resolved = resolve_workspace_root(Some(dir.clone()), Ok(PathBuf::from("/ignored"))) + .expect("existing absolute workspace should resolve"); + + assert_eq!( + resolved, + std::fs::canonicalize(&dir) + .unwrap() + .to_string_lossy() + .as_ref() + ); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn workspace_root_cli_flows_into_config_and_summary() { + let dir = + std::env::temp_dir().join(format!("buzz-acp-workspace-cli-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("AGENTS.md"), b"# Test workspace\n").unwrap(); + let dir_arg = dir.to_string_lossy().into_owned(); + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--workspace-root", + dir_arg.as_str(), + ]) + .expect("workspace flag should parse"); + + let config = Config::from_args(args).expect("workspace flag should validate"); + let canonical = std::fs::canonicalize(&dir) + .unwrap() + .to_string_lossy() + .into_owned(); + assert_eq!(config.workspace_root.as_deref(), Some(canonical.as_str())); + assert!( + config.summary().contains(&format!("workspace={canonical}")), + "startup summary must expose the exact ACP workspace" + ); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn workspace_root_rejects_relative_path() { + let err = resolve_workspace_root( + Some(PathBuf::from("relative/workspace")), + Ok(PathBuf::from("/ignored")), + ) + .expect_err("relative workspace must fail closed"); + + assert!( + err.to_string().contains("absolute"), + "unexpected error: {err}" + ); + } + + #[test] + fn workspace_root_rejects_missing_path() { + let missing = + std::env::temp_dir().join(format!("buzz-acp-workspace-missing-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&missing); + + let err = resolve_workspace_root(Some(missing), Ok(PathBuf::from("/ignored"))) + .expect_err("missing workspace must fail closed"); + + assert!( + err.to_string().contains("does not exist"), + "unexpected error: {err}" + ); + } + + #[test] + fn workspace_root_rejects_file() { + let file = + std::env::temp_dir().join(format!("buzz-acp-workspace-file-{}", std::process::id())); + std::fs::write(&file, b"not a directory").unwrap(); + + let err = resolve_workspace_root(Some(file.clone()), Ok(PathBuf::from("/ignored"))) + .expect_err("workspace file must fail closed"); + + assert!( + err.to_string().contains("not a directory"), + "unexpected error: {err}" + ); + std::fs::remove_file(file).unwrap(); + } + + #[test] + fn workspace_root_rejects_directory_without_agent_guide() { + let dir = std::env::temp_dir().join(format!( + "buzz-acp-workspace-no-agent-guide-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).unwrap(); + + let err = resolve_workspace_root(Some(dir.clone()), Ok(PathBuf::from("/ignored"))) + .expect_err("workspace without AGENTS.md must fail closed"); + + assert!( + err.to_string().contains("AGENTS.md"), + "unexpected error: {err}" + ); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn workspace_root_rejects_filesystem_root() { + let err = resolve_workspace_root(Some(PathBuf::from("/")), Ok(PathBuf::from("/ignored"))) + .expect_err("filesystem root must not be accepted as an agent workspace"); + + assert!( + err.to_string().contains("filesystem root"), + "unexpected error: {err}" + ); + } + + #[test] + fn workspace_root_current_dir_failure_does_not_fallback_to_root() { + let err = resolve_workspace_root( + None, + Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "cwd disappeared", + )), + ) + .expect_err("unreadable current directory must fail closed"); + + assert!( + err.to_string().contains("current working directory"), + "unexpected error: {err}" + ); + } + + #[test] + fn setup_mode_does_not_require_an_acp_workspace() { + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--workspace-root", + "/", + ]) + .expect("clap should parse args"); + + let config = Config::from_args_with_workspace_requirement(args, false) + .expect("setup-listener mode must not validate an unused ACP workspace"); + + assert_eq!(config.workspace_root, None); + } + /// Build a minimal Config for testing without CLI parsing. fn test_config(mode: SubscribeMode) -> Config { Config { @@ -1436,6 +1686,12 @@ mod tests { relay_url: "ws://localhost:3000".into(), agent_command: "goose".into(), agent_args: vec!["acp".into()], + workspace_root: Some( + std::env::current_dir() + .unwrap() + .to_string_lossy() + .into_owned(), + ), mcp_command: "".into(), idle_timeout_secs: DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: DEFAULT_MAX_TURN_DURATION_SECS, @@ -2730,7 +2986,10 @@ channels = "ALL" "owner-only,allowlist", ]) .expect("clap should parse args"); - let result = Config::from_args(args); + let result = Config::from_args(CliArgs { + workspace_root: Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..")), + ..args + }); assert!( result.is_err(), @@ -2760,7 +3019,10 @@ channels = "ALL" "owner-only,allowlist", ]) .expect("clap should parse args"); - let result = Config::from_args(args); + let result = Config::from_args(CliArgs { + workspace_root: Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..")), + ..args + }); assert!( result.is_ok(), @@ -2779,7 +3041,10 @@ channels = "ALL" "anyone", ]) .expect("clap should parse args"); - let result = Config::from_args(args); + let result = Config::from_args(CliArgs { + workspace_root: Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..")), + ..args + }); assert!( result.is_ok(), @@ -2799,7 +3064,10 @@ channels = "ALL" &MAX_TURN_DURATION_CEILING_SECS.to_string(), ]) .expect("clap should parse args"); - let result = Config::from_args(args); + let result = Config::from_args(CliArgs { + workspace_root: Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..")), + ..args + }); assert!( result.is_ok(), @@ -2818,7 +3086,10 @@ channels = "ALL" &over.to_string(), ]) .expect("clap should parse args"); - let result = Config::from_args(args); + let result = Config::from_args(CliArgs { + workspace_root: Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..")), + ..args + }); assert!( result.is_err(), diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..397b79aab3 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1278,20 +1278,26 @@ async fn tokio_main() -> Result<()> { .compact() .init(); - let mut config = Config::from_cli().map_err(|e| anyhow::anyhow!("configuration error: {e}"))?; - // ── Setup-mode early branch ─────────────────────────────────────────────── // // When the desktop determines an agent is not ready (missing credentials, // model, or provider), it spawns buzz-acp with BUZZ_ACP_SETUP_PAYLOAD set. - // We enter the minimal setup-listener path and never start the agent pool. - if let Some(payload) = setup_mode::SetupPayload::from_env() - .map_err(|e| anyhow::anyhow!("setup payload error: {e}"))? - { + // We enter the minimal setup-listener path and never require an ACP workspace + // or start the agent pool. + let setup_payload = setup_mode::SetupPayload::from_env() + .map_err(|e| anyhow::anyhow!("setup payload error: {e}"))?; + let mut config = Config::from_cli(setup_payload.is_none()) + .map_err(|e| anyhow::anyhow!("configuration error: {e}"))?; + if let Some(payload) = setup_payload { tracing::info!("buzz-acp: setup payload present, entering setup-listener mode"); return setup_mode::run_setup_listener(config, payload).await; } + let workspace_root = config.workspace_root.clone().ok_or_else(|| { + anyhow::anyhow!("internal error: normal ACP startup has no validated workspace") + })?; + tracing::info!(workspace = %workspace_root, "ACP workspace resolved"); + tracing::info!("buzz-acp starting: {}", config.summary()); let observer = config @@ -1543,10 +1549,7 @@ async fn tokio_main() -> Result<()> { Some(include_str!("base_prompt.md")) }, heartbeat_prompt: config.heartbeat_prompt.clone(), - cwd: std::env::current_dir() - .unwrap_or_else(|_| std::path::PathBuf::from("/")) - .to_string_lossy() - .to_string(), + cwd: workspace_root, rest_client: relay.rest_client(), channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), context_message_limit: config.context_message_limit, @@ -4044,10 +4047,13 @@ async fn run_models(args: ModelsArgs) -> Result<()> { use acp::{extract_model_config_options, extract_model_state}; let agent_args = config::normalize_agent_args(&args.agent.agent_command, args.agent.agent_args); - let cwd = std::env::current_dir() - .unwrap_or_else(|_| std::path::PathBuf::from("/")) - .to_string_lossy() - .to_string(); + let cwd = match config::resolve_workspace_root(args.workspace_root, std::env::current_dir()) { + Ok(workspace) => workspace, + Err(error) => { + eprintln!("error: invalid ACP workspace: {error}"); + std::process::exit(1); + } + }; // Spawn outside the timeout so we always own the child for cleanup. // `models` subcommand doesn't use persona packs — no extra env, no codex config. @@ -4999,6 +5005,12 @@ mod build_mcp_servers_tests { relay_url: "ws://localhost:3000".into(), agent_command: "goose".into(), agent_args: vec!["acp".into()], + workspace_root: Some( + std::env::current_dir() + .unwrap() + .to_string_lossy() + .into_owned(), + ), mcp_command: "test-mcp-server".into(), idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, @@ -5220,6 +5232,12 @@ mod error_outcome_emission_tests { // feed emission under test. agent_command: "true".into(), agent_args: vec![], + workspace_root: Some( + std::env::current_dir() + .unwrap() + .to_string_lossy() + .into_owned(), + ), mcp_command: "test-mcp-server".into(), idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 348bc138e4..f63b8782b1 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -1207,8 +1207,8 @@ pub(crate) fn prepend_canvas_for_legacy( /// absolute root, so without this anchor a model fills the gap by searching /// `$HOME` (triggering macOS TCC prompts) or by inventing its own workspace /// directory. The line is emitted only when a real base prompt is present and -/// `cwd` is an absolute path other than the `/` fallback — naming `/` as the -/// workspace would itself invite a `$HOME`-wide scan. +/// `cwd` is an absolute non-root path. Harness startup validates this invariant; +/// this renderer remains defensive for direct unit-test callers. fn framed_system_prompt( cwd: &str, base_prompt: Option<&str>, @@ -1234,9 +1234,8 @@ fn framed_system_prompt( /// Render the `[Workspace]` grounding section, or `None` when `cwd` is unusable. /// -/// Skips relative paths and the `/` fallback (`std::env::current_dir()` resolves -/// to `/` on failure): a `/`-rooted workspace line would actively encourage the -/// `$HOME`-wide scan this section exists to prevent. +/// Skips relative paths and `/`: a root workspace line would actively encourage +/// the `$HOME`-wide scan this section exists to prevent. fn workspace_section(cwd: &str) -> Option { if cwd != "/" && cwd.starts_with('/') { Some(format!( diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..d59f81baa7 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -6,10 +6,9 @@ //! early-branch path: //! //! ```text -//! Config::from_cli() -//! └─ SetupPayload::from_env()? -//! ├─ Some(payload) → run_setup_listener(config, payload) [this module] -//! └─ None → normal pool path (unchanged) +//! SetupPayload::from_env()? +//! ├─ Some(payload) → Config::from_cli(false) → run_setup_listener(config, payload) +//! └─ None → Config::from_cli(true) → validated workspace + normal pool //! ``` //! //! # Contract (NON-NEGOTIABLE)