diff --git a/.changeset/require-tarball-integrity.md b/.changeset/require-tarball-integrity.md new file mode 100644 index 0000000000..f8c34b6467 --- /dev/null +++ b/.changeset/require-tarball-integrity.md @@ -0,0 +1,6 @@ +--- +"@pnpm/lockfile.utils": patch +"pnpm": patch +--- + +Reject `pnpm-lock.yaml` entries whose remote tarball `resolution:` block is missing the `integrity` field. Previously the worker that extracts a downloaded tarball skipped hash verification when no integrity was supplied and minted a fresh one from the unverified bytes, so an attacker who could both alter the lockfile (e.g. via a pull request that strips `integrity:`) and serve modified content at the referenced tarball URL could install a tampered package without any error — including under `--frozen-lockfile`. pnpm now fails closed at lockfile-read time with `ERR_PNPM_MISSING_TARBALL_INTEGRITY`. Git-hosted tarballs (`gitHosted: true` or a URL on codeload.github.com / bitbucket.org / gitlab.com) and `file:` tarballs are exempt — the commit SHA in a git-host URL and the user-controlled local path already anchor the bytes. diff --git a/lockfile/utils/package.json b/lockfile/utils/package.json index e07ba2dff9..3d7041af46 100644 --- a/lockfile/utils/package.json +++ b/lockfile/utils/package.json @@ -34,6 +34,7 @@ }, "dependencies": { "@pnpm/dependency-path": "workspace:*", + "@pnpm/error": "workspace:*", "@pnpm/lockfile.types": "workspace:*", "@pnpm/resolver-base": "workspace:*", "@pnpm/types": "workspace:*", diff --git a/lockfile/utils/src/pkgSnapshotToResolution.ts b/lockfile/utils/src/pkgSnapshotToResolution.ts index 3f0e27b596..a24e4a7b8a 100644 --- a/lockfile/utils/src/pkgSnapshotToResolution.ts +++ b/lockfile/utils/src/pkgSnapshotToResolution.ts @@ -1,4 +1,5 @@ import url from 'url' +import { PnpmError } from '@pnpm/error' import { type PackageSnapshot, type TarballResolution } from '@pnpm/lockfile.types' import { type Resolution } from '@pnpm/resolver-base' import { type Registries } from '@pnpm/types' @@ -10,10 +11,33 @@ export function pkgSnapshotToResolution ( pkgSnapshot: PackageSnapshot, registries: Registries ): Resolution { + const resolution = pkgSnapshot.resolution as TarballResolution + // Tarball-shaped resolutions (no `type` field) must carry `integrity`, + // except where the URL itself anchors the bytes: + // - `file:` tarballs (local file on the user's machine; integrity + // adds nothing the user doesn't already control). + // - Git-hosted tarballs (URL contains the commit SHA; git's content- + // addressed model binds the bytes to the commit). The `gitHosted` + // flag may be absent on legacy lockfiles, so fall back to a URL + // match against the known git-host download endpoints. + // For any other tarball entry a missing integrity is what a tampered + // lockfile looks like: the worker would mint a fresh integrity from + // whatever bytes the URL returned, so we fail closed here. if ( - Boolean((pkgSnapshot.resolution as TarballResolution).type) || - (pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:') || - (pkgSnapshot.resolution as TarballResolution).gitHosted === true + resolution.type == null && + resolution.integrity == null && + !resolution.tarball?.startsWith('file:') && + !(resolution.gitHosted === true || (resolution.tarball != null && isGitHostedTarballUrl(resolution.tarball))) + ) { + throw new PnpmError('MISSING_TARBALL_INTEGRITY', + `Cannot install package "${depPath}": its lockfile entry has no "integrity" field, so pnpm cannot verify the downloaded tarball.`, + { 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.' } + ) + } + if ( + Boolean(resolution.type) || + resolution.tarball?.startsWith('file:') || + resolution.gitHosted === true ) { return pkgSnapshot.resolution as Resolution } @@ -47,3 +71,15 @@ export function pkgSnapshotToResolution ( return getNpmTarballUrl(name, version, { registry }) } } + +// Fallback for legacy lockfile entries whose tarball resolution lacks the +// `gitHosted` flag. Matches the known git-host download endpoints so a URL +// rewritten by pnpm's git resolver is still recognized as content-addressed +// and exempt from the integrity requirement. +function isGitHostedTarballUrl (url: string): boolean { + return ( + url.startsWith('https://codeload.github.com/') || + url.startsWith('https://bitbucket.org/') || + url.startsWith('https://gitlab.com/') + ) +} diff --git a/lockfile/utils/test/pkgSnapshotToResolution.ts b/lockfile/utils/test/pkgSnapshotToResolution.ts index bfb34f65df..cce4b79da0 100644 --- a/lockfile/utils/test/pkgSnapshotToResolution.ts +++ b/lockfile/utils/test/pkgSnapshotToResolution.ts @@ -32,9 +32,67 @@ test('pkgSnapshotToResolution()', () => { expect(pkgSnapshotToResolution('@cdn.sheetjs.com/xlsx-0.18.9/xlsx-0.18.9.tgz', { resolution: { + integrity: 'sha512-CCCC', tarball: 'https://cdn.sheetjs.com/xlsx-0.18.9/xlsx-0.18.9.tgz', }, }, { default: 'https://registry.npmjs.org/' })).toEqual({ + integrity: 'sha512-CCCC', tarball: 'https://cdn.sheetjs.com/xlsx-0.18.9/xlsx-0.18.9.tgz', }) }) + +test('pkgSnapshotToResolution() rejects a remote tarball resolution that has no integrity', () => { + // A tampered or malformed lockfile that strips the `integrity` field + // would otherwise let pnpm download the URL contents unchecked. The + // helper must fail closed so neither install path nor any read-only + // consumer (sbom, list, etc.) silently trusts the lockfile entry. + expect(() => pkgSnapshotToResolution('foo@1.0.0', { + resolution: { + tarball: 'https://registry.npmjs.org/foo/-/foo-1.0.0.tgz', + }, + }, { default: 'https://registry.npmjs.org/' })).toThrow(expect.objectContaining({ code: 'ERR_PNPM_MISSING_TARBALL_INTEGRITY' })) + + // A tarball URL on an arbitrary CDN (no `gitHosted` flag, no known git + // host pattern) is still a regular remote tarball — integrity required. + expect(() => pkgSnapshotToResolution('xlsx@https+++cdn.sheetjs.com+xlsx-0.18.9+xlsx-0.18.9.tgz', { + resolution: { + tarball: 'https://cdn.sheetjs.com/xlsx-0.18.9/xlsx-0.18.9.tgz', + }, + }, { default: 'https://registry.npmjs.org/' })).toThrow(expect.objectContaining({ code: 'ERR_PNPM_MISSING_TARBALL_INTEGRITY' })) +}) + +test('pkgSnapshotToResolution() allows git-hosted and file: tarballs to lack integrity', () => { + // Git-hosted tarballs are anchored by the commit SHA in their URL — + // pnpm's own install pipeline writes them without `integrity:` (see + // the `with-git-protocol-dep` fixture). Both the explicit + // `gitHosted: true` flag and a URL on a known git host must bypass + // the integrity check. + expect(pkgSnapshotToResolution('foo@https+++github.com+foo+bar', { + resolution: { + tarball: 'https://codeload.github.com/foo/bar/tar.gz/abc1234', + gitHosted: true, + }, + }, { default: 'https://registry.npmjs.org/' })).toEqual({ + tarball: 'https://codeload.github.com/foo/bar/tar.gz/abc1234', + gitHosted: true, + }) + + expect(pkgSnapshotToResolution('is-negative@https+++codeload.github.com+kevva+is-negative+tar.gz+abc', { + resolution: { + tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/abc1234', + }, + }, { default: 'https://registry.npmjs.org/' })).toEqual({ + tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/abc1234', + }) + + // `file:` tarballs are local files; the user already controls the + // bytes, and the install pipeline may write them without integrity. + expect(pkgSnapshotToResolution('local-pkg@file:local-pkg-1.0.0.tgz', { + resolution: { + tarball: 'file:local-pkg-1.0.0.tgz', + }, + version: '1.0.0', + }, { default: 'https://registry.npmjs.org/' })).toEqual({ + tarball: 'file:local-pkg-1.0.0.tgz', + }) +}) diff --git a/lockfile/utils/tsconfig.json b/lockfile/utils/tsconfig.json index e6aee5c316..dc3564a8aa 100644 --- a/lockfile/utils/tsconfig.json +++ b/lockfile/utils/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../../packages/dependency-path" }, + { + "path": "../../packages/error" + }, { "path": "../../packages/types" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9023568e5e..6ed4b44ac2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4054,6 +4054,9 @@ importers: '@pnpm/dependency-path': specifier: workspace:* version: link:../../packages/dependency-path + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error '@pnpm/lockfile.types': specifier: workspace:* version: link:../types