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
npm · Apr 9, 2024 · c929ed1 · c929ed1
1 parent 70497cb
commit c929ed1
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 2 deletions.
diff --git a/lib/commands/publish.js b/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
   }

diff --git a/lib/commands/unpublish.js b/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)

diff --git a/tap-snapshots/test/lib/commands/publish.js.test.cjs b/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/test-package@1.0.0
 `
 
+exports[`test/lib/commands/publish.js TAP prioritize CLI flags over publishConfig > new package version 1`] = `
++ test-package@1.0.0
+`
+
 exports[`test/lib/commands/publish.js TAP scoped _auth config scoped registry > new package version 1`] = `
 + @npm/test-package@1.0.0
 `

diff --git a/test/lib/commands/publish.js b/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: {

diff --git a/test/lib/commands/unpublish.js b/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, {