fix(build): harden build scripts against shell-injection (CodeQL)#181
Open
simongdavies wants to merge 1 commit into
Open
fix(build): harden build scripts against shell-injection (CodeQL)#181simongdavies wants to merge 1 commit into
simongdavies wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens build tooling against shell command injection (addressing CodeQL js/shell-command-injection-from-environment) by removing string-form shell invocations and switching to execFileSync with argument arrays, invoking local CLIs via process.execPath + require.resolve(...).
Changes:
- Replace the Prettier formatting step in the modules build script with
execFileSync(process.execPath, [...])to avoid shell parsing of interpolated paths/globs. - Replace the
npx esbuild ...shell string in the bash bundle build script withexecFileSync(process.execPath, [...args]), keeping aliases as an array rather than a space-joined string.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scripts/build-modules.js |
Runs Prettier via execFileSync + resolved local Prettier CLI to remove shell interpretation of interpolated paths. |
scripts/bash-bundle/build.mjs |
Runs esbuild via execFileSync + resolved local esbuild CLI with array args (no npx, no shell). |
…ction alerts
Remove the string-form execSync invocations flagged by CodeQL
js/shell-command-injection-from-environment at two build-tooling call sites:
- scripts/build-modules.js: prettier --write now runs via
execFileSync(process.execPath, [require.resolve("prettier/bin/prettier.cjs"),
"--write", join(BUILTIN_DIR, "*.js")]). Prettier expands the glob itself, so
no shell parses the interpolated path.
- scripts/bash-bundle/build.mjs: the esbuild bundle step now uses esbuild's JS
API (await build({ ... })) instead of spawning a CLI through a shell. The CLI
--alias flags are mapped to the alias option and every other flag to its
matching build option (bundle, format, platform, target, mainFields, outfile,
minify, treeShaking). This removes the child process entirely and is portable:
esbuild's CLI bin is a native binary on Linux, so it cannot be launched via
`node`.
Build tooling only; the generated bash.js bundle is byte-for-byte identical and
no other generated artifacts change. Resolves CodeQL alerts #2 and #11.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
simongdavies
force-pushed
the
simongdavies/harden-build-scripts-codeql
branch
from
d65a1fe to
abf2dca
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two CodeQL
js/shell-command-injection-from-environmentalerts by replacing string-formexecSynccalls with array-formexecFileSync(no shell) in build tooling. Resolves CodeQL alerts #2 and #11.Changes
scripts/build-modules.js:71— the Prettier format step previously interpolatedBUILTIN_DIRinto a shell string (execSync(\prettier --write "${BUILTIN_DIR}/.js"`)). It now runs viaexecFileSync(process.execPath, [require.resolve("prettier/bin/prettier.cjs"), "--write", join(BUILTIN_DIR, ".js")]). No shell parses the path; Prettier expands the glob itself.cwd: ROOT` preserved.scripts/bash-bundle/build.mjs:57— the esbuild bundle step previously concatenated args into anpx esbuild ...shell string. It now runs viaexecFileSync(process.execPath, [require.resolve("esbuild/bin/esbuild"), ...])with every flag/value as its own array element.aliasArgsis kept as an array (no longer space-joined). Same flags, aliases,--minify,--tree-shaking, and--outfile— behaviour is identical.Both call sites now invoke the local CLI through
process.execPath(Node) against the resolved package bin, so there is no shell and resolution works on Windows without relying onnpx/PATH.Scope
Build tooling only — no runtime/source changes. Only the two flagged call sites were touched (the bumped-dependency logic, turndown-stub, etc. are untouched).
Validation
npm install(prepare →build:modules) and a standalonenpm run build:modulesboth succeed on Windows, exercising both changed scripts (Prettier step + esbuild bash bundle).builtin-modules/*.js,ha-modules.d.ts, bash bundle + hashes) are unchanged byte-for-byte —git statusshows only the two scripts modified.No native binding found for win32-x64-msvcerrors (the code-validator native addon requires a clang +x86_64-hyperlight-nonecross-compile toolchain not available on this Windows host) — unrelated to this change.Commit is signed (GPG) and signed-off.