fix: emit valid JSON from approve-scripts/deny-scripts --json

owlstronaut · owlstronaut · commit f6270d1069dc · 2026-06-10T14:23:30.000-07:00
diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js
@@ -79,6 +79,10 @@ class AllowScriptsCmd extends BaseCommand {
   }
 
   runPending (unreviewed) {
+    if (this.npm.flatOptions.json) {
+      output.buffer({ allowScripts: this.pendingSummary(unreviewed) })
+      return
+    }
     if (unreviewed.length === 0) {
       output.standard('No packages with unreviewed install scripts.')
       return
@@ -105,8 +109,30 @@ class AllowScriptsCmd extends BaseCommand {
     )
   }
 
+  // Build the same `{ name, changes }` shape printSummary uses for writes,
+  // but tag every entry as `pending` since nothing is written. Names and
+  // versions are derived exactly like the text listing above.
+  pendingSummary (unreviewed) {
+    const groups = new Map()
+    for (const { node } of unreviewed) {
+      const { name, version } = trustedDisplay(node)
+      /* istanbul ignore next: every test node has a name */
+      const display = name || '<unknown>'
+      const key = version ? `${display}@${version}` : display
+      if (!groups.has(display)) {
+        groups.set(display, [])
+      }
+      groups.get(display).push({ key, change: 'pending' })
+    }
+    return [...groups].map(([name, changes]) => ({ name, changes }))
+  }
+
   async runAll (unreviewed) {
     if (unreviewed.length === 0) {
+      if (this.npm.flatOptions.json) {
+        output.buffer({ allowScripts: [] })
+        return
+      }
       output.standard('No packages with unreviewed install scripts.')
       return
     }
diff --git a/test/lib/commands/approve-scripts.js b/test/lib/commands/approve-scripts.js
@@ -178,6 +178,51 @@ t.test('approve-scripts --json outputs structured summary', async t => {
   })
 })
 
