Fix: Percy CLI hangs on Node.js 26 during Chromium extraction — adm-zip variant (PER-10062)

packages/cli-exec/test/start.test.js

@@ -12,6 +12,21 @@ describe('percy exec:start', () => {
     return promise;
   }
 
+  // `start(...)` resolves on process exit, not on listen, so poll the
+  // healthcheck endpoint until the server is reachable. A fixed sleep proved
+  // flaky on slow Windows CI runners where startup can exceed a second and
+  // `request` only retries ECONNREFUSED 5 times at 50ms intervals.
+  async function waitForHealthcheck(port = 5338, deadline = Date.now() + 15000) {
+    while (true) {
+      try {
+        return await request(`http://localhost:${port}/percy/healthcheck`, { retries: 0, noProxy: true });
+      } catch (err) {
+        if (Date.now() >= deadline) throw err;
+        await new Promise(r => setTimeout(r, 100));
+      }
+    }
+  }
+
   beforeEach(async () => {
     process.env.PERCY_TOKEN = '<<PERCY_TOKEN>>';
 
@@ -22,8 +37,8 @@ describe('percy exec:start', () => {
 
     started = start(['--quiet']);
     started.then(() => (started = null));
-    // wait until the process starts
-    await new Promise(r => setTimeout(r, 1000));
+    // wait until the server is reachable
+    await waitForHealthcheck();
     await ping();
   });
 
@@ -71,13 +86,7 @@ describe('percy exec:start', () => {
 
   it('can start on an alternate port', async () => {
     start(['--quiet', '--port=4567']);
-    // `start(...)` resolves on process exit, not on listen. Mirror the
-    // beforeEach's proven pattern — give the server a moment to bind, then
-    // let `ping` confirm reachability. This closes the race that caused
-    // ECONNREFUSED on Windows and, on dual-stack Node 18+ runners, an
-    // AggregateError from Happy-Eyeballs that `request` does not retry
-    // (the wrapping error has no `.code`).
-    await new Promise(r => setTimeout(r, 1000));
+    await waitForHealthcheck(4567);
     await ping(['--port=4567']);
     let response = await request('http://localhost:4567/percy/healthcheck');
     expect(response).toHaveProperty('success', true);

packages/core/package.json

@@ -54,8 +54,8 @@
     "@percy/webdriver-utils": "1.32.4",
     "busboy": "^1.6.0",
     "content-disposition": "^0.5.4",
+    "adm-zip": "^0.6.0",
     "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",

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({

packages/core/src/unzip.js

@@ -0,0 +1,66 @@
+import fs from 'fs';
+import path from 'path';
+import AdmZip from 'adm-zip';
+
+// 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;
+}
+
+// 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). adm-zip inflates
+// synchronously so it is unaffected; its own extractAllTo is not used because
+// it writes symlink entries as regular files, which breaks the Chromium.app
+// bundle on macOS.
+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);
+
+  for (let entry of new AdmZip(archive).getEntries()) {
+    // skip macOS resource fork entries
+    if (entry.entryName.startsWith('__MACOSX/')) continue;
+
+    // `dir` is the trusted install target from install.js, not user input,
+    // and entry names are validated by the traversal guard below
+    // nosemgrep: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
+    let dest = path.join(dir, entry.entryName);
+
+    // guard against zip-slip
+    if (path.relative(dir, dest).startsWith('..')) {
+      throw new Error(`Out of bound path "${dest}" found while processing file ${entry.entryName}`);
+    }
+
+    let mode = (entry.header.attr >> 16) & 0xFFFF;
+    let isSymlink = (mode & IFMT) === IFLNK;
+    let isDir = (mode & IFMT) === IFDIR ||
+      entry.isDirectory ||
+      // directories from some windows archivers lack mode bits
+      ((entry.header.made >> 8) === 0 && entry.header.attr === 16);
+
+    if (isDir) {
+      await fs.promises.mkdir(dest, { recursive: true, mode: extractedMode(mode, true) });
+    } else {
+      await fs.promises.mkdir(path.dirname(dest), { recursive: true });
+      // synchronous inflate with crc validation; throws on corrupted data
+      let contents = entry.getData();
+
+      if (isSymlink) {
+        await fs.promises.symlink(contents.toString('utf8'), dest);
+      } else {
+        await fs.promises.writeFile(dest, contents, { mode: extractedMode(mode, false) });
+      }
+    }
+  }
+}

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(/\]$/, '');

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;

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');
+    });
+  }
+});