test: check cache for root-owned files

This requires that all tests use `common.cache` as their cache folder,
and fix the ownership of any cache files that the test is directly
managing.

It also adds a `sudotest` target which runs the tests as root, and then
verifies that no root-owned files have been dropped into the cache.

Lastly, several setup() and cleanup() methods have been removed, as they
are largely unnecessary.  common-tap does all the required setup for
test package directories and cache folders, and removing the cache
avoids the root-ownership test, which is an important integration
requirement.

Author: isaacs

package.json

@@ -292,6 +292,7 @@
     "tap-cover": "tap -J --nyc-arg=--cache --coverage --timeout 600",
     "pretest": "standard",
     "test": "npm run test-tap --",
+    "sudotest": "sudo npm run tap -- \"test/tap/*.js\"",
     "posttest": "rimraf test/npm_cache*",
     "test-coverage": "npm run tap-cover -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",
     "test-tap": "npm run tap -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",

test/common-tap.js

@@ -22,27 +22,64 @@ if (!global.setImmediate || !require('timers').setImmediate) {
 }
 
 var spawn = require('child_process').spawn
+const spawnSync = require('child_process').spawnSync
 var path = require('path')
 
+// space these out to help prevent collisions
+const testId = 3 * (+process.env.TAP_CHILD_ID || 0)
+
 // provide a working dir unique to each test
 const main = require.main.filename
-exports.pkg = path.resolve(path.dirname(main), path.basename(main, '.js'))
+const testName = path.basename(main, '.js')
+exports.pkg = path.resolve(path.dirname(main), testName)
+var commonCache = path.resolve(__dirname, 'npm_cache_' + testName)
+exports.cache = commonCache
+
 const mkdirp = require('mkdirp')
 const rimraf = require('rimraf')
+rimraf.sync(exports.pkg)
+rimraf.sync(commonCache)
 mkdirp.sync(exports.pkg)
+mkdirp.sync(commonCache)
+// if we're in sudo mode, make sure that the cache is not root-owned
+const isRoot = process.getuid && process.getuid() === 0
+const isSudo = isRoot && process.env.SUDO_UID && process.env.SUDO_GID
+if (isSudo) {
+  const sudoUid = +process.env.SUDO_UID
+  const sudoGid = +process.env.SUDO_GID
+  fs.chownSync(commonCache, sudoUid, sudoGid)
+}
+
+const returnCwd = path.dirname(__dirname)
+const find = require('which').sync('find')
 require('tap').teardown(() => {
+  // work around windows folder locking
+  process.chdir(returnCwd)
   try {
-    rimraf.sync(exports.pkg)
+    if (isSudo) {
+      // running tests as sudo.  ensure we didn't leave any root-owned
+      // files in the cache by mistake.
+      const args = [ commonCache, '-uid', '0' ]
+      const found = spawnSync(find, args)
+      const output = found && found.stdout && found.stdout.toString()
+      if (output.length) {
+        const er = new Error('Root-owned files left in cache!')
+        er.testName = main
+        er.files = output.trim().split('\n')
+        throw er
+      }
+    }
+    if (!process.env.NO_TEST_CLEANUP) {
+      rimraf.sync(exports.pkg)
+      rimraf.sync(commonCache)
+    }
   } catch (e) {
     if (process.platform !== 'win32') {
       throw e
     }
   }
 })
 
-// space these out to help prevent collisions
-const testId = 3 * (+process.env.TAP_CHILD_ID || 0)
-
 var port = exports.port = 15443 + testId
 exports.registry = 'http://localhost:' + port
 
@@ -59,8 +96,8 @@ ourenv.npm_config_progress = 'false'
 ourenv.npm_config_metrics = 'false'
 ourenv.npm_config_audit = 'false'
 
-var npm_config_cache = path.resolve(__dirname, 'npm_cache_' + testId)
-ourenv.npm_config_cache = exports.npm_config_cache = npm_config_cache
+ourenv.npm_config_unsafe_perm = 'true'
+ourenv.npm_config_cache = commonCache
 ourenv.npm_config_userconfig = exports.npm_config_userconfig = configCommon.userconfig
 ourenv.npm_config_globalconfig = exports.npm_config_globalconfig = configCommon.globalconfig
 ourenv.npm_config_global_style = 'false'
