joeywatts commented 4 years ago

What / Why

Generated *.CMD files are unable to start correctly when the path to the CMD file contains certain characters.

When

Whenever the path contains a cmd.exe shell metacharacter (such as &).

Where

Any package with a "bin" entry.

How

Current Behavior

The shell metacharacters are parsed by the shell instead of the full argument being passed into node. With my npm prefix set to "C:/test(n&pm)" and after installing the mkdirp package globally, I see the following error when attempting to run "mkdirp" in cmd.exe.

'pm)\' is not recognized as an internal or external command,
operable program or batch file.
internal/modules/cjs/loader.js:969
    throw err;
    ^

Error: Cannot find module 'C:\test(n\node_modules\mkdirp\bin\cmd.js'
[90m    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:966:17)[39m
[90m    at Function.Module._load (internal/modules/cjs/loader.js:859:27)[39m
[90m    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)[39m
[90m    at internal/main/run_main_module.js:17:47[39m {
  code: [32m'MODULE_NOT_FOUND'[39m,
  requireStack: []
}

Steps to Reproduce

Orchestrate any scenario in which the .CMD files will be generated under a path containing "&" character (in theory the other shell metacharacters would also be problematic, see references for an article on windows command line escaping).

Two scenarios come to mind.

Scenario 1: Set NPM Prefix to New Path

Set npm prefix to "C:/test(n&pm)"
Globally install mkdirp (or any other binary package).
Make sure "C:/test(n&pm)" is in your PATH.
Attempt to run "mkdirp" (or whatever you installed).

Scenario 2: Local Dependencies

Create an npm package in a folder like "C:/test(n&pm)/package"
Install a local dependency which has a "bin" entry (like "mkdirp")
Add a script to package.json which attempts to run the dependency's bin (like "trymkdir": "mkdirp mytestfolder")
Attempt to run your script: npm run trymkdir

Expected Behavior

The .CMD files should be able to run without getting tripped up by shell metacharacters in the path.

Who

n/a

References

Everyone quotes command line arguments the wrong way (Microsoft blog explaining cmd quoting and shell metacharacters)

wycats commented 4 years ago

This is a real issue (and well researched!). Any reason not to do this?

isaacs commented 3 years ago

We recently made the shift in npm v7 to use puka for exactly this purpose when passing arguments to cmd.exe. (And in the process, seem to have broken passing empty strings with "" to npm scripts, which will likely be fixed soon.). Unfortunately, translating the logic required into batch and powershell is nontrivial.

Patch welcome!

jcompagner commented 2 years ago

this commit: https://github.com/npm/cmd-shim/commit/4c37e048dee672237e8962fdffca28e20e9f976d

caused this issue because this:

SET dp0=%~dp0

will result in a wrong dir if the path is something like c:\something&somemore\

thing is if i quoted it

SET dp0="%~dp0"

then it doesn't work further in the script because then the dp0 variable has quotes (a bit stupid of cmd...) what we kind of want is get the full string without quotes

But for me kind of reverting that above commit works for me (so not using the "dp0" variable that has the content of ~dp0 but directly use ~dp0 everywhere..