mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
fix(buzz-dev-mcp): resolve non-WSL bash so the MCP shell works on Windows (#1119)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
20
.github/workflows/ci.yml
vendored
20
.github/workflows/ci.yml
vendored
@@ -688,6 +688,26 @@ jobs:
|
||||
run: cargo clippy --workspace --all-targets --target $env:TARGET -- -D warnings
|
||||
- name: Check (workspace)
|
||||
run: cargo check --workspace --all-targets --target $env:TARGET
|
||||
- name: Test (buzz-dev-mcp)
|
||||
# The Windows-only bash resolver lives in buzz-dev-mcp; its unit tests
|
||||
# only gate if this crate is tested ON Windows.
|
||||
run: cargo test -p buzz-dev-mcp --target $env:TARGET
|
||||
# The bundled-bash staging (PortableGit download, SFX extract, mingw64 drop)
|
||||
# runs only in release.yml on tag — so without this step the agent's only
|
||||
# Windows transport would ship UNEXERCISED until a tagged release hits users.
|
||||
# Stage the tree and spawn the staged bash on a real coreutils pipeline: this
|
||||
# gates the SFX `-o` POSIX-path extraction, that bash.exe still spawns after
|
||||
# mingw64/ is dropped, and that the lazily-loaded MSYS DLL closure (msys-2.0.dll,
|
||||
# coreutils) survives — the exact behaviors unconfirmable off a non-Windows host.
|
||||
- name: Smoke-test bundled bash staging
|
||||
shell: bash
|
||||
run: |
|
||||
set -euo pipefail
|
||||
stage_dir="$RUNNER_TEMP/git-bash"
|
||||
scripts/stage-windows-bash.sh "$stage_dir"
|
||||
out=$("$stage_dir/usr/bin/bash.exe" -c 'echo hello | tr a-z A-Z')
|
||||
[[ "$out" == "HELLO" ]] || { echo "staged bash pipeline failed: got '$out'" >&2; exit 1; }
|
||||
echo "staged bash spawned and ran a coreutils pipeline"
|
||||
- name: Check (Tauri crate)
|
||||
run: cargo check --manifest-path desktop/src-tauri/Cargo.toml --target $env:TARGET
|
||||
env:
|
||||
|
||||
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -894,6 +894,7 @@ dependencies = [
|
||||
"tokio-util",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"windows-sys 0.61.2",
|
||||
"zeroize",
|
||||
]
|
||||
|
||||
|
||||
@@ -38,3 +38,12 @@ image = { version = "0.25", default-features = false, features = ["jpeg", "png",
|
||||
|
||||
[target.'cfg(unix)'.dependencies]
|
||||
nix = { version = "0.31", default-features = false, features = ["signal", "process"] }
|
||||
|
||||
# Windows Job Object APIs for the shell tool's timeout kill path: terminating a
|
||||
# job kills the bash child AND every MSYS grandchild it forked, the Windows
|
||||
# analogue of the Unix killpg above. windows-sys 0.61 is already workspace-
|
||||
# resident (pulled transitively), so this adds no new crate. Win32_Security is
|
||||
# required because CreateJobObjectW takes a SECURITY_ATTRIBUTES parameter;
|
||||
# Win32_System_Threading supplies IO_COUNTERS inside the extended-limit struct.
|
||||
[target.'cfg(windows)'.dependencies]
|
||||
windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_System_JobObjects", "Win32_System_Threading"] }
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
#![forbid(unsafe_code)]
|
||||
#![cfg_attr(not(windows), forbid(unsafe_code))]
|
||||
#![cfg_attr(windows, deny(unsafe_code))]
|
||||
use rmcp::{
|
||||
handler::server::{router::tool::ToolRouter, wrapper::Parameters},
|
||||
model::{CallToolResult, ServerCapabilities, ServerInfo},
|
||||
|
||||
@@ -143,9 +143,16 @@ mod tests {
|
||||
#[test]
|
||||
fn read_allows_absolute_path() {
|
||||
let dir = tempdir().expect("tempdir");
|
||||
// A real file in a SECOND tempdir, genuinely outside the workspace
|
||||
// root — proves absolute paths beyond workdir resolve, without a
|
||||
// Unix-only system path like /etc/hosts (which is C:\etc\hosts on
|
||||
// Windows and does not exist).
|
||||
let outside = tempdir().expect("tempdir");
|
||||
let target = outside.path().join("outside.txt");
|
||||
fs::write(&target, b"localhost").expect("write");
|
||||
let state = make_state(dir.path());
|
||||
let p = ReadFileParams {
|
||||
path: "/etc/hosts".into(),
|
||||
path: target.display().to_string(),
|
||||
offset: None,
|
||||
limit: None,
|
||||
workdir: Some(dir.path().display().to_string()),
|
||||
@@ -153,7 +160,7 @@ mod tests {
|
||||
let out = run(&state, p).expect("ok");
|
||||
assert!(
|
||||
out.contains("localhost"),
|
||||
"expected /etc/hosts content, got: {out}"
|
||||
"expected out-of-workspace file content, got: {out}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -143,7 +143,11 @@ pub async fn run(
|
||||
));
|
||||
}
|
||||
|
||||
let mut cmd = Command::new("bash");
|
||||
let bash = match resolve_bash(&state.shim.path_env) {
|
||||
Ok(path) => path,
|
||||
Err(msg) => return Ok(CallToolResult::error(vec![Content::text(msg)])),
|
||||
};
|
||||
let mut cmd = Command::new(&bash);
|
||||
cmd.arg("-c").arg(&p.command);
|
||||
cmd.current_dir(&workdir);
|
||||
cmd.env("PATH", &state.shim.path_env);
|
||||
@@ -170,15 +174,11 @@ pub async fn run(
|
||||
|
||||
let pid = child.id();
|
||||
|
||||
struct PgidGuard(Option<u32>);
|
||||
impl Drop for PgidGuard {
|
||||
fn drop(&mut self) {
|
||||
if let Some(pid) = self.0 {
|
||||
kill_process_group_immediate(pid as i32);
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut pgid_guard = PgidGuard(pid);
|
||||
// KillGroup ties the spawned bash and all its descendants to a single kill
|
||||
// primitive (Unix process group / Windows Job Object). Built from the live
|
||||
// child so the Windows job can take the process handle, which only exists
|
||||
// after spawn. Held for the whole run; its Drop is the last-resort reaper.
|
||||
let mut kill_group = KillGroup::new(&child, pid);
|
||||
|
||||
let stdout_pipe = child.stdout.take();
|
||||
let stderr_pipe = child.stderr.take();
|
||||
@@ -202,19 +202,17 @@ pub async fn run(
|
||||
biased;
|
||||
_ = ct.cancelled() => {
|
||||
// Kill process group, reap child, abort reader tasks.
|
||||
if let Some(pid) = pid {
|
||||
kill_process_group_immediate(pid as i32);
|
||||
}
|
||||
kill_group.kill_immediate();
|
||||
// Bounded reap so we don't leak zombies. If reap times out,
|
||||
// PgidGuard drop will SIGKILL again as a last resort.
|
||||
// KillGroup drop will kill again as a last resort.
|
||||
match tokio::time::timeout(Duration::from_secs(1), child.wait()).await {
|
||||
Ok(Ok(_)) => { pgid_guard.0 = None; } // reaped; disarm guard
|
||||
Ok(Ok(_)) => { kill_group.disarm(); } // reaped; disarm guard
|
||||
Ok(Err(e)) => {
|
||||
tracing::debug!("cancel: child wait error: {e}");
|
||||
// Leave pgid_guard armed for drop-kill.
|
||||
// Leave kill_group armed for drop-kill.
|
||||
}
|
||||
Err(_) => {
|
||||
tracing::debug!("cancel: child reap timed out; guard will SIGKILL on drop");
|
||||
tracing::debug!("cancel: child reap timed out; guard will kill on drop");
|
||||
}
|
||||
}
|
||||
stdout_handle.abort();
|
||||
@@ -229,9 +227,7 @@ pub async fn run(
|
||||
}
|
||||
Err(_) => {
|
||||
// Kill process group — this closes the pipes, causing reads to EOF.
|
||||
if let Some(pid) = pid {
|
||||
kill_process_group_graceful(pid as i32).await;
|
||||
}
|
||||
kill_group.kill_graceful().await;
|
||||
// Reap the child so it doesn't become a zombie.
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
@@ -261,9 +257,7 @@ pub async fn run(
|
||||
};
|
||||
|
||||
if !timed_out {
|
||||
if let Some(pid) = pid {
|
||||
kill_process_group_graceful(pid as i32).await;
|
||||
}
|
||||
kill_group.kill_graceful().await;
|
||||
}
|
||||
|
||||
let stdout_cap = match tokio::time::timeout(Duration::from_secs(5), &mut stdout_handle).await {
|
||||
@@ -308,10 +302,151 @@ pub async fn run(
|
||||
"notes": notes,
|
||||
});
|
||||
let text = serde_json::to_string_pretty(&body).unwrap_or_else(|_| "{}".into());
|
||||
pgid_guard.0 = None;
|
||||
kill_group.disarm();
|
||||
Ok(CallToolResult::success(vec![Content::text(text)]))
|
||||
}
|
||||
|
||||
/// The bundled bash subtree's directory name under the install root, and the
|
||||
/// relative path to its `bash.exe`. This is the THREE-FILE PATH CONTRACT — it must
|
||||
/// stay byte-identical with:
|
||||
/// 1. `scripts/bundle-sidecars.sh` — stages the bash tree to
|
||||
/// `desktop/src-tauri/binaries/git-bash/` (the bundle-source dir).
|
||||
/// 2. `desktop/scripts/build-release-config.mjs` — emits the Windows-only
|
||||
/// `bundle.resources` Map `{ "binaries/git-bash": "git-bash" }`, whose TARGET
|
||||
/// (`git-bash`) is what Tauri's NSIS/MSI installer stages next to the exe.
|
||||
/// 3. this resolver — joins `current_exe().parent()` + `git-bash\usr\bin\bash.exe`.
|
||||
///
|
||||
/// Drift between (2)'s target and this string ships a working bundle but a broken
|
||||
/// runtime path. Keep all three in lockstep.
|
||||
///
|
||||
/// The bundled constant points at `usr\bin\bash.exe` (the real MSYS2 bash, which
|
||||
/// boots from its co-located `msys-2.0.dll` and needs only `usr/`, no `mingw64/`)
|
||||
/// — NOT the `bin\bash.exe` launcher shim, which refuses to start without a sibling
|
||||
/// `mingw64\bin` marker the bundle deliberately drops. The installed-Git branch
|
||||
/// below stays on `bin\bash.exe` on purpose: a real Git-for-Windows install has
|
||||
/// `mingw64/`, so its launcher is the correct entry and sets up MSYSTEM/PATH.
|
||||
/// Different tree shape -> different correct entry point.
|
||||
#[cfg(windows)]
|
||||
const BUNDLED_BASH_REL: &str = r"git-bash\usr\bin\bash.exe";
|
||||
|
||||
/// Resolve a genuine, non-WSL bash to an absolute path so we spawn it directly
|
||||
/// instead of letting `Command::new("bash")` re-enter PATH search — on Windows
|
||||
/// that search finds `System32\bash.exe` (the WSL launcher), which fails at spawn
|
||||
/// with `0x8007072c` and can never run the agent's POSIX commands.
|
||||
///
|
||||
/// On Unix, bare `bash` resolved via PATH is correct and was never broken, so the
|
||||
/// resolver is a no-op there. The probe logic is Windows-only.
|
||||
#[cfg(not(windows))]
|
||||
fn resolve_bash(_path_env: &str) -> Result<PathBuf, String> {
|
||||
Ok(PathBuf::from("bash"))
|
||||
}
|
||||
|
||||
/// Windows bash resolution. Probe order (first hit wins):
|
||||
/// 1. `GIT_BASH` env override (escape hatch / explicit operator choice).
|
||||
/// 2. Installed Git for Windows (fast path when the user has Git).
|
||||
/// 3. The bundled bash staged next to our exe (guaranteed target — this
|
||||
/// is what makes a bare, Git-less host work since the app is self-contained).
|
||||
/// 4. PATH scan, EXCLUDING System32 (so we never resolve WSL's `bash.exe`).
|
||||
///
|
||||
/// No bash found -> actionable error returned BEFORE spawn.
|
||||
#[cfg(windows)]
|
||||
fn resolve_bash(path_env: &str) -> Result<PathBuf, String> {
|
||||
if let Some(p) = std::env::var_os("GIT_BASH").map(PathBuf::from) {
|
||||
if p.is_file() {
|
||||
return Ok(p);
|
||||
}
|
||||
}
|
||||
|
||||
for root in ["ProgramFiles", "LocalAppData"] {
|
||||
if let Some(base) = std::env::var_os(root) {
|
||||
let candidate = match root {
|
||||
"LocalAppData" => PathBuf::from(&base).join("Programs").join("Git"),
|
||||
_ => PathBuf::from(&base).join("Git"),
|
||||
}
|
||||
.join("bin")
|
||||
.join("bash.exe");
|
||||
if candidate.is_file() {
|
||||
return Ok(candidate);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Bundled bash, located relative to OUR OWN executable. On Windows, Tauri
|
||||
// stages `bundle.resources` flat in the directory that contains the exe
|
||||
// (tauri 2.11.2 `resource_dir()` == exe parent on Windows), and every sidecar
|
||||
// — including this one — lives in that same dir. This relative-to-self resolution
|
||||
// is Windows-ONLY: macOS stages resources to `../Resources` and Linux to
|
||||
// `usr/lib/<app>`, so a cross-platform "resource relative to exe" helper would
|
||||
// be wrong on those platforms.
|
||||
if let Ok(exe) = std::env::current_exe() {
|
||||
if let Some(dir) = exe.parent() {
|
||||
if let Some(p) = bundled_bash(dir) {
|
||||
return Ok(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(p) = scan_path_for_bash(path_env, std::env::var_os("SystemRoot").map(PathBuf::from))
|
||||
{
|
||||
return Ok(p);
|
||||
}
|
||||
|
||||
Err("no bash found: install Git for Windows, or set GIT_BASH to a bash.exe path".into())
|
||||
}
|
||||
|
||||
/// Compute the bundled bash path relative to the install dir (the exe's parent),
|
||||
/// `is_file`-gated so dev/CI builds without a staged resource return None and let
|
||||
/// the caller fall through cleanly — never returning a non-existent path that
|
||||
/// would fail later at spawn with a worse message.
|
||||
#[cfg(windows)]
|
||||
fn bundled_bash(install_dir: &Path) -> Option<PathBuf> {
|
||||
let bundled = install_dir.join(BUNDLED_BASH_REL);
|
||||
bundled.is_file().then_some(bundled)
|
||||
}
|
||||
|
||||
/// True if `dir` is `root` or lives under it, comparing path components
|
||||
/// case-INsensitively. Windows paths are case-insensitive, but `Path::starts_with`
|
||||
/// compares components case-sensitively on every platform — so a PATH entry spelled
|
||||
/// `C:\WINDOWS\System32` would slip past a `%SystemRoot%`=`C:\Windows` prefix test
|
||||
/// and let WSL's `System32\bash.exe` be resolved, reintroducing the `0x8007072c`
|
||||
/// spawn failure. Component-wise comparison (not a lowercased substring match) avoids
|
||||
/// a false hit on a sibling like `C:\Windows2`.
|
||||
#[cfg(windows)]
|
||||
fn is_under_dir(dir: &Path, root: &Path) -> bool {
|
||||
let mut dir_components = dir.components();
|
||||
for root_component in root.components() {
|
||||
match dir_components.next() {
|
||||
Some(d)
|
||||
if d.as_os_str()
|
||||
.eq_ignore_ascii_case(root_component.as_os_str()) => {}
|
||||
_ => return false,
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Scan the child's PATH for `bash.exe`, skipping the Windows system directory
|
||||
/// (`system_root`, normally `%SystemRoot%`) so we never resolve WSL's
|
||||
/// `System32\bash.exe`. PATH is parsed with `std::env::split_paths` (never a
|
||||
/// hand-split on ';') so it matches exactly what the spawned child would see.
|
||||
#[cfg(windows)]
|
||||
fn scan_path_for_bash(path_env: &str, system_root: Option<PathBuf>) -> Option<PathBuf> {
|
||||
for dir in std::env::split_paths(path_env) {
|
||||
if let Some(ref root) = system_root {
|
||||
// Skip System32 (and any other dir under %SystemRoot%) — that's where
|
||||
// WSL's bash.exe lives.
|
||||
if is_under_dir(&dir, root) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
let candidate = dir.join("bash.exe");
|
||||
if candidate.is_file() {
|
||||
return Some(candidate);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn set_process_group(cmd: &mut Command) {
|
||||
cmd.process_group(0);
|
||||
@@ -320,31 +455,182 @@ fn set_process_group(cmd: &mut Command) {
|
||||
#[cfg(not(unix))]
|
||||
fn set_process_group(_cmd: &mut Command) {}
|
||||
|
||||
/// Immediate SIGKILL of the process group. Sync; safe to call from Drop.
|
||||
/// No grace period — used when the parent task is being torn down.
|
||||
/// Kill primitive covering the spawned bash AND every descendant it forks,
|
||||
/// mirroring the same guarantee across platforms.
|
||||
///
|
||||
/// - Unix: the child's process group (set via [`set_process_group`]); kills go
|
||||
/// to the whole group via `killpg`.
|
||||
/// - Windows: a Job Object the child is assigned to at construction. A bare
|
||||
/// `TerminateProcess` on bash leaves MSYS-forked grandchildren (e.g. `sleep`)
|
||||
/// running — they hold the stdout/stderr pipes open, so the reap blocks until
|
||||
/// they self-exit. Terminating the job kills the entire tree atomically.
|
||||
///
|
||||
/// Held for the whole `run`; `Drop` is the last-resort reaper if an explicit
|
||||
/// kill was skipped or failed.
|
||||
#[cfg(unix)]
|
||||
fn kill_process_group_immediate(pid: i32) {
|
||||
use nix::sys::signal::{killpg, Signal};
|
||||
use nix::unistd::Pid;
|
||||
let _ = killpg(Pid::from_raw(pid), Signal::SIGKILL);
|
||||
struct KillGroup(Option<i32>);
|
||||
|
||||
#[cfg(unix)]
|
||||
impl KillGroup {
|
||||
fn new(_child: &tokio::process::Child, pid: Option<u32>) -> Self {
|
||||
Self(pid.map(|p| p as i32))
|
||||
}
|
||||
|
||||
/// Immediate SIGKILL of the process group. Sync; safe to call from Drop.
|
||||
/// No grace period — used when the parent task is being torn down.
|
||||
fn kill_immediate(&self) {
|
||||
use nix::sys::signal::{killpg, Signal};
|
||||
use nix::unistd::Pid;
|
||||
if let Some(pid) = self.0 {
|
||||
let _ = killpg(Pid::from_raw(pid), Signal::SIGKILL);
|
||||
}
|
||||
}
|
||||
|
||||
/// Graceful SIGTERM → 200ms async sleep → SIGKILL. Async; never blocks the runtime.
|
||||
async fn kill_graceful(&self) {
|
||||
use nix::sys::signal::{killpg, Signal};
|
||||
use nix::unistd::Pid;
|
||||
if let Some(pid) = self.0 {
|
||||
let pgid = Pid::from_raw(pid);
|
||||
let _ = killpg(pgid, Signal::SIGTERM);
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
let _ = killpg(pgid, Signal::SIGKILL);
|
||||
}
|
||||
}
|
||||
|
||||
/// Disarm the Drop-time kill once the child has been reaped explicitly.
|
||||
fn disarm(&mut self) {
|
||||
self.0 = None;
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn kill_process_group_immediate(_pid: i32) {}
|
||||
|
||||
/// Graceful SIGTERM → 200ms async sleep → SIGKILL. Async; never blocks the runtime.
|
||||
#[cfg(unix)]
|
||||
async fn kill_process_group_graceful(pid: i32) {
|
||||
use nix::sys::signal::{killpg, Signal};
|
||||
use nix::unistd::Pid;
|
||||
let pgid = Pid::from_raw(pid);
|
||||
let _ = killpg(pgid, Signal::SIGTERM);
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
let _ = killpg(pgid, Signal::SIGKILL);
|
||||
impl Drop for KillGroup {
|
||||
fn drop(&mut self) {
|
||||
self.kill_immediate();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
async fn kill_process_group_graceful(_pid: i32) {}
|
||||
#[cfg(windows)]
|
||||
struct KillGroup {
|
||||
job: windows_sys::Win32::Foundation::HANDLE,
|
||||
}
|
||||
|
||||
// SAFETY: `job` is a raw Win32 HANDLE (`*mut c_void`), which is neither `Send`
|
||||
// nor `Sync` by default. The shell tool's async future holds a `KillGroup`
|
||||
// across an `.await`, so it must be `Send` to be spawned. A job-object handle
|
||||
// is a kernel object reference, not thread-affine: `TerminateJobObject` and
|
||||
// `CloseHandle` are thread-safe, and Rust's `&self`/`&mut self` borrows still
|
||||
// serialize access to the field. Moving or sharing it across threads is sound.
|
||||
#[cfg(windows)]
|
||||
#[allow(unsafe_code)]
|
||||
unsafe impl Send for KillGroup {}
|
||||
#[cfg(windows)]
|
||||
#[allow(unsafe_code)]
|
||||
unsafe impl Sync for KillGroup {}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[allow(unsafe_code)]
|
||||
impl KillGroup {
|
||||
fn new(child: &tokio::process::Child, _pid: Option<u32>) -> Self {
|
||||
use std::mem::{size_of, zeroed};
|
||||
use windows_sys::Win32::Foundation::HANDLE;
|
||||
use windows_sys::Win32::System::JobObjects::{
|
||||
AssignProcessToJobObject, CreateJobObjectW, JobObjectExtendedLimitInformation,
|
||||
SetInformationJobObject, JOBOBJECT_EXTENDED_LIMIT_INFORMATION,
|
||||
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
|
||||
};
|
||||
|
||||
// SAFETY: each call is a documented Win32 FFI call with arguments that
|
||||
// satisfy its contract — a null SECURITY_ATTRIBUTES/name for an
|
||||
// anonymous job, a zeroed #[repr(C)] info struct sized by size_of, and
|
||||
// the live process handle from `child` (valid while it is running).
|
||||
// A null job HANDLE on failure makes every later call a harmless no-op.
|
||||
let job = unsafe {
|
||||
let job: HANDLE = CreateJobObjectW(std::ptr::null(), std::ptr::null());
|
||||
if !job.is_null() {
|
||||
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = zeroed();
|
||||
// KILL_ON_JOB_CLOSE: when the LAST handle to the job closes,
|
||||
// Windows kills every process still in it. This is both the
|
||||
// explicit-kill mechanism and the Drop-time safety net — and the
|
||||
// reason the job HANDLE must outlive the child (see Drop).
|
||||
info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
|
||||
SetInformationJobObject(
|
||||
job,
|
||||
JobObjectExtendedLimitInformation,
|
||||
std::ptr::addr_of!(info).cast(),
|
||||
size_of::<JOBOBJECT_EXTENDED_LIMIT_INFORMATION>() as u32,
|
||||
);
|
||||
if let Some(handle) = child.raw_handle() {
|
||||
AssignProcessToJobObject(job, handle as HANDLE);
|
||||
}
|
||||
}
|
||||
job
|
||||
};
|
||||
Self { job }
|
||||
}
|
||||
|
||||
fn kill_immediate(&self) {
|
||||
self.terminate();
|
||||
}
|
||||
|
||||
async fn kill_graceful(&self) {
|
||||
// A Job Object has no SIGTERM analogue; termination is atomic, so the
|
||||
// graceful path is the same single terminate as the immediate path.
|
||||
self.terminate();
|
||||
}
|
||||
|
||||
fn terminate(&self) {
|
||||
use windows_sys::Win32::System::JobObjects::TerminateJobObject;
|
||||
if !self.job.is_null() {
|
||||
// SAFETY: `self.job` is a valid job HANDLE for this struct's
|
||||
// lifetime; exit code 137 mirrors the SIGKILL (128+9) we report on
|
||||
// Unix.
|
||||
unsafe {
|
||||
TerminateJobObject(self.job, 137);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// No-op on Windows: the job is terminated explicitly, and closing the
|
||||
/// handle on Drop with no live processes left is harmless. Kept for a
|
||||
/// uniform call shape with the Unix guard.
|
||||
fn disarm(&mut self) {}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[allow(unsafe_code)]
|
||||
impl Drop for KillGroup {
|
||||
fn drop(&mut self) {
|
||||
use windows_sys::Win32::Foundation::CloseHandle;
|
||||
if !self.job.is_null() {
|
||||
// Closing the last job handle triggers KILL_ON_JOB_CLOSE, killing any
|
||||
// process still in the job — the last-resort reaper. The handle is
|
||||
// held until here precisely so this fires no earlier than run end.
|
||||
// SAFETY: `self.job` is a valid HANDLE created in `new` and closed
|
||||
// exactly once here.
|
||||
unsafe {
|
||||
CloseHandle(self.job);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback for targets that are neither unix nor windows: no process-tree kill
|
||||
// primitive is wired up, so timeouts rely on the cross-platform start_kill in
|
||||
// `run`. Keeps the crate compiling everywhere.
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
struct KillGroup;
|
||||
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
impl KillGroup {
|
||||
fn new(_child: &tokio::process::Child, _pid: Option<u32>) -> Self {
|
||||
Self
|
||||
}
|
||||
fn kill_immediate(&self) {}
|
||||
async fn kill_graceful(&self) {}
|
||||
fn disarm(&mut self) {}
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct CapturedStream {
|
||||
@@ -520,7 +806,12 @@ mod tests {
|
||||
let r = run(
|
||||
&state,
|
||||
ShellParams {
|
||||
command: "sleep 999".into(),
|
||||
// Short sleep, not 999: the kill path must actually terminate
|
||||
// the process tree on timeout. If a regression leaves the child
|
||||
// (or an MSYS grandchild) orphaned, the test stalls until this
|
||||
// brief sleep self-exits — ~5s, not ~16min — so the failure
|
||||
// stays visible instead of hiding behind a 999s sleep.
|
||||
command: "sleep 5".into(),
|
||||
workdir: None,
|
||||
timeout_ms: Some(150),
|
||||
},
|
||||
@@ -563,3 +854,96 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, windows))]
|
||||
mod windows_resolver_tests {
|
||||
use super::*;
|
||||
use std::env;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn touch(path: &Path) {
|
||||
if let Some(parent) = path.parent() {
|
||||
std::fs::create_dir_all(parent).expect("mkdir");
|
||||
}
|
||||
std::fs::write(path, b"").expect("touch");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bundled_branch_returns_none_when_path_absent() {
|
||||
// Dev/CI: no staged resource next to the exe -> the bundled branch must
|
||||
// yield None so the resolver falls through instead of returning a
|
||||
// non-existent path that would fail at spawn.
|
||||
let dir = tempdir().expect("tempdir");
|
||||
assert!(bundled_bash(dir.path()).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bundled_branch_returns_absolute_path_when_staged() {
|
||||
// A staged PortableGit bash runtime next to the exe resolves to the absolute bash path.
|
||||
let dir = tempdir().expect("tempdir");
|
||||
let bash = dir.path().join(BUNDLED_BASH_REL);
|
||||
touch(&bash);
|
||||
let resolved = bundled_bash(dir.path()).expect("bundled bash");
|
||||
assert!(resolved.is_absolute());
|
||||
assert_eq!(resolved, bash);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_scan_skips_system32_and_returns_absolute() {
|
||||
// A bash.exe under %SystemRoot% (where WSL's launcher lives) must be
|
||||
// skipped; a bash.exe elsewhere on PATH is returned as an absolute path.
|
||||
let sys_root = tempdir().expect("sysroot");
|
||||
let real = tempdir().expect("real");
|
||||
touch(&sys_root.path().join("System32").join("bash.exe"));
|
||||
let real_bash = real.path().join("bash.exe");
|
||||
touch(&real_bash);
|
||||
|
||||
let path_env =
|
||||
env::join_paths([sys_root.path().join("System32"), real.path().to_path_buf()])
|
||||
.expect("join");
|
||||
|
||||
let found = scan_path_for_bash(
|
||||
path_env.to_str().expect("utf8"),
|
||||
Some(sys_root.path().to_path_buf()),
|
||||
)
|
||||
.expect("bash found outside System32");
|
||||
assert!(found.is_absolute());
|
||||
assert!(!found.starts_with(sys_root.path()));
|
||||
assert_eq!(found, real_bash);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_scan_returns_none_when_only_system32_has_bash() {
|
||||
// If the ONLY bash.exe on PATH is under System32, the scan finds nothing.
|
||||
let sys_root = tempdir().expect("sysroot");
|
||||
touch(&sys_root.path().join("System32").join("bash.exe"));
|
||||
let path_env = env::join_paths([sys_root.path().join("System32")]).expect("join");
|
||||
|
||||
let found = scan_path_for_bash(
|
||||
path_env.to_str().expect("utf8"),
|
||||
Some(sys_root.path().to_path_buf()),
|
||||
);
|
||||
assert!(found.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_scan_skips_system32_when_path_case_differs_from_root() {
|
||||
// Windows paths are case-insensitive; a PATH entry spelled differently from
|
||||
// %SystemRoot% (e.g. `...\WINDOWS\System32` vs root `...\Windows`) must STILL
|
||||
// be excluded, or WSL's bash.exe leaks through. Build the System32 dir under a
|
||||
// genuinely upper-cased sibling component so the exclusion can only pass via a
|
||||
// case-insensitive compare, not a literal `starts_with`.
|
||||
let base = tempdir().expect("base");
|
||||
let root = base.path().join("Windows");
|
||||
let upper = base.path().join("WINDOWS");
|
||||
let sys32 = upper.join("System32");
|
||||
touch(&sys32.join("bash.exe"));
|
||||
|
||||
let path_env = env::join_paths([sys32]).expect("join");
|
||||
let found = scan_path_for_bash(path_env.to_str().expect("utf8"), Some(root));
|
||||
assert!(
|
||||
found.is_none(),
|
||||
"case-divergent System32 must still be excluded"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -257,12 +257,17 @@ mod tests {
|
||||
#[test]
|
||||
fn run_allows_path_outside_workspace() {
|
||||
let dir = tempdir().expect("tempdir");
|
||||
// A real file in a SECOND tempdir, genuinely outside the workspace
|
||||
// root, that does NOT contain our old_str — we expect a "not found"
|
||||
// error (proving the path resolved), not a path-escape error. Avoids
|
||||
// the Unix-only /etc/hosts assumption (C:\etc\hosts does not exist).
|
||||
let outside = tempdir().expect("tempdir");
|
||||
let target = outside.path().join("outside.txt");
|
||||
fs::write(&target, b"some content").expect("write");
|
||||
let state = make_state(dir.path());
|
||||
// /etc/hosts is readable but won't contain our old_str — we expect
|
||||
// a "not found" error, not a path-escape error.
|
||||
let p = StrReplaceParams {
|
||||
path: "/etc/hosts".into(),
|
||||
old_str: "UNIQUE_STRING_NOT_IN_HOSTS_FILE_abc123".into(),
|
||||
path: target.display().to_string(),
|
||||
old_str: "UNIQUE_STRING_NOT_IN_FILE_abc123".into(),
|
||||
new_str: "y".into(),
|
||||
replace_all: false,
|
||||
workdir: Some(dir.path().display().to_string()),
|
||||
|
||||
@@ -687,13 +687,18 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn allows_path_outside_workspace() {
|
||||
let dir = tempdir().unwrap();
|
||||
// A real non-image file in a SECOND tempdir, genuinely outside the
|
||||
// workspace root — we expect a format error, not a path-escape error,
|
||||
// proving the traversal limit is gone. Avoids the Unix-only /etc/hosts
|
||||
// assumption (C:\etc\hosts does not exist on Windows).
|
||||
let outside = tempdir().unwrap();
|
||||
let target = outside.path().join("outside.txt");
|
||||
fs::write(&target, b"not an image").unwrap();
|
||||
let state = make_state(dir.path());
|
||||
// /etc/hosts exists but is not an image — we expect a format error,
|
||||
// not a path-escape error, proving the traversal limit is gone.
|
||||
let res = run(
|
||||
&state,
|
||||
ViewImageParams {
|
||||
source: "/etc/hosts".into(),
|
||||
source: target.display().to_string(),
|
||||
max_dim: None,
|
||||
workdir: Some(dir.path().display().to_string()),
|
||||
},
|
||||
|
||||
@@ -52,6 +52,27 @@ const releaseConfig = {
|
||||
},
|
||||
};
|
||||
|
||||
// Windows-only: bundle the PortableGit bash runtime as a resource so the MCP shell
|
||||
// tool always has a genuine, non-WSL bash to spawn on a bare host (the app must
|
||||
// be self-contained — we cannot assume Git for Windows is installed).
|
||||
//
|
||||
// This is emitted ONLY on the Windows runner because the static tauri.conf.json
|
||||
// uses `targets: "all"` with a shared bundle block — a bare `resources` entry
|
||||
// there would ship the ~184MB tree into the macOS .dmg and Linux packages too.
|
||||
// The release build runs THIS generator on each platform's own runner and merges
|
||||
// the output via --config, so guarding on process.platform keeps the tree off
|
||||
// mac/Linux.
|
||||
//
|
||||
// PATH CONTRACT (keep byte-identical across three files):
|
||||
// - source `binaries/git-bash` (relative to src-tauri/) is staged by
|
||||
// scripts/bundle-sidecars.sh.
|
||||
// - target `git-bash` is the install-root subdir; Tauri's Windows installer
|
||||
// stages it next to the exe, and crates/buzz-dev-mcp/src/shell.rs resolves
|
||||
// `git-bash\bin\bash.exe` relative to its own executable at runtime.
|
||||
if (process.platform === "win32") {
|
||||
releaseConfig.bundle.resources = { "binaries/git-bash": "git-bash" };
|
||||
}
|
||||
|
||||
console.log(`Updater enabled -> ${updaterEndpoint}`);
|
||||
|
||||
writeFileSync(outputConfigPath, `${JSON.stringify(releaseConfig, null, 2)}\n`);
|
||||
|
||||
@@ -38,3 +38,12 @@ for bin in "${SIDECARS[@]}"; do
|
||||
cp "$SRC_DIR/${bin}${EXE}" "$BINARIES_DIR/${bin}-${TARGET}${EXE}"
|
||||
done
|
||||
echo "Sidecars bundled for $TARGET"
|
||||
|
||||
# Windows-only: stage a genuine, non-WSL bash next to the sidecars so the MCP
|
||||
# shell tool works on a bare host. The download/extract/drop logic lives in a
|
||||
# self-contained script (no release-binary precondition) so CI can call it
|
||||
# directly to exercise this path on a real Windows runner — see
|
||||
# scripts/stage-windows-bash.sh for the full rationale and the PATH CONTRACT.
|
||||
if [[ "$TARGET" == *windows* ]]; then
|
||||
"$(dirname "$0")/stage-windows-bash.sh" "$BINARIES_DIR/git-bash"
|
||||
fi
|
||||
|
||||
73
scripts/stage-windows-bash.sh
Executable file
73
scripts/stage-windows-bash.sh
Executable file
@@ -0,0 +1,73 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# Stage a genuine, non-WSL bash for the Windows MCP shell tool. The app is
|
||||
# self-contained — we cannot assume Git for Windows is installed — so we bundle
|
||||
# bash rather than probing for an install.
|
||||
#
|
||||
# There is no standalone "bash for Windows" upstream: working Windows bash ships
|
||||
# only inside git-for-windows. We download PortableGit and keep ONLY the MSYS2
|
||||
# bash runtime (`usr/` + `bin/`), dropping the `mingw64/` git-program subtree as
|
||||
# one separable unit (~200MB of git.exe etc. the MCP shell never invokes). We do
|
||||
# NOT trim INSIDE `usr/`: bash loads `msys-2.0.dll` and other libraries lazily,
|
||||
# and load-bearing pieces (terminfo, gawk libs) live alongside the docs there, so
|
||||
# a hand-trimmed copy can pass an existence check yet fail mid-command with a
|
||||
# cryptic error — exactly the bug class this fixes. The retained runtime is the
|
||||
# untouched, complete closure git-for-windows maintains.
|
||||
#
|
||||
# Self-contained (no release-binary precondition) so CI can call it directly to
|
||||
# exercise the download/extract/drop path on a real Windows runner — the only
|
||||
# automated gate on this logic before it ships to users.
|
||||
#
|
||||
# Single arg: the destination dir for the staged tree (the real MSYS2 bash lands
|
||||
# at <dest>/usr/bin/bash.exe). Idempotent: a `.stage-complete` marker, written last,
|
||||
# proves a whole prior stage and skips the re-download; a partial stage lacks it
|
||||
# and re-extracts cleanly.
|
||||
#
|
||||
# PATH CONTRACT (keep byte-identical across three files):
|
||||
# - dest `git-bash` (== desktop/src-tauri/binaries/git-bash) is the
|
||||
# `bundle.resources` SOURCE in desktop/scripts/build-release-config.mjs.
|
||||
# - that resource's TARGET `git-bash` is staged next to the exe by Tauri's
|
||||
# Windows installer, and crates/buzz-dev-mcp/src/shell.rs resolves
|
||||
# `git-bash\usr\bin\bash.exe` relative to its own executable at runtime.
|
||||
|
||||
GIT_BASH_DIR=${1:?usage: stage-windows-bash.sh <dest-dir>}
|
||||
PORTABLEGIT_VERSION="2.54.0"
|
||||
PORTABLEGIT_TAG="v${PORTABLEGIT_VERSION}.windows.1"
|
||||
PORTABLEGIT_EXE="PortableGit-${PORTABLEGIT_VERSION}-64-bit.7z.exe"
|
||||
PORTABLEGIT_URL="https://github.com/git-for-windows/git/releases/download/${PORTABLEGIT_TAG}/${PORTABLEGIT_EXE}"
|
||||
|
||||
STAGE_MARKER="$GIT_BASH_DIR/.stage-complete"
|
||||
if [[ -f "$STAGE_MARKER" ]]; then
|
||||
echo "PortableGit bash already staged at $GIT_BASH_DIR"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
echo "Downloading PortableGit ${PORTABLEGIT_VERSION}..."
|
||||
tmp_dir=$(mktemp -d -t portablegit.XXXXXX)
|
||||
trap 'rm -rf "$tmp_dir"' EXIT
|
||||
tmp_sfx="$tmp_dir/portablegit.7z.exe"
|
||||
extract_dir="$tmp_dir/extract"
|
||||
curl -fsSL "$PORTABLEGIT_URL" -o "$tmp_sfx"
|
||||
# PortableGit is a 7-Zip self-extracting archive; -o/-y are its SFX flags,
|
||||
# so we don't need a separate 7z on PATH.
|
||||
chmod +x "$tmp_sfx"
|
||||
"$tmp_sfx" -y "-o$extract_dir"
|
||||
|
||||
# Keep the bash runtime whole, drop the separable git-program subtree.
|
||||
rm -rf "$extract_dir/mingw64"
|
||||
rm -rf "$GIT_BASH_DIR"
|
||||
mkdir -p "$GIT_BASH_DIR"
|
||||
cp -a "$extract_dir/." "$GIT_BASH_DIR/"
|
||||
|
||||
rm -rf "$tmp_dir"
|
||||
trap - EXIT
|
||||
[[ -f "$GIT_BASH_DIR/usr/bin/bash.exe" ]] || {
|
||||
echo "Error: PortableGit extracted but $GIT_BASH_DIR/usr/bin/bash.exe is missing" >&2
|
||||
exit 1
|
||||
}
|
||||
# Written last, only after cp -a and the integrity check both succeed, so it is
|
||||
# positive proof the whole tree landed. An interrupted stage never writes it, so
|
||||
# the idempotency skip falls through to a clean re-extract.
|
||||
touch "$STAGE_MARKER"
|
||||
echo "PortableGit bash staged at $GIT_BASH_DIR (mingw64/ dropped)"
|
||||
Reference in New Issue
Block a user