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

Make npm install scripts opt-in #488

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tolmasky
Copy link

@tolmasky tolmasky commented Nov 5, 2021

Install scripts that can run just about anything by default pose some pretty serious security considerations, and these are inreasingly moving out of the theoretical realm and becoming actively exploited. See for example here: https://therecord.media/malware-found-in-coa-and-rc-two-npm-packages-with-23m-weekly-downloads/.

At the same time, we have developed better techniques for many of the most common use cases for install scripts in the many years since npm originally included then. In particular, N-API offers a compelling alternative to binary packages that are built on the users' computer. However, even before this, many packages are choosing to just pre-build for multiple platforms ahead of time to handle most of the common installation targets and make the install process easier on the user in general.

Instead of by default always running the install scripts (preinstall, install, postinstall, prepublish, preprepare, prepare, postprepare) if they are present during the install process, provide flags to require users to explicitly allow them to run, either whoelsale as "one big switch", or on a package by package (and optionally version by version) basis. Also provide matching npm config options to do the same globally and permanently instead of on every install.

… pretty serious security considerations, and these are inreasingly moving out of the theoretical realm and becoming actively exploited. See for example here: https://therecord.media/malware-found-in-coa-and-rc-two-npm-packages-with-23m-weekly-downloads/.

At the same time, we have developed better techniques for many of the most common use cases for install scripts in the many years since npm originally included then. In particular, [N-API](https://nodejs.org/api/n-api.html) offers a compelling alternative to binary packages that are built on the users' computer. However, even before this, many packages are choosing to just pre-build for multiple platforms ahead of time to handle most of the common installation targets and make the install process easier on the user in general.

Instead of by default always running the install scripts (`preinstall`, `install`, `postinstall`, `prepublish`, `preprepare`, `prepare`, `postprepare`) if they are present during the install process, provide flags to require users to explicitly allow them to run, either whoelsale as "one big swtich", or on a package by package (and optionally version by version) basis. Also provide matching `npm config` options to do the same globally and permanently instead of on every install.

Reviewed by @tolmasky.
@tolmasky tolmasky changed the title Install scripts that can run just about anything by default pose some… Transition to a model where install scripts must be explicitly allowed during the install process Nov 5, 2021
@tolmasky tolmasky changed the title Transition to a model where install scripts must be explicitly allowed during the install process Make install scripts opt-in Nov 5, 2021
@tolmasky tolmasky changed the title Make install scripts opt-in Make npm install scripts opt-in Nov 5, 2021
@WebReflection
Copy link

WebReflection commented Nov 5, 2021

Hello new comers from the mailing list 👋

This is a gently reminder that discussions and decisions are not taken after thumbs up and down or reactions, but after respectful conversations to align, eventually agree, and move forward.

I'll save you some time, this is exactly what happened here too, so you might want to skip right 'till the end of this discussion to screen latest chats and see were we've headed, thank you 👍


Install scripts that can run just about anything by default pose some pretty serious security considerations

This is nothing new in the software world, to be honest, when you install pretty much any software in Windows it asks you if it's OK to give this software the rights to configure the OS on your behalf and everyone is like "sure thing, whatever mate, let me move on" ... and here there's no difference ... except we miss the TRUST around packages.

What is trust? The fact a package has every allowed publisher behind 2FA to start with, so that publishers are accountable.

Every example to date shows that everyone can steal and publish on behalf of everyone else any kind of crapware, and yet the proposal here is to "provide yet another flag"?

What is a flag good for, when it can become a configurable default? It would solve nothing around the well known issue.

If there's anything npm should do, in my opinion, is to promote 2FA modules any way you can:

  • npm info module shows a special star, flag, check, about security/accountability because of 2FA
  • the site itself down-votes any search if the module is not 100% behind 2FA (or its developer/s)
  • a warning is shown on install ... or ... no pre/post install script is allowed if the module has not 100% 2FA publishers

Now this is thinking in a way that will stop causing the same accident over and over, where developers reputation also could play a role, so that trust become the way to move forward, as opposite of limiting all npm functiomalities because "evil happens".

Thanks for listening, and I hope yet another flag won't be the only move here 👋


P.S. we've been there before with HTTP (not S) and credit card payments or any sort of man in the middle attack, I don't understand why it's so difficult to see how much value a 2FA only ecosystem could bring to the npm's plate

@tolmasky
Copy link
Author

tolmasky commented Nov 5, 2021

This is nothing new in the software world, to be honest, when you install pretty much any software in Windows it asks you if it's OK to give this software the rights to configure the OS on your behalf and everyone is like "sure thing, whatever mate, let me move on" ... and here there's no difference ... except we miss the TRUST around packages.

This is actually a very different situation, as you are comparing the behavior of non-technical users installing non-technical software, vs. developers who do generally care (if allowed to) about the software they use. The other critical difference this analogy misses is that a very large portion of package installs are done not by humans, but in an automated fashion. So it is not a bunch of people saying "OK whatever," it is a fleet of CI machines repeatedly pulling packages and by default running scripts that could compromise them. A package going from not running an install script to running an install script between CI runs should IMO absolutely fail the test: there is a very bizarre change in side effects taking place, and test failures are an expected part of test suites (it's the whole reason they exist), and at that point you can issue another commit that adds the explicit approval of the package, or, as with what happened earlier today, say "wait a minute, none of these should be installing", and avoid having a bunch of junk infect your CI machines.

Another important piece of context is that vast majority of people will actually see no change, as a tremendously small amount of packages on npm today use this feature. If this was some critical package.json field that 10% of packages used or something, then that would be one thing, but the reality is that when we last checked every package (literally every package on npm) at RunKit, the percentage was vanishingly small.

Additionally, many of the packages that do use this legitimately now have better alternatives. A clear example is transpilation that happens on install instead of on publish (which additionally means we're wasting a bunch of energy retranspiling the same code over and over instead of shipping the "built" package). As mentioned in the PR, N-API also provides a better solution to actual binary packages.

What is a flag good for, when it can become a configurable default? It would solve nothing around the well known issue.

If you read the actual configuration options provided, you will see that there isn't a blanket option, only allowing you to globally allow "safely" a specific version of a package, and "unsafely" any version (or range) of a package. In other words, if you encounter that one random package that you know and love that has an install script, you can enable that one "from now on", which makes sense, you are saying "I've checked this package version, I'm good with it". This is very different than saying "I don't care about scripts running just do whatever with anything".

Thinking about the distinction between "automated" installs vs. "user drive installs" also informs the thinking around configuration vs. flags, and why both are provided to some extent: it makes total sense that one would have a different risk model for an individual developer's computer vs. a CI or production machine. If a developer accidentally installs something malicious on their computer, that could be relatively contained. However, that can be a huge profit loss if it manages to install on a production machine. So it makes a lot of sense to use the most explicit flags in the repo, but for individual users to be able to globally turn on certain packages for convenience.

Incidentally, you can even imagine a model where when running npm install interactively it can ask you about packages instead of failing (the same way apt asks you about disk usage), that way this process has even less friction.

Every example to date shows that everyone can steal and publish on behalf of everyone else any kind of crapware, and yet the proposal here is to "provide yet another flag"?

The proposal is to make unexpected behavior default-off. It's like getting angry that browsers now require a iframe attributes to explicitly enable certain features -- no, it should have always been that way, and the people working on it (developers, not end users) are perfectly equipped to understand and update their code. Except again, most people will not even need to use this flag, as the legitimate use cases are small, and only growing smaller. Again, to the point where if this were implemented with a grace period to update packages, very quickly there would be very few packages that "need" this flag. I would argue that this would for example take much less time than the ESM transition that is currently underway. Also, as mentioned in the PR, if the flag is really that bothersome, you can globally allow packages to install.

Now this is thinking in a way that will stop causing the same accident over and over, where developers reputation also could play a role, so that trust become the way to move forward, as opposite of limiting all npm functiomalities because "evil happens".

Trust doesn't fundamentally solve the problem of purposefully malicious actors. If someone steals your 2FA keys and publishes a malicious package, no amount of "ratings" or "stars" in npm info can account for that, especially not before hand. There are two issues at play here: accountability (how to deal with things once they go wrong), and security (how to prevent things from going wrong in the first place). There doesn't exist an accountability system on Earth that would make me feel comfortable with giving browser JavaScript access to my filesystem by default. I want it sandboxed, no matter what other measures are taken.

@WebReflection
Copy link

WebReflection commented Nov 5, 2021

you are comparing the behavior of non-technical users installing non-technical software, vs. developers who do generally care

beside the fact we don't have data around developers that really care VS developers that copy and paste whatever they find on SO or elsewhere, this proposal would break genuine software like this which needs mandatory postinstall, and everything not having it allowed by default would just break.

A breaking change like this in npm needs to take into account all use cases to date (Electron based Apps too), not just CI, but I'll read the rest later, as I think the first line is already a no-go to me, sorry.

@wesleytodd
Copy link

While this is a laudable goal, this is not the first time this has been brought up, and very little has changed now compared to before. This would be a MAJOR breaking change for the ecosystem and despite agreeing that opt-in would have been a better default, all it would do is shift the attack to the next easiest way to compromise the supply chain (post-install scripts currently being the easiest right now in the npm ecosystem). There are no easy fix-all's when it comes to running third-party code, and I am still not sold on the juice being worth the squeeze on this one.

IIRC, the last time this was brought up it was also brought up that the current config is a "all or nothing" thing (although I seem to recall some changes to that recently, so I might be wrong now). IMO, a much better starting point for this discussion would be incremental improvements which would not break the entire ecosystem all at once. I have not been following the RFC calls or PR's much recently, but I am sure someone who has could point to a few others in this space which have been discussed.

@WebReflection
Copy link

WebReflection commented Nov 5, 2021

I am proposing "2FA or nothing" thing instead, which seems way more sensible to me, still as breaking change.

Can anyone please read my suggestion as a long time needed extra, not just as a blocker? Thank you!

@wesleytodd
Copy link

wesleytodd commented Nov 5, 2021

I am proposing "2FA or nothing" thing instead, which seems way more sensible to me, still as breaking change.

Have you opened an RFC for this? I would love to see a --require-2fa config which would fail install if any packages were not published with 2FA as long as it had a good set of tools for ignoring things. Seems like a better way to respond to a credential theft as is the source of these recent issues.

@tolmasky
Copy link
Author

tolmasky commented Nov 5, 2021

A breaking change like this in npm needs to take into account all use cases to date (Electron based Apps too), not just CI, but I'll read the rest later, as I think the first line is already a no-go to me, sorry.

That's certainly unfortunate, considering that had you read the RFC before this, it explicitly discusses a strategy of having a warning transition period to give everyone time to update (Electron apps can trivially take this time to add the flag and avoid any hiccups whatsoever, completely transparently to their users. I'm also not even sure this is an issue since Electron apps can ship "built" and don't have to necessarily themselves npm install on users' machines, but again, that would itself be mitigated completely with a one-line change). Anyways, your goal of showing me disrespect has succeeded, by declaring that a huge post that tried to in good faith address your comments isn't even worth reading since you didn't like the opening sentence for a reason that was covered in the original RFC. This however does not prevent you from continuing to criticize.

@wesleytodd
Copy link

Hot button issues with folks passionate in the ecosystem can get tough to navigate, but I recommend taking a moment to listen. There will always be someone who disagrees in any sufficiently populated OSS community, and that is a good thing. In this case, I think this issue has made SOOO many rounds, and this solution has been recommended and talked through before, that it might lead to folks who have been involved in them in the past to jump to conclusions.

I think we can all agree that you did a great job writing up the RFC @tolmasky, and I highly doubt that going back in time and being able to change things before it was so entrenched, anyone would disagree with you. I would recommend letting folks comment on the RFC, and then attending the next RFC call. The folks who regularly attend are great, and really open to discussion.

@peey
Copy link

peey commented Nov 5, 2021

Install scripts that can run just about anything by default pose some pretty serious security considerations

Since this presupposes that the package you're installing might have malicious code, how do you propose to handle the threat posed by running your own trusted code which imports from these untrusted libraries?

@tolmasky
Copy link
Author

tolmasky commented Nov 5, 2021

While this is a laudable goal, this is not the first time this has been brought up, and very little has changed now compared to before. This would be a MAJOR breaking change for the ecosystem and despite agreeing that opt-in would have been a better default, all it would do is shift the attack to the next easiest way to compromise the supply chain (post-install scripts currently being the easiest right now in the npm ecosystem). There are no easy fix-all's when it comes to running third-party code, and I am still not sold on the juice being worth the squeeze on this one.

A number of things here, hopefully some of which ew at RunKit can be helpful with. We have to regularly analyze the usage of install scripts on RunKit, since we try to install every combination of every version of every package on npm, specially if we can join forces with npm's side of the data. I feel fairly confident that this can be done in a non "break the world" fashion, especially because I think many of the remedies can be handled by authors vs. the developers using the packages. So, in particular:

  1. We should of course no matter what have a transition period where npm simply warns about install scripts.
  2. I think there are also some "common sense" user-driven heuristics that could mitigate many of the problems completely. For example, if a package-lock.json is present then you can don't need to require the flag. This is because there is no danger of picking up a random patch update in this case, you are only installing items you've already installed in the past, and even checking against the checksum. If you wanted to be super safe, you could only apply this rule for package-lock.jsons generated before the "opt-in" transition (this can be determined by the presence of a property in the package lock) This means that you are truly catching the most egregious cases of installing unlocked packages that may pick up unintended script additions.
  3. I think we could fairly reasonably establish the "affected radius" of users by combining download data and dependency-chain info. We could thus establish a fairly good metric of "safety" to reach before pulling the trigger on anything that would have any actual end-user affects.
  4. During this period we can assist package authors in getting their packages updated. We've also done a lot of work in trying to group "install script usage", so we could fold in some "declarative" alternatives for a lot of the "piggy-backing off of install just because its the only way to do it" use cases. For example:
    • Providing an official way to ask for donations, so that doesn't have to go through install (I can't remember if this was already possible, apologies if this is already the case, but if so, then certainly letting people know that if in a year they don't switch to the official declarative support.md format or whatever their packages will require an install flag will probably go a long way to getting everyone transitioned).
    • Providing a system for declaring and installing "outside of npm" dependencies, either as a check, or as an install. We've actually implemented this ourselves for our own packages internally, something like "aptDependencies" and "aptRequirements", where npm is responsible for either making sure you have certain dependencies or fetching them. This would also just be a great ecosystem move to further "surface" the dependency structure of npm, and make it way easier for people to convey that their packages require certain tools to be available, etc.

With regard to the "bar" we're trying to reach with this strategy, I think the goal is of course not to fix everything, which isn't realistically possible. I do however think we can make reasonable progress to make "passive" attacks considerably harder to execute (where packages don't even need to be run for an attack to be successful).

@madjam002
Copy link

Seems like a lot of work for the entire ecosystem when all malicious authors would have to do is just switch to running malware on first run via require/import instead which would probably happen as soon as this was implemented.

Perhaps a flag instead to refuse to install packages that have been published in a recent time period or at least flag them interactively? That way each individual user or organisation could configure their preference on how long packages have to have been in the wild before they even get downloaded onto their system.

@rauchg
Copy link

rauchg commented Nov 5, 2021

Very supportive of this proposal. .*(install|prepare) scripts not only decrease security, they slow down installations, complicate caching and remove reproducibility by introducing side effects, and pose challenges for cross-platform compatibility.

These scripts can't be part of the "happy path" of npm. They need to be painful to use and consume and phased out of the default path of installation.

I thoroughly agree with the sentiment that the ecosystem having developed an excellent alternative for installing pre-built binary addons targeting multiple platforms means "this time is different" (I'm cognizant this has been proposed many times before)

@peey
Copy link

peey commented Nov 5, 2021

For example, if a package-lock.json is present then you can don't need to require the flag. This is because there is no danger of picking up a random patch update in this case, you are only installing items you've already installed in the past, and even checking against the checksum.

I like this idea. I think defaulting to trusting the scripts when a package-lock is present and not trusting them when it's absent is sensible.

@tolmasky I'd also encourage you to file a proposal with yarn and pnpm. npm has been conservative about accepting innovations unless they're already popularized by competing package managers.

@tolmasky
Copy link
Author

tolmasky commented Nov 5, 2021

Since this presupposes that the package you're installing might have malicious code, how do you propose to handle the threat posed by running your own trusted code which imports from these untrusted libraries?

This is a great question and I touched on it briefly in the response above, but I think it's worth addressing clearly, if only to establish expectations: this isn't meant to target those threats (but there are other ideas out there that do!). The goal of this RFC is explicitly to handle a unique class of "passive" threats that exist by simply being present in the dependency chain. This can make them particularly nefarious precisely because they don't need to be imported or run. This fact can make them easier to sneak into codebases (since people often don't review "metadata" files as thoroughly as code files, so the addition of an innocuous package may not raise any eyebrows, or the even harder to find two step approach of initially getting a "safe" package in, only to then modify its internal dependencies afterwards), and can also make these attacks "resistant" against mitigations targeting imported code, since something like code isolation won't help you if the problem is happening during the install. Ultimately, these different kinds of attacks will require different strategies, but I think the install script trick has made the barrier to entry to create these kinds of hard-to-spot "passive" attacks very low.

@mattisdada
Copy link

I think this is a great proposal and have no real issue with it. But, an alternative that would move the burden from users back to NPM is that packages with install must be reviewed and approved by NPM for every publish and probably require an additional cost for said review and 2FA.

I like this proposal more, but if people really don't want breaking changes, this would not cause any breaking changes for end users and motivate publishers to use alternative methods... Just a thought

@dominykas
Copy link

dominykas commented Nov 5, 2021

I'm not sure I agree with the "next goal post" thinking as a reason to stay put. We do not have an easy way to evaluate package contents without installing them (side note: something to fetch the tarball without installing would be super useful as well). Yes, you can run every install with --ignore-scripts, but that's crap DX. So yeah, the next goal post does exist, but that doesn't mean we need to continue suffering and preventing actual improvements in the process for people who do care.

A blanket "always ignore install scripts" as opposed to the currently available "always ignore all scripts" (which does not allow anything npm run, last I checked) would be useful.

It doesn't have to start as opt-in. A more comfortable opt-out would be better than what we have now.

And yes, 2fa would be nice. I wish we could revive the staging discussions (#92, one approach for 2fa for automation). I also have nodejs/package-maintenance#282 parked for a very long time.

Not going to hold my breath, though, see you back here in a year. Notice how it's always around budget planning season that these things are more common? Maybe someday the budget for securing things will actually happen...

@peey
Copy link

peey commented Nov 5, 2021

The goal of this RFC is explicitly to handle a unique class of "passive" threats that exist by simply being present in the dependency chain. This can make them particularly nefarious precisely because they don't need to be imported or run. ... This fact can make them easier to sneak into codebases (since people often don't review "metadata" files as thoroughly as code files,...

I see. If I understand correctly, the attack vector here is code which is contributed by an attacker who does not have commit rights to repo or publishing rights to the package, and then the contribution is merged by a maintainer.

Do you have any recent examples of this so it's easier to understand how such an attack might be carried out? In the examples you have given (coa, rc) the attacker had access to package developer's account (publishing rights).

@totten
Copy link

totten commented Nov 5, 2021

A slightly biased/tangential anecdote: PHP's composer went the other way from the start. They never allowed library-packages to run their own installation steps. (Only root-level/consumer-packages could define install steps.) It was a recurring request for a few years (eg composer/composer#1193), and (from my POV, as a downstream dev) I felt it held composer back from doing some useful things. But I do appreciate that it reduces the attack-surface and has probably discouraged some supply-chain attacks.

Similar proposals were raised to allow installation logic with some kind of whitelist. The ship for that probably sailed a few years ago, but... eventually (late last year) I implemented a version of it. So you can see an example of such behavior here:

Screenshot

For installers/upgraders, it did introduce an extra prompt. I was a little concerned about potential for user-pushback. (Available signals suggest ~1000 of our deployments do composer-based administration.) So far nobody has complained about the extra workflow step being onerous.

@lewis262626
Copy link

lewis262626 commented Nov 5, 2021

@totten Doesn't that have the same problem as Windows' UAC prompt -- every single user just clicks yes without a second's thought. Appreciate composer has more options that just yes and no but I think the point still applies.

@MylesBorins
Copy link

MylesBorins commented Nov 5, 2021

Hey all. Myles from the npm team here. I've been trying to think through how we get to a place where we can disable install / postinstall scripts from running by default.

The biggest gap imho, as others have pointed out, is lazy loading platform specific binary assets. This is something I think we should figure out how to support in the client in a static and consistent manner.

I think with that behavior in place we could safely change defaults in a Semver Major.

I'm just stepping away for the weekend and plan to come back to this when back in the office next week.

It's exciting to see all the momentum for this type of change, even just 2 years ago I don't think we'd see such wide support for such a large breaking change

@mirek
Copy link

mirek commented Nov 5, 2021

Wouldn't signing npms on publish with 2FA protected allowed public keys solve this class of problems?

@totten
Copy link

totten commented Nov 5, 2021

Doesn't that have the same problem as Windows' UAC prompt -- every single user just clicks yes without a second's thought. Appreciate composer has more options that just yes and no but I think the point still applies.

Good point. I don't have data for a systemic answer. Personally, I find it analogous to TOFU in SSH. During the first invocation, you don't really know what to expect, and many folks would blindly accept the suggestion. But then the message goes away (decision is saved into JSON); for most day-to-day tasks (git pull && composer install or git clone && composer install or even composer update), the validation is invisible. The message should only pop-up if something changes, eg

  • (SSH) If a host starts advertising a different public-key, then that is suspicious. The client prompts for confirmation.
  • (Package manager) If a package wants to increase its permissions, then that is suspicious. The client prompts for confirmation.

So trying to apply to the rc example - any downstreams newly adopting rc@1.2.9 would be liable to blind-acceptance. But all existing downstream upgrading from rc@1.2.8=>1.2.9 would get an unusual prompt.

Perhaps your question ultimately turns on "how frequently do SSH hosts change their keys" or "how frequently do packages try to increase their permissions"?

@ivarprudnikov
Copy link

For those who mentioned 2FA, there is still an issue with the automated publishing process that would usually use an access token. A token can be leaked, thus 2FA does not solve everything.

@Okeanos
Copy link

Okeanos commented Nov 6, 2021

Perhaps a flag instead to refuse to install packages that have been published in a recent time period or at least flag them interactively? That way each individual user or organisation could configure their preference on how long packages have to have been in the wild before they even get downloaded onto their system.

I am not sure I understand how this would solve this issue? Wouldn't that mean that in case of (critical) security patches everybody using this flag would've to disable it or else run insecure software? Another question I am asking myself here is: if this flag is opt-in instead of opt-out … the attack surface of install scripts isn't really reduced because a lot of people wouldn't know about the flag or use it. Additionally, if everyone waited for say, a week, before installing it would technically increase the window of time in which malicious scripts etc. can be found. But by whom? Who'd do the auditing of this? I currently imagine this would only postpone the inevitable and recreate a status quo "7 days later" (using the example wait window).

I am a casual user of npm/yarn and wouldn't like this burden shifted to me if a better alternative (e.g. like SSH TOFU as previously mentioned) appears to be feasible. 2FA publishing also sounds like a good idea to me, however, as an additional improvement not as an alternative to requiring explicit consent for install scripts by default.

@ngudbhav
Copy link

ngudbhav commented Nov 6, 2021

What if npm maintains an anti-malware database, just like a regular antivirus? Any install can then be directly halted if the package is marked as unsafe.
To make it a non breaking change, anti-malware can be used as a separate package. npm can issue a warning if installation is triggered without this anti-malware package.
Regular definition updates headache can be removed if the database is maintained online.

A single source of truth for all the vulnerabilities. This can help notify people via tweet/mail about recent alerts.
Organisations can ping the database regularly to find if the package is vulnerable or not.

To make it more streamlined, a simple set of rules like 2FA publish, author's information, last publish source can be kept.

1. `npm config set allow-scripts-canvas@1.0.0 true`: This will allow canvas version 1.0.0 to install scripts whenever `npm install` is used.
2. `npm config set allow-scripts-unsafe-canvas true`: This will allow any version of canvas to install scripts whenever `npm install` is used. It should also probably warn on each case.

## Rationale and Alternatives

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a scope section, your comment here is a good starting place
#488 (comment)


Two open questions for me:

1. Having npm ship with a grandfathered list of "known safe" package versions. For example, every currently shipping version of canvas. That way, for important large packages, this is a "from now on" requirement. Alternatively, there could just be a transition period where npm just warns about script, and then afterward we make it actually refuse to run them without the proper flags.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B. grandfather clause has a pretty nasty history, I try to use 'legacy'.

grandfather clause
a provision in several southern state constitutions designed to enfranchise poor white people and disenfranchise Black people by waiving high voting requirements for descendants of men voting before 1867


## Summary

Instead of by default always running the install scripts (`preinstall`, `install`, `postinstall`, `prepublish`, `preprepare`, `prepare`, `postprepare`) if they are present during the install process, provide flags to require users to explicitly allow them to run, either whoelsale as "one big swtich", or on a package by package (and optionally version by version) basis. Also provide matching `npm config` options to do the same globally and permanently instead of on every install.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please clarify if scripts are ignored or fail the install if they are not allowed to run.

- `npm install canvas --allow-scripts-unsafe canvas`: Installs the latest version of canvas to the current `package.json`, and allows it to run install scripts.
- `npm install canvas --allow-scripts-unsafe canvas --allow-unsafe-scripts ramda`: Installs the latest version of canvas to the current `package.json`, and allows it to run install scripts for canvas and any version of ramda it encounters.

### npm config

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The set of packages that can run scripts should be tracked in source control so developers or CI agents can install the projects dependencies without making decisions about which scripts to run. Choosing scripts to run should be done when adding or updating dependencies, not when installing packages that are already dependencies.

I think package.json is the best location for this option.

  • package-lock.json is ephemeral and may be deleted to get a fresh install.
  • .npmrc may or may not be tracked in source. 1

Footnotes

  1. If it is checked into source developers need to avoid checking in personal changes. For example a dev needs a proxy for one project but not another. If they set the config in project npmrc they must not commit that change, if they set the config in their user npmrc they must remember to remove it before working on another project. I should write a RFC for multiple project npmrc's.

@WebReflection
Copy link

@everett1992 about this

I think you're over indexing on 2FA tho I'm in favor of your suggestion

apparently it's not just me believing is a better way forward ;-)

Screenshot from 2021-11-17 00-25-08

@bmeck
Copy link

bmeck commented Nov 17, 2021

Could we get a STDEV on the total number of transitive dependent packages on packages with scripts?

Two open questions for me:

1. Having npm ship with a legacy list of "known safe" package versions. For example, every currently shipping version of canvas. That way, for important large packages, this is a "from now on" requirement. Alternatively, there could just be a transition period where npm just warns about script, and then afterward we make it actually refuse to run them without the proper flags.
2. I think the automatic node-gyp stuff should be under this umbrella as well. That is to say, node-gyp would only run when gypfiles are detected if the `allow-scripts` flag allows it, since you can also techncially put whatever you want in there I believe, and not doing this would simply mean that any malicious behavior would be transitioned there.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. I think the automatic node-gyp stuff should be under this umbrella as well. That is to say, node-gyp would only run when gypfiles are detected if the `allow-scripts` flag allows it, since you can also techncially put whatever you want in there I believe, and not doing this would simply mean that any malicious behavior would be transitioned there.
2. I think the automatic node-gyp stuff should be under this umbrella as well. That is to say, node-gyp would only run when gypfiles are detected if the `allow-scripts` flag allows it, since you can also techncially put whatever you want in there I believe, and not doing this would simply mean that any malicious behavior would be transitioned there.
## Prior art
- run scripts from allowlist [allow-scripts](https://www.npmjs.com/package/allow-scripts)
- run scripts from allowlist [allow-scripts from lavamoat](https://www.npmjs.com/package/@lavamoat/allow-scripts)
- tools to prevent/detect scripts running when not wanted [preinstall-always-fail](https://www.npmjs.com/package/@lavamoat/preinstall-always-fail) [pentest-my-ci](https://www.npmjs.com/package/@naugtur/pentest-my-ci)
- reviewing which scripts need to run to help build allowlists [can-i-ignore-scripts](https://www.npmjs.com/package/can-i-ignore-scripts)


Two open questions for me:

1. Having npm ship with a legacy list of "known safe" package versions. For example, every currently shipping version of canvas. That way, for important large packages, this is a "from now on" requirement. Alternatively, there could just be a transition period where npm just warns about script, and then afterward we make it actually refuse to run them without the proper flags.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a hardcoded package list is a very bad idea - this incentivizes authors to never publish updates, since existing known-good versions are privileged.

@naugtur
Copy link

naugtur commented Nov 28, 2021

If we disable scripts by default, we need means to run compilation etc. for some packages. npm rebuild could do that.
I have a poc of running arbitrary code if you run npm rebuild on something trustworthy.
Who should I talk to?

@Ilmarinen100
Copy link

Since this wasn't well received in the meeting in January and we had at least one more high profile hack through this path:
What if we make the install scripts opt out in a usable way?
Right now ignore-scripts=false in .npmrc is not practical as it breaks project-local scripts, too. Potentially there could be other solutions like a more fine-grained config key (or value for ignore-scripts), that specifically disables scripts for package installation.
I'm assuming here there is a large group of devs like me that usually do not want to run any install scripts (and that are sufficiently worried about security to turn on such a setting globally or per project).

@fengmk2
Copy link

fengmk2 commented Mar 24, 2022

fengmk2 added a commit to cnpm/bug-versions that referenced this pull request Mar 25, 2022
fengmk2 added a commit to cnpm/bug-versions that referenced this pull request Mar 25, 2022
fengmk2 added a commit to cnpm/bug-versions that referenced this pull request Mar 25, 2022
@mbalabash
Copy link

Install scripts that can run just about anything by default pose some pretty serious security considerations, and these are inreasingly moving out of the theoretical realm and becoming actively exploited. See for example here: https://therecord.media/malware-found-in-coa-and-rc-two-npm-packages-with-23m-weekly-downloads/.

Just wanted to highlight that install scripts are not the only threat and there are cool tools that may help to mitigate those risks. I would suggest seeing sdc-check and scanner.

@naugtur
Copy link

naugtur commented Apr 1, 2022

LavaMoat

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

Successfully merging this pull request may close these issues.