Add RFC for install scripts whitelist

[voxpelli]

Background

After discussions turned towards some security related topics in one of the front end chats I'm frequenting and we touched the fact it's not unheard of for scripts to accidentally delete peoples hard drives, or to even do something intentionally malicious, we moved the topic of npm scripts and the fact that one either allow them all to run any script they like on install, and exposes oneself to such risks, or one allows none to run anything, and then get a hard time with eg. modules that need to compile some native code.

Pretty much everyone in the discussions felt that it must be possible to do something better so I decided to wrap up some of the thoughts into a suggestion that could be proposed to one of the tools and since I use Yarn, I'm posting it here.

Suggestion

In summary the suggestion is to introduce the concept of a whitelist to allow a module to run install scripts so that one may vet them before they get to run anything automatically. That way modules that need to run stuff gets to run it after a vetting, the rest doesn't.

Pinging some of the people involved in the front end chat discussions: @aholstenson, @frozzare, @Kolombiken, @murm

[bestander]

I like the idea.
Not sure which place is better for the whitelisted scripts - yarn.lock or package.json.

We treat package.json as a blueprint for the project and yarn.lock is more like a table of patterns to URLs and we don't have much more features inside it.

[voxpelli]

It seems like there's actually right now a case of such a malicious package being used in the wild to steal peoples environment variables: https://twitter.com/o_cee/status/892306836199800836

Great to hear you like the idea @bestander 👍

I guess one question that impacts where the whitelist goes is what data it should actually contain. Should it whitelist all versions for a package or just a specific version range?

Whitelisting version ranges would be the most powerful. One way to do so could be to just mimic how dependencies are added and do something like this in package.json:

"allowScripts": {
  "package-name": "1.0.0 - 1.2.0",
  "another-package": "*"
}

The existence of an allowScripts key in package.json could then also be what signals if a whitelist should be enforced or not, so if one wants to exclude all scripts from running, then one would simply add "allowScripts": {}.

Such a property could also be possible for npm and other tools to support.

[BYK]

We can also move this into .yarnrc so we can have global, user-level, and project-level granularity?

[Daniel15]

Would the whitelist be for a particular version of a package, or for any version of the package? If it's not version-specific, it wouldn't protect against malicious code being added to an existing package, right?

Edit: Oh, I just noticed you mentioned this above:

I guess one question that impacts where the whitelist goes is what data it should actually contain. Should it whitelist all versions for a package or just a specific version range?

[ljharb]

Can we use a slightly less problematic term than "whitelist"? Perhaps "inclusion list"?

[bestander]

Can we use a slightly less problematic term than "whitelist"? Perhaps "inclusion list"?

Why whitelist is problematic?

[ljharb]

@bestander "whitelist" is good, "blacklist" is bad, which relies on the connotations of one color being good and one bad, which has racial implications. (i really really hope nobody gets into a debate here on this, if anyone doesn't agree with the premise, a simple "no thanks" would be better than devolving into a debate on whether people's feelings matter)

[voxpelli]

@ljharb Right, from my point of view I'll happily switch the wording to "install script opt-in" then, would convey the concept about as well and avoid anyone feeling offended.

As a property in a file I think "allowScripts" make sense, so this would only change the RFC wording I think.

@BYK Interesting idea, .yarnrc could work very well.

One benefit of that over package.json is that it would convey that dependencies themselves can't opt-in any install scripts.

Another is that it would also make it easy to share an opt-in list between projects, so that a team or company could enforce the same list across many projects.

[vmasto]

Given that naming a malicious package similarly to a popular one is a very common attack vector, and as an extra safety blanket, what if yarn could keep a global centralized whitelist based on, say, the top 500 most popular packages and before installing it warned for typos based on Levenshtein distance (or any other suitable algorithm).

I also just found this discussion on npx zkat/npx#66 which is relevant and discussed the same concept.

[tdsmithATabc]

