From 3b8a1cbd304e38f129051f7276efbc4989f32a24 Mon Sep 17 00:00:00 2001 From: SecFathy Date: Thu, 11 Jun 2026 09:07:47 +0300 Subject: [PATCH 1/2] fix: run shell tools via PowerShell on Windows instead of /bin/sh The shell and bash tools hardcoded /bin/sh and /bin/bash, which don't exist on Windows, so every command failed with `ENOENT: no such file or directory, uv_spawn '/bin/sh'` (issue #11). On Windows now: - Spawn commands through PowerShell (-NoProfile -NonInteractive -Command), overridable via PFLOW_WINDOWS_SHELL. - Skip the macOS/BSD portability rewrite and GNU-flag guards (they target Unix tools and would corrupt or wrongly block PowerShell commands). - Tear down the child tree with `taskkill /T /F` and leave the process attached, since POSIX process groups/signals don't apply. - Surface PowerShell-appropriate tool descriptions/schema. POSIX behavior is unchanged. All 596 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tools/shell.ts | 65 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/src/tools/shell.ts b/src/tools/shell.ts index b487550..37eceb7 100644 --- a/src/tools/shell.ts +++ b/src/tools/shell.ts @@ -11,6 +11,25 @@ const DEFAULT_TIMEOUT_MS = 5 * 60 * 1000; const MAX_TIMEOUT_MS = 30 * 60 * 1000; const MAX_OUTPUT_BYTES = 32 * 1024; +// Windows has no /bin/sh or /bin/bash, so spawning the Unix shell path fails +// with `ENOENT: no such file or directory, uv_spawn '/bin/sh'`. On Windows we +// run commands through PowerShell instead — it is closer to the Unix idioms the +// tool descriptions assume than cmd.exe, and supports pipelines/quoting cleanly. +// Override with PFLOW_WINDOWS_SHELL (e.g. "pwsh.exe" or "%ComSpec%") if needed. +const IS_WINDOWS = process.platform === 'win32'; + +/** + * Build the spawn target for a command string on the current platform. + * POSIX: ` -c ""`. Windows: PowerShell `-Command`. + */ +function shellInvocation(unixShell: string, command: string): { cmd: string; argv: string[] } { + if (IS_WINDOWS) { + const shell = process.env.PFLOW_WINDOWS_SHELL || 'powershell.exe'; + return { cmd: shell, argv: ['-NoProfile', '-NonInteractive', '-Command', command] }; + } + return { cmd: unixShell, argv: ['-c', command] }; +} + /** * Advisory denylist for catastrophic commands. This is defense-in-depth * behind the per-command permission prompt, NOT a security boundary — a @@ -99,6 +118,14 @@ export class ShellTool implements Tool { } description(): string { + if (IS_WINDOWS) { + return [ + 'Run a shell command via PowerShell on the local machine. Primary use case is curl/Invoke-WebRequest plus standard utilities for HTTP testing, file inspection, and one-liners. The user will be prompted to approve each command. Capture concise output — pipe through `Select-Object -First` for huge outputs. Do not run interactive commands. Authorized engagements only.', + 'Write PowerShell-compatible commands. Unix-only tools (grep, sed, awk, jq) may be absent; prefer PowerShell equivalents (Select-String, -replace, ConvertFrom-Json) unless you know the tool is installed.', + '', + 'Default to curl/Invoke-WebRequest for HTTP work; only use specialized scanners (ffuf, nuclei, sqlmap, etc.) when the user explicitly asks for them.', + ].join('\n'); + } return [ 'Run a shell command via /bin/sh -c on the local machine. Primary use case is curl + standard Unix utilities (jq, grep, awk, sed, head, sort, uniq) for HTTP testing, file inspection, and bash one-liners. The user will be prompted to approve each command. Capture concise output — pipe through `head` for huge outputs. Do not run interactive commands. Authorized engagements only.', 'Write portable macOS/BSD + Linux commands. Avoid GNU-only flags such as `grep -P`; prefer `grep -E`, `awk`, `sed`, `perl -ne`, or `jq` for extraction.', @@ -113,7 +140,9 @@ export class ShellTool implements Tool { properties: { command: { type: 'string', - description: 'Shell command to execute. Will run via /bin/sh -c.', + description: IS_WINDOWS + ? 'Shell command to execute. Will run via PowerShell -Command.' + : 'Shell command to execute. Will run via /bin/sh -c.', }, timeout_seconds: { type: 'integer', @@ -153,9 +182,13 @@ export class ShellTool implements Tool { throw new Error(`command blocked by denylist (matched ${re.source})`); } } - for (const { re, message } of PORTABILITY_PATTERNS) { - if (re.test(cmdStr)) { - throw new Error(`command blocked for portability: ${message}`); + // The portability guards steer the model away from GNU-only Unix flags; + // they are irrelevant (and their messages misleading) under PowerShell. + if (!IS_WINDOWS) { + for (const { re, message } of PORTABILITY_PATTERNS) { + if (re.test(cmdStr)) { + throw new Error(`command blocked for portability: ${message}`); + } } } @@ -165,11 +198,15 @@ export class ShellTool implements Tool { timeoutMs = Math.min(timeoutArg * 1000, MAX_TIMEOUT_MS); } - return runWithCapture(this.shellPath, ['-c', cmdStr], timeoutMs, signal); + const { cmd, argv } = shellInvocation(this.shellPath, cmdStr); + return runWithCapture(cmd, argv, timeoutMs, signal); } } export function rewritePortableCommand(command: string): string { + // The macOS/BSD portability rewrite targets Unix tools (perl, grep). Under + // PowerShell on Windows it would corrupt commands, so pass them through. + if (IS_WINDOWS) return command; return command.replace( GREP_P_RE, (match, sep: string, lead: string, rawFlags: string, rawPattern: string, rest: string) => { @@ -238,6 +275,9 @@ export class BashTool extends ShellTool { } override description(): string { + if (IS_WINDOWS) { + return 'Run a command via PowerShell on the local machine (no /bin/bash on Windows; this falls back to the same PowerShell host as the shell tool). Same gating as the shell tool (per-command permission, denylist, output truncation).'; + } return "Run a bash command via /bin/bash -c on the local machine. Same gating as the shell tool (per-command permission, denylist, output truncation). Prefer this over `shell` when you need bash features like [[ ]] tests, process substitution <(...), arrays, or $'...' quoting."; } } @@ -266,7 +306,10 @@ function runWithCapture( let timedOut = false; timer.unref?.(); - const child = spawn(cmd, argv, { detached: true, signal: controller.signal }); + // detached lets us kill the whole process group via negative PID on POSIX. + // On Windows it would spawn a new console window and the group-kill model + // differs, so we leave it attached and rely on taskkill /T below. + const child = spawn(cmd, argv, { detached: !IS_WINDOWS, signal: controller.signal }); childPid = child.pid ?? 0; const stdoutChunks: Buffer[] = []; const stderrChunks: Buffer[] = []; @@ -330,6 +373,16 @@ function runWithCapture( function killProcessGroup(pid: number): void { if (!pid) return; + if (IS_WINDOWS) { + // No POSIX signals/process groups on Windows; taskkill /T tears down the + // PowerShell process and its child command tree. + try { + spawn('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + } catch { + /* best effort */ + } + return; + } try { process.kill(-pid, 'SIGTERM'); } catch { From 684427b8c5e35cb137ec2fb2da5d5da13e68daef Mon Sep 17 00:00:00 2001 From: SecFathy Date: Thu, 11 Jun 2026 09:11:35 +0300 Subject: [PATCH 2/2] test: cover Windows PowerShell shell invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the platform check call-time (isWindows()) and export shellInvocation so the Windows code paths are testable from a POSIX CI host. Adds tests that the shell/bash tools target PowerShell (not /bin/sh) on win32, honor PFLOW_WINDOWS_SHELL, skip the Unix grep -P rewrite, and surface PowerShell guidance — guarding against a regression of issue #11. 602 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tools/shell.test.ts | 74 +++++++++++++++++++++++++++++++++++++++-- src/tools/shell.ts | 26 +++++++++------ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/tools/shell.test.ts b/src/tools/shell.test.ts index da469ae..2b4c17a 100644 --- a/src/tools/shell.test.ts +++ b/src/tools/shell.test.ts @@ -1,8 +1,27 @@ // Shell denylist + execution tests. -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest'; import { AlwaysAllow } from '../permission/permission.js'; -import { BashTool, DENY_PATTERNS, ShellTool, rewritePortableCommand } from './shell.js'; +import { + BashTool, + DENY_PATTERNS, + ShellTool, + rewritePortableCommand, + shellInvocation, +} from './shell.js'; + +// Run a callback with process.platform forced to `platform`, then restore it. +// isWindows() reads process.platform at call time, so this exercises the +// Windows code paths from a macOS/Linux CI host without spawning anything. +function withPlatform(platform: NodeJS.Platform, fn: () => T): T { + const original = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { value: platform, configurable: true }); + try { + return fn(); + } finally { + if (original) Object.defineProperty(process, 'platform', original); + } +} describe('shell denylist', () => { const cases: Array<{ name: string; cmd: string; shouldBlock: boolean }> = [ @@ -206,3 +225,54 @@ describe('BashTool', () => { expect(out).toContain('bash-ok'); }); }); + +describe('Windows shell invocation (issue #11)', () => { + afterEach(() => { + process.env.PFLOW_WINDOWS_SHELL = undefined; + // biome-ignore lint/performance/noDelete: restore env to a clean state + delete process.env.PFLOW_WINDOWS_SHELL; + }); + + it('spawns /bin/sh -c on POSIX', () => { + const inv = withPlatform('linux', () => shellInvocation('/bin/sh', 'echo hi')); + expect(inv).toEqual({ cmd: '/bin/sh', argv: ['-c', 'echo hi'] }); + }); + + it('spawns PowerShell -Command on Windows instead of /bin/sh', () => { + const inv = withPlatform('win32', () => shellInvocation('/bin/sh', 'echo hi')); + expect(inv.cmd).toBe('powershell.exe'); + expect(inv.argv).toEqual(['-NoProfile', '-NonInteractive', '-Command', 'echo hi']); + // The /bin/sh path that triggered uv_spawn ENOENT must not be the target. + expect(inv.cmd).not.toBe('/bin/sh'); + expect(inv.argv).not.toContain('-c'); + }); + + it('routes BashTool through PowerShell on Windows too (no /bin/bash)', () => { + const inv = withPlatform('win32', () => shellInvocation('/bin/bash', 'Get-ChildItem')); + expect(inv.cmd).toBe('powershell.exe'); + expect(inv.cmd).not.toBe('/bin/bash'); + }); + + it('honors PFLOW_WINDOWS_SHELL override on Windows', () => { + process.env.PFLOW_WINDOWS_SHELL = 'pwsh.exe'; + const inv = withPlatform('win32', () => shellInvocation('/bin/sh', 'echo hi')); + expect(inv.cmd).toBe('pwsh.exe'); + }); + + it('skips the Unix grep -P → perl rewrite on Windows', () => { + const cmd = "type x | grep -P 'a'"; + expect(withPlatform('win32', () => rewritePortableCommand(cmd))).toBe(cmd); + // Sanity: on POSIX the same input *is* rewritten, proving the guard matters. + expect(withPlatform('linux', () => rewritePortableCommand(cmd))).toContain('perl -ne'); + }); + + it('surfaces PowerShell-appropriate tool guidance on Windows', () => { + const desc = withPlatform('win32', () => new ShellTool().description()); + expect(desc).toContain('PowerShell'); + expect(desc).not.toContain('/bin/sh'); + const schema = withPlatform('win32', () => new ShellTool().schema()) as { + properties: { command: { description: string } }; + }; + expect(schema.properties.command.description).toContain('PowerShell'); + }); +}); diff --git a/src/tools/shell.ts b/src/tools/shell.ts index 37eceb7..cc27308 100644 --- a/src/tools/shell.ts +++ b/src/tools/shell.ts @@ -16,14 +16,20 @@ const MAX_OUTPUT_BYTES = 32 * 1024; // run commands through PowerShell instead — it is closer to the Unix idioms the // tool descriptions assume than cmd.exe, and supports pipelines/quoting cleanly. // Override with PFLOW_WINDOWS_SHELL (e.g. "pwsh.exe" or "%ComSpec%") if needed. -const IS_WINDOWS = process.platform === 'win32'; +// Evaluated at call time (not module load) so tests can exercise both paths. +function isWindows(): boolean { + return process.platform === 'win32'; +} /** * Build the spawn target for a command string on the current platform. * POSIX: ` -c ""`. Windows: PowerShell `-Command`. */ -function shellInvocation(unixShell: string, command: string): { cmd: string; argv: string[] } { - if (IS_WINDOWS) { +export function shellInvocation( + unixShell: string, + command: string, +): { cmd: string; argv: string[] } { + if (isWindows()) { const shell = process.env.PFLOW_WINDOWS_SHELL || 'powershell.exe'; return { cmd: shell, argv: ['-NoProfile', '-NonInteractive', '-Command', command] }; } @@ -118,7 +124,7 @@ export class ShellTool implements Tool { } description(): string { - if (IS_WINDOWS) { + if (isWindows()) { return [ 'Run a shell command via PowerShell on the local machine. Primary use case is curl/Invoke-WebRequest plus standard utilities for HTTP testing, file inspection, and one-liners. The user will be prompted to approve each command. Capture concise output — pipe through `Select-Object -First` for huge outputs. Do not run interactive commands. Authorized engagements only.', 'Write PowerShell-compatible commands. Unix-only tools (grep, sed, awk, jq) may be absent; prefer PowerShell equivalents (Select-String, -replace, ConvertFrom-Json) unless you know the tool is installed.', @@ -140,7 +146,7 @@ export class ShellTool implements Tool { properties: { command: { type: 'string', - description: IS_WINDOWS + description: isWindows() ? 'Shell command to execute. Will run via PowerShell -Command.' : 'Shell command to execute. Will run via /bin/sh -c.', }, @@ -184,7 +190,7 @@ export class ShellTool implements Tool { } // The portability guards steer the model away from GNU-only Unix flags; // they are irrelevant (and their messages misleading) under PowerShell. - if (!IS_WINDOWS) { + if (!isWindows()) { for (const { re, message } of PORTABILITY_PATTERNS) { if (re.test(cmdStr)) { throw new Error(`command blocked for portability: ${message}`); @@ -206,7 +212,7 @@ export class ShellTool implements Tool { export function rewritePortableCommand(command: string): string { // The macOS/BSD portability rewrite targets Unix tools (perl, grep). Under // PowerShell on Windows it would corrupt commands, so pass them through. - if (IS_WINDOWS) return command; + if (isWindows()) return command; return command.replace( GREP_P_RE, (match, sep: string, lead: string, rawFlags: string, rawPattern: string, rest: string) => { @@ -275,7 +281,7 @@ export class BashTool extends ShellTool { } override description(): string { - if (IS_WINDOWS) { + if (isWindows()) { return 'Run a command via PowerShell on the local machine (no /bin/bash on Windows; this falls back to the same PowerShell host as the shell tool). Same gating as the shell tool (per-command permission, denylist, output truncation).'; } return "Run a bash command via /bin/bash -c on the local machine. Same gating as the shell tool (per-command permission, denylist, output truncation). Prefer this over `shell` when you need bash features like [[ ]] tests, process substitution <(...), arrays, or $'...' quoting."; @@ -309,7 +315,7 @@ function runWithCapture( // detached lets us kill the whole process group via negative PID on POSIX. // On Windows it would spawn a new console window and the group-kill model // differs, so we leave it attached and rely on taskkill /T below. - const child = spawn(cmd, argv, { detached: !IS_WINDOWS, signal: controller.signal }); + const child = spawn(cmd, argv, { detached: !isWindows(), signal: controller.signal }); childPid = child.pid ?? 0; const stdoutChunks: Buffer[] = []; const stderrChunks: Buffer[] = []; @@ -373,7 +379,7 @@ function runWithCapture( function killProcessGroup(pid: number): void { if (!pid) return; - if (IS_WINDOWS) { + if (isWindows()) { // No POSIX signals/process groups on Windows; taskkill /T tears down the // PowerShell process and its child command tree. try {