Added opt-in owner permission normalization to @tryghost/zip extract (#761)

ref https://linear.app/ghost/issue/ONC-1838

- Adds an `ensureOwnerPermissions` option (default `false`) to `extract()`
- When enabled, normalizes extracted entry permissions so the owner can
  always read/move/remove the result: directories gain at least owner rwx
  and files gain at least owner rw, while existing execute/group/world bits
  are preserved
- Fixes archives containing read-only directories (e.g. `dr-xr-xr-x`/`0555`)
  that otherwise fail to extract because nested files cannot be written into
  them
- Normalization mutates only the in-memory entry handed to extract-zip; the
  source zip is never rewritten or chmodded, and it runs after the existing
  size-limit, symlink and filename checks
- Defaults are unchanged for existing callers

Author: sagzy

packages/zip/README.md

@@ -26,6 +26,18 @@ let res = await zip.compress('path/to/a/folder', 'path/to/archive.zip', [options
 let res = await zip.extract('path/to/archive.zip', 'path/to/files', [options])
 ```
 
+### `extract` options
+
+- `limits.perEntryUncompressedBytes` / `limits.totalUncompressedBytes` — reject archives whose entries exceed the given uncompressed sizes.
+- `onEntry(entry, zipfile)` — called for every entry before it is written.
+- `ensureOwnerPermissions` (default `false`) — when `true`, normalizes extracted entry permissions so the owner can always read, move and remove the result. Directories gain at least owner `rwx` and files gain at least owner `rw`, while existing execute/group/world bits are preserved. The source zip is never modified.
+
+    This fixes archives that contain read-only directories (for example a `dr-xr-xr-x` / `0555` folder), which otherwise fail to extract because nested files cannot be written into them. It is intended for **trusted temporary extraction** of user-supplied archives (such as theme zips) where the caller must be able to read, move and remove the extracted tree.
+
+    ```
+    let res = await zip.extract('path/to/upload.zip', 'path/to/tmp', {ensureOwnerPermissions: true})
+    ```
+
 ## Develop
 
 This is a mono repository, managed with [Nx](https://nx.dev).

packages/zip/lib/ensure-owner-permissions.js

@@ -0,0 +1,95 @@
+const { S_IFMT, S_IFDIR, getEntryMode } = require('./entry-mode');
+
+// Owner permission bits we guarantee when `ensureOwnerPermissions` is enabled.
+const OWNER_DIR_BITS = 0o700; // rwx, so the tree can be traversed, written and removed
+const OWNER_FILE_BITS = 0o600; // rw, so files can be read and rewritten/removed
+
+// Mirrors extract-zip's directory detection: POSIX directory type bit, a
+// trailing slash, or the Windows directory attribute.
+function isDirectoryEntry(entry, mode) {
+    if ((mode & S_IFMT) === S_IFDIR) {
+        return true;
+    }
+
+    if (entry.fileName.endsWith('/')) {
+        return true;
+    }
+
+    // Windows' way of specifying a directory
+    // https://github.com/maxogden/extract-zip/issues/13#issuecomment-154494566
+    const madeBy = entry.versionMadeBy >> 8;
+    return madeBy === 0 && entry.externalFileAttributes === 16;
+}
+
+// Resolves the mode extract-zip would synthesise for an entry that carries no
+// POSIX mode bits, mirroring its getExtractedMode() fallback: the caller's
+// defaultDirMode/defaultFileMode, or 0o755/0o644 when those are not set.
+function resolveDefaultMode(isDir, opts) {
+    let mode = 0;
+
+    if (isDir) {
+        if (opts.defaultDirMode) {
+            mode = parseInt(opts.defaultDirMode, 10);
+        }
+
+        if (!mode) {
+            mode = 0o755;
+        }
+    } else {
+        if (opts.defaultFileMode) {
+            mode = parseInt(opts.defaultFileMode, 10);
+        }
+
+        if (!mode) {
+            mode = 0o644;
+        }
+    }
+
+    return mode;
+}
+
+/**
+ * Normalises a zip entry's permissions in place so the extracting user can
+ * always read, move and remove the result: directories gain at least owner rwx
+ * and files at least owner rw, while existing execute/group/world bits are
+ * preserved. Only the in-memory entry handed to extract-zip is changed; the
+ * original zip file is never rewritten.
+ *
+ * @param {Object} entry - the yauzl entry extract-zip is about to extract
+ * @param {Object} [opts] - the extract options (read for defaultDirMode/defaultFileMode)
+ */
+function ensureOwnerPermissions(entry, opts = {}) {
+    let mode = getEntryMode(entry);
+    const isDir = isDirectoryEntry(entry, mode);
+
+    // No POSIX mode bits are present, so extract-zip would synthesise the mode
+    // from defaultDirMode/defaultFileMode (falling back to 0o755/0o644). Resolve
+    // that same default here so the owner bits below are guaranteed even when the
+    // caller-supplied defaults omit them.
+    if (mode === 0) {
+        mode = resolveDefaultMode(isDir, opts);
+    }
+
+    const ownerBits = isDir ? OWNER_DIR_BITS : OWNER_FILE_BITS;
+    let nextMode = mode | ownerBits;
+
+    // A directory that was only inferred (trailing slash / Windows attribute)
+    // may lack the POSIX directory type bit. Set it so extract-zip still
+    // classifies the rewritten mode as a directory.
+    if (isDir && (nextMode & S_IFMT) !== S_IFDIR) {
+        nextMode = (nextMode & ~S_IFMT) | S_IFDIR;
+    }
+
+    // The mode extract-zip would already produce grants the owner the required
+    // access and needs no type-bit fix, so leave the entry untouched.
+    if (nextMode === mode) {
+        return;
+    }
+
+    // Write the new POSIX mode back into the high 16 bits while preserving the
+    // low 16 bits (DOS attributes and other zip-specific flags).
+    const lowBits = entry.externalFileAttributes & 0xffff;
+    entry.externalFileAttributes = (((nextMode & 0xffff) << 16) | lowBits) >>> 0;
+}
+
+module.exports = ensureOwnerPermissions;

packages/zip/lib/entry-mode.js

@@ -0,0 +1,18 @@
+// POSIX `stat` mode constants, matching the ones extract-zip uses internally
+// when it derives an entry's type and permissions from its external attributes.
+const S_IFMT = 0o170000; // bit mask for the file type bit fields
+const S_IFDIR = 0o040000; // directory
+const S_IFLNK = 0o120000; // symbolic link
+
+// Reads the POSIX mode an entry carries in the high 16 bits of its external
+// file attributes, exactly as extract-zip does when extracting.
+function getEntryMode(entry) {
+    return (entry.externalFileAttributes >> 16) & 0xffff;
+}
+
+module.exports = {
+    S_IFMT,
+    S_IFDIR,
+    S_IFLNK,
+    getEntryMode,
+};

packages/zip/lib/extract.js

@@ -1,4 +1,6 @@
 const errors = require('@tryghost/errors');
+const { S_IFMT, S_IFLNK, getEntryMode } = require('./entry-mode');
+const ensureOwnerPermissions = require('./ensure-owner-permissions');
 
 const defaultOptions = {};
 
@@ -106,12 +108,9 @@ function throwOnTotalTooLarge(entry, observedBytes, limitBytes, entriesProcessed
 }
 
 function throwOnSymlinks(entry) {
-    // Check if symlink
-    const mode = (entry.externalFileAttributes >> 16) & 0xffff;
-    // check if it's a symlink or dir (using stat mode constants)
-    const IFMT = 61440;
-    const IFLNK = 40960;
-    const symlink = (mode & IFMT) === IFLNK;
+    // Check if symlink (using stat mode constants)
+    const mode = getEntryMode(entry);
+    const symlink = (mode & S_IFMT) === S_IFLNK;
 
     if (symlink) {
         throw new errors.UnsupportedMediaTypeError({
@@ -150,12 +149,16 @@ function throwOnLargeFilenames(entry) {
  * @param {String} zipToExtract - full path to zip file that should be extracted
  * @param {String} destination - full path of the extraction target
  * @param {Object} [options]
- * @param {Integer} options.defaultDirMode - Directory Mode (permissions), defaults to 0o755
- * @param {Integer} options.defaultFileMode - File Mode (permissions), defaults to 0o644
- * @param {Function} options.onEntry - if present, will be called with (entry, zipfile) for every entry in the zip
+ * @param {Integer} [options.defaultDirMode] - Directory Mode (permissions), defaults to 0o755
+ * @param {Integer} [options.defaultFileMode] - File Mode (permissions), defaults to 0o644
+ * @param {Function} [options.onEntry] - if present, will be called with (entry, zipfile) for every entry in the zip
  * @param {Object} [options.limits] - if present, sets maximum uncompressed sizes
- * @param {Integer} options.limits.perEntryUncompressedBytes - maximum uncompressed size of each entry
- * @param {Integer} options.limits.totalUncompressedBytes - maximum total uncompressed size across all entries
+ * @param {Integer} [options.limits.perEntryUncompressedBytes] - maximum uncompressed size of each entry
+ * @param {Integer} [options.limits.totalUncompressedBytes] - maximum total uncompressed size across all entries
+ * @param {Boolean} [options.ensureOwnerPermissions] - when true, normalizes extracted entry permissions so the
+ *   owner can always read/move/remove the result: directories gain at least owner rwx and files at least owner rw,
+ *   while existing execute/group/world bits are preserved. The source zip is never modified. Defaults to false.
+ *   Intended for trusted temporary extraction of user-supplied archives (e.g. theme zips with read-only directories).
  */
 module.exports = async (zipToExtract, destination, options) => {
     const opts = Object.assign({}, defaultOptions, options);
@@ -167,6 +170,7 @@ module.exports = async (zipToExtract, destination, options) => {
 
     opts.dir = destination;
 
+    const shouldEnsureOwnerPermissions = opts.ensureOwnerPermissions === true;
     const originalOnEntry = opts.onEntry;
     opts.onEntry = (entry, zipfile) => {
         const entryUncompressedBytes = getEntryUncompressedSize(entry, limits);
@@ -188,6 +192,12 @@ module.exports = async (zipToExtract, destination, options) => {
         if (originalOnEntry) {
             originalOnEntry(entry, zipfile);
         }
+
+        // Normalize permissions last, immediately before extract-zip writes the
+        // entry. Do not add async work here: extract-zip does not await onEntry.
+        if (shouldEnsureOwnerPermissions) {
+            ensureOwnerPermissions(entry, opts);
+        }
     };
 
     await extract(zipToExtract, opts);