Update 'isExcepted' to check for CVE id

[kyleclark1824]

Making this PR because every time a vulnerability get's updated the ID seems to change. So I constantly have to update my .nsprc file with the new ID.

It looks like the JSON output from the audit contains a cves property with the GitHub CVE ID's. So would something like this work?

So if we don't have a match to the cur.id check the cve id's. Not ideal having numeric a non-numeric ID's but that should allow the CVE ID to be used in the .nsprc as well as the other standard ID's.

Related issue: #60

[leedm777]

I stumbled across this looking for alternatives due to IBM/audit-ci#211.

I think you're going to need to look at the advisory URL; not just the CVEs. There are at least some vulnerabilities that don't get assigned CVEs. See this comment on the audit-ci issue:

Does every vulnerability in the database have a CVE? It looks like here's maybe a few examples of ones that don't have CVEs: GHSA-r8hm-w5f7-wj39, GHSA-pjwm-rvh2-c87w, GHSA-xx4c-jj58-r7x6

[kyleclark1824]

@leedm777, good point. Maybe even the github_advisory_id would be a good place to check. I'm just not positive which "ID's" change as things get updated.

[leedm777]

@kyleclark1824 ,

Maybe even the github_advisory_id would be a good place to check.

I don't see auditReportVersion in the v2 output given by npm v8. Closest I see is via.url.

I'm just not positive which "ID's" change as things get updated.

That's entirely unclear, and probably an unintended consequence. The changing ids feels like a bug, but I'm not even sure where to report it.

What I observe changing is the via.source field from the v2 format, which looks like it's the same thing as the id field in the old format.

[guillermaster]

@kyleclark1824 the solution looks good, it will be very useful to have it merged soon. For some odd reason, those are IDs being currently used are changing frequently.
@jeemok do you mind reviewing this PR?

[leedm777]

This patch only fixes npm v6 handling. npm v7 and newer doesn't even list the CVE in the JSON output; at least not with npm v8.3.1 😕

[kyle-clark1824]

This patch only fixes npm v6 handling. npm v7 and newer doesn't even list the CVE in the JSON output; at least not with npm v8.3.1 😕

Yeah @leedm777, I was worried about that as well. This "fix" shouldn't hurt in those cases but a full fix would definitely be required. I'll see if I can find the JSON output for the other versions and add a more robust fix. Probably poke at it tonight.

I'm not seeing auditReportVersion in v 6.x either. Ideally there is a good way to determine what version is being run, then use that info to determine how to set isExcepted. But I'll take a look, worst case we can just have some more conditionals to handle additional cases.

[kyle-clark1824]

@leedm777, I added some more conditions. The issue with auditReportVersion2 looks like more inconsistent data. Sometimes the via array is just a single string. In those cases I don't see any sort of Identifier.

I updated to check 4 places:

• The id field - original
• The cve array - my original change
• The via array source - looks to be an ID
• The via array url for matching github ID

There are still cases where a v8 vuln (maybe 7?) will not "find" a matching ID with this logic but this gives us much more options for ways to add exceptions. Below is an example of one of those cases using npm@latest

  "@angular-devkit/build-angular": {
      "name": "@angular-devkit/build-angular",
      "severity": "high",
      "isDirect": true,
      "via": [
        "webpack-dev-server"
      ],
      "effects": [],
      "range": "0.8.8 - 0.1102.17 || 0.1200.0-next.0 - 12.2.14 || 13.0.0-next.0 - 13.0.0-rc.3",
      "nodes": [
        "node_modules/@angular-devkit/build-angular"
      ],
      "fixAvailable": {
        "name": "@angular-devkit/build-angular",
        "version": "12.2.16",
        "isSemVerMajor": false
      }
    }
    
    ```
    
    I don't really see any way to capture ID's when the output looks like that (other than `name`?).

[jeemok]

hey @kyle-clark1824, thanks for initiating this change! let's work on this in a separate branch beta, I'll focus on v6 support first and try to cover different version later

[jeemok]

this seems to be v7/v8 report structure, might need to move this checking to below v7 handling

[kyle-clark1824]

Hey @jeemok. Yup I would say the v7/8 work should be down with the v7 handling, sorry about that! Glad you're able to take a look, I was really just hoping to get some ideas out there on how we could improve the ID stability a bit.

[jeemok]

let me merge this in first and help with the eslint issue and adding unit tests, etc.

[kyle-clark1824]

Sounds good @jeemok thanks!