Files
pnpm/fetching/binary-fetcher/test/index.ts
Zoltan Kochan 73c8482a9d 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
2026-04-22 01:51:48 +02:00

405 lines
15 KiB
TypeScript

/// <reference path="../../../__typings__/index.d.ts"/>
import fs from 'node:fs'
import path from 'node:path'
import { describe, expect, it } from '@jest/globals'
import { PnpmError } from '@pnpm/error'
import { createBinaryFetcher, downloadAndUnpackZip } from '@pnpm/fetching.binary-fetcher'
import AdmZip from 'adm-zip'
import ssri from 'ssri'
import { temporaryDirectory } from 'tempy'
// Mock fetch function that returns a ZIP buffer and simulates FetchFromRegistry
function createMockFetch (zipBuffer: Buffer) {
return () => Promise.resolve({
body: (async function * () {
yield zipBuffer
})(),
})
}
describe('extractZipToTarget security', () => {
describe('prefix path traversal (Attack Vector 2)', () => {
it('should reject prefix with ../ path traversal', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('node-v20.0.0/bin/node', Buffer.from('#!/bin/sh\necho "node"'))
const zipBuffer = zip.toBuffer()
// Use real integrity so the check passes and we reach path traversal validation
const integrity = ssri.fromData(zipBuffer).toString()
const mockFetch = createMockFetch(zipBuffer)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '../../evil',
},
targetDir
)
).rejects.toThrow(PnpmError)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '../../evil',
},
targetDir
)
).rejects.toMatchObject({
code: 'ERR_PNPM_PATH_TRAVERSAL',
})
})
it('should reject absolute path prefix', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('node-v20.0.0/bin/node', Buffer.from('#!/bin/sh\necho "node"'))
const zipBuffer = zip.toBuffer()
// Use real integrity so the check passes and we reach path traversal validation
const integrity = ssri.fromData(zipBuffer).toString()
const mockFetch = createMockFetch(zipBuffer)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '/tmp/evil',
},
targetDir
)
).rejects.toMatchObject({
code: 'ERR_PNPM_PATH_TRAVERSAL',
})
})
})
describe('ZIP entry path traversal (Attack Vector 1)', () => {
it('should reject ZIP entries with ../ path traversal', async () => {
const targetDir = temporaryDirectory()
// Load fixture ZIP that has a raw malicious entry path
const zipBuffer = fs.readFileSync(path.join(import.meta.dirname, 'fixtures/path-traversal.zip'))
const integrity = ssri.fromData(zipBuffer).toString()
const mockFetch = createMockFetch(zipBuffer)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '',
},
targetDir
)
).rejects.toMatchObject({
code: 'ERR_PNPM_PATH_TRAVERSAL',
})
// Verify no files were written outside target
const parentDir = path.dirname(targetDir)
expect(fs.existsSync(path.join(parentDir, '.npmrc'))).toBe(false)
})
it('should reject ZIP entries with absolute paths', async () => {
const targetDir = temporaryDirectory()
// Load fixture ZIP that has a raw malicious absolute path entry
const zipBuffer = fs.readFileSync(path.join(import.meta.dirname, 'fixtures/absolute-path.zip'))
const integrity = ssri.fromData(zipBuffer).toString()
const mockFetch = createMockFetch(zipBuffer)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '',
},
targetDir
)
).rejects.toMatchObject({
code: 'ERR_PNPM_PATH_TRAVERSAL',
})
})
// Windows-specific: backslash is a path separator only on Windows
// On Unix, backslash is a valid filename character, so this test only runs on Windows
const isWindows = process.platform === 'win32'
const windowsTest = isWindows ? it : it.skip
windowsTest('should reject ZIP entries with backslash path traversal on Windows', async () => {
const targetDir = temporaryDirectory()
// Load fixture ZIP with Windows-style backslash path traversal
const zipBuffer = fs.readFileSync(path.join(import.meta.dirname, 'fixtures/backslash-traversal.zip'))
const integrity = ssri.fromData(zipBuffer).toString()
const mockFetch = createMockFetch(zipBuffer)
await expect(
downloadAndUnpackZip(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockFetch as any,
{
url: 'https://example.com/node.zip',
integrity,
basename: '',
},
targetDir
)
).rejects.toMatchObject({
code: 'ERR_PNPM_PATH_TRAVERSAL',
})
})
})
describe('legitimate ZIP extraction', () => {
it('should successfully extract a normal ZIP file', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('node-v20.0.0/bin/node', Buffer.from('#!/bin/sh\necho "node"'))
zip.addFile('node-v20.0.0/README.md', Buffer.from('# Node.js'))
const zipBuffer = zip.toBuffer()
// Create a mock fetch that also passes integrity check by using the actual buffer
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',
},
targetDir
)
// Verify files were extracted correctly
expect(fs.existsSync(path.join(targetDir, 'bin/node'))).toBe(true)
expect(fs.existsSync(path.join(targetDir, 'README.md'))).toBe(true)
})
it('should handle empty basename correctly', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('bin/node', Buffer.from('#!/bin/sh\necho "node"'))
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: '',
},
targetDir
)
expect(fs.existsSync(path.join(targetDir, 'bin/node'))).toBe(true)
})
it('skips entries matching ignoreEntry regex (basename stripped)', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('node-v20.0.0/node.exe', Buffer.from('binary'))
zip.addFile('node-v20.0.0/npm', Buffer.from('npm shim'))
zip.addFile('node-v20.0.0/npm.cmd', Buffer.from('npm cmd'))
zip.addFile('node-v20.0.0/node_modules/npm/package.json', Buffer.from('{}'))
zip.addFile('node-v20.0.0/node_modules/corepack/package.json', Buffer.from('{}'))
zip.addFile('node-v20.0.0/node_modules/keep-me/index.js', Buffer.from('kept'))
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)(?:\/|$)|npm(?:\.cmd)?$)/,
},
targetDir
)
expect(fs.existsSync(path.join(targetDir, 'node.exe'))).toBe(true)
expect(fs.existsSync(path.join(targetDir, 'node_modules/keep-me/index.js'))).toBe(true)
expect(fs.existsSync(path.join(targetDir, 'npm'))).toBe(false)
expect(fs.existsSync(path.join(targetDir, 'npm.cmd'))).toBe(false)
expect(fs.existsSync(path.join(targetDir, 'node_modules/npm'))).toBe(false)
expect(fs.existsSync(path.join(targetDir, 'node_modules/corepack'))).toBe(false)
})
it('skips entries matching ignoreEntry regex when basename is empty', async () => {
const targetDir = temporaryDirectory()
const zip = new AdmZip()
zip.addFile('bin/node', Buffer.from('#!/bin/sh\necho "node"'))
zip.addFile('bin/npm', Buffer.from('npm shim'))
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: '',
ignoreEntry: /^bin\/npm$/,
},
targetDir
)
expect(fs.existsSync(path.join(targetDir, 'bin/node'))).toBe(true)
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()
zip.addFile('node-v20.0.0/node.exe', Buffer.from('binary'))
zip.addFile('node-v20.0.0/npm', Buffer.from('npm shim 1'))
zip.addFile('node-v20.0.0/npx', Buffer.from('npx shim 2'))
zip.addFile('node-v20.0.0/corepack', Buffer.from('corepack 3'))
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',
// Deliberately pass a /g regex — a stateful .test() would skip only
// every other matching entry. All three shims must still be dropped.
ignoreEntry: /^(?:npm|npx|corepack)$/g,
},
targetDir
)
expect(fs.existsSync(path.join(targetDir, 'node.exe'))).toBe(true)
expect(fs.existsSync(path.join(targetDir, 'npm'))).toBe(false)
expect(fs.existsSync(path.join(targetDir, 'npx'))).toBe(false)
expect(fs.existsSync(path.join(targetDir, 'corepack'))).toBe(false)
})
})
})
describe('createBinaryFetcher', () => {
it('rejects an invalid archiveFilters regex at creation time', () => {
const noop = (() => {
throw new Error('should not be called')
}) as never
expect(() =>
createBinaryFetcher({
fetch: noop,
fetchFromRemoteTarball: noop,
storeIndex: noop,
archiveFilters: { node: '(' },
})
).toThrow(PnpmError)
expect(() =>
createBinaryFetcher({
fetch: noop,
fetchFromRemoteTarball: noop,
storeIndex: noop,
archiveFilters: { node: '(' },
})
).toThrow(/Invalid archive filter regex for "node"/)
})
it('snapshots archiveFilters so post-creation mutations cannot reintroduce invalid patterns', () => {
const noop = (() => {
throw new Error('should not be called')
}) as never
const filters: Record<string, string> = { node: '^ok$' }
// Must succeed — the pattern is valid at construction time.
expect(() =>
createBinaryFetcher({
fetch: noop,
fetchFromRemoteTarball: noop,
storeIndex: noop,
archiveFilters: filters,
})
).not.toThrow()
// Mutating the caller's object after construction must not affect the fetcher.
// There's no direct read back, but any mutation reaching the fetcher would throw
// on subsequent fetches; the snapshot guarantees it can't.
filters.node = '('
// Reconstructing with the broken pattern fails — demonstrating the original
// fetcher would have failed at construction if it had seen the broken pattern.
expect(() =>
createBinaryFetcher({
fetch: noop,
fetchFromRemoteTarball: noop,
storeIndex: noop,
archiveFilters: filters,
})
).toThrow(/Invalid archive filter regex for "node"/)
})
})