Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Allow npm audit to ignore dev dependencies #20564

Open
westy92 opened this issue May 9, 2018 · 18 comments
Open

Allow npm audit to ignore dev dependencies #20564

westy92 opened this issue May 9, 2018 · 18 comments

Comments

@westy92
Copy link

westy92 commented May 9, 2018

What's the feature?

Add a flag to ignore dev dependencies when running npm audit.
Maybe it could ignore them by default and only check them with a flag.

What problem is the feature intended to solve?

Only receive a report back for true application dependencies.

Is the absence of this feature blocking you or your team? If so, how?

No.

Is this feature similar to an existing feature in another tool?

Snyk ignores dev dependencies by default: https://snyk.io/docs/snyk-for-nodejs#testing-node-js-projects

Is this a feature you're prepared to implement, with support from the npm CLI team?

I'll assist as much as I can.

@nicolasnoble
Copy link

More importantly, npm audit currently silently ignores the --production flag or the NODE_ENV environment variable.

@cronvel
Copy link

cronvel commented May 11, 2018

Hi npm-team,

I agree it would be great if dev-dependencies would not be part of the default audit.
Or at least it should split clearly dev/prod vulnerabilities.

Instead of:

[!] 7 vulnerabilities found [460 packages audited]

... something like this would be great:

[!] 0 vulnerabilities found [212 packages audited]
[!] 7 dev-dependencies vulnerabilities found [124 packages audited]

Currently it just makes some of my packages look vulnerable when actually there are not, and just makes me want to wipe all dev dependencies and instead use system-wide installed tools.

@welwood08
Copy link
Contributor

As a developer, I very much want to know if my dev dependencies contain a vulnerability and would find it counter-productive to remove them from the default audit. On the other hand, I can understand that each project uses dev dependencies differently so an option would be nice, as would separating the report output.

@mischkl
Copy link
Contributor

mischkl commented May 11, 2018

+1! For frontend tooling at the end of the day the dev vulnerabilities are almost irrelevant whereas the non-dev vulnerabilities are of the utmost importance. It would be great to be able to filter the output to see only vulnerabilities in non-dev dependencies.

@westy92
Copy link
Author

westy92 commented May 11, 2018

@welwood08 the long-term solution I am envisioning goes like this:

If a CI build fails, I can either fix or add an exception to make it pass again.

I'd also like to ignore dev dependencies because they seem to get patched much slower than others. For instance, even a popular package like gulp@latest has had the same unpatched vulnerabilities for months.

@welwood08
Copy link
Contributor

@westy92 I agree with pretty much all of that, I just think the default behaviour should remain unchanged.

Consider that the default value for the production config setting is false. It's easy to configure CI to run npm audit --production or something, but if a developer forgets to run npm audit --special-option-to-include-dev-dependencies then they might be missing crucial information without realising it.

(Aside: I just today found out Gulp 4 was released at the start of this year using the next tag so as to continue offering Gulp 3 on the latest tag while they polish it, switching squashes a load of audit warnings.)

@cronvel
Copy link

cronvel commented May 11, 2018

@westy92 to advocate for no dev vulnerabilities as a default: as a developer I rarely hack my own application to ruin my own computer ^^

@westy92
Copy link
Author

westy92 commented May 11, 2018

I fully agree with @welwood08 to look at both by default and take a --production flag. It feels right to maintain consistency with npm install.

welwood08 added a commit to welwood08/npm that referenced this issue May 11, 2018
Having checked how it's done elsewhere in the codebase, this should ignore devDependencies in production.

Aside: This code block is duplicated in several places so I feel like I should be refactoring it into a separate function, but I'm not close enough to the code to know what side-effects that might have.

Fixes npm#20564
@kellerj
Copy link

kellerj commented May 12, 2018

OK - I have two additions here to consider.

One - allow for a severity flag in terms of the return code. If in a build pipeline, I can see returning a non-zero code for High severity issues in order to kill the build and force a look at the dependencies. (I wouldn't expect this to be default - highest security option as the default would always be best.)

Two - as much as I like ignoring dev dependencies...and I found this issue when going to request it myself...

What about dependencies which are bundled by webpack into the production front-end code but that don't need to exist on the server as part of the deployment. I've been moving those to devDependencies so that they can be "npm prune"d before making the deployment bundle. I'm not sure there is any way to tell those types of items apart the way that our dependencies are listed now.

@legodude17
Copy link
Contributor

@kellerj One - That should probably be a separate issue.

Two - That's not really the intended use for devDependencies.

@zkat
Copy link
Contributor

zkat commented May 16, 2018

@kellerj we're doing non-zero exit codes on any vulnerabilities. If you want more refined things, my opinion is that you should use npm audit --json and take care of that logic yourself with a wrapper script. I'm not comfortable making decisions about what level of vulnerabilities it's ok to just ignore.

@benvium
Copy link

benvium commented May 17, 2018

@zkat does npm audit --json exist on npm 6.0.1? It doesn't seem to have any effect when I try it. The option isn't mentioned in npm help audit

@zkat
Copy link
Contributor

zkat commented May 17, 2018

@benvium it does not. It's in release-next, and will be part of 6.1.0. You can try it out now with npx npmc audit --json, since it's been pushed to the canary already :)

@luisrudge
Copy link

luisrudge commented May 21, 2018

why not add another option? check all dependencies by default and add a --ignore-dev-dependencies flag so we don't check that. What it doesn't make sense is to have no option to do that and now everyone will have to rely on the --json flag to figure it out what's going on

@mischkl
Copy link
Contributor

mischkl commented May 21, 2018

@luisrudge agreed. All that will result from forcing the use of --json plus another script is that a package will spring up that processes the json and gives you the options, which most people will end up using instead of the native audit option, which begs the question why the built-in feature didn't provide options in the first place.

@zkat
Copy link
Contributor

zkat commented May 22, 2018

@luisrudge it's already WIP, and we already have flags to manage whether we ignore deveps (--production)

@jordanbtucker
Copy link

The latest version of NPM (6.1.0) only honors the --production and --only=prod flags when running npm audit fix but not when just running npm audit. Are there plans to have the report honor those flags too?

@welwood08
Copy link
Contributor

welwood08 commented Jun 8, 2018

Are there plans to have the report honor those flags too?

@jordanbtucker Yes, I'm waiting for movement on npm/npm-audit-report#16 before spending more time on a PR.

welwood08 added a commit to welwood08/npm-audit-report that referenced this issue Jul 11, 2018
The passed config object can now specify the booleans `excludeDev` and `excludeProd` to hide vulnerabilities affecting only those kinds of dependency. The default in both cases is false to preserve previous behaviour.
Prerequisite for npm/npm#20564.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests