fix: detect reverted catalog entries on install (#12438)

* fix: detect reverted catalog entries on install

After an update bumped a catalog entry in pnpm-workspace.yaml, the workspace
state cache stored the pre-update catalog versions, so reverting the entry back
to its original version was reported as "Already up to date" instead of
reinstalling the previous version.

Fold the catalogs written during the install into the catalogs recorded in the
workspace state so a later install detects the reverted entry as outdated.

Closes https://github.com/pnpm/pnpm/issues/12418

* fix: harden catalog merge against prototype pollution and entry loss

Address review feedback on the catalog-merge helper:

- mergeCatalogs now builds null-prototype records and copies entries with
  Object.defineProperty, so a catalog or dependency name like __proto__
  (which can flow in from parsed pnpm-workspace.yaml) becomes an ordinary
  own property instead of corrupting the result's prototype.
- The recursive per-project install path now accumulates updatedCatalogs
  with mergeCatalogs instead of a shallow Object.assign, so two projects
  updating different entries of the same catalog no longer clobber each
  other.

.changeset/twelve-catalogs-revert.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/catalogs.config": patch
+"@pnpm/installing.commands": patch
+"pnpm": patch
+---
+
+Fix `pnpm install` reporting "Already up to date" after a catalog entry in `pnpm-workspace.yaml` was reverted to a previous version. After an update modified a catalog, the workspace state cache stored the pre-update catalog versions, so reverting the entry back to its original version was not detected as an outdated state [#12418](https://github.com/pnpm/pnpm/issues/12418).

catalogs/config/src/index.ts

@@ -1 +1,2 @@
 export { getCatalogsFromWorkspaceManifest } from './getCatalogsFromWorkspaceManifest.js'
+export { mergeCatalogs } from './mergeCatalogs.js'

catalogs/config/src/mergeCatalogs.ts

@@ -0,0 +1,40 @@
+import type { Catalog, Catalogs } from '@pnpm/catalogs.types'
+
+/**
+ * Deep-merges catalog definitions, later arguments taking precedence over
+ * earlier ones at the individual entry level. Use it to fold the catalog
+ * changes produced by an install (`updatedCatalogs`) back into the catalogs
+ * read at startup, so the result reflects what was actually written to
+ * `pnpm-workspace.yaml`.
+ *
+ * Catalog and dependency names originate from `pnpm-workspace.yaml`, so the
+ * result is built from null-prototype records and entries are copied with
+ * `Object.defineProperty`. A name like `__proto__` then becomes an ordinary
+ * own property instead of mutating a prototype.
+ */
+export function mergeCatalogs (...catalogsList: Array<Catalogs | undefined>): Catalogs {
+  const result = Object.create(null) as Record<string, Catalog>
+  for (const catalogs of catalogsList) {
+    if (catalogs == null) continue
+    for (const catalogName of Object.keys(catalogs)) {
+      const catalog = catalogs[catalogName]
+      if (catalog == null) continue
+      const target: Record<string, string | undefined> = result[catalogName] ?? Object.create(null)
+      for (const dependencyName of Object.keys(catalog)) {
+        Object.defineProperty(target, dependencyName, {
+          value: catalog[dependencyName],
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        })
+      }
+      Object.defineProperty(result, catalogName, {
+        value: target,
+        writable: true,
+        enumerable: true,
+        configurable: true,
+      })
+    }
+  }
+  return result
+}

catalogs/config/test/mergeCatalogs.test.ts

@@ -0,0 +1,53 @@
+import { expect, test } from '@jest/globals'
+import { mergeCatalogs } from '@pnpm/catalogs.config'
+
+test('returns an empty catalog when nothing is passed', () => {
+  expect(mergeCatalogs()).toEqual({})
+  expect(mergeCatalogs(undefined, undefined)).toEqual({})
+})
+
+test('later entries override earlier ones at the dependency level', () => {
+  expect(mergeCatalogs(
+    {
+      default: { foo: '1.0.0', bar: '1.0.0' },
+      named: { baz: '1.0.0' },
+    },
+    {
+      default: { foo: '2.0.0' },
+    }
+  )).toEqual({
+    default: { foo: '2.0.0', bar: '1.0.0' },
+    named: { baz: '1.0.0' },
+  })
+})
+
+test('adds catalog entries that did not exist in the base', () => {
+  expect(mergeCatalogs(
+    { default: { foo: '1.0.0' } },
+    { default: { bar: '1.0.0' }, named: { baz: '1.0.0' } }
+  )).toEqual({
+    default: { foo: '1.0.0', bar: '1.0.0' },
+    named: { baz: '1.0.0' },
+  })
+})
+
+test('skips nullish catalog arguments and nullish named catalogs', () => {
+  expect(mergeCatalogs(
+    undefined,
+    { default: { foo: '1.0.0' }, named: undefined }
+  )).toEqual({
+    default: { foo: '1.0.0' },
+  })
+})
+
+test('treats dangerous catalog and dependency names as ordinary own properties', () => {
+  // Catalogs parsed from YAML/JSON can carry `__proto__` as an own property,
+  // unlike an object literal where `{ __proto__: ... }` sets the prototype.
+  const malicious = JSON.parse('{"__proto__":{"polluted":"yes"},"default":{"__proto__":"1.0.0","constructor":"2.0.0"}}')
+  const merged = mergeCatalogs(malicious)
+  expect(({} as Record<string, unknown>).polluted).toBeUndefined()
+  expect(Object.prototype.hasOwnProperty.call(merged, '__proto__')).toBe(true)
+  expect(Object.prototype.hasOwnProperty.call(merged.default, '__proto__')).toBe(true)
+  expect((merged.default as Record<string, unknown>).__proto__).toBe('1.0.0')
+  expect((merged.default as Record<string, unknown>).constructor).toBe('2.0.0')
+})

installing/commands/package.json

@@ -35,6 +35,7 @@
     "@inquirer/prompts": "catalog:",
     "@pnpm/building.after-install": "workspace:*",
     "@pnpm/building.policy": "workspace:*",
+    "@pnpm/catalogs.config": "workspace:*",
     "@pnpm/catalogs.types": "workspace:*",
     "@pnpm/cli.command": "workspace:*",
     "@pnpm/cli.common-cli-options-help": "workspace:*",

installing/commands/src/installDeps.ts

@@ -1,6 +1,8 @@
 import path from 'node:path'
 
 import { buildProjects } from '@pnpm/building.after-install'
+import { mergeCatalogs } from '@pnpm/catalogs.config'
+import type { Catalogs } from '@pnpm/catalogs.types'
 import type { CommandHandler } from '@pnpm/cli.command'
 import {
   readProjectManifestOnly,
@@ -426,7 +428,7 @@ export async function installDeps (
     if (!opts.lockfileOnly) {
       await updateWorkspaceState({
         allProjects,
-        settings: opts,
+        settings: withUpdatedCatalogs(opts, updatedCatalogs),
         workspaceDir: opts.workspaceDir ?? opts.lockfileDir ?? opts.dir,
         pnpmfiles: opts.pnpmfile,
         filteredInstall: allProjects.length !== Object.keys(opts.selectedProjectsGraph ?? {}).length,
@@ -485,7 +487,7 @@ export async function installDeps (
       selectedProjectsGraph,
       workspaceDir: opts.workspaceDir, // Otherwise TypeScript doesn't understand that is not undefined
       runPacquet,
-    }, 'install')
+    }, 'install', updatedCatalogs)
 
     if (opts.ignoreScripts) return
 
@@ -508,7 +510,7 @@ export async function installDeps (
     if (!opts.lockfileOnly) {
       await updateWorkspaceState({
         allProjects,
-        settings: opts,
+        settings: withUpdatedCatalogs(opts, updatedCatalogs),
         workspaceDir: opts.workspaceDir ?? opts.lockfileDir ?? opts.dir,
         pnpmfiles: opts.pnpmfile,
         filteredInstall: allProjects.length !== Object.keys(opts.selectedProjectsGraph ?? {}).length,
@@ -528,20 +530,36 @@ async function recursiveInstallThenUpdateWorkspaceState (
   allProjects: Project[],
   params: string[],
   opts: RecursiveOptions & WorkspaceStateSettings,
-  cmdFullName: CommandFullName
+  cmdFullName: CommandFullName,
+  updatedCatalogs?: Catalogs
 ): Promise<boolean | string> {
   const recursiveResult = await recursive(allProjects, params, opts, cmdFullName)
   if (!opts.lockfileOnly) {
     await updateWorkspaceState({
       allProjects,
-      settings: opts,
+      settings: withUpdatedCatalogs(opts, updatedCatalogs, recursiveResult.updatedCatalogs),
       workspaceDir: opts.workspaceDir,
       pnpmfiles: opts.pnpmfile,
       filteredInstall: allProjects.length !== Object.keys(opts.selectedProjectsGraph ?? {}).length,
       configDependencies: opts.configDependencies,
     })
   }
-  return recursiveResult
+  return recursiveResult.passed
+}
+
+/**
+ * Folds the catalog entries written to `pnpm-workspace.yaml` during this
+ * install into the catalogs read at startup. The workspace state cache records
+ * these so a later install detects when a catalog entry was reverted; without
+ * this, the cache would keep the stale pre-install catalogs and report
+ * "Already up to date" even though the manifest changed.
+ */
+function withUpdatedCatalogs<T extends { catalogs?: Catalogs }> (
+  settings: T,
+  ...updatedCatalogs: Array<Catalogs | undefined>
+): T {
+  if (updatedCatalogs.every((catalogs) => catalogs == null)) return settings
+  return { ...settings, catalogs: mergeCatalogs(settings.catalogs, ...updatedCatalogs) }
 }
 
 function severityStringToNumber (severity: VulnerabilitySeverity): number {

installing/commands/src/recursive.ts

@@ -1,6 +1,7 @@
 import { promises as fs } from 'node:fs'
 import path from 'node:path'
 
+import { mergeCatalogs } from '@pnpm/catalogs.config'
 import type { Catalogs } from '@pnpm/catalogs.types'
 import type { CommandHandler } from '@pnpm/cli.command'
 import {
@@ -144,21 +145,31 @@ export type RecursiveOptions = CreateStoreControllerOptions & Pick<Config,
 
 export type CommandFullName = 'install' | 'add' | 'remove' | 'update' | 'import'
 
+export interface RecursiveResult {
+  passed: boolean | string
+  /**
+   * Catalog entries written to `pnpm-workspace.yaml` during this install.
+   * The caller folds these into the catalogs recorded in the workspace state
+   * cache so that reverting a catalog entry is detected as an outdated state.
+   */
+  updatedCatalogs?: Catalogs
+}
+
 export async function recursive (
   allProjects: Project[],
   params: string[],
   opts: RecursiveOptions,
   cmdFullName: CommandFullName
-): Promise<boolean | string> {
+): Promise<RecursiveResult> {
   if (allProjects.length === 0) {
     // It might make sense to throw an exception in this case
-    return false
+    return { passed: false }
   }
 
   const pkgs = Object.values(opts.selectedProjectsGraph).map((wsPkg) => wsPkg.package)
 
   if (pkgs.length === 0) {
-    return false
+    return { passed: false }
   }
   const manifestsByPath = getManifestsByPath(allProjects)
 
@@ -225,7 +236,7 @@ export async function recursive (
     const calculatedRepositoryRoot = await fs.realpath(calculateRepositoryRoot(opts.workspaceDir, importers.map(x => x.rootDir)))
     const isFromWorkspace = isSubdir.bind(null, calculatedRepositoryRoot)
     importers = await pFilter(importers, async ({ rootDirRealPath }) => isFromWorkspace(rootDirRealPath))
-    if (importers.length === 0) return true
+    if (importers.length === 0) return { passed: true }
     let mutation: 'install' | 'installSome' | 'uninstallSome'
     switch (cmdFullName) {
       case 'remove':
@@ -341,7 +352,7 @@ export async function recursive (
       await Promise.all(promises)
     }
     await handleIgnoredBuilds(opts, ignoredBuilds)
-    return true
+    return { passed: true, updatedCatalogs }
   }
 
   const pkgPaths = (Object.keys(opts.selectedProjectsGraph) as ProjectRootDir[]).sort()
@@ -459,8 +470,10 @@ export async function recursive (
         if (opts.save !== false) {
           await writeProjectManifest(newManifest)
           if (newCatalogsAddition) {
-            updatedCatalogs ??= {}
-            Object.assign(updatedCatalogs, newCatalogsAddition)
+            // Per-project additions are partial maps keyed by catalog name then
+            // dependency. Merge at the dependency level so two projects updating
+            // different entries of the same catalog don't clobber each other.
+            updatedCatalogs = mergeCatalogs(updatedCatalogs, newCatalogsAddition)
           }
         }
         if (ignoredBuilds?.size) {
@@ -527,7 +540,7 @@ export async function recursive (
       'None of the specified packages were found in the dependencies of any of the projects.')
   }
 
-  return true
+  return { passed: true, updatedCatalogs }
 }
 
 function calculateRepositoryRoot (

installing/commands/tsconfig.json

@@ -27,6 +27,9 @@
     {
       "path": "../../building/policy"
     },
+    {
+      "path": "../../catalogs/config"
+    },
     {
       "path": "../../catalogs/types"
     },

pnpm-lock.yaml

@@ -5246,6 +5246,9 @@ importers:
       '@pnpm/building.policy':
         specifier: workspace:*
         version: link:../../building/policy
+      '@pnpm/catalogs.config':
+        specifier: workspace:*
+        version: link:../../catalogs/config
       '@pnpm/catalogs.types':
         specifier: workspace:*
         version: link:../../catalogs/types

pnpm/test/catalogUpToDate.ts

@@ -0,0 +1,48 @@
+import { expect, test } from '@jest/globals'
+import { prepare } from '@pnpm/prepare'
+import { addDistTag } from '@pnpm/testing.registry-mock'
+import type { ProjectManifest } from '@pnpm/types'
+import { writeYamlFileSync } from 'write-yaml-file'
+
+import { execPnpm } from './utils/index.js'
+
+test('reverting a catalog entry after updating it is detected as an outdated state (#12418)', async () => {
+  await addDistTag({ package: '@pnpm.e2e/foo', version: '100.1.0', distTag: 'latest' })
+
+  const manifest: ProjectManifest = {
+    name: 'test-catalog-up-to-date',
+    version: '0.0.0',
+    private: true,
+    dependencies: {
+      '@pnpm.e2e/foo': 'catalog:',
+    },
+  }
+  const project = prepare(manifest)
+
+  writeYamlFileSync('pnpm-workspace.yaml', {
+    packages: ['.'],
+    catalog: {
+      '@pnpm.e2e/foo': '100.0.0',
+    },
+  })
+
+  await execPnpm(['install'])
+  expect(project.readCurrentLockfile().importers['.'].dependencies?.['@pnpm.e2e/foo'].version).toBe('100.0.0')
+
+  await execPnpm(['update', '@pnpm.e2e/foo', '--latest'])
+  expect(project.readCurrentLockfile().importers['.'].dependencies?.['@pnpm.e2e/foo'].version).toBe('100.1.0')
+
+  // Restore the catalog entry to the version it had before the update. A
+  // subsequent install must notice that the workspace state is no longer up to
+  // date and reinstall the previous version instead of reporting
+  // "Already up to date".
+  writeYamlFileSync('pnpm-workspace.yaml', {
+    packages: ['.'],
+    catalog: {
+      '@pnpm.e2e/foo': '100.0.0',
+    },
+  })
+
+  await execPnpm(['install'])
+  expect(project.readCurrentLockfile().importers['.'].dependencies?.['@pnpm.e2e/foo'].version).toBe('100.0.0')
+})