Consider a useful prompt to mitigate security issues about npx

[zkat]

I really like this idea. I think I'm gonna hack something together that adds both a confirmation prompt, as well as a whitelist (and blacklist?) to npx and see how it feels. This would only happen when installing remote packages, and after you confirm once, you won't get prompted for that package again. Thanks for this issue!

How's that sound?

[tunderdomb]

npx could even display author information and popularity stats

I can imagine this as part of the confirmation prompt proposed by @zkat something like this:

$ npx say-hi Josh
< Execute author/[email protected] (5 stars 10k downloads)? (Y/n)
< y
$ Hi Josh!

Or force it and ignore the propt:

$ npx say-hi --force
$ Hi Josh! # no prompt, just the output

[zkat]

to expand a bit more: I think something like levenshtein is more appropriate for that sort of matching but I'm not quite sure how to get that specifically out of our current APIs. (ideally, we'd do a levenshtein search and prompt the user if, say, there's a much more popular module with better quality metrics at a very close distance) /cc @bcoe because search

[junosuarez]

This would only happen when installing remote packages, and after you confirm once, you won't get prompted for that package again.

sounds great to me for interactive use on my machine.

For scripting use, I'd want to be able to include a flag (--force? --yes? ) to avoid this prompt

[zkat]

@js-n what I want to try is make it so, if you're using it non-interactively and it's going to install something, you must use --whitelist <pkg>. So you need to explicitly type it twice, in your scripts. When running either as a non-tty, or in an isCI environment, npx should just crash instead of prompt for confirmation.

If you're using this in CI, the thing I 💯 expect you to do is install it as a devDependency, in which case npx will never prompt you.

[Martinspire]

