From 9f440cbffacf82c1c3150fae954731ed63d495c9 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:31:55 +0000 Subject: [PATCH 1/7] fix(subprocess): resolve executables against PATH (not cwd) on Windows; reject %VAR% paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Windows-specific hardening fixes to the shared subprocess module, plus a test that actually exercises the .cmd path. 1. resolveExecutable(): on Windows a bare program name spawned without a shell is searched for in the cwd *before* PATH, so a file planted in a handed-over cloud assembly (e.g. docker.bat) could shadow the real binary. run()/runSync() now resolve the executable to an absolute PATH hit up front and refuse a name that is not on PATH rather than let the cwd satisfy it. POSIX is unchanged (execvp already searches PATH only); explicit paths are honored verbatim. 2. quoteShellPart() (toolkit-lib): cmd.exe expands %VAR% even inside double quotes and a `cmd /c` line cannot reliably escape a percent, so a discovered path carrying a %...% reference is now refused loudly instead of being silently rewritten. 3. Adds a Windows-only test that runs a real .cmd shim with hostile arguments — the one path where cross-spawn's cmd.exe escaping is exercised (a plain .exe never is). NOTE: items 1 and 3 change Windows spawn behavior and must be validated by the Windows integ tests; the resolveExecutable logic is unit-tested cross-platform via a `platform` parameter. --- .../private-tools/lib/subprocess/index.ts | 123 +++++++++++++++++- .../test/subprocess/subprocess.test.ts | 84 +++++++++++- .../lib/api/cloud-assembly/environment.ts | 14 ++ .../api/cloud-assembly/environment.test.ts | 16 +++ 4 files changed, 233 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index fe827d0ef..5d156d8f5 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -7,7 +7,9 @@ * 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 (never the working directory) so a binary planted + * in the cwd cannot shadow the real one (see `resolveExecutable`). * * 2. `runUserCommandLine(line)` — an opaque command line **the user themselves * authored** (e.g. the `app` command from `cdk.json`, the `--browser` flag), @@ -17,6 +19,8 @@ * never be assembled from parts by this codebase. */ 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'; @@ -212,7 +216,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); } @@ -240,7 +248,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', @@ -284,6 +296,111 @@ export async function runUserCommandLine(commandLine: string, options: RunOption return monitor(child, commandLine, options); } +/** + * Resolve an executable name to an absolute path against PATH — never the cwd. + * + * On Windows a bare program name spawned without a shell is searched for in the + * current working directory *before* PATH, so a file planted in the working + * directory (e.g. a `docker.bat` inside a handed-over cloud assembly) can run + * instead of the real binary. Resolving to an absolute PATH hit up front closes + * that: the cwd is never consulted, and a name that is not on PATH is refused + * (returns `undefined`) rather than silently satisfied from the cwd. + * + * POSIX `execvp` already searches PATH only (never the cwd), so there the name + * is returned unchanged. An argument that already contains a path separator is + * an explicit location and is honored verbatim on every platform. + * + * @returns the resolved command (absolute on Windows, unchanged elsewhere), or + * `undefined` when a bare Windows name cannot be found 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; + } + + const env = options.env ?? process.env; + const dirs = (envValue(env, 'PATH') ?? '').split(path.delimiter).filter(Boolean); + const exts = windowsExtensions(command, envValue(env, 'PATHEXT')); + + for (const dir of dirs) { + for (const ext of exts) { + const candidate = path.join(dir, command + ext); + if (isFile(candidate)) { + 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; +} + +/** 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 try 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(); + return configured.some((e) => lower.endsWith(e.toLowerCase())) ? [''] : 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..b314bd2cd 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,28 @@ 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. Must run on Windows CI to have any value. + (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 +276,60 @@ 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); + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts index 54e5010b2..b7641d15e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts @@ -1,6 +1,7 @@ import * as path from 'path'; import * as cxapi from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; +import { ToolkitError } from '../../toolkit/toolkit-error'; import type { SdkProvider } from '../aws-auth/private'; import type { Settings } from '../settings'; @@ -260,6 +261,19 @@ function quoteShellPart(part: string) { return part; } if (isWindows) { + // cmd.exe expands `%VAR%` even inside double quotes, and a `cmd /c` command + // line — which is how `runUserCommandLine` reaches the shell on Windows — + // has no reliable way to escape a percent (doubling only works in batch + // files). A discovered path carrying a `%...%` reference would therefore be + // silently rewritten (an env var spliced into the path). Refuse it loudly + // rather than execute something other than what is on disk. + if (/%[^%]*%/.test(part)) { + throw new ToolkitError( + 'UnsafeWindowsPath', + `Cannot safely run a path containing a '%...%' substring through the Windows shell: '${part}'. ` + + 'Rename the file or directory to remove the percent signs.', + ); + } return `"${part}"`; } return `"${part.replace(/([\\"$`])/g, '\\$1')}"`; diff --git a/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts index f007c3725..f82f19fe5 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts @@ -57,6 +57,22 @@ test.each([ expect(actual).toEqual(expected); }); +test('refuses a discovered Windows path containing a %VAR% reference (cmd.exe would expand it)', async () => { + // GIVEN + const appPath = 'C:\\proj\\%USERNAME%\\app'; + Object.defineProperty(process, 'platform', { value: 'win32' }); + jest.spyOn(fs, 'stat').mockImplementation((p) => { + if (p !== appPath) { + throw new Error(`Expected a stat() call on '${appPath}' but got '${p}'`); + } + return Promise.resolve({ mode: 0 }) as any; + }); + + // THEN + await expect(guessExecutable(appPath, (_) => Promise.resolve())) + .rejects.toThrow(/Cannot safely run a path containing a '%\.\.\.%' substring/); +}); + /** * Explode all 'both's in a test array to both false and true */ From 421fa92cbcc03e1be686937894c9ff7392544370 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:24:25 +0000 Subject: [PATCH 2/7] fix(private-tools): only return absolute PATH hits from resolveExecutable Harden the Windows executable resolver against three defects found in review: - Relative PATH entries (e.g. '.') were joined into non-absolute candidates that cross-spawn then re-resolves against the child cwd, reopening the cwd shadowing hole. Skip relative entries so the resolver only ever returns an absolute path and the cwd is never consulted. - Quoted PATH entries ('"C:\Program Files\..."', legal on Windows) were not unwrapped, so an installed tool was reported as not found. Unwrap them, as which does. - A name containing a dot was only probed exactly when it ended in a PATHEXT entry, so 'tool.exe' under a custom PATHEXT or a 'my.tool'-style name got a false ENOENT. Probe the exact name first when the name contains a dot. Also correct the resolver docstring and the module trust-boundary note, and drop the inaccurate 'runs on Windows CI' claim on the .cmd escaping test (CI has no Windows unit-test lane today). --- .../private-tools/lib/subprocess/index.ts | 71 +++++++++++++++---- .../test/subprocess/subprocess.test.ts | 57 ++++++++++++++- 2 files changed, 115 insertions(+), 13 deletions(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index 60d533425..bcf376af6 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -11,12 +11,16 @@ * resolved against PATH (never the working directory) so a binary planted * in the cwd cannot shadow the real one (see `resolveExecutable`). * - * 2. `runUserCommandLine(line)` — an opaque command line **the user themselves - * authored** (e.g. the `app` command from `cdk.json`, the `--browser` flag), - * passed to the platform shell verbatim. The shell is the documented feature - * here and the input is trusted by definition; this function is deliberately - * the only path to a shell and takes no argv form, so command lines can - * never be assembled from parts by this codebase. + * 2. `runUserCommandLine(line)` — an opaque command line passed to the platform + * shell verbatim. It is the only path to a shell. The line is usually one the + * user themselves authored (e.g. the `app` command from `cdk.json`, the + * `--browser` flag) and trusted as such, but callers may also assemble it + * from filesystem-discovered parts (see `toolkit-lib`'s `environment.ts`). + * Every part spliced into a shell line MUST go through that module's + * `quoteShellPart`, which quotes for the platform shell and rejects inputs + * that cannot be quoted safely (e.g. a `%VAR%` on Windows, which cmd.exe + * expands even inside quotes). This is a property of the callers, not + * enforced at this boundary. */ // eslint-disable-next-line no-restricted-imports -- this module IS the sanctioned wrapper around child_process import * as child_process from 'child_process'; @@ -304,9 +308,18 @@ export async function runUserCommandLine(commandLine: string, options: RunOption * On Windows a bare program name spawned without a shell is searched for in the * current working directory *before* PATH, so a file planted in the working * directory (e.g. a `docker.bat` inside a handed-over cloud assembly) can run - * instead of the real binary. Resolving to an absolute PATH hit up front closes - * that: the cwd is never consulted, and a name that is not on PATH is refused - * (returns `undefined`) rather than silently satisfied from the cwd. + * instead of the real binary. Resolving to an *absolute* PATH hit up front + * closes that: the returned path is always absolute (so cross-spawn re-resolves + * it against nothing), the cwd is never consulted, and a name that is not on + * PATH is refused (returns `undefined`) rather than silently satisfied from the + * cwd. + * + * Because the guarantee is "the cwd is never consulted", a *relative* PATH + * entry (classically `.`) is skipped rather than honored: joining `command` + * onto it would produce a non-absolute candidate that cross-spawn would then + * re-resolve against the child's cwd — reopening the exact shadowing hole. + * Quoted PATH entries (`"C:\Program Files\..."`, legal on Windows) are + * unwrapped, matching what `which` does, so an installed tool is still found. * * POSIX `execvp` already searches PATH only (never the cwd), so there the name * is returned unchanged. An argument that already contains a path separator is @@ -334,13 +347,23 @@ export function resolveExecutable( } const env = options.env ?? process.env; - const dirs = (envValue(env, 'PATH') ?? '').split(path.delimiter).filter(Boolean); const exts = windowsExtensions(command, envValue(env, 'PATHEXT')); + const dirs = (envValue(env, 'PATH') ?? '') + .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; } } @@ -350,6 +373,25 @@ export function resolveExecutable( 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) { @@ -364,7 +406,9 @@ function envValue(env: Record, name: string): string * 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 try each PATHEXT entry. + * 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') @@ -372,7 +416,10 @@ function windowsExtensions(command: string, pathext: string | undefined): string .map((e) => e.trim()) .filter(Boolean); const lower = command.toLowerCase(); - return configured.some((e) => lower.endsWith(e.toLowerCase())) ? [''] : configured; + if (configured.some((e) => lower.endsWith(e.toLowerCase()))) { + return ['']; + } + return command.includes('.') ? ['', ...configured] : configured; } function isFile(candidate: string): boolean { 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 b314bd2cd..50fcc8a6d 100644 --- a/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts +++ b/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts @@ -118,7 +118,11 @@ describe('run', () => { // 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. Must run on Windows CI to have any value. + // 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 () => { @@ -332,4 +336,55 @@ describe('resolveExecutable', () => { 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')); + }); }); From 18cbcb52a181ce23f8feb2973d58b3fd8626cf98 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:45:25 +0000 Subject: [PATCH 3/7] chore(toolkit-lib): split %VAR% path rejection out to #1924 The quoteShellPart %VAR% guard is an independent fix on the shell path and now lives in its own PR (#1924). This PR is scoped to the resolveExecutable (PATH-vs-cwd) hardening on the no-shell run() path. --- .../lib/api/cloud-assembly/environment.ts | 14 -------------- .../test/api/cloud-assembly/environment.test.ts | 16 ---------------- 2 files changed, 30 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts index b7641d15e..54e5010b2 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts @@ -1,7 +1,6 @@ import * as path from 'path'; import * as cxapi from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; -import { ToolkitError } from '../../toolkit/toolkit-error'; import type { SdkProvider } from '../aws-auth/private'; import type { Settings } from '../settings'; @@ -261,19 +260,6 @@ function quoteShellPart(part: string) { return part; } if (isWindows) { - // cmd.exe expands `%VAR%` even inside double quotes, and a `cmd /c` command - // line — which is how `runUserCommandLine` reaches the shell on Windows — - // has no reliable way to escape a percent (doubling only works in batch - // files). A discovered path carrying a `%...%` reference would therefore be - // silently rewritten (an env var spliced into the path). Refuse it loudly - // rather than execute something other than what is on disk. - if (/%[^%]*%/.test(part)) { - throw new ToolkitError( - 'UnsafeWindowsPath', - `Cannot safely run a path containing a '%...%' substring through the Windows shell: '${part}'. ` + - 'Rename the file or directory to remove the percent signs.', - ); - } return `"${part}"`; } return `"${part.replace(/([\\"$`])/g, '\\$1')}"`; diff --git a/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts index f82f19fe5..f007c3725 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts @@ -57,22 +57,6 @@ test.each([ expect(actual).toEqual(expected); }); -test('refuses a discovered Windows path containing a %VAR% reference (cmd.exe would expand it)', async () => { - // GIVEN - const appPath = 'C:\\proj\\%USERNAME%\\app'; - Object.defineProperty(process, 'platform', { value: 'win32' }); - jest.spyOn(fs, 'stat').mockImplementation((p) => { - if (p !== appPath) { - throw new Error(`Expected a stat() call on '${appPath}' but got '${p}'`); - } - return Promise.resolve({ mode: 0 }) as any; - }); - - // THEN - await expect(guessExecutable(appPath, (_) => Promise.resolve())) - .rejects.toThrow(/Cannot safely run a path containing a '%\.\.\.%' substring/); -}); - /** * Explode all 'both's in a test array to both false and true */ From 92ffdfc9e99c3d622bb79770354929ef0bcabff9 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:49:36 -0400 Subject: [PATCH 4/7] Refactor comment on executable resolution Simplify comment regarding executable resolution against PATH. --- packages/@aws-cdk/private-tools/lib/subprocess/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index bcf376af6..260b6f525 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -8,8 +8,7 @@ * 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). The executable name is - * resolved against PATH (never the working directory) so a binary planted - * in the cwd cannot shadow the real one (see `resolveExecutable`). + * resolved against PATH. * * 2. `runUserCommandLine(line)` — an opaque command line passed to the platform * shell verbatim. It is the only path to a shell. The line is usually one the From 26b915929261a0d11ed8b614bcf8aab4bbafdd75 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:10:16 +0000 Subject: [PATCH 5/7] docs(private-tools): cite cross-spawn/which cwd-first resolution as the threat basis Substantiate the vulnerability premise in-code: run() resolves through cross-spawn -> node-which, whose getPathInfo searches [process.cwd(), ...PATH] on Windows (cwd first, per which's own source comment). Also note that runSync resolves and spawns against process.env so the two stay in sync. --- packages/@aws-cdk/private-tools/lib/subprocess/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index 1e8d0efbc..2ad028343 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -256,6 +256,8 @@ export interface RunSyncOptions { */ export function runSync(argv: readonly string[], options: RunSyncOptions = {}): string { assertNonEmptyArgv(argv, 'runSync'); + // RunSyncOptions carries no `env`, so both resolution and the spawn below use + // process.env — keep these in sync if an `env` option is ever added here. const command = resolveExecutable(argv[0], {}); if (command === undefined) { throw notFoundError(argv); @@ -311,7 +313,11 @@ export async function runUserCommandLine(commandLine: string, options: RunOption * On Windows a bare program name spawned without a shell is searched for in the * current working directory *before* PATH, so a file planted in the working * directory (e.g. a `docker.bat` inside a handed-over cloud assembly) can run - * instead of the real binary. Resolving to an *absolute* PATH hit up front + * instead of the real binary. This is not raw `child_process` behavior we are + * guessing at: `run()` spawns through cross-spawn, whose resolver is `node-which`, + * and which's `getPathInfo` builds the Windows search path as + * `[process.cwd(), ...PATH]` — cwd first — annotated in its own source with + * "windows always checks the cwd first". Resolving to an *absolute* PATH hit up front * closes that: the returned path is always absolute (so cross-spawn re-resolves * it against nothing), the cwd is never consulted, and a name that is not on * PATH is refused (returns `undefined`) rather than silently satisfied from the From 8d2a601cebebd4eaa8ce5b8301c2d6953cf0d5f2 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:15:37 +0000 Subject: [PATCH 6/7] docs(private-tools): trim resolveExecutable docstring --- .../private-tools/lib/subprocess/index.ts | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index 2ad028343..ad4f046fd 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -256,8 +256,6 @@ export interface RunSyncOptions { */ export function runSync(argv: readonly string[], options: RunSyncOptions = {}): string { assertNonEmptyArgv(argv, 'runSync'); - // RunSyncOptions carries no `env`, so both resolution and the spawn below use - // process.env — keep these in sync if an `env` option is ever added here. const command = resolveExecutable(argv[0], {}); if (command === undefined) { throw notFoundError(argv); @@ -308,34 +306,21 @@ export async function runUserCommandLine(commandLine: string, options: RunOption } /** - * Resolve an executable name to an absolute path against PATH — never the cwd. + * Resolve an executable name to an absolute path on PATH, never the cwd. * - * On Windows a bare program name spawned without a shell is searched for in the - * current working directory *before* PATH, so a file planted in the working - * directory (e.g. a `docker.bat` inside a handed-over cloud assembly) can run - * instead of the real binary. This is not raw `child_process` behavior we are - * guessing at: `run()` spawns through cross-spawn, whose resolver is `node-which`, - * and which's `getPathInfo` builds the Windows search path as - * `[process.cwd(), ...PATH]` — cwd first — annotated in its own source with - * "windows always checks the cwd first". Resolving to an *absolute* PATH hit up front - * closes that: the returned path is always absolute (so cross-spawn re-resolves - * it against nothing), the cwd is never consulted, and a name that is not on - * PATH is refused (returns `undefined`) rather than silently satisfied from 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. * - * Because the guarantee is "the cwd is never consulted", a *relative* PATH - * entry (classically `.`) is skipped rather than honored: joining `command` - * onto it would produce a non-absolute candidate that cross-spawn would then - * re-resolve against the child's cwd — reopening the exact shadowing hole. - * Quoted PATH entries (`"C:\Program Files\..."`, legal on Windows) are - * unwrapped, matching what `which` does, so an installed tool is still found. + * On POSIX the name is returned unchanged (execvp searches PATH only). A name + * that already contains a path separator is honored verbatim everywhere. * - * POSIX `execvp` already searches PATH only (never the cwd), so there the name - * is returned unchanged. An argument that already contains a path separator is - * an explicit location and is honored verbatim on every platform. - * - * @returns the resolved command (absolute on Windows, unchanged elsewhere), or - * `undefined` when a bare Windows name cannot be found on PATH. + * @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, From 8a0a0d792b64310e12713c55cb6d21fea008ab6e Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:09:08 +0000 Subject: [PATCH 7/7] fix(private-tools): fall back to process.env.PATH when caller env lacks PATH resolveExecutable resolved PATH strictly from the caller-provided env, so run(argv, { env: { ...noPath } }) on Windows returned a synthetic ENOENT even when the tool was on the system PATH. cross-spawn/which fall back to process.env.PATH when the spawn env has no PATH key; mirror that. An explicitly empty PATH is still treated as 'no dirs' (uses ?? not ||), so the off-PATH refusal is unchanged. Same fallback applied to PATHEXT. --- .../private-tools/lib/subprocess/index.ts | 12 +++++-- .../test/subprocess/subprocess.test.ts | 33 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts index ad4f046fd..bbeaba6d2 100644 --- a/packages/@aws-cdk/private-tools/lib/subprocess/index.ts +++ b/packages/@aws-cdk/private-tools/lib/subprocess/index.ts @@ -340,9 +340,15 @@ export function resolveExecutable( return command; } - const env = options.env ?? process.env; - const exts = windowsExtensions(command, envValue(env, 'PATHEXT')); - const dirs = (envValue(env, 'PATH') ?? '') + // 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 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 50fcc8a6d..276dcfc28 100644 --- a/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts +++ b/packages/@aws-cdk/private-tools/test/subprocess/subprocess.test.ts @@ -387,4 +387,37 @@ describe('resolveExecutable', () => { 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; + } + }); });