#18143: fix(windows): wrap shell builtins with cmd.exe /c for proper execution
size: S
trusted-contributor
Cluster:
Windows Path and Exec Fixes
Fixes #17806
## What
Wrap Windows shell builtins (echo, dir, type, etc.) with `cmd.exe /c` when spawning processes, so they execute properly without requiring the unsafe `shell: true` flag.
## Why
The security hardening in v2026.2.15 (commit a7eb0dd9a) disabled `shell: true` to prevent command injection on Windows. However, this broke Windows shell builtins which are implemented by cmd.exe, not as separate executables.
**Before (broken):** `echo hello` → Node.js looks for `echo.exe` → not found → empty output
**After (fixed):** `echo hello` → wrapped to `cmd.exe /c echo hello` → works correctly
## How
Instead of re-enabling the unsafe `shell: true` flag, this fix:
1. Defines a set of 50+ known Windows shell builtins
2. Detects when a command is a builtin via `isWindowsShellBuiltin()`
3. Wraps only those commands with `cmd.exe /c` in the argv array via `wrapWindowsBuiltinCommand()`
4. Preserves security by NOT enabling `shell: true` globally
This is exactly the approach recommended in the original security comment:
> "If you need a shell, use an explicit shell-wrapper argv (e.g. `cmd.exe /c ...`)"
## Testing
- [x] Added 10 unit tests for `wrapWindowsBuiltinCommand()`
- [x] All 15 tests in `exec.test.ts` pass
- [x] `oxlint`: 0 warnings, 0 errors
- [x] `pnpm build` succeeds
## AI-Assisted 🤖
This PR was built with AI assistance (Claude via OpenClaw).
- [x] Code reviewed and understood
- [x] Tested via unit tests (Windows execution requires manual verification)
- [x] Session available on request
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR fixes Windows shell builtins (echo, dir, type, etc.) that broke after security hardening disabled `shell: true` in v2026.2.15. Instead of re-enabling the unsafe `shell: true` flag, it wraps known builtins with `cmd.exe /c` in the argv array — matching the recommended approach from the original security comment.
- Defines a set of 50+ known Windows shell builtins and wraps only those commands with `cmd.exe /c` when spawning on win32
- Preserves security by keeping `shell: true` disabled globally
- Only applies wrapping in `runCommandWithTimeout`; `runExec` is unaffected (its callers use standalone executables, not builtins)
- The interaction with `resolveCommand` is correct — `cmd.exe` already has a `.exe` extension and passes through unchanged
- 10 well-structured unit tests cover platform gating, case-insensitive matching, and passthrough of non-builtins
<h3>Confidence Score: 4/5</h3>
- This PR is safe to merge — it follows the security-recommended pattern and only touches the Windows code path with known builtins.
- Score of 4 reflects: correct security approach (wrapping known builtins instead of re-enabling shell: true), proper interaction with existing resolveCommand function, good test coverage, and no functional regressions for non-Windows platforms. Prior thread items (missing /d flag, comment wording) have already been addressed. The one remaining concern — the absence of the /d flag — was raised in a prior thread and is not a blocking issue but a hardening opportunity.
- No files require special attention beyond the prior review feedback on `/d` flag hardening in `src/process/exec.ts`.
<sub>Last reviewed commit: 0a73f42</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#9250: Fix: Add shell:true for Windows .cmd files to prevent spawn EINVAL ...
by vishaltandale00 · 2026-02-05
82.9%
#5168: Fix: force UTF-8 for Windows exec
by ManojINaik · 2026-01-31
79.3%
#11961: fix: exec tool wraps shebang scripts in heredoc to use correct inte...
by scott-memco · 2026-02-08
76.7%
#21215: Windows PowerShell reliability fixes (exec shim + Discord numeric p...
by musosoft · 2026-02-19
75.9%
#13873: fix(sandbox): prevent Windows PATH from poisoning docker exec
by alessandrorodi · 2026-02-11
75.3%
#8515: Lobster: disable Windows shell fallback
by coygeek · 2026-02-04
75.3%
#4295: fix: quote Windows paths with spaces in UI runner command
by Xieweikang123 · 2026-01-30
75.2%
#10708: fix: handle Windows PATH case-sensitivity in exec environment
by Yida-Dev · 2026-02-06
74.9%
#10714: fix: handle Windows PATH case-sensitivity in node register invoke
by Yida-Dev · 2026-02-06
74.8%
#16525: fix(shell): stop rejecting newlines in double-quoted args (#16470)
by yinghaosang · 2026-02-14
74.6%