Skip to content

fix(tools): spawn npm.cmd through a shell on Windows - #3923

Open
rasadregmi wants to merge 1 commit into
HeyPuter:mainfrom
rasadregmi:fix/win32-npm-spawn-einval
Open

rasadregmi wants to merge 1 commit into
HeyPuter:mainfrom
rasadregmi:fix/win32-npm-spawn-einval

Conversation

@rasadregmi

Copy link
Copy Markdown
Contributor

npm start fails on Windows 11 with spawn EINVAL on current Node versions, so the backend never boots. tools/start.mjs and tools/extensionSetup.mjs resolve npm to the npm.cmd batch shim on win32 and spawn it without a shell; Node >= 18.20 refuses to execute .cmd shims without one (CVE-2024-27980 hardening) and throws EINVAL instead of running it.

This is the third appearance of the same bug #294 and #1748 both reported it, #2269 fixed it, and the win32 handling was lost when the 26.07 restructure replaced Kernel.js / DevWatcherService.js with tools/.

Adds tools/npmSpawn.mjs with platform-aware npmCommand() and npmSpawnOptions() (merging shell: true on win32 only) and uses it in both scripts, so the shell option only ever reaches Windows spawns. Covers it with a regression test registered in the backend vitest suite. POSIX behavior is unchanged.

Testing:

  • npx vitest run --config src/backend/vitest.config.ts tools/npmSpawn.test.mjs 6/6 passed; the same run fails when the shell option is removed, confirming the test catches the regression.
  • npx vitest run --config src/backend/vitest.config.ts tools/npmSpawn.test.mjs extensions/devWatcher.test.ts — 22/22 passed.
  • npm run typecheck — no new errors.
  • node ./tools/extensionSetup.mjs — exit 0.
  • npm start (Linux) — reaches "PuterServer has fully booted."

Closes #3911

@rasadregmi

rasadregmi commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

CI failure is an upstream flake, not caused by this change, please re-run the failed job.

The only failing tests are in src/backend/services/share/ShareService.trash.test.ts (ShareService trash + a couple of ShareService trash tests), failing inside FSService.move -> #assertNotUserRoot with Cannot move a home directory (a guard added upstream in HeyPuter/puter#3920). My change only touches the npm-spawn helpers in tools/, a mocked-spawn unit test, and a vitest include glob — there is no shared code path with ShareService.trash tests, and those tests are untouched by this PR.

For reference, HeyPuter main (commit 2690f49, which this branch is based on) and HeyPuter's own merged PRs #3913/#3914/#3915 all passed this exact backend suite, so the suite is green upstream. The failure here stacks through the ShareService trash suite, which upstream has flagged as flaky (#3908, #3910, #3922 touched the same area).

Please re-run the failed test (pr) / coverage job when you get a chance, it is almost certainly a timing-dependent trout in the ShareService suite rather than anything in this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

npm start fails on Windows with spawn EINVAL — regression of #1748 after the tools/ restructure

1 participant