-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fix several issues on NPM Audit reporting #5546
Conversation
… in NPM audit results Fixes #5419
fixes issue #5547
@jeremylong As this update changes the Advisory it will trigger errors logged by the node-audit cache when a report is found in cache for an npm-package that was stored by a previous release. The cache will in that case return null and a new npm audit report will be requested and stored in the cache, but the first time for each such package the log will contain an exception stacktrace. Not quite sure whether we should us a minor or a major revision for this case (scan runs fine, but log appears to indicate errors due to the mismatch on serialVersionUid) I do think it requires an explicit mention in the release-notes to try and avoid bug-reports.
|
And given this required serialization mismatch issue I'm wondering whether we should obtain more info from the NPM audit report into the advisory as well.... I see the CVEs, CWEs and a CVSS v3.1 score in there... maybe take the opportunity to improve on those as well before we integrate this? |
e.g.
"cves": [
"CVE-2020-7788"
],
"cwe": [
"CWE-1321"
],
"cvss": {
"score": 7.3,
"vectorString": "CVSS:3.1\/AV:N\/AC:L\/PR:N\/UI:N\/S:U\/C:L\/I:L\/A:L"
},
In the case of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The existing code tried to retrieve cwe as a string, but in the NPM Audit report it is an array. This commit parses the array and adds each CWE Fixes issue #5551
I'll definitely add notes about the exception linking back to this entry. I'm fine either way if you want to incorporate more data from the API. I was planning on releasing the next version this weekend. |
@jeremylong that schedule matches nicely with my polishing up the final modifications - addition of the CVSS v3 score to NPM that I hope to finish tonight |
Fixes Issue #5419
Description of Change
id
, since as experienced by users theid
returned is not stable over time.Have test cases been added to cover the new functionality?
no