I was going to suggest that it would be nice if you could opt-in to the script "content" itself (for a specific package) so that you didn't have to reapprove it when there had been no changes. The opt-in list would be hashes of the scripts themselves.

But then I convinced myself that this opens up too many weird security edge cases, like a package changing structure in such a way that a previously harmless script became malicious.

Just thought I'd share that one-person conversation. 😛

[peabnuts123]

Allowing only packages at specific semvers is, theoretically, ideal. However, it is totally impractical in reality. Direct dependencies are already specified at specific version ranges, and the version of any subdependencies are specified by their respective package.json manifests. This means that while you would be able to specify only the packages you would like to include, the packages you would be receiving is ultimately up to the developers of the packages you depend on, the packages they depend on, the packages those packages depend on, etc. Essentially creating an exponential amount of overhead upkeep for a project maintainer, the more packages you depend on. This quickly balloons out of control, my work project containing roughly 1000 unique subdependencies directly in node_modules, so, not accounting for version conflicts nested within other dependencies, something you would also have to account for in such an inclusion manifest. This is all also ignoring the fact that you have no reason to trust a package at a certain version anyway, and for a real testament to security, each package would need to be audited individually, at each version.

This is a serious problem. One that really needs to be solved. However, it is not easily solved, and I don't think an inclusion list is practical, meaning it will do little for the security of the general populace of developers using NPM / Yarn, and only really serve the diehard security-conscious types, who probably have alternatives such as "don't use a package manager" to remedy their concerns already.

[voxpelli]

@peabnuts123 I think an opt-in list can be manageable, at least in some cases. I too have a lot of unique modules in my work projects, but very very few of them needs to run any install scripts.

For me personally I would probably run with something like this (in maybe .yarnrc):

"allowScripts": {
  "node-sass": "*",
  "gc-stats": "<=1.0.2",
  "dtrace-provider": "<=0.8.3"
}

That would narrow down the modules with vulnerable install scripts from a couple of hundreds to just a few, which would make me feel much more safe and which would be much more practical for me than todays only option, ignore-scripts.

Of course things can be improved even further. This opt-in list can easily live side-by-side with other security measures and doesn't stop any work from happening on any such ones.

To me this would just be a small simple step towards making install script security a bit more practical and a step that can be iterated on in the future – with eg. addition of better tools, both within yarn itself and within the wider ecosystem.

[rally25rs]

Personally, I'd like to be able to just set ignore-scripts at the global .yarnrc level, but still have my projects able to run scripts for the few packages that are known to need them (PhantomJS for example). This would protect against fat-fingering or typo installs in new projects while still letting existing projects work.

Wherever this is stored, we should be sure to only respect this setting for the current project where Yarn is executing. In other words, an installed dependency should not be allowed to specify an allowScripts list in it's package.json (so that malicious packages can't add to the whitelist). That is probably obvious, but should be explicitly unit tested so as not to allow a regression where the whitelist can be modified by dependencies.

I don't really like the idea of the community having to maintain a list of "bad" packages, because that is always reactionary. Someone has to realize it, add it to the list, and deploy the list. What if a "bad" package is already in my cache and I'm running --prefer-offline? how do I get the updated list? Just seems like a maintenance nightmare and should really be handled by NPM revoking known malicious packages from the registry.

Whitelisting by semver range isn't a bad idea if it isn't too difficult to implement. There was an article recently where a huge number of NPM user's passwords were easily guessable (like just using "password") so an attack vector could certainly be pushing a new malicious version of existing packages if the publisher's credentials are weak. Of course this means that ranges ~ and ^ and >= should be advised against. Specific versions, not a range, should be preferred in the documentation.

[benyitzhaki]

i agree with @rally25rs regarding NPM's responsibility here. i think they should be able to revoke malicious packages or at least the other way around, flag "trusted packages" and make npm recognize only trusted sources in their repository by default. this could be done as a POC in yarn as from my understanding, they use a proxy for the official repo

[voxpelli]

