diff --git a/Cargo.lock b/Cargo.lock index 7d575147f0..61d75b5d19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16200,6 +16200,7 @@ dependencies = [ "serde_json", "smol", "tempfile", + "windows-registry 0.6.1", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4649b38844..b7acc44c89 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -897,6 +897,7 @@ web-time = "1.1.0" webrtc-sys = "0.3.23" wgpu = "29.0.4" windows-core = "0.61" +windows-registry = "0.6.0" yaml-rust2 = "0.8" yawc = { git = "https://github.com/zed-industries/yawc", rev = "71a452f551cac178367eaac5d7418a09afa1f3a2", version = "0.3.3" } zeroize = "1.8" diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 9b7af8a3d2..25fcb9313e 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -5498,9 +5498,12 @@ impl ToolCallEventStream { let (events_tx, events_rx) = mpsc::unbounded::>(); let (_cancellation_tx, cancellation_rx) = watch::channel(false); + // The raw and scoped ids deliberately differ (mirroring + // `scoped_tool_call_id`) so that conflating them fails tests instead + // of silently passing. let stream = ToolCallEventStream::new( "test_id".into(), - acp::ToolCallId::new("test_id"), + acp::ToolCallId::new("0:test_id"), ThreadEventStream(events_tx), None, cancellation_rx, @@ -5516,9 +5519,12 @@ impl ToolCallEventStream { let (events_tx, events_rx) = mpsc::unbounded::>(); let (cancellation_tx, cancellation_rx) = watch::channel(false); + // The raw and scoped ids deliberately differ (mirroring + // `scoped_tool_call_id`) so that conflating them fails tests instead + // of silently passing. let stream = ToolCallEventStream::new( "test_id".into(), - acp::ToolCallId::new("test_id"), + acp::ToolCallId::new("0:test_id"), ThreadEventStream(events_tx), None, cancellation_rx, @@ -5606,6 +5612,11 @@ impl ToolCallEventStream { &self.tool_use_id } + /// The ACP-facing id for this tool call (see [`scoped_tool_call_id`]). + pub fn tool_call_id(&self) -> &acp::ToolCallId { + &self.tool_call_id + } + pub fn update_fields(&self, fields: acp::ToolCallUpdateFields) { self.stream .update_tool_call_fields(&self.tool_call_id, fields, None); @@ -5947,7 +5958,12 @@ impl ToolCallEventStream { ]); let stream = self.stream.clone(); - let tool_use_id = self.tool_use_id.clone(); + // The update must target the ACP-facing (scoped) id: an update keyed + // by the raw provider id matches no existing tool call, and creating + // one from empty fields is rejected downstream ("title is required"), + // which tears down the stream and turns the prompt into a phantom + // decline. + let tool_call_id = self.tool_call_id.clone(); cx.spawn(async move |_cx| { let (response_tx, response_rx) = oneshot::channel(); if let Err(error) = stream @@ -5955,7 +5971,10 @@ impl ToolCallEventStream { .unbounded_send(Ok(ThreadEvent::ToolCallAuthorization( ToolCallAuthorization { tool_call: acp::ToolCallUpdate::new( - tool_use_id.to_string(), + tool_call_id, + // Leave the title untouched so the card keeps + // showing the command (matching the escalation + // flow). acp::ToolCallUpdateFields::new(), ) .meta(acp_thread::meta_with_sandbox_authorization(details)), @@ -5975,6 +5994,7 @@ impl ToolCallEventStream { let outcome = response_rx .await .map_err(|_| anyhow!("authorization channel closed"))?; + ensure_tool_call_authorization_not_interrupted(&outcome)?; match acp_thread::SandboxPermission::from_id(outcome.option_id.0.as_ref()) { Some(acp_thread::SandboxPermission::AllowOnce) => Ok(()), _ => Err(anyhow!("Windows-drive write aborted by user")), @@ -8271,6 +8291,41 @@ mod tests { assert!(!event_stream.sandbox_fallback_granted_for_thread()); } + /// Regression test: the Windows-drive (DrvFs) warning prompt must target + /// the ACP-facing *scoped* tool-call id. When it used the raw provider id, + /// the update matched no existing tool call, the ACP layer rejected the + /// resulting title-less insert ("title is required for a tool call"), the + /// event stream was torn down, and every terminal command was auto-declined + /// without the user ever seeing a prompt. + #[gpui::test] + async fn test_windows_fs_warning_targets_scoped_tool_call_id(cx: &mut TestAppContext) { + crate::tests::init_test(cx); + + let (event_stream, mut receiver) = ToolCallEventStream::test(); + let authorize = cx.update(|cx| event_stream.authorize_windows_fs_warning(cx)); + + let authorization = receiver.expect_authorization().await; + assert_eq!( + &authorization.tool_call.tool_call_id, + event_stream.tool_call_id(), + "the warning prompt must reference the scoped ACP tool-call id, \ + not the raw provider id" + ); + let details = + acp_thread::sandbox_authorization_details_from_meta(&authorization.tool_call.meta) + .expect("warning authorization should include sandbox details"); + assert!(details.warn_windows_fs); + + authorization + .response + .send(acp_thread::SelectedPermissionOutcome::new( + acp::PermissionOptionId::new(acp_thread::SandboxPermission::AllowOnce.as_id()), + acp::PermissionOptionKind::AllowOnce, + )) + .unwrap(); + authorize.await.unwrap(); + } + #[test] fn test_auto_resolve_permission_outcome_uses_once_only_options() { let options = acp_thread::PermissionOptions::Dropdown(vec![ diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 5998153558..97a8566bac 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -634,7 +634,19 @@ async fn run_terminal_tool( // gate is transient (never persisted): on "Continue" the normal flow // (including any escalation prompt) proceeds; on "Abort" the command is // cancelled. It recurs until the warning is disabled in settings. - if sandboxing && !want_unsandboxed && persistent.warn_ntfs_grants { + // + // The warning only makes sense when a WSL sandbox will actually wrap the + // command, so it is skipped when the command is guaranteed to run without + // one (`unsandboxed_floor`: the user already turned the sandbox off for + // this thread) and when WSL is structurally absent (no registered distro — + // sandbox creation is guaranteed to fail, and the creation-failure + // fallback prompt handles that conversation instead). + if sandboxing + && !want_unsandboxed + && !unsandboxed_floor + && persistent.warn_ntfs_grants + && sandbox::wsl_distro_registered() + { let effective = event_stream.effective_sandbox_request(&request, &persistent); let contains_windows_fs = effective .write_paths @@ -650,15 +662,15 @@ async fn run_terminal_tool( .any(|path| sandbox::path_is_on_windows_drive(path)) }); if contains_windows_fs - && cx + && let Err(error) = cx .update(|cx| event_stream.authorize_windows_fs_warning(cx)) .await - .is_err() { - return Ok( - "Command cancelled: the user declined to run a command whose sandbox writes to a Windows drive." - .to_string(), - ); + // Carry the underlying error so a prompt-delivery failure is + // distinguishable from a genuine user abort. + return Ok(format!( + "Command cancelled: the user declined to run a command whose sandbox writes to a Windows drive ({error})." + )); } } diff --git a/crates/recent_projects/Cargo.toml b/crates/recent_projects/Cargo.toml index 99a563aff4..033e47db15 100644 --- a/crates/recent_projects/Cargo.toml +++ b/crates/recent_projects/Cargo.toml @@ -54,7 +54,7 @@ zed_actions.workspace = true indoc.workspace = true [target.'cfg(target_os = "windows")'.dependencies] -windows-registry = "0.6.0" +windows-registry.workspace = true [dev-dependencies] editor = { workspace = true, features = ["test-support"] } diff --git a/crates/sandbox/Cargo.toml b/crates/sandbox/Cargo.toml index 3ff373dd8a..a04f7b2bc2 100644 --- a/crates/sandbox/Cargo.toml +++ b/crates/sandbox/Cargo.toml @@ -74,3 +74,6 @@ tempfile.workspace = true [target.'cfg(target_os = "windows")'.dependencies] smol.workspace = true +# Structural WSL-installed check (registered distros under HKCU\...\Lxss) +# without spawning `wsl.exe`. +windows-registry.workspace = true diff --git a/crates/sandbox/src/sandbox.rs b/crates/sandbox/src/sandbox.rs index 15b3edcec6..866e16a104 100644 --- a/crates/sandbox/src/sandbox.rs +++ b/crates/sandbox/src/sandbox.rs @@ -141,6 +141,23 @@ pub fn path_is_on_windows_drive(path: &Path) -> bool { } } +/// Whether WSL is structurally present (at least one registered distro), via a +/// cheap registry read with no `wsl.exe` round-trip. Always `false` off +/// Windows. Suitable only for UI gating (e.g. suppressing DrvFs +/// sandbox-integrity warnings on machines where no WSL sandbox could ever be +/// constructed); enforcement paths must keep relying on the real in-WSL probe, +/// which is authoritative about whether a sandbox can actually be built. +pub fn wsl_distro_registered() -> bool { + #[cfg(target_os = "windows")] + { + windows_wsl::wsl_distro_registered() + } + #[cfg(not(target_os = "windows"))] + { + false + } +} + impl SandboxPolicy { /// Combine two policy layers. Filesystem/network grants are unioned into the /// least-restrictive policy that satisfies both layers, while protected paths diff --git a/crates/sandbox/src/windows_wsl.rs b/crates/sandbox/src/windows_wsl.rs index 32f24c4b48..21158d4f17 100644 --- a/crates/sandbox/src/windows_wsl.rs +++ b/crates/sandbox/src/windows_wsl.rs @@ -261,6 +261,28 @@ pub fn path_is_on_windows_drive(path: &Path) -> bool { ) } +/// Whether WSL is structurally present: at least one distro is registered +/// under `HKCU\Software\Microsoft\Windows\CurrentVersion\Lxss`. A cheap +/// registry read (no `wsl.exe` spawn), suitable for UI gating decisions like +/// suppressing sandbox-integrity warnings on machines where no WSL sandbox +/// could ever be constructed. It is *not* an availability guarantee: a +/// registered distro can still fail the real environment probe (no `bwrap`, +/// unhealthy WSL service, …), so enforcement paths must keep relying on +/// [`probe_environment`]'s in-WSL round-trip. +pub fn wsl_distro_registered() -> bool { + windows_registry::CURRENT_USER + .open("Software\\Microsoft\\Windows\\CurrentVersion\\Lxss") + .and_then(|lxss_key| { + Ok(lxss_key.keys()?.any(|key| { + lxss_key + .open(&key) + .and_then(|distro| distro.get_string("DistributionName")) + .is_ok() + })) + }) + .unwrap_or(false) +} + /// Resolve a requested write grant to the path we persist for it, in /// **platform-native** form: a Windows drive path stays `C:\...` (NTFS) and a /// WSL path stays a Linux-absolute `/...`. In both cases the path is resolved to