rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

npm audit and audit-resolve.json

Open naugtur opened this issue 6 years ago • 32 comments

[edit] This RFC is very old, some progress on developing the idea led to a Collab Space in OpenJSF and this format proposal: https://github.com/openjs-foundation/pkg-vuln-collab-space/pull/11

Was: npm audit resolve
Updated to only cover the support for audit-resolve.json file in npm.

Currently implemented in userland here: https://www.npmjs.com/package/npm-audit-resolver

The check-audit command from the above package is providing the exact functionality proposed here.

Other places where this was discussed:

https://github.com/npm/cli/pull/10 https://npm.community/t/interactive-tool-to-manage-audit-findings-npm-audit-resolve/197

naugtur avatar Aug 05 '18 16:08 naugtur

What's the latest on this? Would be great to have something to replace .nsprc/exceptions from nsp.

CADBOT avatar Sep 28 '18 17:09 CADBOT

I don't really know what's next other than moving my repo to npm org on GitHub. There's no disagreement on anything here.

I have more free time than usual for the next few days so I'd be happy to move this forward too.

But you can use it now. Install npm-audit-resolver. I'm using it at work with my team for a while now

naugtur avatar Sep 28 '18 18:09 naugtur

In general I'm very excited about the potential of the npm audit resolve feature. I think some work needs to be done to really make it a good interactive experience presenting the information in the best way possible for users.

evilpacket avatar Nov 06 '18 20:11 evilpacket

(Heads up, we are tracking this internally and it's something we intend to get to, but I can't give you an exact timeline right now. Thanks for your patience.)

zkat avatar Feb 07 '19 21:02 zkat

If you want to consider one bigger push to move this forward, my family is out for winter vacation 25-28Feb while I stay at work, so I'll have unusually many hours in the evenings in that period.

naugtur avatar Feb 11 '19 07:02 naugtur

This would be an awesome addition to npm CLI. We used to run npm audit on CI with a custom script outlined here.

Recently we've switched to @naugtur 's npm-resolve-audit module by installing it as a dev dependency and adding a script to run node node_modules/npm-audit-resolver/check.js

Thanks for building this tool @naugtur !

ersel avatar Feb 19 '19 09:02 ersel

@ersel if you're running it inside an npm script, you can use it as a command check-audit and it resolves from dependencies. Otherwise I'd go with npx -p npm-audit-resolver check-audit - feels a little cleaner and you don't need to install resolver as a dependency - npx will download and cache it. (npx is shipped with npm so no need to install it)

Check out my slides on using audit in CI (and other things) https://naugtur.pl/pres3/securedev/#/18

naugtur avatar Feb 19 '19 22:02 naugtur

I've updated the RFC and I'm working on a refactoring to separate the core features from npm-related and interactive ones.

Could we agree on making npm audit use audit-resolve.json being the first step? That way people could produce the file manually or with the interactive resolver, but the means of ignoring a package would be there.

naugtur avatar Mar 27 '19 22:03 naugtur

Hi,

I'm finishing the refactoring I mentioned before. My suggestion to proceed now is that npm (and potentially other package managers) can use the core of npm-audit-resolver to include facts from audit-reslve.json when checking the repository with npm audit command - therefore skipping issues that have been addressed before. The interactive part can (initially or forever) remain in the separate package with potential competitors, while the spec for audit-resolve.json becomes something we can slowly standardize.

I will open another PR to npm some time soon introducing the line to include decisions from audit-resolve.json in the npm audit logic.

Edit: I should probably let you know @zkat @evilpacket

naugtur avatar Jul 07 '19 08:07 naugtur

I couldn't join the RFC meetings, because I tend to give my kid a bath at that time and it's not optional ;) I'll try next time.

@ruyadorno @darcyclarke Let's continue this.

naugtur avatar Jan 23 '20 23:01 naugtur

Hi @naugtur picking up from where the conversation ended last time: https://github.com/npm/cli/issues/525#issuecomment-577907157

