fix(cli): validate .npm-extension hash in npm ci without executing it

Author: manzoorwanijk

lib/commands/ci.js

@@ -66,6 +66,9 @@ class CI extends ArboristWorkspaceCmd {
       save: false, // npm ci should never modify the lockfile or package.json
       workspaces: this.workspaceNames,
       allowScripts: allowScriptsPolicy,
+      // npm ci reifies the locked graph, which already carries extension-influenced edges, so it must never import or execute .npm-extension.
+      // The extension file hash is still validated below, independent of execution.
+      ignoreExtension: true,
     }
 
     // generate an inventory from the virtual tree in the lockfile
@@ -93,7 +96,15 @@ class CI extends ArboristWorkspaceCmd {
     // Verifies that the root packageExtensions state matches the lockfile and is still consistent with the locked tree.
     errors.push(...validatePackageExtensions(virtualArb.virtualTree, arb.idealTree))
     // Verifies that the root .npm-extension file matches the lockfile hash.
-    errors.push(...validateNpmExtension(virtualArb.virtualTree, arb.idealTree))
+    // The hash comes from discovering the file (no import or execution), so this holds even under ignore-extension/ignore-scripts.
+    const { NpmExtension } = require('@npmcli/arborist')
+    let fileHash = null
+    try {
+      fileHash = new NpmExtension({ root: where, extensionFile: opts.extensionFile }).hash
+    } catch (err) {
+      errors.push(`Invalid: ${err.message}`)
+    }
+    errors.push(...validateNpmExtension(virtualArb.virtualTree, fileHash))
     if (errors.length) {
       throw this.usageError(
         '`npm ci` can only install packages when your package.json and package-lock.json are in sync. ' +

lib/utils/validate-lockfile.js

@@ -98,23 +98,31 @@ function validatePackageExtensions (virtualTree, idealTree) {
   return errors
 }
 
-// validates that the root .npm-extension state recorded in the lockfile still matches the selected extension file.
+// validates that the .npm-extension state recorded in the lockfile still matches the selected extension file.
 // Validation is hash-based: arbitrary code has no selector to re-check, so a matching hash is trusted and a mismatch fails.
+// fileHash is computed from the on-disk file (discovery only, no execution), so this holds even under ignore-extension/ignore-scripts.
+// The lockfile carries extension state if it records a root hash or any per-package npmExtensionApplied provenance.
 // Returns an array of human-readable error strings, empty when valid.
-function validateNpmExtension (virtualTree, idealTree) {
-  const lockHash = virtualTree.meta?.npmExtensionHash || null
-  const idealHash = idealTree.meta?.npmExtensionHash || null
+function validateNpmExtension (virtualTree, fileHash) {
+  const lockHash = virtualTree?.meta?.npmExtensionHash || null
+  const hasProvenance = !!virtualTree &&
+    [...virtualTree.inventory.values()].some(node => node.npmExtensionApplied)
+  fileHash = fileHash || null
 
-  if (idealHash === lockHash) {
+  if (fileHash) {
+    if (!lockHash) {
+      return ['Missing: .npm-extension state from lock file']
+    }
+    if (lockHash !== fileHash) {
+      return ['Invalid: .npm-extension file does not match the lock file']
+    }
     return []
   }
-  if (idealHash && !lockHash) {
-    return ['Missing: .npm-extension state from lock file']
-  }
-  if (!idealHash && lockHash) {
+  // no extension file present
+  if (lockHash || hasProvenance) {
     return ['Invalid: lock file records .npm-extension state but no .npm-extension file is present']
   }
-  return ['Invalid: .npm-extension file does not match the lock file']
+  return []
 }
 
 module.exports = validateLockfile

test/lib/commands/ci.js

@@ -139,6 +139,24 @@ t.test('fails when packageExtensions are out of sync with the lock file', async
   )
 })
 
+t.test('fails when both .npm-extension files are present', async t => {
+  const { npm } = await loadMockNpm(t, {
+    config: { audit: false },
+    prefixDir: {
+      abbrev,
+      'package.json': JSON.stringify(packageJson),
+      'package-lock.json': JSON.stringify(packageLock),
+      '.npm-extension.mjs': 'export function transformManifest (p) { return p }\n',
+      '.npm-extension.cjs': 'module.exports = { transformManifest (p) { return p } }\n',
+    },
+  })
+  await t.rejects(
+    npm.exec('ci', []),
+    /keep only one/,
+    'ci surfaces the ambiguous extension file error'
+  )
+})
+
 t.test('--no-audit and --ignore-scripts', async t => {
   const { npm, joinedOutput, registry } = await loadMockNpm(t, {
     config: {

test/lib/utils/validate-lockfile.js

@@ -212,28 +212,38 @@ t.test('missing virtualTree inventory', async t => {
 
 const { validateNpmExtension } = require('../../../lib/utils/validate-lockfile.js')
 
-// build mock virtual/ideal trees for validateNpmExtension (hash-only)
-const extTree = (hash) => ({ meta: { npmExtensionHash: hash } })
+// build a mock virtual tree with a root hash and optional provenance-bearing nodes
+const extVirtual = (hash, nodes = []) => ({
+  meta: { npmExtensionHash: hash },
+  inventory: { values: () => nodes },
+})
 
 t.test('npmExtension: matching hashes', async t => {
-  t.strictSame(validateNpmExtension(extTree('h'), extTree('h')), [], 'no errors when hashes match')
+  t.strictSame(validateNpmExtension(extVirtual('h'), 'h'), [], 'no errors when hashes match')
 })
 
 t.test('npmExtension: both absent', async t => {
-  t.strictSame(validateNpmExtension(extTree(null), extTree(null)), [], 'no errors when both absent')
+  t.strictSame(validateNpmExtension(extVirtual(null), null), [], 'no errors when both absent')
 })
 
 t.test('npmExtension: missing from lock file', async t => {
-  const errors = validateNpmExtension(extTree(null), extTree('h'))
+  const errors = validateNpmExtension(extVirtual(null), 'h')
   t.match(errors[0], /Missing: \.npm-extension state from lock file/, 'reports missing lock state')
 })
 
 t.test('npmExtension: present in lock but no file', async t => {
-  const errors = validateNpmExtension(extTree('h'), extTree(null))
+  const errors = validateNpmExtension(extVirtual('h'), null)
   t.match(errors[0], /records \.npm-extension state but no \.npm-extension file is present/, 'reports stray lock state')
 })
 
 t.test('npmExtension: hash mismatch', async t => {
-  const errors = validateNpmExtension(extTree('h1'), extTree('h2'))
+  const errors = validateNpmExtension(extVirtual('h1'), 'h2')
   t.match(errors[0], /\.npm-extension file does not match the lock file/, 'reports a hash mismatch')
 })
+
+t.test('npmExtension: provenance without a root hash and no file fails', async t => {
+  const nodes = [{ name: 'foo', npmExtensionApplied: { extensionPoint: 'transformManifest', dependencies: ['bar'] } }]
+  const errors = validateNpmExtension(extVirtual(null, nodes), null)
+  t.match(errors[0], /records \.npm-extension state but no \.npm-extension file is present/,
+    'per-node provenance counts as extension state even without a root hash')
+})