fix: rely on npm shrinkwrap to read the installed contents

`npm ls` does not have all the necessary structures.

Author: dominykas

README.md

@@ -17,7 +17,7 @@ Only the explicitly allowed `[pre|post]install` scripts will be executed.
 $ npx allow-scripts [--dry-run]
 ```
 
-Running the command will scan the list of installed dependencies using `npm ls --json`. It will then execute the scripts for allowed dependencies that have them in the following order:
+Running the command will scan the list of installed dependencies (using an existing `package-lock.json` or `npm-shrinkwrap.json` or by creating one on the fly). It will then execute the scripts for allowed dependencies that have them in the following order:
 
 - `preinstall` in the main package
 - `preinstall` in dependencies

lib/index.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const Cp = require('child_process');
+const Fs = require('fs');
 const Npm = require('libnpm');
 const Path = require('path');
 const Semver = require('semver');
@@ -73,22 +74,23 @@ internals.runScript = (stage, { pkg, path, cwd, unsafePerm }, options) => {
     });
 };
 
-internals.getInstalledContents = (cwd) => {
+internals.getLockFile = (cwd) => {
 
-    let output;
-    try {
-        output = Cp.execSync('npm ls --json', { cwd });
-    }
-    catch (err) {
-        output = err.output[1]; // npm will exit with an error when e.g. there's peer deps missing - attempt to ignore that
+    if (Fs.existsSync(Path.join(cwd, 'npm-shrinkwrap.json'))) {
+        return require(Path.join(cwd, 'npm-shrinkwrap.json'));
     }
 
-    try {
-        return JSON.parse(output.toString());
-    }
-    catch (err) {
-        throw new Error(`Failed to read the contents of node_modules. \`npm ls --json\` returned: ${output}`);
+    if (Fs.existsSync(Path.join(cwd, 'package-lock.json'))) {
+        return require(Path.join(cwd, 'package-lock.json'));
     }
