Skip to content

Commit

Permalink
fix: Infer owner of all unpacked files
Browse files Browse the repository at this point in the history
Fix: #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`.
  • Loading branch information
isaacs committed Aug 12, 2019
1 parent 0a0c73c commit f12e7ef
Showing 1 changed file with 45 additions and 24 deletions.
69 changes: 45 additions & 24 deletions extract.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down

0 comments on commit f12e7ef

Please sign in to comment.