mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
Address CodeRabbit review + fix contract test for inner signing
- Update package-electron-runtime-contract test: the old test pinned the pre-inner-signing layout (publish immediately after installer verify); now pins the new fail-open contract (inner verify + evidence upload between verify and publish, warn-only gate, continue-on-error chain). - Budget SignPath waits under the 360-minute hosted-job cap: explicit timeout-minutes on build, inner-signing wait 4h -> 1h (missing the window falls through to today's unsigned-inner flow). - Wrap the evidence gate in try/catch so unexpected script errors cannot block a release while the gate is warn-only. - Rehearsal workflow: persist-credentials false, same 1h inner wait.
This commit is contained in:
Jinwoo Hong
@@ -719,6 +719,10 @@ jobs:
|
||||
~/.cache/electron-builder
|
||||
|
||||
runs-on: ${{ matrix.os }}
|
||||
# Why: hosted runners hard-cap jobs at 6h; the Windows SignPath waits
|
||||
# (1h inner + 4h installer) are budgeted to fit under this with the
|
||||
# build itself, so a slow approval can't kill the job mid-flow.
|
||||
timeout-minutes: 360
|
||||
|
||||
permissions:
|
||||
actions: read
|
||||
@@ -1072,8 +1076,10 @@ jobs:
|
||||
Invoke-RestMethod -Method Post -Uri $env:SLACK_WEBHOOK_URL -ContentType 'application/json' -Body $payload
|
||||
|
||||
# Why gate on the notify outcome too: if nobody was told to approve,
|
||||
# don't hold the release for the 4-hour approval window — fall through
|
||||
# and ship like today instead.
|
||||
# don't hold the release for the approval window — fall through and
|
||||
# ship like today instead. The 1h wait (vs the installer's 4h) keeps
|
||||
# both waits plus the build inside the 360-minute job cap; missing it
|
||||
# falls through to today's unsigned-inner flow rather than blocking.
|
||||
- name: Download signed inner binaries from SignPath
|
||||
id: download-signed-inner
|
||||
if: matrix.platform == 'win' && steps.submit-inner-signing.outcome == 'success' && steps.notify-inner-signing.outcome == 'success'
|
||||
@@ -1089,7 +1095,7 @@ jobs:
|
||||
-SigningRequestId $env:SIGNPATH_REQUEST_ID `
|
||||
-OutputArtifactPath signed-inner.zip `
|
||||
-Force `
|
||||
-WaitForCompletionTimeoutInSeconds 14400
|
||||
-WaitForCompletionTimeoutInSeconds 3600
|
||||
|
||||
New-Item -ItemType Directory -Path signed-inner -Force
|
||||
Expand-Archive -Path signed-inner.zip -DestinationPath signed-inner -Force
|
||||
@@ -1305,40 +1311,48 @@ jobs:
|
||||
exit 0
|
||||
}
|
||||
|
||||
$report = New-Object System.Collections.Generic.List[string]
|
||||
$failures = New-Object System.Collections.Generic.List[string]
|
||||
# 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.
|
||||
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'
|
||||
New-Item -ItemType Directory -Path inner-evidence-extract -Force | Out-Null
|
||||
& $7za x 'dist/orca-windows-setup.exe' '-oinner-evidence-extract' -y | Out-Null
|
||||
# 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'
|
||||
New-Item -ItemType Directory -Path inner-evidence-extract -Force | Out-Null
|
||||
& $7za x 'dist/orca-windows-setup.exe' '-oinner-evidence-extract' -y | Out-Null
|
||||
|
||||
$root = Resolve-Path 'inner-evidence-extract'
|
||||
foreach ($relative in Get-Content 'inner-signing-list.txt') {
|
||||
$path = Join-Path $root $relative
|
||||
if (-not (Test-Path $path)) {
|
||||
$failures.Add("missing from installer payload: $relative")
|
||||
continue
|
||||
$root = Resolve-Path 'inner-evidence-extract'
|
||||
foreach ($relative in Get-Content 'inner-signing-list.txt') {
|
||||
$path = Join-Path $root $relative
|
||||
if (-not (Test-Path $path)) {
|
||||
$failures.Add("missing from installer payload: $relative")
|
||||
continue
|
||||
}
|
||||
$signature = Get-AuthenticodeSignature -FilePath $path
|
||||
$subject = if ($null -eq $signature.SignerCertificate) { '<none>' } else { $signature.SignerCertificate.Subject }
|
||||
$line = "{0,-14} {1} <{2}>" -f $signature.Status, $relative, $subject
|
||||
$report.Add($line)
|
||||
Write-Host $line
|
||||
if ($signature.Status -ne 'Valid' -or $subject -notlike '*CN=SignPath Foundation*') {
|
||||
$failures.Add("not signed by SignPath Foundation: $relative ($($signature.Status), $subject)")
|
||||
}
|
||||
}
|
||||
$signature = Get-AuthenticodeSignature -FilePath $path
|
||||
$subject = if ($null -eq $signature.SignerCertificate) { '<none>' } else { $signature.SignerCertificate.Subject }
|
||||
$line = "{0,-14} {1} <{2}>" -f $signature.Status, $relative, $subject
|
||||
$report.Add($line)
|
||||
Write-Host $line
|
||||
if ($signature.Status -ne 'Valid' -or $subject -notlike '*CN=SignPath Foundation*') {
|
||||
$failures.Add("not signed by SignPath Foundation: $relative ($($signature.Status), $subject)")
|
||||
}
|
||||
}
|
||||
|
||||
Set-Content -Path 'inner-signing-evidence.txt' -Value ($report -join "`n")
|
||||
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'."
|
||||
} else {
|
||||
Write-Host "All $((Get-Content 'inner-signing-list.txt').Count) inner binaries in the shipped installer are signed by SignPath Foundation."
|
||||
Set-Content -Path 'inner-signing-evidence.txt' -Value ($report -join "`n")
|
||||
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'."
|
||||
} else {
|
||||
Write-Host "All $((Get-Content 'inner-signing-list.txt').Count) inner binaries in the shipped installer are signed by SignPath Foundation."
|
||||
}
|
||||
} catch {
|
||||
if ($required) { throw }
|
||||
Write-Host "::warning::Windows inner-binary evidence gate errored: $_ (fail-open, issue #7785)."
|
||||
}
|
||||
|
||||
- name: Upload Windows inner signing evidence
|
||||
|
||||
@@ -41,6 +41,9 @@ jobs:
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
# Why: nothing here pushes; keep the token out of .git/config.
|
||||
persist-credentials: false
|
||||
|
||||
- name: Setup pnpm
|
||||
uses: pnpm/action-setup@v6
|
||||
@@ -180,7 +183,7 @@ jobs:
|
||||
-SigningRequestId $env:SIGNPATH_REQUEST_ID `
|
||||
-OutputArtifactPath signed-inner.zip `
|
||||
-Force `
|
||||
-WaitForCompletionTimeoutInSeconds 14400
|
||||
-WaitForCompletionTimeoutInSeconds 3600
|
||||
New-Item -ItemType Directory -Path signed-inner -Force
|
||||
Expand-Archive -Path signed-inner.zip -DestinationPath signed-inner -Force
|
||||
|
||||
|
||||
@@ -221,7 +221,7 @@ describe('Electron runtime package contract', () => {
|
||||
expect(installRun).not.toMatch(/throw\s+\$_/)
|
||||
})
|
||||
|
||||
it('temporarily allows publishing Windows after verifying the signed installer only', () => {
|
||||
it('verifies Windows inner binary signatures fail-open before publishing', () => {
|
||||
const releaseWorkflow = readFileSync(
|
||||
join(projectDir, '.github/workflows/release-cut.yml'),
|
||||
'utf8'
|
||||
@@ -230,12 +230,37 @@ describe('Electron runtime package contract', () => {
|
||||
const steps = parsedWorkflow.jobs.build.steps
|
||||
const stepNames = steps.map((step) => step.name)
|
||||
const outerVerifyIndex = stepNames.indexOf('Verify signed Windows installer')
|
||||
const innerVerifyIndex = stepNames.indexOf('Verify signed Windows inner executable')
|
||||
const innerVerifyIndex = stepNames.indexOf('Verify Windows inner binary signatures')
|
||||
const evidenceIndex = stepNames.indexOf('Upload Windows inner signing evidence')
|
||||
const publishIndex = stepNames.indexOf('Publish signed Windows release artifacts')
|
||||
|
||||
expect(outerVerifyIndex).toBeGreaterThan(-1)
|
||||
expect(innerVerifyIndex).toBe(-1)
|
||||
expect(publishIndex).toBe(outerVerifyIndex + 1)
|
||||
expect(innerVerifyIndex).toBe(outerVerifyIndex + 1)
|
||||
expect(evidenceIndex).toBe(innerVerifyIndex + 1)
|
||||
expect(publishIndex).toBe(evidenceIndex + 1)
|
||||
|
||||
// Why fail-open: unsigned inner binaries must warn, not block, until the
|
||||
// flow is proven on a real release (issue #7785). Flip this to 'true'
|
||||
// together with the workflow env to make the gate required.
|
||||
expect(steps[innerVerifyIndex].env.ORCA_WINDOWS_INNER_SIGNATURE_REQUIRED).toBe('false')
|
||||
|
||||
// Why: every step in the inner-signing chain must be unable to fail the
|
||||
// release — a SignPath outage or timeout falls through to today's
|
||||
// unsigned-inner flow instead of blocking the cut.
|
||||
const innerChainStepNames = [
|
||||
'Stage unsigned inner PE files for signing',
|
||||
'Upload unsigned inner binaries for SignPath',
|
||||
'Submit inner binaries signing request',
|
||||
'Notify Slack that inner-binary signing is waiting for approval',
|
||||
'Download signed inner binaries from SignPath',
|
||||
'Restore signed inner binaries into unpacked app',
|
||||
'Rebuild NSIS installer from signed unpacked app'
|
||||
]
|
||||
for (const stepName of innerChainStepNames) {
|
||||
const step = steps[stepNames.indexOf(stepName)]
|
||||
expect(step, stepName).toBeDefined()
|
||||
expect(step['continue-on-error'], stepName).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('publishes both Linux release matrix entries', () => {
|
||||
|
||||
Reference in New Issue
Block a user