diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/command-line.ts b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/command-line.ts new file mode 100644 index 000000000..8c5130e66 --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/command-line.ts @@ -0,0 +1,302 @@ +import { ToolkitError } from '../../toolkit/toolkit-error'; + +type ShellSyntax = 'posix' | 'cmd.exe'; + +/** + * Class to help with parsing and formatting command-lines + * + * What syntax we recognize is an attribute of the `parse` and `toString()` operations, + * NOT of the command line itself. Defaults to the current platform's default shell + * syntax. + * + * Because we start with arbitrary shell strings, we may end up stuffing special + * shell syntax inside an `argv: string[]` array, which doesn't necessarily make + * a lot of sense. There could be a lot more modeling here to for example tag + * `argv` elements as literals or bits of shell syntax so we can render them out + * inert or active. Making this class do all of that correctly is weeks worth of + * work. Instead, it's going to be mostly concerned with correctly parsing and + * preserving spaces, so that we can correctly handle command lines with spaces + * in them on Windows. + * + * - Windows: emulates the behavior of `cmd.exe` if `cmd.exe` is the default shell, + * or the behavior of a POSIX shell otherwise. + * - POSIX: emulates the behavior of a standard POSIX shell. + * + * For some insight of the hell this is on Windows, see these links: + * + * - + * - + */ +export class CommandLine { + /** + * Parse a command line into components. + * + * Shell characters will end up as elements of the command-line array without + * being marked as such. `toStringGrouped()` will render them out back again, + * while `toStringInert()` will escape them. + */ + public static parse(cmdLine: string, syntax: ShellSyntax = defaultShellSyntax()) { + const argv = isWindows(syntax) ? parseCommandLineWindows(cmdLine) : parseCommandLinePosix(cmdLine); + return new CommandLine(argv); + } + + constructor(public readonly argv: string[]) { + } + + /** + * Render the command line as a string, quoting only whitespace (and quotes) + * + * Any other special characters are left in exactly as-is. + */ + public toStringGrouped(syntax: ShellSyntax = defaultShellSyntax()) { + if (isWindows(syntax)) { + return formatCommandLineWindows(this.argv, /^\S+$/); + } else { + return formatCommandLinePosix(this.argv, /^\S+$/); + } + } + + /** + * Render the command line as a string, escaping characters that would be interpreted by the shell + * + * The command will be a command invocation with literal parameters, nothing else. + */ + public toStringInert(syntax: ShellSyntax = defaultShellSyntax()) { + if (isWindows(syntax)) { + return formatCommandLineWindows(this.argv, /^[a-zA-Z0-9._\-+=/:]+$/); + } else { + return formatCommandLinePosix(this.argv, /^[a-zA-Z0-9._\-+=/:^]+$/); + } + } + + public toString() { + return this.toStringGrouped(); + } +} + +/** + * Parse command line on Windows + * + * @see https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments?view=msvc-170 + */ +function parseCommandLineWindows(commandLine: string): string[] { + const ret: string[] = []; + let current = ''; + let quoted = false; + let backSlashcount = 0; + + for (let i = 0; i < commandLine.length; i++) { + const c = commandLine[i]; + + if (c === '\\') { + backSlashcount += 1; + continue; + } + + // We also allow quoting " by doubling it up. + if (c === '"' && i + 1 < commandLine.length && commandLine[i + 1] === '"') { + current += '"'; + i += 1; + continue; + } + + // Only type of quote is ", and backslashes only behave specially before a " + if (c === '"') { + if (backSlashcount % 2 === 0) { + current += '\\'.repeat(backSlashcount / 2); + quoted = !quoted; + } else { + current += '\\'.repeat(Math.floor(backSlashcount / 2)) + '"'; + } + backSlashcount = 0; + + continue; + } + + if (backSlashcount > 0) { + current += '\\'.repeat(backSlashcount); + backSlashcount = 0; + } + + if (quoted) { + current += c; + continue; + } + + if (isWhitespace(c)) { + if (current) { + ret.push(current); + } + current = ''; + continue; + } + + current += c; + } + + if (current) { + ret.push(current); + } + + return ret; +} + +function isWhitespace(char: string): boolean { + return char === ' ' || char === '\t'; +} + +function isWindows(x: ShellSyntax) { + return x === 'cmd.exe'; +} + +function parseCommandLinePosix(commandLine: string): string[] { + const result: string[] = []; + let current = ''; + let inDoubleQuote = false; + let inSingleQuote = false; + let escapeNext = false; + + for (let i = 0; i < commandLine.length; i++) { + const char = commandLine[i]; + + // Handle escape character + if (escapeNext) { + // In double quotes, only certain characters are escaped + if (inDoubleQuote && !'\\"$`'.includes(char)) { + current += '\\'; + } + current += char; + escapeNext = false; + continue; + } + + if (char === '\\' && !inSingleQuote) { + escapeNext = true; + continue; + } + + // Handle quotes + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + continue; + } + + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + continue; + } + + // Handle whitespace + if (!inDoubleQuote && !inSingleQuote && /\s/.test(char)) { + if (current) { + result.push(current); + current = ''; + } + continue; + } + + current += char; + } + + // Add the last argument if there is one + if (current) { + result.push(current); + } + + // Check for unclosed quotes + if (inDoubleQuote || inSingleQuote) { + throw new ToolkitError('Unclosed quotes in command line'); + } + + // Check for trailing backslash + if (escapeNext) { + throw new ToolkitError('Trailing backslash in command line'); + } + + return result; +} + +/** + * Format a command line in a sensible way + */ +function formatCommandLinePosix(argv: string[], componentIsSafe: RegExp): string { + return argv.map(arg => { + // Empty string needs quotes + if (arg === '') { + return '\'\''; + } + + // If argument contains no problematic characters, return it as-is + if (componentIsSafe.test(arg)) { + return arg; + } + + const escaped = Array.from(arg).map(char => char === '\'' || char === '\\' ? `\\${char}` : char).join(''); + return `'${escaped}'`; + }).join(' '); +} + +/** + * Format a command line in a sensible way + */ +function formatCommandLineWindows(argv: string[], componentIsSafe: RegExp): string { + return argv.map(arg => { + // Empty string needs quotes + if (arg === '') { + return '""'; + } + + // If argument contains no problematic characters, return it as-is + if (componentIsSafe.test(arg)) { + return arg; + } + + let escaped = '"'; + let backslashCount = 0; + + for (let i = 0; i < arg.length; i++) { + const char = arg[i]; + + if (char === '\\') { + // Count consecutive backslashes + backslashCount++; + } else if (char === '"') { + // Double the backslashes before a quote and escape the quote + escaped += '\\'.repeat(backslashCount * 2 + 1) + '"'; + backslashCount = 0; + } else { + // Add accumulated backslashes if any + if (backslashCount > 0) { + escaped += '\\'.repeat(backslashCount); + backslashCount = 0; + } + escaped += char; + } + } + + // Handle trailing backslashes before the closing quote + if (backslashCount > 0) { + escaped += '\\'.repeat(backslashCount * 2); + } + + escaped += '"'; + return escaped; + }).join(' '); +} + +/** + * @see https://github.com/nodejs/node/blob/b4c5fb4ffbec9f27ba5799070c2e0588b7c7ff0e/lib/child_process.js#L626 + */ +function defaultShellSyntax(): ShellSyntax { + if (process.platform !== 'win32') { + return 'posix'; + } + + const file = process.env.comspec || 'cmd.exe'; + // '/d /s /c' is used only for cmd.exe. + if (/^(?:.*\\)?cmd(?:\.exe)?$/i.test(file)) { + return 'cmd.exe'; + } + + return 'posix'; +} 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 7d76d6df8..63ba1ac1f 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 { CommandLine } from './command-line'; export type Env = { [key: string]: string | undefined }; export type Context = { [key: string]: unknown }; @@ -105,36 +106,45 @@ export function spaceAvailableForContext(env: Env, limit: number) { } /** - * Guess the executable from the command-line argument + * Guess the interpreter from the command-line argument, if the argument is a single file name (no arguments) * - * Only do this if the file is NOT marked as executable. If it is, - * we'll defer to the shebang inside the file itself. + * - On Windows: it's hard to verify if registry associations have or have not + * been set up for this file type (i.e., ShellExec'ing the file will work or not), + * so we'll assume the worst and take control. * - * If we're on Windows, we ALWAYS take the handler, since it's hard to - * verify if registry associations have or have not been set up for this - * file type, so we'll assume the worst and take control. + * - On POSIX: if the file is NOT marked as executable, guess the interpreter. If it is executable, + * executing the file will work and the correct interpreter should be in the file's shebang. + * + * The behavior of only guessing the interpreter if the command line is a single file name + * is a bit limited: we can't put a `.js` file with arguments in the command line and have + * 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(app: string, debugFn: (msg: string) => Promise) { - const commandLine = appToArray(app); - if (commandLine.length === 1) { - let fstat; - - try { - fstat = await fs.stat(commandLine[0]); - } catch { - await debugFn(`Not a file: '${commandLine[0]}'. Using '${commandLine}' as command-line`); - return commandLine; - } - - // eslint-disable-next-line no-bitwise - const isExecutable = (fstat.mode & fs.constants.X_OK) !== 0; - const isWindows = process.platform === 'win32'; - - const handler = EXTENSION_MAP.get(path.extname(commandLine[0])); - if (handler && (!isExecutable || isWindows)) { - return handler(commandLine[0]); - } +export async function guessExecutable(app: string, debugFn: (msg: string) => Promise): Promise { + const commandLine = CommandLine.parse(app); + + if (commandLine.argv.length !== 1) { + return commandLine; + } + + let fstat; + + try { + fstat = await fs.stat(commandLine.argv[0]); + } catch { + await debugFn(`Not a file: '${commandLine.argv[0]}'. Using '${commandLine}' as command-line`); + return commandLine; + } + + // eslint-disable-next-line no-bitwise + const isExecutable = (fstat.mode & fs.constants.X_OK) !== 0; + const isWindows = process.platform === 'win32'; + + const handler = EXTENSION_MAP.get(path.extname(commandLine.argv[0])); + if (handler && (!isExecutable || isWindows)) { + return new CommandLine(handler(commandLine.argv[0])); } + return commandLine; } @@ -142,7 +152,7 @@ export async function guessExecutable(app: string, debugFn: (msg: string) => Pro * Mapping of extensions to command-line generators */ const EXTENSION_MAP = new Map([ - ['.js', executeNode], + ['.js', executeCurrentNode], ]); type CommandGenerator = (file: string) => string[]; @@ -150,15 +160,6 @@ 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[] { +function executeCurrentNode(scriptFile: string): string[] { return [process.execPath, scriptFile]; } - -/** - * Make sure the 'app' is an array - * - * If it's a string, split on spaces as a trivial way of tokenizing the command line. - */ -function appToArray(app: any) { - return typeof app === 'string' ? app.split(' ') : app; -} 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 b54b8e925..59ba815a8 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 @@ -2,6 +2,7 @@ import * as child_process from 'node:child_process'; // eslint-disable-next-line @typescript-eslint/no-require-imports import split = require('split2'); import { ToolkitError } from '../../../toolkit/toolkit-error'; +import type { CommandLine } from '../command-line'; type EventPublisher = (event: 'open' | 'data_stdout' | 'data_stderr' | 'close', line: string) => void; @@ -13,8 +14,14 @@ interface ExecOptions { /** * Execute a command and args in a child process + * + * @param command - The command to execute + * @param args - Optional arguments for the command + * @param options - Additional options for execution */ -export async function execInChildProcess(commandAndArgs: string, options: ExecOptions = {}) { +export async function execInChildProcess(cmd: CommandLine, options: ExecOptions = {}) { + const commandLineString = cmd.toStringGrouped(); + return new Promise((ok, fail) => { // We use a slightly lower-level interface to: // @@ -24,12 +31,18 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp // // - 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, { + const proc = child_process.spawn(commandLineString, { stdio: ['ignore', 'pipe', 'pipe'], detached: false, - shell: true, 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, }); const eventPublisher: EventPublisher = options.eventPublisher ?? ((type, line) => { @@ -54,7 +67,7 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp if (code === 0) { return ok(); } else { - return fail(new ToolkitError(`${commandAndArgs}: Subprocess exited with error ${code}`)); + return fail(new ToolkitError(`${commandLineString}: Subprocess exited with error ${code}`)); } }); }); 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 161409b47..875a1ce57 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 @@ -417,7 +417,11 @@ export abstract class CloudAssemblySourceBuilder { }; } /** - * Use a directory containing an AWS CDK app as source. + * Use an AWS CDK app executable as source. + * + * `app` is a command line that will be executed to produce a Cloud Assembly. + * The command will be executed in a shell, so it must come from a trusted source. + * Use the `CommandLine` class to help you build a safe shell string if necessary. * * The subprocess will execute in `workingDirectory`, which defaults to * the current process' working directory if not given. @@ -443,6 +447,7 @@ export abstract class CloudAssemblySourceBuilder { * all the CDK's default context sources, and updates will be written to * `cdk.context.json`. * + * @param app - the command line to execute to produce a Cloud Assembly * @param props - additional configuration properties * @returns the CloudAssembly source */ @@ -500,7 +505,7 @@ export abstract class CloudAssemblySourceBuilder { }); const cleanupTemp = writeContextToEnv(env, fullContext, 'env-is-complete'); try { - await execInChildProcess(commandLine.join(' '), { + await execInChildProcess(commandLine, { eventPublisher: async (type, line) => { switch (type) { case 'data_stdout': diff --git a/packages/@aws-cdk/toolkit-lib/lib/index.ts b/packages/@aws-cdk/toolkit-lib/lib/index.ts index 6de451e35..5d8182375 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/index.ts @@ -14,6 +14,7 @@ export * from './payloads'; // Supporting acts export * from './api/aws-auth'; export * from './api/cloud-assembly'; +export * from './api/cloud-assembly/command-line'; export * from './api/io'; export * from './api/tags'; export * from './api/plugin'; diff --git a/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/command-line.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/command-line.test.ts new file mode 100644 index 000000000..f7ecfbadc --- /dev/null +++ b/packages/@aws-cdk/toolkit-lib/test/api/cloud-assembly/command-line.test.ts @@ -0,0 +1,55 @@ +import { CommandLine } from '../../../lib/api/cloud-assembly/command-line'; + +////////////////////////////////////////////////////////////////////////////////////////////// +// Windows +// + +test.each([ + ['program.exe "arg with spaces" simple-arg', ['program.exe', 'arg with spaces', 'simple-arg']], + ['program.exe \\silly', ['program.exe', '\\silly']], + // Edge cases from https://learn.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments?view=msvc-170 + ['"a b c" d e', ['a b c', 'd', 'e']], + ['"ab\\"c" "\\\\" d', ['ab"c', '\\', 'd']], + ['a\\\\\\b d"e f"g h', ['a\\\\\\b', 'de fg', 'h']], + ['a\\\\\\"b c d', ['a\\"b', 'c', 'd']], + ['a\\\\\\\\"b c" d e', ['a\\\\b c', 'd', 'e']], + ['a"b"" c d', ['ab" c d']], +])('windows parses %s to %p', (input, expected) => { + const output = CommandLine.parse(input, 'cmd.exe'); + + expect(output.argv).toEqual(expected); +}); + +test.each([ + [['program.exe', 'with spaces'], 'program.exe "with spaces"'], + [['C:\\Program Files\\node.exe', 'hello.js'], '"C:\\Program Files\\node.exe" hello.js'], +])('windows formats grouped %p to %p', (input, expected) => { + const cmd = new CommandLine(input); + expect(cmd.toStringGrouped('cmd.exe')).toEqual(expected); +}); + +////////////////////////////////////////////////////////////////////////////////////////////// +// POSIX +// + +test.each([ + ['program "arg with spaces" simple-arg', ['program', 'arg with spaces', 'simple-arg']], + ['program \'arg with spaces\' simple-arg', ['program', 'arg with spaces', 'simple-arg']], + ['program \\silly', ['program', 'silly']], + ['program \\\\silly', ['program', '\\silly']], + ['program \'"\'', ['program', '"']], + ['program "\'"', ['program', '\'']], + ['program as"d e"f', ['program', 'asd ef']], +])('POSIX parses %s to %p', (input, expected) => { + const output = CommandLine.parse(input, 'posix'); + + expect(output.argv).toEqual(expected); +}); + +test.each([ + [['program', 'with spaces'], 'program \'with spaces\''], + [['/path with spaces', 'hello.js'], '\'/path with spaces\' hello.js'], +])('posix formats grouped %p to %p', (input, expected) => { + const cmd = new CommandLine(input); + expect(cmd.toStringGrouped('posix')).toEqual(expected); +}); diff --git a/packages/aws-cdk/lib/api/cloud-assembly.ts b/packages/aws-cdk/lib/api/cloud-assembly.ts index 4a8554357..05985ff35 100644 --- a/packages/aws-cdk/lib/api/cloud-assembly.ts +++ b/packages/aws-cdk/lib/api/cloud-assembly.ts @@ -1,6 +1,7 @@ /* eslint-disable import/no-relative-packages */ export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly'; export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private'; +export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/command-line'; export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/environment'; export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/stack-collection'; export * from '../../../@aws-cdk/toolkit-lib/lib/api/cloud-assembly/stack-assembly'; diff --git a/packages/aws-cdk/lib/cli/util/npm.ts b/packages/aws-cdk/lib/cli/util/npm.ts index 47d049829..f30c05973 100644 --- a/packages/aws-cdk/lib/cli/util/npm.ts +++ b/packages/aws-cdk/lib/cli/util/npm.ts @@ -1,16 +1,16 @@ -import { exec as _exec } from 'child_process'; +import { execFile as _execFile } from 'child_process'; import { promisify } from 'util'; import { ToolkitError } from '@aws-cdk/toolkit-lib'; -const exec = promisify(_exec); +const execFile = promisify(_execFile); /* c8 ignore start */ export async function execNpmView(currentVersion: string) { try { // eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism const [latestResult, currentResult] = await Promise.all([ - exec('npm view aws-cdk@latest version', { timeout: 3000 }), - exec(`npm view aws-cdk@${currentVersion} name version deprecated --json`, { timeout: 3000 }), + execFile('npm', ['view', 'aws-cdk@latest', 'version'], { timeout: 3000 }), + execFile('npm', ['view', `aws-cdk@${currentVersion}`, 'name', 'version', 'deprecated', '--json'], { timeout: 3000 }), ]); if (latestResult.stderr && latestResult.stderr.trim().length > 0) { diff --git a/packages/aws-cdk/lib/commands/init/init.ts b/packages/aws-cdk/lib/commands/init/init.ts index 8265dc849..009d3b820 100644 --- a/packages/aws-cdk/lib/commands/init/init.ts +++ b/packages/aws-cdk/lib/commands/init/init.ts @@ -368,7 +368,7 @@ async function initializeGitRepository(workDir: string) { try { await execute('git', ['init'], { cwd: workDir }); await execute('git', ['add', '.'], { cwd: workDir }); - await execute('git', ['commit', '--message="Initial commit"', '--no-gpg-sign'], { cwd: workDir }); + await execute('git', ['commit', '--message=Initial commit', '--no-gpg-sign'], { cwd: workDir }); } catch { warning('Unable to initialize git repository for your project.'); } @@ -428,7 +428,7 @@ async function postInstallPython(cwd: string) { warning(`Please run '${python} -m venv .venv'!`); info(`Executing ${chalk.green('Creating virtualenv...')}`); try { - await execute(python, ['-m venv', '.venv'], { cwd }); + await execute(python, ['-m', 'venv', '.venv'], { cwd }); } catch { warning('Unable to create virtualenv automatically'); warning(`Please run '${python} -m venv .venv'!`); @@ -470,7 +470,7 @@ function isRoot(dir: string) { async function execute(cmd: string, args: string[], { cwd }: { cwd: string }) { const child = childProcess.spawn(cmd, args, { cwd, - shell: true, + shell: false, stdio: ['ignore', 'pipe', 'inherit'], }); let stdout = ''; diff --git a/packages/aws-cdk/lib/commands/init/os.ts b/packages/aws-cdk/lib/commands/init/os.ts index cbcef01c0..18ddee2e9 100644 --- a/packages/aws-cdk/lib/commands/init/os.ts +++ b/packages/aws-cdk/lib/commands/init/os.ts @@ -1,5 +1,5 @@ import * as child_process from 'child_process'; -import { ToolkitError } from '@aws-cdk/toolkit-lib'; +import { CommandLine, ToolkitError } from '@aws-cdk/toolkit-lib'; import * as chalk from 'chalk'; import { debug } from '../../logging'; @@ -10,11 +10,10 @@ import { debug } from '../../logging'; * string. */ export async function shell(command: string[]): Promise { - const commandLine = renderCommandLine(command); + const commandLine = new CommandLine(command).toStringGrouped(); debug(`Executing ${chalk.blue(commandLine)}`); - const child = child_process.spawn(command[0], renderArguments(command.slice(1)), { - // Need this for Windows where we want .cmd and .bat to be found as well. - shell: true, + const child = child_process.spawn(command[0], command.slice(1), { + shell: false, stdio: ['ignore', 'pipe', 'inherit'], }); @@ -38,60 +37,3 @@ export async function shell(command: string[]): Promise { }); }); } - -function renderCommandLine(cmd: string[]) { - return renderArguments(cmd).join(' '); -} - -/** - * Render the arguments to include escape characters for each platform. - */ -function renderArguments(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); -} - -/** - * 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(['"', '&', '^', '%']); - return x.split('').map(c => shellMeta.has(x) ? '^' + c : c).join(''); -} diff --git a/packages/aws-cdk/lib/cxapp/exec.ts b/packages/aws-cdk/lib/cxapp/exec.ts index af808f419..a7019c7a5 100644 --- a/packages/aws-cdk/lib/cxapp/exec.ts +++ b/packages/aws-cdk/lib/cxapp/exec.ts @@ -55,7 +55,7 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config: return { assembly: createAssembly(app), lock }; } - const commandLine = await guessExecutable(app, debugFn); + const command = await guessExecutable(app, debugFn); const outdir = config.settings.get(['output']); if (!outdir) { @@ -85,7 +85,8 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config: const cleanupTemp = writeContextToEnv(env, context, 'add-process-env-later'); try { - await exec(commandLine.join(' ')); + // Render a whitespace-aware string of the command + await exec(command.toStringGrouped()); const assembly = createAssembly(outdir); @@ -97,7 +98,7 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config: await cleanupTemp(); } - async function exec(commandAndArgs: string) { + async function exec(commandLine: string) { try { await new Promise((ok, fail) => { // We use a slightly lower-level interface to: @@ -110,10 +111,17 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config: // 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, { + const proc = childProcess.spawn(commandLine, { stdio: ['ignore', 'inherit', 'inherit'], detached: false, + + // 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, + env: { ...process.env, ...env, @@ -126,12 +134,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(`${commandLine}: Subprocess exited with error ${code}`)); } }); }); } catch (e: any) { - await debugFn(`failed command: ${commandAndArgs}`); + await debugFn(`failed command: ${commandLine}`); throw e; } } diff --git a/packages/aws-cdk/test/_helpers/mock-child_process.ts b/packages/aws-cdk/test/_helpers/mock-child_process.ts index 641d652ab..32254f1d3 100644 --- a/packages/aws-cdk/test/_helpers/mock-child_process.ts +++ b/packages/aws-cdk/test/_helpers/mock-child_process.ts @@ -22,8 +22,10 @@ export function mockSpawn(...invocations: Invocation[]) { let mock = (child_process.spawn as any); for (const _invocation of invocations) { const invocation = _invocation; // Mirror into variable for closure - mock = mock.mockImplementationOnce((binary: string, options: child_process.SpawnOptions) => { - expect(binary).toEqual(invocation.commandLine); + mock = mock.mockImplementationOnce((command: string, args: string[] = [], options: child_process.SpawnOptions) => { + // Handle both old and new calling conventions + const fullCommand = args.length > 0 ? `${command} ${args.join(' ')}` : command; + expect(fullCommand).toEqual(invocation.commandLine); if (invocation.cwd != null) { expect(options.cwd).toBe(invocation.cwd);