From b7d3ec65b1024b7814179e862cb620779e78ff7a Mon Sep 17 00:00:00 2001 From: Oren Date: Tue, 9 Dec 2025 00:38:27 +0200 Subject: [PATCH] fix(tarball-resolver): add integrity hash to HTTP tarball dependencies (#10287) * fix(tarball-resolver): add integrity hash to HTTP tarball dependencies * Refactor to download tarball just once * Fix tests * fix: only calc hash when it is not passed in to the fetcher * docs: update changesets --- .changeset/tricky-ravens-obey.md | 9 ++++++ fetching/fetcher-base/src/index.ts | 1 + pkg-manager/core/test/lockfile.ts | 2 ++ .../package-requester/src/packageRequester.ts | 13 ++++++-- pkg-manager/package-requester/test/index.ts | 31 +++++++++++++++++++ store/store-controller-types/src/index.ts | 1 + worker/src/index.ts | 3 +- worker/src/start.ts | 15 ++++++++- 8 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 .changeset/tricky-ravens-obey.md diff --git a/.changeset/tricky-ravens-obey.md b/.changeset/tricky-ravens-obey.md new file mode 100644 index 0000000000..774f8492cf --- /dev/null +++ b/.changeset/tricky-ravens-obey.md @@ -0,0 +1,9 @@ +--- +"@pnpm/package-requester": minor +"@pnpm/store-controller-types": minor +"@pnpm/fetcher-base": minor +"@pnpm/worker": minor +"pnpm": minor +--- + +Compute integrity hash for HTTP tarball dependencies when fetching, storing it in the lockfile to prevent servers from serving altered content on subsequent installs [#10287](https://github.com/pnpm/pnpm/pull/10287). diff --git a/fetching/fetcher-base/src/index.ts b/fetching/fetcher-base/src/index.ts index 2dc33a22a8..f04cc65efa 100644 --- a/fetching/fetcher-base/src/index.ts +++ b/fetching/fetcher-base/src/index.ts @@ -32,6 +32,7 @@ export interface FetchResult { manifest?: DependencyManifest filesIndex: Record requiresBuild: boolean + integrity?: string } export interface GitFetcherOptions { diff --git a/pkg-manager/core/test/lockfile.ts b/pkg-manager/core/test/lockfile.ts index 6b189a478b..9d9f3040a3 100644 --- a/pkg-manager/core/test/lockfile.ts +++ b/pkg-manager/core/test/lockfile.ts @@ -787,6 +787,7 @@ test('packages installed via tarball URL from the default registry are normalize 'is-positive@https://registry.npmjs.org/is-positive/-/is-positive-1.0.0.tgz': { engines: { node: '>=0.10.0' }, resolution: { + integrity: 'sha512-xxzPGZ4P2uN6rROUa5N9Z7zTX6ERuE0hs6GUOc/cKBLF2NqKc16UwqHMt3tFg4CO6EBTE5UecUasg+3jZx3Ckg==', tarball: 'https://registry.npmjs.org/is-positive/-/is-positive-1.0.0.tgz', }, version: '1.0.0', @@ -1168,6 +1169,7 @@ test('tarball installed through non-standard URL endpoint from the registry doma 'is-positive@https://registry.npmjs.org/is-positive/download/is-positive-3.1.0.tgz': { engines: { node: '>=0.10.0' }, resolution: { + integrity: 'sha512-8ND1j3y9/HP94TOvGzr69/FgbkX2ruOldhLEsTWwcJVfo4oRjwemJmJxt7RJkKYH8tz7vYBP9JcKQY8CLuJ90Q==', tarball: 'https://registry.npmjs.org/is-positive/download/is-positive-3.1.0.tgz', }, version: '3.1.0', diff --git a/pkg-manager/package-requester/src/packageRequester.ts b/pkg-manager/package-requester/src/packageRequester.ts index bc0544300e..a1a562f567 100644 --- a/pkg-manager/package-requester/src/packageRequester.ts +++ b/pkg-manager/package-requester/src/packageRequester.ts @@ -314,7 +314,12 @@ async function resolveAndFetch ( }) if (!manifest) { - manifest = (await fetchResult.fetching()).bundledManifest + const fetchedResult = await fetchResult.fetching() + manifest = fetchedResult.bundledManifest + // Add integrity to resolution if it was computed during fetching (only for TarballResolution) + if (fetchedResult.integrity && !resolution.type && !(resolution as TarballResolution).integrity) { + (resolution as TarballResolution).integrity = fetchedResult.integrity + } } return { body: { @@ -641,9 +646,10 @@ Actual package in the store with the given integrity: ${pkgFilesIndex.name}@${pk } ), { priority }) - if (isLocalTarballDep && (opts.pkg.resolution as TarballResolution).integrity) { + const integrity = (opts.pkg.resolution as TarballResolution).integrity ?? fetchedPackage.integrity + if (isLocalTarballDep && integrity) { await fs.mkdir(target, { recursive: true }) - await gfs.writeFile(path.join(target, TARBALL_INTEGRITY_FILENAME), (opts.pkg.resolution as TarballResolution).integrity!, 'utf8') + await gfs.writeFile(path.join(target, TARBALL_INTEGRITY_FILENAME), integrity, 'utf8') } fetching.resolve({ @@ -654,6 +660,7 @@ Actual package in the store with the given integrity: ${pkgFilesIndex.name}@${pk requiresBuild: fetchedPackage.requiresBuild, }, bundledManifest: fetchedPackage.manifest == null ? fetchedPackage.manifest : normalizeBundledManifest(fetchedPackage.manifest), + integrity, }) } catch (err: any) { // eslint-disable-line fetching.reject(err) diff --git a/pkg-manager/package-requester/test/index.ts b/pkg-manager/package-requester/test/index.ts index f36d241b30..2f366a844f 100644 --- a/pkg-manager/package-requester/test/index.ts +++ b/pkg-manager/package-requester/test/index.ts @@ -1069,3 +1069,34 @@ test('should skip store integrity check and resolve manifest if fetchRawManifest }) } }) + +test('HTTP tarball without integrity gets integrity computed during fetch', async () => { + const storeDir = temporaryDirectory() + const cafs = createCafsStore(storeDir) + const requestPackage = createPackageRequester({ + resolve, + fetchers, + cafs, + networkConcurrency: 1, + storeDir, + verifyStoreIntegrity: true, + virtualStoreDirMaxLength: 120, + }) + + const projectDir = temporaryDirectory() + // Request a package via HTTP tarball URL (simulated via the local registry) + const pkgResponse = await requestPackage( + { alias: 'is-positive', bareSpecifier: `http://localhost:${REGISTRY_MOCK_PORT}/is-positive/-/is-positive-1.0.0.tgz` }, + { + downloadPriority: 0, + lockfileDir: projectDir, + preferredVersions: {}, + projectDir, + } + ) + + expect(pkgResponse.body).toBeTruthy() + // The resolution should now include an integrity hash computed during fetch + expect(pkgResponse.body.resolution).toHaveProperty('integrity') + expect((pkgResponse.body.resolution as { integrity?: string }).integrity).toMatch(/^sha512-/) +}) diff --git a/store/store-controller-types/src/index.ts b/store/store-controller-types/src/index.ts index 9acbe73e48..f191ac4c9c 100644 --- a/store/store-controller-types/src/index.ts +++ b/store/store-controller-types/src/index.ts @@ -64,6 +64,7 @@ export interface StoreController { export interface PkgRequestFetchResult { bundledManifest?: BundledManifest files: PackageFilesResponse + integrity?: string } export interface FetchResponse { diff --git a/worker/src/index.ts b/worker/src/index.ts index 76b69be0f2..e758155be6 100644 --- a/worker/src/index.ts +++ b/worker/src/index.ts @@ -70,6 +70,7 @@ interface AddFilesResult { filesIndex: Record manifest: DependencyManifest requiresBuild: boolean + integrity?: string } type AddFilesFromDirOptions = Pick @@ -144,7 +145,7 @@ export async function addFilesFromTarball (opts: AddFilesFromTarballOptions): Pr workerPool = createTarballWorkerPool() } const localWorker = await workerPool.checkoutWorkerAsync(true) - return new Promise<{ filesIndex: Record, manifest: DependencyManifest, requiresBuild: boolean }>((resolve, reject) => { + return new Promise((resolve, reject) => { localWorker.once('message', ({ status, error, value }) => { workerPool!.checkinWorker(localWorker) if (status === 'error') { diff --git a/worker/src/start.ts b/worker/src/start.ts index 9f724cb4bb..21cd7b9e55 100644 --- a/worker/src/start.ts +++ b/worker/src/start.ts @@ -165,7 +165,20 @@ function addTarballToStore ({ buffer, storeDir, integrity, filesIndexFile }: Tar const { filesIndex, manifest } = cafs.addFilesFromTarball(buffer, true) const { filesIntegrity, filesMap } = processFilesIndex(filesIndex) const requiresBuild = writeFilesIndexFile(filesIndexFile, { manifest: manifest ?? {}, files: filesIntegrity }) - return { status: 'success', value: { filesIndex: filesMap, manifest, requiresBuild } } + return { + status: 'success', + value: { + filesIndex: filesMap, + manifest, + requiresBuild, + integrity: integrity ?? calcIntegrity(buffer), + }, + } +} + +function calcIntegrity (buffer: Buffer): string { + const calculatedHash: string = crypto.hash('sha512', buffer, 'hex') + return `sha512-${Buffer.from(calculatedHash, 'hex').toString('base64')}` } interface AddFilesFromDirResult {