From 38ece930c8b9abd76b10e2dcef8343eb7f1bd528 Mon Sep 17 00:00:00 2001 From: Orca Worker Date: Tue, 1 Sep 2026 18:15:43 -0700 Subject: [PATCH] fix(release): stop shipping an unsigned elevate.exe on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release cut swaps the SignPath-signed elevate.exe into the electron-builder toolset cache so the NSIS rebuild's CopyElevateHelper re-copy becomes a no-op. It searched `\nsis`, a directory no app-builder-lib layout creates, and `-ErrorAction SilentlyContinue` plus `exit 0` turned that miss into a green step — v1.4.193 and v1.4.194 shipped an unsigned UAC elevation helper. Move the lookup into a script that covers the real layouts (`nsis-3.0.4.1/…`, `nsis@/…`, `ELECTRON_BUILDER_NSIS_DIR`), asks app-builder-lib for the authoritative path, and exits non-zero with an ::error:: annotation when it finds nothing. The step stays continue-on-error so the inner-signing chain remains fail-open. --- .github/workflows/release-cut.yml | 31 ++- .../scripts/replace-cached-nsis-elevate.mjs | 199 +++++++++++++++ .../replace-cached-nsis-elevate.test.mjs | 233 ++++++++++++++++++ 3 files changed, 452 insertions(+), 11 deletions(-) create mode 100644 config/scripts/replace-cached-nsis-elevate.mjs create mode 100644 config/scripts/replace-cached-nsis-elevate.test.mjs diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 9bc7d415d7b..f25be00ba8f 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -1650,9 +1650,12 @@ jobs: # no-op. Known quirk: the cache persists across releases via actions/cache, # so later runs may see elevate.exe as already signed and skip staging it — # that is fine (the signature is timestamped) and the evidence gate checks - # elevate.exe in the shipped installer unconditionally. If this ever causes - # trouble, delete this step; the only effect is elevate.exe shipping - # unsigned again, which the evidence gate will flag. + # elevate.exe in the shipped installer unconditionally. + # + # The cache lookup lives in a script because the inline path this step used + # (`\nsis`) matches no app-builder-lib layout, and `SilentlyContinue` + # plus `exit 0` turned that miss into a green step — v1.4.193 and v1.4.194 + # shipped an unsigned elevate.exe that way. A miss now fails the step. - name: Replace cached elevate.exe with the signed copy id: sign-elevate-cache if: matrix.platform == 'win' && github.run_attempt == 1 && steps.restore-signed-inner.outcome == 'success' @@ -1664,20 +1667,26 @@ jobs: Write-Host '::warning::No elevate.exe in win-unpacked resources; nothing to protect from the rebuild clobber.' exit 0 } + # Why this guard stays: windows-signing-rehearsal.yml shares the + # electron-builder-win- cache key with this workflow, so a + # test-certificate elevate.exe must never be staged into a release cache. $signature = Get-AuthenticodeSignature -FilePath $signed $subject = if ($null -eq $signature.SignerCertificate) { '' } else { $signature.SignerCertificate.Subject } if ($signature.Status -ne 'Valid' -or $subject -notlike '*CN=SignPath Foundation*') { Write-Host "::warning::win-unpacked elevate.exe is not SignPath-signed ($($signature.Status), $subject); skipping cache swap." exit 0 } - $cached = @(Get-ChildItem "$env:LOCALAPPDATA\electron-builder\Cache\nsis" -Recurse -Filter elevate.exe -ErrorAction SilentlyContinue) - if ($cached.Count -eq 0) { - Write-Host '::warning::No cached elevate.exe found (electron-builder cache layout changed?); the rebuild will pack the unsigned copy and the evidence gate will flag it.' - exit 0 - } - foreach ($file in $cached) { - Copy-Item -Path $signed -Destination $file.FullName -Force - Write-Host "Replaced $($file.FullName) with the SignPath-signed copy." + node config/scripts/replace-cached-nsis-elevate.mjs $signed + if ($LASTEXITCODE -ne 0) { + $message = 'Cached elevate.exe swap found nothing to replace; the rebuilt installer ships an unsigned UAC elevation helper (issue #7785).' + if ($env:GITHUB_STEP_SUMMARY) { + try { + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value "**Windows elevate.exe cache swap:** FAILED — $message" -ErrorAction Stop + } catch { + Write-Host "::warning::Could not write the elevate.exe swap verdict to the job summary: $_" + } + } + throw $message } - name: Rebuild NSIS installer from signed unpacked app diff --git a/config/scripts/replace-cached-nsis-elevate.mjs b/config/scripts/replace-cached-nsis-elevate.mjs new file mode 100644 index 00000000000..18504a73b4d --- /dev/null +++ b/config/scripts/replace-cached-nsis-elevate.mjs @@ -0,0 +1,199 @@ +#!/usr/bin/env node + +// Why: electron-builder re-runs `CopyElevateHelper.copy` on every NSIS pack, so the +// release rebuild overwrites the SignPath-signed `resources/elevate.exe` with the +// unsigned copy sitting in the electron-builder toolset cache. The release workflow +// swapped the cached copy first, but searched `/nsis` — a directory no current +// app-builder-lib layout creates (real ones are `/nsis-3.0.4.1/nsis-3.0.4.1-/` +// and `/nsis@/nsis-bundle--/`), so the swap silently found +// nothing and v1.4.193/v1.4.194 shipped an unsigned UAC elevation helper. + +import { copyFileSync, readdirSync, statSync } from 'node:fs' +import { createRequire } from 'node:module' +import { homedir, platform as osPlatform, tmpdir } from 'node:os' +import { join, parse, resolve } from 'node:path' + +const require = createRequire(import.meta.url) + +const ELEVATE_EXE = 'elevate.exe' + +// `nsis` (the layout the old hardcoded path assumed), `nsis-3.0.4.1` (legacy bundle, +// and `getBinFromCustomLoc('nsis', version)`), `nsis@1.2.1` (unified bundle). +const NSIS_RELEASE_DIR = /^nsis(?:[-@].*)?$/i + +// elevate.exe lives at the bundle root, one level under the release dir. The legacy +// bundle carries thousands of files under Contrib/, so an unbounded walk is both slow +// and a way to match something that is not a toolset copy. +const MAX_DEPTH = 3 + +function isFile(path) { + try { + return statSync(path).isFile() + } catch { + return false + } +} + +/** + * Mirrors `getCacheDirectory` in app-builder-lib's `out/util/electronGet.js`, which is what + * decides where the NSIS bundle is unpacked. Kept as a local port rather than an import + * because the swap must still resolve a cache root when app-builder-lib cannot be loaded. + */ +export function resolveElectronBuilderCacheDir({ + env = process.env, + platform = osPlatform(), + home = homedir(), + temp = tmpdir() +} = {}) { + const override = env.ELECTRON_BUILDER_CACHE?.trim() + if (override && parse(override).root) { + return override + } + if (platform === 'darwin') { + return join(home, 'Library', 'Caches', 'electron-builder') + } + if (platform === 'win32') { + const localAppData = env.LOCALAPPDATA?.trim() + // https://github.com/electron-userland/electron-builder/issues/1164 + const isSystemUser = + localAppData?.toLowerCase().includes('\\windows\\system32\\') === true || + env.USERNAME?.trim().toLowerCase() === 'system' + if (!localAppData || isSystemUser) { + return join(temp, 'electron-builder-cache') + } + return join(localAppData, 'electron-builder', 'Cache') + } + const xdgCache = env.XDG_CACHE_HOME + return xdgCache && parse(xdgCache).root + ? join(xdgCache, 'electron-builder') + : join(home, '.cache', 'electron-builder') +} + +function collectElevateFiles(dir, depth, found) { + let entries + try { + entries = readdirSync(dir, { withFileTypes: true }) + } catch { + return found + } + for (const entry of entries) { + const path = join(dir, entry.name) + if (entry.isFile()) { + if (entry.name.toLowerCase() === ELEVATE_EXE) { + found.push(path) + } + } else if (entry.isDirectory() && depth > 1) { + collectElevateFiles(path, depth - 1, found) + } + } + return found +} + +/** + * Every cached `elevate.exe` under an NSIS release directory of `cacheDir`, plus the + * `ELECTRON_BUILDER_NSIS_DIR` override copy when that is set. + */ +export function findCachedElevatePaths(cacheDir, { env = process.env } = {}) { + const found = [] + const overrideDir = env.ELECTRON_BUILDER_NSIS_DIR?.trim() + if (overrideDir && isFile(join(overrideDir, ELEVATE_EXE))) { + found.push(join(overrideDir, ELEVATE_EXE)) + } + let entries + try { + entries = readdirSync(cacheDir, { withFileTypes: true }) + } catch { + return found + } + for (const entry of entries) { + if (entry.isDirectory() && NSIS_RELEASE_DIR.test(entry.name)) { + collectElevateFiles(join(cacheDir, entry.name), MAX_DEPTH, found) + } + } + return found +} + +/** + * The exact path `CopyElevateHelper` will pack, asked of app-builder-lib itself. Best-effort: + * the internal module path moves between majors, and a cold cache would need the network, + * so a failure here degrades to the directory scan rather than failing the swap. + */ +export async function resolveToolsetElevatePath(projectDir = process.cwd()) { + try { + const configPath = require.resolve(resolve(projectDir, 'config/electron-builder.config.cjs')) + const config = require(configPath) + const { getNsisElevatePath } = require('app-builder-lib/out/toolsets/windows.js') + return await getNsisElevatePath(config.toolsets?.nsis, config.nsis?.customNsisBinary) + } catch (error) { + process.stderr.write( + `Could not resolve elevate.exe through app-builder-lib (${error.message}); ` + + 'falling back to the toolset cache scan.\n' + ) + return null + } +} + +/** + * Replaces every cached copy rather than picking one. Which bundle the rebuild packs + * depends on the toolset version resolved at pack time, and each cached copy is an + * unsigned `elevate.exe` that a later pack could reach for; the helper is a standalone + * UAC shim, not coupled to the NSIS version around it, so overwriting all of them is safe. + */ +export async function replaceCachedElevateHelpers({ + signedPath, + cacheDir = resolveElectronBuilderCacheDir(), + projectDir = process.cwd(), + env = process.env, + probeToolset = true +} = {}) { + if (!isFile(signedPath)) { + throw new Error(`Signed elevate.exe not found: ${signedPath}`) + } + const targets = new Set(findCachedElevatePaths(cacheDir, { env })) + const toolsetPath = probeToolset ? await resolveToolsetElevatePath(projectDir) : null + if (toolsetPath != null && isFile(toolsetPath)) { + targets.add(toolsetPath) + } + + const replaced = [] + for (const target of targets) { + copyFileSync(signedPath, target) + replaced.push(target) + } + return { replaced, cacheDir, toolsetPath } +} + +// Why an exit code and not a warning: a swap that finds nothing exits before the NSIS +// rebuild restores the unsigned helper, so a silent success here is indistinguishable +// from a release that shipped a signed one — which is how this went unnoticed for two +// releases. The workflow step is `continue-on-error`, so this annotates loudly without +// making a release unbuildable. +if (import.meta.filename === process.argv[1]) { + const signedPath = process.argv[2] + if (!signedPath) { + process.stderr.write('Usage: replace-cached-nsis-elevate.mjs \n') + process.exit(2) + } + try { + const { replaced, cacheDir } = await replaceCachedElevateHelpers({ signedPath }) + if (replaced.length === 0) { + process.stdout.write( + `::error::No cached elevate.exe found under ${cacheDir}; the NSIS rebuild will pack the ` + + 'unsigned helper and ship an unsigned UAC elevation binary. The electron-builder ' + + 'toolset cache layout has changed — update config/scripts/replace-cached-nsis-elevate.mjs.\n' + ) + process.exit(1) + } + if (replaced.length > 1) { + process.stdout.write( + `Note: ${replaced.length} cached NSIS bundles were present; replaced the helper in all of them.\n` + ) + } + for (const path of replaced) { + process.stdout.write(`Replaced ${path} with the SignPath-signed copy.\n`) + } + } catch (error) { + process.stdout.write(`::error::Could not replace the cached elevate.exe: ${error.message}\n`) + process.exit(1) + } +} diff --git a/config/scripts/replace-cached-nsis-elevate.test.mjs b/config/scripts/replace-cached-nsis-elevate.test.mjs new file mode 100644 index 00000000000..3985a5fc987 --- /dev/null +++ b/config/scripts/replace-cached-nsis-elevate.test.mjs @@ -0,0 +1,233 @@ +import { spawnSync } from 'node:child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { parse } from 'yaml' + +import { + findCachedElevatePaths, + replaceCachedElevateHelpers, + resolveElectronBuilderCacheDir +} from './replace-cached-nsis-elevate.mjs' + +const projectRoot = resolve(import.meta.dirname, '../..') +const scriptPath = join(projectRoot, 'config/scripts/replace-cached-nsis-elevate.mjs') + +let scratch + +beforeEach(() => { + scratch = mkdtempSync(join(tmpdir(), 'orca elevate swap ')) +}) + +afterEach(() => { + rmSync(scratch, { recursive: true, force: true }) +}) + +function makeCache(...relativeFiles) { + const cacheDir = join(scratch, 'Cache') + for (const relative of relativeFiles) { + const path = join(cacheDir, ...relative.split('/')) + mkdirSync(join(path, '..'), { recursive: true }) + writeFileSync(path, 'unsigned-elevate') + } + mkdirSync(cacheDir, { recursive: true }) + return cacheDir +} + +describe('cached elevate.exe swap covers the real electron-builder layouts', () => { + // Why these exact shapes: `downloadBuilderToolset` unpacks to + // `//-/`, and `releaseName` is + // `nsis-3.0.4.1` on the legacy bundle (`getBinFromUrl`), `nsis@` on the + // unified bundle, and `nsis-` for `customNsisBinary`. The release + // workflow searched `/nsis`, which matches none of them. + it.each([ + ['legacy bundle', 'nsis-3.0.4.1/nsis-3.0.4.1-1mx3n/elevate.exe'], + ['unified bundle', 'nsis@1.2.1/nsis-bundle-3.12-k4d9x/elevate.exe'], + ['custom nsis binary', 'nsis-9f3a1c2b/nsis-custom-3.11-0zqp2/elevate.exe'], + ['bare nsis release dir', 'nsis/nsis-3.0.4.1/elevate.exe'] + ])('finds the cached helper in the %s layout', (_label, relative) => { + const cacheDir = makeCache(relative) + expect(findCachedElevatePaths(cacheDir, { env: {} })).toEqual([ + join(cacheDir, ...relative.split('/')) + ]) + }) + + it('ignores cache siblings that are not NSIS toolset bundles', () => { + const cacheDir = makeCache( + 'winCodeSign/winCodeSign-2.6.0-abc12/elevate.exe', + 'downloads/nsis/elevate.exe', + 'nsis-resources-3.4.1/nsis-resources-3.4.1-p8w1z/plugins/x86-unicode/nsProcess.dll' + ) + expect(findCachedElevatePaths(cacheDir, { env: {} })).toEqual([]) + }) + + // The rebuild picks one bundle, and nothing outside app-builder-lib knows which. + // Replacing every cached copy is the deliberate answer to that ambiguity. + it('replaces every cached copy when several bundles are present', async () => { + const cacheDir = makeCache( + 'nsis-3.0.4.1/nsis-3.0.4.1-1mx3n/elevate.exe', + 'nsis@1.2.1/nsis-bundle-3.12-k4d9x/elevate.exe' + ) + const signed = join(scratch, 'signed-elevate.exe') + writeFileSync(signed, 'signpath-signed-elevate') + + const { replaced } = await replaceCachedElevateHelpers({ + signedPath: signed, + cacheDir, + env: {}, + probeToolset: false + }) + + expect(replaced).toHaveLength(2) + for (const path of replaced) { + expect(readFileSync(path, 'utf8')).toBe('signpath-signed-elevate') + } + }) + + it('covers the ELECTRON_BUILDER_NSIS_DIR override copy', () => { + const overrideDir = join(scratch, 'nsis-override') + mkdirSync(overrideDir, { recursive: true }) + writeFileSync(join(overrideDir, 'elevate.exe'), 'unsigned-elevate') + const cacheDir = makeCache() + + expect( + findCachedElevatePaths(cacheDir, { env: { ELECTRON_BUILDER_NSIS_DIR: overrideDir } }) + ).toEqual([join(overrideDir, 'elevate.exe')]) + }) + + it('resolves the cache root the same way app-builder-lib does', () => { + expect( + resolveElectronBuilderCacheDir({ + env: { LOCALAPPDATA: 'C:\\Users\\runneradmin\\AppData\\Local' }, + platform: 'win32' + }) + ).toBe(join('C:\\Users\\runneradmin\\AppData\\Local', 'electron-builder', 'Cache')) + expect(resolveElectronBuilderCacheDir({ env: {}, platform: 'darwin', home: '/Users/a' })).toBe( + join('/Users/a', 'Library', 'Caches', 'electron-builder') + ) + expect(resolveElectronBuilderCacheDir({ env: { ELECTRON_BUILDER_CACHE: '/mnt/cache' } })).toBe( + '/mnt/cache' + ) + }) + + // Proof against the layout actually on disk, not just the fixtures. Cross-checked + // against an independent unbounded walk so a search that scopes itself wrongly + // cannot pass by finding nothing — which is exactly how the inline path passed. + // Skipped only where no NSIS bundle has been downloaded into the cache yet. + it('finds every elevate.exe the real electron-builder cache holds', () => { + const cacheDir = resolveElectronBuilderCacheDir() + if (!existsSync(cacheDir)) { + return + } + const walk = (dir) => + readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name) + if (entry.isDirectory()) { + return walk(path) + } + return entry.name.toLowerCase() === 'elevate.exe' ? [path] : [] + }) + const onDisk = walk(cacheDir) + if (onDisk.length === 0) { + return + } + expect(findCachedElevatePaths(cacheDir, { env: {} }).sort()).toEqual(onDisk.sort()) + }) +}) + +describe('a cached elevate.exe miss is not silent', () => { + // ELECTRON_BUILDER_NSIS_DIR short-circuits app-builder-lib's own resolution before + // any download, so the probe fails offline instead of fetching the NSIS bundle. + function runScript(cacheDir, nsisDir, signedPath) { + return spawnSync(process.execPath, [scriptPath, signedPath], { + cwd: projectRoot, + encoding: 'utf8', + env: { + ...process.env, + ELECTRON_BUILDER_CACHE: cacheDir, + ELECTRON_BUILDER_NSIS_DIR: nsisDir + } + }) + } + + it('exits non-zero with an ::error:: annotation when no cached copy is found', () => { + const cacheDir = makeCache() + const emptyNsisDir = join(scratch, 'empty-nsis') + mkdirSync(emptyNsisDir, { recursive: true }) + const signed = join(scratch, 'signed-elevate.exe') + writeFileSync(signed, 'signpath-signed-elevate') + + const result = runScript(cacheDir, emptyNsisDir, signed) + + expect(result.status).toBe(1) + expect(result.stdout).toContain('::error::No cached elevate.exe found') + }) + + it('exits zero and rewrites the cached copy when one is found', () => { + const cacheDir = makeCache('nsis-3.0.4.1/nsis-3.0.4.1-1mx3n/elevate.exe') + const emptyNsisDir = join(scratch, 'empty-nsis') + mkdirSync(emptyNsisDir, { recursive: true }) + const signed = join(scratch, 'signed-elevate.exe') + writeFileSync(signed, 'signpath-signed-elevate') + + const result = runScript(cacheDir, emptyNsisDir, signed) + + expect(result.status).toBe(0) + expect(result.stdout).not.toContain('::error::') + expect( + readFileSync(join(cacheDir, 'nsis-3.0.4.1', 'nsis-3.0.4.1-1mx3n', 'elevate.exe'), 'utf8') + ).toBe('signpath-signed-elevate') + }) +}) + +describe('release-cut.yml swaps the cached elevate.exe through the resolver', () => { + function swapStep() { + const workflow = parse( + readFileSync(join(projectRoot, '.github/workflows/release-cut.yml'), 'utf8') + ) + const step = workflow.jobs.build.steps.find( + (candidate) => candidate.name === 'Replace cached elevate.exe with the signed copy' + ) + expect(step).toBeDefined() + return step + } + + it('delegates the cache lookup to the script instead of an inline path', () => { + const step = swapStep() + expect(step.run).toContain('node config/scripts/replace-cached-nsis-elevate.mjs $signed') + // The hardcoded miss that shipped v1.4.193/v1.4.194 unsigned. + expect(step.run).not.toContain('electron-builder\\Cache\\nsis') + expect(step.run).not.toContain('-ErrorAction SilentlyContinue') + }) + + it('fails the step when the swap reports a miss', () => { + const step = swapStep() + // Matched as an executed statement: downgrading this to a Write-Host restores + // the silent fail-open that let the unsigned helper ship. + expect(step.run).toMatch(/if \(\$LASTEXITCODE -ne 0\) \{/) + expect(step.run).toMatch(/^\s*throw \$message\s*$/m) + expect(step.run).toContain('GITHUB_STEP_SUMMARY') + }) + + // Why kept: windows-signing-rehearsal.yml shares the electron-builder-win- + // cache key, so dropping this guard would let a test certificate reach a release cache. + it('still refuses to stage anything but a SignPath-signed helper', () => { + const step = swapStep() + expect(step.run).toContain("$signature.Status -ne 'Valid'") + expect(step.run).toContain("$subject -notlike '*CN=SignPath Foundation*'") + }) + + // The inner-signing chain stays fail-open: a loud red step, not an unbuildable release. + it('keeps the step unable to fail the release job', () => { + expect(swapStep()['continue-on-error']).toBe(true) + }) +})