fix: actually skip scripts set to false

Author: dominykas

lib/index.js

@@ -134,6 +134,7 @@ exports.run = async (options) => {
 
             if (allowScripts[name] === false) {
                 console.warn(`==========> skip ${path} (because it is not allowed in package.json)`);
+                return false;
             }
 
             if (allowScripts[name] === true) {

package.json

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

test/fixtures/allowed-false.json

@@ -0,0 +1,14 @@
+{
+  "private": true,
+  "name": "@example/allowed-false",
+  "version": "0.0.0",
+  "dependencies": {
+    "@example/with-install-script": "*",
+    "@example/with-postinstall-script": "*",
+    "@example/without-scripts": "*"
+  },
+  "allowScripts": {
+    "@example/with-install-script": false,
+    "@example/with-postinstall-script": "*"
+  }
+}

test/fixtures/basic.json

@@ -12,7 +12,7 @@
   "allowScripts": {
     "@example/with-preinstall-script": "*",
     "@example/with-install-script": "*",
-    "@example/with-postinstall-script": "*"
+    "@example/with-postinstall-script": true
   },
   "scripts": {
     "preinstall": "echo preinstall from basic >> ${OUTPUT}",

test/fixtures/index.js

@@ -5,6 +5,7 @@ const Fs = require('fs');
 const Mkdirp = require('mkdirp');
 const Path = require('path');
 const Rimraf = require('rimraf');
+const Sinon = require('sinon');
 
 
 const internals = {
@@ -43,6 +44,16 @@ exports.setup = (main, deps) => {
         process.env.OUTPUT = originalOutput;
     });
 
+    const log = [];
+    const appendLog = (...items) => {
+
+        log.push(items.map((i) => i || '').join(' ').replace(new RegExp(cwd, 'g'), '.'));
+    };
+
+    Sinon.stub(console, 'info').callsFake(appendLog);
+    Sinon.stub(console, 'log').callsFake(appendLog);
+    Sinon.stub(console, 'warn').callsFake(appendLog);
+
     return {
         getExpectedResult: () => {
 
@@ -55,7 +66,7 @@ exports.setup = (main, deps) => {
 
         getLog: () => {
 
-            return console.log.args.map((args) => args[0] || '').join('\n').replace(new RegExp(cwd, 'g'), '.').trim();
+            return log.join('\n').trim();
         }
     };
 };
@@ -64,4 +75,5 @@ exports.restore = () => {
 
     internals.restore.forEach((restore) => restore());
     internals.restore = [];
+    Sinon.restore();
 };

test/index.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const Fixtures = require('./fixtures');
-const Sinon = require('sinon');
 
 
 const Allow = require('..');
@@ -17,15 +16,12 @@ describe('allow-scripts', () => {
         let cwd;
         beforeEach(() => {
 
-            Sinon.stub(console, 'log');
-            Sinon.stub(console, 'info');
             cwd = process.cwd();
         });
 
         afterEach(() => {
 
             Fixtures.restore();
-            Sinon.restore();
             process.chdir(cwd);
         });
 
@@ -56,9 +52,23 @@ describe('allow-scripts', () => {
 
             await expect(Allow.run({})).to.reject('No entry for @example/with-install-script');
 
-            expect(fixture.getActualResult()).not.to.contain('with-postinstall-script');
             expect(fixture.getActualResult()).to.equal('');
             expect(fixture.getLog()).to.equal('');
         });
+
+        it('skips scripts which are forbidden', async () => {
+
+            const fixture = Fixtures.setup('allowed-false', [
+                'with-install-script',
+                'with-postinstall-script',
+                'without-scripts'
+            ]);
+
+            await Allow.run({});
+
+            expect(fixture.getActualResult()).not.to.contain('with-install-script');
+            expect(fixture.getActualResult()).to.equal('postinstall from with-postinstall-script');
+            expect(fixture.getLog()).to.contain('skip node_modules/@example/with-install-script (because it is not allowed in package.json)');
+        });
     });
 });