Add --ignore-(script)-scripts


(Tierney Cyren) #1

One of the security issues I hear most commonly is around pre-install/post-install scripts in modules. We’ve only seen a few instances of this actually being maliciously used AFAIK, but it’s one of the ones that people tend to focus on.

From a discussion with Laurie, I was thinking about the possibility of introducing some more granularity to the already existing --ignore-scripts flag. This granularity could hopefully be implemented simply like this:

--ignore-(script)-scripts

Where (script) is the name of an npm script. This could hopefully be a wildcard, which would follow the same behavior that’s documented in the npm scripts documentation:

Additionally, arbitrary scripts can be executed by running npm run-script <stage> . Pre and post commands with matching names will be run for those as well (e.g. premyscript , myscript , postmyscript ). Scripts from dependencies can be run with npm explore <pkg> -- npm run <stage> .

So, if I want to ignore custom scripts OR ignore normalized scripts (like preinstall, posttest, prestart, and so on) I could just run something like the following flags:

npm install --global --ignore-install-scripts
npm i -g --ignore-test-scripts
npx nodemon --ignore-prestart-scripts # I have no clue if this is would be simple but it would be stellar to have the same implementation in npx. If this is contentious/difficult, the main CLI as a priority is 100% okay! 

(Kat Marchán) #2

We don’t run test during install in most cases, unless a separate instance of npm or another package manager is invoked by the script itself (in which case, this flag won’t be passed in). Literally --ignore-install-scripts would have the exact same effect as --ignore-scripts. I’m not sure what the value in this one would be in practice.


(Tierney Cyren) #3

So the value I see from it is that I can be more granular about what I do and don’t want to allow npm scripts to do. For example, if you run the following commands you’ll be able to run the tests of good-first-issue:

git clone git@github.com:bnb/good-first-issue.git
cd good-first-issue
npm i && npm i standard -g && npm i jest -g
npm test

But, if you add the following line to the project’s .npmrc:

ignore-scripts

You can’t run the project’s npm test and I’m guessing you also couldn’t have the the project’s prepublish script be triggered (which is ETOOUSEFUL :grin:). This is an unfortunate workflow destroyer to prevent preinstall and postinstall scripts from being used.

The solution I’ve proposed would be less prescriptive than a command to just block pre/post install scripts while also giving users a suite of control they didn’t have before. If I, for example, wanted to block install scripts and start scripts (I never use npm start so there’s no legit reason those scripts would ever be used), I could add this to my .npmrc:

ignore-install-scripts
ignore-uninstall-scripts
ignore-start-scripts

And the rest of my scripts – like test and prepublish – would work just fine still.

Hopefully that clears up my intent and use case?


(Daniel Stockman) #4

With the exception of the install lifecycle, no script from any dependency (transitive or otherwise) is ever run (okay, prepare for git deps ;p). The only other scripts are local scripts, which I don’t really see the utility of blocking in a granular fashion since one can simply …not run them?

If you always ignore scripts during install, you’ll also break every package with a binary addon (fsevents, etc). It doesn’t seem feasible to develop a heuristic for determining which postinstall lifecycles are “allowed” to run in such a scenario you envision.


(Rebecca Turner) #5

Couple of things:

the npx nodemon --ignore-prestart-scripts doesn’t apply. npx doesn’t run start scripts. It runs bins bundled with the module. If the module is hostile those bins can do whatever. If you could even attach a script (and you can’t) it would have no access that the code in the same module doesn’t already have.

I find the fascination with install scripts as weird, because:

I can just put malicious js in the module instead of in the script and it’ll be loaded the first time you use it. No amount of script hygiene can help you there. And if you look at actual attacks, despite scripts being available, that’s still a popular vector.

So what this gets down to, I’d like to see a discussion of the attack vector that --ignore-install-scripts is supposed to address discussed first, and then how --ignore-install-scripts or related option would actually mitigate it.


(Tierney Cyren) #6

Having worked with large companies that care more about security than about breaking a module here or there, I feel comfortable saying this is a 100% acceptable problem if this is opt-in. I can think of a handful of the world’s largest Node.js and npm users that would turn this on without a second thought and deal with the modules that break.


(Tierney Cyren) #7

100% get this. That said, everyone I talked to who was running nodemon wasn’t using it via npx – it was a devDependency. Not saying npx nodemon users aren’t out there (I’m sure they are), but it wasn’t the common avenue I was seeing. This is of course biased information based on the people I talked to and my own circles (they certainly lean more towards classical Node.js and JavaScript vs. modern Node.js and JavaScript). Nevertheless, these were people who are vocal about security in our ecosystem.

