mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
c90081176d
* fix(#2598): pass shell: true to npm spawnSync on Windows Since Node's CVE-2024-27980 fix (>= 18.20.2 / >= 20.12.2 / >= 21.7.3), spawnSync refuses to launch .cmd/.bat files on Windows without `shell: true`. installSdkIfNeeded picks npmCmd='npm.cmd' on win32 and then calls spawnSync five times — every one returns { status: null, error: EINVAL } before npm ever runs. The installer checks `status !== 0`, trips the failure path, and emits a bare "Failed to `npm install` in sdk/." with zero diagnostic output because `stdio: 'inherit'` never had a child to stream. Every fresh install on Windows has failed at the SDK build step on any supported Node version for the life of the post-CVE bin/install.js. Introduce a local `spawnNpm(args, opts)` helper inside installSdkIfNeeded that injects `shell: process.platform === 'win32'` when the caller doesn't override it. Route all five npm invocations through it: `npm install`, `npm run build`, `npm install -g .`, and both `npm config get prefix` calls. Adds a static regression test that parses installSdkIfNeeded and asserts no bare `spawnSync(npmCmd, ...)` remains, a shell-aware wrapper exists, and at least five invocations go through it. Closes #2598 * fix(#2598): surface spawnSync diagnostics in SDK install fatal paths Thread result.error / result.signal / result.status into emitSdkFatal for the three npm failure branches (install, run build, install -g .) via a formatSpawnFailure helper. The root cause of #2598 went silent precisely because `{ status: null, error: EINVAL }` was reduced to a generic "Failed to `npm install` in sdk/." with no diagnostic — stdio: 'inherit' had no child process to stream and result.error was swallowed. Any future regression in the same area (EINVAL, ENOENT, signal termination) now prints its real cause in the red fatal banner. Also strengthen the regression test so it cannot pass with only four real npm call sites: the previous `spawnSync(npmCmd, ..., shell)` regex double-counted the spawnNpm helper's own body when a helper existed. Separate arrow-form vs function-form helper detection and exclude the wrapper body from explicitShellNpm so the `>= 5` assertion reflects real invocations only. Add a new test that asserts all three fatal branches now reference formatSpawnFailure / result.error / signal / status. Addresses CodeRabbit review comments on PR #2600: - r3126987409 (bin/install.js): surface underlying spawnSync failure - r3126987419 (test): explicitShellNpm overcounts by one via helper def