fix: refactor engines validation to lint syntax (#6479)

.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,
+  }),
+}

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)
+  }
+}

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))

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)
+    },
+  })
+}

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,

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)
+})