Skip to content

Commit 2a78b96

Browse files
isaacsisaacs
committed
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.
1 parent ba7f146 commit 2a78b96

115 files changed

Lines changed: 372 additions & 737 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@
292292
"tap-cover": "tap -J --nyc-arg=--cache --coverage --timeout 600",
293293
"pretest": "standard",
294294
"test": "npm run test-tap --",
295+
"sudotest": "sudo npm run tap -- \"test/tap/*.js\"",
295296
"posttest": "rimraf test/npm_cache*",
296297
"test-coverage": "npm run tap-cover -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",
297298
"test-tap": "npm run tap -- \"test/tap/*.js\" \"test/network/*.js\" \"test/broken-under-*/*.js\"",

‎test/common-tap.js‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,64 @@ if (!global.setImmediate || !require('timers').setImmediate) {
2222
}
2323

2424
var spawn = require('child_process').spawn
25+
const spawnSync = require('child_process').spawnSync
2526
var path = require('path')
2627

28+
// space these out to help prevent collisions
29+
const testId = 3 * (+process.env.TAP_CHILD_ID || 0)
30+
2731
// provide a working dir unique to each test
2832
const main = require.main.filename
29-
exports.pkg = path.resolve(path.dirname(main), path.basename(main, '.js'))
33+
const testName = path.basename(main, '.js')
34+
exports.pkg = path.resolve(path.dirname(main), testName)
35+
var commonCache = path.resolve(__dirname, 'npm_cache_' + testName)
36+
exports.cache = commonCache
37+
3038
const mkdirp = require('mkdirp')
3139
const rimraf = require('rimraf')
40+
rimraf.sync(exports.pkg)
41+
rimraf.sync(commonCache)
3242
mkdirp.sync(exports.pkg)
43+
mkdirp.sync(commonCache)
44+
// if we're in sudo mode, make sure that the cache is not root-owned
45+
const isRoot = process.getuid && process.getuid() === 0
46+
const isSudo = isRoot && process.env.SUDO_UID && process.env.SUDO_GID
47+
if (isSudo) {
48+
const sudoUid = +process.env.SUDO_UID
49+
const sudoGid = +process.env.SUDO_GID
50+
fs.chownSync(commonCache, sudoUid, sudoGid)
51+
}
52+
53+
const returnCwd = path.dirname(__dirname)
54+
const find = require('which').sync('find')
3355
require('tap').teardown(() => {
56+
// work around windows folder locking
57+
process.chdir(returnCwd)
3458
try {
35-
rimraf.sync(exports.pkg)
59+
if (isSudo) {
60+
// running tests as sudo. ensure we didn't leave any root-owned
61+
// files in the cache by mistake.
62+
const args = [ commonCache, '-uid', '0' ]
63+
const found = spawnSync(find, args)
64+
const output = found && found.stdout && found.stdout.toString()
65+
if (output.length) {
66+
const er = new Error('Root-owned files left in cache!')
67+
er.testName = main
68+
er.files = output.trim().split('\n')
69+
throw er
70+
}
71+
}
72+
if (!process.env.NO_TEST_CLEANUP) {
73+
rimraf.sync(exports.pkg)
74+
rimraf.sync(commonCache)
75+
}
3676
} catch (e) {
3777
if (process.platform !== 'win32') {
3878
throw e
3979
}
4080
}
4181
})
4282

43-
// space these out to help prevent collisions
44-
const testId = 3 * (+process.env.TAP_CHILD_ID || 0)
45-
4683
var port = exports.port = 15443 + testId
4784
exports.registry = 'http://localhost:' + port
4885

@@ -59,8 +96,8 @@ ourenv.npm_config_progress = 'false'
5996
ourenv.npm_config_metrics = 'false'
6097
ourenv.npm_config_audit = 'false'
6198

