Add acknowledged issue list to npm audit
#1494
Conversation
Currently `npm audit` is used in many CI systems. Many of them are blocking, that means that without a positive result from the tool it is not possible to integrate anything. While having a tool that can block CI process based on an external event, like a newly discovered vulnerability, is arguable, it is hard to dismiss lack of options to filter importance of found issues. 'npm audit' already has two options that allows to "ignore" certain vulnerabilities, which is crucial in context of CI. It is "audit-level" and "only". Sadly they do not provide enough granularity. As a result in many cases the only way to dismiss known errors is to disable the audit completely. The change creates an option to add a list of known issues that should not cause `npm audit` to return non-zero exit code. Therefore it is easy to unlock CI and put task of the issue resolution into an adequate, organization/project dependent workflow.
|
Personally, I think this is a good feature, and can certainly see the use for it. I also like that the implementation here is pretty simple, though adding a comma-separated list probably isn't as consistent as an argument that can be specified multiple times, and "acknowledged" is both hard to spell and doesn't quite capture the semantics of what's going on. Ie, maybe instead of All that said, getting such a change into npm v6 at this point is not a great idea. I'd much rather get it into npm v7, so that it's more future-proof and we don't find ourselves having to maintain a new feature in two different audit implementations. Would you mind opening an RFC to solicit feedback? That's how we're trying to drive most new features that add or change some conceptual aspect of the CLI. The implementation is straightforward either way, but that'll help us polish the UX and minimize any confusion for users that comes from this. https://github.com/npm/rfcs If you'd rather just be done with this and let someone else handle it, we can do the RFC, and that's completely fine and understandable. But, it'll probably be done sooner if we can get your input pushing on it while it's top of mind for you. Thanks! |
|
Thank you for a quick feedback! It is really appreciated :-) I’m completely fine with renaming the option to “audit-ignore” and change it from comma separated string. It was developed as such, because I could not find any example of a repeatable parameter, neither in FiggyPudding docs, nor in npm code. A friendly hint/link would be needed to change it :-) Regarding npm v6 vs npm v7, it is fine for me to have the feature in a newer version, assuming that it will be released rather sooner than later. The whole blocked CI issue is a side track for me, so I would prefer just to close it in some way. Currently it looks like https://www.npmjs.com/package/npm-audit-resolver is a bit more robust solution for me, even then this PR. For the same reason I would prefer to not commit myself to a long standing RFC process. From npm project perspective I can understand that a bit more polished UX than just an “ignore” flag could be desired. One can think about config files, CVEs, ignoring with a timeout and so on… Actually there is one RFC ongoing npm/rfcs#18 that covers it more globally. My personal opinion is that npm audit should not take part in blocking CI, unless newly introduced dependencies cause errors. It should be executed regularly on a side, preferably trying to commit a patch (npm audit fix) or if it is not possible create a bug report and commit “ignore” flag somewhere. To summarise, I can do one of:
All are fine for me :-) Thank you again! |
|
See this for comparison npm/rfcs#18 One comment I have is identifying the issues to acknowledge vs new issues with the same advisory or in the same package requires very verbose identifiers. That's why the RFC above uses @nierob I'd appreciate your input on both links above. |
|
I closed the PR as |
Add acknowledged issue list to
npm auditCurrently
npm auditis used in many CI systems. Many of them areblocking, that means that without a positive result from the tool it
is not possible to integrate anything. While having a tool that can
block CI process based on an external event, like a newly discovered
vulnerability, is arguable, it is hard to dismiss lack of options to
filter importance of found issues.
'npm audit' already has two options that allows to "ignore" certain
vulnerabilities, which is crucial in context of CI. It is "audit-level"
and "only". Sadly they do not provide enough granularity. As a result
in many cases the only way to dismiss known errors is to disable
the audit completely.
The change creates an option to add a list of known issues that should
not cause
npm auditto return non-zero exit code. Therefore it iseasy to unlock CI and put task of the issue resolution into an adequate,
organization/project dependent workflow.