Skip to content

Commit aa4a4da

Browse files
lukekarrysNathan Fritz
lukekarrys
authored and
Nathan Fritz
committed
fix(arborist): dont skip adding advisories to audit based on name/range
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 #4681
1 parent e992b4a commit aa4a4da

File tree

2 files changed

+165
-39
lines changed

2 files changed

+165
-39
lines changed

‎workspaces/arborist/lib/audit-report.js‎

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -134,61 +134,58 @@ class AuditReport extends Map {
134134
const seen = new Set()
135135
for (const advisory of advisories) {
136136
const { name, range } = advisory
137-
138-
// don't flag the exact same name/range more than once
139-
// adding multiple advisories with the same range is fine, but no
140-
// need to search for nodes we already would have added.
141137
const k = `${name}@${range}`
142-
if (seen.has(k)) {
143-
continue
144-
}
145-
146-
seen.add(k)
147138

148139
const vuln = this.get(name) || new Vuln({ name, advisory })
149140
if (this.has(name)) {
150141
vuln.addAdvisory(advisory)
151142
}
152143
super.set(name, vuln)
153144

154-
const p = []
155-
for (const node of this.tree.inventory.query('packageName', name)) {
156-
if (!shouldAudit(node, this[_omit], this.filterSet)) {
157-
continue
158-
}
145+
// don't flag the exact same name/range more than once
146+
// adding multiple advisories with the same range is fine, but no
147+
// need to search for nodes we already would have added.
148+
if (!seen.has(k)) {
149+
const p = []
150+
for (const node of this.tree.inventory.query('packageName', name)) {
151+
if (!shouldAudit(node, this[_omit], this.filterSet)) {
152+
continue
153+
}
159154

160-
// if not vulnerable by this advisory, keep searching
161-
if (!advisory.testVersion(node.version)) {
162-
continue
163-
}
155+
// if not vulnerable by this advisory, keep searching
156+
if (!advisory.testVersion(node.version)) {
157+
continue
158+
}
164159

165-
// we will have loaded the source already if this is a metavuln
166-
if (advisory.type === 'metavuln') {
167-
vuln.addVia(this.get(advisory.dependency))
168-
}
160+
// we will have loaded the source already if this is a metavuln
161+
if (advisory.type === 'metavuln') {
162+
vuln.addVia(this.get(advisory.dependency))
163+
}
169164

170-
// already marked this one, no need to do it again
171-
if (vuln.nodes.has(node)) {
172-
continue
173-
}
165+
// already marked this one, no need to do it again
166+
if (vuln.nodes.has(node)) {
167+
continue
168+
}
174169

175-
// haven't marked this one yet. get its dependents.
176-
vuln.nodes.add(node)
177-
for (const { from: dep, spec } of node.edgesIn) {
178-
if (dep.isTop && !vuln.topNodes.has(dep)) {
179-
this[_checkTopNode](dep, vuln, spec)
180-
} else {
170+
// haven't marked this one yet. get its dependents.
171+
vuln.nodes.add(node)
172+
for (const { from: dep, spec } of node.edgesIn) {
173+
if (dep.isTop && !vuln.topNodes.has(dep)) {
174+
this[_checkTopNode](dep, vuln, spec)
175+
} else {
181176
// calculate a metavuln, if necessary
182-
const calc = this.calculator.calculate(dep.packageName, advisory)
183-
p.push(calc.then(meta => {
184-
if (meta.testVersion(dep.version, spec)) {
185-
advisories.add(meta)
186-
}
187-
}))
177+
const calc = this.calculator.calculate(dep.packageName, advisory)
178+
p.push(calc.then(meta => {
179+
if (meta.testVersion(dep.version, spec)) {
180+
advisories.add(meta)
181+
}
182+
}))
183+
}
188184
}
189185
}
186+
await Promise.all(p)
187+
seen.add(k)
190188
}
191-
await Promise.all(p)
192189

193190
// make sure we actually got something. if not, remove it
194191
// this can happen if you are loading from a lockfile created by

‎workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ exports[`test/audit-report.js TAP all severity levels > json version 1`] = `
123123
"severity": "high",
124124
"range": "<3.0.8 || >=4.0.0 <4.5.3"
125125
},
126+
{
127+
"source": 1325,
128+
"name": "handlebars",
129+
"dependency": "handlebars",
130+
"title": "Prototype Pollution",
131+
"url": "https://npmjs.com/advisories/1325",
132+
"severity": "high",
133+
"range": "<3.0.8 || >=4.0.0 <4.5.3"
134+
},
126135
{
127136
"source": 755,
128137
"name": "handlebars",
@@ -448,6 +457,15 @@ exports[`test/audit-report.js TAP all severity levels > json version 1`] = `
448457
"url": "https://npmjs.com/advisories/1478",
449458
"severity": "high",
450459
"range": ">=4.1.0"
460+
},
461+
{
462+
"source": 1479,
463+
"name": "subtext",
464+
"dependency": "subtext",
465+
"title": "Prototype Pollution",
466+
"url": "https://npmjs.com/advisories/1479",
467+
"severity": "high",
468+
"range": ">=0.0.0"
451469
}
452470
],
453471
"effects": [],
@@ -558,6 +576,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp > json version 1
558576
"severity": "high",
559577
"range": "<3.0.8 || >=4.0.0 <4.5.3"
560578
},
579+
{
580+
"source": 1325,
581+
"name": "handlebars",
582+
"dependency": "handlebars",
583+
"title": "Prototype Pollution",
584+
"url": "https://npmjs.com/advisories/1325",
585+
"severity": "high",
586+
"range": "<3.0.8 || >=4.0.0 <4.5.3"
587+
},
561588
{
562589
"source": 755,
563590
"name": "handlebars",
@@ -918,6 +945,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with before: opt
918945
"severity": "high",
919946
"range": "<3.0.8 || >=4.0.0 <4.5.3"
920947
},
948+
{
949+
"source": 1325,
950+
"name": "handlebars",
951+
"dependency": "handlebars",
952+
"title": "Prototype Pollution",
953+
"url": "https://npmjs.com/advisories/1325",
954+
"severity": "high",
955+
"range": "<3.0.8 || >=4.0.0 <4.5.3"
956+
},
921957
{
922958
"source": 755,
923959
"name": "handlebars",
@@ -1278,6 +1314,15 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with newer endpo
12781314
"severity": "high",
12791315
"range": "<3.0.8 || >=4.0.0 <4.5.3"
12801316
},
1317+
{
1318+
"source": 1325,
1319+
"name": "handlebars",
1320+
"dependency": "handlebars",
1321+
"title": "Prototype Pollution",
1322+
"url": "https://npmjs.com/advisories/1325",
1323+
"severity": "high",
1324+
"range": "<3.0.8 || >=4.0.0 <4.5.3"
1325+
},
12811326
{
12821327
"source": 755,
12831328
"name": "handlebars",
@@ -2144,6 +2189,20 @@ Object {
21442189
"versions": undefined,
21452190
"vulnerableVersions": undefined,
21462191
},
2192+
Object {
2193+
"cvss": undefined,
2194+
"cwe": undefined,
2195+
"dependency": "handlebars",
2196+
"id": undefined,
2197+
"name": "handlebars",
2198+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
2199+
"severity": "high",
2200+
"source": 1325,
2201+
"title": "Prototype Pollution",
2202+
"url": "https://npmjs.com/advisories/1325",
2203+
"versions": undefined,
2204+
"vulnerableVersions": undefined,
2205+
},
21472206
Object {
21482207
"cvss": undefined,
21492208
"cwe": undefined,
@@ -2623,6 +2682,20 @@ Object {
26232682
"versions": undefined,
26242683
"vulnerableVersions": undefined,
26252684
},
2685+
Object {
2686+
"cvss": undefined,
2687+
"cwe": undefined,
2688+
"dependency": "handlebars",
2689+
"id": undefined,
2690+
"name": "handlebars",
2691+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
2692+
"severity": "high",
2693+
"source": 1325,
2694+
"title": "Prototype Pollution",
2695+
"url": "https://npmjs.com/advisories/1325",
2696+
"versions": undefined,
2697+
"vulnerableVersions": undefined,
2698+
},
26262699
Object {
26272700
"cvss": undefined,
26282701
"cwe": undefined,
@@ -2791,6 +2864,20 @@ Object {
27912864
"versions": undefined,
27922865
"vulnerableVersions": undefined,
27932866
},
2867+
Object {
2868+
"cvss": undefined,
2869+
"cwe": undefined,
2870+
"dependency": "handlebars",
2871+
"id": undefined,
2872+
"name": "handlebars",
2873+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
2874+
"severity": "high",
2875+
"source": 1325,
2876+
"title": "Prototype Pollution",
2877+
"url": "https://npmjs.com/advisories/1325",
2878+
"versions": undefined,
2879+
"vulnerableVersions": undefined,
2880+
},
27942881
Object {
27952882
"cvss": undefined,
27962883
"cwe": undefined,
@@ -3270,6 +3357,20 @@ Object {
32703357
"versions": undefined,
32713358
"vulnerableVersions": undefined,
32723359
},
3360+
Object {
3361+
"cvss": undefined,
3362+
"cwe": undefined,
3363+
"dependency": "handlebars",
3364+
"id": undefined,
3365+
"name": "handlebars",
3366+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
3367+
"severity": "high",
3368+
"source": 1325,
3369+
"title": "Prototype Pollution",
3370+
"url": "https://npmjs.com/advisories/1325",
3371+
"versions": undefined,
3372+
"vulnerableVersions": undefined,
3373+
},
32733374
Object {
32743375
"cvss": undefined,
32753376
"cwe": undefined,
@@ -3458,6 +3559,20 @@ Object {
34583559
"versions": undefined,
34593560
"vulnerableVersions": undefined,
34603561
},
3562+
Object {
3563+
"cvss": undefined,
3564+
"cwe": undefined,
3565+
"dependency": "handlebars",
3566+
"id": undefined,
3567+
"name": "handlebars",
3568+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
3569+
"severity": "high",
3570+
"source": 1325,
3571+
"title": "Prototype Pollution",
3572+
"url": "https://npmjs.com/advisories/1325",
3573+
"versions": undefined,
3574+
"vulnerableVersions": undefined,
3575+
},
34613576
Object {
34623577
"cvss": undefined,
34633578
"cwe": undefined,
@@ -3927,6 +4042,20 @@ Object {
39274042
"versions": undefined,
39284043
"vulnerableVersions": undefined,
39294044
},
4045+
Object {
4046+
"cvss": undefined,
4047+
"cwe": undefined,
4048+
"dependency": "handlebars",
4049+
"id": undefined,
4050+
"name": "handlebars",
4051+
"range": "<3.0.8 || >=4.0.0 <4.5.3",
4052+
"severity": "high",
4053+
"source": 1325,
4054+
"title": "Prototype Pollution",
4055+
"url": "https://npmjs.com/advisories/1325",
4056+
"versions": undefined,
4057+
"vulnerableVersions": undefined,
4058+
},
39304059
Object {
39314060
"cvss": undefined,
39324061
"cwe": undefined,

0 commit comments

Comments
 (0)