fix(windows): stop the signing rehearsal failing on its own artefact

The rehearsal is the merge gate for this chain, so it must not be able to fail on something that is not the thing under test. It trusted whatever 7-Zip's NSIS handler emitted. That handler produces partial or garbled output on some NSIS builds, and a truncated extract would score NotSigned and be reported as "the shipped uninstaller is unsigned" when nothing was wrong. It now has to reproduce the digest the sign hook recorded before its output is trusted; otherwise it falls through to the silent-install route, which is ground truth. A name miss falls through the same way. The install route only checked the signature. Comparing the on-disk file against the receipt is what actually proves the shipped installer embedded the SignPath-signed bytes — the release job's own comparison is equal by construction, so this is the only place the claim is really tested. Also: bound the silent install (a bare `-Wait` on an installer that ever prompts hangs to the 360-minute job cap) and poll before stopping Orca, since the oneClick installer launches the app as it finishes and the process can appear after the installer has already exited. Two smaller ones: `-ErrorAction Stop` on the staging New-Item/Copy-Item so the catch above them does not depend on GitHub's $ErrorActionPreference default; and the relay-path test now counts every occurrence rather than the first, so a step carrying two paths cannot root one in RUNNER_TEMP and leave the other bare-relative — the exact shape of the bug it guards.
2026-10-02 00:02:05 +00:00 · 2026-09-01 00:58:18 -07:00
parent c04a179607
commit 2fcefeb910
3 changed files with 97 additions and 8 deletions
@@ -1531,8 +1531,8 @@ jobs:
            $exportedUninstaller = Join-Path $env:RUNNER_TEMP 'uninstaller-signing\unsigned\orca-uninstaller.exe'
            if (Test-Path -LiteralPath $exportedUninstaller) {
              $uninstallerStagePath = Join-Path $stage.FullName 'uninstaller\orca-uninstaller.exe'
-              New-Item -ItemType Directory -Force -Path (Split-Path $uninstallerStagePath) | Out-Null
-              Copy-Item -LiteralPath $exportedUninstaller -Destination $uninstallerStagePath -Force
+              New-Item -ItemType Directory -Force -Path (Split-Path $uninstallerStagePath) -ErrorAction Stop | Out-Null
+              Copy-Item -LiteralPath $exportedUninstaller -Destination $uninstallerStagePath -Force -ErrorAction Stop
              Write-Host 'Staged the NSIS uninstaller for signing: uninstaller\orca-uninstaller.exe'
            } else {
              Write-Host "::warning::No exported NSIS uninstaller at $exportedUninstaller; this release ships an unsigned uninstaller (fail-open)."
@@ -387,33 +387,81 @@ jobs:
          # NSIS handler. If it cannot read the section either, fall back to a
          # real silent install.
          $installedUninstaller = $null
+          $installedVia = $null
+          $expectedDigest = if (Test-Path -LiteralPath $receipt) { (Get-Content -LiteralPath $receipt -Raw).Trim() } else { $null }
          $full7z = 'C:\Program Files\7-Zip\7z.exe'
          if (Test-Path -LiteralPath $full7z) {
            New-Item -ItemType Directory -Path nsis-extract -Force | Out-Null
            & $full7z x -tnsis 'dist/orca-windows-setup.exe' '-onsis-extract' -y 2>&1 | Out-Null
            $installedUninstaller = Get-ChildItem -Path nsis-extract -Recurse -File -Filter 'Uninstall*.exe' -ErrorAction SilentlyContinue |
              Select-Object -First 1
+            # Why the digest guard before trusting this route: 7-Zip's NSIS
+            # handler emits partial or garbled output on some NSIS builds, and a
+            # truncated extract would score NotSigned and fail the rehearsal as
+            # "the shipped uninstaller is unsigned" when nothing is wrong. Only
+            # trust it when it reproduces the bytes the relay embedded; otherwise
+            # fall through to the install route, which is ground truth. A name
+            # miss (the handler labelling the entry by its source name) falls
+            # through the same way.
+            if ($null -ne $installedUninstaller -and $null -ne $expectedDigest -and
+                (Get-FileHash -LiteralPath $installedUninstaller.FullName -Algorithm SHA256).Hash.ToLowerInvariant() -ne $expectedDigest) {
+              Write-Host "7-Zip's NSIS output did not match the relayed digest; falling back to a silent install."
+              $installedUninstaller = $null
+            }
            if ($null -ne $installedUninstaller) {
+              $installedVia = "7-Zip's NSIS handler"
              Write-Host "Read the embedded uninstaller with 7-Zip's NSIS handler: $($installedUninstaller.FullName)"
            } else {
-              Write-Host "7-Zip's NSIS handler did not yield an uninstaller; falling back to a silent install."
+              Write-Host "7-Zip's NSIS handler did not yield a usable uninstaller; falling back to a silent install."
            }
          }

          if ($null -eq $installedUninstaller) {
            # Nothing here is published, so mutating this runner is free.
-            Start-Process -FilePath (Resolve-Path 'dist/orca-windows-setup.exe') -ArgumentList '/S' -Wait
-            # The installer launches the app on finish; it is not wanted here.
-            Stop-Process -Name 'Orca' -Force -ErrorAction SilentlyContinue
+            # Why -PassThru and a bounded wait rather than -Wait: a bare -Wait on
+            # an installer that ever prompts hangs to the job's 360-minute cap.
+            $installerProcess = Start-Process -FilePath (Resolve-Path 'dist/orca-windows-setup.exe') -ArgumentList '/S' -PassThru
+            if (-not $installerProcess.WaitForExit(300000)) {
+              $installerProcess | Stop-Process -Force -ErrorAction SilentlyContinue
+              $failures.Add('the silent install did not exit within 5 minutes; it is likely prompting')
+            }
+            # Why a poll rather than one Stop-Process: the oneClick installer
+            # launches the app as it finishes, so Orca.exe can appear *after* the
+            # installer process exits. A single silenced Stop-Process would miss
+            # it and leave Orca plus orca-terminal-daemon.exe holding handles
+            # under %LOCALAPPDATA%\Programs for the rest of the job.
+            for ($attempt = 0; $attempt -lt 20; $attempt++) {
+              $running = @(Get-Process -Name 'Orca' -ErrorAction SilentlyContinue)
+              if ($running.Count -gt 0) {
+                $running | Stop-Process -Force -ErrorAction SilentlyContinue
+                break
+              }
+              Start-Sleep -Milliseconds 500
+            }
+            Get-Process -Name 'orca-terminal-daemon' -ErrorAction SilentlyContinue |
+              Stop-Process -Force -ErrorAction SilentlyContinue
            $installedUninstaller = Get-ChildItem -Path "$env:LOCALAPPDATA\Programs" -Recurse -File -Filter 'Uninstall*.exe' -ErrorAction SilentlyContinue |
              Where-Object { $_.FullName -like '*Orca*' } |
              Select-Object -First 1
+            if ($null -ne $installedUninstaller) { $installedVia = 'a silent install' }
          }

          if ($null -eq $installedUninstaller) {
            $failures.Add('could not obtain the uninstaller the installer ships; neither 7-Zip nor a silent install produced it')
          } else {
-            Test-Signature 'shipped: Uninstall Orca.exe' $installedUninstaller.FullName
+            # Why this digest comparison is the point of the whole rehearsal:
+            # unlike the release job's, it hashes a file NSIS itself wrote out
+            # rather than the file the hook copied, so it is the only check that
+            # proves the shipped installer embedded the SignPath-signed bytes. On
+            # the 7-Zip route the guard above already forced equality; on the
+            # install route this is the first time it is tested.
+            if ($null -ne $expectedDigest) {
+              $shippedDigest = (Get-FileHash -LiteralPath $installedUninstaller.FullName -Algorithm SHA256).Hash.ToLowerInvariant()
+              if ($shippedDigest -ne $expectedDigest) {
+                $failures.Add("the uninstaller the installer ships is not the relayed one (via $installedVia): $shippedDigest vs $expectedDigest")
+              }
+            }
+            Test-Signature "shipped: Uninstall Orca.exe (via $installedVia)" $installedUninstaller.FullName
          }

          foreach ($relative in Get-Content 'inner-signing-list.txt') {
@@ -269,7 +269,14 @@ describe('Windows NSIS uninstaller signing', () => {

    expect(relayScripts.length).toBeGreaterThan(0)
    for (const run of relayScripts) {
-      expect(run).toContain('$env:RUNNER_TEMP')
+      // Why count occurrences rather than assert `toContain` once: a step
+      // carrying two relay paths could root the first in RUNNER_TEMP and leave
+      // the second bare-relative — which resolves against the checkout, and is
+      // exactly the shape of the defect this test exists to catch.
+      const mentions = run.match(/uninstaller-signing/g) ?? []
+      const rooted = run.match(/Join-Path \$env:RUNNER_TEMP 'uninstaller-signing/g) ?? []
+
+      expect(rooted.length, run).toBe(mentions.length)
      expect(run).not.toContain('$env:GITHUB_WORKSPACE')
    }
  })
@@ -311,6 +318,11 @@ describe('Windows NSIS uninstaller signing', () => {
    expect(stage.run).toMatch(/try \{[\s\S]*\$exportedUninstaller[\s\S]*\} catch \{/)
    expect(uninstallerBlock).toContain('::warning::Could not stage the NSIS uninstaller')
    expect(uninstallerBlock).not.toContain('throw')
+    // Explicit, so the catch does not silently depend on GitHub's
+    // $ErrorActionPreference='Stop' default for `shell: pwsh`.
+    expect(uninstallerBlock).toContain('New-Item -ItemType Directory -Force -Path (Split-Path')
+    expect(uninstallerBlock).toMatch(/New-Item[^\r\n]*-ErrorAction Stop/)
+    expect(uninstallerBlock).toMatch(/Copy-Item[^\r\n]*-ErrorAction Stop/)
    // The upload it gates still keys off this step, so the catch is load-bearing.
    expect(stepNamed(releaseSteps(), 'Upload unsigned inner binaries for SignPath').if).toContain(
      "steps.stage-inner.outcome == 'success'"
@@ -373,6 +385,35 @@ describe('Windows NSIS uninstaller signing', () => {
    expect(verify.run).toContain("-ArgumentList '/S'")
  })

+  // This workflow is the merge gate, so it must not be able to fail on its own
+  // artefact: 7-Zip's NSIS handler is unreliable enough that its output has to
+  // be corroborated before a signature verdict is drawn from it.
+  it('never lets an unreliable extract fail the rehearsal', () => {
+    const steps = readWorkflow('.github/workflows/windows-signing-rehearsal.yml').jobs.rehearse
+      .steps
+    const verify = stepNamed(steps, 'Verify signatures end to end')
+
+    // The 7-Zip route is only trusted when it reproduces the relayed bytes;
+    // otherwise it falls through to the install route rather than failing.
+    expect(verify.run).toContain(
+      'Write-Host "7-Zip\'s NSIS output did not match the relayed digest; falling back to a silent install."'
+    )
+    expect(verify.run).toMatch(/\$installedUninstaller = \$null\r?\n\s*\}/)
+
+    // The comparison that is not tautological: a file NSIS wrote out, against
+    // the digest the sign hook recorded.
+    expect(verify.run).toContain('$shippedDigest -ne $expectedDigest')
+    expect(verify.run).toContain('the uninstaller the installer ships is not the relayed one')
+
+    // An installer that prompts must not hang to the 360-minute job cap, and
+    // the app it launches must not outlive the step holding install-dir handles.
+    expect(verify.run).toContain('-PassThru')
+    expect(verify.run).toContain('$installerProcess.WaitForExit(300000)')
+    expect(verify.run).toContain('the silent install did not exit within 5 minutes')
+    expect(verify.run).toMatch(/for \(\$attempt = 0; \$attempt -lt 20; \$attempt\+\+\)/)
+    expect(verify.run).toContain("Get-Process -Name 'orca-terminal-daemon'")
+  })
+
  it('wires the electron-builder sign hook that the relay depends on', () => {
    const require = createRequire(import.meta.url)
    const configPath = resolve(projectDir, 'config/electron-builder.config.cjs')