diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d18c7cb..95d5306 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -155,6 +155,13 @@ jobs: - name: Test the AppContainer backend (core proofs) run: cargo test -p agent-bridle-core --features windows-appcontainer + # Run the shell ENGINES functionally on Windows (#209). The clippy step + # above only COMPILES them; this EXERCISES the in-process brush engine and + # the carried-coreutils dispatch/re-exec — neither ran on Windows before + # 0.7.0-rc.1. HOOK PARITY: mirrored by `just check-windows`. + - name: Run the shell engines on Windows (brush + carried coreutils) + run: cargo test -p agent-bridle-tool-shell --features brush,carried-coreutils --test brush_real --test carried_coreutils + # The launcher's own unit tests: MSVC CommandLineToArgvW quoting (arg-injection # surface) and the flag parser. Windows-only crate — only this job builds it. - name: Test the agent-bridle-aclaunch launcher (unit) diff --git a/agent-bridle-tool-shell/src/brush_shell.rs b/agent-bridle-tool-shell/src/brush_shell.rs index 47d7ffc..f15e1ef 100644 --- a/agent-bridle-tool-shell/src/brush_shell.rs +++ b/agent-bridle-tool-shell/src/brush_shell.rs @@ -243,6 +243,26 @@ fn run_in_brush( .set_global("PATH", ShellVariable::new(path_value)) .map_err(|e| ToolError::Exec(brush_io("seed PATH", &e)))?; + // Windows: a child spawned under `do_not_inherit_env(true)` needs the + // OS-minimal vars (`SystemRoot`, …) or `CreateProcess`/CRT init fails to + // start it at all. These are not secrets — every Windows process needs + // them — so seeding them keeps external commands and the carried-coreutils + // re-exec runnable under confinement. Unix needs none of this. + #[cfg(windows)] + for key in [ + "SystemRoot", + "SystemDrive", + "windir", + "TEMP", + "TMP", + "USERPROFILE", + "NUMBER_OF_PROCESSORS", + ] { + if let Ok(val) = std::env::var(key) { + let _ = shell.env_mut().set_global(key, ShellVariable::new(val)); + } + } + let result = shell .run_dash_c_command(cmd) .await diff --git a/agent-bridle-tool-shell/tests/carried_coreutils.rs b/agent-bridle-tool-shell/tests/carried_coreutils.rs index 9b03e27..c98a4fd 100644 --- a/agent-bridle-tool-shell/tests/carried_coreutils.rs +++ b/agent-bridle-tool-shell/tests/carried_coreutils.rs @@ -8,7 +8,7 @@ //! regressed, the shell would find no `ls`/`cat` at all. #![cfg(feature = "carried-coreutils")] -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::atomic::{AtomicU64, Ordering}; @@ -17,6 +17,18 @@ fn dispatch_host() -> &'static str { env!("CARGO_BIN_EXE_dispatch_host") } +/// Shell-quote a path for splicing into a brush command string. brush's +/// parser is POSIX-style: an unquoted `\` is an escape character. Windows +/// paths are backslash-separated, so without this an unquoted path like +/// `C:\Users\...\hello.txt` gets silently mangled to `C:Users...hello.txt` +/// (issue #209 W4 finding) — `\U`, `\A`, etc. get collapsed to the escaped +/// letter. Single-quoting is a no-op on Unix (paths there never contain `'` +/// in these tests) and makes the Windows path safe. Mirrors +/// `agent-bridle-jaild::vm::shell_quote`. +fn shell_quote(p: &Path) -> String { + format!("'{}'", p.to_string_lossy().replace('\'', "'\\''")) +} + fn unique_temp(tag: &str) -> PathBuf { static N: AtomicU64 = AtomicU64::new(0); std::env::temp_dir().join(format!( @@ -27,6 +39,31 @@ fn unique_temp(tag: &str) -> PathBuf { )) } +/// A `dispatch_host` command with **host tools removed from PATH** (no `PATH` at +/// all) but the OS-minimal environment preserved. On Windows a fully-empty +/// environment breaks process startup (`SystemRoot`, …), so we keep only those +/// non-secret, always-required vars — PATH stays absent, so only carried tools +/// can satisfy a bare `ls`/`cat`. On unix nothing extra is needed. +fn scrubbed() -> Command { + let mut c = Command::new(dispatch_host()); + c.env_clear(); + #[cfg(windows)] + for key in [ + "SystemRoot", + "SystemDrive", + "windir", + "TEMP", + "TMP", + "USERPROFILE", + "NUMBER_OF_PROCESSORS", + ] { + if let Ok(v) = std::env::var(key) { + c.env(key, v); + } + } + c +} + /// Carried `ls` lists a directory with the environment fully scrubbed — no host /// `/bin/ls`, no `PATH`. It resolves to the in-process uutils `ls` via the shim's /// re-exec of the dispatch-capable host binary. @@ -36,9 +73,8 @@ fn carried_ls_runs_in_process_with_env_scrubbed() { std::fs::create_dir_all(&dir).unwrap(); std::fs::write(dir.join("MARKER.txt"), b"x").unwrap(); - let out = Command::new(dispatch_host()) - .env_clear() - .arg(format!("ls {}", dir.to_string_lossy())) + let out = scrubbed() + .arg(format!("ls {}", shell_quote(&dir))) .output() .expect("run dispatch_host"); @@ -64,9 +100,8 @@ fn carried_cat_runs_in_process_with_env_scrubbed() { let file = dir.join("hello.txt"); std::fs::write(&file, b"carried-cat-ok\n").unwrap(); - let out = Command::new(dispatch_host()) - .env_clear() - .arg(format!("cat {}", file.to_string_lossy())) + let out = scrubbed() + .arg(format!("cat {}", shell_quote(&file))) .output() .expect("run dispatch_host"); diff --git a/justfile b/justfile index 65532b4..243b811 100644 --- a/justfile +++ b/justfile @@ -47,6 +47,9 @@ check-windows: set -e cargo clippy --workspace --exclude agent-bridle-py --all-targets --all-features -- -D warnings cargo test -p agent-bridle-core --features windows-appcontainer + # Exercise the shell ENGINES on Windows (#209) — clippy only compiles them. + # HOOK PARITY: mirrors the "Run the shell engines on Windows" step in ci.yml. + cargo test -p agent-bridle-tool-shell --features brush,carried-coreutils --test brush_real --test carried_coreutils cargo test -p agent-bridle-aclaunch --bins BRIDLE_REQUIRE_APPCONTAINER=1 cargo test -p agent-bridle-aclaunch --test kernel_proofs -- --test-threads=1 BRIDLE_REQUIRE_APPCONTAINER=1 cargo test -p agent-bridle-aclaunch --test net_proofs -- --test-threads=1