We would love to start working on npm audit resolve now that v7 is taking shape 😊

That said, do you want to update this RFC to reflect the changes we mentioned in that issue? (specially the part about not making it an interactive command anymore) It's up to you to decide if you want to bring this RFC across the finish line - I realize it's been ages since you first opened it and totally understand if you're not interested in following up anymore 😁 We def appreciate all the work that you've put into this RFC and npm-audit-resolver ❤️

ruyadorno avatar May 21 '20 00:05 ruyadorno

https://www.npmjs.com/package/audit-ci is another pretty popular alternative solution for this problem. Dropping it here to give more info for inspiration.

Den-dp avatar Jul 23 '20 23:07 Den-dp

Thanks for bringing it up @Den-dp The important difference here is audit-ci is only operating on risk levels or advisory allowlist.

audit-resolve.json file is encoding precisely which items were ignored so if you ignore an unreachable ReDos in a sub dependency somewhere in your web app, you still get a notification if ReDos is found in your server framework used.

audit-resolve-core package (work in progress) is meant for embedding and audit-ci could easily switch to using that at any point btw.

naugtur avatar Jul 24 '20 09:07 naugtur

Lately I migrated to npm-audit-resolver, it works Ok and fulfils my needs. Up to now, I have stumbled only on one issue; generated resolve file can be big (lodash case…). As the tool allows to mark every usage per package per CVE, it creates quite redundant list. One could softly compress it by expressing “ignore all usages” per CVE per package (maybe limited to specific versions). Anyway it is just optimisation that would make it a bit more git friendly.

The important difference here is audit-ci is only operating on risk levels or advisory allowlist.

As a user I can confirm that "risk level" is not giving enough flexibility. One really wants to specify at least advisory/package configuration.

My personal opinion is that npm audit should not take part in blocking CI, unless a newly introduced dependencies caused errors, for example in checking PR that changes dependencies. It should be executed regularly on a side, preferably trying to commit a patch like result of npm audit fix or if it is not possible create a bug report. The main issue here is that placing it in blocking CI causes the same negative effects as flaky tests; integrations are broken because of “unknown” / “unrelated” reasons, nobody owns the problem because it is injected into middle of an unrelated process.

nierob avatar Jul 24 '20 14:07 nierob

Thanks for mentioning compression. Although the goal is to be very explicit not to invite future occurrences, you got me thinking. The list can be compressed by putting a wildcard in the path. So each occurrence of a package with certain advisory anywhere in dependency tree of one specific direct dependency could be collapsed into one.

That would limit the number of entities while not breaking the security promises.

naugtur avatar Jul 24 '20 19:07 naugtur

Thanks for mentioning compression. Although the goal is to be very explicit not to invite future occurrences, you got me thinking. The list can be compressed by putting a wildcard in the path. So each occurrence of a package with certain advisory anywhere in dependency tree of one specific direct dependency could be collapsed into one.

That would limit the number of entities while not breaking the security promises.

