From e2f85bcf21de35ee1aecd138c3aebaaef2cbf419 Mon Sep 17 00:00:00 2001 From: shauryagangrade <288927048+shauryagangrade@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:37:01 +0530 Subject: [PATCH] fix: bridge colon-tainted bin paths so hoisted bins resolve in lifecycle scripts When any ancestor directory of a project contains the POSIX PATH delimiter (':' on unix), set-path would emit a node_modules/.bin entry with an embedded ':' that the shell splits into two entries, so hoisted binaries like tsc could not be found (exit 127). Since a ':' cannot be escaped in a PATH entry, present a colon-free symlink mirror instead: for each colon-containing node_modules dir, create a node_modules symlink under a tmp root and put the mirrored .bin path on PATH. Symlinking node_modules (not .bin) preserves the relative /../ resolution used by bin shims. POSIX-only (Windows PATH delimiter is ';' and drive letters contain ':'). Degrades to the existing warning when the tmpdir itself contains the delimiter. No behavior change for colon-free projects. Fixes npm/cli#9910 --- lib/bin-bridge.js | 103 +++++++++++++++++++++++++++ lib/set-path.js | 14 ++-- test/bin-bridge.js | 170 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 283 insertions(+), 4 deletions(-) create mode 100644 lib/bin-bridge.js create mode 100644 test/bin-bridge.js diff --git a/lib/bin-bridge.js b/lib/bin-bridge.js new file mode 100644 index 0000000..e2d3f06 --- /dev/null +++ b/lib/bin-bridge.js @@ -0,0 +1,103 @@ +'use strict' + +const { delimiter, sep, resolve, relative } = require('path') +const { createHash } = require('crypto') +const { mkdirSync, existsSync, realpathSync, symlinkSync } = require('fs') +const os = require('os') + +const POSIX_DELIMITER = ':' + +// Find the node_modules directory enclosing a bin path. +// e.g. `/a/b/node_modules/.bin/tsc` -> `/a/b/node_modules` +const modulesDirOf = (p) => { + const parts = p.split(sep) + const idx = parts.lastIndexOf('node_modules') + /* istanbul ignore next -- defensive */ + if (idx === -1) { + return null + } + /* istanbul ignore next -- defensive: only when path is exactly the fs root */ + return parts.slice(0, idx + 1).join(sep) || sep +} + +// Build a bridge that maps tainted (PATH-delimiter-containing) node_modules +// bin paths to delimiter-free symlink mirrors so POSIX shells can resolve +// them via PATH. Returns a function (p) => mapped path. When bridging is not +// possible or not needed, the function returns its input unchanged. +// +// Only POSIX is bridged: Windows PATH delimiter is ';', and every absolute +// path there contains a drive-letter ':' which would false-positive if we +// treated ':' as the taint marker. +const createBinBridge = (projectPath, opts = {}) => { + const { + tmpdir = os.tmpdir(), + // Injectable for tests only; defaults to the platform delimiter so the + // win guess at which char breaks PATH on POSIX. + delimiterForTaint = delimiter, + } = opts + + // `delimiterForTaint` is the char that makes a PATH entry ambiguous. On + // POSIX that is ':'. On Windows the delimiter is ';' and ':' appears in + // drive letters, so only bridge when the two differ meaningfully. We + // require POSIX semantics: '/' separator AND ':' marker. + const isPosix = sep === '/' && delimiterForTaint === POSIX_DELIMITER + if (!isPosix || tmpdir.includes(delimiterForTaint)) { + return (p) => p + } + + // Resolve the project path to a canonical real path when possible so the + // bridge root is stable even when cwd is accessed via a symlink. + let resolved + try { + resolved = realpathSync(projectPath) + } catch { + resolved = projectPath + } + + const projectHash = createHash('sha256').update(resolved).digest('hex').slice(0, 16) + /* istanbul ignore next -- defensive: every supported Node platform has getuid */ + const uid = typeof process.getuid === 'function' ? process.getuid() : 'user' + const root = resolve(tmpdir, `npm-run-script-bridge-${uid}-${projectHash}`) + + // Lazily created on first bridged path; containers map is reused per call. + let rootCreated = false + const containers = new Map() + + const bridgeBinPath = (binPath) => { + const nm = modulesDirOf(binPath) + if (!nm || !nm.includes(delimiterForTaint)) { + return binPath + } + + // Lazily create the bridge root on first use. + if (!rootCreated) { + mkdirSync(root, { recursive: true, mode: 0o700 }) + rootCreated = true + } + + let container = containers.get(nm) + if (!container) { + const nmHash = createHash('sha256').update(nm).digest('hex').slice(0, 16) + container = resolve(root, nmHash) + const bridgeNm = resolve(container, 'node_modules') + if (!existsSync(bridgeNm)) { + mkdirSync(container, { recursive: true, mode: 0o700 }) + /* istanbul ignore next -- defensive: only hit in a genuine parallel-process race */ + if (!existsSync(bridgeNm)) { + symlinkSync(nm, bridgeNm) + } + } + + containers.set(nm, container) + } + + // Reconstruct the relative portion (e.g. `.bin` or `.bin/tsc`) so the + // bridge path preserves the same depth relationship inside the mirror. + const rel = relative(nm, binPath) + return resolve(container, 'node_modules', rel) + } + + return bridgeBinPath +} + +module.exports = createBinBridge diff --git a/lib/set-path.js b/lib/set-path.js index 0344cc0..e854e20 100644 --- a/lib/set-path.js +++ b/lib/set-path.js @@ -1,5 +1,6 @@ const { log } = require('proc-log') const { resolve, dirname, delimiter } = require('path') +const createBinBridge = require('./bin-bridge.js') // the path here is relative, even though it does not need to be // in order to make the posix tests pass in windows const nodeGypPath = resolve(__dirname, '../lib/node-gyp-bin') @@ -7,30 +8,35 @@ const nodeGypPath = resolve(__dirname, '../lib/node-gyp-bin') // Windows typically calls its PATH environ 'Path', but this is not // guaranteed, nor is it guaranteed to be the only one. Merge them // all together in the order they appear in the object. -const setPATH = (projectPath, binPaths, env) => { +const setPATH = (projectPath, binPaths, env, opts) => { const PATH = Object.keys(env).filter(p => /^path$/i.test(p) && env[p]) .map(p => env[p].split(delimiter)) .reduce((set, p) => set.concat(p.filter(concatted => !set.includes(concatted))), []) .join(delimiter) + const bridgeBinPath = createBinBridge(projectPath, opts) const pathArr = [] + if (binPaths) { for (const bin of binPaths) { - if (bin.includes(delimiter)) { + const bridged = bridgeBinPath(bin) + if (bridged === bin && bin.includes(delimiter)) { const event = env.npm_lifecycle_event const context = event ? `"${event}" script` : 'script execution' log.warn('run-script', `Path contains delimiter ("${delimiter}"), ${context} may not behave as expected.`) } + + pathArr.push(bridged) } - pathArr.push(...binPaths) } + // unshift the ./node_modules/.bin from every folder // walk up until dirname() does nothing, at the root // XXX we should specify a cwd that we don't go above let p = projectPath let pp do { - pathArr.push(resolve(p, 'node_modules', '.bin')) + pathArr.push(bridgeBinPath(resolve(p, 'node_modules', '.bin'))) pp = p p = dirname(p) } while (p !== pp) diff --git a/test/bin-bridge.js b/test/bin-bridge.js new file mode 100644 index 0000000..51ff5fd --- /dev/null +++ b/test/bin-bridge.js @@ -0,0 +1,170 @@ +const t = require('tap') +const { execFileSync } = require('child_process') +const { mkdirSync, writeFileSync, chmodSync, lstatSync } = require('fs') +const { delimiter, resolve, sep } = require('path') +const os = require('os') +const createBinBridge = require('../lib/bin-bridge.js') + +t.test('bin-bridge', async t => { + const isPosix = delimiter === ':' + + await t.test('returns identity when tmpdir contains the delimiter', async t => { + const badTmp = `${os.tmpdir()}${sep}with${delimiter}${delimiter}colon` + const bridge = createBinBridge('/some/clean/project', { tmpdir: badTmp }) + const tainted = '/a/b/node_modules/.bin/tsc' + t.equal(bridge(tainted), tainted, 'tainted path returned unchanged when tmpdir is tainted') + }) + + await t.test('returns identity on a non-POSIX delimiter (simulated Windows)', async t => { + // Force the marker to ';' to exercise the non-POSIX guard. + const bridge = createBinBridge('/c:/some/project', { delimiterForTaint: ';' }) + const tainted = '/c:/project/node_modules/.bin/tsc' + t.equal(bridge(tainted), tainted, 'non-posix path returned unchanged') + }) + + await t.test('returns identity when path has no delimiter', async t => { + const bridge = createBinBridge('/some/clean/project') + const clean = '/a/b/node_modules/.bin/tsc' + t.equal(bridge(clean), clean, 'clean path returned unchanged') + }) + + await t.test('returns identity for non-node_modules paths', async t => { + const bridge = createBinBridge('/some/clean/project') + const random = '/a/b/bin/tsc' + t.equal(bridge(random), random, 'non-node_modules path returned unchanged') + }) + + if (isPosix) { + await t.test('bridges node_modules bin paths containing colon', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-colon-' + process.pid, 'parent:directory', 'mono-repo') + const nmDir = resolve(projectDir, 'node_modules') + const binDir = resolve(nmDir, '.bin') + mkdirSync(binDir, { recursive: true }) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const tainted = resolve(nmDir, '.bin', 'tsc') + const bridged = bridge(tainted) + + t.not(bridged, tainted, 'bridged path differs from input') + t.ok(!bridged.includes(':'), 'bridged path is colon-free') + + // Verify the bridge symlink points to the real node_modules. + const parts = bridged.split(sep) + const nmIdx = parts.lastIndexOf('node_modules') + const bridgeNm = parts.slice(0, nmIdx + 1).join(sep) + t.ok(lstatSync(bridgeNm).isSymbolicLink(), 'bridge node_modules is a symlink') + }) + + await t.test('bridges a path that is exactly the node_modules dir', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-exact-' + process.pid, 'parent:dir', 'mono') + const nmDir = resolve(projectDir, 'node_modules') + mkdirSync(nmDir, { recursive: true }) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const bridged = bridge(resolve(nmDir)) + t.ok(!bridged.includes(':'), 'bridged node_modules path is colon-free') + t.not(bridged, nmDir, 'bridged path differs from input') + }) + + await t.test('reuses existing bridge node_modules without re-symlinking', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-reuse-' + process.pid, 'parent:dir', 'mono') + const nmDir = resolve(projectDir, 'node_modules') + mkdirSync(resolve(nmDir, '.bin'), { recursive: true }) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + bridge(resolve(nmDir, '.bin', 'a')) + + // Second bridge invocation over the same nm (maps to the same container). + const bridge2 = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const out = bridge2(resolve(nmDir, '.bin', 'b')) + t.ok(!out.includes(':'), 'second bridge returns colon-free path') + }) + + await t.test('bridge preserves relative resolution for .bin bins', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-resolve-' + process.pid, 'with:colon', 'mono-repo') + const nmDir = resolve(projectDir, 'node_modules') + const binDir = resolve(nmDir, '.bin') + mkdirSync(binDir, { recursive: true }) + + const binPath = resolve(binDir, 'tsc') + writeFileSync(binPath, '#!/bin/sh\necho bridge-ok') + chmodSync(binPath, 0o755) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const bridged = bridge(binPath) + + const out = execFileSync(bridged, { encoding: 'utf8' }).trim() + t.equal(out, 'bridge-ok', 'bin executable through bridge path') + }) + + await t.test('deterministic: same nm always maps to same container', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-dup-' + process.pid, 'dup:test', 'mono') + const nmDir = resolve(projectDir, 'node_modules') + mkdirSync(resolve(nmDir, '.bin'), { recursive: true }) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const a1 = bridge(resolve(nmDir, '.bin', 'x')) + const a2 = bridge(resolve(nmDir, '.bin', 'y')) + const a3 = bridge(resolve(nmDir, '.bin', 'x')) + const containerOf = (p) => p.slice(0, p.lastIndexOf('/')) + t.equal(containerOf(a1), containerOf(a2), 'same node_modules maps to same container') + t.equal(a1, a3, 'same input returns same output') + }) + + await t.test('integration: hoisted bin found via bridge in colon dir', async t => { + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-integration-' + process.pid, 'parent:dir', 'mono-repo') + const nmDir = resolve(projectDir, 'node_modules') + const binDir = resolve(nmDir, '.bin') + const pkgDir = resolve(nmDir, 'typescript', 'bin') + mkdirSync(pkgDir, { recursive: true }) + mkdirSync(binDir, { recursive: true }) + + writeFileSync(resolve(pkgDir, 'tsc.js'), '#!/usr/bin/env node\nconsole.log("bridge-works")') + chmodSync(resolve(pkgDir, 'tsc.js'), 0o755) + writeFileSync(resolve(binDir, 'tsc'), '#!/bin/sh\nexec node "$(dirname "$0")/../typescript/bin/tsc.js"') + chmodSync(resolve(binDir, 'tsc'), 0o755) + + const bridge = createBinBridge(projectDir, { tmpdir: bridgeRoot }) + const bridgedTsc = bridge(resolve(binDir, 'tsc')) + + const out = execFileSync(bridgedTsc, { encoding: 'utf8' }).trim() + t.equal(out, 'bridge-works', 'script found and executed via bridge') + }) + + await t.test('setPATH applies bridge for colon-tainted walk-up paths', async t => { + const setPATH = require('../lib/set-path.js') + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-setpath-' + process.pid, 'parent:dir', 'mono-repo') + const nmDir = resolve(projectDir, 'node_modules') + mkdirSync(resolve(nmDir, '.bin'), { recursive: true }) + + const env = setPATH(projectDir, [], { PATH: '/usr/bin:/bin' }, { tmpdir: bridgeRoot }) + const pathEntries = env.PATH.split(delimiter) + const bridgeEntry = pathEntries.find(e => e.includes('npm-run-script-bridge')) + t.ok(bridgeEntry, 'PATH contains a bridge entry for the colon-tainted node_modules') + t.ok(!bridgeEntry.includes(':'), 'bridge entry is colon-free') + const rawEntry = pathEntries.find(e => e.includes('parent:dir')) + t.notOk(rawEntry, 'PATH does not contain raw colon-tainted path') + }) + + await t.test('setPATH bridges tainted binPaths passed by caller', async t => { + const setPATH = require('../lib/set-path.js') + const bridgeRoot = t.testdir({}) + const projectDir = resolve(os.tmpdir(), 'npm-bridge-test-binpaths-' + process.pid, 'parent:dir') + const binPaths = [`${projectDir}/node_modules/.bin`] + + const env = setPATH(projectDir, binPaths, { PATH: '/usr/bin:/bin' }, { tmpdir: bridgeRoot }) + const pathEntries = env.PATH.split(delimiter) + const bridgeEntry = pathEntries.find(e => e.includes('npm-run-script-bridge') && e.endsWith('.bin')) + t.ok(bridgeEntry, 'PATH contains bridge entry for tainted binPaths') + const rawEntry = pathEntries.find(e => e.includes('parent:dir') && !e.includes('npm-run-script-bridge')) + t.notOk(rawEntry, 'PATH does not contain raw tainted binPath') + }) + } +})