From c336cd83727d8365ad4ed20a5479b02a368f9845 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Fri, 19 May 2023 17:10:04 -0700 Subject: [PATCH] fix: refactor engines validation to lint syntax --- .eslintrc.local.js | 37 +++++++++++ .eslintrc.local.json | 5 -- lib/cli-entry.js | 74 ++++++++++++++++++++++ lib/cli.js | 104 +------------------------------ lib/es6/validate-engines.js | 49 +++++++++++++++ test/lib/cli.js | 3 +- test/lib/es6/validate-engines.js | 34 ++++++++++ 7 files changed, 199 insertions(+), 107 deletions(-) create mode 100644 .eslintrc.local.js delete mode 100644 .eslintrc.local.json create mode 100644 lib/cli-entry.js create mode 100644 lib/es6/validate-engines.js create mode 100644 test/lib/es6/validate-engines.js diff --git a/.eslintrc.local.js b/.eslintrc.local.js new file mode 100644 index 0000000000000..6d94f64614fb8 --- /dev/null +++ b/.eslintrc.local.js @@ -0,0 +1,37 @@ +const { resolve, relative } = require('path') + +// Create an override to lockdown a file to es6 syntax only +// and only allow it to require an allowlist of files +const rel = (p) => relative(__dirname, resolve(__dirname, p)) +const braces = (a) => a.length > 1 ? `{${a.map(rel).join(',')}}` : a[0] + +const es6Files = (e) => Object.entries(e).map(([file, allow]) => ({ + files: `./${rel(file)}`, + parserOptions: { + ecmaVersion: 6, + }, + rules: Array.isArray(allow) ? { + 'node/no-restricted-require': ['error', [{ + name: ['/**', `!${__dirname}/${braces(allow)}`], + message: `This file can only require: ${allow.join(',')}`, + }]], + } : {}, +})) + +module.exports = { + rules: { + 'no-console': 'error', + }, + overrides: es6Files({ + 'index.js': ['lib/cli.js'], + 'bin/npm-cli.js': ['lib/cli.js'], + 'lib/cli.js': ['lib/es6/validate-engines.js'], + 'lib/es6/validate-engines.js': ['package.json'], + // TODO: This file should also have its requires restricted as well since it + // is an entry point but it currently pulls in config definitions which have + // a large require graph, so that is not currently feasible. A future config + // refactor should keep that in mind and see if only config definitions can + // be exported in a way that is compatible with ES6. + 'bin/npx-cli.js': null, + }), +} diff --git a/.eslintrc.local.json b/.eslintrc.local.json deleted file mode 100644 index 2ff50f91ec326..0000000000000 --- a/.eslintrc.local.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "no-console": "error" - } -} diff --git a/lib/cli-entry.js b/lib/cli-entry.js new file mode 100644 index 0000000000000..42788416a13f2 --- /dev/null +++ b/lib/cli-entry.js @@ -0,0 +1,74 @@ +/* eslint-disable max-len */ + +// Separated out for easier unit testing +module.exports = async (process, validateEngines) => { + // set it here so that regardless of what happens later, we don't + // leak any private CLI configs to other programs + process.title = 'npm' + + // if npm is called as "npmg" or "npm_g", then run in global mode. + if (process.argv[1][process.argv[1].length - 1] === 'g') { + process.argv.splice(1, 1, 'npm', '-g') + } + + const satisfies = require('semver/functions/satisfies') + const exitHandler = require('./utils/exit-handler.js') + const Npm = require('./npm.js') + const npm = new Npm() + exitHandler.setNpm(npm) + + // only log node and npm paths in argv initially since argv can contain sensitive info. a cleaned version will be logged later + const log = require('./utils/log-shim.js') + log.verbose('cli', process.argv.slice(0, 2).join(' ')) + log.info('using', 'npm@%s', npm.version) + log.info('using', 'node@%s', process.version) + + // At this point we've required a few files and can be pretty sure we dont contain invalid syntax for this version of node. It's possible a lazy require would, but that's unlikely enough that it's not worth catching anymore and we attach the more important exit handlers. + validateEngines.off() + process.on('uncaughtException', exitHandler) + process.on('unhandledRejection', exitHandler) + + // It is now safe to log a warning if they are using a version of node that is not going to fail on syntax errors but is still unsupported and untested and might not work reliably. This is safe to use the logger now which we want since this will show up in the error log too. + if (!satisfies(validateEngines.node, validateEngines.engines)) { + log.warn('cli', validateEngines.unsupportedMessage) + } + + let cmd + // Now actually fire up npm and run the command. + // This is how to use npm programmatically: + try { + await npm.load() + + // npm -v + if (npm.config.get('version', 'cli')) { + npm.output(npm.version) + return exitHandler() + } + + // npm --versions + if (npm.config.get('versions', 'cli')) { + npm.argv = ['version'] + npm.config.set('usage', false, 'cli') + } + + cmd = npm.argv.shift() + if (!cmd) { + npm.output(await npm.usage) + process.exitCode = 1 + return exitHandler() + } + + await npm.exec(cmd) + return exitHandler() + } catch (err) { + if (err.code === 'EUNKNOWNCOMMAND') { + const didYouMean = require('./utils/did-you-mean.js') + const suggestions = await didYouMean(npm, npm.localPrefix, cmd) + npm.output(`Unknown command: "${cmd}"${suggestions}\n`) + npm.output('To see a list of supported npm commands, run:\n npm help') + process.exitCode = 1 + return exitHandler() + } + return exitHandler(err) + } +} diff --git a/lib/cli.js b/lib/cli.js index a393626f08291..c85ecb65a7005 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -1,102 +1,4 @@ -/* eslint-disable max-len */ -// Code in this file should work in all conceivably runnable versions of node. -// A best effort is made to catch syntax errors to give users a good error message if they are using a node version that doesn't allow syntax we are using in other files such as private properties, etc +const validateEngines = require('./es6/validate-engines.js') +const cliEntry = require('path').resolve(__dirname, 'cli-entry.js') -// Separated out for easier unit testing -module.exports = async process => { - // set it here so that regardless of what happens later, we don't - // leak any private CLI configs to other programs - process.title = 'npm' - - // if npm is called as "npmg" or "npm_g", then run in global mode. - if (process.argv[1][process.argv[1].length - 1] === 'g') { - process.argv.splice(1, 1, 'npm', '-g') - } - - const nodeVersion = process.version.replace(/-.*$/, '') - const pkg = require('../package.json') - const engines = pkg.engines.node - const npmVersion = `v${pkg.version}` - - const unsupportedMessage = `npm ${npmVersion} does not support Node.js ${nodeVersion}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.` - - const brokenMessage = `ERROR: npm ${npmVersion} is known not to run on Node.js ${nodeVersion}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.` - - // Coverage ignored because this is only hit in very unsupported node versions and it's a best effort attempt to show something nice in those cases - /* istanbul ignore next */ - const syntaxErrorHandler = (err) => { - if (err instanceof SyntaxError) { - // eslint-disable-next-line no-console - console.error(`${brokenMessage}\n\nERROR:`) - // eslint-disable-next-line no-console - console.error(err) - return process.exit(1) - } - throw err - } - - process.on('uncaughtException', syntaxErrorHandler) - process.on('unhandledRejection', syntaxErrorHandler) - - const satisfies = require('semver/functions/satisfies') - const exitHandler = require('./utils/exit-handler.js') - const Npm = require('./npm.js') - const npm = new Npm() - exitHandler.setNpm(npm) - - // only log node and npm paths in argv initially since argv can contain sensitive info. a cleaned version will be logged later - const log = require('./utils/log-shim.js') - log.verbose('cli', process.argv.slice(0, 2).join(' ')) - log.info('using', 'npm@%s', npm.version) - log.info('using', 'node@%s', process.version) - - // At this point we've required a few files and can be pretty sure we dont contain invalid syntax for this version of node. It's possible a lazy require would, but that's unlikely enough that it's not worth catching anymore and we attach the more important exit handlers. - process.off('uncaughtException', syntaxErrorHandler) - process.off('unhandledRejection', syntaxErrorHandler) - process.on('uncaughtException', exitHandler) - process.on('unhandledRejection', exitHandler) - - // It is now safe to log a warning if they are using a version of node that is not going to fail on syntax errors but is still unsupported and untested and might not work reliably. This is safe to use the logger now which we want since this will show up in the error log too. - if (!satisfies(nodeVersion, engines)) { - log.warn('cli', unsupportedMessage) - } - - let cmd - // Now actually fire up npm and run the command. - // This is how to use npm programmatically: - try { - await npm.load() - - // npm -v - if (npm.config.get('version', 'cli')) { - npm.output(npm.version) - return exitHandler() - } - - // npm --versions - if (npm.config.get('versions', 'cli')) { - npm.argv = ['version'] - npm.config.set('usage', false, 'cli') - } - - cmd = npm.argv.shift() - if (!cmd) { - npm.output(await npm.usage) - process.exitCode = 1 - return exitHandler() - } - - await npm.exec(cmd) - return exitHandler() - } catch (err) { - if (err.code === 'EUNKNOWNCOMMAND') { - const didYouMean = require('./utils/did-you-mean.js') - const suggestions = await didYouMean(npm, npm.localPrefix, cmd) - npm.output(`Unknown command: "${cmd}"${suggestions}\n`) - npm.output('To see a list of supported npm commands, run:\n npm help') - process.exitCode = 1 - return exitHandler() - } - return exitHandler(err) - } -} +module.exports = (process) => validateEngines(process, () => require(cliEntry)) diff --git a/lib/es6/validate-engines.js b/lib/es6/validate-engines.js new file mode 100644 index 0000000000000..0eaa549fc3702 --- /dev/null +++ b/lib/es6/validate-engines.js @@ -0,0 +1,49 @@ +// This is separate to indicate that it should contain code we expect to work in +// all versions of node >= 6. This is a best effort to catch syntax errors to +// give users a good error message if they are using a node version that doesn't +// allow syntax we are using such as private properties, etc. This file is +// linted with ecmaVersion=6 so we don't use invalid syntax, which is set in the +// .eslintrc.local.json file + +const { engines: { node: engines }, version } = require('../../package.json') +const npm = `v${version}` + +module.exports = (process, getCli) => { + const node = process.version.replace(/-.*$/, '') + + /* eslint-disable-next-line max-len */ + const unsupportedMessage = `npm ${npm} does not support Node.js ${node}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.` + + /* eslint-disable-next-line max-len */ + const brokenMessage = `ERROR: npm ${npm} is known not to run on Node.js ${node}. This version of npm supports the following node versions: \`${engines}\`. You can find the latest version at https://nodejs.org/.` + + // coverage ignored because this is only hit in very unsupported node versions + // and it's a best effort attempt to show something nice in those cases + /* istanbul ignore next */ + const syntaxErrorHandler = (err) => { + if (err instanceof SyntaxError) { + // eslint-disable-next-line no-console + console.error(`${brokenMessage}\n\nERROR:`) + // eslint-disable-next-line no-console + console.error(err) + return process.exit(1) + } + throw err + } + + process.on('uncaughtException', syntaxErrorHandler) + process.on('unhandledRejection', syntaxErrorHandler) + + // require this only after setting up the error handlers + const cli = getCli() + return cli(process, { + node, + npm, + engines, + unsupportedMessage, + off: () => { + process.off('uncaughtException', syntaxErrorHandler) + process.off('unhandledRejection', syntaxErrorHandler) + }, + }) +} diff --git a/test/lib/cli.js b/test/lib/cli.js index cafd13feabe93..5142901360bc4 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -1,6 +1,7 @@ const t = require('tap') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const tmock = require('../fixtures/tmock') +const validateEngines = require('../../lib/es6/validate-engines.js') const cliMock = async (t, opts) => { let exitHandlerArgs = null @@ -20,7 +21,7 @@ const cliMock = async (t, opts) => { return { Npm, - cli, + cli: (p) => validateEngines(p, () => cli), outputs, exitHandlerCalled: () => exitHandlerArgs, exitHandlerNpm: () => npm, diff --git a/test/lib/es6/validate-engines.js b/test/lib/es6/validate-engines.js new file mode 100644 index 0000000000000..0e6bce726af96 --- /dev/null +++ b/test/lib/es6/validate-engines.js @@ -0,0 +1,34 @@ +const t = require('tap') +const mockGlobals = require('@npmcli/mock-globals') +const tmock = require('../../fixtures/tmock') + +const mockValidateEngines = (t) => { + const validateEngines = tmock(t, '{LIB}/es6/validate-engines.js', { + '{ROOT}/package.json': { version: '1.2.3', engines: { node: '>=0' } }, + }) + mockGlobals(t, { 'process.version': 'v4.5.6' }) + return validateEngines(process, () => (_, r) => r) +} + +t.test('validate engines', async t => { + t.equal(process.listenerCount('uncaughtException'), 0) + t.equal(process.listenerCount('unhandledRejection'), 0) + + const result = mockValidateEngines(t) + + t.equal(process.listenerCount('uncaughtException'), 1) + t.equal(process.listenerCount('unhandledRejection'), 1) + + t.match(result, { + node: 'v4.5.6', + npm: 'v1.2.3', + engines: '>=0', + /* eslint-disable-next-line max-len */ + unsupportedMessage: 'npm v1.2.3 does not support Node.js v4.5.6. This version of npm supports the following node versions: `>=0`. You can find the latest version at https://nodejs.org/.', + }) + + result.off() + + t.equal(process.listenerCount('uncaughtException'), 0) + t.equal(process.listenerCount('unhandledRejection'), 0) +})