From 8e9307d6ecadd1e1e764c7096198050a43dab4d0 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Tue, 9 Jun 2026 14:00:34 -0700 Subject: [PATCH] fix(arborist): enforce allowScripts for file:/link: dep scripts --- workspaces/arborist/lib/arborist/rebuild.js | 18 +++-- workspaces/arborist/test/arborist/rebuild.js | 83 ++++++++++++++++++++ workspaces/arborist/test/arborist/reify.js | 2 +- 3 files changed, 95 insertions(+), 8 deletions(-) diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js index b4a66a0af7c09..994cac843acac 100644 --- a/workspaces/arborist/lib/arborist/rebuild.js +++ b/workspaces/arborist/lib/arborist/rebuild.js @@ -199,16 +199,20 @@ 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. + // allowScripts gate (RFC npm/rfcs#868): `true` runs lifecycle + // scripts; `false` and `null` (unreviewed) block. Bypassed by + // --dangerously-allow-all-scripts and workspaces (owner-managed). + // --ignore-scripts still wins (in #build); bins are never gated. + // + // Checked on node.target, not the Link: a Link's `resolved` is + // node_modules-relative (`file:../../dep`) so it can't match a + // project-root-relative policy key; the target carries the realpath + // and link specs that script-allowed.js matches on (npm/cli#9498). + // For non-links node.target === node, so registry deps are unaffected. const scriptsAllowed = this.options.dangerouslyAllowAllScripts || - node.isLink || node.isWorkspace || - isScriptAllowed(node, this.options.allowScripts) === true + isScriptAllowed(node.target, this.options.allowScripts) === true for (const [key, has] of Object.entries(tests)) { if (!has) { continue diff --git a/workspaces/arborist/test/arborist/rebuild.js b/workspaces/arborist/test/arborist/rebuild.js index 2eaf6bccabe69..ab433f2f97174 100644 --- a/workspaces/arborist/test/arborist/rebuild.js +++ b/workspaces/arborist/test/arborist/rebuild.js @@ -159,6 +159,89 @@ t.test('dangerouslyAllowAllScripts bypasses the deny gate', async t => { ) }) +t.test('allowScripts gates local file: dep scripts (npm/cli#9498)', async t => { + const aPrepare = p => resolve(p, 'a/a-prepare') + const aPostinstall = p => resolve(p, 'a/a-post-install') + + t.test('true: scripts run when the target is allowed', async t => { + const path = fixture(t, 'link-dep-lifecycle-scripts') + const arb = newArb({ + path, + allowScripts: { 'file:../a': true }, + dangerouslyAllowAllScripts: false, + }) + await arb.rebuild() + t.equal(fs.statSync(aPrepare(path)).isFile(), true, 'prepare ran') + t.equal(fs.statSync(aPostinstall(path)).isFile(), true, 'postinstall ran') + }) + + t.test('false: deny entry blocks the scripts', async t => { + const path = fixture(t, 'link-dep-lifecycle-scripts') + const arb = newArb({ + path, + allowScripts: { 'file:../a': false }, + dangerouslyAllowAllScripts: false, + }) + await arb.rebuild() + t.throws(() => fs.statSync(aPrepare(path)), 'prepare did not run') + t.throws(() => fs.statSync(aPostinstall(path)), 'postinstall did not run') + }) + + t.test('absent: default-deny blocks the scripts', async t => { + const path = fixture(t, 'link-dep-lifecycle-scripts') + const arb = newArb({ + path, + allowScripts: {}, + dangerouslyAllowAllScripts: false, + }) + await arb.rebuild() + t.throws(() => fs.statSync(aPrepare(path)), 'prepare did not run') + t.throws(() => fs.statSync(aPostinstall(path)), 'postinstall did not run') + }) + + t.test('dangerouslyAllowAllScripts bypasses the gate for file: deps', async t => { + const path = fixture(t, 'link-dep-lifecycle-scripts') + const arb = newArb({ path, dangerouslyAllowAllScripts: true }) + await arb.rebuild() + t.equal(fs.statSync(aPrepare(path)).isFile(), true, 'prepare ran') + t.equal(fs.statSync(aPostinstall(path)).isFile(), true, 'postinstall ran') + }) +}) + +t.test('workspaces bypass the allowScripts gate (owner-managed)', async t => { + // Workspaces are owner-managed, so their scripts run regardless of the + // allowScripts policy. This must survive the #9498 fix that stopped + // bypassing all link nodes. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'ws-root', + version: '1.0.0', + workspaces: ['ws'], + }), + node_modules: { + ws: t.fixture('symlink', '../ws'), + }, + ws: { + 'package.json': JSON.stringify({ + name: 'ws', + version: '1.0.0', + scripts: { + prepare: `node -e "require('fs').writeFileSync('ws-prepare', '')"`, + }, + }), + }, + }) + // No allowScripts entry and the escape hatch off: only the isWorkspace + // bypass can let this script through. + const arb = newArb({ path, dangerouslyAllowAllScripts: false }) + await arb.rebuild() + t.equal( + fs.statSync(resolve(path, 'ws/ws-prepare')).isFile(), + true, + 'workspace prepare ran despite no allowScripts entry' + ) +}) + t.test('do nothing if ignoreScripts=true and binLinks=false', async t => { const path = fixture(t, 'testing-rebuild-bundle-reified') const file = resolve(path, 'node_modules/@isaacs/testing-rebuild-bundle-a/node_modules/@isaacs/testing-rebuild-bundle-b/cwd') diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 370124e3e645b..8bcaf040102a4 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -2080,7 +2080,7 @@ console.log('ok 1 - this is fine') t.test('running lifecycle scripts of unchanged link nodes on reify', async t => { const path = fixture(t, 'link-dep-lifecycle-scripts') createRegistry(t, false) - t.matchSnapshot(await printReified(path), 'result') + t.matchSnapshot(await printReified(path, { allowScripts: { 'file:../a': true } }), 'result') t.ok(fs.lstatSync(resolve(path, 'a/a-prepare')).isFile(), 'should run prepare lifecycle scripts for links directly linked to the tree')