Skip to content

Commit

Permalink
ci: pass appropriate configs for file/dir modes
Browse files Browse the repository at this point in the history
Re: https://npm.community/t/6-11-2-npm-ci-installs-package-with-wrong-permissions/9720

Still passing a plain old (non-Figgy Pudding) object into libcipm,
duplicating the extra keys added in figgy-config.js.

This is not a clean or nice or elegant solution, but it works, without
regressing the config env var issue.

Pairing with @claudiahdz

PR-URL: #243
Credit: @isaacs
Close: #243
Reviewed-by: @claudiahdz
  • Loading branch information
isaacs committed Aug 31, 2019
1 parent 23ce656 commit cebf542
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 4 deletions.
26 changes: 23 additions & 3 deletions lib/ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,42 @@

const npm = require('./npm.js')
const Installer = require('libcipm')
const npmlog = require('npmlog')
const log = require('npmlog')
const path = require('path')

ci.usage = 'npm ci'

ci.completion = (cb) => cb(null, [])

module.exports = ci
function ci (args, cb) {
const opts = Object.create({ log: npmlog })
const opts = {
// Add some non-npm-config opts by hand.
cache: path.join(npm.config.get('cache'), '_cacache'),
// NOTE: npm has some magic logic around color distinct from the config
// value, so we have to override it here
color: !!npm.color,
hashAlgorithm: 'sha1',
includeDeprecated: false,
log,
'npm-session': npm.session,
'project-scope': npm.projectScope,
refer: npm.referer,
dmode: npm.modes.exec,
fmode: npm.modes.file,
umask: npm.modes.umask,
npmVersion: npm.version,
tmp: npm.tmp
}

for (const key in npm.config.list[0]) {
if (key !== 'log') {
opts[key] = npm.config.list[0][key]
}
}

return new Installer(opts).run().then(details => {
npmlog.disableProgress()
log.disableProgress()
console.log(`added ${details.pkgCount} packages in ${
details.runTime / 1000
}s`)
Expand Down
2 changes: 1 addition & 1 deletion lib/config/figgy-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const npm = require('../npm.js')
const pack = require('../pack.js')
const path = require('path')

const npmSession = crypto.randomBytes(8).toString('hex')
const npmSession = npm.session = crypto.randomBytes(8).toString('hex')
log.verbose('npm-session', npmSession)

const SCOPE_REGISTRY_REGEX = /@.*:registry$/gi
Expand Down
53 changes: 53 additions & 0 deletions test/tap/ci-permissions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const t = require('tap')
const tar = require('tar')
const common = require('../common-tap.js')
const pkg = common.pkg
const rimraf = require('rimraf')
const { writeFileSync, statSync, chmodSync } = require('fs')
const { resolve } = require('path')
const mkdirp = require('mkdirp')

t.test('setup', t => {
mkdirp.sync(resolve(pkg, 'package'))
const pj = resolve(pkg, 'package', 'package.json')
writeFileSync(pj, JSON.stringify({
name: 'foo',
version: '1.2.3'
}))
chmodSync(pj, 0o640)
tar.c({
sync: true,
file: resolve(pkg, 'foo.tgz'),
gzip: true,
cwd: pkg
}, ['package'])
writeFileSync(resolve(pkg, 'package.json'), JSON.stringify({
name: 'root',
version: '1.2.3',
dependencies: {
foo: 'file:foo.tgz'
}
}))
t.end()
})

t.test('run install to generate package-lock', t =>
common.npm(['install'], { cwd: pkg }).then(([code]) => t.equal(code, 0)))

t.test('remove node_modules', t => rimraf(resolve(pkg, 'node_modules'), t.end))

t.test('run ci and check modes', t =>
common.npm(['ci'], { cwd: pkg, stdio: 'inherit' }).then(([code]) => {
t.equal(code, 0)
const file = resolve(pkg, 'node_modules', 'foo', 'package.json')
// bitwise AND against 0o705 so that we can detect whether
// the file is world-readable.
// Typical unix systems would leave the file 0o644
// Travis-ci and some other Linux systems will be 0o664
// Windows is 0o666
// The regression this is detecting (ie, the default in the tarball)
// leaves the file as 0o640.
// Bitwise-AND 0o705 should always result in 0o604, and never 0o600
const mode = statSync(file).mode & 0o705
t.equal(mode, 0o604)
}))

0 comments on commit cebf542

Please sign in to comment.