Merge pull request from GHSA-5r98-f33j-g8h7

* test: extracting files with similar names

* test: manifest extraction

* fix: pick last file from tarball

* fix: pick last file from tarball

* fix: always resolve from the last occurence of the manifest

.changeset/silent-lobsters-taste.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/cafs": patch
+"pnpm": patch
+---
+
+When the same file is appended multiple times into a tarball, pick the last occurence, when unpacking the tarball.

fetching/tarball-fetcher/package.json

@@ -46,15 +46,18 @@
     "ssri": "10.0.4"
   },
   "devDependencies": {
+    "@pnpm/cafs": "workspace:*",
     "@pnpm/cafs-types": "workspace:*",
     "@pnpm/create-cafs-store": "workspace:*",
     "@pnpm/fetch": "workspace:*",
     "@pnpm/tarball-fetcher": "workspace:*",
     "@pnpm/test-fixtures": "workspace:*",
+    "@pnpm/types": "workspace:*",
     "@types/ramda": "0.28.20",
     "@types/retry": "^0.12.2",
     "@types/ssri": "^7.1.1",
     "nock": "13.3.1",
+    "safe-promise-defer": "^1.0.1",
     "tempy": "^1.0.1"
   },
   "funding": "https://opencollective.com/pnpm",

fetching/tarball-fetcher/test/fetch.ts

@@ -4,6 +4,7 @@ import path from 'path'
 import { FetchError, PnpmError } from '@pnpm/error'
 import { createFetchFromRegistry } from '@pnpm/fetch'
 import { createCafsStore } from '@pnpm/create-cafs-store'
