[BUG] npm pack and publish should not run hooks, or hooks need to have opt-out about cli HOT 12 OPEN

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.

from cli.

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)

from cli.

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.

from cli.

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

from cli.

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.

from cli.

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.

from cli.

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

from cli.

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.

from cli.

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.

from cli.

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.

from cli.

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.

from cli.

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.

from cli.