Lets keep to the topic of this RFC though @benyitzhaki, because as stated earlier – nothing in this RFC is incompatible with any such measures from npm:s side. Rather it would be great to have multiple layers of security.

@rally25rs Yeah, version ranges in the opt-in list should rather be for targeting a range of earlier versions than for a range of upcoming ones (even though even such a range can, if wide, leave some room for malicious updates to earlier versions – like patch updates of old minor releases)

I like the idea of using .yarnrc as it makes it clear that only the .yarnrc within the project folder and above will be accounted for – which means any within a dependency will be ignore, which is important as you point out @rally25rs.

Using .yarnrc would also give certain inheritance rules out of the box I guess, which would mean that the allowScripts / allow-scripts option (latter follows .yarnrc naming conventions better) defined in the .yarnrc closest to the project folder would be the one that's used.

One thing regarding such inheritance though that would need to be defined is how ignore-scripts and allow-scripts would interact.

I would say that whenever allow-scripts is defined ignore-scripts should be ignored. The only possible issue with that from a backwards compatibility perspective would be if a local project sets ignore-scripts: true to deactivate all scripts locally and there's a allow-scripts set globally – then that local deactivation will be ignored until they override the allow-scripts instead.

[gordon-to]

What about a prompt scripts option?

Each script would ask permission to run, and the user could choose to remember that choice in yarn.lock. The user could also choose to inspect the script, or not run it.

[bestander]

@xMrWhite, good idea.
From implementation POV could be harder because Yarn runs scripts in parallel and prompting in scripts is not supported

[gordon-to]

It should be able to determine all the packages that have script that could be run during the resolution phase. The scripts should continue to be run in parallel. I guess it depends on if the full package.json is queried during resolution.

[voxpelli]

@xMrWhite I was thinking about something similar as well, but decided to keep this RFC focused on the minimal possible implementation so that it would be easier to later on try out different such interfaces and enable work on multiple such approaches in parallel, without any single one holding up the core mechanism itself.

(For some such a mechanism that you suggest would probably be ideal, for others there can perhaps be a team-wide opt-in list instead, all depending on the type of project and the composition of development team. The sooner we can start experiment with different such interfaces, the better)

[rally25rs]

I like the idea of prompting per script, but it would have to be an opt-in to stop and prompt. Keep in mind a lot of yarn installs are done by automated systems, like CI servers. Would probably have to be something like an add-interactive or install-interactive command, like we do with upgrade and upgrade-interactive.

My thought for an initial implementation would be to just add a name:version hash (same format at dependencies in package.json) to .yarnrc. The existence of said option implies that --ignore-scripts-like behavior is in effect for all packages that are not in this hash, and if the user actually specifies --ignore-scripts then Yarn would ignore even the scripts for packages listed in this setting (truly ignore all scripts).

[gordon-to]

It should go into the yarn lock for the project, that already contains version information

[bestander]

Good point raised, @xMrWhite.
Yarn guarantees consistent node_modules for the same yarn.lock + package.json + execution options (--production etc).

The role of yarn.lock is to store resolutions for all package patterns used in the project.
I don't think we should overload it with a new role that is not related to dependency resolution.

.yarnrc sounds like the right place to store execution option, package whitelist is one of them.

[rally25rs]

Maybe we should discuss the workflow.

Would we expect a user to already have ignore-scripts set in their .yarnrc then do something like yarn add foo --allow-scripts? If that is the case then maybe yarn.lock would make sense, but I bet a lot of people just manually add entries to package.json and run yarn install. In that case, then couldn't manually edit yarn.lock to enable scripts for that package because an entry wouldn't exist yet.

If someone already decided to disable scripts, then they probably already have a .yarnrc file with ignore-scripts in it so it could be manually edited to add vetted packages to an array.

[peabnuts123]

If anybody wants to audit what dependencies have preinstall, install or postinstall hooks you can run this little script in a terminal:

find node_modules/ -iname "package.json" -exec grep '"preinstall"\|"install"\|"postinstall"' {} \; -exec echo 'From: {}' \; -exec echo \;

