Skip to content

Commit

Permalink
fix(git): Do not drop uid/gid when executing in root-owned directory
Browse files Browse the repository at this point in the history
Fix: npm/cli#624
Fix: npm/cli#642
Fix: npm/cli#514

Infer the ownership of a git command invocation based on the cwd, if one is
specified.
  • Loading branch information
isaacs committed Dec 30, 2019
1 parent 2cf8f28 commit d2f4176
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 4 deletions.
23 changes: 19 additions & 4 deletions lib/util/git.js
Expand Up @@ -16,6 +16,7 @@ const promiseRetry = require('promise-retry')
const uniqueFilename = require('unique-filename')
const which = BB.promisify(require('which'))
const semver = require('semver')
const inferOwner = require('infer-owner')

const GOOD_ENV_VARS = new Set([
'GIT_ASKPASS',
Expand Down Expand Up @@ -181,10 +182,24 @@ function revs (repo, opts) {
})
}

// infer the owner from the cwd git is operating in, if not the
// process cwd, but only if we're root.
// See: https://github.com/npm/cli/issues/624
module.exports._cwdOwner = cwdOwner
function cwdOwner (gitOpts, opts) {
const isRoot = process.getuid && process.getuid() === 0
if (!isRoot || !gitOpts.cwd) { return Promise.resolve() }

return BB.resolve(inferOwner(gitOpts.cwd).then(owner => {
gitOpts.uid = owner.uid
gitOpts.gid = owner.gid
}))
}

module.exports._exec = execGit
function execGit (gitArgs, gitOpts, opts) {
opts = optCheck(opts)
return checkGit(opts).then(gitPath => {
return BB.resolve(cwdOwner(gitOpts, opts).then(() => checkGit(opts).then(gitPath => {
return promiseRetry((retry, number) => {
if (number !== 1) {
opts.log.silly('pacote', 'Retrying git command: ' + gitArgs.join(' ') + ' attempt # ' + number)
Expand All @@ -202,13 +217,13 @@ function execGit (gitArgs, gitOpts, opts) {
maxTimeout: opts['fetch-retry-maxtimeout'],
minTimeout: opts['fetch-retry-mintimeout']
})
})
})))
}

module.exports._spawn = spawnGit
function spawnGit (gitArgs, gitOpts, opts) {
opts = optCheck(opts)
return checkGit(opts).then(gitPath => {
return BB.resolve(cwdOwner(gitOpts, opts).then(() => checkGit(opts).then(gitPath => {
return promiseRetry((retry, number) => {
if (number !== 1) {
opts.log.silly('pacote', 'Retrying git command: ' + gitArgs.join(' ') + ' attempt # ' + number)
Expand All @@ -231,7 +246,7 @@ function spawnGit (gitArgs, gitOpts, opts) {
return stdout
})
}, opts.retry)
})
})))
}

module.exports._mkOpts = mkOpts
Expand Down
75 changes: 75 additions & 0 deletions test/git.cwd-owner.js
@@ -0,0 +1,75 @@
'use strict'
const t = require('tap')

process.getuid = () => 0
process.getgid = () => 0

const requireInject = require('require-inject')
const fs = require('fs')
const { _cwdOwner: cwdOwner } = requireInject('../lib/util/git.js', {
fs: Object.assign({}, fs, {
lstat: (path, cb) => {
if (path === '/var/root/.npm/_cacache/bleep/boop') {
return process.nextTick(() => cb(null, { uid: 0, gid: 0 }))
}
if (path === '/home/user/.npm/_cacache/bleep/boop') {
return process.nextTick(() => cb(null, { uid: 69, gid: 420 }))
}
fs.lstat(path, cb)
},
stat: (path, cb) => {
if (path === '/var/root/.npm/_cacache/bleep/boop') {
return process.nextTick(() => cb(null, { uid: 0, gid: 0 }))
}
if (path === '/home/user/.npm/_cacache/bleep/boop') {
return process.nextTick(() => cb(null, { uid: 69, gid: 420 }))
}
fs.stat(path, cb)
}
})
})

t.test('running in root-owned folder, but with non-root uid/gid opts', t => {
const gitOpts = {
cwd: '/var/root/.npm/_cacache/bleep/boop'
}
const opts = {
uid: 69,
gid: 420
}
return cwdOwner(gitOpts, opts).then(() => {
t.strictSame(gitOpts, {
cwd: '/var/root/.npm/_cacache/bleep/boop',
uid: 0,
gid: 0
})
})
})

t.test('running in non-root-owned folder', t => {
const gitOpts = {
cwd: '/home/user/.npm/_cacache/bleep/boop'
}
const opts = {
uid: 12,
gid: 34
}
return cwdOwner(gitOpts, opts).then(() => {
t.strictSame(gitOpts, {
cwd: '/home/user/.npm/_cacache/bleep/boop',
uid: 69,
gid: 420
})
})
})

t.test('running without a cwd', t => {
const gitOpts = {}
const opts = {
uid: 12,
gid: 34
}
return cwdOwner(gitOpts, opts).then(() => {
t.strictSame(gitOpts, {})
})
})

0 comments on commit d2f4176

Please sign in to comment.