diff --git a/Cargo.lock b/Cargo.lock index f94dc9ab43..0411aa0434 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -111,6 +111,7 @@ dependencies = [ "project", "prompt_store", "rand 0.9.4", + "sandbox", "serde", "serde_json", "settings", diff --git a/crates/acp_thread/Cargo.toml b/crates/acp_thread/Cargo.toml index 9123c30107..d115b29b1f 100644 --- a/crates/acp_thread/Cargo.toml +++ b/crates/acp_thread/Cargo.toml @@ -39,6 +39,7 @@ image.workspace = true portable-pty.workspace = true project.workspace = true prompt_store.workspace = true +sandbox.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 4e6be0fe6a..3270bac05f 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2955,6 +2955,7 @@ impl AcpThread { extra_env: Vec, cwd: Option, output_byte_limit: Option, + sandbox_wrap: Option, cx: &mut Context, ) -> Task>> { let env = match &cwd { @@ -2995,6 +2996,8 @@ impl AcpThread { ShellBuilder::new(&Shell::Program(shell), is_windows) .redirect_stdin_to_dev_null() .build(Some(command.clone()), &args); + let (task_command, task_args, sandbox_config) = + apply_sandbox_wrap(task_command, task_args, sandbox_wrap)?; let terminal = project .update(cx, |project, cx| { project.create_terminal_task( @@ -3018,6 +3021,7 @@ impl AcpThread { output_byte_limit.map(|l| l as usize), terminal, language_registry, + sandbox_config, cx, ) })) @@ -3097,6 +3101,9 @@ impl AcpThread { output_byte_limit.map(|l| l as usize), terminal, language_registry, + // External terminal providers manage their own sandboxing + // (if any). We don't wrap their commands. + None, cx, ) }); diff --git a/crates/acp_thread/src/terminal.rs b/crates/acp_thread/src/terminal.rs index 2fe769cb73..32496b8f44 100644 --- a/crates/acp_thread/src/terminal.rs +++ b/crates/acp_thread/src/terminal.rs @@ -17,6 +17,82 @@ use std::{ use task::Shell; use util::get_default_system_shell_preferring_bash; +/// Request to run a terminal command inside an OS-level sandbox. +/// +/// Passed to [`super::AcpThread::create_terminal`]. The actual sandboxing +/// mechanism is platform-specific (today: macOS Seatbelt; nothing on other +/// platforms — the wrap is silently a no-op there), so callers describe the +/// *intent* with plain data here rather than constructing platform-specific +/// types directly. +/// +/// All-zero defaults are the fully-sandboxed run. Setting `allow_network` / +/// `allow_fs_write` requests a relaxation; the caller is responsible for +/// having obtained user approval before reaching this point. +#[derive(Clone, Debug, Default)] +pub struct SandboxWrap { + /// Directory subtrees the sandbox should allow writes to. Pass the + /// project's worktree paths (and any per-command scratch directory) + /// here — *not* the command's working directory, which is model- + /// controlled and would let the model widen its own writable scope. + pub writable_paths: Vec, + /// Allow outbound network access for this command. + pub allow_network: bool, + /// Allow unrestricted filesystem writes (ignores `writable_paths`). + pub allow_fs_write: bool, +} + +/// Opaque RAII handle the sandbox implementation hands back to keep its +/// per-command resources (e.g. an on-disk Seatbelt config file) alive for +/// the duration of the spawned command. `Terminal` holds it in a field +/// whose only job is to drop with the entity. +pub type SandboxConfigHandle = Box; + +/// Apply a [`SandboxWrap`] to a `(program, args)` pair, substituting the +/// platform's sandbox-launcher invocation in place of the original. The +/// returned `SandboxConfigHandle` (when `Some`) must be kept alive for the +/// duration of the spawned command — dropping it deletes any on-disk +/// config the launcher reads at startup. +/// +/// On non-macOS hosts this is a no-op: the inputs pass through unchanged +/// and the returned handle is `None`. (We don't yet have a sandbox +/// integration for other platforms.) +pub(crate) fn apply_sandbox_wrap( + program: String, + args: Vec, + sandbox_wrap: Option, +) -> anyhow::Result<(String, Vec, Option)> { + let Some(sandbox_wrap) = sandbox_wrap else { + return Ok((program, args, None)); + }; + + #[cfg(target_os = "macos")] + { + let writable: Vec<&std::path::Path> = sandbox_wrap + .writable_paths + .iter() + .map(|p| p.as_path()) + .collect(); + let permissions = sandbox::macos_seatbelt::SandboxPermissions { + allow_network: sandbox_wrap.allow_network, + allow_fs_write: sandbox_wrap.allow_fs_write, + }; + let (new_program, new_args, config_file) = + sandbox::macos_seatbelt::wrap_invocation(&program, &args, &writable, permissions)?; + Ok(( + new_program, + new_args, + Some(Box::new(config_file) as SandboxConfigHandle), + )) + } + #[cfg(not(target_os = "macos"))] + { + // No sandbox integration available; ignore the wrap request and + // let the command run with the agent's ambient permissions. + let _ = sandbox_wrap; + Ok((program, args, None)) + } +} + pub struct Terminal { id: acp::TerminalId, command: Entity, @@ -30,6 +106,10 @@ pub struct Terminal { /// (e.g., clicking the Stop button). This is set before kill() is called /// so that code awaiting wait_for_exit() can check it deterministically. user_stopped: Arc, + /// RAII handle kept alive for the duration of the sandboxed command. + /// `None` when the command isn't sandboxed (the common case for + /// terminals not created by the agent). + _sandbox_config: Option, } pub struct TerminalOutput { @@ -48,11 +128,13 @@ impl Terminal { output_byte_limit: Option, terminal: Entity, language_registry: Arc, + sandbox_config: Option, cx: &mut Context, ) -> Self { let command_task = terminal.read(cx).wait_for_completed_task(cx); Self { id, + _sandbox_config: sandbox_config, command: cx.new(|cx| { Markdown::new( format!("```\n{}\n```", command_label).into(), diff --git a/crates/agent/Cargo.toml b/crates/agent/Cargo.toml index f88d828653..8c36cff81d 100644 --- a/crates/agent/Cargo.toml +++ b/crates/agent/Cargo.toml @@ -65,6 +65,7 @@ streaming_diff.workspace = true strsim.workspace = true task.workspace = true telemetry.workspace = true +tempfile.workspace = true text.workspace = true thiserror.workspace = true ui.workspace = true @@ -100,7 +101,6 @@ project = { workspace = true, "features" = ["test-support"] } rand.workspace = true reqwest_client.workspace = true settings = { workspace = true, "features" = ["test-support"] } -tempfile.workspace = true theme = { workspace = true, "features" = ["test-support"] } diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index d1b6cfd305..dd24031760 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -2610,12 +2610,22 @@ impl ThreadEnvironment for NativeThreadEnvironment { fn create_terminal( &self, command: String, + extra_env: Vec, cwd: Option, output_byte_limit: Option, + sandbox_wrap: Option, cx: &mut AsyncApp, ) -> Task>> { let task = self.acp_thread.update(cx, |thread, cx| { - thread.create_terminal(command, vec![], vec![], cwd, output_byte_limit, cx) + thread.create_terminal( + command, + vec![], + extra_env, + cwd, + output_byte_limit, + sandbox_wrap, + cx, + ) }); let acp_thread = self.acp_thread.clone(); diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index e75cfc4fac..16faa56c78 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -200,8 +200,10 @@ impl crate::ThreadEnvironment for FakeThreadEnvironment { fn create_terminal( &self, _command: String, + _extra_env: Vec, _cwd: Option, _output_byte_limit: Option, + _sandbox_wrap: Option, _cx: &mut AsyncApp, ) -> Task>> { self.terminal_creations.fetch_add(1, Ordering::SeqCst); @@ -242,8 +244,10 @@ impl crate::ThreadEnvironment for MultiTerminalEnvironment { fn create_terminal( &self, _command: String, + _extra_env: Vec, _cwd: Option, _output_byte_limit: Option, + _sandbox_wrap: Option, cx: &mut AsyncApp, ) -> Task>> { let handle = Rc::new(cx.update(|cx| FakeTerminalHandle::new_never_exits(cx))); @@ -320,6 +324,7 @@ async fn test_terminal_tool_timeout_kills_handle(cx: &mut TestAppContext) { command: "sleep 1000".to_string(), cd: ".".to_string(), timeout_ms: Some(5), + ..Default::default() }), event_stream, cx, @@ -387,6 +392,7 @@ async fn test_terminal_tool_without_timeout_does_not_kill_handle(cx: &mut TestAp command: "sleep 1000".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -4892,6 +4898,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { command: "rm -rf /".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -4944,6 +4951,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { command: "echo hello".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -5002,6 +5010,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { command: "sudo rm file".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -5049,6 +5058,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { command: "echo hello".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 4c1aa3877e..0202e2efa8 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -668,8 +668,10 @@ pub trait ThreadEnvironment { fn create_terminal( &self, command: String, + extra_env: Vec, cwd: Option, output_byte_limit: Option, + sandbox_wrap: Option, cx: &mut AsyncApp, ) -> Task>>; diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 4f0c6b48c8..cb20662526 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -14,6 +14,7 @@ use std::{ time::Duration, }; +use crate::sandboxing::sandboxing_enabled; use crate::{AgentTool, ThreadEnvironment, ToolCallEventStream, ToolInput}; const COMMAND_OUTPUT_LIMIT: u64 = 16 * 1024; @@ -39,7 +40,7 @@ const COMMAND_OUTPUT_LIMIT: u64 = 16 * 1024; /// - Always insert `--no-pager` immediately after `git` for any read-only git command, including `git log`, `git diff`, `git show`, `git blame`, and `git stash show`. Example: `git --no-pager log -n 5` (NOT `git log -n 5`). /// - Always prepend `GIT_EDITOR=true ` to any git command that may invoke an editor, including `git rebase`, `git commit`, `git merge`, and `git tag`. Example: `GIT_EDITOR=true git rebase origin/main` (NOT `git rebase origin/main`). /// - For other commands that may open a pager or editor, set `PAGER=cat` and/or `EDITOR=true` similarly. -#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] pub struct TerminalToolInput { /// The one-liner command to execute. Do not include shell substitutions or interpolations such as `$VAR`, `${VAR}`, `$(...)`, backticks, `$((...))`, `<(...)`, or `>(...)`; resolve those values first or ask the user. /// @@ -49,6 +50,35 @@ pub struct TerminalToolInput { pub cd: String, /// Optional maximum runtime (in milliseconds). If exceeded, the running terminal task is killed. pub timeout_ms: Option, + /// Request network access for this command. + /// + /// Only meaningful when the system prompt's "Terminal sandbox" section + /// is present — ignored otherwise. By default sandboxed commands + /// cannot make outbound network connections; set this to `true` only + /// when the command needs network access. The user will be prompted + /// to approve before the command runs. + #[serde(default)] + pub allow_network: Option, + /// Request unrestricted filesystem-write access for this command. + /// + /// Only meaningful when the system prompt's "Terminal sandbox" section + /// is present — ignored otherwise. By default sandboxed commands can + /// only write to the project worktree directories and a per-command + /// temporary directory; set this to `true` only when the command + /// needs to write elsewhere. The user will be prompted to approve + /// before the command runs. + #[serde(default)] + pub allow_fs_write: Option, + /// Request to run this command outside the sandbox entirely. + /// + /// Only meaningful when the system prompt's "Terminal sandbox" section + /// is present — ignored otherwise. Prefer `allow_network: true` or + /// `allow_fs_write: true` when one of those is enough. Set this to + /// `true` ONLY when the command needs behavior that the sandbox can't + /// grant on a per-permission basis. The user will be prompted to + /// approve before the command runs without sandbox restrictions. + #[serde(default)] + pub unsandboxed: Option, } pub struct TerminalTool { @@ -96,28 +126,123 @@ impl AgentTool for TerminalTool { cx.spawn(async move |cx| { let input = input.recv().await.map_err(|e| e.to_string())?; - let (working_dir, authorize) = cx.update(|cx| { + let (working_dir, authorize, sandboxing) = cx.update(|cx| { let working_dir = working_dir(&input, &self.project, cx).map_err(|err| err.to_string())?; let context = crate::ToolPermissionContext::new(Self::NAME, vec![input.command.clone()]); let authorize = event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx); - Result::<_, String>::Ok((working_dir, authorize)) + let sandboxing = sandboxing_enabled(cx); + Result::<_, String>::Ok((working_dir, authorize, sandboxing)) })?; authorize.await.map_err(|e| e.to_string())?; + // Sandbox flags only do anything when sandboxing is on. When + // off, we treat them as `None` so the model can't surreptitiously + // change runtime behavior by setting flags described as a no-op + // in the system prompt. + let want_network = sandboxing && input.allow_network == Some(true); + let want_fs_write = sandboxing && input.allow_fs_write == Some(true); + let want_unsandboxed = sandboxing && input.unsandboxed == Some(true); + + // `unsandboxed: true` bypasses the wrap entirely; per-permission + // requests are only meaningful when the command is still being + // sandboxed. + let escalate = !want_unsandboxed && (want_network || want_fs_write); + + if want_unsandboxed || escalate { + let title = sandbox_approval_title(want_network, want_fs_write, want_unsandboxed); + let approve = cx.update(|cx| { + let context = crate::ToolPermissionContext::new( + Self::NAME, + vec![input.command.clone()], + ); + // Sandbox escalations always prompt, even if the user + // has `always_allow` rules for this command — the + // escalation is a stronger trust boundary than the + // baseline command approval. + event_stream.authorize_always_prompt(title, context, cx) + }); + if let Err(error) = approve.await { + return Ok(if want_unsandboxed { + format!( + "Command cancelled: user denied permission to run outside the sandbox ({error})." + ) + } else { + format!( + "Command cancelled: user denied the requested sandbox permissions ({error})." + ) + }); + } + } + + // Always provision a per-command temporary directory and point + // TMPDIR/TMP/TEMP at it. This is independent of sandbox state: + // even an unsandboxed command (or a command on a non-macOS + // host) gets a clean, isolated scratch directory that's + // auto-cleaned when the task ends, rather than polluting the + // user's shared `/tmp`. Decoupling it from sandbox state also + // means the model can't infer the sandbox state by looking at + // `$TMPDIR`. + let temp_dir = tempfile::Builder::new() + .prefix("zed-agent-terminal-") + .tempdir() + .map_err(|e| { + format!("failed to create per-command temporary directory: {e}") + })?; + let temp_dir_path = temp_dir + .path() + .canonicalize() + .unwrap_or_else(|_| temp_dir.path().to_path_buf()); + let temp_dir_string = temp_dir_path.to_string_lossy().into_owned(); + let extra_env = vec![ + acp::EnvVariable::new("TMPDIR", &temp_dir_string), + acp::EnvVariable::new("TMP", &temp_dir_string), + acp::EnvVariable::new("TEMP", &temp_dir_string), + ]; + + // Build the writable scope from the project's worktrees plus + // this command's temporary directory. Crucially we do *not* + // include the resolved `cd` working directory — that's + // model-controlled, and using it as the writable scope would + // let the model widen its own write permissions outside the + // project. + let sandbox_wrap = if sandboxing && !want_unsandboxed { + let mut writable_paths: Vec = cx.update(|cx| { + self.project + .read(cx) + .worktrees(cx) + .map(|w| w.read(cx).abs_path().to_path_buf()) + .collect::>() + }); + writable_paths.push(temp_dir_path.clone()); + Some(acp_thread::SandboxWrap { + writable_paths, + allow_network: want_network, + allow_fs_write: want_fs_write, + }) + } else { + None + }; + let terminal = self .environment .create_terminal( input.command.clone(), + extra_env, working_dir, Some(COMMAND_OUTPUT_LIMIT), + sandbox_wrap, cx, ) .await .map_err(|e| e.to_string())?; + // Hold the TempDir until the spawned command has finished so + // its scratch contents (and `$TMPDIR`) stay valid for the + // command's lifetime. + let _temp_dir = temp_dir; let terminal_id = terminal.id(cx).map_err(|e| e.to_string())?; event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![ @@ -182,6 +307,29 @@ impl AgentTool for TerminalTool { } } +/// User-facing title for the sandbox-escalation approval prompt. +/// +/// `want_unsandboxed` wins over the per-permission flags because +/// `unsandboxed: true` bypasses the per-permission machinery entirely. +fn sandbox_approval_title( + want_network: bool, + want_fs_write: bool, + want_unsandboxed: bool, +) -> &'static str { + if want_unsandboxed { + "Allow this command to run outside the sandbox?" + } else { + match (want_network, want_fs_write) { + (true, true) => "Allow network access and arbitrary filesystem writes?", + (true, false) => "Allow network access?", + (false, true) => "Allow arbitrary filesystem writes?", + // Caller only invokes this when at least one flag is set, so + // this fallback is unreachable in practice. + (false, false) => "Allow this command to run?", + } + } +} + fn process_content( output: acp::TerminalOutputResponse, command: &str, @@ -310,6 +458,7 @@ mod tests { .to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }; let title = format_initial_title(Ok(input)); @@ -369,6 +518,7 @@ mod tests { command: cmd.to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }; let title = format_initial_title(Ok(input)); @@ -406,6 +556,7 @@ mod tests { command: "echo 'hello world'".to_string(), cd: ".".to_string(), timeout_ms: None, + ..Default::default() }; let title = format_initial_title(Ok(input)); @@ -435,6 +586,7 @@ mod tests { command: long_command, cd: ".".to_string(), timeout_ms: None, + ..Default::default() }; let title = format_initial_title(Ok(input)); @@ -641,6 +793,7 @@ mod tests { command: "echo $HOME".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -708,6 +861,7 @@ mod tests { command: "echo $HOME".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -769,6 +923,7 @@ mod tests { command: "echo $(rm -rf /)".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -838,6 +993,7 @@ mod tests { command: "PAGER=blah git log --oneline".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -911,6 +1067,7 @@ mod tests { command: "PAGER=blah git log".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1018,6 +1175,7 @@ mod tests { command: command.to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1185,6 +1343,7 @@ mod tests { command: "echo $(whoami)".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1257,6 +1416,7 @@ mod tests { command: "PAGER=other git log".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1323,6 +1483,7 @@ mod tests { command: "A=1 B=2 git log".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1400,6 +1561,7 @@ mod tests { command: "PAGER=\"less -R\" git log".to_string(), cd: "root".to_string(), timeout_ms: None, + ..Default::default() }), event_stream, cx, @@ -1428,4 +1590,72 @@ mod tests { "unexpected terminal result: {result}" ); } + + #[test] + fn test_sandbox_approval_title_unsandboxed_wins() { + // `unsandboxed: true` skips the sandbox entirely, so the title should + // reflect that even when other flags are also set — they're moot. + assert_eq!( + sandbox_approval_title(true, true, true), + "Allow this command to run outside the sandbox?" + ); + assert_eq!( + sandbox_approval_title(false, false, true), + "Allow this command to run outside the sandbox?" + ); + } + + #[test] + fn test_sandbox_approval_title_per_permission_flags() { + assert_eq!( + sandbox_approval_title(true, true, false), + "Allow network access and arbitrary filesystem writes?" + ); + assert_eq!( + sandbox_approval_title(true, false, false), + "Allow network access?" + ); + assert_eq!( + sandbox_approval_title(false, true, false), + "Allow arbitrary filesystem writes?" + ); + } + + #[test] + fn test_input_schema_includes_sandbox_flags() { + // The model only sees these fields when the sandboxing prompt + // section is rendered, but they're always present in the schema so + // input validation doesn't reject them when sent. Guard against + // accidentally renaming or removing them. + let schema = serde_json::to_string(&schemars::schema_for!(TerminalToolInput)) + .expect("input schema should serialize"); + assert!( + schema.contains("allow_network"), + "schema should advertise allow_network: {schema}" + ); + assert!( + schema.contains("allow_fs_write"), + "schema should advertise allow_fs_write: {schema}" + ); + assert!( + schema.contains("unsandboxed"), + "schema should advertise unsandboxed: {schema}" + ); + } + + #[test] + fn test_sandbox_flags_default_to_none_when_absent() { + // The model is expected to omit the sandbox fields entirely on most + // calls. Make sure deserialization doesn't reject the minimal + // payload and that the fields default to `None` (which the tool + // interprets as "no escalation requested"). + let input: TerminalToolInput = serde_json::from_value(serde_json::json!({ + "command": "echo hi", + "cd": ".", + })) + .expect("minimal input should deserialize"); + assert_eq!(input.allow_network, None); + assert_eq!(input.allow_fs_write, None); + assert_eq!(input.unsandboxed, None); + } }