62-
var npm_config_cache = path.resolve(__dirname, 'npm_cache_' + testId)
63-
ourenv.npm_config_cache = exports.npm_config_cache = npm_config_cache
99+
ourenv.npm_config_unsafe_perm = 'true'
100+
ourenv.npm_config_cache = commonCache
64101
ourenv.npm_config_userconfig = exports.npm_config_userconfig = configCommon.userconfig
65102
ourenv.npm_config_globalconfig = exports.npm_config_globalconfig = configCommon.globalconfig
66103
ourenv.npm_config_global_style = 'false'
@@ -94,7 +131,10 @@ exports.npm = function (cmd, opts, cb) {
94131
opts.env = opts.env || process.env
95132
if (opts.env._storage) opts.env = Object.assign({}, opts.env._storage)
96133
if (!opts.env.npm_config_cache) {
97-
opts.env.npm_config_cache = npm_config_cache
134+
opts.env.npm_config_cache = commonCache
135+
}
136+
if (!opts.env.npm_config_unsafe_perm) {
137+
opts.env.npm_config_unsafe_perm = 'true'
98138
}
99139
if (!opts.env.npm_config_send_metrics) {
100140
opts.env.npm_config_send_metrics = 'false'

‎test/tap/404-parent.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ var osenv = require('osenv')
55
var path = require('path')
66
var fs = require('fs')
77
var rimraf = require('rimraf')
8-
var mkdirp = require('mkdirp')
98
const pkg = common.pkg
109
var mr = require('npm-registry-mock')
1110

@@ -26,8 +25,6 @@ test('cleanup', function (t) {
2625
})
2726

2827
function setup () {
29-
mkdirp.sync(pkg)
30-
mkdirp.sync(path.resolve(pkg, 'cache'))
3128
fs.writeFileSync(path.resolve(pkg, 'package.json'), JSON.stringify({
3229
author: 'Evan Lucas',
3330
name: '404-parent-test',

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

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,9 @@
11
var test = require('tap').test
2-
var mkdirp = require('mkdirp')
3-
var rimraf = require('rimraf')
42
var common = require('../common-tap.js')
53
var mr = common.fakeRegistry.compat
64
var server
75

8-
var testdir = common.pkg
9-
10-
function setup () {
11-
cleanup()
12-
mkdirp.sync(testdir)
13-
}
14-
15-
function cleanup () {
16-
rimraf.sync(testdir)
17-
}
18-
196
test('setup', function (t) {
20-
setup()
217
mr({port: common.port, throwOnUnmatched: true}, function (err, s) {
228
t.ifError(err, 'registry mocked successfully')
239
server = s
@@ -30,7 +16,7 @@ test('scoped package names not mangled on error with non-root registry', functio
3016
common.npm(
3117
[
3218
'--registry=' + common.registry,
33-
'--cache=' + testdir,
19+
'--cache=' + common.cache,
3420
'cache',
3521
'add',
3622
'@scope/foo@*',
@@ -43,14 +29,8 @@ test('scoped package names not mangled on error with non-root registry', functio
4329
t.match(stderr, /E404/, 'should notify the sort of error as a 404')
4430
t.match(stderr, /@scope(?:%2f|\/)foo/, 'should have package name in error')
4531
server.done()
32+
server.close()
4633
t.end()
4734
}
4835
)
4936
})
50-
51-
test('cleanup', function (t) {
52-
server.close()
53-
cleanup()
54-
t.pass('cleaned up')
55-
t.end()
56-
})

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

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,12 @@
11
var test = require('tap').test
22
var path = require('path')
3-
var mkdirp = require('mkdirp')
4-
var rimraf = require('rimraf')
53
var common = require('../common-tap.js')
64
var mr = common.fakeRegistry.compat
75
var server
86

97
var packageName = path.basename(__filename, '.js')
10-
var testdir = common.pkg
11-
12-
function setup () {
13-
cleanup()
14-
mkdirp.sync(testdir)
15-
}
16-
17-
function cleanup () {
18-
rimraf.sync(testdir)
19-
}
208

219
test('setup', function (t) {
22-
setup()
2310
mr({port: common.port, throwOnUnmatched: true}, function (err, s) {
2411
t.ifError(err, 'registry mocked successfully')
2512
server = s
@@ -32,7 +19,7 @@ test('package names not mangled on error with non-root registry', function (t) {
3219
common.npm(
3320
[
3421
'--registry=' + common.registry,
35-
'--cache=' + testdir,
22+
'--cache=' + common.cache,
3623
'cache',
3724
'add',
3825
packageName + '@*'
@@ -43,14 +30,8 @@ test('package names not mangled on error with non-root registry', function (t) {
4330
t.equal(code, 1, 'exited with error')
4431
t.match(stderr, packageName, 'should have package name in error')
4532
server.done()
33+
server.close()
4634
t.end()
4735
}
4836
)
4937
})
50-
51-
test('cleanup', function (t) {
52-
server.close()
53-
cleanup()
54-
t.pass('cleaned up')
55-
t.end()
56-
})

‎test/tap/404.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const invalidPackage = /Your package name is not valid, because[\s\S]+1\. name c
1111

1212
const basedir = common.pkg
1313
const testdir = path.join(basedir, 'testdir')
14-
const cachedir = path.join(basedir, 'cache')
14+
const cachedir = common.cache
1515
const globaldir = path.join(basedir, 'global')
1616
const tmpdir = path.join(basedir, 'tmp')
1717

‎test/tap/access.js‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,18 @@ const scoped = {
1919
}
2020

2121
test('setup', function (t) {
22-
mkdirp(pkg, function (er) {
23-
t.ifError(er, pkg + ' made successfully')
24-
25-
mr({port: common.port}, function (err, s) {
26-
t.ifError(err, 'registry mocked successfully')
27-
server = s
28-
29-
fs.writeFile(
30-
path.join(pkg, 'package.json'),
31-
JSON.stringify(scoped),
32-
function (er) {
33-
t.ifError(er, 'wrote package.json')
34-
t.end()
35-
}
36-
)
37-
})
22+
mr({port: common.port}, function (err, s) {
23+
t.ifError(err, 'registry mocked successfully')
24+
server = s
25+
26+
fs.writeFile(
27+
path.join(pkg, 'package.json'),
28+
JSON.stringify(scoped),
29+
function (er) {
30+
t.ifError(er, 'wrote package.json')
31+
t.end()
32+
}
33+
)
3834
})
3935
})
4036

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,30 @@ const common = require('../common-tap.js')
55
const getStream = require('get-stream')
66
const mkdirp = require('mkdirp')
77
const path = require('path')
8-
const rimraf = require('rimraf')
98
const Tacks = require('tacks')
109
const {test} = require('tap')
1110

1211
const {File} = Tacks
1312

1413
const _createCacheEntryStream = require('../../lib/search/all-package-metadata.js')._createCacheEntryStream
1514

16-
const PKG_DIR = common.pkg
17-
const CACHE_DIR = path.resolve(PKG_DIR, 'cache')
18-
15+
// this test uses a fresh cache for each test block
16+
// create them all in common.cache so that we can verify
17+
// them for root-owned files in sudotest
18+
let CACHE_DIR
19+
let cacheCounter = 1
20+
const chownr = require('chownr')
1921
function setup () {
22+
CACHE_DIR = common.cache + '/' + cacheCounter++
2023
mkdirp.sync(CACHE_DIR)
24+
fixOwner(CACHE_DIR)
2125
}
2226

23-
function cleanup () {
24-
rimraf.sync(PKG_DIR)
25-
}
27+
const fixOwner = (
28+
process.getuid && process.getuid() === 0 &&
29+
process.env.SUDO_UID && process.env.SUDO_GID
30+
) ? (path) => chownr.sync(path, +process.env.SUDO_UID, +process.env.SUDO_GID)
31+
: () => {}
2632

2733
test('createCacheEntryStream basic', t => {
2834
setup()
@@ -39,6 +45,7 @@ test('createCacheEntryStream basic', t => {
3945
}
4046
}))
4147
fixture.create(cachePath)
48+
fixOwner(cachePath)
4249
return _createCacheEntryStream(cachePath, {}).then(({
4350
updateStream: stream,
4451
updatedLatest: latest
@@ -53,7 +60,6 @@ test('createCacheEntryStream basic', t => {
5360
name: 'foo',
5461
version: '1.0.0'
5562
}])
56-
cleanup()
5763
})
5864
})
5965
})
@@ -63,12 +69,12 @@ test('createCacheEntryStream empty cache', t => {
6369
const cachePath = path.join(CACHE_DIR, '.cache.json')
6470
const fixture = new Tacks(File({}))
6571
fixture.create(cachePath)
72+
fixOwner(cachePath)
6673
return _createCacheEntryStream(cachePath, {}).then(
6774
() => { throw new Error('should not succeed') },
6875
err => {
6976
t.ok(err, 'returned an error because there was no _updated')
7077
t.match(err.message, /Empty or invalid stream/, 'useful error message')
71-
cleanup()
7278
}
7379
)
7480
})
@@ -80,6 +86,7 @@ test('createCacheEntryStream no entry cache', t => {
8086
'_updated': 1234
8187
}))
8288
fixture.create(cachePath)
89+
fixOwner(cachePath)
8390
return _createCacheEntryStream(cachePath, {}).then(({
8491
updateStream: stream,
8592
updatedLatest: latest
@@ -88,7 +95,6 @@ test('createCacheEntryStream no entry cache', t => {
8895
t.ok(stream, 'returned a stream')
8996
return getStream.array(stream).then(results => {
9097
t.deepEquals(results, [], 'no results')
91-
cleanup()
9298
})
9399
})
94100
})
@@ -101,7 +107,6 @@ test('createCacheEntryStream missing cache', t => {
101107
err => {
102108
t.ok(err, 'returned an error because there was no cache')
103109
t.equals(err.code, 'ENOENT', 'useful error message')
104-
cleanup()
105110
}
106111
)
107112
})

0 commit comments

Comments
 (0)