fix(private-tools): resolve executables against PATH not cwd on Windows

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<RunResult> {
   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<string, string | undefined>; 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<string, string | undefined>, 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.
  *

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