From 2c61ce65d6b67100fdf3fcb9729055b669cb1a1d Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 15 Jul 2019 16:19:01 -0700 Subject: [PATCH] correct-mkdir: infer cache owner from parent dir When running as sudo (without the -H flag), the HOME environment variable is /var/root by default. The correct-mkdir module was reading env.HOME to determine the appropriate uid and gid for files written to the cache folder. However, this is rarely correct. In practice, this module was reading env.SUDO_UID and env.SUDO_GID in some cases, but matching ownership on /var/root in others. This patch changes correct-mkdir to match the behavior of cacache v12, where ownership of a file or folder is inferred from the ownership of its nearest parent directory. --- lib/utils/correct-mkdir.js | 146 +++++++++++-------------------------- test/tap/correct-mkdir.js | 8 +- 2 files changed, 48 insertions(+), 106 deletions(-) diff --git a/lib/utils/correct-mkdir.js b/lib/utils/correct-mkdir.js index 68c4a4ad79ae2..0c92201128080 100644 --- a/lib/utils/correct-mkdir.js +++ b/lib/utils/correct-mkdir.js @@ -1,123 +1,65 @@ -var chownr = require('chownr') -var dezalgo = require('dezalgo') -var fs = require('graceful-fs') -var inflight = require('inflight') -var log = require('npmlog') -var mkdirp = require('mkdirp') +const chownr = require('chownr') +const fs = require('graceful-fs') +const inflight = require('inflight') +const log = require('npmlog') +const mkdirp = require('mkdirp') +const { dirname } = require('path') + +// retain ownership of the parent dir +// this matches behavior in cacache to infer the cache ownership +// based on the ownership of the cache folder or it is parent. -// memoize the directories created by this step -var stats = {} -var effectiveOwner module.exports = function correctMkdir (path, cb) { - cb = dezalgo(cb) - cb = inflight('correctMkdir:' + path, cb) + cb = inflight('correctMkdir: ' + path, cb) if (!cb) { return log.verbose('correctMkdir', path, 'correctMkdir already in flight; waiting') } else { log.verbose('correctMkdir', path, 'correctMkdir not in flight; initializing') } - if (stats[path]) return cb(null, stats[path]) - - fs.stat(path, function (er, st) { - if (er) return makeDirectory(path, cb) + if (!process.getuid) { + log.verbose('makeCacheDir', 'UID & GID are irrelevant on', process.platform) + return mkdirp(path, (er, made) => cb(er, { uid: 0, gid: 0 })) + } - if (!st.isDirectory()) { - log.error('correctMkdir', 'invalid dir %s', path) + inferOwner(path, (er, owner) => { + if (er) { + log.error('correctMkdir', 'failed to infer path ownership %s', path) return cb(er) } - var ownerStats = calculateOwner() - // there's always a chance the permissions could have been frobbed, so fix - if (st.uid !== ownerStats.uid) { - stats[path] = ownerStats - setPermissions(path, ownerStats, cb) - } else { - stats[path] = st - cb(null, stats[path]) - } - }) -} - -function calculateOwner () { - if (!effectiveOwner) { - effectiveOwner = { uid: 0, gid: 0 } - - // Pretty much only on windows - if (!process.getuid) { - return effectiveOwner - } - - effectiveOwner.uid = +process.getuid() - effectiveOwner.gid = +process.getgid() - - if (effectiveOwner.uid === 0) { - if (process.env.SUDO_UID) effectiveOwner.uid = +process.env.SUDO_UID - if (process.env.SUDO_GID) effectiveOwner.gid = +process.env.SUDO_GID - } - } - - return effectiveOwner -} - -function makeDirectory (path, cb) { - cb = inflight('makeDirectory:' + path, cb) - if (!cb) { - return log.verbose('makeDirectory', path, 'creation already in flight; waiting') - } else { - log.verbose('makeDirectory', path, 'creation not in flight; initializing') - } - - var owner = calculateOwner() - - if (!process.getuid) { - return mkdirp(path, function (er) { - log.verbose('makeCacheDir', 'UID & GID are irrelevant on', process.platform) - - stats[path] = owner - return cb(er, stats[path]) - }) - } - - if (owner.uid !== 0 || !process.env.HOME) { - log.silly( - 'makeDirectory', path, - 'uid:', owner.uid, - 'gid:', owner.gid - ) - stats[path] = owner - mkdirp(path, afterMkdir) - } else { - fs.stat(process.env.HOME, function (er, st) { + mkdirp(path, (er, made) => { if (er) { - log.error('makeDirectory', 'homeless?') + log.error('correctMkdir', 'failed to make directory %s', path) return cb(er) } - - log.silly( - 'makeDirectory', path, - 'uid:', st.uid, - 'gid:', st.gid - ) - stats[path] = st - mkdirp(path, afterMkdir) + chownr(made || path, owner.uid, owner.gid, (er) => cb(er, owner)) }) - } - - function afterMkdir (er, made) { - if (er || !stats[path] || isNaN(stats[path].uid) || isNaN(stats[path].gid)) { - return cb(er, stats[path]) - } - - if (!made) return cb(er, stats[path]) + }) +} - setPermissions(made, stats[path], cb) +const pathToOwner = new Map() +const inferOwner = (path, cb) => { + if (pathToOwner.has(path)) { + return cb(null, pathToOwner.get(path)) } -} -function setPermissions (path, st, cb) { - chownr(path, st.uid, st.gid, function (er) { - if (er && er.code === 'ENOENT') return cb(null, st) - return cb(er, st) + const parent = dirname(path) + fs.lstat(path, (er, st) => { + if (er && parent !== path) { + inferOwner(parent, (er, owner) => { + if (er) { + return cb(er) + } + pathToOwner.set(path, owner) + return cb(null, owner) + }) + } else if (er) { + cb(er) + } else { + const owner = { uid: st.uid, gid: st.gid } + pathToOwner.set(path, owner) + cb(null, owner) + } }) } diff --git a/test/tap/correct-mkdir.js b/test/tap/correct-mkdir.js index c69f8b00b55d5..a03448a6ac9fc 100644 --- a/test/tap/correct-mkdir.js +++ b/test/tap/correct-mkdir.js @@ -8,7 +8,7 @@ var cache_dir = common.pkg test('correct-mkdir: no race conditions', function (t) { var mock_fs = {} var did_hook = false - mock_fs.stat = function (path, cb) { + mock_fs.lstat = function (path, cb) { if (path === cache_dir) { // Return a non-matching owner cb(null, { @@ -60,7 +60,7 @@ test('correct-mkdir: no race conditions', function (t) { test('correct-mkdir: ignore ENOENTs from chownr', function (t) { var mock_fs = {} - mock_fs.stat = function (path, cb) { + mock_fs.lstat = function (path, cb) { if (path === cache_dir) { cb(null, { isDirectory: function () { @@ -99,7 +99,7 @@ test('correct-mkdir: SUDO_UID and SUDO_GID non-Windows', function (t) { process.getuid = function () { return 0 } process.getgid = function () { return 0 } var mock_fs = {} - mock_fs.stat = function (path, cb) { + mock_fs.lstat = function (path, cb) { if (path === cache_dir) { cb(null, { uid: 0, @@ -134,7 +134,7 @@ test('correct-mkdir: SUDO_UID and SUDO_GID Windows', function (t) { delete process.getuid delete process.getgid var mock_fs = {} - mock_fs.stat = function (path, cb) { + mock_fs.lstat = function (path, cb) { if (path === cache_dir) { cb(null, { uid: 0,