fix: preserve reference overrides in pnpm audit --fix (#10478)

close #10325

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/fuzzy-cups-float.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/workspace.manifest-writer": minor
+"@pnpm/config.config-writer": minor
+---
+
+New option added: updatedOverrides.

.changeset/pink-dragons-sing.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-audit": patch
+"pnpm": patch
+---
+
+Fix `pnpm audit --fix` replacing reference overrides (e.g. `$foo`) with concrete versions [#10325](https://github.com/pnpm/pnpm/issues/10325).

config/config-writer/src/index.ts

@@ -4,7 +4,8 @@ import { updateWorkspaceManifest } from '@pnpm/workspace.manifest-writer'
 import equals from 'ramda/src/equals'
 
 export interface WriteSettingsOptions {
-  updatedSettings: PnpmSettings
+  updatedSettings?: PnpmSettings
+  updatedOverrides?: Record<string, string>
   rootProjectManifest?: ProjectManifest
   rootProjectManifestDir: string
   workspaceDir: string
@@ -16,13 +17,24 @@ export async function writeSettings (opts: WriteSettingsOptions): Promise<void>
     if (manifest) {
       manifest.pnpm ??= {}
       let shouldBeUpdated = false
-      for (const [key, value] of Object.entries(opts.updatedSettings)) {
-        if (!equals(manifest.pnpm[key as keyof PnpmSettings], value)) {
-          shouldBeUpdated = true
-          if (value == null) {
-            delete manifest.pnpm[key as keyof PnpmSettings]
-          } else {
-            manifest.pnpm[key as keyof PnpmSettings] = value
+      if (opts.updatedSettings) {
+        for (const [key, value] of Object.entries(opts.updatedSettings)) {
+          if (!equals(manifest.pnpm[key as keyof PnpmSettings], value)) {
+            shouldBeUpdated = true
+            if (value == null) {
+              delete manifest.pnpm[key as keyof PnpmSettings]
+            } else {
+              manifest.pnpm[key as keyof PnpmSettings] = value
+            }
+          }
+        }
+      }
+      if (opts.updatedOverrides) {
+        manifest.pnpm.overrides ??= {}
+        for (const [key, value] of Object.entries(opts.updatedOverrides)) {
+          if (!equals(manifest.pnpm.overrides[key], value)) {
+            shouldBeUpdated = true
+            manifest.pnpm.overrides[key] = value
           }
         }
       }
@@ -37,5 +49,6 @@ export async function writeSettings (opts: WriteSettingsOptions): Promise<void>
   }
   await updateWorkspaceManifest(opts.workspaceDir, {
     updatedFields: opts.updatedSettings,
+    updatedOverrides: opts.updatedOverrides,
   })
 }

lockfile/plugin-commands-audit/src/fix.ts

@@ -7,12 +7,7 @@ export async function fix (auditReport: AuditReport, opts: AuditOptions): Promis
   const vulnOverrides = createOverrides(Object.values(auditReport.advisories), opts.auditConfig?.ignoreCves)
   if (Object.values(vulnOverrides).length === 0) return vulnOverrides
   await writeSettings({
-    updatedSettings: {
-      overrides: {
-        ...opts.overrides,
-        ...vulnOverrides,
-      },
-    },
+    updatedOverrides: vulnOverrides,
     rootProjectManifest: opts.rootProjectManifest,
     rootProjectManifestDir: opts.rootProjectManifestDir,
     workspaceDir: opts.workspaceDir ?? opts.rootProjectManifestDir,

lockfile/plugin-commands-audit/test/fixtures/preserve-reference-overrides/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "repro-10325",
+  "version": "1.0.0",
+  "dependencies": {
+    "is-positive": "1.0.0",
+    "axios": "0.18.0"
+  }
+}

lockfile/plugin-commands-audit/test/fixtures/preserve-reference-overrides/pnpm-lock.yaml

@@ -0,0 +1,24 @@
+lockfileVersion: '9.0'
+
+settings:
+  autoInstallPeers: true
+  excludeLinksFromLockfile: false
+
+importers:
+
+  .:
+    dependencies:
+      is-positive:
+        specifier: 1.0.0
+        version: 1.0.0
+      axios:
+        specifier: 0.18.0
+        version: 0.18.0
+
+packages:
+
+  is-positive@1.0.0:
+    resolution: {integrity: sha512-XXHOl7s9z2r8d64Z9s/r3P+C1tCjkd5atfFCXAAuExPXq64/t07jjiWDp5yY++5X+tSWwb3S8WJtTLCsCGL+wQ==}
+
+  axios@0.18.0:
+    resolution: {integrity: sha512-1qjL8847bdp87/g7G5nCW12s5J0D1Xv45Z6M4Z5Tsp8sTuXj5w8e0HIiln4Wj12v2H2tX4f6j62/j/k7u0/g==}

lockfile/plugin-commands-audit/test/fixtures/preserve-reference-overrides/pnpm-workspace.yaml

@@ -0,0 +1,3 @@
+overrides:
+  is-positive: "$is-positive"
+

lockfile/plugin-commands-audit/test/preserveReferenceOverrides.ts

@@ -0,0 +1,46 @@
+import path from 'path'
+import { audit } from '@pnpm/plugin-commands-audit'
+import { fixtures } from '@pnpm/test-fixtures'
+import { readProjectManifest } from '@pnpm/read-project-manifest'
+import { sync as readYamlFile } from 'read-yaml-file'
+import nock from 'nock'
+import * as responses from './utils/responses/index.js'
+
+const f = fixtures(import.meta.dirname)
+const registries = {
+  default: 'https://registry.npmjs.org/',
+}
+const rawConfig = {
+  registry: registries.default,
+}
+
+test('overrides with references (via $) are preserved during audit --fix', async () => {
+  const tmp = f.prepare('preserve-reference-overrides')
+
+  nock(registries.default)
+    .post('/-/npm/v1/security/audits')
+    .reply(200, responses.ALL_VULN_RESP)
+
+  const { manifest: initialManifest } = await readProjectManifest(tmp)
+
+  const { exitCode, output } = await audit.handler({
+    auditLevel: 'moderate',
+    dir: tmp,
+    rootProjectManifestDir: tmp,
+    rootProjectManifest: initialManifest,
+    fix: true,
+    userConfig: {},
+    rawConfig,
+    registries,
+    virtualStoreDirMaxLength: process.platform === 'win32' ? 60 : 120,
+    overrides: {
+      'is-positive': '1.0.0',
+    },
+  })
+
+  expect(exitCode).toBe(0)
+  expect(output).toMatch(/overrides were added/)
+
+  const manifest = readYamlFile<any>(path.join(tmp, 'pnpm-workspace.yaml')) // eslint-disable-line
+  expect(manifest.overrides?.['is-positive']).toBe('$is-positive')
+})

workspace/manifest-writer/src/index.ts

@@ -28,6 +28,7 @@ async function writeManifestFile (dir: string, manifest: Partial<WorkspaceManife
 export async function updateWorkspaceManifest (dir: string, opts: {
   updatedFields?: Partial<WorkspaceManifest>
   updatedCatalogs?: Catalogs
+  updatedOverrides?: Record<string, string>
   cleanupUnusedCatalogs?: boolean
   allProjects?: Project[]
 }): Promise<void> {
@@ -73,6 +74,15 @@ export async function updateWorkspaceManifest (dir: string, opts: {
       }
     }
   }
+  if (opts.updatedOverrides) {
+    manifest.overrides ??= {}
+    for (const [key, value] of Object.entries(opts.updatedOverrides)) {
+      if (!equals(manifest.overrides[key], value)) {
+        shouldBeUpdated = true
+        manifest.overrides[key] = value
+      }
+    }
+  }
   if (!shouldBeUpdated) {
     return
   }

workspace/manifest-writer/test/updateWorkspaceManifest.test.ts

@@ -60,6 +60,95 @@ test('updateWorkspaceManifest updates allowBuilds', async () => {
   })
 })
 
+test('updateWorkspaceManifest with updatedOverrides adds overrides when none exist', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+  writeYamlFile(filePath, { packages: ['*'] })
+  await updateWorkspaceManifest(dir, {
+    updatedOverrides: { foo: '1.0.0', bar: '2.0.0' },
+  })
+  expect(readYamlFile(filePath)).toStrictEqual({
+    packages: ['*'],
+    overrides: {
+      bar: '2.0.0',
+      foo: '1.0.0',
+    },
+  })
+})
+
+test('updateWorkspaceManifest with updatedOverrides merges into existing overrides', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+  writeYamlFile(filePath, { packages: ['*'], overrides: { existing: '1.0.0' } })
+  await updateWorkspaceManifest(dir, {
+    updatedOverrides: { newPkg: '2.0.0' },
+  })
+  expect(readYamlFile(filePath)).toStrictEqual({
+    packages: ['*'],
+    overrides: {
+      existing: '1.0.0',
+      newPkg: '2.0.0',
+    },
+  })
+})
+
+test('updateWorkspaceManifest with updatedOverrides updates existing override values', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+  writeYamlFile(filePath, { packages: ['*'], overrides: { foo: '1.0.0', bar: '1.0.0' } })
+  await updateWorkspaceManifest(dir, {
+    updatedOverrides: { foo: '2.0.0' },
+  })
+  expect(readYamlFile(filePath)).toStrictEqual({
+    packages: ['*'],
+    overrides: {
+      bar: '1.0.0',
+      foo: '2.0.0',
+    },
+  })
+})
+
+test('updateWorkspaceManifest with updatedOverrides does not update when values are equal', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+  const originalContent = 'packages:\n  - \'*\'\noverrides:\n  foo: \'1.0.0\'\n'
+  fs.writeFileSync(filePath, originalContent)
+  await updateWorkspaceManifest(dir, {
+    updatedOverrides: { foo: '1.0.0' },
+  })
+  expect(fs.readFileSync(filePath).toString()).toStrictEqual(originalContent)
+})
+
+test('updateWorkspaceManifest with updatedOverrides preserves comments', async () => {
+  const dir = tempDir(false)
+  const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)
+
+  const manifest = `\
+packages:
+  - '*'
+
+overrides:
+  # Comment on existing
+  existing: '1.0.0'
+`
+
+  const expected = `\
+packages:
+  - '*'
+
+overrides:
+  # Comment on existing
+  existing: '1.0.0'
+  newPkg: ^2.0.0
+`
+
+  fs.writeFileSync(filePath, manifest)
+  await updateWorkspaceManifest(dir, {
+    updatedOverrides: { newPkg: '^2.0.0' },
+  })
+  expect(fs.readFileSync(filePath).toString()).toStrictEqual(expected)
+})
+
 test('updateWorkspaceManifest adds a new catalog', async () => {
   const dir = tempDir(false)
   const filePath = path.join(dir, WORKSPACE_MANIFEST_FILENAME)