+t.test('approve-scripts --pending --json lists unreviewed packages as JSON', async t => {
+  const { npm, joinedOutput } = await mockNpm(t, {
+    prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }),
+    config: { 'allow-scripts-pending': true, json: true },
+  })
+  await npm.exec('approve-scripts', [])
+  const parsed = JSON.parse(joinedOutput())
+  const byName = Object.fromEntries(parsed.allowScripts.map((e) => [e.name, e.changes]))
+  t.strictSame(byName, {
+    canvas: [{ key: 'canvas@1.0.0', change: 'pending' }],
+    sharp: [{ key: 'sharp@1.0.0', change: 'pending' }],
+  })
+})
+
+t.test('approve-scripts --pending --json with no unreviewed emits empty list', async t => {
+  const { npm, joinedOutput } = await mockNpm(t, {
+    prefixDir: setupProject({
+      allowScripts: { canvas: true },
+      withScripts: ['canvas'],
+    }),
+    config: { 'allow-scripts-pending': true, json: true },
+  })
+  await npm.exec('approve-scripts', [])
+  t.strictSame(JSON.parse(joinedOutput()), { allowScripts: [] })
+})
+
+t.test('approve-scripts --all --json with no unreviewed emits empty list', async t => {
+  const { npm, joinedOutput } = await _mockNpm(t, {
+    prefixDir: {
+      'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }),
+      'package-lock.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        lockfileVersion: 3,
+        requires: true,
+        packages: { '': { name: 'host', version: '1.0.0' } },
+      }),
+      node_modules: {},
+    },
+    config: { all: true, json: true },
+  })
+  await npm.exec('approve-scripts', [])
+  t.strictSame(JSON.parse(joinedOutput()), { allowScripts: [] })
+})
+
 t.test('approve-scripts --all with no unreviewed packages prints message', async t => {
   const { npm, joinedOutput } = await _mockNpm(t, {
     prefixDir: {
@@ -231,63 +276,83 @@ t.test('approve-scripts --pending lists package with no version', async t => {
   t.pass()
 })
 
-t.test('approve-scripts groups multiple installed versions of the same package', async t => {
-  // Two versions of lodash exist in the tree; both have install scripts.
-  // groupByPackage should put them in the same group (hits the
-  // `if (!groups[key])` falsy branch on the second node).
-  const { npm, prefix } = await _mockNpm(t, {
-    prefixDir: {
-      'package.json': JSON.stringify({
-        name: 'host',
-        version: '1.0.0',
-        dependencies: { 'top-of-tree': '*' },
-      }),
-      'package-lock.json': JSON.stringify({
-        name: 'host',
+const twoVersionFixture = {
+  'package.json': JSON.stringify({
+    name: 'host',
+    version: '1.0.0',
+    dependencies: { 'top-of-tree': '*' },
+  }),
+  'package-lock.json': JSON.stringify({
+    name: 'host',
+    version: '1.0.0',
+    lockfileVersion: 3,
+    requires: true,
+    packages: {
+      '': { name: 'host', version: '1.0.0', dependencies: { 'top-of-tree': '*' } },
+      'node_modules/lodash': {
+        version: '4.17.21',
+        resolved: 'https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz',
+        hasInstallScript: true,
+      },
+      'node_modules/top-of-tree': {
         version: '1.0.0',
-        lockfileVersion: 3,
-        requires: true,
-        packages: {
-          '': { name: 'host', version: '1.0.0', dependencies: { 'top-of-tree': '*' } },
-          'node_modules/lodash': {
-            version: '4.17.21',
-            resolved: 'https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz',
-            hasInstallScript: true,
-          },
-          'node_modules/top-of-tree': {
-            version: '1.0.0',
-            resolved: 'https://registry.npmjs.org/top-of-tree/-/top-of-tree-1.0.0.tgz',
-            dependencies: { lodash: '3.10.1' },
-          },
-          'node_modules/top-of-tree/node_modules/lodash': {
-            version: '3.10.1',
-            resolved: 'https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz',
-            hasInstallScript: true,
-          },
-        },
+        resolved: 'https://registry.npmjs.org/top-of-tree/-/top-of-tree-1.0.0.tgz',
+        dependencies: { lodash: '3.10.1' },
+      },
+      'node_modules/top-of-tree/node_modules/lodash': {
+        version: '3.10.1',
+        resolved: 'https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz',
+        hasInstallScript: true,
+      },
+    },
+  }),
+  node_modules: {
+    lodash: {
+      'package.json': JSON.stringify({
+        name: 'lodash',
+        version: '4.17.21',
+        scripts: { install: 'echo install' },
       }),
+    },
+    'top-of-tree': {
+      'package.json': JSON.stringify({ name: 'top-of-tree', version: '1.0.0' }),
       node_modules: {
         lodash: {
           'package.json': JSON.stringify({
             name: 'lodash',
-            version: '4.17.21',
+            version: '3.10.1',
             scripts: { install: 'echo install' },
           }),
         },
-        'top-of-tree': {
-          'package.json': JSON.stringify({ name: 'top-of-tree', version: '1.0.0' }),
-          node_modules: {
-            lodash: {
-              'package.json': JSON.stringify({
-                name: 'lodash',
-                version: '3.10.1',
-                scripts: { install: 'echo install' },
-              }),
-            },
-          },
-        },
       },
     },
+  },
+}
+
+t.test('approve-scripts --pending --json groups multiple versions under one name', async t => {
+  // Two versions of lodash are unreviewed; pendingSummary must collapse
+  // them into a single `lodash` entry (hits the `groups.has(display)`
+  // truthy branch on the second node).
+  const { npm, joinedOutput } = await _mockNpm(t, {
+    prefixDir: twoVersionFixture,
+    config: { 'allow-scripts-pending': true, json: true },
+  })
+  await npm.exec('approve-scripts', [])
+  const parsed = JSON.parse(joinedOutput())
+  t.strictSame(parsed.allowScripts.map((e) => e.name), ['lodash'])
+  t.strictSame(parsed.allowScripts[0].changes.map((c) => c.key).sort(), [
+    'lodash@3.10.1',
+    'lodash@4.17.21',
+  ])
+  t.ok(parsed.allowScripts[0].changes.every((c) => c.change === 'pending'))
+})
+
+t.test('approve-scripts groups multiple installed versions of the same package', async t => {
+  // Two versions of lodash exist in the tree; both have install scripts.
+  // groupByPackage should put them in the same group (hits the
+  // `if (!groups[key])` falsy branch on the second node).
+  const { npm, prefix } = await _mockNpm(t, {
+    prefixDir: twoVersionFixture,
   })
   await npm.exec('approve-scripts', ['lodash'])
 
@@ -328,6 +393,37 @@ t.test('approve-scripts --pending handles node with no version', async t => {
   t.match(mockSync.joinedOutput(), / no-version-pkg \(install: do-stuff\)/)
 })
 
+t.test('approve-scripts --pending --json handles node with no version', async t => {
+  // Exercise pendingSummary's `version ? ... : display` falsy branch: the
+  // key is the bare name when the node has no version field.
+  const { npm, joinedOutput } = await _mockNpm(t, {
+    prefixDir: {
+      'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }),
+      'package-lock.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        lockfileVersion: 3,
+        requires: true,
+        packages: { '': { name: 'host', version: '1.0.0' } },
+      }),
+      node_modules: {},
+    },
+    config: { 'allow-scripts-pending': true, json: true },
+    mocks: {
+      '{LIB}/utils/check-allow-scripts.js': async () => [{
+        node: { packageName: 'no-version-pkg', name: 'no-version-pkg', version: undefined },
+        scripts: { install: 'do-stuff' },
+      }],
+    },
+  })
+  await npm.exec('approve-scripts', [])
+  t.strictSame(JSON.parse(joinedOutput()), {
+    allowScripts: [
+      { name: 'no-version-pkg', changes: [{ key: 'no-version-pkg', change: 'pending' }] },
+    ],
+  })
+})
+
 t.test('forbidden semver range in package.json#allowScripts is dropped with a warning', async t => {
   // End-to-end: project declares a caret range in allowScripts. The
   // resolver must drop the entry, emit a warning, and the matching node
diff --git a/test/lib/commands/deny-scripts.js b/test/lib/commands/deny-scripts.js
@@ -130,6 +130,25 @@ t.test('deny-scripts --all with no unreviewed packages prints message', async t
   t.match(joinedOutput(), /No packages with unreviewed install scripts/)
 })
 
+t.test('deny-scripts --all --json with no unreviewed emits empty list', async t => {
+  const { npm, joinedOutput } = await _mockNpm(t, {
+    prefixDir: {
+      'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }),
+      'package-lock.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        lockfileVersion: 3,
+        requires: true,
+        packages: { '': { name: 'host', version: '1.0.0' } },
+      }),
+      node_modules: {},
+    },
+    config: { all: true, json: true },
+  })
+  await npm.exec('deny-scripts', [])
+  t.strictSame(JSON.parse(joinedOutput()), { allowScripts: [] })
+})
+
 t.test('deny-scripts fails on global', async t => {
   const { npm } = await _mockNpm(t, {
     config: { global: true },