@@ -94,7 +131,10 @@ exports.npm = function (cmd, opts, cb) {
   opts.env = opts.env || process.env
   if (opts.env._storage) opts.env = Object.assign({}, opts.env._storage)
   if (!opts.env.npm_config_cache) {
-    opts.env.npm_config_cache = npm_config_cache
+    opts.env.npm_config_cache = commonCache
+  }
+  if (!opts.env.npm_config_unsafe_perm) {
+    opts.env.npm_config_unsafe_perm = 'true'
   }
   if (!opts.env.npm_config_send_metrics) {
     opts.env.npm_config_send_metrics = 'false'

test/tap/404-parent.js

@@ -5,7 +5,6 @@ var osenv = require('osenv')
 var path = require('path')
 var fs = require('fs')
 var rimraf = require('rimraf')
-var mkdirp = require('mkdirp')
 const pkg = common.pkg
 var mr = require('npm-registry-mock')
 
@@ -26,8 +25,6 @@ test('cleanup', function (t) {
 })
 
 function setup () {
-  mkdirp.sync(pkg)
-  mkdirp.sync(path.resolve(pkg, 'cache'))
   fs.writeFileSync(path.resolve(pkg, 'package.json'), JSON.stringify({
     author: 'Evan Lucas',
     name: '404-parent-test',

test/tap/404-private-registry-scoped.js

@@ -1,23 +1,9 @@
 var test = require('tap').test
-var mkdirp = require('mkdirp')
-var rimraf = require('rimraf')
 var common = require('../common-tap.js')
 var mr = common.fakeRegistry.compat
 var server
 
-var testdir = common.pkg
-
-function setup () {
-  cleanup()
-  mkdirp.sync(testdir)
-}
-
-function cleanup () {
-  rimraf.sync(testdir)
-}
-
 test('setup', function (t) {
-  setup()
   mr({port: common.port, throwOnUnmatched: true}, function (err, s) {
     t.ifError(err, 'registry mocked successfully')
     server = s
@@ -30,7 +16,7 @@ test('scoped package names not mangled on error with non-root registry', functio
   common.npm(
     [
       '--registry=' + common.registry,
-      '--cache=' + testdir,
+      '--cache=' + common.cache,
       'cache',
       'add',
       '@scope/foo@*',
@@ -43,14 +29,8 @@ test('scoped package names not mangled on error with non-root registry', functio
       t.match(stderr, /E404/, 'should notify the sort of error as a 404')
       t.match(stderr, /@scope(?:%2f|\/)foo/, 'should have package name in error')
       server.done()
+      server.close()
       t.end()
     }
   )
 })
-
-test('cleanup', function (t) {
-  server.close()
-  cleanup()
-  t.pass('cleaned up')
-  t.end()
-})

test/tap/404-private-registry.js

@@ -1,25 +1,12 @@
 var test = require('tap').test
 var path = require('path')
-var mkdirp = require('mkdirp')
-var rimraf = require('rimraf')
 var common = require('../common-tap.js')
 var mr = common.fakeRegistry.compat
 var server
 
 var packageName = path.basename(__filename, '.js')
-var testdir = common.pkg
-
-function setup () {
-  cleanup()
-  mkdirp.sync(testdir)
-}
-
-function cleanup () {
-  rimraf.sync(testdir)
-}
 
 test('setup', function (t) {
-  setup()
   mr({port: common.port, throwOnUnmatched: true}, function (err, s) {
     t.ifError(err, 'registry mocked successfully')
     server = s
@@ -32,7 +19,7 @@ test('package names not mangled on error with non-root registry', function (t) {
   common.npm(
     [
       '--registry=' + common.registry,
-      '--cache=' + testdir,
+      '--cache=' + common.cache,
       'cache',
       'add',
       packageName + '@*'
@@ -43,14 +30,8 @@ test('package names not mangled on error with non-root registry', function (t) {
       t.equal(code, 1, 'exited with error')
       t.match(stderr, packageName, 'should have package name in error')
       server.done()
+      server.close()
       t.end()
     }
   )
 })
-
-test('cleanup', function (t) {
-  server.close()
-  cleanup()
-  t.pass('cleaned up')
-  t.end()
-})

test/tap/404.js

@@ -11,7 +11,7 @@ const invalidPackage = /Your package name is not valid, because[\s\S]+1\. name c
 
 const basedir = common.pkg
 const testdir = path.join(basedir, 'testdir')
-const cachedir = path.join(basedir, 'cache')
+const cachedir = common.cache
 const globaldir = path.join(basedir, 'global')
 const tmpdir = path.join(basedir, 'tmp')
 

test/tap/access.js

@@ -19,22 +19,18 @@ const scoped = {
 }
 
 test('setup', function (t) {
-  mkdirp(pkg, function (er) {
-    t.ifError(er, pkg + ' made successfully')
-
-    mr({port: common.port}, function (err, s) {
-      t.ifError(err, 'registry mocked successfully')
-      server = s
-
-      fs.writeFile(
-        path.join(pkg, 'package.json'),
-        JSON.stringify(scoped),
-        function (er) {
-          t.ifError(er, 'wrote package.json')
-          t.end()
-        }
-      )
-    })
+  mr({port: common.port}, function (err, s) {
+    t.ifError(err, 'registry mocked successfully')
+    server = s
+
+    fs.writeFile(
+      path.join(pkg, 'package.json'),
+      JSON.stringify(scoped),
+      function (er) {
+        t.ifError(er, 'wrote package.json')
+        t.end()
+      }
+    )
   })
 })
 

test/tap/all-package-metadata-cache-stream-unit.js

@@ -5,24 +5,30 @@ const common = require('../common-tap.js')
 const getStream = require('get-stream')
 const mkdirp = require('mkdirp')
 const path = require('path')
-const rimraf = require('rimraf')
 const Tacks = require('tacks')
 const {test} = require('tap')
 
 const {File} = Tacks
 
 const _createCacheEntryStream = require('../../lib/search/all-package-metadata.js')._createCacheEntryStream
 
-const PKG_DIR = common.pkg
-const CACHE_DIR = path.resolve(PKG_DIR, 'cache')
-
+// this test uses a fresh cache for each test block
+// create them all in common.cache so that we can verify
+// them for root-owned files in sudotest
+let CACHE_DIR
+let cacheCounter = 1
+const chownr = require('chownr')
 function setup () {
+  CACHE_DIR = common.cache + '/' + cacheCounter++
   mkdirp.sync(CACHE_DIR)
+  fixOwner(CACHE_DIR)
 }
 
-function cleanup () {
-  rimraf.sync(PKG_DIR)
-}
+const fixOwner = (
+  process.getuid && process.getuid() === 0 &&
+  process.env.SUDO_UID && process.env.SUDO_GID
+) ? (path) => chownr.sync(path, +process.env.SUDO_UID, +process.env.SUDO_GID)
+  : () => {}
 
 test('createCacheEntryStream basic', t => {
   setup()
@@ -39,6 +45,7 @@ test('createCacheEntryStream basic', t => {
     }
   }))
   fixture.create(cachePath)
+  fixOwner(cachePath)
   return _createCacheEntryStream(cachePath, {}).then(({
     updateStream: stream,
     updatedLatest: latest
@@ -53,7 +60,6 @@ test('createCacheEntryStream basic', t => {
         name: 'foo',
         version: '1.0.0'
       }])
-      cleanup()
     })
   })
 })
@@ -63,12 +69,12 @@ test('createCacheEntryStream empty cache', t => {
   const cachePath = path.join(CACHE_DIR, '.cache.json')
   const fixture = new Tacks(File({}))
   fixture.create(cachePath)
+  fixOwner(cachePath)
   return _createCacheEntryStream(cachePath, {}).then(
     () => { throw new Error('should not succeed') },
     err => {
       t.ok(err, 'returned an error because there was no _updated')
       t.match(err.message, /Empty or invalid stream/, 'useful error message')
-      cleanup()
     }
   )
 })
@@ -80,6 +86,7 @@ test('createCacheEntryStream no entry cache', t => {
     '_updated': 1234
   }))
   fixture.create(cachePath)
+  fixOwner(cachePath)
   return _createCacheEntryStream(cachePath, {}).then(({
     updateStream: stream,
     updatedLatest: latest
@@ -88,7 +95,6 @@ test('createCacheEntryStream no entry cache', t => {
     t.ok(stream, 'returned a stream')
     return getStream.array(stream).then(results => {
       t.deepEquals(results, [], 'no results')
-      cleanup()
     })
   })
 })
@@ -101,7 +107,6 @@ test('createCacheEntryStream missing cache', t => {
     err => {
       t.ok(err, 'returned an error because there was no cache')
       t.equals(err.code, 'ENOENT', 'useful error message')
-      cleanup()
     }
   )
 })