Skip to content

Commit be8053c

Browse files
authored
feat: warn when min-release-age blocks an audit fix (#9544)
`npm audit fix` left a package on its vulnerable version, with no warning, when the only patched version was newer than the `min-release-age`/`before` cutoff. It now warns which fix was blocked and exits non-zero. Honors `min-release-age-exclude`.
1 parent fc3ef5a commit be8053c

10 files changed

Lines changed: 308 additions & 0 deletions

File tree

lib/commands/audit.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,19 @@ class Audit extends ArboristWorkspaceCmd {
7373
await arb.audit({ fix })
7474
if (fix) {
7575
await reifyFinish(this.npm, arb)
76+
// Report any fix that a `min-release-age`/`before` window blocked from
77+
// installing, and exit non-zero so a blocked fix is not missed.
78+
const report = arb.auditReport
79+
const blocked = report instanceof Map
80+
? [...report.values()].filter(v => v.fixBlockedByReleaseAge).map(v => v.name)
81+
: []
82+
if (blocked.length) {
83+
log.warn('audit', `${blocked.length} package(s) left at a vulnerable version because ` +
84+
`a fix is newer than the release-age cutoff: ${blocked.join(', ')}.\n` +
85+
'Add the package to min-release-age-exclude, or relax min-release-age or before, ' +
86+
'to install the fix.')
87+
process.exitCode = 1
88+
}
7689
} else {
7790
// will throw if there's an error, because this is an audit command
7891
auditError(this.npm, arb.auditReport)

tap-snapshots/test/lib/docs.js.test.cjs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,10 @@ sources, the standard precedence applies (cli > env > project > user >
410410
global), so a higher-priority source can always relax or override a
411411
lower-priority one.
412412
413+
As with \`min-release-age\`, when this cutoff blocks a fix that \`npm audit
414+
fix\` would install, npm keeps the vulnerable version, warns, and exits with
415+
a non-zero code.
416+
413417
Packages whose names match \`min-release-age-exclude\` are exempt from this
414418
filter.
415419
@@ -1303,6 +1307,12 @@ your \`.npmrc\` is preserved when npm internally spawns a sub-process with
13031307
apply, \`before\` wins within a single source and across sources the standard
13041308
precedence rules apply.
13051309
1310+
When this window stops \`npm audit fix\` from installing a patched version
1311+
(because the fix was published too recently), npm keeps the package at its
1312+
vulnerable version, warns that the fix was blocked, and exits with a
1313+
non-zero code. To install the fix, add the package to
1314+
\`min-release-age-exclude\`, or relax \`min-release-age\` or \`before\`.
1315+
13061316
Packages whose names match \`min-release-age-exclude\` are exempt from this
13071317
filter.
13081318

test/lib/commands/audit.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,86 @@ t.test('audit fix - bulk endpoint', async t => {
170170
)
171171
})
172172

173+
t.test('audit fix exits non-zero when min-release-age blocks a fix', async t => {
174+
const { npm, logs } = await loadMockNpm(t, {
175+
prefixDir: tree,
176+
config: { 'min-release-age': 30 },
177+
})
178+
const registry = new MockRegistry({
179+
tap: t,
180+
registry: npm.config.get('registry'),
181+
})
182+
const manifest = registry.manifest({
183+
name: 'test-dep-a',
184+
packuments: [{ version: '1.0.0' }, { version: '1.0.1' }],
185+
})
186+
// 1.0.0 is old enough to install; the fix 1.0.1 was published too recently.
187+
manifest.time['1.0.0'] = '2020-01-01T00:00:00.000Z'
188+
manifest.time['1.0.1'] = new Date().toISOString()
189+
await registry.package({
190+
manifest,
191+
tarballs: {
192+
'1.0.0': path.join(npm.prefix, 'test-dep-a-vuln'),
193+
},
194+
times: 2,
195+
})
196+
const advisory = registry.advisory({ id: 100, vulnerable_versions: '<1.0.1' })
197+
registry.nock.post('/-/npm/v1/security/advisories/bulk', body => {
198+
const unzipped = JSON.parse(gunzip(Buffer.from(body, 'hex')))
199+
return t.same(unzipped, { 'test-dep-a': ['1.0.0'] })
200+
})
201+
.reply(200, { 'test-dep-a': [advisory] })
202+
.post('/-/npm/v1/security/advisories/bulk', body => {
203+
const unzipped = JSON.parse(gunzip(Buffer.from(body, 'hex')))
204+
return t.same(unzipped, { 'test-dep-a': ['1.0.0'] })
205+
})
206+
.reply(200, { 'test-dep-a': [advisory] })
207+
208+
await npm.exec('audit', ['fix'])
209+
210+
t.equal(process.exitCode, 1, 'exits non-zero because the fix was blocked')
211+
t.ok(
212+
logs.warn.some(w =>
213+
/left at a vulnerable version because a fix is newer than the release-age cutoff/.test(w)),
214+
'warns that the fix was blocked by min-release-age'
215+
)
216+
const lock = JSON.parse(fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8'))
217+
t.equal(lock.packages['node_modules/test-dep-a'].version, '1.0.0',
218+
'test-dep-a was left at the vulnerable version')
219+
})
220+
221+
t.test('json audit reports fixBlockedByReleaseAge when a fix is too new', async t => {
222+
const { npm, joinedOutput } = await loadMockNpm(t, {
223+
prefixDir: tree,
224+
config: {
225+
json: true,
226+
'min-release-age': 30,
227+
},
228+
})
229+
const registry = new MockRegistry({
230+
tap: t,
231+
registry: npm.config.get('registry'),
232+
})
233+
const manifest = registry.manifest({
234+
name: 'test-dep-a',
235+
packuments: [{ version: '1.0.0' }, { version: '1.0.1' }],
236+
})
237+
manifest.time['1.0.0'] = '2020-01-01T00:00:00.000Z'
238+
manifest.time['1.0.1'] = new Date().toISOString()
239+
await registry.package({ manifest })
240+
const advisory = registry.advisory({ id: 100, vulnerable_versions: '<1.0.1' })
241+
const bulkBody = gzip(JSON.stringify({ 'test-dep-a': ['1.0.0'] }))
242+
registry.nock.post('/-/npm/v1/security/advisories/bulk', bulkBody)
243+
.reply(200, {
244+
'test-dep-a': [advisory],
245+
})
246+
247+
await npm.exec('audit', [])
248+
const report = JSON.parse(joinedOutput())
249+
t.match(report.vulnerabilities['test-dep-a'].fixBlockedByReleaseAge, { version: '1.0.1' },
250+
'json output flags the fix that min-release-age blocked')
251+
})
252+
173253
t.test('audit fix no package lock', async t => {
174254
const { npm } = await loadMockNpm(t, {
175255
config: {

workspaces/arborist/lib/arborist/build-ideal-tree.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,17 @@ module.exports = cls => class IdealTreeBuilder extends cls {
579579
// and leaving the user subject to getting it overwritten later anyway.
580580
async #queueVulnDependents (options) {
581581
for (const vuln of this.auditReport.values()) {
582+
// A fix is available in-range but a release-age window blocks the patched
583+
// version, so audit fix leaves this package at a vulnerable version.
584+
if (vuln.fixBlockedByReleaseAge) {
585+
const { version, before } = vuln.fixBlockedByReleaseAge
586+
const cutoff = new Date(before).toISOString().slice(0, 10)
587+
log.warn('audit', `A fix for ${vuln.name} is available (${vuln.name}@${version}) ` +
588+
`but was published after the configured release-age cutoff (${cutoff}), so ` +
589+
`${vuln.name} was left at a vulnerable version.\n` +
590+
`To install it, add "${vuln.name}" to min-release-age-exclude, or relax ` +
591+
'min-release-age or before.')
592+
}
582593
for (const node of vuln.nodes) {
583594
const bundler = node.getBundler()
584595

workspaces/arborist/lib/audit-report.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const pickManifest = require('npm-pick-manifest')
66

77
const Vuln = require('./vuln.js')
88
const Calculator = require('@npmcli/metavuln-calculator')
9+
const { isReleaseAgeExcluded } = require('./release-age-exclude.js')
910

1011
const { log, time } = require('proc-log')
1112

@@ -173,6 +174,11 @@ class AuditReport extends Map {
173174
// depends on root.
174175
this.topVulns.set(vuln.name, vuln)
175176
vuln.topNodes.add(dep)
177+
} else {
178+
// An in-range fix exists, but a `min-release-age`/`before`
179+
// window may put the patched version out of reach, leaving the
180+
// vulnerable version installed.
181+
vuln.fixBlockedByReleaseAge = this.#fixBlockedByReleaseAge(vuln, spec)
176182
}
177183
} else {
178184
// calculate a metavuln, if necessary
@@ -251,6 +257,53 @@ class AuditReport extends Map {
251257
}
252258
}
253259

260+
// A fix that `#fixAvailable` reports as installable in-range can still be
261+
// unreachable when a release-age window (`before` / `min-release-age`) is set,
262+
// because the patched version was published after the cutoff. `npm audit fix`
263+
// then resolves back to a version that is still vulnerable, so detect that
264+
// here and let callers warn about it. Only meaningful when an in-range fix
265+
// exists (fixAvailable === true). Returns the blocked fix as
266+
// `{ version, before }`, or `false` when nothing is blocked.
267+
#fixBlockedByReleaseAge (vuln, spec) {
268+
const { before, minReleaseAgeExclude } = this.options
269+
// No window, or this package is explicitly exempt from it.
270+
if (!before || isReleaseAgeExcluded(vuln.name, minReleaseAgeExclude)) {
271+
return false
272+
}
273+
274+
// For `npm:` aliases the fix resolves against the alias target, so feed
275+
// pickManifest the underlying range rather than the alias spec (which it
276+
// rejects). Mirrors `#fixAvailable`.
277+
const specObj = npa(spec)
278+
if (specObj.subSpec) {
279+
spec = specObj.subSpec.rawSpec
280+
}
281+
282+
// The version that would be installed if the window were lifted.
283+
const { version } = pickManifest(vuln.packument, spec, {
284+
...this.options,
285+
before: null,
286+
avoid: vuln.range,
287+
})
288+
289+
// What resolution can actually reach within the window. This mirrors how
290+
// `build-ideal-tree` re-resolves the edge (non-strict avoid + `before`).
291+
let windowed
292+
try {
293+
windowed = pickManifest(vuln.packument, spec, {
294+
...this.options,
295+
avoid: vuln.range,
296+
})
297+
} catch {
298+
// Nothing is old enough to install at all: the fix is blocked.
299+
return { version, before }
300+
}
301+
302+
// `_shouldAvoid` means the best version available within the window is still
303+
// in the vulnerable range, so the only non-vulnerable fix is too new.
304+
return windowed._shouldAvoid ? { version, before } : false
305+
}
306+
254307
set () {
255308
throw new Error('do not call AuditReport.set() directly')
256309
}

workspaces/arborist/lib/vuln.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class Vuln {
4141
this.effects = new Set()
4242
this.topNodes = new Set()
4343
this.nodes = new Set()
44+
this.fixBlockedByReleaseAge = false
4445
this.addAdvisory(advisory)
4546
this.packument = advisory.packument
4647
this.versions = advisory.versions
@@ -126,6 +127,9 @@ class Vuln {
126127
range: this.simpleRange,
127128
nodes: [...this.nodes].map(n => n.location).sort(localeCompare),
128129
fixAvailable: this.#fixAvailable,
130+
...(this.fixBlockedByReleaseAge
131+
? { fixBlockedByReleaseAge: this.fixBlockedByReleaseAge }
132+
: {}),
129133
}
130134
}
131135

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,10 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with before: opt
11311131
"name": "nyc",
11321132
"version": "15.1.0",
11331133
"isSemVerMajor": true
1134+
},
1135+
"fixBlockedByReleaseAge": {
1136+
"version": "0.5.5",
1137+
"before": "2020-01-01T00:00:00.000Z"
11341138
}
11351139
},
11361140
"nyc": {

workspaces/arborist/test/arborist/build-ideal-tree.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,27 @@ t.test('force a new nyc (and update mkdirp nicely)', async t => {
588588
t.equal(arb.idealTree.children.get('nyc').package.version, '15.1.0')
589589
})
590590

591+
t.test('audit fix warns when min-release-age blocks a fix', async t => {
592+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
593+
const registry = createRegistry(t, true)
594+
registry.audit({ convert: true, results: require('../fixtures/audit-nyc-mkdirp/audit.json') })
595+
const warnings = warningTracker(t)
596+
597+
// mkdirp's fix (0.5.5) was published after this cutoff, so audit fix can't
598+
// install it and should warn that the package is left vulnerable.
599+
const arb = newArb(path, { before: new Date('2020-01-01') })
600+
await arb.audit()
601+
await arb.buildIdealTree()
602+
603+
t.not(arb.idealTree.children.get('mkdirp').package.version, '0.5.5',
604+
'mkdirp was not upgraded to the release-age-blocked fix')
605+
t.ok(
606+
warnings.some(w => w[1] === 'audit' &&
607+
/A fix for mkdirp is available \(mkdirp@0\.5\.5\) but was published after/.test(w[2])),
608+
'warned that the mkdirp fix is blocked by the release-age window'
609+
)
610+
})
611+
591612
t.test('force a new mkdirp (but not semver major)', async t => {
592613
const path = resolve(fixtures, 'mkdirp-pinned')
593614
const registry = createRegistry(t, true)

workspaces/arborist/test/audit-report.js

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,71 @@ t.test('audit outdated nyc and mkdirp with before: option', async t => {
181181
t.equal(report.get('mkdirp').simpleRange, '0.4.1 - 0.5.1')
182182
})
183183

184+
t.test('min-release-age blocks an available fix', async t => {
185+
// mkdirp's fix (0.5.5, published 2020-04) is newer than a 2020-01-01 cutoff,
186+
// so the only versions old enough are still vulnerable and audit fix can't
187+
// apply the fix it reported as available.
188+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
189+
const registry = createRegistry(t)
190+
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
191+
registry.mocks({ dir: join(__dirname, 'fixtures') })
192+
const cache = t.testdir()
193+
const arb = newArb(path, { before: new Date('2020-01-01'), cache })
194+
195+
const tree = await arb.loadVirtual()
196+
const report = await AuditReport.load(tree, arb.options)
197+
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
198+
'mkdirp fix flagged as blocked by the release-age window')
199+
})
200+
201+
t.test('min-release-age does not block a fix that is old enough', async t => {
202+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
203+
const registry = createRegistry(t)
204+
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
205+
registry.mocks({ dir: join(__dirname, 'fixtures') })
206+
const cache = t.testdir()
207+
// a cutoff after mkdirp@0.5.5 was published: the fix is reachable
208+
const arb = newArb(path, { before: new Date('2021-01-01'), cache })
209+
210+
const tree = await arb.loadVirtual()
211+
const report = await AuditReport.load(tree, arb.options)
212+
t.notOk(report.get('mkdirp').fixBlockedByReleaseAge,
213+
'fix reachable within the window, so not flagged')
214+
})
215+
216+
t.test('min-release-age-exclude exempts a package from the block', async t => {
217+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
218+
const registry = createRegistry(t)
219+
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
220+
registry.mocks({ dir: join(__dirname, 'fixtures') })
221+
const cache = t.testdir()
222+
const arb = newArb(path, {
223+
before: new Date('2020-01-01'),
224+
minReleaseAgeExclude: ['mkdirp'],
225+
cache,
226+
})
227+
228+
const tree = await arb.loadVirtual()
229+
const report = await AuditReport.load(tree, arb.options)
230+
t.notOk(report.get('mkdirp').fixBlockedByReleaseAge,
231+
'excluded package is not flagged even when its fix is too new')
232+
})
233+
234+
t.test('min-release-age blocks when no version is old enough at all', async t => {
235+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
236+
const registry = createRegistry(t)
237+
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
238+
registry.mocks({ dir: join(__dirname, 'fixtures') })
239+
const cache = t.testdir()
240+
// a cutoff before any mkdirp version was published: nothing is installable
241+
const arb = newArb(path, { before: new Date('2000-01-01'), cache })
242+
243+
const tree = await arb.loadVirtual()
244+
const report = await AuditReport.load(tree, arb.options)
245+
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
246+
'flagged as blocked when nothing is installable within the window')
247+
})
248+
184249
t.test('audit returns an error', async t => {
185250
const path = resolve(fixtures, 'audit-nyc-mkdirp')
186251
const registry = createRegistry(t)
@@ -421,6 +486,43 @@ t.test('audit supports alias deps', async t => {
421486
t.equal(report.get('mkdirp').simpleRange, '0.4.1 - 0.5.1')
422487
})
423488

489+
t.test('release-age block detection unwraps alias specs', async t => {
490+
// An npm: alias edge must be resolved against its target, not fed to
491+
// pickManifest as an alias spec (which it rejects). With a release-age
492+
// window the alias fix (mkdirp@0.5.5) is too new, so it should be flagged.
493+
const path = resolve(fixtures, 'audit-nyc-mkdirp')
494+
const registry = createRegistry(t)
495+
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
496+
registry.mocks({ dir: join(__dirname, 'fixtures') })
497+
const cache = t.testdir()
498+
const arb = newArb(path, { before: new Date('2020-01-01'), cache })
499+
const tree = new Node({
500+
path,
501+
pkg: {
502+
name: 'mkdirp',
503+
version: '0.5.0',
504+
dependencies: {
505+
novulnshereiswear: 'npm:mkdirp@^0.5.0',
506+
},
507+
},
508+
children: [
509+
{
510+
name: 'novulnshereiswear',
511+
pkg: {
512+
name: 'mkdirp',
513+
version: '0.5.1',
514+
dependencies: { minimist: '0.0.8' },
515+
},
516+
},
517+
{ pkg: { name: 'minimist', version: '0.0.8' } },
518+
],
519+
})
520+
521+
const report = await AuditReport.load(tree, arb.options)
522+
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
523+
'alias spec is unwrapped and the blocked fix is detected')
524+
})
525+
424526
t.test('linked local package should not be audited against the registry', async t => {
425527
const path = resolve(fixtures, 'audit-linked-package')
426528
// No registry.audit() mock needed — no request should be made

0 commit comments

Comments
 (0)