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

[BUG] $EDITOR environment variable broken on Windows #6716

Open
2 tasks done
rotu opened this issue Aug 17, 2023 · 1 comment
Open
2 tasks done

[BUG] $EDITOR environment variable broken on Windows #6716

rotu opened this issue Aug 17, 2023 · 1 comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release

Comments

@rotu
Copy link
Contributor

rotu commented Aug 17, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

On Windows, if the EDITOR environment variable is set, npm config edit does not resolve it as either a literal path nor a shell command.

Expected Behavior

I expect either:

  1. Editor to be a path to an executable
  2. Normal shell-like resolution applies to the command (including escaping spaces)

Steps To Reproduce

  1. On windows 11, using PowerShell or the Environment Variable Editor, set the value of EDITOR
  2. Call npm config edit
  • If the value of EDITOR has a space in it, it is truncated there (even if the executable is surrounded by ", meaning you can't use a path with a space in it. None of the following work:
# any of the below values
$env:EDITOR=(gcm code).source
$env:EDITOR="C:\Users\dan\AppData\Local\Programs\Microsoft VS Code\bin\code"
$env:EDITOR='"C:\Users\dan\AppData\Local\Programs\Microsoft VS Code\bin\code"'
$env:EDITOR="C:\Users\dan\AppData\Local\Programs\Microsoft^ VS^ Code\bin\code"
npm config edit

The error message indicates that the path is truncated at the first space, e.g.

npm config edit
npm ERR! code ENOENT
npm ERR! syscall spawn C:\Users\dan\AppData\Local\Programs\Microsoft
npm ERR! path C:\Users\dan\AppData\Local\Programs\Microsoft
npm ERR! errno -4058
npm ERR! enoent spawn C:\Users\dan\AppData\Local\Programs\Microsoft ENOENT
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent

  • normal shell resolution of a command does not work:
$env:EDITOR="code"
  • The following does work:
$env:EDITOR="code.cmd"

Environment

  • npm: 9.8.1
  • Node.js: v18.17.1
  • OS Name: Windows 11
  • System Model Name:
  • npm config:
npm config ls
; "builtin" config from C:\Users\dan\AppData\Roaming\npm\node_modules\npm\npmrc

prefix = "C:\\Users\\dan\\AppData\\Roaming\\npm"

; "user" config from C:\Users\dan\.npmrc

@cs:registry = "http://reg.example.com"
//registry.npmjs.org/:_authToken = (protected)

; node bin location = C:\Program Files\nodejs\node.exe
; node version = v18.17.1
; npm local prefix = C:\Users\dan\Source\Cerulean\SonarView
; npm version = 9.8.1
; cwd = C:\Users\dan\Source\Cerulean\SonarView
; HOME = C:\Users\dan
; Run `npm config ls -l` to show all defaults.
@rotu rotu added Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release labels Aug 17, 2023
@rotu
Copy link
Contributor Author

rotu commented Aug 19, 2023

I think this is related to a deeper issue with @npmcli/run-script. spawnWithShell seems VERY suspect:

  1. It finds the executable by looking at the string and naively scanning for quotes. This doesn't respect ^-escaped spaces.
  2. This quote-scanning doesn't match types of quotes. For instance, "'my double quoted path'" will open quotes at the first " character and then say the quotes have ended at the ' character.
  3. Finally, the path is passed to which.sync with the quotes intact. So it's looking for an executable with those quotes in its path.
  4. The isCmd regex searches for \ but / is a legal alternative path separator on windows.
  5. Shell can be passed in. It's not unlikely that, if the user is on Windows, they're using PowerShell. In that case, I don't know what's supposed to happen.

https://github.com/npm/promise-spawn/blob/d0e078944659d34ac883c8e108e01aeeb8d7e18a/lib/index.js#L65-L124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release
Projects
None yet
Development

No branches or pull requests

1 participant