fix: set audit exit code properly

lib/utils/reify-output.js

@@ -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,
@@ -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
     }
@@ -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
 
@@ -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 }) => {

test/lib/utils/reify-output.js

@@ -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()
 })