fix: fix for assumed patch paths

src/cli/commands/protect/wizard.js

@@ -13,7 +13,6 @@ const chalk = require('chalk');
 const url = require('url');
 const _ = require('lodash');
 const exec = require('child_process').exec;
-const undefsafe = require('undefsafe');
 const auth = require('../auth');
 const getVersion = require('../version');
 const allPrompts = require('./prompts');
@@ -276,7 +275,6 @@ function processAnswers(answers, policy, options) {
   var packageManager = detect.detectPackageManager(cwd, options);
   var targetFile = options.file || detect.detectPackageFile(cwd);
   const isLockFileBased = targetFile.endsWith('package-lock.json') || targetFile.endsWith('yarn.lock');
-
   // TODO: fix this by providing better patch support for yarn
   // yarn hoists packages up a tree so we can't assume their location
   // on disk without traversing node_modules
@@ -423,19 +421,25 @@ function processAnswers(answers, policy, options) {
       pkg.snyk = true;
     })
     .then(function () {
-      var lbl = 'Updating package.json...';
-      if (answers['misc-add-test'] || answers['misc-add-protect']) {
+      let lbl = 'Updating package.json...';
+      const addSnykToDependencies = answers['misc-add-test'] || answers['misc-add-protect'];
+      let updateSnykFunc = () => protect.install(packageManager, ['snyk'], live);
+
+      if (addSnykToDependencies) {
         debug('updating %s', packageFile);
 
-        if (undefsafe(pkg, 'dependencies.snyk') ||
-          undefsafe(pkg, 'peerDependencies.snyk') ||
-          undefsafe(pkg, 'optionalDependencies.snyk')) {
+        if (_.get(pkg, 'dependencies.snyk') ||
+          _.get(pkg, 'peerDependencies.snyk') ||
+          _.get(pkg, 'optionalDependencies.snyk')) {
         // nothing to do as the user already has Snyk
         // TODO decide whether we should update the version being used
         // and how do we reconcile if the global install is older
         // than the local version?
         } else {
-          if (answers['misc-add-protect']) {
+          const addSnykToProdDeps = answers['misc-add-protect'];
+          const snykIsInDevDeps = _.get(pkg, 'devDependencies.snyk');
+
+          if (addSnykToProdDeps) {
             if (!pkg.dependencies) {
               pkg.dependencies = {};
             }
@@ -444,20 +448,22 @@ function processAnswers(answers, policy, options) {
                 '(used by snyk protect)';
 
             // but also check if we should remove it from devDependencies
-            if (undefsafe(pkg, 'devDependencies.snyk')) {
+            if (snykIsInDevDeps) {
               delete pkg.devDependencies.snyk;
             }
-          } else if (!undefsafe(pkg, 'devDependencies.snyk')) {
+          } else if (!snykIsInDevDeps) {
             if (!pkg.devDependencies) {
               pkg.devDependencies = {};
             }
             lbl = 'Adding Snyk to devDependencies (used by npm test)';
             pkg.devDependencies.snyk = snykVersion;
+            updateSnykFunc = () => protect.installDev(packageManager, ['snyk'], live);
+
           }
         }
       }
 
-      if (answers['misc-add-test'] || answers['misc-add-protect'] ||
+      if (addSnykToDependencies ||
           tasks.update.length) {
         var packageString = options.packageLeading + JSON.stringify(pkg, '', 2) +
                           options.packageTrailing;
@@ -467,7 +473,7 @@ function processAnswers(answers, policy, options) {
             if (isLockFileBased) {
               // we need to trigger a lockfile update after adding snyk
               // as a dep
-              return protect.update(['snyk'], live, packageManager);
+              return updateSnykFunc();
             }
           })
           // clear spinner in case of success or failure

src/lib/protect/get-vuln-source.js

@@ -6,13 +6,13 @@ var path = require('path');
 var statSync = require('fs').statSync;
 var moduleToObject = require('snyk-module');
 
-function getVulnSource(vuln, cwd, live) {
+function getVulnSource(vuln, live) {
   var from = vuln.from.slice(1).map(function (pkg) {
     return moduleToObject(pkg).name;
   });
 
   var viaPath = path.resolve(
-    cwd || process.cwd(),
+    process.cwd(),
     'node_modules',
     from.join('/node_modules/')
   );

src/lib/protect/index.js

@@ -1,6 +1,8 @@
 var protect = module.exports = {
   ignore: require('./ignore'),
   update: require('./update').update,
+  install: require('./update').install,
+  installDev: require('./update').installDev,
   patch: require('./patch'),
   patchesForPackage: require('./patches-for-package'),
   generatePolicy: generatePolicy,

src/lib/protect/patch.js

@@ -20,16 +20,13 @@ var analytics = require('../analytics');
 var getPatchFile = require('./fetch-patch');
 var ensurePatchUtilExists = require('./ensure-patch');
 
-// note: cwd is optional and mostly used for testing
-function patch(vulns, live, cwd) {
+function patch(vulns, live) {
   var lbl = 'Applying patches...';
   var errorList = [];
 
   return ensurePatchUtilExists().then(spinner(lbl)).then(function () {
     // the target directory where our module name will live
-    vulns.forEach(function (vuln) {
-      vuln.source = getVulnSource(vuln, cwd, live);
-    });
+    vulns.forEach((vuln) => vuln.source = getVulnSource(vuln, live));
 
     var deduped = dedupe(vulns);
     debug('patching %s vulns after dedupe', deduped.packages.length);
@@ -47,6 +44,7 @@ function patch(vulns, live, cwd) {
         }
 
         analytics.add('patch', vuln.from.slice(1).join(' > '));
+        debug(`Patching vuln: ${vuln.id} ${vuln.from}`);
 
         // the colon doesn't like Windows, ref: https://git.io/vw2iO
         var fileSafeId = vuln.id.replace(/:/g, '-');

src/lib/protect/update.js

@@ -1,5 +1,6 @@
 module.exports.update = update;
 module.exports.install = install;
+module.exports.installDev = installDev;
 
 var debug = require('debug')('snyk');
 var chalk = require('chalk');

test/deduped-package-patch.test.js

@@ -2,8 +2,9 @@ var test = require('tap').test;
 var protect = require('../src/lib/protect');
 var answers = require('./fixtures/deduped-dep/answers.json');
 
-test('npm deduped packages are found and patched correctly', function (t) {
-  protect.patch(answers, false, __dirname + '/fixtures/deduped-dep/').then(function (res) {
-    t.equal(Object.keys(res.patch).length, 1, 'found and patched 1 file');
-  }).catch(t.threw).then(t.end);
-});
+test('npm deduped packages are found and patched correctly', async (t) => {
+  process.chdir(__dirname + '/fixtures/deduped-dep/');
+  const res = await protect.patch(answers, false);
+  t.equal(Object.keys(res.patch).length, 1, 'found and patched 1 file');
+  process.chdir(__dirname);
+});

test/wizard-package-changes.test.js

@@ -1,10 +1,19 @@
-var tap = require('tap');
-var test = require('tap').test;
-var path = require('path');
-var sinon = require('sinon');
-var proxyquire = require('proxyquire');
-var policySpy = sinon.spy();
-var writeSpy = sinon.spy();
+const tap = require('tap');
+const test = require('tap').test;
+const sinon = require('sinon').createSandbox();
+const proxyquire = require('proxyquire');
+const policySpy = sinon.spy();
+const writeSpy = sinon.spy();
+const detect = require('../src/lib/detect');
+
+sinon.stub(detect, 'detectPackageManager')
+  .returns('npm');
+sinon.stub(detect, 'detectPackageFile')
+  .returns('package.json');
+
+tap.tearDown(() => {
+  sinon.restore();
+});
 
 var mockPackage;
 
@@ -18,8 +27,19 @@ var wizard = proxyquire('../src/cli/commands/protect/wizard', {
     },
     readFile: function (filename) {
       return Promise.resolve(JSON.stringify(mockPackage));
-    }
-  }
+    },
+    '../../../lib/npm': {
+      getVersion: function() {
+        return new Promise(function(resolve) {
+          return resolve('5.0.1');
+        });
+      },
+    },
+    '../../../lib/protect': {
+      install: () => new Promise((resolve) => resolve()),
+      installDev: () => new Promise((resolve) => resolve()),
+    },
+  },
 });
 
 var save = p => {
@@ -29,17 +49,16 @@ var save = p => {
 
 var policy = proxyquire('snyk-policy', { save: save });
 var mockPolicy;
-var noop = () => {};
 
 tap.beforeEach(done => {
   // reset the mock package
   mockPackage = {
     name: 'snyk-test',
     dependencies: {
-      foo: '1.1.1'
+      foo: '1.1.1',
     },
     devDependencies: {
-      bar: '2.2.2'
+      bar: '2.2.2',
     },
   };
 
@@ -51,75 +70,85 @@ tap.beforeEach(done => {
   }).then(done);
 });
 
-test('user deps are left alone if they do not test or protect', t => {
-  return wizard.processAnswers({
-    'misc-test-no-monitor': true
-  }, mockPolicy).then(res => {
-    t.equal(writeSpy.called, false, 'the package is not touched');
-  });
+test('user deps are left alone if they do not test or protect', async (t) => {
+  process.chdir(__dirname, '/fixtures/protect');
+
+  await wizard.processAnswers({
+    'misc-test-no-monitor': true,
+  }, mockPolicy);
+
+  t.equal(writeSpy.called, false, 'the package is not touched');
 });
 
-test('snyk adds to devDeps when test only is selected', t => {
-  return wizard.processAnswers({
+test('snyk adds to devDeps when test only is selected', async (t)  => {
+  process.chdir(__dirname, '/fixtures/protect');
+  await wizard.processAnswers({
     'misc-add-test': true,
-    'misc-test-no-monitor': true
-  }, mockPolicy).then(res => {
-    var pkg = writeSpy.args[0][0];
-    t.equal(writeSpy.called, true, 'the package was updated');
-    t.equal(pkg.scripts.test.includes('snyk test'), true, 'snyk test found in npm test');
-    t.equal(pkg.dependencies.snyk, undefined, 'snyk not in production deps');
-    t.notEqual(pkg.devDependencies.snyk, undefined, 'snyk IS in dev deps');
-  });
+    'misc-test-no-monitor': true,
+  }, mockPolicy);
+
+  var pkg = writeSpy.args[0][0];
+  t.equal(writeSpy.called, true, 'the package was updated');
+  t.equal(pkg.scripts.test.includes('snyk test'), true, 'snyk test found in npm test');
+  t.equal(pkg.dependencies.snyk, undefined, 'snyk not in production deps');
+  t.notEqual(pkg.devDependencies.snyk, undefined, 'snyk IS in dev deps');
+  process.chdir(__dirname);
 });
 
-test('snyk adds to prod deps when protect only is selected', t => {
-  return wizard.processAnswers({
+test('snyk adds to prod deps when protect only is selected', async (t) => {
+  process.chdir(__dirname, '/fixtures/protect');
+  await wizard.processAnswers({
     'misc-add-protect': true,
-    'misc-test-no-monitor': true
-  }, mockPolicy).then(res => {
-    var pkg = writeSpy.args[0][0];
-
-    t.equal(writeSpy.called, true, 'the package was updated');
-    t.equal(pkg.scripts.test, undefined, 'snyk test not added');
-    t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
-    t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
-    t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
-  });
+    'misc-test-no-monitor': true,
+  }, mockPolicy);
+
+  var pkg = writeSpy.args[0][0];
+
+  t.equal(writeSpy.called, true, 'the package was updated');
+  t.equal(pkg.scripts.test, undefined, 'snyk test not added');
+  t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
+  t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
+  t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
+  process.chdir(__dirname);
 });
 
-test('snyk adds to prod deps when both protect AND test are selected', t => {
-  return wizard.processAnswers({
+test('snyk adds to prod deps when both protect AND test are selected',async (t) => {
+  process.chdir(__dirname, '/fixtures/debug-package');
+  await wizard.processAnswers({
     'misc-add-protect': true,
     'misc-add-test': true,
-    'misc-test-no-monitor': true
-  }, mockPolicy).then(res => {
-    var pkg = writeSpy.args[0][0];
-
-    t.equal(writeSpy.called, true, 'the package was updated');
-    t.equal(pkg.scripts.test.includes('snyk test'), true, 'snyk test is added');
-    t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
-    t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
-    t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
-  });
+    'misc-test-no-monitor': true,
+  }, mockPolicy);
+  var pkg = writeSpy.args[0][0];
+
+  t.equal(writeSpy.called, true, 'the package was updated');
+  t.equal(pkg.scripts.test.includes('snyk test'), true, 'snyk test is added');
+  t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
+  t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
+  t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
+  process.chdir(__dirname);
 });
 
-test('upgrades snyk from devDeps to prod deps if protect is used', t => {
+test('upgrades snyk from devDeps to prod deps if protect is used', async (t) => {
+  process.chdir(__dirname, '/fixtures/debug-package');
+
   mockPackage = {
     name: 'snyk-test',
     devDependencies: {
-      snyk: '*'
+      snyk: '*',
     },
   };
 
   return wizard.processAnswers({
     'misc-add-protect': true,
-    'misc-test-no-monitor': true
-  }, mockPolicy).then(res => {
-    var pkg = writeSpy.args[0][0];
-
-    t.equal(writeSpy.called, true, 'the package was updated');
-    t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
-    t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
-    t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
-  });
+    'misc-test-no-monitor': true,
+  }, mockPolicy);
+
+  var pkg = writeSpy.args[0][0];
+
+  t.equal(writeSpy.called, true, 'the package was updated');
+  t.equal(pkg.scripts['snyk-protect'].includes('snyk protect'), true, 'snyk protect added');
+  t.notEqual(pkg.dependencies.snyk, undefined, 'snyk is in production deps');
+  t.equal(pkg.devDependencies.snyk, undefined, 'snyk is not in dev deps');
+  process.chdir(__dirname);
 });

test/wizard-prepare.test.js

@@ -19,6 +19,10 @@ var wizard = proxyquire('../src/cli/commands/protect/wizard', {
       });
     },
   },
+  '../../../lib/protect': {
+    install: () => new Promise((resolve) => resolve()),
+    installDev: () => new Promise((resolve) => resolve()),
+  },
   'then-fs': {
     readFile: function () {
       return Promise.resolve(JSON.stringify(fixture));