From dd23d19860613919676d602d87f6215d2d38e556 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 22 Apr 2026 01:51:48 +0200 Subject: [PATCH] fix(binary-fetcher): skip zip directory entries during Node.js runtime extraction (#11333) * fix(binary-fetcher): skip zip directory entries during Node.js extraction When a Node.js Windows zip contains explicit directory entries (which real `node-vX.Y.Z-win-.zip` archives do), `extractEntryTo` for the top-level directory recurses over every descendant via `getEntryChildren(subfolders: true)`, writing every child file directly and bypassing the `ignoreEntry` filter. That re-materialized the `npm`, `npx`, and `corepack` files stripped in #11325. Skip directory entries in the loop and let file extraction create parent directories implicitly. Add a regression test that constructs a zip with explicit directory entries. Closes the regression on `installing/deps-installer/test/install/nodeRuntime.ts` observed on Windows after #11325. * docs: remove 'subfolders' cspell-flagged word from fix commit --- .../fix-binary-fetcher-zip-dir-entries.md | 6 ++++ fetching/binary-fetcher/src/index.ts | 9 ++++- fetching/binary-fetcher/test/index.ts | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-binary-fetcher-zip-dir-entries.md diff --git a/.changeset/fix-binary-fetcher-zip-dir-entries.md b/.changeset/fix-binary-fetcher-zip-dir-entries.md new file mode 100644 index 0000000000..86f0983b58 --- /dev/null +++ b/.changeset/fix-binary-fetcher-zip-dir-entries.md @@ -0,0 +1,6 @@ +--- +"@pnpm/fetching.binary-fetcher": patch +"pnpm": patch +--- + +Fix Windows Node.js runtime installs still extracting bundled `npm`, `npx`, and `corepack` when the archive contains explicit directory entries. `extractZipToTarget` now skips directory entries: AdmZip's `extractEntryTo` for a directory recurses over every descendant internally, which bypassed the `ignoreEntry` filter and re-materialized the files the filter was supposed to drop. File extraction creates parent directories implicitly, so skipping directory entries doesn't regress the install layout. diff --git a/fetching/binary-fetcher/src/index.ts b/fetching/binary-fetcher/src/index.ts index 180e519a02..60e6b2e0d0 100644 --- a/fetching/binary-fetcher/src/index.ts +++ b/fetching/binary-fetcher/src/index.ts @@ -202,8 +202,15 @@ async function extractZipToTarget ( // entries in this loop. const testEntry = toStatelessTester(ignoreEntry) - // Extract each entry with path validation to prevent path traversal attacks + // Extract each entry with path validation to prevent path traversal attacks. + // Directory entries are skipped: AdmZip's `extractEntryTo(dir, ...)` expands + // to every descendant via `getEntryChildren`, which would bypass the + // `ignoreEntry` filter (e.g. the archive's top-level + // `node-vX.Y.Z--/` entry would pull in npm/corepack files + // even though the per-file relative paths match the ignore regex). + // File extraction creates parent directories implicitly. for (const entry of zip.getEntries()) { + if (entry.isDirectory) continue const entryPath = entry.entryName validatePathSecurity(nodeDir, entryPath) if (testEntry) { diff --git a/fetching/binary-fetcher/test/index.ts b/fetching/binary-fetcher/test/index.ts index 7778b8f7b5..b2ad4f0b3f 100644 --- a/fetching/binary-fetcher/test/index.ts +++ b/fetching/binary-fetcher/test/index.ts @@ -280,6 +280,41 @@ describe('extractZipToTarget security', () => { expect(fs.existsSync(path.join(targetDir, 'bin/npm'))).toBe(false) }) + it('still honors ignoreEntry when the archive contains directory entries (regression for #11325)', async () => { + // Real Node.js Windows zips include directory entries in addition to file + // entries. AdmZip's extractEntryTo(dirEntry, …) expands to every descendant + // via getEntryChildren, which previously bypassed the ignoreEntry filter. + // Covering that path explicitly here. + const targetDir = temporaryDirectory() + const zip = new AdmZip() + zip.addFile('node-v20.0.0/', Buffer.alloc(0)) + zip.addFile('node-v20.0.0/node.exe', Buffer.from('binary')) + zip.addFile('node-v20.0.0/node_modules/', Buffer.alloc(0)) + zip.addFile('node-v20.0.0/node_modules/npm/', Buffer.alloc(0)) + zip.addFile('node-v20.0.0/node_modules/npm/package.json', Buffer.from('{}')) + zip.addFile('node-v20.0.0/node_modules/corepack/', Buffer.alloc(0)) + zip.addFile('node-v20.0.0/node_modules/corepack/package.json', Buffer.from('{}')) + const zipBuffer = zip.toBuffer() + const integrity = ssri.fromData(zipBuffer).toString() + const mockFetch = createMockFetch(zipBuffer) + + await downloadAndUnpackZip( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockFetch as any, + { + url: 'https://example.com/node.zip', + integrity, + basename: 'node-v20.0.0', + ignoreEntry: /^node_modules\/(?:npm|corepack)(?:\/|$)/, + }, + targetDir + ) + + expect(fs.existsSync(path.join(targetDir, 'node.exe'))).toBe(true) + expect(fs.existsSync(path.join(targetDir, 'node_modules/npm'))).toBe(false) + expect(fs.existsSync(path.join(targetDir, 'node_modules/corepack'))).toBe(false) + }) + it('strips /g /y flags from ignoreEntry so .test() is not stateful across entries', async () => { const targetDir = temporaryDirectory() const zip = new AdmZip()