Skip to content

Fix spawn of jetls.bat#372

Merged
aviatesk merged 2 commits intoaviatesk:masterfrom
visr:jetls-bat
Dec 8, 2025
Merged

Fix spawn of jetls.bat#372
aviatesk merged 2 commits intoaviatesk:masterfrom
visr:jetls-bat

Conversation

@visr
Copy link
Contributor

@visr visr commented Dec 8, 2025

Fixes #339.

There are two separate commits. The first replaces jetls.exe with jetls.bat, as discussed in #332 and previously in #226.

As noted in #339 we can only spawn batch scripts on Windows after setting shell to true, for security considerations. The spawn docs mention:

If the shell option is enabled, do not pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution.

Users can configure the path with

"jetls-client.executable": {
    "path": "jetls.bat",
    "threads": "auto",
}

I'm not sure there is more to do from a security perspective, because any bad jetls.bat that is earlier on the path, or bad custom path that is entered here can do harm.

https://nodejs.org/api/child_process.html#child_processspawncommand-args-options
https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2

I'd prefer if Pkg apps would be .exe rather than .bat shims, using something like the Pixi trampoline:

https://pixi.sh/latest/global_tools/trampolines/
https://github.com/prefix-dev/pixi/tree/main/trampoline

But that is more of a Pkg issue.

Copy link
Owner

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

Thank you for investigating and fixing this issue.
Since I don't use Windows, I haven't been able to actually verify the behavior, but I think your investigation and explanations are just accurate.

@aviatesk aviatesk merged commit a60c25a into aviatesk:master Dec 8, 2025
2 checks passed
visr added a commit to visr/JETLS.jl that referenced this pull request Dec 9, 2025
aviatesk pushed a commit that referenced this pull request Dec 9, 2025
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.

Server startup issue on Windows

2 participants