I'm not sure if a prompt would even be secure enough. If something isn't downloading directly but installes some other package that does, it would still be a security risk. Also being able to run npx from every folder seems like a risk as well. Preventing to run it everywhere by having a line in your package.json (like allowNpx: true or something, might also improve it a bit, but I feel there is still a major security risk to handle. Especially if you are going to ship it by default.

[zkat]

I had a thought recently: Should trust be based on user instead of package? Like, if you thought a package by @zkat was trustworthy before, does it make sense to assume you don't mind the rest of their packages getting installed?

[ScottFreeCode]

having a line in your package.json (like allowNpx: true or something

Anything that requires legitimate users to write something into package.json in order to run it would defeat at least half the purpose of npx and probably closer to the whole purpose.

(And probably wouldn't stop any malicious hackers anyway, as all it does is make usage less convenient; unless the convenience itself is the vulnerability, in which case we may as well delete npx since convenience is all it adds as far as I can tell.)

---

On a related note, I'd suggest that protection against typosquatting belongs in npm itself.

The whole job of npm is to download code off the internet and the whole reason to download code is to run it (whether through the CLI, through require, or automatically through its lifecycle scripts); if there's some way that we're at risk of getting malicious code instead of what we wanted (and I think the original issue of typosquatting is one such way), surely it applies to use the package manager in the first place.

[zkat]

@ScottFreeCode I agree, in re typosquatting being better off in npm itself. Part of the reason we never did this before is after seeing the results of a researcher running analysis on the registry and finding that typos, specially typosquatting, were pretty rare overall in the registry (I don't have the numbers on me -- this was before my time, but it was the response when I asked).

As far as getting something like this into npm, I like the idea, and my thinking here was that I could hack this into npx to try out the UX, since it's less likely to be disruptive, and if we find something that works really well, we can look into moving that workflow over to npm itself (and have npx do things through that). I'd love to prevent small typos from affecting users like this 👍

[junosuarez]

(sidebar, and not really related to the issue)

in CI, the thing I 💯 expect you to do is install it as a devDependency

Yep, yep- I've got a variety of non-ci things that are nonetheless scripted across different environments where a prior npm install step - or even the idea of a package.json don't make sense: things like one-off administrative tasks, some data integrity stuff, etc.

Something like npx --whitelist foobar foobar works fine

[zkat]

@Martinspire as soon as you start talking about transitive dependencies, the risk profile (imo) becomes identical to npm's.

I think it's important not just to have specific attacks to demonstrate, but to show that they're actually different from the risks imposed by using npm at all.

I'm also wondering about this allowNpx: true suggestion. I don't really understand the point of this? What's the attack here?

[junosuarez]

any package that has less than 100 weekly downloads.

The problem here is it's really trivial for a malicious user to just download their package 101 times.

I like the idea of reusing some of the work and thoughts around "quality metrics" that the npm registry (and others) has already done with search.

[dotchev]

As a start you can do it like ssh does. Upon first install ask the user if he trusts that thing (and show some relevant metrics as discussed above). Then remember his choice.

[bcoe]

@zkat it looks like this could potentially give you what you need:

https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-fuzzy-query.html

perhaps you could figure out on a test data-set of package names what an appropriate edit distance would be, and we could try exposing a new API endpoint that implements fuzziness on name.

[wbinnssmith]

😍😍😍

Whitelist and blacklist sounds awesome. I wonder if it could even be a shared resource (published to npm itself?). For example has similar behavior for yarn create.

@js-n, something like that for environments without a tty like CI sounds like a good idea to me!

[zkat]

I'm gonna cross-link to yarnpkg/yarn#3909 in case there's interesting/useful stuff in discussions over there, and hopefully so this is discoverable from there as well.

[Martinspire]

Well, I'm still thinking about it, but I can imagine hacked dependencies becoming a thing. Or that virusses will use npx if it is available to download and run code. A prompt would probably require a way to override that too, as to make it usable on automated systems as well.

A big nono would be to override system files or to go outside of its own folder to access files (or even write). Or how about a keylogger that activated by downloading some hacked app.

AllowNpx would probably make no sense as a task could write it anyways and thus ignore the whole security, but I still feel it should not be so easy for every folder to execute something.

Also take into account that some apps get shipped with node nowadays (to make desktop apps or mobile apps) and that many subdependencies get bundled, without the user looking at whether they contain something fishy.

[zkat]

Update: I've talked to registry folks about ways to efficiently get some of this metadata, what metadata would be appropriate, etc.

Turns out we have some actual quality metrics, too, since npm's search runs ~the same code as npms.io. We just don't display it in the search pages yet, but it's there 👍

[vpicone]

I wonder if it would be helpful to simply confirm any package that has less than 100 weekly downloads.

[trisys3]

@wbinnssmith, Just wanted to note that create-react-appp (with 3 p's) doesn't exist anymore, 3 days later. I guess your criticism and this issue's massive view count spurred instantaneous action to purge it from npm. Great job if that's what happened!

[Martinspire]

@zkat and what if a package of their dependencies or their dependencies dependencies gets hacked? How would you know?

The problem is probably not the top packages, its the nested children.

I think there should be something that defines what you can and cannot execute (without overrides) and that perhaps NPM should watch what files are being changed in order to make sure it isn't something fishy. I think its mostly fine to run something within your project folder, but outside should be a no-go (perhaps except for some exceptions like python, c++, etc). Though even then you are not paying attention to a virus that is executed or something.

Just running anything anywhere doesn't sound like a good idea, so rules should be put in place and enforced. I feel if you just let this happen, NPM would be the delivery system of many of new viruses. It runs on almost every platform, so it would be easy to send around the place.

[gabrielcsapo]

Could a simple Regex to check to replicated values be simple enough to warn users that they have “misspelled” a package? In the example of create-react-appp would yield good results for this but a package called crete-react-app would have to bring more actual language processing to define errors in grammar or spelling. It is an interesting problem, almost like a social engineering problem with any system that deals with these issues of centralized trust.

[zkat]

@gabrielcsapo the registry has APIs that provide information about package quality, popularity, maintenance, and authorship, and includes a pretty efficient full-text search endpoint that can be nudged into being useful. This is the route I'm planning to take.

[dotchev]

See how it is done in direnv