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.

Activity

  1. added
    triageThis item has not been triaged by a maintainer, please wait
    internalPRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
    on Aug 24, 2026
  2. changed the title [-]Maintenance: harden layer bundler to avoid shell command injection from env (CodeQL #250)[/-] [+]Maintenance: harden layer bundler to avoid shell command injection from env (CodeQL alert 250)[/+] on Aug 24, 2026
  3. self-assigned this
    on Aug 31, 2026
  4. removed
    triageThis item has not been triaged by a maintainer, please wait
    on Aug 31, 2026
  5. powertools-for-aws-oss-automation commented on Aug 31, 2026

    @powertools-for-aws-oss-automation

    Warning

    This issue is now closed. Please be mindful that future comments are hard for our team to see.
    If you need more assistance, please either reopen the issue, or open a new issue referencing this one.
    If you wish to keep having a conversation with other community members under this issue feel free to do so.

  6. removed
    internalPRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
    on Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions