fix(tools): block MSYS and WSL shells under the Windows sandbox by euxaristia · Pull Request #587 · Gitlawb/zero

euxaristia · 2026-07-08T03:57:00Z

Git-for-Windows bash.exe and sh.exe are MSYS binaries: the MSYS runtime secures its signal pipe and shared-memory sections with explicit DACLs granting only the user, Administrators, and SYSTEM (sigproc.cc sigproc_init -> sec_user_nih), so under the WRITE_RESTRICTED token the second access check can never pass and startup dies with "couldn't create signal pipe" or "CreateFileMapping .1", Win32 error 5. The System32 WSL bash launcher fails equivalently: the restricted token cannot connect to the WSL service (Bash/Service/CreateInstance/ E_ACCESSDENIED).

Every executable a bare bash can resolve to fails, so block bash and sh preflight like the MSYS coreutils from #476, mention the indirect spawn paths (git hooks, git/gh credential helpers) in the shared suggestion, detect the WSL launcher's UTF-16LE service-denied stderr, and document the root cause in the runner's KNOWN LIMITATION comment beside the Schannel note.

Changes

internal/sandbox/windows_command_runner_windows.go: document MSYS/WSL root cause in KNOWN LIMITATION; detect WSL launcher's E_ACCESSDENIED stderr.
internal/tools/shell_runtime.go: block bash/sh preflight like MSYS coreutils; surface indirect spawn paths in shared suggestion.
internal/tools/shell_runtime_test.go: cover the new detection/blocking cases.

Summary by CodeRabbit

Bug Fixes
- Improved Windows shell sandbox detection to recognize additional MSYS/Git Bash failure modes, including indirect Bash spawns, and to detect WSL bash launcher service-connection denials from captured output (even with embedded/Windows-style rendering).
Documentation
- Expanded restricted-token “known limitation” guidance explaining when common MSYS/WSL components fail due to DACL/SID interactions.
Tests
- Added/updated Windows shell runtime and output-detection test coverage for bare bash/sh cases and WSL-denied output shapes, plus a small TUI picker test adjustment.

Git-for-Windows bash.exe and sh.exe are MSYS binaries: the MSYS runtime secures its signal pipe and shared-memory sections with explicit DACLs granting only the user, Administrators, and SYSTEM (sigproc.cc sigproc_init -> sec_user_nih), so under the WRITE_RESTRICTED token the second access check can never pass and startup dies with "couldn't create signal pipe" or "CreateFileMapping <SID>.1", Win32 error 5. The System32 WSL bash launcher fails equivalently: the restricted token cannot connect to the WSL service (Bash/Service/CreateInstance/ E_ACCESSDENIED). Every executable a bare bash can resolve to fails, so block bash and sh preflight like the MSYS coreutils from Gitlawb#476, mention the indirect spawn paths (git hooks, git/gh credential helpers) in the shared suggestion, detect the WSL launcher's UTF-16LE service-denied stderr, and document the root cause in the runner's KNOWN LIMITATION comment beside the Schannel note.

coderabbitai · 2026-07-08T04:00:23Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b559fd5d-8af3-41fd-b335-c72a18173bb2

📥 Commits

Reviewing files that changed from the base of the PR and between d1c4a1f and ba23479.

📒 Files selected for processing (1)

internal/tools/shell_runtime_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

internal/tools/shell_runtime_test.go

Walkthrough

This PR expands Windows sandbox restricted-token guidance and shell-runtime detection for MSYS2/Cygwin and WSL bash launcher failures, adds tests for the new detection paths, and updates one picker test for an expanded return signature.

Changes

Windows Shell Sandbox Detection

Layer / File(s)	Summary
Restricted-token limitation documentation `internal/sandbox/windows_command_runner_windows.go`	Documents MSYS2/Cygwin and WSL bash launcher failure modes under the restricted token, noting no in-token fix exists.
MSYS/WSL detection logic and suggestion text `internal/tools/shell_runtime.go`	Expands `windowsMsysSandboxSuggestion` text, adds a WSL service-denied output check in `detectShellOutputIssue`, and introduces `wslServiceDeniedInOutput` to match WSL failure markers after stripping NUL bytes.
Detection test coverage `internal/tools/shell_runtime_test.go`	Adds tests flagging bash/sh executable invocations as `windows_msys_sandbox`, verifies WSL service-denied detection against UTF-16LE-like stderr, and tightens `MsysProneCommandName("bash")` expectations.

Picker Test Signature Update

Layer / File(s)	Summary
SwitchProviderModel test signature alignment `internal/tui/picker_test.go`	Updates the `switchProviderModel` test call to ignore an additional return value while preserving the existing assertions.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

Gitlawb/zero#476: Both PRs extend Windows shell-runtime sandbox detection in internal/tools/shell_runtime.go/internal/tools/shell_runtime_test.go, adding more output-based triggers that map to windows_msys_sandbox.
Gitlawb/zero#543: Both PRs modify internal/tools/shell_runtime.go’s Windows/MSYS sandbox guidance text.

Suggested reviewers: Vasanthdev2004, gnanam1990, kevincodex1, jatmn

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: blocking MSYS and WSL shells in the Windows sandbox.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

jatmn

please rebase on main and fix smoke issues.

euxaristia · 2026-07-08T05:18:04Z

