Add uid/gid options to extract and unpack

isaacs · isaacs · commit cb297ca16c14 · 2017-08-16T17:55:03.000-07:00
Enables forcibly setting the ownership of all extracted items, regardless of the uid/gid of the process or the value in the archive. This also avoids calling chown on files and directories if they are already going to be owned by the proper uid/gid (that is, if the specified uid/gid matches the process uid and gid). Note that it does _not_ set the ownership of pre-existing folders that are already in place when the extract is performed. It only guarantees that files and directories created by the unpack will have their ownership set as specified. Thus it provides a reliable gaurantee of file ownership only if extracting into a previously empty directory. Fix #133
diff --git a/README.md b/README.md
@@ -295,6 +295,16 @@ The following options are supported:
   `tar(1)`, but ACLs and other system-specific data is never unpacked
   in this implementation, and modes are set by default already.
   [Alias: `p`]
+- `uid` Set to a number to force ownership of all extracted files and
+  folders, and all implicitly created directories, to be owned by the
+  specified user id, regardless of the `uid` field in the archive.
+  Cannot be used along with `preserveOwner`.  Requires also setting a
+  `gid` option.
+- `gid` Set to a number to force ownership of all extracted files and
+  folders, and all implicitly created directories, to be owned by the
+  specified group id, regardless of the `gid` field in the archive.
+  Cannot be used along with `preserveOwner`.  Requires also setting a
+  `uid` option.
 
 The following options are mostly internal, but can be modified in some
 advanced use cases, such as re-using caches between runs.
@@ -554,6 +564,16 @@ mode.
 - `win32` True if on a windows platform.  Causes behavior where
   filenames containing `<|>?` chars are converted to
   windows-compatible values while being unpacked.
+- `uid` Set to a number to force ownership of all extracted files and
+  folders, and all implicitly created directories, to be owned by the
+  specified user id, regardless of the `uid` field in the archive.
+  Cannot be used along with `preserveOwner`.  Requires also setting a
+  `gid` option.
+- `gid` Set to a number to force ownership of all extracted files and
+  folders, and all implicitly created directories, to be owned by the
+  specified group id, regardless of the `gid` field in the archive.
+  Cannot be used along with `preserveOwner`.  Requires also setting a
+  `uid` option.
 
 ### class tar.Unpack.Sync
 
diff --git a/lib/mkdir.js b/lib/mkdir.js
@@ -3,6 +3,7 @@
 const mkdirp = require('mkdirp')
 const fs = require('fs')
 const path = require('path')
