Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Build fails on Windows on latest Node versions #13613

Closed
msujew opened this issue Apr 17, 2024 · 5 comments · Fixed by #13614
Closed

Build fails on Windows on latest Node versions #13613

msujew opened this issue Apr 17, 2024 · 5 comments · Fixed by #13614
Labels
OS/Windows issues related to the Windows OS
Milestone

Comments

@msujew
Copy link
Member

msujew commented Apr 17, 2024

Bug Description:

Due to the latest LTS update of Node v18/v20, running cp.spawn without setting the options to { shell: true } simply yields an error on Windows. You can see this here.

Steps to Reproduce:

  1. Try to build Theia on Windows using an updated LTS version of Node
  2. Assert that the build fails due to spawn EINVAL errors

Additional Information

  • Operating System: Windows 11
@msujew msujew added the OS/Windows issues related to the Windows OS label Apr 17, 2024
@tsmaeder
Copy link
Contributor

@msujew any idea why this breaks our build and how to fix it?

@msujew
Copy link
Member Author

msujew commented Apr 17, 2024

@tsmaeder Yes, the node-pty version we're using is using a cp.spawn call that doesn't have the { shell: true } options on Windows. This is basically illegal now :)

We had a few of those as well, some even in our own build processes. See #13614 for an extensive list of changes.

@tsmaeder
Copy link
Contributor

IMO, even though the node version update is a security update, I don't think building with older node versions is a problem for Theia: the CVE allows for arbitrary commands to be executed as the back-end user by using spawn parameters. However, the user can do this anyway: he just has to open a terminal in the IDE. The only vector I can see where the user is unaware of the commands being executed is covered by workspace trust and that's an entirely different problem. What do you think @msujew ?

@msujew
Copy link
Member Author

msujew commented Apr 22, 2024

@tsmaeder Same thoughts. The only issue for our adopters right now is that it's not very obvious why builds are consistently failing on GitHub Actions and on their PCs (in case they installed the latest node.js update). From a security standpoint, there is little to worry about for adopters of Theia.

@tsmaeder
Copy link
Contributor

This is still one we need to fix: broken builds with up-to date version of node LTS is likely to break downstream builds. This build will be a community build, so doubly so 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Windows issues related to the Windows OS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants