From f1b278286a89851f8eb2b9b690f2cfce5b1dc049 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Tue, 1 Sep 2026 14:32:06 +0500 Subject: [PATCH] fix: dedupe allowScripts keys under install-strategy=linked --- lib/utils/allow-scripts-cmd.js | 8 +- test/lib/commands/approve-scripts.js | 106 +++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/lib/utils/allow-scripts-cmd.js b/lib/utils/allow-scripts-cmd.js index f07dc8d1dc504..3ed024da7565a 100644 --- a/lib/utils/allow-scripts-cmd.js +++ b/lib/utils/allow-scripts-cmd.js @@ -220,14 +220,18 @@ class AllowScriptsCmd extends BaseCommand { // use the URL-derived name; non-registry deps fall back to the dependency // edge name. A version or range on the arg narrows the match to installed // versions that satisfy it. Bundled deps are excluded for the same reason - // as --all. Args that match nothing are returned in `unmatched`. + // as --all. Links are excluded for the same reason the policy gate skips + // them: identity lives on the target, while a link only knows the relative + // path it points at, so under install-strategy=linked a package would + // otherwise get one `file:` key per symlink depth. Args that match nothing + // are returned in `unmatched`. const matched = [] const unmatched = [] for (const arg of args) { const { name: wantName, range } = parsePositional(arg) const found = [] for (const node of arb.actualTree.inventory.values()) { - if (node.isProjectRoot || node.isWorkspace || node.inBundle) { + if (node.isProjectRoot || node.isWorkspace || node.isLink || node.inBundle) { continue } const { name, version } = trustedDisplay(node) diff --git a/test/lib/commands/approve-scripts.js b/test/lib/commands/approve-scripts.js index 449382d19e987..0432f55618cd9 100644 --- a/test/lib/commands/approve-scripts.js +++ b/test/lib/commands/approve-scripts.js @@ -53,6 +53,92 @@ const setupProject = ({ allowScripts, withScripts = ['canvas'], noResolved = [] } } +// Mirrors the on-disk shape of an install-strategy=linked tree: real packages +// live in node_modules/.store/@-/node_modules/ and +// every consumer reaches them through a symlink, so canvas is reachable both +// from the root and from inside dep's store entry. +const setupLinkedProject = (t) => ({ + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { canvas: '*', dep: '*' }, + }, null, 2), + node_modules: { + '.package-lock.json': JSON.stringify({ + name: 'host', + lockfileVersion: 3, + packages: { + 'node_modules/.store/canvas@1.0.0-aaa/node_modules/canvas': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/canvas/-/canvas-1.0.0.tgz', + hasInstallScript: true, + }, + 'node_modules/.store/dep@1.0.0-bbb/node_modules/dep': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/dep/-/dep-1.0.0.tgz', + }, + 'node_modules/.store/dep@1.0.0-bbb/node_modules/canvas': { + resolved: 'node_modules/.store/canvas@1.0.0-aaa/node_modules/canvas', + link: true, + }, + 'node_modules/canvas': { + resolved: 'node_modules/.store/canvas@1.0.0-aaa/node_modules/canvas', + link: true, + }, + 'node_modules/dep': { + resolved: 'node_modules/.store/dep@1.0.0-bbb/node_modules/dep', + link: true, + }, + }, + }), + '.store': { + 'canvas@1.0.0-aaa': { + node_modules: { + canvas: { + 'package.json': JSON.stringify({ + name: 'canvas', + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + }, + }, + }, + 'dep@1.0.0-bbb': { + node_modules: { + dep: { + 'package.json': JSON.stringify({ + name: 'dep', + version: '1.0.0', + dependencies: { canvas: '*' }, + }), + }, + canvas: t.fixture('symlink', '../../canvas@1.0.0-aaa/node_modules/canvas'), + }, + }, + }, + canvas: t.fixture('symlink', '.store/canvas@1.0.0-aaa/node_modules/canvas'), + dep: t.fixture('symlink', '.store/dep@1.0.0-bbb/node_modules/dep'), + }, +}) + +const setupLocalProject = (t) => ({ + 'package.json': JSON.stringify({ + name: 'host', + version: '1.0.0', + dependencies: { local: 'file:./local' }, + }, null, 2), + local: { + 'package.json': JSON.stringify({ + name: 'local', + version: '1.0.0', + scripts: { install: 'echo install' }, + }), + }, + node_modules: { + local: t.fixture('symlink', '../local'), + }, +}) + t.test('approve-scripts --pending lists unreviewed packages', async t => { const { npm, joinedOutput } = await mockNpm(t, { prefixDir: setupProject({ withScripts: ['canvas', 'sharp'] }), @@ -99,6 +185,26 @@ t.test('approve-scripts writes pinned entry by default', async t => { t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true }) }) +t.test('approve-scripts writes one entry per package under install-strategy=linked', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupLinkedProject(t), + }) + 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 writes the file: entry for a local dependency', async t => { + const { npm, prefix } = await mockNpm(t, { + prefixDir: setupLocalProject(t), + }) + await npm.exec('approve-scripts', ['local']) + + const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8')) + t.strictSame(pkg.allowScripts, { 'file:../local': true }) +}) + t.test('approve-scripts --no-pin writes name-only entry', async t => { const { npm, prefix } = await mockNpm(t, { prefixDir: setupProject({ withScripts: ['canvas'] }),