+const chownr = require('chownr')
 
 class SymlinkError extends Error {
   constructor (symlink, path) {
@@ -23,17 +24,25 @@ const mkdir = module.exports = (dir, opt, cb) => {
   const mode = opt.mode | 0o0700
   const needChmod = (mode & umask) !== 0
 
+  const uid = opt.uid
+  const gid = opt.gid
+  const doChown = typeof uid === 'number' &&
+    typeof gid === 'number' &&
+    ( uid !== opt.processUid || gid !== opt.processGid )
+
   const preserve = opt.preserve
   const unlink = opt.unlink
   const cache = opt.cache
   const cwd = opt.cwd
 
-  const done = er => {
+  const done = (er, created) => {
     if (er)
       cb(er)
     else {
       cache.set(dir, true)
-      if (needChmod)
+      if (created && doChown)
+        chownr(created, uid, gid, er => done(er))
+      else if (needChmod)
         fs.chmod(dir, mode, cb)
       else
         cb()
@@ -48,39 +57,41 @@ const mkdir = module.exports = (dir, opt, cb) => {
 
   const sub = path.relative(cwd, dir)
   const parts = sub.split(/\/|\\/)
-  mkdir_(cwd, parts, mode, cache, unlink, done)
+  mkdir_(cwd, parts, mode, cache, unlink, null, done)
 }
 
-const mkdir_ = (base, parts, mode, cache, unlink, cb) => {
+const mkdir_ = (base, parts, mode, cache, unlink, created, cb) => {
   if (!parts.length)
-    return cb()
+    return cb(null, created)
   const p = parts.shift()
   const part = base + '/' + p
   if (cache.get(part))
-    return mkdir_(part, parts, mode, cache, unlink, cb)
-  fs.mkdir(part, mode, onmkdir(part, parts, mode, cache, unlink, cb))
+    return mkdir_(part, parts, mode, cache, unlink, created, cb)
+  fs.mkdir(part, mode, onmkdir(part, parts, mode, cache, unlink, created, cb))
 }
 
-const onmkdir = (part, parts, mode, cache, unlink, cb) => er => {
+const onmkdir = (part, parts, mode, cache, unlink, created, cb) => er => {
   if (er) {
     fs.lstat(part, (statEr, st) => {
       if (statEr)
         cb(statEr)
       else if (st.isDirectory())
-        mkdir_(part, parts, mode, cache, unlink, cb)
+        mkdir_(part, parts, mode, cache, unlink, created, cb)
       else if (unlink)
         fs.unlink(part, er => {
           if (er)
             return cb(er)
-          fs.mkdir(part, mode, onmkdir(part, parts, mode, cache, unlink, cb))
+          fs.mkdir(part, mode, onmkdir(part, parts, mode, cache, unlink, created, cb))
         })
       else if (st.isSymbolicLink())
         return cb(new SymlinkError(part, part + '/' + parts.join('/')))
       else
         cb(er)
     })
-  } else
-    mkdir_(part, parts, mode, cache, unlink, cb)
+  } else {
+    created = created || part
+    mkdir_(part, parts, mode, cache, unlink, created, cb)
+  }
 }
 
 const mkdirSync = module.exports.sync = (dir, opt) => {
@@ -90,13 +101,21 @@ const mkdirSync = module.exports.sync = (dir, opt) => {
   const mode = opt.mode | 0o0700
   const needChmod = (mode & umask) !== 0
 
+  const uid = opt.uid
+  const gid = opt.gid
+  const doChown = typeof uid === 'number' &&
+    typeof gid === 'number' &&
+    ( uid !== opt.processUid || gid !== opt.processGid )
+
   const preserve = opt.preserve
   const unlink = opt.unlink
   const cache = opt.cache
   const cwd = opt.cwd
 
-  const done = er => {
+  const done = (created) => {
     cache.set(dir, true)
+    if (created && doChown)
+      chownr.sync(created, uid, gid)
     if (needChmod)
       fs.chmodSync(dir, mode)
     cache.set(dir, true)
@@ -105,14 +124,12 @@ const mkdirSync = module.exports.sync = (dir, opt) => {
   if (cache && cache.get(dir) === true || dir === cwd)
     return done()
 
-  if (preserve) {
-    mkdirp.sync(dir, mode)
-    cache.set(dir, true)
-    return
-  }
+  if (preserve)
+    return done(mkdirp.sync(dir, mode))
 
   const sub = path.relative(cwd, dir)
   const parts = sub.split(/\/|\\/)
+  let created = null
   for (let p = parts.shift(), part = cwd;
        p && (part += '/' + p);
        p = parts.shift()) {
@@ -122,6 +139,7 @@ const mkdirSync = module.exports.sync = (dir, opt) => {
 
     try {
       fs.mkdirSync(part, mode)
+      created = created || part
       cache.set(part, true)
     } catch (er) {
       const st = fs.lstatSync(part)
@@ -131,12 +149,13 @@ const mkdirSync = module.exports.sync = (dir, opt) => {
       } else if (unlink) {
         fs.unlinkSync(part)
         fs.mkdirSync(part, mode)
+        created = created || part
         cache.set(part, true)
         continue
       } else if (st.isSymbolicLink())
         return new SymlinkError(part, part + '/' + parts.join('/'))
     }
   }
 
-  return done()
+  return done(created)
 }
diff --git a/lib/unpack.js b/lib/unpack.js
@@ -29,6 +29,8 @@ const ENDED = Symbol('ended')
 const MAYBECLOSE = Symbol('maybeClose')
 const SKIP = Symbol('skip')
 const DOCHOWN = Symbol('doChown')
+const UID = Symbol('uid')
+const GID = Symbol('gid')
 
 class Unpack extends Parser {
   constructor (opt) {
@@ -50,14 +52,31 @@ class Unpack extends Parser {
 
     this.dirCache = opt.dirCache || new Map()
 
-    if (opt.preserveOwner === undefined)
+    if (typeof opt.uid === 'number' || typeof opt.gid === 'number') {
+      // need both or neither
+      if (typeof opt.uid !== 'number' || typeof opt.gid !== 'number')
+        throw new TypeError('cannot set owner without number uid and gid')
+      if (opt.preserveOwner)
+        throw new TypeError(
+          'cannot preserve owner in archive and also set owner explicitly')
+      this.uid = opt.uid
+      this.gid = opt.gid
+      this.setOwner = true
+    } else {
+      this.uid = null
+      this.gid = null
+      this.setOwner = false
+    }
+
+    // default true for root
+    if (opt.preserveOwner === undefined && typeof opt.uid !== 'number')
       this.preserveOwner = process.getuid && process.getuid() === 0
     else
       this.preserveOwner = !!opt.preserveOwner
 
-    this.processUid = this.preserveOwner && process.getuid ?
+    this.processUid = (this.preserveOwner || this.setOwner) && process.getuid ?
       process.getuid() : null
-    this.processGid = this.preserveOwner && process.getgid ?
+    this.processGid = (this.preserveOwner || this.setOwner) && process.getgid ?
       process.getgid() : null
 
     // turn ><?| in filenames into 0xf000-higher encoded forms
@@ -173,6 +192,10 @@ class Unpack extends Parser {
 
   [MKDIR] (dir, mode, cb) {
     mkdir(dir, {
+      uid: this.uid,
+      gid: this.gid,
+      processUid: this.processUid,
+      processGid: this.processGid,
       umask: this.processUmask,
       preserve: this.preservePaths,
       unlink: this.unlink,
@@ -183,9 +206,26 @@ class Unpack extends Parser {
   }
 
   [DOCHOWN] (entry) {
+    // in preserve owner mode, chown if the entry doesn't match process
+    // in set owner mode, chown if setting doesn't match process
     return this.preserveOwner &&
       ( typeof entry.uid === 'number' && entry.uid !== this.processUid ||
         typeof entry.gid === 'number' && entry.gid !== this.processGid )
+      ||
+      ( typeof this.uid === 'number' && this.uid !== this.processUid ||
+        typeof this.gid === 'number' && this.gid !== this.processGid )
+  }
+
+  [UID] (entry) {
+    return typeof this.uid === 'number' ? this.uid
+      : typeof entry.uid === 'number' ? entry.uid
+      : this.processUid
+  }
+
+  [GID] (entry) {
+    return typeof this.gid === 'number' ? this.gid
+      : typeof entry.gid === 'number' ? entry.gid
+      : this.processGid
   }
 
   [FILE] (entry) {
@@ -208,7 +248,7 @@ class Unpack extends Parser {
           fs.utimes(entry.absolute, entry.atime || new Date(), entry.mtime, cb))
       if (this[DOCHOWN](entry))
         queue.push(cb =>
-          fs.chown(entry.absolute, entry.uid, entry.gid || this.processGid, cb))
+          fs.chown(entry.absolute, this[UID](entry), this[GID](entry), cb))
       processQueue()
     })
     entry.pipe(stream)
@@ -236,7 +276,7 @@ class Unpack extends Parser {
           fs.utimes(entry.absolute, entry.atime || new Date(), entry.mtime, cb))
       if (this[DOCHOWN](entry))
         queue.push(cb =>
-          fs.chown(entry.absolute, entry.uid, entry.gid || this.processGid, cb))
+          fs.chown(entry.absolute, this[UID](entry), this[GID](entry), cb))
 
       processQueue()
     })
@@ -375,7 +415,7 @@ class UnpackSync extends Unpack {
         }
         if (this[DOCHOWN](entry)) {
           try {
-            fs.fchownSync(fd, entry.uid, entry.gid || this.processGid)
+            fs.fchownSync(fd, this[UID](entry), this[GID](entry))
           } catch (er) {}
         }
         try { fs.closeSync(fd) } catch (er) { this[ONERROR](er, entry) }
@@ -395,7 +435,7 @@ class UnpackSync extends Unpack {
     }
     if (this[DOCHOWN](entry)) {
       try {
-        fs.chownSync(entry.absolute, entry.uid, entry.gid || this.processGid)
+        fs.chownSync(entry.absolute, this[UID](entry), this[GID](entry))
       } catch (er) {}
     }
     entry.resume()
@@ -404,6 +444,10 @@ class UnpackSync extends Unpack {
   [MKDIR] (dir, mode) {
     try {
       return mkdir.sync(dir, {
+        uid: this.uid,
+        gid: this.gid,
+        processUid: this.processUid,
+        processGid: this.processGid,
         umask: this.processUmask,
         preserve: this.preservePaths,
         unlink: this.unlink,
diff --git a/test/unpack.js b/test/unpack.js

-Original file line number
+Diff line change
       size: 3
     },
     'qux',
 +    {
 +      gid: 5108675309,
 +      path: 'foo/different-gid-nouid/bar',
 +      type: 'File',
 +      size: 3
 +    },
 +    'qux',
+    {
       uid: myUid,
       gid: myGid,
       called = 0
       const u = new Unpack.Sync({ cwd: dir, preserveOwner: true })
       u.end(data)
 -      t.equal(called, 4, 'called chowns')
 +      t.equal(called, 5, 'called chowns')
       t.end()
     })
       const u = new Unpack({ cwd: dir, preserveOwner: true })
       u.end(data)
       u.on('close', _ => {
 -        t.equal(called, 4, 'called chowns')
 +        t.equal(called, 5, 'called chowns')
         t.end()
       })
     })
     check(t)
   })
 })
++
 +t.test('chown implicit dirs and also the entries', t => {
 +  const basedir = path.resolve(unpackdir, 'chownr')
++
 +  // club these so that the test can run as non-root
 +  const chown = fs.chown
 +  const chownSync = fs.chownSync
++
 +  const getuid = process.getuid
 +  const getgid = process.getgid
 +  t.teardown(_ => {
 +    fs.chown = chown
 +    fs.chownSync = chownSync
 +    process.getgid = getgid
 +  })
++
 +  let chowns = 0
++
 +  let currentTest = null
 +  fs.chown = (path, uid, gid, cb) => {
 +    currentTest.equal(uid, 420, 'chown(' + path + ') uid')
 +    currentTest.equal(gid, 666, 'chown(' + path + ') gid')
 +    chowns ++
 +    cb()
 +  }
++
 +  fs.chownSync = (path, uid, gid) => {
 +    currentTest.equal(uid, 420, 'chownSync(' + path + ') uid')
 +    currentTest.equal(gid, 666, 'chownSync(' + path + ') gid')
 +    chowns ++
 +  }
++
 +  fs.fchownSync = (path, uid, gid) => {
 +    currentTest.equal(uid, 420, 'chownSync(' + path + ') uid')
 +    currentTest.equal(gid, 666, 'chownSync(' + path + ') gid')
 +    chowns ++
 +  }
++
 +  const data = makeTar([
 +    {
 +      path: 'a/b/c',
 +      mode: 0o775,
 +      type: 'File',
 +      size: 1,
 +      uid: null,
 +      gid: null
 +    },
 +    '.',
 +    {
 +      path: 'x/y/z',
 +      mode: 0o775,
 +      uid: 12345,
 +      gid: 54321,
 +      type: 'File',
 +      size: 1
 +    },
 +    '.',
 +    '',
 +    ''
 +  ])
++
 +  const check = t => {
 +    currentTest = null
 +    t.equal(chowns, 6)
 +    chowns = 0
 +    rimraf.sync(basedir)
 +    t.end()
 +  }
++
 +  t.test('throws when setting uid/gid improperly', t => {
 +    t.throws(_ => new Unpack({ uid: 420 }),
 +      TypeError('cannot set owner without number uid and gid'))
 +    t.throws(_ => new Unpack({ gid: 666 }),
 +      TypeError('cannot set owner without number uid and gid'))
 +    t.throws(_ => new Unpack({ uid: 1, gid: 2, preserveOwner: true }),
 +      TypeError('cannot preserve owner in archive and also set owner explicitly'))
 +    t.end()
 +  })
++
 +  const tests = () =>
 +    t.test('async', t => {
 +      currentTest = t
 +      mkdirp.sync(basedir)
 +      const unpack = new Unpack({ cwd: basedir, uid: 420, gid: 666 })
 +      unpack.on('close', _ => check(t))
 +      unpack.end(data)
 +    }).then(t.test('sync', t => {
 +      currentTest = t
 +      mkdirp.sync(basedir)
 +      const unpack = new Unpack.Sync({ cwd: basedir, uid: 420, gid: 666 })
 +      unpack.end(data)
 +      check(t)
 +    }))
++
 +  tests()
++
 +  t.test('make it look like processUid is 420', t => {
 +    process.getuid = () => 420
 +    t.end()
 +  })
++
 +  tests()
++
 +  t.test('make it look like processGid is 666', t => {
 +    process.getuid = getuid
 +    process.getgid = () => 666
 +    t.end()
 +  })
++
 +  return tests()
 +})