mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-13 11:05:52 -04:00
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:
6
.changeset/fix-binary-fetcher-zip-dir-entries.md
Normal file
6
.changeset/fix-binary-fetcher-zip-dir-entries.md
Normal 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.
|
||||
@@ -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) {
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user