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 4ec5ff561..7482657f0 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 @@ -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): Promise { +export function guessExecutable(command: Command, debugFn: (msg: string) => Promise): Promise { + switch (command.type) { + case 'argv': + return guessArgvExecutable(command, debugFn); + case 'shell': + return guessShellExecutable(command, debugFn); + } +} + +export async function guessArgvExecutable(command: Extract, debugFn: (msg: string) => Promise): Promise { + // 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, debugFn: (msg: string) => Promise): Promise { // 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,7 +183,7 @@ 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)]; @@ -158,7 +191,7 @@ function guessInterpreter(file: FileInfo): string { return handler(file.fileName); } - return quoteSpaces(file.fileName); + return [file.fileName]; } /** @@ -168,13 +201,13 @@ const EXTENSION_MAP: Record = { '.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]; } /** diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/exec.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/exec.ts index f2888b242..57a3f8627 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/exec.ts +++ b/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((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; + const spawnOpts: child_process.SpawnOptionsWithStdioTuple = { 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)); } }); }); diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/prepare-source.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/prepare-source.ts index a4dcf1df7..e157ce0a9 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/prepare-source.ts +++ b/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 { - return guessExecutable(app, this.debugFn); + public guessExecutable(command: Command): Promise { + return guessExecutable(command, this.debugFn); } /** diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts index e8e8e670a..5fdf72e2a 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts +++ b/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); 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 431e5b093..074a56ae7 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 @@ -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); }); /** diff --git a/packages/aws-cdk/lib/cli/parse-command-line-arguments.ts b/packages/aws-cdk/lib/cli/parse-command-line-arguments.ts index 1f8e7377b..b4b39acd2 100644 --- a/packages/aws-cdk/lib/cli/parse-command-line-arguments.ts +++ b/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): any { diff --git a/packages/aws-cdk/lib/cxapp/exec.ts b/packages/aws-cdk/lib/cxapp/exec.ts index 88934cc16..290abedcd 100644 --- a/packages/aws-cdk/lib/cxapp/exec.ts +++ b/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,28 +99,40 @@ 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((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; + const spawnOpts: childProcess.SpawnOptionsWithStdioTuple = { 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); @@ -134,12 +140,12 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config: 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; } }