Skip to content
This repository has been archived by the owner on Apr 7, 2021. It is now read-only.

Consider disabling npx auto-install by default #9

Open
1 task done
niedzielski opened this issue Sep 18, 2019 · 13 comments
Open
1 task done

Consider disabling npx auto-install by default #9

niedzielski opened this issue Sep 18, 2019 · 13 comments

Comments

@niedzielski
Copy link
Contributor

Copied from npm/npm#19673

I'm opening this issue because:

  • npm is doing something I don't understand.

What's going wrong?

A feature of npx that I was not aware of until today is that it will auto-install and auto-execute any module not found in path. I discovered this by typing npx ts (ts module) instead of npx tsc (typescript). By the time I realized what happened, the module had already installed and executed on my machine.

The module could be malicious, or just plain annoying to undo (e.g. writing certain files around my system, modifying configs, deleting files in current folder, etc). It's hard to know what just got run on my machine without downloading & inspecting the JS in the tarballs.

How can the CLI team reproduce the problem?

Running npx [command] will auto-install and auto-run a module not installed. This is a handy feature but IMHO it should be opt-in to avoid catastrophic situations where somebody mis-types a module, or runs code from a gist.

I realize it's not really different from npm install [bad-code], but at least the latter is much more explicit and obvious what is happening. I was under the impression npx was used for running local node_modules bin scripts, but not much else. I also tend to type npx [cmd] more frequently than npm install, thus it seems more prone to typos/errors.

Thoughts? Or is it just me?

EDIT:

Here is a scenario where npx [cmd] is a bit more problematic than just npm i.

  • A popular package, cool-mvc, has a bin script called funkytown. The docs guide users to run npm install cool-mvc && npx funkytown to launch the script in their own project.
  • At some point, the user loses reference to the bin script, e.g. by deleting the local node_modules or cd'ing out of the project folder.
  • The user runs npx funkytown again, and the result is surprising. It installs and executes the bin script in the funkytown module, not the cool-mvc module. This new module may be malicious, or just an unlucky coincidence.
@niedzielski
Copy link
Contributor Author

Hey there! I hope it isn't bad form but I've copied this preexisting issue from npm/npm#19673. I think the behavior this issue describes is a very surprising default and hope it can be moved to feature flag or removed.

@gabrielfern
Copy link

I agree, it could at least ask if you want to install package "name_of_package".

@heyimalex
Copy link

This is, for better or worse, an incredibly common usecase for npx. Downloading code from the internet and running it on your machine in one command line may be dangerous (foolish?), but it's also very convenient. People making clis love it because it allows regular releases without needing to worry about leaving 99% of your install base behind, and almost completely avoids issues from end users running outdated versions. End users love it because it's simple. npx is currently the preferred method of running create-react-app and create-next-app.

Just some food for thought. Adding a short flag that means both --ignore-existing and the opposite of --no-install (supposing this change makes it) would be nice for these types of tools and just a minor documentation change.

@niedzielski
Copy link
Contributor Author

@heyimalex, thanks for the perspective with references.

Maybe there could be a some transition time where relying on the old auto-install behavior shows a warning in one version and then a prompt to install in the next version to allow toolmakers and users to transition.

Just some food for thought. Adding a short flag that means both --ignore-existing and the opposite of --no-install (supposing this change makes it) would be nice for these types of tools and just a minor documentation change.

+1

@karfau
Copy link

karfau commented Jan 2, 2020

