From 6b75c68f69ce79cc4f90ab80ad8d12fc503568f2 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 17 Jul 2026 10:45:14 +0530 Subject: [PATCH 1/5] fix(core): replace extract-zip with yauzl@3 unzip to fix Node 26 hang (PER-10062) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chromium archive extraction hangs forever on Node.js 26 because extract-zip@2.0.1 depends on yauzl@2, which relies on undefined stream destroy() behavior that Node 26 tightened — openReadStream callbacks never fire on deflate entries (thejoshwolfe/yauzl#176, fixed in 3.3.1). Since percy exec launches the discovery browser before running the wrapped command, the hang means customer test suites never start and builds finalize with no_snapshots. Replaces extract-zip with a local unzip module built on yauzl@^3.4.0 preserving prior behavior: file modes, directory entries, symlinks, __MACOSX/ skip, zip-slip guard, absolute target requirement. Verified extraction of chrome-mac.zip is byte/mode/symlink-identical to extract-zip output on Node 14, 22, and 26. Co-Authored-By: Claude Fable 5 --- packages/core/package.json | 4 +- packages/core/src/install.js | 2 +- packages/core/src/unzip.js | 100 ++++++++++++++ packages/core/test/unit/install.test.js | 12 +- packages/core/test/unit/unzip.test.js | 168 ++++++++++++++++++++++++ yarn.lock | 62 ++------- 6 files changed, 285 insertions(+), 63 deletions(-) create mode 100644 packages/core/src/unzip.js create mode 100644 packages/core/test/unit/unzip.test.js 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..ea94577bd --- /dev/null +++ b/packages/core/src/unzip.js @@ -0,0 +1,100 @@ +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 => { + 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(); + } + + let dest = path.join(dir, entry.fileName); + + // guard against zip-slip + 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/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..670d96f70 --- /dev/null +++ b/packages/core/test/unit/unzip.test.js @@ -0,0 +1,168 @@ +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 } +function makeZip(entries) { + let locals = []; + let centrals = []; + let offset = 0; + + for (let { name, data = '', mode = 0o644, deflate = 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; + + 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((3 << 8) | 20, 4); // version made by unix + 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((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('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/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" From 708869b7b89306b4d5fca00f2970e9c71b249a15 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 17 Jul 2026 10:52:18 +0530 Subject: [PATCH 2/5] chore(core): sanitize entry names pre-join and suppress path-join FP for trusted dir The remaining semgrep path-join-resolve-traversal finding is on the trusted install target directory parameter; entry names are sanitized with the leading-traversal strip plus the relative-path guard, and yauzl 3 additionally rejects relative entry paths at parse time. Co-Authored-By: Claude Fable 5 --- packages/core/src/unzip.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/core/src/unzip.js b/packages/core/src/unzip.js index ea94577bd..df13885e6 100644 --- a/packages/core/src/unzip.js +++ b/packages/core/src/unzip.js @@ -59,9 +59,14 @@ export default async function unzip(archive, { dir }) { return zipfile.readEntry(); } - let dest = path.join(dir, entry.fileName); - - // guard against zip-slip + // 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 if (path.relative(dir, dest).split(path.sep).includes('..')) { throw new Error(`Out of bound path "${dest}" found while processing file ${entry.fileName}`); } From 1a77510b7378e61cac353fddb4b7640c4c8edb8c Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 17 Jul 2026 11:19:55 +0530 Subject: [PATCH 3/5] test(core): reach 100% coverage on unzip module Adds fixtures for default modes, windows-style directory attributes, and corrupted deflate data (entry error path). Marks the double-finish guard and the redundant zip-slip throw (yauzl 3 validates entry names at parse time) as istanbul-ignored defense-in-depth. Co-Authored-By: Claude Fable 5 --- packages/core/src/unzip.js | 4 +++ packages/core/test/unit/unzip.test.js | 50 ++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/packages/core/src/unzip.js b/packages/core/src/unzip.js index df13885e6..3a458cf7d 100644 --- a/packages/core/src/unzip.js +++ b/packages/core/src/unzip.js @@ -44,6 +44,8 @@ export default async function unzip(archive, { dir }) { 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(); @@ -67,6 +69,8 @@ export default async function unzip(archive, { dir }) { 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}`); } diff --git a/packages/core/test/unit/unzip.test.js b/packages/core/test/unit/unzip.test.js index 670d96f70..3eb26fc45 100644 --- a/packages/core/test/unit/unzip.test.js +++ b/packages/core/test/unit/unzip.test.js @@ -18,19 +18,26 @@ function crc32(buf) { return ~crc >>> 0; } -// builds a zip buffer from entry descriptors: { name, data, mode, deflate } +// 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 } of entries) { + 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 @@ -43,14 +50,14 @@ function makeZip(entries) { let central = Buffer.alloc(46); central.writeUInt32LE(0x02014b50, 0); - central.writeUInt16LE((3 << 8) | 20, 4); // version made by unix + 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((mode << 16) >>> 0, 38); // external attributes + central.writeUInt32LE(attrs !== null ? attrs : (mode << 16) >>> 0, 38); // external attributes central.writeUInt32LE(offset, 42); centrals.push(central, nameBuf); @@ -130,6 +137,41 @@ describe('Unit / Unzip', () => { 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' } From 5752528cd79c70bb1de3d2d4e95ab04063a49450 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 17 Jul 2026 14:10:23 +0530 Subject: [PATCH 4/5] test(core): cover canonicalHost URL-parse fallback (fixes master coverage break) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master's Test @percy/core job has been red since a4b6b8a8 — global 100% coverage fails on src/utils.js:103, the canonicalHost catch for colon-containing hosts that are not valid IPv6. No spec fed such a host through the metadata gates. Adds one isMetadataIP spec ('abc:def') that deterministically exercises the fallback on Node 14/20/26. Co-Authored-By: Claude Fable 5 --- packages/core/test/utils.test.js | 5 +++++ 1 file changed, 5 insertions(+) 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', () => { From ed70ae7b92b04cc3d06adea34b2d613f8315ee4a Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 17 Jul 2026 14:40:09 +0530 Subject: [PATCH 5/5] test(core): istanbul-ignore canonicalHost falsy guard (unreachable via callers) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second half of the master coverage break: the falsy-host guard in canonicalHost is unreachable — matchMetadataHost early-returns on falsy hosts before calling it and METADATA_IPS maps hardcoded literals. Kept as a defensive no-op with an ignore pragma per repo convention. Co-Authored-By: Claude Fable 5 --- packages/core/src/utils.js | 3 +++ 1 file changed, 3 insertions(+) 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(/\]$/, '');