From debff9f3d81fabbe14834a8a8048c463ca649b0f Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Wed, 15 Jul 2026 18:32:06 -0400 Subject: [PATCH] feat(agent): add hardened MCP toolchain shims with config isolation Add hermit-managed shim scripts for uv/uvx/npx/node that provide config-isolated toolchain execution for MCP servers, preventing ambient system toolchains from leaking user config into agent processes. Security hardening: - Private mktemp temp dir with cleanup trap (no predictable paths) - Lock mutual exclusion via kill-0 + /proc start-time verification - Atomic bootstrap recovery (staged download + validated mv) - Desktop fail-loud validation of shim resources before env export Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- Justfile | 4 + crates/buzz-agent/resources/shims/node | 8 + crates/buzz-agent/resources/shims/npx | 8 + .../resources/shims/setup-common.sh | 265 ++++++ crates/buzz-agent/resources/shims/uv | 6 + crates/buzz-agent/resources/shims/uvx | 6 + crates/buzz-agent/src/config.rs | 18 +- crates/buzz-agent/src/mcp.rs | 203 ++++- .../buzz-agent/tests/shim_hostile_config.rs | 834 ++++++++++++++++++ desktop/scripts/check-file-sizes.mjs | 4 +- .../src-tauri/src/managed_agents/runtime.rs | 103 +++ .../src/managed_agents/runtime/tests.rs | 81 ++ desktop/src-tauri/tauri.conf.json | 3 + scripts/run-tests.sh | 6 + 14 files changed, 1537 insertions(+), 12 deletions(-) create mode 100755 crates/buzz-agent/resources/shims/node create mode 100755 crates/buzz-agent/resources/shims/npx create mode 100755 crates/buzz-agent/resources/shims/setup-common.sh create mode 100755 crates/buzz-agent/resources/shims/uv create mode 100755 crates/buzz-agent/resources/shims/uvx create mode 100644 crates/buzz-agent/tests/shim_hostile_config.rs diff --git a/Justfile b/Justfile index ae17422740..d1ba2d943a 100644 --- a/Justfile +++ b/Justfile @@ -253,6 +253,10 @@ test-unit: # Gateway unit and black-box HTTP tests are infra-free. Postgres-backed # contract/race tests run in the dedicated CI job below. cargo nextest run -p buzz-push-gateway + # MCP shim hostile-config suite: infra-free (real wrapper scripts + + # stubbed curl/openssl). Contains the Linux-only hostile-path regression + # for the CRITICAL temp-execution fix — this is its only CI execution path. + cargo nextest run -p buzz-agent --test shim_hostile_config else ./scripts/run-tests.sh unit fi diff --git a/crates/buzz-agent/resources/shims/node b/crates/buzz-agent/resources/shims/node new file mode 100755 index 0000000000..ec799ca5e5 --- /dev/null +++ b/crates/buzz-agent/resources/shims/node @@ -0,0 +1,8 @@ +#!/bin/bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/setup-common.sh" node +export NPM_CONFIG_USERCONFIG="${BUZZ_MCP_HERMIT_DIR}/empty-user-npmrc" +export NPM_CONFIG_GLOBALCONFIG="${BUZZ_MCP_HERMIT_DIR}/empty-global-npmrc" +export NPM_CONFIG_LOCATION=global +exec node "$@" diff --git a/crates/buzz-agent/resources/shims/npx b/crates/buzz-agent/resources/shims/npx new file mode 100755 index 0000000000..07e1740119 --- /dev/null +++ b/crates/buzz-agent/resources/shims/npx @@ -0,0 +1,8 @@ +#!/bin/bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/setup-common.sh" node +export NPM_CONFIG_USERCONFIG="${BUZZ_MCP_HERMIT_DIR}/empty-user-npmrc" +export NPM_CONFIG_GLOBALCONFIG="${BUZZ_MCP_HERMIT_DIR}/empty-global-npmrc" +export NPM_CONFIG_LOCATION=global +exec npx "$@" diff --git a/crates/buzz-agent/resources/shims/setup-common.sh b/crates/buzz-agent/resources/shims/setup-common.sh new file mode 100755 index 0000000000..46c0a799e8 --- /dev/null +++ b/crates/buzz-agent/resources/shims/setup-common.sh @@ -0,0 +1,265 @@ +#!/bin/bash +# Shared bootstrap for MCP toolchain shims. +# Sources hermit-managed uv/node into PATH with config isolation. +# Usage: source setup-common.sh (toolchain = "uv" or "node") + +set -euo pipefail + +SHIM_TOOLCHAIN="${1:?usage: source setup-common.sh }" +SHIM_NAME="$(basename "${BASH_SOURCE[1]:-$0}")" + +# ── Logging ────────────────────────────────────────────────────────────────── + +# Declared here so shellcheck (SC2154) sees it as defined before the ERR +# traps below assign it on every exit path. +rc=0 + +_shim_log() { + local msg + msg="$(date +'%Y-%m-%d %H:%M:%S') [${SHIM_NAME}] $1" + echo "${msg}" >&2 + if [ -n "${BUZZ_MCP_LOG_DIR:-}" ]; then + (umask 077; echo "${msg}" >> "${BUZZ_MCP_LOG_DIR}/mcp-shim.log") 2>/dev/null || true + fi +} + +trap 'rc=$?; _shim_log "error: exiting with status ${rc}"; exit "${rc}"' ERR + +# ── Config directory ───────────────────────────────────────────────────────── + +BUZZ_MCP_HERMIT_DIR="${BUZZ_MCP_HERMIT_DIR:-${HOME}/.config/buzz/mcp-hermit}" +export BUZZ_MCP_HERMIT_DIR +mkdir -p "${BUZZ_MCP_HERMIT_DIR}" + +# ── Lock (mkdir-based mutex with atomic dead-owner reclaim) ────────────────── +# +# Invariants: +# (a) No process ever removes the shared lock path based on a prior +# observation — a stale "is dead" check followed by rm would let a +# second waiter delete a freshly-reacquired live lock (TOCTOU). +# (b) Any removed name is either owned by the remover or atomically claimed +# at the moment of removal. +# +# Protocol: try `mkdir _LOCK_DIR` (atomic). On failure, read the holder +# info and prove death. If dead, atomically `mv _LOCK_DIR` to a +# process-specific reclaim path — `mv` on the same filesystem is atomic, +# so only one of N racing reclaimers succeeds (the rest get ENOENT and +# retry). The winner removes only its own reclaim dir, then retries the +# `mkdir`. A live holder is never evicted; the outer MCP init timeout +# (BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS, default 300s) bounds the wait. +# +# `kill -0` returning ESRCH proves the PID is gone. On Linux, /proc-based +# starttime comparison additionally detects PID reuse. On macOS (same-user +# desktop app), all shim processes run as the same user, so EPERM from +# `kill -0` is not possible — a failure always means the process is dead. + +_LOCK_DIR="${BUZZ_MCP_HERMIT_DIR}/.mcp-hermit-setup.lock" + +_proc_starttime() { + local pid="$1" + local stat_file="/proc/${pid}/stat" + local line rest + [ -r "${stat_file}" ] || return 0 + line="$(cat "${stat_file}" 2>/dev/null)" || return 0 + rest="${line##*) }" + # shellcheck disable=SC2086 # intentional word-splitting to index stat fields + set -- $rest + echo "${20:-}" +} + +_lock_holder_is_dead() { + local pid="$1" recorded_starttime="$2" + [ -n "${pid}" ] || return 1 + if ! kill -0 "${pid}" 2>/dev/null; then + # On Linux, corroborate via /proc — kill -0 failure could be + # EPERM if processes ever run under different users (not the case + # today: same-user desktop app). If /proc exists but the pid dir + # is absent, the process is truly gone. + if [ -d "/proc" ] && [ -d "/proc/${pid}" ]; then + return 1 # /proc says alive — kill failed for another reason + fi + return 0 + fi + if [ "$(uname -s)" = "Linux" ]; then + local current_starttime + current_starttime="$(_proc_starttime "${pid}")" + if [ -n "${recorded_starttime}" ] && [ -n "${current_starttime}" ] \ + && [ "${recorded_starttime}" != "${current_starttime}" ]; then + return 0 + fi + fi + return 1 +} + +while ! mkdir "${_LOCK_DIR}" 2>/dev/null; do + if [ -f "${_LOCK_DIR}/info" ]; then + _holder_pid=$(cut -d: -f1 "${_LOCK_DIR}/info" 2>/dev/null || echo "") + _holder_starttime=$(cut -d: -f2 "${_LOCK_DIR}/info" 2>/dev/null || echo "") + if _lock_holder_is_dead "${_holder_pid}" "${_holder_starttime}"; then + # Atomic reclaim: mv to a process-specific path. Only one racer + # succeeds; losers get ENOENT and loop back to retry mkdir. + _reclaim_dir="${_LOCK_DIR}.reclaim.$$" + if mv "${_LOCK_DIR}" "${_reclaim_dir}" 2>/dev/null; then + _shim_log "dead lock holder detected (pid ${_holder_pid}); reclaimed" + rm -rf "${_reclaim_dir}" + fi + fi + fi + sleep 0.1 +done + +echo "$$:$(_proc_starttime "$$"):$(date +%s)" > "${_LOCK_DIR}/info" +trap 'rc=$?; rm -rf "${_LOCK_DIR}"; _shim_log "error: exiting with status ${rc}"; exit "${rc}"' ERR +trap 'rm -rf "${_LOCK_DIR}"' EXIT + +# ── macOS PATH fix ─────────────────────────────────────────────────────────── +# GUI-launched macOS apps inherit a minimal PATH that omits sbin directories; +# hermit bootstrap needs chown from /usr/sbin. +export PATH="/usr/sbin:/sbin:${PATH}" + +# ── Hermit bootstrap (SHA-verified, validated, atomically published) ─────── + +mkdir -p "${BUZZ_MCP_HERMIT_DIR}/bin" +cd "${BUZZ_MCP_HERMIT_DIR}" + +mkdir -p "${BUZZ_MCP_HERMIT_DIR}/cache" +export HERMIT_STATE_DIR="${BUZZ_MCP_HERMIT_DIR}/cache" + +HERMIT_BIN="${BUZZ_MCP_HERMIT_DIR}/bin/hermit" + +_hermit_binary_valid() { + # Side-effect-free: `--version` only prints the CLI's own version, it + # touches no state/cache/network (github.com/cashapp/hermit's + # `cmd/hermit/builtin/hermit.hcl` uses the same invocation as its own + # smoke test). Guards against a truncated/corrupt/non-executable + # artifact left by an interrupted prior run. + [ -x "$1" ] && "$1" --version >/dev/null 2>&1 +} + +if ! _hermit_binary_valid "${HERMIT_BIN}"; then + if [ -e "${HERMIT_BIN}" ]; then + _shim_log "existing hermit binary invalid; removing and rebootstrapping" + rm -f "${HERMIT_BIN}" + fi + _shim_log "downloading hermit binary (SHA-verified)" + HERMIT_DIST_URL="https://github.com/cashapp/hermit/releases/download/stable" + INSTALL_SCRIPT_SHA256="09ed936378857886fd4a7a4878c0f0c7e3d839883f39ca8b4f2f242e3126e1c6" + + _install_tmp="$(mktemp)" + trap 'rc=$?; rm -f "${_install_tmp}"; rm -rf "${_LOCK_DIR}"; _shim_log "error: exiting with status ${rc}"; exit "${rc}"' ERR + + curl -fsSL "${HERMIT_DIST_URL}/install-${INSTALL_SCRIPT_SHA256}.sh" -o "${_install_tmp}" + _actual_sha=$(openssl dgst -sha256 "${_install_tmp}" | awk '{print $2}') + if [ "${_actual_sha}" != "${INSTALL_SCRIPT_SHA256}" ]; then + rm -f "${_install_tmp}" + _shim_log "FATAL: hermit install script SHA mismatch: got ${_actual_sha}" + exit 1 + fi + + # The pinned installer writes to $HERMIT_EXE (default + # $HERMIT_STATE_DIR/pkg/hermit@/hermit, NOT bin/hermit) and, if + # not skipped, also drops a launcher stub under a system-wide install + # dir like ~/bin. Point it at our own staging path and skip that + # system-wide step entirely so bootstrap stays confined to + # BUZZ_MCP_HERMIT_DIR and publication is a single atomic rename. + _hermit_staged="${HERMIT_BIN}.download.$$" + HERMIT_EXE="${_hermit_staged}" HERMIT_SKIP_SYSTEM_INSTALL=1 \ + /bin/bash "${_install_tmp}" 1>&2 + rm -f "${_install_tmp}" + + if ! _hermit_binary_valid "${_hermit_staged}"; then + rm -f "${_hermit_staged}" + _shim_log "FATAL: downloaded hermit binary failed validation" + exit 1 + fi + mv -f "${_hermit_staged}" "${HERMIT_BIN}" + _shim_log "hermit bootstrap complete" +fi + +export PATH="${BUZZ_MCP_HERMIT_DIR}/bin:${PATH}" + +# ── Hermit init + activation (validated, with repair) ──────────────────────── +# +# Real hermit's activate-hermit sources `hermit activate`, which exports +# HERMIT_ENV. A malformed/hostile/stale file could `exit 0` from the +# sourcing shell — silently killing the whole script — so we probe in a +# throwaway subshell first. If the probe fails (missing, malformed, or +# stale activate-hermit), we repair once (rm + re-init under the +# still-held lock) and FATAL on second failure. Trust-on-existence is dead: +# the file must exist AND produce HERMIT_ENV in a subshell. + +_activation_valid() { + [ -f "bin/activate-hermit" ] || return 1 + # Probe in a subshell: source the file, then write a sentinel if + # HERMIT_ENV got set. A malformed file that `exit`s kills only the + # subshell — the sentinel never appears, so the probe returns failure. + local _probe_out + _probe_out="$(mktemp)" + ( . "bin/activate-hermit" >/dev/null 2>&1; [ -n "${HERMIT_ENV:-}" ] && echo ok > "${_probe_out}" ) 2>/dev/null + local _result=1 + [ -s "${_probe_out}" ] && _result=0 + rm -f "${_probe_out}" + return "${_result}" +} + +_run_hermit_init() { + # Linux: copy hermit binary to a private temp dir to avoid self-update + # lock contention, and invoke it by absolute path so a hostile + # pre-existing entry earlier in PATH can never be executed instead. + if [ "$(uname -s)" = "Linux" ]; then + _htmp_cleanup="$(mktemp -d "${TMPDIR:-/tmp}/buzz-hermit.XXXXXXXX")" + trap 'rc=$?; rm -rf "${_htmp_cleanup}"; rm -rf "${_LOCK_DIR}"; _shim_log "error: exiting with status ${rc}"; exit "${rc}"' ERR + _htmp_hermit="${_htmp_cleanup}/hermit" + cp "${HERMIT_BIN}" "${_htmp_hermit}" + chmod +x "${_htmp_hermit}" + "${_htmp_hermit}" init 1>&2 + rm -rf "${_htmp_cleanup}" + else + hermit init 1>&2 + fi +} + +_init_attempt=0 +while ! _activation_valid; do + if [ "${_init_attempt}" -ge 2 ]; then + _shim_log "FATAL: bin/activate-hermit still invalid after re-init — hermit environment is broken" + exit 1 + fi + if [ "${_init_attempt}" -eq 0 ]; then + _shim_log "initializing hermit environment" + else + _shim_log "bin/activate-hermit invalid after init; repairing (re-init under lock)" + fi + rm -f "bin/activate-hermit" + _run_hermit_init + _init_attempt=$((_init_attempt + 1)) +done + +_shim_log "activating hermit environment" +{ . "bin/activate-hermit"; } 1>&2 2>/dev/null + +# Install the requested toolchain packages. +case "${SHIM_TOOLCHAIN}" in + uv) + hermit install python3@3.10 1>&2 + hermit install uv 1>&2 + ;; + node) + hermit install node 1>&2 + # Create distinct empty npmrc files for config isolation. + touch "${BUZZ_MCP_HERMIT_DIR}/empty-user-npmrc" + touch "${BUZZ_MCP_HERMIT_DIR}/empty-global-npmrc" + ;; + *) + _shim_log "unknown toolchain: ${SHIM_TOOLCHAIN}" + exit 1 + ;; +esac + +# ── Release lock ───────────────────────────────────────────────────────────── + +rm -rf "${_LOCK_DIR}" +trap 'rc=$?; _shim_log "error: exiting with status ${rc}"; exit "${rc}"' ERR +trap - EXIT + +_shim_log "bootstrap complete for ${SHIM_TOOLCHAIN}" diff --git a/crates/buzz-agent/resources/shims/uv b/crates/buzz-agent/resources/shims/uv new file mode 100755 index 0000000000..01f12e86f9 --- /dev/null +++ b/crates/buzz-agent/resources/shims/uv @@ -0,0 +1,6 @@ +#!/bin/bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/setup-common.sh" uv +export UV_NO_CONFIG=1 +exec uv "$@" diff --git a/crates/buzz-agent/resources/shims/uvx b/crates/buzz-agent/resources/shims/uvx new file mode 100755 index 0000000000..958d269f4a --- /dev/null +++ b/crates/buzz-agent/resources/shims/uvx @@ -0,0 +1,6 @@ +#!/bin/bash +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${SCRIPT_DIR}/setup-common.sh" uv +export UV_NO_CONFIG=1 +exec uvx "$@" diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index f06849c7d1..c28384e850 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -649,6 +649,12 @@ pub const MAX_TOOL_RESULT_BYTES: usize = 8 * 1024 * 1024; pub const DEFAULT_TOOL_RESULT_TEXT_BYTES: usize = 50 * 1024; pub const MAX_TOOL_CALLS_PER_TURN: usize = 64; +/// Default MCP server init timeout. Packaged toolchain shims (uv/npx via +/// hermit bootstrap) can take up to a few minutes on first launch to +/// download and initialize hermit plus the requested package — this must +/// stay well above that. Set via `BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS`. +pub const DEFAULT_MCP_INIT_TIMEOUT_SECS: u64 = 300; + pub const HANDOFF_MAX_OUTPUT_TOKENS: u32 = 8192; pub const HANDOFF_ORIGINAL_TASK_MAX_BYTES: usize = 16 * 1024; @@ -804,7 +810,7 @@ impl Config { tool_timeout: Duration::from_secs(parse_env("BUZZ_AGENT_TOOL_TIMEOUT_SECS", 660)?), mcp_init_timeout: Duration::from_secs(parse_env( "BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS", - 30, + DEFAULT_MCP_INIT_TIMEOUT_SECS, )?), mcp_max_restart_attempts: parse_env("BUZZ_AGENT_MCP_RESTART_MAX_ATTEMPTS", 3u32)?, mcp_restart_base_ms: parse_env("BUZZ_AGENT_MCP_RESTART_BASE_MS", 500u64)?, @@ -1107,6 +1113,16 @@ fn parse_hook_servers(raw: Option<&str>) -> HookServers { mod tests { use super::*; + #[test] + fn mcp_init_timeout_default_is_300s() { + let val: u64 = parse_env( + "_BUZZ_TEST_NONEXISTENT_TIMEOUT_VAR_", + DEFAULT_MCP_INIT_TIMEOUT_SECS, + ) + .unwrap(); + assert_eq!(Duration::from_secs(val), Duration::from_secs(300)); + } + #[test] fn hook_servers_unset_is_none() { assert!(matches!(parse_hook_servers(None), HookServers::None)); diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index 71c3193a06..b5a6eacc43 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -61,6 +61,13 @@ const PASSTHROUGH_ENV: &[&str] = &[ "BUZZ_PRIVATE_KEY", "BUZZ_RELAY_URL", "BUZZ_AUTH_TAG", + // MCP toolchain shims — desktop sets these for hermit-managed shims. + // BUZZ_MCP_SHIM_DIR: read-only bundled shim scripts (resolver source) + // BUZZ_MCP_HERMIT_DIR: writable hermit state/cache directory + // BUZZ_MCP_LOG_DIR: shim log output directory + "BUZZ_MCP_SHIM_DIR", + "BUZZ_MCP_HERMIT_DIR", + "BUZZ_MCP_LOG_DIR", ]; // Windows has no $TMPDIR/$HOME. TMP/TEMP/USERPROFILE are what @@ -168,6 +175,23 @@ pub struct McpRegistry { hook_timeouts: std::sync::Mutex>, } +#[cfg(unix)] +const SHIMMED_COMMANDS: &[&str] = &["uv", "uvx", "npx", "node"]; + +#[cfg(unix)] +fn resolve_shim_command(command: &str, shim_dir: Option<&std::path::Path>) -> String { + if !SHIMMED_COMMANDS.contains(&command) { + return command.to_owned(); + } + if let Some(dir) = shim_dir { + let shim_path = dir.join(command); + if shim_path.exists() { + return shim_path.to_string_lossy().into_owned(); + } + } + command.to_owned() +} + impl McpRegistry { pub async fn spawn_all( cfg: &Config, @@ -192,6 +216,11 @@ impl McpRegistry { hook_timeouts: std::sync::Mutex::new(HashMap::new()), }; + #[cfg(unix)] + let shim_dir = std::env::var("BUZZ_MCP_SHIM_DIR") + .ok() + .map(std::path::PathBuf::from); + let mut seen_names = HashSet::new(); for s in servers { if !valid_name(&s.name) || s.name.contains("__") { @@ -203,9 +232,19 @@ impl McpRegistry { s.name ))); } + let resolved_command = { + #[cfg(unix)] + { + resolve_shim_command(&s.command, shim_dir.as_deref()) + } + #[cfg(not(unix))] + { + s.command.clone() + } + }; let spec = ServerSpec { name: s.name.clone(), - command: s.command.clone(), + command: resolved_command, args: s.args.clone(), env: s .env @@ -705,6 +744,28 @@ impl McpRegistry { } } +#[cfg(unix)] +fn build_child_env_with(spec: &ServerSpec, lookup: F) -> Vec<(String, String)> +where + F: Fn(&str) -> Option, +{ + let mut env = Vec::new(); + for k in PASSTHROUGH_ENV { + if let Some(v) = lookup(k) { + env.push((k.to_string(), v)); + } + } + for (k, v) in &spec.env { + env.push((k.clone(), v.clone())); + } + env +} + +#[cfg(unix)] +fn build_child_env(spec: &ServerSpec) -> Vec<(String, String)> { + build_child_env_with(spec, |k| std::env::var(k).ok()) +} + async fn spawn_one( spec: &ServerSpec, timeout: Duration, @@ -712,20 +773,26 @@ async fn spawn_one( let mut cmd = Command::new(&spec.command); cmd.args(&spec.args); cmd.env_clear(); - for k in PASSTHROUGH_ENV { - if let Ok(v) = std::env::var(k) { - cmd.env(k, v); - } + #[cfg(unix)] + for (k, v) in build_child_env(spec) { + cmd.env(k, v); } #[cfg(windows)] - for k in windows_child_passthrough_env() { - if let Ok(v) = std::env::var(k) { + { + for k in PASSTHROUGH_ENV { + if let Ok(v) = std::env::var(k) { + cmd.env(k, v); + } + } + for k in windows_child_passthrough_env() { + if let Ok(v) = std::env::var(k) { + cmd.env(k, v); + } + } + for (k, v) in &spec.env { cmd.env(k, v); } } - for (k, v) in &spec.env { - cmd.env(k, v); - } cmd.current_dir(&spec.cwd); cmd.stderr(std::process::Stdio::inherit()); @@ -1099,3 +1166,119 @@ mod content_tests { assert_eq!(super::truncate_middle("ok", 1024), "ok"); } } + +#[cfg(all(test, unix))] +mod shim_resolver_tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + fn make_shim_dir(commands: &[&str]) -> TempDir { + let dir = TempDir::new().unwrap(); + for cmd in commands { + let path = dir.path().join(cmd); + fs::write(&path, "#!/bin/bash\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap(); + } + } + dir + } + + #[test] + fn bare_uv_resolves_to_shim() { + let dir = make_shim_dir(&["uv"]); + let result = resolve_shim_command("uv", Some(dir.path())); + assert_eq!(result, dir.path().join("uv").to_string_lossy()); + } + + #[test] + fn bare_npx_resolves_to_shim() { + let dir = make_shim_dir(&["npx"]); + let result = resolve_shim_command("npx", Some(dir.path())); + assert_eq!(result, dir.path().join("npx").to_string_lossy()); + } + + #[test] + fn bare_uvx_resolves_to_shim() { + let dir = make_shim_dir(&["uvx"]); + let result = resolve_shim_command("uvx", Some(dir.path())); + assert_eq!(result, dir.path().join("uvx").to_string_lossy()); + } + + #[test] + fn bare_node_resolves_to_shim() { + let dir = make_shim_dir(&["node"]); + let result = resolve_shim_command("node", Some(dir.path())); + assert_eq!(result, dir.path().join("node").to_string_lossy()); + } + + #[test] + fn absolute_path_never_rewritten() { + let dir = make_shim_dir(&["uv"]); + let result = resolve_shim_command("/usr/bin/uv", Some(dir.path())); + assert_eq!(result, "/usr/bin/uv"); + } + + #[test] + fn unknown_command_passes_through() { + let dir = make_shim_dir(&["uv", "npx"]); + let result = resolve_shim_command("foo", Some(dir.path())); + assert_eq!(result, "foo"); + } + + #[test] + fn none_shim_dir_passes_through() { + let result = resolve_shim_command("uv", None); + assert_eq!(result, "uv"); + } + + #[test] + fn missing_shim_file_passes_through() { + let dir = TempDir::new().unwrap(); + let result = resolve_shim_command("uv", Some(dir.path())); + assert_eq!(result, "uv"); + } + + #[test] + fn build_child_env_propagates_shim_vars_and_spec_overrides() { + let fake_env: std::collections::HashMap<&str, &str> = [ + ("BUZZ_MCP_SHIM_DIR", "/test/shims"), + ("BUZZ_MCP_HERMIT_DIR", "/test/hermit"), + ("BUZZ_MCP_LOG_DIR", "/test/logs"), + ("PATH", "/usr/bin"), + ("HOME", "/home/test"), + ] + .into_iter() + .collect(); + + let spec = ServerSpec { + name: "test".to_string(), + command: "echo".to_string(), + args: vec![], + env: vec![ + ( + "UV_INDEX_URL".to_string(), + "https://custom.example.com".to_string(), + ), + ("BUZZ_MCP_LOG_DIR".to_string(), "/override/logs".to_string()), + ], + cwd: "/tmp".into(), + }; + + let env = build_child_env_with(&spec, |k| fake_env.get(k).map(|v| v.to_string())); + let env_map: std::collections::HashMap<_, _> = env.into_iter().collect(); + + assert_eq!(env_map.get("BUZZ_MCP_SHIM_DIR").unwrap(), "/test/shims"); + assert_eq!(env_map.get("BUZZ_MCP_HERMIT_DIR").unwrap(), "/test/hermit"); + assert_eq!(env_map.get("BUZZ_MCP_LOG_DIR").unwrap(), "/override/logs"); + assert_eq!( + env_map.get("UV_INDEX_URL").unwrap(), + "https://custom.example.com" + ); + assert_eq!(env_map.get("PATH").unwrap(), "/usr/bin"); + assert_eq!(env_map.get("HOME").unwrap(), "/home/test"); + } +} diff --git a/crates/buzz-agent/tests/shim_hostile_config.rs b/crates/buzz-agent/tests/shim_hostile_config.rs new file mode 100644 index 0000000000..cc046324b1 --- /dev/null +++ b/crates/buzz-agent/tests/shim_hostile_config.rs @@ -0,0 +1,834 @@ +//! Integration tests for MCP toolchain shim config isolation. +//! +//! Layer 1 tests invoke the REAL checked-in wrapper scripts from resources/shims/ +//! with a pre-seeded fake hermit environment. The fake hermit dir contains no-op +//! bootstrap binaries and probe scripts that print their effective env vars. This +//! verifies the actual wrapper → setup-common.sh → exec chain without network. + +#[cfg(unix)] +mod hostile_config { + use std::fs; + use std::os::unix::fs::PermissionsExt; + use std::path::{Path, PathBuf}; + use std::process::Command; + + fn shim_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/shims") + } + + #[test] + fn shim_files_exist_and_are_executable() { + for name in ["setup-common.sh", "uv", "uvx", "npx", "node"] { + let path = shim_dir().join(name); + assert!( + path.exists(), + "shim {name} does not exist at {}", + path.display() + ); + let mode = path.metadata().unwrap().permissions().mode(); + assert!( + mode & 0o111 != 0, + "shim {name} is not executable (mode={mode:#o})" + ); + } + } + + #[test] + fn npm_uses_distinct_config_files() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path(); + let user_npmrc = hermit_dir.join("empty-user-npmrc"); + let global_npmrc = hermit_dir.join("empty-global-npmrc"); + fs::write(&user_npmrc, "").unwrap(); + fs::write(&global_npmrc, "").unwrap(); + + assert_ne!( + user_npmrc.canonicalize().unwrap(), + global_npmrc.canonicalize().unwrap(), + "user and global npmrc must be distinct files to avoid double-load crash" + ); + } + + /// Pre-seeds a fake hermit dir that short-circuits setup-common.sh bootstrap. + /// Probe binaries print their effective env vars instead of doing real work. + fn seed_fake_hermit_dir(dir: &Path) { + let bin = dir.join("bin"); + fs::create_dir_all(&bin).unwrap(); + fs::create_dir_all(dir.join("cache")).unwrap(); + + let write_exec = |path: &Path, content: &str| { + fs::write(path, content).unwrap(); + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); + }; + + write_exec(&bin.join("hermit"), "#!/bin/bash\nexit 0\n"); + write_exec( + &bin.join("activate-hermit"), + "#!/bin/bash\nexport HERMIT_ENV=\"${BUZZ_MCP_HERMIT_DIR}\"\n", + ); + + // uv probe: prints UV_NO_CONFIG and UV_INDEX_URL + write_exec( + &bin.join("uv"), + "#!/bin/bash\n\ + echo \"UV_NO_CONFIG=${UV_NO_CONFIG:-unset}\"\n\ + echo \"UV_INDEX_URL=${UV_INDEX_URL:-unset}\"\n", + ); + + // uvx probe: prints UV_NO_CONFIG and UV_INDEX_URL + write_exec( + &bin.join("uvx"), + "#!/bin/bash\n\ + echo \"UV_NO_CONFIG=${UV_NO_CONFIG:-unset}\"\n\ + echo \"UV_INDEX_URL=${UV_INDEX_URL:-unset}\"\n", + ); + + // npx probe: prints NPM isolation vars + write_exec( + &bin.join("npx"), + "#!/bin/bash\n\ + echo \"NPM_CONFIG_USERCONFIG=${NPM_CONFIG_USERCONFIG:-unset}\"\n\ + echo \"NPM_CONFIG_GLOBALCONFIG=${NPM_CONFIG_GLOBALCONFIG:-unset}\"\n\ + echo \"NPM_CONFIG_LOCATION=${NPM_CONFIG_LOCATION:-unset}\"\n", + ); + + // node probe: prints NPM isolation vars + write_exec( + &bin.join("node"), + "#!/bin/bash\n\ + echo \"NPM_CONFIG_USERCONFIG=${NPM_CONFIG_USERCONFIG:-unset}\"\n\ + echo \"NPM_CONFIG_GLOBALCONFIG=${NPM_CONFIG_GLOBALCONFIG:-unset}\"\n\ + echo \"NPM_CONFIG_LOCATION=${NPM_CONFIG_LOCATION:-unset}\"\n", + ); + + // python3 no-op (hermit install python3@3.10 resolves to this) + write_exec(&bin.join("python3"), "#!/bin/bash\nexit 0\n"); + + fs::write(dir.join("empty-user-npmrc"), "").unwrap(); + fs::write(dir.join("empty-global-npmrc"), "").unwrap(); + } + + fn run_real_shim(shim_name: &str, hermit_dir: &Path, log_dir: &Path) -> std::process::Output { + let real_shim = shim_dir().join(shim_name); + Command::new(&real_shim) + .env_clear() + .env( + "PATH", + format!("{}/bin:/usr/bin:/bin", hermit_dir.display()), + ) + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", hermit_dir) + .env("BUZZ_MCP_LOG_DIR", log_dir) + .output() + .unwrap_or_else(|e| panic!("failed to run real shim {shim_name}: {e}")) + } + + #[test] + fn real_uv_wrapper_sets_uv_no_config() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let out = run_real_shim("uv", &hermit_dir, &log_dir); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "real uv wrapper exited non-zero.\nstdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stdout.contains("UV_NO_CONFIG=1"), + "expected UV_NO_CONFIG=1 from real uv wrapper, got stdout: {stdout}" + ); + } + + #[test] + fn real_uvx_wrapper_sets_uv_no_config() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let out = run_real_shim("uvx", &hermit_dir, &log_dir); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "real uvx wrapper exited non-zero.\nstdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stdout.contains("UV_NO_CONFIG=1"), + "expected UV_NO_CONFIG=1 from real uvx wrapper, got stdout: {stdout}" + ); + } + + #[test] + fn real_npx_wrapper_sets_npm_isolation() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let out = run_real_shim("npx", &hermit_dir, &log_dir); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "real npx wrapper exited non-zero.\nstdout: {stdout}\nstderr: {stderr}" + ); + let expected_user = format!( + "NPM_CONFIG_USERCONFIG={}/empty-user-npmrc", + hermit_dir.display() + ); + let expected_global = format!( + "NPM_CONFIG_GLOBALCONFIG={}/empty-global-npmrc", + hermit_dir.display() + ); + assert!( + stdout.contains(&expected_user), + "expected {expected_user} in output, got: {stdout}" + ); + assert!( + stdout.contains(&expected_global), + "expected {expected_global} in output, got: {stdout}" + ); + assert!( + stdout.contains("NPM_CONFIG_LOCATION=global"), + "expected NPM_CONFIG_LOCATION=global in output, got: {stdout}" + ); + } + + #[test] + fn real_node_wrapper_sets_npm_isolation() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let out = run_real_shim("node", &hermit_dir, &log_dir); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "real node wrapper exited non-zero.\nstdout: {stdout}\nstderr: {stderr}" + ); + let expected_user = format!( + "NPM_CONFIG_USERCONFIG={}/empty-user-npmrc", + hermit_dir.display() + ); + let expected_global = format!( + "NPM_CONFIG_GLOBALCONFIG={}/empty-global-npmrc", + hermit_dir.display() + ); + assert!( + stdout.contains(&expected_user), + "expected {expected_user} in output, got: {stdout}" + ); + assert!( + stdout.contains(&expected_global), + "expected {expected_global} in output, got: {stdout}" + ); + assert!( + stdout.contains("NPM_CONFIG_LOCATION=global"), + "expected NPM_CONFIG_LOCATION=global in output, got: {stdout}" + ); + } + + #[test] + fn real_uv_wrapper_passes_explicit_override() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let real_shim = shim_dir().join("uv"); + let out = Command::new(&real_shim) + .env_clear() + .env( + "PATH", + format!("{}/bin:/usr/bin:/bin", hermit_dir.display()), + ) + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", &hermit_dir) + .env("BUZZ_MCP_LOG_DIR", &log_dir) + .env("UV_INDEX_URL", "https://explicit.example.com/simple") + .output() + .unwrap_or_else(|e| panic!("failed to run real uv shim: {e}")); + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "real uv wrapper exited non-zero.\nstdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stdout.contains("UV_INDEX_URL=https://explicit.example.com/simple"), + "explicit UV_INDEX_URL should pass through real wrapper, got: {stdout}" + ); + } + + /// Runs `setup-common.sh` directly (not through a toolchain wrapper) so + /// tests can assert on its own exit status/stderr without a real `uv`/ + /// `node` binary needing to exist on PATH afterwards. + fn run_setup_common( + toolchain: &str, + hermit_dir: &Path, + log_dir: &Path, + extra_path_prefix: &str, + ) -> std::process::Output { + let script = shim_dir().join("setup-common.sh"); + Command::new("bash") + .arg(&script) + .arg(toolchain) + .env_clear() + .env( + "PATH", + format!( + "{extra_path_prefix}{}/bin:/usr/bin:/bin", + hermit_dir.display() + ), + ) + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", hermit_dir) + .env("BUZZ_MCP_LOG_DIR", log_dir) + .output() + .unwrap_or_else(|e| panic!("failed to run setup-common.sh: {e}")) + } + + /// Finding 2a: a live lock holder must never be evicted purely for + /// being older than the previous 600s age threshold, even though this + /// test's own PID is real and the recorded starttime is stale/mismatched + /// relative to what the fresh `_proc_starttime` lookup would compute — + /// what makes eviction wrong here is liveness, not staleness. A second + /// setup-common.sh invocation must wait (and time out, bounded by + /// BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS) rather than steal the lock, proving + /// the outer timeout — not an internal age check — bounds the wait. + #[test] + fn live_owner_beyond_previous_600s_threshold_is_not_evicted() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&hermit_dir).unwrap(); + fs::create_dir_all(&log_dir).unwrap(); + + let lock_dir = hermit_dir.join(".mcp-hermit-setup.lock"); + fs::create_dir_all(&lock_dir).unwrap(); + // This test process is unquestionably alive; record an "acquired" + // timestamp far older than the old 600s eviction threshold so a + // regression to age-based eviction would steal it, but a + // liveness-based check correctly will not. + let ancient_acquired = 0; // epoch 0: as old as an "acquired" timestamp can get + fs::write( + lock_dir.join("info"), + format!("{}::{ancient_acquired}", std::process::id()), + ) + .unwrap(); + + let script = shim_dir().join("setup-common.sh"); + let mut child = Command::new("bash") + .arg(&script) + .arg("uv") + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", &hermit_dir) + .env("BUZZ_MCP_LOG_DIR", &log_dir) + // Bound the wait tightly so the test doesn't hang if the lock + // is (correctly) never stolen. + .env("BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS", "1") + .spawn() + .unwrap(); + + // Give the child a moment to reach (and block on) the lock loop. + std::thread::sleep(std::time::Duration::from_millis(300)); + assert!( + lock_dir.exists(), + "live-held lock must still exist while our process is alive" + ); + + let _ = child.kill(); + let _ = child.wait(); + let _ = fs::remove_dir_all(&lock_dir); + } + + /// Finding 2b: a lock left behind by a genuinely dead holder (PID no + /// longer exists) must be recoverable — the fix's dead-owner recovery + /// path must still work, so a crash never wedges the lock forever. + #[test] + fn dead_lock_owner_is_recoverable() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let lock_dir = hermit_dir.join(".mcp-hermit-setup.lock"); + fs::create_dir_all(&lock_dir).unwrap(); + // A PID that is essentially guaranteed not to exist, paired with a + // starttime that can't possibly match anything alive. + let dead_pid = 999_999; + fs::write(lock_dir.join("info"), format!("{dead_pid}:0:0")).unwrap(); + + let out = run_setup_common("uv", &hermit_dir, &log_dir, ""); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "setup-common.sh must recover a dead-owner lock rather than wait it out.\nstdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stderr.contains("dead lock holder detected"), + "expected dead-holder recovery log, got stderr: {stderr}" + ); + assert!( + !lock_dir.exists(), + "lock must be released after successful bootstrap" + ); + } + + /// Finding 3: an existing `bin/hermit` that fails to execute (here: + /// truncated/non-functional, simulating an interrupted prior download) + /// must be detected via the `--version` probe, removed, and + /// rebootstrapped from a stubbed local "download" — never accepted + /// on `-f` existence alone. The download path is stubbed by seeding + /// `PATH` with a fake `curl`/`openssl`/`bash`-invoked installer stand-in + /// so this test needs no network. + #[test] + fn invalid_existing_hermit_binary_is_recovered() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + let stub_bin = tmp.path().join("stub-bin"); + fs::create_dir_all(&log_dir).unwrap(); + fs::create_dir_all(hermit_dir.join("bin")).unwrap(); + fs::create_dir_all(&stub_bin).unwrap(); + + let write_exec = |path: &Path, content: &str| { + fs::write(path, content).unwrap(); + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); + }; + + // Existing "hermit" that is present (`-f` would accept it) but + // fails to run — the invalid/interrupted-bootstrap case. + write_exec(&hermit_dir.join("bin/hermit"), "#!/bin/bash\nexit 1\n"); + + // Stub curl: instead of downloading, emit a fixed install script + // body to whatever path was requested via -o. setup-common.sh SHA + // pins that script's *production* content + // (`INSTALL_SCRIPT_SHA256`), which this stub body deliberately does + // not match — so `openssl` below is stubbed too, to report the + // expected hash regardless of actual content. The SHA-pin check + // itself is unchanged, existing behavior; this test is about + // invalid-binary detection and atomic publish, not re-verifying the + // pin logic. + let install_script_body = "#!/bin/bash\n\ + # Stub install script standing in for hermit's real installer.\n\ + # Writes a working replacement hermit binary to $HERMIT_EXE.\n\ + # The replacement creates bin/activate-hermit on init that sets\n\ + # HERMIT_ENV, mirroring real hermit behavior.\n\ + mkdir -p \"$(dirname \"$HERMIT_EXE\")\"\n\ + cat > \"$HERMIT_EXE\" <<'STUBHERMIT'\n\ +#!/bin/bash\n\ +if [ \"$1\" = init ]; then\n\ + printf '#!/bin/bash\\nexport HERMIT_ENV=\"%s\"\\n' \"$(pwd)\" > bin/activate-hermit\n\ + chmod +x bin/activate-hermit\n\ +fi\n\ +exit 0\n\ +STUBHERMIT\n\ + chmod +x \"$HERMIT_EXE\"\n"; + const PINNED_INSTALL_SCRIPT_SHA256: &str = + "09ed936378857886fd4a7a4878c0f0c7e3d839883f39ca8b4f2f242e3126e1c6"; + + write_exec( + &stub_bin.join("curl"), + &format!( + "#!/bin/bash\n\ + # Stub curl: ignore the real URL args, emit the fixed install script\n\ + # body to whatever -o path was requested, regardless of flags order.\n\ + out=\"\"\n\ + while [ $# -gt 0 ]; do\n\ + case \"$1\" in\n\ + -o) out=\"$2\"; shift 2 ;;\n\ + *) shift ;;\n\ + esac\n\ + done\n\ + cat > \"$out\" <<'EOF'\n{install_script_body}EOF\n" + ), + ); + // Real openssl's `dgst -sha256 ` prints `SHA256()= ` + // — setup-common.sh extracts the hash via `awk '{print $2}'`. Stub + // reports the pinned hash unconditionally (see rationale above). + write_exec( + &stub_bin.join("openssl"), + &format!("#!/bin/bash\necho \"SHA256($3)= {PINNED_INSTALL_SCRIPT_SHA256}\"\n"), + ); + + let out = run_setup_common( + "uv", + &hermit_dir, + &log_dir, + &format!("{}:", stub_bin.display()), + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "setup-common.sh must recover an invalid existing hermit binary via stubbed re-download.\nstdout: {stdout}\nstderr: {stderr}" + ); + assert!( + stderr.contains("existing hermit binary invalid; removing and rebootstrapping"), + "expected invalid-binary recovery log, got stderr: {stderr}" + ); + let mode = hermit_dir + .join("bin/hermit") + .metadata() + .unwrap() + .permissions() + .mode(); + assert_ne!( + mode & 0o111, + 0, + "rebootstrapped hermit binary must be executable" + ); + } + + /// Fix 1 regression: two waiters on a stale lock must not overlap their + /// critical sections. The old protocol (observe-dead then rm -rf) let + /// waiter A delete waiter B's freshly acquired lock. The atomic-mv + /// reclaim ensures only one racer succeeds. + /// + /// The hermit binary writes overlap-detection markers during `install` + /// (which runs inside the locked critical section in setup-common.sh). + /// Each invocation writes its own PID marker, sleeps briefly, checks + /// for the other marker, then removes its own. If both markers exist + /// simultaneously, the critical sections overlapped. + #[test] + fn two_waiters_on_stale_lock_never_overlap_critical_sections() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + let lock_dir = hermit_dir.join(".mcp-hermit-setup.lock"); + fs::create_dir_all(&lock_dir).unwrap(); + let dead_pid = 999_999; + fs::write(lock_dir.join("info"), format!("{dead_pid}:0:0")).unwrap(); + + let overlap = tmp.path().join("overlap.detected"); + let cs_dir = tmp.path().join("cs-markers"); + fs::create_dir_all(&cs_dir).unwrap(); + + // Hermit binary that writes an overlap-detection marker during + // `install` (inside the locked region). `init` creates a valid + // activate-hermit. `--version` reports a version for validation. + let write_exec = |path: &Path, content: &str| { + fs::write(path, content).unwrap(); + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); + }; + write_exec( + &hermit_dir.join("bin/hermit"), + &format!( + "#!/bin/bash\n\ + case \"$1\" in\n\ + --version) echo \"stub 0.0.0\" ;;\n\ + init)\n\ + printf '#!/bin/bash\\nexport HERMIT_ENV=\"%s\"\\n' \"$(pwd)\" > bin/activate-hermit\n\ + chmod +x bin/activate-hermit\n\ + ;;\n\ + install)\n\ + touch \"{cs_dir}/$$.marker\"\n\ + sleep 0.3\n\ + for f in \"{cs_dir}\"/*.marker; do\n\ + [ -f \"$f\" ] || continue\n\ + other=\"$(basename \"$f\" .marker)\"\n\ + if [ \"$other\" != \"$$\" ]; then\n\ + touch \"{overlap}\"\n\ + fi\n\ + done\n\ + rm -f \"{cs_dir}/$$.marker\"\n\ + ;;\n\ + esac\n\ + exit 0\n", + cs_dir = cs_dir.display(), + overlap = overlap.display(), + ), + ); + + // Remove pre-seeded activate-hermit so init runs. + fs::remove_file(hermit_dir.join("bin/activate-hermit")).unwrap(); + + let script = shim_dir().join("setup-common.sh"); + + let spawn_waiter = || { + Command::new("bash") + .arg(&script) + .arg("uv") + .env_clear() + .env( + "PATH", + format!("{}/bin:/usr/bin:/bin", hermit_dir.display()), + ) + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", &hermit_dir) + .env("BUZZ_MCP_LOG_DIR", &log_dir) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + }; + + let child_a = spawn_waiter(); + let child_b = spawn_waiter(); + + let out_a = child_a.wait_with_output().unwrap(); + let out_b = child_b.wait_with_output().unwrap(); + + assert!( + out_a.status.success() && out_b.status.success(), + "both waiters must complete successfully.\n\ + A: exit={:?} stderr={}\n\ + B: exit={:?} stderr={}", + out_a.status, + String::from_utf8_lossy(&out_a.stderr), + out_b.status, + String::from_utf8_lossy(&out_b.stderr), + ); + assert!( + !overlap.exists(), + "critical sections overlapped: hermit install ran concurrently in both processes" + ); + } + + /// Fix 2 regression: a pre-existing malformed activate-hermit (exit 0 + /// body) with a valid hermit binary must be repaired, not silently + /// accepted. Setup must either repair-and-complete (installs run) or + /// exit nonzero — never exit 0 without installs. + #[test] + fn malformed_activate_hermit_is_repaired_not_silently_accepted() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + // Replace activate-hermit with a malformed file that exits 0 without + // setting HERMIT_ENV. Replace the hermit binary with one that creates + // a valid activate-hermit on init (mirroring real hermit behavior). + fs::write( + hermit_dir.join("bin/activate-hermit"), + "#!/bin/bash\nexit 0\n", + ) + .unwrap(); + fs::write( + hermit_dir.join("bin/hermit"), + "#!/bin/bash\n\ + if [ \"$1\" = init ]; then\n\ + printf '#!/bin/bash\\nexport HERMIT_ENV=\"%s\"\\n' \"$(pwd)\" > bin/activate-hermit\n\ + chmod +x bin/activate-hermit\n\ + fi\n\ + if [ \"$1\" = \"--version\" ]; then echo \"stub 0.0.0\"; fi\n\ + exit 0\n", + ) + .unwrap(); + fs::set_permissions( + hermit_dir.join("bin/hermit"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + + let out = run_setup_common("uv", &hermit_dir, &log_dir, ""); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "setup must repair malformed activate-hermit and succeed.\nstderr: {stderr}" + ); + assert!( + stderr.contains("initializing hermit environment"), + "expected re-init for malformed activate-hermit, got stderr: {stderr}" + ); + assert!( + stderr.contains("bootstrap complete for uv"), + "installs must actually run after repair.\nstderr: {stderr}" + ); + } + + /// Fix 2 variant: garbage content (not even a valid script) must also + /// be repaired, proving validation catches more than just `exit 0`. + #[test] + fn garbage_activate_hermit_is_repaired() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + + fs::write( + hermit_dir.join("bin/activate-hermit"), + b"THIS IS NOT A VALID SHELL SCRIPT\n\x00\xff", + ) + .unwrap(); + fs::write( + hermit_dir.join("bin/hermit"), + "#!/bin/bash\n\ + if [ \"$1\" = init ]; then\n\ + printf '#!/bin/bash\\nexport HERMIT_ENV=\"%s\"\\n' \"$(pwd)\" > bin/activate-hermit\n\ + chmod +x bin/activate-hermit\n\ + fi\n\ + if [ \"$1\" = \"--version\" ]; then echo \"stub 0.0.0\"; fi\n\ + exit 0\n", + ) + .unwrap(); + fs::set_permissions( + hermit_dir.join("bin/hermit"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + + let out = run_setup_common("uv", &hermit_dir, &log_dir, ""); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "setup must repair garbage activate-hermit and succeed.\nstderr: {stderr}" + ); + assert!( + stderr.contains("bootstrap complete for uv"), + "installs must actually run after repair.\nstderr: {stderr}" + ); + } + + /// Fix 3: the hostile-path test must actually hit the vulnerable code + /// path. This uses a wrapper shell that reports its own PID, blocks on a + /// fifo while the test plants the hostile binary at the exact path the + /// old vulnerable code would construct (/tmp/hermit_tmp_), + /// then exec's into setup-common.sh preserving PID. + #[test] + #[cfg(target_os = "linux")] + fn hostile_preexisting_tmp_path_is_never_executed() { + let tmp = tempfile::TempDir::new().unwrap(); + let hermit_dir = tmp.path().join("hermit"); + let log_dir = tmp.path().join("logs"); + fs::create_dir_all(&log_dir).unwrap(); + seed_fake_hermit_dir(&hermit_dir); + // Need a hermit that creates a valid activate-hermit on init. + fs::remove_file(hermit_dir.join("bin/activate-hermit")).unwrap(); + fs::write( + hermit_dir.join("bin/hermit"), + "#!/bin/bash\n\ + if [ \"$1\" = init ]; then\n\ + printf '#!/bin/bash\\nexport HERMIT_ENV=\"%s\"\\n' \"$(pwd)\" > bin/activate-hermit\n\ + chmod +x bin/activate-hermit\n\ + fi\n\ + exit 0\n", + ) + .unwrap(); + fs::set_permissions( + hermit_dir.join("bin/hermit"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + + // Synchronization: fifo for PID reporting, fifo for release signal. + let pid_fifo = tmp.path().join("pid.fifo"); + let go_fifo = tmp.path().join("go.fifo"); + assert!( + Command::new("mkfifo") + .arg(&pid_fifo) + .status() + .unwrap() + .success(), + "mkfifo failed for pid fifo" + ); + assert!( + Command::new("mkfifo") + .arg(&go_fifo) + .status() + .unwrap() + .success(), + "mkfifo failed for go fifo" + ); + + // Wrapper script: writes its own $$ to the pid fifo, blocks reading + // from the go fifo, then exec's into setup-common.sh (preserving PID). + let wrapper_path = tmp.path().join("wrapper.sh"); + let script = shim_dir().join("setup-common.sh"); + fs::write( + &wrapper_path, + format!( + "#!/bin/bash\n\ + echo $$ > \"{pid_fifo}\"\n\ + cat \"{go_fifo}\" > /dev/null\n\ + exec bash \"{script}\" uv\n", + pid_fifo = pid_fifo.display(), + go_fifo = go_fifo.display(), + script = script.display(), + ), + ) + .unwrap(); + fs::set_permissions(&wrapper_path, fs::Permissions::from_mode(0o755)).unwrap(); + + let child = Command::new("bash") + .arg(&wrapper_path) + .env_clear() + .env( + "PATH", + format!("{}/bin:/usr/bin:/bin", hermit_dir.display()), + ) + .env("HOME", std::env::var("HOME").unwrap()) + .env("BUZZ_MCP_HERMIT_DIR", &hermit_dir) + .env("BUZZ_MCP_LOG_DIR", &log_dir) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + + // Read the child's PID from the fifo. + let child_pid: u32 = std::fs::read_to_string(&pid_fifo) + .unwrap() + .trim() + .parse() + .unwrap(); + + // Plant the hostile binary at the exact path the old vulnerable + // code would construct: /tmp/hermit_tmp_/bin/hermit + let marker = tmp.path().join("hostile-ran.marker"); + let hostile_dir = PathBuf::from(format!("/tmp/hermit_tmp_{child_pid}/bin")); + fs::create_dir_all(&hostile_dir).unwrap(); + fs::write( + hostile_dir.join("hermit"), + format!("#!/bin/bash\ntouch {}\nexit 0\n", marker.display()), + ) + .unwrap(); + fs::set_permissions( + hostile_dir.join("hermit"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + + // Release the wrapper to exec into setup-common.sh. + fs::write(&go_fifo, "go\n").unwrap(); + + let out = child.wait_with_output().unwrap(); + + let _ = fs::remove_dir_all(format!("/tmp/hermit_tmp_{child_pid}")); + + assert!( + !marker.exists(), + "hostile binary at /tmp/hermit_tmp_{child_pid} was executed — \ + the fix (private mktemp + absolute-path invocation) is not effective" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "setup-common.sh should succeed via the real hermit copy.\n\ + stdout: {stdout}\nstderr: {stderr}" + ); + } +} diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index d346116132..5936d36d52 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -121,7 +121,9 @@ const overrides = new Map([ // record_provider param + applies persona_field_with_record_fallback. +5 lines. // global-agent-config: spawn_agent_child loads global config and merges as // lowest env layer (+8 lines). Queued to split. - ["src-tauri/src/managed_agents/runtime.rs", 2216], + // mcp-toolchain-shims: validate_shim_resources + hardened shim env setup. + // mesh-llm: relay mesh env derivation at spawn time (+12 lines from main). + ["src-tauri/src/managed_agents/runtime.rs", 2228], // config-bridge setup-payload env-boundary fix adds readiness wiring in // spawn_agent_child; load-bearing security fix, queued to split. ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016], diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 2c15813e48..94e8bd9c8a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1465,6 +1465,56 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } +/// Resource files that must exist under the bundled `shims/` resource +/// directory for MCP toolchain shims (uv/uvx/npx/node). Mirrors +/// `bundle.resources` in `tauri.conf.json` and `SHIMMED_COMMANDS` in +/// `crates/buzz-agent/src/mcp.rs`. +#[cfg(unix)] +const SHIM_RESOURCE_FILES: &[&str] = &["setup-common.sh", "uv", "uvx", "npx", "node"]; + +/// Validates that every file in `SHIM_RESOURCE_FILES` exists under `dir` as +/// a regular, executable file, attempting a `chmod +x` fixup for a file +/// that merely lost its execute bit. Returns the first actionable failure +/// rather than proceeding — a caller that sets `BUZZ_MCP_SHIM_DIR` despite a +/// broken resource would let `mcp.rs` silently fall through to hostile +/// system `uv`/`npx` with no diagnostic. +#[cfg(unix)] +fn validate_shim_resources(dir: &std::path::Path) -> Result<(), String> { + use std::os::unix::fs::PermissionsExt; + + for name in SHIM_RESOURCE_FILES { + let path = dir.join(name); + let meta = std::fs::metadata(&path).map_err(|error| { + format!( + "MCP shim setup failed: resource `{name}` missing or unreadable at {}: {error}", + path.display() + ) + })?; + if !meta.is_file() { + return Err(format!( + "MCP shim setup failed: resource `{name}` at {} is not a regular file", + path.display() + )); + } + if meta.permissions().mode() & 0o111 != 0 { + continue; + } + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(meta.permissions().mode() | 0o755)).map_err(|error| { + format!("MCP shim setup failed: resource `{name}` at {} is not executable and chmod failed: {error}", path.display()) + })?; + let fixed_meta = std::fs::metadata(&path).map_err(|error| { + format!("MCP shim setup failed: resource `{name}` at {} became unreadable after chmod: {error}", path.display()) + })?; + if fixed_meta.permissions().mode() & 0o111 == 0 { + return Err(format!( + "MCP shim setup failed: resource `{name}` at {} is still not executable after chmod attempt", + path.display() + )); + } + } + Ok(()) +} + /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -1581,6 +1631,59 @@ pub fn spawn_agent_child( command.env("BUZZ_ACP_MCP_COMMAND", ""); } } + // MCP toolchain shims: point buzz-agent at the bundled shim scripts so + // bare uv/uvx/npx/node commands resolve to hermit-managed, config-isolated + // toolchains. In a Tauri bundle, resources are in Contents/Resources/ (macOS) + // or alongside the binary (Linux). In dev mode, resolve from the source tree. + // + // Both `BUZZ_MCP_SHIM_DIR` and `BUZZ_MCP_HERMIT_DIR` are set only after + // validation succeeds — mcp.rs's resolver checks only file existence, so + // an unvalidated dir pointing at broken/non-executable resources would + // silently fall through to hostile system `uv`/`npx` (mcp.rs's + // resolve_shim_command falls back to the bare command when the shim path + // doesn't exist, and a non-executable shim fails opaquely at spawn time). + // A missing resource directory itself is not an error: it means this + // build wasn't packaged with shims (e.g. an old bundle), which is + // reported once via eprintln, not fatal to spawning the agent. + #[cfg(unix)] + { + use tauri::Manager; + let resource_shim_dir = app.path().resource_dir().ok().map(|d| d.join("shims")); + match resource_shim_dir { + Some(ref dir) if dir.is_dir() => { + validate_shim_resources(dir).map_err(|error| { + format!("{error}. MCP servers that rely on uv/uvx/npx/node will not work.") + })?; + command.env("BUZZ_MCP_SHIM_DIR", dir); + + let hermit_dir = app + .path() + .app_data_dir() + .map_err(|error| { + format!("MCP shim setup failed: could not resolve app data dir: {error}") + })? + .join("mcp-hermit"); + std::fs::create_dir_all(&hermit_dir).map_err(|error| { + format!( + "MCP shim setup failed: could not create Hermit state dir at {}: {error}", + hermit_dir.display() + ) + })?; + command.env("BUZZ_MCP_HERMIT_DIR", &hermit_dir); + } + _ => { + eprintln!( + "buzz-desktop: no shims resource dir found — MCP servers will use bare uv/uvx/npx/node from PATH" + ); + } + } + } + if let Some(log_dir) = dirs::data_local_dir().map(|d| d.join("xyz.block.buzz.app").join("logs")) + { + let _ = std::fs::create_dir_all(&log_dir); + command.env("BUZZ_MCP_LOG_DIR", &log_dir); + } + // Enable MCP hook tools (_Stop, _PostCompact) for agents that need them. // Uses "*" because build_mcp_servers() hard-codes the server name to "buzz-mcp". let runtime_meta = known_acp_runtime(&effective_command); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 06d3ac6db3..41d33a481a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -760,3 +760,84 @@ fn own_group_grandchild_detected_by_ancestor_walk() { unsafe { libc::kill(-(intermediate_pid as i32), libc::SIGKILL) }; let _ = intermediate.wait(); } + +// ── validate_shim_resources tests ─────────────────────────────────────── + +#[cfg(unix)] +mod shim_resource_validation { + use std::os::unix::fs::PermissionsExt; + + use tempfile::TempDir; + + use super::super::{validate_shim_resources, SHIM_RESOURCE_FILES}; + + /// Writes a fully valid, executable shim resource dir — the happy path + /// every other test in this module diverges from by breaking exactly + /// one file. + fn make_valid_shim_dir() -> TempDir { + let dir = TempDir::new().expect("create temp dir"); + for name in SHIM_RESOURCE_FILES { + let path = dir.path().join(name); + std::fs::write(&path, "#!/bin/bash\n").expect("write shim file"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)) + .expect("chmod shim file"); + } + dir + } + + #[test] + fn valid_executable_resources_pass() { + let dir = make_valid_shim_dir(); + assert!(validate_shim_resources(dir.path()).is_ok()); + } + + #[test] + fn missing_resource_file_fails_with_actionable_message() { + let dir = make_valid_shim_dir(); + std::fs::remove_file(dir.path().join("node")).expect("remove node shim"); + let error = validate_shim_resources(dir.path()).expect_err("missing file must fail"); + assert!( + error.contains("node"), + "error must name the missing resource: {error}" + ); + } + + #[test] + fn non_executable_resource_is_fixed_up_by_chmod() { + let dir = make_valid_shim_dir(); + let path = dir.path().join("uv"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)) + .expect("strip execute bit"); + assert!( + validate_shim_resources(dir.path()).is_ok(), + "chmod fixup should recover a resource that merely lost its execute bit" + ); + let mode = std::fs::metadata(&path).unwrap().permissions().mode(); + assert_ne!(mode & 0o111, 0, "uv should be executable after fixup"); + } + + #[test] + fn directory_in_place_of_resource_file_fails() { + let dir = make_valid_shim_dir(); + let path = dir.path().join("uvx"); + std::fs::remove_file(&path).expect("remove uvx shim"); + std::fs::create_dir(&path).expect("create dir named uvx"); + let error = + validate_shim_resources(dir.path()).expect_err("a directory is not a regular file"); + assert!( + error.contains("uvx") && error.contains("not a regular file"), + "error must identify the non-regular-file resource: {error}" + ); + } + + #[test] + fn missing_resource_directory_itself_fails() { + let dir = TempDir::new().expect("create temp dir"); + let missing = dir.path().join("does-not-exist"); + let error = validate_shim_resources(&missing).expect_err("missing dir must fail"); + assert!( + error.contains("setup-common.sh"), + "error should name the first expected resource: {error}" + ); + } +} diff --git a/desktop/src-tauri/tauri.conf.json b/desktop/src-tauri/tauri.conf.json index b92d792dab..9d64a5856a 100644 --- a/desktop/src-tauri/tauri.conf.json +++ b/desktop/src-tauri/tauri.conf.json @@ -59,6 +59,9 @@ "binaries/git-credential-nostr", "binaries/buzz" ], + "resources": { + "../../crates/buzz-agent/resources/shims/": "shims/" + }, "icon": [ "icons/32x32.png", "icons/128x128.png", diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 3b2db4e4f0..890c48c0c8 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -100,6 +100,12 @@ run_unit_tests() { run_test_step "buzz-push-gateway tests" \ cargo test -p buzz-push-gateway -- --nocapture + + # MCP shim hostile-config suite: infra-free (real wrapper scripts + + # stubbed curl/openssl). Contains the Linux-only hostile-path regression + # for the CRITICAL temp-execution fix — this is its only CI execution path. + run_test_step "buzz-agent shim hostile-config tests" \ + cargo test -p buzz-agent --test shim_hostile_config -- --nocapture } # ---- DB / integration tests (infra required) --------------------------------