chore(lambda-nodejs): input sanitisation for bundling options by vishaalmehrishi · Pull Request #37019 · aws/aws-cdk

vishaalmehrishi · 2026-02-18T12:52:58Z

Issue # (if applicable)

Closes #.

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

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

aws-cdk-automation

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

…ling Refactored local bundling to execute esbuild via spawnSync with shell=false, passing arguments as an array instead of interpolating into a shell command string. For Docker bundling, all esbuild arguments are shell-escaped before interpolation into the bash -c command string. - Added execDirect() and shellEscapeForBundlingCommand() utilities - Extracted buildEsbuildArgs() for reuse across both bundling paths - Added runBinCommandAsArray() to PackageManager - Refactored toCliArgs() to return string[] without shell quoting - Refactored getLocalBundlingProvider() into 3 phases (hooks/tsc, esbuild, deps) - Added comprehensive tests for shell metacharacter handling

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

+ * Spawn sync with error handling.
+ *
+ * NOTE: This function is typically used with a shell interpreter (bash/cmd) as the command,
+ * which means the args are interpreted by the shell. When user input is involved,


General approach: Avoid giving the shell a command string containing unescaped, user‑influenced values. Either pass arguments directly as an array (bypassing the shell) or, if we must use bash -c / cmd /c, rigorously escape any data that can contain special characters before embedding it into the command string.

Best fix here: We already have shellEscapeForBundlingCommand(value, platform) and we already use it for the esbuild command. The remaining surface that CodeQL traces into is the OsCommand helper plus the chain combination used to build depsCommand and then localCommand. The tainted data are:

filePath, src, dest, dir path arguments in OsCommand methods.

data in OsCommand.write/writeJson (JSON of dependencies built from extractDependencies).

The resulting per-step command strings combined by chain and finally passed as localCommand to bash -c / cmd /c.

We can fix this without altering observable behavior by:

Making OsCommand build its commands using shellEscapeForBundlingCommand for both Unix and Windows paths/data instead of naive "${...}" or raw '${data}'. This prevents quotes, semicolons, &&, etc., in file paths or JSON data from breaking out of their intended position in the command.

Leaving chain as a simple join, but ensuring that every command fragment it receives is already correctly shell‑escaped by construction. Since depsCommand elements come exclusively from OsCommand and packageManager.installCommand.join(' ') (controlled by the library), tightening OsCommand is sufficient for the tainted flow CodeQL identified.

This keeps the overall structure the same: we still generate one string localCommand and pass it to bash -c/cmd /c, but now all user‑influenced values embedded in that string are safely escaped, so the “shell command built from environment values” issue is mitigated.

Concretely:

In packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts, within the OsCommand class, update all string templates that interpolate filePath, src, dest, dir, or data to first run them through shellEscapeForBundlingCommand(…, this.osPlatform) and then use those escaped values. For Unix, this replaces manual '${data}' quoting with robust escaping; for Windows, it protects paths and content when invoking via cmd /c. No other files need changes.

…ate methods Split getLocalBundlingProvider into: - executePreBundlingCommands: hooks and tsc pre-compilation (shell) - executeEsbuild: direct spawnSync without shell - executePostBundlingCommands: deps installation and hooks (shell)

…ution Extract executeShellCommands() helper that encapsulates the win32/posix shell selection, flag, windowsVerbatimArguments, and shared exec options. Used by both executePreBundlingCommands and executePostBundlingCommands.

…bundling Use a single shell session for local bundling (same as Docker) so that environment variables and shell state propagate across hooks, tsc, esbuild, and deps installation. Esbuild arguments are shell-escaped via shellEscapeForBundlingCommand() to neutralize metacharacters while preserving backward compatibility for hooks. This removes the need for execDirect(), runBinCommandAsArray(), and the multi-phase execution model. Both bundling paths now share the same createBundlingCommand() logic with shell escaping.

aws-cdk-automation requested a review from a team February 18, 2026 12:53

github-actions bot added the p2 label Feb 18, 2026

vishaalmehrishi changed the title ~~Mehrishi/input sanitisation bundling options~~ chore(aws-lambda-nodejs): input sanitisation for bundling options Feb 18, 2026

mergify bot added the contribution/core This is a PR that came from AWS. label Feb 18, 2026

aws-cdk-automation previously requested changes Feb 18, 2026

View reviewed changes

vishaalmehrishi changed the title ~~chore(aws-lambda-nodejs): input sanitisation for bundling options~~ chore(lambda-nodejs): input sanitisation for bundling options Feb 18, 2026

