Set more portable 'mode' value in portable mode

Fix #235

The `mode` value in `portable` mode will be set to a reasonable default
across Unix systems.

First, the user must always have read and write permissions.  (Ie, a
mode of 0o044 becomes 0o644.)

Next, the mode is masked against 0o022.  (Ie, a file of 0o777 becomes
0o755.  0o666 becomes 0o644.)

In practice, this means that all entries will usually have a mode of
either 0644 or 0o755, depending on whether the executable bit was set.

This avoids a situation where an archive created on a system with a
umask of 0o2 creates an archive with files having modes 0o775 and 0o664,
whereas an archive created on a system with a umask of 0o22 creates an
archive with files having modes 0o755 and 0o644, for the same content.
(Especially, for a check-out of the same git repository.)  Without this
consistency, it's impossible to honor the contract that the same content
will produce the same archive, which is necessary for npm to create
reprodicible build artifacts that match both in CI and development.

This unfortunately still does not address windows, where the executable
bit is never set on files.  (This bit the mocha project when a publish
from a Windows machine did not archive binaries with executable flags
set: #210 (comment))

README.md

@@ -284,8 +284,9 @@ The following options are supported:
   or `false` to omit it.
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `preservePaths` Allow absolute paths.  By default, `/` is stripped
   from absolute paths. [Alias: `P`]
 - `mode` The mode to set on the created file archive
@@ -484,8 +485,9 @@ The following options are supported:
   or `false` to omit it.
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `preservePaths` Allow absolute paths.  By default, `/` is stripped
   from absolute paths. [Alias: `P`]
 - `maxReadSize` The maximum buffer size for `fs.read()` operations.
@@ -535,8 +537,9 @@ The following options are supported:
   or `false` to omit it.
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `preservePaths` Allow absolute paths.  By default, `/` is stripped
   from absolute paths. [Alias: `P`]
 - `maxReadSize` The maximum buffer size for `fs.read()` operations.
@@ -582,8 +585,9 @@ The following options are supported:
   or `false` to omit it.
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `preservePaths` Allow absolute paths.  By default, `/` is stripped
   from absolute paths.
 - `linkCache` A Map object containing the device and inode value for
@@ -795,8 +799,9 @@ It has the following fields:
   object.
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `myuid` If supported, the uid of the user running the current
   process.
 - `myuser` The `env.USER` string if set, or `''`.  Set as the entry
@@ -834,8 +839,9 @@ The following options are supported:
 
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `maxReadSize` The maximum buffer size for `fs.read()` operations.
   Defaults to 1 MB.
 - `linkCache` A Map object containing the device and inode value for
@@ -883,8 +889,9 @@ The following options are supported:
 
 - `portable` Omit metadata that is system-specific: `ctime`, `atime`,
   `uid`, `gid`, `uname`, `gname`, `dev`, `ino`, and `nlink`.  Note
-  that `mtime` is still included, because this is necessary other
-  time-based operations.
+  that `mtime` is still included, because this is necessary for other
+  time-based operations.  Additionally, `mode` is set to a "reasonable
+  default" for most unix systems, based on a `umask` value of `0o22`.
 - `preservePaths` Allow absolute paths.  By default, `/` is stripped
   from absolute paths.
 - `strict` Treat warnings as crash-worthy errors.  Default false.

lib/mode-fix.js

@@ -1,6 +1,16 @@
 'use strict'
-module.exports = (mode, isDir) => {
+module.exports = (mode, isDir, portable) => {
   mode &= 0o7777
+
+  // in portable mode, use the minimum reasonable umask
+  // if this system creates files with 0o664 by default
+  // (as some linux distros do), then we'll write the
+  // archive with 0o644 instead.  Also, don't ever create
+  // a file that is not readable/writable by the owner.
+  if (portable) {
+    mode = (mode | 0o600) &~0o22
+  }
+
   // if dirs are readable, then they should be listable
   if (isDir) {
     if (mode & 0o400)

lib/write-entry.js

@@ -115,7 +115,7 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
   }
 
   [MODE] (mode) {
-    return modeFix(mode, this.type === 'Directory')
+    return modeFix(mode, this.type === 'Directory', this.portable)
   }
 
   [HEADER] () {
@@ -406,7 +406,7 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
   }
 
   [MODE] (mode) {
-    return modeFix(mode, this.type === 'Directory')
+    return modeFix(mode, this.type === 'Directory', this.portable)
   }
 
   write (data) {

test/mode-fix.js

@@ -7,3 +7,10 @@ t.equal(mf(0o10644, true),  0o755)
 t.equal(mf(0o10604, true),  0o705)
 t.equal(mf(0o10600, true),  0o700)
 t.equal(mf(0o10066, true),  0o077)
+
+t.equal(mf(0o10664, false, true), 0o644)
+t.equal(mf(0o10066, false, true),  0o644)
+t.equal(mf(0o10666, true, true),  0o755)
+t.equal(mf(0o10604, true, true),  0o705)
+t.equal(mf(0o10600, true, true),  0o700)
+t.equal(mf(0o10066, true, true),  0o755)