diff --git a/.changeset/git-tarball-integrity.md b/.changeset/git-tarball-integrity.md new file mode 100644 index 0000000000..0c809e985b --- /dev/null +++ b/.changeset/git-tarball-integrity.md @@ -0,0 +1,20 @@ +--- +"@pnpm/building.after-install": patch +"@pnpm/fetching.pick-fetcher": patch +"@pnpm/fetching.tarball-fetcher": patch +"@pnpm/installing.deps-installer": patch +"@pnpm/installing.package-requester": patch +"@pnpm/lockfile.fs": patch +"@pnpm/lockfile.types": patch +"@pnpm/lockfile.utils": patch +"@pnpm/modules-mounter.daemon": patch +"@pnpm/resolving.git-resolver": patch +"@pnpm/resolving.resolver-base": patch +"@pnpm/store.commands": patch +"@pnpm/store.pkg-finder": patch +"pnpm": patch +--- + +Pin the integrity of git-hosted tarballs (codeload.github.com, gitlab.com, bitbucket.org) in the lockfile so that subsequent installs detect a tampered or substituted tarball and refuse to install it. Previously the lockfile only stored the tarball URL for git dependencies, so a compromised git host or a man-in-the-middle could serve arbitrary code on later installs without lockfile changes. + +A new `gitHosted: true` field is recorded on git-hosted tarball resolutions in the lockfile, letting every reader/writer route them by a single typed check instead of pattern-matching the tarball URL in each call site. Lockfiles written by older pnpm versions are enriched on load (URL fallback) so the field can be relied on uniformly across the codebase. diff --git a/building/after-install/src/index.ts b/building/after-install/src/index.ts index 474341e965..0f9ffc00e9 100644 --- a/building/after-install/src/index.ts +++ b/building/after-install/src/index.ts @@ -33,7 +33,7 @@ import npa from '@pnpm/npm-package-arg' import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader' import type { PackageFilesIndex } from '@pnpm/store.cafs' import { createStoreController } from '@pnpm/store.connection-manager' -import { StoreIndex, storeIndexKey } from '@pnpm/store.index' +import { pickStoreIndexKey, StoreIndex } from '@pnpm/store.index' import type { DepPath, IgnoredBuilds, @@ -358,9 +358,12 @@ async function _rebuild ( } const resolution = (pkgSnapshot.resolution as TarballResolution) let sideEffectsCacheKey: string | undefined - const pkgId = `${pkgInfo.name}@${pkgInfo.version}` - if (opts.skipIfHasSideEffectsCache && resolution.integrity) { - const filesIndexFile = storeIndexKey(resolution.integrity!.toString(), pkgId) + // Match the resolver-supplied pkg.id used by the writer in + // @pnpm/installing.package-requester: that's the tarball URL for + // git-hosted packages (nonSemverVersion) and `name@version` otherwise. + const pkgId = pkgInfo.nonSemverVersion ?? `${pkgInfo.name}@${pkgInfo.version}` + if (opts.skipIfHasSideEffectsCache && (resolution.gitHosted || resolution.integrity)) { + const filesIndexFile = pickStoreIndexKey(resolution, pkgId, { built: true }) const pkgFilesIndex = storeIndex!.get(filesIndexFile) as PackageFilesIndex | undefined if (pkgFilesIndex) { sideEffectsCacheKey = calcDepState(depGraph, depsStateCache, depPath, { @@ -393,9 +396,9 @@ async function _rebuild ( unsafePerm: opts.unsafePerm || false, userAgent: opts.userAgent, }) - if (hasSideEffects && (opts.sideEffectsCacheWrite ?? true) && resolution.integrity) { + if (hasSideEffects && (opts.sideEffectsCacheWrite ?? true) && (resolution.gitHosted || resolution.integrity)) { builtDepPaths.add(depPath) - const filesIndexFile = storeIndexKey(resolution.integrity!.toString(), pkgId) + const filesIndexFile = pickStoreIndexKey(resolution, pkgId, { built: true }) try { if (!sideEffectsCacheKey) { sideEffectsCacheKey = calcDepState(depGraph, depsStateCache, depPath, { diff --git a/fetching/pick-fetcher/src/index.ts b/fetching/pick-fetcher/src/index.ts index b87b157602..62722d04f9 100644 --- a/fetching/pick-fetcher/src/index.ts +++ b/fetching/pick-fetcher/src/index.ts @@ -40,7 +40,12 @@ export async function pickFetcher ( if ('tarball' in resolution && resolution.tarball) { if (resolution.tarball.startsWith('file:')) { fetcherType = 'localTarball' - } else if (isGitHostedPkgUrl(resolution.tarball)) { + } else if ( + ('gitHosted' in resolution && resolution.gitHosted === true) || + // URL fallback for resolutions that didn't go through the resolver or + // the lockfile loader (e.g., constructed ad-hoc). + isGitHostedPkgUrl(resolution.tarball) + ) { fetcherType = 'gitHostedTarball' } else { fetcherType = 'remoteTarball' diff --git a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts index 1be2c0f200..c570b8391a 100644 --- a/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts +++ b/fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts @@ -26,7 +26,7 @@ export interface CreateGitHostedTarballFetcher { export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction, fetcherOpts: CreateGitHostedTarballFetcher): FetchFunction { const fetch = async (cafs: Cafs, resolution: Resolution, opts: FetchOptions) => { const rawFilesIndexFile = `${opts.filesIndexFile}\traw` - const { filesMap, manifest, requiresBuild } = await fetchRemoteTarball(cafs, resolution, { + const { filesMap, manifest, requiresBuild, integrity } = await fetchRemoteTarball(cafs, resolution, { ...opts, filesIndexFile: rawFilesIndexFile, }) @@ -42,6 +42,9 @@ export function createGitHostedTarballFetcher (fetchRemoteTarball: FetchFunction filesMap: prepareResult.filesMap, manifest: prepareResult.manifest ?? manifest, requiresBuild, + // Propagate the raw tarball integrity so the lockfile pins it and + // future installs detect a tampered tarball from the git host. + integrity, } } catch (err: unknown) { assert(util.types.isNativeError(err)) diff --git a/fetching/tarball-fetcher/test/fetch.ts b/fetching/tarball-fetcher/test/fetch.ts index 9d0ad7db58..2aea9598c8 100644 --- a/fetching/tarball-fetcher/test/fetch.ts +++ b/fetching/tarball-fetcher/test/fetch.ts @@ -506,6 +506,29 @@ test('take only the files included in the package, when fetching a git-hosted pa 'dist/index.js', 'package.json', ]) + // The fetcher must surface the integrity of the downloaded git tarball so + // that the lockfile can pin it (CVE: malicious codeload.github.com responses). + expect(result.integrity).toMatch(/^sha512-/) +}) + +test('verify integrity of git-hosted tarball against the resolution', async () => { + // Enable network for this test + mockAgent.enableNetConnect(/codeload\.github\.com/) + + process.chdir(temporaryDirectory()) + + const resolution = { + tarball: 'https://codeload.github.com/pnpm-e2e/pkg-with-ignored-files/tar.gz/958d6d487217512bb154d02836e9b5b922a600d8', + // A well-formed sha512 SRI that doesn't match the actual tarball — should + // surface as TarballIntegrityError when the buffer is verified. + integrity: 'sha512-MRqvs50psUtGELoeBcJwDUi7lT6RUXBzTHsU3U701V/DIouBQSZo+tx5xSXDJLEcItepyZPjIncx8Xy4qPFlKw==', + } + + await expect(fetch.gitHostedTarball(cafs, resolution, { + filesIndexFile, + lockfileDir: process.cwd(), + pkg, + })).rejects.toThrow(TarballIntegrityError) }) test('fail when extracting a broken tarball', async () => { diff --git a/installing/deps-installer/src/install/index.ts b/installing/deps-installer/src/install/index.ts index e34612578d..eb0cdcb604 100644 --- a/installing/deps-installer/src/install/index.ts +++ b/installing/deps-installer/src/install/index.ts @@ -2181,7 +2181,11 @@ async function installFromPnpmRegistry ( await fileDownloads const resolution = fetchOpts.pkg.resolution const integrity = resolution?.integrity - if (integrity) { + // Fall through to the regular store controller for git-hosted tarballs. + // Their cached entry lives under gitHostedStoreIndexKey (preserves the + // built/not-built dimension), not the integrity-keyed path the agent + // uses for npm tarballs. See @pnpm/store.pkg-finder for the rationale. + if (integrity && !resolution?.gitHosted) { const filesIndexFile = _storeIndexKey(integrity, fetchOpts.pkg.id) const result = await readPkgFromCafs( { storeDir: opts.storeDir, verifyStoreIntegrity: false }, diff --git a/installing/deps-installer/test/install/fromRepo.ts b/installing/deps-installer/test/install/fromRepo.ts index cc2e5bd3bb..4dd906763b 100644 --- a/installing/deps-installer/test/install/fromRepo.ts +++ b/installing/deps-installer/test/install/fromRepo.ts @@ -295,7 +295,11 @@ test('re-adding a git repo with a different tag', async () => { expect(lockfile.packages).toEqual( { 'is-negative@https://codeload.github.com/kevva/is-negative/tar.gz/163360a8d3ae6bee9524541043197ff356f8ed99': { - resolution: { tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/163360a8d3ae6bee9524541043197ff356f8ed99' }, + resolution: { + tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/163360a8d3ae6bee9524541043197ff356f8ed99', + integrity: expect.stringMatching(/^sha512-/), + gitHosted: true, + }, version: '1.0.0', engines: { node: '>=0.10.0' }, }, @@ -312,7 +316,11 @@ test('re-adding a git repo with a different tag', async () => { expect(lockfile.packages).toEqual( { 'is-negative@https://codeload.github.com/kevva/is-negative/tar.gz/f7dec4d66a5a56719e49b9f94a24d73f924ddeb3': { - resolution: { tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/f7dec4d66a5a56719e49b9f94a24d73f924ddeb3' }, + resolution: { + tarball: 'https://codeload.github.com/kevva/is-negative/tar.gz/f7dec4d66a5a56719e49b9f94a24d73f924ddeb3', + integrity: expect.stringMatching(/^sha512-/), + gitHosted: true, + }, version: '1.0.1', engines: { node: '>=0.10.0' }, }, diff --git a/installing/package-requester/src/packageRequester.ts b/installing/package-requester/src/packageRequester.ts index 662eb17da8..c7c9d78d33 100644 --- a/installing/package-requester/src/packageRequester.ts +++ b/installing/package-requester/src/packageRequester.ts @@ -43,7 +43,7 @@ import type { RequestPackageOptions, WantedDependency, } from '@pnpm/store.controller-types' -import { gitHostedStoreIndexKey, storeIndexKey } from '@pnpm/store.index' +import { pickStoreIndexKey } from '@pnpm/store.index' import type { DependencyManifest, SupportedArchitectures } from '@pnpm/types' import { calcMaxWorkers, @@ -345,28 +345,18 @@ function getFilesIndexFilePath ( ): GetFilesIndexFilePathResult { const targetRelative = depPathToFilename(opts.pkg.id, ctx.virtualStoreDirMaxLength) const target = path.join(ctx.storeDir, targetRelative) - if ((opts.pkg.resolution as TarballResolution).integrity) { - return { - target, - filesIndexFile: storeIndexKey((opts.pkg.resolution as TarballResolution).integrity!, opts.pkg.id), - resolution: opts.pkg.resolution as AtomicResolution, - } - } - let resolution!: AtomicResolution + const built = !opts.ignoreScripts + let resolution: AtomicResolution if (opts.pkg.resolution.type === 'variations') { resolution = findResolution(opts.pkg.resolution.variants, opts.supportedArchitectures) - if ((resolution as TarballResolution).integrity) { - return { - target, - filesIndexFile: storeIndexKey((resolution as TarballResolution).integrity!, opts.pkg.id), - resolution, - } - } } else { resolution = opts.pkg.resolution } - const filesIndexFile = gitHostedStoreIndexKey(opts.pkg.id, { built: !opts.ignoreScripts }) - return { filesIndexFile, target, resolution } + return { + target, + filesIndexFile: pickStoreIndexKey(resolution as TarballResolution, opts.pkg.id, { built }), + resolution, + } } function findResolution (resolutionVariants: PlatformAssetResolution[], supportedArchitectures?: SupportedArchitectures): AtomicResolution { diff --git a/lockfile/fs/src/lockfileFormatConverters.ts b/lockfile/fs/src/lockfileFormatConverters.ts index fb2470ade8..d8415db4ed 100644 --- a/lockfile/fs/src/lockfileFormatConverters.ts +++ b/lockfile/fs/src/lockfileFormatConverters.ts @@ -10,10 +10,24 @@ import type { PackageSnapshots, ProjectSnapshot, ResolvedDependencies, + TarballResolution, } from '@pnpm/lockfile.types' 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 = {} const snapshots: Record = {} @@ -131,6 +145,7 @@ export function convertToLockfileObject (lockfile: LockfileFile): LockfileObject for (const [depPath, pkg] of Object.entries(lockfile.snapshots ?? {})) { const pkgId = removeSuffix(depPath) packages[depPath as DepPath] = Object.assign(pkg, lockfile.packages?.[pkgId]) + enrichGitHostedFlag(packages[depPath as DepPath]?.resolution as TarballResolution | undefined) } return { ...omit(['snapshots'], rest), @@ -140,6 +155,18 @@ export function convertToLockfileObject (lockfile: LockfileFile): LockfileObject } } +// Backfill the `gitHosted` flag for tarball resolutions written by older +// pnpm versions. Doing it once at load time lets every downstream reader +// rely on the typed field instead of repeating URL prefix matches. +function enrichGitHostedFlag (resolution: TarballResolution | undefined): void { + if (resolution == null) return + if (resolution.type !== undefined) return + if (resolution.gitHosted != null) return + if (resolution.tarball != null && isGitHostedTarballUrl(resolution.tarball)) { + resolution.gitHosted = true + } +} + function migratePatchedDependencies (patchedDependencies: Record | undefined): Record | undefined { if (!patchedDependencies) return undefined const result: Record = {} diff --git a/lockfile/types/src/index.ts b/lockfile/types/src/index.ts index 6965cdfd7e..aea9523ced 100644 --- a/lockfile/types/src/index.ts +++ b/lockfile/types/src/index.ts @@ -91,6 +91,19 @@ export interface TarballResolution { tarball: string integrity?: string path?: string + /** + * True for tarballs sourced from a git host (codeload.github.com / + * gitlab.com / bitbucket.org). Such tarballs need preparation + * (preparePackage / packlist) on extraction, and their cached content + * depends on whether build scripts ran, so they're addressed by + * gitHostedStoreIndexKey rather than the integrity-based key. + * + * The git resolver sets this when it produces the resolution; the + * lockfile loader sets it on entries whose URL matches a known git host + * for backward compatibility with lockfiles written before this field + * existed. + */ + gitHosted?: boolean } /** diff --git a/lockfile/utils/package.json b/lockfile/utils/package.json index 488d5764dd..efcc91d092 100644 --- a/lockfile/utils/package.json +++ b/lockfile/utils/package.json @@ -35,7 +35,6 @@ "dependencies": { "@pnpm/deps.path": "workspace:*", "@pnpm/error": "workspace:*", - "@pnpm/fetching.pick-fetcher": "workspace:*", "@pnpm/hooks.types": "workspace:*", "@pnpm/lockfile.types": "workspace:*", "@pnpm/resolving.resolver-base": "workspace:*", diff --git a/lockfile/utils/src/pkgSnapshotToResolution.ts b/lockfile/utils/src/pkgSnapshotToResolution.ts index f7ce515447..021b1369bb 100644 --- a/lockfile/utils/src/pkgSnapshotToResolution.ts +++ b/lockfile/utils/src/pkgSnapshotToResolution.ts @@ -1,7 +1,6 @@ import url from 'node:url' import * as dp from '@pnpm/deps.path' -import { isGitHostedPkgUrl } from '@pnpm/fetching.pick-fetcher' import type { PackageSnapshot, TarballResolution } from '@pnpm/lockfile.types' import type { Resolution } from '@pnpm/resolving.resolver-base' import type { Registries } from '@pnpm/types' @@ -17,7 +16,7 @@ export function pkgSnapshotToResolution ( if ( Boolean((pkgSnapshot.resolution as TarballResolution).type) || (pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:') || - isGitHostedPkgUrl((pkgSnapshot.resolution as TarballResolution).tarball ?? '') + (pkgSnapshot.resolution as TarballResolution).gitHosted === true ) { return pkgSnapshot.resolution as Resolution } diff --git a/lockfile/utils/src/toLockfileResolution.ts b/lockfile/utils/src/toLockfileResolution.ts index cf0340751e..a06ac5eeed 100644 --- a/lockfile/utils/src/toLockfileResolution.ts +++ b/lockfile/utils/src/toLockfileResolution.ts @@ -1,6 +1,5 @@ -import { isGitHostedPkgUrl } from '@pnpm/fetching.pick-fetcher' import type { LockfileResolution } from '@pnpm/lockfile.types' -import type { Resolution } from '@pnpm/resolving.resolver-base' +import type { Resolution, TarballResolution } from '@pnpm/resolving.resolver-base' import getNpmTarballUrl from 'get-npm-tarball-url' export function toLockfileResolution ( @@ -16,21 +15,26 @@ export function toLockfileResolution ( return resolution as LockfileResolution } const tarball = resolution['tarball'] as string | undefined + // Honor the resolver-supplied flag, with a URL fallback for resolutions + // that didn't go through the git resolver (e.g. config-dep migrations or + // legacy lockfiles read by callers that don't enrich the field). + const gitHosted = (resolution as TarballResolution).gitHosted === true || + (tarball != null && isGitHostedTarballUrl(tarball)) if (lockfileIncludeTarballUrl) { - return { + return preservingGitHosted({ integrity: resolution['integrity'], tarball, - } + }, gitHosted) } // Tarball URLs that cannot be reconstructed from the package name, version, // and registry must always stay in the lockfile, otherwise the package can // no longer be re-fetched. This covers local `file:` tarballs and tarballs // served by git providers (GitHub, GitLab, Bitbucket). - if (tarball != null && (tarball.startsWith('file:') || isGitHostedPkgUrl(tarball))) { - return { + if (tarball != null && (tarball.startsWith('file:') || gitHosted)) { + return preservingGitHosted({ integrity: resolution['integrity'], tarball, - } + }, gitHosted) } if (lockfileIncludeTarballUrl === false) { return { @@ -43,16 +47,34 @@ export function toLockfileResolution ( const expectedTarball = getNpmTarballUrl(pkg.name, pkg.version, { registry }) const actualTarball = tarball!.replaceAll('%2f', '/') if (removeProtocol(expectedTarball) !== removeProtocol(actualTarball)) { - return { + return preservingGitHosted({ integrity: resolution['integrity'], tarball, - } + }, gitHosted) } return { integrity: resolution['integrity'], } } +function preservingGitHosted ( + resolution: T, + gitHosted: boolean +): T & { gitHosted?: boolean } { + return gitHosted ? { ...resolution, gitHosted: true } : resolution +} + +// Inlined to avoid pulling @pnpm/fetching.pick-fetcher into the lockfile-utils +// dep graph. Used as a fallback when callers haven't pre-set the +// `gitHosted` field on TarballResolution. +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') +} + function removeProtocol (url: string): string { return url.split('://')[1] } diff --git a/lockfile/utils/test/toLockfileResolution.ts b/lockfile/utils/test/toLockfileResolution.ts index 3f966a29f1..99ff998bbb 100644 --- a/lockfile/utils/test/toLockfileResolution.ts +++ b/lockfile/utils/test/toLockfileResolution.ts @@ -81,5 +81,19 @@ test('keeps git-hosted tarballs when lockfileIncludeTarballUrl is false', () => )).toEqual({ integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', + gitHosted: true, + }) +}) + +test('records gitHosted on the lockfile entry when set on the resolution', () => { + expect(toLockfileResolution( + { name: 'foo', version: '1.0.0' }, + { integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', gitHosted: true }, + REGISTRY, + true + )).toEqual({ + integrity: 'sha512-AAAA', + tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', + gitHosted: true, }) }) diff --git a/lockfile/utils/tsconfig.json b/lockfile/utils/tsconfig.json index b362516053..653388c953 100644 --- a/lockfile/utils/tsconfig.json +++ b/lockfile/utils/tsconfig.json @@ -18,9 +18,6 @@ { "path": "../../deps/path" }, - { - "path": "../../fetching/pick-fetcher" - }, { "path": "../../hooks/types" }, diff --git a/modules-mounter/daemon/src/createFuseHandlers.ts b/modules-mounter/daemon/src/createFuseHandlers.ts index d580bb0f9a..a9ce7f0bfd 100644 --- a/modules-mounter/daemon/src/createFuseHandlers.ts +++ b/modules-mounter/daemon/src/createFuseHandlers.ts @@ -6,7 +6,7 @@ import { nameVerFromPkgSnapshot, } from '@pnpm/lockfile.utils' import { getFilePathByModeInCafs, type PackageFilesIndex } from '@pnpm/store.cafs' -import { StoreIndex, storeIndexKey } from '@pnpm/store.index' +import { pickStoreIndexKey, StoreIndex } from '@pnpm/store.index' import type { DepPath } from '@pnpm/types' import Fuse from 'fuse-native' import schemas from 'hyperdrive-schemas' @@ -176,7 +176,9 @@ export function createFuseHandlersFromLockfile (lockfile: LockfileObject, storeD const pkgSnapshot = lockfile.packages?.[depPath as DepPath] if (pkgSnapshot == null) return undefined const nameVer = nameVerFromPkgSnapshot(depPath, pkgSnapshot) - const pkgIndexFilePath = storeIndexKey((pkgSnapshot.resolution as TarballResolution).integrity!, `${nameVer.name}@${nameVer.version}`) + const resolution = pkgSnapshot.resolution as TarballResolution + const pkgId = nameVer.nonSemverVersion ?? `${nameVer.name}@${nameVer.version}` + const pkgIndexFilePath = pickStoreIndexKey(resolution, pkgId, { built: true }) const pkgIndex = storeIndex.get(pkgIndexFilePath) as PackageFilesIndex pkgSnapshotCache.set(depPath, { ...nameVer, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 80dac1ed4e..874b591a7b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6954,9 +6954,6 @@ importers: '@pnpm/error': specifier: workspace:* version: link:../../core/error - '@pnpm/fetching.pick-fetcher': - specifier: workspace:* - version: link:../../fetching/pick-fetcher '@pnpm/hooks.types': specifier: workspace:* version: link:../../hooks/types @@ -9159,9 +9156,6 @@ importers: store/pkg-finder: dependencies: - '@pnpm/deps.path': - specifier: workspace:* - version: link:../../deps/path '@pnpm/fetching.directory-fetcher': specifier: workspace:* version: link:../../fetching/directory-fetcher diff --git a/resolving/git-resolver/src/index.ts b/resolving/git-resolver/src/index.ts index 820dd53a41..0182b96771 100644 --- a/resolving/git-resolver/src/index.ts +++ b/resolving/git-resolver/src/index.ts @@ -66,7 +66,7 @@ export function createGitResolver ( const tarball = hosted.tarball?.() if (tarball) { - resolution = { tarball } + resolution = { tarball, gitHosted: true } } } diff --git a/resolving/git-resolver/test/index.ts b/resolving/git-resolver/test/index.ts index 460c23bc93..cf8c8dc3ba 100644 --- a/resolving/git-resolver/test/index.ts +++ b/resolving/git-resolver/test/index.ts @@ -36,6 +36,7 @@ test('resolveFromGit() with commit', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#163360a8d3ae6bee9524541043197ff356f8ed99', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/163360a8d3ae6bee9524541043197ff356f8ed99', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -53,6 +54,7 @@ test('resolveFromGit() with no commit', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -66,6 +68,7 @@ test('resolveFromGit() with no commit, when main branch is not master', async () normalizedBareSpecifier: 'github:zoli-forks/cmd-shim', resolution: { tarball: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a1593edb6e395d3ce41f2ef70edf7e2cf5', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -78,6 +81,7 @@ test('resolveFromGit() with partial commit', async () => { normalizedBareSpecifier: 'github:zoli-forks/cmd-shim#a00a83a', resolution: { tarball: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a1593edb6e395d3ce41f2ef70edf7e2cf5', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -96,6 +100,7 @@ test('resolveFromGit() with branch', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#canary', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/4c39fbc124cd4944ee51cb082ad49320fab58121', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -108,6 +113,7 @@ test('resolveFromGit() with branch relative to refs', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#heads/canary', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/4c39fbc124cd4944ee51cb082ad49320fab58121', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -120,6 +126,7 @@ test('resolveFromGit() with tag', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#2.0.1', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/6dcce91c268805d456b8a575b67d7febc7ae2933', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -132,6 +139,7 @@ test.skip('resolveFromGit() with tag (v-prefixed tag)', async () => { normalizedBareSpecifier: 'github:andreineculau/npm-publish-git#v0.0.7', resolution: { tarball: 'https://codeload.github.com/andreineculau/npm-publish-git/tar.gz/a2f8d94562884e9529cb12c0818312ac87ab7f0b', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -144,6 +152,7 @@ test('resolveFromGit() with strict semver', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#semver:1.0.0', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/163360a8d3ae6bee9524541043197ff356f8ed99', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -156,6 +165,7 @@ test.skip('resolveFromGit() with strict semver (v-prefixed tag)', async () => { normalizedBareSpecifier: 'github:andreineculau/npm-publish-git#semver:v0.0.7', resolution: { tarball: 'https://codeload.github.com/andreineculau/npm-publish-git/tar.gz/a2f8d94562884e9529cb12c0818312ac87ab7f0b', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -168,6 +178,7 @@ test('resolveFromGit() with range semver', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#semver:^1.0.0', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/f7dec4d66a5a56719e49b9f94a24d73f924ddeb3', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -180,6 +191,7 @@ test.skip('resolveFromGit() with range semver (v-prefixed tag)', async () => { normalizedBareSpecifier: 'github:andreineculau/npm-publish-git#semver:<=v0.0.7', resolution: { tarball: 'https://codeload.github.com/andreineculau/npm-publish-git/tar.gz/a2f8d94562884e9529cb12c0818312ac87ab7f0b', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -203,6 +215,7 @@ test('resolveFromGit() with sub folder', async () => { resolution: { tarball: `https://codeload.github.com/RexSkz/test-git-subfolder-fetch/tar.gz/${headCommit}`, path: '/packages/simple-react-app', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -226,6 +239,7 @@ test('resolveFromGit() with both sub folder and branch', async () => { resolution: { tarball: `https://codeload.github.com/RexSkz/test-git-subfolder-fetch/tar.gz/${betaCommit}`, path: '/packages/simple-react-app', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -292,6 +306,7 @@ test.skip('resolveFromGit() bitbucket with commit', async () => { normalizedBareSpecifier: 'bitbucket:pnpmjs/git-resolver#988c61e11dc8d9ca0b5580cb15291951812549dc', resolution: { tarball: 'https://bitbucket.org/pnpmjs/git-resolver/get/988c61e11dc8d9ca0b5580cb15291951812549dc.tar.gz', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -335,6 +350,7 @@ test.skip('resolveFromGit() bitbucket with tag', async () => { normalizedBareSpecifier: 'bitbucket:pnpmjs/git-resolver#0.3.4', resolution: { tarball: 'https://bitbucket.org/pnpmjs/git-resolver/get/87cf6a67064d2ce56e8cd20624769a5512b83ff9.tar.gz', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -362,6 +378,7 @@ test.skip('resolveFromGit() gitlab with commit', async () => { normalizedBareSpecifier: 'gitlab:pnpm/git-resolver#988c61e11dc8d9ca0b5580cb15291951812549dc', resolution: { tarball: 'https://gitlab.com/api/v4/projects/pnpm%2Fgit-resolver/repository/archive.tar.gz?ref=988c61e11dc8d9ca0b5580cb15291951812549dc', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -405,6 +422,7 @@ test.skip('resolveFromGit() gitlab with tag', async () => { normalizedBareSpecifier: 'gitlab:pnpm/git-resolver#0.3.4', resolution: { tarball: 'https://gitlab.com/api/v4/projects/pnpm%2Fgit-resolver/repository/archive.tar.gz?ref=87cf6a67064d2ce56e8cd20624769a5512b83ff9', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -417,6 +435,7 @@ test('resolveFromGit() normalizes full url', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#2.0.1', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/6dcce91c268805d456b8a575b67d7febc7ae2933', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -429,6 +448,7 @@ test('resolveFromGit() normalizes full url with port', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#2.0.1', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/6dcce91c268805d456b8a575b67d7febc7ae2933', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -441,6 +461,7 @@ test('resolveFromGit() normalizes full url (alternative form)', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#2.0.1', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/6dcce91c268805d456b8a575b67d7febc7ae2933', + gitHosted: true, }, resolvedVia: 'git-repository', }) @@ -453,6 +474,7 @@ test('resolveFromGit() normalizes full url (alternative form 2)', async () => { normalizedBareSpecifier: 'github:zkochan/is-negative#2.0.1', resolution: { tarball: 'https://codeload.github.com/zkochan/is-negative/tar.gz/6dcce91c268805d456b8a575b67d7febc7ae2933', + gitHosted: true, }, resolvedVia: 'git-repository', }) diff --git a/resolving/resolver-base/src/index.ts b/resolving/resolver-base/src/index.ts index 392de2544f..9a58a30d11 100644 --- a/resolving/resolver-base/src/index.ts +++ b/resolving/resolver-base/src/index.ts @@ -18,6 +18,14 @@ export interface TarballResolution { tarball: string integrity?: string path?: string + /** + * True for tarballs sourced from a git host (codeload.github.com / + * gitlab.com / bitbucket.org). Such tarballs need preparation + * (preparePackage / packlist) on extraction, and their cached content + * depends on whether build scripts ran, so they're addressed by + * gitHostedStoreIndexKey rather than the integrity-based key. + */ + gitHosted?: boolean } export interface BinaryResolution { diff --git a/store/commands/src/store/storeStatus/index.ts b/store/commands/src/store/storeStatus/index.ts index 378b547bbf..941d7ea20a 100644 --- a/store/commands/src/store/storeStatus/index.ts +++ b/store/commands/src/store/storeStatus/index.ts @@ -11,7 +11,7 @@ import { import { streamParser } from '@pnpm/logger' import type { PackageFilesIndex } from '@pnpm/store.cafs' import type { TarballResolution } from '@pnpm/store.controller-types' -import { gitHostedStoreIndexKey, storeIndexKey } from '@pnpm/store.index' +import { pickStoreIndexKey } from '@pnpm/store.index' import { StoreIndex } from '@pnpm/store.index' import type { DepPath } from '@pnpm/types' import dint from 'dint' @@ -43,10 +43,11 @@ export async function storeStatus (maybeOpts: StoreStatusOptions): Promise !skipped.has(depPath)) .map(([depPath, pkgSnapshot]) => { const id = packageIdFromSnapshot(depPath, pkgSnapshot) + const resolution = pkgSnapshot.resolution as TarballResolution return { depPath, id, - integrity: (pkgSnapshot.resolution as TarballResolution).integrity, + resolution, pkgPath: depPath, ...nameVerFromPkgSnapshot(depPath, pkgSnapshot), } @@ -54,10 +55,8 @@ export async function storeStatus (maybeOpts: StoreStatusOptions): Promise { - const pkgIndexFilePath = integrity - ? storeIndexKey(integrity, id) - : gitHostedStoreIndexKey(id, { built: true }) + const modified = await pFilter(pkgs, async ({ id, resolution, depPath, name }) => { + const pkgIndexFilePath = pickStoreIndexKey(resolution, id, { built: true }) const pkgFilesIndex = storeIndex.get(pkgIndexFilePath) as PackageFilesIndex | undefined if (!pkgFilesIndex) { return false diff --git a/store/index/src/index.ts b/store/index/src/index.ts index 521d16c5f6..8823073fa0 100644 --- a/store/index/src/index.ts +++ b/store/index/src/index.ts @@ -66,6 +66,33 @@ export function gitHostedStoreIndexKey (pkgId: string, opts: { built: boolean }) return storeIndexKey(pkgId, opts.built ? 'built' : 'not-built') } +/** + * Pick the store index key for a tarball-shaped resolution. + * + * Git-hosted tarballs (`resolution.gitHosted === true`) are addressed by + * `gitHostedStoreIndexKey(pkgId, { built })` — their cached content depends + * on whether build scripts ran during fetch (`preparePackage`), so the + * `built` dimension is part of the key. The integrity-only key would + * collapse the built/not-built variants into one slot. + * + * Tarballs with integrity that aren't git-hosted are addressed by + * `storeIndexKey(integrity, pkgId)`. + * + * Resolutions that have neither flag fall through to + * `gitHostedStoreIndexKey` — these are typically lockfile entries written + * by older pnpm versions that lacked integrity. + */ +export function pickStoreIndexKey ( + resolution: { gitHosted?: boolean, integrity?: string }, + pkgId: string, + opts: { built: boolean } +): string { + if (resolution.gitHosted || !resolution.integrity) { + return gitHostedStoreIndexKey(pkgId, opts) + } + return storeIndexKey(resolution.integrity, pkgId) +} + const openInstances = new Set() /** diff --git a/store/pkg-finder/package.json b/store/pkg-finder/package.json index 1963faf149..25f3efdd28 100644 --- a/store/pkg-finder/package.json +++ b/store/pkg-finder/package.json @@ -31,7 +31,6 @@ ".test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules --disable-warning=ExperimentalWarning --disable-warning=DEP0169\" jest" }, "dependencies": { - "@pnpm/deps.path": "workspace:*", "@pnpm/fetching.directory-fetcher": "workspace:*", "@pnpm/resolving.resolver-base": "workspace:*", "@pnpm/store.cafs": "workspace:*", diff --git a/store/pkg-finder/src/index.ts b/store/pkg-finder/src/index.ts index 7912a18eb0..810c66a443 100644 --- a/store/pkg-finder/src/index.ts +++ b/store/pkg-finder/src/index.ts @@ -1,10 +1,9 @@ import path from 'node:path' -import { parse } from '@pnpm/deps.path' import { fetchFromDir } from '@pnpm/fetching.directory-fetcher' -import type { Resolution } from '@pnpm/resolving.resolver-base' +import type { Resolution, TarballResolution } from '@pnpm/resolving.resolver-base' import { getFilePathByModeInCafs, type PackageFilesIndex } from '@pnpm/store.cafs' -import { gitHostedStoreIndexKey, type StoreIndex, storeIndexKey } from '@pnpm/store.index' +import { pickStoreIndexKey, type StoreIndex } from '@pnpm/store.index' export interface ReadPackageFileMapOptions { storeDir: string @@ -17,10 +16,18 @@ export interface ReadPackageFileMapOptions { * Reads the file index for a package and returns a `Map` * mapping filenames to absolute paths on disk. * - * Handles three types of package resolutions: - * - Directory packages: fetches the file list from the local directory - * - Packages with integrity: looks up the index file in the CAFS by integrity hash - * - Tarball packages: looks up the index file by package directory name + * Picks the store key by resolution shape: + * - Directory packages: fetches the file list from the local directory. + * - Git-hosted tarballs (codeload.github.com / gitlab.com / bitbucket.org): + * keyed by `gitHostedStoreIndexKey(packageId, { built: true })`. The + * lockfile pins their integrity for security, but the cached payload + * depends on whether build scripts ran during fetch (preparePackage), so + * the `built` dimension is part of the key. Folding them under the + * integrity-only key would collapse that distinction. + * - npm-registry tarballs with integrity: keyed by + * `storeIndexKey(integrity, packageId)`. + * - Other tarball / git resolutions without integrity: keyed by + * `gitHostedStoreIndexKey(packageId, { built: true })`. * * For CAFS packages, the content-addressed digests are resolved to file * paths upfront, so callers get a uniform map regardless of resolution type. @@ -44,19 +51,16 @@ export async function readPackageFileMap ( return localInfo.filesMap } - const isPackageWithIntegrity = 'integrity' in packageResolution - let pkgIndexFilePath: string - if (isPackageWithIntegrity) { - const parsedId = parse(packageId) - pkgIndexFilePath = storeIndexKey( - packageResolution.integrity as string, - parsedId.nonSemverVersion ?? `${parsedId.name}@${parsedId.version}` + if ( + (!packageResolution.type && 'tarball' in packageResolution && packageResolution.tarball) || + packageResolution.type === 'git' + ) { + pkgIndexFilePath = pickStoreIndexKey( + packageResolution as TarballResolution, + packageId, + { built: true } ) - } else if (!packageResolution.type && 'tarball' in packageResolution && packageResolution.tarball) { - pkgIndexFilePath = gitHostedStoreIndexKey(packageId, { built: true }) - } else if (packageResolution.type === 'git') { - pkgIndexFilePath = gitHostedStoreIndexKey(packageId, { built: true }) } else { return undefined } diff --git a/store/pkg-finder/test/readPackageFileMap.test.ts b/store/pkg-finder/test/readPackageFileMap.test.ts index a6728bb146..924af55614 100644 --- a/store/pkg-finder/test/readPackageFileMap.test.ts +++ b/store/pkg-finder/test/readPackageFileMap.test.ts @@ -64,6 +64,29 @@ describe('readPackageFileMap', () => { expect(result!.has('index.js')).toBe(true) }) + it('should resolve git-hosted tarball packages with integrity by gitHostedStoreIndexKey', async () => { + // Git-hosted tarballs are keyed by gitHostedStoreIndexKey to preserve the + // built/not-built dimension that the integrity-only key would collapse. + // The lockfile still pins integrity for security; the routing is driven + // by the `gitHosted` field set by the resolver / lockfile loader. + const pkgId = 'https://codeload.github.com/stevemao/left-pad/tar.gz/cafe1234' + const key = gitHostedStoreIndexKey(pkgId, { built: true }) + + storeIndex.set(key, createFilesIndex()) + + const resolution: TarballResolution = { + integrity: 'sha512-abc123gitHosted', + tarball: pkgId, + gitHosted: true, + } + + const result = await readPackageFileMap(resolution, pkgId, defaultOpts()) + + expect(result).toBeDefined() + expect(result!.has('package.json')).toBe(true) + expect(result!.has('index.js')).toBe(true) + }) + it('should resolve git-hosted tarball packages (no type, has tarball)', async () => { const pkgId = 'left-pad@https://codeload.github.com/stevemao/left-pad/tar.gz/abc123' const key = gitHostedStoreIndexKey(pkgId, { built: true }) diff --git a/store/pkg-finder/tsconfig.json b/store/pkg-finder/tsconfig.json index 456209ce1f..93be8933ba 100644 --- a/store/pkg-finder/tsconfig.json +++ b/store/pkg-finder/tsconfig.json @@ -9,9 +9,6 @@ "../../__typings__/**/*.d.ts" ], "references": [ - { - "path": "../../deps/path" - }, { "path": "../../fetching/directory-fetcher" },