diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index 220e6c294..bbeaba6d2 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -7,7 +7,8 @@ * the arguments, so shell injection is impossible by construction. * Windows `.cmd`/`.bat` shims (npm, yarn, …) are handled by cross-spawn, * which spawns `cmd.exe /d /s /c` with correct quoting — modern Node does - * not spawn batch shims directly (CVE-2024-27980). + * not spawn batch shims directly (CVE-2024-27980). The executable name is + * resolved against PATH. * * 2. `runUserCommandLine(line)` — an opaque command line **the user themselves * authored** (e.g. the `app` command from `cdk.json`, the `--browser` flag), @@ -26,6 +27,8 @@ */ // eslint-disable-next-line no-restricted-imports -- this module IS the sanctioned wrapper around child_process import * as child_process from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; import { StringDecoder } from 'string_decoder'; import spawn from 'cross-spawn'; @@ -221,7 +224,11 @@ function errorMessage(cause: unknown): string { */ export async function run(argv: readonly string[], options: RunOptions = {}): Promise { assertNonEmptyArgv(argv, 'run'); - const child = spawn(argv[0], argv.slice(1), spawnOptions(options)); + const command = resolveExecutable(argv[0], { env: options.env }); + if (command === undefined) { + return Promise.reject(notFoundError(argv)); + } + const child = spawn(command, argv.slice(1), spawnOptions(options)); return monitor(child, renderForDisplay(argv), options); } @@ -249,7 +256,11 @@ export interface RunSyncOptions { */ export function runSync(argv: readonly string[], options: RunSyncOptions = {}): string { assertNonEmptyArgv(argv, 'runSync'); - const result = spawn.sync(argv[0], argv.slice(1), { + const command = resolveExecutable(argv[0], {}); + if (command === undefined) { + throw notFoundError(argv); + } + const result = spawn.sync(command, argv.slice(1), { cwd: options.cwd, timeout: options.timeoutMs, killSignal: 'SIGTERM', @@ -294,6 +305,151 @@ export async function runUserCommandLine(commandLine: string, options: RunOption return monitor(child, commandLine, options); } +/** + * Resolve an executable name to an absolute path on PATH, never the cwd. + * + * On Windows a bare name spawned without a shell resolves from the cwd before + * PATH (cross-spawn delegates to `which`, which searches the cwd first), so a + * file planted in a handed-over cwd can shadow the real binary. Returning an + * absolute PATH hit — and refusing a name not on PATH (`undefined`) — closes + * that; the path is absolute so cross-spawn does not re-resolve it. For the same + * reason relative PATH entries (e.g. `.`) are skipped; quoted entries are + * unwrapped and a name with a dot is matched exactly, both as `which` does. + * + * On POSIX the name is returned unchanged (execvp searches PATH only). A name + * that already contains a path separator is honored verbatim everywhere. + * + * @returns an absolute path on Windows, the name unchanged elsewhere, or + * `undefined` when a bare Windows name is not on PATH. + */ +export function resolveExecutable( + command: string, + options: { readonly env?: Record; readonly platform?: NodeJS.Platform } = {}, +): string | undefined { + const platform = options.platform ?? process.platform; + + // An explicit path (absolute, or containing a separator / drive) is used + // verbatim; there is no PATH search to harden. `\\` is checked directly + // because path.isAbsolute uses the *running* platform's rules. + if (path.isAbsolute(command) || command.includes('/') || command.includes('\\')) { + return command; + } + + // POSIX execvp searches PATH only; nothing to harden. + if (platform !== 'win32') { + return command; + } + + // Resolve PATH/PATHEXT from the caller's env, falling back to process.env when + // that env lacks the key — mirroring cross-spawn/which, which resolve against + // process.env.PATH when the spawn env has no PATH. Without this fallback a + // caller passing a PATH-less custom env would wrongly get ENOENT on Windows. + const search = options.env ?? process.env; + const pathVar = envValue(search, 'PATH') ?? envValue(process.env, 'PATH'); + const pathExt = envValue(search, 'PATHEXT') ?? envValue(process.env, 'PATHEXT'); + const exts = windowsExtensions(command, pathExt); + const dirs = (pathVar ?? '') + .split(path.delimiter) + .filter(Boolean) + // Windows PATH entries may be wrapped in double quotes; unwrap them (as + // `which` does) so a quoted directory still matches on disk. + .map(stripSurroundingQuotes) + // Only absolute entries: a relative one (e.g. `.`) would be joined into a + // non-absolute candidate that resolves against the cwd — the thing we + // refuse to consult. + .filter(isAbsolutePathEntry); + + for (const dir of dirs) { + for (const ext of exts) { + const candidate = path.join(dir, command + ext); + if (isFile(candidate)) { + // Absolute (dir is absolute), so cross-spawn will not re-search. + return candidate; + } + } + } + // Not on PATH. Deliberately do NOT fall back to the bare name: that would let + // Windows resolve it from the cwd, which is exactly the risk we are closing. + return undefined; +} + +/** Strip a single pair of wrapping double quotes from a PATH entry, if present. */ +function stripSurroundingQuotes(dir: string): string { + return /^".*"$/.test(dir) ? dir.slice(1, -1) : dir; +} + +/** + * Whether a PATH entry is absolute. Recognizes both POSIX-absolute and + * Windows-absolute forms directly, rather than relying on `path.isAbsolute` + * (which uses the *running* platform's rules) — the resolver must behave the + * same under the `platform: 'win32'` test override on a POSIX host. + */ +function isAbsolutePathEntry(dir: string): boolean { + return path.isAbsolute(dir) // running-platform rule (native runtime + POSIX-absolute test dirs) + || /^[a-zA-Z]:[\\/]/.test(dir) // C:\ or C:/ + || dir.startsWith('\\\\') // UNC \\server\share + || dir.startsWith('\\') // drive-relative-but-rooted \dir + || dir.startsWith('/'); // forward-slash absolute +} + +/** Look up an environment variable case-insensitively (the Windows env is). */ +function envValue(env: Record, name: string): string | undefined { + if (env[name] !== undefined) { + return env[name]; + } + const lower = name.toLowerCase(); + const key = Object.keys(env).find((k) => k.toLowerCase() === lower); + return key !== undefined ? env[key] : undefined; +} + +/** + * The extensions to append when searching for `command` on Windows. + * + * If the name already ends in a known executable extension, search for it + * exactly (empty suffix). Otherwise, if the name contains a dot it may itself + * be a literal file on PATH, so probe the exact name first (as Windows and + * cross-spawn's `which` do) before appending each PATHEXT entry. + */ +function windowsExtensions(command: string, pathext: string | undefined): string[] { + const configured = (pathext ?? '.COM;.EXE;.BAT;.CMD') + .split(';') + .map((e) => e.trim()) + .filter(Boolean); + const lower = command.toLowerCase(); + if (configured.some((e) => lower.endsWith(e.toLowerCase()))) { + return ['']; + } + return command.includes('.') ? ['', ...configured] : configured; +} + +function isFile(candidate: string): boolean { + try { + return fs.statSync(candidate).isFile(); + } catch { + return false; + } +} + +/** + * A `SubprocessError` shaped like a real spawn ENOENT, for the case where a + * bare Windows name could not be resolved on PATH. Keeps `kind: 'spawn-failed'` + * and a `cause` carrying `code: 'ENOENT'` so downstream guidance (e.g. + * cdk-assets' "please install docker") still fires. + */ +function notFoundError(argv: readonly string[]): SubprocessError { + const cause = Object.assign(new Error(`spawn ${argv[0]} ENOENT`), { + code: 'ENOENT', errno: -2, syscall: 'spawn', path: argv[0], + }); + return new SubprocessError({ + command: renderForDisplay(argv), + exitCode: null, + signal: null, + stdout: '', + stderr: '', + cause, + }); +} + /** * Render an argv array as a single string for logs and error messages. * diff --git a/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts b/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts index 1aaf16df8..276dcfc28 100644 --- a/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts +++ b/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts @@ -1,5 +1,8 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; import type { OutputStream } from '../../lib/subprocess'; -import { run, runSync, runUserCommandLine, renderForDisplay, SubprocessError } from '../../lib/subprocess'; +import { run, runSync, runUserCommandLine, renderForDisplay, resolveExecutable, SubprocessError } from '../../lib/subprocess'; // A cross-platform argv that echoes its arguments exactly as received, // proving no shell interpreted them. `node -e` exists everywhere the @@ -113,6 +116,32 @@ describe('run', () => { expect(result.stdout.trim()).toMatch(/^\d+\.\d+\.\d+/); }, 30000); + // Windows-only: a plain .exe (as in every nodeEval test above) never routes + // through cmd.exe, so this is the one path where cross-spawn's escaping is + // actually exercised. NOTE: CI has no Windows unit-test lane today (unit + // tests run only on the Ubuntu `build` job; the Windows CI jobs run the + // black-box integ suites), so this currently executes only when the suite is + // run on a Windows dev machine. It is kept as an executable spec until a + // Windows unit-test lane exists. + (process.platform === 'win32' ? test : test.skip)( + 'a .cmd shim receives hostile arguments verbatim (cross-spawn escaping)', + async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cmd-shim')); + const shim = path.join(dir, 'echo-args.cmd'); + // The shim forwards its args to node, which echoes them back as JSON. + fs.writeFileSync(shim, '@node -e "process.stdout.write(JSON.stringify(process.argv.slice(1)))" %*\r\n'); + try { + // Every one of these would do something (or break) if cmd.exe parsed it. + const hostile = ['a&echo PWNED', 'b|whoami', 'c>out', 'd"q', '%PATH%', 'e^f', '(g)', 'two spaces']; + const result = await run([shim, ...hostile]); + expect(JSON.parse(result.stdout)).toEqual(hostile); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, + 30000, + ); + test('multi-byte UTF-8 characters split across chunks decode correctly', async () => { // 'é' is 2 bytes in UTF-8; the child writes them in separate chunks with a // delay so they arrive as separate 'data' events. @@ -251,3 +280,144 @@ describe('renderForDisplay', () => { expect(renderForDisplay(['plain'])).toEqual('plain'); }); }); + +describe('resolveExecutable', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'resolve-exe')); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test('POSIX leaves a bare name unchanged (execvp already searches PATH only)', () => { + expect(resolveExecutable('docker', { platform: 'linux' })).toEqual('docker'); + }); + + test('an explicit path is honored verbatim on every platform', () => { + expect(resolveExecutable('/usr/bin/docker', { platform: 'win32' })).toEqual('/usr/bin/docker'); + expect(resolveExecutable('C:\\tools\\docker.exe', { platform: 'win32' })).toEqual('C:\\tools\\docker.exe'); + expect(resolveExecutable('./local-tool', { platform: 'linux' })).toEqual('./local-tool'); + }); + + test('Windows resolves a bare name to its absolute location on PATH', () => { + const target = path.join(dir, 'docker.CMD'); + fs.writeFileSync(target, ''); + + expect(resolveExecutable('docker', { platform: 'win32', env: { PATH: dir, PATHEXT: '.CMD' } })) + .toEqual(target); + }); + + test('Windows searches for an already-suffixed name exactly (no double extension)', () => { + // Casing kept consistent so the assertion is meaningful on a case-sensitive + // filesystem; on Windows the FS match is itself case-insensitive. + fs.writeFileSync(path.join(dir, 'tool.exe'), ''); + + expect(resolveExecutable('tool.exe', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE' } })) + .toEqual(path.join(dir, 'tool.exe')); + }); + + test('Windows refuses a name that is not on PATH — never falls back to the cwd', () => { + // The binary exists on disk, but in a directory that is NOT on PATH. + // Resolution must fail rather than let Windows satisfy the bare name from + // the working directory (the shadowing risk this closes). + fs.writeFileSync(path.join(dir, 'docker.CMD'), ''); + + expect(resolveExecutable('docker', { platform: 'win32', env: { PATH: '', PATHEXT: '.CMD' } })) + .toBeUndefined(); + }); + + test('Windows PATH lookup is case-insensitive in the env var name (Path vs PATH)', () => { + const target = path.join(dir, 'git.EXE'); + fs.writeFileSync(target, ''); + + expect(resolveExecutable('git', { platform: 'win32', env: { Path: dir, PATHEXT: '.EXE' } })) + .toEqual(target); + }); + + test('Windows skips a relative PATH entry — the cwd is never consulted', () => { + // A relative entry (classically `.`) would join into a non-absolute + // candidate that cross-spawn re-resolves against the child cwd, reopening + // the shadowing hole. It must be ignored; the absolute entry wins, and the + // result is always absolute. + const target = path.join(dir, 'docker.CMD'); + fs.writeFileSync(target, ''); + + const resolved = resolveExecutable('docker', { + platform: 'win32', + env: { PATH: `.${path.delimiter}${dir}`, PATHEXT: '.CMD' }, + }); + + expect(resolved).toEqual(target); + expect(path.isAbsolute(resolved!)).toBe(true); + }); + + test('Windows resolves nothing when PATH holds only relative entries', () => { + // Even though a matching file could exist relative to the cwd, a PATH of + // only relative entries must never satisfy the name from the cwd. + expect(resolveExecutable('docker', { platform: 'win32', env: { PATH: `.${path.delimiter}tools`, PATHEXT: '.CMD' } })) + .toBeUndefined(); + }); + + test('Windows unwraps double-quoted PATH entries (as which does)', () => { + // Windows PATH entries may be wrapped in quotes (e.g. paths with spaces). + const target = path.join(dir, 'docker.CMD'); + fs.writeFileSync(target, ''); + + expect(resolveExecutable('docker', { platform: 'win32', env: { PATH: `"${dir}"`, PATHEXT: '.CMD' } })) + .toEqual(target); + }); + + test('Windows probes a suffixed name exactly even when its extension is not in PATHEXT', () => { + // `tool.exe` under `PATHEXT=.CMD` must still be found as `tool.exe`, not + // only as `tool.exe.CMD`. + fs.writeFileSync(path.join(dir, 'tool.exe'), ''); + + expect(resolveExecutable('tool.exe', { platform: 'win32', env: { PATH: dir, PATHEXT: '.CMD' } })) + .toEqual(path.join(dir, 'tool.exe')); + }); + + test('Windows probes a dotted name exactly (no known extension)', () => { + // A name containing a dot may be a literal file on PATH; it is tried before + // any PATHEXT extension is appended. + fs.writeFileSync(path.join(dir, 'my.tool'), ''); + + expect(resolveExecutable('my.tool', { platform: 'win32', env: { PATH: dir, PATHEXT: '.CMD' } })) + .toEqual(path.join(dir, 'my.tool')); + }); + + test('Windows falls back to process.env.PATH when the caller env has no PATH', () => { + // cross-spawn/which resolve against process.env.PATH when the spawn env has + // no PATH key, so a caller passing a PATH-less custom env must still resolve + // the tool (not get a synthetic ENOENT). + const target = path.join(dir, 'docker.CMD'); + fs.writeFileSync(target, ''); + const savedPath = process.env.PATH; + const savedPathExt = process.env.PATHEXT; + process.env.PATH = dir; + process.env.PATHEXT = '.CMD'; + try { + expect(resolveExecutable('docker', { platform: 'win32', env: { FOO: 'bar' } })) + .toEqual(target); + } finally { + process.env.PATH = savedPath; + process.env.PATHEXT = savedPathExt; + } + }); + + test('Windows treats an explicitly empty PATH as no directories (no fallback)', () => { + // An explicit empty PATH means "no search dirs" and must not fall back to + // process.env.PATH — distinct from an absent PATH key. + const savedPath = process.env.PATH; + process.env.PATH = dir; + fs.writeFileSync(path.join(dir, 'docker.CMD'), ''); + try { + expect(resolveExecutable('docker', { platform: 'win32', env: { PATH: '', PATHEXT: '.CMD' } })) + .toBeUndefined(); + } finally { + process.env.PATH = savedPath; + } + }); +});