mirror of https://github.com/garrytan/gstack.git
fix(browse): bash.exe wrap for telemetry on Windows
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.
This commit is contained in:
parent
bf65487162
commit
468e94dc55
|
|
@ -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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue