diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 4f3f886e8997a..cce3aaea83190 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -25,6 +25,7 @@ const debug = require('../debug.js') const fromPath = require('../from-path.js') const calcDepFlags = require('../calc-dep-flags.js') const { isReleaseAgeExcluded, trustedSpecName } = require('../release-age-exclude.js') +const { isScriptAllowed } = require('../script-allowed.js') const { resolvePatchedDependencies } = require('../patched-dependencies.js') const PackageExtensions = require('../package-extensions.js') const NpmExtension = require('../npm-extension.js') @@ -1028,6 +1029,12 @@ This is a one-time fix-up, please be patient... Arborist, resolved: node.resolved, integrity: node.integrity, + // Cracking open a git dep's tarball makes pacote run its `prepare` + // script, so the allowScripts gate applies here the same as it does + // when reify extracts the node for real. + ignoreScripts: this.options.ignoreScripts || + !(this.options.dangerouslyAllowAllScripts || + isScriptAllowed(node, this.options.allowScripts) === true), }) await new Arborist({ ...this.options, path }) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index b099d4d72c6a4..6ee2afea7070b 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -19,6 +19,7 @@ const { walkUp } = require('walk-up-path') const AuditReport = require('../audit-report.js') const Diff = require('../diff.js') +const { isScriptAllowed } = require('../script-allowed.js') const calcDepFlags = require('../calc-dep-flags.js') const debug = require('../debug.js') const onExit = require('../signal-handling.js') @@ -747,6 +748,12 @@ module.exports = cls => class Reifier extends cls { _isRoot: node.isRootDependency || [...node.edgesIn].some(e => e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace) ), + // pacote runs `prepare` for git and directory deps while extracting, before the + // node ever reaches the rebuild queues where the allowScripts gate is applied, + // so the policy has to be enforced here too. Both fetchers honor ignoreScripts. + ignoreScripts: this.options.ignoreScripts || + !(this.options.dangerouslyAllowAllScripts || + isScriptAllowed(node, this.options.allowScripts) === true), // pacote's npa re-parses our `name@URL` spec as type=remote, so allowRemote would mis-fire on registry tarballs. // Override only when we can prove the URL is registry-mediated; see #isRegistryResolvedTarball. ...(this.#isRegistryResolvedTarball(node) ? { allowRemote: 'all' } : {}), diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 943e340b22c1f..87f148a3b7259 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -2156,6 +2156,54 @@ t.test('running lifecycle scripts of unchanged link nodes on reify', async t => 'should run postinstall lifecycle scripts for links directly linked to the tree') }) +t.test('allowScripts gates the scripts pacote runs while extracting', async t => { + const Pacote = require('pacote') + + // pacote runs `prepare` for git and directory deps as part of extract, so the + // policy has to reach it through ignoreScripts rather than the rebuild queues. + const extractIgnoreScripts = async (t, opt) => { + const seen = [] + const MockedArborist = t.mock('../../lib/arborist', { + pacote: { + ...Pacote, + extract: async (spec, dest, extractOpt) => { + seen.push(extractOpt.ignoreScripts) + return Pacote.extract(spec, dest, extractOpt) + }, + }, + }) + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + dependencies: { abbrev: '1.1.1' }, + }), + }) + createRegistry(t, true) + await new MockedArborist({ audit: false, cache: path, path, ...opt }).reify() + return seen + } + + t.test('unreviewed dep extracts with scripts off', async t => { + t.strictSame(await extractIgnoreScripts(t, { allowScripts: {} }), [true]) + }) + + t.test('approved dep extracts with scripts on', async t => { + t.strictSame(await extractIgnoreScripts(t, { allowScripts: { abbrev: true } }), [false]) + }) + + t.test('dangerouslyAllowAllScripts extracts with scripts on', async t => { + t.strictSame(await extractIgnoreScripts(t, { dangerouslyAllowAllScripts: true }), [false]) + }) + + t.test('ignoreScripts still wins', async t => { + t.strictSame(await extractIgnoreScripts(t, { + ignoreScripts: true, + dangerouslyAllowAllScripts: true, + }), [true]) + }) +}) + t.test('save-prod, with optional', async t => { const path = t.testdir({ 'package.json': JSON.stringify({