fix: prioritize CLI flags over publishConfig settings (#7321)

This PR addresses an issue where CLI flags were not taking precedence
over publishConfig settings. To ensure CLI flags have higher priority,
properties from the publishConfig object that also exist in CLI flags
are filtered out.


  Related to #6400

Author: roni-berlin

lib/commands/publish.js

@@ -220,7 +220,12 @@ class Publish extends BaseCommand {
       })
     }
     if (manifest.publishConfig) {
-      flatten(manifest.publishConfig, opts)
+      const cliFlags = this.npm.config.data.get('cli').raw
+      // Filter out properties set in CLI flags to prioritize them over
+      // corresponding `publishConfig` settings
+      const filteredPublishConfig = Object.fromEntries(
+        Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags)))
+      flatten(filteredPublishConfig, opts)
     }
     return manifest
   }

lib/commands/unpublish.js

@@ -141,7 +141,12 @@ class Unpublish extends BaseCommand {
     // If localPrefix has a package.json with a name that matches the package
     // being unpublished, load up the publishConfig
     if (manifest?.name === spec.name && manifest.publishConfig) {
-      flatten(manifest.publishConfig, opts)
+      const cliFlags = this.npm.config.data.get('cli').raw
+      // Filter out properties set in CLI flags to prioritize them over
+      // corresponding `publishConfig` settings
+      const filteredPublishConfig = Object.fromEntries(
+        Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags)))
+      flatten(filteredPublishConfig, opts)
     }
 
     const versions = await Unpublish.getKeysOfVersions(spec.name, opts)

tap-snapshots/test/lib/commands/publish.js.test.cjs

@@ -452,6 +452,10 @@ exports[`test/lib/commands/publish.js TAP restricted access > new package versio
 + @npm/[email protected]
 `
 
+exports[`test/lib/commands/publish.js TAP prioritize CLI flags over publishConfig > new package version 1`] = `
++ [email protected]
+`
+
 exports[`test/lib/commands/publish.js TAP scoped _auth config scoped registry > new package version 1`] = `
 + @npm/[email protected]
 `

test/lib/commands/publish.js

@@ -131,6 +131,58 @@ t.test('re-loads publishConfig.registry if added during script process', async t
   t.matchSnapshot(joinedOutput(), 'new package version')
 })
 
+t.test('prioritize CLI flags over publishConfig', async t => {
+  const publishConfig = { registry: 'http://publishconfig' }
+  const { joinedOutput, npm } = await loadMockNpm(t, {
+    config: {
+      [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token',
+    },
+    prefixDir: {
+      'package.json': JSON.stringify({
+        ...pkgJson,
+        scripts: {
+          prepare: 'cp new.json package.json',
+        },
+      }, null, 2),
+      'new.json': JSON.stringify({
+        ...pkgJson,
+        publishConfig,
+      }),
+    },
+    argv: ['--registry', alternateRegistry],
+  })
+  const registry = new MockRegistry({
+    tap: t,
+    registry: alternateRegistry,
+    authorization: 'test-other-token',
+  })
+  registry.nock.put(`/${pkg}`, body => {
+    return t.match(body, {
+      _id: pkg,
+      name: pkg,
+      'dist-tags': { latest: '1.0.0' },
+      access: null,
+      versions: {
+        '1.0.0': {
+          name: pkg,
+          version: '1.0.0',
+          _id: `${pkg}@1.0.0`,
+          dist: {
+            shasum: /\.*/,
+            tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-1.0.0.tgz`,
+          },
+          publishConfig,
+        },
+      },
+      _attachments: {
+        [`${pkg}-1.0.0.tgz`]: {},
+      },
+    })
+  }).reply(200, {})
+  await npm.exec('publish', [])
+  t.matchSnapshot(joinedOutput(), 'new package version')
+})
+
 t.test('json', async t => {
   const { joinedOutput, npm, logs } = await loadMockNpm(t, {
     config: {

test/lib/commands/unpublish.js

@@ -408,6 +408,36 @@ t.test('publishConfig no spec', async t => {
   t.equal(joinedOutput(), '- test-package')
 })
 
+t.test('prioritize CLI flags over publishConfig no spec', async t => {
+  const alternateRegistry = 'https://other.registry.npmjs.org'
+  const publishConfig = { registry: 'http://publishconfig' }
+  const { joinedOutput, npm } = await loadMockNpm(t, {
+    config: {
+      force: true,
+      '//other.registry.npmjs.org/:_authToken': 'test-other-token',
+    },
+    prefixDir: {
+      'package.json': JSON.stringify({
+        name: pkg,
+        version: '1.0.0',
+        publishConfig,
+      }, null, 2),
+    },
+    argv: ['--registry', alternateRegistry],
+  })
+
+  const registry = new MockRegistry({
+    tap: t,
+    registry: alternateRegistry,
+    authorization: 'test-other-token',
+  })
+  const manifest = registry.manifest({ name: pkg })
+  await registry.package({ manifest, query: { write: true }, times: 2 })
+  registry.unpublish({ manifest })
+  await npm.exec('unpublish', [])
+  t.equal(joinedOutput(), '- test-package')
+})
+
 t.test('publishConfig with spec', async t => {
   const alternateRegistry = 'https://other.registry.npmjs.org'
   const { joinedOutput, npm } = await loadMockNpm(t, {