You’re 100% correct. Tooling like Certified Modules can help with this, as can manual code review. Heck, even building your own tooling to do some analysis is possible. I don’t disagree with you at all, but I would add the caveat that I do think they are slightly different.

With npm scripts you can run shell commands without additional tooling/dependencies/code. You don’t need to create a child process. it’s very simple, which is incredible from a DX perspective but perhaps not so much from a security perspective.

This suggestion isn’t meant to solve all security problems, because I honestly think that’s impossible. Nevertheless, it would provide a path to remove one of the lowest barriers to entry for executing malicious code without sacrificing the incredible DX that the npm CLI has built out.

Addressing what you said about the past incidents vs. --ignore-install-scripts. Before left-pad we’d not seen someone delete a module that was so widely depended upon. While not a vulnerability, a few different measures were put into place after the fact. I could be wrong, but I’d guess y’all would have placed those or similar measures into the platform if you’d known left-pad was going to happen to mitigate it proactively. This kind of feature around install scripts (and other scripts, per my definition of the feature should it be accepted!) could certainly be seen in a similar light – a preventative security feature rather than a retroactive one.

As someone who regularly goes to bat for why npm is a good company, why the CLI is a good tool, and why the registry is basic infrastructure for continuing to enable the world to work as it does today, I feel really comfortable saying that perception is reality when it comes to npm and security… and install scripts are one of the issues that creates the strongest negative perception, whether or not they are actually one of the largest issues.

There are definitely people like those who’ve engaged in this thread already who understand the benefits and edge cases of security and the npm ecosystem. There are others who… aren’t there yet.

When others are out there advocating, I’ve seen “but install scripts” come up more often than not as a way to dismiss the npm ecosystem as more of a security risk than the world’s largest registry of instantly useful code. Being able to say there’s a damn simple way to disable that would both help npm advocates in the ecosystem and help individual contributors work more effectively when their teams and companies are more restrictive than is 100% necessary.

Hopefully that clears up some of my thought processes behind this and why I’m on board with your assertion that published code is equally if not more harmful but also think this would be an incredibly valuable, proactive addition.


(Liran Tal) #8

I agree with Tierney’s concern about seemingly poor usability of the ignore-scripts option but seeing a similar suggestion raised in the past and didn’t get traction either I’m not sure what else is required to convince and make the point.

That said, I see Rebecca’s point too with regards to the prestart script perhaps not making much sense. I can’t help but wonder if we’re not over-complicating things and just updating the behavior of ignore-scripts to ignore install (or other lifecycle event scripts) but not an own project general script.

To make it clearer, I’m asking what would be the use-case where a user would want to completely disable all run-scripts?


(Daniel Stockman) #9

Speaking for Lerna, we use --ignore-scripts when calling npm install during lerna bootstrap because we want to run the {pre,post}install lifecycles at a different point in the process (after all the potential symlinks to sibling dependencies have been created), not immediately.


(Liran Tal) #10

Understood and agreed, however that’s a use-case of a tool that wraps npm and is very specific to package installs. Even if I were an avid user of lerna for some project, most of my work with other projects on the npm ecosystem would probably not revolve around lerna or similar tools, and will require me to invoke npm directly.


(Dominykas Blyžė) #11

Implemented some of the related ideas as a separate package: https://www.npmjs.com/package/allow-scripts

tl;dr:

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


(Kat Marchán) #12

I like this approach a lot, @dominykas!

I would love to have this particular approach, including the user prompting on installations, as an RFC and I think this has a very good chance of landing. Anyone interested in writing such a thing? Are you?


(Isaac Z. Schlueter) #13

Yeah, I agree. Ideally, I’d like to be able to have some workflow where scripts prompt for user confirmation by default, with an yes/always/no/always-no kind of option. (And maybe “same answer for all remaining scripts”.) If you choose “always” or “always no” then it’s written to the root package.json as appropriate.

So, the data model might be a bit more like:

"runScripts": {
  "allow": { "package": "versionRange" },
  "deny": { "package": "versionRange" }
}

Or something like that? Then if the user doesn’t have a setting, we prompt them for input. Some work to be done to figure out the ideal solution for headless installs. Maybe just deny anything that isn’t explicitly allowed in non-TTY mode, since presumably you’ll have made a decision about it and saved in your app’s package.json before pushing to CI/CD or wherever it’s happening headlessly?

Allowing by default in non-TTY mode sets up a troubling security issue, where an attacker can trigger a subprocess npm install which is then implicitly allowed. We technically have that issue today, but at least everyone knows about it. If we start going down the road of letting people feel safe that scripts won’t be run without their approval, they might get more lax about checking for this sort of thing.


