From dd6539d7cc26900c8c3d09c7900baae9d4191f26 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Wed, 24 Jun 2026 20:18:59 +0530 Subject: [PATCH] feat: namespace install-script approval commands under npm install-scripts (#9629) Add a namespaced `npm install-scripts` command that groups the install-script approval operations, following npm's existing `npm cache ` / `npm token ` convention: - `npm install-scripts approve ... | --all` - `npm install-scripts deny ... | --all` - `npm install-scripts ls` (list packages with unreviewed install scripts) The shipped `npm approve-scripts` and `npm deny-scripts` commands keep working as aliases for `approve` and `deny`, so this is additive and backwards compatible. The shared `AllowScriptsCmd` base now dispatches through `runMode(mode, args)`; the standalone commands route through it via `static verb`. The `--allow-scripts-pending` flag is only honored by the commands that declare it, so the namespace lists exclusively through `ls`. Closes #9545 Follow-up from RFC npm/rfcs#868. (cherry picked from commit 0c4dd414f213971c82fe0d620bbf34d7fae25f0d) --- .../content/commands/npm-install-scripts.md | 93 +++++++++ docs/lib/content/nav.yml | 3 + lib/commands/install-scripts.js | 49 +++++ lib/commands/rebuild.js | 4 +- lib/utils/allow-scripts-cmd.js | 46 +++-- lib/utils/allow-scripts-writer.js | 6 +- lib/utils/cmd-list.js | 1 + lib/utils/reify-output.js | 6 +- lib/utils/strict-allow-scripts-preflight.js | 16 +- .../tap-snapshots/test/index.js.test.cjs | 13 +- tap-snapshots/test/lib/docs.js.test.cjs | 41 ++++ tap-snapshots/test/lib/npm.js.test.cjs | 70 ++++--- test/lib/commands/deny-scripts.js | 5 +- test/lib/commands/install-scripts.js | 190 ++++++++++++++++++ test/lib/utils/allow-scripts-writer.js | 4 +- test/lib/utils/reify-output.js | 2 +- .../utils/strict-allow-scripts-preflight.js | 4 +- 17 files changed, 482 insertions(+), 71 deletions(-) create mode 100644 docs/lib/content/commands/npm-install-scripts.md create mode 100644 lib/commands/install-scripts.js create mode 100644 test/lib/commands/install-scripts.js diff --git a/docs/lib/content/commands/npm-install-scripts.md b/docs/lib/content/commands/npm-install-scripts.md new file mode 100644 index 0000000000000..6a003f5badf03 --- /dev/null +++ b/docs/lib/content/commands/npm-install-scripts.md @@ -0,0 +1,93 @@ +--- +title: npm-install-scripts +section: 1 +description: Manage install-script approvals for 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 is the recommended way to maintain that field. + +Dependency install scripts are blocked by default. Install commands +silently skip lifecycle scripts for any dependency that does not have a +matching entry in `allowScripts`, and end with a list of the packages +whose scripts were skipped so you can review them here. + +This command only works inside a project that has a `package.json`. Running +it with `--global` (`-g`) fails with an `EGLOBAL` error, since global +installs (`npm install -g`) and one-off executions (`npm exec` / `npx`) have +no project `package.json` to write to. To allow install scripts in those +contexts, use the `--allow-scripts` flag at install time (for example +`npm install -g --allow-scripts=canvas,sharp`) or persist the setting with +`npm config set allow-scripts=canvas,sharp --location=user`. + +There are three subcommands: + +```bash +npm install-scripts approve [ ...] +npm install-scripts approve --all +npm install-scripts deny [ ...] +npm install-scripts deny --all +npm install-scripts ls +``` + +`approve` allows install scripts for the named packages. `` matches +every installed version of that package. By default it writes pinned entries +(`pkg@1.2.3`), which keep their approval narrowed to the specific version you +reviewed. Pass `--no-allow-scripts-pin` to write name-only entries that allow +any future version. `--all` approves every package with unreviewed install +scripts in one go. + +`deny` records an explicit denial for the named packages (a name-only `false` +entry), which survives `npm install-scripts approve --all` and excludes the +package from any future blanket approval. `--all` denies every package with +unreviewed install scripts. + +`ls` is read-only: it lists every package whose install scripts are not yet +covered by `allowScripts`, without modifying `package.json`. + +`approve` 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` will not silently re-allow a package you +previously denied. + +The standalone commands [`npm approve-scripts`](/commands/npm-approve-scripts) +and [`npm deny-scripts`](/commands/npm-deny-scripts) are aliases for +`npm install-scripts approve` and `npm install-scripts deny`. + +### Examples + +```bash +# Approve all currently-installed install scripts after reviewing them +npm install-scripts approve --all + +# Approve specific packages, pinned to their installed version +npm install-scripts approve canvas sharp + +# Deny a package so it stays blocked +npm install-scripts deny telemetry-pkg + +# Preview which packages still need review +npm install-scripts ls +``` + +### Configuration + + + +### See Also + +* [npm approve-scripts](/commands/npm-approve-scripts) +* [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/nav.yml b/docs/lib/content/nav.yml index 0fa0ada53e03b..93a72bc2389d4 100644 --- a/docs/lib/content/nav.yml +++ b/docs/lib/content/nav.yml @@ -93,6 +93,9 @@ - title: npm install-ci-test url: /commands/npm-install-ci-test description: Install a project with a clean slate and run tests + - title: npm install-scripts + url: /commands/npm-install-scripts + description: Manage install-script approvals for dependencies - title: npm install-test url: /commands/npm-install-test description: Install package(s) and run tests diff --git a/lib/commands/install-scripts.js b/lib/commands/install-scripts.js new file mode 100644 index 0000000000000..ab2eafc85b7b1 --- /dev/null +++ b/lib/commands/install-scripts.js @@ -0,0 +1,49 @@ +const AllowScriptsCmd = require('../utils/allow-scripts-cmd.js') + +// Namespaced front-end for managing install-script approvals. +// `approve` and `deny` write the `allowScripts` policy; `ls` lists packages with unreviewed install scripts. +// The standalone `npm approve-scripts` and `npm deny-scripts` commands remain as aliases for `approve` and `deny`. +class InstallScripts extends AllowScriptsCmd { + static description = 'Manage install-script approvals for dependencies' + static name = 'install-scripts' + static usage = [ + 'approve [ ...]', + 'approve --all', + 'deny [ ...]', + 'deny --all', + 'ls', + ] + + static params = ['all', 'allow-scripts-pin', 'json'] + + static async completion (opts) { + const argv = opts.conf.argv.remain + const subcommands = ['approve', 'deny', 'ls'] + if (argv.length === 2) { + return subcommands + } + if (subcommands.includes(argv[2])) { + return [] + } + throw new Error(`${argv[2]} not recognized`) + } + + async exec (args) { + const [sub, ...rest] = args + switch (sub) { + case 'approve': + return this.runMode('approve', rest) + case 'deny': + return this.runMode('deny', rest) + case 'ls': + case 'list': + return this.runMode('list', rest) + default: + throw this.usageError( + sub ? `\`${sub}\` is not a recognized subcommand.` : undefined + ) + } + } +} + +module.exports = InstallScripts diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js index 3f8cb98154d55..17595aa4eb72e 100644 --- a/lib/commands/rebuild.js +++ b/lib/commands/rebuild.js @@ -75,13 +75,13 @@ class Rebuild extends ArboristWorkspaceCmd { if (unreviewed.length > 0) { const count = unreviewed.length const noun = count === 1 ? 'package has' : 'packages have' - // `npm approve-scripts` writes to a project package.json, which doesn't + // `npm install-scripts` writes to a project package.json, which doesn't // exist for global rebuilds. Point global users at `npm config set`, // which writes the `allow-scripts` setting to their user .npmrc. const names = unreviewed.map(({ node }) => trustedDisplay(node).name) const remediation = this.npm.global ? `Run \`${configSetAllowScripts(names)}\` to allow their scripts.` - : 'Run `npm approve-scripts --allow-scripts-pending` to review.' + : 'Run `npm install-scripts ls` to review.' log.warn( 'rebuild', `${count} ${noun} install scripts not yet covered by allowScripts. ` + diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js index b6ec663124fe8..da557165c97dc 100644 --- a/lib/utils/allow-scripts-cmd.js +++ b/lib/utils/allow-scripts-cmd.js @@ -34,8 +34,9 @@ const parsePositional = (arg) => { return { name, range: null } } -// Shared implementation for `npm approve-scripts` and `npm deny-scripts`. -// Subclasses set `verb` to `'approve'` or `'deny'`. +// Shared implementation for `npm approve-scripts`, `npm deny-scripts`, and the `npm install-scripts` namespace. +// `npm install-scripts` dispatches to `runMode('approve' | 'deny' | 'list', ...)`. +// The standalone commands set `static verb` and run through the default `exec`. // // Extends `BaseCommand` rather than `ArboristCmd` on purpose. Per RFC, // `allowScripts` is read from the workspace root's `package.json` only; @@ -48,13 +49,23 @@ class AllowScriptsCmd extends BaseCommand { static params = ['all', 'allow-scripts-pending', 'allow-scripts-pin', 'json'] static ignoreImplicitWorkspace = false - // Subclasses set `static verb = 'approve' | 'deny'`. + // Mode of the current run, set by runMode. + // One of 'approve', 'deny', or 'list'. + #mode = null + + // verb drives the writers and summaries, which only run in the two write modes, so it is never read while listing. get verb () { - /* istanbul ignore next: every concrete subclass declares static verb */ - return this.constructor.verb + return this.#mode } + // Standalone `npm approve-scripts` / `npm deny-scripts` pick their mode from `static verb`. async exec (args) { + return this.runMode(this.constructor.verb, args) + } + + async runMode (mode, args) { + this.#mode = mode + if (this.npm.global) { throw Object.assign( new Error(`\`npm ${this.constructor.name}\` does not work for global installs`), @@ -62,19 +73,27 @@ class AllowScriptsCmd extends BaseCommand { ) } - const pending = !!this.npm.config.get('allow-scripts-pending') + // `--allow-scripts-pending` is only honored by commands that declare it; the namespace lists via `ls` instead. + const pending = this.constructor.params.includes('allow-scripts-pending') && + !!this.npm.config.get('allow-scripts-pending') const all = !!this.npm.config.get('all') + // The `ls` subcommand lists, and so does `--allow-scripts-pending` on the write commands. + const list = mode === 'list' || pending - if (pending && (args.length > 0 || all)) { + if (list && (args.length > 0 || all)) { + const what = mode === 'list' ? '`npm install-scripts ls`' : '`--allow-scripts-pending`' throw this.usageError( - '`--allow-scripts-pending` cannot be combined with positional arguments or `--all`.' + `${what} cannot be combined with positional arguments or \`--all\`.` ) } - if (!pending && !all && args.length === 0) { + if (!list && !all && args.length === 0) { throw this.usageError() } - if (this.verb === 'deny' && pending) { - throw this.usageError('`npm deny-scripts --allow-scripts-pending` is not supported.') + if (mode === 'deny' && pending) { + throw this.usageError( + '`npm deny-scripts --allow-scripts-pending` is not supported; ' + + 'run `npm install-scripts ls` to list unreviewed packages.' + ) } const Arborist = require('@npmcli/arborist') @@ -91,7 +110,7 @@ class AllowScriptsCmd extends BaseCommand { // only lists; nothing runs. const unreviewed = await checkAllowScripts({ arb, npm: this.npm, includeWhenIgnored: true }) - if (pending) { + if (list) { return this.runPending(unreviewed) } @@ -129,7 +148,8 @@ class AllowScriptsCmd extends BaseCommand { } output.standard('') output.standard( - 'Run `npm approve-scripts ` to allow, or `npm deny-scripts ` to deny.' + 'Run `npm install-scripts approve ` to allow, ' + + 'or `npm install-scripts deny ` to deny.' ) } diff --git a/lib/utils/allow-scripts-writer.js b/lib/utils/allow-scripts-writer.js index d5c6dfa21f38b..b9476905d427b 100644 --- a/lib/utils/allow-scripts-writer.js +++ b/lib/utils/allow-scripts-writer.js @@ -108,15 +108,15 @@ const isSingleVersionPin = (key) => { // 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. +// the user can either widen it via `npm install-scripts deny ` 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}\` ` + + return `${key} is a versioned deny; run \`npm install-scripts deny ${widenTarget}\` ` + `to widen the deny to all versions of ${widenTarget}, or remove the entry ` + `to approve ${subject}.` } diff --git a/lib/utils/cmd-list.js b/lib/utils/cmd-list.js index 1909df0d04546..8816d8294716e 100644 --- a/lib/utils/cmd-list.js +++ b/lib/utils/cmd-list.js @@ -31,6 +31,7 @@ const commands = [ 'init', 'install', 'install-ci-test', + 'install-scripts', 'install-test', 'link', 'll', diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js index 1ccbd4d0cad71..dcef8d0aac845 100644 --- a/lib/utils/reify-output.js +++ b/lib/utils/reify-output.js @@ -268,7 +268,7 @@ const unreviewedScriptsMessage = (npm, unreviewedScripts) => { ) } -// `npm approve-scripts` writes to a project package.json, which doesn't +// `npm install-scripts` writes to a project package.json, which doesn't // exist for global installs (it throws EGLOBAL). For those, point users at // the mechanism that does work globally: the `--allow-scripts` flag for a // one-off, or `npm config set allow-scripts` to persist it. @@ -282,8 +282,8 @@ const remediationLines = (npm, names) => { ] } return [ - 'Run `npm approve-scripts --allow-scripts-pending` to review, ' + - 'or `npm approve-scripts ` to allow.', + 'Run `npm install-scripts ls` to review, ' + + 'or `npm install-scripts approve ` to allow.', ] } diff --git a/lib/utils/strict-allow-scripts-preflight.js b/lib/utils/strict-allow-scripts-preflight.js index d3575289fa8ed..0c500018184c2 100644 --- a/lib/utils/strict-allow-scripts-preflight.js +++ b/lib/utils/strict-allow-scripts-preflight.js @@ -48,19 +48,19 @@ const strictAllowScriptsPreflight = async ({ arb, npm, idealTreeOpts }) => { return ` ${label} (${events})` }).join('\n') - // `npm approve-scripts` / `npm deny-scripts` write to a project - // package.json, which doesn't exist for global installs. Point global - // users at the `--allow-scripts` flag and `npm config set allow-scripts`, - // which both work for global installs. Use the trusted display identity - // so the suggested `npm config set` value matches what the policy matches - // on, not the tarball's self-reported name. + // `npm install-scripts` writes to a project package.json, which doesn't + // exist for global installs. Point global users at the `--allow-scripts` + // flag and `npm config set allow-scripts`, which both work for global + // installs. Use the trusted display identity so the suggested `npm config + // set` value matches what the policy matches on, not the tarball's + // self-reported name. const names = unreviewed.map(({ node }) => trustedDisplay(node).name) const remediation = npm.global ? 'Allow them with `--allow-scripts`, persist them with ' + `\`${configSetAllowScripts(names)}\`, or bypass this ` + 'check with `--dangerously-allow-all-scripts`.' - : 'Approve them with `npm approve-scripts`, deny them with ' + - '`npm deny-scripts`, or bypass this check with ' + + : 'Approve them with `npm install-scripts approve`, deny them with ' + + '`npm install-scripts deny`, or bypass this check with ' + '`--dangerously-allow-all-scripts`.' throw Object.assign( diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs index 4818e012c02ac..dcdfc3df55abf 100644 --- a/smoke-tests/tap-snapshots/test/index.js.test.cjs +++ b/smoke-tests/tap-snapshots/test/index.js.test.cjs @@ -25,12 +25,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, update, version, + view, whoami Specify configs in the ini-formatted file: {NPM}/{TESTDIR}/home/.npmrc diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 78f2ea78422aa..fda666dd49e98 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -125,6 +125,7 @@ Array [ "init", "install", "install-ci-test", + "install-scripts", "install-test", "link", "ll", @@ -4587,6 +4588,46 @@ aliases: cit, clean-install-test, sit #### \`install-links\` ` +exports[`test/lib/docs.js TAP usage install-scripts > must match snapshot 1`] = ` +Manage install-script approvals for dependencies + +Usage: +npm install-scripts approve [ ...] +npm install-scripts approve --all +npm install-scripts deny [ ...] +npm install-scripts deny --all +npm install-scripts ls + +Options: +[-a|--all] [--no-allow-scripts-pin] [--json] + + -a|--all + Show or act on all packages, not just the ones your project directly + + --allow-scripts-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 install-scripts" for more info + +\`\`\`bash +npm install-scripts approve [ ...] +npm install-scripts approve --all +npm install-scripts deny [ ...] +npm install-scripts deny --all +npm install-scripts ls +\`\`\` + +Note: This command is unaware of workspaces. + +#### \`all\` +#### \`allow-scripts-pin\` +#### \`json\` +` + exports[`test/lib/docs.js TAP usage install-test > must match snapshot 1`] = ` Install package(s) and run tests diff --git a/tap-snapshots/test/lib/npm.js.test.cjs b/tap-snapshots/test/lib/npm.js.test.cjs index dc1c95cd3c576..a3837dc72ab76 100644 --- a/tap-snapshots/test/lib/npm.js.test.cjs +++ b/tap-snapshots/test/lib/npm.js.test.cjs @@ -35,12 +35,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, update, version, + view, whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -81,6 +82,7 @@ All commands: help-search, init, install, install-ci-test, + install-scripts, install-test, link, ll, login, logout, ls, org, outdated, owner, pack, @@ -136,6 +138,7 @@ All commands: help-search, init, install, install-ci-test, + install-scripts, install-test, link, ll, login, logout, ls, org, outdated, owner, pack, @@ -182,12 +185,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, update, version, + view, whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -228,6 +232,7 @@ All commands: help-search, init, install, install-ci-test, + install-scripts, install-test, link, ll, login, logout, ls, org, outdated, owner, pack, @@ -283,6 +288,7 @@ All commands: help-search, init, install, install-ci-test, + install-scripts, install-test, link, ll, login, logout, ls, org, outdated, owner, pack, @@ -337,6 +343,7 @@ All commands: fund, get, help, help-search, init, install, install-ci-test, + install-scripts, install-test, link, ll, login, logout, ls, org, outdated, owner, pack, @@ -383,12 +390,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, update, version, + view, whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -420,12 +428,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, update, version, + view, whoami Specify configs in the ini-formatted file: {USERCONFIG} @@ -457,12 +466,13 @@ All commands: 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, shrinkwrap, stage, star, stars, start, - stop, team, test, token, trust, undeprecate, uninstall, - unpublish, unstar, update, version, view, whoami + install-ci-test, install-scripts, 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, shrinkwrap, stage, + star, stars, start, stop, team, test, token, trust, + undeprecate, uninstall, unpublish, unstar, 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 358b71d7da7bd..1caf93b715134 100644 --- a/test/lib/commands/deny-scripts.js +++ b/test/lib/commands/deny-scripts.js @@ -80,7 +80,10 @@ t.test('deny-scripts --pending is rejected', async t => { prefixDir: setupProject({ withScripts: ['core-js'] }), config: { 'allow-scripts-pending': true }, }) - await t.rejects(npm.exec('deny-scripts', []), { code: 'EUSAGE' }) + await t.rejects(npm.exec('deny-scripts', []), { + code: 'EUSAGE', + message: /`npm deny-scripts --allow-scripts-pending` is not supported; run `npm install-scripts ls` to list unreviewed packages/, + }) }) t.test('deny-scripts --all denies every unreviewed package', async t => { diff --git a/test/lib/commands/install-scripts.js b/test/lib/commands/install-scripts.js new file mode 100644 index 0000000000000..6548dd617a32a --- /dev/null +++ b/test/lib/commands/install-scripts.js @@ -0,0 +1,190 @@ +const t = require('tap') +const fs = require('node:fs') +const { resolve } = require('node:path') +const _mockNpm = require('../../fixtures/mock-npm') +const InstallScripts = require('../../../lib/commands/install-scripts.js') + +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) { + nodeModules[name] = { + 'package.json': JSON.stringify({ + name, + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + } + lockPackages[`node_modules/${name}`] = { + version: '1.0.0', + hasInstallScript: true, + resolved: `https://registry.npmjs.org/${name}/-/${name}-1.0.0.tgz`, + } + } + + 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('completion', async t => { + const comp = (argv) => + InstallScripts.completion({ conf: { argv: { remain: argv } } }) + + t.resolveMatch(comp(['npm', 'install-scripts']), ['approve', 'deny', 'ls']) + t.resolveMatch(comp(['npm', 'install-scripts', 'approve']), []) + t.resolveMatch(comp(['npm', 'install-scripts', 'deny']), []) + t.resolveMatch(comp(['npm', 'install-scripts', 'ls']), []) + await t.rejects(comp(['npm', 'install-scripts', 'frobnicate']), { + message: 'frobnicate not recognized', + }) +}) + +t.test('install-scripts approve writes a pinned entry', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await npm.exec('install-scripts', ['approve', 'canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true }) +}) + +t.test('install-scripts approve --all approves every unreviewed package', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), + config: { all: true }, + }) + await npm.exec('install-scripts', ['approve']) + + 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('install-scripts deny writes a name-only false entry', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await npm.exec('install-scripts', ['deny', 'canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { canvas: false }) +}) + +t.test('install-scripts deny --all denies every unreviewed package', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), + config: { all: true }, + }) + await npm.exec('install-scripts', ['deny']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { canvas: false, sharp: false }) +}) + +t.test('install-scripts ignores allow-scripts-pending and still writes', async t => { + // The namespace exposes listing through `ls`, so a stray + // `allow-scripts-pending` config must not divert approve into list mode. + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { 'allow-scripts-pending': true }, + }) + await npm.exec('install-scripts', ['approve', 'canvas']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true }) +}) + +t.test('install-scripts ls lists unreviewed packages', async t => { + const { npm, joinedOutput } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), + }) + await npm.exec('install-scripts', ['ls']) + const out = joinedOutput() + t.match(out, /2 packages have install scripts blocked because they are not covered by allowScripts/) + t.match(out, /canvas@1\.0\.0/) + t.match(out, /sharp@1\.0\.0/) +}) + +t.test('install-scripts ls with no unreviewed says so', async t => { + const { npm, joinedOutput } = await mockNpm(t, { + prefixDir: setupProject({ allowScripts: { canvas: true }, withScripts: ['canvas'] }), + }) + await npm.exec('install-scripts', ['ls']) + t.match(joinedOutput(), /No packages with unreviewed install scripts/) +}) + +t.test('install-scripts ls rejects positional args', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects( + npm.exec('install-scripts', ['ls', 'canvas']), + /cannot be combined with positional arguments/ + ) +}) + +t.test('install-scripts with no subcommand errors with usage', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects( + npm.exec('install-scripts', []), + { code: 'EUSAGE' } + ) +}) + +t.test('install-scripts with an unknown subcommand errors', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects( + npm.exec('install-scripts', ['frobnicate']), + /`frobnicate` is not a recognized subcommand/ + ) +}) + +t.test('install-scripts approve errors on unknown package', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + }) + await t.rejects( + npm.exec('install-scripts', ['approve', 'not-installed']), + { code: 'ENOMATCH' } + ) +}) + +t.test('install-scripts fails for global installs', async t => { + const { npm } = await mockNpm(t, { + prefixDir: setupProject({ withScripts: ['canvas'] }), + config: { global: true }, + }) + await t.rejects( + npm.exec('install-scripts', ['approve', 'canvas']), + { code: 'EGLOBAL' } + ) +}) diff --git a/test/lib/utils/allow-scripts-writer.js b/test/lib/utils/allow-scripts-writer.js index e4e933e2f7c0d..8edf25be3079c 100644 --- a/test/lib/utils/allow-scripts-writer.js +++ b/test/lib/utils/allow-scripts-writer.js @@ -704,7 +704,7 @@ t.test('denyWarning branches on key shape per RFC §approve-scripts', async t => { pin: true } ) t.match(pinned.warning, /versioned deny/) - t.match(pinned.warning, /npm deny-scripts canvas/) + t.match(pinned.warning, /npm install-scripts deny canvas/) t.match(pinned.warning, /widen the deny to all versions/) t.match(pinned.warning, /remove the entry/) @@ -715,7 +715,7 @@ t.test('denyWarning branches on key shape per RFC §approve-scripts', async t => { pin: true } ) t.match(multi.warning, /versioned deny/) - t.match(multi.warning, /npm deny-scripts canvas/) + t.match(multi.warning, /npm install-scripts deny canvas/) }) t.test('denyWarning: tag-type key (pkg@latest: false) is name-only', async t => { diff --git a/test/lib/utils/reify-output.js b/test/lib/utils/reify-output.js index 597a03374815b..596272f21fa97 100644 --- a/test/lib/utils/reify-output.js +++ b/test/lib/utils/reify-output.js @@ -484,7 +484,7 @@ t.test('prints unreviewed install scripts summary', async t => { t.match(warn, /2 packages have install scripts not yet covered/) t.match(warn, /canvas@2\.11\.0 \(install: node-gyp rebuild\)/) t.match(warn, /sharp@0\.33\.2 \(preinstall: pre; postinstall: post\)/) - t.match(warn, /npm approve-scripts --allow-scripts-pending/) + t.match(warn, /npm install-scripts ls/) }) t.test('global install suggests --allow-scripts, not approve-scripts', async t => { diff --git a/test/lib/utils/strict-allow-scripts-preflight.js b/test/lib/utils/strict-allow-scripts-preflight.js index e85a31f2c1a39..c67a6e4853a12 100644 --- a/test/lib/utils/strict-allow-scripts-preflight.js +++ b/test/lib/utils/strict-allow-scripts-preflight.js @@ -203,7 +203,7 @@ t.test('error label falls back to node.name when package.version is missing', as ) }) -t.test('project-scoped error suggests approve-scripts / deny-scripts', async t => { +t.test('project-scoped error suggests install-scripts approve / deny', async t => { const arb = makeArb({ ideal: tree([node({ name: 'canvas' })]) }) await t.rejects( preflight({ @@ -211,7 +211,7 @@ t.test('project-scoped error suggests approve-scripts / deny-scripts', async t = npm: { flatOptions: { strictAllowScripts: true } }, idealTreeOpts: {}, }), - { message: /Approve them with `npm approve-scripts`/ } + { message: /Approve them with `npm install-scripts approve`/ } ) })