Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/light-carrots-fetch.md
Original file line number Diff line number Diff line change
@@ -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).
11 changes: 10 additions & 1 deletion deps/graph-builder/src/lockfileToDepGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -53,6 +60,7 @@ export interface DependenciesGraph {
}

export interface LockfileToDepGraphOptions {
allowBuild?: AllowBuild
autoInstallPeers: boolean
enableGlobalVirtualStore?: boolean
engineStrict: boolean
Expand Down Expand Up @@ -231,6 +239,7 @@ async function buildGraphFromPackages (

try {
fetchResponse = await opts.storeController.fetchPackage({
allowBuild: opts.allowBuild,
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,
Expand Down
1 change: 1 addition & 0 deletions exec/plugin-commands-script-runners/test/dlx.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
16 changes: 15 additions & 1 deletion exec/prepare-package/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -20,6 +20,7 @@ const PREPUBLISH_SCRIPTS = [
]

export interface PreparePackageOptions {
allowBuild?: AllowBuild
ignoreScripts?: boolean
rawConfig: Record<string, unknown>
unsafePerm?: boolean
Expand All @@ -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}`,
Expand Down
7 changes: 4 additions & 3 deletions exec/prepare-package/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
])
Expand All @@ -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',
])
Expand All @@ -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',
])
Expand Down
4 changes: 3 additions & 1 deletion fetching/fetcher-base/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ 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
version?: string
}

export interface FetchOptions {
allowBuild?: AllowBuild
filesIndexFile: string
lockfileDir: string
onStart?: (totalSize: number | null, attempt: number) => void
Expand All @@ -36,6 +37,7 @@ export interface FetchResult {
}

export interface GitFetcherOptions {
allowBuild?: AllowBuild
readManifest?: boolean
filesIndexFile: string
pkg?: PkgNameVersion
Expand Down
12 changes: 6 additions & 6 deletions fetching/git-fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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.`)
Expand Down
47 changes: 45 additions & 2 deletions fetching/git-fetcher/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
{
Expand All @@ -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'),
}
)
Expand Down Expand Up @@ -194,14 +197,17 @@ 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),
{
commit: 'ba58874aae1210a777eb309dd01a9fdacc7e54e7',
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/[email protected] npm-install: `npm install`')
Expand All @@ -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/[email protected]" 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'] : []
}
Expand Down
1 change: 1 addition & 0 deletions fetching/tarball-fetcher/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:",
Expand Down
5 changes: 4 additions & 1 deletion fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions fetching/tarball-fetcher/test/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion pkg-manager/core/src/install/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
forgetResolutionsOfAllPrevWantedDeps(ctx.wantedLockfile)
}

const allowBuild = createAllowBuildFunction(opts)
let {
dependenciesGraph,
dependenciesByProjectId,
Expand All @@ -1196,6 +1197,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
} = await resolveDependencies(
projects,
{
allowBuild,
allowedDeprecatedVersions: opts.allowedDeprecatedVersions,
allowUnusedPatches: opts.allowUnusedPatches,
autoInstallPeers: opts.autoInstallPeers,
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion pkg-manager/core/test/install/lifecycleScripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
3 changes: 2 additions & 1 deletion pkg-manager/headless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,10 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
}
}

const allowBuild = createAllowBuildFunction(opts)
const lockfileToDepGraphOpts = {
...opts,
allowBuild,
importerIds,
lockfileDir,
skipped,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion pkg-manager/headless/src/lockfileToHoistedDepGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,6 +29,7 @@ import {
} from '@pnpm/deps.graph-builder'

export interface LockfileToHoistedDepGraphOptions {
allowBuild?: AllowBuild
autoInstallPeers: boolean
engineStrict: boolean
force: boolean
Expand Down Expand Up @@ -225,6 +226,7 @@ async function fetchDeps (
} else {
try {
fetchResponse = opts.storeController.fetchPackage({
allowBuild: opts.allowBuild,
force: false,
lockfileDir: opts.lockfileDir,
ignoreScripts: opts.ignoreScripts,
Expand Down
2 changes: 2 additions & 0 deletions pkg-manager/package-requester/src/packageRequester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading