Skip to content
Merged
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
27 changes: 13 additions & 14 deletions workspaces/arborist/lib/arborist/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,21 @@ 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 }
// A denied package (allowScripts resolves to `false`) still gets its
// bins linked; only its lifecycle scripts are skipped, matching
// --ignore-scripts (npm/cli#9681). --dangerously-allow-all-scripts
// bypasses the gate.
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 +237,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
47 changes: 43 additions & 4 deletions workspaces/arborist/test/arborist/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,9 @@ t.test('do not run scripts if ignoreScripts=true', async t => {
t.throws(() => fs.statSync(file), 'bundle build script not run')
})

t.test('allowScripts deny entry skips the build set entry for that node', async t => {
// Verifies the deny gate in #addToBuildSet: when `allowScripts` resolves
// a node's policy to `false`, that node's scripts are skipped while
// others continue to run.
t.test('allowScripts deny entry skips scripts for the denied node', async t => {
// Deny gate in #buildQueues: a `false` policy skips that node's
// scripts while others still run.
const path = fixture(t, 'testing-rebuild-script-env-flags')
const arb = newArb({
path,
Expand Down Expand Up @@ -154,6 +153,46 @@ t.test('dangerouslyAllowAllScripts bypasses the deny gate', async t => {
)
})

t.test('allowScripts deny keeps the bin link (npm/cli#9681)', async t => {
// Denying a package's scripts must not drop its bin links: the script
// is skipped but `.bin/<name>` is still created, like --ignore-scripts.
const path = t.testdir({
'package.json': JSON.stringify({
name: 'project',
version: '1.0.0',
dependencies: { 'denied-dep': '1.0.0' },
}),
node_modules: {
'denied-dep': {
'package.json': JSON.stringify({
name: 'denied-dep',
version: '1.0.0',
bin: { 'denied-bin': 'index.js' },
scripts: {
postinstall: `node -e "require('fs').writeFileSync('ran', '1')"`,
},
}),
'index.js': '#!/usr/bin/env node\nconsole.log("hi")\n',
},
},
})
const arb = newArb({
path,
allowScripts: { 'denied-dep': false },
})
await arb.rebuild()

t.throws(
() => fs.statSync(resolve(path, 'node_modules/denied-dep/ran')),
'denied postinstall did not run'
)
t.equal(
fs.statSync(resolve(path, 'node_modules/.bin/denied-bin')).isFile(),
true,
'denied package bin link is still created'
)
})

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')
Expand Down
Loading