Add option to require install script approval

[everett1992]

This is an attempt to revive #488 and control which packages can run install lifecycle scripts. Unlike Make npm install scripts opt-in this RFC doesn't change any default behavior. It adds a new flag that when enabled errors or skips unapproved postinstall scripts.

Rendered here

I'm able to implement this.

References

Related to #488
npm/npm#15144
npm/cli#619
npm/cli#538
#80

[wraithgar]

This problem space overlaps in a lot of ways with the new allow-git flag (which is soon to expand to other non-registry dependency types).

It is important that as we think about how to solve/configure for these use cases that it is done in a way that is applicable to these other approaches.

Script execution in npm has a fundamental problem: there is no distinction between your package.json scripts and the scripts in your dependencies. Even the solution for allow-git which says "root" "all" or "none" doesn't help because there is no concept of "git actions in the package.json itself" baked in.

This has always been the first problem to solve, how do we say "I want my own scripts to run but not my dependencies'" and now "I want my top level dependencies' scripts to run". The use case of allowing an individual package (at any depth or version) is also not addressed with this.

The "min-release-age" flag is now also coming up against this problem. How do we override this in a secure way (i.e. does not invite misconfigurations that reduce security), and is not a wholly unique approach to all other exceptions we might write?

Not to overload this rfc with scope creep, but I do want to have these things in mind as we architect here. Is there a paradigm or a pattern that will allow us to grow into these solutions without having to invent wholly new configs for each iteration?

To summarize, the end goal is to have a config approach for:

• non-registry dependencies
• min-release-age
• script lifecycle events
• others?

That allows us to

