Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 20 additions & 14 deletions workspaces/arborist/lib/arborist/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 &&
Expand Down
23 changes: 23 additions & 0 deletions workspaces/arborist/test/arborist/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}