diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 730583e3404..25fe58d08aa 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -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 diff --git a/.github/workflows/windows-signing-rehearsal.yml b/.github/workflows/windows-signing-rehearsal.yml index 4ca080d803d..8ffa9441f4f 100644 --- a/.github/workflows/windows-signing-rehearsal.yml +++ b/.github/workflows/windows-signing-rehearsal.yml @@ -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 diff --git a/config/scripts/resolve-7za-path.mjs b/config/scripts/resolve-7za-path.mjs new file mode 100644 index 00000000000..7edf0f65a41 --- /dev/null +++ b/config/scripts/resolve-7za-path.mjs @@ -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) + } +} diff --git a/config/scripts/resolve-7za-path.test.mjs b/config/scripts/resolve-7za-path.test.mjs new file mode 100644 index 00000000000..e7b1eb40bce --- /dev/null +++ b/config/scripts/resolve-7za-path.test.mjs @@ -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) +}) diff --git a/config/scripts/windows-signing-gate-toolset.test.mjs b/config/scripts/windows-signing-gate-toolset.test.mjs new file mode 100644 index 00000000000..38de8182a1b --- /dev/null +++ b/config/scripts/windows-signing-gate-toolset.test.mjs @@ -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 — ". + 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) + } + }) +})