fix(child_env): preserve MSVC toolchain vars for Windows exec_shell
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user