+import { getFilePathInCafs } from '@pnpm/cafs'
 import { globalWarn } from '@pnpm/logger'
 import { fixtures } from '@pnpm/test-fixtures'
 import {
@@ -11,7 +12,9 @@ import {
   BadTarballError,
   TarballIntegrityError,
 } from '@pnpm/tarball-fetcher'
+import { type DependencyManifest } from '@pnpm/types'
 import nock from 'nock'
+import safePromiseDefer from 'safe-promise-defer'
 import ssri from 'ssri'
 import tempy from 'tempy'
 
@@ -429,3 +432,21 @@ test('do not build the package when scripts are ignored', async () => {
   expect(filesIndex).not.toHaveProperty(['prepare.txt'])
   expect(globalWarn).toHaveBeenCalledWith(`The git-hosted package fetched from "${tarball}" has to be built but the build scripts were ignored.`)
 })
+
+test('when extracting files with the same name, pick the last ones', async () => {
+  const tar = f.find('tarball-with-duplicate-files/archive.tar')
+  const resolution = {
+    tarball: `file:${tar}`,
+  }
+
+  const manifest = safePromiseDefer<DependencyManifest | undefined>()
+  const { filesIndex } = await fetch.localTarball(cafs, resolution, {
+    lockfileDir: process.cwd(),
+    manifest,
+  })
+  const { integrity } = await (filesIndex['package.json'] as any).writeResult // eslint-disable-line
+  const filePath = getFilePathInCafs(path.join(cafsDir, 'files'), integrity, 'nonexec')
+  const pkgJson = JSON.parse(fs.readFileSync(filePath, 'utf8'))
+  expect(pkgJson.name).toBe('pkg2')
+  expect((await manifest())?.name).toBe('pkg2')
+})

fetching/tarball-fetcher/test/fixtures/tarball-with-duplicate-files/create.sh

@@ -0,0 +1,12 @@
+rm -rf package
+mkdir package
+
+echo "{\"name\":\"pkg1\"}" > package/package.json
+tar -cvf archive.tar package/package.json
+
+# Append another package.json with the same path but different content
+rm package/package.json
+echo "{\"name\":\"pkg2\"}" > package/package.json
+tar -rvf archive.tar package/package.json
+
+rm -rf package

fetching/tarball-fetcher/tsconfig.json

@@ -30,6 +30,12 @@
     {
       "path": "../../packages/error"
     },
+    {
+      "path": "../../packages/types"
+    },
+    {
+      "path": "../../store/cafs"
+    },
     {
       "path": "../../store/cafs-types"
     },

pnpm-lock.yaml

@@ -1586,6 +1586,9 @@ importers:
         specifier: 10.0.4
         version: 10.0.4
     devDependencies:
+      '@pnpm/cafs':
+        specifier: workspace:*
+        version: link:../../store/cafs
       '@pnpm/cafs-types':
         specifier: workspace:*
         version: link:../../store/cafs-types
@@ -1601,6 +1604,9 @@ importers:
       '@pnpm/test-fixtures':
         specifier: workspace:*
         version: link:../../__utils__/test-fixtures
+      '@pnpm/types':
+        specifier: workspace:*
+        version: link:../../packages/types
       '@types/ramda':
         specifier: 0.28.20
         version: 0.28.20
@@ -1613,6 +1619,9 @@ importers:
       nock:
         specifier: 13.3.1
         version: 13.3.1
+      safe-promise-defer:
+        specifier: ^1.0.1
+        version: 1.0.1
       tempy:
         specifier: ^1.0.1
         version: 1.0.1
@@ -5491,6 +5500,9 @@ importers:
       rename-overwrite:
         specifier: ^4.0.3
         version: 4.0.3
+      safe-promise-defer:
+        specifier: ^1.0.1
+        version: 1.0.1
       ssri:
         specifier: 10.0.4
         version: 10.0.4

store/cafs/package.json

@@ -26,6 +26,7 @@
     "p-limit": "^3.1.0",
     "path-temp": "^2.0.0",
     "rename-overwrite": "^4.0.3",
+    "safe-promise-defer": "^1.0.1",
     "ssri": "10.0.4",
     "strip-bom": "^4.0.0",
     "tar-stream": "^2.2.0"

store/cafs/src/addFilesFromTarball.ts

@@ -1,6 +1,8 @@
 import { type PassThrough } from 'stream'
 import type { DeferredManifestPromise, FilesIndex, FileWriteResult } from '@pnpm/cafs-types'
+import { type DependencyManifest } from '@pnpm/types'
 import gunzip from 'gunzip-maybe'
+import safePromiseDefer, { type SafePromiseDefer } from 'safe-promise-defer'
 import tar from 'tar-stream'
 import { parseJsonStream } from './parseJson'
 
@@ -13,21 +15,26 @@ export async function addFilesFromTarball (
   const ignore = _ignore ?? (() => false)
   const extract = tar.extract({ allowUnknownFormat: true })
   const filesIndex: FilesIndex = {}
+  let unpipeManifestStream: () => void
+  let lastManifest: SafePromiseDefer<DependencyManifest | undefined> | undefined
   await new Promise<void>((resolve, reject) => {
     extract.on('entry', (header, fileStream, next) => {
       // There are some edge cases, where the same files are extracted multiple times.
       // So there will be an entry for "lib/index.js" and another one for "lib//index.js",
       // which are the same file.
-      // Hence, we are normalizing the file name, replacing // with / and checking for duplicates.
+      // Hence, we are normalizing the file name, replacing // with /.
+      // When there are duplicate files, the last instances are picked.
       // Example of such package: @pnpm/colorize-semver-diff@1.0.1
       const filename = header.name.slice(header.name.indexOf('/') + 1).replace(/\/\//g, '/')
-      if (header.type !== 'file' || ignore(filename) || filesIndex[filename]) {
+      if (header.type !== 'file' || ignore(filename)) {
         fileStream.resume()
         next()
         return
       }
       if (filename === 'package.json' && (manifest != null)) {
-        parseJsonStream(fileStream, manifest)
+        unpipeManifestStream?.()
+        lastManifest = safePromiseDefer<DependencyManifest | undefined>()
+        unpipeManifestStream = parseJsonStream(fileStream, lastManifest)
       }
       const writeResult = addStreamToCafs(fileStream, header.mode!)
       filesIndex[filename] = {
@@ -51,6 +58,8 @@ export async function addFilesFromTarball (
   })
   if (!filesIndex['package.json'] && manifest != null) {
     manifest.resolve(undefined)
+  } else if (lastManifest && manifest) {
+    lastManifest().then(manifest.resolve).catch(manifest.reject)
   }
   return filesIndex
 }

store/cafs/src/parseJson.ts

@@ -18,9 +18,11 @@ export function parseJsonStream (
   stream: PassThrough,
   deferred: DeferredManifestPromise
 ) {
-  stream.pipe(
-    concatStream((buffer) => {
-      parseJsonBuffer(buffer, deferred)
-    })
-  )
+  const _concatStream = concatStream((buffer) => {
+    parseJsonBuffer(buffer, deferred)
+  })
+  stream.pipe(_concatStream)
+  return () => {
+    stream.unpipe(_concatStream)
+  }
 }