diff --git a/README.md b/README.md index beb1d55..9a5b85c 100644 --- a/README.md +++ b/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 diff --git a/lib/index.js b/lib/index.js index 0ade286..fba9934 100644 --- a/lib/index.js +++ b/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 || {}; diff --git a/package.json b/package.json index 4035584..1ed8b37 100644 --- a/package.json +++ b/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" diff --git a/test/fixtures/deep.json b/test/fixtures/deep.json new file mode 100644 index 0000000..ea6bd91 --- /dev/null +++ b/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 + } +} diff --git a/test/fixtures/basic.incomplete.txt b/test/fixtures/deep.txt similarity index 54% rename from test/fixtures/basic.incomplete.txt rename to test/fixtures/deep.txt index 5a7d4cd..4bb0f41 100644 --- a/test/fixtures/basic.incomplete.txt +++ b/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 diff --git a/test/fixtures/index.js b/test/fixtures/index.js index de828d4..10b6f15 100644 --- a/test/fixtures/index.js +++ b/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'), '.')); }; diff --git a/test/index.js b/test/index.js index 35fd9db..fecdc36 100644 --- a/test/index.js +++ b/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,9 +149,9 @@ 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', @@ -143,18 +159,7 @@ describe('allow-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`.'); }); }); });