Wrap agent terminal commands in macOS Seatbelt sandbox (#57431)
Stacked on top of #57430. When the `sandboxing` feature flag is on (macOS only), agent-run terminal commands are launched under `/usr/bin/sandbox-exec` with a per-command Seatbelt policy: - **Reads:** any path on the filesystem. - **Writes:** each project worktree plus a per-command `$TMPDIR`. - **Network:** blocked. The model can request relaxations on individual calls via three new input flags on the `terminal` tool: - `allow_network: true` - `allow_fs_write: true` - `unsandboxed: true` Each one triggers a separate always-prompt user approval (bypassing any `always_allow` rules, since escalation is a stronger trust boundary than the baseline command approval). The flags are silently ignored when sandboxing is off, so the model can't surreptitiously change runtime behavior by setting them in the no-op case. The per-command tempdir is provisioned regardless of sandbox state so the model can't infer whether the sandbox is in effect by looking at `$TMPDIR`. The Seatbelt config file's lifetime is tied to the `Terminal` entity via an opaque RAII handle (`SandboxConfigHandle = Box<dyn Any + Send>`), so cancellation paths drop it automatically. ### Notes for review - The writable scope passed to the sandbox is the project's worktree paths plus the per-command tempdir, **not** the resolved `cd` working directory — `cd` is model-controlled, and using it as the writable scope would let the model widen its own write permissions outside the project. - The new tool flags are always present in the JSON schema (with `#[serde(default)]`), even when the sandbox prompt section isn't rendered. The system prompt only documents them when the section is present, so the model shouldn't try to use them when it's not — but the schema doesn't actively reject them, just ignores them. - I opted not to add an end-to-end test for the escalation prompt path: the existing `FakeThreadEnvironment` ignores the new params and toggling the feature flag in tests is more wiring than felt worth it for a first cut. The pure-function pieces (`sandbox_approval_title`, schema, deserialization) are covered, and the sandbox crate itself has end-to-end tests that actually invoke `sandbox-exec`. Release Notes: - N/A --------- Co-authored-by: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com>
This commit is contained in:
1 parent
18c70a73a6
commit
356c8cae07
9 files changed
+348
-5
No files matched your search
Generated
+1
@@ -111,6 +111,7 @@ dependencies = [
|
||||
"project",
|
||||
"prompt_store",
|
||||
"rand 0.9.4",
|
||||
"sandbox",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"settings",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -2955,6 +2955,7 @@ impl AcpThread {
|
||||
extra_env: Vec<acp::EnvVariable>,
|
||||
cwd: Option<PathBuf>,
|
||||
output_byte_limit: Option<u64>,
|
||||
sandbox_wrap: Option<SandboxWrap>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Task<Result<Entity<Terminal>>> {
|
||||
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,
|
||||
)
|
||||
});
|
||||
|
||||
@@ -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<PathBuf>,
|
||||
/// 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<dyn std::any::Any + Send>;
|
||||
|
||||
/// 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<String>,
|
||||
sandbox_wrap: Option<SandboxWrap>,
|
||||
) -> anyhow::Result<(String, Vec<String>, Option<SandboxConfigHandle>)> {
|
||||
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<Markdown>,
|
||||
@@ -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<AtomicBool>,
|
||||
/// 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<SandboxConfigHandle>,
|
||||
}
|
||||
|
||||
pub struct TerminalOutput {
|
||||
@@ -48,11 +128,13 @@ impl Terminal {
|
||||
output_byte_limit: Option<usize>,
|
||||
terminal: Entity<terminal::Terminal>,
|
||||
language_registry: Arc<LanguageRegistry>,
|
||||
sandbox_config: Option<SandboxConfigHandle>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> 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(),
|
||||
|
||||
@@ -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"] }
|
||||
|
||||
|
||||
@@ -2610,12 +2610,22 @@ impl ThreadEnvironment for NativeThreadEnvironment {
|
||||
fn create_terminal(
|
||||
&self,
|
||||
command: String,
|
||||
extra_env: Vec<acp::EnvVariable>,
|
||||
cwd: Option<PathBuf>,
|
||||
output_byte_limit: Option<u64>,
|
||||
sandbox_wrap: Option<acp_thread::SandboxWrap>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Task<Result<Rc<dyn TerminalHandle>>> {
|
||||
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();
|
||||
|
||||
@@ -200,8 +200,10 @@ impl crate::ThreadEnvironment for FakeThreadEnvironment {
|
||||
fn create_terminal(
|
||||
&self,
|
||||
_command: String,
|
||||
_extra_env: Vec<acp::EnvVariable>,
|
||||
_cwd: Option<std::path::PathBuf>,
|
||||
_output_byte_limit: Option<u64>,
|
||||
_sandbox_wrap: Option<acp_thread::SandboxWrap>,
|
||||
_cx: &mut AsyncApp,
|
||||
) -> Task<Result<Rc<dyn crate::TerminalHandle>>> {
|
||||
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<acp::EnvVariable>,
|
||||
_cwd: Option<std::path::PathBuf>,
|
||||
_output_byte_limit: Option<u64>,
|
||||
_sandbox_wrap: Option<acp_thread::SandboxWrap>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Task<Result<Rc<dyn crate::TerminalHandle>>> {
|
||||
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,
|
||||
|
||||
@@ -668,8 +668,10 @@ pub trait ThreadEnvironment {
|
||||
fn create_terminal(
|
||||
&self,
|
||||
command: String,
|
||||
extra_env: Vec<acp::EnvVariable>,
|
||||
cwd: Option<PathBuf>,
|
||||
output_byte_limit: Option<u64>,
|
||||
sandbox_wrap: Option<acp_thread::SandboxWrap>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Task<Result<Rc<dyn TerminalHandle>>>;
|
||||
|
||||
|
||||
@@ -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<u64>,
|
||||
/// 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<bool>,
|
||||
/// 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<bool>,
|
||||
/// 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<bool>,
|
||||
}
|
||||
|
||||
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<PathBuf> = cx.update(|cx| {
|
||||
self.project
|
||||
.read(cx)
|
||||
.worktrees(cx)
|
||||
.map(|w| w.read(cx).abs_path().to_path_buf())
|
||||
.collect::<Vec<_>>()
|
||||
});
|
||||
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);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user