feat: bypass shell if app is given as a string array

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment.ts

@@ -3,6 +3,7 @@ import * as cxapi from '@aws-cdk/cx-api';
 import * as fs from 'fs-extra';
 import type { SdkProvider } from '../aws-auth/private';
 import type { Settings } from '../settings';
+import type { Command } from './private/exec';
 
 export type Env = { [key: string]: string | undefined };
 export type Context = { [key: string]: unknown };
@@ -105,7 +106,7 @@ export function spaceAvailableForContext(env: Env, limit: number) {
 }
 
 /**
- * Guess the executable from the command-line argument
+ * Guess the executable from the command
  *
  * Input is the "app" string the user gave us. Output is the command line we are going to execute.
  *
@@ -121,25 +122,57 @@ export function spaceAvailableForContext(env: Env, limit: number) {
  * it work properly. Nevertheless, this is the behavior we have had for a long time and nobody
  * has really complained about it, so we'll keep it for now.
  */
-export async function guessExecutable(commandLine: string, debugFn: (msg: string) => Promise<void>): Promise<string> {
+export function guessExecutable(command: Command, debugFn: (msg: string) => Promise<void>): Promise<Command> {
+  switch (command.type) {
+    case 'argv':
+      return guessArgvExecutable(command, debugFn);
+    case 'shell':
+      return guessShellExecutable(command, debugFn);
+  }
+}
+
+export async function guessArgvExecutable(command: Extract<Command, { type: 'argv' }>, debugFn: (msg: string) => Promise<void>): Promise<Command> {
+  // We perform "guessInterpreter" on the first value in the array to execute successfully on Windows and on POSIX without the executable
+  // bit, and nothing else.
+  const [first, ...rest] = command.argv;
+  const firstFile = await checkFile(first);
+  if (firstFile) {
+    return {
+      type: 'argv',
+      argv: [...guessInterpreter(firstFile), ...rest],
+    };
+  }
+
+  // Not a file, so just use the given command line
+  await debugFn(`Not a file: '${first}'. Using ${JSON.stringify(command.argv)} as command`);
+  return command;
+}
+
+export async function guessShellExecutable(command: Extract<Command, { type: 'shell' }>, debugFn: (msg: string) => Promise<void>): Promise<Command> {
   // The command line with spaces in it could reference a file on disk. If true,
   // we quote it and return that, optionally by prefixing an interpreter.
-  const fullFile = await checkFile(commandLine);
+  const fullFile = await checkFile(command.command);
   if (fullFile) {
-    return guessInterpreter(fullFile);
+    return {
+      type: 'shell',
+      command: guessInterpreter(fullFile).map(quoteSpaces).join(' '),
+    };
   }
 
   // Otherwise, the first word on the command line could reference a file on
   // disk (quoted or non-quoted). If true, we optionally prefix an interpreter.
-  const [first, rest] = splitFirstShellWord(commandLine);
+  const [first, rest] = splitFirstShellWord(command.command);
   const firstFile = await checkFile(first);
   if (firstFile) {
-    return `${guessInterpreter(firstFile)} ${rest}`.trim();
+    return {
+      type: 'shell',
+      command: [...guessInterpreter(firstFile).map(quoteSpaces), rest].join(' ').trim(),
+    };
   }
 
   // We couldn't parse it, so just use the given command line.
-  await debugFn(`Not a file: '${commandLine}'. Using '${commandLine} as command-line`);
-  return commandLine;
+  await debugFn(`Not a file: '${command.command}'. Using '${command.command} as command-line`);
+  return command;
 }
 
 /**
@@ -150,15 +183,15 @@ export async function guessExecutable(commandLine: string, debugFn: (msg: string
  * - Prefixing an interpreter if necessary
  * - Quoting the file name if necessary
  */
-function guessInterpreter(file: FileInfo): string {
+function guessInterpreter(file: FileInfo): string[] {
   const isWindows = process.platform === 'win32';
 
   const handler = EXTENSION_MAP[path.extname(file.fileName)];
   if (handler && (!file.isExecutable || isWindows)) {
     return handler(file.fileName);
   }
 
-  return quoteSpaces(file.fileName);
+  return [file.fileName];
 }
 
 /**
@@ -168,13 +201,13 @@ const EXTENSION_MAP: Record<string, CommandGenerator> = {
   '.js': executeNode,
 };
 
-type CommandGenerator = (file: string) => string;
+type CommandGenerator = (file: string) => string[];
 
 /**
  * Execute the given file with the same 'node' process as is running the current process
  */
-function executeNode(scriptFile: string): string {
-  return `${quoteSpaces(process.execPath)} ${quoteSpaces(scriptFile)}`;
+function executeNode(scriptFile: string): string[] {
+  return [process.execPath, scriptFile];
 }
 
 /**

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/exec.ts

@@ -1,4 +1,5 @@
 import * as child_process from 'node:child_process';
+import type { Readable } from 'node:stream';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import split = require('split2');
 import { AssemblyError } from '../../../toolkit/toolkit-error';
@@ -11,32 +12,60 @@ interface ExecOptions {
   cwd?: string;
 }
 
+export type Command =
+  | { type: 'argv'; argv: string[] }
+  | { type: 'shell'; command: string }
+  ;
+
+/**
+ * Turn a user input into a `Command` type
+ */
+export function toCommand(input: string | string[]): Command {
+  if (Array.isArray(input)) {
+    return { type: 'argv', argv: input };
+  } else {
+    return { type: 'shell', command: input };
+  }
+}
+
+export function renderCommand(command: Command): string {
+  switch (command.type) {
+    case 'shell':
+      return command.command;
+    case 'argv':
+      return JSON.stringify(command.argv);
+  }
+}
+
 /**
  * Execute a command line in a child process
  */
-export async function execInChildProcess(commandAndArgs: string, options: ExecOptions = {}) {
+export async function execInChildProcess(command: Command, options: ExecOptions = {}) {
   return new Promise<void>((ok, fail) => {
-    // We use a slightly lower-level interface to:
-    //
-    // - Pass arguments in an array instead of a string, to get around a
-    //   number of quoting issues introduced by the intermediate shell layer
-    //   (which would be different between Linux and Windows).
-    //
-    // - We have to capture any output to stdout and stderr sp we can pass it on to the IoHost
-    //   To ensure messages get to the user fast, we will emit every full line we receive.
-    const proc = child_process.spawn(commandAndArgs, {
+    // Depending on the type of command we have to execute, spawn slightly differently
+    let proc : child_process.ChildProcessByStdio<null, Readable, Readable>;
+    const spawnOpts: child_process.SpawnOptionsWithStdioTuple<child_process.StdioNull, child_process.StdioPipe, child_process.StdioPipe> = {
       stdio: ['ignore', 'pipe', 'pipe'],
       detached: false,
       cwd: options.cwd,
       env: options.env,
+    };
 
-      // We are using 'shell: true' on purprose. Traditionally we have allowed shell features in
-      // this string, so we have to continue to do so into the future. On Windows, this is simply
-      // necessary to run .bat and .cmd files properly.
-      // Code scanning tools will flag this as a risk. The input comes from a trusted source,
-      // so it does not represent a security risk.
-      shell: true,
-    });
+    switch (command.type) {
+      case 'argv':
+        proc = child_process.spawn(command.argv[0], command.argv.slice(1), spawnOpts);
+        break;
+      case 'shell':
+        proc = child_process.spawn(command.command, {
+          ...spawnOpts,
+          // Command lines need a shell; necessary on windows for .bat and .cmd files, necessary on
+          // Linux to use the shell features we've traditionally supported.
+          // Code scanning tools will flag this as a risk. The input comes from a trusted source,
+          // so it does not represent a security risk.
+          shell: true,
+        });
+        break;
+    }
 
     const eventPublisher: EventPublisher = options.eventPublisher ?? ((type, line) => {
       switch (type) {
@@ -71,7 +100,7 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
           cause = new Error(stderr.join('\n'));
           cause.name = 'ExecutionError';
         }
-        return fail(AssemblyError.withCause(`${commandAndArgs}: Subprocess exited with error ${code}`, cause));
+        return fail(AssemblyError.withCause(`${renderCommand(command)}: Subprocess exited with error ${code}`, cause));
       }
     });
   });

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/prepare-source.ts

@@ -21,6 +21,7 @@ import { loadTree, some } from '../../tree';
 import type { Context, Env } from '../environment';
 import { prepareDefaultEnvironment, spaceAvailableForContext, guessExecutable, synthParametersFromSettings } from '../environment';
 import type { AppSynthOptions, LoadAssemblyOptions } from '../source-builder';
+import type { Command } from './exec';
 
 export interface ExecutionEnvironmentOptions {
   /**
@@ -163,8 +164,8 @@ export class ExecutionEnvironment implements AsyncDisposable {
    * verify if registry associations have or have not been set up for this
    * file type, so we'll assume the worst and take control.
    */
-  public guessExecutable(app: string): Promise<string> {
-    return guessExecutable(app, this.debugFn);
+  public guessExecutable(command: Command): Promise<Command> {
+    return guessExecutable(command, this.debugFn);
   }
 
   /**

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts

@@ -8,7 +8,7 @@ import { RWLock } from '../rwlock';
 import { CachedCloudAssembly } from './cached-source';
 import type { ContextAwareCloudAssemblyProps } from './private/context-aware-source';
 import { ContextAwareCloudAssemblySource } from './private/context-aware-source';
-import { execInChildProcess } from './private/exec';
+import { execInChildProcess, toCommand } from './private/exec';
 import { ExecutionEnvironment, assemblyFromDirectory, parametersFromSynthOptions, writeContextToEnv } from './private/prepare-source';
 import { ReadableCloudAssembly } from './private/readable-assembly';
 import type { ICloudAssemblySource } from './types';
@@ -491,7 +491,7 @@ export abstract class CloudAssemblySourceBuilder {
             resolveDefaultAppEnv: props.resolveDefaultEnvironment ?? true,
           });
 
-          const commandLine = await execution.guessExecutable(app);
+          const commandLine = await execution.guessExecutable(toCommand(app));
 
           const synthParams = parametersFromSynthOptions(props.synthOptions);
 

packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/environment.test.ts

@@ -1,5 +1,6 @@
 import * as fs from 'fs-extra';
 import { guessExecutable } from '../../../lib/api/cloud-assembly/environment';
+import { toCommand } from '../../../lib/api/cloud-assembly/private/exec';
 
 const BOTH = 'both' as const;
 const DONTCARE = 'DONT-CARE';
@@ -45,10 +46,10 @@ test.each([
   });
 
   // WHEN
-  const actual = await guessExecutable(commandLine, (_) => Promise.resolve());
+  const actual = await guessExecutable(toCommand(commandLine), (_) => Promise.resolve());
 
   // THEN
-  expect(actual).toEqual(expected);
+  expect(actual.type === 'shell' && actual.command).toEqual(expected);
 });
 
 /**

packages/aws-cdk/lib/cli/parse-command-line-arguments.ts

@@ -3,8 +3,8 @@
 // Do not edit by hand; all changes will be overwritten at build time from the config file.
 // -------------------------------------------------------------------------------------------
 /* eslint-disable @stylistic/max-len, @typescript-eslint/consistent-type-imports */
-import { Argv } from 'yargs';
 import * as helpers from './util/yargs-helpers';
+import { Argv } from 'yargs';
 
 // @ts-ignore TS6133
 export function parseCommandLineArguments(args: Array<string>): any {

packages/aws-cdk/lib/cxapp/exec.ts

@@ -6,8 +6,8 @@ import * as cxapi from '@aws-cdk/cx-api';
 import { ToolkitError } from '@aws-cdk/toolkit-lib';
 import * as fs from 'fs-extra';
 import type { IoHelper } from '../../lib/api-private';
-import type { SdkProvider, IReadLock } from '../api';
-import { RWLock, guessExecutable, prepareDefaultEnvironment, writeContextToEnv, synthParametersFromSettings } from '../api';
+import type { SdkProvider, IReadLock, Command } from '../api';
+import { RWLock, guessExecutable, prepareDefaultEnvironment, writeContextToEnv, synthParametersFromSettings, toCommand, renderCommand } from '../api';
 import type { Configuration } from '../cli/user-configuration';
 import { PROJECT_CONFIG, USER_DEFAULTS } from '../cli/user-configuration';
 import { versionNumber } from '../cli/version';
@@ -38,7 +38,7 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
 
   const build = config.settings.get(['build']);
   if (build) {
-    await exec(build);
+    await exec(toCommand(build));
   }
 
   let app = config.settings.get(['app']);
@@ -56,14 +56,8 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
     return { assembly: createAssembly(app), lock };
   }
 
-  // Traditionally it has been possible, though not widely advertised, to put a string[] into `cdk.json`.
-  // However, we would just quickly join this array back up to string with spaces (unquoted even!) and proceed as usual,
-  // thereby losing all the benefits of a pre-segmented command line. This coercion is just here for backwards
-  // compatibility with existing configurations. An upcoming PR might retain the benefit of the string[].
-  if (Array.isArray(app)) {
-    app = app.join(' ');
-  }
-  const commandLine = await guessExecutable(app, debugFn);
+  const command = toCommand(app);
+  const commandLine = command.type === 'shell' ? await guessExecutable(app, debugFn) : command;
 
   const outdir = config.settings.get(['output']);
   if (!outdir) {
@@ -105,41 +99,53 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
     await cleanupTemp();
   }
 
-  async function exec(commandAndArgs: string) {
+  async function exec(command: Command) {
     try {
       await new Promise<void>((ok, fail) => {
-        // We use a slightly lower-level interface to:
-        //
-        // - Pass arguments in an array instead of a string, to get around a
-        //   number of quoting issues introduced by the intermediate shell layer
-        //   (which would be different between Linux and Windows).
-        //
+        // Depending on the type of command we have to execute, spawn slightly differently
+
         // - Inherit stderr from controlling terminal. We don't use the captured value
         //   anyway, and if the subprocess is printing to it for debugging purposes the
         //   user gets to see it sooner. Plus, capturing doesn't interact nicely with some
         //   processes like Maven.
-        const proc = childProcess.spawn(commandAndArgs, {
+        let proc : childProcess.ChildProcessByStdio<null, null, null>;
+        const spawnOpts: childProcess.SpawnOptionsWithStdioTuple<childProcess.StdioNull, childProcess.StdioNull, childProcess.StdioNull> = {
           stdio: ['ignore', 'inherit', 'inherit'],
           detached: false,
-          shell: true,
           env: {
             ...process.env,
             ...env,
           },
-        });
+        };
+
+        switch (command.type) {
+          case 'argv':
+            proc = childProcess.spawn(command.argv[0], command.argv.slice(1), spawnOpts);
+            break;
+          case 'shell':
+            proc = childProcess.spawn(command.command, {
+              ...spawnOpts,
+              // Command lines need a shell; necessary on windows for .bat and .cmd files, necessary on
+              // Linux to use the shell features we've traditionally supported.
+              // Code scanning tools will flag this as a risk. The input comes from a trusted source,
+              // so it does not represent a security risk.
+              shell: true,
+            });
+            break;
+        }
 
         proc.on('error', fail);
 
         proc.on('exit', code => {
           if (code === 0) {
             return ok();
           } else {
-            return fail(new ToolkitError(`${commandAndArgs}: Subprocess exited with error ${code}`));
+            return fail(new ToolkitError(`${renderCommand(command)}: Subprocess exited with error ${code}`));
           }
         });
       });
     } catch (e: any) {
-      await debugFn(`failed command: ${commandAndArgs}`);
+      await debugFn(`failed command: ${renderCommand(command)}`);
       throw e;
     }
   }