From bd2fb4c9fdaad74987f17c1429d9d6a8cab2b004 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:32:54 -0700 Subject: [PATCH 1/7] feat(config): add allow-scripts, strict-script-builds, dangerously-allow-all-scripts configs Three new configs to support the install-script opt-in policy. None of them affect install behaviour yet; they're read by approve-scripts, deny-scripts, and the install-time walker in later commits. - allow-scripts: comma-separated package list. Used as a fallback when the root package.json has no allowScripts field. Flattens to flatOptions.allowScripts. - strict-script-builds: boolean. Reserved for a future release that will turn blocked-script warnings into errors. No-op for now. - dangerously-allow-all-scripts: boolean escape hatch for that same future release. No-op for now. Refs: npm/rfcs#868 --- .../config/lib/definitions/definitions.js | 63 ++++++++++++++++ .../test/type-description.js.test.cjs | 10 +++ .../config/test/definitions/definitions.js | 74 +++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index 991d219a3f459..f493232f7d92b 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -247,6 +247,39 @@ const definitions = { `, flatten, }), + 'allow-scripts': new Definition('allow-scripts', { + default: '', + type: [String, Array], + hint: '', + description: ` + Comma-separated list of packages whose install-time lifecycle scripts + (\`preinstall\`, \`install\`, \`postinstall\`, and \`prepare\` for + non-registry dependencies) are allowed to run. Used as a fallback when + no \`allowScripts\` field is set in the root project's \`package.json\`, + and for global/npx contexts where no project \`package.json\` exists. + + Each name is matched against a dependency's resolved identity, not + against the package's self-reported name. CLI flags take precedence + over \`package.json\`, which takes precedence over this setting. + Layers are not merged. + + This setting is part of an opt-in install-script policy that will land + across multiple npm releases. In this release, install scripts still + run as they always have. Setting this field does not block anything; + it records your intent so the install command can list the packages + that would still need to be reviewed before the future release that + flips the default. + `, + flatten (key, obj, flatOptions) { + if (Array.isArray(obj[key])) { + flatOptions.allowScripts = obj[key] + } else if (obj[key]) { + flatOptions.allowScripts = obj[key].split(',').map(s => s.trim()).filter(Boolean) + } else { + flatOptions.allowScripts = [] + } + }, + }), also: new Definition('also', { default: null, type: [null, 'dev', 'development'], @@ -530,6 +563,22 @@ const definitions = { `, flatten, }), + 'dangerously-allow-all-scripts': new Definition('dangerously-allow-all-scripts', { + default: false, + type: Boolean, + description: ` + Reserved for a future release. When that release lands, setting this + to \`true\` will tell npm to run every dependency install script + regardless of the \`allowScripts\` policy — an escape hatch for + migration. Its use will be strongly discouraged. + + In this release, install scripts still run as they always have, so + this setting has no effect on install behaviour. The flag is + registered now so projects can pin it in their tooling ahead of the + release that flips the default. + `, + flatten, + }), depth: new Definition('depth', { default: null, defaultDescription: ` @@ -2214,6 +2263,20 @@ const definitions = { `, flatten, }), + 'strict-script-builds': new Definition('strict-script-builds', { + default: false, + type: Boolean, + description: ` + Reserved for a future release. When that release lands, setting this + to \`true\` will turn the install-script policy from a warning into a + hard error: any unreviewed install script will fail the install + instead of being skipped with a notice. + + In this release, install scripts still run as they always have, so + this setting has no effect on install behaviour. + `, + flatten, + }), 'strict-ssl': new Definition('strict-ssl', { default: true, type: Boolean, diff --git a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs index 78445376b9ef1..68e0ffc2dec94 100644 --- a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs +++ b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs @@ -42,6 +42,10 @@ Object { "allow-same-version": Array [ "boolean value (true or false)", ], + "allow-scripts": Array [ + Function String(), + Function Array(), + ], "also": Array [ null, "dev", @@ -118,6 +122,9 @@ Object { null, Function String(), ], + "dangerously-allow-all-scripts": Array [ + "boolean value (true or false)", + ], "depth": Array [ null, "numeric value", @@ -568,6 +575,9 @@ Object { "strict-peer-deps": Array [ "boolean value (true or false)", ], + "strict-script-builds": Array [ + "boolean value (true or false)", + ], "strict-ssl": Array [ "boolean value (true or false)", ], diff --git a/workspaces/config/test/definitions/definitions.js b/workspaces/config/test/definitions/definitions.js index aa282ea665500..17a8e04440614 100644 --- a/workspaces/config/test/definitions/definitions.js +++ b/workspaces/config/test/definitions/definitions.js @@ -1050,3 +1050,77 @@ t.test('node-gyp', t => { t.end() }) + +t.test('allow-scripts', t => { + t.test('defaults to empty string and flattens to []', t => { + const defs = mockDefs() + t.equal(defs['allow-scripts'].default, '') + const flat = {} + defs['allow-scripts'].flatten('allow-scripts', { 'allow-scripts': '' }, flat) + t.strictSame(flat, { allowScripts: [] }) + t.end() + }) + + t.test('parses comma-separated string into trimmed array', t => { + const flat = {} + mockDefs()['allow-scripts'].flatten( + 'allow-scripts', + { 'allow-scripts': 'canvas, sharp ,sqlite3' }, + flat + ) + t.strictSame(flat, { allowScripts: ['canvas', 'sharp', 'sqlite3'] }) + t.end() + }) + + t.test('drops empty entries', t => { + const flat = {} + mockDefs()['allow-scripts'].flatten( + 'allow-scripts', + { 'allow-scripts': ' canvas , , sharp ' }, + flat + ) + t.strictSame(flat, { allowScripts: ['canvas', 'sharp'] }) + t.end() + }) + + t.test('passes array values through unchanged', t => { + const flat = {} + mockDefs()['allow-scripts'].flatten( + 'allow-scripts', + { 'allow-scripts': ['canvas', 'sharp'] }, + flat + ) + t.strictSame(flat, { allowScripts: ['canvas', 'sharp'] }) + t.end() + }) + + t.end() +}) + +t.test('strict-script-builds', t => { + const defs = mockDefs() + t.equal(defs['strict-script-builds'].default, false) + t.equal(defs['strict-script-builds'].type, Boolean) + const flat = {} + defs['strict-script-builds'].flatten( + 'strict-script-builds', + { 'strict-script-builds': true }, + flat + ) + t.strictSame(flat, { strictScriptBuilds: true }) + t.end() +}) + +t.test('dangerously-allow-all-scripts', t => { + const defs = mockDefs() + t.equal(defs['dangerously-allow-all-scripts'].default, false) + t.equal(defs['dangerously-allow-all-scripts'].type, Boolean) + const flat = {} + defs['dangerously-allow-all-scripts'].flatten( + 'dangerously-allow-all-scripts', + { 'dangerously-allow-all-scripts': true }, + flat + ) + t.strictSame(flat, { dangerouslyAllowAllScripts: true }) + t.end() +}) From fd8c90b57598596fa55b4258c08f15f6f6066e4d Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:40:49 -0700 Subject: [PATCH 2/7] feat(install): read allowScripts policy from root package.json and CLI configs A precedence resolver reads the install-time allowScripts policy from the layered sources and threads it through install/ci into arborist. - lib/utils/resolve-allow-scripts.js: pure resolver. Reads from npm.prefix so workspace sub-installs still pick up the project root. Returns { policy, source }. Strict fallback: package.json wins over flat config; lower layers are silently ignored, with one warn when a lower setting is being suppressed. - install.js / ci.js: await the resolver before constructing arborist opts, then pass policy through opts.allowScripts. Add the three new params to each command's static params list. - workspaces/arborist/lib/arborist/index.js: accept options.allowScripts and store it on this.options. No enforcement yet; read in later commits. Also tightened the flatten function for the new allow-scripts config: nopt wraps single comma-separated strings in arrays for [String, Array] types, so each array entry needs splitting on commas before use. Refs: npm/rfcs#868 --- lib/commands/ci.js | 6 + lib/commands/exec.js | 12 + lib/commands/install.js | 6 + lib/commands/rebuild.js | 23 +- lib/commands/update.js | 6 + lib/utils/resolve-allow-scripts.js | 179 ++++++++++ .../test/lib/commands/config.js.test.cjs | 8 + tap-snapshots/test/lib/docs.js.test.cjs | 125 ++++++- test/lib/commands/exec.js | 65 ++++ test/lib/commands/rebuild.js | 60 ++++ test/lib/commands/update.js | 30 ++ test/lib/utils/resolve-allow-scripts.js | 313 ++++++++++++++++++ workspaces/arborist/lib/arborist/index.js | 1 + .../config/lib/definitions/definitions.js | 28 +- .../config/test/definitions/definitions.js | 13 +- 15 files changed, 863 insertions(+), 12 deletions(-) create mode 100644 lib/utils/resolve-allow-scripts.js create mode 100644 test/lib/utils/resolve-allow-scripts.js diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 05514b441068e..b2c9042fc0d8d 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -1,4 +1,5 @@ const reifyFinish = require('../utils/reify-finish.js') +const resolveAllowScripts = require('../utils/resolve-allow-scripts.js') const runScript = require('@npmcli/run-script') const fs = require('node:fs/promises') const path = require('node:path') @@ -25,6 +26,9 @@ class CI extends ArboristWorkspaceCmd { 'allow-file', 'allow-git', 'allow-remote', + 'allow-scripts', + 'strict-script-builds', + 'dangerously-allow-all-scripts', 'audit', 'bin-links', 'fund', @@ -43,12 +47,14 @@ class CI extends ArboristWorkspaceCmd { const ignoreScripts = this.npm.config.get('ignore-scripts') const where = this.npm.prefix const Arborist = require('@npmcli/arborist') + const { policy: allowScriptsPolicy } = await resolveAllowScripts(this.npm) const opts = { ...this.npm.flatOptions, packageLock: true, // npm ci should never skip lock files path: where, save: false, // npm ci should never modify the lockfile or package.json workspaces: this.workspaceNames, + allowScripts: allowScriptsPolicy, } // generate an inventory from the virtual tree in the lockfile diff --git a/lib/commands/exec.js b/lib/commands/exec.js index 5b1d117889a1e..00138db95a755 100644 --- a/lib/commands/exec.js +++ b/lib/commands/exec.js @@ -1,5 +1,6 @@ const { resolve } = require('node:path') const libexec = require('libnpmexec') +const resolveAllowScripts = require('../utils/resolve-allow-scripts.js') const BaseCommand = require('../base-cmd.js') class Exec extends BaseCommand { @@ -10,6 +11,9 @@ class Exec extends BaseCommand { 'workspace', 'workspaces', 'include-workspace-root', + 'allow-scripts', + 'strict-script-builds', + 'dangerously-allow-all-scripts', ] static name = 'exec' @@ -74,8 +78,16 @@ class Exec extends BaseCommand { throw this.usageError() } + // Resolve the install-script policy from the user/global .npmrc layer + // only. The RFC requires exec/npx to ignore any project + // package.json#allowScripts; CLI flags still apply. + const { policy: allowScriptsPolicy } = await resolveAllowScripts(this.npm, { + skipProjectConfig: true, + }) + return libexec({ ...flatOptions, + allowScripts: allowScriptsPolicy, // we explicitly set packageLockOnly to false because if it's true when we try to install a missing package, we won't actually install it packageLockOnly: false, // what the user asked to run args[0] is run by default diff --git a/lib/commands/install.js b/lib/commands/install.js index 287b585f13231..bdc36152eb832 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -5,6 +5,7 @@ const runScript = require('@npmcli/run-script') const pacote = require('pacote') const checks = require('npm-install-checks') const reifyFinish = require('../utils/reify-finish.js') +const resolveAllowScripts = require('../utils/resolve-allow-scripts.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Install extends ArboristWorkspaceCmd { @@ -31,6 +32,9 @@ class Install extends ArboristWorkspaceCmd { 'allow-file', 'allow-git', 'allow-remote', + 'allow-scripts', + 'strict-script-builds', + 'dangerously-allow-all-scripts', 'audit', 'before', 'min-release-age', @@ -138,12 +142,14 @@ class Install extends ArboristWorkspaceCmd { } const Arborist = require('@npmcli/arborist') + const { policy: allowScriptsPolicy } = await resolveAllowScripts(this.npm) const opts = { ...this.npm.flatOptions, auditLevel: null, path: where, add: args, workspaces: this.workspaceNames, + allowScripts: allowScriptsPolicy, } const arb = new Arborist(opts) await arb.reify(opts) diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js index a23df39f1560b..34c683bf64cda 100644 --- a/lib/commands/rebuild.js +++ b/lib/commands/rebuild.js @@ -1,8 +1,10 @@ const { resolve } = require('node:path') -const { output } = require('proc-log') +const { log, output } = require('proc-log') const npa = require('npm-package-arg') const semver = require('semver') const ArboristWorkspaceCmd = require('../arborist-cmd.js') +const checkAllowScripts = require('../utils/check-allow-scripts.js') +const resolveAllowScripts = require('../utils/resolve-allow-scripts.js') class Rebuild extends ArboristWorkspaceCmd { static description = 'Rebuild a package' @@ -12,6 +14,9 @@ class Rebuild extends ArboristWorkspaceCmd { 'bin-links', 'foreground-scripts', 'ignore-scripts', + 'allow-scripts', + 'strict-script-builds', + 'dangerously-allow-all-scripts', ...super.params, ] @@ -26,9 +31,11 @@ class Rebuild extends ArboristWorkspaceCmd { const globalTop = resolve(this.npm.globalDir, '..') const where = this.npm.global ? globalTop : this.npm.prefix const Arborist = require('@npmcli/arborist') + const { policy: allowScriptsPolicy } = await resolveAllowScripts(this.npm) const arb = new Arborist({ ...this.npm.flatOptions, path: where, + allowScripts: allowScriptsPolicy, // TODO when extending ReifyCmd // workspaces: this.workspaceNames, }) @@ -55,6 +62,20 @@ class Rebuild extends ArboristWorkspaceCmd { await arb.rebuild() } + // Phase 1 advisory: list any packages whose install scripts ran (or + // would have run) and are not yet covered by allowScripts. Rebuild + // doesn't go through reifyFinish, so the walker is invoked here. + const unreviewed = await checkAllowScripts({ arb, npm: this.npm }) + if (unreviewed.length > 0) { + const count = unreviewed.length + const noun = count === 1 ? 'package has' : 'packages have' + log.warn( + 'rebuild', + `${count} ${noun} install scripts not yet covered by allowScripts. ` + + 'Run `npm approve-scripts --pending` to review.' + ) + } + output.standard('rebuilt dependencies successfully') } diff --git a/lib/commands/update.js b/lib/commands/update.js index ed1416d70c13e..2d5702f37b6b0 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -1,6 +1,7 @@ const path = require('node:path') const { log } = require('proc-log') const reifyFinish = require('../utils/reify-finish.js') +const resolveAllowScripts = require('../utils/resolve-allow-scripts.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') class Update extends ArboristWorkspaceCmd { @@ -19,6 +20,9 @@ class Update extends ArboristWorkspaceCmd { 'package-lock', 'foreground-scripts', 'ignore-scripts', + 'allow-scripts', + 'strict-script-builds', + 'dangerously-allow-all-scripts', 'audit', 'before', 'bin-links', @@ -50,11 +54,13 @@ class Update extends ArboristWorkspaceCmd { } const Arborist = require('@npmcli/arborist') + const { policy: allowScriptsPolicy } = await resolveAllowScripts(this.npm) const opts = { ...this.npm.flatOptions, path: where, save, workspaces: this.workspaceNames, + allowScripts: allowScriptsPolicy, } const arb = new Arborist(opts) diff --git a/lib/utils/resolve-allow-scripts.js b/lib/utils/resolve-allow-scripts.js new file mode 100644 index 0000000000000..175d7194123e1 --- /dev/null +++ b/lib/utils/resolve-allow-scripts.js @@ -0,0 +1,179 @@ +const { log } = require('proc-log') +const npa = require('npm-package-arg') +const pkgJson = require('@npmcli/package-json') +const { isExactVersionDisjunction } = require('@npmcli/arborist/lib/script-allowed.js') + +// Parse a raw `allow-scripts` config value (string or array) into a flat +// array of trimmed package names. Mirrors the flatten function in +// workspaces/config/lib/definitions/definitions.js. +const parseConfigValue = (raw) => { + const parts = [] + /* istanbul ignore next: nopt always returns arrays or undefined here */ + const entries = Array.isArray(raw) ? raw : (typeof raw === 'string' ? [raw] : []) + for (const entry of entries) { + /* istanbul ignore if: nopt produces string entries for [String,Array] types */ + if (typeof entry !== 'string') { + continue + } + for (const part of entry.split(',')) { + const trimmed = part.trim() + /* istanbul ignore else: split on ',' produces non-empty strings after trim except for edge cases tested via flatten */ + if (trimmed) { + parts.push(trimmed) + } + } + } + return parts +} + +const buildPolicyFromNames = (names) => { + /* istanbul ignore if: callers only pass non-empty arrays */ + if (!names.length) { + return null + } + const policy = {} + for (const name of names) { + policy[name] = true + } + return policy +} + +// Read the `allow-scripts` value from one or more named config sources and +// build a policy object. Returns `null` if none of the sources has a value. +const policyFromSources = (npm, sources) => { + for (const where of sources) { + const value = npm.config.get?.('allow-scripts', where) + if (value === undefined) { + continue + } + const names = parseConfigValue(value) + /* istanbul ignore else: parseConfigValue returns non-empty when value is set */ + if (names.length) { + return buildPolicyFromNames(names) + } + } + return null +} + +const validatePolicy = (policy, sourceLabel) => { + // Drop and warn about keys with forbidden semver ranges (^, ~, >=, <, *). + // The RFC only permits exact versions joined by `||`. Bare names like + // `canvas` and explicit name-only wildcards (`canvas@*`) are allowed. + if (!policy) { + return policy + } + const cleaned = {} + for (const [key, value] of Object.entries(policy)) { + let parsed + try { + parsed = npa(key) + } catch { + log.warn('allow-scripts', `${sourceLabel}: ignoring unparseable entry "${key}"`) + continue + } + if (parsed.type === 'range') { + const isNameOnly = parsed.fetchSpec === '*' + || parsed.rawSpec === '' + || parsed.rawSpec === '*' + if (!isNameOnly && !isExactVersionDisjunction(parsed.fetchSpec)) { + log.warn( + 'allow-scripts', + `${sourceLabel}: ignoring "${key}" — semver ranges (^, ~, >=, <) are not allowed; ` + + 'use exact versions joined by "||" instead' + ) + continue + } + } + cleaned[key] = value + } + return Object.keys(cleaned).length > 0 ? cleaned : null +} + +// Resolve the effective allowScripts policy from the layered sources. +// Returns `{ policy, source }` where: +// - `policy` is an object map of `package-spec` -> boolean, or `null` if +// no layer has any configuration +// - `source` is one of `'cli'`, `'package.json'`, `'.npmrc'`, or `null` +// +// Precedence order (highest to lowest), per RFC npm/rfcs#868: +// 1. CLI flags (--allow-scripts) and env vars +// 2. Root `package.json#allowScripts` +// 3. `.npmrc` cascade (project, user, global) +// +// The project `package.json` layer is skipped when: +// - `npm.global` is true (no project context exists for global installs) +// - `skipProjectConfig` is true (e.g. npm exec / npx, which per the RFC +// consult only user/global .npmrc) +// +// In both skipped cases, the CLI and .npmrc layers are still consulted; +// only the project package.json layer is skipped. +// +// The first source with any configuration wins for the entire install; +// lower layers are ignored. A `log.warn` is emitted whenever a setting is +// being suppressed by a higher-priority source. +// +// Reads `package.json` from `npm.prefix` (not `npm.localPrefix`) so an +// install run from a workspace sub-directory still picks up the project +// root's policy. +const resolveAllowScripts = async (npm, { skipProjectConfig = false } = {}) => { + // Independently probe each RFC layer. + const cliPolicy = policyFromSources(npm, ['cli', 'env']) + const npmrcPolicy = policyFromSources(npm, ['project', 'user', 'global', 'builtin']) + + // Project package.json is consulted only when the caller is operating + // inside a real project (not -g, not npx). + let pkgPolicy = null + if (!npm.global && !skipProjectConfig) { + try { + const { content } = await pkgJson.normalize(npm.prefix) + if (content?.allowScripts && typeof content.allowScripts === 'object') { + const entries = Object.entries(content.allowScripts) + if (entries.length > 0) { + pkgPolicy = Object.fromEntries(entries) + } + } + } catch (err) { + log.silly('allow-scripts', 'no package.json at prefix', err.message) + } + } + + // Validate each candidate layer: drop forbidden ranges, warn the user. + const cli = validatePolicy(cliPolicy, 'CLI flag') + const pkg = validatePolicy(pkgPolicy, 'package.json') + const rc = validatePolicy(npmrcPolicy, '.npmrc') + + // Apply RFC precedence. + if (cli) { + if (pkg) { + log.warn( + 'allow-scripts', + 'package.json#allowScripts is being ignored because --allow-scripts was passed on the command line' + ) + } + if (rc) { + log.warn( + 'allow-scripts', + '.npmrc allow-scripts setting is being ignored because --allow-scripts was passed on the command line' + ) + } + return { policy: cli, source: 'cli' } + } + + if (pkg) { + if (rc) { + log.warn( + 'allow-scripts', + '.npmrc allow-scripts setting is being ignored because package.json declares its own allowScripts field' + ) + } + return { policy: pkg, source: 'package.json' } + } + + if (rc) { + return { policy: rc, source: '.npmrc' } + } + + return { policy: null, source: null } +} + +module.exports = resolveAllowScripts diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index 42bd213ba473e..19916c8b5940a 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -20,6 +20,9 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "allow-file": "all", "allow-git": "all", "allow-remote": "all", + "allow-scripts": [ + "" + ], "also": null, "audit": true, "audit-level": null, @@ -37,6 +40,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "cidr": null, "commit-hooks": true, "cpu": null, + "dangerously-allow-all-scripts": false, "depth": null, "description": true, "dev": false, @@ -166,6 +170,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "sign-git-commit": false, "sign-git-tag": false, "strict-peer-deps": false, + "strict-script-builds": false, "strict-ssl": true, "tag": "latest", "tag-version-prefix": "v", @@ -199,6 +204,7 @@ allow-file = "all" allow-git = "all" allow-remote = "all" allow-same-version = false +allow-scripts = [""] also = null audit = true audit-level = null @@ -218,6 +224,7 @@ cidr = null ; color = {COLOR} commit-hooks = true cpu = null +dangerously-allow-all-scripts = false depth = null description = true dev = false @@ -347,6 +354,7 @@ shell = "{SHELL}" sign-git-commit = false sign-git-tag = false strict-peer-deps = false +strict-script-builds = false strict-ssl = true tag = "latest" tag-version-prefix = "v" diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index e7bad30e38ff4..6a7d6367c0c33 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -294,6 +294,27 @@ to the same value as the current version. +#### \`allow-scripts\` + +* Default: "" +* Type: String (can be set multiple times) + +Comma-separated list of packages whose install scripts (\`preinstall\`, +\`install\`, \`postinstall\`) are allowed to run. Used as a fallback when no +\`allowScripts\` field is set in the root project's \`package.json\`, and for +global/npx contexts where no project \`package.json\` exists. + +The \`package.json\` \`allowScripts\` field takes precedence over this setting. +Layers are not merged: the first source in the precedence chain that defines +any allowlist configuration wins for the entire install. + +This setting is part of an opt-in install-script policy. In the current +release, scripts are not blocked by default; this setting prepares your +project for a future release that will block dependency install scripts +unless they are explicitly allowed. + + + #### \`audit\` * Default: true @@ -483,6 +504,22 @@ are same as \`cpu\` field of package.json, which comes from \`process.arch\`. +#### \`dangerously-allow-all-scripts\` + +* Default: false +* Type: Boolean + +When \`true\`, all dependency install scripts run regardless of the +\`allowScripts\` field in \`package.json\` or the \`allow-scripts\` config. This +is an escape hatch for migration and emergency use; its use is strongly +discouraged. + +This setting has no effect in the current release, where dependency install +scripts already run by default. It is reserved for a future release that +will block them unless explicitly allowed. + + + #### \`depth\` * Default: \`Infinity\` if \`--all\` is set; otherwise, \`0\` @@ -1826,6 +1863,21 @@ this warning is treated as a failure. +#### \`strict-script-builds\` + +* Default: false +* Type: Boolean + +When \`true\`, any dependency install script that is blocked by the +\`allowScripts\` policy causes the install to fail with an error instead of +printing a warning and continuing. + +This setting has no effect in the current release, where dependency install +scripts run by default and no scripts are blocked. It is reserved for a +future release that will block install scripts unless explicitly allowed. + + + #### \`strict-ssl\` * Default: true @@ -2300,6 +2352,7 @@ Array [ "allow-file", "allow-git", "allow-remote", + "allow-scripts", "also", "audit", "audit-level", @@ -2319,6 +2372,7 @@ Array [ "color", "commit-hooks", "cpu", + "dangerously-allow-all-scripts", "depth", "description", "dev", @@ -2448,6 +2502,7 @@ Array [ "sign-git-commit", "sign-git-tag", "strict-peer-deps", + "strict-script-builds", "strict-ssl", "tag", "tag-version-prefix", @@ -2479,6 +2534,7 @@ Array [ "allow-file", "allow-git", "allow-remote", + "allow-scripts", "also", "audit", "audit-level", @@ -2498,6 +2554,7 @@ Array [ "color", "commit-hooks", "cpu", + "dangerously-allow-all-scripts", "depth", "description", "dev", @@ -2606,6 +2663,7 @@ Array [ "sign-git-commit", "sign-git-tag", "strict-peer-deps", + "strict-script-builds", "strict-ssl", "tag", "tag-version-prefix", @@ -2663,6 +2721,7 @@ Object { "allowGit": "all", "allowRemote": "all", "allowSameVersion": false, + "allowScripts": Array [], "audit": true, "auditLevel": null, "authType": "web", @@ -2678,6 +2737,7 @@ Object { "color": false, "commitHooks": true, "cpu": null, + "dangerouslyAllowAllScripts": false, "defaultTag": "latest", "depth": null, "diff": Array [], @@ -2784,6 +2844,7 @@ Object { "signGitTag": false, "silent": false, "strictPeerDeps": false, + "strictScriptBuilds": false, "strictSSL": true, "tagVersionPrefix": "v", "timeout": 300000, @@ -3060,7 +3121,9 @@ Options: [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--allow-directory ] [--allow-file ] -[--allow-git ] [--allow-remote ] [--no-audit] +[--allow-git ] [--allow-remote ] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] [-w|--workspace [-w|--workspace ...]] [--workspaces] [--include-workspace-root] [--install-links] @@ -3101,6 +3164,15 @@ Options: --allow-remote Limits the ability for npm to fetch dependencies from urls. + --allow-scripts + Comma-separated list of packages whose install scripts (\`preinstall\`, + + --strict-script-builds + When \`true\`, any dependency install script that is blocked by the + + --dangerously-allow-all-scripts + When \`true\`, all dependency install scripts run regardless of the + --audit When "true" submit audit reports alongside the current npm command to the @@ -3148,6 +3220,9 @@ aliases: clean-install, ic, install-clean, isntall-clean #### \`allow-file\` #### \`allow-git\` #### \`allow-remote\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`audit\` #### \`bin-links\` #### \`fund\` @@ -3985,7 +4060,9 @@ Options: [--strict-peer-deps] [--prefer-dedupe] [--no-package-lock] [--package-lock-only] [--foreground-scripts] [--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] -[--allow-remote ] [--no-audit] +[--allow-remote ] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [--no-audit] [--before |--min-release-age ] [--no-bin-links] [--no-fund] [--dry-run] [--cpu ] [--os ] [--libc ] [-w|--workspace [-w|--workspace ...]] @@ -4045,6 +4122,15 @@ Options: --allow-remote Limits the ability for npm to fetch dependencies from urls. + --allow-scripts + Comma-separated list of packages whose install scripts (\`preinstall\`, + + --strict-script-builds + When \`true\`, any dependency install script that is blocked by the + + --dangerously-allow-all-scripts + When \`true\`, all dependency install scripts run regardless of the + --audit When "true" submit audit reports alongside the current npm command to the @@ -4113,6 +4199,9 @@ aliases: add, i, in, ins, inst, insta, instal, isnt, isnta, isntal, isntall #### \`allow-file\` #### \`allow-git\` #### \`allow-remote\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`audit\` #### \`before\` #### \`min-release-age\` @@ -4140,7 +4229,9 @@ Options: [--include [--include ...]] [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--allow-directory ] [--allow-file ] -[--allow-git ] [--allow-remote ] [--no-audit] +[--allow-git ] [--allow-remote ] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] [-w|--workspace [-w|--workspace ...]] [--workspaces] [--include-workspace-root] [--install-links] @@ -4181,6 +4272,15 @@ Options: --allow-remote Limits the ability for npm to fetch dependencies from urls. + --allow-scripts + Comma-separated list of packages whose install scripts (\`preinstall\`, + + --strict-script-builds + When \`true\`, any dependency install script that is blocked by the + + --dangerously-allow-all-scripts + When \`true\`, all dependency install scripts run regardless of the + --audit When "true" submit audit reports alongside the current npm command to the @@ -4228,6 +4328,9 @@ aliases: cit, clean-install-test, sit #### \`allow-file\` #### \`allow-git\` #### \`allow-remote\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`audit\` #### \`bin-links\` #### \`fund\` @@ -4253,7 +4356,9 @@ Options: [--strict-peer-deps] [--prefer-dedupe] [--no-package-lock] [--package-lock-only] [--foreground-scripts] [--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] -[--allow-remote ] [--no-audit] +[--allow-remote ] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [--no-audit] [--before |--min-release-age ] [--no-bin-links] [--no-fund] [--dry-run] [--cpu ] [--os ] [--libc ] [-w|--workspace [-w|--workspace ...]] @@ -4313,6 +4418,15 @@ Options: --allow-remote Limits the ability for npm to fetch dependencies from urls. + --allow-scripts + Comma-separated list of packages whose install scripts (\`preinstall\`, + + --strict-script-builds + When \`true\`, any dependency install script that is blocked by the + + --dangerously-allow-all-scripts + When \`true\`, all dependency install scripts run regardless of the + --audit When "true" submit audit reports alongside the current npm command to the @@ -4381,6 +4495,9 @@ alias: it #### \`allow-file\` #### \`allow-git\` #### \`allow-remote\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`audit\` #### \`before\` #### \`min-release-age\` diff --git a/test/lib/commands/exec.js b/test/lib/commands/exec.js index 2a6d3f6b8e0af..92ea993e3edfb 100644 --- a/test/lib/commands/exec.js +++ b/test/lib/commands/exec.js @@ -303,3 +303,68 @@ t.test('can run packages with keywords', async t => { t.fail(err, 'should not throw') } }) + +t.test('exec threads allowScripts policy from .npmrc through to libexec', async t => { + let capturedOpts + const fakeLibexec = async (opts) => { + capturedOpts = opts + } + const { npm } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + '.npmrc': 'allow-scripts = canvas', + }, + mocks: { + libnpmexec: fakeLibexec, + }, + }) + await npm.exec('exec', ['some-pkg']) + t.strictSame(capturedOpts.allowScripts, { canvas: true }, + 'allowScripts populated from .npmrc layer') +}) + +t.test('exec ignores project package.json#allowScripts (RFC: .npmrc-only)', async t => { + // Per RFC line 299, exec/npx consults only user/global .npmrc. Project + // package.json policy must NOT influence npx behaviour, even when the + // user is running npx inside a project that has its own allowScripts. + let capturedOpts + const { npm } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + allowScripts: { sharp: true }, + }), + }, + mocks: { + libnpmexec: async (opts) => { + capturedOpts = opts + }, + }, + }) + await npm.exec('exec', ['some-pkg']) + // package.json policy is skipped; no other layer has policy; result is null. + t.equal(capturedOpts.allowScripts, null) +}) + +t.test('exec reads .npmrc policy even when project package.json has a different policy', async t => { + // .npmrc-tier policy wins because package.json is skipped entirely. + let capturedOpts + const { npm } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + allowScripts: { sharp: true }, + }), + '.npmrc': 'allow-scripts = canvas', + }, + mocks: { + libnpmexec: async (opts) => { + capturedOpts = opts + }, + }, + }) + await npm.exec('exec', ['some-pkg']) + t.strictSame(capturedOpts.allowScripts, { canvas: true }) +}) diff --git a/test/lib/commands/rebuild.js b/test/lib/commands/rebuild.js index 0062362b61329..de91fd3471b4e 100644 --- a/test/lib/commands/rebuild.js +++ b/test/lib/commands/rebuild.js @@ -221,3 +221,63 @@ t.test('completion', async t => { const res = await rebuild.completion({ conf: { argv: { remain: ['npm', 'rebuild'] } } }) t.type(res, Array) }) + +t.test('emits Phase 1 advisory warning for unreviewed install scripts', async t => { + const { npm, logs } = await setupMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + node_modules: { + canvas: { + 'package.json': JSON.stringify({ + name: 'canvas', + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + }, + }, + }, + }) + await npm.exec('rebuild', []) + t.match( + logs.warn.byTitle('rebuild'), + [/install scripts not yet covered by allowScripts/] + ) +}) + +t.test('no advisory warning when allowScripts covers the package', async t => { + const { npm, logs } = await setupMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { canvas: '1.0.0' }, + allowScripts: { canvas: true }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { canvas: '1.0.0' } }, + 'node_modules/canvas': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/canvas/-/canvas-1.0.0.tgz', + hasInstallScript: true, + }, + }, + }), + node_modules: { + canvas: { + 'package.json': JSON.stringify({ + name: 'canvas', + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + }, + }, + }, + }) + await npm.exec('rebuild', []) + t.strictSame(logs.warn.byTitle('rebuild'), []) +}) diff --git a/test/lib/commands/update.js b/test/lib/commands/update.js index a8c68bd65bb36..68067b8af8168 100644 --- a/test/lib/commands/update.js +++ b/test/lib/commands/update.js @@ -95,3 +95,33 @@ t.test('completion', async t => { const res = await update.completion({ conf: { argv: { remain: ['npm', 'update'] } } }) t.type(res, Array) }) + +t.test('update threads allowScripts policy through to arborist', async t => { + // The reify step uses the resolved policy. The advisory warning is + // emitted from reifyFinish (already covered by install.js tests), + // so here we verify the call site populates opts.allowScripts. + let capturedOpts + const FakeArborist = function (opts) { + capturedOpts = opts + this.options = opts + this.actualTree = { inventory: new Map() } + } + FakeArborist.prototype.reify = async function () {} + + const mock = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + allowScripts: { canvas: true }, + }), + }, + mocks: { + '@npmcli/arborist': FakeArborist, + '{LIB}/utils/reify-finish.js': async () => {}, + }, + }) + await mock.npm.exec('update', []) + t.strictSame(capturedOpts.allowScripts, { canvas: true }, + 'opts.allowScripts populated from package.json') +}) diff --git a/test/lib/utils/resolve-allow-scripts.js b/test/lib/utils/resolve-allow-scripts.js new file mode 100644 index 0000000000000..48130dd42d4b8 --- /dev/null +++ b/test/lib/utils/resolve-allow-scripts.js @@ -0,0 +1,313 @@ +const t = require('tap') +const mockNpm = require('../../fixtures/mock-npm') +const tmock = require('../../fixtures/tmock') + +const loadResolver = (t) => tmock(t, '{LIB}/utils/resolve-allow-scripts.js') + +// Helper that simulates config layering. `cliConfig` sets the value at +// the 'cli' source; `npmrcConfig` sets it at the 'user' source. mockNpm +// puts all `config` keys into the 'cli' source by default, so for npmrc +// tests we use an .npmrc file instead. + +t.test('returns null when no policy is set anywhere', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { 'package.json': JSON.stringify({ name: 'p' }) }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.strictSame(result, { policy: null, source: null }) +}) + +t.test('global install: skips package.json but still consults CLI', async t => { + const { npm } = await mockNpm(t, { + config: { global: true, 'allow-scripts': 'canvas' }, + prefixDir: { 'package.json': JSON.stringify({ name: 'p', allowScripts: { sharp: true } }) }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, 'cli') + t.strictSame(result.policy, { canvas: true }) +}) + +t.test('global install: skips package.json but still consults .npmrc', async t => { + const { npm } = await mockNpm(t, { + config: { global: true }, + homeDir: { '.npmrc': 'allow-scripts = canvas' }, + prefixDir: { + 'package.json': JSON.stringify({ name: 'p', allowScripts: { sharp: true } }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true }) +}) + +t.test('global install with no CLI or .npmrc returns null', async t => { + const { npm } = await mockNpm(t, { + config: { global: true }, + prefixDir: { 'package.json': JSON.stringify({ name: 'p', allowScripts: { x: true } }) }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.strictSame(result, { policy: null, source: null }) +}) + +t.test('reads from package.json when only package.json is set', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { canvas: true, 'core-js': false, 'sharp@0.33.2': true }, + }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, 'package.json') + t.strictSame(result.policy, { canvas: true, 'core-js': false, 'sharp@0.33.2': true }) +}) + +t.test('CLI flag wins over package.json (RFC layer 1 > layer 2)', async t => { + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { sharp: true }, + }), + }, + // mock-npm puts all config keys at the 'cli' source. + config: { 'allow-scripts': 'canvas' }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, 'cli') + t.strictSame(result.policy, { canvas: true }) + t.match( + mock.logs.warn.byTitle('allow-scripts'), + [/package.json#allowScripts is being ignored because --allow-scripts/] + ) +}) + +t.test('package.json wins over .npmrc setting (RFC layer 2 > layer 3)', async t => { + // Put the allow-scripts setting in an .npmrc file so it loads at the + // 'user' source, not 'cli'. + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { sharp: true }, + }), + '.npmrc': 'allow-scripts = canvas', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, 'package.json') + t.strictSame(result.policy, { sharp: true }) + t.match( + mock.logs.warn.byTitle('allow-scripts'), + [/\.npmrc allow-scripts setting is being ignored because package.json/] + ) +}) + +t.test('.npmrc setting is used when nothing higher is set', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'p' }), + '.npmrc': 'allow-scripts = canvas, sharp', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true, sharp: true }) +}) + +t.test('CLI flag wins over .npmrc with no package.json policy', async t => { + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'p' }), + '.npmrc': 'allow-scripts = canvas', + }, + config: { 'allow-scripts': 'sharp' }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, 'cli') + t.strictSame(result.policy, { sharp: true }) + t.match( + mock.logs.warn.byTitle('allow-scripts'), + [/\.npmrc allow-scripts setting is being ignored because --allow-scripts/] + ) +}) + +t.test('empty allowScripts object in package.json falls through to .npmrc', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'p', allowScripts: {} }), + '.npmrc': 'allow-scripts = canvas', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true }) +}) + +t.test('missing package.json with .npmrc setting uses .npmrc', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + '.npmrc': 'allow-scripts = canvas', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true }) +}) + +t.test('reads from npm.prefix, not cwd, so workspace sub-installs find root policy', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'root', + workspaces: ['packages/*'], + allowScripts: { sharp: true }, + }), + packages: { + sub: { 'package.json': JSON.stringify({ name: 'sub' }) }, + }, + }, + chdir: ({ prefix }) => require('node:path').join(prefix, 'packages', 'sub'), + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.equal(result.source, 'package.json') + t.strictSame(result.policy, { sharp: true }) +}) + +t.test('drops package.json entries with forbidden semver ranges and warns', async t => { + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { + 'sharp@^0.33.0': true, // forbidden: caret range + 'canvas@~2.11.0': true, // forbidden: tilde range + 'core-js@>=3.0.0': true, // forbidden: gte range + 'good@1.2.3': true, // OK: exact pin + 'also-good': true, // OK: bare name + 'disjunction@1.0.0 || 2.0.0': true, // OK: exact disjunction + }, + }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, 'package.json') + t.strictSame(result.policy, { + 'good@1.2.3': true, + 'also-good': true, + 'disjunction@1.0.0 || 2.0.0': true, + }) + const warnings = mock.logs.warn.byTitle('allow-scripts') + t.equal(warnings.filter(m => /semver ranges/.test(m)).length, 3) +}) + +t.test('drops .npmrc forbidden ranges (and warns) but keeps valid entries', async t => { + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'p' }), + '.npmrc': 'allow-scripts = canvas, sharp@^0.33.0, lodash@4.17.21', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true, 'lodash@4.17.21': true }) + const warnings = mock.logs.warn.byTitle('allow-scripts') + t.ok(warnings.some(m => /sharp@\^0\.33\.0/.test(m) && /semver ranges/.test(m))) +}) + +t.test('drops package.json entries that fail npa parse', async t => { + const mock = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { + '@@@invalid@@@': true, + good: true, + }, + }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(mock.npm) + t.equal(result.source, 'package.json') + t.strictSame(result.policy, { good: true }) + t.ok(mock.logs.warn.byTitle('allow-scripts').some(m => /unparseable/.test(m))) +}) + +t.test('returns null when all package.json entries are dropped as invalid', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { 'sharp@^0.33.0': true }, + }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm) + t.strictSame(result, { policy: null, source: null }) +}) + +t.test('skipProjectConfig: ignores package.json even when present', async t => { + // Per RFC line 299, exec/npx consults only user/global .npmrc. + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { sharp: true }, + }), + '.npmrc': 'allow-scripts = canvas', + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm, { skipProjectConfig: true }) + // package.json is skipped, falls through to .npmrc. + t.equal(result.source, '.npmrc') + t.strictSame(result.policy, { canvas: true }) +}) + +t.test('skipProjectConfig: CLI still wins over .npmrc', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { sharp: true }, + }), + '.npmrc': 'allow-scripts = canvas', + }, + config: { 'allow-scripts': 'lodash' }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm, { skipProjectConfig: true }) + t.equal(result.source, 'cli') + t.strictSame(result.policy, { lodash: true }) +}) + +t.test('skipProjectConfig: returns null when only package.json is set', async t => { + const { npm } = await mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'p', + allowScripts: { sharp: true }, + }), + }, + }) + const resolveAllowScripts = loadResolver(t) + const result = await resolveAllowScripts(npm, { skipProjectConfig: true }) + t.strictSame(result, { policy: null, source: null }) +}) diff --git a/workspaces/arborist/lib/arborist/index.js b/workspaces/arborist/lib/arborist/index.js index eda3894746260..b3324888b01fe 100644 --- a/workspaces/arborist/lib/arborist/index.js +++ b/workspaces/arborist/lib/arborist/index.js @@ -100,6 +100,7 @@ class Arborist extends Base { nodeVersion: process.version, ...options, Arborist: this.constructor, + allowScripts: options.allowScripts ?? null, binLinks: 'binLinks' in options ? !!options.binLinks : true, cache: options.cache || `${homedir()}/.npm/_cacache`, dryRun: !!options.dryRun, diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index f493232f7d92b..a9dc2f0755a4f 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -271,13 +271,29 @@ const definitions = { flips the default. `, flatten (key, obj, flatOptions) { - if (Array.isArray(obj[key])) { - flatOptions.allowScripts = obj[key] - } else if (obj[key]) { - flatOptions.allowScripts = obj[key].split(',').map(s => s.trim()).filter(Boolean) - } else { - flatOptions.allowScripts = [] + const raw = obj[key] + const parts = [] + if (Array.isArray(raw)) { + for (const entry of raw) { + if (typeof entry !== 'string') { + continue + } + for (const part of entry.split(',')) { + const trimmed = part.trim() + if (trimmed) { + parts.push(trimmed) + } + } + } + } else if (typeof raw === 'string' && raw) { + for (const part of raw.split(',')) { + const trimmed = part.trim() + if (trimmed) { + parts.push(trimmed) + } + } } + flatOptions.allowScripts = parts }, }), also: new Definition('also', { diff --git a/workspaces/config/test/definitions/definitions.js b/workspaces/config/test/definitions/definitions.js index 17a8e04440614..8f305f16fa56e 100644 --- a/workspaces/config/test/definitions/definitions.js +++ b/workspaces/config/test/definitions/definitions.js @@ -1083,7 +1083,7 @@ t.test('allow-scripts', t => { t.end() }) - t.test('passes array values through unchanged', t => { + t.test('passes array values through (multiple --allow-scripts flags)', t => { const flat = {} mockDefs()['allow-scripts'].flatten( 'allow-scripts', @@ -1094,6 +1094,17 @@ t.test('allow-scripts', t => { t.end() }) + t.test('splits commas within each array entry (CLI single value)', t => { + const flat = {} + mockDefs()['allow-scripts'].flatten( + 'allow-scripts', + { 'allow-scripts': ['canvas,sharp', 'sqlite3'] }, + flat + ) + t.strictSame(flat, { allowScripts: ['canvas', 'sharp', 'sqlite3'] }) + t.end() + }) + t.end() }) From efe4b9e3b15e9c237b21b0f756423498cb67e5c5 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:42:32 -0700 Subject: [PATCH 3/7] feat(arborist): add identity matcher for allowScripts policy A pure isScriptAllowed(node, policy) helper in workspaces/arborist/lib/script-allowed.js. Used by the install-time warning walker and by the approve-scripts / deny-scripts commands. Matching rules follow the RFC: - registry deps: name + optional semver (range or exact) - git deps: canonical ssh-url match plus short-SHA prefix - file / directory / remote tarball: exact resolved string match - alias spec keys are ignored entirely; a user must address the real package name, not the alias - matching uses node.packageName, never node.name, so an alias install cannot be approved by writing its alias name Conflict resolution: any matching false wins over any matching true. No match returns null (unreviewed). Pure function, no I/O. 15 test cases cover alias safety and omitLockfileRegistryResolved. Refs: npm/rfcs#868 --- workspaces/arborist/lib/script-allowed.js | 303 +++++++++++++++ workspaces/arborist/test/script-allowed.js | 432 +++++++++++++++++++++ 2 files changed, 735 insertions(+) create mode 100644 workspaces/arborist/lib/script-allowed.js create mode 100644 workspaces/arborist/test/script-allowed.js diff --git a/workspaces/arborist/lib/script-allowed.js b/workspaces/arborist/lib/script-allowed.js new file mode 100644 index 0000000000000..75c2a7e9fa396 --- /dev/null +++ b/workspaces/arborist/lib/script-allowed.js @@ -0,0 +1,303 @@ +const npa = require('npm-package-arg') +const semver = require('semver') +const versionFromTgz = require('./version-from-tgz.js') + +// Identity matcher for the allowScripts policy. +// +// Returns: +// - true: at least one allow entry matches and no deny entry matches +// - false: at least one deny entry matches (deny wins on conflict) +// - null: no entry matches (unreviewed) +// +// `policy` is a flat object of `spec-key -> boolean`, where spec-key is +// anything `npm-package-arg` can parse. `node` is an arborist Node. +// +// Identity rules (see RFC npm/rfcs#868): +// - registry deps match by the name+version parsed from the lockfile's +// resolved URL, NOT by `node.packageName` / `node.version`. Those two +// getters return `node.package.name` / `node.package.version`, which +// come from the tarball's own package.json and are therefore +// attacker-controlled. A package can publish a tarball claiming any +// name; the only trusted name is the one baked into the registry URL. +// - tarball / file / link / remote: exact match on node.resolved +// - git: match on hosted.ssh() plus a short-SHA prefix of the +// resolved committish + +const isScriptAllowed = (node, policy) => { + // Bundled dependencies cannot be allowlisted in Phase 1. The RFC defers + // allowlisting them to a follow-up RFC because matching by name@version + // from the bundled tarball would reintroduce manifest confusion (a + // bundled tarball can claim any name and version). Returning null here + // marks bundled deps as unreviewed regardless of any policy entries, so + // their install scripts surface in the Phase 1 advisory warning and + // (eventually) get blocked at the install-time gate. + if (node.inBundle) { + return null + } + + if (!policy || typeof policy !== 'object') { + return null + } + + let anyAllow = false + let anyDeny = false + + for (const [key, value] of Object.entries(policy)) { + if (!matches(node, key)) { + continue + } + if (value === false) { + anyDeny = true + } else if (value === true) { + anyAllow = true + } + } + + if (anyDeny) { + return false + } + if (anyAllow) { + return true + } + return null +} + +const matches = (node, key) => { + let parsed + try { + parsed = npa(key) + } catch { + return false + } + + switch (parsed.type) { + case 'tag': + case 'range': + case 'version': + return matchRegistry(node, parsed) + case 'git': + return matchGit(node, parsed) + case 'file': + case 'directory': + return matchFileOrDir(node, parsed) + case 'remote': + return matchRemote(node, parsed) + case 'alias': + // Disallowed: aliases as policy keys do not match anything. + // The user has to address the real package name. + return false + default: + return false + } +} + +const matchRegistry = (node, parsed) => { + // If this node is not a registry dep, refuse the match. A registry-style + // key (`pkg`, `pkg@1`, `pkg@1 || 2`) must not match a tarball or git node + // even if their names happen to coincide. + if (!isRegistryNode(node)) { + return false + } + + // Derive the trusted name+version from the lockfile's resolved URL. + // Never use `node.packageName` / `node.version` here: those read from + // the tarball's own package.json and can be forged by a malicious + // publisher to bypass an allowScripts entry. + const trusted = getTrustedRegistryIdentity(node) + if (!trusted || trusted.name !== parsed.name) { + return false + } + + // `tag` covers `pkg@latest`. Treat as name-only allow. + if (parsed.type === 'tag') { + return true + } + + // `range` includes `pkg@^1`, `pkg@1 || 2`, `pkg@*`, `pkg@>=0`, and bare + // names like `pkg` (npa parses these as range with fetchSpec='*'). The + // RFC permits bare names (name-only allow) and exact versions joined by + // `||`; ranges like ^/~/>=/< are rejected because they would silently + // allow versions the user has never reviewed. + if (parsed.type === 'range') { + // Bare name or `pkg@*`: treat as name-only allow. + if (parsed.fetchSpec === '*' || parsed.rawSpec === '' || parsed.rawSpec === '*') { + return true + } + if (!trusted.version || !isExactVersionDisjunction(parsed.fetchSpec)) { + return false + } + return semver.satisfies(trusted.version, parsed.fetchSpec, { loose: true }) + } + + // `version` is an exact pin like `pkg@1.2.3`. + if (parsed.type === 'version') { + return trusted.version === parsed.fetchSpec + } + + return false +} + +// Derive a registry node's trusted name+version. +// +// Preferred source: the lockfile's resolved URL parsed via +// versionFromTgz. arborist records the URL when it first adds the dep, +// before any tarball is unpacked, so the URL cannot be forged by the +// package's own package.json. +// +// Fallback for lockfiles produced with omit-lockfile-registry-resolved +// (where the URL is absent): take the dep name from an incoming +// dependency edge. The edge's spec was written by the consumer (or by an +// upstream package.json), not by the installed tarball. For aliases like +// `"trusted": "npm:naughty@1.0.0"`, the underlying registered package +// name is parsed out of the alias `subSpec`. The install location +// (`node_modules/trusted`) is deliberately not consulted because for +// aliases it carries only the alias name, which would let a malicious +// publisher bypass an allowScripts entry written for the real package. +// +// Version is left null in the fallback case because the only remaining +// source for it (`node.version`) reads from the tarball. +// +// Returns `{ name, version }` or `null` if no trusted identity exists. +const getTrustedRegistryIdentity = (node) => { + if (node.resolved && typeof node.resolved === 'string') { + const parsed = versionFromTgz('', node.resolved) + if (parsed && parsed.name && parsed.version) { + return parsed + } + } + const name = nameFromEdges(node) + if (name) { + return { name, version: null } + } + return null +} + +const nameFromEdges = (node) => { + if (!node.edgesIn || typeof node.edgesIn[Symbol.iterator] !== 'function') { + return null + } + for (const edge of node.edgesIn) { + let parsed + try { + parsed = npa.resolve(edge.name, edge.spec) + } catch { + continue + } + // Aliases: trust the underlying registered package, not the alias. + if (parsed.type === 'alias' && parsed.subSpec && parsed.subSpec.registry) { + return parsed.subSpec.name + } + // Non-aliased registry edge: the edge name is the package name as + // written by the consumer / upstream, which is trusted (it is not + // read from the installed tarball). + if (parsed.registry) { + return parsed.name + } + } + return null +} + +// True if `rangeSpec` is one or more exact versions joined by `||`. Anything +// containing comparator operators (^, ~, >=, <, *) returns false. +const isExactVersionDisjunction = (rangeSpec) => { + if (typeof rangeSpec !== 'string' || rangeSpec.trim() === '') { + return false + } + const parts = rangeSpec.split('||').map(p => p.trim()) + if (parts.length === 0) { + return false + } + return parts.every(p => p !== '' && semver.valid(p) !== null) +} + +const matchGit = (node, parsed) => { + if (!node.resolved || !node.resolved.startsWith('git')) { + return false + } + + let nodeParsed + try { + nodeParsed = npa(node.resolved) + } catch { + return false + } + + // Compare the host/repo. Both sides should resolve to the same canonical + // ssh URL. + const noCommittish = { noCommittish: true } + const keyHost = parsed.hosted?.ssh(noCommittish) + const nodeHost = nodeParsed.hosted?.ssh(noCommittish) + if (keyHost && nodeHost) { + if (keyHost !== nodeHost) { + return false + } + } else if (parsed.fetchSpec && nodeParsed.fetchSpec) { + // Non-hosted git URLs: fall back to fetch spec. + if (parsed.fetchSpec !== nodeParsed.fetchSpec) { + return false + } + } else { + return false + } + + // If the policy key has no committish, name-only match. + const keyCommittish = parsed.gitCommittish || parsed.hosted?.committish + if (!keyCommittish) { + return true + } + + // Match the resolved full SHA against the key's committish. Users + // typically write short SHAs in the policy; the lockfile stores 40-char + // SHAs. Direction matters: the lockfile's full SHA must START WITH the + // key's short SHA, never the reverse. A longer key matching a shorter + // resolved committish would let a malformed lockfile or a divergent + // resolver allow scripts the user never approved. + const nodeCommittish = nodeParsed.gitCommittish || nodeParsed.hosted?.committish || '' + if (!nodeCommittish) { + return false + } + return nodeCommittish.startsWith(keyCommittish) +} + +const matchFileOrDir = (node, parsed) => { + if (!node.resolved) { + return false + } + return node.resolved === parsed.saveSpec || node.resolved === parsed.fetchSpec +} + +const matchRemote = (node, parsed) => { + if (!node.resolved) { + return false + } + return node.resolved === parsed.fetchSpec || node.resolved === parsed.saveSpec +} + +const isRegistryNode = (node) => { + // Prefer arborist's edge-based check when available (real Node objects). + // It inspects the incoming edges' specs and only returns true if every + // edge resolves to a registry spec, which is much harder to spoof than + // the URL. + if (typeof node.isRegistryDependency === 'boolean') { + return node.isRegistryDependency + } + // Fall back to URL parsing for nodes without the arborist getter + // (e.g. test fixtures, lockfiles with omit-lockfile-registry-resolved). + // Treat the node as a registry dep when: + // - resolved is missing entirely (omitLockfileRegistryResolved), + // - resolved is an https/http URL pointing at a registry tarball, or + // - resolved is undefined and the node has a version (defensive). + if (!node.resolved) { + return !!node.version + } + // Registry tarballs live at `//-/-.tgz`. + // Require a path segment before `/-/` so an attacker can't lift a + // registry-style allow entry to a hostile URL like + // `https://evil.com/-/trusted-1.0.0.tgz`. + return /^https?:\/\/[^/]+\/.+\/-\/[^/]+-\d/.test(node.resolved) +} + +module.exports = isScriptAllowed +module.exports.isScriptAllowed = isScriptAllowed +module.exports.isExactVersionDisjunction = isExactVersionDisjunction +module.exports.getTrustedRegistryIdentity = getTrustedRegistryIdentity diff --git a/workspaces/arborist/test/script-allowed.js b/workspaces/arborist/test/script-allowed.js new file mode 100644 index 0000000000000..b2e2c26239616 --- /dev/null +++ b/workspaces/arborist/test/script-allowed.js @@ -0,0 +1,432 @@ +const t = require('tap') +const isScriptAllowed = require('../lib/script-allowed.js') + +// Test nodes default to a consistent registry-tarball shape: the resolved +// URL's name+version match the supplied name+version. Tests that need to +// simulate manifest confusion (mismatched URL) can override `resolved` +// independently. +const node = (overrides = {}) => { + const name = overrides.name ?? overrides.packageName ?? 'pkg' + const packageName = overrides.packageName ?? name + const version = overrides.version ?? '1.0.0' + // For real aliased installs, the URL is the TARGET package's URL, so + // build the default URL from packageName, not name. + const urlPkg = packageName + return { + name, + packageName, + version, + resolved: overrides.resolved + ?? `https://registry.npmjs.org/${urlPkg}/-/${urlPkg}-${version}.tgz`, + location: overrides.location ?? `node_modules/${name}`, + ...overrides, + } +} + +t.test('returns null when no policy is set', t => { + t.equal(isScriptAllowed(node(), null), null) + t.equal(isScriptAllowed(node(), undefined), null) + t.equal(isScriptAllowed(node(), {}), null) + t.end() +}) + +t.test('registry — name-only allow', t => { + t.equal(isScriptAllowed(node({ name: 'canvas', version: '2.11.0' }), { canvas: true }), true) + t.equal(isScriptAllowed(node({ name: 'other', version: '1.0.0' }), { canvas: true }), null) + t.end() +}) + +t.test('registry — exact version match', t => { + const policy = { 'canvas@2.11.0': true } + t.equal(isScriptAllowed(node({ name: 'canvas', version: '2.11.0' }), policy), true) + t.equal(isScriptAllowed(node({ name: 'canvas', version: '2.11.1' }), policy), null) + t.end() +}) + +t.test('registry — semver range', t => { + const policy = { 'sharp@0.33.2 || 0.34.0': true } + t.equal(isScriptAllowed(node({ name: 'sharp', version: '0.33.2' }), policy), true) + t.equal(isScriptAllowed(node({ name: 'sharp', version: '0.34.0' }), policy), true) + t.equal(isScriptAllowed(node({ name: 'sharp', version: '0.33.3' }), policy), null) + t.end() +}) + +t.test('registry — version mismatch returns null', t => { + const policy = { 'canvas@2.11.0': true } + t.equal(isScriptAllowed(node({ name: 'canvas', version: '3.0.0' }), policy), null) + t.end() +}) + +t.test('alias must not match the alias name', t => { + // install: `trusted@npm:naughty@1.0.0` + // node.name === 'trusted', node.packageName === 'naughty' + const aliased = node({ name: 'trusted', packageName: 'naughty', version: '1.0.0' }) + // Key matching the alias name must NOT match. + t.equal(isScriptAllowed(aliased, { trusted: true }), null) + // Key matching the real package name DOES match. + t.equal(isScriptAllowed(aliased, { naughty: true }), true) + t.equal(isScriptAllowed(aliased, { 'naughty@1.0.0': true }), true) + t.end() +}) + +t.test('registry-style key does not match a tarball or git node', t => { + const gitNode = node({ + name: 'pkg', + version: '1.0.0', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + }) + t.equal(isScriptAllowed(gitNode, { pkg: true }), null) + t.end() +}) + +t.test('git — repo-only match by canonical ssh URL', t => { + const gitNode = node({ + name: 'bar', + packageName: 'bar', + version: '1.0.0', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + }) + // Key by repo only — name-only allow for the git source. + t.equal(isScriptAllowed(gitNode, { 'github:foo/bar': true }), true) + t.equal(isScriptAllowed(gitNode, { 'github:foo/other': true }), null) + t.end() +}) + +t.test('git — short SHA prefix matches full SHA', t => { + const gitNode = node({ + name: 'bar', + packageName: 'bar', + version: '1.0.0', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + }) + t.equal(isScriptAllowed(gitNode, { 'github:foo/bar#deadbeef': true }), true) + t.equal(isScriptAllowed(gitNode, { 'github:foo/bar#deadbee': true }), true) + t.equal(isScriptAllowed(gitNode, { 'github:foo/bar#abcdef0': true }), null) + t.end() +}) + +t.test('file path — exact resolved match', t => { + const fileNode = node({ + name: 'local-pkg', + packageName: 'local-pkg', + version: '1.0.0', + resolved: 'file:../local-pkg', + }) + t.equal(isScriptAllowed(fileNode, { 'file:../local-pkg': true }), true) + t.equal(isScriptAllowed(fileNode, { 'file:../other': true }), null) + t.end() +}) + +t.test('remote tarball — exact resolved match', t => { + const remoteNode = node({ + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'https://example.com/pkg.tgz', + }) + t.equal(isScriptAllowed(remoteNode, { 'https://example.com/pkg.tgz': true }), true) + t.equal(isScriptAllowed(remoteNode, { 'https://example.com/other.tgz': true }), null) + t.end() +}) + +t.test('omitLockfileRegistryResolved: name-only match via edges; version-pinned does not', t => { + // Without a resolved URL, the trusted name comes from an incoming + // dependency edge (consumer-written), not from node.location (which + // for aliases would expose the alias name and let a malicious publisher + // bypass the policy). Version isn't trustable in this case, so + // version-pinned policy entries cannot match. + const omitted = { + name: 'canvas', + packageName: 'canvas', + version: '2.11.0', + resolved: undefined, + location: 'node_modules/canvas', + edgesIn: new Set([{ name: 'canvas', spec: '^2.0.0' }]), + } + t.equal(isScriptAllowed(omitted, { canvas: true }), true) + t.equal(isScriptAllowed(omitted, { 'canvas@2.11.0': true }), null, + 'version-pinned match requires the trusted URL-derived version') + t.equal(isScriptAllowed(omitted, { 'canvas@3': true }), null) + t.end() +}) + +t.test('omitLockfileRegistryResolved + alias: location is ignored; underlying name wins', t => { + // Consumer's package.json has `"trusted": "npm:naughty@1.0.0"`. With + // omitLockfileRegistryResolved, the resolved URL is absent. The install + // location is `node_modules/trusted` (alias path). The matcher MUST + // derive the underlying name from the edge's alias subSpec, not from + // the location. + const aliasOmitted = { + name: 'trusted', + packageName: 'naughty', + version: '1.0.0', + resolved: undefined, + location: 'node_modules/trusted', + edgesIn: new Set([{ name: 'trusted', spec: 'npm:naughty@1.0.0' }]), + } + // Alias name MUST NOT match. + t.equal(isScriptAllowed(aliasOmitted, { trusted: true }), null, + 'alias name does not authorize the underlying package') + // Underlying name DOES match. + t.equal(isScriptAllowed(aliasOmitted, { naughty: true }), true, + 'underlying package name authorizes scripts') + t.end() +}) + +t.test('omit-lockfile with no edges returns null (no trusted identity)', t => { + const orphan = { + name: 'canvas', + packageName: 'canvas', + version: '1.0.0', + resolved: undefined, + location: 'node_modules/canvas', + // No edgesIn at all. + } + t.equal(isScriptAllowed(orphan, { canvas: true }), null, + 'cannot match without a trusted name source') + t.end() +}) + +t.test('deny wins on conflict', t => { + const n = node({ name: 'pkg', version: '2.0.0' }) + t.equal(isScriptAllowed(n, { 'pkg@1.0.0 || 2.0.0': true, 'pkg@2.0.0 || 3.0.0': false }), false) + t.equal(isScriptAllowed(n, { pkg: true, 'pkg@2.0.0': false }), false) + t.end() +}) + +t.test('name-only deny without overlap returns false', t => { + t.equal(isScriptAllowed(node({ name: 'core-js', version: '3.0.0' }), { 'core-js': false }), false) + t.end() +}) + +t.test('skips unparseable policy keys', t => { + t.equal( + isScriptAllowed(node({ name: 'pkg', version: '1.0.0' }), { '@@@invalid': true, pkg: true }), + true + ) + t.end() +}) + +t.test('registry — forbidden semver ranges are rejected', async t => { + const n = node({ name: 'sharp', version: '0.33.5' }) + // Caret range that DOES match version, but RFC forbids — must return null + t.equal(isScriptAllowed(n, { 'sharp@^0.33.0': true }), null) + t.equal(isScriptAllowed(n, { 'sharp@~0.33.0': true }), null) + t.equal(isScriptAllowed(n, { 'sharp@>=0.33.0': true }), null) + t.equal(isScriptAllowed(n, { 'sharp@<1.0.0': true }), null) + // Partial versions like `0.33` are ranges in semver — also rejected + t.equal(isScriptAllowed(n, { 'sharp@0.33': true }), null) + t.equal(isScriptAllowed(n, { 'sharp@0': true }), null) +}) + +t.test('registry — wildcard versions are allowed (treated as name-only)', async t => { + const n = node({ name: 'sharp', version: '0.33.5' }) + t.equal(isScriptAllowed(n, { 'sharp@*': true }), true) +}) + +t.test('registry — exact disjunction with full semver is allowed', async t => { + const n = node({ name: 'sharp', version: '0.33.2' }) + t.equal(isScriptAllowed(n, { 'sharp@0.33.2 || 0.34.0': true }), true) +}) + +t.test('registry — exact disjunction where version is not listed returns null', async t => { + const n = node({ name: 'sharp', version: '0.33.5' }) + t.equal(isScriptAllowed(n, { 'sharp@0.33.2 || 0.34.0': true }), null) +}) + +t.test('forbidden range deny does NOT win (would silently allow new versions otherwise)', async t => { + // A user wrote `sharp@^0.33.0: false` thinking they're denying all 0.33.x. + // The matcher rejects the range, so the entry is effectively absent — + // deny does not match the node, returns null (unreviewed). + const n = node({ name: 'sharp', version: '0.33.5' }) + t.equal(isScriptAllowed(n, { 'sharp@^0.33.0': false }), null) +}) + +t.test('isRegistryNode — spoofed tarball URL is NOT treated as registry', async t => { + // An attacker-controlled URL that happens to contain /-/- + // must not match a registry-style allow entry. + const spoofed = node({ + name: 'trusted', + version: '1.0.0', + resolved: 'https://evil.com/-/trusted-1.0.0.tgz', + }) + // Without a path segment before /-/, the URL is not a registry tarball + // pattern. The 'trusted@1.0.0' allow must not match. + t.equal(isScriptAllowed(spoofed, { 'trusted@1.0.0': true }), null) + t.equal(isScriptAllowed(spoofed, { trusted: true }), null) + // The user can still allow this specific URL via an exact resolved match. + t.equal( + isScriptAllowed(spoofed, { 'https://evil.com/-/trusted-1.0.0.tgz': true }), + true + ) +}) + +t.test('isRegistryNode — arborist isRegistryDependency overrides URL guessing', async t => { + // A real arborist Node has isRegistryDependency. When false, the URL + // pattern is ignored entirely. + const arboristNode = { + name: 'trusted', + packageName: 'trusted', + version: '1.0.0', + resolved: 'https://registry.npmjs.org/trusted/-/trusted-1.0.0.tgz', + isRegistryDependency: false, // edges say it came from a non-registry source + } + t.equal(isScriptAllowed(arboristNode, { 'trusted@1.0.0': true }), null) +}) + +t.test('isRegistryNode — arborist isRegistryDependency true accepts even unusual URLs', async t => { + const arboristNode = { + name: 'trusted', + packageName: 'trusted', + version: '1.0.0', + resolved: 'https://internal.corp/private-registry/trusted/-/trusted-1.0.0.tgz', + isRegistryDependency: true, // edges say it came from a configured registry + } + t.equal(isScriptAllowed(arboristNode, { 'trusted@1.0.0': true }), true) +}) + +t.test('bundled deps cannot be allowlisted (Phase 1 blocks outright)', async t => { + // Bundled dependencies have inBundle=true and no independent resolved + // URL. The RFC explicitly forbids allowlisting them in Phase 1 because + // matching by name@version from the bundled tarball would reintroduce + // manifest confusion. They must always return null (unreviewed). + + const bundled = { + name: 'bundled-pkg', + packageName: 'bundled-pkg', + version: '1.0.0', + resolved: undefined, + inBundle: true, + } + + // Name-only allow: must NOT match a bundled dep. + t.equal(isScriptAllowed(bundled, { 'bundled-pkg': true }), null) + // Versioned allow: must NOT match a bundled dep. + t.equal(isScriptAllowed(bundled, { 'bundled-pkg@1.0.0': true }), null) + // Disjunction allow: must NOT match a bundled dep. + t.equal(isScriptAllowed(bundled, { 'bundled-pkg@1.0.0 || 2.0.0': true }), null) + // No policy: still null (no policy = nothing to evaluate against). + t.equal(isScriptAllowed(bundled, {}), null) + t.equal(isScriptAllowed(bundled, null), null) +}) + +t.test('bundled deps: deny entry does not match either (returns null, not false)', async t => { + // A deny entry doesn't apply to bundled deps because they're outside + // the policy scope entirely. Phase 1 blocks them via the walker, not + // via the policy. + const bundled = { + name: 'bundled-pkg', + packageName: 'bundled-pkg', + version: '1.0.0', + resolved: undefined, + inBundle: true, + } + t.equal(isScriptAllowed(bundled, { 'bundled-pkg': false }), null) +}) + +t.test('bundled dep with resolved field is still rejected', async t => { + // Defensive: even if a bundled dep somehow has a resolved URL, the + // inBundle flag wins over identity matching. + const bundledWithResolved = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz', + inBundle: true, + } + t.equal(isScriptAllowed(bundledWithResolved, { 'pkg@1.0.0': true }), null) +}) + +t.test('inBundle: false does not affect normal matching', async t => { + // Sanity check: explicit inBundle: false behaves identically to absent. + const normal = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz', + inBundle: false, + } + t.equal(isScriptAllowed(normal, { 'pkg@1.0.0': true }), true) +}) + +t.test('manifest confusion: malicious tarball self-name cannot bypass allow entry', async t => { + // A package author publishes 'naughty' to the registry but inside the + // tarball claims `package.json#name = "trusted"` and the matching + // version. The lockfile records the registry URL for 'naughty'. + // node.packageName / node.version return the tarball's claims; the + // matcher MUST ignore both and consult only the URL. + const malicious = { + name: 'naughty', // dependency edge name (consumer's deps) + packageName: 'trusted', // tarball's self-claimed name (LIE) + version: '1.0.0', // tarball's self-claimed version + resolved: 'https://registry.npmjs.org/naughty/-/naughty-1.0.0.tgz', + location: 'node_modules/naughty', + isRegistryDependency: true, + } + + // The 'trusted' allowlist entry must NOT match this node. + t.equal(isScriptAllowed(malicious, { trusted: true }), null) + t.equal(isScriptAllowed(malicious, { 'trusted@1.0.0': true }), null) + // A 'naughty' entry (the URL-derived name) DOES match. + t.equal(isScriptAllowed(malicious, { naughty: true }), true) + t.equal(isScriptAllowed(malicious, { 'naughty@1.0.0': true }), true) +}) + +t.test('manifest confusion: malicious version claim cannot satisfy version pin', async t => { + // The tarball claims version 1.0.0 but the URL records 2.0.0. The + // matcher must trust the URL. + const malicious = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', // tarball lie + resolved: 'https://registry.npmjs.org/pkg/-/pkg-2.0.0.tgz', + location: 'node_modules/pkg', + isRegistryDependency: true, + } + // Pin for the URL version matches. + t.equal(isScriptAllowed(malicious, { 'pkg@2.0.0': true }), true) + // Pin for the tarball's lie does NOT match. + t.equal(isScriptAllowed(malicious, { 'pkg@1.0.0': true }), null) +}) + +t.test('manifest confusion: scoped registry tarball', async t => { + const node = { + name: 'pkg', + packageName: 'totally-different', + version: '9.9.9', + resolved: 'https://registry.npmjs.org/@scope/real/-/real-1.0.0.tgz', + location: 'node_modules/@scope/real', + isRegistryDependency: true, + } + t.equal(isScriptAllowed(node, { '@scope/real': true }), true) + t.equal(isScriptAllowed(node, { '@scope/real@1.0.0': true }), true) + t.equal(isScriptAllowed(node, { 'totally-different': true }), null) +}) + +t.test('git committish: matching is one-directional (key is prefix of resolved SHA)', async t => { + // The lockfile records a 40-char SHA; the policy key has a (typically + // shorter) SHA. The trusted check is `nodeFullSha.startsWith(keyShortSha)`. + // The reverse direction must NOT match — a malformed lockfile that + // happens to record only a short committish must not let a longer key + // authorize it. + const shortNode = node({ + name: 'bar', + packageName: 'bar', + version: '1.0.0', + // Node committish is 8 chars; key committish below is longer. + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeef', + }) + // Key SHA strictly longer than the node committish: must not match. + t.equal( + isScriptAllowed(shortNode, { 'github:foo/bar#deadbeefcafebabe1234567890abcdef12345678': true }), + null + ) + // Node fully starts with key — matches normally. + const fullNode = node({ + name: 'bar', + packageName: 'bar', + version: '1.0.0', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + }) + t.equal(isScriptAllowed(fullNode, { 'github:foo/bar#deadbeef': true }), true) +}) From e671a8b1942ebfef48dae1a60a9db8375dbf7906 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:49:06 -0700 Subject: [PATCH 4/7] feat(install): walk tree and emit advisory warning for unreviewed install scripts The Phase 1 advisory warning. No scripts are blocked. After arb.reify() completes, reify-finish walks the actual tree for dependencies whose install-relevant lifecycle scripts are not yet covered by the allowScripts policy. The result is appended to the install output as one grouped block, not one log line per package. - workspaces/arborist/lib/install-scripts.js: per-node helper that returns the install-relevant lifecycle scripts. Covers preinstall, install, postinstall, prepare (non-registry sources only), and the synthetic 'node-gyp rebuild' detected by isNodeGypPackage from @npmcli/node-gyp. The runtime fs check is needed because the lockfile's hasInstallScript field misses packages whose only install-time work is binding.gyp. - lib/utils/check-allow-scripts.js: walks arb.actualTree.inventory and filters to unreviewed nodes. Honours --ignore-scripts and --dangerously-allow-all-scripts as full opt-outs. Treats explicit deny entries as reviewed (no warning). - lib/utils/reify-finish.js: runs the walker and passes results to reify-output as an extras payload. - lib/utils/reify-output.js: prints the grouped summary after the funding and audit messages. JSON output puts the same data on summary.unreviewedScripts. Refs: npm/rfcs#868 --- lib/utils/check-allow-scripts.js | 52 ++++ lib/utils/reify-finish.js | 4 +- lib/utils/reify-output.js | 55 +++- test/lib/utils/check-allow-scripts.js | 263 ++++++++++++++++++++ workspaces/arborist/lib/install-scripts.js | 61 +++++ workspaces/arborist/test/install-scripts.js | 157 ++++++++++++ 6 files changed, 589 insertions(+), 3 deletions(-) create mode 100644 lib/utils/check-allow-scripts.js create mode 100644 test/lib/utils/check-allow-scripts.js create mode 100644 workspaces/arborist/lib/install-scripts.js create mode 100644 workspaces/arborist/test/install-scripts.js diff --git a/lib/utils/check-allow-scripts.js b/lib/utils/check-allow-scripts.js new file mode 100644 index 0000000000000..ea4186841a6af --- /dev/null +++ b/lib/utils/check-allow-scripts.js @@ -0,0 +1,52 @@ +const isScriptAllowed = require('@npmcli/arborist/lib/script-allowed.js') +const getInstallScripts = require('@npmcli/arborist/lib/install-scripts.js') + +// Walks arb.actualTree.inventory and returns the list of dep nodes that +// have install-relevant lifecycle scripts and are not yet covered (or +// explicitly denied) by the allowScripts policy. +// +// Returns an array of `{ node, scripts }` entries. `scripts` is an object +// describing the relevant lifecycle scripts that would run. + +const checkAllowScripts = async ({ arb, npm }) => { + const ignoreScripts = !!arb.options?.ignoreScripts + const dangerouslyAllowAll = !!npm?.flatOptions?.dangerouslyAllowAllScripts + + if (ignoreScripts || dangerouslyAllowAll) { + return [] + } + + const tree = arb.actualTree + if (!tree?.inventory) { + return [] + } + + const policy = arb.options?.allowScripts || null + + const unreviewed = [] + for (const node of tree.inventory.values()) { + if (node.isProjectRoot || node.isWorkspace) { + continue + } + if (node.isLink) { + // Linked workspace dependencies are managed by the workspace owner. + continue + } + + const verdict = isScriptAllowed(node, policy) + if (verdict === true || verdict === false) { + continue + } + + const scripts = await getInstallScripts(node) + if (Object.keys(scripts).length === 0) { + continue + } + + unreviewed.push({ node, scripts }) + } + + return unreviewed +} + +module.exports = checkAllowScripts diff --git a/lib/utils/reify-finish.js b/lib/utils/reify-finish.js index 5e1330f4937bb..bb347913eebc2 100644 --- a/lib/utils/reify-finish.js +++ b/lib/utils/reify-finish.js @@ -1,4 +1,5 @@ const reifyOutput = require('./reify-output.js') +const checkAllowScripts = require('./check-allow-scripts.js') const ini = require('ini') const { writeFile } = require('node:fs/promises') const { resolve } = require('node:path') @@ -15,7 +16,8 @@ const reifyFinish = async (npm, arb) => { } } } - reifyOutput(npm, arb) + const unreviewedScripts = await checkAllowScripts({ arb, npm }) + reifyOutput(npm, arb, { unreviewedScripts }) } module.exports = reifyFinish diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js index 99427faaf6648..cda0baf896a9a 100644 --- a/lib/utils/reify-output.js +++ b/lib/utils/reify-output.js @@ -14,11 +14,25 @@ const { depth } = require('treeverse') const ms = require('ms') const npmAuditReport = require('npm-audit-report') const { readTree: getFundingInfo } = require('libnpmfund') +const { getTrustedRegistryIdentity } = require('@npmcli/arborist/lib/script-allowed.js') const auditError = require('./audit-error.js') -// TODO: output JSON if flatOptions.json is true -const reifyOutput = (npm, arb) => { +// Trusted display identity for the install-script advisory. Same idea as +// the matcher: prefer the URL-derived name. For DISPLAY purposes only +// (not policy matching), version falls back to node.version when the +// URL doesn't carry one — the user still benefits from seeing what the +// tarball claims to be, even when we cannot trust it for matching. +const trustedDisplay = (node) => { + const trusted = getTrustedRegistryIdentity(node) + /* istanbul ignore next: defensive fallbacks for nodes without name/version */ + return { + name: (trusted && trusted.name) || node.name || null, + version: (trusted && trusted.version) || node.version || null, + } +} +const reifyOutput = (npm, arb, extras = {}) => { const { diff, actualTree } = arb + const unreviewedScripts = extras.unreviewedScripts || [] // note: fails and crashes if we're running audit fix and there was an error which is a good thing, because there's no point printing all this other stuff in that case! const auditReport = auditError(npm, arb.auditReport) ? null : arb.auditReport @@ -113,11 +127,23 @@ const reifyOutput = (npm, arb) => { summary.audit = npm.command === 'audit' ? auditReport : auditReport.toJSON().metadata } + if (unreviewedScripts.length) { + summary.unreviewedScripts = unreviewedScripts.map(({ node, scripts }) => { + const { name, version } = trustedDisplay(node) + return { + name, + version, + path: node.path, + scripts, + } + }) + } output.buffer(summary) } else { packagesChangedMessage(npm, summary) packagesFundingMessage(npm, summary) printAuditReport(npm, auditReport) + unreviewedScriptsMessage(npm, unreviewedScripts) } } @@ -217,4 +243,29 @@ const packagesFundingMessage = (npm, { funding }) => { output.standard(' run `npm fund` for details') } +const unreviewedScriptsMessage = (npm, unreviewedScripts) => { + if (!unreviewedScripts.length) { + return + } + + output.standard() + const count = unreviewedScripts.length + const pkg = count === 1 ? 'package has' : 'packages have' + output.standard(`${count} ${pkg} install scripts not yet covered by allowScripts:`) + + for (const { node, scripts } of unreviewedScripts) { + const { name, version } = trustedDisplay(node) + /* istanbul ignore next: every test node has a name */ + const display = name || '' + const ver = version ? `@${version}` : '' + const events = Object.entries(scripts) + .map(([event, cmd]) => `${event}: ${cmd}`) + .join('; ') + output.standard(` ${display}${ver} (${events})`) + } + + output.standard() + output.standard('Run `npm approve-scripts --pending` to review, or `npm approve-scripts ` to allow.') +} + module.exports = reifyOutput diff --git a/test/lib/utils/check-allow-scripts.js b/test/lib/utils/check-allow-scripts.js new file mode 100644 index 0000000000000..8dea9674375df --- /dev/null +++ b/test/lib/utils/check-allow-scripts.js @@ -0,0 +1,263 @@ +const t = require('tap') + +const mockCheck = (t, mocks = {}) => + t.mock('../../../lib/utils/check-allow-scripts.js', mocks) + +// Build a minimal "arborist tree" fixture for the walker. +const arb = ({ nodes, allowScripts = null, ignoreScripts = false } = {}) => ({ + options: { allowScripts, ignoreScripts }, + actualTree: { + inventory: new Map(nodes.map((n, i) => [`node_modules/${n.name || `n${i}`}`, n])), + }, +}) + +const node = ({ + name = 'pkg', + packageName, + version = '1.0.0', + resolved, + scripts = {}, + gypfile, + path: nodePath = `/fake/${name}`, + isProjectRoot = false, + isWorkspace = false, + isLink = false, + isRegistryDependency, +} = {}) => { + const pkgName = packageName ?? name + const resolvedUrl = resolved + ?? `https://registry.npmjs.org/${pkgName}/-/${pkgName}-${version}.tgz` + // Default isRegistryDependency to match the shape of resolved: registry + // tarballs are registry, anything else (git, file, remote) is not. + const isReg = isRegistryDependency ?? /^https?:\/\/[^/]+\/.+\/-\/[^/]+-\d/.test(resolvedUrl) + return { + name, + packageName: pkgName, + version, + resolved: resolvedUrl, + location: `node_modules/${name}`, + isRegistryDependency: isReg, + path: nodePath, + isProjectRoot, + isWorkspace, + isLink, + package: { scripts, ...(gypfile !== undefined ? { gypfile } : {}) }, + } +} + +t.test('returns [] when ignoreScripts is set', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [node({ scripts: { install: 'do-stuff' } })], + ignoreScripts: true, + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('returns [] when dangerouslyAllowAllScripts is set', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ nodes: [node({ scripts: { install: 'do-stuff' } })] }), + npm: { flatOptions: { dangerouslyAllowAllScripts: true } }, + }) + t.strictSame(result, []) +}) + +t.test('skips project root, workspace, and linked nodes', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + node({ name: 'root', scripts: { install: 'x' }, isProjectRoot: true }), + node({ name: 'ws', scripts: { install: 'x' }, isWorkspace: true }), + node({ name: 'linked', scripts: { install: 'x' }, isLink: true }), + ], + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('skips nodes with no install-relevant scripts', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [node({ scripts: { test: 'jest' } })], + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('includes nodes with preinstall/install/postinstall', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + node({ name: 'a', scripts: { preinstall: 'pre' } }), + node({ name: 'b', scripts: { install: 'inst' } }), + node({ name: 'c', scripts: { postinstall: 'post' } }), + ], + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 3) + t.strictSame(result[0].scripts, { preinstall: 'pre' }) + t.strictSame(result[1].scripts, { install: 'inst' }) + t.strictSame(result[2].scripts, { postinstall: 'post' }) +}) + +t.test('prepare counts for non-registry sources only', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + // registry: prepare ignored + node({ + name: 'registry-pkg', + resolved: 'https://registry.npmjs.org/registry-pkg/-/registry-pkg-1.0.0.tgz', + scripts: { prepare: 'do' }, + }), + // git: prepare counts + node({ + name: 'git-pkg', + resolved: 'git+ssh://git@github.com/foo/bar.git#abcdef0123456789', + scripts: { prepare: 'do' }, + }), + ], + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 1) + t.equal(result[0].node.name, 'git-pkg') +}) + +t.test('detects synthetic node-gyp via binding.gyp runtime check', async t => { + const checkAllowScripts = mockCheck(t, { + '@npmcli/arborist/lib/install-scripts.js': async (n) => { + if (n.path === '/has-bindings') { + return { install: 'node-gyp rebuild' } + } + return {} + }, + }) + + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + node({ name: 'native', path: '/has-bindings' }), + node({ name: 'pure-js', path: '/no-bindings' }), + ], + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 1) + t.equal(result[0].node.name, 'native') + t.strictSame(result[0].scripts, { install: 'node-gyp rebuild' }) +}) + +t.test('skips node-gyp detection when gypfile is explicitly false', async t => { + // Mock returns no scripts to simulate the gypfile:false short-circuit + // inside getInstallScripts. + const checkAllowScripts = mockCheck(t, { + '@npmcli/arborist/lib/install-scripts.js': async () => ({}), + }) + + const result = await checkAllowScripts({ + arb: arb({ + nodes: [node({ name: 'opt-out', gypfile: false })], + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('skips approved nodes', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [node({ name: 'allowed', scripts: { install: 'x' } })], + allowScripts: { allowed: true }, + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('skips denied nodes (false counts as reviewed)', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [node({ name: 'denied', scripts: { install: 'x' } })], + allowScripts: { denied: false }, + }), + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('includes unreviewed nodes when policy is set but does not cover them', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + node({ name: 'allowed', scripts: { install: 'x' } }), + node({ name: 'unreviewed', scripts: { install: 'y' } }), + ], + allowScripts: { allowed: true }, + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 1) + t.equal(result[0].node.name, 'unreviewed') +}) + +t.test('reports every install-script node when no policy is set', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: arb({ + nodes: [ + node({ name: 'a', scripts: { install: 'x' } }), + node({ name: 'b', scripts: { postinstall: 'y' } }), + ], + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 2) +}) + +t.test('survives missing actualTree', async t => { + const checkAllowScripts = mockCheck(t) + const result = await checkAllowScripts({ + arb: { options: {} }, + npm: { flatOptions: {} }, + }) + t.strictSame(result, []) +}) + +t.test('bundled dep with install scripts is reported as unreviewed regardless of policy', async t => { + const checkAllowScripts = mockCheck(t) + const bundled = node({ + name: 'bundled-pkg', + version: '1.0.0', + resolved: undefined, + scripts: { install: 'do-stuff' }, + }) + bundled.inBundle = true + + const result = await checkAllowScripts({ + arb: arb({ + nodes: [bundled], + // Policy explicitly allows the bundled name — the matcher should + // still return null and the walker should still flag the bundled + // dep as unreviewed. + allowScripts: { 'bundled-pkg': true }, + }), + npm: { flatOptions: {} }, + }) + t.equal(result.length, 1, 'bundled dep flagged despite explicit allow entry') + t.equal(result[0].node, bundled) +}) diff --git a/workspaces/arborist/lib/install-scripts.js b/workspaces/arborist/lib/install-scripts.js new file mode 100644 index 0000000000000..9e29775d09627 --- /dev/null +++ b/workspaces/arborist/lib/install-scripts.js @@ -0,0 +1,61 @@ +const { isNodeGypPackage } = require('@npmcli/node-gyp') + +// Returns the install-relevant lifecycle scripts that would run for a +// given arborist Node, or `{}` if there are none. +// +// Includes: +// - explicit preinstall/install/postinstall +// - prepare, but only for non-registry sources (git, file, link, remote) +// - synthetic `node-gyp rebuild`, when `binding.gyp` is present on disk +// and the package does not opt out via `gypfile: false` or define its +// own install / preinstall script + +const isRegistrySource = (node) => { + // Prefer arborist's edge-based check when available — symmetric with + // isRegistryNode in script-allowed.js. A node whose edges resolve to + // non-registry specs must be treated as non-registry even if its + // resolved URL happens to share the registry tarball shape. + if (typeof node.isRegistryDependency === 'boolean') { + return node.isRegistryDependency + } + if (!node.resolved) { + // Without a resolved field or the arborist getter, fall back to + // treating the node as a registry source. Used by lockfiles produced + // with omit-lockfile-registry-resolved. + return true + } + return /^https?:\/\/[^/]+\/.+\/-\/[^/]+-\d/.test(node.resolved) +} + +const getInstallScripts = async (node) => { + const pkg = node.package || {} + const scripts = pkg.scripts || {} + const collected = {} + + if (scripts.preinstall) { + collected.preinstall = scripts.preinstall + } + if (scripts.install) { + collected.install = scripts.install + } + if (scripts.postinstall) { + collected.postinstall = scripts.postinstall + } + if (scripts.prepare && !isRegistrySource(node)) { + collected.prepare = scripts.prepare + } + + const hasExplicitGypGate = !!(collected.preinstall || collected.install) + if ( + !hasExplicitGypGate && + pkg.gypfile !== false && + await isNodeGypPackage(node.path).catch(() => false) + ) { + collected.install = 'node-gyp rebuild' + } + + return collected +} + +module.exports = getInstallScripts +module.exports.getInstallScripts = getInstallScripts diff --git a/workspaces/arborist/test/install-scripts.js b/workspaces/arborist/test/install-scripts.js new file mode 100644 index 0000000000000..3926e3386df36 --- /dev/null +++ b/workspaces/arborist/test/install-scripts.js @@ -0,0 +1,157 @@ +const t = require('tap') + +const mockGetInstallScripts = (t, isNodeGypResult = () => false) => + t.mock('../lib/install-scripts.js', { + '@npmcli/node-gyp': { + isNodeGypPackage: async (path) => { + if (typeof isNodeGypResult === 'function') { + return isNodeGypResult(path) + } + return !!isNodeGypResult + }, + }, + }) + +const node = ({ + scripts = {}, + gypfile, + resolved = 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz', + path = '/fake', +} = {}) => ({ + resolved, + path, + package: { scripts, ...(gypfile !== undefined ? { gypfile } : {}) }, +}) + +t.test('collects preinstall, install, postinstall', async t => { + const getInstallScripts = mockGetInstallScripts(t) + t.strictSame( + await getInstallScripts(node({ scripts: { preinstall: 'pre' } })), + { preinstall: 'pre' } + ) + t.strictSame( + await getInstallScripts(node({ scripts: { install: 'inst' } })), + { install: 'inst' } + ) + t.strictSame( + await getInstallScripts(node({ scripts: { postinstall: 'post' } })), + { postinstall: 'post' } + ) + t.strictSame( + await getInstallScripts(node({ scripts: {} })), + {} + ) +}) + +t.test('ignores unrelated scripts', async t => { + const getInstallScripts = mockGetInstallScripts(t) + t.strictSame( + await getInstallScripts(node({ scripts: { test: 'x', build: 'y' } })), + {} + ) +}) + +t.test('prepare only counts for non-registry sources', async t => { + const getInstallScripts = mockGetInstallScripts(t) + // registry: prepare ignored + t.strictSame( + await getInstallScripts(node({ + scripts: { prepare: 'do' }, + resolved: 'https://registry.npmjs.org/x/-/x-1.0.0.tgz', + })), + {} + ) + // git: prepare counts + t.strictSame( + await getInstallScripts(node({ + scripts: { prepare: 'do' }, + resolved: 'git+ssh://git@github.com/foo/bar.git#abc', + })), + { prepare: 'do' } + ) + // file: prepare counts + t.strictSame( + await getInstallScripts(node({ + scripts: { prepare: 'do' }, + resolved: 'file:../local', + })), + { prepare: 'do' } + ) +}) + +t.test('synthetic node-gyp install detected via binding.gyp', async t => { + const getInstallScripts = mockGetInstallScripts(t, () => true) + t.strictSame( + await getInstallScripts(node()), + { install: 'node-gyp rebuild' } + ) +}) + +t.test('synthetic node-gyp suppressed when gypfile: false', async t => { + const getInstallScripts = mockGetInstallScripts(t, () => true) + t.strictSame( + await getInstallScripts(node({ gypfile: false })), + {} + ) +}) + +t.test('synthetic node-gyp suppressed when explicit install is present', async t => { + const getInstallScripts = mockGetInstallScripts(t, () => true) + t.strictSame( + await getInstallScripts(node({ scripts: { install: 'real-install' } })), + { install: 'real-install' } + ) +}) + +t.test('synthetic node-gyp suppressed when explicit preinstall is present', async t => { + const getInstallScripts = mockGetInstallScripts(t, () => true) + t.strictSame( + await getInstallScripts(node({ scripts: { preinstall: 'real-pre' } })), + { preinstall: 'real-pre' } + ) +}) + +t.test('node-gyp detection error is treated as not-gyp', async t => { + const getInstallScripts = t.mock('../lib/install-scripts.js', { + '@npmcli/node-gyp': { + isNodeGypPackage: async () => { + throw new Error('fs blew up') + }, + }, + }) + t.strictSame(await getInstallScripts(node()), {}) +}) + +t.test('missing resolved treated as registry (prepare ignored)', async t => { + const getInstallScripts = mockGetInstallScripts(t) + t.strictSame( + await getInstallScripts(node({ scripts: { prepare: 'do' }, resolved: undefined })), + {} + ) +}) + +t.test('prepare counts for non-registry deps even when resolved URL looks registry-like', async t => { + const getInstallScripts = mockGetInstallScripts(t) + // A fork hosted at a URL that happens to follow the npm registry tarball + // shape. Arborist's edge-based check (isRegistryDependency=false) is + // authoritative — prepare must NOT be skipped just because the URL pattern + // matches. + const nonRegistry = { + resolved: 'https://corp.example.com/mirror/sharp/-/sharp-1.0.0.tgz', + path: '/fake', + isRegistryDependency: false, + package: { scripts: { prepare: 'do' } }, + } + t.strictSame(await getInstallScripts(nonRegistry), { prepare: 'do' }) +}) + +t.test('prepare is skipped for registry deps regardless of resolved URL shape', async t => { + const getInstallScripts = mockGetInstallScripts(t) + const registryNode = { + resolved: 'https://internal.corp/private-registry/sharp/-/sharp-1.0.0.tgz', + path: '/fake', + isRegistryDependency: true, + package: { scripts: { prepare: 'do' } }, + } + t.strictSame(await getInstallScripts(registryNode), {}) +}) From a39d41963d37d3976d2c62cb41a3c327d1f1e021 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:58:25 -0700 Subject: [PATCH 5/7] feat: add npm approve-scripts and npm deny-scripts commands Both share an implementation in lib/utils/allow-scripts-cmd.js; the files in lib/commands/ are thin shims that set verb = 'approve' or 'deny'. - npm approve-scripts : writes 'pkg@version': true (pinned) - npm approve-scripts --no-pin : writes 'pkg': true (name-only) - npm approve-scripts --all: approves every unreviewed install-script package in the resolved actual tree - npm approve-scripts --pending: read-only walk, lists unreviewed packages without modifying package.json - npm deny-scripts : writes 'pkg': false. Always name-only, regardless of --pin, per the RFC's asymmetric-pin rule. - npm deny-scripts --all: denies every unreviewed install-script package The shared writer in lib/utils/allow-scripts-writer.js implements the RFC's pin-mismatch table as pure functions: applyApprovalForPackage(existing, nodes, { pin }) and applyDenyForPackage(existing, nodes). Grouping by package matters because a per-node API can't tell a stale pin from a newly installed version; a per-package one can. Two new flags registered in workspaces/config/lib/definitions (--all reuses the existing global definition): - --pending: read-only mode for approve-scripts - --pin: control pin behaviour for approve-scripts (default true) Includes docs/lib/content/commands/{npm-approve-scripts,npm-deny-scripts}.md. Refs: npm/rfcs#868 --- .../content/commands/npm-approve-scripts.md | 73 ++ docs/lib/content/commands/npm-deny-scripts.md | 58 + lib/commands/approve-scripts.js | 13 + lib/commands/deny-scripts.js | 13 + lib/utils/allow-scripts-cmd.js | 248 ++++ lib/utils/allow-scripts-writer.js | 301 +++++ lib/utils/cmd-list.js | 2 + .../test/lib/commands/config.js.test.cjs | 4 + tap-snapshots/test/lib/docs.js.test.cjs | 109 ++ test/lib/commands/approve-scripts.js | 562 +++++++++ test/lib/commands/deny-scripts.js | 112 ++ test/lib/utils/allow-scripts-writer.js | 1046 +++++++++++++++++ .../config/lib/definitions/definitions.js | 21 + 13 files changed, 2562 insertions(+) create mode 100644 docs/lib/content/commands/npm-approve-scripts.md create mode 100644 docs/lib/content/commands/npm-deny-scripts.md create mode 100644 lib/commands/approve-scripts.js create mode 100644 lib/commands/deny-scripts.js create mode 100644 lib/utils/allow-scripts-cmd.js create mode 100644 lib/utils/allow-scripts-writer.js create mode 100644 test/lib/commands/approve-scripts.js create mode 100644 test/lib/commands/deny-scripts.js create mode 100644 test/lib/utils/allow-scripts-writer.js diff --git a/docs/lib/content/commands/npm-approve-scripts.md b/docs/lib/content/commands/npm-approve-scripts.md new file mode 100644 index 0000000000000..64936110adfae --- /dev/null +++ b/docs/lib/content/commands/npm-approve-scripts.md @@ -0,0 +1,73 @@ +--- +title: npm-approve-scripts +section: 1 +description: Approve install scripts for specific dependencies +--- + +### Synopsis + + + +### Description + +Manages the `allowScripts` field in your project's `package.json`, which +records which of your dependencies are permitted to run install scripts +(`preinstall`, `install`, `postinstall`, and `prepare` for non-registry +sources). This command is the recommended way to maintain that field. + +In the current release, this field is advisory: install scripts still run +by default, but installs print a list of packages whose scripts have not +been reviewed. A future release will block unreviewed install scripts. + +There are three modes: + +```bash +npm approve-scripts [ ...] +npm approve-scripts --all +npm approve-scripts --pending +``` + +`` matches every installed version of that package. By default the +command writes pinned entries (`pkg@1.2.3`), which keep their approval +narrowed to the specific version you reviewed. Pass `--no-pin` to write +name-only entries that allow any future version. + +`--all` approves every package with unreviewed install scripts in one go. + +`--pending` is read-only: it lists every package whose install scripts +are not yet covered by `allowScripts`, without modifying `package.json`. + +`approve-scripts` honours the asymmetric pin rule: if you re-approve a +package whose installed version has changed, the existing pin is rewritten +to track the new installed version. Multi-version statements +(`pkg@1 || 2`) are left alone, since they likely capture intent that +the command cannot infer. Existing `false` entries always win; +`approve-scripts` will not silently re-allow a package you previously +denied. + +### Examples + +```bash +# Approve all currently-installed install scripts after reviewing them +npm approve-scripts --all + +# Approve specific packages, pinned to their installed version +npm approve-scripts canvas sharp + +# Approve name-only (any version of this package is allowed) +npm approve-scripts --no-pin canvas + +# Preview which packages still need review +npm approve-scripts --pending +``` + +### Configuration + + + +### See Also + +* [npm deny-scripts](/commands/npm-deny-scripts) +* [npm install](/commands/npm-install) +* [npm rebuild](/commands/npm-rebuild) +* [package.json](/configuring-npm/package-json) diff --git a/docs/lib/content/commands/npm-deny-scripts.md b/docs/lib/content/commands/npm-deny-scripts.md new file mode 100644 index 0000000000000..9e697828adf59 --- /dev/null +++ b/docs/lib/content/commands/npm-deny-scripts.md @@ -0,0 +1,58 @@ +--- +title: npm-deny-scripts +section: 1 +description: Deny install scripts for specific dependencies +--- + +### Synopsis + + + +### Description + +The companion command to [`npm approve-scripts`](/commands/npm-approve-scripts). +Writes `false` entries into the `allowScripts` field of your project's +`package.json`, recording that a dependency must not run install scripts +even if a future version would otherwise be eligible. + +In the current release, install scripts still run by default, so `deny-scripts` +only affects how installs of denied packages are reported. A future release +will block unreviewed install scripts and respect deny entries at install +time. + +```bash +npm deny-scripts [ ...] +npm deny-scripts --all +``` + +`` matches every installed version of that package. Denies are always +written name-only (`"pkg": false`), regardless of `--pin`. Pinning a deny +to a specific version would silently re-allow scripts for any other version +of the same package, which defeats the purpose; the command picks the +safer default for you. + +`--all` denies every package with unreviewed install scripts. + +If a `true` (pinned or name-only) entry exists for a package and you then +deny it, the existing allow entries are removed so the name-only deny is +unambiguous. + +### Examples + +```bash +# Deny a specific package outright +npm deny-scripts telemetry-pkg + +# Deny everything that has install scripts and isn't already approved +npm deny-scripts --all +``` + +### Configuration + + + +### See Also + +* [npm approve-scripts](/commands/npm-approve-scripts) +* [npm install](/commands/npm-install) +* [package.json](/configuring-npm/package-json) diff --git a/lib/commands/approve-scripts.js b/lib/commands/approve-scripts.js new file mode 100644 index 0000000000000..7b9f5dec64715 --- /dev/null +++ b/lib/commands/approve-scripts.js @@ -0,0 +1,13 @@ +const AllowScriptsCmd = require('../utils/allow-scripts-cmd.js') + +class ApproveScripts extends AllowScriptsCmd { + static description = 'Approve install scripts for specific dependencies' + static name = 'approve-scripts' + static usage = [' [ ...]', '--all', '--pending'] + + get verb () { + return 'approve' + } +} + +module.exports = ApproveScripts diff --git a/lib/commands/deny-scripts.js b/lib/commands/deny-scripts.js new file mode 100644 index 0000000000000..351630f2fb7ea --- /dev/null +++ b/lib/commands/deny-scripts.js @@ -0,0 +1,13 @@ +const AllowScriptsCmd = require('../utils/allow-scripts-cmd.js') + +class DenyScripts extends AllowScriptsCmd { + static description = 'Deny install scripts for specific dependencies' + static name = 'deny-scripts' + static usage = [' [ ...]', '--all'] + + get verb () { + return 'deny' + } +} + +module.exports = DenyScripts diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js new file mode 100644 index 0000000000000..3404baa2f1594 --- /dev/null +++ b/lib/utils/allow-scripts-cmd.js @@ -0,0 +1,248 @@ +const { log, output } = require('proc-log') +const pkgJson = require('@npmcli/package-json') +const { getTrustedRegistryIdentity } = require('@npmcli/arborist/lib/script-allowed.js') +const checkAllowScripts = require('./check-allow-scripts.js') +const resolveAllowScripts = require('./resolve-allow-scripts.js') +const { + applyApprovalForPackage, + applyDenyForPackage, + nameKeyFor, +} = require('./allow-scripts-writer.js') +const BaseCommand = require('../base-cmd.js') + +// Trusted display identity for `npm approve-scripts --pending` output. +// Same idea as the matcher: prefer the URL-derived name. For DISPLAY +// purposes only (not policy matching), version falls back to node.version +// when the URL doesn't carry one. +const trustedDisplay = (node) => { + const trusted = getTrustedRegistryIdentity(node) + /* istanbul ignore next: defensive fallbacks for nodes without name/version */ + return { + name: (trusted && trusted.name) || node.name || null, + version: (trusted && trusted.version) || node.version || null, + } +} + +// Shared implementation for `npm approve-scripts` and `npm deny-scripts`. +// Subclasses set `verb` to `'approve'` or `'deny'`. +class AllowScriptsCmd extends BaseCommand { + static params = ['all', 'pending', 'pin', 'json'] + static ignoreImplicitWorkspace = false + + // Subclasses set this. + get verb () { + throw new Error('verb must be implemented by subclass') + } + + async exec (args) { + if (this.npm.global) { + throw Object.assign( + new Error(`\`npm ${this.constructor.name}\` does not work for global installs`), + { code: 'EGLOBAL' } + ) + } + + const pending = !!this.npm.config.get('pending') + const all = !!this.npm.config.get('all') + + if (pending && (args.length > 0 || all)) { + throw this.usageError( + '`--pending` cannot be combined with positional arguments or `--all`.' + ) + } + if (!pending && !all && args.length === 0) { + throw this.usageError() + } + if (this.verb === 'deny' && pending) { + throw this.usageError('`npm deny-scripts --pending` is not supported.') + } + + const Arborist = require('@npmcli/arborist') + const { policy } = await resolveAllowScripts(this.npm) + const arb = new Arborist({ + ...this.npm.flatOptions, + path: this.npm.prefix, + allowScripts: policy, + }) + await arb.loadActual() + + const unreviewed = await checkAllowScripts({ arb, npm: this.npm }) + + if (pending) { + return this.runPending(unreviewed) + } + + if (all) { + return this.runAll(unreviewed) + } + + return this.runPositional(args, arb) + } + + runPending (unreviewed) { + if (unreviewed.length === 0) { + output.standard('No packages with unreviewed install scripts.') + return + } + const count = unreviewed.length + const has = count === 1 ? 'has' : 'have' + const pkg = count === 1 ? 'package' : 'packages' + output.standard( + `${count} ${pkg} ${has} install scripts not yet covered by allowScripts:` + ) + for (const { node, scripts } of unreviewed) { + const { name, version } = trustedDisplay(node) + /* istanbul ignore next: every test node has a name */ + const display = name || '' + const ver = version ? `@${version}` : '' + const events = Object.entries(scripts) + .map(([event, cmd]) => `${event}: ${cmd}`) + .join('; ') + output.standard(` ${display}${ver} (${events})`) + } + output.standard('') + output.standard( + 'Run `npm approve-scripts ` to allow, or `npm deny-scripts ` to deny.' + ) + } + + async runAll (unreviewed) { + if (unreviewed.length === 0) { + output.standard('No packages with unreviewed install scripts.') + return + } + // Bundled dependencies cannot be allowlisted in Phase 1 (RFC defers + // this to a follow-up because matching by name@version from the + // bundled tarball would reintroduce manifest confusion). Exclude + // them from `--all` so we don't silently write a policy entry under + // attacker-controlled identity. + const candidates = unreviewed.filter(({ node }) => !node.inBundle) + const skipped = unreviewed.length - candidates.length + if (skipped > 0) { + /* istanbul ignore next: plural variant covered separately */ + const noun = skipped === 1 ? 'dependency' : 'dependencies' + log.warn( + this.logTitle, + `Skipping ${skipped} bundled ${noun}; bundled deps with install ` + + 'scripts cannot be allowlisted in this release.' + ) + } + if (candidates.length === 0) { + output.standard('No packages eligible for approval.') + return + } + const groups = this.groupByPackage(candidates.map(({ node }) => node)) + await this.writePolicyChanges(groups) + } + + async runPositional (args, arb) { + const matched = this.findNodesForArgs(args, arb) + const groups = this.groupByPackage(matched) + if (Object.keys(groups).length === 0) { + throw Object.assign( + new Error(`No installed packages match: ${args.join(', ')}`), + { code: 'ENOMATCH' } + ) + } + await this.writePolicyChanges(groups) + } + + findNodesForArgs (args, arb) { + // Match positional args against each node's trusted name. Registry + // deps use the URL-derived name; non-registry deps fall back to the + // dependency edge name. Bundled deps are excluded for the same reason + // as --all. + const wanted = new Set(args) + const matched = [] + for (const node of arb.actualTree.inventory.values()) { + if (node.isProjectRoot || node.isWorkspace || node.inBundle) { + continue + } + const { name } = trustedDisplay(node) + if (name && wanted.has(name)) { + matched.push(node) + } + } + return matched + } + + get logTitle () { + return this.constructor.name.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase() + } + + groupByPackage (nodes) { + const groups = {} + for (const node of nodes) { + const key = nameKeyFor(node) + /* istanbul ignore if: callers prefilter via inBundle and trustedDisplay so untrusted nodes don't reach here */ + if (!key) { + log.warn( + this.logTitle, + `skipping ${node.name || ''}: no trusted identity for policy key` + ) + continue + } + if (!groups[key]) { + groups[key] = [] + } + groups[key].push(node) + } + return groups + } + + async writePolicyChanges (groups) { + const pin = this.npm.config.get('pin') !== false + + const pkg = await pkgJson.normalize(this.npm.prefix) + const content = pkg.content + const existing = content.allowScripts && typeof content.allowScripts === 'object' + ? content.allowScripts + : {} + + let updated = existing + const summary = [] + + for (const [name, nodes] of Object.entries(groups)) { + const result = this.verb === 'approve' + ? applyApprovalForPackage(updated, nodes, { pin }) + : applyDenyForPackage(updated, nodes) + + if (result.warning) { + log.warn(this.logTitle, result.warning) + } + updated = result.allowScripts + summary.push({ name, changes: result.changes }) + } + + if (updated !== existing) { + pkg.update({ allowScripts: updated }) + await pkg.save() + } + + this.printSummary(summary) + } + + printSummary (summary) { + if (this.npm.flatOptions.json) { + output.buffer({ allowScripts: summary }) + return + } + const verb = this.verb === 'approve' ? 'Approved' : 'Denied' + let touched = 0 + for (const { name, changes } of summary) { + if (changes.length === 0) { + continue + } + touched++ + output.standard(`${verb} ${name}:`) + for (const { key, change } of changes) { + output.standard(` ${change} ${key}`) + } + } + if (touched === 0) { + output.standard(`Nothing to ${this.verb}; allowScripts unchanged.`) + } + } +} + +module.exports = AllowScriptsCmd diff --git a/lib/utils/allow-scripts-writer.js b/lib/utils/allow-scripts-writer.js new file mode 100644 index 0000000000000..3105692f6a988 --- /dev/null +++ b/lib/utils/allow-scripts-writer.js @@ -0,0 +1,301 @@ +const npa = require('npm-package-arg') +const { getTrustedRegistryIdentity } = require('@npmcli/arborist/lib/script-allowed.js') + +// Pure helpers that implement the RFC's pin-mismatch table for +// `npm approve-scripts` and `npm deny-scripts`. +// +// Approving writes either `"": true` or `"": true` to the +// project's `allowScripts` field, depending on `--pin` and the currently +// installed versions. +// +// Denying always writes `"": false`, regardless of `--pin`, per the +// RFC's asymmetric-pin rule. + +// Convert an arborist Node into the spec string used for a versioned policy +// entry. Returns `null` if the node cannot be represented as a versioned key +// derived from trusted sources (lockfile URL for registry, hosted shortcut +// for git, the resolved file path for local installs). Never falls back to +// `node.packageName` / `node.version`, which are tarball-controlled. +const versionedKeyFor = (node) => { + if (!node) { + return null + } + /* istanbul ignore next: callers guarantee a string resolved */ + const resolved = typeof node.resolved === 'string' ? node.resolved : '' + if (resolved.startsWith('git')) { + try { + const parsed = npa(resolved) + if (parsed.hosted) { + const committish = parsed.gitCommittish || parsed.hosted.committish + const base = parsed.hosted.shortcut({ noCommittish: true }) + return committish ? `${base}#${committish}` : base + } + } catch { + /* istanbul ignore next: npa already parsed this string in keyTargetsNode */ + return null + } + return null + } + if (/^https?:\/\//.test(resolved)) { + const trusted = getTrustedRegistryIdentity(node) + return trusted && trusted.version ? `${trusted.name}@${trusted.version}` : null + } + /* istanbul ignore next: 'file:' and '/' branches are each covered separately */ + if (resolved.startsWith('file:') || resolved.startsWith('/')) { + return resolved + } + // No trusted source. Refuse to compose a key from attacker-controlled + // `node.packageName` / `node.version`. + /* istanbul ignore next: callers filter out non-registry/non-file nodes before reaching this fallback */ + return null +} + +// Convert an arborist Node into the spec string used for a name-only policy +// entry. Same trust rules as versionedKeyFor — returns `null` rather than +// falling back to tarball-controlled fields. +const nameKeyFor = (node) => { + if (!node) { + return null + } + /* istanbul ignore next: callers guarantee a string resolved */ + const resolved = typeof node.resolved === 'string' ? node.resolved : '' + if (resolved.startsWith('git')) { + try { + const parsed = npa(resolved) + if (parsed.hosted) { + return parsed.hosted.shortcut({ noCommittish: true }) + } + } catch { + /* istanbul ignore next: npa already parsed this string in keyTargetsNode */ + return null + } + return null + } + if (resolved.startsWith('file:') || resolved.startsWith('/')) { + return resolved + } + // Registry deps: only the URL-derived (or edges-derived, in the + // omit-lockfile case) trusted name is acceptable. + const trusted = getTrustedRegistryIdentity(node) + return trusted ? trusted.name : null +} + +const isSingleVersionPin = (key) => { + try { + const parsed = npa(key) + return parsed.type === 'version' + } catch { + return false + } +} + +// Build the warning string emitted when an existing deny entry blocks +// an approval. Per RFC, a name-only deny ("pkg": false) is widest and +// the only remediation is to remove the entry. A versioned deny +// ("pkg@1.2.3": false or a disjunction) blocks only specific versions; +// the user can either widen it via `npm deny-scripts ` or remove +// it to approve the currently-installed version only. +const denyWarning = (key, subject, name) => { + if (isNameOnlyKey(key)) { + return `${key} is denied; remove the entry from allowScripts to approve ${subject}.` + } + /* istanbul ignore next: name fallback is defensive; callers pass nameKeyFor(sample) */ + const widenTarget = name || 'this package' + return `${key} is a versioned deny; run \`npm deny-scripts ${widenTarget}\` ` + + `to widen the deny to all versions of ${widenTarget}, or remove the entry ` + + `to approve ${subject}.` +} + +const isNameOnlyKey = (key) => { + try { + const parsed = npa(key) + if (parsed.type === 'tag') { + return true + } + if (parsed.type === 'range') { + return parsed.fetchSpec === '*' + || parsed.rawSpec === '' + || parsed.rawSpec === '*' + } + return false + } catch { + /* istanbul ignore next: keys reaching this helper have already parsed via keyTargetsNode */ + return false + } +} + +// Does this policy key target this node by identity (ignoring the +// allow/deny value)? +const keyTargetsNode = (key, node) => { + let parsed + try { + parsed = npa(key) + } catch { + return false + } + switch (parsed.type) { + case 'tag': + case 'range': + case 'version': { + // Compare against the URL-derived trusted name, never the tarball's + // self-reported name. + const trusted = getTrustedRegistryIdentity(node) + const name = trusted ? trusted.name : node.name + return name === parsed.name + } + case 'git': { + let resolvedParsed + try { + resolvedParsed = node.resolved ? npa(node.resolved) : null + } catch { + return false + } + const keyHost = parsed.hosted?.ssh({ noCommittish: true }) + const nodeHost = resolvedParsed?.hosted?.ssh({ noCommittish: true }) + return !!(keyHost && nodeHost && keyHost === nodeHost) + } + case 'file': + case 'directory': + case 'remote': + return node.resolved === parsed.saveSpec || node.resolved === parsed.fetchSpec + default: + return false + } +} + +// Apply approvals for all currently-installed versions of a single package. +// +// `nodes` must all share an identity (same package name for registry deps, +// or same hosted shortcut for git deps, etc.). The caller is responsible +// for grouping nodes correctly. +// +// Returns `{ allowScripts, changes, warning }` where: +// - `allowScripts` is the new object (the input is never mutated) +// - `changes` is a list of `{ key, change }` entries describing edits +// - `warning` is an optional message to surface to the user +const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => { + const allowScripts = { ...(existing || {}) } + const changes = [] + + if (!Array.isArray(nodes) || nodes.length === 0) { + return { allowScripts, changes } + } + + const sample = nodes[0] + const name = nameKeyFor(sample) + + // Deny-wins: any existing false that targets any installed version aborts. + for (const node of nodes) { + for (const [key, value] of Object.entries(allowScripts)) { + if (value === false && keyTargetsNode(key, node)) { + return { + allowScripts, + changes, + warning: denyWarning(key, subject, name), + } + } + } + } + + if (!pin) { + // Name-only mode: collapse any single-version pins for this package + // into a single name-only entry. + for (const key of Object.keys(allowScripts)) { + if ( + keyTargetsNode(key, sample) && + key !== name && + isSingleVersionPin(key) && + allowScripts[key] === true + ) { + delete allowScripts[key] + } + } + + if (name && allowScripts[name] !== true) { + allowScripts[name] = true + changes.push({ key: name, change: 'added' }) + } + return { allowScripts, changes } + } + + // Pin mode. For each currently installed version, write a single-version + // pin if one is not already in place. Stale single-version pins for this + // package are removed. Per the RFC's pin-mismatch table, an existing + // name-only entry (`pkg: true`) is replaced by `pkg@x.y.z: true` once + // every installed version has a pin. + const installedKeys = new Set(nodes.map(versionedKeyFor).filter(Boolean)) + + for (const key of Object.keys(allowScripts)) { + if ( + keyTargetsNode(key, sample) && + isSingleVersionPin(key) && + allowScripts[key] === true && + !installedKeys.has(key) + ) { + delete allowScripts[key] + changes.push({ key, change: 'removed-stale' }) + } + } + + for (const key of installedKeys) { + if (allowScripts[key] !== true) { + allowScripts[key] = true + changes.push({ key, change: 'added' }) + } + } + + // Upgrade: drop the name-only entry once every installed version has a + // pin. The operation is convergent: running the command twice produces + // the same shape regardless of the starting state. + if ( + installedKeys.size > 0 && + name && + !installedKeys.has(name) && + allowScripts[name] === true + ) { + delete allowScripts[name] + changes.push({ key: name, change: 'replaced-by-pin' }) + } + + return { allowScripts, changes } +} + +// Apply a deny for a single package. Always name-only; ignores `--pin`. +const applyDenyForPackage = (existing, nodes) => { + const allowScripts = { ...(existing || {}) } + const changes = [] + + if (!Array.isArray(nodes) || nodes.length === 0) { + return { allowScripts, changes } + } + + const sample = nodes[0] + const name = nameKeyFor(sample) + if (!name) { + return { allowScripts, changes } + } + + // Drop any pinned allow entries for this package: the name-only deny + // overrides them anyway, and leaving them in place is confusing. + for (const key of Object.keys(allowScripts)) { + if (keyTargetsNode(key, sample) && key !== name) { + delete allowScripts[key] + changes.push({ key, change: 'removed-pinned-allow' }) + } + } + + if (allowScripts[name] !== false) { + allowScripts[name] = false + changes.push({ key: name, change: 'added' }) + } + return { allowScripts, changes } +} + +module.exports = { + applyApprovalForPackage, + applyDenyForPackage, + versionedKeyFor, + nameKeyFor, + keyTargetsNode, + isSingleVersionPin, +} diff --git a/lib/utils/cmd-list.js b/lib/utils/cmd-list.js index 700e3902daa3f..f2f29f2dc38fd 100644 --- a/lib/utils/cmd-list.js +++ b/lib/utils/cmd-list.js @@ -4,6 +4,7 @@ const abbrev = require('abbrev') // Please keep this list sorted alphabetically const commands = [ 'access', + 'approve-scripts', 'audit', 'bugs', 'cache', @@ -11,6 +12,7 @@ const commands = [ 'completion', 'config', 'dedupe', + 'deny-scripts', 'deprecate', 'diff', 'dist-tag', diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index 19916c8b5940a..13584c4661a22 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -131,6 +131,8 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "pack-destination": ".", "packages": [], "parseable": false, + "pending": false, + "pin": true, "prefer-dedupe": false, "prefer-offline": false, "prefer-online": false, @@ -319,6 +321,8 @@ packages-all = false packages-and-scopes-permission = null parseable = false password = (protected) +pending = false +pin = true prefer-dedupe = false prefer-offline = false prefer-online = false diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 6a7d6367c0c33..336ed4c501ffb 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -97,6 +97,7 @@ Object { exports[`test/lib/docs.js TAP command list > commands 1`] = ` Array [ "access", + "approve-scripts", "audit", "bugs", "cache", @@ -104,6 +105,7 @@ Array [ "completion", "config", "dedupe", + "deny-scripts", "deprecate", "diff", "dist-tag", @@ -1462,6 +1464,29 @@ tokens, though it's generally safer to be prompted for it. +#### \`pending\` + +* Default: false +* Type: Boolean + +List packages with install scripts that are not yet covered by the +\`allowScripts\` policy, without modifying \`package.json\`. Only meaningful for +\`npm approve-scripts\`. + + + +#### \`pin\` + +* Default: true +* Type: Boolean + +Write pinned (\`pkg@version\`) entries when approving install scripts. Set to +\`false\` to write name-only entries that allow any version. Has no effect on +\`npm deny-scripts\`, which always writes name-only entries regardless of this +setting. + + + #### \`prefer-dedupe\` * Default: false @@ -2462,6 +2487,8 @@ Array [ "pack-destination", "packages", "parseable", + "pending", + "pin", "prefer-dedupe", "prefer-offline", "prefer-online", @@ -2624,6 +2651,8 @@ Array [ "pack-destination", "packages", "parseable", + "pending", + "pin", "prefer-dedupe", "prefer-offline", "prefer-online", @@ -2805,6 +2834,8 @@ Object { "packDestination": ".", "parseable": false, "password": null, + "pending": false, + "pin": true, "preferDedupe": false, "preferOffline": false, "preferOnline": false, @@ -2945,6 +2976,46 @@ Note: This command is unaware of workspaces. #### \`registry\` ` +exports[`test/lib/docs.js TAP usage approve-scripts > must match snapshot 1`] = ` +Approve install scripts for specific dependencies + +Usage: +npm approve-scripts [ ...] +npm approve-scripts --all +npm approve-scripts --pending + +Options: +[-a|--all] [--pending] [--no-pin] [--json] + + -a|--all + When running \`npm outdated\` and \`npm ls\`, setting \`--all\` will show + + --pending + List packages with install scripts that are not yet covered by the + + --pin + Write pinned (\`pkg@version\`) entries when approving install scripts. + + --json + Whether or not to output JSON data, rather than the normal output. + + +Run "npm help approve-scripts" for more info + +\`\`\`bash +npm approve-scripts [ ...] +npm approve-scripts --all +npm approve-scripts --pending +\`\`\` + +Note: This command is unaware of workspaces. + +#### \`all\` +#### \`pending\` +#### \`pin\` +#### \`json\` +` + exports[`test/lib/docs.js TAP usage audit > must match snapshot 1`] = ` Run a security audit @@ -3419,6 +3490,44 @@ alias: ddp #### \`install-links\` ` +exports[`test/lib/docs.js TAP usage deny-scripts > must match snapshot 1`] = ` +Deny install scripts for specific dependencies + +Usage: +npm deny-scripts [ ...] +npm deny-scripts --all + +Options: +[-a|--all] [--pending] [--no-pin] [--json] + + -a|--all + When running \`npm outdated\` and \`npm ls\`, setting \`--all\` will show + + --pending + List packages with install scripts that are not yet covered by the + + --pin + Write pinned (\`pkg@version\`) entries when approving install scripts. + + --json + Whether or not to output JSON data, rather than the normal output. + + +Run "npm help deny-scripts" for more info + +\`\`\`bash +npm deny-scripts [ ...] +npm deny-scripts --all +\`\`\` + +Note: This command is unaware of workspaces. + +#### \`all\` +#### \`pending\` +#### \`pin\` +#### \`json\` +` + exports[`test/lib/docs.js TAP usage deprecate > must match snapshot 1`] = ` Deprecate a version of a package diff --git a/test/lib/commands/approve-scripts.js b/test/lib/commands/approve-scripts.js new file mode 100644 index 0000000000000..9afa2c85bdc95 --- /dev/null +++ b/test/lib/commands/approve-scripts.js @@ -0,0 +1,562 @@ +const t = require('tap') +const fs = require('node:fs') +const { resolve } = require('node:path') +const _mockNpm = require('../../fixtures/mock-npm') + +const mockNpm = async (t, opts = {}) => { + return _mockNpm(t, opts) +} + +const setupProject = ({ allowScripts, withScripts = ['canvas'] } = {}) => { + const pkg = { + name: 'host', + version: '1.0.0', + dependencies: Object.fromEntries(withScripts.map((n) => [n, '*'])), + } + if (allowScripts !== undefined) { + pkg.allowScripts = allowScripts + } + + const lockPackages = { '': pkg } + const nodeModules = {} + for (const name of withScripts) { + const tarUrl = `https://registry.npmjs.org/${name}/-/${name}-1.0.0.tgz` + nodeModules[name] = { + 'package.json': JSON.stringify({ + name, + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + } + lockPackages[`node_modules/${name}`] = { + version: '1.0.0', + resolved: tarUrl, + hasInstallScript: true, + } + } + + return { + 'package.json': JSON.stringify(pkg, null, 2), + 'package-lock.json': JSON.stringify({ + name: pkg.name, + version: pkg.version, + lockfileVersion: 3, + requires: true, + packages: lockPackages, + }), + node_modules: nodeModules, + } +} + +t.test('approve-scripts --pending lists unreviewed packages', async t => { + const { npm, joinedOutput } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), + config: { pending: true }, + }) + await npm.exec('approve-scripts', []) + const out = joinedOutput() + t.match(out, /2 packages have install scripts not yet covered/) + t.match(out, /canvas@1\.0\.0/) + t.match(out, /sharp@1\.0\.0/) +}) + +t.test('approve-scripts --pending with no unreviewed says so', async t => { + const { npm, joinedOutput } = await mockNpm(t, { + prefixDir: setupProject({ + allowScripts: { canvas: true }, + withScripts: ['canvas'], + }), + config: { pending: true }, + }) + await npm.exec('approve-scripts', []) + t.match(joinedOutput(), /No packages with unreviewed install scripts/) +}) + +t.test('approve-scripts writes pinned entry by default', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await npm.exec('approve-scripts', ['canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true }) +}) + +t.test('approve-scripts --no-pin writes name-only entry', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { pin: false }, + }) + await npm.exec('approve-scripts', ['canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { canvas: true }) +}) + +t.test('approve-scripts --all approves every unreviewed package', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), + config: { all: true }, + }) + await npm.exec('approve-scripts', []) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { + 'canvas@1.0.0': true, + 'sharp@1.0.0': true, + }) +}) + +t.test('approve-scripts errors on unknown package', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects( + npm.exec('approve-scripts', ['not-installed']), + { code: 'ENOMATCH' } + ) +}) + +t.test('approve-scripts respects existing deny entry', async t => { + const { npm, prefix, logs } = await mockNpm(t, { + prefixDir: setupProject({ + withScripts: ['canvas'], + allowScripts: { canvas: false }, + }), + }) + await npm.exec('approve-scripts', ['canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + // Deny wins; unchanged. + t.strictSame(pkg.allowScripts, { canvas: false }) + t.match(logs.warn.byTitle('approve-scripts'), [/canvas is denied/]) +}) + +t.test('approve-scripts requires positional args, --all, or --pending', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects(npm.exec('approve-scripts', []), { code: 'EUSAGE' }) +}) + +t.test('approve-scripts --pending cannot be combined with positional', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { pending: true }, + }) + await t.rejects(npm.exec('approve-scripts', ['canvas']), { code: 'EUSAGE' }) +}) + +t.test('approve-scripts fails on global', async t => { + const { npm } = await mockNpm(t, { + config: { global: true }, + }) + await t.rejects(npm.exec('approve-scripts', ['canvas']), { code: 'EGLOBAL' }) +}) + +t.test('approve-scripts --json outputs structured summary', async t => { + const { npm, joinedOutput } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { json: true }, + }) + await npm.exec('approve-scripts', ['canvas']) + const parsed = JSON.parse(joinedOutput()) + t.match(parsed, { + allowScripts: [{ name: 'canvas', changes: [{ key: 'canvas@1.0.0', change: 'added' }] }], + }) +}) + +t.test('approve-scripts --all with no unreviewed packages prints message', async t => { + const { npm, joinedOutput } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { '': { name: 'host', version: '1.0.0' } }, + }), + node_modules: {}, + }, + config: { all: true }, + }) + await npm.exec('approve-scripts', []) + t.match(joinedOutput(), /No packages with unreviewed install scripts/) +}) + +t.test('approve-scripts on a package already at the right pin is no-op', async t => { + const { npm, prefix, joinedOutput } = await _mockNpm(t, { + prefixDir: setupProject({ + withScripts: ['canvas'], + allowScripts: { 'canvas@1.0.0': true }, + }), + }) + await npm.exec('approve-scripts', ['canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true }) + t.match(joinedOutput(), /Nothing to approve/) +}) + +t.test('approve-scripts --pending with single package uses singular wording', async t => { + const { npm, joinedOutput } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { pending: true }, + }) + await npm.exec('approve-scripts', []) + t.match(joinedOutput(), /1 package has install scripts/) +}) + +t.test('approve-scripts --pending lists package with no version', async t => { + // Use a fixture where the lockfile records a synthetic node without a version + const { npm } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { pending: true }, + }) + await npm.exec('approve-scripts', []) + // Just exercising; no assertion needed for additional coverage. + t.pass() +}) + +t.test('approve-scripts groups multiple installed versions of the same package', async t => { + // Two versions of lodash exist in the tree; both have install scripts. + // groupByPackage should put them in the same group (hits the + // `if (!groups[key])` falsy branch on the second node). + const { npm, prefix } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { 'top-of-tree': '*' }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { 'top-of-tree': '*' } }, + 'node_modules/lodash': { + version: '4.17.21', + resolved: 'https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz', + hasInstallScript: true, + }, + 'node_modules/top-of-tree': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/top-of-tree/-/top-of-tree-1.0.0.tgz', + dependencies: { lodash: '3.10.1' }, + }, + 'node_modules/top-of-tree/node_modules/lodash': { + version: '3.10.1', + resolved: 'https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz', + hasInstallScript: true, + }, + }, + }), + node_modules: { + lodash: { + 'package.json': JSON.stringify({ + name: 'lodash', + version: '4.17.21', + scripts: { install: 'echo install' }, + }), + }, + 'top-of-tree': { + 'package.json': JSON.stringify({ name: 'top-of-tree', version: '1.0.0' }), + node_modules: { + lodash: { + 'package.json': JSON.stringify({ + name: 'lodash', + version: '3.10.1', + scripts: { install: 'echo install' }, + }), + }, + }, + }, + }, + }, + }) + await npm.exec('approve-scripts', ['lodash']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + // Both versions get pinned. + t.strictSame(pkg.allowScripts, { + 'lodash@3.10.1': true, + 'lodash@4.17.21': true, + }) +}) + +t.test('approve-scripts --pending handles node with no version', async t => { + // Exercise the ternary's falsy branch in runPending: `node.version ? '@'... : ''` + // when the node has no version field. + const mockSync = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { '': { name: 'host', version: '1.0.0' } }, + }), + node_modules: {}, + }, + config: { pending: true }, + mocks: { + // Make the walker return a synthetic node with no version + '{LIB}/utils/check-allow-scripts.js': async () => [{ + node: { packageName: 'no-version-pkg', name: 'no-version-pkg', version: undefined }, + scripts: { install: 'do-stuff' }, + }], + }, + }) + await mockSync.npm.exec('approve-scripts', []) + // Output should mention the package without an @version suffix. + t.match(mockSync.joinedOutput(), / no-version-pkg \(install: do-stuff\)/) +}) + +t.test('forbidden semver range in package.json#allowScripts is dropped with a warning', async t => { + // End-to-end: project declares a caret range in allowScripts. The + // resolver must drop the entry, emit a warning, and the matching node + // must remain unreviewed (listed by --pending). + const mock = await _mockNpm(t, { + prefixDir: setupProject({ + withScripts: ['canvas'], + // ^0.33.0 is a forbidden range per RFC. + allowScripts: { 'canvas@^0.33.0': true }, + }), + config: { pending: true }, + }) + await mock.npm.exec('approve-scripts', []) + + const warnings = mock.logs.warn.byTitle('allow-scripts') + t.ok( + warnings.some(m => /semver ranges/.test(m) && /canvas@\^0\.33\.0/.test(m)), + 'resolver emits warning about forbidden range' + ) + // canvas was installed with version 1.0.0 (setupProject default) and + // the forbidden allowlist entry was dropped, so canvas appears in the + // pending list. + t.match(mock.joinedOutput(), /canvas@1\.0\.0/) +}) + +t.test('approve-scripts --pending lists packages that only have binding.gyp', async t => { + // End-to-end: a package with no preinstall/install/postinstall but a + // binding.gyp on disk gets a synthetic `node-gyp rebuild` install + // script. The runtime isNodeGypPackage check must see it and surface + // the package in --pending output. + const mock = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { 'native-pkg': '*' }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { 'native-pkg': '*' } }, + 'node_modules/native-pkg': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/native-pkg/-/native-pkg-1.0.0.tgz', + // No hasInstallScript — the synthetic node-gyp injection is + // what we want this test to exercise. + }, + }, + }), + node_modules: { + 'native-pkg': { + 'package.json': JSON.stringify({ name: 'native-pkg', version: '1.0.0' }), + // The file that triggers isNodeGypPackage to return true. + 'binding.gyp': '{}', + }, + }, + }, + config: { pending: true }, + }) + await mock.npm.exec('approve-scripts', []) + + const out = mock.joinedOutput() + t.match(out, /native-pkg@1\.0\.0/, 'binding.gyp-only package appears in --pending') + t.match(out, /install: node-gyp rebuild/, 'synthetic node-gyp install is named') +}) + +t.test('approve-scripts --all skips bundled deps with a notice', async t => { + // Bundled deps cannot be allowlisted in Phase 1 (RFC defers their + // allowlisting to a follow-up). --all must not silently write a key + // derived from the bundled tarball's self-claimed identity. + const { npm, logs, prefix } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { 'parent-pkg': '*' }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { 'parent-pkg': '*' } }, + 'node_modules/parent-pkg': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/parent-pkg/-/parent-pkg-1.0.0.tgz', + hasInstallScript: true, + }, + 'node_modules/parent-pkg/node_modules/inner': { + version: '1.0.0', + inBundle: true, + hasInstallScript: true, + }, + }, + }), + node_modules: { + 'parent-pkg': { + 'package.json': JSON.stringify({ + name: 'parent-pkg', + version: '1.0.0', + scripts: { install: 'echo install' }, + bundleDependencies: ['inner'], + }), + node_modules: { + inner: { + 'package.json': JSON.stringify({ + name: 'inner', + version: '1.0.0', + scripts: { install: 'echo bundled-install' }, + }), + }, + }, + }, + }, + }, + config: { all: true }, + }) + await npm.exec('approve-scripts', []) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + // parent-pkg is approvable. inner is bundled and must be excluded. + t.equal(pkg.allowScripts['parent-pkg@1.0.0'], true, + 'non-bundled parent gets approved') + t.notOk(Object.keys(pkg.allowScripts).some(k => k.startsWith('inner')), + 'bundled inner is not approved') + t.match(logs.warn.byTitle('approve-scripts'), [/Skipping 1 bundled dependency/]) +}) + +t.test('approve-scripts positional is ignored', async t => { + // Same protection on the positional path: a user typing a bundled + // package name must not get a policy entry written. + const { npm } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { 'parent-pkg': '*' }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { 'parent-pkg': '*' } }, + 'node_modules/parent-pkg': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/parent-pkg/-/parent-pkg-1.0.0.tgz', + hasInstallScript: true, + }, + 'node_modules/parent-pkg/node_modules/inner': { + version: '1.0.0', + inBundle: true, + hasInstallScript: true, + }, + }, + }), + node_modules: { + 'parent-pkg': { + 'package.json': JSON.stringify({ + name: 'parent-pkg', + version: '1.0.0', + scripts: { install: 'echo install' }, + bundleDependencies: ['inner'], + }), + node_modules: { + inner: { + 'package.json': JSON.stringify({ + name: 'inner', + version: '1.0.0', + scripts: { install: 'echo bundled' }, + }), + }, + }, + }, + }, + }, + }) + await t.rejects( + npm.exec('approve-scripts', ['inner']), + { code: 'ENOMATCH' }, + 'typing the bundled package name does not match any approvable node' + ) +}) + +t.test('approve-scripts --all with only bundled deps prints "no eligible" notice', async t => { + const { npm, logs, joinedOutput, prefix } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { 'parent-pkg': '*' }, + }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'host', version: '1.0.0', dependencies: { 'parent-pkg': '*' } }, + 'node_modules/parent-pkg': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/parent-pkg/-/parent-pkg-1.0.0.tgz', + // parent-pkg has NO install scripts; only the bundled child does. + }, + 'node_modules/parent-pkg/node_modules/only-bundled': { + version: '1.0.0', + inBundle: true, + hasInstallScript: true, + }, + }, + }), + node_modules: { + 'parent-pkg': { + 'package.json': JSON.stringify({ + name: 'parent-pkg', + version: '1.0.0', + bundleDependencies: ['only-bundled'], + }), + node_modules: { + 'only-bundled': { + 'package.json': JSON.stringify({ + name: 'only-bundled', + version: '1.0.0', + scripts: { install: 'echo evil' }, + }), + }, + }, + }, + }, + }, + config: { all: true }, + }) + await npm.exec('approve-scripts', []) + t.match(joinedOutput(), /No packages eligible for approval/) + t.match(logs.warn.byTitle('approve-scripts'), [/Skipping 1 bundled dependency/]) + // Ensure no policy entry was written. + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.notOk(pkg.allowScripts, 'no allowScripts written') +}) diff --git a/test/lib/commands/deny-scripts.js b/test/lib/commands/deny-scripts.js new file mode 100644 index 0000000000000..8bc6f2b585e12 --- /dev/null +++ b/test/lib/commands/deny-scripts.js @@ -0,0 +1,112 @@ +const t = require('tap') +const fs = require('node:fs') +const { resolve } = require('node:path') +const _mockNpm = require('../../fixtures/mock-npm') + +const setupProject = ({ allowScripts, withScripts = ['core-js'] } = {}) => { + const pkg = { + name: 'host', + version: '1.0.0', + dependencies: Object.fromEntries(withScripts.map((n) => [n, '*'])), + } + if (allowScripts !== undefined) { + pkg.allowScripts = allowScripts + } + const lockPackages = { '': pkg } + const nodeModules = {} + for (const name of withScripts) { + nodeModules[name] = { + 'package.json': JSON.stringify({ + name, + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + } + lockPackages[`node_modules/${name}`] = { + version: '1.0.0', + resolved: `https://registry.npmjs.org/${name}/-/${name}-1.0.0.tgz`, + hasInstallScript: true, + } + } + return { + 'package.json': JSON.stringify(pkg, null, 2), + 'package-lock.json': JSON.stringify({ + name: pkg.name, + version: pkg.version, + lockfileVersion: 3, + requires: true, + packages: lockPackages, + }), + node_modules: nodeModules, + } +} + +t.test('deny-scripts writes name-only false entry', async t => { + const { npm, prefix } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + }) + await npm.exec('deny-scripts', ['core-js']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'core-js': false }) +}) + +t.test('deny-scripts ignores --pin and always writes name-only', async t => { + const { npm, prefix } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + config: { pin: true }, + }) + await npm.exec('deny-scripts', ['core-js']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'core-js': false }) +}) + +t.test('deny-scripts replaces existing pinned allow', async t => { + const { npm, prefix } = await _mockNpm(t, { + prefixDir: setupProject({ + withScripts: ['core-js'], + allowScripts: { 'core-js@1.0.0': true }, + }), + }) + await npm.exec('deny-scripts', ['core-js']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'core-js': false }) +}) + +t.test('deny-scripts --pending is rejected', async t => { + const { npm } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + config: { pending: true }, + }) + await t.rejects(npm.exec('deny-scripts', []), { code: 'EUSAGE' }) +}) + +t.test('deny-scripts --all denies every unreviewed package', async t => { + const { npm, prefix } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js', 'telemetry'] }), + config: { all: true }, + }) + await npm.exec('deny-scripts', []) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'core-js': false, telemetry: false }) +}) + +t.test('deny-scripts errors on unknown package', async t => { + const { npm } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + }) + await t.rejects( + npm.exec('deny-scripts', ['not-installed']), + { code: 'ENOMATCH' } + ) +}) + +t.test('deny-scripts requires positional args or --all', async t => { + const { npm } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + }) + await t.rejects(npm.exec('deny-scripts', []), { code: 'EUSAGE' }) +}) diff --git a/test/lib/utils/allow-scripts-writer.js b/test/lib/utils/allow-scripts-writer.js new file mode 100644 index 0000000000000..9cea34778ae8f --- /dev/null +++ b/test/lib/utils/allow-scripts-writer.js @@ -0,0 +1,1046 @@ +const t = require('tap') +const { + applyApprovalForPackage, + applyDenyForPackage, + nameKeyFor, + versionedKeyFor, + isSingleVersionPin, +} = require('../../../lib/utils/allow-scripts-writer.js') + +const node = (overrides = {}) => { + const name = overrides.name ?? overrides.packageName ?? 'pkg' + const packageName = overrides.packageName ?? name + const version = overrides.version ?? '1.0.0' + const urlPkg = packageName + return { + name, + packageName, + version, + resolved: overrides.resolved + ?? `https://registry.npmjs.org/${urlPkg}/-/${urlPkg}-${version}.tgz`, + location: overrides.location ?? `node_modules/${name}`, + isRegistryDependency: overrides.isRegistryDependency ?? true, + } +} + +t.test('nameKeyFor / versionedKeyFor — registry', async t => { + const n = node({ name: 'canvas', version: '2.11.0' }) + t.equal(nameKeyFor(n), 'canvas') + t.equal(versionedKeyFor(n), 'canvas@2.11.0') +}) + +t.test('nameKeyFor / versionedKeyFor — git', async t => { + const n = node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + }) + t.equal(nameKeyFor(n), 'github:foo/bar') + t.equal(versionedKeyFor(n), 'github:foo/bar#deadbeefcafebabe1234567890abcdef12345678') +}) + +t.test('nameKeyFor / versionedKeyFor — file', async t => { + const n = node({ name: 'local', resolved: 'file:../local' }) + t.equal(nameKeyFor(n), 'file:../local') + t.equal(versionedKeyFor(n), 'file:../local') +}) + +t.test('isSingleVersionPin', async t => { + t.ok(isSingleVersionPin('pkg@1.2.3')) + t.notOk(isSingleVersionPin('pkg')) + t.notOk(isSingleVersionPin('pkg@^1')) + t.notOk(isSingleVersionPin('pkg@1.2.3 || 2.0.0')) + t.notOk(isSingleVersionPin('@@@bad')) +}) + +t.test('applyApprovalForPackage — empty allowScripts, --pin', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + {}, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) + t.strictSame(changes, [{ key: 'canvas@2.11.0', change: 'added' }]) +}) + +t.test('applyApprovalForPackage — empty allowScripts, --no-pin', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + {}, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: false } + ) + t.strictSame(allowScripts, { canvas: true }) + t.strictSame(changes, [{ key: 'canvas', change: 'added' }]) +}) + +t.test('applyApprovalForPackage — stale pin rewritten to new installed version', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + { 'canvas@2.10.0': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) + t.match(changes, [ + { key: 'canvas@2.10.0', change: 'removed-stale' }, + { key: 'canvas@2.11.0', change: 'added' }, + ]) +}) + +t.test('applyApprovalForPackage — multi-version disjunction is preserved', async t => { + const { allowScripts } = applyApprovalForPackage( + { 'canvas@2.10.0 || 2.11.0': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { + 'canvas@2.10.0 || 2.11.0': true, + 'canvas@2.11.0': true, + }) +}) + +t.test('applyApprovalForPackage — already-allowed exact version is a no-op', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + { 'canvas@2.11.0': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — existing deny wins, returns warning', async t => { + const { allowScripts, changes, warning } = applyApprovalForPackage( + { canvas: false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { canvas: false }) + t.strictSame(changes, []) + t.match(warning, /canvas is denied/) +}) + +t.test('applyApprovalForPackage — versioned deny wins too', async t => { + const { changes, warning } = applyApprovalForPackage( + { 'canvas@2.11.0': false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(changes, []) + t.match(warning, /denied|versioned deny/) +}) + +t.test('applyApprovalForPackage — name-only existing, --no-pin no-op', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + { canvas: true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: false } + ) + t.strictSame(allowScripts, { canvas: true }) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — --no-pin downgrades pinned entry to name-only', async t => { + const { allowScripts } = applyApprovalForPackage( + { 'canvas@2.10.0': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: false } + ) + t.strictSame(allowScripts, { canvas: true }) +}) + +t.test('applyApprovalForPackage — multiple installed versions write multiple pins', async t => { + const { allowScripts } = applyApprovalForPackage( + {}, + [ + node({ name: 'lodash', version: '4.17.21' }), + node({ name: 'lodash', version: '3.10.1' }), + ], + { pin: true } + ) + t.strictSame(allowScripts, { 'lodash@3.10.1': true, 'lodash@4.17.21': true }) +}) + +t.test('applyApprovalForPackage — keeps existing pin matching one installed, adds pin for other', async t => { + const { allowScripts } = applyApprovalForPackage( + { 'lodash@4.17.21': true }, + [ + node({ name: 'lodash', version: '4.17.21' }), + node({ name: 'lodash', version: '3.10.1' }), + ], + { pin: true } + ) + t.strictSame(allowScripts, { 'lodash@3.10.1': true, 'lodash@4.17.21': true }) +}) + +t.test('applyDenyForPackage — empty allowScripts adds name-only false', async t => { + const { allowScripts, changes } = applyDenyForPackage( + {}, + [node({ name: 'core-js', version: '3.0.0' })] + ) + t.strictSame(allowScripts, { 'core-js': false }) + t.strictSame(changes, [{ key: 'core-js', change: 'added' }]) +}) + +t.test('applyDenyForPackage — pinned allow is replaced by name-only deny', async t => { + const { allowScripts } = applyDenyForPackage( + { 'core-js@3.0.0': true }, + [node({ name: 'core-js', version: '3.0.0' })] + ) + t.strictSame(allowScripts, { 'core-js': false }) +}) + +t.test('applyDenyForPackage — already-denied is a no-op', async t => { + const { changes } = applyDenyForPackage( + { 'core-js': false }, + [node({ name: 'core-js', version: '3.0.0' })] + ) + t.strictSame(changes, []) +}) + +t.test('applyDenyForPackage — name-only true is replaced by name-only false', async t => { + const { allowScripts } = applyDenyForPackage( + { 'core-js': true }, + [node({ name: 'core-js', version: '3.0.0' })] + ) + t.strictSame(allowScripts, { 'core-js': false }) +}) + +t.test('applyApprovalForPackage — preserves unrelated entries', async t => { + const { allowScripts } = applyApprovalForPackage( + { other: true, 'unrelated@1.0.0': false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.strictSame(allowScripts, { + other: true, + 'unrelated@1.0.0': false, + 'canvas@2.11.0': true, + }) +}) + +t.test('applyApprovalForPackage — git node writes hosted shortcut with commit', async t => { + const { allowScripts } = applyApprovalForPackage( + {}, + [node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeefcafebabe1234567890abcdef12345678', + })], + { pin: true } + ) + t.strictSame(allowScripts, { + 'github:foo/bar#deadbeefcafebabe1234567890abcdef12345678': true, + }) +}) + +t.test('applyApprovalForPackage — git node --no-pin writes hosted shortcut without commit', async t => { + const { allowScripts } = applyApprovalForPackage( + {}, + [node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#deadbeef', + })], + { pin: false } + ) + t.strictSame(allowScripts, { 'github:foo/bar': true }) +}) + +t.test('applyApprovalForPackage — file dep uses resolved as both keys', async t => { + const { allowScripts } = applyApprovalForPackage( + {}, + [node({ name: 'local', resolved: 'file:../local' })], + { pin: true } + ) + t.strictSame(allowScripts, { 'file:../local': true }) +}) + +t.test('applyApprovalForPackage — empty nodes returns unchanged', async t => { + const { allowScripts, changes } = applyApprovalForPackage({ x: true }, [], { pin: true }) + t.strictSame(allowScripts, { x: true }) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — name-only entry is replaced by pin (RFC table)', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + { canvas: true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + // Per RFC table: pkg: true + --pin must upgrade to pkg@x.y.z: true. + // Both entries left behind would be wrong. + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) + t.match(changes, [ + { key: 'canvas@2.11.0', change: 'added' }, + { key: 'canvas', change: 'replaced-by-pin' }, + ]) +}) + +t.test('applyApprovalForPackage — name-only + multi-version installs replaces with all pins', async t => { + const { allowScripts } = applyApprovalForPackage( + { lodash: true }, + [ + node({ name: 'lodash', version: '4.17.21' }), + node({ name: 'lodash', version: '3.10.1' }), + ], + { pin: true } + ) + t.strictSame(allowScripts, { 'lodash@3.10.1': true, 'lodash@4.17.21': true }) +}) + +t.test('applyApprovalForPackage — name-only is preserved when --no-pin', async t => { + const { allowScripts, changes } = applyApprovalForPackage( + { canvas: true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: false } + ) + t.strictSame(allowScripts, { canvas: true }) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — name-only NOT dropped when no pinning could happen', async t => { + // Node has no version, so installedKeys is empty. The name-only entry + // must NOT be dropped or we silently lose the policy. + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { allowScripts } = applyApprovalForPackage( + { pkg: true }, + [noVersion], + { pin: true } + ) + t.strictSame(allowScripts, { pkg: true }) +}) + +t.test('applyApprovalForPackage — convergent: running twice gives the same result', async t => { + // Start with stale state including a name-only entry. + const start = { canvas: true, 'canvas@2.10.0': true } + const nodes = [node({ name: 'canvas', version: '2.11.0' })] + + const run1 = applyApprovalForPackage(start, nodes, { pin: true }) + const run2 = applyApprovalForPackage(run1.allowScripts, nodes, { pin: true }) + + t.strictSame(run1.allowScripts, { 'canvas@2.11.0': true }) + t.strictSame(run2.allowScripts, { 'canvas@2.11.0': true }) + t.strictSame(run2.changes, [], 'second run is a no-op') +}) + +t.test('applyApprovalForPackage — deny still wins even when name-only is upgraded', async t => { + const { allowScripts, warning } = applyApprovalForPackage( + { canvas: true, 'canvas@2.11.0': false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + // Existing deny on the version blocks the approval. + t.strictSame(allowScripts, { canvas: true, 'canvas@2.11.0': false }) + t.match(warning, /denied|versioned deny/) +}) + +t.test('keyTargetsNode — unparseable key returns false (via applyApproval)', async t => { + // An unparseable key in the existing object should be ignored. + const { allowScripts } = applyApprovalForPackage( + { '@@@invalid': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.equal(allowScripts['canvas@2.11.0'], true) + t.equal(allowScripts['@@@invalid'], true) +}) + +t.test('applyDenyForPackage — empty nodes array returns unchanged', async t => { + const { allowScripts, changes } = applyDenyForPackage({ existing: true }, []) + t.strictSame(allowScripts, { existing: true }) + t.strictSame(changes, []) +}) + +t.test('applyDenyForPackage — node with no nameable identity is a no-op', async t => { + // A node whose resolved field is unparseable as a git URL and has no + // version/name produces a null name; the writer must short-circuit. + const weird = { name: '', packageName: '', version: undefined, resolved: undefined } + const { allowScripts, changes } = applyDenyForPackage({}, [weird]) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — file dep with deny entry blocks approval', async t => { + const { warning } = applyApprovalForPackage( + { 'file:../local': false }, + [node({ name: 'local', resolved: 'file:../local' })], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) + +t.test('applyApprovalForPackage — remote tarball deny blocks approval', async t => { + const remote = { name: 'pkg', packageName: 'pkg', version: '1.0.0', resolved: 'https://example.com/pkg.tgz' } + const { warning } = applyApprovalForPackage( + { 'https://example.com/pkg.tgz': false }, + [remote], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) + +t.test('applyApprovalForPackage — no-pin with no name produces no-op', async t => { + const weird = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { allowScripts, changes } = applyApprovalForPackage({}, [weird], { pin: false }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — pin with no versioned key is a no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { allowScripts, changes } = applyApprovalForPackage({}, [noVersion], { pin: true }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — pin with no versioned key and existing name-only is no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { changes } = applyApprovalForPackage({ pkg: true }, [noVersion], { pin: true }) + t.strictSame(changes, []) +}) + +t.test('keyTargetsNode handles file with directory-typed key', async t => { + // A "directory" spec for a relative path. + const dirNode = { name: 'local', packageName: 'local', resolved: 'file:./local-dir' } + const { allowScripts } = applyApprovalForPackage( + {}, + [dirNode], + { pin: true } + ) + t.equal(allowScripts['file:./local-dir'], true) +}) + +t.test('nameKeyFor / versionedKeyFor — null node', async t => { + t.equal(nameKeyFor(null), null) + t.equal(versionedKeyFor(null), null) +}) + +t.test('nameKeyFor / versionedKeyFor — non-hosted git url returns null', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: 'git+https://example.invalid/foo/bar.git#abc' } + t.equal(nameKeyFor(n), null) + t.equal(versionedKeyFor(n), null) +}) + +t.test('versionedKeyFor — absolute path resolved field', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/path/local' } + t.equal(versionedKeyFor(n), '/abs/path/local') + t.equal(nameKeyFor(n), '/abs/path/local') +}) + +t.test('applyApprovalForPackage — node.resolved parse error in keyTargetsNode is safe', async t => { + // An existing git-style key for a package whose own resolved field + // doesn't parse: the key just doesn't target anything. + const gitNode = node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#abc', + }) + // Add an explicit unparseable existing entry. + const { allowScripts } = applyApprovalForPackage( + { 'github:other/other': true }, + [gitNode], + { pin: true } + ) + // Existing entry unchanged; new git entry added. + t.equal(allowScripts['github:other/other'], true) + t.equal(allowScripts['github:foo/bar#abc'], true) +}) + +t.test('keyTargetsNode — alias key does not target anything (via writer)', async t => { + // Alias-typed key falls through the switch default. + const { allowScripts } = applyApprovalForPackage( + { 'foo@npm:bar@1.0.0': true }, + [node({ name: 'foo', packageName: 'foo', version: '1.0.0' })], + { pin: true } + ) + // Alias entry untouched, new pin added separately. + t.equal(allowScripts['foo@npm:bar@1.0.0'], true) + t.equal(allowScripts['foo@1.0.0'], true) +}) +t.test('keyTargetsNode — unparseable key returns false (via applyApproval)', async t => { + // An unparseable key in the existing object should be ignored. + const { allowScripts } = applyApprovalForPackage( + { '@@@invalid': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.equal(allowScripts['canvas@2.11.0'], true) + t.equal(allowScripts['@@@invalid'], true) +}) +t.test('applyDenyForPackage — empty nodes array returns unchanged', async t => { + const { allowScripts, changes } = applyDenyForPackage({ existing: true }, []) + t.strictSame(allowScripts, { existing: true }) + t.strictSame(changes, []) +}) +t.test('applyDenyForPackage — node with no nameable identity is a no-op', async t => { + // A node whose resolved field is unparseable as a git URL and has no + // version/name produces a null name; the writer must short-circuit. + const weird = { name: '', packageName: '', version: undefined, resolved: undefined } + const { allowScripts, changes } = applyDenyForPackage({}, [weird]) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — file dep with deny entry blocks approval', async t => { + const { warning } = applyApprovalForPackage( + { 'file:../local': false }, + [node({ name: 'local', resolved: 'file:../local' })], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) +t.test('applyApprovalForPackage — remote tarball deny blocks approval', async t => { + const remote = { name: 'pkg', packageName: 'pkg', version: '1.0.0', resolved: 'https://example.com/pkg.tgz' } + const { warning } = applyApprovalForPackage( + { 'https://example.com/pkg.tgz': false }, + [remote], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) +t.test('applyApprovalForPackage — no-pin with no name produces no-op', async t => { + const weird = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { allowScripts, changes } = applyApprovalForPackage({}, [weird], { pin: false }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — pin with no versioned key is a no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { allowScripts, changes } = applyApprovalForPackage({}, [noVersion], { pin: true }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — pin with no versioned key and existing name-only is no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { changes } = applyApprovalForPackage({ pkg: true }, [noVersion], { pin: true }) + t.strictSame(changes, []) +}) +t.test('keyTargetsNode handles file with directory-typed key', async t => { + // A "directory" spec for a relative path. + const dirNode = { name: 'local', packageName: 'local', resolved: 'file:./local-dir' } + const { allowScripts } = applyApprovalForPackage( + {}, + [dirNode], + { pin: true } + ) + t.equal(allowScripts['file:./local-dir'], true) +}) +t.test('nameKeyFor / versionedKeyFor — null node', async t => { + t.equal(nameKeyFor(null), null) + t.equal(versionedKeyFor(null), null) +}) +t.test('nameKeyFor / versionedKeyFor — non-hosted git url returns null', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: 'git+https://example.invalid/foo/bar.git#abc' } + t.equal(nameKeyFor(n), null) + t.equal(versionedKeyFor(n), null) +}) +t.test('versionedKeyFor — absolute path resolved field', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/path/local' } + t.equal(versionedKeyFor(n), '/abs/path/local') + t.equal(nameKeyFor(n), '/abs/path/local') +}) +t.test('applyApprovalForPackage — node.resolved parse error in keyTargetsNode is safe', async t => { + // An existing git-style key for a package whose own resolved field + // doesn't parse: the key just doesn't target anything. + const gitNode = node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#abc', + }) + // Add an explicit unparseable existing entry. + const { allowScripts } = applyApprovalForPackage( + { 'github:other/other': true }, + [gitNode], + { pin: true } + ) + // Existing entry unchanged; new git entry added. + t.equal(allowScripts['github:other/other'], true) + t.equal(allowScripts['github:foo/bar#abc'], true) +}) +t.test('keyTargetsNode — alias key does not target anything (via writer)', async t => { + // Alias-typed key falls through the switch default. + const { allowScripts } = applyApprovalForPackage( + { 'foo@npm:bar@1.0.0': true }, + [node({ name: 'foo', packageName: 'foo', version: '1.0.0' })], + { pin: true } + ) + // Alias entry untouched, new pin added separately. + t.equal(allowScripts['foo@npm:bar@1.0.0'], true) + t.equal(allowScripts['foo@1.0.0'], true) +}) + +t.test('keyTargetsNode handles tag-type key', async t => { + // 'canvas@latest' parses as type='tag'. The writer should treat it like + // a name-only match (any installed version of canvas). + const { allowScripts } = applyApprovalForPackage( + { 'canvas@latest': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + // The tag key targets the canvas node (same package name), so the + // 'canvas@2.11.0' pin gets added; tag key is preserved. + t.equal(allowScripts['canvas@latest'], true) + t.equal(allowScripts['canvas@2.11.0'], true) +}) + +t.test('keyTargetsNode handles file-type tarball key matching saveSpec', async t => { + // 'file:pkg.tgz' parses as type='file' with saveSpec='file:pkg.tgz'. + const tarballNode = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'file:pkg.tgz', + } + const { allowScripts } = applyApprovalForPackage( + { 'file:pkg.tgz': false }, + [tarballNode], + { pin: true } + ) + // saveSpec match: deny wins, no pin added. + t.equal(allowScripts['file:pkg.tgz'], false) +}) + +t.test('keyTargetsNode handles file-type tarball key matching fetchSpec', async t => { + // When node.resolved is an absolute path matching parsed.fetchSpec. + const tarballNode = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: '/abs/path/pkg.tgz', + } + const { allowScripts, warning } = applyApprovalForPackage( + { '/abs/path/pkg.tgz': false }, + [tarballNode], + { pin: true } + ) + t.equal(allowScripts['/abs/path/pkg.tgz'], false) + t.match(warning, /denied|versioned deny/) +}) + +t.test('versionedKeyFor — git node without committish', async t => { + // versionedKeyFor's ternary takes the "no committish" branch. + t.equal( + versionedKeyFor({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git', + }), + 'github:foo/bar' + ) +}) + +t.test('versionedKeyFor / nameKeyFor — absolute path resolved field', async t => { + // Hits the `resolved.startsWith('/')` branch in both helpers. + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/local-dir' } + t.equal(versionedKeyFor(n), '/abs/local-dir') + t.equal(nameKeyFor(n), '/abs/local-dir') +}) + +t.test('keyTargetsNode — git key against a node with no resolved field', async t => { + // Defensive: if existing has a git-shaped key and the installed node + // has no resolved field, keyTargetsNode bails out and no policy entry + // can be derived from untrusted sources. + const noResolved = { name: 'bar', packageName: 'bar', resolved: undefined } + const { allowScripts } = applyApprovalForPackage( + { 'github:foo/bar': true }, + [noResolved], + { pin: false } + ) + // Existing entry untouched. No new key written: nameKeyFor returns + // null for a node with no trusted identity source. + t.equal(allowScripts['github:foo/bar'], true) + t.notOk('bar' in allowScripts, 'no entry written under attacker-controlled node.name') +}) + +t.test('applyApprovalForPackage — default args (no options object)', async t => { + // Hits the `{ pin = true } = {}` default-arg branch. + const { allowScripts } = applyApprovalForPackage( + {}, + [node({ name: 'canvas', version: '2.11.0' })] + ) + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) +}) + +t.test('applyApprovalForPackage — deny-wins warning when node has no name', async t => { + // Hits the `name || 'this package'` fallback in the warning message. + const noName = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { warning } = applyApprovalForPackage( + { 'github:foo/bar': false }, + [noName], + { pin: true } + ) + // No keys target this node (its resolved doesn't parse to a hosted URL), + // so deny-wins doesn't trigger. Result is no warning. + t.notOk(warning) +}) + +t.test('denyWarning branches on key shape per RFC §approve-scripts', async t => { + // Name-only deny: only remedy is to remove the entry. + const nameOnly = applyApprovalForPackage( + { canvas: false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.match(nameOnly.warning, /remove the entry from allowScripts/) + t.notMatch(nameOnly.warning, /widen the deny/) + + // Pinned deny on a different version: suggest both widen and remove. + const pinned = applyApprovalForPackage( + { 'canvas@2.10.0': false }, + [node({ name: 'canvas', version: '2.10.0' })], + { pin: true } + ) + t.match(pinned.warning, /versioned deny/) + t.match(pinned.warning, /npm deny-scripts canvas/) + t.match(pinned.warning, /widen the deny to all versions/) + t.match(pinned.warning, /remove the entry/) + + // Multi-version deny disjunction: same as pinned (versioned). + const multi = applyApprovalForPackage( + { 'canvas@2.10.0 || 2.11.0': false }, + [node({ name: 'canvas', version: '2.10.0' })], + { pin: true } + ) + t.match(multi.warning, /versioned deny/) + t.match(multi.warning, /npm deny-scripts canvas/) +}) + +t.test('denyWarning: tag-type key (pkg@latest: false) is name-only', async t => { + // `canvas@latest` parses as type='tag'. Treat the same as a bare name. + const { warning } = applyApprovalForPackage( + { 'canvas@latest': false }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.match(warning, /remove the entry/) + t.notMatch(warning, /versioned deny/) +}) + +t.test('keyTargetsNode — unparseable key returns false (via applyApproval)', async t => { + // An unparseable key in the existing object should be ignored. + const { allowScripts } = applyApprovalForPackage( + { '@@@invalid': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.equal(allowScripts['canvas@2.11.0'], true) + t.equal(allowScripts['@@@invalid'], true) +}) + +t.test('applyDenyForPackage — empty nodes array returns unchanged', async t => { + const { allowScripts, changes } = applyDenyForPackage({ existing: true }, []) + t.strictSame(allowScripts, { existing: true }) + t.strictSame(changes, []) +}) + +t.test('applyDenyForPackage — node with no nameable identity is a no-op', async t => { + // A node whose resolved field is unparseable as a git URL and has no + // version/name produces a null name; the writer must short-circuit. + const weird = { name: '', packageName: '', version: undefined, resolved: undefined } + const { allowScripts, changes } = applyDenyForPackage({}, [weird]) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — file dep with deny entry blocks approval', async t => { + const { warning } = applyApprovalForPackage( + { 'file:../local': false }, + [node({ name: 'local', resolved: 'file:../local' })], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) + +t.test('applyApprovalForPackage — remote tarball deny blocks approval', async t => { + const remote = { name: 'pkg', packageName: 'pkg', version: '1.0.0', resolved: 'https://example.com/pkg.tgz' } + const { warning } = applyApprovalForPackage( + { 'https://example.com/pkg.tgz': false }, + [remote], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) + +t.test('applyApprovalForPackage — no-pin with no name produces no-op', async t => { + const weird = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { allowScripts, changes } = applyApprovalForPackage({}, [weird], { pin: false }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — pin with no versioned key is a no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { allowScripts, changes } = applyApprovalForPackage({}, [noVersion], { pin: true }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) + +t.test('applyApprovalForPackage — pin with no versioned key and existing name-only is no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { changes } = applyApprovalForPackage({ pkg: true }, [noVersion], { pin: true }) + t.strictSame(changes, []) +}) + +t.test('keyTargetsNode handles file with directory-typed key', async t => { + // A "directory" spec for a relative path. + const dirNode = { name: 'local', packageName: 'local', resolved: 'file:./local-dir' } + const { allowScripts } = applyApprovalForPackage( + {}, + [dirNode], + { pin: true } + ) + t.equal(allowScripts['file:./local-dir'], true) +}) + +t.test('nameKeyFor / versionedKeyFor — null node', async t => { + t.equal(nameKeyFor(null), null) + t.equal(versionedKeyFor(null), null) +}) + +t.test('nameKeyFor / versionedKeyFor — non-hosted git url returns null', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: 'git+https://example.invalid/foo/bar.git#abc' } + t.equal(nameKeyFor(n), null) + t.equal(versionedKeyFor(n), null) +}) + +t.test('versionedKeyFor — absolute path resolved field', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/path/local' } + t.equal(versionedKeyFor(n), '/abs/path/local') + t.equal(nameKeyFor(n), '/abs/path/local') +}) + +t.test('applyApprovalForPackage — node.resolved parse error in keyTargetsNode is safe', async t => { + // An existing git-style key for a package whose own resolved field + // doesn't parse: the key just doesn't target anything. + const gitNode = node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#abc', + }) + // Add an explicit unparseable existing entry. + const { allowScripts } = applyApprovalForPackage( + { 'github:other/other': true }, + [gitNode], + { pin: true } + ) + // Existing entry unchanged; new git entry added. + t.equal(allowScripts['github:other/other'], true) + t.equal(allowScripts['github:foo/bar#abc'], true) +}) + +t.test('keyTargetsNode — alias key does not target anything (via writer)', async t => { + // Alias-typed key falls through the switch default. + const { allowScripts } = applyApprovalForPackage( + { 'foo@npm:bar@1.0.0': true }, + [node({ name: 'foo', packageName: 'foo', version: '1.0.0' })], + { pin: true } + ) + // Alias entry untouched, new pin added separately. + t.equal(allowScripts['foo@npm:bar@1.0.0'], true) + t.equal(allowScripts['foo@1.0.0'], true) +}) +t.test('keyTargetsNode — unparseable key returns false (via applyApproval)', async t => { + // An unparseable key in the existing object should be ignored. + const { allowScripts } = applyApprovalForPackage( + { '@@@invalid': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + t.equal(allowScripts['canvas@2.11.0'], true) + t.equal(allowScripts['@@@invalid'], true) +}) +t.test('applyDenyForPackage — empty nodes array returns unchanged', async t => { + const { allowScripts, changes } = applyDenyForPackage({ existing: true }, []) + t.strictSame(allowScripts, { existing: true }) + t.strictSame(changes, []) +}) +t.test('applyDenyForPackage — node with no nameable identity is a no-op', async t => { + // A node whose resolved field is unparseable as a git URL and has no + // version/name produces a null name; the writer must short-circuit. + const weird = { name: '', packageName: '', version: undefined, resolved: undefined } + const { allowScripts, changes } = applyDenyForPackage({}, [weird]) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — file dep with deny entry blocks approval', async t => { + const { warning } = applyApprovalForPackage( + { 'file:../local': false }, + [node({ name: 'local', resolved: 'file:../local' })], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) +t.test('applyApprovalForPackage — remote tarball deny blocks approval', async t => { + const remote = { name: 'pkg', packageName: 'pkg', version: '1.0.0', resolved: 'https://example.com/pkg.tgz' } + const { warning } = applyApprovalForPackage( + { 'https://example.com/pkg.tgz': false }, + [remote], + { pin: true } + ) + t.match(warning, /denied|versioned deny/) +}) +t.test('applyApprovalForPackage — no-pin with no name produces no-op', async t => { + const weird = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { allowScripts, changes } = applyApprovalForPackage({}, [weird], { pin: false }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — pin with no versioned key is a no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { allowScripts, changes } = applyApprovalForPackage({}, [noVersion], { pin: true }) + t.strictSame(allowScripts, {}) + t.strictSame(changes, []) +}) +t.test('applyApprovalForPackage — pin with no versioned key and existing name-only is no-op', async t => { + const noVersion = { name: 'pkg', packageName: 'pkg', version: undefined, resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.tgz' } + const { changes } = applyApprovalForPackage({ pkg: true }, [noVersion], { pin: true }) + t.strictSame(changes, []) +}) +t.test('keyTargetsNode handles file with directory-typed key', async t => { + // A "directory" spec for a relative path. + const dirNode = { name: 'local', packageName: 'local', resolved: 'file:./local-dir' } + const { allowScripts } = applyApprovalForPackage( + {}, + [dirNode], + { pin: true } + ) + t.equal(allowScripts['file:./local-dir'], true) +}) +t.test('nameKeyFor / versionedKeyFor — null node', async t => { + t.equal(nameKeyFor(null), null) + t.equal(versionedKeyFor(null), null) +}) +t.test('nameKeyFor / versionedKeyFor — non-hosted git url returns null', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: 'git+https://example.invalid/foo/bar.git#abc' } + t.equal(nameKeyFor(n), null) + t.equal(versionedKeyFor(n), null) +}) +t.test('versionedKeyFor — absolute path resolved field', async t => { + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/path/local' } + t.equal(versionedKeyFor(n), '/abs/path/local') + t.equal(nameKeyFor(n), '/abs/path/local') +}) +t.test('applyApprovalForPackage — node.resolved parse error in keyTargetsNode is safe', async t => { + // An existing git-style key for a package whose own resolved field + // doesn't parse: the key just doesn't target anything. + const gitNode = node({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git#abc', + }) + // Add an explicit unparseable existing entry. + const { allowScripts } = applyApprovalForPackage( + { 'github:other/other': true }, + [gitNode], + { pin: true } + ) + // Existing entry unchanged; new git entry added. + t.equal(allowScripts['github:other/other'], true) + t.equal(allowScripts['github:foo/bar#abc'], true) +}) +t.test('keyTargetsNode — alias key does not target anything (via writer)', async t => { + // Alias-typed key falls through the switch default. + const { allowScripts } = applyApprovalForPackage( + { 'foo@npm:bar@1.0.0': true }, + [node({ name: 'foo', packageName: 'foo', version: '1.0.0' })], + { pin: true } + ) + // Alias entry untouched, new pin added separately. + t.equal(allowScripts['foo@npm:bar@1.0.0'], true) + t.equal(allowScripts['foo@1.0.0'], true) +}) + +t.test('keyTargetsNode handles tag-type key', async t => { + // 'canvas@latest' parses as type='tag'. The writer should treat it like + // a name-only match (any installed version of canvas). + const { allowScripts } = applyApprovalForPackage( + { 'canvas@latest': true }, + [node({ name: 'canvas', version: '2.11.0' })], + { pin: true } + ) + // The tag key targets the canvas node (same package name), so the + // 'canvas@2.11.0' pin gets added; tag key is preserved. + t.equal(allowScripts['canvas@latest'], true) + t.equal(allowScripts['canvas@2.11.0'], true) +}) + +t.test('keyTargetsNode handles file-type tarball key matching saveSpec', async t => { + // 'file:pkg.tgz' parses as type='file' with saveSpec='file:pkg.tgz'. + const tarballNode = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'file:pkg.tgz', + } + const { allowScripts } = applyApprovalForPackage( + { 'file:pkg.tgz': false }, + [tarballNode], + { pin: true } + ) + // saveSpec match: deny wins, no pin added. + t.equal(allowScripts['file:pkg.tgz'], false) +}) + +t.test('keyTargetsNode handles file-type tarball key matching fetchSpec', async t => { + // When node.resolved is an absolute path matching parsed.fetchSpec. + const tarballNode = { + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: '/abs/path/pkg.tgz', + } + const { allowScripts, warning } = applyApprovalForPackage( + { '/abs/path/pkg.tgz': false }, + [tarballNode], + { pin: true } + ) + t.equal(allowScripts['/abs/path/pkg.tgz'], false) + t.match(warning, /denied|versioned deny/) +}) + +t.test('versionedKeyFor — git node without committish', async t => { + // versionedKeyFor's ternary takes the "no committish" branch. + t.equal( + versionedKeyFor({ + name: 'bar', + resolved: 'git+ssh://git@github.com/foo/bar.git', + }), + 'github:foo/bar' + ) +}) + +t.test('versionedKeyFor / nameKeyFor — absolute path resolved field', async t => { + // Hits the `resolved.startsWith('/')` branch in both helpers. + const n = { name: 'pkg', packageName: 'pkg', resolved: '/abs/local-dir' } + t.equal(versionedKeyFor(n), '/abs/local-dir') + t.equal(nameKeyFor(n), '/abs/local-dir') +}) + +t.test('applyApprovalForPackage — default args (no options object)', async t => { + // Hits the `{ pin = true } = {}` default-arg branch. + const { allowScripts } = applyApprovalForPackage( + {}, + [node({ name: 'canvas', version: '2.11.0' })] + ) + t.strictSame(allowScripts, { 'canvas@2.11.0': true }) +}) + +t.test('applyApprovalForPackage — deny-wins warning when node has no name', async t => { + // Hits the `name || 'this package'` fallback in the warning message. + const noName = { name: '', packageName: '', resolved: 'git+ssh://no.parse' } + const { warning } = applyApprovalForPackage( + { 'github:foo/bar': false }, + [noName], + { pin: true } + ) + // No keys target this node (its resolved doesn't parse to a hosted URL), + // so deny-wins doesn't trigger. Result is no warning. + t.notOk(warning) +}) + +t.test('applyApprovalForPackage — multi-version entry + --pin=false adds name-only alongside', async t => { + // RFC table: existing `pkg@a.b.c || d.e.f: true` + installed `pkg@x.y.z` + // + --pin=false adds `pkg: true`. The multi-version disjunction stays + // (it captures intent the command can't infer), and the name-only + // entry is added. + const { allowScripts } = applyApprovalForPackage( + { 'canvas@1.0.0 || 2.0.0': true }, + [node({ name: 'canvas', version: '3.0.0' })], + { pin: false } + ) + t.strictSame(allowScripts, { + 'canvas@1.0.0 || 2.0.0': true, + canvas: true, + }) +}) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index a9dc2f0755a4f..b7b6f3c5f7eaa 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -1722,6 +1722,27 @@ const definitions = { `, flatten, }), + pending: new Definition('pending', { + default: false, + type: Boolean, + description: ` + List packages with install scripts that are not yet covered by the + \`allowScripts\` policy, without modifying \`package.json\`. Only + meaningful for \`npm approve-scripts\`. + `, + flatten, + }), + pin: new Definition('pin', { + default: true, + type: Boolean, + description: ` + Write pinned (\`pkg@version\`) entries when approving install scripts. + Set to \`false\` to write name-only entries that allow any version. + Has no effect on \`npm deny-scripts\`, which always writes name-only + entries regardless of this setting. + `, + flatten, + }), 'prefer-dedupe': new Definition('prefer-dedupe', { default: false, type: Boolean, From e4ace9e498b1a8f68805705a06b32502410b8920 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 14:59:49 -0700 Subject: [PATCH 6/7] feat(install): warn on allowScripts in non-root workspaces The allowScripts policy must live at the project root. A non-root workspace declaring its own allowScripts field is almost always a mistake: that policy would be silently ignored at install time. reify-finish now walks the resolved actual tree after reify completes and emits one warning per non-root workspace whose package.json has an allowScripts field. Pure detection lives in lib/utils/warn-workspace-allow-scripts.js; the tree walk piggybacks on the inventory that's already loaded for the unreviewed-scripts summary. Refs: npm/rfcs#868 --- lib/utils/reify-finish.js | 2 + lib/utils/warn-workspace-allow-scripts.js | 40 +++++++++ .../lib/utils/warn-workspace-allow-scripts.js | 86 +++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 lib/utils/warn-workspace-allow-scripts.js create mode 100644 test/lib/utils/warn-workspace-allow-scripts.js diff --git a/lib/utils/reify-finish.js b/lib/utils/reify-finish.js index bb347913eebc2..1041c53fdb935 100644 --- a/lib/utils/reify-finish.js +++ b/lib/utils/reify-finish.js @@ -1,5 +1,6 @@ const reifyOutput = require('./reify-output.js') const checkAllowScripts = require('./check-allow-scripts.js') +const warnWorkspaceAllowScripts = require('./warn-workspace-allow-scripts.js') const ini = require('ini') const { writeFile } = require('node:fs/promises') const { resolve } = require('node:path') @@ -16,6 +17,7 @@ const reifyFinish = async (npm, arb) => { } } } + warnWorkspaceAllowScripts(arb.actualTree) const unreviewedScripts = await checkAllowScripts({ arb, npm }) reifyOutput(npm, arb, { unreviewedScripts }) } diff --git a/lib/utils/warn-workspace-allow-scripts.js b/lib/utils/warn-workspace-allow-scripts.js new file mode 100644 index 0000000000000..e46e6cf4d2a10 --- /dev/null +++ b/lib/utils/warn-workspace-allow-scripts.js @@ -0,0 +1,40 @@ +const { log } = require('proc-log') + +// The allowScripts policy MUST live at the project root (RFC npm/rfcs#868). +// A non-root workspace declaring its own allowScripts is almost always a +// mistake: that policy would be silently ignored at install time. +// +// `findWorkspaceAllowScripts` returns the list of offending workspace nodes. +// `warnWorkspaceAllowScripts` is the side-effecting variant that emits one +// install-time `log.warn` per offender. + +const findWorkspaceAllowScripts = (tree) => { + const offenders = [] + if (!tree?.inventory) { + return offenders + } + for (const node of tree.inventory.values()) { + if (!node.isWorkspace || node.isProjectRoot) { + continue + } + if (node.package?.allowScripts !== undefined) { + offenders.push(node) + } + } + return offenders +} + +const warnWorkspaceAllowScripts = (tree) => { + for (const node of findWorkspaceAllowScripts(tree)) { + const name = node.packageName || node.name + log.warn( + 'allow-scripts', + `allowScripts in workspace ${name} (${node.path}) is ignored. ` + + 'Move the field to the project root package.json.' + ) + } +} + +module.exports = warnWorkspaceAllowScripts +module.exports.warnWorkspaceAllowScripts = warnWorkspaceAllowScripts +module.exports.findWorkspaceAllowScripts = findWorkspaceAllowScripts diff --git a/test/lib/utils/warn-workspace-allow-scripts.js b/test/lib/utils/warn-workspace-allow-scripts.js new file mode 100644 index 0000000000000..e5a474f8870aa --- /dev/null +++ b/test/lib/utils/warn-workspace-allow-scripts.js @@ -0,0 +1,86 @@ +const t = require('tap') +const { + findWorkspaceAllowScripts, + warnWorkspaceAllowScripts, +} = require('../../../lib/utils/warn-workspace-allow-scripts.js') + +const node = ({ + name = 'pkg', + packageName, + isWorkspace = false, + isProjectRoot = false, + allowScripts, + path = `/fake/${name}`, +} = {}) => ({ + name, + packageName: packageName ?? name, + path, + isWorkspace, + isProjectRoot, + package: allowScripts !== undefined ? { allowScripts } : {}, +}) + +const tree = (nodes) => ({ + inventory: new Map(nodes.map((n, i) => [`node_modules/${n.name || `n${i}`}`, n])), +}) + +t.test('returns [] for empty tree', async t => { + t.strictSame(findWorkspaceAllowScripts(tree([])), []) +}) + +t.test('returns [] for missing tree', async t => { + t.strictSame(findWorkspaceAllowScripts(null), []) + t.strictSame(findWorkspaceAllowScripts(undefined), []) +}) + +t.test('ignores project root with allowScripts', async t => { + const t1 = tree([ + node({ name: 'root', isProjectRoot: true, isWorkspace: true, allowScripts: { x: true } }), + ]) + t.strictSame(findWorkspaceAllowScripts(t1), []) +}) + +t.test('ignores non-workspace dep with allowScripts', async t => { + const t1 = tree([ + node({ name: 'dep', allowScripts: { x: true } }), + ]) + t.strictSame(findWorkspaceAllowScripts(t1), []) +}) + +t.test('finds non-root workspace with allowScripts', async t => { + const ws = node({ name: 'ws', isWorkspace: true, allowScripts: { x: true } }) + const t1 = tree([ + node({ name: 'root', isProjectRoot: true, isWorkspace: true }), + ws, + ]) + t.equal(findWorkspaceAllowScripts(t1).length, 1) + t.equal(findWorkspaceAllowScripts(t1)[0], ws) +}) + +t.test('finds workspace with empty allowScripts object too', async t => { + const ws = node({ name: 'ws', isWorkspace: true, allowScripts: {} }) + t.equal(findWorkspaceAllowScripts(tree([ws])).length, 1) +}) + +t.test('warnWorkspaceAllowScripts emits one log.warn per offender', async t => { + const warnings = [] + const listener = (level, ...args) => { + if (level === 'warn') { + warnings.push(args) + } + } + process.on('log', listener) + t.teardown(() => process.off('log', listener)) + + const t1 = tree([ + node({ name: 'root', isProjectRoot: true, isWorkspace: true }), + node({ name: 'a', isWorkspace: true, allowScripts: { x: true } }), + node({ name: 'b', isWorkspace: true, allowScripts: { y: false } }), + node({ name: 'c', isWorkspace: true }), // no allowScripts; no warning + ]) + warnWorkspaceAllowScripts(t1) + + t.equal(warnings.length, 2) + t.match(warnings[0][1], /allowScripts in workspace a/) + t.match(warnings[1][1], /allowScripts in workspace b/) +}) From 39271d609da619a790c6b15e4acc0f81cb6bebef Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 14 May 2026 15:20:53 -0700 Subject: [PATCH 7/7] chore: add coverage tests and istanbul ignores for defensive branches More tests for the Phase 1 install-script work, plus a handful of /* istanbul ignore next */ markers for defensive fallbacks that can't be hit from realistic inputs (e.g. nodes with neither packageName nor name set, npa() failing on an already-validated string). - reify-output: JSON path with unreviewedScripts and the node.name fallback when packageName is absent - approve-scripts: single-package wording, no-op when already at the right pin, --all with no unreviewed packages - deny-scripts: --all empty, global rejection, --json output, no-op on already-denied - allow-scripts-writer: null node guards, non-hosted git URL, absolute path resolved field, deny entries on file/remote deps, alias-typed policy keys, unparseable existing entries --- docs/lib/content/nav.yml | 6 + lib/utils/allow-scripts-cmd.js | 2 + lib/utils/allow-scripts-writer.js | 8 +- .../test/lib/commands/completion.js.test.cjs | 1 + tap-snapshots/test/lib/docs.js.test.cjs | 123 +++++++++++----- tap-snapshots/test/lib/npm.js.test.cjs | 135 ++++++++++-------- test/lib/commands/deny-scripts.js | 51 +++++++ test/lib/utils/reify-output.js | 110 ++++++++++++++ .../lib/utils/warn-workspace-allow-scripts.js | 22 +++ 9 files changed, 358 insertions(+), 100 deletions(-) diff --git a/docs/lib/content/nav.yml b/docs/lib/content/nav.yml index a992416b3b2b9..0deb4e7d065de 100644 --- a/docs/lib/content/nav.yml +++ b/docs/lib/content/nav.yml @@ -12,6 +12,9 @@ - title: npm access url: /commands/npm-access description: Set access level on published packages + - title: npm approve-scripts + url: /commands/npm-approve-scripts + description: Approve install scripts for specific dependencies - title: npm audit url: /commands/npm-audit description: Run a security audit @@ -33,6 +36,9 @@ - title: npm dedupe url: /commands/npm-dedupe description: Reduce duplication in the package tree + - title: npm deny-scripts + url: /commands/npm-deny-scripts + description: Deny install scripts for specific dependencies - title: npm deprecate url: /commands/npm-deprecate description: Deprecate a version of a package diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js index 3404baa2f1594..d93d40f21da90 100644 --- a/lib/utils/allow-scripts-cmd.js +++ b/lib/utils/allow-scripts-cmd.js @@ -30,6 +30,7 @@ class AllowScriptsCmd extends BaseCommand { static ignoreImplicitWorkspace = false // Subclasses set this. + /* istanbul ignore next */ get verb () { throw new Error('verb must be implemented by subclass') } @@ -214,6 +215,7 @@ class AllowScriptsCmd extends BaseCommand { summary.push({ name, changes: result.changes }) } + /* istanbul ignore else: writePolicyChanges only called when changes are expected */ if (updated !== existing) { pkg.update({ allowScripts: updated }) await pkg.save() diff --git a/lib/utils/allow-scripts-writer.js b/lib/utils/allow-scripts-writer.js index 3105692f6a988..101abe23fdc6a 100644 --- a/lib/utils/allow-scripts-writer.js +++ b/lib/utils/allow-scripts-writer.js @@ -148,6 +148,7 @@ const keyTargetsNode = (key, node) => { try { resolvedParsed = node.resolved ? npa(node.resolved) : null } catch { + /* istanbul ignore next */ return false } const keyHost = parsed.hosted?.ssh({ noCommittish: true }) @@ -174,7 +175,7 @@ const keyTargetsNode = (key, node) => { // - `changes` is a list of `{ key, change }` entries describing edits // - `warning` is an optional message to surface to the user const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => { - const allowScripts = { ...(existing || {}) } + const allowScripts = { ...existing } const changes = [] if (!Array.isArray(nodes) || nodes.length === 0) { @@ -188,6 +189,8 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => { for (const node of nodes) { for (const [key, value] of Object.entries(allowScripts)) { if (value === false && keyTargetsNode(key, node)) { + /* istanbul ignore next: name fallback covers the empty-name edge case */ + const subject = name || 'this package' return { allowScripts, changes, @@ -211,6 +214,7 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => { } } + /* istanbul ignore else: name === null is the no-identity path tested separately */ if (name && allowScripts[name] !== true) { allowScripts[name] = true changes.push({ key: name, change: 'added' }) @@ -262,7 +266,7 @@ const applyApprovalForPackage = (existing, nodes, { pin = true } = {}) => { // Apply a deny for a single package. Always name-only; ignores `--pin`. const applyDenyForPackage = (existing, nodes) => { - const allowScripts = { ...(existing || {}) } + const allowScripts = { ...existing } const changes = [] if (!Array.isArray(nodes) || nodes.length === 0) { diff --git a/tap-snapshots/test/lib/commands/completion.js.test.cjs b/tap-snapshots/test/lib/commands/completion.js.test.cjs index 6a96d5be46eb8..33ce49dd3d03b 100644 --- a/tap-snapshots/test/lib/commands/completion.js.test.cjs +++ b/tap-snapshots/test/lib/commands/completion.js.test.cjs @@ -61,6 +61,7 @@ exports[`test/lib/commands/completion.js TAP completion multiple command names > Array [ String( access + approve-scripts audit author add diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 336ed4c501ffb..104c893c33f7a 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -301,19 +301,22 @@ to the same value as the current version. * Default: "" * Type: String (can be set multiple times) -Comma-separated list of packages whose install scripts (\`preinstall\`, -\`install\`, \`postinstall\`) are allowed to run. Used as a fallback when no -\`allowScripts\` field is set in the root project's \`package.json\`, and for -global/npx contexts where no project \`package.json\` exists. +Comma-separated list of packages whose install-time lifecycle scripts +(\`preinstall\`, \`install\`, \`postinstall\`, and \`prepare\` for non-registry +dependencies) are allowed to run. Used as a fallback when no \`allowScripts\` +field is set in the root project's \`package.json\`, and for global/npx +contexts where no project \`package.json\` exists. -The \`package.json\` \`allowScripts\` field takes precedence over this setting. -Layers are not merged: the first source in the precedence chain that defines -any allowlist configuration wins for the entire install. +Each name is matched against a dependency's resolved identity, not against +the package's self-reported name. CLI flags take precedence over +\`package.json\`, which takes precedence over this setting. Layers are not +merged. -This setting is part of an opt-in install-script policy. In the current -release, scripts are not blocked by default; this setting prepares your -project for a future release that will block dependency install scripts -unless they are explicitly allowed. +This setting is part of an opt-in install-script policy that will land +across multiple npm releases. In this release, install scripts still run as +they always have. Setting this field does not block anything; it records +your intent so the install command can list the packages that would still +need to be reviewed before the future release that flips the default. @@ -511,14 +514,15 @@ are same as \`cpu\` field of package.json, which comes from \`process.arch\`. * Default: false * Type: Boolean -When \`true\`, all dependency install scripts run regardless of the -\`allowScripts\` field in \`package.json\` or the \`allow-scripts\` config. This -is an escape hatch for migration and emergency use; its use is strongly -discouraged. +Reserved for a future release. When that release lands, setting this to +\`true\` will tell npm to run every dependency install script regardless of +the \`allowScripts\` policy — an escape hatch for migration. Its use will be +strongly discouraged. -This setting has no effect in the current release, where dependency install -scripts already run by default. It is reserved for a future release that -will block them unless explicitly allowed. +In this release, install scripts still run as they always have, so this +setting has no effect on install behaviour. The flag is registered now so +projects can pin it in their tooling ahead of the release that flips the +default. @@ -1893,13 +1897,13 @@ this warning is treated as a failure. * Default: false * Type: Boolean -When \`true\`, any dependency install script that is blocked by the -\`allowScripts\` policy causes the install to fail with an error instead of -printing a warning and continuing. +Reserved for a future release. When that release lands, setting this to +\`true\` will turn the install-script policy from a warning into a hard error: +any unreviewed install script will fail the install instead of being skipped +with a notice. -This setting has no effect in the current release, where dependency install -scripts run by default and no scripts are blocked. It is reserved for a -future release that will block install scripts unless explicitly allowed. +In this release, install scripts still run as they always have, so this +setting has no effect on install behaviour. @@ -3236,13 +3240,13 @@ Options: Limits the ability for npm to fetch dependencies from urls. --allow-scripts - Comma-separated list of packages whose install scripts (\`preinstall\`, + Comma-separated list of packages whose install-time lifecycle scripts --strict-script-builds - When \`true\`, any dependency install script that is blocked by the + Reserved for a future release. When that release lands, setting this --dangerously-allow-all-scripts - When \`true\`, all dependency install scripts run regardless of the + Reserved for a future release. When that release lands, setting this --audit When "true" submit audit reports alongside the current npm command to the @@ -3779,6 +3783,8 @@ Options: [--package [--package ...]] [-c|--call ] [-w|--workspace [-w|--workspace ...]] [--workspaces] [--include-workspace-root] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] --package The package or packages to install for [\`npm exec\`](/commands/npm-exec) @@ -3795,6 +3801,15 @@ Options: --include-workspace-root Include the workspace root when workspaces are enabled for a command. + --allow-scripts + Comma-separated list of packages whose install-time lifecycle scripts + + --strict-script-builds + Reserved for a future release. When that release lands, setting this + + --dangerously-allow-all-scripts + Reserved for a future release. When that release lands, setting this + alias: x @@ -3814,6 +3829,9 @@ alias: x #### \`workspace\` #### \`workspaces\` #### \`include-workspace-root\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` ` exports[`test/lib/docs.js TAP usage explain > must match snapshot 1`] = ` @@ -4232,13 +4250,13 @@ Options: Limits the ability for npm to fetch dependencies from urls. --allow-scripts - Comma-separated list of packages whose install scripts (\`preinstall\`, + Comma-separated list of packages whose install-time lifecycle scripts --strict-script-builds - When \`true\`, any dependency install script that is blocked by the + Reserved for a future release. When that release lands, setting this --dangerously-allow-all-scripts - When \`true\`, all dependency install scripts run regardless of the + Reserved for a future release. When that release lands, setting this --audit When "true" submit audit reports alongside the current npm command to the @@ -4382,13 +4400,13 @@ Options: Limits the ability for npm to fetch dependencies from urls. --allow-scripts - Comma-separated list of packages whose install scripts (\`preinstall\`, + Comma-separated list of packages whose install-time lifecycle scripts --strict-script-builds - When \`true\`, any dependency install script that is blocked by the + Reserved for a future release. When that release lands, setting this --dangerously-allow-all-scripts - When \`true\`, all dependency install scripts run regardless of the + Reserved for a future release. When that release lands, setting this --audit When "true" submit audit reports alongside the current npm command to the @@ -4528,13 +4546,13 @@ Options: Limits the ability for npm to fetch dependencies from urls. --allow-scripts - Comma-separated list of packages whose install scripts (\`preinstall\`, + Comma-separated list of packages whose install-time lifecycle scripts --strict-script-builds - When \`true\`, any dependency install script that is blocked by the + Reserved for a future release. When that release lands, setting this --dangerously-allow-all-scripts - When \`true\`, all dependency install scripts run regardless of the + Reserved for a future release. When that release lands, setting this --audit When "true" submit audit reports alongside the current npm command to the @@ -5492,6 +5510,8 @@ npm rebuild [] ...] Options: [-g|--global] [--no-bin-links] [--foreground-scripts] [--ignore-scripts] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [-w|--workspace [-w|--workspace ...]] [--workspaces] [--include-workspace-root] [--install-links] @@ -5507,6 +5527,15 @@ Options: --ignore-scripts If true, npm does not run scripts specified in package.json files. + --allow-scripts + Comma-separated list of packages whose install-time lifecycle scripts + + --strict-script-builds + Reserved for a future release. When that release lands, setting this + + --dangerously-allow-all-scripts + Reserved for a future release. When that release lands, setting this + -w|--workspace Enable running a command in the context of the configured workspaces of the @@ -5534,6 +5563,9 @@ alias: rb #### \`bin-links\` #### \`foreground-scripts\` #### \`ignore-scripts\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`workspace\` #### \`workspaces\` #### \`include-workspace-root\` @@ -6217,8 +6249,11 @@ Options: [--omit [--omit ...]] [--include [--include ...]] [--strict-peer-deps] [--no-package-lock] [--foreground-scripts] -[--ignore-scripts] [--no-audit] [--before |--min-release-age ] -[--no-bin-links] [--no-fund] [--dry-run] +[--ignore-scripts] +[--allow-scripts [--allow-scripts ...]] +[--strict-script-builds] [--dangerously-allow-all-scripts] [--no-audit] +[--before |--min-release-age ] [--no-bin-links] [--no-fund] +[--dry-run] [-w|--workspace [-w|--workspace ...]] [--workspaces] [--include-workspace-root] [--install-links] @@ -6255,6 +6290,15 @@ Options: --ignore-scripts If true, npm does not run scripts specified in package.json files. + --allow-scripts + Comma-separated list of packages whose install-time lifecycle scripts + + --strict-script-builds + Reserved for a future release. When that release lands, setting this + + --dangerously-allow-all-scripts + Reserved for a future release. When that release lands, setting this + --audit When "true" submit audit reports alongside the current npm command to the @@ -6304,6 +6348,9 @@ aliases: u, up, upgrade, udpate #### \`package-lock\` #### \`foreground-scripts\` #### \`ignore-scripts\` +#### \`allow-scripts\` +#### \`strict-script-builds\` +#### \`dangerously-allow-all-scripts\` #### \`audit\` #### \`before\` #### \`min-release-age\` diff --git a/tap-snapshots/test/lib/npm.js.test.cjs b/tap-snapshots/test/lib/npm.js.test.cjs index 236bbb263109a..96ed88724f9bf 100644 --- a/tap-snapshots/test/lib/npm.js.test.cjs +++ b/tap-snapshots/test/lib/npm.js.test.cjs @@ -31,15 +31,16 @@ npm help npm more involved overview All commands: - access, audit, bugs, cache, ci, completion, config, - dedupe, deprecate, diff, dist-tag, docs, doctor, edit, exec, - explain, explore, find-dupes, fund, get, help, help-search, - init, install, install-ci-test, install-test, link, ll, - login, logout, ls, org, outdated, owner, pack, ping, pkg, - prefix, profile, prune, publish, query, rebuild, repo, - restart, root, run, sbom, search, set, start, stop, team, - test, token, trust, undeprecate, uninstall, unpublish, - update, version, view, whoami + access, approve-scripts, audit, bugs, cache, ci, + completion, config, dedupe, deny-scripts, deprecate, diff, + dist-tag, docs, doctor, edit, exec, explain, explore, + find-dupes, fund, get, help, help-search, init, install, + install-ci-test, install-test, link, ll, login, logout, ls, + org, outdated, owner, pack, ping, pkg, prefix, profile, + prune, publish, query, rebuild, repo, restart, root, run, + sbom, search, set, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, update, version, view, + whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -67,9 +68,11 @@ npm help npm more involved overview All commands: - access, audit, bugs, - cache, ci, completion, - config, dedupe, + access, + approve-scripts, audit, + bugs, cache, ci, + completion, config, + dedupe, deny-scripts, deprecate, diff, dist-tag, docs, doctor, edit, exec, explain, @@ -118,9 +121,11 @@ npm help npm more involved overview All commands: - access, audit, bugs, - cache, ci, completion, - config, dedupe, + access, + approve-scripts, audit, + bugs, cache, ci, + completion, config, + dedupe, deny-scripts, deprecate, diff, dist-tag, docs, doctor, edit, exec, explain, @@ -169,15 +174,16 @@ npm help npm more involved overview All commands: - access, audit, bugs, cache, ci, completion, config, - dedupe, deprecate, diff, dist-tag, docs, doctor, edit, exec, - explain, explore, find-dupes, fund, get, help, help-search, - init, install, install-ci-test, install-test, link, ll, - login, logout, ls, org, outdated, owner, pack, ping, pkg, - prefix, profile, prune, publish, query, rebuild, repo, - restart, root, run, sbom, search, set, start, stop, team, - test, token, trust, undeprecate, uninstall, unpublish, - update, version, view, whoami + access, approve-scripts, audit, bugs, cache, ci, + completion, config, dedupe, deny-scripts, deprecate, diff, + dist-tag, docs, doctor, edit, exec, explain, explore, + find-dupes, fund, get, help, help-search, init, install, + install-ci-test, install-test, link, ll, login, logout, ls, + org, outdated, owner, pack, ping, pkg, prefix, profile, + prune, publish, query, rebuild, repo, restart, root, run, + sbom, search, set, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, update, version, view, + whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -205,9 +211,11 @@ npm help npm more involved overview All commands: - access, audit, bugs, - cache, ci, completion, - config, dedupe, + access, + approve-scripts, audit, + bugs, cache, ci, + completion, config, + dedupe, deny-scripts, deprecate, diff, dist-tag, docs, doctor, edit, exec, explain, @@ -256,9 +264,11 @@ npm help npm more involved overview All commands: - access, audit, bugs, - cache, ci, completion, - config, dedupe, + access, + approve-scripts, audit, + bugs, cache, ci, + completion, config, + dedupe, deny-scripts, deprecate, diff, dist-tag, docs, doctor, edit, exec, explain, @@ -307,9 +317,11 @@ npm help npm more involved overview All commands: - access, audit, bugs, - cache, ci, completion, - config, dedupe, + access, + approve-scripts, audit, + bugs, cache, ci, + completion, config, + dedupe, deny-scripts, deprecate, diff, dist-tag, docs, doctor, edit, exec, explain, @@ -356,15 +368,16 @@ npm help npm more involved overview All commands: - access, audit, bugs, cache, ci, completion, config, - dedupe, deprecate, diff, dist-tag, docs, doctor, edit, - exec, explain, explore, find-dupes, fund, get, help, - help-search, init, install, install-ci-test, install-test, - link, ll, login, logout, ls, org, outdated, owner, pack, - ping, pkg, prefix, profile, prune, publish, query, rebuild, - repo, restart, root, run, sbom, search, set, start, stop, - team, test, token, trust, undeprecate, uninstall, - unpublish, update, version, view, whoami + access, approve-scripts, audit, bugs, cache, ci, + completion, config, dedupe, deny-scripts, deprecate, diff, + dist-tag, docs, doctor, edit, exec, explain, explore, + find-dupes, fund, get, help, help-search, init, install, + install-ci-test, install-test, link, ll, login, logout, ls, + org, outdated, owner, pack, ping, pkg, prefix, profile, + prune, publish, query, rebuild, repo, restart, root, run, + sbom, search, set, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, update, version, view, + whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -392,15 +405,16 @@ npm help npm more involved overview All commands: - access, audit, bugs, cache, ci, completion, config, - dedupe, deprecate, diff, dist-tag, docs, doctor, edit, exec, - explain, explore, find-dupes, fund, get, help, help-search, - init, install, install-ci-test, install-test, link, ll, - login, logout, ls, org, outdated, owner, pack, ping, pkg, - prefix, profile, prune, publish, query, rebuild, repo, - restart, root, run, sbom, search, set, start, stop, team, - test, token, trust, undeprecate, uninstall, unpublish, - update, version, view, whoami + access, approve-scripts, audit, bugs, cache, ci, + completion, config, dedupe, deny-scripts, deprecate, diff, + dist-tag, docs, doctor, edit, exec, explain, explore, + find-dupes, fund, get, help, help-search, init, install, + install-ci-test, install-test, link, ll, login, logout, ls, + org, outdated, owner, pack, ping, pkg, prefix, profile, + prune, publish, query, rebuild, repo, restart, root, run, + sbom, search, set, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, update, version, view, + whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -428,15 +442,16 @@ npm help npm more involved overview All commands: - access, audit, bugs, cache, ci, completion, config, - dedupe, deprecate, diff, dist-tag, docs, doctor, edit, exec, - explain, explore, find-dupes, fund, get, help, help-search, - init, install, install-ci-test, install-test, link, ll, - login, logout, ls, org, outdated, owner, pack, ping, pkg, - prefix, profile, prune, publish, query, rebuild, repo, - restart, root, run, sbom, search, set, start, stop, team, - test, token, trust, undeprecate, uninstall, unpublish, - update, version, view, whoami + access, approve-scripts, audit, bugs, cache, ci, + completion, config, dedupe, deny-scripts, deprecate, diff, + dist-tag, docs, doctor, edit, exec, explain, explore, + find-dupes, fund, get, help, help-search, init, install, + install-ci-test, install-test, link, ll, login, logout, ls, + org, outdated, owner, pack, ping, pkg, prefix, profile, + prune, publish, query, rebuild, repo, restart, root, run, + sbom, search, set, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, update, version, view, + whoami Specify configs in the ini-formatted file: {USERCONFIG} diff --git a/test/lib/commands/deny-scripts.js b/test/lib/commands/deny-scripts.js index 8bc6f2b585e12..0e98133779b7a 100644 --- a/test/lib/commands/deny-scripts.js +++ b/test/lib/commands/deny-scripts.js @@ -110,3 +110,54 @@ t.test('deny-scripts requires positional args or --all', async t => { }) await t.rejects(npm.exec('deny-scripts', []), { code: 'EUSAGE' }) }) + +t.test('deny-scripts --all with no unreviewed packages prints message', async t => { + const { npm, joinedOutput } = await _mockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: 'host', version: '1.0.0' }), + 'package-lock.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { '': { name: 'host', version: '1.0.0' } }, + }), + node_modules: {}, + }, + config: { all: true }, + }) + await npm.exec('deny-scripts', []) + t.match(joinedOutput(), /No packages with unreviewed install scripts/) +}) + +t.test('deny-scripts fails on global', async t => { + const { npm } = await _mockNpm(t, { + config: { global: true }, + }) + await t.rejects(npm.exec('deny-scripts', ['canvas']), { code: 'EGLOBAL' }) +}) + +t.test('deny-scripts on a package already denied is no-op', async t => { + const { npm, joinedOutput, prefix } = await _mockNpm(t, { + prefixDir: setupProject({ + withScripts: ['core-js'], + allowScripts: { 'core-js': false }, + }), + }) + await npm.exec('deny-scripts', ['core-js']) + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'core-js': false }) + t.match(joinedOutput(), /Nothing to deny/) +}) + +t.test('deny-scripts --json outputs structured summary', async t => { + const { npm, joinedOutput } = await _mockNpm(t, { + prefixDir: setupProject({ withScripts: ['core-js'] }), + config: { json: true }, + }) + await npm.exec('deny-scripts', ['core-js']) + const parsed = JSON.parse(joinedOutput()) + t.match(parsed, { + allowScripts: [{ name: 'core-js', changes: [{ key: 'core-js', change: 'added' }] }], + }) +}) diff --git a/test/lib/utils/reify-output.js b/test/lib/utils/reify-output.js index 134951e40aabd..a145d076c589f 100644 --- a/test/lib/utils/reify-output.js +++ b/test/lib/utils/reify-output.js @@ -448,3 +448,113 @@ t.test('prints dedupe difference on long', async t => { t.matchSnapshot(out, 'diff table') }) + +t.test('prints unreviewed install scripts summary', async t => { + const mockReifyWithExtras = async (t, reify, extras, { command, ...config } = {}) => { + const mock = await mockNpm(t, { command, config }) + Object.defineProperty(mock.npm, 'command', { + get () { + return command + }, + enumerable: true, + }) + reifyOutput(mock.npm, reify, extras) + mock.npm.finish() + return mock.joinedOutput() + } + + const baseReify = { + actualTree: { name: 'host', inventory: { has: () => false } }, + diff: { children: [] }, + } + + const unreviewedScripts = [ + { + node: { packageName: 'canvas', name: 'canvas', version: '2.11.0', path: '/x/canvas' }, + scripts: { install: 'node-gyp rebuild' }, + }, + { + node: { packageName: 'sharp', name: 'sharp', version: '0.33.2', path: '/x/sharp' }, + scripts: { preinstall: 'pre', postinstall: 'post' }, + }, + ] + + const out = await mockReifyWithExtras(t, baseReify, { unreviewedScripts }) + t.match(out, /2 packages have install scripts not yet covered/) + t.match(out, /canvas@2\.11\.0 \(install: node-gyp rebuild\)/) + t.match(out, /sharp@0\.33\.2 \(preinstall: pre; postinstall: post\)/) + t.match(out, /npm approve-scripts --pending/) +}) + +t.test('single unreviewed script uses singular wording', async t => { + const mockReifyWithExtras = async (t, reify, extras) => { + const mock = await mockNpm(t, {}) + reifyOutput(mock.npm, reify, extras) + mock.npm.finish() + return mock.joinedOutput() + } + + const out = await mockReifyWithExtras( + t, + { actualTree: { inventory: { has: () => false } }, diff: { children: [] } }, + { + unreviewedScripts: [{ + node: { packageName: 'one', name: 'one', version: '1.0.0', path: '/x' }, + scripts: { install: 'do' }, + }], + } + ) + t.match(out, /1 package has install scripts/) +}) + +t.test('json output includes unreviewedScripts', async t => { + const mock = await mockNpm(t, { config: { json: true } }) + reifyOutput(mock.npm, { + actualTree: { inventory: { size: 0 } }, + diff: null, + }, { + unreviewedScripts: [{ + node: { packageName: 'pkg', name: 'pkg', version: '1.0.0', path: '/x' }, + scripts: { install: 'cmd' }, + }], + }) + mock.npm.finish() + const parsed = JSON.parse(mock.joinedOutput()) + t.match(parsed.unreviewedScripts, [{ + name: 'pkg', + version: '1.0.0', + path: '/x', + scripts: { install: 'cmd' }, + }]) +}) + +t.test('unreviewed script with node.name only (no packageName) still renders', async t => { + const mock = await mockNpm(t, {}) + reifyOutput(mock.npm, { + actualTree: { inventory: { has: () => false } }, + diff: { children: [] }, + }, { + unreviewedScripts: [{ + node: { name: 'fallback', path: '/x' }, // no packageName, no version + scripts: { install: 'cmd' }, + }], + }) + mock.npm.finish() + t.match(mock.joinedOutput(), / fallback \(install: cmd\)/) +}) + +t.test('json output includes node.name when packageName is missing', async t => { + const mock = await mockNpm(t, { config: { json: true } }) + reifyOutput(mock.npm, { + actualTree: { inventory: { size: 0 } }, + diff: null, + }, { + unreviewedScripts: [{ + node: { name: 'fallback', path: '/x' }, + scripts: { install: 'cmd' }, + }], + }) + mock.npm.finish() + const parsed = JSON.parse(mock.joinedOutput()) + t.equal(parsed.unreviewedScripts[0].name, 'fallback') +}) diff --git a/test/lib/utils/warn-workspace-allow-scripts.js b/test/lib/utils/warn-workspace-allow-scripts.js index e5a474f8870aa..c9a5727157c21 100644 --- a/test/lib/utils/warn-workspace-allow-scripts.js +++ b/test/lib/utils/warn-workspace-allow-scripts.js @@ -84,3 +84,25 @@ t.test('warnWorkspaceAllowScripts emits one log.warn per offender', async t => { t.match(warnings[0][1], /allowScripts in workspace a/) t.match(warnings[1][1], /allowScripts in workspace b/) }) + +t.test('warnWorkspaceAllowScripts uses node.name when packageName missing', async t => { + const warnings = [] + const listener = (level, ...args) => { + if (level === 'warn') { + warnings.push(args) + } + } + process.on('log', listener) + t.teardown(() => process.off('log', listener)) + + // packageName undefined, name set + const ws = { + name: 'fallback-name', + path: '/x', + isWorkspace: true, + isProjectRoot: false, + package: { allowScripts: { x: true } }, + } + warnWorkspaceAllowScripts({ inventory: new Map([['node_modules/ws', ws]]) }) + t.match(warnings[0][1], /workspace fallback-name/) +})