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

@@ -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).

fetching/fetcher-base/src/index.ts

@@ -32,6 +32,7 @@ export interface FetchResult {
   manifest?: DependencyManifest
   filesIndex: Record<string, string>
   requiresBuild: boolean
+  integrity?: string
 }
 
 export interface GitFetcherOptions {

pkg-manager/core/test/lockfile.ts

@@ -788,6 +788,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',
@@ -1169,6 +1170,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',

pkg-manager/package-requester/src/packageRequester.ts

@@ -316,7 +316,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: {
@@ -643,9 +648,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({
@@ -656,6 +662,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)

pkg-manager/package-requester/test/index.ts

@@ -1070,3 +1070,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-/)
+})

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 {

worker/src/index.ts

@@ -70,6 +70,7 @@ interface AddFilesResult {
   filesIndex: Record<string, string>
   manifest: DependencyManifest
   requiresBuild: boolean
+  integrity?: string
 }
 
 type AddFilesFromDirOptions = Pick<AddDirToStoreMessage, 'storeDir' | 'dir' | 'filesIndexFile' | 'sideEffectsCacheKey' | 'readManifest' | 'pkg' | 'files'>
@@ -144,7 +145,7 @@ export async function addFilesFromTarball (opts: AddFilesFromTarballOptions): Pr
     workerPool = createTarballWorkerPool()
   }
   const localWorker = await workerPool.checkoutWorkerAsync(true)
-  return new Promise<{ filesIndex: Record<string, string>, manifest: DependencyManifest, requiresBuild: boolean }>((resolve, reject) => {
+  return new Promise<AddFilesResult>((resolve, reject) => {
     localWorker.once('message', ({ status, error, value }) => {
       workerPool!.checkinWorker(localWorker)
       if (status === 'error') {

worker/src/start.ts

@@ -166,7 +166,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 {