From 930aba64671524a4123cfa1140f178c2ce14c40a Mon Sep 17 00:00:00 2001 From: Sanjays2402 <51058514+Sanjays2402@users.noreply.github.com> Date: Sat, 27 Jun 2026 14:16:36 -0700 Subject: [PATCH] fix(arborist): preserve bin links when allowScripts denies a node (#9681) A node denied via `allowScripts: { "": false }` was dropped from the build set entirely, so its `node_modules/.bin/` symlinks were never created. This stranded any package that declared both an install script and a bin (e.g. esbuild, nx): the binary lived under `node_modules/` but nothing put it on PATH, breaking downstream build steps with `command not found`. Move the deny check out of the early-return in `#addToBuildSet` and into the per-queue dispatch, mirroring the v12 fix. Lifecycle scripts (preinstall/install/postinstall/prepare) for denied nodes are still skipped; only bin linking continues, matching the semantics of `--ignore-scripts`. Regression test added in test/arborist/rebuild.js: with `allowScripts: { 'denied-with-bin': false }` the postinstall does not run but `.bin/denied-with-bin` is still created. --- workspaces/arborist/lib/arborist/rebuild.js | 34 +++++++------ workspaces/arborist/test/arborist/rebuild.js | 23 +++++++++ .../testing-allow-scripts-deny-bin.js | 48 +++++++++++++++++++ 3 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 workspaces/arborist/test/fixtures/reify-cases/testing-allow-scripts-deny-bin.js diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js index 49b70f72ae534..c56c98ab0e19b 100644 --- a/workspaces/arborist/lib/arborist/rebuild.js +++ b/workspaces/arborist/lib/arborist/rebuild.js @@ -199,10 +199,28 @@ 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 (#9681): a `false` verdict from the policy + // matcher means the user explicitly denied install scripts for this + // node, so skip its lifecycle scripts. Bin linking is intentionally + // not gated — a denied package must still get its `.bin/` symlinks, + // matching the semantics of `--ignore-scripts`. + // `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 the gate + // entirely. + const scriptsDenied = + !this.options.dangerouslyAllowAllScripts && + isScriptAllowed(node, this.options.allowScripts) === false for (const [key, has] of Object.entries(tests)) { - if (has) { - this.#queues[key].push(node) + if (!has) { + continue } + if (key !== 'bin' && scriptsDenied) { + continue + } + this.#queues[key].push(node) } } timeEnd() @@ -226,18 +244,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 index df2eac9eec25b..b19213ceeb684 100644 --- a/workspaces/arborist/test/arborist/rebuild.js +++ b/workspaces/arborist/test/arborist/rebuild.js @@ -163,6 +163,29 @@ t.test('do nothing if ignoreScripts=true and binLinks=false', async t => { t.throws(() => fs.statSync(file), 'bundle build script not run') }) +t.test('allowScripts deny still links bins for that node', async t => { + // Regression for #9681: a `false` verdict from allowScripts must skip + // that node's install scripts but must NOT drop its bin link. Behaves + // like --ignore-scripts (scripts off, bins still linked). + const path = fixture(t, 'testing-allow-scripts-deny-bin') + const arb = newArb({ + path, + allowScripts: { 'denied-with-bin': false }, + }) + await arb.rebuild() + // The denied package's postinstall must NOT have created its sentinel. + t.throws( + () => fs.statSync(resolve(path, 'node_modules/denied-with-bin/ran')), + 'denied postinstall did not run' + ) + // The bin link must still exist. + t.equal( + fs.lstatSync(resolve(path, 'node_modules/.bin/denied-with-bin')).isSymbolicLink(), + true, + 'bin link for denied package still created' + ) +}) + t.test('do not link bins for nodes on trash list', async t => { const path = fixture(t, 'dep-installed-without-bin-link') const semver = resolve(path, 'node_modules/.bin/semver') diff --git a/workspaces/arborist/test/fixtures/reify-cases/testing-allow-scripts-deny-bin.js b/workspaces/arborist/test/fixtures/reify-cases/testing-allow-scripts-deny-bin.js new file mode 100644 index 0000000000000..e381a5955768a --- /dev/null +++ b/workspaces/arborist/test/fixtures/reify-cases/testing-allow-scripts-deny-bin.js @@ -0,0 +1,48 @@ +// Fixture for #9681: a node with `bin` denied by allowScripts should still +// get its `.bin/` symlink created, while its install scripts are skipped. +module.exports = t => { + const path = t.testdir({ + 'node_modules': { + 'denied-with-bin': { + 'package.json': JSON.stringify({ + name: 'denied-with-bin', + version: '1.0.0', + bin: { 'denied-with-bin': 'bin/cli.js' }, + scripts: { + postinstall: 'node -e "require(\'fs\').writeFileSync(\'ran\', \'\')"', + }, + }), + 'bin': { + 'cli.js': '#!/usr/bin/env node\nconsole.log("denied-with-bin")\n', + }, + }, + }, + 'package-lock.json': JSON.stringify({ + name: 'testing-allow-scripts-deny-bin', + version: '1.0.0', + lockfileVersion: 2, + requires: true, + packages: { + '': { + name: 'testing-allow-scripts-deny-bin', + version: '1.0.0', + dependencies: { 'denied-with-bin': '1.0.0' }, + }, + 'node_modules/denied-with-bin': { + version: '1.0.0', + hasInstallScript: true, + bin: { 'denied-with-bin': 'bin/cli.js' }, + }, + }, + dependencies: { + 'denied-with-bin': { version: '1.0.0' }, + }, + }), + 'package.json': JSON.stringify({ + name: 'testing-allow-scripts-deny-bin', + version: '1.0.0', + dependencies: { 'denied-with-bin': '1.0.0' }, + }), + }) + return path +}