From a6f9044dd5b21161a0396957e5226466fb1cc0ad Mon Sep 17 00:00:00 2001 From: arjun-vegeta <149392539+arjun-vegeta@users.noreply.github.com> Date: Thu, 25 Jun 2026 01:37:28 +0530 Subject: [PATCH 1/3] fix(exec): prevent shared binPaths pollution across workspace runs --- workspaces/libnpmexec/lib/index.js | 10 +++-- workspaces/libnpmexec/test/local.js | 63 +++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 35452f796246b..4d7c126654689 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -19,8 +19,6 @@ const runScript = require('./run-script.js') const isWindows = require('./is-windows.js') const withLock = require('./with-lock.js') -const binPaths = [] - // when checking the local tree we look up manifests, cache those results by // spec.raw so we don't have to fetch again when we check npxCache const manifests = new Map() @@ -113,6 +111,8 @@ const exec = async (opts) => { ...flatOptions } = opts + const binPaths = [] + let pkgPaths = opts.pkgPath if (typeof pkgPaths === 'string') { pkgPaths = [pkgPaths] @@ -196,7 +196,11 @@ const exec = async (opts) => { let commandManifest await Promise.all(packages.map(async (pkg, i) => { const spec = npa(pkg, path) - const { manifest, node } = await missingFromTree({ spec, tree: localTree, flatOptions }) + const { manifest, node } = await missingFromTree({ + spec, + tree: localTree, + flatOptions, + }) if (manifest) { // Package does not exist in the local tree needInstall.push({ spec, manifest }) diff --git a/workspaces/libnpmexec/test/local.js b/workspaces/libnpmexec/test/local.js index 2423440c9d82c..46700ea535f81 100644 --- a/workspaces/libnpmexec/test/local.js +++ b/workspaces/libnpmexec/test/local.js @@ -469,3 +469,66 @@ for (const allowDirectory of ['none', 'root']) { }) }) } + +t.test('npm exec sequential workspace runs with same-named local bins', async t => { + t.plan(2) + const { path, readOutput, rmOutput, registry } = setup(t, { + testdir: { + packages: { + a: { + 'package.json': { + name: 'workspace-a', + version: '1.0.0', + bin: { 'shared-bin': 'cli.js' }, + }, + 'cli.js': { key: 'shared-bin', value: 'A' }, + }, + b: { + 'package.json': { + name: 'workspace-b', + version: '1.0.0', + bin: { 'shared-bin': 'cli.js' }, + }, + 'cli.js': { key: 'shared-bin', value: 'B' }, + }, + }, + }, + }) + + // We mock the module exactly once. This simulates two sequential calls + // hitting the same module instance, allowing us to prove that state + // (like binPaths) is correctly cleared between runs and NOT preserved. + const libnpmexec = t.mock('../lib/index.js') + + const baseOpts = { + path, + runPath: path, + npxCache: resolve(path, 'npxCache'), + registry: registry.origin + '/', + localBin: resolve(path, 'node_modules', '.bin'), + call: '', + scriptShell: 'sh', + yes: true, // skip interactive prompt for npxCache install + } + + // Workspace A + await libnpmexec({ + ...baseOpts, + pkgPath: resolve(path, 'packages/a'), + args: ['shared-bin'], + }) + + const outputA = await readOutput('shared-bin') + t.equal(outputA.value, 'A', 'should run workspace A bin') + await rmOutput('shared-bin') + + // Workspace B + await libnpmexec({ + ...baseOpts, + pkgPath: resolve(path, 'packages/b'), + args: ['shared-bin'], + }) + + const outputB = await readOutput('shared-bin') + t.equal(outputB.value, 'B', 'should run workspace B bin, not cached workspace A bin') +}) From 94a0d84dc5289bd75a2f95286e3984348448769a Mon Sep 17 00:00:00 2001 From: arjun-vegeta <149392539+arjun-vegeta@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:05:46 +0530 Subject: [PATCH 2/3] fix(libnpmexec): correctly resolve binaries from workspace dependencies Fixes #9640 by preventing fallback to hoisted .bin directories for workspace packages. --- workspaces/libnpmexec/lib/index.js | 74 ++++++++++-- workspaces/libnpmexec/test/index.js | 174 ++++++++++++++++++++++++++++ workspaces/libnpmexec/test/local.js | 94 +++++++++++++-- 3 files changed, 323 insertions(+), 19 deletions(-) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 4d7c126654689..0528682fa59ee 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -137,6 +137,13 @@ const exec = async (opts) => { return run() } + // Detect workspace context before appending the root path. + // In workspace mode, the CLI explicitly sets pkgPath to the workspace dir. + // When pkgPath is not provided (undefined), we're not in workspace context. + const explicitPkgPath = opts.pkgPath + const inWorkspaceContext = explicitPkgPath != null + && resolve(String(explicitPkgPath)) !== resolve(path) + // Look in the local tree too pkgPaths.push(path) @@ -146,6 +153,7 @@ const exec = async (opts) => { // - in any local packages (pkgPaths can have workspaces in them or just the root) // - in the local tree (path) // - globally + let localTree if (needPackageCommandSwap) { // Local packages and local tree for (const p of pkgPaths) { @@ -163,18 +171,58 @@ const exec = async (opts) => { } } if (needPackageCommandSwap) { - // no bin entry in local packages or in tree, now we look for binPaths - const dir = dirname(dirname(localBin)) - const localBinPath = await localFileExists(dir, args[0], '/') - if (localBinPath) { - binPaths.push(localBinPath) - return await run() - } else if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) { - binPaths.push(globalBin) + const localArb = new Arborist({ ...flatOptions, path }) + localTree = await localArb.loadActual() + + let targetNode = null + for (const p of pkgPaths) { + let wsNode = localTree + for (const node of localTree.inventory.values()) { + if (node.path === p) { + wsNode = node; break + } + } + + // If we didn't find the workspace node in the inventory, and it's not the root path, skip it. + if (wsNode === localTree && p !== path) { + continue + } + + for (const edge of wsNode.edgesOut.values()) { + if (edge.to?.package?.bin?.[args[0]]) { + targetNode = edge.to + break + } + } + if (targetNode) { + break + } + } + + if (targetNode) { + const binFile = targetNode.package.bin[args[0]] + const execNode = isWindows ? `"${process.execPath}"` : process.execPath + args.splice(0, 1, execNode, resolve(targetNode.path, binFile)) return await run() + } else { + // In workspace context, the dep-graph lookup is authoritative — + // don't fall back to the hoisted .bin which may contain bins from + // other workspaces' dependencies that this workspace doesn't declare. + if (!inWorkspaceContext) { + const dir = dirname(dirname(localBin)) + const localBinPath = await localFileExists(dir, args[0], '/') + if (localBinPath) { + binPaths.push(localBinPath) + return await run() + } + } + if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) { + binPaths.push(globalBin) + return await run() + } + // We swap out args[0] with the bin from the manifest later + packages.push(args[0]) } - // We swap out args[0] with the bin from the manifest later - packages.push(args[0]) } } @@ -188,8 +236,10 @@ const exec = async (opts) => { } } - const localArb = new Arborist({ ...flatOptions, path }) - const localTree = await localArb.loadActual() + if (!localTree) { + const localArb = new Arborist({ ...flatOptions, path }) + localTree = await localArb.loadActual() + } // Find anything that isn't installed locally const needInstall = [] diff --git a/workspaces/libnpmexec/test/index.js b/workspaces/libnpmexec/test/index.js index 008560efc7018..e47f6203d5e94 100644 --- a/workspaces/libnpmexec/test/index.js +++ b/workspaces/libnpmexec/test/index.js @@ -14,3 +14,177 @@ t.test('no args', async t => { await exec() }) + +t.test('resolves binary from workspace dependencies', async t => { + t.plan(2) + + const { exec, path } = setup(t, { + testdir: { + 'tool-v1': { + 'package.json': { name: 'tool-v1', version: '1.0.0', bin: { 'shared-bin': 'cli.js' } }, + 'cli.js': { key: 'tool-v1', value: 'hello' }, + }, + packages: { + a: {}, + }, + }, + mocks: { + '@npmcli/arborist': class MockArborist { + constructor (options) { + this.path = options.path + } + + async loadActual () { + const resolve = require('path').resolve + const wsPath = resolve(this.path, 'packages/a') + const wsNode = { + path: wsPath, + name: 'workspace-a', + edgesOut: new Map([ + ['tool-v1', { + to: { + path: `${this.path}/tool-v1`, + package: { name: 'tool-v1', bin: { 'shared-bin': 'cli.js' } }, + }, + }], + ]), + } + const inventory = new Map([ + [wsPath, wsNode], + ]) + inventory.query = () => [] + return { + inventory, + edgesOut: new Map(), + } + } + + async reify () {} + }, + '../../lib/run-script': ({ args }) => { + t.equal(args[0], process.execPath, 'args[0] should be process.execPath') + t.match(args[1], /tool-v1[/\\]cli\.js$/, 'args[1] should point to correct cli.js') + }, + }, + }) + + const resolve = require('path').resolve + await exec({ pkgPath: resolve(path, 'packages/a'), args: ['shared-bin'] }) +}) + +t.test('resolves binary from workspace dependencies (windows)', async t => { + t.plan(2) + + const { exec, path } = setup(t, { + testdir: { + 'tool-v1': { + 'package.json': { name: 'tool-v1', version: '1.0.0', bin: { 'shared-bin': 'cli.js' } }, + 'cli.js': { key: 'tool-v1', value: 'hello' }, + }, + packages: { + a: {}, + }, + }, + mocks: { + '../../lib/is-windows.js': true, + '@npmcli/arborist': class MockArborist { + constructor (options) { + this.path = options.path + } + + async loadActual () { + const resolve = require('path').resolve + const wsPath = resolve(this.path, 'packages/a') + const wsNode = { + path: wsPath, + name: 'workspace-a', + edgesOut: new Map([ + ['tool-v1', { + to: { + path: `${this.path}/tool-v1`, + package: { name: 'tool-v1', bin: { 'shared-bin': 'cli.js' } }, + }, + }], + ]), + } + const inventory = new Map([ + [wsPath, wsNode], + ]) + inventory.query = () => [] + return { + inventory, + edgesOut: new Map(), + } + } + + async reify () {} + }, + '../../lib/run-script': ({ args }) => { + t.equal(args[0], `"${process.execPath}"`, 'args[0] should be securely quoted on Windows') + t.match(args[1], /tool-v1[/\\]cli\.js$/, 'args[1] should point to correct cli.js') + }, + }, + }) + + const resolve = require('path').resolve + await exec({ pkgPath: resolve(path, 'packages/a'), args: ['shared-bin'] }) +}) + +t.test('does not fallback to hoisted .bin for missing workspace dep', async t => { + t.plan(1) + + const { exec, path } = setup(t, { + testdir: { + packages: { + a: {}, + }, + }, + mocks: { + '@npmcli/arborist': class MockArborist { + constructor (options) { + this.path = options.path + } + + async loadActual () { + const resolve = require('path').resolve + const wsPath = resolve(this.path, 'packages/a') + const wsNode = { + path: wsPath, + name: 'workspace-a', + edgesOut: new Map(), + } + const inventory = new Map([ + [wsPath, wsNode], + ]) + inventory.query = () => [] + return { + inventory, + edgesOut: new Map(), + } + } + + async reify () {} + }, + '../../lib/file-exists.js': { + localFileExists: async () => { + t.fail('localFileExists should not be called in workspace context for undeclared bins') + return null + }, + fileExists: async () => false, + }, + pacote: { + manifest: async () => { + throw new Error('ABORT_TEST') + }, + }, + }, + }) + + const resolve = require('path').resolve + try { + await exec({ pkgPath: resolve(path, 'packages/a'), args: ['missing-bin'] }) + t.fail('should have aborted') + } catch (err) { + t.equal(err.message, 'ABORT_TEST', 'skipped localFileExists and proceeded to missingFromTree') + } +}) diff --git a/workspaces/libnpmexec/test/local.js b/workspaces/libnpmexec/test/local.js index 46700ea535f81..7735613faa6a7 100644 --- a/workspaces/libnpmexec/test/local.js +++ b/workspaces/libnpmexec/test/local.js @@ -470,26 +470,49 @@ for (const allowDirectory of ['none', 'root']) { }) } -t.test('npm exec sequential workspace runs with same-named local bins', async t => { +t.test('npm exec sequential workspace runs with linked same-named dependency bins', async t => { t.plan(2) const { path, readOutput, rmOutput, registry } = setup(t, { testdir: { + 'package.json': { + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + }, packages: { a: { 'package.json': { name: 'workspace-a', version: '1.0.0', - bin: { 'shared-bin': 'cli.js' }, + dependencies: { 'tool-a': '1.0.0' }, + }, + node_modules: { + 'tool-a': { + 'package.json': { + name: 'tool-a', + version: '1.0.0', + bin: { 'shared-bin': 'cli.js' }, + }, + 'cli.js': { key: 'shared-bin', value: 'A' }, + }, }, - 'cli.js': { key: 'shared-bin', value: 'A' }, }, b: { 'package.json': { name: 'workspace-b', version: '1.0.0', - bin: { 'shared-bin': 'cli.js' }, + dependencies: { 'tool-b': '1.0.0' }, + }, + node_modules: { + 'tool-b': { + 'package.json': { + name: 'tool-b', + version: '1.0.0', + bin: { 'shared-bin': 'cli.js' }, + }, + 'cli.js': { key: 'shared-bin', value: 'B' }, + }, }, - 'cli.js': { key: 'shared-bin', value: 'B' }, }, }, }, @@ -519,7 +542,7 @@ t.test('npm exec sequential workspace runs with same-named local bins', async t }) const outputA = await readOutput('shared-bin') - t.equal(outputA.value, 'A', 'should run workspace A bin') + t.equal(outputA.value, 'A', 'should run workspace A bin via dependency tool-a') await rmOutput('shared-bin') // Workspace B @@ -530,5 +553,62 @@ t.test('npm exec sequential workspace runs with same-named local bins', async t }) const outputB = await readOutput('shared-bin') - t.equal(outputB.value, 'B', 'should run workspace B bin, not cached workspace A bin') + t.equal(outputB.value, 'B', 'should run workspace B bin via dependency tool-b, bypassing hoist collisions') +}) + +t.test('resolves binary from hoisted workspace dependency', async t => { + t.plan(1) + const { chmod, readOutput, path, registry } = setup(t, { + testdir: { + 'package.json': { + name: 'root', + workspaces: ['packages/*'], + }, + node_modules: { + 'tool-a': { + 'package.json': { + name: 'tool-a', + version: '1.0.0', + bin: { 'shared-bin': 'cli.js' }, + }, + 'cli.js': { key: 'shared-bin', value: 'HOISTED A' }, + }, + }, + packages: { + a: { + 'package.json': { + name: 'workspace-a', + version: '1.0.0', + dependencies: { 'tool-a': '1.0.0' }, + }, + }, + }, + }, + }) + + const libnpmexec = t.mock('../lib/index.js') + + // Intentionally not creating a .bin/shared-bin symlink — this test verifies that the fix + // resolves the binary directly via the dependency graph, bypassing the hoisted .bin directory. + await chmod('node_modules/tool-a/cli.js') + + const baseOpts = { + path, + runPath: path, + npxCache: resolve(path, 'npxCache'), + registry: registry.origin + '/', + localBin: resolve(path, 'node_modules', '.bin'), + call: '', + scriptShell: 'sh', + yes: true, + } + + await libnpmexec({ + ...baseOpts, + pkgPath: resolve(path, 'packages/a'), + args: ['shared-bin'], + }) + + const output = await readOutput('shared-bin') + t.equal(output.value, 'HOISTED A', 'should correctly resolve hoisted binary via dependency graph') }) From d1ed92ddfb18de7d1d753a13993572183310ca53 Mon Sep 17 00:00:00 2001 From: arjun-vegeta <149392539+arjun-vegeta@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:11:07 +0530 Subject: [PATCH 3/3] test(libnpmexec): fix branch coverage drop on Windows runners --- workspaces/libnpmexec/test/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/workspaces/libnpmexec/test/index.js b/workspaces/libnpmexec/test/index.js index e47f6203d5e94..2c61345f9e2ef 100644 --- a/workspaces/libnpmexec/test/index.js +++ b/workspaces/libnpmexec/test/index.js @@ -29,6 +29,7 @@ t.test('resolves binary from workspace dependencies', async t => { }, }, mocks: { + '../../lib/is-windows.js': false, '@npmcli/arborist': class MockArborist { constructor (options) { this.path = options.path