fix: list pending scripts in approve-scripts when ignore-scripts is set (#9482)

Backport of #9479 to `release/v11`.

Co-authored-by: Jamie Magee <jamie.magee@gmail.com>

Author: github-actions[bot]

lib/utils/allow-scripts-cmd.js

@@ -62,7 +62,10 @@ class AllowScriptsCmd extends BaseCommand {
     })
     await arb.loadActual()
 
-    const unreviewed = await checkAllowScripts({ arb, npm: this.npm })
+    // Keep listing unreviewed packages even with ignore-scripts set, so
+    // you can move from a blanket ignore-scripts to an allowlist. This
+    // only lists; nothing runs.
+    const unreviewed = await checkAllowScripts({ arb, npm: this.npm, includeWhenIgnored: true })
 
     if (pending) {
       return this.runPending(unreviewed)

lib/utils/check-allow-scripts.js

@@ -8,11 +8,16 @@ const getInstallScripts = require('@npmcli/arborist/lib/install-scripts.js')
 // Returns an array of `{ node, scripts }` entries. `scripts` is an object
 // describing the relevant lifecycle scripts that would run.
 
-const checkAllowScripts = async ({ arb, npm, tree }) => {
+const checkAllowScripts = async ({ arb, npm, tree, includeWhenIgnored = false }) => {
   const ignoreScripts = !!arb.options?.ignoreScripts
   const dangerouslyAllowAll = !!npm?.flatOptions?.dangerouslyAllowAllScripts
 
-  if (ignoreScripts || dangerouslyAllowAll) {
+  // With ignore-scripts set, no scripts run, so execution callers
+  // (install, rebuild, strict preflight) bail out here. approve/deny pass
+  // includeWhenIgnored so they keep listing unreviewed packages, which is
+  // what you need to move from a blanket ignore-scripts to an allowlist.
+  // Listing never runs anything.
+  if ((ignoreScripts && !includeWhenIgnored) || dangerouslyAllowAll) {
     return []
   }
 

test/lib/commands/approve-scripts.js

@@ -60,6 +60,18 @@ t.test('approve-scripts --pending lists unreviewed packages', async t => {
   t.match(out, /sharp@1\.0\.0/)
 })
 
+t.test('approve-scripts --pending lists unreviewed packages even with ignore-scripts set', async t => {
+  const { npm, joinedOutput } = await mockNpm(t, {
+    prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }),
+    config: { 'allow-scripts-pending': true, 'ignore-scripts': true },
+  })
+  await npm.exec('approve-scripts', [])
+  const out = joinedOutput()
+  t.match(out, /2 packages have install scripts not yet covered/)
+  t.match(out, /canvas@1\.0\.0/)
+  t.match(out, /sharp@1\.0\.0/)
+})
+
 t.test('approve-scripts --pending with no unreviewed says so', async t => {
   const { npm, joinedOutput } = await mockNpm(t, {
     prefixDir: setupProject({

test/lib/utils/check-allow-scripts.js

@@ -57,6 +57,20 @@ t.test('returns [] when ignoreScripts is set', async t => {
   t.strictSame(result, [])
 })
 
+t.test('returns unreviewed nodes when ignoreScripts is set but includeWhenIgnored is true', async t => {
+  const checkAllowScripts = mockCheck(t)
+  const result = await checkAllowScripts({
+    arb: arb({
+      nodes: [node({ name: 'a', scripts: { install: 'do-stuff' } })],
+      ignoreScripts: true,
+    }),
+    npm: { flatOptions: {} },
+    includeWhenIgnored: true,
+  })
+  t.equal(result.length, 1)
+  t.strictSame(result[0].scripts, { install: 'do-stuff' })
+})
+
 t.test('returns [] when dangerouslyAllowAllScripts is set', async t => {
   const checkAllowScripts = mockCheck(t)
   const result = await checkAllowScripts({