You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Command run failed with error : spawn EINVAL #52554
This seems like a very significant breaking behavior change to make in a patch version? My team observed this same issue from a different tool which spawns npm.cmd to run scripts on Windows, and it seems like it would break any tool using this approach unless they already use { shell: true }.
Reacted by Felipe Escoto, Zhe Sun and Nick Gerleman
@ecraig12345, while the change maybe is a bit more than a patch, it has been released, so there's really nothing we can do about that specifically, but maybe we can make a sort-of note that this change occurred, so that users will understand how to continue using their projects, WDYT @RafaelGSS?
I know we have the blog posts, but TBH not many end users will read all the way through the blogs to figure out this one issue, as not everyone reads the security updates
I know we have the blog posts, but TBH not many end users will read all the way through the blogs to figure out this one issue, as not everyone reads the security updates
Maybe a notice in the documentation?
I think we can update the security release blog post to provide alternatives, then I can comment in the commit link to point to the blog post
This seems like a very significant breaking behavior change to make in a patch version? My team observed this same issue from a different tool which spawns npm.cmd to run scripts on Windows, and it seems like it would break any tool using this approach unless they already use { shell: true }.
According to our policy. We can only perform a breaking change in a semver-patch release if it's a security vulnerability is found. That was the case.
We also provide a mechanism to revert this behaviour such as --security-revert=CVE-2024-27980. Note, that this will disable the security patch, which is not recommended.
This is quite an invasisve change and because of #52017 I cannot propagate the --security-revert to usages of spawn in libraries and/or subprocesses. Is there any approach to making this work?
This is quite an invasisve change and because of #52017 I cannot propagate the --security-revert to usages of spawn in libraries and/or subprocesses. Is there any approach to making this work?
If you don't have control of the process to pass the --security-revert (on its child processes too), then there's no official workaround for that. The #52365 should solve it, but it hasn't been released yet. You can try monkey-patching .spawnSync/spawn to bind { shell: true } by default, but monkey-patching is not really a recommended option, it might work until new PRs arrises.
Please note, that this was a coordinated security release with other teams (Rust, PHP, ...) and per policy, we must fix it -- even if it breaks people, security is a must. We always attempt to create revert options (such as --security-revert, but sometimes it might not apply to all environments).
Activity
avivkeller commentedon Apr 16, 2024
Hi! This repo is mostly for issues regarding the NodeJS core, and we need further context/information to understand the problem.
If you have an issue with VSCode or Fiori, please report it to them.
Otherwise, please provide more context about the issue, such as stack traces and minimally reproducible examples.
Thank you!
reduckted commentedon Apr 16, 2024
This is a problem with
spawnandspawnSyncon Windows.Platform: Windows 10.0.19045 N/A Build 19045
Using Node.js v20.12.1:
Using Node.js 20.12.2:
cjihrig commentedon Apr 16, 2024
https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
reduckted commentedon Apr 16, 2024
Caused by 69ffc6d.
@bnoordhuis @RafaelGSS Now that spawning
.cmdfiles is blocked, what is the alternative? Leaving the.cmdextension off doesn't work.RafaelGSS commentedon Apr 16, 2024
Use
{ shell: true }when spawning if the input is sanitized. @bnoordhuis is there any other suggested way?reduckted commentedon Apr 16, 2024
Awesome, that solves the problem for me. Thanks for the quick response @RafaelGSS ❤️
avivkeller commentedon Apr 16, 2024
Thanks for the info! Sorry if I misunderstood the initial request!
ecraig12345 commentedon Apr 16, 2024
This seems like a very significant breaking behavior change to make in a patch version? My team observed this same issue from a different tool which spawns
npm.cmdto run scripts on Windows, and it seems like it would break any tool using this approach unless they already use{ shell: true }.avivkeller commentedon Apr 16, 2024
@ecraig12345, while the change maybe is a bit more than a patch, it has been released, so there's really nothing we can do about that specifically, but maybe we can make a sort-of note that this change occurred, so that users will understand how to continue using their projects, WDYT @RafaelGSS?
I know we have the blog posts, but TBH not many end users will read all the way through the blogs to figure out this one issue, as not everyone reads the security updates
Maybe a notice in the documentation?
Fix JS build on Windows
fix: spawn issue on windows relative to nodejs/node#52554
fix: spawn issue on windows relative to nodejs/node#52554
fix: spawn issue on windows relative to nodejs/node#52554
fix: spawn issue on windows relative to nodejs/node#52554
fix: spawn issue on windows relative to nodejs/node#52554
fix: spawn issue on windows relative to nodejs/node#52554
RafaelGSS commentedon Apr 17, 2024
I think we can update the security release blog post to provide alternatives, then I can comment in the commit link to point to the blog post
avivkeller commentedon Apr 17, 2024
Great!
RafaelGSS commentedon Apr 17, 2024
According to our policy. We can only perform a breaking change in a semver-patch release if it's a security vulnerability is found. That was the case.
We also provide a mechanism to revert this behaviour such as
--security-revert=CVE-2024-27980. Note, that this will disable the security patch, which is not recommended.fix: spawn issue on windows relative to nodejs/node#52554
Fix broken spawn on windows
fix(node): support for Node 20.12.2 on Windows
JoostK commentedon Apr 18, 2024
This is quite an invasisve change and because of #52017 I cannot propagate the
--security-revertto usages ofspawnin libraries and/or subprocesses. Is there any approach to making this work?RafaelGSS commentedon Apr 18, 2024
If you don't have control of the process to pass the
--security-revert(on its child processes too), then there's no official workaround for that. The #52365 should solve it, but it hasn't been released yet. You can try monkey-patching.spawnSync/spawnto bind{ shell: true }by default, but monkey-patching is not really a recommended option, it might work until new PRs arrises.Please note, that this was a coordinated security release with other teams (Rust, PHP, ...) and per policy, we must fix it -- even if it breaks people, security is a must. We always attempt to create revert options (such as
--security-revert, but sometimes it might not apply to all environments).