refactor: migrate all subprocess call sites to the shared subprocess module

.projenrc.ts

@@ -775,6 +775,7 @@ const cdkAssetsLib = configureProject(
     ]),
   }),
 );
+cdkAssetsLib.with(tools.subprocess);
 cdkAssetsLib.with(tools.zip);
 cdkAssetsLib.with(tools['s3-path-style']);
 fixupTestTask(cdkAssetsLib);
@@ -942,7 +943,6 @@ const toolkitLib = configureProject(
       'picomatch',
       'p-limit@^3',
       'semver',
-      'split2',
       'wrap-ansi@^7', // Last non-ESM version
       'yaml@^1',
     ],
@@ -955,7 +955,6 @@ const toolkitLib = configureProject(
       '@smithy/util-stream',
       '@types/fs-extra@^11',
       '@types/picomatch',
-      '@types/split2',
       'aws-cdk-lib',
       'aws-sdk-client-mock',
       'aws-sdk-client-mock-jest',
@@ -1001,6 +1000,7 @@ const toolkitLib = configureProject(
   }),
 );
 fixupTestTask(toolkitLib);
+toolkitLib.with(tools.subprocess);
 toolkitLib.with(tools.zip);
 toolkitLib.with(tools['s3-path-style']);
 toolkitLib.tasks.tryFind('test')?.updateStep(0, {
@@ -1370,6 +1370,7 @@ const cli = configureProject(
     releasableCommits: transitiveToolkitPackages('aws-cdk'),
   }),
 );
