Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/commands/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ class Audit extends ArboristWorkspaceCmd {
await arb.audit({ fix })
if (fix) {
await reifyFinish(this.npm, arb)
// Report any fix that a `min-release-age`/`before` window blocked from
// installing, and exit non-zero so a blocked fix is not missed.
const report = arb.auditReport
const blocked = report instanceof Map
? [...report.values()].filter(v => v.fixBlockedByReleaseAge).map(v => v.name)
: []
if (blocked.length) {
log.warn('audit', `${blocked.length} package(s) left at a vulnerable version because ` +
`a fix is newer than the release-age cutoff: ${blocked.join(', ')}.\n` +
'Add the package to min-release-age-exclude, or relax min-release-age or before, ' +
'to install the fix.')
process.exitCode = 1
}
} else {
// will throw if there's an error, because this is an audit command
auditError(this.npm, arb.auditReport)
Expand Down
10 changes: 10 additions & 0 deletions tap-snapshots/test/lib/docs.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ sources, the standard precedence applies (cli > env > project > user >
global), so a higher-priority source can always relax or override a
lower-priority one.

As with \`min-release-age\`, when this cutoff blocks a fix that \`npm audit
fix\` would install, npm keeps the vulnerable version, warns, and exits with
a non-zero code.

Packages whose names match \`min-release-age-exclude\` are exempt from this
filter.

Expand Down Expand Up @@ -1303,6 +1307,12 @@ your \`.npmrc\` is preserved when npm internally spawns a sub-process with
apply, \`before\` wins within a single source and across sources the standard
precedence rules apply.

When this window stops \`npm audit fix\` from installing a patched version
(because the fix was published too recently), npm keeps the package at its
vulnerable version, warns that the fix was blocked, and exits with a
non-zero code. To install the fix, add the package to
\`min-release-age-exclude\`, or relax \`min-release-age\` or \`before\`.

Packages whose names match \`min-release-age-exclude\` are exempt from this
filter.

Expand Down
80 changes: 80 additions & 0 deletions test/lib/commands/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,86 @@ t.test('audit fix - bulk endpoint', async t => {
)
})

t.test('audit fix exits non-zero when min-release-age blocks a fix', async t => {
const { npm, logs } = await loadMockNpm(t, {
prefixDir: tree,
config: { 'min-release-age': 30 },
})
const registry = new MockRegistry({
tap: t,
registry: npm.config.get('registry'),
})
const manifest = registry.manifest({
name: 'test-dep-a',
packuments: [{ version: '1.0.0' }, { version: '1.0.1' }],
})
// 1.0.0 is old enough to install; the fix 1.0.1 was published too recently.
manifest.time['1.0.0'] = '2020-01-01T00:00:00.000Z'
manifest.time['1.0.1'] = new Date().toISOString()
await registry.package({
manifest,
tarballs: {
'1.0.0': path.join(npm.prefix, 'test-dep-a-vuln'),
},
times: 2,
})
const advisory = registry.advisory({ id: 100, vulnerable_versions: '<1.0.1' })
registry.nock.post('/-/npm/v1/security/advisories/bulk', body => {
const unzipped = JSON.parse(gunzip(Buffer.from(body, 'hex')))
return t.same(unzipped, { 'test-dep-a': ['1.0.0'] })
})
.reply(200, { 'test-dep-a': [advisory] })
.post('/-/npm/v1/security/advisories/bulk', body => {
const unzipped = JSON.parse(gunzip(Buffer.from(body, 'hex')))
return t.same(unzipped, { 'test-dep-a': ['1.0.0'] })
})
.reply(200, { 'test-dep-a': [advisory] })

await npm.exec('audit', ['fix'])

t.equal(process.exitCode, 1, 'exits non-zero because the fix was blocked')
t.ok(
logs.warn.some(w =>
/left at a vulnerable version because a fix is newer than the release-age cutoff/.test(w)),
'warns that the fix was blocked by min-release-age'
)
const lock = JSON.parse(fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8'))
t.equal(lock.packages['node_modules/test-dep-a'].version, '1.0.0',
'test-dep-a was left at the vulnerable version')
})

t.test('json audit reports fixBlockedByReleaseAge when a fix is too new', async t => {
const { npm, joinedOutput } = await loadMockNpm(t, {
prefixDir: tree,
config: {
json: true,
'min-release-age': 30,
},
})
const registry = new MockRegistry({
tap: t,
registry: npm.config.get('registry'),
})
const manifest = registry.manifest({
name: 'test-dep-a',
packuments: [{ version: '1.0.0' }, { version: '1.0.1' }],
})
manifest.time['1.0.0'] = '2020-01-01T00:00:00.000Z'
manifest.time['1.0.1'] = new Date().toISOString()
await registry.package({ manifest })
const advisory = registry.advisory({ id: 100, vulnerable_versions: '<1.0.1' })
const bulkBody = gzip(JSON.stringify({ 'test-dep-a': ['1.0.0'] }))
registry.nock.post('/-/npm/v1/security/advisories/bulk', bulkBody)
.reply(200, {
'test-dep-a': [advisory],
})

await npm.exec('audit', [])
const report = JSON.parse(joinedOutput())
t.match(report.vulnerabilities['test-dep-a'].fixBlockedByReleaseAge, { version: '1.0.1' },
'json output flags the fix that min-release-age blocked')
})

t.test('audit fix no package lock', async t => {
const { npm } = await loadMockNpm(t, {
config: {
Expand Down
11 changes: 11 additions & 0 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,17 @@ module.exports = cls => class IdealTreeBuilder extends cls {
// and leaving the user subject to getting it overwritten later anyway.
async #queueVulnDependents (options) {
for (const vuln of this.auditReport.values()) {
// A fix is available in-range but a release-age window blocks the patched
// version, so audit fix leaves this package at a vulnerable version.
if (vuln.fixBlockedByReleaseAge) {
const { version, before } = vuln.fixBlockedByReleaseAge
const cutoff = new Date(before).toISOString().slice(0, 10)
log.warn('audit', `A fix for ${vuln.name} is available (${vuln.name}@${version}) ` +
`but was published after the configured release-age cutoff (${cutoff}), so ` +
`${vuln.name} was left at a vulnerable version.\n` +
`To install it, add "${vuln.name}" to min-release-age-exclude, or relax ` +
'min-release-age or before.')
}
for (const node of vuln.nodes) {
const bundler = node.getBundler()

Expand Down
53 changes: 53 additions & 0 deletions workspaces/arborist/lib/audit-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const pickManifest = require('npm-pick-manifest')

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

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

Expand Down Expand Up @@ -173,6 +174,11 @@ class AuditReport extends Map {
// depends on root.
this.topVulns.set(vuln.name, vuln)
vuln.topNodes.add(dep)
} else {
// An in-range fix exists, but a `min-release-age`/`before`
// window may put the patched version out of reach, leaving the
// vulnerable version installed.
vuln.fixBlockedByReleaseAge = this.#fixBlockedByReleaseAge(vuln, spec)
}
} else {
// calculate a metavuln, if necessary
Expand Down Expand Up @@ -251,6 +257,53 @@ class AuditReport extends Map {
}
}

// A fix that `#fixAvailable` reports as installable in-range can still be
// unreachable when a release-age window (`before` / `min-release-age`) is set,
// because the patched version was published after the cutoff. `npm audit fix`
// then resolves back to a version that is still vulnerable, so detect that
// here and let callers warn about it. Only meaningful when an in-range fix
// exists (fixAvailable === true). Returns the blocked fix as
// `{ version, before }`, or `false` when nothing is blocked.
#fixBlockedByReleaseAge (vuln, spec) {
const { before, minReleaseAgeExclude } = this.options
// No window, or this package is explicitly exempt from it.
if (!before || isReleaseAgeExcluded(vuln.name, minReleaseAgeExclude)) {
return false
}

// For `npm:` aliases the fix resolves against the alias target, so feed
// pickManifest the underlying range rather than the alias spec (which it
// rejects). Mirrors `#fixAvailable`.
const specObj = npa(spec)
if (specObj.subSpec) {
spec = specObj.subSpec.rawSpec
}

// The version that would be installed if the window were lifted.
const { version } = pickManifest(vuln.packument, spec, {
...this.options,
before: null,
avoid: vuln.range,
})

// What resolution can actually reach within the window. This mirrors how
// `build-ideal-tree` re-resolves the edge (non-strict avoid + `before`).
let windowed
try {
windowed = pickManifest(vuln.packument, spec, {
...this.options,
avoid: vuln.range,
})
} catch {
// Nothing is old enough to install at all: the fix is blocked.
return { version, before }
}

// `_shouldAvoid` means the best version available within the window is still
// in the vulnerable range, so the only non-vulnerable fix is too new.
return windowed._shouldAvoid ? { version, before } : false
}

set () {
throw new Error('do not call AuditReport.set() directly')
}
Expand Down
4 changes: 4 additions & 0 deletions workspaces/arborist/lib/vuln.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class Vuln {
this.effects = new Set()
this.topNodes = new Set()
this.nodes = new Set()
this.fixBlockedByReleaseAge = false
this.addAdvisory(advisory)
this.packument = advisory.packument
this.versions = advisory.versions
Expand Down Expand Up @@ -126,6 +127,9 @@ class Vuln {
range: this.simpleRange,
nodes: [...this.nodes].map(n => n.location).sort(localeCompare),
fixAvailable: this.#fixAvailable,
...(this.fixBlockedByReleaseAge
? { fixBlockedByReleaseAge: this.fixBlockedByReleaseAge }
: {}),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,10 @@ exports[`test/audit-report.js TAP audit outdated nyc and mkdirp with before: opt
"name": "nyc",
"version": "15.1.0",
"isSemVerMajor": true
},
"fixBlockedByReleaseAge": {
"version": "0.5.5",
"before": "2020-01-01T00:00:00.000Z"
}
},
"nyc": {
Expand Down
21 changes: 21 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,27 @@ t.test('force a new nyc (and update mkdirp nicely)', async t => {
t.equal(arb.idealTree.children.get('nyc').package.version, '15.1.0')
})

t.test('audit fix warns when min-release-age blocks a fix', async t => {
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t, true)
registry.audit({ convert: true, results: require('../fixtures/audit-nyc-mkdirp/audit.json') })
const warnings = warningTracker(t)

// mkdirp's fix (0.5.5) was published after this cutoff, so audit fix can't
// install it and should warn that the package is left vulnerable.
const arb = newArb(path, { before: new Date('2020-01-01') })
await arb.audit()
await arb.buildIdealTree()

t.not(arb.idealTree.children.get('mkdirp').package.version, '0.5.5',
'mkdirp was not upgraded to the release-age-blocked fix')
t.ok(
warnings.some(w => w[1] === 'audit' &&
/A fix for mkdirp is available \(mkdirp@0\.5\.5\) but was published after/.test(w[2])),
'warned that the mkdirp fix is blocked by the release-age window'
)
})

t.test('force a new mkdirp (but not semver major)', async t => {
const path = resolve(fixtures, 'mkdirp-pinned')
const registry = createRegistry(t, true)
Expand Down
102 changes: 102 additions & 0 deletions workspaces/arborist/test/audit-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,71 @@ t.test('audit outdated nyc and mkdirp with before: option', async t => {
t.equal(report.get('mkdirp').simpleRange, '0.4.1 - 0.5.1')
})

t.test('min-release-age blocks an available fix', async t => {
// mkdirp's fix (0.5.5, published 2020-04) is newer than a 2020-01-01 cutoff,
// so the only versions old enough are still vulnerable and audit fix can't
// apply the fix it reported as available.
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
registry.mocks({ dir: join(__dirname, 'fixtures') })
const cache = t.testdir()
const arb = newArb(path, { before: new Date('2020-01-01'), cache })

const tree = await arb.loadVirtual()
const report = await AuditReport.load(tree, arb.options)
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
'mkdirp fix flagged as blocked by the release-age window')
})

t.test('min-release-age does not block a fix that is old enough', async t => {
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
registry.mocks({ dir: join(__dirname, 'fixtures') })
const cache = t.testdir()
// a cutoff after mkdirp@0.5.5 was published: the fix is reachable
const arb = newArb(path, { before: new Date('2021-01-01'), cache })

const tree = await arb.loadVirtual()
const report = await AuditReport.load(tree, arb.options)
t.notOk(report.get('mkdirp').fixBlockedByReleaseAge,
'fix reachable within the window, so not flagged')
})

t.test('min-release-age-exclude exempts a package from the block', async t => {
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
registry.mocks({ dir: join(__dirname, 'fixtures') })
const cache = t.testdir()
const arb = newArb(path, {
before: new Date('2020-01-01'),
minReleaseAgeExclude: ['mkdirp'],
cache,
})

const tree = await arb.loadVirtual()
const report = await AuditReport.load(tree, arb.options)
t.notOk(report.get('mkdirp').fixBlockedByReleaseAge,
'excluded package is not flagged even when its fix is too new')
})

t.test('min-release-age blocks when no version is old enough at all', async t => {
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
registry.mocks({ dir: join(__dirname, 'fixtures') })
const cache = t.testdir()
// a cutoff before any mkdirp version was published: nothing is installable
const arb = newArb(path, { before: new Date('2000-01-01'), cache })

const tree = await arb.loadVirtual()
const report = await AuditReport.load(tree, arb.options)
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
'flagged as blocked when nothing is installable within the window')
})

t.test('audit returns an error', async t => {
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
Expand Down Expand Up @@ -421,6 +486,43 @@ t.test('audit supports alias deps', async t => {
t.equal(report.get('mkdirp').simpleRange, '0.4.1 - 0.5.1')
})

t.test('release-age block detection unwraps alias specs', async t => {
// An npm: alias edge must be resolved against its target, not fed to
// pickManifest as an alias spec (which it rejects). With a release-age
// window the alias fix (mkdirp@0.5.5) is too new, so it should be flagged.
const path = resolve(fixtures, 'audit-nyc-mkdirp')
const registry = createRegistry(t)
registry.audit({ results: require(resolve(path, 'advisory-bulk.json')) })
registry.mocks({ dir: join(__dirname, 'fixtures') })
const cache = t.testdir()
const arb = newArb(path, { before: new Date('2020-01-01'), cache })
const tree = new Node({
path,
pkg: {
name: 'mkdirp',
version: '0.5.0',
dependencies: {
novulnshereiswear: 'npm:mkdirp@^0.5.0',
},
},
children: [
{
name: 'novulnshereiswear',
pkg: {
name: 'mkdirp',
version: '0.5.1',
dependencies: { minimist: '0.0.8' },
},
},
{ pkg: { name: 'minimist', version: '0.0.8' } },
],
})

const report = await AuditReport.load(tree, arb.options)
t.match(report.get('mkdirp').fixBlockedByReleaseAge, { version: '0.5.5' },
'alias spec is unwrapped and the blocked fix is detected')
})

t.test('linked local package should not be audited against the registry', async t => {
const path = resolve(fixtures, 'audit-linked-package')
// No registry.audit() mock needed — no request should be made
Expand Down
Loading
Loading