diff --git a/packages/zip/README.md b/packages/zip/README.md index 4a1acc563..d4c6f3383 100644 --- a/packages/zip/README.md +++ b/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). diff --git a/packages/zip/lib/ensure-owner-permissions.js b/packages/zip/lib/ensure-owner-permissions.js new file mode 100644 index 000000000..64990871b --- /dev/null +++ b/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; diff --git a/packages/zip/lib/entry-mode.js b/packages/zip/lib/entry-mode.js new file mode 100644 index 000000000..c100d709b --- /dev/null +++ b/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, +}; diff --git a/packages/zip/lib/extract.js b/packages/zip/lib/extract.js index 4cc6d8b80..1a4d995fc 100644 --- a/packages/zip/lib/extract.js +++ b/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); diff --git a/packages/zip/test/zip.test.js b/packages/zip/test/zip.test.js index 4ab0aff61..e35246075 100644 --- a/packages/zip/test/zip.test.js +++ b/packages/zip/test/zip.test.js @@ -5,6 +5,7 @@ const { hashElement } = require('folder-hash'); const EventEmitter = require('events'); const Module = require('module'); const createArchive = require('../lib/create-archive'); +const ensureOwnerPermissions = require('../lib/ensure-owner-permissions'); // Mimic how we expect this to be required const { compress, extract } = require('../'); @@ -62,6 +63,33 @@ function createZipWithEntries(zipPath, entries) { }); } +// Builds a zip where each entry can carry an explicit `mode` and `type` +// (e.g. a read-only `0o555` directory). Only defined keys are forwarded so that +// archiver applies its own defaults for the rest. A `null` source produces an +// empty entry, which is what directory entries need. +function createZipWithModedEntries(zipPath, entries) { + return createZip(zipPath, (archive) => { + for (const entry of entries) { + const data = { name: entry.name }; + + if (entry.type) { + data.type = entry.type; + } + + if (typeof entry.mode === 'number') { + data.mode = entry.mode; + } + + archive.append(entry.content === undefined ? null : entry.content, data); + } + }); +} + +// Reads the permission bits (lowest 9 bits) of a path's mode. +function permissionBits(targetPath) { + return fs.statSync(targetPath).mode & 0o777; +} + describe('Compress and Extract should be opposite functions', function () { let symlinkPath, themeFolder, zipDestination, unzipDestination; @@ -469,3 +497,195 @@ describe('Extract zip', function () { } }); }); + +describe('Extract zip with ensureOwnerPermissions', function () { + let zipDestination, unzipDestination; + + beforeAll(function () { + zipDestination = path.join(__dirname, 'fixtures', 'perms-theme.zip'); + unzipDestination = path.join(__dirname, 'fixtures', 'perms-theme-unzipped'); + }); + + afterEach(function () { + if (fs.existsSync(zipDestination)) { + fs.rmSync(zipDestination, { recursive: true, force: true }); + } + + if (fs.existsSync(unzipDestination)) { + // chmod the tree back to writable first so a read-only directory left + // behind by a default-mode extraction can still be removed. + for (const entry of fs.readdirSync(unzipDestination, { recursive: true })) { + fs.chmodSync(path.join(unzipDestination, entry), 0o755); + } + + fs.rmSync(unzipDestination, { recursive: true, force: true }); + } + }); + + it('preserves default read-only directory permissions when the option is omitted', async function () { + await createZipWithModedEntries(zipDestination, [ + { name: 'readonly-dir/', type: 'directory', mode: 0o555 }, + ]); + + await extract(zipDestination, unzipDestination); + + const dirPath = path.join(unzipDestination, 'readonly-dir'); + assert.equal(fs.existsSync(dirPath), true); + // Default behaviour is unchanged: the owner write bit is not added. + assert.equal(permissionBits(dirPath) & 0o200, 0); + }); + + it('extracts an archive with a read-only directory and read-only files', async function () { + await createZipWithModedEntries(zipDestination, [ + { name: 'readonly-dir/', type: 'directory', mode: 0o555 }, + { name: 'readonly-dir/nested.txt', content: 'nested content', mode: 0o444 }, + { name: 'readonly-file.txt', content: 'top-level content', mode: 0o444 }, + { name: 'writable-file.txt', content: 'already writable', mode: 0o644 }, + ]); + + await extract(zipDestination, unzipDestination, { ensureOwnerPermissions: true }); + + const dirPath = path.join(unzipDestination, 'readonly-dir'); + const nestedPath = path.join(dirPath, 'nested.txt'); + const filePath = path.join(unzipDestination, 'readonly-file.txt'); + const writablePath = path.join(unzipDestination, 'writable-file.txt'); + + // The directory gains owner rwx so the nested file can be written into it. + assert.equal((permissionBits(dirPath) & 0o700) === 0o700, true); + // Existing group/world read+execute bits are preserved. + assert.equal(permissionBits(dirPath) & 0o055, 0o055); + + // The nested file is extracted and readable. + assert.equal(fs.readFileSync(nestedPath, 'utf8'), 'nested content'); + + // Read-only files gain owner write while keeping group/world read. + assert.equal(permissionBits(filePath) & 0o600, 0o600); + assert.equal(permissionBits(filePath) & 0o044, 0o044); + assert.equal(fs.readFileSync(filePath, 'utf8'), 'top-level content'); + + // Files that already grant owner write are left untouched. + assert.equal(permissionBits(writablePath), 0o644); + assert.equal(fs.readFileSync(writablePath, 'utf8'), 'already writable'); + }); + + it('keeps execute bits on executable files while adding owner write', async function () { + await createZipWithModedEntries(zipDestination, [ + { name: 'script.sh', content: '#!/bin/sh\necho hi\n', mode: 0o555 }, + ]); + + await extract(zipDestination, unzipDestination, { ensureOwnerPermissions: true }); + + const scriptPath = path.join(unzipDestination, 'script.sh'); + const bits = permissionBits(scriptPath); + + // Owner gains write... + assert.equal(bits & 0o200, 0o200); + // ...without losing any of the original r-xr-xr-x execute/read bits. + assert.equal(bits & 0o555, 0o555); + }); + + it('still rejects symlink entries when ensureOwnerPermissions is enabled', async function () { + await createZip(zipDestination, (archive) => { + archive.symlink('themes/test-target', 'symlink-entry'); + }); + + await assert.rejects( + () => + extract(zipDestination, unzipDestination, { + ensureOwnerPermissions: true, + }), + (err) => { + assert.equal(err.errorType, 'UnsupportedMediaTypeError'); + assert.equal(err.message, 'Symlinks are not allowed in the zip folder.'); + assert.equal(err.code, 'SYMLINK_NOT_ALLOWED'); + assert.deepEqual(err.errorDetails, { + entryName: 'themes/test-target', + }); + return true; + }, + ); + }); +}); + +describe('ensureOwnerPermissions', function () { + it('sets the directory type bit on inferred directories that lack POSIX type bits', function () { + // A directory recognised only by its trailing slash, carrying 0o555 + // permission bits but no POSIX directory type bit, plus a low DOS bit. + const entry = { + fileName: 'inferred-dir/', + externalFileAttributes: (((0o555 << 16) >>> 0) | 0x10) >>> 0, + versionMadeBy: 0x0300, // unix host, so the Windows directory rule does not apply + }; + + ensureOwnerPermissions(entry); + + const mode = (entry.externalFileAttributes >> 16) & 0xffff; + // The directory type bit is now present... + assert.equal(mode & 0o170000, 0o040000); + // ...owner gains rwx while the original r-x bits are preserved. + assert.equal(mode & 0o777, 0o755); + // Low DOS attribute bits are preserved. + assert.equal(entry.externalFileAttributes & 0xffff, 0x10); + }); + + it('leaves entries without POSIX mode bits untouched when defaults already grant owner access', function () { + // No POSIX mode bits and no custom defaults: extract-zip applies its own + // owner-accessible defaults (0o755/0o644), so the entry is left as-is. + const entry = { fileName: 'no-mode.txt', externalFileAttributes: 0 }; + + ensureOwnerPermissions(entry); + + assert.equal(entry.externalFileAttributes, 0); + }); + + it('guarantees owner permissions for zero-mode entries even when defaultDirMode/defaultFileMode omit them', function () { + // Zero-mode entries (e.g. Windows / no-POSIX archives) whose mode would + // otherwise be synthesised from owner-less caller defaults. + const dirEntry = { fileName: 'win-dir/', externalFileAttributes: 0, versionMadeBy: 0 }; + const fileEntry = { fileName: 'win-file.txt', externalFileAttributes: 0, versionMadeBy: 0 }; + const opts = { defaultDirMode: 0o555, defaultFileMode: 0o444 }; + + ensureOwnerPermissions(dirEntry, opts); + ensureOwnerPermissions(fileEntry, opts); + + const dirMode = (dirEntry.externalFileAttributes >> 16) & 0xffff; + const fileMode = (fileEntry.externalFileAttributes >> 16) & 0xffff; + + // Directory: owner rwx is guaranteed, the default r-x bits are preserved, + // and the POSIX directory type bit is set. + assert.equal(dirMode & 0o700, 0o700); + assert.equal(dirMode & 0o055, 0o055); + assert.equal(dirMode & 0o170000, 0o040000); + + // File: owner rw is guaranteed while the default read bits are preserved. + assert.equal(fileMode & 0o600, 0o600); + assert.equal(fileMode & 0o044, 0o044); + }); + + it('normalizes a zero-mode directory to the owner-accessible default when no defaults are supplied', function () { + // A Windows-style directory entry with no POSIX mode bits and no caller + // defaults: extract-zip would fall back to 0o755, which already grants + // owner rwx, but we still stamp the directory type bit so it round-trips. + const dirEntry = { fileName: 'win-dir/', externalFileAttributes: 0, versionMadeBy: 0 }; + + ensureOwnerPermissions(dirEntry); + + const dirMode = (dirEntry.externalFileAttributes >> 16) & 0xffff; + assert.equal(dirMode & 0o170000, 0o040000); + assert.equal(dirMode & 0o777, 0o755); + }); + + it('recognizes a directory via the Windows directory attribute (no trailing slash)', function () { + // No POSIX type bit and no trailing slash: detected as a directory only + // by the Windows directory attribute (externalFileAttributes === 16, + // versionMadeBy host byte === 0). + const dirEntry = { fileName: 'win-dir', externalFileAttributes: 16, versionMadeBy: 0 }; + + ensureOwnerPermissions(dirEntry); + + const dirMode = (dirEntry.externalFileAttributes >> 16) & 0xffff; + // Treated as a directory: type bit set and owner rwx guaranteed. + assert.equal(dirMode & 0o170000, 0o040000); + assert.equal(dirMode & 0o700, 0o700); + }); +});