From 2053048a832d382a80e5e9c0f30fbb91b29f2fe7 Mon Sep 17 00:00:00 2001
From: Liliana Kastilio <lili@snyk.io>
Date: Wed, 22 May 2019 13:10:02 +0100
Subject: [PATCH 1/3] feat: Vulns to use exit code 1, errors 2

---
 src/cli/commands/test.ts | 42 ++++++++++++++-----
 src/cli/index.ts         | 88 ++++++++++++++++++++++++----------------
 2 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/src/cli/commands/test.ts b/src/cli/commands/test.ts
index a60c8e16c9..4b50773c78 100644
--- a/src/cli/commands/test.ts
+++ b/src/cli/commands/test.ts
@@ -103,6 +103,11 @@ async function test(...args): Promise<string> {
     }
   }
 
+  const vulnerableResults = results.filter((res) => res.vulnerabilities && res.vulnerabilities.length);
+  const errorResults = results.filter((res) => res instanceof Error);
+  const notSuccess = errorResults.length > 0;
+  const foundVulnerabilities = vulnerableResults.length > 0;
+
   // resultOptions is now an array of 1 or more options used for
   // the tests results is now an array of 1 or more test results
   // values depend on `options.json` value - string or object
@@ -121,22 +126,27 @@ async function test(...args): Promise<string> {
 
     // backwards compat - strip array IFF only one result
     const dataToSend = results.length === 1 ? results[0] : results;
-    const json = JSON.stringify(dataToSend, null, 2);
+    const stringifiedError = JSON.stringify(dataToSend, null, 2);
 
     if (results.every((res) => res.ok)) {
-      return json;
+      return stringifiedError;
+    }
+    const err = new Error(stringifiedError) as any;
+    if (foundVulnerabilities) {
+      err.code = 'VULNS';
+      const dataToSendNoVulns = dataToSend;
+      delete dataToSendNoVulns.vulnerabilities;
+      err.jsonNoVulns = dataToSendNoVulns;
     }
 
-    throw new Error(json);
+    err.json = stringifiedError;
+    throw err;
   }
 
   let response = results.map((unused, i) => displayResult(results[i], resultOptions[i]))
     .join(`\n${SEPARATOR}`);
 
