Skip to content

Commit

Permalink
fix(arborist): dont skip adding advisories to audit based on name/range
Browse files Browse the repository at this point in the history 
When generating an audit report, a cache of seen advisories is kept to
avoid doing any repeat fanout work on its nodes. Previously this cache
was also preventing audits from being added to the report. This has been
fixed so the cache is only used to prevent extra work, but all valid
advisories are added to the output.

Fixes npm#4681
  • Loading branch information
@lukekarrys @fritzy
lukekarrys authored and fritzy committed Apr 13, 2022
1 parent e992b4a commit aa4a4da
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 39 deletions.
75 changes: 36 additions & 39 deletions workspaces/arborist/lib/audit-report.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,61 +134,58 @@ class AuditReport extends Map {
const seen = new Set()
for (const advisory of advisories) {
const { name, range } = advisory

// don't flag the exact same name/range more than once
// adding multiple advisories with the same range is fine, but no
// need to search for nodes we already would have added.
const k = `${name}@${range}`
if (seen.has(k)) {
continue
}

seen.add(k)

const vuln = this.get(name) || new Vuln({ name, advisory })
if (this.has(name)) {
vuln.addAdvisory(advisory)
}
super.set(name, vuln)

const p = []
for (const node of this.tree.inventory.query('packageName', name)) {
if (!shouldAudit(node, this[_omit], this.filterSet)) {
continue
}
// don't flag the exact same name/range more than once
// adding multiple advisories with the same range is fine, but no
// need to search for nodes we already would have added.
if (!seen.has(k)) {
const p = []
for (const node of this.tree.inventory.query('packageName', name)) {
if (!shouldAudit(node, this[_omit], this.filterSet)) {
continue
}

// if not vulnerable by this advisory, keep searching
if (!advisory.testVersion(node.version)) {
continue
}
// if not vulnerable by this advisory, keep searching
if (!advisory.testVersion(node.version)) {
continue
}

// we will have loaded the source already if this is a metavuln
if (advisory.type === 'metavuln') {
vuln.addVia(this.get(advisory.dependency))
}
// we will have loaded the source already if this is a metavuln
if (advisory.type === 'metavuln') {
vuln.addVia(this.get(advisory.dependency))
}

// already marked this one, no need to do it again
if (vuln.nodes.has(node)) {
continue
}
// already marked this one, no need to do it again
if (vuln.nodes.has(node)) {
continue
}

// haven't marked this one yet. get its dependents.
vuln.nodes.add(node)
for (const { from: dep, spec } of node.edgesIn) {
if (dep.isTop && !vuln.topNodes.has(dep)) {
this[_checkTopNode](dep, vuln, spec)
} else {
// haven't marked this one yet. get its dependents.
vuln.nodes.add(node)
for (const { from: dep, spec } of node.edgesIn) {
if (dep.isTop && !vuln.topNodes.has(dep)) {
this[_checkTopNode](dep, vuln, spec)
} else {
// calculate a metavuln, if necessary
const calc = this.calculator.calculate(dep.packageName, advisory)
p.push(calc.then(meta => {
if (meta.testVersion(dep.version, spec)) {
advisories.add(meta)
}
}))
const calc = this.calculator.calculate(dep.packageName, advisory)
p.push(calc.then(meta => {
if (meta.testVersion(dep.version, spec)) {
advisories.add(meta)
}
}))
}
}
}
await Promise.all(p)
seen.add(k)
}
await Promise.all(p)

// make sure we actually got something. if not, remove it
// this can happen if you are loading from a lockfile created by
Expand Down
129 changes: 129 additions & 0 deletions workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,15 @@ exports[`test/audit-report.js TAP all severity levels > json version 1`] = `
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 1325,
"name": "handlebars",
"dependency": "handlebars",
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 755,
"name": "handlebars",
Expand Down Expand Up @@ -448,6 +457,15 @@ exports[`test/audit-report.js TAP all severity levels > json version 1`] = `
"url": "https://npmjs.com/advisories/1478",
"severity": "high",
"range": ">=4.1.0"
},
{
"source": 1479,
"name": "subtext",
"dependency": "subtext",
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1479",
"severity": "high",
"range": ">=0.0.0"
}
],
"effects": [],
Expand Down Expand Up @@ -558,6 +576,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp > json version 1
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 1325,
"name": "handlebars",
"dependency": "handlebars",
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 755,
"name": "handlebars",
Expand Down Expand Up @@ -918,6 +945,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with before: opt
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 1325,
"name": "handlebars",
"dependency": "handlebars",
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 755,
"name": "handlebars",
Expand Down Expand Up @@ -1278,6 +1314,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with newer endpo
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 1325,
"name": "handlebars",
"dependency": "handlebars",
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"severity": "high",
"range": "<3.0.8 || >=4.0.0 <4.5.3"
},
{
"source": 755,
"name": "handlebars",
Expand Down Expand Up @@ -2144,6 +2189,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down Expand Up @@ -2623,6 +2682,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down Expand Up @@ -2791,6 +2864,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down Expand Up @@ -3270,6 +3357,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down Expand Up @@ -3458,6 +3559,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down Expand Up @@ -3927,6 +4042,20 @@ Object {
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
"dependency": "handlebars",
"id": undefined,
"name": "handlebars",
"range": "<3.0.8 || >=4.0.0 <4.5.3",
"severity": "high",
"source": 1325,
"title": "Prototype Pollution",
"url": "https://npmjs.com/advisories/1325",
"versions": undefined,
"vulnerableVersions": undefined,
},
Object {
"cvss": undefined,
"cwe": undefined,
Expand Down

0 comments on commit aa4a4da

Please sign in to comment.