• distinguish between package.json things, top level dependencies, and subdependencies
• override in a consistent way that does not compromise security
• iterate with our solutions (we don't have to ship the every iteration just to have the feature)

[wraithgar]

--allow-git already exists, and the underlying packages already support --allow-remote --allow-registry --allow-file and --allow-directory, we just need to add those to npm itself. We can consider those "prior art" that already exists with the pattern of those allowed values being root all or none. Adding package into the mix for lifecycle scripts may be a logical place to start?

[everett1992]

IMO --allow-* root|all|none is not granular enough, I want explicit approval for each exception. But I also see the value in shorthands like 'all my direct dependencies'. All direct seems okay for allow-git but not for allow-install-scripts because your direct git dependencies are explicitly spelled out in source, while your direct dependencies can unexpectedly gain install scripts when upgrading versions.

Adding package:... or spec:... options to match nodes at any depth of the tree seems like a good addition for --allow-git et al. and can work for a new --allow-install-scripts.

The ability to share configurations across packages is important to me, because I plan to opt 200,000 packages into using an allowlist of install scripts. Without updating each package, or breaking their builds, I'll populate a list with known packages. Central configuration will reduce toil if a popular package adds legitimate postinstall scripts. I don't know how we would merge option into --allow-* arguments.

---

What if we reuse npm query, or a subset, and have policies that allow the matched dependency to be resolved or installed?

{
   // No git dependencies can be resolved or installed
  ":type(git)": { "allowed": false },
  
  // better-sqlite3 11.x may run install scripts
  "#better-sqlite3@^11.0.0": { "install-scripts": false },
  
  // Use latest release of @scope packages.
  "[name^='@scope/']": { "min-release-age": 0 },
  
  // Package is allowed to declare dev git dependencies
  ":root > .dev:not(.prod):type(git)": { "allowed": false },
}

There's a lot I like about this, but it probably fails to "not invite misconfigurations that reduce security"

[wraithgar]

What if we reuse npm query, or a subset, and have policies that allow the matched dependency to be resolved or installed?

We have discussed this a lot in the past but never implemented it anywhere. It does seem like the logical solution here.

[wraithgar]

Where would this policy be configured? The cli itself is often where it is frustrating to construct larger query strings.

[wraithgar]

Is there any value in distinguishing WHAT lifecycle events we want to allow? This seems like a bad idea since it may give a false sense of security (any of those scripts can have security issues) and is another layer of code we don't need to write if we don't want it.

[everett1992]

Is there any value in distinguishing WHAT lifecycle events we want to allow? This seems like a bad idea...

I agree, I don't think there's a difference between prepare or postinstall. Maybe distinguish between postinstall and implicit node-gyp?

I also don't think there's any point to check the scripts contents, like approve esbuild to run ./install.sh. There's too much possible indirection to say if its 'the same' script.

[everett1992]

Where would this policy be configured?

New file(s) configured by a cli flag. Multiple files allows a per-project config and shared global config.

[wraithgar]

npm already has a config file, adding a second file for npm to look at seems like a bad pattern.

[everett1992]

Whether it's a new file or not isn't important to me, but the ability to have a shared config that is extended by a package is. I didn't see precedence for that in .npmrc, it looked like project npmrc always overwrites global.

[wraithgar]

Yes, there is no way to "merge" configs across sources. That may be something we have to give up here.

[wraithgar]

Does the query language approach allow us to distinguish between the scripts in my own package.json and those of my dependencies? This is one of the core problems to solve to even be able to block install scripts in the first place without blocking the root scripts.

"root" in the context of "allow-git" means "git references I have in my package.json" so "root" would have to mean "my own local lifecycle scripts" for an "allow-scripts" flag. Unfortunately that leaves out "lifecycle scripts for top level dependencies" which is closer to what the analog means in "allow-git"

Trying to have something like "root" "all" "none" or "dependency selector" doesn't work because the dependency selector can't include "root". We need something that says "root and these limited dependencies" OR we need an extra config that works in addition that lets an 'allow-scripts' flag to limit to root/all/none and then also open up extra permissions through a dependency selector.

--allow-scripts=root
--allow-scripts-selector='.prod:root > :not(:outdated)'

The potential for confusion here is that the selector adds to the list. If I did --allow-scripts=all then the selector would be meaningless.

[everett1992]

I wrote in the RFC that root and workspace's scripts are always allowed. I can't imagine a use case for skipping the repo's own scripts, but running other package's scripts. --ignore-scripts would still exist as an escape hatch to install dependencies without running any scripts.

the dependency selector can't include "root"

Is that right? :root returns only the root package.

[wraithgar]

I wrote in the RFC that root and workspace's scripts are always allowed. I can't imagine a use case for skipping the repo's own scripts

The way the feature currently works is a very very helpful ux affordance. If I run npm test --ignore-scripts it means I want to run the tests without the posttest linting script that happens normally. I'd hate to break this functionality.

[wraithgar]

Is that right? :root returns only the root package.

Yes. Perhaps this is just me not fully understanding css selectors. How would I query "the root package itself AND its direct dependencies"?

~/D/s/bundled-test $ npm pkg get dependencies
{
  "semver": "7.5.0"
}
$ npm query ':root'|npx json -a name
gar-test
$ npm query ':root .prod'|npx json -a name
lru-cache
semver
yallist
$ npm query ':root > .prod'|npx json -a name
semver

[wraithgar]

Aaand it was just me being rusty

$ npm query ':root, :root > .prod'|npx json -a name
gar-test
semver

Selectors totally solve the entire problem space!

[wraithgar]

There are MANY more lifecycle scripts than just install. While we have decided not to filter based on script type, it's probably worth naming the config something less specific to just the install scripts. --ignore-scripts is the currentl config that turns these totally on or off, and "lifecycle script" is how they're referred to in the documentation.

[everett1992]

I wrote the RFC with 'scripts defined by dependencies' in mind, my goal is to limit the exposure to running arbitrary code when installing dependencies. That's why I didn't account for scripts in the root, or lifecycle scripts run on test/start ect non-install commands.

It makes sense to pick a more generic name if the setting can apply to the root package and it's other hooks. Could we repurpose the ignore-scripts option to accept query filters and apply the policies? Or allow-scripts to be consistent with allow-git and ignore-scripts?

[wraithgar]

Does this approach still make sense if we are using a query selector? We only have a few config items that point to files and we usually don't let those paths traverse outside of the project itself. Having a "user level" config with a user-defined path like this seems unwise from a security standpoint. I think we may just want to have these as bare config strings and folks can solve the "shared config" problem in the ways they already are for other config concerns.

[everett1992]

Do you have examples of how people solve shared configs problems in npm?
pnpm has config dependencies as a general way to share setting values.

I'll be able to populate a default allowlist in a global .npmrc, but package's won't be able to extend that, any local .npmrc would overwrite.

[ljharb]

It hasn't been much of a problem, so it's usually just symlinked npmrc files, or a separate script to set config values.

[wraithgar]

It's good to call this out and keep it. It largely solves the UX issue I was bringing up about "running tests without posttest" etc. This catchall will be what folks can use in those non-install cases.

[wraithgar]

This should apply to all lifecycle scripts. Let's not limit here. Exceptions usually have more surprises than rules that apply to everything. It also future-proofs us against new scripts that may happen to come up.

[wraithgar]

I left some more specific commentary inline. I hope you understand where I'm coming from with my suggestion that this isn't limited to just install scripts. This is admittedly coming from my own approach to having thought about this in the past, and having always included ALL lifecycle scripts in this problem space.

This is by no means a line in the sand. If the community says "no this should only be for lifecycle scripts in my dependencies, my own top level scripts are outside the scope of this" I'd be ok with that. My only pushback is that there is no way to disable my own package's install/postinstall but allow my dependencies' to still run. The use cases of disabling project level scripts and dependency scripts are very separate, and the current --ignore-scripts does not have any granularity. I think that granularity is important here.

[ljharb]

no this should only be for lifecycle scripts in my dependencies, my own top level scripts are outside the scope of this

I think that's the entire point. If you don't want to run lifecycle scripts in your own project you can just delete them.