fix: use semver.eq() to make version comparison when checking lockfile integrity (#4036)

.changeset/stale-parents-type.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/package-requester": patch
+"pnpm": patch
+---
+
+When checking the correctness of the package data in the lockfile, don't use exact version comparison. `v1.0.0` should be considered to be the same as `1.0.0`. This fixes some edge cases when a package is published with a non-normalized version specifier in its `package.json`.

packages/package-requester/package.json

@@ -57,6 +57,7 @@
     "promise-share": "^1.0.0",
     "ramda": "^0.27.1",
     "rename-overwrite": "^4.0.0",
+    "semver": "^7.3.4",
     "ssri": "^8.0.1"
   },
   "devDependencies": {

packages/package-requester/src/packageRequester.ts

@@ -45,6 +45,7 @@ import pathTemp from 'path-temp'
 import pShare from 'promise-share'
 import pick from 'ramda/src/pick'
 import renameOverwrite from 'rename-overwrite'
+import semver from 'semver'
 import ssri from 'ssri'
 import safeDeferredPromise from './safeDeferredPromise'
 
@@ -426,8 +427,20 @@ function fetchToStore (
             ? safeDeferredPromise<DependencyManifest>()
             : undefined
           if (
-            pkgFilesIndex.name != null && opts.pkg.name != null && pkgFilesIndex.name !== opts.pkg.name ||
-            pkgFilesIndex.version != null && opts.pkg.version != null && pkgFilesIndex.version !== opts.pkg.version
+            (
+              pkgFilesIndex.name != null &&
+              opts.pkg.name != null &&
+              pkgFilesIndex.name !== opts.pkg.name
+            ) ||
+            (
+              pkgFilesIndex.version != null &&
+              opts.pkg.version != null &&
+              // We used to not normalize the package versions before writing them to the lockfile and store.
+              // So it may happen that the version will be in different formats.
+              // For instance, v1.0.0 and 1.0.0
+              // Hence, we need to use semver.eq() to compare them.
+              !semver.eq(pkgFilesIndex.version, opts.pkg.version, { loose: true })
+            )
           ) {
             /* eslint-disable @typescript-eslint/restrict-template-expressions */
             throw new PnpmError('UNEXPECTED_PKG_CONTENT_IN_STORE', `\

packages/package-requester/test/index.ts

@@ -849,3 +849,99 @@ test('fetch a git package without a package.json', async () => {
     expect(pkgResponse.body.id).toBe(`github.com/${repo}/${commit}`)
   }
 })
+
+test('throw exception if the package data in the store differs from the expected data', async () => {
+  const storeDir = tempy.directory()
+  const cafs = createCafsStore(storeDir)
+  let pkgResponse!: PackageResponse
+
+  {
+    const requestPackage = createPackageRequester({
+      resolve,
+      fetchers,
+      cafs,
+      networkConcurrency: 1,
+      storeDir,
+      verifyStoreIntegrity: true,
+    })
+
+    const projectDir = tempy.directory()
+    pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
+      downloadPriority: 0,
+      lockfileDir: projectDir,
+      preferredVersions: {},
+      projectDir,
+      registry,
+    })
+    await pkgResponse.finishing!()
+  }
+
+  // Fail when the name of the package is different in the store
+  {
+    const requestPackage = createPackageRequester({
+      resolve,
+      fetchers,
+      cafs,
+      networkConcurrency: 1,
+      storeDir,
+      verifyStoreIntegrity: true,
+    })
+    const { files } = requestPackage.fetchPackageToStore({
+      force: false,
+      lockfileDir: tempy.directory(),
+      pkg: {
+        name: 'is-negative',
+        version: '1.0.0',
+        id: pkgResponse.body.id,
+        resolution: pkgResponse.body.resolution,
+      },
+    })
+    await expect(files()).rejects.toThrow(/Package name mismatch found while reading/)
+  }
+
+  // Fail when the version of the package is different in the store
+  {
+    const requestPackage = createPackageRequester({
+      resolve,
+      fetchers,
+      cafs,
+      networkConcurrency: 1,
+      storeDir,
+      verifyStoreIntegrity: true,
+    })
+    const { files } = requestPackage.fetchPackageToStore({
+      force: false,
+      lockfileDir: tempy.directory(),
+      pkg: {
+        name: 'is-negative',
+        version: '2.0.0',
+        id: pkgResponse.body.id,
+        resolution: pkgResponse.body.resolution,
+      },
+    })
+    await expect(files()).rejects.toThrow(/Package name mismatch found while reading/)
+  }
+
+  // Do not fail when the versions are the same but written in a differnt format (1.0.0 is the same as v1.0.0)
+  {
+    const requestPackage = createPackageRequester({
+      resolve,
+      fetchers,
+      cafs,
+      networkConcurrency: 1,
+      storeDir,
+      verifyStoreIntegrity: true,
+    })
+    const { files } = requestPackage.fetchPackageToStore({
+      force: false,
+      lockfileDir: tempy.directory(),
+      pkg: {
+        name: 'is-positive',
+        version: 'v1.0.0',
+        id: pkgResponse.body.id,
+        resolution: pkgResponse.body.resolution,
+      },
+    })
+    await expect(files()).resolves.toStrictEqual(expect.anything())
+  }
+})

pnpm-lock.yaml

@@ -1824,6 +1824,7 @@ importers:
       promise-share: ^1.0.0
       ramda: ^0.27.1
       rename-overwrite: ^4.0.0
+      semver: ^7.3.4
       ssri: ^8.0.1
       tempy: ^1.0.0
     dependencies:
@@ -1846,6 +1847,7 @@ importers:
       promise-share: 1.0.0
       ramda: 0.27.1
       rename-overwrite: 4.0.0
+      semver: 7.3.5
       ssri: 8.0.1
     devDependencies:
       '@pnpm/client': link:../client