+
+    Cp.execSync('npm shrinkwrap');
+
+    const lockFilePath = Path.join(cwd, 'npm-shrinkwrap.json');
+    const lockFileContents = require(lockFilePath);
+
+    Fs.unlinkSync(lockFilePath);
+    return lockFileContents;
 };
 
 exports.run = async (options) => {
@@ -98,7 +100,13 @@ exports.run = async (options) => {
 
     pkg._id = `${pkg.name}@${pkg.version}`; // @todo: find an official way to do this for top level package
 
-    const tree = Npm.logicalTree(pkg, internals.getInstalledContents(cwd));
+    try {
+        var tree = Npm.logicalTree(pkg, internals.getLockFile(cwd));
+    }
+    catch (err) {
+        throw new Error('Failed to read the installed tree - you might want to `rm -rf node_modules && npm i --ignore-scripts`.');
+    }
+
     const queue = internals.queue(tree);
 
     const allowScripts = pkg.allowScripts || {};

package.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/dominykas/allow-scripts.git"
   },
   "scripts": {
-    "test": "lab -L -t 96 -m 5000"
+    "test": "lab -L -t 93 -m 5000"
   },
   "bin": {
     "allow-scripts": "./bin/allow-scripts.js"

test/fixtures/deep.json

@@ -0,0 +1,14 @@
+{
+  "private": true,
+  "name": "@example/deep",
+  "version": "0.0.0",
+  "dependencies": {
+    "@example/basic": "*"
+  },
+  "allowScripts": {
+    "@example/basic": "*",
+    "@example/with-preinstall-script": "*",
+    "@example/with-install-script": "*",
+    "@example/with-postinstall-script": true
+  }
+}

test/fixtures/basic.incomplete.txt renamed to test/fixtures/deep.txt

@@ -1,6 +1,6 @@
+preinstall from with-preinstall-script
 preinstall from basic
 install from with-install-script
 install from basic
+postinstall from with-postinstall-script
 postinstall from basic
-prepublish from basic
-prepare from basic

test/fixtures/index.js

@@ -22,14 +22,14 @@ exports.setup = (main, deps) => {
     Mkdirp.sync(cwd);
     Fs.copyFileSync(Path.join(__dirname, `${main}.json`), Path.join(cwd, 'package.json'));
     Fs.writeFileSync(Path.join(cwd, 'res.txt'), '');
-    delete require.cache[Path.join(cwd, 'package.json')];
 
     deps.forEach((dep) => {
 
         const pkg = require(`./${dep}.json`);
 
         Mkdirp.sync(Path.join(cwd, 'node_modules', pkg.name));
-        Fs.writeFileSync(Path.join(cwd, 'node_modules', pkg.name, 'package.json'), JSON.stringify(Object.assign({}, pkg, {
+        const pkgJsonPath = Path.join(cwd, 'node_modules', pkg.name, 'package.json');
+        Fs.writeFileSync(pkgJsonPath, JSON.stringify(Object.assign({}, pkg, {
             _id: `${pkg.name}@${pkg.version}`
         })));
     });
@@ -42,11 +42,22 @@ exports.setup = (main, deps) => {
     internals.restore.push(() => {
 
         process.env.OUTPUT = originalOutput;
+        Object.keys(require.cache).forEach((k) => {
+
+            if (k.startsWith(cwd)) {
+                delete require.cache[k];
+            }
+        });
     });
 
     const log = [];
     const appendLog = (...items) => {
 
+        // @todo: should suppress this in production code
+        if (items[0] === 'npm notice created a lockfile as npm-shrinkwrap.json. You should commit this file.\n') {
+            return;
+        }
+
         log.push(items.map((i) => i || '').join(' ').replace(new RegExp(cwd, 'g'), '.'));
     };
 

test/index.js

@@ -1,10 +1,8 @@
 'use strict';
 
-const Cp = require('child_process');
 const Fs = require('fs');
 const Fixtures = require('./fixtures');
 const Path = require('path');
-const Sinon = require('sinon');
 
 const Allow = require('..');
 
@@ -61,6 +59,24 @@ describe('allow-scripts', () => {
             expect(fixture.getLog()).to.equal(Fs.readFileSync(Path.join(__dirname, 'fixtures', 'basic.dry-run.txt')).toString().trim());
         });
 
+        it('executes allowed scripts (subdeps)', async () => {
+
+            const fixture = Fixtures.setup('deep', [
+                'basic',
+                'with-preinstall-script',
+                'with-install-script',
+                'with-postinstall-script',
+                'without-scripts',
+                'without-install-scripts'
+            ]);
+
+            await Allow.run({});
+
+            expect(fixture.getActualResult()).to.equal(Fs.readFileSync(Path.join(__dirname, 'fixtures', 'deep.txt')).toString().trim());
+            expect(fixture.getLog()).not.to.contain('without-scripts');
+            expect(fixture.getLog()).not.to.contain('without-install-scripts');
+        });
+
         it('crashes on script not in allowed list', async () => {
 
             const fixture = Fixtures.setup('not-in-allowed', [
@@ -133,28 +149,17 @@ describe('allow-scripts', () => {
             expect(fixture.getLog()).to.equal('');
         });
 
-        it('deals with incomplete installed tree', async () => {
+        it('crashes on incomplete installed tree', async () => {
 
-            const fixture = Fixtures.setup('basic', [
+            Fixtures.setup('basic', [
                 // 'with-preinstall-script',
                 'with-install-script',
                 // 'with-postinstall-script',
                 'without-scripts',
                 'without-install-scripts'
             ]);
 
-            await Allow.run({});
-
-            expect(fixture.getActualResult()).to.equal(Fs.readFileSync(Path.join(__dirname, 'fixtures', 'basic.incomplete.txt')).toString().trim());
-        });
-
-        it('deals with unparseable tree', async () => {
-
-            Sinon.stub(Cp, 'execSync').returns('not-json');
-
-            Fixtures.setup('basic', []);
-
-            await expect(Allow.run({})).to.reject('Failed to read the contents of node_modules. `npm ls --json` returned: not-json');
+            await expect(Allow.run({})).to.reject('Failed to read the installed tree - you might want to `rm -rf node_modules && npm i --ignore-scripts`.');
         });
     });
 });