feat: default-deny install scripts (allowScripts opt-in) [v12]

JamieMagee · owlstronaut · commit 5cd5150d3e85 · 2026-06-03T14:20:43.000-07:00
Phase 2 of the RFC #868 install-script policy: flip the default so unreviewed lifecycle scripts are blocked unless covered by allowScripts. Stacked on the behavior-neutral tooling PR; this commit carries ONLY the v12-only default flip: - arborist: gate preinstall/install/postinstall/prepare in rebuild on the allowScripts policy (default-deny) - user-facing "blocked because not covered by allowScripts" wording in rebuild/reify-output/allow-scripts-cmd - config definition docs + approve/deny command docs + snapshots - flip tests
diff --git a/docs/lib/content/commands/npm-approve-scripts.md b/docs/lib/content/commands/npm-approve-scripts.md
@@ -15,9 +15,10 @@ records which of your dependencies are permitted to run install scripts
 (`preinstall`, `install`, `postinstall`, and `prepare` for non-registry
 sources). This command is the recommended way to maintain that field.
 
-In the current release, this field is advisory: install scripts still run
-by default, but installs print a list of packages whose scripts have not
-been reviewed. A future release will block unreviewed install scripts.
+Dependency install scripts are blocked by default. Install commands
+silently skip lifecycle scripts for any dependency that does not have a
+matching entry in `allowScripts`, and end with a list of the packages
+whose scripts were skipped so you can review them with this command.
 
 This command only works inside a project that has a `package.json`. It does
 not apply to global installs (`npm install -g`) or one-off executions
diff --git a/docs/lib/content/commands/npm-deny-scripts.md b/docs/lib/content/commands/npm-deny-scripts.md
@@ -15,10 +15,10 @@ Writes `false` entries into the `allowScripts` field of your project's
 `package.json`, recording that a dependency must not run install scripts
 even if a future version would otherwise be eligible.
 
