From f12e7ef632b9c744fef1a09ed9fc5586f8f5d766 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sun, 11 Aug 2019 21:58:55 -0700 Subject: [PATCH] fix: Infer owner of all unpacked files Fix: https://github.com/npm/pacote/issues/2 Infer the uid/gid setttings for unpacked files and mkdir'ed directories from the ownership of the parent directory into which files are being unpacked. This prevents user-owned files from ending up in a root-owned location (as happens when someone runs `sudo npm i -g ...`) and root-owned files in a user-owned location (as happens when someone runs `sudo npm i ...`). The argument can be made that when running a program as root, the user *shouldn't* be surprised to find root-owned files were created in a "non-hidden" folder like `node_modules`, even if putting root-owned files in the `~/.npm` cache is inappropriate. However: - In practice, this is indeed unexpected to a huge number of users. Since they often don't realize at first that npm installs local deps locally in the project, they may think that they need to use `sudo` simply because other package managers require it. - `node_modules` is, for most npm users, just as "hidden" as `~/.npm`. --- extract.js | 69 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/extract.js b/extract.js index f49a5442..c4f7b20b 100644 --- a/extract.js +++ b/extract.js @@ -10,41 +10,54 @@ const optCheck = require('./lib/util/opt-check.js') const path = require('path') const rimraf = BB.promisify(require('rimraf')) const withTarballStream = require('./lib/with-tarball-stream.js') +const inferOwner = require('infer-owner') +const chown = BB.promisify(fs.chown) const truncateAsync = BB.promisify(fs.truncate) const readFileAsync = BB.promisify(fs.readFile) const appendFileAsync = BB.promisify(fs.appendFile) +// you used to call me on my... +const selfOwner = process.getuid ? { + uid: process.getuid(), + gid: process.getgid() +} : { + uid: undefined, + gid: undefined +} + module.exports = extract function extract (spec, dest, opts) { opts = optCheck(opts) spec = npa(spec, opts.where) const startTime = Date.now() - - return withTarballStream(spec, opts, stream => { - return tryExtract(spec, stream, dest, opts) - }) - .then(() => { - if (!opts.resolved) { - const pjson = path.join(dest, 'package.json') - return readFileAsync(pjson, 'utf8') - .then(str => truncateAsync(pjson) - .then(() => appendFileAsync(pjson, str.replace( - /}\s*$/, - `\n,"_resolved": ${ - JSON.stringify(opts.resolved || '') - }\n,"_integrity": ${ - JSON.stringify(opts.integrity || '') - }\n,"_from": ${ - JSON.stringify(spec.toString()) - }\n}` - )))) - } + return inferOwner(dest).then(({ uid, gid }) => { + opts = opts.concat({ uid, gid }) + return withTarballStream(spec, opts, stream => { + return tryExtract(spec, stream, dest, opts) }) - .then(() => opts.log.silly( - 'extract', - `${spec} extracted to ${dest} (${Date.now() - startTime}ms)` - )) + .then(() => { + if (!opts.resolved) { + const pjson = path.join(dest, 'package.json') + return readFileAsync(pjson, 'utf8') + .then(str => truncateAsync(pjson) + .then(() => appendFileAsync(pjson, str.replace( + /}\s*$/, + `\n,"_resolved": ${ + JSON.stringify(opts.resolved || '') + }\n,"_integrity": ${ + JSON.stringify(opts.integrity || '') + }\n,"_from": ${ + JSON.stringify(spec.toString()) + }\n}` + )))) + } + }) + .then(() => opts.log.silly( + 'extract', + `${spec} extracted to ${dest} (${Date.now() - startTime}ms)` + )) + }) } function tryExtract (spec, tarStream, dest, opts) { @@ -53,6 +66,14 @@ function tryExtract (spec, tarStream, dest, opts) { rimraf(dest) .then(() => mkdirp(dest)) + .then(() => { + // respect the current ownership of unpack targets + if (typeof selfOwner.uid === 'number' && + typeof selfOwner.gid === 'number' && + selfOwner.uid !== opts.uid && selfOwner.gid !== opts.gid) { + return chown(dest, opts.uid, opts.gid) + } + }) .then(() => { const xtractor = extractStream(spec, dest, opts) xtractor.on('error', reject)