It depends what kind of use case you want to solve. For me, partially ignoring issues doesn't make sense because it would still block CI. That means that path as such is useful when deciding if an issue can be ignored, later on it is not needed. That is because the final decision is binary; "do I accept this package or not". So instead of path I would use package version:

 {
   "version": 1,
   "decisions": {
     "ADVISORY_NUMBER": [
        {
           "AFFECTED_PACKAGE_NAME_AND_SHA1_FROM_LOCK_FILE":{
             "decision": "RESOLUTION_TYPE",
...

If one update package version, technically the security analysis needs to be re-done, from scratch. If you put path wildcard, this situation will likely not be detected.

nierob avatar Aug 05 '20 07:08 nierob

@nierob that doesn't cover when a package from your dev deps gets ignored because the vulnerable cod is obviously unreachable, but then it resurfaces as a prod dependency pulled by something else and you're busted.

On the app level (the resolver app that asks you what to do) the decision can be aggregated, but the stuff written in audit-resolve.json needs to be a snapshot of where the package was found at the time you decided to ignore it.

And it's not affected by npm's deduplication, that happens on a differnt leve, so the entries in package-lock are not moved around.

naugtur avatar Aug 05 '20 08:08 naugtur

@nierob that doesn't cover when a package from your dev deps gets ignored because the vulnerable cod is obviously unreachable, but then it resurfaces as a prod dependency pulled by something else and you're busted.

True. With the current config definition, the opposite is also true. One can update package version and previously unreachable code path may became reachable.

You convinced me that it needs to contain all three; path, package version and vulnerability id :-) In such case no much can be done regarding the file size. Maybe path prefixes can be used... Still I would discuss that as a format update, in a different RFC, it looks like a side-track.

nierob avatar Aug 05 '20 09:08 nierob

Please allow this this for the open source libraries to have time to fix the vulnerabilities. We're using npm audit for our systems for security and 1 security advisory can block the entire deployment for us. If we have the ignore for x number of days/hours to give ample time for the libraries/dependencies to update their libraries, that would be great.

jfaylon avatar Dec 09 '20 09:12 jfaylon

@jfaylon There's a collaboration space being set up under the OpenJS Foundation to tackle this.

For now you can use https://www.npmjs.com/package/npm-audit-resolver for your vulnerability management. My team's been doing that for a few years now.

naugtur avatar Dec 09 '20 19:12 naugtur

[edit] This RFC is very old, some progress on developing the idea led to a Collab Space in OpenJSF and this format proposal: openjs-foundation/pkg-vuln-collab-space#11

if this RFC is not reflective of a proposal, should it be closed? It's already been extremely misleading given that it's not the "real" representation of what the proposal is.

bnb avatar Aug 11 '21 18:08 bnb

if this RFC is not reflective of a proposal, should it be closed? It's already been extremely misleading given that it's not the "real" representation of what the proposal is.

After the last meeting I knew I should rewrite it now and I know what to put in it. Family matters and chores got in the way. I will rewrite it to reflect what it should.

naugtur avatar Aug 11 '21 18:08 naugtur

Are there any progress updates on this? With the new move of the advisory database of NPM now on GitHub (https://github.blog/changelog/2021-10-03-the-npm-advisory-database-is-now-part-of-the-github-advisory-database/) some of the moderate vulnerabilities that we can't resolve right now have been tagged as High - breaking our builds.

It would be great if we can accept the risks on them specifically to allow the build to pass again.

nukithelegend avatar Oct 05 '21 07:10 nukithelegend

Looks like it didn't actually get discussed at the meeting: https://youtu.be/nkohPHyvzw4?t=3582

@bnb Sounds like this was an action item for you; did you get a chance to look at the state of this?

trevyn avatar Oct 05 '21 14:10 trevyn

@bnb Sounds like this was an action item for you; did you get a chance to look at the state of this?

I'm not going to try to be speculative on an RFC that the creator has very specific ideas about without it actually being in a PR. I don't think that'd be particularly fair to them or me 😅

bnb avatar Oct 05 '21 15:10 bnb

Ok, so we're waiting on @naugtur , got it! :)

trevyn avatar Oct 05 '21 22:10 trevyn

Waiting for this like many others...

devdev-dev avatar Feb 18 '22 12:02 devdev-dev

Still waiting for this.

bertolo1988 avatar Sep 21 '22 14:09 bertolo1988

BTW. I'm very near releasing a new major version of npm-audit-resolver, if anyone wants to help out.

Happy to go back to discussing getting the policy check into npm cli. Still bullish on not doing it per package name but per dependency path.

Also, it used to be a PR before it was an RFC :D I'm gonna need to figure out what the expectations are now.

naugtur avatar Sep 21 '22 14:09 naugtur

if anyone wants to help out.

I can help, you can reach me using the contacts in my Github profile.

bertolo1988 avatar Sep 21 '22 15:09 bertolo1988