fix: harden allowBuilds artifact approvals (#12294)

## Summary

Package-name `allowBuilds` entries no longer approve lifecycle scripts for artifacts whose identity a name cannot pin: git, git-hosted tarball, direct tarball, and local directory dependencies. To approve such an artifact explicitly, use its peer-suffix-free lockfile depPath as the `allowBuilds` key — error hints, `pnpm ignored-builds`, and `pnpm approve-builds` print exactly that key.

- `AllowBuild` policy functions identify packages by `DepPath` instead of caller-supplied name/version. The policy parses name and version out of the depPath itself, so name-keyed rules can never be fed an identity that disagrees with the gated artifact. `AllowBuildContext` carries only an explicit `trustPackageIdentity` override, used to evaluate a previously recorded policy under its original semantics when detecting revoked approvals.
- Identity trust is derived from the depPath shape: a registry-style depPath (`name@semver`) is a trusted identity. This is sound because lockfile verification runs a new unconditional, offline structural pass that rejects lockfiles where such a key is backed by a git, directory, or git-hosted tarball resolution (`ERR_PNPM_RESOLUTION_SHAPE_MISMATCH`), while the npm resolution verifier already binds explicit tarball URLs of semver-keyed entries to the registry's own `dist.tarball` unconditionally. The pass runs inside the existing candidate walk and participates in the verification cache key (`resolutionShapeCheck`) on both the gate's and the fresh-resolve record paths, so the stat-only cache fast path stays sound and records written before the rule existed are re-verified.
- Installs detect approvals that were revoked (or stopped applying) for git/tarball artifacts and surface those packages as ignored builds; approvals granted for previously ignored builds trigger a rebuild of exactly those packages.
- `preparePackage` always treats the fetched manifest as an untrusted identity: it requires a `pkgResolutionId` and gates on the synthesized `name@<resolution id>` depPath. scp-style git URLs are normalized to `ssh://` form in resolution ids, and the git fetcher reuses `createGitHostedPkgId` from the resolver instead of re-deriving ids.
- Under the global virtual store, `pnpm rebuild` locates a projection created before the approval was granted by following the project's node_modules link, since the projection hash includes the allowBuilds verdict (relocating the projection instead is tracked in https://github.com/pnpm/pnpm/issues/12302).
- New shared helpers: `removePeersSuffix()` in `@pnpm/deps.path` (string-level peer-suffix stripping for user-written keys) and `allowBuildKeyFromIgnoredBuild()` in `@pnpm/building.policy` (the key under which an ignored build is approved).
- pacquet mirrors the whole policy: `AllowBuildPolicy::check(dep_path)` derives trust from the dep path, the git-fetcher allow-build closures take only the dep path, `pacquet-lockfile-verification` gains the same structural pass, error code, and cache identity, and dep-path keys normalize via `remove_suffix`.
- `shell-quote` is overridden to 1.8.4 (GHSA-w7jw-789q-3m8p / CVE-2026-9277).
- Test-harness fix: a module-level drain keeps the global log stream flowing in the deps-installer lifecycle tests, so reporter assertions no longer receive the buffered backlog of earlier installs that ran without a reporter.
This commit is contained in:
Zoltan Kochan
2026-06-10 12:05:28 +02:00
committed by GitHub
parent 2c0b91d6b9
commit bf1b731ee6
62 changed files with 1574 additions and 232 deletions

View File

@@ -0,0 +1,17 @@
---
"@pnpm/building.after-install": patch
"@pnpm/building.during-install": patch
"@pnpm/building.policy": patch
"@pnpm/deps.graph-builder": patch
"@pnpm/deps.graph-hasher": patch
"@pnpm/exec.prepare-package": patch
"@pnpm/fetching.git-fetcher": patch
"@pnpm/fetching.tarball-fetcher": patch
"@pnpm/installing.deps-installer": patch
"@pnpm/installing.deps-resolver": patch
"@pnpm/installing.deps-restorer": patch
"@pnpm/types": patch
"pnpm": patch
---
Require trusted package identity before package-name `allowBuilds` entries can approve lifecycle scripts for git, git-hosted tarball, direct tarball, and local directory artifacts. To approve one of those artifacts explicitly, use its peer-suffix-free lockfile depPath as the `allowBuilds` key. Lockfile verification now rejects lockfiles where a registry-style dependency path (`name@semver`) is backed by a git, directory, or git-hosted tarball resolution (`ERR_PNPM_RESOLUTION_SHAPE_MISMATCH`), so the dependency path is a reliable artifact identity by the time scripts can run.

View File

@@ -1,4 +1,5 @@
import assert from 'node:assert'
import fs from 'node:fs'
import path from 'node:path'
import util from 'node:util'
@@ -37,6 +38,7 @@ import { pickStoreIndexKey, StoreIndex } from '@pnpm/store.index'
import type {
DepPath,
IgnoredBuilds,
PkgIdWithPatchHash,
ProjectId,
ProjectManifest,
ProjectRootDir,
@@ -78,19 +80,23 @@ function findPackages (
})
return false
}
return matches(searched, pkgInfo)
return matches(searched, pkgInfo, dp.getPkgIdWithPatchHash(relativeDepPath))
})
}
// TODO: move this logic to separate package as this is also used in tree-builder
function matches (
searched: PackageSelector[],
manifest: { name: string, version?: string }
manifest: { name: string, version?: string },
pkgIdWithPatchHash: PkgIdWithPatchHash
): boolean {
return searched.some((searchedPkg) => {
if (typeof searchedPkg === 'string') {
return manifest.name === searchedPkg
}
if ('pkgIdWithPatchHash' in searchedPkg) {
return searchedPkg.pkgIdWithPatchHash === pkgIdWithPatchHash
}
return searchedPkg.name === manifest.name && !!manifest.version &&
semver.satisfies(manifest.version, searchedPkg.range)
})
@@ -99,6 +105,9 @@ function matches (
type PackageSelector = string | {
name: string
range: string
} | {
/** A user-written depPath spec, normalized with the peer suffix stripped. */
pkgIdWithPatchHash: string
}
export async function buildSelectedPkgs (
@@ -117,6 +126,9 @@ export async function buildSelectedPkgs (
const packages = ctx.currentLockfile.packages
const searched: PackageSelector[] = pkgSpecs.map((arg) => {
if (matchesDepPath(packages, arg)) {
return { pkgIdWithPatchHash: dp.removePeersSuffix(arg) }
}
const { fetchSpec, name, raw, type } = npa(arg)
if (raw === name) {
return name
@@ -168,6 +180,11 @@ export async function buildSelectedPkgs (
}
}
function matchesDepPath (packages: PackageSnapshots, pkgSpec: string): boolean {
const normalizedPkgSpec = dp.removePeersSuffix(pkgSpec)
return Object.keys(packages).some((depPath) => dp.removePeersSuffix(depPath) === normalizedPkgSpec)
}
export async function buildProjects (
projects: Array<{ buildIndex: number, manifest: ProjectManifest, rootDir: ProjectRootDir }>,
maybeOpts: BuildOptions
@@ -330,8 +347,8 @@ async function _rebuild (
const ignoredPkgs = new Set<DepPath>()
const _allowBuild = createAllowBuildFunction(opts) ?? (() => undefined)
const allowBuild = (pkgName: string, version: string, depPath: DepPath) => {
switch (_allowBuild(pkgName, version)) {
const allowBuild = (depPath: DepPath) => {
switch (_allowBuild(depPath)) {
case true: return true
case undefined: {
ignoredPkgs.add(depPath)
@@ -356,7 +373,10 @@ async function _rebuild (
opts.supportedArchitectures,
nodeVersion
)) {
gvsDirByDepPath.set(pkgMeta.depPath, path.join(globalVirtualStoreDir, hash))
const preferredGvsDir = path.join(globalVirtualStoreDir, hash)
gvsDirByDepPath.set(pkgMeta.depPath, fs.existsSync(preferredGvsDir)
? preferredGvsDir
: findLinkedGvsDir(pkgMeta.name, Object.values(ctx.projects), globalVirtualStoreDir) ?? preferredGvsDir)
}
}
const pkgModulesDir = (depPath: DepPath): string =>
@@ -438,7 +458,7 @@ async function _rebuild (
requiresBuild = pkgRequiresBuild(pgkManifest, new Map())
}
const hasSideEffects = requiresBuild && allowBuild(pkgInfo.name, pkgInfo.version, depPath) && await runPostinstallHooks({
const hasSideEffects = requiresBuild && allowBuild(depPath) && await runPostinstallHooks({
depPath,
extraBinPaths,
extraEnv: opts.extraEnv,
@@ -530,6 +550,38 @@ async function _rebuild (
return { pkgsThatWereRebuilt, ignoredPkgs }
}
// TODO: delete once rebuild relocates GVS projections to the newly computed
// hash instead of building in place (https://github.com/pnpm/pnpm/issues/12302).
function findLinkedGvsDir (
pkgName: string,
projects: Array<{ rootDir: ProjectRootDir }>,
globalVirtualStoreDir: string
): string | undefined {
const normalizedGvsRoot = `${path.resolve(globalVirtualStoreDir)}${path.sep}`
for (const { rootDir } of projects) {
const pkgLink = path.join(rootDir, 'node_modules', pkgName)
try {
const target = fs.readlinkSync(pkgLink)
const pkgRoot = path.resolve(path.dirname(pkgLink), target)
if (!pkgRoot.startsWith(normalizedGvsRoot)) continue
return nthAncestorDir(pkgRoot, pkgName.split('/').length + 1)
} catch (err: unknown) {
// EINVAL: pkgLink exists but is not a symlink.
if (util.types.isNativeError(err) && 'code' in err && (err.code === 'EINVAL' || err.code === 'ENOENT')) continue
throw err
}
}
return undefined
}
function nthAncestorDir (dir: string, levels: number): string {
let result = dir
for (let i = 0; i < levels; i++) {
result = path.dirname(result)
}
return result
}
function binDirsInAllParentDirs (pkgRoot: string, lockfileDir: string): string[] {
const binDirs: string[] = []
let dir = pkgRoot

View File

@@ -34,6 +34,7 @@
"dependencies": {
"@inquirer/prompts": "catalog:",
"@pnpm/building.after-install": "workspace:*",
"@pnpm/building.policy": "workspace:*",
"@pnpm/cli.command": "workspace:*",
"@pnpm/cli.common-cli-options-help": "workspace:*",
"@pnpm/cli.utils": "workspace:*",

View File

@@ -1,8 +1,8 @@
import { checkbox, confirm } from '@inquirer/prompts'
import { allowBuildKeyFromIgnoredBuild } from '@pnpm/building.policy'
import type { CommandHandlerMap } from '@pnpm/cli.command'
import type { Config, ConfigContext } from '@pnpm/config.reader'
import { writeSettings } from '@pnpm/config.writer'
import { parse } from '@pnpm/deps.path'
import { PnpmError } from '@pnpm/error'
import { install } from '@pnpm/installing.commands'
import { type StrictModules, writeModulesManifest } from '@pnpm/installing.modules-yaml'
@@ -195,7 +195,7 @@ export async function handler (opts: ApproveBuildsCommandOpts & RebuildCommandOp
if (params.length) {
const decided = new Set([...approved, ...denied])
for (const depPath of Array.from(modulesManifest.ignoredBuilds)) {
const name = parse(depPath).name ?? depPath
const name = allowBuildKeyFromIgnoredBuild(depPath)
if (decided.has(name)) {
modulesManifest.ignoredBuilds.delete(depPath)
}

View File

@@ -1,6 +1,6 @@
import path from 'node:path'
import { parse } from '@pnpm/deps.path'
import { allowBuildKeyFromIgnoredBuild } from '@pnpm/building.policy'
import { type Modules, readModulesManifest } from '@pnpm/installing.modules-yaml'
import type { IgnoredBuildsCommandOpts } from './ignoredBuilds.js'
@@ -18,7 +18,7 @@ export async function getAutomaticallyIgnoredBuilds (opts: IgnoredBuildsCommandO
if (modulesManifest?.ignoredBuilds) {
const ignoredPkgNames = new Set<string>()
for (const depPath of modulesManifest.ignoredBuilds) {
ignoredPkgNames.add(parse(depPath).name ?? depPath)
ignoredPkgNames.add(allowBuildKeyFromIgnoredBuild(depPath))
}
automaticallyIgnoredBuilds = Array.from(ignoredPkgNames)
} else {

View File

@@ -47,6 +47,7 @@ test('rebuilds dependencies', async () => {
'@pnpm.e2e/pre-and-postinstall-scripts-example@1.0.0',
'test-git-fetch@https://codeload.github.com/pnpm/test-git-fetch/tar.gz/8b333f12d5357f4f25a654c305c826294cb073bf',
])
const gitDepPath = modules!.pendingBuilds[1]
const modulesManifest = project.readModulesManifest()
await rebuild.handler({
@@ -56,7 +57,7 @@ test('rebuilds dependencies', async () => {
pending: false,
registries: modulesManifest!.registries!,
storeDir,
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true, 'test-git-fetch': true },
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true, [gitDepPath]: true },
}, [])
modules = project.readModulesManifest()
@@ -329,6 +330,7 @@ test('rebuilds specific dependencies', async () => {
])
const modulesManifest = project.readModulesManifest()
const gitDepPath = modulesManifest!.pendingBuilds.find((depPath) => depPath.startsWith('install-scripts-example-for-pnpm@'))!
await rebuild.handler({
...DEFAULT_OPTS,
cacheDir,
@@ -336,7 +338,7 @@ test('rebuilds specific dependencies', async () => {
pending: false,
registries: modulesManifest!.registries!,
storeDir,
allowBuilds: { 'install-scripts-example-for-pnpm': true },
allowBuilds: { [gitDepPath]: true },
}, ['install-scripts-example-for-pnpm'])
project.hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall')
@@ -382,6 +384,7 @@ test('rebuild with pending option', async () => {
// not to
// install-scripts-example-for-pnpm@https://codeload.github.com/pnpm-e2e/install-scripts-example/tar.gz/b6cfdb8af6f8d5ebc5e7de6831af9d38084d765b
expect(modules!.pendingBuilds[1]).toMatch(/^install-scripts-example-for-pnpm@.*b6cfdb8af6f8d5ebc5e7de6831af9d38084d765b.*/)
const gitDepPath = modules!.pendingBuilds[1]
project.hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall')
project.hasNot('@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall')
@@ -396,7 +399,7 @@ test('rebuild with pending option', async () => {
pending: true,
registries: modules!.registries!,
storeDir,
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true, 'install-scripts-example-for-pnpm': true },
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true, [gitDepPath]: true },
}, [])
modules = project.readModulesManifest()
@@ -538,3 +541,47 @@ test(`rebuild should not fail on incomplete ${WANTED_LOCKFILE}`, async () => {
}, [])
})
test('rebuilds in the global virtual store when the approval was granted after the install', async () => {
const project = prepare()
const cacheDir = path.resolve('cache')
const storeDir = path.resolve('store')
// No allowBuilds at install time: the GVS projection is created under the
// not-built hash. The approval arrives only at rebuild time, so the rebuild
// recomputes a different (built) hash and must locate the existing
// projection through the project's node_modules link.
fs.writeFileSync('pnpm-workspace.yaml', [
'enableGlobalVirtualStore: true',
'',
].join('\n'))
await execa('node', [
pnpmBin,
'add',
'--save-dev',
'@pnpm.e2e/pre-and-postinstall-scripts-example@1.0.0',
`--registry=${REGISTRY}`,
`--store-dir=${storeDir}`,
'--ignore-scripts',
`--cache-dir=${cacheDir}`,
])
const pkgVersionDir = path.join(storeDir, STORE_VERSION, 'links/@pnpm.e2e/pre-and-postinstall-scripts-example/1.0.0')
const hash = fs.readdirSync(pkgVersionDir)[0]
const pkgInGvs = path.join(pkgVersionDir, hash, 'node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example')
expect(fs.existsSync(path.join(pkgInGvs, 'generated-by-postinstall.js'))).toBeFalsy()
const modulesManifest = project.readModulesManifest()
await rebuild.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
enableGlobalVirtualStore: true,
pending: false,
registries: modulesManifest!.registries!,
storeDir,
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true },
}, [])
expect(fs.existsSync(path.join(pkgInGvs, 'generated-by-postinstall.js'))).toBeTruthy()
})

View File

@@ -86,6 +86,9 @@
},
{
"path": "../after-install"
},
{
"path": "../policy"
}
]
}

View File

@@ -91,7 +91,7 @@ export async function buildModules<T extends string> (
if (!ignoreScripts) {
const node = depGraph[depPath]
if (node.requiresBuild) {
const allowed = allowBuild(node.name, node.version)
const allowed = allowBuild(node.depPath)
switch (allowed) {
case false:
// Explicitly disallowed - don't report as ignored

View File

@@ -1,12 +1,9 @@
import { expandPackageVersionSpecs } from '@pnpm/config.version-policy'
import * as dp from '@pnpm/deps.path'
import type { AllowBuild, DepPath } from '@pnpm/types'
import type { AllowBuild, AllowBuildContext, DepPath } from '@pnpm/types'
export function isBuildExplicitlyDisallowed (depPath: DepPath, allowBuild?: AllowBuild): boolean {
if (!allowBuild) return false
const { name, version } = dp.parse(depPath)
if (!name || !version) return false
return allowBuild(name, version) === false
return allowBuild?.(depPath) === false
}
export function createAllowBuildFunction (
@@ -17,26 +14,59 @@ export function createAllowBuildFunction (
): undefined | AllowBuild {
if (opts.dangerouslyAllowAllBuilds) return () => true
if (opts.allowBuilds != null) {
const allowedBuilds = new Set<string>()
const disallowedBuilds = new Set<string>()
const allowedPackageBuilds = new Set<string>()
const disallowedPackageBuilds = new Set<string>()
const allowedDepPathBuilds = new Set<string>()
const disallowedDepPathBuilds = new Set<string>()
for (const [pkg, value] of Object.entries(opts.allowBuilds)) {
switch (value) {
case true:
allowedBuilds.add(pkg)
addAllowBuildRule(pkg, {
depPaths: allowedDepPathBuilds,
packageSpecs: allowedPackageBuilds,
})
break
case false:
disallowedBuilds.add(pkg)
addAllowBuildRule(pkg, {
depPaths: disallowedDepPathBuilds,
packageSpecs: disallowedPackageBuilds,
})
break
}
}
const expandedAllowed = expandPackageVersionSpecs(Array.from(allowedBuilds))
const expandedDisallowed = expandPackageVersionSpecs(Array.from(disallowedBuilds))
return (pkgName, version) => {
const pkgWithVersion = `${pkgName}@${version}`
if (expandedDisallowed.has(pkgName) || expandedDisallowed.has(pkgWithVersion)) {
const expandedAllowed = expandPackageVersionSpecs(Array.from(allowedPackageBuilds))
const expandedDisallowed = expandPackageVersionSpecs(Array.from(disallowedPackageBuilds))
return (depPath, context?: AllowBuildContext) => {
const pkgIdWithPatchHash = dp.getPkgIdWithPatchHash(depPath)
if (disallowedDepPathBuilds.has(pkgIdWithPatchHash)) {
return false
}
if (expandedAllowed.has(pkgName) || expandedAllowed.has(pkgWithVersion)) {
const { name, version, nonSemverVersion } = dp.parse(depPath)
const nameAtVersion = name != null && version != null ? `${name}@${version}` : undefined
if (
(name != null && expandedDisallowed.has(name)) ||
(nameAtVersion != null && expandedDisallowed.has(nameAtVersion))
) {
return false
}
if (allowedDepPathBuilds.has(pkgIdWithPatchHash)) {
return true
}
// Package-name rules require a trusted package identity. A
// registry-style depPath (name@semver) is the trust signal: the
// lockfile verification gate rejects lockfiles where such a key is
// backed by a non-registry resolution, so by the time scripts can
// run, the shape proves the artifact came from a registry. The
// override exists for callers that must evaluate name rules under
// legacy semantics (e.g. comparing against a policy recorded before
// identity trust existed).
const trustPackageIdentity = context?.trustPackageIdentity ??
(name != null && version != null && nonSemverVersion == null)
if (!trustPackageIdentity) return undefined
if (
(name != null && expandedAllowed.has(name)) ||
(nameAtVersion != null && expandedAllowed.has(nameAtVersion))
) {
return true
}
return undefined
@@ -44,3 +74,42 @@ export function createAllowBuildFunction (
}
return undefined
}
/**
* The `allowBuilds` key under which an ignored build should be approved:
* the package name for registry packages, the peer-suffix-free depPath for
* git/tarball artifacts, whose name alone must not approve builds.
*/
export function allowBuildKeyFromIgnoredBuild (depPath: DepPath): string {
const pkgIdWithPatchHash = dp.getPkgIdWithPatchHash(depPath)
const parsed = dp.parse(pkgIdWithPatchHash)
if (parsed.nonSemverVersion != null || parsed.name == null) return pkgIdWithPatchHash
return parsed.name
}
function addAllowBuildRule (
pkg: string,
target: {
depPaths: Set<string>
packageSpecs: Set<string>
}
): void {
if (isDepPathAllowBuildKey(pkg)) {
target.depPaths.add(dp.removePeersSuffix(pkg))
} else {
target.packageSpecs.add(pkg)
}
}
function isDepPathAllowBuildKey (pkg: string): boolean {
if (dp.removePeersSuffix(pkg) !== pkg) return true
if (pkg.includes('||')) return false
const parsed = dp.parse(pkg)
if (parsed.nonSemverVersion != null) return isSourceLikeDepPathVersion(parsed.nonSemverVersion)
if (parsed.name != null || pkg.startsWith('@')) return false
return pkg.includes('/') || pkg.includes(':')
}
function isSourceLikeDepPathVersion (version: string): boolean {
return version.includes(':') || version.includes('/') || version.includes('#')
}

View File

@@ -2,16 +2,20 @@ import { expect, it } from '@jest/globals'
import { createAllowBuildFunction, isBuildExplicitlyDisallowed } from '@pnpm/building.policy'
import type { DepPath } from '@pnpm/types'
function depPath (value: string): DepPath {
return value as DepPath
}
it('should allowBuilds with true value', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { foo: true, 'qar@1.0.0 || 2.0.0': true },
})
expect(typeof allowBuild).toBe('function')
expect(allowBuild!('foo', '1.0.0')).toBe(true)
expect(allowBuild!('bar', '1.0.0')).toBeUndefined()
expect(allowBuild!('qar', '1.1.0')).toBeUndefined()
expect(allowBuild!('qar', '1.0.0')).toBe(true)
expect(allowBuild!('qar', '2.0.0')).toBe(true)
expect(allowBuild!(depPath('foo@1.0.0'))).toBe(true)
expect(allowBuild!(depPath('bar@1.0.0'))).toBeUndefined()
expect(allowBuild!(depPath('qar@1.1.0'))).toBeUndefined()
expect(allowBuild!(depPath('qar@1.0.0'))).toBe(true)
expect(allowBuild!(depPath('qar@2.0.0'))).toBe(true)
})
it('should allowBuilds with false value', () => {
@@ -19,9 +23,9 @@ it('should allowBuilds with false value', () => {
allowBuilds: { foo: false, bar: true },
})
expect(typeof allowBuild).toBe('function')
expect(allowBuild!('foo', '1.0.0')).toBe(false)
expect(allowBuild!('bar', '1.0.0')).toBe(true)
expect(allowBuild!('baz', '1.0.0')).toBeUndefined()
expect(allowBuild!(depPath('foo@1.0.0'))).toBe(false)
expect(allowBuild!(depPath('bar@1.0.0'))).toBe(true)
expect(allowBuild!(depPath('baz@1.0.0'))).toBeUndefined()
})
it('should not allow patterns in allowBuilds', () => {
@@ -29,7 +33,13 @@ it('should not allow patterns in allowBuilds', () => {
allowBuilds: { 'is-*': true },
})
expect(typeof allowBuild).toBe('function')
expect(allowBuild!('is-odd', '1.0.0')).toBeUndefined()
expect(allowBuild!(depPath('is-odd@1.0.0'))).toBeUndefined()
})
it('should reject invalid package versions in allowBuilds', () => {
expect(() => createAllowBuildFunction({
allowBuilds: { 'foo@not-a-version': true },
})).toThrow(expect.objectContaining({ code: 'ERR_PNPM_INVALID_VERSION_UNION' }))
})
it('should return undefined if no policy is set', () => {
@@ -41,23 +51,81 @@ it('should allow everything when dangerouslyAllowAllBuilds is true', () => {
dangerouslyAllowAllBuilds: true,
})
expect(typeof allowBuild).toBe('function')
expect(allowBuild!('foo', '1.0.0')).toBeTruthy()
expect(allowBuild!(depPath('foo@1.0.0'))).toBeTruthy()
expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBeTruthy()
})
it('should not apply package-name rules to artifact depPaths', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { foo: true, bar: true },
})
expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBeUndefined()
expect(allowBuild!(depPath('bar@https://example.com/bar.tgz'))).toBeUndefined()
expect(allowBuild!(depPath('foo@1.0.0'))).toBe(true)
})
it('should apply package-name rules to artifact depPaths when identity trust is overridden', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { foo: true },
})
expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'), {
trustPackageIdentity: true,
})).toBe(true)
expect(allowBuild!(depPath('foo@1.0.0'), { trustPackageIdentity: false })).toBeUndefined()
})
it('should deny by package name regardless of identity trust', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { foo: false },
})
expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBe(false)
expect(allowBuild!(depPath('foo@1.0.0'))).toBe(false)
})
it('should allow artifact depPaths by depPath key', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: {
'foo@git+https://github.com/org/foo.git#abc123': true,
'bar@https://codeload.github.com/org/bar/tar.gz/abc123': false,
foo: true,
},
})
expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123(react@19.0.0)'))).toBe(true)
expect(allowBuild!(depPath('foo@git+https://github.com/attacker/foo.git#abc123'))).toBeUndefined()
expect(allowBuild!(depPath('bar@https://codeload.github.com/org/bar/tar.gz/abc123'))).toBe(false)
})
it('should preserve patch hash in depPath allowBuild keys', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: {
'foo@https://example.com/foo.tgz(patch_hash=aaaa)': true,
},
})
expect(allowBuild!(depPath('foo@https://example.com/foo.tgz(patch_hash=aaaa)(react@19.0.0)'))).toBe(true)
expect(allowBuild!(depPath('foo@https://example.com/foo.tgz(patch_hash=bbbb)(react@19.0.0)'))).toBeUndefined()
})
it('should allow untrusted package identity by source-only depPath', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { 'github.com/org/foo/abc123': true },
})
expect(allowBuild!(depPath('github.com/org/foo/abc123(react@19.0.0)'))).toBe(true)
})
it('isBuildExplicitlyDisallowed() flags only builds the policy explicitly forbids', () => {
const allowBuild = createAllowBuildFunction({
allowBuilds: { foo: false, bar: true },
})
expect(isBuildExplicitlyDisallowed('foo@1.0.0' as DepPath, allowBuild)).toBe(true)
expect(isBuildExplicitlyDisallowed('bar@1.0.0' as DepPath, allowBuild)).toBe(false)
expect(isBuildExplicitlyDisallowed('baz@1.0.0' as DepPath, allowBuild)).toBe(false)
expect(isBuildExplicitlyDisallowed(depPath('foo@1.0.0'), allowBuild)).toBe(true)
expect(isBuildExplicitlyDisallowed(depPath('bar@1.0.0'), allowBuild)).toBe(false)
expect(isBuildExplicitlyDisallowed(depPath('baz@1.0.0'), allowBuild)).toBe(false)
})
it('isBuildExplicitlyDisallowed() returns false when no policy is set', () => {
expect(isBuildExplicitlyDisallowed('foo@1.0.0' as DepPath, undefined)).toBe(false)
expect(isBuildExplicitlyDisallowed(depPath('foo@1.0.0'), undefined)).toBe(false)
})
it('isBuildExplicitlyDisallowed() returns false for unparsable depPaths', () => {
const allowBuild = createAllowBuildFunction({ allowBuilds: { foo: false } })
expect(isBuildExplicitlyDisallowed('not-a-valid-dep-path' as DepPath, allowBuild)).toBe(false)
expect(isBuildExplicitlyDisallowed(depPath('not-a-valid-dep-path'), allowBuild)).toBe(false)
})

View File

@@ -1,5 +1,11 @@
import type { DepPath } from './misc.js'
export type PackageVersionPolicy = (pkgName: string) => boolean | string[]
export type AllowBuild = (pkgName: string, pkgVersion: string) => boolean | undefined
export interface AllowBuildContext {
trustPackageIdentity?: boolean
}
export type AllowBuild = (depPath: DepPath, context?: AllowBuildContext) => boolean | undefined
export type TrustPolicy = 'no-downgrade' | 'off'

View File

@@ -2,6 +2,6 @@
"name": "with-git-protocol-peer-deps",
"version": "1.0.0",
"dependencies": {
"ajv-keywords": "github:ajv-validator/ajv-keywords"
"ajv-keywords": "github:ajv-validator/ajv-keywords#a11389b4d1934d360fb2a24dd920ec597295c8fc"
}
}

View File

@@ -313,7 +313,9 @@ test('pnpm licenses should work with git protocol dep that have peerDependencies
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
allowBuilds: { 'ajv-keywords': true },
allowBuilds: {
'ajv-keywords@https://codeload.github.com/ajv-validator/ajv-keywords/tar.gz/a11389b4d1934d360fb2a24dd920ec597295c8fc': true,
},
pnpmHomeDir: '',
storeDir,
})

View File

@@ -241,7 +241,7 @@ async function buildGraphFromPackages (
// marker indicating a previous build failed or was interrupted. When the
// marker is present, skip the fast path to force a re-fetch/re-import/re-build.
const mightNeedBuild = opts.enableGlobalVirtualStore &&
opts.allowBuild?.(pkgName, pkgVersion) === true
opts.allowBuild?.(depPath) === true
let dirExists: boolean | undefined
if (

View File

@@ -331,13 +331,13 @@ export function lockfileToDepGraph (
}
function computeBuiltDepPaths (
entries: Iterable<{ depPath: DepPath; name: string; version: string }>,
entries: Iterable<PkgMeta>,
allowBuild: AllowBuild
): Set<DepPath> {
const builtDepPaths = new Set<DepPath>()
for (const { depPath, name, version } of entries) {
if (allowBuild(name, version) === true) {
builtDepPaths.add(depPath)
for (const entry of entries) {
if (allowBuild(entry.depPath) === true) {
builtDepPaths.add(entry.depPath)
}
}
return builtDepPaths

View File

@@ -0,0 +1,41 @@
import { expect, it } from '@jest/globals'
import { iterateHashedGraphNodes } from '@pnpm/deps.graph-hasher'
import type { AllowBuild, DepPath } from '@pnpm/types'
it('gates built dep paths through the allowBuild policy by depPath', () => {
const registryDepPath = 'foo@1.0.0' as DepPath
const directTarballDepPath = 'foo@https://example.com/foo.tgz' as DepPath
const checkedDepPaths: DepPath[] = []
const allowBuild: AllowBuild = (depPath) => {
checkedDepPaths.push(depPath)
return depPath === registryDepPath
}
Array.from(iterateHashedGraphNodes(
{
[registryDepPath]: {
children: {},
fullPkgId: 'foo@1.0.0:sha512-abc',
},
[directTarballDepPath]: {
children: {},
fullPkgId: 'foo@1.0.0:sha512-def',
},
},
[
{
depPath: registryDepPath,
name: 'foo',
version: '1.0.0',
},
{
depPath: directTarballDepPath,
name: 'foo',
version: '1.0.0',
},
].values(),
allowBuild
))
expect(checkedDepPaths).toStrictEqual([registryDepPath, directTarballDepPath])
})

View File

@@ -60,13 +60,16 @@ export function removeSuffix (relDepPath: string): string {
return relDepPath
}
export function getPkgIdWithPatchHash (depPath: DepPath): PkgIdWithPatchHash {
let pkgId: string = depPath
const { peersIndex: sepIndex } = indexOfDepPathSuffix(pkgId)
if (sepIndex !== -1) {
pkgId = pkgId.substring(0, sepIndex)
export function removePeersSuffix (relDepPath: string): string {
const { peersIndex } = indexOfDepPathSuffix(relDepPath)
if (peersIndex !== -1) {
return relDepPath.substring(0, peersIndex)
}
return pkgId as PkgIdWithPatchHash
return relDepPath
}
export function getPkgIdWithPatchHash (depPath: DepPath): PkgIdWithPatchHash {
return removePeersSuffix(depPath) as PkgIdWithPatchHash
}
export function tryGetPackageId (relDepPath: DepPath): PkgId {

View File

@@ -106,7 +106,9 @@ test('dlx install from git', async () => {
dir: process.cwd(),
storeDir: path.resolve('store'),
cacheDir: path.resolve('cache'),
allowBuild: ['shx'],
// A git-hosted artifact has an untrusted package identity, so it has to
// be approved by its depPath, not by package name.
allowBuild: ['shx@https://codeload.github.com/shelljs/shx/tar.gz/0dcbb9d1022037268959f8b706e0f06a6fd43fde'],
}, ['shelljs/shx#0dcbb9d1022037268959f8b706e0f06a6fd43fde', 'touch', 'foo'])
expect(fs.existsSync('foo')).toBeTruthy()

View File

@@ -6,7 +6,7 @@ import util from 'node:util'
import { PnpmError } from '@pnpm/error'
import { runLifecycleHook, type RunLifecycleHookOptions } from '@pnpm/exec.lifecycle'
import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader'
import type { AllowBuild, PackageManifest } from '@pnpm/types'
import type { AllowBuild, DepPath, PackageManifest } from '@pnpm/types'
import { rimraf } from '@zkochan/rimraf'
import { preferredPM } from 'preferred-pm'
@@ -22,6 +22,7 @@ const PREPUBLISH_SCRIPTS = [
export interface PreparePackageOptions {
allowBuild?: AllowBuild
ignoreScripts?: boolean
pkgResolutionId: string
unsafePerm?: boolean
userAgent?: string
}
@@ -32,15 +33,19 @@ export async function preparePackage (opts: PreparePackageOptions, gitRootDir: s
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)) {
// If allowBuild is undefined or returns false, block the build.
// The depPath is synthesized from the resolution id rather than read from
// a lockfile; resolution ids of git and tarball artifacts are never
// semver-shaped, so the policy derives an untrusted package identity.
const depPath = `${manifest.name}@${opts.pkgResolutionId}` as DepPath
if (!opts.allowBuild?.(depPath)) {
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 "allowBuilds" allowlist.`,
{
hint: `Add the package to "allowBuilds" in your project's pnpm-workspace.yaml to allow it to run scripts. For example:
allowBuilds:
${manifest.name}: true`,
${depPath}: true`,
}
)
}

View File

@@ -7,23 +7,35 @@ import { fixtures } from '@pnpm/test-fixtures'
import { createTestIpcServer } from '@pnpm/test-ipc-server'
const f = fixtures(import.meta.dirname)
const pkgResolutionId = 'https://codeload.example.com/org/repo/tar.gz/0000000000000000000000000000000000000000'
const allowBuild = () => true
const allowRegistryArtifactsOnly = (depPath: string) => !depPath.includes('://')
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({ allowBuild }, tmp, '')
await preparePackage({ allowBuild, pkgResolutionId }, tmp, '')
expect(server.getLines()).toStrictEqual([
'prepublish',
])
})
test('prepare package gates the build on the artifact depPath', async () => {
const tmp = tempDir()
f.copy('has-prepublish-script', tmp)
await expect(preparePackage({
allowBuild: allowRegistryArtifactsOnly,
pkgResolutionId,
}, tmp, '')).rejects.toThrow('needs to execute build scripts but is not in the "allowBuilds" allowlist')
})
test('prepare package does not run the prepublish script if the main file is present', async () => {
const tmp = tempDir()
await using server = await createTestIpcServer(path.join(tmp, 'test.sock'))
f.copy('has-prepublish-script-and-main-file', tmp)
await preparePackage({ allowBuild }, tmp, '')
await preparePackage({ allowBuild, pkgResolutionId }, tmp, '')
expect(server.getLines()).toStrictEqual([
'prepublish',
])
@@ -33,7 +45,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({ allowBuild }, tmp, 'packages/foo')
await preparePackage({ allowBuild, pkgResolutionId }, tmp, 'packages/foo')
expect(server.getLines()).toStrictEqual([
'prepublish',
])

View File

@@ -36,6 +36,7 @@
"@pnpm/exec.prepare-package": "workspace:*",
"@pnpm/fetching.fetcher-base": "workspace:*",
"@pnpm/fs.packlist": "workspace:*",
"@pnpm/resolving.git-resolver": "workspace:*",
"@pnpm/store.index": "workspace:*",
"@zkochan/rimraf": "catalog:",
"execa": "catalog:"

View File

@@ -8,6 +8,7 @@ import { preparePackage } from '@pnpm/exec.prepare-package'
import type { GitFetcher } from '@pnpm/fetching.fetcher-base'
import { packlist } from '@pnpm/fs.packlist'
import { globalWarn } from '@pnpm/logger'
import { createGitHostedPkgId } from '@pnpm/resolving.git-resolver'
import type { StoreIndex } from '@pnpm/store.index'
import { addFilesFromDir } from '@pnpm/worker'
import { rimraf } from '@zkochan/rimraf'
@@ -47,6 +48,7 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G
const prepareResult = await preparePackage({
allowBuild: opts.allowBuild,
ignoreScripts: createOpts.ignoreScripts,
pkgResolutionId: createGitHostedPkgId(resolution),
unsafePerm: createOpts.unsafePerm,
userAgent: createOpts.userAgent,
}, tempLocation, resolution.path ?? '')

View File

@@ -139,7 +139,7 @@ test('fetch a package from Git that has a prepare script', async () => {
type: 'git',
},
{
allowBuild: (pkgName) => pkgName === 'test-git-fetch',
allowBuild: (depPath) => depPath.startsWith('test-git-fetch@'),
filesIndexFile: path.join(storeDir, 'index.json'),
}
)
@@ -221,7 +221,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',
allowBuild: (depPath) => depPath.startsWith('@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`')
@@ -306,7 +306,7 @@ test('allow git package with prepare script', async () => {
repo: 'https://github.com/pnpm-e2e/prepare-script-works.git',
type: 'git',
}, {
allowBuild: (pkgName) => pkgName === '@pnpm.e2e/prepare-script-works',
allowBuild: (depPath) => depPath.startsWith('@pnpm.e2e/prepare-script-works@'),
filesIndexFile: path.join(storeDir, 'index.json'),
})
expect(filesMap.has('package.json')).toBeTruthy()

View File

@@ -24,6 +24,9 @@
{
"path": "../../fs/packlist"
},
{
"path": "../../resolving/git-resolver"
},
{
"path": "../../store/cafs"
},

View File

@@ -83,6 +83,7 @@ async function prepareGitHostedPkg (
const { shouldBeBuilt, pkgDir } = await preparePackage({
...opts,
allowBuild: fetcherOpts.allowBuild,
pkgResolutionId: createGitHostedTarballPkgResolutionId(resolution),
}, tempLocation, resolution.path ?? '')
const files = await packlist(pkgDir)
const { storeIndex } = opts
@@ -123,3 +124,11 @@ async function prepareGitHostedPkg (
ignoredBuild: Boolean(opts.ignoreScripts),
}
}
function createGitHostedTarballPkgResolutionId (resolution: Resolution): string {
let pkgResolutionId = resolution.tarball
if (resolution.path) {
pkgResolutionId += `#path:${resolution.path}`
}
return pkgResolutionId
}

View File

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

View File

@@ -34,6 +34,7 @@
"dependencies": {
"@inquirer/prompts": "catalog:",
"@pnpm/building.after-install": "workspace:*",
"@pnpm/building.policy": "workspace:*",
"@pnpm/catalogs.types": "workspace:*",
"@pnpm/cli.command": "workspace:*",
"@pnpm/cli.common-cli-options-help": "workspace:*",

View File

@@ -1,5 +1,5 @@
import { allowBuildKeyFromIgnoredBuild } from '@pnpm/building.policy'
import { writeSettings } from '@pnpm/config.writer'
import { parse } from '@pnpm/deps.path'
import {
IgnoredBuildsError,
} from '@pnpm/installing.deps-installer'
@@ -47,5 +47,5 @@ async function writeIgnoredBuildsToAllowBuilds (
}
function packageNamesFromIgnoredBuilds (ignoredBuilds: IgnoredBuilds): string[] {
return Array.from(new Set(Array.from(ignoredBuilds).map((dp) => parse(dp).name ?? dp))).sort(lexCompare)
return Array.from(new Set(Array.from(ignoredBuilds).map(allowBuildKeyFromIgnoredBuild))).sort(lexCompare)
}

View File

@@ -24,6 +24,9 @@
{
"path": "../../building/after-install"
},
{
"path": "../../building/policy"
},
{
"path": "../../catalogs/types"
},

View File

@@ -465,7 +465,7 @@ export async function mutateModules (
let ignoredBuilds = result.ignoredBuilds
if (!opts.ignoreScripts && ignoredBuilds?.size) {
ignoredBuilds = await runUnignoredDependencyBuilds(opts, ignoredBuilds, allowBuild)
ignoredBuilds = await runUnignoredDependencyBuilds(opts, ignoredBuilds, ctx.wantedLockfile, allowBuild)
}
// Detect packages whose build approval was revoked between the previous
// and current install. A package is considered revoked when it was
@@ -481,9 +481,12 @@ export async function mutateModules (
if (oldAllowBuild) {
for (const depPath of Object.keys(ctx.wantedLockfile.packages) as DepPath[]) {
if (ignoredBuilds?.has(depPath)) continue
const { name, version } = dp.parse(depPath)
if (!name || !version) continue
if (oldAllowBuild(name, version) === true && allowBuild?.(name, version) === undefined) {
// The old policy is evaluated with identity trust overridden so that
// package-name approvals count as they did when they were granted,
// even for git/tarball artifacts that the current policy no longer
// approves by name.
if (oldAllowBuild(depPath, { trustPackageIdentity: true }) !== true) continue
if (allowBuild?.(depPath) === undefined) {
ignoredBuilds ??= new Set()
ignoredBuilds.add(depPath)
}
@@ -1091,6 +1094,7 @@ Note that in CI environments, this setting is enabled by default.`,
async function runUnignoredDependencyBuilds (
opts: StrictInstallOptions,
previousIgnoredBuilds: IgnoredBuilds,
currentLockfile: LockfileObject,
allowBuild?: AllowBuild
): Promise<Set<DepPath>> {
if (!allowBuild) {
@@ -1098,12 +1102,10 @@ async function runUnignoredDependencyBuilds (
}
const pkgsToBuild: string[] = []
for (const ignoredPkg of previousIgnoredBuilds) {
const parsed = dp.parse(ignoredPkg)
if (!parsed.name || !parsed.version) continue
const allowed = allowBuild(parsed.name, parsed.version)
if (allowed === true) {
if (currentLockfile.packages?.[ignoredPkg] == null) continue
if (allowBuild(ignoredPkg) === true) {
// Package is explicitly allowed - rebuild it
pkgsToBuild.push(`${parsed.name}@${parsed.version}`)
pkgsToBuild.push(dp.getPkgIdWithPatchHash(ignoredPkg))
}
}
if (pkgsToBuild.length) {
@@ -2049,7 +2051,7 @@ export class IgnoredBuildsError extends PnpmError {
}
function dedupePackageNamesFromIgnoredBuilds (ignoredBuilds: IgnoredBuilds): string[] {
return Array.from(new Set(Array.from(ignoredBuilds ?? []).map(dp.removeSuffix))).sort(lexCompare)
return Array.from(new Set(Array.from(ignoredBuilds ?? []).map(depPath => dp.getPkgIdWithPatchHash(depPath)))).sort(lexCompare)
}
/**

View File

@@ -486,7 +486,7 @@ async function linkAllPkgs (
depNode.requiresBuild = files.requiresBuild
let sideEffectsCacheKey: string | undefined
if (opts.sideEffectsCacheRead && files.sideEffectsMaps && !isEmpty(files.sideEffectsMaps)) {
if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
if (opts.allowBuild?.(depNode.depPath) === true) {
sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.depPath, {
includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
patchFileHash: depNode.patch?.hash,

View File

@@ -2,6 +2,7 @@ import { hashObject } from '@pnpm/crypto.object-hasher'
import type { LockfileObject } from '@pnpm/lockfile.fs'
import type { ResolutionVerifier } from '@pnpm/resolving.resolver-base'
import { withResolutionShapeCacheIdentity } from './verifyLockfileResolutions.js'
import { recordVerification } from './verifyLockfileResolutionsCache.js'
export interface RecordLockfileVerifiedOptions {
@@ -30,7 +31,7 @@ export function recordLockfileVerified (opts: RecordLockfileVerifiedOptions): vo
if (!opts.lockfile.packages) return
recordVerification(opts.cacheDir, {
lockfilePath: opts.lockfilePath,
verifiers: opts.resolutionVerifiers,
verifiers: withResolutionShapeCacheIdentity(opts.resolutionVerifiers),
hashLockfile: () => hashObject(opts.lockfile),
})
}

View File

@@ -2,7 +2,7 @@ import { lockfileVerificationLogger } from '@pnpm/core-loggers'
import { hashObject } from '@pnpm/crypto.object-hasher'
import { PnpmError } from '@pnpm/error'
import type { LockfileObject } from '@pnpm/lockfile.fs'
import { nameVerFromPkgSnapshot } from '@pnpm/lockfile.utils'
import { isGitHostedTarballUrl, nameVerFromPkgSnapshot } from '@pnpm/lockfile.utils'
import type {
Resolution,
ResolutionPolicyViolation,
@@ -14,6 +14,7 @@ import pLimit from 'p-limit'
import {
recordVerification,
tryLockfileVerificationCache,
type VerifierCacheIdentity,
} from './verifyLockfileResolutionsCache.js'
// Re-exported for back-compat with the existing import surface.
@@ -31,6 +32,25 @@ const MAX_VIOLATIONS_TO_PRINT = 20
// verification pass doesn't push past what the rest of the install respects.
const DEFAULT_CONCURRENCY = 16
export const RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE = 'RESOLUTION_SHAPE_MISMATCH'
const RESOLUTION_SHAPE_CACHE_IDENTITY: VerifierCacheIdentity = {
policy: { resolutionShapeCheck: true },
canTrustPastCheck: (cached) => cached.resolutionShapeCheck === true,
}
/**
* Every verifier list that flows into the verification cache must carry
* the resolution-shape identity, so records written before the shape rule
* existed cannot stat-fast-path around it. Used by the gate itself and by
* {@link recordLockfileVerified}, whose freshly-resolved lockfile satisfies
* the shape invariant by construction (the writer derives every key from
* the resolution it just produced).
*/
export function withResolutionShapeCacheIdentity (verifiers: readonly VerifierCacheIdentity[]): VerifierCacheIdentity[] {
return [...verifiers, RESOLUTION_SHAPE_CACHE_IDENTITY]
}
export interface VerifyLockfileResolutionsOptions {
concurrency?: number
/**
@@ -78,7 +98,6 @@ export async function verifyLockfileResolutions (
verifiers: ResolutionVerifier[],
options?: VerifyLockfileResolutionsOptions
): Promise<void> {
if (verifiers.length === 0) return
if (!lockfile.packages) return
// Caching kicks in only when the caller surfaced both a writable
@@ -89,6 +108,8 @@ export async function verifyLockfileResolutions (
? { cacheDir: options.cacheDir, lockfilePath: options.lockfilePath }
: undefined
const cacheVerifiers = withResolutionShapeCacheIdentity(verifiers)
// Cache lookup runs before any registry I/O — the fast path is a
// single stat() of the lockfile when the previous install already
// verified it under a policy that's at least as strict as today's.
@@ -106,7 +127,7 @@ export async function verifyLockfileResolutions (
if (cache) {
const result = tryLockfileVerificationCache(cache.cacheDir, {
lockfilePath: cache.lockfilePath,
verifiers,
verifiers: cacheVerifiers,
hashLockfile,
})
if (result.hit) return
@@ -120,12 +141,16 @@ export async function verifyLockfileResolutions (
// A degenerate lockfile where every snapshot fails the
// name/version extraction (so candidates is empty) skips emission
// entirely — no work, no noise.
const candidates = collectCandidates(lockfile)
const { candidates, shapeViolations } = collectCandidates(lockfile)
if (shapeViolations.length > 0) {
throw buildVerificationError(shapeViolations)
}
if (verifiers.length === 0) return
if (candidates.size === 0) {
if (cache) {
recordVerification(cache.cacheDir, {
lockfilePath: cache.lockfilePath,
verifiers,
verifiers: cacheVerifiers,
hashLockfile,
}, cachePrecomputed)
}
@@ -152,7 +177,7 @@ export async function verifyLockfileResolutions (
if (cache) {
recordVerification(cache.cacheDir, {
lockfilePath: cache.lockfilePath,
verifiers,
verifiers: cacheVerifiers,
hashLockfile,
}, cachePrecomputed)
}
@@ -225,7 +250,49 @@ export async function collectResolutionPolicyViolations (
): Promise<ResolutionPolicyViolation[]> {
if (verifiers.length === 0) return []
if (!lockfile.packages) return []
return iterateLockfileViolations(collectCandidates(lockfile), verifiers, options?.concurrency)
// Shape violations are deliberately not collected here: they are hard
// tampering failures, not policy picks a caller may auto-exclude.
return iterateLockfileViolations(collectCandidates(lockfile).candidates, verifiers, options?.concurrency)
}
function isRegistryShapedResolution (resolution: unknown): boolean {
if (resolution == null) return true
if (typeof resolution !== 'object') return false
const { type, gitHosted, tarball, variants } = resolution as {
type?: unknown
gitHosted?: unknown
tarball?: unknown
variants?: unknown
}
if (type === 'variations') {
return Array.isArray(variants) && variants.every(
(variant) => isRegistryShapedResolution((variant as { resolution?: unknown })?.resolution)
)
}
// Custom resolver protocols (`type: 'custom:*'`) are a legitimate
// non-registry source the user opted into. They can only be materialized by
// a project-configured custom fetcher — an unrecognized custom type throws at
// fetch time (see @pnpm/fetching.pick-fetcher) — so a forged custom type
// cannot launder an artifact past this gate into a build.
if (typeof type === 'string' && type.startsWith('custom:')) return true
if (type != null) return false
// Plain tarball / registry resolution. The lockfile is parsed from YAML
// without schema validation, so the `gitHosted` flag is not trustworthy on
// its own: a tampered entry could set a non-boolean (dodging a strict
// `=== true`) or an explicit `false` on a git-host URL (the loader only
// backfills the flag when absent). Treat any non-boolean flag as git-hosted
// and gate on the URL so the verdict never depends on the flag alone.
if (gitHosted != null && (typeof gitHosted !== 'boolean' || gitHosted)) return false
// A registry resolution reconstructs its tarball URL from name+version, so
// an absent/empty `tarball` is registry-shaped. When a URL is present it
// must be an http(s) registry artifact: the npm verifier's tarball-URL
// binding skips non-http(s) schemes (file:, etc.), so a `file:` tarball
// under a name@semver key would otherwise be trusted with no safety net.
if (typeof tarball === 'string' && tarball !== '') {
if (!/^https?:\/\//i.test(tarball)) return false
if (isGitHostedTarballUrl(tarball)) return false
}
return true
}
interface Candidate {
@@ -248,11 +315,26 @@ interface Candidate {
// skips the tarball/policy checks, so if the wrong shape wins the dedup the
// surviving entry is verified under the wrong rules and the real one is
// never checked.
function collectCandidates (lockfile: LockfileObject): Map<string, Candidate> {
function collectCandidates (lockfile: LockfileObject): { candidates: Map<string, Candidate>, shapeViolations: ResolutionPolicyViolation[] } {
const candidates = new Map<string, Candidate>()
const shapeViolations: ResolutionPolicyViolation[] = []
for (const [depPath, snapshot] of Object.entries(lockfile.packages ?? {})) {
const { name, version, nonSemverVersion } = nameVerFromPkgSnapshot(depPath as DepPath, snapshot)
if (!name || !version) continue
// A registry-style depPath (name@semver) must be backed by a
// registry-shaped resolution: the allowBuilds policy derives a
// trusted package identity from that key shape, which is only sound
// while this invariant holds. The check is offline, so it applies
// even when no policy verifiers are active.
if (nonSemverVersion == null && !isRegistryShapedResolution(snapshot.resolution)) {
shapeViolations.push({
name,
version,
resolution: snapshot.resolution as Resolution,
code: RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE,
reason: 'a registry-style dependency path is backed by a non-registry resolution',
})
}
const key = `${name}@${version}@${nonSemverVersion ?? ''}@${JSON.stringify(snapshot.resolution)}`
candidates.set(key, {
name,
@@ -261,7 +343,7 @@ function collectCandidates (lockfile: LockfileObject): Map<string, Candidate> {
resolution: snapshot.resolution as Resolution,
})
}
return candidates
return { candidates, shapeViolations }
}
async function iterateLockfileViolations (

View File

@@ -1,5 +1,6 @@
import fs from 'node:fs'
import * as path from 'node:path'
import { pathToFileURL } from 'node:url'
import { expect, jest, test } from '@jest/globals'
import { assertProject } from '@pnpm/assert-project'
@@ -10,17 +11,26 @@ import {
mutateModules,
mutateModulesInSingleProject,
} from '@pnpm/installing.deps-installer'
import { streamParser } from '@pnpm/logger'
import { prepareEmpty, preparePackages } from '@pnpm/prepare'
import { createTestIpcServer } from '@pnpm/test-ipc-server'
import { REGISTRY_MOCK_PORT } from '@pnpm/testing.registry-mock'
import type { ProjectRootDir } from '@pnpm/types'
import { restartWorkerPool } from '@pnpm/worker'
import { rimrafSync } from '@zkochan/rimraf'
import { safeExeca as execa } from 'execa'
import isWindows from 'is-windows'
import { loadJsonFileSync } from 'load-json-file'
import PATH from 'path-name'
import { testDefaults } from '../utils/index.js'
// Until a first `data` listener appears, the paused log stream buffers
// every event, and the first test to attach a reporter receives the
// whole backlog from earlier installs that ran without a reporter. Keep
// the stream flowing so each reporter only sees its own installs' events.
streamParser.on('data', () => {})
const testOnNonWindows = isWindows() ? test.skip : test
test('run pre/postinstall scripts', async () => {
@@ -310,10 +320,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()
const gitDependency = await createGitPreparePackage()
await addDependenciesToPackage({}, ['pnpm/test-git-fetch#8b333f12d5357f4f25a654c305c826294cb073bf'], testDefaults({
await addDependenciesToPackage({}, [gitDependency], testDefaults({
fastUnpack: false,
allowBuilds: { 'test-git-fetch': true },
allowBuilds: { [`test-git-fetch@${gitDependency}`]: true },
}))
const scripts = project.requireModule('test-git-fetch/output.json')
@@ -328,6 +339,112 @@ test('run prepare script for git-hosted dependencies', async () => {
])
})
async function createGitPreparePackage (): Promise<string> {
const repoDir = path.resolve('test-git-fetch-src')
fs.mkdirSync(repoDir)
fs.writeFileSync(path.join(repoDir, 'append.js'), [
'const fs = require(\'fs\')',
'const file = \'output.json\'',
'let scripts = []',
'try { scripts = JSON.parse(fs.readFileSync(file, \'utf8\')) } catch {}',
'scripts.push(process.argv[2])',
'fs.writeFileSync(file, JSON.stringify(scripts))',
'',
].join('\n'))
fs.writeFileSync(path.join(repoDir, 'index.js'), "module.exports = 'ok'\n")
fs.writeFileSync(path.join(repoDir, 'package.json'), JSON.stringify({
name: 'test-git-fetch',
version: '1.0.0',
main: 'index.js',
scripts: {
prepare: 'node append prepare',
preinstall: 'node append preinstall',
install: 'node append install',
postinstall: 'node append postinstall',
},
}))
fs.writeFileSync(path.join(repoDir, 'package-lock.json'), JSON.stringify({
name: 'test-git-fetch',
version: '1.0.0',
lockfileVersion: 3,
requires: true,
packages: {
'': {
name: 'test-git-fetch',
version: '1.0.0',
},
},
}))
await execa('git', ['init', '-q', '-b', 'main'], { cwd: repoDir })
await execa('git', ['config', 'user.email', 'test@example.invalid'], { cwd: repoDir })
await execa('git', ['config', 'user.name', 'Test'], { cwd: repoDir })
await execa('git', ['add', '-A'], { cwd: repoDir })
await execa('git', ['-c', 'commit.gpgsign=false', 'commit', '-q', '-m', 'init'], { cwd: repoDir })
const commit = String((await execa('git', ['rev-parse', 'HEAD'], { cwd: repoDir })).stdout).trim()
return `git+${pathToFileURL(repoDir).href}#${commit}`
}
test('allowBuilds does not run lifecycle scripts for direct tarball identities', async () => {
prepareEmpty()
const registries = {
default: `http://localhost:${REGISTRY_MOCK_PORT}/`,
'@direct': `http://127.0.0.1:${REGISTRY_MOCK_PORT}/`,
}
const tarball = `http://127.0.0.1:${REGISTRY_MOCK_PORT}/@pnpm.e2e/pre-and-postinstall-scripts-example/-/pre-and-postinstall-scripts-example-1.0.0.tgz`
const depPath = `@pnpm.e2e/pre-and-postinstall-scripts-example@${tarball}`
const { updatedManifest: manifest } = await addDependenciesToPackage({}, [tarball], testDefaults({
fastUnpack: false,
allowBuilds: { '@pnpm.e2e/pre-and-postinstall-scripts-example': true },
registries,
}, { registries }))
expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')).toBe(false)
expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBe(false)
rimrafSync('node_modules')
await install(manifest, testDefaults({
fastUnpack: false,
allowBuilds: { [depPath]: true },
registries,
}, { registries }))
expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-preinstall.js')).toBe(true)
expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBe(true)
})
test('surfaces a tarball artifact as an ignored build when its depPath approval is revoked', async () => {
prepareEmpty()
const registries = {
default: `http://localhost:${REGISTRY_MOCK_PORT}/`,
'@direct': `http://127.0.0.1:${REGISTRY_MOCK_PORT}/`,
}
const tarball = `http://127.0.0.1:${REGISTRY_MOCK_PORT}/@pnpm.e2e/pre-and-postinstall-scripts-example/-/pre-and-postinstall-scripts-example-1.0.0.tgz`
const depPath = `@pnpm.e2e/pre-and-postinstall-scripts-example@${tarball}`
// First install approves the artifact by its depPath, so the build runs and
// the approval is recorded in .modules.yaml.
const { updatedManifest: manifest } = await addDependenciesToPackage({}, [tarball], testDefaults({
fastUnpack: false,
allowBuilds: { [depPath]: true },
registries,
}, { registries }))
expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBe(true)
// Reinstalling with the approval removed must surface the artifact as an
// ignored build. This is the revocation path that iterates the lockfile by
// snapshot name/version, so it reaches non-semver artifact depPaths.
const { ignoredBuilds } = await install(manifest, testDefaults({
fastUnpack: false,
frozenLockfile: true,
allowBuilds: {},
registries,
}, { registries }))
expect(Array.from(ignoredBuilds ?? [])).toContain(depPath)
})
test('lifecycle scripts run before linking bins', async () => {
const project = prepareEmpty()

View File

@@ -142,15 +142,16 @@ test('dedupes peer/patch-suffix variants and invokes the verifier once per (name
test('does not collapse same (name, version) with different resolutions', async () => {
// Two entries sharing a name@version but pinned via different protocols
// (npm registry vs. git). If the dedup key were just `name@version` one
// would silently overwrite the other and a protocol-scoped verifier
// would short-circuit on the survivor — letting the real entry skip
// the gate.
// (npm registry vs. a URL-keyed tarball whose snapshot copied the same
// semver `version` from its manifest). If the dedup key were just
// `name@version` one would silently overwrite the other and a
// protocol-scoped verifier would short-circuit on the survivor —
// letting the real entry skip the gate.
const npmResolution = tarballResolution('sha512-a')
const gitResolution = { type: 'git', repo: 'x', commit: 'abc' }
const directTarballResolution = { integrity: 'sha512-b', tarball: 'https://example.com/foo.tgz' }
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: npmResolution },
'foo@1.0.0(peer-x)': { resolution: gitResolution },
'foo@https://example.com/foo.tgz': { resolution: directTarballResolution, version: '1.0.0' },
})
const seenResolutions: unknown[] = []
const verifier = wrap(async (resolution) => {
@@ -159,7 +160,7 @@ test('does not collapse same (name, version) with different resolutions', async
})
await verifyLockfileResolutions(lockfile, [verifier])
expect(seenResolutions).toEqual(expect.arrayContaining([npmResolution, gitResolution]))
expect(seenResolutions).toEqual(expect.arrayContaining([npmResolution, directTarballResolution]))
expect(seenResolutions).toHaveLength(2)
})
@@ -168,7 +169,7 @@ test('the verifier sees the resolution shape verbatim', async () => {
const gitResolution = { type: 'git', repo: 'x', commit: 'abc' }
const lockfile = makeLockfile({
'npm-pkg@1.0.0': { resolution: npmResolution },
'git-pkg@1.0.0': { resolution: gitResolution },
'git-pkg@git+https://example.com/git-pkg.git#abc': { resolution: gitResolution, version: '1.0.0' },
})
const received: unknown[] = []
const verifier = wrap(async (resolution) => {
@@ -290,3 +291,134 @@ test('does not write a cache record when verification rejects', async () => {
await fs.promises.rm(tmpDir, { recursive: true, force: true })
}
})
test('rejects a registry-style depPath backed by a git resolution, even with no verifiers', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { type: 'git', repo: 'https://example.com/foo.git', commit: 'abc123' } },
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
message: expect.stringMatching(/foo@1\.0\.0/),
})
})
test('rejects a registry-style depPath backed by a git-hosted tarball resolution', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://codeload.github.com/org/foo/tar.gz/abc123', gitHosted: true } },
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
})
test('rejects a registry-style depPath backed by a directory resolution', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { type: 'directory', directory: '../foo' } },
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
})
test('accepts registry-style depPaths with registry and all-registry variations resolutions', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: tarballResolution() },
'bar@1.0.0': {
resolution: {
type: 'variations',
variants: [
{ targets: [{ os: 'darwin' }], resolution: tarballResolution('sha512-a') },
{ targets: [{ os: 'linux' }], resolution: tarballResolution('sha512-b') },
],
},
},
})
await expect(verifyLockfileResolutions(lockfile, [])).resolves.toBeUndefined()
})
test('rejects a registry-style depPath whose variations resolution hides a git variant', async () => {
const lockfile = makeLockfile({
'bar@1.0.0': {
resolution: {
type: 'variations',
variants: [
{ targets: [{ os: 'darwin' }], resolution: tarballResolution('sha512-a') },
{ targets: [{ os: 'linux' }], resolution: { type: 'git', repo: 'https://example.com/bar.git', commit: 'abc123' } },
],
},
},
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
})
test('does not flag artifact depPaths with non-registry resolutions', async () => {
const lockfile = makeLockfile({
'foo@git+https://example.com/foo.git#abc123': { resolution: { type: 'git', repo: 'https://example.com/foo.git', commit: 'abc123' }, version: '1.0.0' },
'bar@https://example.com/bar.tgz': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://example.com/bar.tgz' }, version: '1.0.0' },
})
await expect(verifyLockfileResolutions(lockfile, [])).resolves.toBeUndefined()
})
test('rejects a registry-style depPath whose git-host tarball clears the gitHosted flag', async () => {
// A tampered lockfile sets a non-truthy gitHosted on a codeload URL to
// dodge a flag-only check. The URL itself must still flag it.
for (const gitHosted of [false, 'true', 'false', 0, 1]) {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://codeload.github.com/org/foo/tar.gz/abc123', gitHosted } as never },
})
// eslint-disable-next-line no-await-in-loop
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
}
})
test('rejects a registry-style depPath with a non-boolean gitHosted flag', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://registry.npmjs.org/foo/-/foo-1.0.0.tgz', gitHosted: 'true' } as never },
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
})
test('accepts a registry-style depPath backed by a custom resolver resolution', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { type: 'custom:cdn', source: 'foo' } as never },
})
await expect(verifyLockfileResolutions(lockfile, [])).resolves.toBeUndefined()
})
test('rejects a registry-style depPath backed by a non-http(s) tarball URL', async () => {
// The npm verifier skips non-http(s) tarballs, so a file: artifact under a
// semver key would be trusted with no tarball-URL binding to catch it.
for (const tarball of ['file:///tmp/evil.tgz', 'ftp://example.com/evil.tgz']) {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball } as never },
})
// eslint-disable-next-line no-await-in-loop
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
}
})
test('accepts a registry-style depPath whose tarball is an http(s) registry URL', async () => {
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://registry.npmjs.org/foo/-/foo-1.0.0.tgz' } as never },
})
await expect(verifyLockfileResolutions(lockfile, [])).resolves.toBeUndefined()
})
test('rejects a registry-style depPath whose git-host tarball varies the host casing', async () => {
// Hostnames are case-insensitive; an upper-case codeload host paired with
// gitHosted: false must not pass as registry-shaped.
const lockfile = makeLockfile({
'foo@1.0.0': { resolution: { integrity: 'sha512-deadbeef', tarball: 'https://CODELOAD.GITHUB.COM/org/foo/tar.gz/abc123', gitHosted: false } as never },
})
await expect(verifyLockfileResolutions(lockfile, [])).rejects.toMatchObject({
code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH',
})
})

View File

@@ -932,7 +932,7 @@ async function linkAllPkgs (
depNode.requiresBuild = filesResponse.requiresBuild
let sideEffectsCacheKey: string | undefined
if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
if (opts.allowBuild?.(depNode.depPath) === true) {
sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.dir, {
includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
patchFileHash: depNode.patch?.hash,

View File

@@ -135,7 +135,7 @@ async function linkAllPkgsInOrder (
depNode.requiresBuild = filesResponse.requiresBuild
let sideEffectsCacheKey: string | undefined
if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
if (opts.allowBuild?.(depNode.depPath) === true) {
sideEffectsCacheKey = calcDepState(graph, opts.depsStateCache, dir, {
includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
patchFileHash: depNode.patch?.hash,

View File

@@ -12,22 +12,10 @@ import type {
ResolvedDependencies,
TarballResolution,
} from '@pnpm/lockfile.types'
import { isGitHostedTarballUrl } from '@pnpm/lockfile.utils'
import { DEPENDENCIES_FIELDS, type DepPath } from '@pnpm/types'
import { isEmpty, map as _mapValues, omit, pick, pickBy } from 'ramda'
// Minimal duplicate of `isGitHostedPkgUrl` from `@pnpm/fetching.pick-fetcher`,
// inlined to avoid pulling the fetcher dep into the lockfile I/O layer. Used
// to enrich entries written by older pnpm versions (which didn't record the
// `gitHosted` field on TarballResolution) so every downstream reader can rely
// on the field directly.
function isGitHostedTarballUrl (url: string): boolean {
return (
url.startsWith('https://codeload.github.com/') ||
url.startsWith('https://bitbucket.org/') ||
url.startsWith('https://gitlab.com/')
) && url.includes('tar.gz')
}
export function convertToLockfileFile (lockfile: LockfileObject): LockfileFile {
const packages: Record<string, LockfilePackageInfo> = {}
const snapshots: Record<string, LockfilePackageSnapshot> = {}

View File

@@ -5,7 +5,7 @@ export { packageIdFromSnapshot } from './packageIdFromSnapshot.js'
export { packageIsIndependent } from './packageIsIndependent.js'
export { pkgSnapshotToResolution } from './pkgSnapshotToResolution.js'
export { refIsLocalDirectory, refIsLocalTarball } from './refIsLocalTarball.js'
export { toLockfileResolution } from './toLockfileResolution.js'
export { isGitHostedTarballUrl, toLockfileResolution } from './toLockfileResolution.js'
export * from '@pnpm/lockfile.types'
// for backward compatibility

View File

@@ -75,11 +75,16 @@ function preservingGitHosted<T extends { tarball?: string, integrity: string }>
// dep graph. Used as a fallback when callers haven't pre-set the
// `gitHosted` field on TarballResolution.
export function isGitHostedTarballUrl (url: string): boolean {
// Schemes and hostnames are case-insensitive, so match against a lowercased
// copy: a tampered `https://CODELOAD.GITHUB.COM/...` must not slip past as a
// non-git-hosted (and therefore registry-trusted) tarball. Only the
// lowercased copy is inspected; the original URL is never rewritten.
const lowerUrl = url.toLowerCase()
return (
url.startsWith('https://codeload.github.com/') ||
url.startsWith('https://bitbucket.org/') ||
url.startsWith('https://gitlab.com/')
) && url.includes('tar.gz')
lowerUrl.startsWith('https://codeload.github.com/') ||
lowerUrl.startsWith('https://bitbucket.org/') ||
lowerUrl.startsWith('https://gitlab.com/')
) && lowerUrl.includes('tar.gz')
}
function removeProtocol (url: string): string {

View File

@@ -16,7 +16,7 @@ use crate::{
cas_io::{ImportedFiles, import_into_cas},
error::{GitFetcherError, PreparePackageError},
packlist::packlist,
prepare_package::{PreparePackageOptions, PreparedPackage, prepare_package},
prepare_package::{AllowBuildRef, PreparePackageOptions, PreparedPackage, prepare_package},
};
use pacquet_executor::ScriptsPrependNodePath;
use pacquet_package_manifest::safe_read_package_json_from_dir;
@@ -44,7 +44,7 @@ pub struct GitFetcher<'a> {
/// `allow_build`. The caller (typically the install dispatcher) is
/// responsible for plumbing whatever policy structure it has into
/// this closure shape.
pub allow_build: &'a (dyn Fn(&str, &str) -> bool + Send + Sync),
pub allow_build: AllowBuildRef<'a>,
pub ignore_scripts: bool,
pub unsafe_perm: bool,
pub user_agent: Option<&'a str>,
@@ -146,7 +146,8 @@ impl<'a> GitFetcher<'a> {
// brittle to future expression-reshape edits in this block.
let empty_env: HashMap<String, String> = HashMap::new();
let prepare_opts = PreparePackageOptions {
allow_build: Box::new(|name, version| (self.allow_build)(name, version)),
allow_build: Box::new(|dep_path| (self.allow_build)(dep_path)),
dep_path: self.package_id,
ignore_scripts: self.ignore_scripts,
unsafe_perm: self.unsafe_perm,
user_agent: self.user_agent,

View File

@@ -1,5 +1,5 @@
use super::{GitFetcher, exec_git_with, extract_host, is_valid_commit_hash, should_use_shallow};
use crate::error::GitFetcherError;
use crate::{error::GitFetcherError, prepare_package::AllowBuildRef};
use pacquet_executor::ScriptsPrependNodePath;
use pacquet_reporter::SilentReporter;
use pacquet_store_dir::StoreDir;
@@ -47,8 +47,8 @@ fn make_bare_repo_with_prepare_script(tmp: &Path, prepare_script: &str) -> (Path
(bare, commit)
}
fn allow_all_builds<'a>() -> &'a (dyn Fn(&str, &str) -> bool + Send + Sync) {
&|_, _| true
fn allow_all_builds<'a>() -> AllowBuildRef<'a> {
&|_| true
}
/// Create a tiny bare git repo whose single commit ships a
@@ -77,8 +77,8 @@ fn make_bare_repo(tmp: &Path) -> (PathBuf, String) {
(bare, commit)
}
fn deny_all_builds<'a>() -> &'a (dyn Fn(&str, &str) -> bool + Send + Sync) {
&|_, _| false
fn deny_all_builds<'a>() -> AllowBuildRef<'a> {
&|_| false
}
#[test]
@@ -659,10 +659,9 @@ async fn fetcher_runs_prepare_when_allow_build_returns_true() {
let repo_url = format!("file://{}", bare.display());
// Targeted allow_build that returns true for *this* package only —
// catches a regression where the gate ignores the (name, version)
// pair and falls through to default-allow or default-deny.
let allow_x_only: &(dyn Fn(&str, &str) -> bool + Send + Sync) =
&|name, version| name == "x" && version == "1.0.0";
// catches a regression where the gate ignores the dep path and
// falls through to default-allow or default-deny.
let allow_x_only: AllowBuildRef<'_> = &|dep_path| dep_path == "x@1.0.0";
let received = GitFetcher {
repo: &repo_url,
@@ -699,6 +698,95 @@ async fn fetcher_runs_prepare_when_allow_build_returns_true() {
);
}
#[tokio::test(flavor = "multi_thread")]
async fn fetcher_rejects_untrusted_manifest_identity() {
let tmp = tempdir().unwrap();
let (bare, commit) = make_bare_repo_with_prepare_script(
tmp.path(),
r#"node -e "require('fs').writeFileSync('BUILD_RAN.marker', 'ok')""#,
);
let store_root = tempdir().unwrap();
let store_dir = StoreDir::from(store_root.path().to_path_buf());
let repo_url = format!("file://{}", bare.display());
let allow_registry_artifacts_only: AllowBuildRef<'_> = &|dep_path| !dep_path.contains("://");
let err = GitFetcher {
repo: &repo_url,
commit: &commit,
path: None,
git_shallow_hosts: &[],
allow_build: allow_registry_artifacts_only,
ignore_scripts: false,
unsafe_perm: true,
user_agent: None,
scripts_prepend_node_path: ScriptsPrependNodePath::Never,
script_shell: None,
node_execpath: None,
npm_execpath: None,
store_dir: &store_dir,
package_id: "x@git+file:///tmp/repo.git#abc123",
requester: "/test",
store_index_writer: None,
files_index_file: "x@git+file:///tmp/repo.git#abc123\tbuilt",
git_bin: None,
}
.run::<SilentReporter>()
.await
.unwrap_err();
match err {
GitFetcherError::Prepare(crate::error::PreparePackageError::NotAllowed {
name,
version,
}) => {
assert_eq!(name, "x");
assert_eq!(version, "1.0.0");
}
other => panic!("expected NotAllowed, got {other:?}"),
}
}
#[tokio::test(flavor = "multi_thread")]
async fn fetcher_allows_untrusted_manifest_identity_by_dep_path() {
let tmp = tempdir().unwrap();
let (bare, commit) = make_bare_repo_with_prepare_script(
tmp.path(),
r#"node -e "require('fs').writeFileSync('BUILD_RAN.marker', 'ok')""#,
);
let store_root = tempdir().unwrap();
let store_dir = StoreDir::from(store_root.path().to_path_buf());
let repo_url = format!("file://{}", bare.display());
let package_id = "x@git+file:///tmp/repo.git#abc123";
let allow_dep_path: AllowBuildRef<'_> = &|dep_path| dep_path == package_id;
let received = GitFetcher {
repo: &repo_url,
commit: &commit,
path: None,
git_shallow_hosts: &[],
allow_build: allow_dep_path,
ignore_scripts: false,
unsafe_perm: true,
user_agent: None,
scripts_prepend_node_path: ScriptsPrependNodePath::Never,
script_shell: None,
node_execpath: None,
npm_execpath: None,
store_dir: &store_dir,
package_id,
requester: "/test",
store_index_writer: None,
files_index_file: "x@git+file:///tmp/repo.git#abc123\tbuilt",
git_bin: None,
}
.run::<SilentReporter>()
.await
.unwrap();
assert!(received.built);
assert!(received.cas_paths.contains_key("BUILD_RAN.marker"));
}
/// Write a `git` shim shell script to `dir/git` that:
///
/// - Appends every invocation to `$PACQUET_GIT_SHIM_LOG` as one

View File

@@ -32,20 +32,20 @@ use std::{
/// nor Yarn run it for git-hosted deps.
const PREPUBLISH_SCRIPTS: &[&str] = &["prepublish", "prepack", "publish"];
/// Closure shape used to ask the install policy whether a package
/// (`name`, `version`) is allowed to run lifecycle scripts. Mirrors
/// upstream's `opts.allowBuild?.(name, version)` at
/// [`index.ts:36`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts#L36).
/// Closure shape used to ask the install policy whether the package at
/// a dep path is allowed to run lifecycle scripts.
///
/// We pass a closure rather than `&AllowBuildPolicy` so the
/// `pacquet-git-fetcher` crate stays free of a back-edge into
/// `pacquet-package-manager`. The caller adapts whatever policy
/// structure it has into this shape.
pub type AllowBuildFn<'a> = Box<dyn Fn(&str, &str) -> bool + Send + Sync + 'a>;
pub type AllowBuildFn<'a> = Box<dyn Fn(&str) -> bool + Send + Sync + 'a>;
pub type AllowBuildRef<'a> = &'a (dyn Fn(&str) -> bool + Send + Sync);
/// Caller-supplied context for [`prepare_package`].
pub struct PreparePackageOptions<'a> {
pub allow_build: AllowBuildFn<'a>,
pub dep_path: &'a str,
pub ignore_scripts: bool,
pub unsafe_perm: bool,
pub user_agent: Option<&'a str>,
@@ -96,11 +96,13 @@ pub fn prepare_package<Reporter: self::Reporter>(
}
// `allowBuild` check before any spawn. Upstream throws when
// `opts.allowBuild?.(name, version)` is missing or false, with
// GIT_DEP_PREPARE_NOT_ALLOWED.
// `opts.allowBuild?.(depPath)` is missing or false, with
// GIT_DEP_PREPARE_NOT_ALLOWED. The manifest comes from the fetched
// artifact itself, so its name and version only feed the error
// message; the dep path is the gated identity.
let name = manifest.get("name").and_then(Value::as_str).unwrap_or("");
let version = manifest.get("version").and_then(Value::as_str).unwrap_or("");
if !(opts.allow_build)(name, version) {
if !(opts.allow_build)(opts.dep_path) {
return Err(PreparePackageError::NotAllowed {
name: name.to_string(),
version: version.to_string(),

View File

@@ -31,7 +31,8 @@ fn write_manifest(dir: &Path, manifest: &serde_json::Value) {
fn opts<'a>(allow: bool, ignore_scripts: bool) -> PreparePackageOptions<'a> {
static EMPTY_BIN_PATHS: &[std::path::PathBuf] = &[];
PreparePackageOptions {
allow_build: Box::new(move |_name, _version| allow),
allow_build: Box::new(move |_dep_path| allow),
dep_path: "x@https://example.com/x.tgz",
ignore_scripts,
unsafe_perm: true,
user_agent: None,
@@ -44,6 +45,40 @@ fn opts<'a>(allow: bool, ignore_scripts: bool) -> PreparePackageOptions<'a> {
}
}
fn opts_allow_registry_artifacts_only<'a>() -> PreparePackageOptions<'a> {
static EMPTY_BIN_PATHS: &[std::path::PathBuf] = &[];
PreparePackageOptions {
allow_build: Box::new(move |dep_path| !dep_path.contains("://")),
dep_path: "x@https://example.com/x.tgz",
ignore_scripts: false,
unsafe_perm: true,
user_agent: None,
scripts_prepend_node_path: ScriptsPrependNodePath::Never,
script_shell: None,
node_execpath: None,
npm_execpath: None,
extra_bin_paths: EMPTY_BIN_PATHS,
extra_env: empty_env(),
}
}
fn opts_allow_dep_path<'a>(dep_path: &'a str) -> PreparePackageOptions<'a> {
static EMPTY_BIN_PATHS: &[std::path::PathBuf] = &[];
PreparePackageOptions {
allow_build: Box::new(move |actual_dep_path| actual_dep_path == dep_path),
dep_path,
ignore_scripts: false,
unsafe_perm: true,
user_agent: None,
scripts_prepend_node_path: ScriptsPrependNodePath::Never,
script_shell: None,
node_execpath: None,
npm_execpath: None,
extra_bin_paths: EMPTY_BIN_PATHS,
extra_env: empty_env(),
}
}
#[test]
fn package_should_be_built_false_for_empty_scripts() {
let dir = tempdir().unwrap();
@@ -143,6 +178,50 @@ fn prepare_rejects_when_allow_build_returns_false() {
}
}
#[test]
fn prepare_rejects_untrusted_manifest_identity() {
let dir = tempdir().unwrap();
write_manifest(
dir.path(),
&json!({
"name": "naughty", "version": "1.0.0",
"scripts": { "prepare": "tsc" },
}),
);
let err =
prepare_package::<SilentReporter>(&opts_allow_registry_artifacts_only(), dir.path(), None)
.unwrap_err();
match err {
PreparePackageError::NotAllowed { name, version } => {
assert_eq!(name, "naughty");
assert_eq!(version, "1.0.0");
}
other => panic!("expected NotAllowed, got {other:?}"),
}
}
#[test]
fn prepare_allows_untrusted_manifest_identity_by_dep_path() {
let dir = tempdir().unwrap();
write_manifest(
dir.path(),
&json!({
"name": "trusted-name",
"version": "1.0.0",
"scripts": { "prepack": r#"node -e "require('fs').writeFileSync('built.txt', 'ok')""# },
}),
);
let dep_path = "trusted-name@git+https://example.com/org/repo.git#abc123";
let result =
prepare_package::<SilentReporter>(&opts_allow_dep_path(dep_path), dir.path(), None)
.expect("depPath-specific allow should permit prepare");
assert!(result.should_be_built);
assert!(dir.path().join("built.txt").exists());
}
#[test]
fn safe_join_path_rejects_escapes() {
let dir = tempdir().unwrap();

View File

@@ -33,7 +33,7 @@ use crate::{
error::GitFetcherError,
fetcher::GitFetchOutput,
packlist::packlist,
prepare_package::{PreparePackageOptions, PreparedPackage, prepare_package},
prepare_package::{AllowBuildRef, PreparePackageOptions, PreparedPackage, prepare_package},
};
use pacquet_executor::ScriptsPrependNodePath;
use pacquet_package_manifest::safe_read_package_json_from_dir;
@@ -63,7 +63,7 @@ pub struct GitHostedTarballFetcher<'a> {
/// `None` packs the tarball root.
pub path: Option<&'a str>,
/// Routed through to [`crate::prepare_package()`]'s `allow_build`.
pub allow_build: &'a (dyn Fn(&str, &str) -> bool + Send + Sync),
pub allow_build: AllowBuildRef<'a>,
pub ignore_scripts: bool,
pub unsafe_perm: bool,
pub user_agent: Option<&'a str>,
@@ -108,7 +108,8 @@ impl<'a> GitHostedTarballFetcher<'a> {
// `should_be_built` flag.
let empty_env: HashMap<String, String> = HashMap::new();
let prepare_opts = PreparePackageOptions {
allow_build: Box::new(|name, version| (self.allow_build)(name, version)),
allow_build: Box::new(|dep_path| (self.allow_build)(dep_path)),
dep_path: self.package_id,
ignore_scripts: self.ignore_scripts,
unsafe_perm: self.unsafe_perm,
user_agent: self.user_agent,

View File

@@ -1,13 +1,16 @@
use super::GitHostedTarballFetcher;
use crate::error::{GitFetcherError, PreparePackageError};
use crate::{
error::{GitFetcherError, PreparePackageError},
prepare_package::AllowBuildRef,
};
use pacquet_executor::ScriptsPrependNodePath;
use pacquet_reporter::SilentReporter;
use pacquet_store_dir::{StoreDir, StoreIndex, StoreIndexWriter};
use std::{collections::HashMap, fs, path::PathBuf, sync::Arc};
use tempfile::tempdir;
fn deny_all_builds<'a>() -> &'a (dyn Fn(&str, &str) -> bool + Send + Sync) {
&|_, _| false
fn deny_all_builds<'a>() -> AllowBuildRef<'a> {
&|_| false
}
/// Build the `cas_paths` map the dispatcher would hand the fetcher

View File

@@ -68,6 +68,17 @@ pub enum VerifyError {
breakdown: String,
},
/// Every violation tripped `RESOLUTION_SHAPE_MISMATCH` — a
/// registry-style dependency path backed by a non-registry
/// resolution.
#[display("{count} lockfile entries failed verification:\n{breakdown}")]
#[diagnostic(code(ERR_PNPM_RESOLUTION_SHAPE_MISMATCH), help("{HINT}"))]
ResolutionShapeMismatch {
#[error(not(source))]
count: usize,
breakdown: String,
},
/// Mixed batch — at least two distinct violation codes — so the
/// throw code escalates to the generic
/// `LOCKFILE_RESOLUTION_VERIFICATION` and each entry's code goes
@@ -134,6 +145,9 @@ impl VerifyError {
pacquet_resolving_npm_resolver_violation_codes::TRUST_DOWNGRADE => {
VerifyError::TrustDowngrade { count, breakdown }
}
crate::RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE => {
VerifyError::ResolutionShapeMismatch { count, breakdown }
}
// Unknown verifier code (future-proofing): fall back
// to the generic envelope rather than fabricating a
// variant we don't have.

View File

@@ -38,6 +38,6 @@ pub use errors::{RenderedViolation, VerifyError};
pub use hash_lockfile::hash_lockfile;
pub use record_lockfile_verified::record_lockfile_verified;
pub use verify_lockfile_resolutions::{
VerifyLockfileResolutionsOptions, collect_resolution_policy_violations,
verify_lockfile_resolutions,
RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE, VerifyLockfileResolutionsOptions,
collect_resolution_policy_violations, verify_lockfile_resolutions,
};

View File

@@ -20,7 +20,10 @@ use std::{path::Path, sync::Arc};
use pacquet_lockfile::Lockfile;
use pacquet_resolving_resolver_base::ResolutionVerifier;
use crate::{cache::record_verification, hash_lockfile};
use crate::{
cache::record_verification, hash_lockfile,
verify_lockfile_resolutions::with_resolution_shape_cache_identity,
};
/// Persist the post-resolution lockfile as already-verified.
/// Inputs match upstream's `RecordLockfileVerifiedOptions`:
@@ -46,7 +49,7 @@ pub fn record_lockfile_verified(
record_verification(
cache_dir,
lockfile_path,
verifiers,
&with_resolution_shape_cache_identity(verifiers),
|| hash_lockfile(lockfile),
Default::default(),
);

View File

@@ -19,12 +19,12 @@
use std::{collections::BTreeMap, path::Path, sync::Arc, time::Instant};
use futures_util::{StreamExt, stream::FuturesUnordered};
use pacquet_lockfile::{Lockfile, LockfileResolution, PkgName};
use pacquet_lockfile::{Lockfile, LockfileResolution, PkgName, is_git_hosted_tarball_url};
use pacquet_reporter::{
LockfileVerificationLog, LockfileVerificationMessage, LogEvent, LogLevel, Reporter,
};
use pacquet_resolving_resolver_base::{
ResolutionPolicyViolation, ResolutionVerification, ResolutionVerifier, VerifyCtx,
ResolutionPolicyViolation, ResolutionVerification, ResolutionVerifier, VerifyCtx, VerifyFuture,
};
use tokio::sync::Semaphore;
@@ -82,7 +82,7 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
verifiers: &[Arc<dyn ResolutionVerifier>],
opts: &VerifyLockfileResolutionsOptions<'_>,
) -> Result<(), VerifyError> {
if verifiers.is_empty() || lockfile.packages.is_none() {
if lockfile.packages.is_none() {
return Ok(());
}
@@ -93,6 +93,8 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
// logic via the same code path).
let cache_inputs = opts.cache_dir.zip(opts.lockfile_path);
let cache_verifiers = with_resolution_shape_cache_identity(verifiers);
// Memoised content hash. Used by both the lookup (when the
// stat-shortcut doesn't apply) and the recorder (after the
// gate passes). The closure is `FnMut` so multiple lazy calls
@@ -109,15 +111,25 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
let mut cache_precomputed: CachePrecomputed = CachePrecomputed::default();
if let Some((cache_dir, lockfile_path)) = cache_inputs {
let result =
try_lockfile_verification_cache(cache_dir, lockfile_path, verifiers, &mut hash_once);
let result = try_lockfile_verification_cache(
cache_dir,
lockfile_path,
&cache_verifiers,
&mut hash_once,
);
if result.hit {
return Ok(());
}
cache_precomputed = result.precomputed;
}
let candidates = collect_candidates(lockfile);
let (candidates, shape_violations) = collect_candidates(lockfile);
if !shape_violations.is_empty() {
return Err(build_verification_error(shape_violations));
}
if verifiers.is_empty() {
return Ok(());
}
let lockfile_path_str = opts.lockfile_path.map(|path| path.to_string_lossy().into_owned());
if candidates.is_empty() {
// Persist the success so the next install can stat-only the
@@ -128,7 +140,7 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
record_verification(
cache_dir,
lockfile_path,
verifiers,
&cache_verifiers,
&mut hash_once,
cache_precomputed,
);
@@ -161,7 +173,7 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
record_verification(
cache_dir,
lockfile_path,
verifiers,
&cache_verifiers,
&mut hash_once,
cache_precomputed,
);
@@ -184,10 +196,98 @@ pub async fn collect_resolution_policy_violations(
if verifiers.is_empty() || lockfile.packages.is_none() {
return Vec::new();
}
let candidates = collect_candidates(lockfile);
// Shape violations are deliberately not collected here: they are
// hard tampering failures, not policy picks a caller may
// auto-exclude.
let (candidates, _shape_violations) = collect_candidates(lockfile);
run_fan_out(candidates, verifiers, concurrency).await
}
pub const RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE: &str = "RESOLUTION_SHAPE_MISMATCH";
/// Cache-key participant for the offline resolution-shape pass: a
/// record written before the rule existed lacks the flag, so
/// `can_trust_past_check` rejects it and forces a re-verification.
/// `verify` is never invoked — the identity is appended only to the
/// verifier lists handed to the cache lookup and recorder.
struct ResolutionShapeCacheIdentity {
policy: serde_json::Map<String, serde_json::Value>,
}
fn resolution_shape_cache_identity() -> Arc<dyn ResolutionVerifier> {
let mut policy = serde_json::Map::new();
policy.insert("resolutionShapeCheck".to_string(), serde_json::Value::Bool(true));
Arc::new(ResolutionShapeCacheIdentity { policy })
}
/// Every verifier list that flows into the verification cache must
/// carry the resolution-shape identity, so records written before the
/// shape rule existed cannot stat-fast-path around it. Used by the
/// gate itself and by [`crate::record_lockfile_verified()`], whose
/// freshly-resolved lockfile satisfies the shape invariant by
/// construction (the writer derives every key from the resolution it
/// just produced).
pub(crate) fn with_resolution_shape_cache_identity(
verifiers: &[Arc<dyn ResolutionVerifier>],
) -> Vec<Arc<dyn ResolutionVerifier>> {
verifiers.iter().cloned().chain(std::iter::once(resolution_shape_cache_identity())).collect()
}
impl ResolutionVerifier for ResolutionShapeCacheIdentity {
fn verify<'a>(
&'a self,
_resolution: &'a LockfileResolution,
_ctx: VerifyCtx<'a>,
) -> VerifyFuture<'a> {
Box::pin(async { ResolutionVerification::Ok })
}
fn policy(&self) -> &serde_json::Map<String, serde_json::Value> {
&self.policy
}
fn can_trust_past_check(&self, cached: &serde_json::Map<String, serde_json::Value>) -> bool {
cached.get("resolutionShapeCheck") == Some(&serde_json::Value::Bool(true))
}
}
/// Mirrors upstream's `isRegistryShapedResolution`: a plain tarball
/// resolution is registry-shaped because the npm verifier unconditionally
/// binds explicit tarball URLs of semver-keyed entries to the registry's
/// own `dist.tarball`. A git-hosted tarball is not — and trust is gated on
/// the tarball URL rather than the `gitHosted` flag alone, so a tampered
/// entry that clears the flag (`gitHosted: false`) on a git-host URL is
/// still rejected.
fn is_registry_shaped_resolution(resolution: &LockfileResolution) -> bool {
match resolution {
LockfileResolution::Registry(_) => true,
LockfileResolution::Tarball(tarball) => {
// The tarball URL must be an http(s) registry artifact and not
// git-hosted. The npm verifier's tarball-URL binding skips
// non-http(s) schemes (file:, etc.), so a `file:` tarball under a
// name@semver key would otherwise be trusted with no safety net.
is_http_tarball_url(&tarball.tarball)
&& tarball.git_hosted != Some(true)
&& !is_git_hosted_tarball_url(&tarball.tarball)
}
LockfileResolution::Variations(variations) => variations
.variants
.iter()
.all(|variant| is_registry_shaped_resolution(&variant.resolution)),
LockfileResolution::Directory(_)
| LockfileResolution::Git(_)
| LockfileResolution::Binary(_) => false,
}
}
/// Whether a tarball URL uses an http(s) scheme — the only schemes a
/// registry artifact is served over. Case-insensitive to reject a
/// tampered uppercase scheme.
fn is_http_tarball_url(url: &str) -> bool {
let lower = url.to_ascii_lowercase();
lower.starts_with("https://") || lower.starts_with("http://")
}
/// One `(name, version, resolution)` tuple deduplicated from
/// `lockfile.packages`. Mirrors upstream's inline `Candidate`
/// interface.
@@ -207,14 +307,34 @@ struct Candidate {
/// `BTreeMap` over a serialized key gives deterministic iteration
/// order for tests; the fan-out runs across the value iter so order
/// doesn't affect correctness, only the reproducibility of failures.
fn collect_candidates(lockfile: &Lockfile) -> Vec<Candidate> {
fn collect_candidates(lockfile: &Lockfile) -> (Vec<Candidate>, Vec<ResolutionPolicyViolation>) {
let Some(packages) = lockfile.packages.as_ref() else {
return Vec::new();
return (Vec::new(), Vec::new());
};
let mut deduped: BTreeMap<String, Candidate> = BTreeMap::new();
let mut shape_violations = Vec::new();
for (key, metadata) in packages {
let name = key.name.clone();
let version = key.suffix.version().to_string();
// A registry-style dep path (`name@semver`, no `runtime:`-style
// prefix) must be backed by a registry-shaped resolution: the
// allowBuilds policy derives a trusted package identity from
// that key shape, which is only sound while this invariant
// holds. The check is offline, so it applies even when no
// policy verifiers are active.
if key.suffix.prefix() == pacquet_lockfile::Prefix::None
&& matches!(key.suffix.version(), pacquet_lockfile::VersionPart::Semver(_))
&& !is_registry_shaped_resolution(&metadata.resolution)
{
shape_violations.push(ResolutionPolicyViolation {
name: name.clone(),
version: version.clone(),
resolution: metadata.resolution.clone(),
code: RESOLUTION_SHAPE_MISMATCH_VIOLATION_CODE,
reason: "a registry-style dependency path is backed by a non-registry resolution"
.to_string(),
});
}
// Every `LockfileResolution` variant derives `Serialize`, and
// the wire shape never contains non-string keys or non-finite
// numbers — the only way this `expect` could fire is a future
@@ -230,7 +350,7 @@ fn collect_candidates(lockfile: &Lockfile) -> Vec<Candidate> {
resolution: metadata.resolution.clone(),
});
}
deduped.into_values().collect()
(deduped.into_values().collect(), shape_violations)
}
/// Run every active verifier against every candidate with a

View File

@@ -527,3 +527,181 @@ async fn ctx_borrows_have_expected_lifetimes() {
.await;
assert!(result.is_ok());
}
#[tokio::test]
async fn rejects_registry_style_key_backed_by_git_resolution_even_with_no_verifiers() {
let lockfile = parse(
"lockfileVersion: '9.0'
importers:
.:
dependencies:
acme:
specifier: ^1.0.0
version: 1.0.0
packages:
acme@1.0.0:
resolution: {type: git, repo: https://example.com/acme.git, commit: 0123456789abcdef0123456789abcdef01234567}
snapshots:
acme@1.0.0: {}
",
);
let err = verify_lockfile_resolutions::<SilentReporter>(
&lockfile,
&[],
&VerifyLockfileResolutionsOptions::default(),
)
.await
.expect_err("registry-style key with a git resolution must be rejected");
let VerifyError::ResolutionShapeMismatch { count, breakdown } = err else {
panic!("expected ResolutionShapeMismatch, got {err:?}");
};
assert_eq!(count, 1);
assert!(breakdown.contains("acme@1.0.0"), "breakdown: {breakdown}");
}
#[tokio::test]
async fn accepts_artifact_keys_with_non_registry_resolutions() {
let lockfile = parse(
"lockfileVersion: '9.0'
importers:
.:
dependencies:
acme:
specifier: github:org/acme
version: git+https://example.com/acme.git#0123456789abcdef0123456789abcdef01234567
packages:
acme@git+https://example.com/acme.git#0123456789abcdef0123456789abcdef01234567:
resolution: {type: git, repo: https://example.com/acme.git, commit: 0123456789abcdef0123456789abcdef01234567}
version: 1.0.0
snapshots:
acme@git+https://example.com/acme.git#0123456789abcdef0123456789abcdef01234567: {}
",
);
verify_lockfile_resolutions::<SilentReporter>(
&lockfile,
&[],
&VerifyLockfileResolutionsOptions::default(),
)
.await
.expect("artifact-keyed git entry passes the structural gate");
}
#[tokio::test]
async fn rejects_git_host_tarball_when_git_hosted_flag_is_cleared() {
// A tampered lockfile sets gitHosted: false on a codeload URL under a
// semver key to dodge the flag-only check; the URL must still flag it.
let lockfile = parse(
"lockfileVersion: '9.0'
importers:
.:
dependencies:
acme:
specifier: ^1.0.0
version: 1.0.0
packages:
acme@1.0.0:
resolution: {integrity: sha512-deadbeef, tarball: 'https://codeload.github.com/org/acme/tar.gz/abc123', gitHosted: false}
snapshots:
acme@1.0.0: {}
",
);
let err = verify_lockfile_resolutions::<SilentReporter>(
&lockfile,
&[],
&VerifyLockfileResolutionsOptions::default(),
)
.await
.expect_err("git-host tarball under a semver key must be rejected regardless of the flag");
let VerifyError::ResolutionShapeMismatch { count, .. } = err else {
panic!("expected ResolutionShapeMismatch, got {err:?}");
};
assert_eq!(count, 1);
}
#[tokio::test]
async fn rejects_semver_key_backed_by_non_http_tarball() {
// A file: tarball under a semver key is not registry-backed and the npm
// verifier skips non-http(s) tarballs, so the shape pass must reject it.
let lockfile = parse(
"lockfileVersion: '9.0'
importers:
.:
dependencies:
acme:
specifier: ^1.0.0
version: 1.0.0
packages:
acme@1.0.0:
resolution: {integrity: sha512-deadbeef, tarball: 'file:///tmp/evil.tgz'}
snapshots:
acme@1.0.0: {}
",
);
let err = verify_lockfile_resolutions::<SilentReporter>(
&lockfile,
&[],
&VerifyLockfileResolutionsOptions::default(),
)
.await
.expect_err("file: tarball under a semver key must be rejected");
assert!(matches!(err, VerifyError::ResolutionShapeMismatch { .. }), "got {err:?}");
}
#[tokio::test]
async fn rejects_git_host_tarball_with_uppercased_host() {
// Hostnames are case-insensitive; an uppercased codeload host with
// gitHosted: false must still be rejected under a semver key.
let lockfile = parse(
"lockfileVersion: '9.0'
importers:
.:
dependencies:
acme:
specifier: ^1.0.0
version: 1.0.0
packages:
acme@1.0.0:
resolution: {integrity: sha512-deadbeef, tarball: 'https://CODELOAD.GITHUB.COM/org/acme/tar.gz/abc123', gitHosted: false}
snapshots:
acme@1.0.0: {}
",
);
let err = verify_lockfile_resolutions::<SilentReporter>(
&lockfile,
&[],
&VerifyLockfileResolutionsOptions::default(),
)
.await
.expect_err("uppercased git-host tarball must be rejected");
assert!(matches!(err, VerifyError::ResolutionShapeMismatch { .. }), "got {err:?}");
}

View File

@@ -380,14 +380,19 @@ impl From<ResolutionSerde> for LockfileResolution {
}
/// Best-effort URL-prefix check used to back-fill `gitHosted` on tarball
/// resolutions written by older pnpm versions. Mirrors upstream's
/// `isGitHostedTarballUrl` at
/// resolutions written by older pnpm versions, and to gate trust on the
/// tarball URL rather than the (tamper-prone) `gitHosted` flag. Mirrors
/// upstream's `isGitHostedTarballUrl` at
/// <https://github.com/pnpm/pnpm/blob/94240bc046/lockfile/fs/src/lockfileFormatConverters.ts#L23-L29>.
fn is_git_hosted_tarball_url(url: &str) -> bool {
(url.starts_with("https://codeload.github.com/")
|| url.starts_with("https://bitbucket.org/")
|| url.starts_with("https://gitlab.com/"))
&& url.contains("tar.gz")
pub fn is_git_hosted_tarball_url(url: &str) -> bool {
// Schemes and hostnames are case-insensitive, so match against a lowercased
// copy: a tampered `https://CODELOAD.GITHUB.COM/...` must not slip past as a
// non-git-hosted (and therefore registry-trusted) tarball.
let lower = url.to_ascii_lowercase();
(lower.starts_with("https://codeload.github.com/")
|| lower.starts_with("https://bitbucket.org/")
|| lower.starts_with("https://gitlab.com/"))
&& lower.contains("tar.gz")
}
impl From<LockfileResolution> for ResolutionSerde {

View File

@@ -6,6 +6,7 @@ use crate::{
use derive_more::{Display, Error};
use miette::Diagnostic;
use pacquet_config::Config;
use pacquet_deps_path::remove_suffix;
use pacquet_executor::{
LifecycleScriptError, RunPostinstallHooks, ScriptsPrependNodePath, run_postinstall_hooks,
};
@@ -84,6 +85,8 @@ pub enum BuildModulesError {
pub struct AllowBuildPolicy {
expanded_allowed: HashSet<String>,
expanded_disallowed: HashSet<String>,
allowed_dep_paths: HashSet<String>,
disallowed_dep_paths: HashSet<String>,
dangerously_allow_all: bool,
}
@@ -99,7 +102,29 @@ impl AllowBuildPolicy {
expanded_disallowed: HashSet<String>,
dangerously_allow_all: bool,
) -> Self {
Self { expanded_allowed, expanded_disallowed, dangerously_allow_all }
Self {
expanded_allowed,
expanded_disallowed,
allowed_dep_paths: HashSet::new(),
disallowed_dep_paths: HashSet::new(),
dangerously_allow_all,
}
}
pub fn new_with_dep_paths(
expanded_allowed: HashSet<String>,
expanded_disallowed: HashSet<String>,
allowed_dep_paths: HashSet<String>,
disallowed_dep_paths: HashSet<String>,
dangerously_allow_all: bool,
) -> Self {
Self {
expanded_allowed,
expanded_disallowed,
allowed_dep_paths,
disallowed_dep_paths,
dangerously_allow_all,
}
}
/// Build the policy from a resolved [`Config`]. Reads
@@ -120,16 +145,32 @@ impl AllowBuildPolicy {
pub fn from_config(config: &Config) -> Result<Self, VersionPolicyError> {
let mut allowed_specs: Vec<&str> = Vec::new();
let mut disallowed_specs: Vec<&str> = Vec::new();
let mut allowed_dep_paths = HashSet::new();
let mut disallowed_dep_paths = HashSet::new();
for (spec, &value) in &config.allow_builds {
if value {
allowed_specs.push(spec);
if is_dep_path_allow_build_key(spec) {
if value {
allowed_dep_paths.insert(normalize_build_dep_path(spec));
} else {
disallowed_dep_paths.insert(normalize_build_dep_path(spec));
}
} else {
disallowed_specs.push(spec);
if value {
allowed_specs.push(spec);
} else {
disallowed_specs.push(spec);
}
}
}
let expanded_allowed = expand_package_version_specs(allowed_specs)?;
let expanded_disallowed = expand_package_version_specs(disallowed_specs)?;
Ok(Self::new(expanded_allowed, expanded_disallowed, config.dangerously_allow_all_builds))
Ok(Self::new_with_dep_paths(
expanded_allowed,
expanded_disallowed,
allowed_dep_paths,
disallowed_dep_paths,
config.dangerously_allow_all_builds,
))
}
/// Check whether a package is allowed to run build scripts.
@@ -146,18 +187,34 @@ impl AllowBuildPolicy {
/// lookup means `*` wildcards in specs do NOT match real
/// package names — see [`expand_package_version_specs`] for
/// the rationale.
pub fn check(&self, name: &str, version: &str) -> Option<bool> {
pub fn check(&self, dep_path: &str) -> Option<bool> {
if self.dangerously_allow_all {
return Some(true);
}
let normalized_dep_path = normalize_build_dep_path(dep_path);
if self.disallowed_dep_paths.contains(&normalized_dep_path) {
return Some(false);
}
let (name, version) = parse_name_version_from_key(&normalized_dep_path);
let name_at_version = format!("{name}@{version}");
if self.expanded_disallowed.contains(name)
if self.expanded_disallowed.contains(&name)
|| self.expanded_disallowed.contains(&name_at_version)
{
return Some(false);
}
if self.expanded_allowed.contains(name) || self.expanded_allowed.contains(&name_at_version)
if self.allowed_dep_paths.contains(&normalized_dep_path) {
return Some(true);
}
// Package-name rules require a trusted package identity. A
// registry-style dep path (`name@semver`) is the trust signal: the
// lockfile verification gate rejects lockfiles where such a key is
// backed by a non-registry resolution, so by the time scripts can
// run, the shape proves the artifact came from a registry.
if node_semver::Version::parse(&version).is_err() {
return None;
}
if self.expanded_allowed.contains(&name) || self.expanded_allowed.contains(&name_at_version)
{
return Some(true);
}
@@ -166,6 +223,33 @@ impl AllowBuildPolicy {
}
}
/// Strips the peer suffix (and, matching [`PkgVerPeer::without_peer`]'s
/// lumped suffix handling, the patch hash) so config keys compare equal
/// to the `metadata_key.to_string()` form used at the runtime call sites.
///
/// [`PkgVerPeer::without_peer`]: pacquet_lockfile::PkgVerPeer::without_peer
pub(crate) fn normalize_build_dep_path(dep_path: &str) -> String {
remove_suffix(dep_path).to_string()
}
fn is_dep_path_allow_build_key(spec: &str) -> bool {
if normalize_build_dep_path(spec) != spec {
return true;
}
if spec.contains("||") {
return false;
}
let (_, version) = parse_name_version_from_key(spec);
if version.is_empty() {
return !spec.starts_with('@') && (spec.contains('/') || spec.contains(':'));
}
node_semver::Version::parse(&version).is_err() && is_source_like_dep_path_version(&version)
}
fn is_source_like_dep_path_version(version: &str) -> bool {
version.contains(':') || version.contains('/') || version.contains('#')
}
/// Run lifecycle scripts for all packages that require a build.
///
/// Ports the core of `buildModules` from
@@ -547,7 +631,8 @@ fn build_one_snapshot<Reporter: self::Reporter>(
// `ignoreScripts = true; break` pattern.
let mut should_run_scripts = requires_build;
if requires_build {
match allow_build_policy.check(&name, &version) {
let dep_path = metadata_key.to_string();
match allow_build_policy.check(&dep_path) {
Some(false) => {
should_run_scripts = false;
}
@@ -560,10 +645,7 @@ fn build_one_snapshot<Reporter: self::Reporter>(
// the end of `BuildModules::run` for the safety
// argument (BTreeSet insertion is atomic from the
// data-structure's POV).
ignored_builds
.lock()
.unwrap_or_else(|e| e.into_inner())
.insert(metadata_key.to_string());
ignored_builds.lock().unwrap_or_else(|e| e.into_inner()).insert(dep_path);
should_run_scripts = false;
}
Some(true) => {}

View File

@@ -21,26 +21,19 @@ use tempfile::tempdir;
/// Build an [`AllowBuildPolicy`] from a list of `(spec, allowed)`
/// pairs, mirroring how `pnpm-workspace.yaml`'s `allowBuilds` map
/// would arrive at the policy. Each spec is parsed through
/// [`crate::expand_package_version_specs`] so version unions
/// (`foo@1.0.0 || 2.0.0`) work the same way they do at runtime.
/// [`AllowBuildPolicy::from_config`] so version unions and depPath
/// keys work the same way they do at runtime.
/// Panics on any parse failure — test inputs must be valid.
fn policy_from_specs<const LEN: usize>(
entries: [(&str, bool); LEN],
dangerously_allow_all: bool,
) -> AllowBuildPolicy {
use crate::expand_package_version_specs;
let mut allowed_specs: Vec<&str> = Vec::new();
let mut disallowed_specs: Vec<&str> = Vec::new();
let mut config = Config::new();
config.dangerously_allow_all_builds = dangerously_allow_all;
for (spec, value) in entries {
if value {
allowed_specs.push(spec);
} else {
disallowed_specs.push(spec);
}
config.allow_builds.insert(spec.to_string(), value);
}
let expanded_allowed = expand_package_version_specs(allowed_specs).expect("valid specs");
let expanded_disallowed = expand_package_version_specs(disallowed_specs).expect("valid specs");
AllowBuildPolicy::new(expanded_allowed, expanded_disallowed, dangerously_allow_all)
AllowBuildPolicy::from_config(&config).expect("valid specs")
}
#[test]
@@ -74,25 +67,55 @@ fn parse_key_without_leading_slash() {
#[test]
fn default_policy_denies_all() {
let policy = AllowBuildPolicy::default();
assert_eq!(policy.check("any-package", "1.0.0"), None);
assert_eq!(policy.check("any-package@1.0.0"), None);
}
#[test]
fn explicit_allow() {
let policy = policy_from_specs([("@pnpm.e2e/install-script-example", true)], false);
assert_eq!(policy.check("@pnpm.e2e/install-script-example", "1.0.0"), Some(true));
assert_eq!(policy.check("@pnpm.e2e/install-script-example@1.0.0"), Some(true));
}
#[test]
fn explicit_allow_requires_registry_style_dep_path() {
let policy = policy_from_specs([("@pnpm.e2e/install-script-example", true)], false);
assert_eq!(
policy.check("@pnpm.e2e/install-script-example@git+https://example.com/x.git#abc123"),
None,
);
assert_eq!(policy.check("@pnpm.e2e/install-script-example@1.0.0"), Some(true));
}
#[test]
fn explicit_allow_by_dep_path_allows_untrusted_package_identity() {
let policy = policy_from_specs(
[("foo@git+https://github.com/org/foo.git#abc123", true), ("foo", true)],
false,
);
assert_eq!(
policy.check("foo@git+https://github.com/org/foo.git#abc123(react@19.0.0)"),
Some(true),
);
assert_eq!(policy.check("foo@git+https://github.com/attacker/foo.git#abc123"), None);
}
#[test]
fn explicit_allow_by_tarball_dep_path_allows_untrusted_package_identity() {
let policy = policy_from_specs([("foo@https://example.com/foo.tgz", true)], false);
assert_eq!(policy.check("foo@https://example.com/foo.tgz"), Some(true));
}
#[test]
fn explicit_deny() {
let policy = policy_from_specs([("@pnpm.e2e/bad-package", false)], false);
assert_eq!(policy.check("@pnpm.e2e/bad-package", "1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/bad-package@1.0.0"), Some(false));
}
#[test]
fn unlisted_returns_none() {
let policy = policy_from_specs([("@pnpm.e2e/allowed", true)], false);
assert_eq!(policy.check("@pnpm.e2e/not-listed", "1.0.0"), None);
assert_eq!(policy.check("@pnpm.e2e/not-listed@1.0.0"), None);
}
/// Upstream checks `expandedDisallowed` before `expandedAllowed`
@@ -106,8 +129,8 @@ fn unlisted_returns_none() {
fn disallow_bare_name_wins_over_allow_exact_version() {
let policy =
policy_from_specs([("@pnpm.e2e/pkg@1.0.0", true), ("@pnpm.e2e/pkg", false)], false);
assert_eq!(policy.check("@pnpm.e2e/pkg", "1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/pkg", "2.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/pkg@1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/pkg@2.0.0"), Some(false));
}
/// The converse: a bare-name allow combined with an exact-version
@@ -117,27 +140,33 @@ fn disallow_bare_name_wins_over_allow_exact_version() {
fn disallow_exact_version_with_allow_bare_name() {
let policy =
policy_from_specs([("@pnpm.e2e/pkg", true), ("@pnpm.e2e/pkg@1.0.0", false)], false);
assert_eq!(policy.check("@pnpm.e2e/pkg", "1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/pkg", "2.0.0"), Some(true));
assert_eq!(policy.check("@pnpm.e2e/pkg@1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/pkg@2.0.0"), Some(true));
}
#[test]
fn empty_rules_denies_all() {
let policy = policy_from_specs([], false);
assert_eq!(policy.check("any-package", "1.0.0"), None);
assert_eq!(policy.check("any-package@1.0.0"), None);
}
#[test]
fn dangerously_allow_all_builds() {
let policy = policy_from_specs([], true);
assert_eq!(policy.check("any-package", "1.0.0"), Some(true));
assert_eq!(policy.check("other-package", "2.0.0"), Some(true));
assert_eq!(policy.check("any-package@1.0.0"), Some(true));
assert_eq!(policy.check("other-package@2.0.0"), Some(true));
}
#[test]
fn dangerously_allow_all_allows_artifact_dep_paths() {
let policy = policy_from_specs([], true);
assert_eq!(policy.check("anything@git+https://example.com/x.git#abc123"), Some(true));
}
#[test]
fn dangerously_allow_all_overrides_deny() {
let policy = policy_from_specs([("@pnpm.e2e/pkg", false)], true);
assert_eq!(policy.check("@pnpm.e2e/pkg", "1.0.0"), Some(true));
assert_eq!(policy.check("@pnpm.e2e/pkg@1.0.0"), Some(true));
}
/// Mirrors upstream's
@@ -148,11 +177,11 @@ fn dangerously_allow_all_overrides_deny() {
#[test]
fn allow_via_version_union() {
let policy = policy_from_specs([("foo", true), ("qar@1.0.0 || 2.0.0", true)], false);
assert_eq!(policy.check("foo", "1.0.0"), Some(true));
assert_eq!(policy.check("bar", "1.0.0"), None);
assert_eq!(policy.check("qar", "1.0.0"), Some(true));
assert_eq!(policy.check("qar", "2.0.0"), Some(true));
assert_eq!(policy.check("qar", "1.1.0"), None);
assert_eq!(policy.check("foo@1.0.0"), Some(true));
assert_eq!(policy.check("bar@1.0.0"), None);
assert_eq!(policy.check("qar@1.0.0"), Some(true));
assert_eq!(policy.check("qar@2.0.0"), Some(true));
assert_eq!(policy.check("qar@1.1.0"), None);
}
/// Mirrors upstream's
@@ -165,8 +194,8 @@ fn allow_via_version_union() {
#[test]
fn wildcard_name_in_allow_builds_does_not_match_real_package() {
let policy = policy_from_specs([("is-*", true)], false);
assert_eq!(policy.check("is-odd", "1.0.0"), None);
assert_eq!(policy.check("is-positive", "1.0.0"), None);
assert_eq!(policy.check("is-odd@1.0.0"), None);
assert_eq!(policy.check("is-positive@1.0.0"), None);
}
/// `from_config` propagates `expand_package_version_specs` errors —
@@ -201,7 +230,7 @@ fn from_config_propagates_name_pattern_in_version_union() {
#[test]
fn empty_config_denies_all() {
let policy = AllowBuildPolicy::from_config(&Config::new()).expect("empty config never errors");
assert_eq!(policy.check("anything", "1.0.0"), None);
assert_eq!(policy.check("anything@1.0.0"), None);
}
#[test]
@@ -212,9 +241,9 @@ fn from_config_consumes_allow_builds_and_dangerously_allow_all_builds() {
config.allow_builds.insert("@pnpm.e2e/bad-package".to_string(), false);
let policy = AllowBuildPolicy::from_config(&config).expect("valid specs");
assert_eq!(policy.check("@pnpm.e2e/install-script-example", "1.0.0"), Some(true));
assert_eq!(policy.check("@pnpm.e2e/bad-package", "1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/unrelated", "1.0.0"), None);
assert_eq!(policy.check("@pnpm.e2e/install-script-example@1.0.0"), Some(true));
assert_eq!(policy.check("@pnpm.e2e/bad-package@1.0.0"), Some(false));
assert_eq!(policy.check("@pnpm.e2e/unrelated@1.0.0"), None);
}
fn name(text: &str) -> PkgName {

View File

@@ -280,7 +280,7 @@ impl<'a> InstallPackageBySnapshot<'a> {
// (`None → false`) matches pnpm v11's policy: build scripts
// have to be explicitly opted in to run.
let allow_build_closure =
|name: &str, version: &str| allow_build_policy.check(name, version).unwrap_or(false);
|dep_path: &str| allow_build_policy.check(dep_path).unwrap_or(false);
let scripts_prepend_node_path = match config.scripts_prepend_node_path {
pacquet_config::ScriptsPrependNodePath::Always => ExecScriptsPrependNodePath::Always,
pacquet_config::ScriptsPrependNodePath::Never => ExecScriptsPrependNodePath::Never,

View File

@@ -198,12 +198,7 @@ impl VirtualStoreLayout {
let built_dep_paths: Option<HashSet<PackageKey>> = allow_build_policy.map(|policy| {
snapshots
.keys()
.filter(|key| {
let metadata_key = key.without_peer();
let name = metadata_key.name.to_string();
let version = metadata_key.suffix.version().to_string();
policy.check(&name, &version) == Some(true)
})
.filter(|key| policy.check(&key.without_peer().to_string()) == Some(true))
.cloned()
.collect()
});

View File

@@ -5,7 +5,10 @@ use pacquet_lockfile::{
SnapshotEntry,
};
use pretty_assertions::{assert_eq, assert_ne};
use std::{collections::HashMap, path::PathBuf};
use std::{
collections::{HashMap, HashSet},
path::PathBuf,
};
/// Build a `Config` test-double with the GVS-relevant fields
/// wired explicitly. `gvs_dir` populates `global_virtual_store_dir`
@@ -259,6 +262,38 @@ fn slot_dir_engine_specific_when_snapshot_is_built() {
assert_ne!(darwin, linux, "builder snapshot must partition GVS slot by engine string");
}
#[test]
fn missing_metadata_keeps_source_dep_path_untrusted_for_gvs() {
let config = make_config(
true,
PathBuf::from("/tmp/proj/node_modules/.pnpm"),
PathBuf::from("/tmp/store/links"),
);
let key: PackageKey = "spoofed@git-hosted#abc123".parse().unwrap();
let mut snapshots = HashMap::new();
snapshots.insert(key.clone(), SnapshotEntry::default());
let packages = HashMap::new();
let allowed: HashSet<String> = ["spoofed".to_string()].into_iter().collect();
let policy = crate::AllowBuildPolicy::new(allowed, HashSet::new(), false);
let darwin = VirtualStoreLayout::new(
&config,
Some("darwin-arm64-node20"),
Some(&snapshots),
Some(&packages),
Some(&policy),
)
.slot_dir(&key);
let linux = VirtualStoreLayout::new(
&config,
Some("linux-x64-node22"),
Some(&snapshots),
Some(&packages),
Some(&policy),
)
.slot_dir(&key);
assert_eq!(darwin, linux, "source depPath with missing metadata must not be name-allowed");
}
/// Per-snapshot `engines.runtime` resolution: two builder
/// siblings that pin *different* Node majors must land on
/// different GVS slots even when given the same install-wide

10
pnpm-lock.yaml generated
View File

@@ -1051,6 +1051,7 @@ overrides:
send@<0.19.0: ^0.19.0
shell-quote@<1.8.4: '>=1.8.4'
serve-static@<1.16.0: ^1.16.0
shell-quote: 1.8.4
socks@2: ^2.8.1
tar@<=7.5.9: '>=7.5.10'
tmp@<=0.2.3: '>=0.2.4'
@@ -1818,6 +1819,9 @@ importers:
'@pnpm/building.after-install':
specifier: workspace:*
version: link:../after-install
'@pnpm/building.policy':
specifier: workspace:*
version: link:../policy
'@pnpm/cli.command':
specifier: workspace:*
version: link:../../cli/command
@@ -4601,6 +4605,9 @@ importers:
'@pnpm/fs.packlist':
specifier: workspace:*
version: link:../../fs/packlist
'@pnpm/resolving.git-resolver':
specifier: workspace:*
version: link:../../resolving/git-resolver
'@pnpm/store.index':
specifier: workspace:*
version: link:../../store/index
@@ -5214,6 +5221,9 @@ importers:
'@pnpm/building.after-install':
specifier: workspace:*
version: link:../../building/after-install
'@pnpm/building.policy':
specifier: workspace:*
version: link:../../building/policy
'@pnpm/catalogs.types':
specifier: workspace:*
version: link:../../catalogs/types

View File

@@ -440,6 +440,8 @@ overrides:
send@<0.19.0: ^0.19.0
shell-quote@<1.8.4: '>=1.8.4'
serve-static@<1.16.0: ^1.16.0
# GHSA-w7jw-789q-3m8p / CVE-2026-9277: 1.8.4 patches critical shell-quote vulnerabilities in <=1.8.3.
shell-quote: 1.8.4
socks@2: ^2.8.1
tar@<=7.5.9: '>=7.5.10'
tmp@<=0.2.3: '>=0.2.4'

View File

@@ -331,7 +331,13 @@ 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, '--allow-build=shx', 'dlx', cmd, ...args])))
// The git-hosted artifact has an untrusted package identity, so it has to
// be approved by its depPath; the registry shx is approved by name.
const allowBuilds = [
'--allow-build=shx',
'--allow-build=shx@https://codeload.github.com/shelljs/shx/tar.gz/61aca968cd7afc712ca61a4fc4ec3201e3770dc7',
]
await Promise.all(Object.entries(commands).map(([cmd, args]) => execPnpm([...settings, ...allowBuilds, 'dlx', cmd, ...args])))
// ensure that the dlx cache has certain structure
const dlxBaseDir = path.resolve('cache', 'dlx')

View File

@@ -1,10 +1,21 @@
import type { PkgResolutionId } from '@pnpm/resolving.resolver-base'
export function createGitHostedPkgId ({ repo, commit, path }: { repo: string, commit: string, path?: string }): PkgResolutionId {
let id = `${repo.includes('://') ? '' : 'https://'}${repo}#${commit}`
const normalizedRepo = normalizeGitRepoForPkgResolutionId(repo)
let id = `${normalizedRepo.includes('://') ? '' : 'https://'}${normalizedRepo}#${commit}`
if (!id.startsWith('git+')) id = `git+${id}`
if (path) {
id += `&path:${path}`
}
return id as PkgResolutionId
}
function normalizeGitRepoForPkgResolutionId (repo: string): string {
// Only scp-style shorthand (`user@host:path`) needs rewriting. A repo that
// already carries a URL scheme (e.g. `ssh://user@host:2222/path`) is left
// alone — its `@host:port` would otherwise match the scp pattern and get
// mangled into `ssh://ssh://…`.
if (repo.includes('://')) return repo
const scp = /^([^@\s]+@[^:\s]+):(.+)$/.exec(repo)
return scp == null ? repo : `ssh://${scp[1]}/${scp[2]}`
}

View File

@@ -3,6 +3,10 @@ import { createGitHostedPkgId } from '@pnpm/resolving.git-resolver'
test.each([
[{ repo: 'ssh://git@example.com/org/repo.git', commit: 'cba04669e621b85fbdb33371604de1a2898e68e9' }, 'git+ssh://git@example.com/org/repo.git#cba04669e621b85fbdb33371604de1a2898e68e9'],
// A fully-qualified ssh URL with a port must not be rewritten by the scp
// shorthand normalization (its `@host:2222` looks scp-like).
[{ repo: 'ssh://git@example.com:2222/org/repo.git', commit: 'cba04669e621b85fbdb33371604de1a2898e68e9' }, 'git+ssh://git@example.com:2222/org/repo.git#cba04669e621b85fbdb33371604de1a2898e68e9'],
[{ repo: 'git@example.com:org/repo.git', commit: 'cba04669e621b85fbdb33371604de1a2898e68e9', path: 'packages/pkg' }, 'git+ssh://git@example.com/org/repo.git#cba04669e621b85fbdb33371604de1a2898e68e9&path:packages/pkg'],
[{ repo: 'https://0000000000000000000000000000000000000000:x-oauth-basic@github.com/foo/bar.git', commit: '0000000000000000000000000000000000000000' }, 'git+https://0000000000000000000000000000000000000000:x-oauth-basic@github.com/foo/bar.git#0000000000000000000000000000000000000000'],
[{ repo: 'file:///Users/zoltan/src/pnpm/pnpm/resolving/git-resolver', commit: '988c61e11dc8d9ca0b5580cb15291951812549dc' }, 'git+file:///Users/zoltan/src/pnpm/pnpm/resolving/git-resolver#988c61e11dc8d9ca0b5580cb15291951812549dc'],
])('createGitHostedPkgId', (resolution, id) => {