Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 32 additions & 10 deletions crates/agent/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5031,6 +5031,30 @@ impl ToolCallEventStream {
(stream, receiver)
}

/// Like [`Self::test`], but the returned stream shares the provided
/// thread-scoped sandbox grants. This mirrors how a real [`Thread`] builds a
/// distinct event stream per tool call while sharing one set of grants, so
/// tests can exercise sequences of tool calls within the same conversation.
// Only the macOS-gated terminal sandbox regression test uses this, so match
// its `cfg` to avoid a dead-code error on other platforms.
#[cfg(all(test, target_os = "macos"))]
pub(crate) fn test_with_grants(
sandbox_grants: Rc<RefCell<ThreadSandboxGrants>>,
) -> (Self, ToolCallEventStreamReceiver) {
let (events_tx, events_rx) = mpsc::unbounded::<Result<ThreadEvent>>();
let (_cancellation_tx, cancellation_rx) = watch::channel(false);

let stream = ToolCallEventStream::new(
"test_id".into(),
ThreadEventStream(events_tx),
None,
cancellation_rx,
sandbox_grants,
);

(stream, ToolCallEventStreamReceiver(events_rx))
}

#[cfg(any(test, feature = "test-support"))]
pub fn test_with_cancellation() -> (Self, ToolCallEventStreamReceiver, watch::Sender<bool>) {
let (events_tx, events_rx) = mpsc::unbounded::<Result<ThreadEvent>>();
Expand Down Expand Up @@ -5419,7 +5443,6 @@ impl ToolCallEventStream {
Ok(())
}
Some(acp_thread::SandboxPermission::AllowAlways) => {
sandbox_grants.borrow_mut().record(request);
Self::persist_sandbox_always_permission(request, fs, cx);
Ok(())
}
Expand Down Expand Up @@ -7019,7 +7042,9 @@ mod tests {
}

#[gpui::test]
async fn test_authorize_sandbox_allow_always_records_current_grant(cx: &mut TestAppContext) {
async fn test_authorize_sandbox_allow_always_does_not_cache_thread_grant(
cx: &mut TestAppContext,
) {
crate::tests::init_test(cx);

let (event_stream, mut receiver) = ToolCallEventStream::test();
Expand Down Expand Up @@ -7095,18 +7120,15 @@ mod tests {
assert!(send_result.is_ok());
authorize.await.unwrap();

// "Allow always" persists to settings only
let effective = event_stream.effective_sandbox_request(
&SandboxRequest::default(),
&agent_settings::SandboxPermissions::default(),
);
assert_eq!(
effective.write_paths,
vec![
PathBuf::from("/tmp/build"),
PathBuf::from("/tmp/cache"),
PathBuf::from("/tmp/logs"),
PathBuf::from("/tmp/secret"),
]
assert!(
effective.write_paths.is_empty(),
"allow always should not record an in-memory thread grant: {:?}",
effective.write_paths
);
}

Expand Down
125 changes: 125 additions & 0 deletions crates/agent/src/tools/terminal_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2928,6 +2928,131 @@ mod tests {
assert_eq!(environment.terminal_creation_count(), 0);
}

/// Regression test: choosing "Allow always" on a sandbox prompt must persist
/// the grant to settings *only* — it must not also cache an in-memory thread
/// grant. Otherwise removing the entry from settings.json wouldn't revoke it
/// within the same conversation, and a later identical command would run
/// without prompting again (the bug this guards against).
#[cfg(target_os = "macos")]
#[gpui::test]
async fn test_allow_always_grant_is_revocable_via_settings(cx: &mut gpui::TestAppContext) {
use feature_flags::FeatureFlagAppExt as _;

crate::tests::init_test(cx);
// Auto-allow the terminal tool itself so only the *sandbox* escalation
// prompts, and start with no persisted sandbox grants (mirroring a
// settings.json that doesn't grant the path — e.g. after the user
// removed it).
cx.update(|cx| {
cx.update_flags(true, vec!["sandboxing".to_string()]);
let mut settings = agent_settings::AgentSettings::get_global(cx).clone();
settings.tool_permissions.default = settings::ToolPermissionMode::Allow;
settings.tool_permissions.tools.remove(TerminalTool::NAME);
settings.sandbox_permissions = agent_settings::SandboxPermissions::default();
agent_settings::AgentSettings::override_global(settings, cx);
});

let fs = fs::FakeFs::new(cx.executor());
fs.insert_tree("/root", serde_json::json!({})).await;
let project = project::Project::test(fs, ["/root".as_ref()], cx).await;

// Both tool calls belong to the same conversation, so they share one set
// of in-memory thread sandbox grants, exactly like a real `Thread`.
let sandbox_grants = std::rc::Rc::new(std::cell::RefCell::new(
crate::sandboxing::ThreadSandboxGrants::default(),
));

let input = serde_json::json!({
"command": "touch build/output",
"cd": "root",
"fs_write_paths": ["build"],
"reason": "needs to write build artifacts",
});

// ---- First call: the user picks "Allow always".
let environment = std::rc::Rc::new(cx.update(|cx| {
crate::tests::FakeThreadEnvironment::default().with_terminal(
crate::tests::FakeTerminalHandle::new_with_immediate_exit(cx, 0),
)
}));
#[allow(clippy::arc_with_non_send_sync)]
let tool = std::sync::Arc::new(SandboxedTerminalTool::new(
project.clone(),
environment.clone(),
));
let (event_stream, mut receiver) =
crate::ToolCallEventStream::test_with_grants(sandbox_grants.clone());
let resolved: SandboxedTerminalToolInput = serde_json::from_value(input.clone()).unwrap();
let task = cx.update(|cx| tool.run(crate::ToolInput::resolved(resolved), event_stream, cx));

let authorization = receiver.expect_authorization().await;
authorization
.response
.send(acp_thread::SelectedPermissionOutcome::new(
acp::PermissionOptionId::new("allow_always"),
acp::PermissionOptionKind::AllowAlways,
))
.expect("authorization response should send");
task.await.expect("granted command should run");
assert_eq!(environment.terminal_creation_count(), 1);

// The grant must NOT have been cached in the shared thread grants: with
// empty persistent settings, the thread grants should cover nothing.
let cached = sandbox_grants.borrow().effective_with_persistent(
&crate::sandboxing::SandboxRequest::default(),
&agent_settings::SandboxPermissions::default(),
);
assert!(
cached.write_paths.is_empty(),
"\"Allow always\" must not cache an in-memory thread grant: {:?}",
cached.write_paths
);

// ---- Second call: the same request, with the path absent from
// settings, must prompt again instead of being silently allowed.
let environment2 = std::rc::Rc::new(cx.update(|cx| {
crate::tests::FakeThreadEnvironment::default().with_terminal(
crate::tests::FakeTerminalHandle::new_with_immediate_exit(cx, 0),
)
}));
#[allow(clippy::arc_with_non_send_sync)]
let tool2 = std::sync::Arc::new(SandboxedTerminalTool::new(
project.clone(),
environment2.clone(),
));
let (event_stream2, mut receiver2) =
crate::ToolCallEventStream::test_with_grants(sandbox_grants.clone());
let resolved2: SandboxedTerminalToolInput = serde_json::from_value(input).unwrap();
let task2 =
cx.update(|cx| tool2.run(crate::ToolInput::resolved(resolved2), event_stream2, cx));

let authorization2 = receiver2.expect_authorization().await;
let details =
acp_thread::sandbox_authorization_details_from_meta(&authorization2.tool_call.meta)
.expect("the identical request should prompt for sandbox authorization again");
assert!(
details
.write_paths
.iter()
.any(|path| path.ends_with("build")),
"re-prompt should request the same write path: {:?}",
details.write_paths
);

authorization2
.response
.send(acp_thread::SelectedPermissionOutcome::new(
acp::PermissionOptionId::new("deny"),
acp::PermissionOptionKind::RejectOnce,
))
.expect("authorization response should send");
let result = task2
.await
.expect("denied sandbox request returns model-readable output");
assert!(result.contains("user denied the requested sandbox permissions"));
assert_eq!(environment2.terminal_creation_count(), 0);
}

fn host_request(list: &[&str]) -> NetworkRequest {
NetworkRequest::Hosts(
list.iter()
Expand Down
5 changes: 2 additions & 3 deletions crates/agent_ui/src/conversation_view/thread_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7315,12 +7315,11 @@ impl ThreadView {
)
.icon_size(IconSize::XSmall)
.icon_color(Color::Muted)
.tooltip(Tooltip::text("Open the unsandboxed execution setting"))
.tooltip(Tooltip::text("Open the sandbox permission settings"))
.on_click(|_event, window, cx| {
window.dispatch_action(
Box::new(zed_actions::OpenSettingsAt {
path: zed_actions::AGENT_ALLOW_UNSANDBOXED_SETTINGS_PATH
.to_string(),
path: zed_actions::AGENT_SANDBOX_SETTINGS_PATH.to_string(),
target: None,
}),
cx,
Expand Down
1 change: 1 addition & 0 deletions crates/settings_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ fuzzy.workspace = true
gpui.workspace = true
heck.workspace = true
http_client.workspace = true
http_proxy.workspace = true
itertools.workspace = true
language.workspace = true
language_model.workspace = true
Expand Down
43 changes: 14 additions & 29 deletions crates/settings_ui/src/page_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use crate::{
SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names,
pages::{
open_audio_test_window, render_edit_prediction_setup_page, render_external_agents_page,
render_llm_providers_page, render_mcp_servers_page, render_skills_setup_page,
render_tool_permissions_setup_page,
render_llm_providers_page, render_mcp_servers_page, render_sandbox_settings_page,
render_skills_setup_page, render_tool_permissions_setup_page,
},
};

Expand Down Expand Up @@ -7892,6 +7892,18 @@ fn ai_page(cx: &App) -> SettingsPage {
files: USER | PROJECT,
render: render_skills_setup_page,
}),
SettingsPageItem::SubPageLink(SubPageLink {
title: "Sandbox".into(),
r#type: Default::default(),
json_path: Some(zed_actions::AGENT_SANDBOX_SETTINGS_PATH),
description: Some(
"Review and change the elevated terminal sandbox permissions that are always allowed without prompting."
.into(),
),
in_json: true,
files: USER,
render: render_sandbox_settings_page,
}),
SettingsPageItem::SubPageLink(SubPageLink {
title: "Tool Permissions".into(),
r#type: Default::default(),
Expand Down Expand Up @@ -7930,33 +7942,6 @@ fn ai_page(cx: &App) -> SettingsPage {
}

items.extend([
SettingsPageItem::SettingItem(SettingItem {
title: "Allow Unsandboxed Terminal Commands",
description: "When enabled, agent terminal commands run without the OS sandbox instead of prompting when the sandbox can't be created.",
field: Box::new(SettingField {
organization_override: None,
json_path: Some(zed_actions::AGENT_ALLOW_UNSANDBOXED_SETTINGS_PATH),
pick: |settings_content| {
settings_content
.agent
.as_ref()?
.sandbox_permissions
.as_ref()?
.allow_unsandboxed
.as_ref()
},
write: |settings_content, value, _| {
settings_content
.agent
.get_or_insert_default()
.sandbox_permissions
.get_or_insert_default()
.allow_unsandboxed = value;
},
}),
metadata: None,
files: USER,
}),
SettingsPageItem::SettingItem(SettingItem {
title: "Single File Review",
description: "When enabled, agent edits will also be displayed in single-file buffers for review.",
Expand Down
2 changes: 2 additions & 0 deletions crates/settings_ui/src/pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod external_agents_page;
mod feature_flags;
mod llm_providers_page;
mod mcp_servers_page;
mod sandbox_settings;
mod skill_creator;
mod skills_setup;
mod tool_permissions_setup;
Expand All @@ -18,6 +19,7 @@ pub(crate) use external_agents_page::{CustomAgentForm, render_external_agents_pa
pub(crate) use feature_flags::render_feature_flags_page;
pub(crate) use llm_providers_page::render_llm_providers_page;
pub(crate) use mcp_servers_page::{McpServerForm, render_mcp_servers_page};
pub(crate) use sandbox_settings::render_sandbox_settings_page;
pub use skill_creator::SkillCreatorOpenMode;
pub(crate) use skill_creator::{
SkillCreatorEvent, SkillCreatorPage, render_skill_creator_page, skill_url_from_clipboard,
Expand Down
Loading
Loading