diff --git a/lib/commands/audit.js b/lib/commands/audit.js index a8d742cc73a6e..6a55af4313584 100644 --- a/lib/commands/audit.js +++ b/lib/commands/audit.js @@ -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) diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 1dd3b0d7e6b5c..a5f68d63edc15 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -404,6 +404,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. @@ -1279,6 +1283,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. diff --git a/test/lib/commands/audit.js b/test/lib/commands/audit.js index 3bd5e5033c499..61887417216eb 100644 --- a/test/lib/commands/audit.js +++ b/test/lib/commands/audit.js @@ -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: { diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 68527972b1fc3..b8b00698c8a59 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -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() diff --git a/workspaces/arborist/lib/audit-report.js b/workspaces/arborist/lib/audit-report.js index 3e74e100b6c53..de2e092a96ac1 100644 --- a/workspaces/arborist/lib/audit-report.js +++ b/workspaces/arborist/lib/audit-report.js @@ -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') @@ -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 @@ -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') } diff --git a/workspaces/arborist/lib/vuln.js b/workspaces/arborist/lib/vuln.js index 2bffe54f2dacd..e6c6bbab383ea 100644 --- a/workspaces/arborist/lib/vuln.js +++ b/workspaces/arborist/lib/vuln.js @@ -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 @@ -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 } + : {}), } } diff --git a/workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs b/workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs index 2d2374afff4af..a31e072da2f79 100644 --- a/workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/audit-report.js.test.cjs @@ -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": { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 1abd022a6723e..f399bea2864d6 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -630,6 +630,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) diff --git a/workspaces/arborist/test/audit-report.js b/workspaces/arborist/test/audit-report.js index b0b742bd30fd8..0322b48dbd534 100644 --- a/workspaces/arborist/test/audit-report.js +++ b/workspaces/arborist/test/audit-report.js @@ -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) @@ -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 diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index a4933facff631..ead04cfc5cc17 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -341,6 +341,10 @@ const definitions = { 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. `, @@ -1475,6 +1479,12 @@ const definitions = { \`github:\` dependency); when both 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. `,