feat(publish): fail publish for non-private packages containing packageExtensions

Author: manzoorwanijk

docs/lib/content/configuring-npm/package-json.md

@@ -1046,6 +1046,7 @@ For changing the resolved version of a dependency that is already declared, use
 
 Like `overrides`, `packageExtensions` is only honored in the root `package.json` of a project (the workspace root in a workspace).
 The field in installed dependencies and in non-root workspace packages is ignored.
+Because it is root-only project policy, npm refuses to publish a non-private package that contains `packageExtensions`; it remains available to private packages and unpublished local projects.
 
 Each key is a package selector: a package name with an optional semver range.
 

lib/commands/publish.js

@@ -96,6 +96,9 @@ class Publish extends BaseCommand {
     const spec = npa(args[0])
     let manifest = await this.#getManifest(spec, opts)
 
+    // packageExtensions is root-only project policy and must never be published; fail fast so dry-run reports it too
+    this.#assertNoPackageExtensions(manifest)
+
     // only run scripts for directory type publishes
     if (spec.type === 'directory' && !ignoreScripts) {
       await runScript({
@@ -122,6 +125,8 @@ class Publish extends BaseCommand {
 
     // The purpose of re-reading the manifest is in case it changed, so that we send the latest and greatest thing to the registry note that publishConfig might have changed as well!
     manifest = await this.#getManifest(spec, opts, true)
+    // re-check the authoritative manifest in case a lifecycle script introduced packageExtensions
+    this.#assertNoPackageExtensions(manifest)
     const force = this.npm.config.get('force')
     const isDefaultTag = this.npm.config.isDefault('tag') && !manifest.publishConfig?.tag
 
@@ -273,6 +278,16 @@ class Publish extends BaseCommand {
     }
   }
 
+  // packageExtensions is root-only project policy and must never reach the registry; private packages may keep it for local use
+  #assertNoPackageExtensions (manifest) {
+    if (!manifest.private && manifest.packageExtensions !== undefined) {
+      throw Object.assign(
+        new Error('packageExtensions is only honored at the project root and must not be published.'),
+        { code: 'EPACKAGEEXTENSIONS' }
+      )
+    }
+  }
+
   // if it's a directory, read it from the file system
   // otherwise, get the full metadata from whatever it is
   // XXX can't pacote read the manifest from a directory?

test/lib/commands/publish.js

@@ -218,6 +218,67 @@ t.test('shows usage with wrong set of arguments', async t => {
   await t.rejects(publish.exec(['a', 'b', 'c']), publish.usage)
 })
 
+t.test('fails for a non-private package containing packageExtensions', async t => {
+  const { npm } = await loadNpmWithRegistry(t, {
+    config: { ...auth },
+    prefixDir: {
+      'package.json': JSON.stringify({
+        ...pkgJson,
+        packageExtensions: { 'foo@1': { dependencies: { bar: '^1.0.0' } } },
+      }, null, 2),
+    },
+    authorization: token,
+  })
+  await t.rejects(
+    npm.exec('publish', []),
+    { code: 'EPACKAGEEXTENSIONS', message: /must not be published/ },
+    'refuses to publish'
+  )
+})
+
+t.test('fails on --dry-run for a package containing packageExtensions', async t => {
+  const { npm } = await loadNpmWithRegistry(t, {
+    config: { 'dry-run': true, ...auth },
+    prefixDir: {
+      'package.json': JSON.stringify({
+        ...pkgJson,
+        packageExtensions: { 'foo@1': { dependencies: { bar: '^1.0.0' } } },
+      }, null, 2),
+    },
+    authorization: token,
+  })
+  await t.rejects(
+    npm.exec('publish', []),
+    { code: 'EPACKAGEEXTENSIONS' },
+    'dry-run also reports the failure'
+  )
+})
+
+t.test('fails when a lifecycle script injects packageExtensions before the re-read', async t => {
+  const { npm } = await loadNpmWithRegistry(t, {
+    config: { ...auth },
+    prefixDir: {
+      'package.json': JSON.stringify({
+        ...pkgJson,
+        scripts: { prepublishOnly: 'node inject.js' },
+      }, null, 2),
+      // the first manifest read is clean; this hook adds packageExtensions before the authoritative re-read
+      'inject.js': [
+        "const fs = require('fs')",
+        "const p = JSON.parse(fs.readFileSync('package.json'))",
+        "p.packageExtensions = { 'foo@1': { dependencies: { bar: '^1.0.0' } } }",
+        "fs.writeFileSync('package.json', JSON.stringify(p))",
+      ].join('\n'),
+    },
+    authorization: token,
+  })
+  await t.rejects(
+    npm.exec('publish', []),
+    { code: 'EPACKAGEEXTENSIONS' },
+    'the post-script manifest re-read catches the injected field'
+  )
+})
+
 t.test('throws when invalid tag is semver', async t => {
   const { npm } = await loadNpmWithRegistry(t, {
     config: {

workspaces/libnpmpublish/lib/publish.js

@@ -20,10 +20,12 @@ Remove the 'private' field from the package.json to publish it.`),
     )
   }
 
-  // packageExtensions is consumer-side root policy; warn that publishing it has no effect on consumers
+  // packageExtensions is root-only project policy and must never reach the registry manifest or the published tarball
   if (manifest.packageExtensions !== undefined) {
-    log.warn('publish',
-      'packageExtensions is only honored at the project root and will not affect consumers of this package.')
+    throw Object.assign(
+      new Error('packageExtensions is only honored at the project root and must not be published.'),
+      { code: 'EPACKAGEEXTENSIONS' }
+    )
   }
 
   // spec is used to pick the appropriate registry/auth combo

workspaces/libnpmpublish/test/publish.js

@@ -75,58 +75,21 @@ t.test('basic publish - no npmVersion', async t => {
   t.ok(ret, 'publish succeeded')
 })
 
-t.test('warns when publishing a package with packageExtensions', async t => {
+t.test('fails when publishing a package with packageExtensions', async t => {
   const { publish } = t.mock('..')
-  const registry = new MockRegistry({
-    tap: t,
-    registry: opts.registry,
-    authorization: token,
-  })
+  // no registry interceptor: the publish must fail before any request is made
   const manifest = {
     name: 'libnpmpublish-test',
     version: '1.0.0',
     description: 'test libnpmpublish package',
     packageExtensions: { 'foo@1': { dependencies: { bar: '^1.0.0' } } },
   }
-  const spec = npa(manifest.name)
-
-  // the published version still carries packageExtensions; the nock body proves it is not stripped
-  const packument = {
-    _id: manifest.name,
-    name: manifest.name,
-    description: manifest.description,
-    'dist-tags': { latest: '1.0.0' },
-    versions: {
-      '1.0.0': {
-        _id: `${manifest.name}@${manifest.version}`,
-        _nodeVersion: process.versions.node,
-        ...manifest,
-        dist: {
-          shasum,
-          integrity: integrity.sha512[0].toString(),
-          tarball: 'http://mock.reg/libnpmpublish-test/-/libnpmpublish-test-1.0.0.tgz',
-        },
-      },
-    },
-    access: null,
-    _attachments: {
-      'libnpmpublish-test-1.0.0.tgz': {
-        content_type: 'application/octet-stream',
-        data: tarData.toString('base64'),
-        length: tarData.length,
-      },
-    },
-  }
 
-  const warnings = []
-  const onlog = (level, ...args) => level === 'warn' && warnings.push(args.join(' '))
-  process.on('log', onlog)
-  t.teardown(() => process.removeListener('log', onlog))
-
-  registry.nock.put(`/${spec.escapedName}`, packument).reply(201, {})
-  await publish(manifest, tarData, { ...opts, npmVersion: null })
-  t.match(warnings.join('\n'), /packageExtensions is only honored at the project root/,
-    'warns that packageExtensions does not affect consumers')
+  await t.rejects(
+    publish(manifest, tarData, { ...opts, npmVersion: null }),
+    { code: 'EPACKAGEEXTENSIONS', message: /must not be published/ },
+    'refuses to publish a package containing packageExtensions'
+  )
 })
 
 t.test('scoped publish', async t => {