-  const vulnerableResults = results.filter((res) => res.vulnerabilities && res.vulnerabilities.length);
-  const errorResults = results.filter((res) => res instanceof Error);
-
-  if (errorResults.length > 0) {
+  if (notSuccess) {
     debug(`Failed to test ${errorResults.length} projects, errors:`);
     errorResults.forEach((err) => {
       const errString = err.stack ? err.stack.toString() : err.toString();
@@ -153,8 +163,6 @@ async function test(...args): Promise<string> {
       summariseErrorResults(errorResults) + '\n';
   }
 
-  const notSuccess = vulnerableResults.length > 0 || errorResults.length > 0;
-
   if (notSuccess) {
     response += chalk.bold.red(summaryMessage);
     const error = new Error(response) as any;
@@ -162,8 +170,20 @@ async function test(...args): Promise<string> {
     // translation
     // HACK as there can be different errors, and we pass only the
     // first one
-    error.code = (vulnerableResults[0] || errorResults[0]).code;
-    error.userMessage = (vulnerableResults[0] || errorResults[0]).userMessage;
+    error.code = errorResults[0].code;
+    error.userMessage = errorResults[0].userMessage;
+    throw error;
+  }
+
+  if (foundVulnerabilities) {
+    response += chalk.bold.red(summaryMessage);
+    const error = new Error(response) as any;
+    // take the code of the first problem to go through error
+    // translation
+    // HACK as there can be different errors, and we pass only the
+    // first one
+    error.code = vulnerableResults[0].code || 'VULNS';
+    error.userMessage = vulnerableResults[0].userMessage;
     throw error;
   }
 
diff --git a/src/cli/index.ts b/src/cli/index.ts
index 2def129b22..27fd882f6b 100755
--- a/src/cli/index.ts
+++ b/src/cli/index.ts
@@ -1,5 +1,6 @@
 #!/usr/bin/env node
 import 'source-map-support/register';
+import * as Debug from 'debug';
 
 // assert supported node runtime version
 import * as runtime from './runtime';
@@ -16,9 +17,14 @@ import {isPathToPackageFile} from '../lib/detect';
 import {updateCheck} from '../lib/updater';
 import { MissingTargetFileError } from '../lib/errors/missing-targetfile-error';
 
+const debug = Debug('snyk');
+const EXIT_CODES = {
+  VULNS_FOUND: 1,
+  ERROR: 2,
+};
+
 async function runCommand(args) {
   const result = await args.method(...args.options._);
-
   const res = analytics({
     args: args.options._,
     command: args.command,
@@ -39,38 +45,22 @@ async function runCommand(args) {
 async function handleError(args, error) {
   spinner.clearAll();
   let command = 'bad-command';
+  let exitCode = EXIT_CODES.ERROR;
 
-  if (error.code === 'VULNS') {
+  const vulnsFound = (error.code === 'VULNS');
+  if (vulnsFound) {
     // this isn't a bad command, so we won't record it as such
     command = args.command;
-  } else if (!error.stack) { // log errors that are not error objects
-    analytics.add('error', JSON.stringify(error));
-    analytics.add('command', args.command);
-  } else {
-    // remove vulnerabilities from the errors
-    // to keep the logs small
-    if (error.stack && error.stack.vulnerabilities) {
-      delete error.vulnerabilities;
-    }
-    if (error.message && error.message.vulnerabilities) {
-      delete error.message.vulnerabilities;
-    }
-    analytics.add('error-message', error.message);
-    analytics.add('error', error.stack);
-    analytics.add('error-code', error.code);
-    analytics.add('command', args.command);
+    exitCode = EXIT_CODES.VULNS_FOUND;
   }
 
-  const res = analytics({
-    args: args.options._,
-    command,
-  });
-
-  if (args.options.debug) {
-    console.log(error.stack);
+  if (args.options.debug && !args.options.json) {
+    const output = vulnsFound ? error.message : error.stack;
+    console.log(output);
+  } else if (args.options.json) {
+    console.log(error.json || error.stack);
   } else {
     if (!args.options.quiet) {
-
       const result = errors.message(error);
       if (args.options.copy) {
         copy(result);
@@ -86,7 +76,32 @@ async function handleError(args, error) {
     }
   }
 
-  return res;
+  const analyticsError = vulnsFound ? {
+    stack: error.jsonNoVulns,
+    code: error.code,
+    message: 'Vulnerabilities found',
+   } : {
+    stack: error.stack,
+    code: error.code,
+    message: error.message,
+   };
+
+  if (!vulnsFound && !error.stack) { // log errors that are not error objects
+    analytics.add('error', JSON.stringify(analyticsError));
+    analytics.add('command', args.command);
+  } else {
+    analytics.add('error-message', analyticsError.message);
+    analytics.add('error', analyticsError.stack);
+    analytics.add('error-code', error.code);
+    analytics.add('command', args.command);
+  }
+
+  const res = analytics({
+    args: args.options._,
+    command,
+  });
+
+  return { res, exitCode };
 }
 
 function checkRuntime() {
@@ -94,7 +109,7 @@ function checkRuntime() {
     console.error(`${process.versions.node} is an unsupported nodejs ` +
       `runtime! Supported runtime range is '${runtime.supportedRange}'`);
     console.error('Please upgrade your nodejs runtime version and try again.');
-    process.exit(1);
+    process.exit(EXIT_CODES.ERROR);
   }
 }
 
@@ -113,9 +128,9 @@ async function main() {
   checkRuntime();
 
   const args = argsLib(process.argv);
-  let res = null;
+  let res;
   let failed = false;
-
+  let exitCode = EXIT_CODES.ERROR;
   try {
     if (args.options.file && args.options.file.match(/\.sln$/)) {
       sln.updateArgs(args);
@@ -124,7 +139,10 @@ async function main() {
     res = await runCommand(args);
   } catch (error) {
     failed = true;
-    res = await handleError(args, error);
+
+    const response = await handleError(args, error);
+    res = response.res;
+    exitCode = response.exitCode;
   }
 
   if (!args.options.json) {
@@ -132,15 +150,17 @@ async function main() {
   }
 
   if (!process.env.TAP && failed) {
-    process.exit(1);
+    debug('Exit code: ' + exitCode);
+    process.exitCode = exitCode;
   }
 
   return res;
 }
 
 const cli = main().catch((e) => {
-  console.log('super fail', e.stack);
-  process.exit(1);
+  console.error('Something unexpected went wrong: ', e.stack);
+  console.error('Exit code: ' + EXIT_CODES.ERROR);
+  process.exit(EXIT_CODES.ERROR);
 });
 
 if (module.parent) {

From 812cada8c3e94ab3b7a664dc569368f655e3e06e Mon Sep 17 00:00:00 2001
From: Liliana Kastilio <lili@snyk.io>
Date: Fri, 24 May 2019 15:04:00 +0100
Subject: [PATCH 2/3] chore: refactor analytics test

---
 test/analytics.test.js | 136 -------------------------------
 test/analytics.test.ts | 178 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 136 deletions(-)
 delete mode 100644 test/analytics.test.js
 create mode 100644 test/analytics.test.ts

diff --git a/test/analytics.test.js b/test/analytics.test.js
deleted file mode 100644
index 90819910a9..0000000000
--- a/test/analytics.test.js
+++ /dev/null
@@ -1,136 +0,0 @@
-var tap = require('tap');
-var test = require('tap').test;
-var proxyquire = require('proxyquire').noPreserveCache();
-var sinon = require('sinon');
-var snyk = require('../src/lib');
-var old;
-var iswindows = require('os-name')().toLowerCase().indexOf('windows') === 0;
-
-tap.beforeEach(function (done) {
-  old = snyk.config.get('disable-analytics');
-  snyk.config.delete('disable-analytics');
-  done();
-});
-
-tap.afterEach(function (done) {
-  if (old === undefined) {
-    snyk.config.delete('disable-analytics');
-  } else {
-    snyk.config.set('disable-analytics', old);
-  }
-  done();
-});
-
-test('analytics disabled', function (t) {
-  var spy = sinon.spy();
-  snyk.config.set('disable-analytics', '1');
-  var analytics = proxyquire('../src/lib/analytics', {
-    './request': spy,
-  });
-
-  return analytics().then(function () {
-    t.equal(spy.called, false, 'the request should not have been made');
-  });
-});
-
-test('analytics', function (t) {
-  var spy = sinon.spy();
-  var analytics = proxyquire('../src/lib/analytics', {
-    './request': spy,
-  });
-
-  analytics.add('foo', 'bar');
-
-  return analytics({
-    command: '__test__',
-    args: [],
-  }).then(function () {
-    var body = spy.lastCall.args[0].body.data;
-    t.deepEqual(Object.keys(body).sort(), ['command', 'os', 'version', 'id', 'ci', 'metadata', 'args', 'nodeVersion', 'durationMs'].sort(), 'keys as expected');
-  });
-});
-
-test('bad command', function (t) {
-  var spy = sinon.spy();
-  process.argv = ['node', 'script.js', 'random command', '-q'];
-  var cli = proxyquire('../src/cli', {
-    '../lib/analytics': proxyquire('../src/lib/analytics', {
-      './request': spy,
-    })
-  });
-
-  return cli.then(function () {
-    t.equal(spy.callCount, 1, 'analytics was called');
-
-    var payload = spy.args[0][0].body;
-    t.equal(payload.data.command, 'bad-command', 'correct event name');
-    t.equal(payload.data.metadata.command, 'random command', 'found original command');
-    t.equal(payload.data.metadata['error-message'],
-      'Unknown command "random command"', 'got correct error');
-  });
-});
-
-test('bad command with string error', function (t) {
-  var spy = sinon.spy();
-  process.argv = ['node', 'script.js', 'test', '-q'];
-  var cli = proxyquire('../src/cli', {
-    '../lib/analytics': proxyquire('../src/lib/analytics', {
-      './request': spy,
-    }),
-
-    './args': proxyquire('../src/cli/args', {
-      './commands': proxyquire('../src/cli/commands', {
-        '../../lib/hotload': proxyquire('../src/lib/hotload', {
-          // windows-based testing uses windows path separator
-          '..\\cli\\commands\\test': function() {
-            return Promise.reject('string error');
-          },
-          '../cli/commands/test': function()  {
-            return Promise.reject('string error');
-          }
-        })
-      })
-    })
-  });
-
-  return cli.then(function () {
-    t.equal(spy.callCount, 1, 'analytics was called');
-
-    var payload = spy.args[0][0].body;
-    t.equal(payload.data.command, 'bad-command', 'correct event name');
-    t.equal(payload.data.metadata.command, 'test', 'found original command');
-    t.equal(payload.data.metadata.error, '"string error"', 'got correct error');
-  });
-});
-
-test('test includes data', { skip: iswindows }, function (t) {
-  var spy = sinon.spy();
-  process.argv = ['node', 'script.js', 'test', 'snyk-demo-app', '-q'];
-
-  var analytics = proxyquire('../src/lib/analytics', {
-    './request': spy,
-  });
-
-  var cli = proxyquire('../src/cli', {
-    '../lib/analytics': analytics,
-    './args': proxyquire('../src/cli/args', {
-      './commands': proxyquire('../src/cli/commands', {
-        '../../lib/hotload': proxyquire('../src/lib/hotload', {
-          '../cli/commands/test': proxyquire('../src/lib/snyk-test', {
-            './run-test': proxyquire('../src/lib/snyk-test/run-test', {
-              '../analytics': analytics,
-            })
-          })
-        })
-      })
-    }),
-  });
-
-  return cli.then(function () {
-    t.equal(spy.callCount, 1, 'analytics was called');
-
-    var payload = spy.args[0][0].body;
-    t.equal(payload.data.command, 'test', 'correct event name');
-    t.equal(payload.data.metadata.package, 'snyk-demo-app@*', 'includes package');
-  });
-});
diff --git a/test/analytics.test.ts b/test/analytics.test.ts
new file mode 100644
index 0000000000..d43991853d
--- /dev/null
+++ b/test/analytics.test.ts
@@ -0,0 +1,178 @@
+import * as tap from 'tap';
+import * as Proxyquire from 'proxyquire';
+// tslint:disable-next-line
+const osName = require('os-name');
+import * as sinon from 'sinon';
+import * as snyk from '../src/lib';
+let old;
+const iswindows = osName().toLowerCase().indexOf('windows') === 0;
+const proxyquire = Proxyquire.noPreserveCache();
+const {test} = tap;
+
+tap.beforeEach((done) => {
+  old = snyk.config.get('disable-analytics');
+  snyk.config.delete('disable-analytics');
+  done();
+});
+
+tap.afterEach((done) => {
+  if (old === undefined) {
+    snyk.config.delete('disable-analytics');
+  } else {
+    snyk.config.set('disable-analytics', old);
+  }
+  done();
+});
+
+test('analytics disabled', (t) => {
+  const spy = sinon.spy();
+  snyk.config.set('disable-analytics', '1');
+  const analytics = proxyquire('../src/lib/analytics', {
+    './request': spy,
+  });
+
+  return analytics().then(() => {
+    t.equal(spy.called, false, 'the request should not have been made');
+  });
+});
+
+test('analytics', (t) => {
+  const spy = sinon.spy();
+  const analytics = proxyquire('../src/lib/analytics', {
+    './request': spy,
+  });
+
+  analytics.add('foo', 'bar');
+
+  return analytics({
+    command: '__test__',
+    args: [],
+  }).then(() => {
+    const body = spy.lastCall.args[0].body.data;
+    t.deepEqual(Object.keys(body).sort(),
+      ['command', 'os', 'version', 'id', 'ci', 'metadata', 'args', 'nodeVersion', 'durationMs'].sort(),
+      'keys as expected');
+  });
+});
+
+test('bad command', (t) => {
+  const spy = sinon.spy();
+  process.argv = ['node', 'script.js', 'random command', '-q'];
+  const cli = proxyquire('../src/cli', {
+    '../lib/analytics': proxyquire('../src/lib/analytics', {
+      './request': spy,
+    }),
+  });
+
+  return cli.then(() => {
+    t.equal(spy.callCount, 1, 'analytics was called');
+
+    const payload = spy.args[0][0].body;
+    t.equal(payload.data.command, 'bad-command', 'correct event name');
+    t.equal(payload.data.metadata.command, 'random command', 'found original command');
+    t.equal(payload.data.metadata['error-message'],
+      'Unknown command "random command"', 'got correct error');
+  });
+});
+
+test('bad command with string error', (t) => {
+  const spy = sinon.spy();
+  process.argv = ['node', 'script.js', 'test', '-q'];
+  const error = new Error('Some error') as any;
+  error.code = 'CODE';
+  const cli = proxyquire('../src/cli', {
+    '../lib/analytics': proxyquire('../src/lib/analytics', {
+      './request': spy,
+    }),
+
+    './args': proxyquire('../src/cli/args', {
+      './commands': proxyquire('../src/cli/commands', {
+        '../../lib/hotload': proxyquire('../src/lib/hotload', {
+          // windows-based testing uses windows path separator
+          '..\\cli\\commands\\test'() {
+            return Promise.reject(error);
+          },
+          '../cli/commands/test'()  {
+            return Promise.reject(error);
+          },
+        }),
+      }),
+    }),
+  });
+
+  return cli.then(() => {
+    t.equal(spy.callCount, 1, 'analytics was called');
+
+    const payload = spy.args[0][0].body;
+    t.equal(payload.data.command, 'bad-command', 'correct event name');
+    t.equal(payload.data.metadata.command, 'test', 'found original command');
+    t.match(payload.data.metadata.error, 'Some error', 'got correct error');
+  });
+});
+
+test('vulns found (thrown as an error)', (t) => {
+  const spy = sinon.spy();
+  process.argv = ['node', 'script.js', 'test', '-q'];
+  const error = new Error('7 vulnerable dependency paths') as any;
+  error.code = 'VULNS';
+  const cli = proxyquire('../src/cli', {
+    '../lib/analytics': proxyquire('../src/lib/analytics', {
+      './request': spy,
+    }),
+
+    './args': proxyquire('../src/cli/args', {
+      './commands': proxyquire('../src/cli/commands', {
+        '../../lib/hotload': proxyquire('../src/lib/hotload', {
+          // windows-based testing uses windows path separator
+          '..\\cli\\commands\\test'() {
+            return Promise.reject(error);
+          },
+          '../cli/commands/test'()  {
+            return Promise.reject(error);
+          },
+        }),
+      }),
+    }),
+  });
+
+  return cli.then(() => {
+    t.equal(spy.callCount, 1, 'analytics was called');
+
+    const payload = spy.args[0][0].body;
+    t.equal(payload.data.command, 'test', 'correct event name');
+    t.equal(payload.data.metadata.command, 'test', 'found original command');
+    t.equal(payload.data.metadata['error-message'], 'Vulnerabilities found', 'got correct vuln count');
+  });
+});
+
+test('test includes data', { skip: iswindows }, (t) => {
+  const spy = sinon.spy();
+  process.argv = ['node', 'script.js', 'test', 'snyk-demo-app', '-q'];
+
+  const analytics = proxyquire('../src/lib/analytics', {
+    './request': spy,
+  });
+
+  const cli = proxyquire('../src/cli', {
+    '../lib/analytics': analytics,
+    './args': proxyquire('../src/cli/args', {
+      './commands': proxyquire('../src/cli/commands', {
+        '../../lib/hotload': proxyquire('../src/lib/hotload', {
+          '../cli/commands/test': proxyquire('../src/lib/snyk-test', {
+            './run-test': proxyquire('../src/lib/snyk-test/run-test', {
+              '../analytics': analytics,
+            }),
+          }),
+        }),
+      }),
+    }),
+  });
+
+  return cli.then(() => {
+    t.equal(spy.callCount, 1, 'analytics was called');
+
+    const payload = spy.args[0][0].body;
+    t.equal(payload.data.command, 'test', 'correct event name');
+    t.equal(payload.data.metadata.package, 'snyk-demo-app@*', 'includes package');
+  });
+});

From 636628f2afa77c26e1d11cc95146b166295d9cea Mon Sep 17 00:00:00 2001
From: Liliana Kastilio <lili@snyk.io>
Date: Fri, 24 May 2019 21:57:58 +0100
Subject: [PATCH 3/3] chore: refactor analytics to use const

---
 src/lib/analytics.js | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/lib/analytics.js b/src/lib/analytics.js
index c203ff9d71..64cb34b58d 100644
--- a/src/lib/analytics.js
+++ b/src/lib/analytics.js
@@ -1,20 +1,20 @@
 module.exports = analytics;
 module.exports.single = postAnalytics;
 
-var snyk = require('../lib');
-var config = require('./config');
-var version = require('./version');
-var request = require('./request');
-var isCI = require('./is-ci').isCI;
-var debug = require('debug')('snyk');
-var os = require('os');
-var osName = require('os-name');
-var crypto = require('crypto');
-var uuid = require('uuid');
+const snyk = require('../lib');
+const config = require('./config');
+const version = require('./version');
+const request = require('./request');
+const isCI = require('./is-ci').isCI;
+const debug = require('debug')('snyk');
+const os = require('os');
+const osName = require('os-name');
+const crypto = require('crypto');
+const uuid = require('uuid');
 
-var metadata = {};
+const metadata = {};
 // analytics module is required at the beginning of the CLI run cycle
-var startTime = Date.now();
+const startTime = Date.now();
 
 function analytics(data) {
   if (!data) {
@@ -48,11 +48,11 @@ function postAnalytics(data) {
     data.os = osName(os.platform(), os.release());
     data.nodeVersion = process.version;
 
-    var seed = uuid.v4();
-    var shasum = crypto.createHash('sha1');
+    const seed = uuid.v4();
+    const shasum = crypto.createHash('sha1');
     data.id = shasum.update(seed).digest('hex');
 
-    var headers = {};
+    const headers = {};
     if (snyk.api) {
       headers.authorization = 'token ' + snyk.api;
     }
@@ -76,12 +76,8 @@ function postAnalytics(data) {
   });
 }
 
-analytics.reset = function () {
-  metadata = {};
-};
-
 analytics.add = function (key, value) {
-  debug('analytics add', key, value);
+  debug('analytics adding to metadata: ', key, value);
   if (metadata[key]) {
     if (!Array.isArray(metadata[key])) {
       metadata[key] = [metadata[key]];