-In the current release, install scripts still run by default, so `deny-scripts`
-only affects how installs of denied packages are reported. A future release
-will block unreviewed install scripts and respect deny entries at install
-time.
+Dependency install scripts are blocked by default. Adding a `false`
+entry with `deny-scripts` makes the denial explicit (so it survives
+`npm approve-scripts --all`) and excludes the package from any future
+`--allow-scripts-pending` review prompts.
 
 ```bash
 npm deny-scripts <pkg> [<pkg> ...]
diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js
@@ -68,13 +68,12 @@ class Rebuild extends ArboristWorkspaceCmd {
       await arb.rebuild()
     }
 
-    // Phase 1 advisory: list any packages whose install scripts ran (or
-    // would have run) and are not yet covered by allowScripts. Rebuild
-    // doesn't go through reifyFinish, so the walker is invoked here.
+    // Rebuild skips reifyFinish, so run the walker here to list any
+    // packages whose install scripts were blocked.
     const unreviewed = await checkAllowScripts({ arb, npm: this.npm })
     if (unreviewed.length > 0) {
       const count = unreviewed.length
-      const noun = count === 1 ? 'package has' : 'packages have'
+      const noun = count === 1 ? 'package had' : 'packages had'
       // `npm approve-scripts` writes to a project package.json, which doesn't
       // exist for global rebuilds. Point global users at `npm config set`,
       // which writes the `allow-scripts` setting to their user .npmrc.
@@ -84,7 +83,7 @@ class Rebuild extends ArboristWorkspaceCmd {
         : 'Run `npm approve-scripts --allow-scripts-pending` to review.'
       log.warn(
         'rebuild',
-        `${count} ${noun} install scripts not yet covered by allowScripts. ` +
+        `${count} ${noun} install scripts blocked because they are not covered by allowScripts. ` +
         remediation
       )
     }
diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js
@@ -87,7 +87,7 @@ class AllowScriptsCmd extends BaseCommand {
     const has = count === 1 ? 'has' : 'have'
     const pkg = count === 1 ? 'package' : 'packages'
     output.standard(
-      `${count} ${pkg} ${has} install scripts not yet covered by allowScripts:`
+      `${count} ${pkg} ${has} install scripts blocked because they are not covered by allowScripts:`
     )
     for (const { node, scripts } of unreviewed) {
       const { name, version } = trustedDisplay(node)
diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js
@@ -241,8 +241,9 @@ const unreviewedScriptsMessage = (npm, unreviewedScripts) => {
   // stdout is reserved for things the user explicitly asked to see
   // (npm ls, npm view).
   const count = unreviewedScripts.length
-  const pkg = count === 1 ? 'package has' : 'packages have'
-  const header = `${count} ${pkg} install scripts not yet covered by allowScripts:`
+  const pkg = count === 1 ? 'package had' : 'packages had'
+  const header =
+    `${count} ${pkg} install scripts blocked because they are not covered by allowScripts:`
 
   const names = []
   const lines = unreviewedScripts.map(({ node, scripts }) => {
diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs
@@ -107,7 +107,7 @@ npm error   --allow-scripts
 npm error     Comma-separated list of packages whose install-time lifecycle scripts
 npm error
 npm error   --strict-allow-scripts
-npm error     If \`true\`, turn the install-script policy from a warning into a hard
+npm error     If \`true\`, turn the install-script policy from a silent skip into a
 npm error
 npm error   --dangerously-allow-all-scripts
 npm error     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs
@@ -1912,12 +1912,13 @@ this to work properly.
 * Default: false
 * Type: Boolean
 
-If \`true\`, turn the install-script policy from a warning into a hard error:
-any dependency with install scripts not covered by \`allowScripts\` will fail
-the install instead of running with a notice.
+If \`true\`, turn the install-script policy from a silent skip into a hard
+error: any dependency with install scripts not covered by \`allowScripts\`
+will fail the install instead of being silently skipped.
 
-Dependencies explicitly denied with \`false\` in \`allowScripts\` are always
-silently skipped; this setting only affects unreviewed entries.
+By default, dependencies whose install scripts are not approved in
+\`allowScripts\` are silently skipped; this setting promotes that silent skip
+into a hard failure, which is the recommended posture for CI.
 \`--ignore-scripts\` and \`--dangerously-allow-all-scripts\` both override this
 setting.
 
@@ -3283,7 +3284,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -3845,7 +3846,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -4293,7 +4294,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -4443,7 +4444,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -4589,7 +4590,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -5574,7 +5575,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
@@ -6392,7 +6393,7 @@ Options:
     Comma-separated list of packages whose install-time lifecycle scripts
 
   --strict-allow-scripts
-    If \`true\`, turn the install-script policy from a warning into a hard
+    If \`true\`, turn the install-script policy from a silent skip into a
 
   --dangerously-allow-all-scripts
     If \`true\`, bypass the \`allowScripts\` policy entirely and run every
diff --git a/test/lib/commands/approve-scripts.js b/test/lib/commands/approve-scripts.js
@@ -55,7 +55,7 @@ t.test('approve-scripts --pending lists unreviewed packages', async t => {
   })
   await npm.exec('approve-scripts', [])
   const out = joinedOutput()
-  t.match(out, /2 packages have install scripts not yet covered/)
+  t.match(out, /2 packages have install scripts blocked because they are not covered by allowScripts/)
   t.match(out, /canvas@1\.0\.0/)
   t.match(out, /sharp@1\.0\.0/)
 })
@@ -67,7 +67,7 @@ t.test('approve-scripts --pending lists unreviewed packages even with ignore-scr
   })
   await npm.exec('approve-scripts', [])
   const out = joinedOutput()
-  t.match(out, /2 packages have install scripts not yet covered/)
+  t.match(out, /2 packages have install scripts blocked because they are not covered by allowScripts/)
   t.match(out, /canvas@1\.0\.0/)
   t.match(out, /sharp@1\.0\.0/)
 })
diff --git a/test/lib/commands/edit.js b/test/lib/commands/edit.js
@@ -22,6 +22,11 @@ const spawk = tspawk(t)
 const npmConfig = {
   config: {
     'ignore-scripts': false,
+    // Phase 2 gates dependency install scripts by default. `npm edit`
+    // rebuilds the edited package via `npm rebuild`, which honors the
+    // allowScripts gate, so opt every script in for these tests to exercise
+    // the editor -> rebuild -> install-script flow.
+    'dangerously-allow-all-scripts': true,
     editor: 'testeditor',
     'script-shell': process.platform === 'win32' ? process.env.COMSPEC : 'sh',
   },
diff --git a/test/lib/commands/rebuild.js b/test/lib/commands/rebuild.js
@@ -222,7 +222,7 @@ t.test('completion', async t => {
   t.type(res, Array)
 })
 
-t.test('emits Phase 1 advisory warning for unreviewed install scripts', async t => {
+t.test('emits blocked warning for unreviewed install scripts', async t => {
   const { npm, logs } = await setupMockNpm(t, {
     prefixDir: {
       'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }),
@@ -240,7 +240,7 @@ t.test('emits Phase 1 advisory warning for unreviewed install scripts', async t
   await npm.exec('rebuild', [])
   t.match(
     logs.warn.byTitle('rebuild'),
-    [/install scripts not yet covered by allowScripts/]
+    [/install scripts blocked because they are not covered by allowScripts/]
   )
 })
 
@@ -264,7 +264,7 @@ t.test('global advisory warning points at npm config set, not approve-scripts',
   })
   await npm.exec('rebuild', [])
   const warn = logs.warn.byTitle('rebuild').join('\n')
-  t.match(warn, /install scripts not yet covered by allowScripts/)
+  t.match(warn, /install scripts blocked because they are not covered by allowScripts/)
   t.match(warn, /npm config set allow-scripts=canvas/)
   t.notMatch(warn, /approve-scripts/)
 })
@@ -307,6 +307,54 @@ t.test('no advisory warning when allowScripts covers the package', async t => {
   t.strictSame(logs.warn.byTitle('rebuild'), [])
 })
 
+t.test('rebuild <pkg> honors the gate for an unreviewed package', async t => {
+  const { npm, logs, prefix: path } = await setupMockNpm(t, {
+    prefixDir: {
+      'package.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        dependencies: { canvas: '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', dependencies: { canvas: '1.0.0' } },
+          'node_modules/canvas': {
+            version: '1.0.0',
+            resolved: 'https://registry.npmjs.org/canvas/-/canvas-1.0.0.tgz',
+            hasInstallScript: true,
+          },
+        },
+      }),
+      node_modules: {
+        canvas: {
+          'package.json': JSON.stringify({
+            name: 'canvas',
+            version: '1.0.0',
+            scripts: {
+              install: "node -e \"require('fs').writeFileSync('ran', '')\"",
+            },
+          }),
+        },
+      },
+    },
+  })
+
+  const ranFile = resolve(path, 'node_modules/canvas/ran')
+  t.throws(() => fs.statSync(ranFile))
+
+  await npm.exec('rebuild', ['canvas'])
+
+  t.throws(() => fs.statSync(ranFile), 'unreviewed install script must not run')
+  t.match(
+    logs.warn.byTitle('rebuild'),
+    [/install scripts blocked because they are not covered by allowScripts/]
+  )
+})
+
 t.test('rebuild <name> never targets a bundled dependency', async t => {
   const { npm, prefix: path } = await setupMockNpm(t, {
     prefixDir: {
diff --git a/test/lib/utils/reify-output.js b/test/lib/utils/reify-output.js
@@ -481,7 +481,7 @@ t.test('prints unreviewed install scripts summary', async t => {
 
   const mock = await mockReifyWithExtras(t, baseReify, { unreviewedScripts })
   const warn = mock.logs.warn.byTitle('allow-scripts').join('\n')
-  t.match(warn, /2 packages have install scripts not yet covered/)
+  t.match(warn, /2 packages had install scripts blocked because they are not covered by allowScripts/)
   t.match(warn, /canvas@2\.11\.0 \(install: node-gyp rebuild\)/)
   t.match(warn, /sharp@0\.33\.2 \(preinstall: pre; postinstall: post\)/)
   t.match(warn, /npm approve-scripts --allow-scripts-pending/)
@@ -513,7 +513,7 @@ t.test('global install suggests --allow-scripts, not approve-scripts', async t =
 
   const mock = await mockReifyWithExtras(t, baseReify, { unreviewedScripts }, { global: true })
   const warn = mock.logs.warn.byTitle('allow-scripts').join('\n')
-  t.match(warn, /2 packages have install scripts not yet covered/)
+  t.match(warn, /2 packages had install scripts blocked because they are not covered by allowScripts/)
   t.match(warn, /canvas@2\.11\.0 \(install: node-gyp rebuild\)/)
   t.match(warn, /npm install -g --allow-scripts=canvas,sharp/)
   t.match(warn, /npm config set allow-scripts=canvas,sharp/)
@@ -538,7 +538,31 @@ t.test('single unreviewed script uses singular wording', async t => {
       }],
     }
   )
-  t.match(mock.logs.warn.byTitle('allow-scripts').join('\n'), /1 package has install scripts/)
+  t.match(mock.logs.warn.byTitle('allow-scripts').join('\n'), /1 package had install scripts blocked/)
+})
+
+t.test('optional dep with blocked scripts appears in the summary', async t => {
+  const mock = await mockNpm(t, {})
+  reifyOutput(mock.npm, {
+    actualTree: { inventory: { has: () => false } },
+    diff: { children: [] },
+  }, {
+    unreviewedScripts: [{
+      node: {
+        packageName: 'opt',
+        name: 'opt',
+        version: '1.0.0',
+        path: '/x/opt',
+        optional: true,
+        devOptional: true,
+      },
+      scripts: { install: 'cmd' },
+    }],
+  })
+  mock.npm.finish()
+  const warn = mock.logs.warn.byTitle('allow-scripts').join('\n')
+  t.match(warn, /1 package had install scripts blocked/)
+  t.match(warn, /opt@1\.0\.0 \(install: cmd\)/)
 })
 
 t.test('json output includes unreviewedScripts', async t => {
diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js
@@ -199,10 +199,24 @@ module.exports = cls => class Builder extends cls {
       const { package: { bin, scripts = {} } } = node.target
       const { preinstall, install, postinstall, prepare } = scripts
       const tests = { bin, preinstall, install, postinstall, prepare }
+      // allowScripts gate (RFC npm/rfcs#868). `true` lets lifecycle
+      // scripts run; `false` and `null` (unreviewed) both block.
+      // --ignore-scripts in #build() still wins. Bypasses:
+      // --dangerously-allow-all-scripts, links and workspaces (the
+      // owner is responsible). Bin linking is not gated.
+      const scriptsAllowed =
+        this.options.dangerouslyAllowAllScripts ||
+        node.isLink ||
+        node.isWorkspace ||
+        isScriptAllowed(node, this.options.allowScripts) === true
       for (const [key, has] of Object.entries(tests)) {
-        if (has) {
-          this.#queues[key].push(node)
+        if (!has) {
+          continue
         }
+        if (key !== 'bin' && !scriptsAllowed) {
+          continue
+        }
+        this.#queues[key].push(node)
       }
     }
     timeEnd()
@@ -226,18 +240,6 @@ module.exports = cls => class Builder extends cls {
       return
     }
 
-    // Phase 1 allowScripts gate: a `false` verdict from the policy matcher
-    // means the user explicitly denied install scripts for this node, so skip
-    // it. `true` and `null` (unreviewed) both fall through to the existing
-    // detection logic — unreviewed nodes still run their scripts in Phase 1
-    // and are surfaced via the post-reify advisory warning. The global
-    // --ignore-scripts kill switch in #build() still takes precedence, and
-    // --dangerously-allow-all-scripts bypasses this gate entirely.
-    if (!this.options.dangerouslyAllowAllScripts &&
-        isScriptAllowed(node, this.options.allowScripts) === false) {
-      return
-    }
-
     if (this.#oldMeta === null) {
       const { root: { meta } } = node
       this.#oldMeta = meta && meta.loadedFromDisk &&
diff --git a/workspaces/arborist/test/arborist/rebuild.js b/workspaces/arborist/test/arborist/rebuild.js
diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js
diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js
diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js

Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@ class AllowScriptsCmd extends BaseCommand {`
`87`	`87`	`const has = count === 1 ? 'has' : 'have'`
`88`	`88`	`const pkg = count === 1 ? 'package' : 'packages'`
`89`	`89`	`output.standard(`
`90`		- `${count} ${pkg} ${has} install scripts not yet covered by allowScripts:`
	`90`	+ `${count} ${pkg} ${has} install scripts blocked because they are not covered by allowScripts:`
`91`	`91`	`)`
`92`	`92`	`for (const { node, scripts } of unreviewed) {`
`93`	`93`	`const { name, version } = trustedDisplay(node)`