From 383ed44fe84165e1cecbf4eab4b1e25f38e8832f Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 11 May 2026 23:19:44 -0500 Subject: [PATCH] fix(child_env): preserve MSVC toolchain vars for Windows exec_shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harvested from PR #1487 by @Jianfengwu2024 — the rest of that PR (the TriadMind architecture-governance crate) needs a Discussion- level design conversation before it can land, but this Windows env-allowlist fix is a clean independent improvement and stands on its own. When the parent shell has already loaded VsDevCmd / vcvars (the standard pattern for running Rust + MSVC on Windows), `exec_shell` was stripping the toolchain env on its way to the child. The result: the model finds `link.exe` via `PATH` but the linker can't resolve `kernel32.lib` / `ucrt.lib` because LIB and the SDK roots were filtered out. Any model-driven `cargo build` from inside the TUI silently broke on Windows installs that don't run inside a Developer Command Prompt. Adds 13 MSVC-related env vars to the `is_allowed_parent_env_key` allowlist so they survive the sanitization pass: LIB / LIBPATH / INCLUDE VSINSTALLDIR / VCINSTALLDIR / VCTOOLSINSTALLDIR WINDOWSSDKDIR / WINDOWSSDKVERSION UNIVERSALCRTSDKDIR / UCRTVERSION EXTENSIONSDKDIR / DEVENVDIR / VISUALSTUDIOVERSION Also extends the `mcp_env_allowlist_inherits_base_keys` fixture and adds `sanitized_child_env_preserves_windows_toolchain_vars` as a regression test (locked under `env_lock()` so it serialises with the other env-mutating tests in the file). Pure additive — no non-Windows behaviour changes. Harvested from PR #1487 by @Jianfengwu2024 --- CHANGELOG.md | 10 +++++ crates/tui/src/child_env.rs | 85 ++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f69e0024..170ca76a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,16 @@ into `main`, so credit lands at the same moment the fix does. ### Fixed +- **Windows `exec_shell` preserves MSVC toolchain env** (harvested + from PR #1487 by **@Jianfengwu2024**). When the parent shell has + already loaded `VsDevCmd` / `vcvars` (Developer Command Prompt, + the standard way to run Rust + MSVC on Windows), `exec_shell` was + stripping `LIB` / `LIBPATH` / `INCLUDE` and the related VS / SDK / + CRT root variables on its way to the child. That made + model-driven `cargo build` calls fail to resolve `kernel32.lib` + even though `link.exe` was reachable via `PATH`. The allowlist + in `child_env.rs` now preserves the 13 MSVC env vars so the + toolchain context survives the sanitisation pass. - **`code_execution` no longer fails with "program not found" on Windows** (and any other host without `python3` on `PATH`). Before v0.8.31 the tool hardcoded `python3` and was unconditionally diff --git a/crates/tui/src/child_env.rs b/crates/tui/src/child_env.rs index 411fe022..8c389961 100644 --- a/crates/tui/src/child_env.rs +++ b/crates/tui/src/child_env.rs @@ -149,6 +149,26 @@ fn is_allowed_parent_env_key(key: &OsStr) -> bool { | "USERPROFILE" | "HOMEDRIVE" | "HOMEPATH" + // Preserve Windows toolchain context when the parent shell has + // already loaded VsDevCmd / vcvars. Without these, `exec_shell` + // can find `link.exe` via PATH but still fail to resolve + // SDK/CRT libraries like `kernel32.lib`, so any model-driven + // `cargo build` from inside the TUI silently breaks on + // Windows installs that don't run inside a Developer Command + // Prompt. Harvested from PR #1487. + | "LIB" + | "LIBPATH" + | "INCLUDE" + | "VSINSTALLDIR" + | "VCINSTALLDIR" + | "VCTOOLSINSTALLDIR" + | "WINDOWSSDKDIR" + | "WINDOWSSDKVERSION" + | "UNIVERSALCRTSDKDIR" + | "UCRTVERSION" + | "EXTENSIONSDKDIR" + | "DEVENVDIR" + | "VISUALSTUDIOVERSION" ) || normalized.starts_with("LC_") } @@ -232,7 +252,19 @@ mod tests { #[test] fn mcp_env_allowlist_inherits_base_keys() { - for key in ["PATH", "HOME", "USER", "TERM", "LANG", "SHELL"] { + for key in [ + "PATH", + "HOME", + "USER", + "TERM", + "LANG", + "SHELL", + "LIB", + "LIBPATH", + "INCLUDE", + "VCTOOLSINSTALLDIR", + "WINDOWSSDKDIR", + ] { assert!( is_allowed_mcp_env_key(OsStr::new(key)), "MCP allowlist should inherit base key {key}" @@ -451,4 +483,55 @@ mod tests { .map(|(_, value)| value); assert_eq!(path, Some(&OsString::from("/explicit/bin"))); } + + #[test] + fn sanitized_child_env_preserves_windows_toolchain_vars() { + let _guard = env_lock().lock().expect("env lock"); + let prev_lib = std::env::var_os("LIB"); + let prev_include = std::env::var_os("INCLUDE"); + let prev_sdk = std::env::var_os("WINDOWSSDKDIR"); + // SAFETY: serialised by env_lock above. Restoring after the + // assertion is also under the same guard so concurrent tests + // never see our staged values. + unsafe { + std::env::set_var("LIB", r"C:\sdk\lib"); + std::env::set_var("INCLUDE", r"C:\sdk\include"); + std::env::set_var("WINDOWSSDKDIR", r"C:\sdk"); + } + + let env = sanitized_child_env(std::iter::empty::<(OsString, OsString)>()); + + // Restore prior state before asserting so a panic still leaves + // the process env clean for the next test. + unsafe { + match prev_lib { + Some(value) => std::env::set_var("LIB", value), + None => std::env::remove_var("LIB"), + } + match prev_include { + Some(value) => std::env::set_var("INCLUDE", value), + None => std::env::remove_var("INCLUDE"), + } + match prev_sdk { + Some(value) => std::env::set_var("WINDOWSSDKDIR", value), + None => std::env::remove_var("WINDOWSSDKDIR"), + } + } + + assert!( + env.iter() + .any(|(key, value)| key == "LIB" && value == r"C:\sdk\lib"), + "child env should preserve LIB" + ); + assert!( + env.iter() + .any(|(key, value)| key == "INCLUDE" && value == r"C:\sdk\include"), + "child env should preserve INCLUDE" + ); + assert!( + env.iter() + .any(|(key, value)| key == "WINDOWSSDKDIR" && value == r"C:\sdk"), + "child env should preserve WINDOWSSDKDIR" + ); + } }