From 765582219fee6ca599ad65681c7b63168801aa15 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Tue, 23 Jun 2026 08:36:43 -0500 Subject: [PATCH] fix(arborist): don't flag inert optional deps in strict-allow-scripts (#9597) Fixes #9562. `strict-allow-scripts` walked the ideal tree and rejected inert optional deps like `fsevents` on Linux, even though reify removes them before any install script runs. The shared `collectUnreviewedScripts` walk now skips inert nodes, so the strict check matches what actually installs. `approve-scripts` (actual tree) was already correct. --- tap-snapshots/test/lib/docs.js.test.cjs | 4 ++++ .../utils/strict-allow-scripts-preflight.js | 13 ++++++++++++ workspaces/arborist/lib/unreviewed-scripts.js | 7 +++++++ .../arborist/test/unreviewed-scripts.js | 10 ++++++++++ .../config/lib/definitions/definitions.js | 4 ++++ .../test/strict-allow-scripts-preflight.js | 20 +++++++++++++++++++ 6 files changed, 58 insertions(+) diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index c28955615d600..40aafd3e0ee9a 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -2062,6 +2062,10 @@ with install scripts that are neither approved nor denied). \`--ignore-scripts\` and \`--dangerously-allow-all-scripts\` both override this setting. +Optional dependencies that cannot be installed on the current platform or +engine (a non-matching \`os\`, \`cpu\`, or \`libc\`) are not flagged, because +their install scripts never run. + #### \`strict-peer-deps\` diff --git a/test/lib/utils/strict-allow-scripts-preflight.js b/test/lib/utils/strict-allow-scripts-preflight.js index e0c54105f8863..e85a31f2c1a39 100644 --- a/test/lib/utils/strict-allow-scripts-preflight.js +++ b/test/lib/utils/strict-allow-scripts-preflight.js @@ -76,6 +76,19 @@ t.test('throws when unreviewed install scripts exist (idealTree path)', async t ) }) +t.test('passes when the only unreviewed node is inert (platform-incompatible optional dep)', async t => { + // An inert dep is in the ideal tree but removed before any script runs, so + // strict mode must not reject it (npm/cli#9562). + const inertNode = { ...node({ name: 'fsevents' }), inert: true } + const arb = makeArb({ ideal: tree([inertNode]) }) + await preflight({ + arb, + npm: { flatOptions: { strictAllowScripts: true } }, + idealTreeOpts: {}, + }) + t.pass('no error thrown for inert node') +}) + t.test('passes when all install-script nodes are explicitly approved', async t => { const arb = makeArb({ ideal: tree([node({ name: 'canvas' })]), diff --git a/workspaces/arborist/lib/unreviewed-scripts.js b/workspaces/arborist/lib/unreviewed-scripts.js index 587d06bf8fe56..f7bed387845ec 100644 --- a/workspaces/arborist/lib/unreviewed-scripts.js +++ b/workspaces/arborist/lib/unreviewed-scripts.js @@ -50,6 +50,13 @@ const collectUnreviewedScripts = async ({ // its own lifecycle scripts. continue } + if (node.inert) { + // Inert = an optional dep that can't be installed here (failed the + // os/cpu/libc or engine check, or failed to load). reify drops it + // before any script runs, so its install scripts never execute and it + // must not be flagged (npm/cli#9562). + continue + } const verdict = isScriptAllowed(node, resolvedPolicy) if (verdict === true || verdict === false) { diff --git a/workspaces/arborist/test/unreviewed-scripts.js b/workspaces/arborist/test/unreviewed-scripts.js index 7f26f56db6875..34e4878b4625a 100644 --- a/workspaces/arborist/test/unreviewed-scripts.js +++ b/workspaces/arborist/test/unreviewed-scripts.js @@ -28,6 +28,7 @@ const node = ({ isWorkspace = false, isLink = false, inBundle = false, + inert = false, resolved, } = {}) => ({ name, @@ -39,6 +40,7 @@ const node = ({ isWorkspace, isLink, inBundle, + inert, isRegistryDependency: true, package: { name, version, scripts }, }) @@ -82,6 +84,14 @@ t.test('collectUnreviewedScripts', async t => { t.strictSame(result, []) }) + t.test('skips inert (platform/engine-incompatible) optional nodes', async t => { + const result = await collectUnreviewedScripts({ + tree: tree([node({ name: 'fsevents', scripts: { install: 'x' }, inert: true })]), + policy: null, + }) + t.strictSame(result, []) + }) + t.test('skips nodes with no install-relevant scripts', async t => { const result = await collectUnreviewedScripts({ tree: tree([node({ scripts: { test: 'jest' } })]), diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index ecb491cb32f62..623dd6e149d1c 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -2484,6 +2484,10 @@ const definitions = { (packages with install scripts that are neither approved nor denied). \`--ignore-scripts\` and \`--dangerously-allow-all-scripts\` both override this setting. + + Optional dependencies that cannot be installed on the current platform + or engine (a non-matching \`os\`, \`cpu\`, or \`libc\`) are not flagged, + because their install scripts never run. `, flatten, }), diff --git a/workspaces/libnpmexec/test/strict-allow-scripts-preflight.js b/workspaces/libnpmexec/test/strict-allow-scripts-preflight.js index 1d6795703ed31..5fc803c26c8d1 100644 --- a/workspaces/libnpmexec/test/strict-allow-scripts-preflight.js +++ b/workspaces/libnpmexec/test/strict-allow-scripts-preflight.js @@ -64,6 +64,26 @@ t.test('throws when unreviewed scripts are present', async t => { ) }) +t.test('resolves when the only unreviewed node is inert', async t => { + // Inert deps (platform/engine-incompatible) are removed before any script + // runs, so strict mode must not reject them (npm/cli#9562). + const inertTree = { + inventory: new Map([ + ['node_modules/has-scripts', { + name: 'has-scripts', + location: 'node_modules/has-scripts', + inert: true, + package: { scripts: { install: 'node-gyp rebuild' } }, + }], + ]), + } + const arb = fakeArb(inertTree) + await t.resolves( + strictAllowScriptsPreflight(arb, { strictAllowScripts: true }), + 'no error when the unreviewed node is inert' + ) +}) + t.test('resolves when no unreviewed scripts are present', async t => { const cleanTree = { inventory: new Map([