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-<arch>.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
This commit is contained in:
Zoltan Kochan
2026-04-22 01:51:48 +02:00
parent 4d7cd56ccc
commit dd23d19860
3 changed files with 49 additions and 1 deletions

View File

@@ -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.

View File

@@ -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-<platform>-<arch>/` 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) {

View File

@@ -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()