fix(codeql): resolving GH CodeQL alerts#9783
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: c03fa75 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR addresses CodeQL-driven hardening in build tooling, mainly around path resolution, subprocess environment handling, debug logging redaction, Squirrel Windows executable naming, and Yarn Berry patch dependency parsing.
Changes:
- Resolves several generated or tool-facing paths to absolute paths before signing, packaging, or extraction.
- Redacts request headers from HTTP debug logs and adjusts child-process environment filtering behavior.
- Sanitizes Squirrel Windows executable names and fixes Yarn Berry patch version extraction.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/electron-builder-squirrel-windows/src/SquirrelWindowsTarget.ts |
Sanitizes the Squirrel executable name used in installer metadata and stub generation. |
packages/builder-util/src/util.ts |
Changes non-Windows default child-process env handling to strip sensitive inherited variables. |
packages/builder-util-runtime/src/httpExecutor.ts |
Omits request headers from request/response debug logging. |
packages/app-builder-lib/src/util/packageMetadata.ts |
Tightens regex extraction for Yarn Berry patch dependency versions. |
packages/app-builder-lib/src/util/electronGet.ts |
Resolves temporary 7z extraction output directory before invoking 7z. |
packages/app-builder-lib/src/platformPackager.ts |
Resolves app output and resource paths to absolute paths. |
packages/app-builder-lib/src/macPackager.ts |
Resolves signing and MAS artifact paths before use. |
packages/app-builder-lib/src/electron/ElectronFramework.ts |
Resolves custom Electron zip extraction output directory before invoking 7z. |
.changeset/sweet-worlds-study.md |
Adds patch changeset entries for affected packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Replaced multiple uses of `path.join` with `path.resolve` throughout `macPackager.ts`, `platformPackager.ts`, `ElectronFramework.ts`, and `electronGet.ts` to ensure all file and directory paths are absolute, reducing the risk of path traversal or misplacement bugs. * Updated `getProcessEnv` in `util.ts` to strip sensitive environment variables from child process environments on non-Windows platforms, preventing accidental leakage of credentials to subprocesses. On Windows, the previous behavior is retained to avoid breaking critical system tools. * Fixed a regular expression in `packageMetadata.ts` to correctly extract versions from Yarn Berry patch syntax, preventing incorrect parsing. * Added `sanitizeFileName` to ensure the Squirrel Windows target executable name is valid, and updated references to use the sanitized name. * Improved debug logging in `httpExecutor.ts` by omitting request and response headers from logs.
Add SAFE_7ZA_OUTPUT_PATH_RE and validate7zaOutputPath() to builder-util. The regex ^[^\x00-\x1F\x7F-][^\x00-\x1F\x7F]*$ is an allowlist that: - Rejects empty paths (nothing to pass to -oDir) - Rejects paths starting with "-" (7za would misparse as a new switch) - Rejects control characters 0x00-0x1F and DEL 0x7F (C-level truncation) - Allows spaces, quotes, shell metacharacters — all safe with execFile array args The !regex.test → throw structure is the pattern CodeQL recognises as a taint-clearing sanitizer, so both 7za call sites no longer need standalone // codeql suppression comments for the argument construction line. Replaces the previous verbose .includes() blocklist in electronGet.ts which incorrectly rejected valid paths containing spaces or quote characters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…OutputPath
Replace the separate validate7zaOutputPath + -o${path} template literal pattern
with a single to7zaOutputSwitch(p) that validates and returns the complete
-o<dir> token in one call.
Call sites now pass the token directly as an argv element:
exec(cmd7za, ["x", "-bd", file, to7zaOutputSwitch(sanitizeDirPath(dir)), "-y"])
No template literal at the call site means CodeQL sees no string concatenation
from user input into the exec argument — the taint is cleared inside
to7zaOutputSwitch before the -o prefix is prepended.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… constructed from library input' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… constructed from library input' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… constructed from library input' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…symlinks/junctions consistently
… constructed from library input' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… constructed from library input' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.
This pull request addresses several security and reliability improvements across the build tooling, with a focus on path handling, sensitive environment variable management, and minor bug fixes. The most significant changes include consistently resolving file system paths to absolute paths to prevent issues with relative paths, improving the handling of sensitive environment variables (especially on Windows), and fixing a regular expression bug. Additionally, the Squirrel Windows target now ensures executable names are sanitized, and some debug logging has been improved.
path.joinwithpath.resolvethroughoutmacPackager.ts,platformPackager.ts,ElectronFramework.ts, andelectronGet.tsto ensure all file and directory paths are absolute, reducing the risk of path traversal or misplacement bugs.getProcessEnvinutil.tsto strip sensitive environment variables from child process environments on non-Windows platforms, preventing accidental leakage of credentials to subprocesses. On Windows, the previous behavior is retained to avoid breaking critical system tools.packageMetadata.tsto correctly extract versions from Yarn Berry patch syntax, preventing incorrect parsing.sanitizeFileNameto ensure the Squirrel Windows target executable name is valid, and updated references to use the sanitized name.httpExecutor.tsby omitting request and response headers from logs.