[BUG] npm pack and publish should not run hooks, or hooks need to have opt-out

[EvanCarroll]

Is there an existing issue for this?

• [X] I have searched the existing issues

This issue exists in the latest npm version

• [X] I am using the latest npm

Current Behavior

Currently npm pack and npm publish run a prepare hook. This presents a problem in practice because the prepare script as in the case of Husky is used to install githooks which is almost certainly going to break CI..

I would also argue this is a security problem. The prepare script gives an area for code injection. By placing code in the prepare hook you can leaks secrets with npm pack and npm publish (like the NPM_TOKEN they're often exposed to). This is a bigger problem too because codeowners is ineffective at mitigating this. But all of this raises a few questions,

1. Why does npm pack and npm publish need to run any hooks? One of them is effectively a fancy tar alternative, and the other one just does an HTTP push with credentials sending the tar to npmjs. Why does either one of these things need to be hookable.
2. Why doesn't npm pack and npm publish have an --ignore-scripts option like npm install. Especially for the purposes of CI!
3. Why doesn't npm pack and npm publish document these hooks in the help pages?
4. Since npm install --ignore-scripts works, I would think that the functionality of npm pack --ignore-scripts should be to error if that functionality isn't supported. Is there any reason why npm cli supports sending unsupported options to subcommands?

Expected Behavior

A package.json with prepare: "exit 1" should permit npm pack. I can't see the utility in having a hook for an npm pack.

Steps To Reproduce

mkdir foo
cd foo;
npm init -f -y
jq '.scripts.prepare = "exit 42;"' ./package.json | sponge package.json;
npm pack;

Workaround

The only workaround that I know of here to securely run npm pack and npm prepare in CI without risking code execution and secret leaking is to trim these lifecycle hooks out of the package.json,

jq 'del(.scripts.prepare) | del(.scripts.prepublish) | del(scripts.prepublishOnly) | del(scripts.prepack) | del(scripts.postpack)' ./package.json | sponge package.json

Environment

• npm: 10.2.4
• Node.js: v21.4.0
• OS Name: Debian

[EvanCarroll]

While I do believe this is a security bug because of the implications on GitHub, it's not technically a security bug as I read it because of this text,

Warning: Any user with write access to your repository has read access to all secrets configured in your repository. Therefore, you should ensure that the credentials being used within workflows have the least privileges required.

GitHub doesn't differentiate between a code-contributor and repo-owner for the purposes of secrets. This is silly and unfortunate. Outside of GitHub, it's quite possible for CI to be controlled and to only inject secrets into a specific job that only runs trusted code. For example, to only inject the NPM_TOKEN into a trusted publish job, and to exclude it in the build job. In this case, I would consider this to be a security vulnerability because you can't trust npm pack and npm publish as they will execute the hooks in the text of the package.json.

[ljharb]

husky (and any tool that automatically installs git hooks) is definitely a security attack vector (there's a reason git chooses not to support that), and should be avoided in all cases for this reason.

Does --ignore-scripts actually not work with pack/publish, regardless of whether it's documented? I would assume it works everywhere.

I use the prepack hook in all of my projects to autogenerate an npmignore file, or to run a build process - both pack and publish hooks are critical to exist in top-level projects (altho i wouldn't expect them to be invoked for installed deps)

[EvanCarroll]

Does --ignore-scripts actually not work with pack/publish, regardless of whether it's documented? I would assume it works everywhere.

No, it doesn't work. =( But we're in agreement, it should.

mkdir foo
cd foo;
npm init -f -y
jq '.scripts.prepare = "exit 42;"' ./package.json | sponge package.json;
npm pack --ignore-scripts;

I use the prepack hook in all of my projects to autogenerate an npmignore file, or to run a build process - both pack and publish hooks are critical to exist in top-level projects (altho i wouldn't expect them to be invoked for installed deps)

Doesn't that seem like a build-stage thing? I wouldn't put that in a pack or ignore stage. You're generating files. By any means, I don't want that functionality and --ignore-scripts would help me protect against it being inadvertently hijacked by stuff like husky, and bad actors.

[EvanCarroll]

Maybe as a side note, if we're all in agreement that Husky is crazy in even suggesting this we can at least send them an issue to remove this from their home page.

Aligns with npm best practices using prepare script

According to Husky they have "over 1.3M projects on GitHub". Which is insane

[ljharb]

pack is the build stage - it's when you're making the tarball, and is the only place the build step should run. That kind of logic should never have been in prepublish or "prepare" in the first place. The confusion is only because people wrongly believe that it's reasonable to install from a git repo.

[EvanCarroll]

I've never seen npm pack referred to as a build stage, and that's not how the docs refer to it,

Create a tarball from a package

I think everything creates a new script called "build" which is the build script. In Angular, npm run build is an alias for ng build, and create react app does the same. Not to say any of these are right or wrong, but I wouldn't generate an npmignore anywhere except in a special npm run build script. And if I wanted to that to happen, I would explicitly call it before npm pack.

[ljharb]

oh totally, i use build also - i just run it in prepack (i used to run it in prepublish/prepublishOnly).

[EvanCarroll]

I don't understand why you'd want that. As compared to just running npm run build; npm pack. I've never seen anyone calling build from pack. But either way, we're in agreement that npm pack and npm publish should have a --ignore-scripts option like npm install? That would solve my problem, and allow secure deployment (though it would be opt in) in CI.

By secure, I mean to say, I wouldn't have to worry about a change to package.json injecting code that leaks my NPM secrets when I'm just running npm pack and npm publish to push up to verdaccio.

[ljharb]

Because I do, actually, run npm pack --dry-run often to figure out what files will be included in my package, and so i want the build process to reflect in that. Otherwise, as long as it runs when I run npm publish then it's fine.

Why should they have that option? Unlike npm install, this is your own project's scripts. If you don't want them to run, don't have them in there.

I'm not convinced "what if something on my filesystem is changed without me knowing it" is a realistically defensible attack vector - if they can do that they could change npm itself, or your shell profile, and get their script to run anyways.

[EvanCarroll]

Why should they have that option? Unlike npm install, this is your own project's scripts. If you don't want them to run, don't have them in there.

The point is that "in there", can usually be narrowed down to in your CI. That is to say, CI changes can be placed under ownership of a different team (such as a release team which owns the token). This token can be restricted such that it's only injected into the publication job. This isn't possible when the publication job (which requires npm publish) is subject to script injections by anyone who can contribute.

Does that answer your concern? And to go back to your initial statement,

Does --ignore-scripts actually not work with pack/publish, regardless of whether it's documented? I would assume it works everywhere.

I agree with that. Why is desirable that --ignore-scripts does not work on npm publish and npm pack. I never want anything except the default behavior on these.

[ljharb]

I'm still very unclear on why this is a concern - if you don't want non-default behavior, you just don't put it in package.json. If another team at your company is modifying your files in a way you don't like, lobby them to stop doing that.

No need to keep going back and forth here, tho, maybe the npm folks will understand your concern better than I.

[EvanCarroll]

Because security isn't about "lobbying" actors to be good. It's about protecting against innocent mistakes, and malice. Developers need to be able to modify the package.json -- that's how they add dependencies to the project. They don't need to be able to inject code into the script that pushes a tarball up to npm, because that gives them the ability to leak the npm token in CI. I want to stop them from doing this.

I want two classes of users, "developers" who can edit code anywhere. And "publishers" who have access to the NPM_TOKEN, and can control the publication step which calls npm pack; npm publish;. It's just basic user-access control on a secret. If you don't need it, you shouldn't have access to it.

[lukekarrys]

To shine a light on what is currently happening with regards to npm pack and --ignore-scripts as of 10.7.0 (the latest at the time of this comment):

• For npm pack both prepack and prepare are run by default
• --ignore-scripts will only ignore prepack. prepare will always be run

I think this is probably an oversight. The only functional difference in those scripts is that prepack is run in libnpmpack and prepare is run in pacote. The code in pacote also has to account for git deps being prepared as the docs mention:

NOTE: If a package being installed through git contains a prepare script, its dependencies and devDependencies will be installed, and the prepare script will be run, before the package is packaged and installed.

This leads me to believe it's an oversight because pacote doesn't ever ignore scripts. It expects the caller to have made that choice.

So I think the following could all help with this issue:

• Make prepare respect --ignore-scripts (after investigating if this is expected and going to break a bunch of people)
• Link to https://docs.npmjs.com/cli/v10/using-npm/scripts#life-cycle-scripts in the pack and publish command docs.

[lukekarrys]

Doing some more digging, I think making prepare respect the --ignore-scripts option would be a breaking change so I'm going to track this over in our v11 roadmap issue: https://github.com/npm/statusboard/issues/488

I do think documentation could be improved on the current specifics of prepare as well as linking to the using-npm/scripts page from more commands.