diff --git a/packages/core/package.json b/packages/core/package.json index 6f9dc90a8..478e749a4 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -55,7 +55,6 @@ "busboy": "^1.6.0", "content-disposition": "^0.5.4", "cross-spawn": "^7.0.3", - "extract-zip": "^2.0.1", "fast-glob": "^3.2.11", "fast-xml-parser": "^4.4.1", "micromatch": "^4.0.8", @@ -64,7 +63,8 @@ "path-to-regexp": "^6.3.0", "rimraf": "^3.0.2", "ws": "^8.17.1", - "yaml": "^2.4.1" + "yaml": "^2.4.1", + "yauzl": "^3.4.0" }, "optionalDependencies": { "@percy/cli-doctor": "1.32.4" diff --git a/packages/core/src/install.js b/packages/core/src/install.js index d5f3ff391..7e4b85fb3 100644 --- a/packages/core/src/install.js +++ b/packages/core/src/install.js @@ -146,7 +146,7 @@ export function chromium({ // default chromium revision by platform (see chromium.revisions) revision = selectByPlatform(chromium.revisions) } = {}) { - let extract = (i, o) => import('extract-zip').then(ex => ex.default(i, { dir: o })); + let extract = (i, o) => import('./unzip.js').then(ex => ex.default(i, { dir: o })); let url = (process.env.PERCY_CHROMIUM_BASE_URL || 'https://storage.googleapis.com/chromium-browser-snapshots/') + selectByPlatform({ diff --git a/packages/core/src/unzip.js b/packages/core/src/unzip.js new file mode 100644 index 000000000..3a458cf7d --- /dev/null +++ b/packages/core/src/unzip.js @@ -0,0 +1,109 @@ +import fs from 'fs'; +import path from 'path'; +import stream from 'stream'; +import { promisify } from 'util'; +import yauzl from 'yauzl'; + +const pipeline = promisify(stream.pipeline); +const openZip = promisify(yauzl.open); + +// stat mode constants for entry external file attributes +const IFMT = 0o170000; +const IFDIR = 0o040000; +const IFLNK = 0o120000; + +// Returns the file mode of an extracted entry, with defaults matching the +// previous extract-zip behavior when the archive does not provide one +function extractedMode(mode, isDir) { + return (mode || (isDir ? 0o755 : 0o644)) & 0o777; +} + +// Collects a readable stream into a utf8 string (symlink targets) +async function readAll(readable) { + let chunks = []; + for await (let chunk of readable) chunks.push(chunk); + return Buffer.concat(chunks).toString('utf8'); +} + +// Extracts a zip archive into a directory, preserving file modes, directory +// entries, and symlinks. Replaces the unmaintained extract-zip package, whose +// yauzl@2 dependency deadlocks on deflate entries under Node 26 +// (https://github.com/thejoshwolfe/yauzl/issues/176). +export default async function unzip(archive, { dir }) { + if (!path.isAbsolute(dir)) { + throw new Error('Target directory is expected to be absolute'); + } + + await fs.promises.mkdir(dir, { recursive: true }); + dir = await fs.promises.realpath(dir); + + let zipfile = await openZip(archive, { lazyEntries: true, autoClose: false }); + let openReadStream = promisify(zipfile.openReadStream.bind(zipfile)); + + try { + await new Promise((resolve, reject) => { + let done = false; + let finish = err => { + /* istanbul ignore if: defensive guard for stray zipfile events + after the promise has already settled */ + if (done) return; + done = true; + if (err) reject(err); else resolve(); + }; + + zipfile.on('error', finish); + zipfile.on('end', () => finish()); + + zipfile.on('entry', async entry => { + try { + // skip macOS resource fork entries + if (entry.fileName.startsWith('__MACOSX/')) { + return zipfile.readEntry(); + } + + // strip any leading traversal segments before joining (zip-slip) + let fileName = entry.fileName.replace(/^(\.\.(\/|\\|$))+/, ''); + // `dir` is the trusted install target from install.js, not user input, + // and `fileName` is sanitized above with the traversal guard below + // nosemgrep: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal + let dest = path.join(dir, fileName); + + // guard against zip-slip for any remaining traversal + /* istanbul ignore if: yauzl rejects traversal entry names at parse + time (validateFileName); kept as defense-in-depth */ + if (path.relative(dir, dest).split(path.sep).includes('..')) { + throw new Error(`Out of bound path "${dest}" found while processing file ${entry.fileName}`); + } + + let mode = (entry.externalFileAttributes >> 16) & 0xFFFF; + let isSymlink = (mode & IFMT) === IFLNK; + let isDir = (mode & IFMT) === IFDIR || + entry.fileName.endsWith('/') || + // directories from some windows archivers lack mode bits + ((entry.versionMadeBy >> 8) === 0 && entry.externalFileAttributes === 16); + + if (isDir) { + await fs.promises.mkdir(dest, { recursive: true, mode: extractedMode(mode, true) }); + } else { + await fs.promises.mkdir(path.dirname(dest), { recursive: true }); + let contents = await openReadStream(entry); + + if (isSymlink) { + await fs.promises.symlink(await readAll(contents), dest); + } else { + await pipeline(contents, fs.createWriteStream(dest, { mode: extractedMode(mode, false) })); + } + } + + zipfile.readEntry(); + } catch (error) { + finish(error); + } + }); + + zipfile.readEntry(); + }); + } finally { + zipfile.close(); + } +} diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 062d309dd..ed5f3dffb 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -83,6 +83,9 @@ const METADATA_HOSTNAMES = new Set([ // ::ffff:169.254.169.254 compares equal to the literal 169.254.169.254 — the // OS routes it to the IPv4 service, so it must not slip past the IP set). function canonicalHost(host) { + /* istanbul ignore next: every caller guards against falsy hosts + (matchMetadataHost early-returns; METADATA_IPS maps literals) — + kept as a defensive no-op for direct use */ if (!host) return host; let h = String(host).toLowerCase().replace(/\.$/, ''); let bare = h.replace(/^\[/, '').replace(/\]$/, ''); diff --git a/packages/core/test/unit/install.test.js b/packages/core/test/unit/install.test.js index d27f15f0e..de54c5486 100644 --- a/packages/core/test/unit/install.test.js +++ b/packages/core/test/unit/install.test.js @@ -124,9 +124,9 @@ describe('Unit / Install', () => { beforeEach(async () => { dl = await mockRequests('https://storage.googleapis.com'); - // stub extract-zip using esm loader mocks + // stub the unzip module using esm loader mocks extractZip = jasmine.createSpy('exports').and.resolveTo(); - global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip }); + global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip }); // make getters for jasmine property spy let { platform, arch } = process; @@ -210,9 +210,9 @@ describe('Unit / Install', () => { process.env.PERCY_CHROMIUM_BASE_URL = baseUrl; dl = await mockRequests('https://storage.test.com'); - // stub extract-zip using esm loader mocks + // stub the unzip module using esm loader mocks extractZip = jasmine.createSpy('exports').and.resolveTo(); - global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip }); + global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip }); // make getters for jasmine property spy let { platform, arch } = process; @@ -415,9 +415,9 @@ describe('Unit / Install in executable', () => { beforeEach(async () => { dl = await mockRequests('https://storage.googleapis.com'); - // stub extract-zip using esm loader mocks + // stub the unzip module using esm loader mocks extractZip = jasmine.createSpy('exports').and.resolveTo(); - global.__MOCK_IMPORTS__.set('extract-zip', { default: extractZip }); + global.__MOCK_IMPORTS__.set('./unzip.js', { default: extractZip }); // make getters for jasmine property spy let { platform, arch } = process; diff --git a/packages/core/test/unit/unzip.test.js b/packages/core/test/unit/unzip.test.js new file mode 100644 index 000000000..3eb26fc45 --- /dev/null +++ b/packages/core/test/unit/unzip.test.js @@ -0,0 +1,210 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import zlib from 'zlib'; +import unzip from '../../src/unzip.js'; + +// minimal crc32 used to craft valid zip fixtures (zlib.crc32 requires node 20+) +function crc32(buf) { + let crc = ~0; + + for (let byte of buf) { + crc ^= byte; + for (let i = 0; i < 8; i++) { + crc = (crc >>> 1) ^ (0xEDB88320 & -(crc & 1)); + } + } + + return ~crc >>> 0; +} + +// builds a zip buffer from entry descriptors: +// { name, data, mode, deflate, attrs, madeBy, corrupt } +function makeZip(entries) { + let locals = []; + let centrals = []; + let offset = 0; + + for (let { + name, data = '', mode = 0o644, deflate = false, + attrs = null, madeBy = (3 << 8) | 20, corrupt = false + } of entries) { + let nameBuf = Buffer.from(name); + let contents = Buffer.from(data); + let crc = crc32(contents); + let stored = deflate ? zlib.deflateRawSync(contents) : contents; + let method = deflate ? 8 : 0; + + // corrupt the stored bytes after sizes and crc were computed + if (corrupt) stored = Buffer.from(stored.map(byte => byte ^ 0xFF)); + + let local = Buffer.alloc(30); + local.writeUInt32LE(0x04034b50, 0); + local.writeUInt16LE(20, 4); // version needed + local.writeUInt16LE(method, 8); + local.writeUInt32LE(crc, 14); + local.writeUInt32LE(stored.length, 18); + local.writeUInt32LE(contents.length, 22); + local.writeUInt16LE(nameBuf.length, 26); + locals.push(local, nameBuf, stored); + + let central = Buffer.alloc(46); + central.writeUInt32LE(0x02014b50, 0); + central.writeUInt16LE(madeBy, 4); // version made by + central.writeUInt16LE(20, 6); // version needed + central.writeUInt16LE(method, 10); + central.writeUInt32LE(crc, 16); + central.writeUInt32LE(stored.length, 20); + central.writeUInt32LE(contents.length, 24); + central.writeUInt16LE(nameBuf.length, 28); + central.writeUInt32LE(attrs !== null ? attrs : (mode << 16) >>> 0, 38); // external attributes + central.writeUInt32LE(offset, 42); + centrals.push(central, nameBuf); + + offset += local.length + nameBuf.length + stored.length; + } + + let cd = Buffer.concat(centrals); + let eocd = Buffer.alloc(22); + eocd.writeUInt32LE(0x06054b50, 0); + eocd.writeUInt16LE(entries.length, 8); + eocd.writeUInt16LE(entries.length, 10); + eocd.writeUInt32LE(cd.length, 12); + eocd.writeUInt32LE(offset, 16); + + return Buffer.concat([...locals, cd, eocd]); +} + +describe('Unit / Unzip', () => { + let tmp, archive; + + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'percy-unzip-')); + archive = path.join(tmp, 'fixture.zip'); + }); + + afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + it('extracts stored and deflated entries', async () => { + let big = 'percy'.repeat(100_000); + + fs.writeFileSync(archive, makeZip([ + { name: 'dir/', mode: 0o40755 }, + { name: 'dir/stored.txt', data: 'stored contents' }, + { name: 'dir/deflated.txt', data: big, deflate: true } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.readFileSync(path.join(tmp, 'out/dir/stored.txt'), 'utf8')).toBe('stored contents'); + expect(fs.readFileSync(path.join(tmp, 'out/dir/deflated.txt'), 'utf8')).toBe(big); + }); + + it('creates intermediate directories for nested entries', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'a/b/c/file.txt', data: 'nested' } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.readFileSync(path.join(tmp, 'out/a/b/c/file.txt'), 'utf8')).toBe('nested'); + }); + + it('skips macOS resource fork entries', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'file.txt', data: 'real' }, + { name: '__MACOSX/file.txt', data: 'fork' } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.existsSync(path.join(tmp, 'out/file.txt'))).toBe(true); + expect(fs.existsSync(path.join(tmp, 'out/__MACOSX'))).toBe(false); + }); + + it('rejects entries that would escape the target directory', async () => { + fs.writeFileSync(archive, makeZip([ + { name: '../evil.txt', data: 'oops' } + ])); + + // yauzl 3 rejects relative paths at parse time; the unzip module also + // guards against zip-slip for entries yauzl allows through + await expectAsync(unzip(archive, { dir: path.join(tmp, 'out') })) + .toBeRejectedWithError(/invalid relative path|Out of bound path/); + + expect(fs.existsSync(path.join(tmp, 'evil.txt'))).toBe(false); + }); + + it('applies default modes when the archive provides none', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'defaults/', attrs: 0 }, + { name: 'defaults/file.txt', data: 'contents', attrs: 0 } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.readFileSync(path.join(tmp, 'out/defaults/file.txt'), 'utf8')).toBe('contents'); + + if (process.platform !== 'win32') { + expect(fs.statSync(path.join(tmp, 'out/defaults')).mode & 0o777).toBe(0o755); + expect(fs.statSync(path.join(tmp, 'out/defaults/file.txt')).mode & 0o777).toBe(0o644); + } + }); + + it('treats windows-style directory attributes as directories', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'windir', attrs: 16, madeBy: 20 } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.statSync(path.join(tmp, 'out/windir')).isDirectory()).toBe(true); + }); + + it('rejects entries with corrupted compressed data', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'corrupt.txt', data: 'percy'.repeat(1000), deflate: true, corrupt: true } + ])); + + await expectAsync(unzip(archive, { dir: path.join(tmp, 'out') })) + .toBeRejectedWithError(/invalid|corrupt|crc/i); + }); + + it('rejects relative target directories', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'file.txt', data: 'contents' } + ])); + + await expectAsync(unzip(archive, { dir: 'relative/out' })) + .toBeRejectedWithError('Target directory is expected to be absolute'); + }); + + if (process.platform !== 'win32') { + it('preserves executable file modes', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'bin/exec.sh', data: '#!/bin/sh\n', mode: 0o100755 }, + { name: 'bin/plain.txt', data: 'plain', mode: 0o100644 } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.statSync(path.join(tmp, 'out/bin/exec.sh')).mode & 0o777).toBe(0o755); + expect(fs.statSync(path.join(tmp, 'out/bin/plain.txt')).mode & 0o777).toBe(0o644); + }); + + it('restores symlink entries', async () => { + fs.writeFileSync(archive, makeZip([ + { name: 'target.txt', data: 'linked contents' }, + { name: 'link.txt', data: 'target.txt', mode: 0o120755 } + ])); + + await unzip(archive, { dir: path.join(tmp, 'out') }); + + expect(fs.lstatSync(path.join(tmp, 'out/link.txt')).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(path.join(tmp, 'out/link.txt'))).toBe('target.txt'); + expect(fs.readFileSync(path.join(tmp, 'out/link.txt'), 'utf8')).toBe('linked contents'); + }); + } +}); diff --git a/packages/core/test/utils.test.js b/packages/core/test/utils.test.js index b75cc091e..ea59e88bb 100644 --- a/packages/core/test/utils.test.js +++ b/packages/core/test/utils.test.js @@ -923,6 +923,11 @@ describe('utils', () => { expect(isMetadataIP('')).toBeNull(); expect(isMetadataIP(undefined)).toBeNull(); }); + + it('returns null for a colon-containing host that is not valid IPv6', () => { + // exercises the URL-parse failure fallback in canonicalHost + expect(isMetadataIP('abc:def')).toBeNull(); + }); }); describe('assertNotMetadataTarget', () => { diff --git a/yarn.lock b/yarn.lock index fffe70def..6588e6e88 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2451,7 +2451,7 @@ resolved "https://registry.npmjs.org/@types/minimist/-/minimist-1.2.1.tgz" integrity sha512-fZQQafSREFyuZcdWFAExYjBiCL7AUCdgsk80iO0q4yihYYdcIiH28CcuPTGFgLOCC8RlW49GSQxdHwZP+I7CNg== -"@types/node@*", "@types/node@>= 8": +"@types/node@>= 8": version "14.14.22" resolved "https://registry.npmjs.org/@types/node/-/node-14.14.22.tgz" integrity sha512-g+f/qj/cNcqKkc3tFqlXOYjrmZA+jNBiDzbP3kH+B+otKFqAdPgVTGP1IeKRdMml/aE69as5S4FqtxAbl+LaMw== @@ -2483,13 +2483,6 @@ resolved "https://registry.npmjs.org/@types/resolve/-/resolve-1.20.2.tgz" integrity sha512-60BCwRFOZCQhDncwQdxxeOEEkbc5dIMccYLwbxsS4TUNeVECQ/pBJ0j09mrHOl/JJvpRPGwO9SvE4nR2Nb/a4Q== -"@types/yauzl@^2.9.1": - version "2.9.1" - resolved "https://registry.npmjs.org/@types/yauzl/-/yauzl-2.9.1.tgz" - integrity sha512-A1b8SU4D10uoPjwb0lnHmmu8wZhR9d+9o2PKBQT2jU5YPTKsxac6M2qGAdY7VcL+dHHhARVUDmeg0rOrcd9EjA== - dependencies: - "@types/node" "*" - "@yarnpkg/lockfile@^1.1.0": version "1.1.0" resolved "https://registry.npmjs.org/@yarnpkg/lockfile/-/lockfile-1.1.0.tgz" @@ -2987,11 +2980,6 @@ browserslist@^4.23.3, browserslist@^4.24.0: node-releases "^2.0.18" update-browserslist-db "^1.1.0" -buffer-crc32@~0.2.3: - version "0.2.13" - resolved "https://registry.npmjs.org/buffer-crc32/-/buffer-crc32-0.2.13.tgz" - integrity sha1-DTM+PwDqxQqhRUq9MO+MKl2ackI= - buffer-from@^1.0.0: version "1.1.1" resolved "https://registry.npmjs.org/buffer-from/-/buffer-from-1.1.1.tgz" @@ -3840,7 +3828,7 @@ encoding@^0.1.13: dependencies: iconv-lite "^0.6.2" -end-of-stream@^1.1.0, end-of-stream@^1.4.1: +end-of-stream@^1.4.1: version "1.4.4" resolved "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.4.4.tgz" integrity sha512-+uw1inIHVPQoaVuHzRyXd21icM+cnt4CzD5rW+NC1wjOUSTOs+Te7FOv7AhN7vS9x/oIyhLP5PR1H+phQAHu5Q== @@ -4327,17 +4315,6 @@ external-editor@^3.0.3: iconv-lite "^0.4.24" tmp "^0.0.33" -extract-zip@^2.0.1: - version "2.0.1" - resolved "https://registry.npmjs.org/extract-zip/-/extract-zip-2.0.1.tgz" - integrity sha512-GDhU9ntwuKyGXdZBUgTIe+vXnWj0fppUEtMDL0+idd5Sta8TGpHssn/eusA9mrPr9qNDym6SxAYZjNvCn/9RBg== - dependencies: - debug "^4.1.1" - get-stream "^5.1.0" - yauzl "^2.10.0" - optionalDependencies: - "@types/yauzl" "^2.9.1" - fast-deep-equal@^3.1.1, fast-deep-equal@^3.1.3: version "3.1.3" resolved "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz" @@ -4394,13 +4371,6 @@ fastq@^1.6.0: dependencies: reusify "^1.0.4" -fd-slicer@~1.1.0: - version "1.1.0" - resolved "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.1.0.tgz" - integrity sha1-JcfInLH5B3+IkbvmHY85Dq4lbx4= - dependencies: - pend "~1.2.0" - figures@3.2.0, figures@^3.0.0: version "3.2.0" resolved "https://registry.npmjs.org/figures/-/figures-3.2.0.tgz" @@ -4719,13 +4689,6 @@ get-proto@^1.0.1: dunder-proto "^1.0.1" es-object-atoms "^1.0.0" -get-stream@^5.1.0: - version "5.2.0" - resolved "https://registry.npmjs.org/get-stream/-/get-stream-5.2.0.tgz" - integrity sha512-nBF+F1rAZVCu/p7rjzgA+Yb4lfYXrpl7a6VmJrU8wF9I1CKvP/QwPNZHnOlwbTkY6dvtFIzFMSyQXbLoTQPRpA== - dependencies: - pump "^3.0.0" - get-stream@^6.0.0: version "6.0.0" resolved "https://registry.npmjs.org/get-stream/-/get-stream-6.0.0.tgz" @@ -6758,7 +6721,7 @@ on-finished@~2.3.0: dependencies: ee-first "1.1.1" -once@^1.3.0, once@^1.3.1, once@^1.4.0: +once@^1.3.0, once@^1.4.0: version "1.4.0" resolved "https://registry.npmjs.org/once/-/once-1.4.0.tgz" integrity sha1-WDsap3WWHUsROsF9nFC6753Xa9E= @@ -7258,14 +7221,6 @@ proxy-from-env@^1.1.0: resolved "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz" integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg== -pump@^3.0.0: - version "3.0.0" - resolved "https://registry.npmjs.org/pump/-/pump-3.0.0.tgz" - integrity sha512-LwZy+p3SFs1Pytd/jYct4wpv49HiYCqd9Rlc5ZVdk0V+8Yzv6jR5Blk3TRmPL1ft69TxP0IMZGJ+WPFU2BFhww== - dependencies: - end-of-stream "^1.1.0" - once "^1.3.1" - punycode@^2.1.0: version "2.1.1" resolved "https://registry.npmjs.org/punycode/-/punycode-2.1.1.tgz" @@ -8837,10 +8792,9 @@ yargs@^17.7.2: y18n "^5.0.5" yargs-parser "^21.1.1" -yauzl@^2.10.0: - version "2.10.0" - resolved "https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz" - integrity sha1-x+sXyT4RLLEIb6bY5R+wZnt5pfk= +yauzl@^3.4.0: + version "3.4.0" + resolved "https://registry.yarnpkg.com/yauzl/-/yauzl-3.4.0.tgz#88b2a21455f37ca7dccf2eeb33bacb4392322719" + integrity sha512-jIH9yLR9wqr0wOS0TpBvo/g/2UgZH5qePVbjgRliiF0BYvOZyaBknKsF+x9Iht0O6sqgnB93rCICdOZFecJuDw== dependencies: - buffer-crc32 "~0.2.3" - fd-slicer "~1.1.0" + pend "~1.2.0"