Skip to content

Commit 9bf4263

Browse files
authored
fix(lambda-nodejs): use direct spawn for local bundling (#37292)
### Reason for this change The local bundling path builds a single shell command string and executes it via `spawnSync("bash", ["-c", command])`. User-controlled bundling properties are interpolated into this string without sanitization. Using direct `spawnSync` with argument arrays is the idiomatic Node.js approach and avoids shell interpretation entirely. ### Description of changes Replace shell-based command execution in the local bundling path with direct `spawnSync` calls using argument arrays. - `PackageManager.runBinCommand()` returns `string[]` instead of a joined string. Docker-path callers use `.join(" ")`. - New `BundlingStep` discriminated union type: `shell` (commandHooks), `spawn` (esbuild/tsc/install), `fs` (file operations). - `createLocalBundlingSteps()` builds the step sequence for local bundling using `toCliArgsArray()` and `getTsconfigCompilerOptionsArray()` (no shell quoting needed). - `tryBundle()` executes steps sequentially by type. - Docker bundling path's command is escaped, the default command relies on bash already existing so we can escape it with the bash syntax. - `commandHooks` remain shell-executed (user-provided by contract). ### Describe any new or updated permissions being added N/A ### Description of how you validated changes - All 125 existing aws-lambda-nodejs tests pass (6 suites). - Updated `Local bundling` test to assert direct spawn calls. - Added 6 new tests: esbuild options via spawn, nodeModules with fs operations, commandHooks via shell, preCompilation with tsc spawn, shell metacharacter handling, and pnpm workspace/cleanup. - Full `lerna run build --scope=aws-cdk-lib` passes. - Locally created a stack comparing the docker and local asset output ### Checklist - [x] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES --- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent c0849ea commit 9bf4263

File tree

6 files changed

+741
-107
lines changed

6 files changed

+741
-107
lines changed

packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts

Lines changed: 226 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
import * as fs from 'fs';
12
import * as os from 'os';
23
import * as path from 'path';
34
import type { IConstruct } from 'constructs';
45
import { PackageInstallation } from './package-installation';
56
import { LockFile, PackageManager } from './package-manager';
67
import type { BundlingOptions } from './types';
78
import { OutputFormat, SourceMapMode } from './types';
8-
import { exec, extractDependencies, findUp, getTsconfigCompilerOptions, isSdkV2Runtime } from './util';
9+
import { exec, extractDependencies, findUp, getTsconfigCompilerOptionsArray, isSdkV2Runtime } from './util';
910
import type { Architecture, AssetCode } from '../../aws-lambda';
1011
import { Code, Runtime } from '../../aws-lambda';
1112
import * as cdk from '../../core';
@@ -216,24 +217,16 @@ export class Bundling implements cdk.BundlingOptions {
216217
}
217218
}
218219

