Skip to content

Maintenance: harden layer bundler to avoid shell command injection from env (CodeQL alert 250) #5579

Description

@svozza

Summary

The Lambda layer bundler (layers/src/layer-publisher-stack.ts, tryBundle) builds shell command strings that interpolate absolute paths derived from __dirname (tmpBuildDir, tmpBuildPath, outputDir, esmTracerPath) and passes them to execSync. CodeQL flags this as js/shell-command-injection-from-environment (medium; code scanning alert 250): "Building a shell command string with values from the enclosing environment may cause subtle bugs or vulnerabilities." The alert is pre-existing on main (it re-surfaced on #5577, which touches this file, but the CodeQL check there still passes since it's not newly introduced).

Why is this needed?

Beyond clearing the alert:

  • Robustness: the interpolated paths are unquoted in the shell (rm -rf ${tmpBuildDir}, mv …*.tgz ${tmpBuildDir}, cp -R ${tmpBuildPath}${sep}* ${outputDir}). A checkout path containing a space or shell metacharacter breaks the build.
  • Latent bug: two filesToRemove entries have a trailing space — 'node_modules/@smithy/**/README.md ' and 'node_modules/@aws-sdk/**/README.md ' — so those globs never match and the READMEs are never removed.

Real-world security risk is low (__dirname comes from a trusted build/CI context, not user input), but the shell usage is avoidable.

Which area does this relate to?

Automation, Other

Solution

Replace the shell filesystem operations with Node fs APIs so no shell is involved:

  • Phase 1: rmSync(tmpBuildDir, { recursive: true, force: true }) + mkdirSync(tmpBuildDir, { recursive: true }).
  • Phase 2 mv …*.tgz: renameSync the already-resolved packed tarball instead of shell mv.
  • Phase 4 rm -rf <globs>: expand the ** patterns with fs.globSync (stable on Node 22+, our runtime floor) and rmSync each match; fix the two trailing-space README.md entries while here.
  • Phase 5 ESM-tracer patch: read the file with readFileSync, prepend the existing createRequire shim (unchanged — it's a runtime workaround for the non-ESM AWS X-Ray SDK, not related to our build tooling), and writeFileSync it back, instead of echo "…$(cat …)" > ….
  • Phase 6 copy: cpSync(tmpBuildPath, outputDir, { recursive: true }) instead of cp -R tmp/* out.

Keep the npm build/pack/install subprocess calls. The build scripts are ESM-only and stay that way. Verify with the layer unit test (buildFromLocal synth) and the layer E2E.

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

Metadata

Metadata

Assignees

No one assigned

    Labels

    internalPRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)triageThis item has not been triaged by a maintainer, please wait

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions