From 14bceb1e0b2a71f4f670774db261feb03f38ec23 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 10 Jun 2026 15:10:00 +0200 Subject: [PATCH] fix(security): harden build-approval artifact identities on v10 (#12306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): harden build-approval artifact identities on v10 Port of pnpm/pnpm#12294 (commit bf1b731ee6) to release/10. Package-name entries in onlyBuiltDependencies (and allowBuilds) 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 key — the GIT_DEP_PREPARE_NOT_ALLOWED hint, pnpm ignored-builds, and pnpm approve-builds print exactly that key. - AllowBuild policy functions identify packages by DepPath instead of caller-supplied name/version. Identity trust is derived from the depPath shape: a registry-style depPath (name@semver) is a trusted identity. - The trust is sound because lockfile entries are structurally checked wherever they are materialized into fetchable resolutions (pkgSnapshotToResolution) and before rebuild runs scripts: a registry-style key backed by a git, directory, or git-hosted tarball resolution is rejected with ERR_PNPM_RESOLUTION_SHAPE_MISMATCH. - preparePackage requires a pkgResolutionId and gates on the synthesized name@ depPath; scp-style git URLs are normalized to ssh:// form in resolution ids and the git fetcher reuses createGitHostedPkgId from the resolver. - isGitHostedTarballUrl matches case-insensitively and is shared from @pnpm/lockfile.utils; new removePeersSuffix() in @pnpm/dependency-path and allowBuildKeyFromIgnoredBuild() in @pnpm/builder.policy. - pnpm rebuild and approve-builds accept depPath specs for selecting and approving artifact builds; installs rebuild ignored builds approved by depPath keys. - shell-quote is overridden to 1.8.4 (GHSA-w7jw-789q-3m8p / CVE-2026-9277). Differences from main: v10 has no lockfile resolution verifier, so the structural pass lives in pkgSnapshotToResolution and the rebuild loop; the AllowBuild policy keeps v10's boolean return; the revoked-approval detection and global-virtual-store hash changes have no v10 counterpart. * test: serve the direct-tarball fixture from an off-registry origin The registry mock binds only localhost, so a hard-coded 127.0.0.1 URL is refused in CI, and a tarball URL on the registry's own origin resolves as a registry package, which defeats the direct-tarball identity the test needs. An in-test HTTP server redirecting to the mock provides a separate origin whose binding the test controls. * test: approve the git-protocol licenses fixture by its depPath A git-hosted artifact has an untrusted package identity, so the ajv-keywords build has to be approved by its depPath. Pin the fixture to the commit the depPath names. --- .changeset/tough-allow-builds-identities.md | 19 +++ builder/policy/package.json | 1 + builder/policy/src/index.ts | 57 ++++++- builder/policy/test/index.ts | 92 +++++++++-- builder/policy/tsconfig.json | 3 + exec/build-commands/package.json | 2 +- .../src/getAutomaticallyIgnoredBuilds.ts | 4 +- exec/build-commands/tsconfig.json | 6 +- exec/build-modules/src/index.ts | 2 +- .../src/implementation/index.ts | 28 +++- .../test/dlx.e2e.ts | 4 +- exec/prepare-package/src/index.ts | 13 +- exec/prepare-package/test/index.ts | 19 ++- fetching/git-fetcher/package.json | 1 + fetching/git-fetcher/src/index.ts | 2 + fetching/git-fetcher/test/index.ts | 6 +- fetching/git-fetcher/tsconfig.json | 3 + .../src/gitHostedTarballFetcher.ts | 9 ++ fetching/tarball-fetcher/test/fetch.ts | 2 +- lockfile/fs/src/lockfileFormatConverters.ts | 14 +- .../src/assertRegistryShapedResolution.ts | 82 ++++++++++ lockfile/utils/src/index.ts | 1 + lockfile/utils/src/pkgSnapshotToResolution.ts | 2 + .../test/assertRegistryShapedResolution.ts | 145 ++++++++++++++++++ packages/dependency-path/src/index.ts | 15 +- packages/types/src/config.ts | 4 +- pkg-manager/core/src/install/index.ts | 23 ++- pkg-manager/core/src/install/link.ts | 2 +- .../core/test/install/lifecycleScripts.ts | 49 +++++- pkg-manager/headless/src/index.ts | 2 +- .../headless/src/linkHoistedModules.ts | 2 +- pnpm-lock.yaml | 13 +- pnpm-workspace.yaml | 2 + pnpm/package.json | 1 + pnpm/test/dlx.ts | 8 +- .../git-resolver/src/createGitHostedPkgId.ts | 13 +- .../test/createGitHostedPkgId.test.ts | 4 + .../with-git-protocol-peer-deps/package.json | 2 +- .../plugin-commands-licenses/test/index.ts | 4 +- 39 files changed, 572 insertions(+), 89 deletions(-) create mode 100644 .changeset/tough-allow-builds-identities.md create mode 100644 lockfile/utils/src/assertRegistryShapedResolution.ts create mode 100644 lockfile/utils/test/assertRegistryShapedResolution.ts diff --git a/.changeset/tough-allow-builds-identities.md b/.changeset/tough-allow-builds-identities.md new file mode 100644 index 0000000000..41dbd5d14b --- /dev/null +++ b/.changeset/tough-allow-builds-identities.md @@ -0,0 +1,19 @@ +--- +"@pnpm/types": patch +"@pnpm/dependency-path": patch +"@pnpm/builder.policy": patch +"@pnpm/lockfile.utils": patch +"@pnpm/lockfile.fs": patch +"@pnpm/build-modules": patch +"@pnpm/prepare-package": patch +"@pnpm/exec.build-commands": patch +"@pnpm/plugin-commands-rebuild": patch +"@pnpm/git-fetcher": patch +"@pnpm/tarball-fetcher": patch +"@pnpm/git-resolver": patch +"@pnpm/core": patch +"@pnpm/headless": patch +"pnpm": patch +--- + +Require trusted package identity before package-name `onlyBuiltDependencies` (and `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 key. Lockfile entries are now rejected when 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. diff --git a/builder/policy/package.json b/builder/policy/package.json index 7c7bf4dbee..3ddcd325f5 100644 --- a/builder/policy/package.json +++ b/builder/policy/package.json @@ -32,6 +32,7 @@ }, "dependencies": { "@pnpm/config.version-policy": "workspace:*", + "@pnpm/dependency-path": "workspace:*", "@pnpm/types": "workspace:*" }, "devDependencies": { diff --git a/builder/policy/src/index.ts b/builder/policy/src/index.ts index 1c1969533e..e826840b94 100644 --- a/builder/policy/src/index.ts +++ b/builder/policy/src/index.ts @@ -1,5 +1,6 @@ -import { type AllowBuild } from '@pnpm/types' +import { type AllowBuild, type DepPath } from '@pnpm/types' import { expandPackageVersionSpecs } from '@pnpm/config.version-policy' +import * as dp from '@pnpm/dependency-path' import fs from 'fs' export function createAllowBuildFunction ( @@ -16,12 +17,60 @@ export function createAllowBuildFunction ( if (opts.onlyBuiltDependenciesFile) { onlyBuiltDeps.push(...JSON.parse(fs.readFileSync(opts.onlyBuiltDependenciesFile, 'utf8'))) } - const onlyBuiltDependencies = expandPackageVersionSpecs(onlyBuiltDeps) - return (pkgName, version) => onlyBuiltDependencies.has(pkgName) || onlyBuiltDependencies.has(`${pkgName}@${version}`) + const allowedDepPaths = new Set() + const allowedPackageSpecs: string[] = [] + for (const entry of onlyBuiltDeps) { + if (isDepPathAllowBuildKey(entry)) { + allowedDepPaths.add(dp.removePeersSuffix(entry)) + } else { + allowedPackageSpecs.push(entry) + } + } + const onlyBuiltDependencies = expandPackageVersionSpecs(allowedPackageSpecs) + return (depPath) => { + if (allowedDepPaths.has(dp.getPkgIdWithPatchHash(depPath))) return true + const { name, version, nonSemverVersion } = dp.parse(depPath) + // Package-name rules require a trusted package identity. A + // registry-style depPath (name@semver) is the trust signal: the + // resolution shape check 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 (name == null || version == null || nonSemverVersion != null) return false + return onlyBuiltDependencies.has(name) || onlyBuiltDependencies.has(`${name}@${version}`) + } } if (opts.neverBuiltDependencies != null && opts.neverBuiltDependencies.length > 0) { const neverBuiltDependencies = new Set(opts.neverBuiltDependencies) - return (pkgName) => !neverBuiltDependencies.has(pkgName) + return (depPath) => { + const { name } = dp.parse(depPath) + return name == null || !neverBuiltDependencies.has(name) + } } return undefined } + +/** + * The `onlyBuiltDependencies` 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 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('#') +} diff --git a/builder/policy/test/index.ts b/builder/policy/test/index.ts index 19fc126f93..b9998b5ddd 100644 --- a/builder/policy/test/index.ts +++ b/builder/policy/test/index.ts @@ -1,13 +1,26 @@ import path from 'path' -import { createAllowBuildFunction } from '@pnpm/builder.policy' +import { allowBuildKeyFromIgnoredBuild, createAllowBuildFunction } from '@pnpm/builder.policy' +import { type DepPath } from '@pnpm/types' + +function depPath (value: string): DepPath { + return value as DepPath +} it('should neverBuiltDependencies', () => { const allowBuild = createAllowBuildFunction({ neverBuiltDependencies: ['foo'], }) expect(typeof allowBuild).toBe('function') - expect(allowBuild!('foo', '1.0.0')).toBeFalsy() - expect(allowBuild!('bar', '1.0.0')).toBeTruthy() + expect(allowBuild!(depPath('foo@1.0.0'))).toBeFalsy() + expect(allowBuild!(depPath('bar@1.0.0'))).toBeTruthy() +}) + +it('should deny neverBuiltDependencies by name even for artifact depPaths', () => { + const allowBuild = createAllowBuildFunction({ + neverBuiltDependencies: ['foo'], + }) + expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBeFalsy() + expect(allowBuild!(depPath('bar@git+https://github.com/org/bar.git#abc123'))).toBeTruthy() }) it('should onlyBuiltDependencies', () => { @@ -15,11 +28,11 @@ it('should onlyBuiltDependencies', () => { onlyBuiltDependencies: ['foo', 'qar@1.0.0 || 2.0.0'], }) expect(typeof allowBuild).toBe('function') - expect(allowBuild!('foo', '1.0.0')).toBeTruthy() - expect(allowBuild!('bar', '1.0.0')).toBeFalsy() - expect(allowBuild!('qar', '1.1.0')).toBeFalsy() - expect(allowBuild!('qar', '1.0.0')).toBeTruthy() - expect(allowBuild!('qar', '2.0.0')).toBeTruthy() + expect(allowBuild!(depPath('foo@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('bar@1.0.0'))).toBeFalsy() + expect(allowBuild!(depPath('qar@1.1.0'))).toBeFalsy() + expect(allowBuild!(depPath('qar@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('qar@2.0.0'))).toBeTruthy() }) it('should not allow patterns in onlyBuiltDependencies', () => { @@ -27,7 +40,7 @@ it('should not allow patterns in onlyBuiltDependencies', () => { onlyBuiltDependencies: ['is-*'], }) expect(typeof allowBuild).toBe('function') - expect(allowBuild!('is-odd', '1.0.0')).toBeFalsy() + expect(allowBuild!(depPath('is-odd@1.0.0'))).toBeFalsy() }) it('should onlyBuiltDependencies set via a file', () => { @@ -35,9 +48,9 @@ it('should onlyBuiltDependencies set via a file', () => { onlyBuiltDependenciesFile: path.join(__dirname, 'onlyBuild.json'), }) expect(typeof allowBuild).toBe('function') - expect(allowBuild!('zoo', '1.0.0')).toBeTruthy() - expect(allowBuild!('qar', '1.0.0')).toBeTruthy() - expect(allowBuild!('bar', '1.0.0')).toBeFalsy() + expect(allowBuild!(depPath('zoo@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('qar@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('bar@1.0.0'))).toBeFalsy() }) it('should onlyBuiltDependencies set via a file and config', () => { @@ -46,10 +59,10 @@ it('should onlyBuiltDependencies set via a file and config', () => { onlyBuiltDependenciesFile: path.join(__dirname, 'onlyBuild.json'), }) expect(typeof allowBuild).toBe('function') - expect(allowBuild!('zoo', '1.0.0')).toBeTruthy() - expect(allowBuild!('qar', '1.0.0')).toBeTruthy() - expect(allowBuild!('bar', '1.0.0')).toBeTruthy() - expect(allowBuild!('esbuild', '1.0.0')).toBeFalsy() + expect(allowBuild!(depPath('zoo@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('qar@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('bar@1.0.0'))).toBeTruthy() + expect(allowBuild!(depPath('esbuild@1.0.0'))).toBeFalsy() }) it('should return undefined if no policy is set', () => { @@ -61,5 +74,50 @@ 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({ + onlyBuiltDependencies: ['foo', 'bar'], + }) + expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBeFalsy() + expect(allowBuild!(depPath('bar@https://example.com/bar.tgz'))).toBeFalsy() + expect(allowBuild!(depPath('foo@1.0.0'))).toBeTruthy() +}) + +it('should allow artifact depPaths by depPath key', () => { + const allowBuild = createAllowBuildFunction({ + onlyBuiltDependencies: [ + 'foo@git+https://github.com/org/foo.git#abc123', + 'foo', + ], + }) + expect(allowBuild!(depPath('foo@git+https://github.com/org/foo.git#abc123(react@19.0.0)'))).toBeTruthy() + expect(allowBuild!(depPath('foo@git+https://github.com/attacker/foo.git#abc123'))).toBeFalsy() +}) + +it('should preserve patch hash in depPath onlyBuiltDependencies keys', () => { + const allowBuild = createAllowBuildFunction({ + onlyBuiltDependencies: [ + 'foo@https://example.com/foo.tgz(patch_hash=aaaa)', + ], + }) + expect(allowBuild!(depPath('foo@https://example.com/foo.tgz(patch_hash=aaaa)(react@19.0.0)'))).toBeTruthy() + expect(allowBuild!(depPath('foo@https://example.com/foo.tgz(patch_hash=bbbb)(react@19.0.0)'))).toBeFalsy() +}) + +it('should allow untrusted package identity by source-only depPath', () => { + const allowBuild = createAllowBuildFunction({ + onlyBuiltDependencies: ['github.com/org/foo/abc123'], + }) + expect(allowBuild!(depPath('github.com/org/foo/abc123(react@19.0.0)'))).toBeTruthy() +}) + +it('allowBuildKeyFromIgnoredBuild() returns the name for registry packages and the depPath for artifacts', () => { + expect(allowBuildKeyFromIgnoredBuild(depPath('foo@1.0.0'))).toBe('foo') + expect(allowBuildKeyFromIgnoredBuild(depPath('foo@1.0.0(react@19.0.0)'))).toBe('foo') + expect(allowBuildKeyFromIgnoredBuild(depPath('foo@git+https://github.com/org/foo.git#abc123'))).toBe('foo@git+https://github.com/org/foo.git#abc123') + expect(allowBuildKeyFromIgnoredBuild(depPath('bar@https://example.com/bar.tgz(react@19.0.0)'))).toBe('bar@https://example.com/bar.tgz') }) diff --git a/builder/policy/tsconfig.json b/builder/policy/tsconfig.json index 16f1ca0599..de46be49c7 100644 --- a/builder/policy/tsconfig.json +++ b/builder/policy/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../../config/version-policy" }, + { + "path": "../../packages/dependency-path" + }, { "path": "../../packages/types" } diff --git a/exec/build-commands/package.json b/exec/build-commands/package.json index 6ecd3ac268..4b2f25f0d1 100644 --- a/exec/build-commands/package.json +++ b/exec/build-commands/package.json @@ -32,9 +32,9 @@ "compile": "tsc --build && pnpm run lint --fix" }, "dependencies": { + "@pnpm/builder.policy": "workspace:*", "@pnpm/config": "workspace:*", "@pnpm/config.config-writer": "workspace:*", - "@pnpm/dependency-path": "workspace:*", "@pnpm/modules-yaml": "workspace:*", "@pnpm/plugin-commands-rebuild": "workspace:*", "@pnpm/prepare-temp-dir": "workspace:*", diff --git a/exec/build-commands/src/getAutomaticallyIgnoredBuilds.ts b/exec/build-commands/src/getAutomaticallyIgnoredBuilds.ts index bae3095e6c..750b5cda14 100644 --- a/exec/build-commands/src/getAutomaticallyIgnoredBuilds.ts +++ b/exec/build-commands/src/getAutomaticallyIgnoredBuilds.ts @@ -1,5 +1,5 @@ import path from 'path' -import { parse } from '@pnpm/dependency-path' +import { allowBuildKeyFromIgnoredBuild } from '@pnpm/builder.policy' import { type Modules, readModulesManifest } from '@pnpm/modules-yaml' import { type IgnoredBuildsCommandOpts } from './ignoredBuilds.js' @@ -16,7 +16,7 @@ export async function getAutomaticallyIgnoredBuilds (opts: IgnoredBuildsCommandO if (modulesManifest?.ignoredBuilds) { const ignoredPkgNames = new Set() for (const depPath of modulesManifest?.ignoredBuilds) { - ignoredPkgNames.add(parse(depPath).name ?? depPath) + ignoredPkgNames.add(allowBuildKeyFromIgnoredBuild(depPath)) } automaticallyIgnoredBuilds = Array.from(ignoredPkgNames) } else { diff --git a/exec/build-commands/tsconfig.json b/exec/build-commands/tsconfig.json index 5731bbc60f..63e73f2d0d 100644 --- a/exec/build-commands/tsconfig.json +++ b/exec/build-commands/tsconfig.json @@ -15,15 +15,15 @@ { "path": "../../__utils__/prepare-temp-dir" }, + { + "path": "../../builder/policy" + }, { "path": "../../config/config" }, { "path": "../../config/config-writer" }, - { - "path": "../../packages/dependency-path" - }, { "path": "../../packages/types" }, diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts index 34864fff43..8bea2ba15f 100644 --- a/exec/build-modules/src/index.ts +++ b/exec/build-modules/src/index.ts @@ -82,7 +82,7 @@ export async function buildModules ( () => { let ignoreScripts = Boolean(buildDepOpts.ignoreScripts) if (!ignoreScripts) { - if (depGraph[depPath].requiresBuild && !allowBuild(depGraph[depPath].name, depGraph[depPath].version)) { + if (depGraph[depPath].requiresBuild && !allowBuild(depGraph[depPath].depPath)) { ignoredBuilds.add(depGraph[depPath].depPath) ignoreScripts = true } diff --git a/exec/plugin-commands-rebuild/src/implementation/index.ts b/exec/plugin-commands-rebuild/src/implementation/index.ts index 25d47544e2..1d30b0a853 100644 --- a/exec/plugin-commands-rebuild/src/implementation/index.ts +++ b/exec/plugin-commands-rebuild/src/implementation/index.ts @@ -17,6 +17,7 @@ import { import { linkBins } from '@pnpm/link-bins' import { type TarballResolution } from '@pnpm/lockfile.types' import { + assertRegistryShapedResolution, type LockfileObject, nameVerFromPkgSnapshot, packageIsIndependent, @@ -29,6 +30,7 @@ import { createOrConnectStoreController } from '@pnpm/store-connection-manager' import { type DepPath, type IgnoredBuilds, + type PkgIdWithPatchHash, type ProjectManifest, type ProjectId, type ProjectRootDir, @@ -71,19 +73,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 dependencies-hierarchy 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) }) @@ -92,6 +98,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 rebuildSelectedPkgs ( @@ -110,6 +119,9 @@ export async function rebuildSelectedPkgs ( 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 @@ -160,6 +172,11 @@ export async function rebuildSelectedPkgs ( } } +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 rebuildProjects ( projects: Array<{ buildIndex: number, manifest: ProjectManifest, rootDir: ProjectRootDir }>, maybeOpts: RebuildOptions @@ -317,8 +334,8 @@ async function _rebuild ( const ignoredPkgs = new Set() const _allowBuild = createAllowBuildFunction(opts) ?? (() => true) - const allowBuild = (pkgName: string, version: string, depPath: DepPath) => { - if (_allowBuild(pkgName, version)) return true + const allowBuild = (depPath: DepPath, pkgName: string) => { + if (_allowBuild(depPath)) return true if (!opts.ignoredBuiltDependencies?.includes(pkgName)) { ignoredPkgs.add(depPath) } @@ -379,7 +396,8 @@ async function _rebuild ( requiresBuild = pkgRequiresBuild(pgkManifest, {}) } - const hasSideEffects = requiresBuild && allowBuild(pkgInfo.name, pkgInfo.version, depPath) && await runPostinstallHooks({ + assertRegistryShapedResolution(depPath, pkgSnapshot) + const hasSideEffects = requiresBuild && allowBuild(depPath, pkgInfo.name) && await runPostinstallHooks({ depPath, extraBinPaths, extraEnv: opts.extraEnv, diff --git a/exec/plugin-commands-script-runners/test/dlx.e2e.ts b/exec/plugin-commands-script-runners/test/dlx.e2e.ts index 078c60ad2e..4ceffd871f 100644 --- a/exec/plugin-commands-script-runners/test/dlx.e2e.ts +++ b/exec/plugin-commands-script-runners/test/dlx.e2e.ts @@ -78,7 +78,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() diff --git a/exec/prepare-package/src/index.ts b/exec/prepare-package/src/index.ts index cb55e1792c..aef5485ea9 100644 --- a/exec/prepare-package/src/index.ts +++ b/exec/prepare-package/src/index.ts @@ -5,7 +5,7 @@ import util from 'util' import { PnpmError } from '@pnpm/error' import { runLifecycleHook, type RunLifecycleHookOptions } from '@pnpm/lifecycle' import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json' -import { type AllowBuild, type PackageManifest } from '@pnpm/types' +import { type AllowBuild, type DepPath, type PackageManifest } from '@pnpm/types' import rimraf from '@zkochan/rimraf' import preferredPM from 'preferred-pm' import omit from 'ramda/src/omit' @@ -22,6 +22,7 @@ const PREPUBLISH_SCRIPTS = [ export interface PreparePackageOptions { allowBuild?: AllowBuild ignoreScripts?: boolean + pkgResolutionId: string rawConfig: Record unsafePerm?: boolean } @@ -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 "onlyBuiltDependencies" allowlist.`, { hint: `Add the package to "onlyBuiltDependencies" in your project's pnpm-workspace.yaml to allow it to run scripts. For example: onlyBuiltDependencies: - - "${manifest.name}"`, + - "${depPath}"`, } ) } diff --git a/exec/prepare-package/test/index.ts b/exec/prepare-package/test/index.ts index f8f86f31a5..f79620f43d 100644 --- a/exec/prepare-package/test/index.ts +++ b/exec/prepare-package/test/index.ts @@ -5,23 +5,36 @@ import { createTestIpcServer } from '@pnpm/test-ipc-server' import { fixtures } from '@pnpm/test-fixtures' const f = fixtures(__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, rawConfig: {} }, tmp, '') + await preparePackage({ allowBuild, pkgResolutionId, rawConfig: {} }, 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, + rawConfig: {}, + }, tmp, '')).rejects.toThrow('needs to execute build scripts but is not in the "onlyBuiltDependencies" 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, rawConfig: {} }, tmp, '') + await preparePackage({ allowBuild, pkgResolutionId, rawConfig: {} }, tmp, '') expect(server.getLines()).toStrictEqual([ 'prepublish', ]) @@ -31,7 +44,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, rawConfig: {} }, tmp, 'packages/foo') + await preparePackage({ allowBuild, pkgResolutionId, rawConfig: {} }, tmp, 'packages/foo') expect(server.getLines()).toStrictEqual([ 'prepublish', ]) diff --git a/fetching/git-fetcher/package.json b/fetching/git-fetcher/package.json index 9acde94a67..9b7f470c92 100644 --- a/fetching/git-fetcher/package.json +++ b/fetching/git-fetcher/package.json @@ -35,6 +35,7 @@ "@pnpm/error": "workspace:*", "@pnpm/fetcher-base": "workspace:*", "@pnpm/fs.packlist": "workspace:*", + "@pnpm/git-resolver": "workspace:*", "@pnpm/prepare-package": "workspace:*", "@zkochan/rimraf": "catalog:", "execa": "catalog:" diff --git a/fetching/git-fetcher/src/index.ts b/fetching/git-fetcher/src/index.ts index 224770ce97..4daf6628f0 100644 --- a/fetching/git-fetcher/src/index.ts +++ b/fetching/git-fetcher/src/index.ts @@ -2,6 +2,7 @@ import assert from 'assert' import path from 'path' import util from 'util' import type { GitFetcher } from '@pnpm/fetcher-base' +import { createGitHostedPkgId } from '@pnpm/git-resolver' import { packlist } from '@pnpm/fs.packlist' import { globalWarn } from '@pnpm/logger' import { preparePackage } from '@pnpm/prepare-package' @@ -44,6 +45,7 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G const prepareResult = await preparePackage({ allowBuild: opts.allowBuild, ignoreScripts: createOpts.ignoreScripts, + pkgResolutionId: createGitHostedPkgId(resolution), rawConfig: createOpts.rawConfig, unsafePerm: createOpts.unsafePerm, }, tempLocation, resolution.path ?? '') diff --git a/fetching/git-fetcher/test/index.ts b/fetching/git-fetcher/test/index.ts index 113ecf69b1..c7f2023e92 100644 --- a/fetching/git-fetcher/test/index.ts +++ b/fetching/git-fetcher/test/index.ts @@ -119,7 +119,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'), } ) @@ -199,7 +199,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`') @@ -284,7 +284,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(filesIndex['package.json']).toBeTruthy() diff --git a/fetching/git-fetcher/tsconfig.json b/fetching/git-fetcher/tsconfig.json index ae5b50e25e..810a032891 100644 --- a/fetching/git-fetcher/tsconfig.json +++ b/fetching/git-fetcher/tsconfig.json @@ -24,6 +24,9 @@ { "path": "../../packages/types" }, + { + "path": "../../resolving/git-resolver" + }, { "path": "../../store/cafs" }, diff --git a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts index 3812325acf..31bf67b0b9 100644 --- a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts @@ -81,6 +81,7 @@ async function prepareGitHostedPkg ( const { shouldBeBuilt, pkgDir } = await preparePackage({ ...opts, allowBuild: fetcherOpts.allowBuild, + pkgResolutionId: createGitHostedTarballPkgResolutionId(resolution), }, tempLocation, resolution.path ?? '') const files = await packlist(pkgDir) if (!resolution.path && files.length === Object.keys(filesIndex).length) { @@ -119,3 +120,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 +} diff --git a/fetching/tarball-fetcher/test/fetch.ts b/fetching/tarball-fetcher/test/fetch.ts index 0cd5f6fd22..2c0b28314e 100644 --- a/fetching/tarball-fetcher/test/fetch.ts +++ b/fetching/tarball-fetcher/test/fetch.ts @@ -444,7 +444,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, diff --git a/lockfile/fs/src/lockfileFormatConverters.ts b/lockfile/fs/src/lockfileFormatConverters.ts index 2b5ba38bb9..9d9c7fec2e 100644 --- a/lockfile/fs/src/lockfileFormatConverters.ts +++ b/lockfile/fs/src/lockfileFormatConverters.ts @@ -11,6 +11,7 @@ import { type PackageSnapshots, type TarballResolution, } from '@pnpm/lockfile.types' +import { isGitHostedTarballUrl } from '@pnpm/lockfile.utils' import { type DepPath, DEPENDENCIES_FIELDS } from '@pnpm/types' import isEmpty from 'ramda/src/isEmpty' import _mapValues from 'ramda/src/map' @@ -19,19 +20,6 @@ import pickBy from 'ramda/src/pickBy' import pick from 'ramda/src/pick' import { LOCKFILE_VERSION } from '@pnpm/constants' -// 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 = {} const snapshots: Record = {} diff --git a/lockfile/utils/src/assertRegistryShapedResolution.ts b/lockfile/utils/src/assertRegistryShapedResolution.ts new file mode 100644 index 0000000000..8b9895bc94 --- /dev/null +++ b/lockfile/utils/src/assertRegistryShapedResolution.ts @@ -0,0 +1,82 @@ +import { PnpmError } from '@pnpm/error' +import { type PackageSnapshot } from '@pnpm/lockfile.types' +import * as dp from '@pnpm/dependency-path' + +/** + * Matches the known git-host tarball download endpoints. Schemes and + * hostnames are case-insensitive, so the URL is matched 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. + */ +export function isGitHostedTarballUrl (url: string): boolean { + const lowerUrl = url.toLowerCase() + return ( + lowerUrl.startsWith('https://codeload.github.com/') || + lowerUrl.startsWith('https://bitbucket.org/') || + lowerUrl.startsWith('https://gitlab.com/') + ) && lowerUrl.includes('tar.gz') +} + +/** + * A registry-style depPath (name@semver) must be backed by a registry-shaped + * resolution: the allowBuild policy derives a trusted package identity from + * that key shape, which is only sound while this invariant holds. The check + * is offline and runs wherever a lockfile entry is materialized into a + * fetchable resolution or its scripts are about to run. + */ +export function assertRegistryShapedResolution (depPath: string, pkgSnapshot: PackageSnapshot): void { + const { name, version, nonSemverVersion } = dp.parse(depPath) + if (name == null || version == null || nonSemverVersion != null) return + if (isRegistryShapedResolution(pkgSnapshot.resolution)) return + throw new PnpmError('RESOLUTION_SHAPE_MISMATCH', + `Cannot use the lockfile entry of "${depPath}": its registry-style dependency path is backed by a non-registry resolution.`, + { hint: 'The lockfile may be corrupted or have been tampered with. Restore it from a trusted source, or delete it and re-run installation without --frozen-lockfile to regenerate.' } + ) +} + +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) + ) + } + 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 with a scheme is + // present it must be an http(s) artifact: a `file:` tarball under a + // name@semver key is a local artifact that a package-name rule must not + // approve. + if (typeof tarball === 'string' && tarball !== '') { + if (hasUrlScheme(tarball)) { + if (!/^https?:\/\//i.test(tarball)) return false + if (isGitHostedTarballUrl(tarball)) return false + } else if (tarball.startsWith('/') || tarball.startsWith('\\')) { + // Protocol-relative and path-absolute forms (`//host`, `/\host`, ...) + // can escape the configured registry host when resolved as a URL. + return false + } + // A scheme-less relative path is resolved against the configured + // registry, so it cannot point off-registry. + } + return true +} + +function hasUrlScheme (url: string): boolean { + return /^[a-z][a-z0-9+.-]*:/i.test(url) +} diff --git a/lockfile/utils/src/index.ts b/lockfile/utils/src/index.ts index d3f5486402..abd2d6a2ad 100644 --- a/lockfile/utils/src/index.ts +++ b/lockfile/utils/src/index.ts @@ -1,5 +1,6 @@ import { refToRelative } from '@pnpm/dependency-path' +export { assertRegistryShapedResolution, isGitHostedTarballUrl } from './assertRegistryShapedResolution.js' export { nameVerFromPkgSnapshot } from './nameVerFromPkgSnapshot.js' export { packageIdFromSnapshot } from './packageIdFromSnapshot.js' export { packageIsIndependent } from './packageIsIndependent.js' diff --git a/lockfile/utils/src/pkgSnapshotToResolution.ts b/lockfile/utils/src/pkgSnapshotToResolution.ts index a24e4a7b8a..f6fbaf3dc9 100644 --- a/lockfile/utils/src/pkgSnapshotToResolution.ts +++ b/lockfile/utils/src/pkgSnapshotToResolution.ts @@ -4,6 +4,7 @@ import { type PackageSnapshot, type TarballResolution } from '@pnpm/lockfile.typ import { type Resolution } from '@pnpm/resolver-base' import { type Registries } from '@pnpm/types' import getNpmTarballUrl from 'get-npm-tarball-url' +import { assertRegistryShapedResolution } from './assertRegistryShapedResolution.js' import { nameVerFromPkgSnapshot } from './nameVerFromPkgSnapshot.js' export function pkgSnapshotToResolution ( @@ -11,6 +12,7 @@ export function pkgSnapshotToResolution ( pkgSnapshot: PackageSnapshot, registries: Registries ): Resolution { + assertRegistryShapedResolution(depPath, pkgSnapshot) const resolution = pkgSnapshot.resolution as TarballResolution // Tarball-shaped resolutions (no `type` field) must carry `integrity`, // except where the URL itself anchors the bytes: diff --git a/lockfile/utils/test/assertRegistryShapedResolution.ts b/lockfile/utils/test/assertRegistryShapedResolution.ts new file mode 100644 index 0000000000..e7fd057d13 --- /dev/null +++ b/lockfile/utils/test/assertRegistryShapedResolution.ts @@ -0,0 +1,145 @@ +import { assertRegistryShapedResolution, isGitHostedTarballUrl } from '@pnpm/lockfile.utils' +import { type PackageSnapshot } from '@pnpm/lockfile.types' + +function snapshot (resolution: unknown): PackageSnapshot { + return { resolution } as PackageSnapshot +} + +const MISMATCH = expect.objectContaining({ code: 'ERR_PNPM_RESOLUTION_SHAPE_MISMATCH' }) + +test('rejects a registry-style depPath backed by a git resolution', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + type: 'git', repo: 'https://example.com/foo.git', commit: 'abc123', + })) + }).toThrow(MISMATCH) +}) + +test('rejects a registry-style depPath backed by a git-hosted tarball resolution', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://codeload.github.com/org/foo/tar.gz/abc123', gitHosted: true, + })) + }).toThrow(MISMATCH) +}) + +test('rejects a registry-style depPath backed by a directory resolution', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + type: 'directory', directory: '../foo', + })) + }).toThrow(MISMATCH) +}) + +test('accepts registry-style depPaths with registry and all-registry variations resolutions', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-a', + })) + }).not.toThrow() + expect(() => { + assertRegistryShapedResolution('bar@1.0.0', snapshot({ + type: 'variations', + variants: [ + { targets: [{ os: 'darwin' }], resolution: { integrity: 'sha512-a' } }, + { targets: [{ os: 'linux' }], resolution: { integrity: 'sha512-b' } }, + ], + })) + }).not.toThrow() +}) + +test('rejects a registry-style depPath whose variations resolution hides a git variant', () => { + expect(() => { + assertRegistryShapedResolution('bar@1.0.0', snapshot({ + type: 'variations', + variants: [ + { targets: [{ os: 'darwin' }], resolution: { integrity: 'sha512-a' } }, + { targets: [{ os: 'linux' }], resolution: { type: 'git', repo: 'https://example.com/bar.git', commit: 'abc123' } }, + ], + })) + }).toThrow(MISMATCH) +}) + +test('does not flag artifact depPaths with non-registry resolutions', () => { + expect(() => { + assertRegistryShapedResolution('foo@git+https://example.com/foo.git#abc123', snapshot({ + type: 'git', repo: 'https://example.com/foo.git', commit: 'abc123', + })) + }).not.toThrow() + expect(() => { + assertRegistryShapedResolution('bar@https://example.com/bar.tgz', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://example.com/bar.tgz', + })) + }).not.toThrow() +}) + +test('rejects a registry-style depPath whose git-host tarball clears the gitHosted flag', () => { + // 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]) { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://codeload.github.com/org/foo/tar.gz/abc123', gitHosted, + })) + }).toThrow(MISMATCH) + } +}) + +test('rejects a registry-style depPath with a non-boolean gitHosted flag', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://registry.npmjs.org/foo/-/foo-1.0.0.tgz', gitHosted: 'true', + })) + }).toThrow(MISMATCH) +}) + +test('rejects a registry-style depPath backed by a non-http(s) tarball URL', () => { + for (const tarball of ['file:///tmp/evil.tgz', 'ftp://example.com/evil.tgz']) { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball, + })) + }).toThrow(MISMATCH) + } +}) + +test('rejects a registry-style depPath whose tarball escapes the registry host without a scheme', () => { + for (const tarball of ['//evil.example.com/foo.tgz', '/\\evil.example.com/foo.tgz', '\\\\evil.example.com\\foo.tgz']) { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball, + })) + }).toThrow(MISMATCH) + } +}) + +test('accepts a registry-style depPath whose tarball is an http(s) or registry-relative URL', () => { + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://registry.npmjs.org/foo/-/foo-1.0.0.tgz', + })) + }).not.toThrow() + // Lockfiles written by older pnpm versions store registry-relative tarball + // paths, which resolve against the configured registry. + expect(() => { + assertRegistryShapedResolution('@mycompany/mypackage@2.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: '@mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz', + })) + }).not.toThrow() +}) + +test('rejects a registry-style depPath whose git-host tarball varies the host casing', () => { + // Hostnames are case-insensitive; an upper-case codeload host paired with + // gitHosted: false must not pass as registry-shaped. + expect(() => { + assertRegistryShapedResolution('foo@1.0.0', snapshot({ + integrity: 'sha512-deadbeef', tarball: 'https://CODELOAD.GITHUB.COM/org/foo/tar.gz/abc123', gitHosted: false, + })) + }).toThrow(MISMATCH) +}) + +test('isGitHostedTarballUrl() matches the known git hosts regardless of casing', () => { + expect(isGitHostedTarballUrl('https://codeload.github.com/org/foo/tar.gz/abc123')).toBe(true) + expect(isGitHostedTarballUrl('https://CODELOAD.GITHUB.COM/org/foo/TAR.GZ/abc123')).toBe(true) + expect(isGitHostedTarballUrl('https://registry.npmjs.org/foo/-/foo-1.0.0.tgz')).toBe(false) +}) diff --git a/packages/dependency-path/src/index.ts b/packages/dependency-path/src/index.ts index d6e7c23d07..91ce6e5116 100644 --- a/packages/dependency-path/src/index.ts +++ b/packages/dependency-path/src/index.ts @@ -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 { diff --git a/packages/types/src/config.ts b/packages/types/src/config.ts index c5182bd1a5..546e5c6ae9 100644 --- a/packages/types/src/config.ts +++ b/packages/types/src/config.ts @@ -1,5 +1,7 @@ +import { type DepPath } from './misc.js' + export type PackageVersionPolicy = (pkgName: string) => boolean | string[] -export type AllowBuild = (pkgName: string, pkgVersion: string) => boolean +export type AllowBuild = (depPath: DepPath) => boolean export type TrustPolicy = 'no-downgrade' | 'off' diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index 72edfe755b..497a4ee567 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -4,7 +4,6 @@ import { createAllowBuildFunction } from '@pnpm/builder.policy' import { parseCatalogProtocol } from '@pnpm/catalogs.protocol-parser' import { resolveFromCatalog, matchCatalogResolveResult, type CatalogResultMatcher } from '@pnpm/catalogs.resolver' import { type Catalogs } from '@pnpm/catalogs.types' -import { createPackageVersionPolicy } from '@pnpm/config.version-policy' import { LAYOUT_VERSION, LOCKFILE_VERSION, @@ -871,18 +870,16 @@ async function runUnignoredDependencyBuilds (opts: StrictInstallOptions, previou if (!opts.onlyBuiltDependencies?.length) { return previousIgnoredBuilds } - const onlyBuiltDeps = createPackageVersionPolicy(opts.onlyBuiltDependencies) - const pkgsToBuild = Array.from(previousIgnoredBuilds).flatMap((ignoredPkg) => { - const ignoredPkgName = dp.parse(ignoredPkg).name - if (!ignoredPkgName) return [] - const matchResult = onlyBuiltDeps(ignoredPkgName) - if (matchResult === true) { - return [ignoredPkgName] - } else if (Array.isArray(matchResult)) { - return matchResult.map(version => `${ignoredPkgName}@${version}`) + const allowBuild = createAllowBuildFunction(opts) + if (!allowBuild) { + return previousIgnoredBuilds + } + const pkgsToBuild: string[] = [] + for (const ignoredPkg of previousIgnoredBuilds) { + if (allowBuild(ignoredPkg)) { + pkgsToBuild.push(dp.getPkgIdWithPatchHash(ignoredPkg)) } - return [] - }) + } if (pkgsToBuild.length) { return (await rebuildSelectedPkgs(opts.allProjects, pkgsToBuild, { ...opts, @@ -1724,7 +1721,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) } /** diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index 98c24098e9..744d9e5e7c 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -474,7 +474,7 @@ async function linkAllPkgs ( depNode.requiresBuild = files.requiresBuild let sideEffectsCacheKey: string | undefined if (opts.sideEffectsCacheRead && files.sideEffects && !isEmpty(files.sideEffects)) { - if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { + if (opts?.allowBuild?.(depNode.depPath) !== false) { sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.depPath, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.file.hash, diff --git a/pkg-manager/core/test/install/lifecycleScripts.ts b/pkg-manager/core/test/install/lifecycleScripts.ts index c610d0c44f..c8d7dc8562 100644 --- a/pkg-manager/core/test/install/lifecycleScripts.ts +++ b/pkg-manager/core/test/install/lifecycleScripts.ts @@ -1,5 +1,7 @@ import * as path from 'path' import fs from 'fs' +import http from 'http' +import { type AddressInfo } from 'net' import { assertProject } from '@pnpm/assert-project' import { type LifecycleLog } from '@pnpm/core-loggers' import { prepareEmpty, preparePackages } from '@pnpm/prepare' @@ -12,6 +14,7 @@ import { } from '@pnpm/core' import { createTestIpcServer } from '@pnpm/test-ipc-server' import { type ProjectRootDir } from '@pnpm/types' +import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock' import { restartWorkerPool } from '@pnpm/worker' import { sync as rimraf } from '@zkochan/rimraf' import isWindows from 'is-windows' @@ -310,7 +313,9 @@ test('run prepare script for git-hosted dependencies', async () => { await addDependenciesToPackage({}, ['pnpm/test-git-fetch#8b333f12d5357f4f25a654c305c826294cb073bf'], testDefaults({ fastUnpack: false, - onlyBuiltDependencies: ['test-git-fetch'], + // A git-hosted artifact has an untrusted package identity, so it has to + // be approved by its depPath, not by package name. + onlyBuiltDependencies: ['test-git-fetch@https://codeload.github.com/pnpm/test-git-fetch/tar.gz/8b333f12d5357f4f25a654c305c826294cb073bf'], neverBuiltDependencies: undefined, })) @@ -326,6 +331,48 @@ test('run prepare script for git-hosted dependencies', async () => { ]) }) +test('onlyBuiltDependencies does not run lifecycle scripts for direct tarball identities', async () => { + prepareEmpty() + // A tarball URL on the configured registry host resolves as a registry + // package, so serve the artifact from a separate origin to keep it a + // direct-tarball identity. The server redirects to the registry mock. + const server = http.createServer((req, res) => { + res.statusCode = 302 + res.setHeader('location', `http://localhost:${REGISTRY_MOCK_PORT}${req.url ?? ''}`) + res.end() + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + try { + const { port } = server.address() as AddressInfo + const tarball = `http://127.0.0.1:${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, + onlyBuiltDependencies: ['@pnpm.e2e/pre-and-postinstall-scripts-example'], + neverBuiltDependencies: undefined, + })) + + 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) + + rimraf('node_modules') + + await install(manifest, testDefaults({ + fastUnpack: false, + onlyBuiltDependencies: [depPath], + neverBuiltDependencies: undefined, + })) + + 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) + } finally { + server.close() + } +}) + test('lifecycle scripts run before linking bins', async () => { const project = prepareEmpty() diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 9163e648c5..fe5fe5e2a0 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -887,7 +887,7 @@ async function linkAllPkgs ( depNode.requiresBuild = filesResponse.requiresBuild let sideEffectsCacheKey: string | undefined if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) { - if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { + if (opts?.allowBuild?.(depNode.depPath) !== false) { sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.dir, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.file.hash, diff --git a/pkg-manager/headless/src/linkHoistedModules.ts b/pkg-manager/headless/src/linkHoistedModules.ts index 62502805a4..961ef1f9b0 100644 --- a/pkg-manager/headless/src/linkHoistedModules.ts +++ b/pkg-manager/headless/src/linkHoistedModules.ts @@ -116,7 +116,7 @@ async function linkAllPkgsInOrder ( depNode.requiresBuild = filesResponse.requiresBuild let sideEffectsCacheKey: string | undefined if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) { - if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) { + if (opts?.allowBuild?.(depNode.depPath) !== false) { sideEffectsCacheKey = _calcDepState(dir, { includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built patchFileHash: depNode.patch?.file.hash, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c0eb88627f..14657d77d5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -820,6 +820,7 @@ overrides: send@<0.19.0: ^0.19.0 serve-static@<1.16.0: ^1.16.0 shell-quote@<1.8.4: '>=1.8.4' + shell-quote: 1.8.4 socks@2: ^2.8.1 tar@<=7.5.10: '>=7.5.11' tmp@<0.2.6: '>=0.2.6' @@ -1257,6 +1258,9 @@ importers: '@pnpm/config.version-policy': specifier: workspace:* version: link:../../config/version-policy + '@pnpm/dependency-path': + specifier: workspace:* + version: link:../../packages/dependency-path '@pnpm/types': specifier: workspace:* version: link:../../packages/types @@ -2499,15 +2503,15 @@ importers: exec/build-commands: dependencies: + '@pnpm/builder.policy': + specifier: workspace:* + version: link:../../builder/policy '@pnpm/config': specifier: workspace:* version: link:../../config/config '@pnpm/config.config-writer': specifier: workspace:* version: link:../../config/config-writer - '@pnpm/dependency-path': - specifier: workspace:* - version: link:../../packages/dependency-path '@pnpm/logger': specifier: 'catalog:' version: 1001.0.1 @@ -3204,6 +3208,9 @@ importers: '@pnpm/fs.packlist': specifier: workspace:* version: link:../../fs/packlist + '@pnpm/git-resolver': + specifier: workspace:* + version: link:../../resolving/git-resolver '@pnpm/prepare-package': specifier: workspace:* version: link:../../exec/prepare-package diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index ca1f1aa278..7b7576ec5f 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -398,6 +398,8 @@ overrides: send@<0.19.0: ^0.19.0 serve-static@<1.16.0: ^1.16.0 shell-quote@<1.8.4: '>=1.8.4' + # 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.10: '>=7.5.11' tmp@<0.2.6: '>=0.2.6' diff --git a/pnpm/package.json b/pnpm/package.json index 072b3a2f41..9ddc0b5f60 100644 --- a/pnpm/package.json +++ b/pnpm/package.json @@ -219,6 +219,7 @@ "send@<0.19.0": "^0.19.0", "serve-static@<1.16.0": "^1.16.0", "shell-quote@<1.8.4": ">=1.8.4", + "shell-quote": "1.8.4", "socks@2": "^2.8.1", "tar@<=7.5.10": ">=7.5.11", "tmp@<0.2.6": ">=0.2.6", diff --git a/pnpm/test/dlx.ts b/pnpm/test/dlx.ts index 047a769e4f..1066adde2f 100644 --- a/pnpm/test/dlx.ts +++ b/pnpm/test/dlx.ts @@ -173,7 +173,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') diff --git a/resolving/git-resolver/src/createGitHostedPkgId.ts b/resolving/git-resolver/src/createGitHostedPkgId.ts index b9d6542a5b..12ae9a7f16 100644 --- a/resolving/git-resolver/src/createGitHostedPkgId.ts +++ b/resolving/git-resolver/src/createGitHostedPkgId.ts @@ -1,10 +1,21 @@ import { type PkgResolutionId } from '@pnpm/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]}` +} diff --git a/resolving/git-resolver/test/createGitHostedPkgId.test.ts b/resolving/git-resolver/test/createGitHostedPkgId.test.ts index 6a6f64dd25..2b96e3ff5d 100644 --- a/resolving/git-resolver/test/createGitHostedPkgId.test.ts +++ b/resolving/git-resolver/test/createGitHostedPkgId.test.ts @@ -2,6 +2,10 @@ import { createGitHostedPkgId } from '@pnpm/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) => { diff --git a/reviewing/plugin-commands-licenses/test/fixtures/with-git-protocol-peer-deps/package.json b/reviewing/plugin-commands-licenses/test/fixtures/with-git-protocol-peer-deps/package.json index 6bdb81b2fc..24d78025ef 100644 --- a/reviewing/plugin-commands-licenses/test/fixtures/with-git-protocol-peer-deps/package.json +++ b/reviewing/plugin-commands-licenses/test/fixtures/with-git-protocol-peer-deps/package.json @@ -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" } } diff --git a/reviewing/plugin-commands-licenses/test/index.ts b/reviewing/plugin-commands-licenses/test/index.ts index 55f7a021bd..5137e0096c 100644 --- a/reviewing/plugin-commands-licenses/test/index.ts +++ b/reviewing/plugin-commands-licenses/test/index.ts @@ -310,7 +310,9 @@ test('pnpm licenses should work with git protocol dep that have peerDependencies await install.handler({ ...DEFAULT_OPTS, dir: workspaceDir, - onlyBuiltDependencies: ['ajv-keywords'], + onlyBuiltDependencies: [ + 'ajv-keywords@https://codeload.github.com/ajv-validator/ajv-keywords/tar.gz/a11389b4d1934d360fb2a24dd920ec597295c8fc', + ], pnpmHomeDir: '', storeDir, })