fix(release): restore the Windows inner-binary signature gate

electron-builder 26.9+ dropped the bundled 7zip-bin package, so the gate's
hardcoded node_modules/7zip-bin path stopped resolving in 1d2cd33c83. The
gate is fail-open, so it swallowed the error and 11 releases shipped with
no signature verification and an evidence artifact that looked clean.

Resolve 7za through app-builder-lib's toolset instead, and always record a
verdict so a degraded gate can't pass for a healthy one.

Refs #6487
This commit is contained in:
Neil
2026-07-26 15:23:23 -07:00
parent a51e675804
commit 1338becde5
5 changed files with 495 additions and 5 deletions
+63 -4
View File
@@ -1644,8 +1644,41 @@ jobs:
INNER_SIGNING_COMPLETED: ${{ steps.rebuild-nsis-signed.outcome == 'success' }}
run: |
$required = $env:ORCA_WINDOWS_INNER_SIGNATURE_REQUIRED -eq 'true'
# Why: a fail-open gate that writes nothing is indistinguishable from a
# gate that passed. Always leave a verdict in the evidence artifact and
# the job summary so a silent degradation is visible (#6487).
# Why best-effort: while warn-only, a disk-full or permission error
# writing the verdict must not become the thing that fails the release.
function Add-GateEvidence([string]$line) {
try {
Add-Content -Path 'inner-signing-evidence.txt' -Value "`n$line" -ErrorAction Stop
} catch {
Write-Host "::warning::Could not append to the inner-signing evidence file: $_"
}
}
function Add-GateSummary([string]$verdict) {
if (-not $env:GITHUB_STEP_SUMMARY) { return }
try {
Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value "**Windows inner-binary signing:** $verdict" -ErrorAction Stop
} catch {
Write-Host "::warning::Could not write inner-signing verdict to the job summary: $_"
}
}
function Write-GateVerdict([string]$verdict) {
try {
Set-Content -Path 'inner-signing-evidence.txt' -Value $verdict -ErrorAction Stop
} catch {
Write-Host "::warning::Could not persist inner-signing verdict: $_"
}
Add-GateSummary $verdict
}
if ($env:INNER_SIGNING_COMPLETED -ne 'true') {
$message = 'Windows inner-binary signing did not complete; this release ships unsigned inner binaries (fail-open, issue #7785).'
Write-GateVerdict "NOT VERIFIED — $message"
if ($required) { throw $message }
Write-Host "::warning::$message"
exit 0
@@ -1654,13 +1687,23 @@ jobs:
# Why try/catch: while the gate is warn-only, even an unexpected
# script error (extraction hiccup, missing file) must not block
# the release — only the flip to required makes failures fatal.
# Why tracked separately: a required-mode signature failure must not be
# rewritten as ERRORED by the catch below, which would replace the
# per-file report with an exception string and lose the diagnostics.
$policyFailure = $null
try {
$report = New-Object System.Collections.Generic.List[string]
$failures = New-Object System.Collections.Generic.List[string]
# Why: verify the files a user actually gets on disk, not the build
# tree — 7z parses the NSIS exe directly as its embedded payload.
$7za = 'node_modules/7zip-bin/win/x64/7za.exe'
# Resolve 7za via app-builder-lib; electron-builder 26.9+ dropped the
# bundled 7zip-bin package the old hardcoded path relied on (#6487).
$7za = (node config/scripts/resolve-7za-path.mjs).Trim()
if ($LASTEXITCODE -ne 0 -or -not (Test-Path $7za)) {
throw "Could not resolve a 7za executable for the inner-binary evidence gate: $7za"
}
New-Item -ItemType Directory -Path inner-evidence-extract -Force | Out-Null
& $7za x 'dist/orca-windows-setup.exe' '-oinner-evidence-extract' -y | Out-Null
@@ -1694,16 +1737,32 @@ jobs:
if ($failures.Count -gt 0) {
$failures | ForEach-Object { Write-Host "::warning::$_" }
$message = "Windows inner-binary evidence gate found $($failures.Count) problems."
if ($required) { throw $message }
Write-Host "::warning::$message Fail-open until ORCA_WINDOWS_INNER_SIGNATURE_REQUIRED is 'true'."
# Why assigned before any I/O: a write that throws here would reach
# the catch with $policyFailure still null, so a required-mode
# signature failure would be re-reported as ERRORED and the per-file
# report overwritten — the exact masking the hoist exists to prevent.
if ($required) {
$policyFailure = $message
} else {
Write-Host "::warning::$message Fail-open until ORCA_WINDOWS_INNER_SIGNATURE_REQUIRED is 'true'."
}
Add-GateEvidence "VERDICT: FAILED — $message"
Add-GateSummary "FAILED — $message"
} else {
Write-Host "All $($targets.Count) inner binaries in the shipped installer are signed by SignPath Foundation."
$ok = "All $($targets.Count) inner binaries in the shipped installer are signed by SignPath Foundation."
Add-GateEvidence "VERDICT: PASSED — $ok"
Add-GateSummary "PASSED — $ok"
Write-Host $ok
}
} catch {
Write-GateVerdict "ERRORED — $_"
if ($required) { throw }
Write-Host "::warning::Windows inner-binary evidence gate errored: $_ (fail-open, issue #7785)."
}
# Outside the catch so the FAILED evidence report survives intact.
if ($policyFailure) { throw $policyFailure }
- name: Upload Windows inner signing evidence
if: always() && matrix.platform == 'win'
uses: actions/upload-artifact@v7
@@ -325,7 +325,11 @@ jobs:
# Extract the signed installer and verify the files a user actually
# gets on disk — including the exact file from issue #7785.
$7za = 'node_modules/7zip-bin/win/x64/7za.exe'
# electron-builder 26.9+ removed the bundled 7zip-bin package (#6487).
$7za = (node config/scripts/resolve-7za-path.mjs).Trim()
if ($LASTEXITCODE -ne 0 -or -not (Test-Path $7za)) {
throw "Could not resolve a 7za executable for the signing rehearsal: $7za"
}
New-Item -ItemType Directory -Path extracted-app -Force | Out-Null
& $7za x 'dist/orca-windows-setup.exe' '-oextracted-app' -y | Out-Null
+102
View File
@@ -0,0 +1,102 @@
#!/usr/bin/env node
// Why: electron-builder 26.9+ dropped the bundled `7zip-bin` package in favour of a
// toolset downloaded at build time, so the hardcoded `node_modules/7zip-bin/...` path
// the release signing gates used silently stopped resolving (#6487).
import { createRequire } from 'node:module'
import { existsSync, statSync } from 'node:fs'
import { resolve } from 'node:path'
const require = createRequire(import.meta.url)
// Why not existsSync: PowerShell's `Test-Path` is true for directories too, so a
// override pointing at a folder would satisfy both checks and only fail later as
// an opaque exec error inside the gate.
function isFile(path) {
try {
return statSync(path).isFile()
} catch {
return false
}
}
// Legacy layout, still valid if a transitive dep reintroduces 7zip-bin. The
// package ships `mac/`, not `darwin/`, and keeps a separate ia32 build.
export function legacy7zaRelativePath(platform = process.platform, arch = process.arch) {
if (platform === 'win32') {
return ['node_modules', '7zip-bin', 'win', arch, '7za.exe']
}
const dir = platform === 'darwin' ? 'mac' : platform
return ['node_modules', '7zip-bin', dir, arch, '7za']
}
// app-builder-lib logs download progress to stdout, which would corrupt the
// single-path contract the PowerShell gates parse. Divert it to stderr so a
// cold toolset cache stays debuggable without breaking the caller.
//
// Why refcounted: patching `process.stdout.write` is process-global, so two
// concurrent callers would each capture the other's patched function as their
// "original" and the last `finally` would restore a diverting stub permanently.
let stdoutDivertDepth = 0
let originalStdoutWrite = null
async function withStdoutDivertedToStderr(run) {
if (stdoutDivertDepth === 0) {
originalStdoutWrite = process.stdout.write
process.stdout.write = (chunk, encoding, callback) =>
process.stderr.write(chunk, encoding, callback)
}
stdoutDivertDepth += 1
try {
return await run()
} finally {
stdoutDivertDepth -= 1
if (stdoutDivertDepth === 0) {
process.stdout.write = originalStdoutWrite
originalStdoutWrite = null
}
}
}
export async function resolve7zaPath(projectDir = process.cwd()) {
const override = process.env.ELECTRON_BUILDER_7ZIP_PATH
if (override && isFile(override)) {
return override
}
const legacy = resolve(projectDir, ...legacy7zaRelativePath())
if (isFile(legacy)) {
return legacy
}
// app-builder-lib reads the same env var and hard-fails on a stale value, so a
// dangling override must be cleared rather than passed through to the download.
const restoreOverride = override !== undefined
if (restoreOverride) {
delete process.env.ELECTRON_BUILDER_7ZIP_PATH
}
try {
// The toolset is cached after the first download, so a release build has
// already paid this cost by the time the signing gate runs.
const { getPath7za } = require('app-builder-lib/out/toolsets/7zip.js')
const toolsetPath = await withStdoutDivertedToStderr(() => getPath7za())
if (!existsSync(toolsetPath)) {
throw new Error(`app-builder-lib returned a 7za path that does not exist: ${toolsetPath}`)
}
return toolsetPath
} finally {
if (restoreOverride) {
process.env.ELECTRON_BUILDER_7ZIP_PATH = override
}
}
}
if (import.meta.filename === process.argv[1]) {
try {
process.stdout.write(`${await resolve7zaPath()}\n`)
} catch (error) {
process.stderr.write(`Could not resolve a 7za executable: ${error.message}\n`)
process.exit(1)
}
}
+170
View File
@@ -0,0 +1,170 @@
import { spawnSync } from 'node:child_process'
import { existsSync, mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'
import { legacy7zaRelativePath, resolve7zaPath } from './resolve-7za-path.mjs'
const projectRoot = resolve(import.meta.dirname, '../..')
describe('7za path resolution for the Windows signing gates (#6487)', () => {
// Why fixtures: 7zip-bin@5.2.0 ships mac/{arm64,x64}, win/{arm64,ia32,x64}
// and linux/{arm,arm64,ia32,x64} — a `darwin/` or arch-collapsed guess
// resolves to nothing, which is how the gate degraded silently in the first place.
it.each([
['darwin', 'arm64', 'mac/arm64/7za'],
['darwin', 'x64', 'mac/x64/7za'],
['win32', 'x64', 'win/x64/7za.exe'],
['win32', 'ia32', 'win/ia32/7za.exe'],
['win32', 'arm64', 'win/arm64/7za.exe'],
['linux', 'x64', 'linux/x64/7za'],
['linux', 'arm64', 'linux/arm64/7za']
])('maps the %s/%s legacy layout to %s', (platform, arch, expected) => {
expect(legacy7zaRelativePath(platform, arch).join('/')).toBe(
`node_modules/7zip-bin/${expected}`
)
})
it('prefers a real legacy binary over the downloaded toolset', async () => {
// Only meaningful if a transitive dep reintroduces the package.
const legacy = join(projectRoot, ...legacy7zaRelativePath())
if (!existsSync(legacy)) {
return
}
await expect(resolve7zaPath(projectRoot)).resolves.toBe(legacy)
})
it('resolves an executable 7za that can extract an archive', async () => {
const path7za = await resolve7zaPath(projectRoot)
expect(existsSync(path7za)).toBe(true)
const scratch = mkdtempSync(join(tmpdir(), 'orca 7za resolve '))
try {
const payloadDir = join(scratch, 'payload')
mkdirSync(payloadDir, { recursive: true })
writeFileSync(join(payloadDir, 'Orca.exe'), 'not-a-real-pe')
const archive = join(scratch, 'bundle.7z')
const created = spawnSync(path7za, ['a', archive, payloadDir], { encoding: 'utf8' })
expect(created.status).toBe(0)
const outDir = join(scratch, 'out')
const extracted = spawnSync(path7za, ['x', archive, `-o${outDir}`, '-y'], {
encoding: 'utf8'
})
expect(extracted.status).toBe(0)
expect(existsSync(join(outDir, 'payload', 'Orca.exe'))).toBe(true)
} finally {
rmSync(scratch, { recursive: true, force: true })
}
}, 120_000)
it('prefers an explicit ELECTRON_BUILDER_7ZIP_PATH override', async () => {
const scratch = mkdtempSync(join(tmpdir(), 'orca 7za override '))
const previous = process.env.ELECTRON_BUILDER_7ZIP_PATH
try {
const fake = join(scratch, 'my7za')
writeFileSync(fake, '#!/bin/sh\n')
process.env.ELECTRON_BUILDER_7ZIP_PATH = fake
await expect(resolve7zaPath(projectRoot)).resolves.toBe(fake)
} finally {
if (previous === undefined) {
delete process.env.ELECTRON_BUILDER_7ZIP_PATH
} else {
process.env.ELECTRON_BUILDER_7ZIP_PATH = previous
}
rmSync(scratch, { recursive: true, force: true })
}
})
// Why a directory and not a missing path: PowerShell's `Test-Path $7za` is true
// for directories, so a folder-valued override would clear both the resolver's
// check and the gate's, then fail as an opaque exec error mid-extraction.
it('ignores an override that points at a directory', async () => {
const scratch = mkdtempSync(join(tmpdir(), 'orca 7za dir override '))
const previous = process.env.ELECTRON_BUILDER_7ZIP_PATH
try {
process.env.ELECTRON_BUILDER_7ZIP_PATH = scratch
const resolved = await resolve7zaPath(projectRoot)
expect(resolved).not.toBe(scratch)
expect(statSync(resolved).isFile()).toBe(true)
} finally {
if (previous === undefined) {
delete process.env.ELECTRON_BUILDER_7ZIP_PATH
} else {
process.env.ELECTRON_BUILDER_7ZIP_PATH = previous
}
rmSync(scratch, { recursive: true, force: true })
}
}, 120_000)
// Why concurrency: the stdout diversion patches a process-global function. A
// naive save/restore lets the first finisher reinstate a still-diverting stub
// as "the original", permanently swallowing stdout for the rest of the process.
it('restores stdout after concurrent resolutions', async () => {
const before = process.stdout.write
await Promise.all([
resolve7zaPath(projectRoot),
resolve7zaPath(projectRoot),
resolve7zaPath(projectRoot)
])
expect(process.stdout.write).toBe(before)
}, 120_000)
// Why a subprocess: app-builder-lib memoises the resolved toolset, so an in-process
// assertion passes on the cached value even when a dangling override would abort a
// cold release runner.
it('ignores an override that points at a missing file, in a cold process', () => {
const dangling = join(tmpdir(), 'orca-7za-does-not-exist')
const result = spawnSync(process.execPath, ['config/scripts/resolve-7za-path.mjs'], {
cwd: projectRoot,
encoding: 'utf8',
env: { ...process.env, ELECTRON_BUILDER_7ZIP_PATH: dangling },
timeout: 120_000
})
expect(result.stderr ?? '').not.toContain('does not exist')
expect(result.status).toBe(0)
const resolved = result.stdout.trim()
expect(resolved).not.toBe(dangling)
expect(existsSync(resolved)).toBe(true)
}, 120_000)
it('prints exactly one clean line the PowerShell gate can consume', () => {
const result = spawnSync(process.execPath, ['config/scripts/resolve-7za-path.mjs'], {
cwd: projectRoot,
encoding: 'utf8',
timeout: 120_000
})
expect(result.status).toBe(0)
expect(result.stdout.trimEnd().split('\n')).toHaveLength(1)
expect(existsSync(result.stdout.trim())).toBe(true)
}, 120_000)
// Why a cold cache with VITEST unset: app-builder-lib prints download
// progress to stdout, and builder-util suppresses that logging under Vitest —
// so an inherited VITEST makes this exact failure invisible. `$7za = (node
// ...).Trim()` would otherwise receive two lines and the gate would break on
// any runner whose toolset cache was evicted or repaired.
it('keeps stdout to one path even when the toolset cache is cold', () => {
const cache = mkdtempSync(join(tmpdir(), 'orca 7za cold cache '))
try {
const { VITEST: _vitest, ...envWithoutVitest } = process.env
const result = spawnSync(process.execPath, ['config/scripts/resolve-7za-path.mjs'], {
cwd: projectRoot,
encoding: 'utf8',
env: { ...envWithoutVitest, ELECTRON_BUILDER_CACHE: cache },
timeout: 300_000
})
expect(result.status).toBe(0)
const lines = result.stdout.trimEnd().split('\n')
expect(lines).toHaveLength(1)
expect(existsSync(lines[0])).toBe(true)
} finally {
rmSync(cache, { recursive: true, force: true })
}
}, 300_000)
})
@@ -0,0 +1,155 @@
import { readFileSync } from 'node:fs'
import { join, resolve } from 'node:path'
import { describe, expect, it } from 'vitest'
// Why: the inner-binary signing gate silently degraded for four releases because it
// shelled out to a hardcoded `node_modules/7zip-bin/...` path that electron-builder
// 26.9+ no longer installs. Pin both workflows to the resolver instead (#6487).
const workflowsDir = resolve(import.meta.dirname, '../..', '.github', 'workflows')
const GATED_WORKFLOWS = ['release-cut.yml', 'windows-signing-rehearsal.yml']
function workflowSource(name) {
return readFileSync(join(workflowsDir, name), 'utf8')
}
// Why a scanner and not a regex: PowerShell here-docs in these gates contain
// braces inside strings (`"{0,-14} {1} <{2}>" -f ...`) and inside comments, so
// naive brace counting mis-pairs and every scope assertion below silently
// degrades into "some text appears somewhere in the file".
function findBlockEnd(source, openIndex) {
let depth = 0
let i = openIndex
while (i < source.length) {
const char = source[i]
if (char === '#') {
const newline = source.indexOf('\n', i)
i = newline === -1 ? source.length : newline
continue
}
if (char === "'") {
const end = source.indexOf("'", i + 1)
i = end === -1 ? source.length : end + 1
continue
}
if (char === '"') {
i += 1
while (i < source.length && source[i] !== '"') {
i += source[i] === '`' ? 2 : 1
}
i += 1
continue
}
if (char === '{') {
depth += 1
} else if (char === '}') {
depth -= 1
if (depth === 0) {
return i
}
}
i += 1
}
throw new Error(`unbalanced block starting at ${openIndex}`)
}
/** The `{ ... }` block that opens after `marker`, as `{ start, end, body }`. */
function blockAfter(source, marker, from = 0) {
const markerIndex = source.indexOf(marker, from)
expect(markerIndex, `missing marker: ${marker}`).toBeGreaterThan(-1)
const start = source.indexOf('{', markerIndex)
expect(start, `no block opens after: ${marker}`).toBeGreaterThan(-1)
const end = findBlockEnd(source, start)
return { start, end, body: source.slice(start, end + 1) }
}
describe('Windows signing gates resolve 7za through the toolset resolver (#6487)', () => {
for (const name of GATED_WORKFLOWS) {
it(`${name} does not hardcode the removed 7zip-bin path`, () => {
expect(workflowSource(name)).not.toContain('node_modules/7zip-bin')
})
it(`${name} resolves 7za via resolve-7za-path.mjs`, () => {
expect(workflowSource(name)).toContain('node config/scripts/resolve-7za-path.mjs')
})
// Why scope-checked: downgrading this `throw` to a `Write-Host` warning
// restores the original silent fail-open — the gate extracts nothing, finds
// no files, and reports success. A `toContain` on the guard condition alone
// does not notice.
it(`${name} aborts the gate when 7za cannot be resolved`, () => {
const source = workflowSource(name)
const guard = blockAfter(source, '$LASTEXITCODE -ne 0 -or -not (Test-Path $7za)')
expect(guard.body).toMatch(/\bthrow\b/)
})
}
// Why sliced to one step: release-cut.yml runs several PowerShell gates that
// share idioms (`$failures`, `} catch {`), so a whole-file search silently
// asserts against the wrong block.
function innerBinaryStep() {
const source = workflowSource('release-cut.yml')
const start = source.indexOf('- name: Verify Windows inner binary signatures')
expect(start).toBeGreaterThan(-1)
const end = source.indexOf('\n - name:', start + 1)
expect(end).toBeGreaterThan(start)
return source.slice(start, end)
}
// Why parse the function body rather than grep the file: asserting that the
// string 'Write-GateVerdict' appears somewhere passes even if the body is
// gutted to a Write-Host, which is exactly the silent degradation this gate
// exists to prevent.
function gateVerdictFunctionBody() {
return blockAfter(innerBinaryStep(), 'function Write-GateVerdict').body
}
it('persists the verdict to the evidence file the artifact upload collects', () => {
const body = gateVerdictFunctionBody()
expect(body).toMatch(/Set-Content\s+-Path\s+'inner-signing-evidence\.txt'/)
expect(body).toContain('Add-GateSummary')
})
it('never lets verdict persistence itself fail a warn-only release', () => {
// Every persistence helper is best-effort: a disk-full or read-only runner
// must not turn evidence-writing into the thing that fails the release.
const step = innerBinaryStep()
for (const helper of ['function Add-GateEvidence', 'function Add-GateSummary']) {
const body = blockAfter(step, helper).body
expect(body, helper).toContain('-ErrorAction Stop')
expect(body, helper).toMatch(/\bcatch\b/)
}
expect(gateVerdictFunctionBody()).toMatch(/\bcatch\b/)
})
it('records a verdict on every terminal branch of the gate', () => {
const step = innerBinaryStep()
for (const verdict of ['NOT VERIFIED', 'ERRORED', 'VERDICT: FAILED', 'VERDICT: PASSED']) {
expect(step).toContain(verdict)
}
})
it('throws a required-mode signature failure outside the catch that would mask it', () => {
// Why: throwing inside `try` re-enters the catch, whose Set-Content
// replaces the per-file report with "ERRORED — <exception>".
const step = innerBinaryStep()
const policyThrow = step.indexOf('if ($policyFailure) { throw $policyFailure }')
expect(policyThrow).toBeGreaterThan(-1)
const catchBlock = blockAfter(step, '} catch {')
expect(policyThrow).toBeGreaterThan(catchBlock.end)
})
// Why: the assignment is what survives a write failure. With it after the
// evidence/summary writes, a throwing Add-Content lands in the catch with
// $policyFailure still null — required mode reports ERRORED and overwrites the
// per-file report, reintroducing exactly the loss the hoist prevents.
it('records the required-mode failure before attempting any evidence write', () => {
const branch = blockAfter(innerBinaryStep(), 'if ($failures.Count -gt 0)').body
const assignment = branch.indexOf('$policyFailure = $message')
expect(assignment).toBeGreaterThan(-1)
for (const write of ['Add-GateEvidence', 'Add-GateSummary']) {
expect(branch.indexOf(write), write).toBeGreaterThan(assignment)
}
})
})