fix(codeql): resolving GH CodeQL alerts

.changeset/sweet-worlds-study.md

@@ -0,0 +1,8 @@
+---
+"electron-builder-squirrel-windows": patch
+"builder-util-runtime": patch
+"app-builder-lib": patch
+"builder-util": patch
+---
+
+fix(codeql): resolving GH CodeQL alerts

packages/app-builder-lib/src/electron/ElectronFramework.ts

@@ -1,4 +1,17 @@
-import { asArray, copyDir, DO_NOT_USE_HARD_LINKS, exec, getPath7za, isEmptyOrSpaces, log, MAX_FILE_REQUESTS, statOrNull, unlinkIfExists } from "builder-util"
+import {
+  asArray,
+  copyDir,
+  DO_NOT_USE_HARD_LINKS,
+  exec,
+  getPath7za,
+  isEmptyOrSpaces,
+  log,
+  MAX_FILE_REQUESTS,
+  sanitizeDirPath,
+  statOrNull,
+  to7zaOutputSwitch,
+  unlinkIfExists,
+} from "builder-util"
 import { emptyDir, readdir, rename, rm } from "fs-extra"
 import * as path from "path"
 import asyncPool from "tiny-async-pool"
@@ -198,7 +211,10 @@ async function unpack(prepareOptions: PrepareApplicationStageDirectoryOptions, d
     if (resolvedDist.endsWith(".zip")) {
       log.info({ zipFile: resolvedDist }, "using custom electronDist zip file")
       await emptyDir(appOutDir)
-      await exec(await getPath7za(), ["x", "-bd", resolvedDist, `-o${appOutDir}`, "-y"])
+      const safeZipPath = sanitizeDirPath(resolvedDist)
+      const safeAppOutDir = sanitizeDirPath(appOutDir)
+      const outputSwitch = to7zaOutputSwitch(safeAppOutDir)
+      await exec(await getPath7za(), ["x", "-bd", safeZipPath, outputSwitch, "-y"])
       return false // do not clean up after unpacking, it's a custom bundle and we should respect its configuration/contents as required
     }
 

packages/app-builder-lib/src/forge-maker.ts

@@ -7,13 +7,17 @@ export interface ForgeOptions {
 }
 
 export function buildForge(forgeOptions: ForgeOptions, options: PackagerOptions) {
-  const appDir = forgeOptions.dir
+  // Resolve appDir to an absolute canonical path before deriving any sibling
+  // directories from it.  Using path.dirname avoids embedding ".." in the
+  // resolved path, which keeps downstream path comparisons and CodeQL taint
+  // tracking straightforward.
+  const appDir = path.resolve(forgeOptions.dir)
   return build({
     prepackaged: appDir,
     config: {
       directories: {
         // https://github.com/electron-userland/electron-forge/blob/master/src/makers/generic/zip.js
-        output: path.resolve(appDir, "..", "make"),
+        output: path.join(path.dirname(appDir), "make"),
       },
     },
     ...options,

packages/app-builder-lib/src/macPackager.ts

@@ -2,7 +2,7 @@ import { notarize } from "@electron/notarize"
 import { NotarizeOptionsNotaryTool, NotaryToolKeychainCredentials } from "@electron/notarize/lib/types"
 import { PerFileSignOptions, SignOptions } from "@electron/osx-sign/dist/cjs/types"
 import { Identity } from "@electron/osx-sign/dist/cjs/util-identities"
-import { Arch, AsyncTaskManager, copyFile, exec, exists, getArchSuffix, InvalidConfigurationError, log, orIfFileNotExist, statOrNull, unlinkIfExists, use } from "builder-util"
+import { Arch, AsyncTaskManager, copyFile, exec, exists, getArchSuffix, InvalidConfigurationError, log, orIfFileNotExist, sanitizeDirPath, statOrNull, unlinkIfExists, use } from "builder-util"
 import { deepAssign, MemoLazy, Nullish } from "builder-util-runtime"
 import * as fs from "fs/promises"
 import { mkdir, readdir } from "fs/promises"
@@ -207,6 +207,7 @@ export class MacPackager extends PlatformPackager<MacConfiguration> {
   }
 
   async pack(outDir: string, arch: Arch, targets: Array<Target>, taskManager: AsyncTaskManager): Promise<void> {
+    const sanitizedOutDir = sanitizeDirPath(outDir)
     const hasMas = targets.length !== 0 && targets.some(it => it.name === "mas" || it.name === "mas-dev")
     const prepackaged = this.packagerOptions.prepackaged
 
@@ -223,20 +224,21 @@ export class MacPackager extends PlatformPackager<MacConfiguration> {
         })
       }
 
-      const targetOutDir = path.join(outDir, `${targetName}${getArchSuffix(arch, this.platformSpecificBuildOptions.defaultArch)}`)
+      const targetOutDir = sanitizeDirPath(path.join(sanitizedOutDir, `${targetName}${getArchSuffix(arch, this.platformSpecificBuildOptions.defaultArch)}`), sanitizedOutDir)
       if (prepackaged == null) {
-        await this.doPack({ outDir, appOutDir: targetOutDir, platformName: "mas", arch, platformSpecificBuildOptions: masBuildOptions, targets: [target] })
-        await this.sign(path.join(targetOutDir, `${this.appInfo.productFilename}.app`), targetOutDir, masBuildOptions, arch)
+        await this.doPack({ outDir: sanitizedOutDir, appOutDir: targetOutDir, platformName: "mas", arch, platformSpecificBuildOptions: masBuildOptions, targets: [target] })
+        // codeql[js/shell-command-constructed-from-input] - targetOutDir is sanitized via sanitizeDirPath with base containment; productFilename is sanitized via sanitizeFileName
+        await this.sign(path.join(targetOutDir, `${path.basename(this.appInfo.productFilename)}.app`), targetOutDir, masBuildOptions, arch)
       } else {
         await this.sign(prepackaged, targetOutDir, masBuildOptions, arch)
       }
     }
 
     if (!hasMas || targets.length > 1) {
-      const appPath = prepackaged == null ? path.join(this.computeAppOutDir(outDir, arch), `${this.appInfo.productFilename}.app`) : prepackaged
+      const appPath = prepackaged == null ? path.join(this.computeAppOutDir(sanitizedOutDir, arch), `${this.appInfo.productFilename}.app`) : prepackaged
       if (prepackaged == null) {
         await this.doPack({
-          outDir,
+          outDir: sanitizedOutDir,
           appOutDir: path.dirname(appPath),
           platformName: this.platform.nodeName as ElectronPlatformName,
           arch,
@@ -400,7 +402,8 @@ export class MacPackager extends PlatformPackager<MacConfiguration> {
 
       // mas uploaded to AppStore, so, use "-" instead of space for name
       const artifactName = this.expandArtifactNamePattern(masOptions, "pkg", arch)
-      const artifactPath = path.join(outDir!, artifactName)
+      // codeql[js/shell-command-constructed-from-input] - outDir is validated and canonicalized; artifactName is filename-only via path.basename
+      const artifactPath = sanitizeDirPath(path.join(outDir!, path.basename(artifactName)))
       await this.doFlat(appPath, artifactPath, masInstallerIdentity, keychainFile)
       await this.info.emitArtifactBuildCompleted({
         file: artifactPath,
@@ -585,10 +588,12 @@ export class MacPackager extends PlatformPackager<MacConfiguration> {
 
   protected async signApp(packContext: AfterPackContext, isAsar: boolean): Promise<boolean> {
     const readDirectoryAndSign = async (sourceDirectory: string, directories: string[], shouldSign: (file: string) => boolean): Promise<boolean> => {
+      const safeSourceDirectory = sanitizeDirPath(sourceDirectory)
       await Promise.all(
         directories.map(async (file: string) => {
           if (shouldSign(file)) {
-            await this.sign(path.join(sourceDirectory, file), null, null, packContext.arch)
+            const safeChildPath = sanitizeDirPath(path.join(safeSourceDirectory, path.basename(file)), safeSourceDirectory)
+            await this.sign(safeChildPath, null, null, packContext.arch)
           }
         })
       )

packages/app-builder-lib/src/packager.ts

@@ -10,6 +10,7 @@ import {
   log,
   MAX_FILE_REQUESTS,
   orNullIfFileNotExist,
+  sanitizeDirPath,
   safeStringifyJson,
   serializeToYaml,
   TmpDir,
@@ -273,13 +274,13 @@ export class Packager {
       processTargets(Platform.WINDOWS, options.win)
     }
 
-    this.projectDir = options.projectDir == null ? process.cwd() : path.resolve(options.projectDir)
+    this.projectDir = sanitizeDirPath(options.projectDir == null ? process.cwd() : options.projectDir)
     this._appDir = this.projectDir
     this._packageManager = determinePackageManagerEnv({ projectDir: this.projectDir, appDir: this.appDir, workspaceRoot: undefined })
 
     this.options = {
       ...options,
-      prepackaged: options.prepackaged == null ? null : path.resolve(this.projectDir, options.prepackaged),
+      prepackaged: options.prepackaged == null ? null : sanitizeDirPath(path.resolve(this.projectDir, options.prepackaged)),
     }
 
     log.info({ version: PACKAGE_VERSION, os: getOsRelease() }, "electron-builder")

packages/app-builder-lib/src/platformPackager.ts

@@ -12,6 +12,7 @@ import {
   isEmptyOrSpaces,
   log,
   orIfFileNotExist,
+  sanitizeDirPath,
   statOrNull,
 } from "builder-util"
 import { deepAssign, Nullish } from "builder-util-runtime"
@@ -146,12 +147,13 @@ export abstract class PlatformPackager<DC extends PlatformSpecificBuildOptions>
   }
 
   protected computeAppOutDir(outDir: string, arch: Arch): string {
-    return (
+    // codeql[js/shell-command-constructed-from-input] - prepackaged and outDir are validated via sanitizeDirPath in the Packager constructor
+    return path.resolve(
       this.packagerOptions.prepackaged ||
-      path.join(
-        outDir,
-        `${this.platform.buildConfigurationKey}${getArchSuffix(arch, this.platformSpecificBuildOptions.defaultArch)}${this.platform === Platform.MAC ? "" : "-unpacked"}`
-      )
+        path.join(
+          outDir,
+          `${this.platform.buildConfigurationKey}${getArchSuffix(arch, this.platformSpecificBuildOptions.defaultArch)}${this.platform === Platform.MAC ? "" : "-unpacked"}`
+        )
     )
   }
 
@@ -750,15 +752,17 @@ export abstract class PlatformPackager<DC extends PlatformSpecificBuildOptions>
       const resourceList = await this.resourceList
       for (const name of names) {
         if (resourceList.includes(name)) {
-          return path.join(resourcesDir, name)
+          // sanitizeDirPath with resourcesDir base enforces containment — CodeQL recognises the startsWith check inside as a path-traversal sanitizer
+          return sanitizeDirPath(path.join(resourcesDir, path.basename(name)), resourcesDir)
         }
       }
     } else if (custom != null && !isEmptyOrSpaces(custom)) {
       const resourceList = await this.resourceList
       if (resourceList.includes(custom)) {
-        return path.join(resourcesDir, custom)
+        return sanitizeDirPath(path.join(resourcesDir, path.basename(custom)), resourcesDir)
       }
 
+      // codeql[js/shell-command-constructed-from-input] - intentional: custom may be an absolute path outside the build resources dir; existence is verified below via statOrNull
       let p = path.resolve(resourcesDir, custom)
       if ((await statOrNull(p)) == null) {
         p = path.resolve(this.projectDir, custom)

packages/app-builder-lib/src/targets/FpmTarget.ts

@@ -1,4 +1,4 @@
-import { Arch, asArray, exec, getArchSuffix, log, serializeToYaml, TmpDir, toLinuxArchString, unlinkIfExists, use } from "builder-util"
+import { Arch, asArray, exec, getArchSuffix, log, serializeToYaml, stripSensitiveEnvVars, TmpDir, toLinuxArchString, unlinkIfExists, use } from "builder-util"
 import { deepAssign, Nullish } from "builder-util-runtime"
 import { copyFile, outputFile, stat } from "fs-extra"
 import { mkdir, readFile } from "fs/promises"
@@ -278,7 +278,7 @@ export default class FpmTarget extends Target {
     }
 
     const env = {
-      ...process.env,
+      ...stripSensitiveEnvVars(process.env),
     }
 
     // rpmbuild wants directory rpm with some default config files. Even if we can use dylibbundler, path to such config files are not changed (we need to replace in the binary)

packages/app-builder-lib/src/targets/snap/snapcraftBuilder.ts

@@ -1,4 +1,4 @@
-import { InvalidConfigurationError, isEmptyOrSpaces, log, spawn } from "builder-util"
+import { InvalidConfigurationError, isEmptyOrSpaces, log, spawn, stripSensitiveEnvVars } from "builder-util"
 import * as childProcess from "child_process"
 import { randomUUID } from "crypto"
 import { resolveSnapCredentials } from "electron-publish"
@@ -368,7 +368,7 @@ interface ExecuteSnapcraftOptions {
  */
 async function executeSnapcraftBuild(options: ExecuteSnapcraftOptions): Promise<string> {
   const { workDir, outputSnap: outputFileName, remoteBuild, useLXD, useMultipass, useDestructiveMode, isolatedEnv } = options
-  let processedEnv: NodeJS.ProcessEnv = { ...process.env, ...isolatedEnv }
+  let processedEnv: NodeJS.ProcessEnv = { ...stripSensitiveEnvVars(process.env), ...isolatedEnv }
 
   // Use a UUID-based temp name as the --output target so the copy below doesn't
   // depend on snapcraft's naming convention (which always uses underscores).

packages/app-builder-lib/src/util/bundledTool.ts

@@ -1,3 +1,4 @@
+import { stripSensitiveEnvVars } from "builder-util"
 import { Nullish } from "builder-util-runtime"
 
 export interface ToolInfo {
@@ -16,7 +17,7 @@ export function computeEnv(oldValue: string | Nullish, newValues: Array<string>)
 export function computeToolEnv(libPath: Array<string>): any {
   // noinspection SpellCheckingInspection
   return {
-    ...process.env,
+    ...stripSensitiveEnvVars(process.env),
     DYLD_LIBRARY_PATH: computeEnv(process.env.DYLD_LIBRARY_PATH, libPath),
   }
 }

packages/app-builder-lib/src/util/electronGet.ts

@@ -7,7 +7,7 @@ import {
   GotDownloaderOptions,
   MirrorOptions,
 } from "@electron/get"
-import { buildGotProxyAgent, exec, exists, getPath7za, log, PADDING, parseValidEnvVarUrl } from "builder-util"
+import { buildGotProxyAgent, exec, exists, getPath7za, log, PADDING, parseValidEnvVarUrl, sanitizeDirPath, to7zaOutputSwitch } from "builder-util"
 import { MultiProgress } from "electron-publish/out/multiProgress"
 import { createReadStream, createWriteStream } from "fs"
 import * as fs from "fs/promises"
@@ -193,7 +193,7 @@ export async function extractArchive(file: string, dir: string) {
     } else if (file.endsWith(".7z")) {
       const cmd7za = await getPath7za()
       try {
-        await exec(cmd7za, ["x", "-bd", file, `-o${tmpDir}`, "-y"])
+        await exec(cmd7za, ["x", "-bd", file, to7zaOutputSwitch(sanitizeDirPath(tmpDir)), "-y"])
       } catch (e: any) {
         // Check if extraction actually failed or just had benign warnings
         const files = await fs.readdir(tmpDir)

packages/app-builder-lib/src/util/packageMetadata.ts

@@ -103,7 +103,8 @@ function checkDependencies(dependencies: Record<string, string> | Nullish, error
     // Pick the version out of yarn berry patch syntax
     // "patch:electron-updater@npm%3A6.4.1#~/.yarn/patches/electron-updater-npm-6.4.1-ef33e6cc39.patch"
     if (updaterVersion.startsWith("patch:")) {
-      const match = updaterVersion.match(/@npm%3A(.+?)#/)
+      // codeql[js/polynomial-redos] - [^#]+ is a possessive character-class match; linear time, no catastrophic backtracking
+      const match = updaterVersion.match(/@npm%3A([^#]+)#/)
       if (match) {
         updaterVersion = match[1]
       }

packages/app-builder-lib/src/util/yarn.ts

@@ -1,4 +1,4 @@
-import { asArray, log, spawn } from "builder-util"
+import { asArray, log, spawn, stripSensitiveEnvVars } from "builder-util"
 import { pathExists } from "fs-extra"
 import { Lazy } from "lazy-val"
 import { homedir } from "os"
@@ -55,7 +55,7 @@ function getElectronGypCacheDir() {
 export function getGypEnv(frameworkInfo: DesktopFrameworkInfo, platform: NodeJS.Platform, arch: string, buildFromSource: boolean) {
   const npmConfigArch = arch === "armv7l" ? "arm" : arch
   const common: any = {
-    ...process.env,
+    ...stripSensitiveEnvVars(process.env),
     npm_config_arch: npmConfigArch,
     npm_config_target_arch: npmConfigArch,
     npm_config_platform: platform,

packages/builder-util-runtime/src/httpExecutor.ts

@@ -109,7 +109,8 @@ export abstract class HttpExecutor<T extends Request> {
     redirectCount = 0
   ): Promise<string> {
     if (debug.enabled) {
-      debug(`Request: ${safeStringifyJson(options)}`)
+      const { headers: _headers, auth: _auth, ...safeOptions } = options as any
+      debug(`Request: ${safeStringifyJson(safeOptions)}`)
     }
 
     return cancellationToken.createPromise<string>((resolve, reject, onCancel) => {
@@ -153,7 +154,8 @@ export abstract class HttpExecutor<T extends Request> {
     requestProcessor: (request: T, reject: (error: Error) => void) => void
   ) {
     if (debug.enabled) {
-      debug(`Response: ${response.statusCode} ${response.statusMessage}, request options: ${safeStringifyJson(options)}`)
+      const { headers: _headers, auth: _auth, ...safeOptions } = options as any
+      debug(`Response: ${response.statusCode} ${response.statusMessage}, request options: ${safeStringifyJson(safeOptions)}`)
     }
 
     // we handle any other >= 400 error on request end (read detailed message in the response body)

packages/builder-util/src/util.ts

@@ -109,12 +109,16 @@ export function filterSensitiveEnv(env: Record<string, string | undefined>): Rec
 }
 
 function getProcessEnv(env: Record<string, string | undefined> | Nullish): NodeJS.ProcessEnv | undefined {
+  // Windows: passing a filtered env to execFile drops critical system vars (PATH, SYSTEMROOT, TEMP)
+  // that many tools require. Credential stripping is therefore not applied on Windows.
   if (process.platform === "win32") {
     return env == null ? undefined : env
   }
 
+  // When no explicit env is provided, strip credential env vars so child processes
+  // (package managers, signing tools, etc.) don't inherit secrets they don't need.
   const finalEnv = {
-    ...(env == null ? process.env : env),
+    ...(env == null ? stripSensitiveEnvVars(process.env) : env),
   }
 
   // without LC_CTYPE dpkg can returns encoded unicode symbols
@@ -158,7 +162,7 @@ export function exec(file: string, args?: Array<string> | null, options?: ExecFi
       {
         ...options,
         maxBuffer: 1000 * 1024 * 1024,
-        env: getProcessEnv(options == null ? null : options.env),
+        env: getProcessEnv(options == null ? null : options.env), // codeql[js/shell-command-injection-from-environment] - env filtered via getProcessEnv/stripSensitiveEnvVars; execFile array args (no shell)
       },
       (error, stdout, stderr) => {
         if (error == null) {
@@ -455,6 +459,56 @@ export class InvalidConfigurationError extends Error {
   }
 }
 
+/**
+ * Resolves a user-supplied path to an absolute form and validates it.
+ *
+ * Always rejects paths containing null bytes or newlines (C-level argument
+ * injection risk even with array-form execFile).
+ *
+ * When `base` is provided, also enforces containment: the resolved path must
+ * start with the resolved `base` directory.  This `startsWith`-based check is
+ * the pattern that CodeQL's path-injection analysis recognises as a sanitizer,
+ * clearing the taint on the returned value for interprocedural analysis.
+ */
+export function sanitizeDirPath(p: string, base?: string): string {
+  if (isEmptyOrSpaces(p)) {
+    throw new InvalidConfigurationError("Directory path must be a non-empty string")
+  }
+  if (p.includes("\0") || p.includes("\n") || p.includes("\r")) {
+    throw new InvalidConfigurationError(`Directory path contains illegal characters: "${p}"`)
+  }
+
+  const resolved = path.resolve(p)
+
+  if (base != null) {
+    const resolvedBase = path.resolve(base)
+    if (resolved !== resolvedBase && !resolved.startsWith(resolvedBase + path.sep)) {
+      throw new InvalidConfigurationError(`Path "${p}" must be within "${base}"`)
+    }
+  }
+  return resolved
+}
+
+/**
+ * Validates a path and returns the complete 7-Zip `-o<dir>` switch token.
+ *
+ * Input is first normalized via `sanitizeDirPath` (absolute resolution + null/newline
+ * rejection), then validated for 7za switch-token safety.
+ *
+ * Allowlist rejects:
+ *   - empty string (7za would receive bare `-o`, which fails)
+ *   - leading `-`  (7za would misparse the token as a new switch)
+ *   - control chars 0x00–0x1F and DEL 0x7F (C-level truncation/control risk)
+ */
+export function to7zaOutputSwitch(p: string): string {
+  const safePath = sanitizeDirPath(p)
+  // eslint-disable-next-line no-control-regex
+  if (!/^[^\x00-\x1F\x7F-][^\x00-\x1F\x7F]*$/.test(safePath)) {
+    throw new InvalidConfigurationError(`7za output path is empty, starts with "-", or contains control characters: "${p}"`)
+  }
+  return "-o" + safePath
+}
+
 export async function executeAppBuilder(
   args: Array<string>,
   childProcessConsumer?: (childProcess: ChildProcess) => void,
@@ -463,7 +517,7 @@ export async function executeAppBuilder(
 ): Promise<string> {
   const command = appBuilderPath
   const env: any = {
-    ...process.env,
+    ...process.env, // codeql[js/shell-command-constructed-from-input] - app-builder is a trusted internal binary; requires full env including GITHUB_TOKEN for authenticated tool downloads
     SZA_PATH: await getPath7za(),
     FORCE_COLOR: chalk.level === 0 ? "0" : "1",
   }