fix(git-fetcher): block git dependencies from running prepare scripts unless allowed (#10288)

* fix(git-fetcher): block git dependencies from running prepare scripts unless allowed

* Update exec/prepare-package/src/index.ts

Co-authored-by: Zoltan Kochan <z@kochan.io>

* Also implement in gitHostedTarballFetcher

* refactor: move allowBuild function creation to the store manager

* refactor: pass allowBuild function to fetch function directly

* refactor: revert not needed changes and update changesets

* test: fix

* fix: implemented CR suggestions

* test: fix

* test: fix

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>
This commit is contained in:
Oren
2025-12-09 19:25:07 +02:00
committed by GitHub
parent 98a0410aa1
commit ba065f6a8b
22 changed files with 136 additions and 30 deletions

View File

@@ -0,0 +1,14 @@
---
"@pnpm/prepare-package": major
"@pnpm/git-fetcher": patch
"@pnpm/tarball-fetcher": patch
"@pnpm/core": patch
"@pnpm/headless": patch
"@pnpm/fetcher-base": minor
"@pnpm/package-requester": minor
"@pnpm/resolve-dependencies": minor
"@pnpm/store-controller-types": minor
"pnpm": patch
---
Block git-hosted dependencies from running prepare scripts unless explicitly allowed in onlyBuiltDependencies [#10288](https://github.com/pnpm/pnpm/pull/10288).

View File

@@ -13,7 +13,14 @@ import { type IncludedDependencies } from '@pnpm/modules-yaml'
import { packageIsInstallable } from '@pnpm/package-is-installable'
import { type PatchGroupRecord, getPatchInfo } from '@pnpm/patching.config'
import { type PatchInfo } from '@pnpm/patching.types'
import { type DepPath, type SupportedArchitectures, type Registries, type PkgIdWithPatchHash, type ProjectId } from '@pnpm/types'
import {
type DepPath,
type SupportedArchitectures,
type Registries,
type PkgIdWithPatchHash,
type ProjectId,
type AllowBuild,
} from '@pnpm/types'
import {
type PkgRequestFetchResult,
type FetchResponse,
@@ -53,6 +60,7 @@ export interface DependenciesGraph {
}
export interface LockfileToDepGraphOptions {
allowBuild?: AllowBuild
autoInstallPeers: boolean
enableGlobalVirtualStore?: boolean
engineStrict: boolean
@@ -231,6 +239,7 @@ async function buildGraphFromPackages (
try {
fetchResponse = await opts.storeController.fetchPackage({
allowBuild: opts.allowBuild,
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,

View File

@@ -91,6 +91,7 @@ test('dlx install from git', async () => {
dir: process.cwd(),
storeDir: path.resolve('store'),
cacheDir: path.resolve('cache'),
allowBuild: ['shx'],
}, ['shelljs/shx#0dcbb9d1022037268959f8b706e0f06a6fd43fde', 'touch', 'foo'])
expect(fs.existsSync('foo')).toBeTruthy()

View File

@@ -5,7 +5,7 @@ import util from 'util'
import { PnpmError } from '@pnpm/error'
import { runLifecycleHook, type RunLifecycleHookOptions } from '@pnpm/lifecycle'
import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
import { type PackageManifest } from '@pnpm/types'
import { type AllowBuild, type PackageManifest } from '@pnpm/types'
import rimraf from '@zkochan/rimraf'
import preferredPM from 'preferred-pm'
import { omit } from 'ramda'
@@ -20,6 +20,7 @@ const PREPUBLISH_SCRIPTS = [
]
export interface PreparePackageOptions {
allowBuild?: AllowBuild
ignoreScripts?: boolean
rawConfig: Record<string, unknown>
unsafePerm?: boolean
@@ -30,6 +31,19 @@ export async function preparePackage (opts: PreparePackageOptions, gitRootDir: s
const manifest = await safeReadPackageJsonFromDir(pkgDir)
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return { shouldBeBuilt: false, pkgDir }
if (opts.ignoreScripts) return { shouldBeBuilt: true, pkgDir }
// Check if the package is allowed to run build scripts
// If allowBuild is undefined or returns false, block the build
if (!opts.allowBuild?.(manifest.name, manifest.version)) {
throw new PnpmError(
'GIT_DEP_PREPARE_NOT_ALLOWED',
`The git-hosted package "${manifest.name}@${manifest.version}" needs to execute build scripts but is not in the "onlyBuiltDependencies" allowlist.`,
{
hint: `Add the package to "onlyBuiltDependencies" in your project's pnpm-workspace.yaml to allow it to run scripts. For example:
onlyBuiltDependencies:
- "${manifest.name}"`,
}
)
}
const pm = (await preferredPM(gitRootDir))?.name ?? 'npm'
const execOpts: RunLifecycleHookOptions = {
depPath: `${manifest.name}@${manifest.version}`,

View File

@@ -5,12 +5,13 @@ import { createTestIpcServer } from '@pnpm/test-ipc-server'
import { fixtures } from '@pnpm/test-fixtures'
const f = fixtures(import.meta.dirname)
const allowBuild = () => true
test('prepare package runs the prepublish script', async () => {
const tmp = tempDir()
await using server = await createTestIpcServer(path.join(tmp, 'test.sock'))
f.copy('has-prepublish-script', tmp)
await preparePackage({ rawConfig: {} }, tmp, '')
await preparePackage({ allowBuild, rawConfig: {} }, tmp, '')
expect(server.getLines()).toStrictEqual([
'prepublish',
])
@@ -20,7 +21,7 @@ test('prepare package does not run the prepublish script if the main file is pre
const tmp = tempDir()
await using server = await createTestIpcServer(path.join(tmp, 'test.sock'))
f.copy('has-prepublish-script-and-main-file', tmp)
await preparePackage({ rawConfig: {} }, tmp, '')
await preparePackage({ allowBuild, rawConfig: {} }, tmp, '')
expect(server.getLines()).toStrictEqual([
'prepublish',
])
@@ -30,7 +31,7 @@ test('prepare package runs the prepublish script in the sub folder if pkgDir is
const tmp = tempDir()
await using server = await createTestIpcServer(path.join(tmp, 'test.sock'))
f.copy('has-prepublish-script-in-workspace', tmp)
await preparePackage({ rawConfig: {} }, tmp, 'packages/foo')
await preparePackage({ allowBuild, rawConfig: {} }, tmp, 'packages/foo')
expect(server.getLines()).toStrictEqual([
'prepublish',
])

View File

@@ -5,7 +5,7 @@ import {
type BinaryResolution,
} from '@pnpm/resolver-base'
import { type Cafs } from '@pnpm/cafs-types'
import { type DependencyManifest } from '@pnpm/types'
import { type AllowBuild, type DependencyManifest } from '@pnpm/types'
export interface PkgNameVersion {
name?: string
@@ -13,6 +13,7 @@ export interface PkgNameVersion {
}
export interface FetchOptions {
allowBuild?: AllowBuild
filesIndexFile: string
lockfileDir: string
onStart?: (totalSize: number | null, attempt: number) => void
@@ -36,6 +37,7 @@ export interface FetchResult {
}
export interface GitFetcherOptions {
allowBuild?: AllowBuild
readManifest?: boolean
filesIndexFile: string
pkg?: PkgNameVersion

View File

@@ -20,11 +20,6 @@ export interface CreateGitFetcherOptions {
export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: GitFetcher } {
const allowedHosts = new Set(createOpts?.gitShallowHosts ?? [])
const ignoreScripts = createOpts.ignoreScripts ?? false
const preparePkg = preparePackage.bind(null, {
ignoreScripts: createOpts.ignoreScripts,
rawConfig: createOpts.rawConfig,
unsafePerm: createOpts.unsafePerm,
})
const gitFetcher: GitFetcher = async (cafs, resolution, opts) => {
const tempLocation = await cafs.tempDir()
@@ -38,7 +33,12 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G
await execGit(['checkout', resolution.commit], { cwd: tempLocation })
let pkgDir: string
try {
const prepareResult = await preparePkg(tempLocation, resolution.path ?? '')
const prepareResult = await preparePackage({
allowBuild: opts.allowBuild,
ignoreScripts: createOpts.ignoreScripts,
rawConfig: createOpts.rawConfig,
unsafePerm: createOpts.unsafePerm,
}, tempLocation, resolution.path ?? '')
pkgDir = prepareResult.pkgDir
if (ignoreScripts && prepareResult.shouldBeBuilt) {
globalWarn(`The git-hosted package fetched from "${resolution.repo}" has to be built but the build scripts were ignored.`)

View File

@@ -114,7 +114,9 @@ test('prevent directory traversal attack when using Git sub folder #2', async ()
test('fetch a package from Git that has a prepare script', async () => {
const storeDir = temporaryDirectory()
const fetch = createGitFetcher({ rawConfig: {} }).git
const fetch = createGitFetcher({
rawConfig: {},
}).git
const { filesIndex } = await fetch(
createCafsStore(storeDir),
{
@@ -123,6 +125,7 @@ test('fetch a package from Git that has a prepare script', async () => {
type: 'git',
},
{
allowBuild: (pkgName) => pkgName === 'test-git-fetch',
filesIndexFile: path.join(storeDir, 'index.json'),
}
)
@@ -194,7 +197,9 @@ test('still able to shallow fetch for allowed hosts', async () => {
test('fail when preparing a git-hosted package', async () => {
const storeDir = temporaryDirectory()
const fetch = createGitFetcher({ rawConfig: {} }).git
const fetch = createGitFetcher({
rawConfig: {},
}).git
await expect(
fetch(createCafsStore(storeDir),
{
@@ -202,6 +207,7 @@ test('fail when preparing a git-hosted package', async () => {
repo: 'https://github.com/pnpm-e2e/prepare-script-fails.git',
type: 'git',
}, {
allowBuild: (pkgName) => pkgName === '@pnpm.e2e/prepare-script-fails',
filesIndexFile: path.join(storeDir, 'index.json'),
})
).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-fails.git": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`')
@@ -223,6 +229,43 @@ test('do not build the package when scripts are ignored', async () => {
expect(globalWarn).toHaveBeenCalledWith('The git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-works.git" has to be built but the build scripts were ignored.')
})
test('block git package with prepare script', async () => {
const storeDir = temporaryDirectory()
const fetch = createGitFetcher({ rawConfig: {} }).git
const repo = 'https://github.com/pnpm-e2e/prepare-script-works.git'
await expect(
fetch(createCafsStore(storeDir),
{
commit: '55416a9c468806a935636c0ad0371a14a64df8c9',
repo,
type: 'git',
}, {
allowBuild: () => false,
filesIndexFile: path.join(storeDir, 'index.json'),
})
).rejects.toThrow('The git-hosted package "@pnpm.e2e/prepare-script-works@1.0.0" needs to execute build scripts but is not in the "onlyBuiltDependencies" allowlist')
})
test('allow git package with prepare script', async () => {
const storeDir = temporaryDirectory()
const fetch = createGitFetcher({
rawConfig: {},
}).git
// This should succeed without throwing because the package is in the allowlist
const { filesIndex } = await fetch(createCafsStore(storeDir),
{
commit: '55416a9c468806a935636c0ad0371a14a64df8c9',
repo: 'https://github.com/pnpm-e2e/prepare-script-works.git',
type: 'git',
}, {
allowBuild: (pkgName) => pkgName === '@pnpm.e2e/prepare-script-works',
filesIndexFile: path.join(storeDir, 'index.json'),
})
expect(filesIndex['package.json']).toBeTruthy()
// Note: prepare.txt is in .gitignore so it won't be in the files index
// The fact that no error was thrown proves the prepare script was allowed to run
})
function prefixGitArgs (): string[] {
return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : []
}

View File

@@ -40,6 +40,7 @@
"@pnpm/fs.packlist": "catalog:",
"@pnpm/graceful-fs": "workspace:*",
"@pnpm/prepare-package": "workspace:*",
"@pnpm/types": "workspace:*",
"@zkochan/retry": "catalog:",
"lodash.throttle": "catalog:",
"p-map-values": "catalog:",

View File

@@ -75,7 +75,10 @@ async function prepareGitHostedPkg (
},
force: true,
})
const { shouldBeBuilt, pkgDir } = await preparePackage(opts, tempLocation, resolution.path ?? '')
const { shouldBeBuilt, pkgDir } = await preparePackage({
...opts,
allowBuild: fetcherOpts.allowBuild,
}, tempLocation, resolution.path ?? '')
const files = await packlist(pkgDir)
if (!resolution.path && files.length === Object.keys(filesIndex).length) {
if (!shouldBeBuilt) {

View File

@@ -445,6 +445,7 @@ test('fail when preparing a git-hosted package', async () => {
await expect(
fetch.gitHostedTarball(cafs, resolution, {
allowBuild: (pkgName) => pkgName === '@pnpm.e2e/prepare-script-fails',
filesIndexFile,
lockfileDir: process.cwd(),
pkg,

View File

@@ -1183,6 +1183,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
forgetResolutionsOfAllPrevWantedDeps(ctx.wantedLockfile)
}
const allowBuild = createAllowBuildFunction(opts)
let {
dependenciesGraph,
dependenciesByProjectId,
@@ -1196,6 +1197,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
} = await resolveDependencies(
projects,
{
allowBuild,
allowedDeprecatedVersions: opts.allowedDeprecatedVersions,
allowUnusedPatches: opts.allowUnusedPatches,
autoInstallPeers: opts.autoInstallPeers,
@@ -1296,7 +1298,6 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
}
let stats: InstallationResultStats | undefined
const allowBuild = createAllowBuildFunction(opts)
let ignoredBuilds: IgnoredBuilds | undefined
if (!opts.lockfileOnly && !isInstallationOnlyForLockfileCheck && opts.enableModulesDir) {
const result = await linkPackages(

View File

@@ -309,7 +309,11 @@ test('run lifecycle scripts of dependent packages after running scripts of their
test('run prepare script for git-hosted dependencies', async () => {
const project = prepareEmpty()
await addDependenciesToPackage({}, ['pnpm/test-git-fetch#8b333f12d5357f4f25a654c305c826294cb073bf'], testDefaults({ fastUnpack: false }))
await addDependenciesToPackage({}, ['pnpm/test-git-fetch#8b333f12d5357f4f25a654c305c826294cb073bf'], testDefaults({
fastUnpack: false,
onlyBuiltDependencies: ['test-git-fetch'],
neverBuiltDependencies: undefined,
}))
const scripts = project.requireModule('test-git-fetch/output.json')
expect(scripts).toStrictEqual([

View File

@@ -320,8 +320,10 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
}
}
const allowBuild = createAllowBuildFunction(opts)
const lockfileToDepGraphOpts = {
...opts,
allowBuild,
importerIds,
lockfileDir,
skipped,
@@ -382,7 +384,6 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
let newHoistedDependencies!: HoistedDependencies
let linkedToRoot = 0
const allowBuild = createAllowBuildFunction(opts)
if (opts.nodeLinker === 'hoisted' && hierarchy && prevGraph) {
await linkHoistedModules(opts.storeController, graph, prevGraph, hierarchy, {
allowBuild,

View File

@@ -14,7 +14,7 @@ import { type IncludedDependencies } from '@pnpm/modules-yaml'
import { packageIsInstallable } from '@pnpm/package-is-installable'
import { type PatchGroupRecord, getPatchInfo } from '@pnpm/patching.config'
import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
import { type DepPath, type SupportedArchitectures, type ProjectId, type Registries } from '@pnpm/types'
import { type DepPath, type SupportedArchitectures, type ProjectId, type Registries, type AllowBuild } from '@pnpm/types'
import {
type FetchPackageToStoreFunction,
type StoreController,
@@ -29,6 +29,7 @@ import {
} from '@pnpm/deps.graph-builder'
export interface LockfileToHoistedDepGraphOptions {
allowBuild?: AllowBuild
autoInstallPeers: boolean
engineStrict: boolean
force: boolean
@@ -225,6 +226,7 @@ async function fetchDeps (
} else {
try {
fetchResponse = opts.storeController.fetchPackage({
allowBuild: opts.allowBuild,
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,

View File

@@ -299,6 +299,7 @@ async function resolveAndFetch (
const pkg: PkgNameVersion = manifest != null ? pick(['name', 'version'], manifest) : {}
const fetchResult = ctx.fetchPackageToStore({
allowBuild: options.allowBuild,
fetchRawManifest: true,
force: forceFetch,
ignoreScripts: options.ignoreScripts,
@@ -623,6 +624,7 @@ Actual package in the store with the given integrity: ${pkgFilesIndex.name}@${pk
opts.pkg.id,
resolution,
{
allowBuild: opts.allowBuild,
filesIndexFile,
lockfileDir: opts.lockfileDir,
readManifest: opts.fetchRawManifest,

View File

@@ -31,6 +31,7 @@ import {
type StoreController,
} from '@pnpm/store-controller-types'
import {
type AllowBuild,
type DepPath,
type SupportedArchitectures,
type AllowedDeprecatedVersions,
@@ -139,6 +140,7 @@ export interface ChildrenByParentId {
}
export interface ResolutionContext {
allowBuild?: AllowBuild
allPeerDepNames: Set<string>
autoInstallPeers: boolean
autoInstallPeersFromHighestMatch: boolean
@@ -1311,6 +1313,7 @@ async function resolveDependency (
wantedDependency.bareSpecifier = replaceVersionInBareSpecifier(wantedDependency.bareSpecifier, options.preferredVersion)
}
pkgResponse = await ctx.storeController.requestPackage(wantedDependency, {
allowBuild: ctx.allowBuild,
alwaysTryWorkspacePackages: ctx.linkWorkspacePackagesDepth >= options.currentDepth,
currentPkg: currentPkg
? {

View File

@@ -8,6 +8,7 @@ import { type PatchGroupRecord } from '@pnpm/patching.config'
import { type PreferredVersions, type Resolution, type WorkspacePackages } from '@pnpm/resolver-base'
import { type StoreController } from '@pnpm/store-controller-types'
import {
type AllowBuild,
type SupportedArchitectures,
type AllowedDeprecatedVersions,
type PinnedVersion,
@@ -102,6 +103,7 @@ export interface ImporterToResolveGeneric<WantedDepExtraProps> extends Importer<
}
export interface ResolveDependenciesOptions {
allowBuild?: AllowBuild
autoInstallPeers?: boolean
autoInstallPeersFromHighestMatch?: boolean
allowedDeprecatedVersions: AllowedDeprecatedVersions
@@ -163,6 +165,7 @@ export async function resolveDependencyTree<T> (
const wantedToBeSkippedPackageIds = new Set<PkgResolutionId>()
const autoInstallPeers = opts.autoInstallPeers === true
const ctx: ResolutionContext = {
allowBuild: opts.allowBuild,
autoInstallPeers,
autoInstallPeersFromHighestMatch: opts.autoInstallPeersFromHighestMatch === true,
allowedDeprecatedVersions: opts.allowedDeprecatedVersions,

6
pnpm-lock.yaml generated
View File

@@ -3296,6 +3296,9 @@ importers:
'@pnpm/prepare-package':
specifier: workspace:*
version: link:../../exec/prepare-package
'@pnpm/types':
specifier: workspace:*
version: link:../../packages/types
'@pnpm/worker':
specifier: workspace:^
version: link:../../worker
@@ -3339,9 +3342,6 @@ importers:
'@pnpm/test-fixtures':
specifier: workspace:*
version: link:../../__utils__/test-fixtures
'@pnpm/types':
specifier: workspace:*
version: link:../../packages/types
'@types/lodash.throttle':
specifier: 'catalog:'
version: 4.1.7

View File

@@ -173,7 +173,7 @@ test('dlx creates cache and store prune cleans cache', async () => {
'--config.dlx-cache-max-age=50', // big number to avoid false negative should test unexpectedly takes too long to run
]
await Promise.all(Object.entries(commands).map(([cmd, args]) => execPnpm([...settings, 'dlx', cmd, ...args])))
await Promise.all(Object.entries(commands).map(([cmd, args]) => execPnpm([...settings, '--allow-build=shx', 'dlx', cmd, ...args])))
// ensure that the dlx cache has certain structure
const dlxBaseDir = path.resolve('cache', 'dlx')
@@ -191,11 +191,11 @@ test('dlx creates cache and store prune cleans cache', async () => {
[dlxDirs[3]]: 123,
} satisfies Record<string, number>
const now = new Date()
await Promise.all(Object.entries(ageTable).map(async ([dlxDir, age]) => {
Object.entries(ageTable).forEach(([dlxDir, age]) => {
const newDate = new Date(now.getTime() - age * 60_000)
const dlxCacheLink = path.resolve('cache', 'dlx', dlxDir, 'pkg')
await fs.promises.lutimes(dlxCacheLink, newDate, newDate)
}))
fs.lutimesSync(dlxCacheLink, newDate, newDate)
})
await execPnpm([...settings, 'store', 'prune'])
@@ -215,10 +215,7 @@ test('dlx creates cache and store prune cleans cache', async () => {
'store', 'prune'])
// test to see if all dlx cache items are deleted
expect(
fs.readdirSync(path.resolve('cache', 'dlx'))
.sort()
).toStrictEqual([])
expect(fs.readdirSync(path.resolve('cache', 'dlx'))).toStrictEqual([])
})
test('dlx should ignore non-auth info from .npmrc in the current directory', async () => {

View File

@@ -310,6 +310,7 @@ test('pnpm licenses should work with git protocol dep that have peerDependencies
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
onlyBuiltDependencies: ['ajv-keywords'],
pnpmHomeDir: '',
storeDir,
})

View File

@@ -14,6 +14,7 @@ import {
type ResolvedFrom,
} from '@pnpm/cafs-types'
import {
type AllowBuild,
type SupportedArchitectures,
type DependencyManifest,
type PackageManifest,
@@ -87,6 +88,7 @@ export interface PkgNameVersion {
}
export interface FetchPackageToStoreOptions {
allowBuild?: AllowBuild
fetchRawManifest?: boolean
force: boolean
ignoreScripts?: boolean
@@ -107,6 +109,7 @@ export type RequestPackageFunction = (
) => Promise<PackageResponse>
export interface RequestPackageOptions {
allowBuild?: AllowBuild
alwaysTryWorkspacePackages?: boolean
currentPkg?: {
id?: PkgResolutionId