From 468e94dc551a5268e324244f7cc3f571f67ed178 Mon Sep 17 00:00:00 2001 From: Samuel Carson Date: Sun, 3 May 2026 15:46:04 -0500 Subject: [PATCH] fix(browse): bash.exe wrap for telemetry on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reportAttemptTelemetry() in browse/src/security.ts calls spawn(bin, args) where bin is the gstack-telemetry-log bash script. On Windows this fails silently with ENOENT — CreateProcess can't dispatch on shebang lines. Adopts v1.24.0.0's Bun.which + GSTACK_*_BIN override pattern (from browse/src/claude-bin.ts:resolveClaudeCommand, introduced in #1252) for resolving bash.exe. resolveBashBinary() honors GSTACK_BASH_BIN absolute-path or PATH-resolvable override, falling back to Bun.which('bash') which finds Git Bash on the standard Windows install. buildTelemetrySpawnCommand() wraps the script invocation on win32 only; POSIX path is bit-identical. Returns null when bash can't be resolved on Windows so caller skips spawn — local attempts.jsonl audit trail keeps working without surfacing a Windows-only failure. 8 new unit tests cover resolveBashBinary (POSIX bash, absolute override, quote-stripping, BASH_BIN fallback, empty-PATH null) and buildTelemetrySpawnCommand (POSIX pass-through, win32 bash wrap, win32 null on unresolvable, arg-array immutability). POSIX path is bit-identical — Bun.which('bash') on Linux/macOS returns the same /bin/bash or /usr/bin/bash that the old hardcoded spawn relied on. --- browse/src/security.ts | 61 ++++++++++++++++++++++++++++- browse/test/security.test.ts | 76 ++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/browse/src/security.ts b/browse/src/security.ts index 22009e0c36..056d6f7df1 100644 --- a/browse/src/security.ts +++ b/browse/src/security.ts @@ -413,6 +413,61 @@ function findTelemetryBinary(): string | null { return null; } +/** + * Resolve a bash binary for invoking shebang scripts on Windows. Mirrors the + * GSTACK_*_BIN override pattern from `browse/src/claude-bin.ts:resolveClaudeCommand` + * (introduced in v1.24.0.0 #1252) so users on WSL/MSYS2/non-default Git Bash + * installs can redirect. + * + * Override precedence: + * 1. GSTACK_BASH_BIN (or BASH_BIN) — absolute path or PATH-resolvable command. + * 2. Plain Bun.which('bash') — finds Git Bash on the standard Windows install. + * + * Returns null if nothing resolves; callers must degrade gracefully (telemetry + * already swallows spawn errors, so a null here means the local attempts.jsonl + * audit trail keeps working without surfacing a Windows-only failure). + */ +export function resolveBashBinary(env: NodeJS.ProcessEnv = process.env): string | null { + const PATH = env.PATH ?? env.Path ?? ''; + const override = (env.GSTACK_BASH_BIN ?? env.BASH_BIN)?.trim(); + if (override) { + const trimmed = override.replace(/^"(.*)"$/, '$1'); + return path.isAbsolute(trimmed) ? trimmed : (Bun.which(trimmed, { PATH }) ?? null); + } + return Bun.which('bash', { PATH }) ?? null; +} + +/** + * Build the [cmd, args] tuple for invoking a bash-script telemetry binary + * in a way that works on both POSIX and Windows. + * + * POSIX: returns [bin, args] unchanged — shebang gets honored by execve. + * Win32: wraps in bash explicitly. `gstack-telemetry-log` is a shell script + * (`#!/usr/bin/env bash`) and Windows `CreateProcess` can't dispatch on a + * shebang — it tries to load the file as a PE image, fails with ENOEXEC, + * and our 'error' handler silently swallows it. Resolves bash via the same + * Bun.which + GSTACK_*_BIN override pattern as claude-bin.ts. + * + * Returns null when bash can't be resolved on Windows (rare — Git Bash ships + * with the standard gstack install path). Caller skips spawn; the local + * attempts.jsonl write still gives the audit trail. + * + * Exported for testability — resolution is a pure function of (platform, + * env, bin, args) so we can assert on it without actually spawning. + */ +export function buildTelemetrySpawnCommand( + bin: string, + args: string[], + env: NodeJS.ProcessEnv = process.env, +): { cmd: string; cmdArgs: string[] } | null { + if (process.platform === 'win32') { + const bashPath = resolveBashBinary(env); + if (!bashPath) return null; + return { cmd: bashPath, cmdArgs: [bin, ...args] }; + } + return { cmd: bin, cmdArgs: args }; +} + /** * Fire-and-forget subprocess invocation of gstack-telemetry-log with the * attack_attempt event type. The binary handles tier gating internally @@ -426,14 +481,16 @@ function reportAttemptTelemetry(record: AttemptRecord): void { const bin = findTelemetryBinary(); if (!bin) return; try { - const child = spawn(bin, [ + const result = buildTelemetrySpawnCommand(bin, [ '--event-type', 'attack_attempt', '--url-domain', record.urlDomain || '', '--payload-hash', record.payloadHash, '--confidence', String(record.confidence), '--layer', record.layer, '--verdict', record.verdict, - ], { + ]); + if (!result) return; + const child = spawn(result.cmd, result.cmdArgs, { stdio: 'ignore', detached: true, }); diff --git a/browse/test/security.test.ts b/browse/test/security.test.ts index 43888cd3aa..a452b435e2 100644 --- a/browse/test/security.test.ts +++ b/browse/test/security.test.ts @@ -20,6 +20,8 @@ import { readSessionState, getStatus, extractDomain, + buildTelemetrySpawnCommand, + resolveBashBinary, type LayerSignal, } from '../src/security'; @@ -325,3 +327,77 @@ describe('extractDomain', () => { expect(extractDomain('')).toBe(''); }); }); + +// ─── Bash binary resolution (Windows shebang-script invocation) ───── + +describe('resolveBashBinary', () => { + test('on POSIX, returns the system bash via Bun.which', () => { + if (process.platform === 'win32') return; + const out = resolveBashBinary({ PATH: process.env.PATH ?? '' }); + expect(out).toBeTruthy(); + expect(out!.endsWith('bash')).toBe(true); + }); + + test('honors GSTACK_BASH_BIN absolute-path override', () => { + // Construct a synthetic absolute path; the helper short-circuits on + // path.isAbsolute and never touches the filesystem, so this is portable. + const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; + const out = resolveBashBinary({ GSTACK_BASH_BIN: fake, PATH: '' }); + expect(out).toBe(fake); + }); + + test('strips wrapping double quotes from override values', () => { + const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; + const out = resolveBashBinary({ GSTACK_BASH_BIN: `"${fake}"`, PATH: '' }); + expect(out).toBe(fake); + }); + + test('BASH_BIN works as a fallback when GSTACK_BASH_BIN is unset', () => { + const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; + const out = resolveBashBinary({ BASH_BIN: fake, PATH: '' }); + expect(out).toBe(fake); + }); + + test('returns null when nothing resolves (override is unset and PATH is empty)', () => { + // Empty PATH means Bun.which finds nothing. + const out = resolveBashBinary({ PATH: '' }); + expect(out).toBeNull(); + }); +}); + +// ─── Telemetry spawn command (Windows bash wrapper, v1.24-aligned) ── + +describe('buildTelemetrySpawnCommand', () => { + const bin = '/home/user/.claude/skills/gstack/bin/gstack-telemetry-log'; + const args = ['--event-type', 'attack_attempt', '--confidence', '0.95']; + + test('on POSIX, returns the binary path and args unchanged', () => { + if (process.platform === 'win32') return; + const out = buildTelemetrySpawnCommand(bin, args); + expect(out).not.toBeNull(); + expect(out!.cmd).toBe(bin); + expect(out!.cmdArgs).toEqual(args); + }); + + test('on win32 with bash resolvable, wraps the call in bash with the script as first arg', () => { + if (process.platform !== 'win32') return; + const fakeBash = 'C:\\Program Files\\Git\\bin\\bash.exe'; + const out = buildTelemetrySpawnCommand(bin, args, { GSTACK_BASH_BIN: fakeBash, PATH: '' }); + expect(out).not.toBeNull(); + expect(out!.cmd).toBe(fakeBash); + expect(out!.cmdArgs).toEqual([bin, ...args]); + }); + + test('on win32 with bash unresolvable, returns null so caller skips spawn', () => { + if (process.platform !== 'win32') return; + // No override, empty PATH — Bun.which finds nothing on Windows. + const out = buildTelemetrySpawnCommand(bin, args, { PATH: '' }); + expect(out).toBeNull(); + }); + + test('does not mutate the caller-supplied args array', () => { + const originalArgs = [...args]; + buildTelemetrySpawnCommand(bin, args); + expect(args).toEqual(originalArgs); + }); +});