Output:

    "install": "node install",
From: node_modules//fsevents/package.json

    "install": "node scripts/install.js",
    "postinstall": "node scripts/build.js",
From: node_modules//node-sass/package.json

    "postinstall": "node postinstall",
From: node_modules//spawn-sync/package.json

in my IDE (vscode) I can just click these file references and see exactly they are invoking, etc.

[ljharb]

find "$HOME/.npm" -iname "package.json" -exec grep '"preinstall"\|"install"\|"postinstall"' {} \; -exec echo 'From: {}' \; -exec echo \; will search your local npm cache, for a broader search :-)

[voxpelli]

This issue seems to have come up again: eslint/eslint-scope#39 A module stealing credentials (I think on install)

I dropped the ball on this one due to life getting in the way pretty majorly, sorry 😐 I’ll try to pick this up again soon and rewrite the proposal to include the given feedback.

[RumataEstor]

Would an approach to allow specific features be more convenient?

For example, when a script is executed, a custom "require", "fs", "http" and whatever else is provided. Whenever they are executed, they check if the requested operation has been already allowed in the yarn.lock previously, or if not would ask the user interactively if it's safe to execute.

The allowed operations might be like "fs read relative_path", "http get url_wildcard" and so on.

[dominykas]

Implemented this as a separate package: https://www.npmjs.com/package/allow-scripts

tl;dr:

• add allowScripts: { pkg: versionRange } in your package.json
• npm install --ignore-scripts or yarn install --ignore-scripts
• npx allow-scripts

fwiw, I hope I can deprecate that package soon, as --ignore-scripts + allowScripts should be the default in the package installers.

[voxpelli]

Awesome @dominykas! Thanks for picking this up

I see that npm is showing interest in this:
https://twitter.com/maybekatz/status/1090351371809746944?s=21

I’ll happily try to update this PR with whatever the conclusion is there, or if there is none, whatever we would like to push here instead 👍

[arcanis]

Fyi the v2 includes a way to disable postinstall scripts, via dependenciesMeta:

{
  "dependenciesMeta": {
    "[email protected]": {
      "build": false
    },
    "webpack-cli": {
      "build": false
    }
  }
}

[dominykas]

@arcanis interesting, does that mean it has them enabled by default?

[arcanis]

Yes, even though I was considering disabling them by default. This will likely be a rfc since it has far reaching consequences.

Whatever we end up doing, the end goal is to give you the ability to choose whether they should execute or not both on a global basis and a per-package basis.

[voxpelli]

@arcanis How do you see the status of this RFC and the ideas that npm puts forward, when seen in the light of the work in v2 here?

Will the work in v2 be synced with npm in any way?

Should this RFC be updated in any way to take the work you mention into consideration?

[arcanis]

Will the work in v2 be synced with npm in any way?

Not particularly. Working with npm proved emotionally exhausting for me during this past year, and I've decided to stop being the only one of us making efforts to be the good neighboor, waiting months for them to share their wisdom, or seeing our features rebranded without so much as a notice.

Even now I'd be happy to include them and discuss as peers, but it takes two to tango and I've lost hope seeing it happening under the present circumstances. npm be npm.

Should this RFC be updated in any way to take the work you mention into consideration?

That sounds like a good idea - would you like me to update it? Or you can pop into Discord to discuss it, both are fine by me 🙂

[arcanis]

I'd like to ask everyone to assume good intent when addressing someone else. If you disagree with something someone else says, please consider that they believe that what they're doing is the right thing. In this context, using strong words like "absurd" is best avoided.

Anyway, I believe the use case for the feature described in the RFC has been solved under the form of the new field dependenciesMeta.built, currently exclusive to the v2. Together with enableScripts it can be used to form either of the two common modes (exclusion list / inclusion list), and supports targeting packages by name and versions alike. What do you think, @voxpelli?

[voxpelli]

Very neat @arcanis 👌 I would very much say that it solves this and it does so very elegantly! Thanks 🎉