And it could still be a feature that people can/need to opt-in for, e.g. using npm config mechanism (or any other that is already in place.

@jcrben
Copy link

jcrben commented Jan 18, 2020

I'm trying to find out where it's installing things and how to remove them? It seems like it's automatically uninstall afterwards.

@watsoncj
Copy link

watsoncj commented Jan 28, 2020

From an organizational security perspective the auto-install behavior is extremely concerning. This should be considered a security vulnerability.

Here is a workaround:
alias npx="npx --no-install $@"

wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-MobileFrontend that referenced this issue Jun 5, 2020
check_bundle.sh assumes it will be invoked by package.json with an NPM
PATH environment. This means you can't run it directly without the
bundlesize executable in your PATH.

The issue is resolved by wrapping the `bundlesize` invocation in npx.
Since [npx will auto-install by default], disable that behavior for
safety since the assumption is that the script will be invoked from the
MobileFrontend repo.

Unfortunately, CI does not have npx so the following hack is used
instead:

  PATH="$(npm bin):$(npm bin -g):$PATH" bundlesize

That is, get the local package.json executable directory, get the global
bin directory too, and prepend it to PATH.

[npx will auto-install by default]: npm/npx#9

Change-Id: I2e13633b1394ad7d7f2ede56416ca932a82a63db
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions that referenced this issue Jun 5, 2020
* Update MobileFrontend from branch 'master'
  to cd256de71d44ee31d88513999375368159c54a4b
  - [dev][fix] use NPM pathing when running bundlesize
    
    check_bundle.sh assumes it will be invoked by package.json with an NPM
    PATH environment. This means you can't run it directly without the
    bundlesize executable in your PATH.
    
    The issue is resolved by wrapping the `bundlesize` invocation in npx.
    Since [npx will auto-install by default], disable that behavior for
    safety since the assumption is that the script will be invoked from the
    MobileFrontend repo.
    
    Unfortunately, CI does not have npx so the following hack is used
    instead:
    
      PATH="$(npm bin):$(npm bin -g):$PATH" bundlesize
    
    That is, get the local package.json executable directory, get the global
    bin directory too, and prepend it to PATH.
    
    [npx will auto-install by default]: npm/npx#9
    
    Change-Id: I2e13633b1394ad7d7f2ede56416ca932a82a63db
@just-boris
Copy link

just-boris commented Oct 25, 2020

Isn't this fixed in NPM 7: https://blog.npmjs.org/post/617484925547986944/npm-v7-series-introduction

In the blog post, they mention that it now asks for confirmation before downloading a package.\

For reference, npx command is now bundled into the main npm/cli repository, this one is no longer used. The commit with the fix is here: npm/cli@3aba8d6#diff-62fc6e653ef5d0f48270b34d19b0fe951ae78674a0fde733eb23295a2d15fb00

@niedzielski
Copy link
Contributor Author

@just-boris, I believe you are correct (although I missed the reference in the blog post):

$ npm --version
7.0.3
$ npx --version
7.0.3
$ npx --verbose create-react-app foo
npm verb cli [
npm verb cli   '/home/stephen/opt/node-v15.0.1-linux-x64/bin/node',
npm verb cli   '/home/stephen/opt/node-v15.0.1-linux-x64/lib/node_modules/npm/bin/npm-cli.js',
npm verb cli   'exec',
npm verb cli   '--loglevel',
npm verb cli   'warn',
npm verb cli   '--yes=false',
npm verb cli   '--loglevel',
npm verb cli   'verbose',
npm verb cli   '--',
npm verb cli   'create-react-app',
npm verb cli   'foo'
npm verb cli ]
npm info using npm@7.0.3
npm info using node@v15.0.1
npm timing config:load:defaults Completed in 1ms
npm timing config:load:file:/home/stephen/opt/node-v15.0.1-linux-x64/lib/node_modules/npm/npmrc Completed in 1ms
npm timing config:load:builtin Completed in 1ms
npm timing config:load:cli Completed in 2ms
npm timing config:load:env Completed in 0ms
npm timing config:load:file:/home/stephen/desk/foo/.npmrc Completed in 0ms
npm timing config:load:project Completed in 1ms
npm timing config:load:file:/home/stephen/.npmrc Completed in 2ms
npm timing config:load:user Completed in 2ms
npm timing config:load:file:/home/stephen/opt/node-v15.0.1-linux-x64/etc/npmrc Completed in 0ms
npm timing config:load:global Completed in 0ms
npm timing config:load:cafile Completed in 0ms
npm timing config:load:validate Completed in 1ms
npm timing config:load:setUserAgent Completed in 0ms
npm timing config:load:setEnvs Completed in 1ms
npm timing config:load Completed in 9ms
npm verb npm-session b8325f59a825e23a
npm timing npm:load Completed in 15ms
npm http fetch GET 304 https://registry.npmjs.org/create-react-app 156ms (from cache)
npm timing arborist:ctor Completed in 1ms
npm timing arborist:ctor Completed in 0ms
npm timing command:exec Completed in 179ms
npm ERR! canceled
npm verb exit 1
npm timing npm Completed in 327ms
npm verb code 1

I'll leave the maintainers to decide whether this ticket is still valid or not.

@lander0s
Copy link

lander0s commented Oct 27, 2020

wow!, a malicious user could create npm packages with common typos in the name to compromise random machines.

@DenizBasgoren
Copy link

Spent an hour trying to understand what was the problem with preact-cli: it was running in 1 minute. Turns out npx was reinstalling it every time I run the command! Correct command was preact, not preact-cli. It is an awful idea to have npx installing things. npx is supposed to be only a tool to run executables. for installing things there is npm. I vote for default --no-install

@cefn
Copy link

cefn commented Feb 26, 2021

Agreed this is very worrying, especially given https://github.com/basarat/tsc/blob/master/bin/tsc is the auto-install alias for tsc, creating mysterious problems (associated with running a 6 year-old version of Typescript) and leaving it as a huge target for hijacking.

Roll on version 7 and no auto-install being bundled by default.

@ljharb
Copy link

ljharb commented Feb 26, 2021

In npm 7, npx prompts and does not install by default.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests