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

Node security-release breaks the plugin on Windows #223

Closed
CarstenKoenig opened this issue Aug 7, 2024 · 15 comments
Closed

Node security-release breaks the plugin on Windows #223

CarstenKoenig opened this issue Aug 7, 2024 · 15 comments

Comments

@CarstenKoenig
Copy link

Hi,

maybe I'm a bit late with updating my node version but this here: https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
did just totally brick the plugin for me.

I was able to fix it (by editing the server.js on my install and adding the needed option.shell=true in two places which seems to work fine) for me but I think a better solution would be to set this flag anyway - at least when you detect that you run on Windows.

If you are interested I'm willing to provide a PR for this.

Thanks for your work and time.

@CarstenKoenig CarstenKoenig changed the title Node security-release breaks the plugin in Windows Node security-release breaks the plugin on Windows Aug 7, 2024
@MithraicMagic
Copy link

@CarstenKoenig would you be so kind to share the locations in the server.js file where this needs to be adjusted, or maybe share your own server.js we could use until this is fixed? I'd love to start trying Purescript in VSCode, but this is currently stopping me from doing so.

Thanks in advance!

@CarstenKoenig
Copy link
Author

Hi,

I wrote a short post on how I did it here.

Locations are a bit tricky as I used a JS formatter to get the file into a readable form.

Is this ok for you? If not I can try and grab a copy and post it here - just give me a few minutes as I have to fire up a Windows device first.

@MithraicMagic
Copy link

MithraicMagic commented Sep 17, 2024

Will give it a try and get back to you, thanks!
EDIT: That worked, thanks! I'm getting some sort port error now, but I bet that's just my installation.

@karanveersp
Copy link

karanveersp commented Sep 21, 2024

Really appreciate @CarstenKoenig's solution. Got my Purescript ide server resolved. I'm also getting the port error now which is preventing proper autocomplete and hover docs.

[Error - 12:30:35 PM] Could not start IDE server process. Check the configured port number is valid.
Error code returned: 1

@MithraicMagic
Copy link

Hi @karanveersp, are you running on Windows by any chance?

I'm running it on Windows and it wasn't actually a port issue in my case, it was an issue with that my Node installation was in a sub-folder of "Program Files" (the default location).

I think it doesn't input the path as a string correctly which is why a space in the path to the Purescript executable is problematic.

Try re-installing or moving your Node installation so that the full path to it has no spaces, and try again. Hope it helps!

@karanveersp
Copy link

karanveersp commented Sep 22, 2024

Yup, on Windows. I just re-installed node in the ProgramData dir and then all purescript IDE features are working!
Thanks @MithraicMagic!

@wclr
Copy link
Contributor

wclr commented Sep 26, 2024

@nwolverson

So far i'm on windows with node 21.7.3 got the errors. Updated psc-ide and language-server to the latest package-set (with new ChildProcess modules), and added shell param to all spawn/execFile calls in LS (In psc-ide I added shell param to startServer/findBins funcs). So this fixed the erros.

If you don't have concens and suggestions on shell enabled everywhere in LS, I can make a PR.

@nwolverson
Copy link
Owner

I don't think I really have a problem enabling shell for these calls, fundamentally they are calling fixed executables or workspace configured overrides (if I recall correctly). The security issue essentially is saying "you're already implicitly enabling shells", any configuration of weird stuff in the workspace (say you have loaded someone else's workspace config) already could just call some weird shell script I think anyway, you're already in a position of trust.

@wclr if you don't mind please go ahead and PR, I'd love if someone would test this on windows, I absolutely do not do any development on a windows machine

@AZMCode
Copy link

AZMCode commented Oct 9, 2024

I got a windows machine and this bug is driving me crazy lol.

I'm willing to help trying to fix this, shouldn't be too hard given I can get the extension up and running.
Are there any instructions on how to set up a development environment? Do i need to rebuild both the server and extension, or only some of them?

@nwolverson
Copy link
Owner

There is already a PR merged, I was distracted from making a release. Will do so soon

@AZMCode
Copy link

AZMCode commented Oct 9, 2024

I have managed to remove all other copies of both packages from my system, and built both the plugin and the server from the latest commit on both projects. It appears I still have this issue. Maybe it's machine specific?

@nwolverson
Copy link
Owner

@AZMCode I will resolve this issue when there is a release. It looks like the development instructions are not up to date, possibly missing some hint as to the bundling, but I'd rather spend the time getting this much delayed release out than supporting. Some windows person can test this once it is available.

@AZMCode
Copy link

AZMCode commented Oct 9, 2024

👍
Understandable. Hope to see the release soon.

@nwolverson
Copy link
Owner

0.26.5 should be published now with these changes (ls 0.18.2).

🤞

@AZMCode
Copy link

AZMCode commented Oct 10, 2024

Yeah, that did it! Appreciate it deeply. That was a lot quicker than expected. Tested on windows and can confirm works as expected.

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

No branches or pull requests

6 participants