Get rid of `cross-spawn` dependency

[sindresorhus]

It's pretty unmaintained.

We do a lot to the shell, so would be simpler to do that too in execa ourselves.

Maybe some of the things it fixes are already fixed in Node.js too.

https://github.com/moxystudio/node-cross-spawn/blob/master/lib/parse.js

[ehmicky]

I agree that it not being maintained is a problem. Some of the bug reports seem rather important and are not currently looked into.

Breaking down cross-spawn feature by feature (which are all fixing problems with Windows):

1. Executing files that have a shabang (since that's a Unix concept). Workaround for users would be to specify the interpreter explicitly instead. This would not work if the script location relies on the PATH environment variable, as opposed to being a file path. It does so by using which on the command, then reading the first bytes of the file to detect the shabang.
2. Allows the command to a file path using the Unix path syntax. It does so by using path.normalize(), after having run which on the command.
3. Some issues with escaping (commands with spaces, etc.) although I am not completely sure whether this is fixed or not in the latest version of Node. cross-spawn seems to fix this by wrapping the command in an additional cmd.exe call and manually escaping the command and arguments, using cmd.exe-specific syntax.
4. It supports PATHEXT even when the shell option is false.

I am feeling conflicted.

• On one hand, I am concerned that some of those fixes might be important to some users and any change might break their usage of Execa.
• On the other hand, the current logic seems rather intricate and apparently buggy (according to the current GitHub issues on that repository), and therefore might be a high burden to maintain ourselves.

What are your thoughts?

[sindresorhus]

Relevant: bcoe/awesome-cross-platform-nodejs#26

[sindresorhus]

Maybe we could convince Bun to fix some of the things mention here to force Node.js' hand 🤣

[sindresorhus]

What are your thoughts?

I think it's worth doing it. cross-spawn is already buggy and unmaintained. We would release it as a major version to reduce the potential impact of any regressions.

[ehmicky]

Yes, that sounds good.

We would also need to add automated tests for the features covered by cross-spawn, although our current tests already cover some of it.

That's some major undertaking, so maybe we should start by just copy/pasting the code as is in an initial PR, and then start refactoring it. For example, some of it can be:

• Trimmed, e.g. we do not want the ENOENT logic
• Simplified
• Fixed, i.e. by looking into the GitHub issues of cross-spawn

[sindresorhus]

That's some major undertaking, so maybe we should start by just copy/pasting the code as is in an initial PR, and then start refactoring it.

👍

[ehmicky]

I have spent some time figuring out what node-cross-spawn precisely does, and what are the problems with node:child_process on Windows it is fixing. This issue is a list of those problems.

Shebangs

Shebangs are a Unix concept. Windows does not support them.

node-cross-spawn emulates shebang support on Windows by:

• Resolving the full path of the executable file
• Reading its contents. If it contains a shebang, use it as the main executable file instead.

Arguments splitting

In Unix, a subprocess file and arguments are specified as a file string and an arguments array of strings to the underlying C syscall. In Windows, those are passed as a single string, even when no shell is involved. The caller must escape the file and arguments with Windows-specific quoting rules so they are correctly split.

Node.js fixes this with the windowsVerbatimArguments option. When false, it performs that quoting automatically, which is basically, for each file or argument:

• surround with double quotes, if contains ", \, tab or space
• backslash escape double quotes and (if followed by double quotes) backslashes

It does not escape all shell-specific characters, so it is only meant for arguments splitting, not full-on shell escaping. In particular, it does not prevent shell injection.

windowsVerbatimArguments is false by default. Is is always true when the shell option is true, which is good, since it allows users that execute cmd.exe manually to performs that quoting themselves.

node-cross-spawn does not do anything there: the Node.js behavior is already good.

Node modules binaries

On Windows, *.cmd and *.bat files require a shell (like cmd.exe) to be run. This also includes other file extensions, basically anything but *.exe or *.com.

OS binaries are often .exe/.com, e.g. node itself, git, etc. However, Node modules binaries (installed either globally or locally) are *.cmd/*.ps1/*.sh files.

This means that, to run Node modules binaries on Windows, one must use the shell: true option. That option wraps the command with cmd.exe /d /s /c "file arg...". It does not escape file and arg, and just joins the arguments with spaces.

This has a few problems:

• It requires users to remember to set shell: true. Or shell: process.platform === 'win32' if they don't want to spawn Bash on Unix.
• It requires users to know whether the binary's file extension, which they might not know. For example, Execa does not easily know that.
• Using a shell leads to potential command injection. This means the user must escape any user-provided input, using cmd.exe-specific quoting rules (which are very different from what developers are used to).
• Whitespaces and quotes must be escaped. This includes spaces in the file path, which is common on Windows due to C:\\Program Files.

node-cross-spawn fixes this by emulating shell: true when shell: false is used. In addition, it performs some full escaping of the file and arguments.

PATHEXT

Windows has an environment variable PATHEXT, which is the list of file extensions which can be omitted by users when executing files. However, Windows only supports it when using a shell.

node-cross-spawn emulates its support without shells by taking it into account when resolving the full path of the executable file.

windowsHide

The windowsHide is left as is by node-cross-spawn.

[sindresorhus]

Great summary 👍

[wtto00]

Regular Expression Denial of Service (ReDoS) in cross-spawn

[ehmicky]

This security vulnerability has been fixed in the latest version of cross-spawn, which Execa uses.

[wtto00]

Hello @ehmicky ,
I see that the latest version of execa uses cross-swap@7.0.3, while the fixed version mentioned in the security vulnerability is 7.0.5.
So my repository was opened with this security vulnerability alert by GitHub's dependency bot.