vishaalmehrishi force-pushed the mehrishi/input-sanitisation-bundling-options branch from eabb485 to 9b5fd2d Compare February 18, 2026 14:18

pahud mentioned this pull request Feb 18, 2026

Failed CodeBuild CI - 25 PR(s) pahud/aws-cdk#17

Open

vishaalmehrishi added 7 commits February 19, 2026 19:19

chore(aws-lambda-nodejs): input sanitisation for bundling options

cb16c12

reorganising regexes

a6da0d9

code reuse

a537bc0

updating tests

53a9f07

imports

1631431

linting

717ed09

fix

de35360

vishaalmehrishi force-pushed the mehrishi/input-sanitisation-bundling-options branch from 496ad55 to de35360 Compare February 19, 2026 13:49

vishaalmehrishi added 4 commits February 19, 2026 15:12

addressing build failures

02ff2d2

fix for integ tests

a85aabc

unit test fix

055dbfd

github-advanced-security bot found potential problems Feb 20, 2026

View reviewed changes

vishaalmehrishi added 2 commits February 20, 2026 11:01

aws-cdk-automation removed the pr/needs-further-review PR requires additional review from our team specialists due to the scope or complexity of changes. label Feb 20, 2026

@@ -431,14 +431,19 @@
               constructor(private readonly osPlatform: NodeJS.Platform) {}
               public write(filePath: string, data: string): string {
+                const escapedFilePath = shellEscapeForBundlingCommand(filePath, this.osPlatform);
+                const escapedData = shellEscapeForBundlingCommand(data, this.osPlatform);
                 if (this.osPlatform === 'win32') {
+                  // On Windows, preserve the existing semantics for empty data vs non-empty data,
+                  // but ensure the file path and data are safely escaped for cmd.
                   if (!data) { // if `data` is empty, echo a blank line, otherwise the file will contain a `^` character
-                    return `echo. > "${filePath}"`;
+                    return `echo. > ${escapedFilePath}`;
                   }
-                  return `echo ^${data}^ > "${filePath}"`;
+                  return `echo ${escapedData} > ${escapedFilePath}`;
                 }
-                return `echo '${data}' > "${filePath}"`;
+                return `echo ${escapedData} > ${escapedFilePath}`;
               }
               public writeJson(filePath: string, data: any): string {
@@ -447,32 +447,41 @@
               }
               public copy(src: string, dest: string): string {
+                const escapedSrc = shellEscapeForBundlingCommand(src, this.osPlatform);
+                const escapedDest = shellEscapeForBundlingCommand(dest, this.osPlatform);
                 if (this.osPlatform === 'win32') {
-                  return `copy "${src}" "${dest}"`;
+                  return `copy ${escapedSrc} ${escapedDest}`;
                 }
-                return `cp "${src}" "${dest}"`;
+                return `cp ${escapedSrc} ${escapedDest}`;
               }
               public changeDirectory(dir: string): string {
-                return `cd "${dir}"`;
+                const escapedDir = shellEscapeForBundlingCommand(dir, this.osPlatform);
+                return `cd ${escapedDir}`;
               }
               public remove(filePath: string, force: boolean = false): string {
+                const escapedFilePath = shellEscapeForBundlingCommand(filePath, this.osPlatform);
                 if (this.osPlatform === 'win32') {
-                  return `del "${filePath}"`;
+                  return `del ${escapedFilePath}`;
                 }
                 const opts = force ? ['-f'] : [];
-                return `rm ${opts.join(' ')} "${filePath}"`;
+                const escapedOpts = opts.join(' ');
+                return `rm ${escapedOpts} ${escapedFilePath}`.trim();
               }
               public removeDir(dir: string): string {
+                const escapedDir = shellEscapeForBundlingCommand(dir, this.osPlatform);
                 if (this.osPlatform === 'win32') {
-                  return `rmdir /s /q "${dir}"`;
+                  return `rmdir /s /q ${escapedDir}`;
                 }
-                return `rm -rf "${dir}"`;
+                return `rm -rf ${escapedDir}`;
               }
             }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

chore(lambda-nodejs): input sanitisation for bundling options#37019

chore(lambda-nodejs): input sanitisation for bundling options#37019
vishaalmehrishi wants to merge 14 commits intomainfrom
mehrishi/input-sanitisation-bundling-options

vishaalmehrishi commented Feb 18, 2026

Uh oh!

aws-cdk-automation left a comment •

edited

Loading

Uh oh!

Check warning

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

vishaalmehrishi commented Feb 18, 2026

Issue # (if applicable)

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check warning

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aws-cdk-automation left a comment •

edited

Loading