From 7eab680d706cf51812f94f7790398471cea02355 Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Mon, 24 Aug 2026 18:50:07 +0000 Subject: [PATCH 1/2] fix(arborist): match allowScripts keys for local paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/utils/allow-scripts-writer.js | 4 +- test/lib/utils/allow-scripts-prune.js | 21 +++++ test/lib/utils/allow-scripts-writer.js | 39 +++++++++ workspaces/arborist/lib/script-allowed.js | 12 ++- workspaces/arborist/test/script-allowed.js | 97 +++++++++++++++++++++- 5 files changed, 169 insertions(+), 4 deletions(-) diff --git a/lib/utils/allow-scripts-writer.js b/lib/utils/allow-scripts-writer.js index 6964279f2f2e0..26d13b164e342 100644 --- a/lib/utils/allow-scripts-writer.js +++ b/lib/utils/allow-scripts-writer.js @@ -2,6 +2,7 @@ const npa = require('npm-package-arg') const { log } = require('proc-log') const { getTrustedRegistryIdentity, + matchFileOrDir, resolvedSourceSpecs, } = require('@npmcli/arborist/lib/script-allowed.js') @@ -150,7 +151,7 @@ const isNameOnlyKey = (key) => { const keyTargetsNode = (key, node) => { let parsed try { - parsed = npa(key) + parsed = npa(key, node?.root?.path) } catch { return false } @@ -179,6 +180,7 @@ const keyTargetsNode = (key, node) => { } case 'file': case 'directory': + return matchFileOrDir(node, parsed) case 'remote': return resolvedSourceSpecs(node) .some(resolved => resolved === parsed.saveSpec || resolved === parsed.fetchSpec) diff --git a/test/lib/utils/allow-scripts-prune.js b/test/lib/utils/allow-scripts-prune.js index 880b1dfe34371..ea39bc6ecffc7 100644 --- a/test/lib/utils/allow-scripts-prune.js +++ b/test/lib/utils/allow-scripts-prune.js @@ -1,4 +1,5 @@ const t = require('tap') +const path = require('node:path') const { classifyUnusedEntries } = require('../../../lib/utils/allow-scripts-prune.js') // Minimal registry node: `matches` derives name/version from the resolved URL. @@ -29,6 +30,26 @@ t.test('keeps entries that match an installed package with scripts', t => { t.end() }) +t.test('keeps a local file key matching its absolute resolved source', t => { + const rootPath = path.resolve('project') + const key = `file:${path.resolve(rootPath, 'local.tgz')}` + const local = { + name: 'local', + version: '1.0.0', + resolved: key, + root: { path: rootPath }, + isRegistryDependency: false, + } + const { remaining, removed } = classifyUnusedEntries( + { [key]: true }, + [{ node: local, hasScripts: true }] + ) + + t.same(remaining, { [key]: true }) + t.same(removed, []) + t.end() +}) + t.test('removes entries for packages no longer installed', t => { const { remaining, removed } = classifyUnusedEntries( { canvas: true, gone: true }, diff --git a/test/lib/utils/allow-scripts-writer.js b/test/lib/utils/allow-scripts-writer.js index 8edf25be3079c..c434bd35f4041 100644 --- a/test/lib/utils/allow-scripts-writer.js +++ b/test/lib/utils/allow-scripts-writer.js @@ -1,5 +1,6 @@ const t = require('tap') const path = require('node:path') +const isScriptAllowed = require('../../../workspaces/arborist/lib/script-allowed.js') const { applyApprovalForPackage, applyDenyForPackage, @@ -379,6 +380,21 @@ t.test('applyApprovalForPackage — file dep uses resolved as both keys', async t.strictSame(allowScripts, { 'file:../local': true }) }) +t.test('versionedKeyFor — local file key round-trips through policy matching', async t => { + const rootPath = path.resolve('project') + const local = { + name: 'local', + packageName: 'local', + version: '1.0.0', + resolved: `file:${path.resolve(rootPath, 'local.tgz')}`, + root: { path: rootPath }, + isRegistryDependency: false, + } + const key = versionedKeyFor(local) + + t.equal(isScriptAllowed(local, { [key]: true }), true) +}) + t.test('applyApprovalForPackage — empty nodes returns unchanged', async t => { const { allowScripts, changes } = applyApprovalForPackage({ x: true }, [], { pin: true }) t.strictSame(allowScripts, { x: true }) @@ -493,6 +509,29 @@ t.test('applyApprovalForPackage — file dep with deny entry blocks approval', a t.match(warning, /denied|versioned deny/) }) +t.test('applyApprovalForPackage — relative file deny matches absolute resolved', async t => { + const rootPath = path.resolve('project') + const resolved = `file:${path.resolve(rootPath, 'local.tgz')}` + const local = { + name: 'local', + packageName: 'local', + version: '1.0.0', + resolved, + root: { path: rootPath }, + isRegistryDependency: false, + } + const existing = { 'file:local.tgz': false } + const { allowScripts, changes, warning } = applyApprovalForPackage( + existing, + [local], + { pin: true } + ) + + t.strictSame(allowScripts, existing) + t.strictSame(changes, []) + 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( diff --git a/workspaces/arborist/lib/script-allowed.js b/workspaces/arborist/lib/script-allowed.js index 8c9b3fe118a8e..629625f8e4f8c 100644 --- a/workspaces/arborist/lib/script-allowed.js +++ b/workspaces/arborist/lib/script-allowed.js @@ -71,7 +71,7 @@ const isScriptAllowed = (node, policy) => { const matches = (node, key, failClosed) => { let parsed try { - parsed = npa(key) + parsed = npa(key, node?.root?.path) } catch { return false } @@ -328,8 +328,15 @@ const matchGit = (node, parsed) => { } const matchFileOrDir = (node, parsed) => { + // consistentResolve stores local sources as `file:` plus npa's absolute, + // platform-native fetchSpec. + const absoluteFileSpec = parsed.fetchSpec && `file:${parsed.fetchSpec}` return resolvedSourceSpecs(node) - .some(resolved => resolved === parsed.saveSpec || resolved === parsed.fetchSpec) + .some(resolved => + resolved === parsed.saveSpec || + resolved === parsed.fetchSpec || + resolved === absoluteFileSpec + ) } const matchRemote = (node, parsed) => { @@ -381,4 +388,5 @@ module.exports.matches = matches module.exports.isExactVersionDisjunction = isExactVersionDisjunction module.exports.getTrustedRegistryIdentity = getTrustedRegistryIdentity module.exports.resolvedSourceSpecs = resolvedSourceSpecs +module.exports.matchFileOrDir = matchFileOrDir module.exports.trustedDisplay = trustedDisplay diff --git a/workspaces/arborist/test/script-allowed.js b/workspaces/arborist/test/script-allowed.js index 218ccf1e28888..97f17613b7f90 100644 --- a/workspaces/arborist/test/script-allowed.js +++ b/workspaces/arborist/test/script-allowed.js @@ -1,6 +1,6 @@ const t = require('tap') const isScriptAllowed = require('../lib/script-allowed.js') -const { trustedDisplay } = isScriptAllowed +const { matchFileOrDir, trustedDisplay } = isScriptAllowed // Test nodes default to a consistent registry-tarball shape: the resolved // URL's name+version match the supplied name+version. Tests that need to @@ -196,6 +196,90 @@ t.test('local tarball key — npa parses *.tgz paths as type=file', t => { t.end() }) +t.test('local tarball key — relative key resolves from the project root', t => { + const path = require('node:path') + const rootPath = path.resolve('project') + const tgzPath = path.resolve(rootPath, 'local-pkg.tgz') + const tgzNode = node({ + name: 'local-pkg', + packageName: 'local-pkg', + version: '1.0.0', + resolved: `file:${tgzPath}`, + root: { path: rootPath }, + }) + + t.equal(isScriptAllowed(tgzNode, { 'file:local-pkg.tgz': true }), true) + t.equal(isScriptAllowed(tgzNode, { 'file:other-pkg.tgz': true }), null) + t.end() +}) + +t.test('local tarball key — matches consistentResolve Windows representation', t => { + const resolved = String.raw`file:C:\absolute\path\local-pkg.tgz` + const parsed = { + saveSpec: 'file:C:/absolute/path/local-pkg.tgz', + fetchSpec: String.raw`C:\absolute\path\local-pkg.tgz`, + } + + t.equal(matchFileOrDir({ resolved }, parsed), true) + t.equal(matchFileOrDir({ resolved }, { + saveSpec: 'file:C:/absolute/path/other-pkg.tgz', + fetchSpec: String.raw`C:\absolute\path\other-pkg.tgz`, + }), false) + t.end() +}) + +t.test('local tarball key — Windows absolute key forms match', { + skip: process.platform !== 'win32', +}, t => { + const resolved = String.raw`file:C:\project\local-pkg.tgz` + const tgzNode = node({ + name: 'local-pkg', + packageName: 'local-pkg', + version: '1.0.0', + resolved, + root: { path: String.raw`C:\project` }, + }) + + t.equal(isScriptAllowed(tgzNode, { + [String.raw`file:C:\project\local-pkg.tgz`]: true, + }), true) + t.equal(isScriptAllowed(tgzNode, { + 'file:C:/project/local-pkg.tgz': true, + }), true) + t.equal(isScriptAllowed(tgzNode, { + 'file:C:/project/other-pkg.tgz': true, + }), null) + t.end() +}) + +t.test('local tarball key — Windows UNC key matches', { + skip: process.platform !== 'win32', +}, t => { + const consistentResolve = require('../lib/consistent-resolve.js') + const key = 'file://server/share/local-pkg.tgz' + const tgzNode = node({ + name: 'local-pkg', + packageName: 'local-pkg', + version: '1.0.0', + resolved: consistentResolve(key), + root: { path: String.raw`C:\project` }, + }) + + t.equal(isScriptAllowed(tgzNode, { [key]: true }), true) + t.end() +}) + +t.test('file source matching keeps POSIX backslashes distinct', t => { + const resolved = String.raw`file:/tmp/local\pkg.tgz` + const parsed = { + saveSpec: 'file:/tmp/local/pkg.tgz', + fetchSpec: '/tmp/local/pkg.tgz', + } + + t.equal(matchFileOrDir({ resolved }, parsed), false) + t.end() +}) + t.test('remote tarball — exact resolved match', t => { const remoteNode = node({ name: 'pkg', @@ -208,6 +292,17 @@ t.test('remote tarball — exact resolved match', t => { t.end() }) +t.test('remote tarball key does not match a file source', t => { + const fileNode = node({ + name: 'pkg', + packageName: 'pkg', + version: '1.0.0', + resolved: 'file:https://example.com/pkg.tgz', + }) + t.equal(isScriptAllowed(fileNode, { 'https://example.com/pkg.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 From 4e5890afe9617faf0da2d2b9248a45ba6d1b793d Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Mon, 24 Aug 2026 19:39:33 +0000 Subject: [PATCH 2/2] test: fix allowScripts path coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/lib/utils/allow-scripts-writer.js | 2 ++ workspaces/arborist/test/script-allowed.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/lib/utils/allow-scripts-writer.js b/test/lib/utils/allow-scripts-writer.js index c434bd35f4041..f13389c8c258a 100644 --- a/test/lib/utils/allow-scripts-writer.js +++ b/test/lib/utils/allow-scripts-writer.js @@ -4,6 +4,7 @@ const isScriptAllowed = require('../../../workspaces/arborist/lib/script-allowed const { applyApprovalForPackage, applyDenyForPackage, + keyTargetsNode, nameKeyFor, versionedKeyFor, isSingleVersionPin, @@ -540,6 +541,7 @@ t.test('applyApprovalForPackage — remote tarball deny blocks approval', async { pin: true } ) t.match(warning, /denied|versioned deny/) + t.equal(keyTargetsNode('https://example.com/other.tgz', remote), false) }) t.test('applyApprovalForPackage — no-pin with no name produces no-op', async t => { diff --git a/workspaces/arborist/test/script-allowed.js b/workspaces/arborist/test/script-allowed.js index 97f17613b7f90..dccc674c7a79b 100644 --- a/workspaces/arborist/test/script-allowed.js +++ b/workspaces/arborist/test/script-allowed.js @@ -256,7 +256,7 @@ t.test('local tarball key — Windows UNC key matches', { skip: process.platform !== 'win32', }, t => { const consistentResolve = require('../lib/consistent-resolve.js') - const key = 'file://server/share/local-pkg.tgz' + const key = 'file:////server/share/local-pkg.tgz' const tgzNode = node({ name: 'local-pkg', packageName: 'local-pkg',