Preserve builtin conf when installing npm globally

When a file named 'npmrc' is in the root of the npm module that is
currently running, it adds config values that override the defaults (but
not the global or user configs).

This is a way for a system package installer to tell the npm that it
installs to put its globals somewhere other than the default.  In order
to keep these configs around when users self-update npm with `npm i -g
npm`, these config values must be "sticky", and ride along into the
newly globally installed npm.

This commit restores this behavior, fixing self-updating npm for Windows
users, and any other systems that may make use of this functionality.

Fixes: #2002

PR-URL: https://github.com/npm/cli/pull/2184
Credit: @isaacs
Close: #2184
Reviewed-by: @ruyadorno

12 files changed, 173 insertions, 52 deletions

diff --git a/lib/audit.js b/lib/audit.js
index e77beab1e..cb8ab5b3a 100644
--- a/lib/audit.js
+++ b/lib/audit.js
@@ -2,7 +2,7 @@ const Arborist = require('@npmcli/arborist')
 const auditReport = require('npm-audit-report')
 const npm = require('./npm.js')
 const output = require('./utils/output.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 const auditError = require('./utils/audit-error.js')
 
 const audit = async args => {
@@ -14,7 +14,7 @@ const audit = async args => {
   const fix = args[0] === 'fix'
   await arb.audit({ fix })
   if (fix)
-    reifyOutput(arb)
+    await reifyFinish(arb)
   else {
     // will throw if there's an error, because this is an audit command
     auditError(arb.auditReport)
diff --git a/lib/ci.js b/lib/ci.js
index a1632e7e9..1255fbc26 100644
--- a/lib/ci.js
+++ b/lib/ci.js
@@ -1,7 +1,7 @@
 const util = require('util')
 const Arborist = require('@npmcli/arborist')
 const rimraf = util.promisify(require('rimraf'))
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 
 const log = require('npmlog')
 const npm = require('./npm.js')
@@ -35,7 +35,7 @@ const ci = async () => {
   ])
   // npm ci should never modify the lockfile or package.json
   await arb.reify({ ...npm.flatOptions, save: false })
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 module.exports = Object.assign(cmd, { completion, usage })
diff --git a/lib/dedupe.js b/lib/dedupe.js
index a08c9f3f8..fe8243e21 100644
--- a/lib/dedupe.js
+++ b/lib/dedupe.js
@@ -2,7 +2,7 @@
 const npm = require('./npm.js')
 const Arborist = require('@npmcli/arborist')
 const usageUtil = require('./utils/usage.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 
 const usage = usageUtil('dedupe', 'npm dedupe')
 const completion = require('./utils/completion/none.js')
@@ -18,7 +18,7 @@ const dedupe = async (args) => {
     dryRun,
   })
   await arb.dedupe(npm.flatOptions)
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 module.exports = Object.assign(cmd, { usage, completion })
diff --git a/lib/install.js b/lib/install.js
index 5f04fcd4f..f621c85c2 100644
--- a/lib/install.js
+++ b/lib/install.js
@@ -6,13 +6,15 @@ const util = require('util')
 const readdir = util.promisify(fs.readdir)
 const npm = require('./npm.js')
 const usageUtil = require('./utils/usage.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 const log = require('npmlog')
 const { resolve, join } = require('path')
 const Arborist = require('@npmcli/arborist')
 const runScript = require('@npmcli/run-script')
 
-const install = async (args, cb) => {
+const cmd = async (args, cb) => install(args).then(() => cb()).catch(cb)
+
+const install = async args => {
   // the /path/to/node_modules/..
   const globalTop = resolve(npm.globalDir, '..')
   const { ignoreScripts, global: isGlobalInstall } = npm.flatOptions
@@ -34,38 +36,33 @@ const install = async (args, cb) => {
     path: where,
   })
 
-  try {
-    await arb.reify({
-      ...npm.flatOptions,
-      add: args,
-    })
-    if (!args.length && !isGlobalInstall && !ignoreScripts) {
-      const { scriptShell } = npm.flatOptions
-      const scripts = [
-        'preinstall',
-        'install',
-        'postinstall',
-        'prepublish', // XXX should we remove this finally??
-        'preprepare',
-        'prepare',
-        'postprepare',
-      ]
-      for (const event of scripts) {
-        await runScript({
-          path: where,
-          args: [],
-          scriptShell,
-          stdio: 'inherit',
-          stdioString: true,
-          event,
-        })
-      }
+  await arb.reify({
+    ...npm.flatOptions,
+    add: args,
+  })
+  if (!args.length && !isGlobalInstall && !ignoreScripts) {
+    const { scriptShell } = npm.flatOptions
+    const scripts = [
+      'preinstall',
+      'install',
+      'postinstall',
+      'prepublish', // XXX should we remove this finally??
+      'preprepare',
+      'prepare',
+      'postprepare',
+    ]
+    for (const event of scripts) {
+      await runScript({
+        path: where,
+        args: [],
+        scriptShell,
+        stdio: 'inherit',
+        stdioString: true,
+        event,
+      })
     }
-    reifyOutput(arb)
-    cb()
-  } catch (er) {
-    cb(er)
   }
+  await reifyFinish(arb)
 }
 
 const usage = usageUtil(
@@ -144,4 +141,4 @@ const completion = async (opts, cb) => {
   cb()
 }
 
-module.exports = Object.assign(install, { usage, completion })
+module.exports = Object.assign(cmd, { usage, completion })
diff --git a/lib/link.js b/lib/link.js
index d7303fd08..bee44d43a 100644
--- a/lib/link.js
+++ b/lib/link.js
@@ -10,7 +10,7 @@ const semver = require('semver')
 
 const npm = require('./npm.js')
 const usageUtil = require('./utils/usage.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 
 const completion = (opts, cb) => {
   const dir = npm.globalDir
@@ -122,7 +122,7 @@ const linkInstall = async args => {
     add: names.map(l => `file:${resolve(globalTop, 'node_modules', l)}`),
   })
 
-  reifyOutput(localArb)
+  await reifyFinish(localArb)
 }
 
 const linkPkg = async () => {
@@ -133,7 +133,7 @@ const linkPkg = async () => {
     global: true,
   })
   await arb.reify({ add: [`file:${npm.prefix}`] })
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 module.exports = Object.assign(cmd, { completion, usage })
diff --git a/lib/prune.js b/lib/prune.js
index aa2ed3780..ea6ed4108 100644
--- a/lib/prune.js
+++ b/lib/prune.js
@@ -3,7 +3,7 @@ const npm = require('./npm.js')
 const Arborist = require('@npmcli/arborist')
 const usageUtil = require('./utils/usage.js')
 
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 
 const usage = usageUtil('prune',
   'npm prune [[<@scope>/]<pkg>...] [--production]'
@@ -19,7 +19,7 @@ const prune = async () => {
     path: where,
   })
   await arb.prune(npm.flatOptions)
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 module.exports = Object.assign(cmd, { usage, completion })
diff --git a/lib/uninstall.js b/lib/uninstall.js
index ec997ae64..dbaa992f5 100644
--- a/lib/uninstall.js
+++ b/lib/uninstall.js
@@ -5,7 +5,7 @@ const npm = require('./npm.js')
 const rpj = require('read-package-json-fast')
 const { resolve } = require('path')
 const usageUtil = require('./utils/usage.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 
 const cmd = (args, cb) => rm(args).then(() => cb()).catch(cb)
 
@@ -32,7 +32,7 @@ const rm = async args => {
     ...npm.flatOptions,
     rm: args,
   })
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 const usage = usageUtil(
diff --git a/lib/update.js b/lib/update.js
index 791e67e40..0a786e30f 100644
--- a/lib/update.js
+++ b/lib/update.js
@@ -4,7 +4,7 @@ const Arborist = require('@npmcli/arborist')
 const log = require('npmlog')
 const npm = require('./npm.js')
 const usageUtil = require('./utils/usage.js')
-const reifyOutput = require('./utils/reify-output.js')
+const reifyFinish = require('./utils/reify-finish.js')
 const completion = require('./utils/completion/installed-deep.js')
 
 const usage = usageUtil(
@@ -32,7 +32,7 @@ const update = async args => {
   })
 
   await arb.reify({ update })
-  reifyOutput(arb)
+  await reifyFinish(arb)
 }
 
 module.exports = Object.assign(cmd, { usage, completion })
diff --git a/lib/utils/reify-finish.js b/lib/utils/reify-finish.js
new file mode 100644
index 000000000..76dba06cb
--- /dev/null
+++ b/lib/utils/reify-finish.js
@@ -0,0 +1,31 @@
+const reifyOutput = require('./reify-output.js')
+const npm = require('../npm.js')
+const ini = require('ini')
+const {writeFile} = require('fs').promises
+const {resolve} = require('path')
+
+const reifyFinish = async arb => {
+  await saveBuiltinConfig(arb)
+  reifyOutput(arb)
+}
+
+const saveBuiltinConfig = async arb => {
+  const { options: { global }, actualTree } = arb
+  if (!global)
+    return
+
+  // if we are using a builtin config, and just installed npm as
+  // a top-level global package, we have to preserve that config.
+  const npmNode = actualTree.inventory.get('node_modules/npm')
+  if (!npmNode)
+    return
+
+  const builtinConf = npm.config.data.get('builtin')
+  if (builtinConf.loadError)
+    return
+
+  const content = ini.stringify(builtinConf.raw).trim() + '\n'
+  await writeFile(resolve(npmNode.path, 'npmrc'), content)
+}
+
+module.exports = reifyFinish
diff --git a/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js b/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js
new file mode 100644
index 000000000..a82905a39
--- /dev/null
+++ b/tap-snapshots/test-lib-utils-reify-finish.js-TAP.test.js
@@ -0,0 +1,15 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`test/lib/utils/reify-finish.js TAP should write if everything above passes > written config 1`] = `
+hasBuiltinConfig=true
+x=y
+
+[nested]
+foo=bar
+
+`
diff --git a/test/lib/audit.js b/test/lib/audit.js
index 4918cb2fc..723675c32 100644
--- a/test/lib/audit.js
+++ b/test/lib/audit.js
@@ -5,7 +5,7 @@ const audit = require('../../lib/audit.js')
 t.test('should audit using Arborist', t => {
   let ARB_ARGS = null
   let AUDIT_CALLED = false
-  let REIFY_OUTPUT_CALLED = false
+  let REIFY_FINISH_CALLED = false
   let AUDIT_REPORT_CALLED = false
   let OUTPUT_CALLED = false
   let ARB_OBJ = null
@@ -32,11 +32,11 @@ t.test('should audit using Arborist', t => {
         this.auditReport = {}
       }
     },
-    '../../lib/utils/reify-output.js': arb => {
+    '../../lib/utils/reify-finish.js': arb => {
       if (arb !== ARB_OBJ) {
         throw new Error('got wrong object passed to reify-output')
       }
-      REIFY_OUTPUT_CALLED = true
+      REIFY_FINISH_CALLED = true
     },
     '../../lib/utils/output.js': () => {
       OUTPUT_CALLED = true
@@ -55,7 +55,7 @@ t.test('should audit using Arborist', t => {
 
   t.test('audit fix', t => {
     audit(['fix'], () => {
-      t.equal(REIFY_OUTPUT_CALLED, true, 'called reify output')
+      t.equal(REIFY_FINISH_CALLED, true, 'called reify output')
       t.end()
     })
   })
diff --git a/test/lib/utils/reify-finish.js b/test/lib/utils/reify-finish.js
new file mode 100644
index 000000000..c19956f0b
--- /dev/null
+++ b/test/lib/utils/reify-finish.js
@@ -0,0 +1,78 @@
+const t = require('tap')
+const requireInject = require('require-inject')
+
+const npm = {
+  config: {
+    data: {
+      get: () => builtinConfMock,
+    },
+  },
+}
+
+const builtinConfMock = {
+  loadError: new Error('no builtin config'),
+  raw: { hasBuiltinConfig: true, x: 'y', nested: { foo: 'bar' }},
+}
+
+const reifyOutput = () => {}
+
+let expectWrite = false
+const realFs = require('fs')
+const fs = {
+  ...realFs,
+  promises: {
+    ...realFs.promises,
+    writeFile: async (path, data) => {
+      if (!expectWrite)
+        throw new Error('did not expect to write builtin config file')
+      return realFs.promises.writeFile(path, data)
+    },
+  },
+}
+
+const reifyFinish = requireInject('../../../lib/utils/reify-finish.js', {
+  fs,
+  '../../../lib/npm.js': npm,
+  '../../../lib/utils/reify-output.js': reifyOutput,
+})
+
+t.test('should not write if not global', async t => {
+  expectWrite = false
+  await reifyFinish({
+    options: { global: false },
+    actualTree: {},
+  })
+})
+
+t.test('should not write if no global npm module', async t => {
+  expectWrite = false
+  await reifyFinish({
+    options: { global: true },
+    actualTree: {
+      inventory: new Map(),
+    },
+  })
+})
+
+t.test('should not write if builtin conf had load error', async t => {
+  expectWrite = false
+  await reifyFinish({
+    options: { global: true },
+    actualTree: {
+      inventory: new Map([['node_modules/npm', {}]]),
+    },
+  })
+})
+
+t.test('should write if everything above passes', async t => {
+  expectWrite = true
+  delete builtinConfMock.loadError
+  const path = t.testdir()
+  await reifyFinish({
+    options: { global: true },
+    actualTree: {
+      inventory: new Map([['node_modules/npm', {path}]]),
+    },
+  })
+  t.matchSnapshot(fs.readFileSync(`${path}/npmrc`, 'utf8'), 'written config')
+})