Follow-up to #587 (review). Rebased on main and the smoke failures are fixed. @jatmn The original CHANGES_REQUESTED was driven by CI failing at internal/tui/picker_test.go:771. m.switchProviderModel gained a 4th return value and the test only captured 3, so go test/go vet aborted the whole internal/tui build. The actual PR change (MSYS/WSL shell blocking in internal/tools/shell_runtime.go) was fine. The break was a pre-existing caller that hadn't been updated. Fixed in d1c4a1f by aligning picker_test to the 4-value signature, on top of 45261a2 (the sandbox blocking work). Branch is rebased on upstream/main and all smoke/CI checks are green. Please mark resolved when you re-review.

gnanam1990 · 2026-07-08T05:55:29Z

Review: PR #587

Local run

Merge: clean | gofmt: clean | go vet: clean | go build: clean
go test -race ./internal/tools/...: PASS
go test -race ./internal/tui/...: PASS
./internal/sandbox/... FAIL: pre-existing on main, not from this PR
Fixes pre-existing compile error: picker_test.go 3-vs-4 return value mismatch

Blockers

None.

Suggestions (non-blocking)

Blocking message says "POSIX coreutil" for bash/sh — a shell-specific message would be clearer.
WSL detection reuses windows_msys_sandbox kind — separate kind would help telemetry later.

Security

Strengthens the sandbox. No bypass, no secret exposure. All changes Windows-only, Linux/macOS unaffected.

Tests

Good coverage. Minor gap: no dedicated sh.exe test.

gnanam1990

Reviewed locally: gofmt, go vet, go build, and affected tests all pass. Clean merge with main. Fixes a pre-existing compile error in picker_test.go. Changes are Windows-only, well-tested, and strengthen the sandbox. No blockers.

Vasanthdev2004

I reviewed the MSYS/WSL block. Failing closed when the detected shell is MSYS or a WSL shell-distribution is the right call — those shells break the sandbox's path/argv assumptions, and a silent passthrough would be worse than a clear refusal. The detection keys on the right environment markers and the error tells the user how to point at a native shell.

One nit before this merges: the diff carries a hunk in picker_test.go that updates the switchProviderModel call site from a 3-value to a 4-value return. That same fix is already pending on main as PR #589 (a recent merge added the bool return but missed that call site). Drop that hunk here — keeping it will collide with #589 once it lands, and it's redundant once main is fixed.

Heads up: main has that unrelated test-build break right now (PR #589 is the fix, queued for merge), so Smoke is red here too until that lands — not from this change. Approving once the picker_test hunk is dropped.

euxaristia · 2026-07-08T07:10:40Z

@gnanam1990 thanks for the review. Added the dedicated sh.exe case to TestDetectShellCommandIssueFlagsShells in ba23479, closing the test gap you noted. Leaving the two non-blocking suggestions (shell-specific blocking message, separate WSL detection kind) for a follow-up so this stays small.

kevincodex1

LGTM

coderabbitai Bot approved these changes Jul 8, 2026

View reviewed changes

fix(tui): align picker_test switchProviderModel call with 4-value return

d1c4a1f

jatmn requested changes Jul 8, 2026

View reviewed changes

gnanam1990 approved these changes Jul 8, 2026

View reviewed changes

euxaristia mentioned this pull request Jul 8, 2026

test(config): make Resolve local-control tests env-independent #585

Merged

Vasanthdev2004 approved these changes Jul 8, 2026

View reviewed changes

test(tools): cover sh.exe in Windows shell blocking test

ba23479

Vasanthdev2004 mentioned this pull request Jul 8, 2026

Windows: bash tool with piped POSIX commands (e.g. | cat) causes "Not enough memory resources are available to process this command" #467

Closed

kevincodex1 approved these changes Jul 8, 2026

View reviewed changes

kevincodex1 merged commit 0666818 into Gitlawb:main Jul 8, 2026
7 checks passed

github-actions Bot mentioned this pull request Jul 8, 2026

chore(main): release 0.3.0 #537

Merged

BrewTestBot mentioned this pull request Jul 9, 2026

zero 0.3.0 Homebrew/homebrew-core#292199

Merged

euxaristia deleted the pr/windows-sandbox-block-msys-shells branch July 17, 2026 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tools): block MSYS and WSL shells under the Windows sandbox - #587

fix(tools): block MSYS and WSL shells under the Windows sandbox#587
kevincodex1 merged 3 commits into
Gitlawb:mainfrom
euxaristia:pr/windows-sandbox-block-msys-shells

euxaristia commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

jatmn left a comment

Uh oh!

euxaristia commented Jul 8, 2026 •

edited

Loading

Uh oh!

gnanam1990 commented Jul 8, 2026 •

edited

Loading

Uh oh!

gnanam1990 left a comment

Uh oh!

Vasanthdev2004 left a comment

Uh oh!

euxaristia commented Jul 8, 2026

Uh oh!

kevincodex1 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

euxaristia commented Jul 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

jatmn left a comment

Choose a reason for hiding this comment

Uh oh!

euxaristia commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnanam1990 commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review: PR #587

Local run

Blockers

Suggestions (non-blocking)

Security

Tests

Uh oh!

gnanam1990 left a comment

Choose a reason for hiding this comment

Uh oh!

Vasanthdev2004 left a comment

Choose a reason for hiding this comment

Uh oh!

euxaristia commented Jul 8, 2026

Uh oh!

kevincodex1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

euxaristia commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

euxaristia commented Jul 8, 2026 •

edited

Loading

gnanam1990 commented Jul 8, 2026 •

edited

Loading