219-
private createBundlingCommand(scope: IConstruct, options: BundlingCommandOptions): string {
220-
const pathJoin = osPathJoin(options.osPlatform);
221-
let relativeEntryPath = pathJoin(options.inputDir, this.relativeEntryPath);
222-
let tscCommand = '';
223-
224-
if (this.props.preCompilation) {
225-
const tsconfig = this.props.tsconfig ?? findUp('tsconfig.json', path.dirname(this.props.entry));
226-
if (!tsconfig) {
227-
throw new ValidationError('CannotFindTsconfigJsonPre', 'Cannot find a `tsconfig.json` but `preCompilation` is set to `true`, please specify it via `tsconfig`', scope);
228-
}
229-
const compilerOptions = getTsconfigCompilerOptions(tsconfig);
230-
tscCommand = `${options.tscRunner} "${relativeEntryPath}" ${compilerOptions}`;
231-
relativeEntryPath = relativeEntryPath.replace(/\.ts(x?)$/, '.js$1');
232-
}
233-
234-
const loaders = Object.entries(this.props.loader ?? {});
235-
const defines = Object.entries(this.props.define ?? {});
236-
220+
/**
221+
* Builds the raw esbuild CLI arguments as an array of strings.
222+
* No shell quoting — callers apply their own formatting.
223+
*/
224+
private buildEsbuildArgs(
225+
scope: IConstruct,
226+
inputDir: string,
227+
outputDir: string,
228+
pathJoin: (...parts: string[]) => string,
229+
): string[] {
237230
if (this.props.sourceMap === false && this.props.sourceMapMode) {
238231
throw new ValidationError('SourceMapModeCannotSource', 'sourceMapMode cannot be used when sourceMap is false', scope);
239232
}
@@ -242,31 +235,54 @@ export class Bundling implements cdk.BundlingOptions {
242235
const sourceMapMode = this.props.sourceMapMode ?? SourceMapMode.DEFAULT;
243236
const sourceMapValue = sourceMapMode === SourceMapMode.DEFAULT ? '' : `=${this.props.sourceMapMode}`;
244237
const sourcesContent = this.props.sourcesContent ?? true;
245-
246238
const outFile = this.props.format === OutputFormat.ESM ? 'index.mjs' : 'index.js';
247-
const esbuildCommand: string[] = [
248-
options.esbuildRunner,
249-
'--bundle', `"${relativeEntryPath}"`,
239+
240+
return [
250241
`--target=${this.props.target ?? toTarget(scope, this.props.runtime)}`,
251242
'--platform=node',
252243
...this.props.format ? [`--format=${this.props.format}`] : [],
253-
`--outfile="${pathJoin(options.outputDir, outFile)}"`,
244+
`--outfile=${pathJoin(outputDir, outFile)}`,
254245
...this.props.minify ? ['--minify'] : [],
255246
...sourceMapEnabled ? [`--sourcemap${sourceMapValue}`] : [],
256247
...sourcesContent ? [] : [`--sources-content=${sourcesContent}`],
257248
...this.externals.map(external => `--external:${external}`),
258-
...loaders.map(([ext, name]) => `--loader:${ext}=${name}`),
259-
...defines.map(([key, value]) => `--define:${key}=${JSON.stringify(value)}`),
249+
...Object.entries(this.props.loader ?? {}).map(([ext, name]) => `--loader:${ext}=${name}`),
250+
...Object.entries(this.props.define ?? {}).map(([key, value]) => `--define:${key}=${value}`),
260251
...this.props.logLevel ? [`--log-level=${this.props.logLevel}`] : [],
261252
...this.props.keepNames ? ['--keep-names'] : [],
262-
...this.relativeTsconfigPath ? [`--tsconfig="${pathJoin(options.inputDir, this.relativeTsconfigPath)}"`] : [],
263-
...this.props.metafile ? [`--metafile="${pathJoin(options.outputDir, 'index.meta.json')}"`] : [],
264-
...this.props.banner ? [`--banner:js=${JSON.stringify(this.props.banner)}`] : [],
265-
...this.props.footer ? [`--footer:js=${JSON.stringify(this.props.footer)}`] : [],
253+
...this.relativeTsconfigPath ? [`--tsconfig=${pathJoin(inputDir, this.relativeTsconfigPath)}`] : [],
254+
...this.props.metafile ? [`--metafile=${pathJoin(outputDir, 'index.meta.json')}`] : [],
255+
...this.props.banner ? [`--banner:js=${this.props.banner}`] : [],
256+
...this.props.footer ? [`--footer:js=${this.props.footer}`] : [],
266257
...this.props.mainFields ? [`--main-fields=${this.props.mainFields.join(',')}`] : [],
267-
...this.props.inject ? this.props.inject.map(i => `--inject:"${i}"`) : [],
268-
...this.props.esbuildArgs ? [toCliArgs(this.props.esbuildArgs)] : [],
258+
...this.props.inject ? this.props.inject.map(i => `--inject:${i}`) : [],
269259
];
260+
}
261+
262+
private createBundlingCommand(scope: IConstruct, options: BundlingCommandOptions): string {
263+
const pathJoin = osPathJoin(options.osPlatform);
264+
let relativeEntryPath = pathJoin(options.inputDir, this.relativeEntryPath);
265+
let tscCommand = '';
266+
267+
if (this.props.preCompilation) {
268+
const tsconfig = this.props.tsconfig ?? findUp('tsconfig.json', path.dirname(this.props.entry));
269+
if (!tsconfig) {
270+
throw new ValidationError('CannotFindTsconfigJsonPre', 'Cannot find a `tsconfig.json` but `preCompilation` is set to `true`, please specify it via `tsconfig`', scope);
271+
}
272+
const compilerOptionsArray = getTsconfigCompilerOptionsArray(tsconfig);
273+
tscCommand = preparePosixShellCommand([options.tscRunner!, relativeEntryPath, ...compilerOptionsArray]);
274+
relativeEntryPath = relativeEntryPath.replace(/\.ts(x?)$/, '.js$1');
275+
}
276+
277+
const rawArgs = this.buildEsbuildArgs(scope, options.inputDir, options.outputDir, pathJoin);
278+
279+
const esbuildArgv: string[] = [
280+
options.esbuildRunner,
281+
'--bundle', relativeEntryPath,
282+
...rawArgs,
283+
...this.props.esbuildArgs ? toCliArgsArray(this.props.esbuildArgs) : [],
284+
];
285+
const esbuildCommand = preparePosixShellCommand(esbuildArgv);
270286

271287
let depsCommand = '';
272288
if (this.props.nodeModules) {
@@ -301,7 +317,7 @@ export class Bundling implements cdk.BundlingOptions {
301317
return chain([
302318
...this.props.commandHooks?.beforeBundling(options.inputDir, options.outputDir) ?? [],
303319
tscCommand,
304-
esbuildCommand.join(' '),
320+
esbuildCommand,
305321
...(this.props.nodeModules && this.props.commandHooks?.beforeInstall(options.inputDir, options.outputDir)) ?? [],
306322
depsCommand,
307323
...this.props.commandHooks?.afterBundling(options.inputDir, options.outputDir) ?? [],
@@ -310,15 +326,10 @@ export class Bundling implements cdk.BundlingOptions {
310326

311327
private getLocalBundlingProvider(scope: IConstruct): cdk.ILocalBundling {
312328
const osPlatform = os.platform();
313-
const createLocalCommand = (outputDir: string, esbuild: PackageInstallation, tsc?: PackageInstallation) => this.createBundlingCommand(scope, {
314-
inputDir: this.projectRoot,
315-
outputDir,
316-
esbuildRunner: esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild',
317-
tscRunner: tsc && (tsc.isLocal ? this.packageManager.runBinCommand('tsc') : 'tsc'),
318-
osPlatform,
319-
});
320329
const environment = this.props.environment ?? {};
321330
const cwd = this.projectRoot;
331+
const createSteps = (outputDir: string, esbuild: PackageInstallation, tsc?: PackageInstallation) =>
332+
this.createLocalBundlingSteps(scope, outputDir, esbuild, tsc);
322333

323334
return {
324335
tryBundle(outputDir: string) {
@@ -331,31 +342,162 @@ export class Bundling implements cdk.BundlingOptions {
331342
throw new ValidationError('ExpectedEsbuildVersion', `Expected esbuild version ${ESBUILD_MAJOR_VERSION}.x but got ${Bundling.esbuildInstallation.version}`, scope);
332343
}
333344

334-
const localCommand = createLocalCommand(outputDir, Bundling.esbuildInstallation, Bundling.tscInstallation);
335-
336-
exec(
337-
osPlatform === 'win32' ? 'cmd' : 'bash',
338-
[
339-
osPlatform === 'win32' ? '/c' : '-c',
340-
localCommand,
341-
],
342-
{
343-
env: { ...process.env, ...environment },
344-
stdio: [ // show output
345-
'ignore', // ignore stdio
346-
process.stderr, // redirect stdout to stderr
347-
'inherit', // inherit stderr
348-
],
349-
cwd,
350-
windowsVerbatimArguments: osPlatform === 'win32',
351-
});
345+
const execOptions = {
346+
env: { ...process.env, ...environment },
347+
stdio: [
348+
'ignore', // ignore stdio
349+
process.stderr, // redirect stdout to stderr
350+
'inherit', // inherit stderr
351+
] as ['ignore', NodeJS.WriteStream, 'inherit'],
352+
cwd,
353+
};
354+
355+
const steps = createSteps(outputDir, Bundling.esbuildInstallation, Bundling.tscInstallation);
356+
for (const step of steps) {
357+
switch (step.type) {
358+
case 'shell':
359+
for (const cmd of step.commands) {
360+
exec(
361+
osPlatform === 'win32' ? (process.env.COMSPEC ?? 'cmd') : 'bash',
362+
[osPlatform === 'win32' ? '/c' : '-c', cmd],
363+
{ ...execOptions, windowsVerbatimArguments: osPlatform === 'win32' },
364+
);
365+
}
366+
break;
367+
case 'spawn':
368+
exec(step.command[0], step.command.slice(1), {
369+
...execOptions,
370+
cwd: step.cwd ?? cwd,
371+
});
372+
break;
373+
case 'callback':
374+
try {
375+
step.operation();
376+
} catch (err) {
377+
throw new ValidationError('LocalBundlingFileOperationFailed', `Local bundling file operation failed: ${err instanceof Error ? err.message : String(err)}`, scope);
378+
}
379+
break;
380+
}
381+
}
352382

353383
return true;
354384
},
355385
};
356386
}
387+
388+
/**
389+
* Creates structured bundling steps for local execution via direct spawn (no shell).
390+
*/
391+
private createLocalBundlingSteps(
392+
scope: IConstruct,
393+
outputDir: string,
394+
esbuild: PackageInstallation,
395+
tsc?: PackageInstallation,
396+
): BundlingStep[] {
397+
const steps: BundlingStep[] = [];
398+
399+
let relativeEntryPath = path.join(this.projectRoot, this.relativeEntryPath);
400+
401+
// Before bundling hooks
402+
const beforeBundling = this.props.commandHooks?.beforeBundling(this.projectRoot, outputDir) ?? [];
403+
if (beforeBundling.length) {
404+
steps.push({ type: 'shell', commands: beforeBundling });
405+
}
406+
407+
// Pre-compilation with tsc
408+
if (this.props.preCompilation) {
409+
const tsconfig = this.props.tsconfig ?? findUp('tsconfig.json', path.dirname(this.props.entry));
410+
if (!tsconfig) {
411+
throw new ValidationError('CannotFindTsconfigJsonPre', 'Cannot find a `tsconfig.json` but `preCompilation` is set to `true`, please specify it via `tsconfig`', scope);
412+
}
413+
const compilerOptionsArray = getTsconfigCompilerOptionsArray(tsconfig);
414+
const tscRunner = tsc && (tsc.isLocal ? this.packageManager.runBinCommand('tsc') : ['tsc']);
415+
if (tscRunner) {
416+
steps.push({ type: 'spawn', command: [...tscRunner, relativeEntryPath, ...compilerOptionsArray] });
417+
}
418+
relativeEntryPath = relativeEntryPath.replace(/\.ts(x?)$/, '.js$1');
419+
}
420+
421+
// Esbuild
422+
const esbuildRunner = esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : ['esbuild'];
423+
424+
const esbuildArgs: string[] = [
425+
...this.buildEsbuildArgs(scope, this.projectRoot, outputDir, (...args: string[]) => path.join(...args)),
426+
...this.props.esbuildArgs ? toCliArgsArray(this.props.esbuildArgs) : [],
427+
];
428+
429+
steps.push({ type: 'spawn', command: [...esbuildRunner, '--bundle', relativeEntryPath, ...esbuildArgs] });
430+
431+
// Node modules installation
432+
if (this.props.nodeModules) {
433+
const pkgPath = findUp('package.json', path.dirname(this.props.entry));
434+
if (!pkgPath) {
435+
throw new ValidationError('CannotFindPackageJsonProject', 'Cannot find a `package.json` in this project. Using `nodeModules` requires a `package.json`.', scope);
436+
}
437+
438+
// Before install hooks
439+
const beforeInstall = this.props.commandHooks?.beforeInstall(this.projectRoot, outputDir) ?? [];
440+
if (beforeInstall.length) {
441+
steps.push({ type: 'shell', commands: beforeInstall });
442+
}
443+
444+
const dependencies = extractDependencies(pkgPath, this.props.nodeModules);
445+
const lockFilePath = path.join(this.projectRoot, this.relativeDepsLockFilePath ?? this.packageManager.lockFile);
446+
const isPnpm = this.packageManager.lockFile === LockFile.PNPM;
447+
const isBun = this.packageManager.lockFile === LockFile.BUN_LOCK || this.packageManager.lockFile === LockFile.BUN;
448+
449+
steps.push({
450+
type: 'callback',
451+
operation: () => {
452+
if (isPnpm) {
453+
fs.writeFileSync(path.join(outputDir, 'pnpm-workspace.yaml'), '');
454+
}
455+
fs.writeFileSync(path.join(outputDir, 'package.json'), JSON.stringify({ dependencies }));
456+
fs.copyFileSync(lockFilePath, path.join(outputDir, this.packageManager.lockFile));
457+
},
458+
});
459+
460+
steps.push({ type: 'spawn', command: [...this.packageManager.installCommand], cwd: outputDir });
461+
462+
if (isPnpm || isBun) {
463+
steps.push({
464+
type: 'callback',
465+
operation: () => {
466+
if (isPnpm) {
467+
const modulesYaml = path.join(outputDir, 'node_modules', '.modules.yaml');
468+
if (fs.existsSync(modulesYaml)) {
469+
fs.rmSync(modulesYaml, { force: true });
470+
}
471+
}
472+
if (isBun) {
473+
const cacheDir = path.join(outputDir, 'node_modules', '.cache');
474+
if (fs.existsSync(cacheDir)) {
475+
fs.rmSync(cacheDir, { recursive: true, force: true });
476+
}
477+
}
478+
},
479+
});
480+
}
481+
}
482+
483+
// After bundling hooks
484+
const afterBundling = this.props.commandHooks?.afterBundling(this.projectRoot, outputDir) ?? [];
485+
if (afterBundling.length) {
486+
steps.push({ type: 'shell', commands: afterBundling });
487+
}
488+
489+
return steps;
490+
}
357491
}
358492

493+
/**
494+
* A single step in the local bundling process.
495+
*/
496+
type BundlingStep =
497+
| { type: 'shell'; commands: string[] }
498+
| { type: 'spawn'; command: string[]; cwd?: string }
499+
| { type: 'callback'; operation: () => void };
500+
359501
interface BundlingCommandOptions {
360502
readonly inputDir: string;
361503
readonly outputDir: string;
@@ -416,6 +558,24 @@ class OsCommand {
416558
}
417559
}
418560

561+
/**
562+
* Converts a clean argv array into a single POSIX shell command string.
563+
* Each argument is escaped if it contains characters that have special
564+
* meaning in a shell. Safe characters (alphanumeric plus a few punctuation
565+
* marks commonly found in CLI flags) are left unquoted for readability.
566+
*/
567+
function preparePosixShellCommand(argv: string[]): string {
568+
return argv.map(posixShellEscape).join(' ');
569+
}
570+
571+
/**
572+
* Escapes a single argument for safe inclusion in a POSIX shell command.
573+
* Every argument is single-quoted unconditionally (like Python's shlex.quote).
574+
*/
575+
function posixShellEscape(arg: string): string {
576+
return "'" + arg.replace(/'/g, "'\\''") + "'";
577+
}
578+
419579
/**
420580
* Chain commands
421581
*/
@@ -450,21 +610,24 @@ function toTarget(scope: IConstruct, runtime: Runtime): string {
450610
return `node${match[1]}`;
451611
}
452612

453-
function toCliArgs(esbuildArgs: { [key: string]: string | boolean }): string {
454-
const args = new Array<string>();
613+
/**
614+
* Converts esbuild args to an array of CLI arguments for direct spawn (no shell quoting).
615+
*/
616+
function toCliArgsArray(esbuildArgs: { [key: string]: string | boolean }): string[] {
617+
const args: string[] = [];
455618
const reSpecifiedKeys = ['--alias', '--drop', '--pure', '--log-override', '--out-extension'];
456619

457620
for (const [key, value] of Object.entries(esbuildArgs)) {
458621
if (value === true || value === '') {
459622
args.push(key);
460623
} else if (reSpecifiedKeys.includes(key)) {
461-
args.push(`${key}:"${value}"`);
624+
args.push(`${key}:${value}`);
462625
} else if (value) {
463-
args.push(`${key}="${value}"`);
626+
args.push(`${key}=${value}`);
464627
}
465628
}
466629

467-
return args.join(' ');
630+
return args;
468631
}
469632

470633
/**

packages/aws-cdk-lib/aws-lambda-nodejs/lib/package-manager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,13 @@ export class PackageManager {
7878
this.argsSeparator = props.argsSeparator;
7979
}
8080

81-
public runBinCommand(bin: string): string {
81+
public runBinCommand(bin: string): string[] {
8282
const [runCommand, ...runArgs] = this.runCommand;
8383
return [
8484
os.platform() === 'win32' ? `${runCommand}.cmd` : runCommand,
8585
...runArgs,
8686
...(this.argsSeparator ? [this.argsSeparator] : []),
8787
bin,
88-
].join(' ');
88+
];
8989
}
9090
}

0 commit comments

Comments
 (0)