+cli.with(tools.subprocess);
 cli.with(tools.zip);
 
 new pj.javascript.UpgradeDependencies(cli, {

packages/@aws-cdk/cdk-assets-lib/lib/private/shell.ts

@@ -1,79 +1,71 @@
-import * as child_process from 'child_process';
 import type { SubprocessOutputDestination } from './asset-handler';
+import { run, renderForDisplay, SubprocessError } from './tools';
 
 export type ShellEventType = 'open' | 'data_stdout' | 'data_stderr' | 'close';
 
 export type ShellEventPublisher = (event: ShellEventType, message: string) => void;
 
-export interface ShellOptions extends child_process.SpawnOptions {
+export interface ShellOptions {
   readonly shellEventPublisher: ShellEventPublisher;
+  readonly cwd?: string;
+  readonly env?: Record<string, string | undefined>;
   readonly input?: string;
   readonly subprocessOutputDestination?: SubprocessOutputDestination;
 }
 
 /**
  * OS helpers
  *
- * Shell function which both prints to stdout and collects the output into a
- * string.
+ * Executes the given command as an argv array (never through a shell) and
+ * returns its stdout, routing intermediate output to the configured
+ * destination.
  */
 export async function shell(command: string[], options: ShellOptions): Promise<string> {
-  handleShellOutput(renderCommandLine(command), options, 'open');
-  const child = child_process.spawn(command[0], command.slice(1), {
-    ...options,
-    stdio: [options.input ? 'pipe' : 'ignore', 'pipe', 'pipe'],
-  });
-
-  return new Promise<string>((resolve, reject) => {
-    if (options.input) {
-      child.stdin!.write(options.input);
-      child.stdin!.end();
-    }
-
-    const stdout = new Array<any>();
-    const stderr = new Array<any>();
-
-    // Both emit event and collect output
-    child.stdout!.on('data', (chunk) => {
-      handleShellOutput(chunk, options, 'data_stdout');
-      stdout.push(chunk);
+  const displayCommand = renderForDisplay(command);
+  handleShellOutput(displayCommand, options, 'open');
+
+  try {
+    const result = await run(command, {
+      cwd: options.cwd,
+      env: options.env,
+      input: options.input,
+      onOutput: (stream, data) =>
+        handleShellOutput(data, options, stream === 'stdout' ? 'data_stdout' : 'data_stderr'),
     });
-
-    child.stderr!.on('data', (chunk) => {
-      handleShellOutput(chunk, options, 'data_stderr');
-      stderr.push(chunk);
-    });
-
-    child.once('error', reject);
-
-    child.once('close', (code, signal) => {
-      handleShellOutput(renderCommandLine(command), options, 'close');
-      if (code === 0) {
-        resolve(Buffer.concat(stdout).toString('utf-8'));
-      } else {
-        const out = Buffer.concat(stderr).toString('utf-8').trim();
-        reject(
-          new ProcessFailed(
-            code,
-            signal,
-            `${renderCommandLine(command)} exited with ${code != null ? 'error code' : 'signal'} ${code ?? signal}: ${out}`,
-          ),
-        );
+    handleShellOutput(displayCommand, options, 'close');
+    return result.stdout;
+  } catch (e: any) {
+    if (e instanceof SubprocessError) {
+      // A process that never started has no exit to report; rethrow the OS
+      // error (ENOENT, EACCES, …) as-is — callers key off its `code` (e.g.
+      // docker.ts turns ENOENT into "please install docker" guidance).
+      // No `instanceof Error` on the cause: errno errors come from the host
+      // realm and fail instanceof under test sandboxes.
+      if (e.kind === 'spawn-failed' && e.cause != null) {
+        throw e.cause;
       }
-    });
-  });
+      handleShellOutput(displayCommand, options, 'close');
+      const stderr = e.stderr.trim();
+      throw new ProcessFailed(
+        e.exitCode,
+        e.signal,
+        stderr ? `${e.message}: ${stderr}` : e.message,
+      );
+    }
+    throw e;
+  }
 }
 
 function handleShellOutput(
-  chunk: Buffer | string,
+  chunk: string,
   options: ShellOptions,
   shellEventType: ShellEventType,
 ): void {
   switch (options.subprocessOutputDestination) {
     case 'ignore':
       return;
     case 'publish':
-      options.shellEventPublisher(shellEventType, chunk.toString('utf-8'));
+      options.shellEventPublisher(shellEventType, chunk);
       break;
     case 'stdio':
     default:
@@ -85,7 +77,7 @@ function handleShellOutput(
           process.stderr.write(chunk);
           break;
         case 'open':
-          options.shellEventPublisher(shellEventType, chunk.toString('utf-8'));
+          options.shellEventPublisher(shellEventType, chunk);
           break;
       }
       break;
@@ -104,65 +96,3 @@ class ProcessFailed extends Error {
     super(message);
   }
 }
-
-/**
- * Render the given command line as a string
- *
- * Probably missing some cases but giving it a good effort.
- */
-function renderCommandLine(cmd: string[]) {
-  if (process.platform !== 'win32') {
-    return doRender(cmd, hasAnyChars(' ', '\\', '!', '"', "'", '&', '$'), posixEscape);
-  } else {
-    return doRender(cmd, hasAnyChars(' ', '"', '&', '^', '%'), windowsEscape);
-  }
-}
-
-/**
- * Render a UNIX command line
- */
-function doRender(
-  cmd: string[],
-  needsEscaping: (x: string) => boolean,
-  doEscape: (x: string) => string,
-): string {
-  return cmd.map((x) => (needsEscaping(x) ? doEscape(x) : x)).join(' ');
-}
-
-/**
- * Return a predicate that checks if a string has any of the indicated chars in it
- */
-function hasAnyChars(...chars: string[]): (x: string) => boolean {
-  return (str: string) => {
-    return chars.some((c) => str.indexOf(c) !== -1);
-  };
-}
-
-/**
- * Escape a shell argument for POSIX shells
- *
- * Wrapping in single quotes and escaping single quotes inside will do it for us.
- */
-function posixEscape(x: string) {
-  // Turn ' -> '"'"'
-  x = x.replace(/'/g, "'\"'\"'");
-  return `'${x}'`;
-}
-
-/**
- * Escape a shell argument for cmd.exe
- *
- * This is how to do it right, but I'm not following everything:
- *
- * https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
- */
-function windowsEscape(x: string): string {
-  // First surround by double quotes, ignore the part about backslashes
-  x = `"${x}"`;
-  // Now escape all special characters
-  const shellMeta = new Set<string>(['"', '&', '^', '%']);
-  return x
-    .split('')
-    .map((c) => (shellMeta.has(x) ? '^' + c : c))
-    .join('');
-}

packages/@aws-cdk/cdk-assets-lib/test/shell.test.ts

@@ -0,0 +1,38 @@
+import { shell } from '../lib/private/shell';
+
+describe('shell', () => {
+  test('spawn failures propagate the OS error so callers can key off e.code', async () => {
+    // docker.ts turns ENOENT into "please install docker" guidance; the
+    // wrapper must not swallow the errno code into a generic ProcessFailed.
+    await expect(
+      shell(['this-binary-does-not-exist-xyz'], {
+        shellEventPublisher: () => {
+        },
+        subprocessOutputDestination: 'ignore',
+      }),
+    ).rejects.toThrow(expect.objectContaining({ code: 'ENOENT' }));
+  });
+
+  test('non-zero exits throw ProcessFailed with the exit code and stderr', async () => {
+    await expect(
+      shell([process.execPath, '-e', 'process.stderr.write("boom"); process.exit(3);'], {
+        shellEventPublisher: () => {
+        },
+        subprocessOutputDestination: 'ignore',
+      }),
+    ).rejects.toThrow(expect.objectContaining({
+      code: 'PROCESS_FAILED',
+      exitCode: 3,
+    }));
+  });
+
+  test('returns stdout on success', async () => {
+    const output = await shell([process.execPath, '-e', 'process.stdout.write("hello")'], {
+      shellEventPublisher: () => {
+      },
+      subprocessOutputDestination: 'ignore',
+    });
+
+    expect(output).toEqual('hello');
+  });
+});