Skip to content

Commit 554e8a5

Browse files
isaacswraithgar
authored andcommitted
fix: set audit exit code properly
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. PR-URL: #3311 Credit: @isaacs Close: #3311 Reviewed-by: @wraithgar
1 parent 3c53d63 commit 554e8a5

File tree

2 files changed

+171
-28
lines changed

2 files changed

+171
-28
lines changed

lib/utils/reify-output.js

+27-7
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,20 @@ const auditError = require('./audit-error.js')
1818

1919
// TODO: output JSON if flatOptions.json is true
2020
const reifyOutput = (npm, arb) => {
21-
// don't print any info in --silent mode
22-
if (log.levels[log.level] > log.levels.error)
23-
return
24-
2521
const { diff, actualTree } = arb
2622

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

28+
// don't print any info in --silent mode, but we still need to
29+
// set the exitCode properly from the audit report, if we have one.
30+
if (log.levels[log.level] > log.levels.error) {
31+
getAuditReport(npm, auditReport)
32+
return
33+
}
34+
3235
const summary = {
3336
added: 0,
3437
removed: 0,
@@ -68,6 +71,8 @@ const reifyOutput = (npm, arb) => {
6871

6972
if (npm.flatOptions.json) {
7073
if (auditReport) {
74+
// call this to set the exit code properly
75+
getAuditReport(npm, auditReport)
7176
summary.audit = npm.command === 'audit' ? auditReport
7277
: auditReport.toJSON().metadata
7378
}
@@ -83,11 +88,25 @@ const reifyOutput = (npm, arb) => {
8388
// at the end if there's still stuff, because it's silly for `npm audit`
8489
// to tell you to run `npm audit` for details. otherwise, use the summary
8590
// report. if we get here, we know it's not quiet or json.
91+
// If the loglevel is set higher than 'error', then we just run the report
92+
// to get the exitCode set appropriately.
8693
const printAuditReport = (npm, report) => {
94+
const res = getAuditReport(npm, report)
95+
if (!res || !res.report)
96+
return
97+
npm.output(`\n${res.report}`)
98+
}
99+
100+
const getAuditReport = (npm, report) => {
87101
if (!report)
88102
return
89103

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

@@ -96,8 +115,9 @@ const printAuditReport = (npm, report) => {
96115
...npm.flatOptions,
97116
auditLevel,
98117
})
99-
process.exitCode = process.exitCode || res.exitCode
100-
npm.output('\n' + res.report)
118+
if (npm.command === 'audit')
119+
process.exitCode = process.exitCode || res.exitCode
120+
return res
101121
}
102122

103123
const packagesChangedMessage = (npm, { added, removed, changed, audited }) => {

test/lib/utils/reify-output.js

+144-21
Original file line numberDiff line numberDiff line change
@@ -187,31 +187,154 @@ t.test('print appropriate message for many packages', (t) => {
187187
})
188188
})
189189

190-
t.test('no output when silent', t => {
191-
npm.output = out => {
192-
t.fail('should not get output when silent', { actual: out })
193-
}
194-
t.teardown(() => log.level = 'warn')
195-
log.level = 'silent'
196-
reifyOutput(npm, {
197-
actualTree: { inventory: { size: 999 }, children: [] },
198-
auditReport: {
199-
toJSON: () => {
200-
throw new Error('this should not get called')
201-
},
202-
vulnerabilities: {},
203-
metadata: {
204-
vulnerabilities: {
205-
total: 99,
206-
},
190+
t.test('showing and not showing audit report', async t => {
191+
const auditReport = {
192+
toJSON: () => auditReport,
193+
auditReportVersion: 2,
194+
vulnerabilities: {
195+
minimist: {
196+
name: 'minimist',
197+
severity: 'low',
198+
via: [
199+
{
200+
id: 1179,
201+
url: 'https://npmjs.com/advisories/1179',
202+
title: 'Prototype Pollution',
203+
severity: 'low',
204+
vulnerable_versions: '<0.2.1 || >=1.0.0 <1.2.3',
205+
},
206+
],
207+
effects: [],
208+
range: '<0.2.1 || >=1.0.0 <1.2.3',
209+
nodes: [
210+
'node_modules/minimist',
211+
],
212+
fixAvailable: true,
207213
},
208214
},
209-
diff: {
210-
children: [
211-
{ action: 'ADD', ideal: { location: 'loc' } },
212-
],
215+
metadata: {
216+
vulnerabilities: {
217+
info: 0,
218+
low: 1,
219+
moderate: 0,
220+
high: 0,
221+
critical: 0,
222+
total: 1,
223+
},
224+
dependencies: {
225+
prod: 1,
226+
dev: 0,
227+
optional: 0,
228+
peer: 0,
229+
peerOptional: 0,
230+
total: 1,
231+
},
213232
},
233+
}
234+
235+
t.test('no output when silent', t => {
236+
npm.output = out => {
237+
t.fail('should not get output when silent', { actual: out })
238+
}
239+
t.teardown(() => log.level = 'warn')
240+
log.level = 'silent'
241+
reifyOutput(npm, {
242+
actualTree: { inventory: { size: 999 }, children: [] },
243+
auditReport,
244+
diff: {
245+
children: [
246+
{ action: 'ADD', ideal: { location: 'loc' } },
247+
],
248+
},
249+
})
250+
t.end()
214251
})
252+
253+
t.test('output when not silent', t => {
254+
const OUT = []
255+
npm.output = out => {
256+
OUT.push(out)
257+
}
258+
reifyOutput(npm, {
259+
actualTree: { inventory: new Map(), children: [] },
260+
auditReport,
261+
diff: {
262+
children: [
263+
{ action: 'ADD', ideal: { location: 'loc' } },
264+
],
265+
},
266+
})
267+
t.match(OUT.join('\n'), /Run `npm audit` for details\.$/, 'got audit report')
268+
t.end()
269+
})
270+
271+
for (const json of [true, false]) {
272+
t.test(`json=${json}`, t => {
273+
t.teardown(() => {
274+
delete npm.flatOptions.json
275+
})
276+
npm.flatOptions.json = json
277+
t.test('set exit code when cmd is audit', t => {
278+
npm.output = () => {}
279+
const { exitCode } = process
280+
const { command } = npm
281+
npm.flatOptions.auditLevel = 'low'
282+
t.teardown(() => {
283+
delete npm.flatOptions.auditLevel
284+
npm.command = command
285+
// only set exitCode back if we're passing tests
286+
if (t.passing())
287+
process.exitCode = exitCode
288+
})
289+
290+
process.exitCode = 0
291+
npm.command = 'audit'
292+
reifyOutput(npm, {
293+
actualTree: { inventory: new Map(), children: [] },
294+
auditReport,
295+
diff: {
296+
children: [
297+
{ action: 'ADD', ideal: { location: 'loc' } },
298+
],
299+
},
300+
})
301+
302+
t.equal(process.exitCode, 1, 'set exit code')
303+
t.end()
304+
})
305+
306+
t.test('do not set exit code when cmd is install', t => {
307+
npm.output = () => {}
308+
const { exitCode } = process
309+
const { command } = npm
310+
npm.flatOptions.auditLevel = 'low'
311+
t.teardown(() => {
312+
delete npm.flatOptions.auditLevel
313+
npm.command = command
314+
// only set exitCode back if we're passing tests
315+
if (t.passing())
316+
process.exitCode = exitCode
317+
})
318+
319+
process.exitCode = 0
320+
npm.command = 'install'
321+
reifyOutput(npm, {
322+
actualTree: { inventory: new Map(), children: [] },
323+
auditReport,
324+
diff: {
325+
children: [
326+
{ action: 'ADD', ideal: { location: 'loc' } },
327+
],
328+
},
329+
})
330+
331+
t.equal(process.exitCode, 0, 'did not set exit code')
332+
t.end()
333+
})
334+
t.end()
335+
})
336+
}
337+
215338
t.end()
216339
})
217340

0 commit comments

Comments
 (0)