(Pelle Wessman) #14

Original author of the Yarn RFC here :wave:

I’ll happily update the Yarn RFC with any conclusions from here + write an equivalent npm RFC if not eg. @dominykas wouldn’t prefer to do that instead.

I do like the evolvement of the syntax and behavior that @isaacs is proposing.


(Tierney Cyren) #15

Love the solution that @isaacs proposed. Additionally, I assume this wouldn’t interfere with my project’s scripts like npm test or npm run build since it seems to just apply to packages, right?

One question I do have: How does this propagate down the dependency tree? If a dependency allows its dependencies to run scripts but I decide to block that dependency’s scripts, would the decision to block propagate down the tree?


(Joel Chen) #16

For companies with large internal NodeJS ecosystem like us, this is becoming more important.

I think @isaacs’ proposal is great.

Our primary concern is with automated CI systems and sometimes even npm install on deployment as some of our teams actually does that. So some more help to maintain global control would be nice.

  • globally default npm to never run any scripts during install (I think we can do this through configuring our build systems with /etc/.npmrc or prefix)
  • a way to specify a global white list config
    – through .npmrc?
    – specify a remote file that npm downloads?

Some other thoughts on specifying what packages are allowed/denied, in addition to versionRange:
I don’t know the actual feasibility of doing these so this is just a wishlist.

  • by scope (entire scoped packages are default to allowed/denied, but can be override with more specifics)
  • by package deprecated status. if it’s deprecated, then default to deny
  • by npm audit? any packages with vulnerabilities are default to deny
  • by package name, including regex?

Bottom line is, we want to be able to default our automated systems to be default to safe, some way to have a allowed/denied list that’s applied to all, and finally let users have finer control in their package.json.


(Isaac Z. Schlueter) #17

Oh, yeah, I guess a per-package .npmrc is another option. But that tends to be less visible, just from a UX pov.

I would not want to treat settings in dep package.json files as anything other than informative. Only the root project should decide what gets run. Consider:

my-project { "runScripts":{"allow":{"a":"*"}}}
+— a  { "runScripts": { "allow": { "b": "*" }},
   |    "scripts": {"install": "./known-good.sh" }}
   +— b { "scripts": { "install": "./maybe-evil-script.sh" }}

When I run npm instal a, I’d get prompted to allow ./maybe-evil-script.sh for the b install. The prompt could indicate that this is trusted by the a package, and I could use that in my assessment. (Do I know the a author, or is it some stranger? etc.)

In any event, it gives me a moment to go off and investigate.

@bnb No, I assume this wouldn’t have any effect on stuff like npm test.

@jchip Presumably the whole thing would be configurable via a config option. I’m sure @zkat or @iarna might have better ideas, but if (a) headless installs always deny, and (b) there’s some kind of way to say --run-scripts=never to mean “I don’t care what package.json says, really never ever”, or --run-scripts={no,yes,always} to pre-answer the questions in headless mode, then that should be plenty of flexibility for varying security stances.


(Dominykas Blyžė) #18

I’m totally interested in writing that RFC, but seeing as there’s a Yarn RFC already started, it’s a bit of a duplicated effort. I also have very limited time resources right now, so I won’t mind if someone else does. If nobody else does - at some point I might just do it, but now that I have the PoC package released, it no longer hurts as much.

I’ll be more than happy to update allow-scripts to whatever the standard emerges.

In your proposal you have allow and deny as separate entries. Is that so you can deny/allow based on package version? Or is there some other use case you’re thinking of?

The way I have implemented that in the allow-scripts is to have a blanket "packageName": false, rather than a separate section for deny list. Having separate allow / deny sections also means that you have to resolve the conflicts between the too, etc - I felt that it’s simpler to have that in one place.

Additionally, I thought that if there is a need to make this more complex in the future, it can be extended by making the value an array or an object, e.g.

"runScripts": {
  "package": [
    { "allow": false },
    { "range": "1.x", "allow": true },
    { "publishedBy": "Prolific Author", "allow": true }
  ]
}

But that said, this is probably a case of massively over-engineering it.

I agree that prompting for a missing entry is a good idea (allow-scripts crashes, but that’s obviously less than ideal). In CI, though, it possibly should abort on missing entry? Silently allowing defeats the purpose and silently denying might make things go unnoticed and break. Locally you could also enable abort-on-unknown via --strict-scripts or --no-prompt or similar (and via equivalent in .npmrc).


While building this I had a question… does this apply only to install (and uninstall) scripts? Are there any other cases when dependency run scripts get executed as part of some process?