Skip to content

Commit

Permalink
fix: set audit exit code properly
Browse files Browse the repository at this point in the history
When running 'npm audit', we properly exited correctly with the
appropriate exitCode based on the audit level config and the report
results.

However, when going through the reifyFinish() function (as we do for
'npm audit fix'), we were not setting that properly if the auditLevel
was not set.

Furthermore, if the auditLevel WAS set, we were setting the exit code to
non-zero for *other* reify commands (install, update, etc.), where the
audit information should be strictly advisory.

When --json and --loglevel=silent were set, the exitCode was never being
set properly.

This fixes all these problems.
  • Loading branch information
isaacs committed May 26, 2021
1 parent 83590d4 commit 41edaec
Show file tree
Hide file tree
Showing 2 changed files with 171 additions and 28 deletions.
34 changes: 27 additions & 7 deletions lib/utils/reify-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,20 @@ const auditError = require('./audit-error.js')

// TODO: output JSON if flatOptions.json is true
const reifyOutput = (npm, arb) => {
// don't print any info in --silent mode
if (log.levels[log.level] > log.levels.error)
return

const { diff, actualTree } = arb

// note: fails and crashes if we're running audit fix and there was an error
// which is a good thing, because there's no point printing all this other
// stuff in that case!
const auditReport = auditError(npm, arb.auditReport) ? null : arb.auditReport

// don't print any info in --silent mode, but we still need to
// set the exitCode properly from the audit report, if we have one.
if (log.levels[log.level] > log.levels.error) {
getAuditReport(npm, auditReport)
return
}

const summary = {
added: 0,
removed: 0,
Expand Down Expand Up @@ -68,6 +71,8 @@ const reifyOutput = (npm, arb) => {

if (npm.flatOptions.json) {
if (auditReport) {
// call this to set the exit code properly
getAuditReport(npm, auditReport)
summary.audit = npm.command === 'audit' ? auditReport
: auditReport.toJSON().metadata
}
Expand All @@ -83,11 +88,25 @@ const reifyOutput = (npm, arb) => {
// at the end if there's still stuff, because it's silly for `npm audit`
// to tell you to run `npm audit` for details. otherwise, use the summary
// report. if we get here, we know it's not quiet or json.
// If the loglevel is set higher than 'error', then we just run the report
// to get the exitCode set appropriately.
const printAuditReport = (npm, report) => {
const res = getAuditReport(npm, report)
if (!res || !res.report)
return
npm.output(`\n${res.report}`)
}

const getAuditReport = (npm, report) => {
if (!report)
return

const reporter = npm.command !== 'audit' ? 'install' : 'detail'
// when in silent mode, we print nothing. the JSON output is
// going to just JSON.stringify() the report object.
const reporter = log.levels[log.level] > log.levels.error ? 'quiet'
: npm.flatOptions.json ? 'quiet'
: npm.command !== 'audit' ? 'install'
: 'detail'
const defaultAuditLevel = npm.command !== 'audit' ? 'none' : 'low'
const auditLevel = npm.flatOptions.auditLevel || defaultAuditLevel

Expand All @@ -96,8 +115,9 @@ const printAuditReport = (npm, report) => {
...npm.flatOptions,
auditLevel,
})
process.exitCode = process.exitCode || res.exitCode
npm.output('\n' + res.report)
if (npm.command === 'audit')
process.exitCode = process.exitCode || res.exitCode
return res
}

const packagesChangedMessage = (npm, { added, removed, changed, audited }) => {
Expand Down
165 changes: 144 additions & 21 deletions test/lib/utils/reify-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,31 +187,154 @@ t.test('print appropriate message for many packages', (t) => {
})
})

t.test('no output when silent', t => {
npm.output = out => {
t.fail('should not get output when silent', { actual: out })
}
t.teardown(() => log.level = 'warn')
log.level = 'silent'
reifyOutput(npm, {
actualTree: { inventory: { size: 999 }, children: [] },
auditReport: {
toJSON: () => {
throw new Error('this should not get called')
},
vulnerabilities: {},
metadata: {
vulnerabilities: {
total: 99,
},
t.test('showing and not showing audit report', async t => {
const auditReport = {
toJSON: () => auditReport,
auditReportVersion: 2,
vulnerabilities: {
minimist: {
name: 'minimist',
severity: 'low',
via: [
{
id: 1179,
url: 'https://npmjs.com/advisories/1179',
title: 'Prototype Pollution',
severity: 'low',
vulnerable_versions: '<0.2.1 || >=1.0.0 <1.2.3',
},
],
effects: [],
range: '<0.2.1 || >=1.0.0 <1.2.3',
nodes: [
'node_modules/minimist',
],
fixAvailable: true,
},
},
diff: {
children: [
{ action: 'ADD', ideal: { location: 'loc' } },
],
metadata: {
vulnerabilities: {
info: 0,
low: 1,
moderate: 0,
high: 0,
critical: 0,
total: 1,
},
dependencies: {
prod: 1,
dev: 0,
optional: 0,
peer: 0,
peerOptional: 0,
total: 1,
},
},
}

t.test('no output when silent', t => {
npm.output = out => {
t.fail('should not get output when silent', { actual: out })
}
t.teardown(() => log.level = 'warn')
log.level = 'silent'
reifyOutput(npm, {
actualTree: { inventory: { size: 999 }, children: [] },
auditReport,
diff: {
children: [
{ action: 'ADD', ideal: { location: 'loc' } },
],
},
})
t.end()
})

t.test('output when not silent', t => {
const OUT = []
npm.output = out => {
OUT.push(out)
}
reifyOutput(npm, {
actualTree: { inventory: new Map(), children: [] },
auditReport,
diff: {
children: [
{ action: 'ADD', ideal: { location: 'loc' } },
],
},
})
t.match(OUT.join('\n'), /Run `npm audit` for details\.$/, 'got audit report')
t.end()
})

for (const json of [true, false]) {
t.test(`json=${json}`, t => {
t.teardown(() => {
delete npm.flatOptions.json
})
npm.flatOptions.json = json
t.test('set exit code when cmd is audit', t => {
npm.output = () => {}
const { exitCode } = process
const { command } = npm
npm.flatOptions.auditLevel = 'low'
t.teardown(() => {
delete npm.flatOptions.auditLevel
npm.command = command
// only set exitCode back if we're passing tests
if (t.passing())
process.exitCode = exitCode
})

process.exitCode = 0
npm.command = 'audit'
reifyOutput(npm, {
actualTree: { inventory: new Map(), children: [] },
auditReport,
diff: {
children: [
{ action: 'ADD', ideal: { location: 'loc' } },
],
},
})

t.equal(process.exitCode, 1, 'set exit code')
t.end()
})

t.test('do not set exit code when cmd is install', t => {
npm.output = () => {}
const { exitCode } = process
const { command } = npm
npm.flatOptions.auditLevel = 'low'
t.teardown(() => {
delete npm.flatOptions.auditLevel
npm.command = command
// only set exitCode back if we're passing tests
if (t.passing())
process.exitCode = exitCode
})

process.exitCode = 0
npm.command = 'install'
reifyOutput(npm, {
actualTree: { inventory: new Map(), children: [] },
auditReport,
diff: {
children: [
{ action: 'ADD', ideal: { location: 'loc' } },
],
},
})

t.equal(process.exitCode, 0, 'did not set exit code')
t.end()
})
t.end()
})
}

t.end()
})

Expand Down

0 comments on commit 41edaec

Please sign in to comment.