fix: bridge colon-tainted bin paths so hoisted bins resolve in lifecycle scripts

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

lib/set-path.js

@@ -1,36 +1,42 @@
 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')
 
 // 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)

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')
+    })
+  }
+})