Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix Node 20.12.2 child_process.spawn .cmd EINVAL on Windows #1455

Merged
merged 6 commits into from
Apr 30, 2024

Conversation

longzheng
Copy link
Contributor

Fixes #1453 incompatibility with Node 20.12.2 on Windows. https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2

I looked for all usages of child_process.spawn which invoked a .cmd file and added shell: true (conditionally since I didn't want to change non-Windows behavior)

I also wrapped the NPX_PATH_ESCAPED because the path will contain spaces e.g. C:\\Program Files\\nodejs\\npx.cmd which doesn't work with shell: true

There were other usages of child_process.spawn which were for ffmpeg and ffplay which I don't think needs any change since those are .exe files.

@@ -34,7 +34,8 @@ $SCRYPTED_HOME_ESCAPED_PATH = $SCRYPTED_HOME.replace('\', '\\')
npm install --prefix $SCRYPTED_HOME @koush/node-windows --save

$NPX_PATH = (Get-Command npx).Path
$NPX_PATH_ESCAPED = $NPX_PATH.replace('\', '\\')
# The path needs double quotes to handle spaces in the directory path
$NPX_PATH_ESCAPED = `"${$NPX_PATH.replace('\', '\\')}"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koush I couldn't quite figure out why you changed the NPX path to be the full path in ddb6a3a as opposed to just npx.cmd, but I guess you had a good reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing because it's running as a service it needs the full path since it doesn't have the npm directory in the Path environment variable?

@koush koush merged commit 4b055f5 into koush:main Apr 30, 2024
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatible with Windows Node 20.12.2
2 participants