From dd408e9958225d71e10da5788e001deb15cd078c Mon Sep 17 00:00:00 2001 From: agent-of-mkmeral Date: Wed, 24 Jun 2026 17:40:26 +0000 Subject: [PATCH 1/2] feat: add Windows support Strands Shell did not compile on Windows because src/vfs_kernel.rs used an unconditional Linux-only TOCTOU check (std::os::unix::io::AsRawFd plus /proc/self/fd) in the host-file read path, and two integration tests created symlinks via std::os::unix::fs::symlink. Core fix (src/vfs_kernel.rs): - Gate the /proc/self/fd defense-in-depth TOCTOU verification behind #[cfg(target_os = "linux")]. /proc/self/fd is a Linux-only procfs interface (macOS has no /proc; Windows has no procfs), so the check only ever functioned on Linux. The canonical-path check performed before open_host() remains the primary security boundary on all platforms, so this narrows the platform of an extra layer rather than weakening the guarantee on Linux. - Silence the now-conditionally-unused canon_base arg on non-Linux targets. Tests (tests/shell_integration.rs): - Gate bind_direct_symlink_escape_blocked and bind_direct_dangling_symlink_blocked behind #[cfg(unix)] since std::os::unix::fs::symlink is unavailable on Windows (symlink creation there also requires elevated privileges). CI (.github/workflows/ci.yml): - Add windows-latest to the rust, python, and node test matrices. - Make the python job cross-platform: create the venv and prepend its bin/Scripts dir to GITHUB_PATH instead of hardcoding .venv/bin. - On Windows, set npm script-shell to bash so package.json's $(npm run --silent host-triple) command substitution works. CD (.github/workflows/release.yml, package.json): - Build + publish Windows artifacts: x86_64-pc-windows-msvc Python wheel, node addon, and the strands-agents-shell-win32-x64-msvc npm package. - Update the inspect coverage check (win_amd64 wheel) and the npm pack count guard (5 -> 6 packages). Validated by cross-compiling lib, bins and all tests to x86_64-pc-windows-gnu (clean), cross-checking the python feature with PYO3_CROSS_PYTHON_VERSION, and running the full 1296-test suite on Linux. --- .github/workflows/ci.yml | 33 ++++++++++++++++++++++++------- .github/workflows/release.yml | 12 +++++++++--- package.json | 3 ++- src/vfs_kernel.rs | 37 ++++++++++++++++++++++++----------- tests/shell_integration.rs | 6 ++++++ 5 files changed, 69 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 33dc8a7..2342c55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 @@ -43,7 +43,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] python: ["3.10", "3.11", "3.12", "3.13", "3.14"] runs-on: ${{ matrix.os }} steps: @@ -62,17 +62,27 @@ jobs: with: python-version: ${{ matrix.python }} + # Create the venv and put its bin/Scripts dir on PATH so the + # subsequent steps work identically on Unix (.venv/bin) and + # Windows (.venv/Scripts). - name: Create virtualenv - run: python -m venv .venv + shell: bash + run: | + python -m venv .venv + if [ "$RUNNER_OS" == "Windows" ]; then + echo "$PWD/.venv/Scripts" >> "$GITHUB_PATH" + else + echo "$PWD/.venv/bin" >> "$GITHUB_PATH" + fi - name: Install build deps - run: .venv/bin/pip install maturin pytest + run: pip install maturin pytest - name: Build and install wheel - run: .venv/bin/maturin develop --release + run: maturin develop --release - name: pytest - run: .venv/bin/pytest tests/python -v + run: pytest tests/python -v audit: name: Security audit @@ -115,7 +125,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] node: ["20", "22", "24"] runs-on: ${{ matrix.os }} steps: @@ -134,6 +144,15 @@ jobs: with: node-version: ${{ matrix.node }} + # The package.json build scripts use `$(npm run --silent host-triple)` + # command substitution. npm runs script bodies with its configured + # script-shell, which defaults to cmd.exe on Windows (no `$(...)` + # support). Point it at the Git Bash that ships on the windows-latest + # runner so the substitution works identically across platforms. + - name: Use bash as npm script-shell (Windows) + if: runner.os == 'Windows' + run: npm config set script-shell bash + - name: npm install # package-lock.json is gitignored, so `npm ci` can't run; use install. run: npm install diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3b18191..c5a9945 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -60,6 +60,8 @@ jobs: - os: ubuntu-latest target: aarch64-unknown-linux-gnu manylinux: "2_28" + - os: windows-latest + target: x86_64-pc-windows-msvc runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 @@ -158,7 +160,7 @@ jobs: set -euo pipefail echo "Dists:"; ls -1 dist/ missing=() - for target in aarch64-apple-darwin x86_64-apple-darwin x86_64-unknown-linux-gnu aarch64-unknown-linux-gnu; do + for target in aarch64-apple-darwin x86_64-apple-darwin x86_64-unknown-linux-gnu aarch64-unknown-linux-gnu x86_64-pc-windows-msvc; do # Maturin encodes the target in the wheel filename's platform tag: # aarch64-apple-darwin → macosx_*_arm64 # x86_64-apple-darwin → macosx_*_x86_64 @@ -169,6 +171,7 @@ jobs: x86_64-apple-darwin) pattern="macosx_*_x86_64" ;; x86_64-unknown-linux-gnu) pattern="manylinux_*_x86_64" ;; aarch64-unknown-linux-gnu) pattern="manylinux_*_aarch64" ;; + x86_64-pc-windows-msvc) pattern="win_amd64" ;; esac if ! ls dist/*${pattern}*.whl >/dev/null 2>&1; then missing+=("$target") @@ -228,6 +231,8 @@ jobs: target: x86_64-unknown-linux-gnu - os: ubuntu-latest target: aarch64-unknown-linux-gnu + - os: windows-latest + target: x86_64-pc-windows-msvc runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 @@ -353,11 +358,11 @@ jobs: # changed the targets to update node-publish-platform's matrix to match. run: | set -euo pipefail - expected=5 + expected=6 actual=$(ls dist-npm/*.tgz | wc -l | tr -d ' ') echo "Packed tarballs ($actual):"; ls -1 dist-npm/*.tgz if [ "$actual" -ne "$expected" ]; then - echo "::error::Expected $expected npm packages (1 main + 4 platform) but packed $actual. If napi.targets changed, update node-publish-platform's matrix to match." + echo "::error::Expected $expected npm packages (1 main + 5 platform) but packed $actual. If napi.targets changed, update node-publish-platform's matrix to match." exit 1 fi @@ -401,6 +406,7 @@ jobs: - strands-agents-shell-darwin-arm64 - strands-agents-shell-linux-x64-gnu - strands-agents-shell-linux-arm64-gnu + - strands-agents-shell-win32-x64-msvc env: V: ${{ needs.version.outputs.version }} steps: diff --git a/package.json b/package.json index cfa4aa3..f1d0cab 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "x86_64-apple-darwin", "aarch64-apple-darwin", "x86_64-unknown-linux-gnu", - "aarch64-unknown-linux-gnu" + "aarch64-unknown-linux-gnu", + "x86_64-pc-windows-msvc" ] }, "engines": { diff --git a/src/vfs_kernel.rs b/src/vfs_kernel.rs index 82c5b74..56bfd42 100644 --- a/src/vfs_kernel.rs +++ b/src/vfs_kernel.rs @@ -121,20 +121,35 @@ impl VfsKernel { canon_base: &std::path::Path, ) -> io::Result { if flags.read && !flags.write { - // Open the file, then verify via /proc/self/fd that the - // opened fd still points within the bind mount. This - // eliminates the TOCTOU between canonicalize and read. let file = std::fs::File::open(host_path)?; - use std::os::unix::io::AsRawFd; - let fd_path = format!("/proc/self/fd/{}", file.as_raw_fd()); - if let Ok(real) = std::fs::read_link(&fd_path) - && !real.starts_with(canon_base) + + // Defense-in-depth TOCTOU check: after opening, verify via + // /proc/self/fd that the opened fd still points within the bind + // mount. This closes the race between canonicalize() and open(). + // + // This is Linux-only: /proc/self/fd is a Linux procfs interface + // with no portable equivalent (macOS has no /proc; Windows has no + // procfs at all). On other platforms the canonical-path check + // performed before this function is called is the primary security + // boundary, so we simply skip this extra verification there. + #[cfg(target_os = "linux")] { - return Err(io::Error::new( - io::ErrorKind::PermissionDenied, - "access denied: path escaped bind mount", - )); + use std::os::unix::io::AsRawFd; + let fd_path = format!("/proc/self/fd/{}", file.as_raw_fd()); + if let Ok(real) = std::fs::read_link(&fd_path) + && !real.starts_with(canon_base) + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "access denied: path escaped bind mount", + )); + } } + // `canon_base` is only consumed by the Linux-only check above; on + // other targets it is intentionally unused. + #[cfg(not(target_os = "linux"))] + let _ = canon_base; + use std::io::Read; let mut data = Vec::new(); let mut file = file; diff --git a/tests/shell_integration.rs b/tests/shell_integration.rs index c4512fb..b2c5302 100644 --- a/tests/shell_integration.rs +++ b/tests/shell_integration.rs @@ -4277,6 +4277,9 @@ fn builder_bind_direct_readonly_host() { })); } +// Uses std::os::unix::fs::symlink — symlink creation differs on Windows +// (requires elevated privileges), so this escape test is Unix-only. +#[cfg(unix)] #[test] fn bind_direct_symlink_escape_blocked() { let (rt, local) = rt(); @@ -7381,6 +7384,9 @@ expect!(cmd_sleep_zero, "command sleep 0 && echo ok", "ok"); // ── dangling symlink escape prevention ────────────────────────────── +// Uses std::os::unix::fs::symlink — symlink creation differs on Windows +// (requires elevated privileges), so this escape test is Unix-only. +#[cfg(unix)] #[test] fn bind_direct_dangling_symlink_blocked() { let (rt, local) = rt(); From f26bd58413ce470027f8cf01c19742cb2b1a6db3 Mon Sep 17 00:00:00 2001 From: agent-of-mkmeral Date: Thu, 25 Jun 2026 19:09:41 +0000 Subject: [PATCH 2/2] fix(ci): make Windows Python/Rust CI legs pass Two native-Windows-only failures that the windows-gnu cross-compile gate could not catch (cross-compile never runs tests nor builds the Python module): - tests/shell_integration.rs: the config_file_with_binds_and_creds test interpolated a Windows temp path (C:\Users\...) into a TOML *basic* string, where \U/\A are parsed as invalid escape sequences -> TOML parse error. Switched to a TOML *literal* string (single quotes), which does no escape processing; Windows paths never contain single quotes. - .github/workflows/ci.yml: on Windows the venv Scripts dir was added to GITHUB_PATH using the Git Bash $PWD (an MSYS path like /d/a/shell/shell) that the Windows PATH cannot resolve, so the venv was silently ignored and pytest ran from the host interpreter -> ModuleNotFoundError: strands_shell. Convert with cygpath -w so .venv\Scripts is actually used. Also refresh stale doc-comment counts in release.yml (4->5 platform packages, 5->6 total) now that win32-x64-msvc is included; the enforced expected=6 guard and matrices were already correct. Verified locally: cargo test --workspace --all-targets (1296+ pass), maturin develop + pytest tests/python (44 pass), cargo fmt/clippy/doc. --- .github/workflows/ci.yml | 7 ++++++- .github/workflows/release.yml | 10 +++++----- tests/shell_integration.rs | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2342c55..48a80c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,8 +69,13 @@ jobs: shell: bash run: | python -m venv .venv + # GITHUB_PATH entries must be native paths. In Git Bash on Windows + # $PWD is an MSYS path (/d/a/shell/shell) that the runner's Windows + # PATH cannot resolve, so the venv would be silently ignored and + # later steps would fall back to the host interpreter. Convert to a + # native Windows path with cygpath so .venv\Scripts is actually used. if [ "$RUNNER_OS" == "Windows" ]; then - echo "$PWD/.venv/Scripts" >> "$GITHUB_PATH" + cygpath -w "$PWD/.venv/Scripts" >> "$GITHUB_PATH" else echo "$PWD/.venv/bin" >> "$GITHUB_PATH" fi diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c5a9945..6c1d826 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -290,9 +290,9 @@ jobs: native.js native.d.ts - # ── Pack all 5 npm packages (fan-in from the per-platform builds) ───────── + # ── Pack all 6 npm packages (fan-in from the per-platform builds) ───────── # Assembles the exact publishable set — the main @strands-agents/shell package - # plus the 4 per-platform packages — using the napi-rs v3 release flow, then + # plus the 5 per-platform packages — using the napi-rs v3 release flow, then # packs each to a .tgz and uploads them. Download the `npm-packages` artifact # to install/test the real tarballs locally before any publish happens. node-pack: @@ -351,8 +351,8 @@ jobs: for d in npm/*/; do (cd "$d" && npm pack --pack-destination "$GITHUB_WORKSPACE/dist-npm"); done - name: Verify the packaged set matches the publish matrix - # Guard against drift: the publish stage uses a static matrix of the 4 - # platform packages (+ the main package = 5 total). If package.json's + # Guard against drift: the publish stage uses a static matrix of the 5 + # platform packages (+ the main package = 6 total). If package.json's # napi.targets gains/loses a target, the packed count changes here and # this fails the run BEFORE anything is published — prompting whoever # changed the targets to update node-publish-platform's matrix to match. @@ -379,7 +379,7 @@ jobs: name: npm-packages path: dist-npm/*.tgz - # ── Publish the 4 per-platform packages (one independently retriable job each) + # ── Publish the 5 per-platform packages (one independently retriable job each) # Each leg publishes exactly one tarball that node-inspect packed + uploaded, # so a flaky single platform can be re-run on its own via "Re-run failed jobs" # without touching the others. The matrix is static — its entries must match diff --git a/tests/shell_integration.rs b/tests/shell_integration.rs index b2c5302..2547018 100644 --- a/tests/shell_integration.rs +++ b/tests/shell_integration.rs @@ -5352,7 +5352,7 @@ umask = "077" [[bind]] mode = "copy" -source = "{}" +source = '{}' destination = "/workspace" readonly = true