[only21mil]

Problem

Windows updates can fail at 100% when electron-updater's PowerShell Get-AuthenticodeSignature check exceeds 20 seconds. Endpoint security scanning the new installer can cause that timeout. The reporter in #20345 reran the same command on the same file and it passed in 4.2 seconds.

Change

Wrap the Windows NSIS updater's verifier before downloading. The first check is unchanged from upstream. Retry once if it rejects with killed === true, Node's timeout-kill error shape, using the same verifier, publisher names and installer path.

electron-updater 6.8.9 returns null both for a valid signature and when its PowerShell capability probe fails and it skips validation, logging "Ignoring signature validation". During the retry, a forwarding logger detects that warning, then the original logger is restored. A skipped retry fails closed. A clean null passes; signature or publisher rejection, other errors and a second timeout still fail. The first check's upstream skip behavior is unchanged. There is no checksum-only fallback added by this PR.

A WeakSet prevents stacked wrappers on repeat downloads. macOS and Linux lack this verifier property. The change is 57 non-test lines, 147 total.

Scope and approval

Grok's October 16 triage on behalf of Julius accepted #20345 as a bug. It left the direction open between retrying after a timeout and in-process verification. This PR takes the retry option; no maintainer has agreed to this implementation. In-process verification and clearer timeout reporting remain separate work.

Prior PRs: none found on this issue, open or closed. No code is reused from another PR.

Verification

The following Linux checks are retained from the held verification record; this Windows run did not rerun them.

From apps/desktop, with a stand-in NSIS verifier:

vp test run src/electron/ElectronUpdater.test.ts
vp test run src/electron/ElectronUpdater.test.ts src/updates

The original tests on main 3ff5b3c82a, test commit 1c1a24d0e6, give 2 failures and 5 passes. With the new tests on badd70c9e5, the skipped-retry regression fails, 8 pass. At 6ea17522cb, all 9 pass; the suite passes 100 tests across 7 files. Removing the WeakSet guard makes the reused-verifier test fail with 6 calls instead of 4.

Changed-file vp lint and vp fmt --check, desktop tsc-rs --noEmit and vp run knip:check passed in that held record.

The held record also includes a standalone Linux x64 Node v24.22.0 check using a 2-second execFile limit and a 5-second stand-in command. The actual Linux timeout reports killed=true, SIGTERM, null exit code and empty stderr; an exit-1 failure reports killed=false and does not match the retry. This checks Node error shape on Linux, not PowerShell behavior on Windows.

On October 18, 2026, a real Windows 11 Enterprise Evaluation VM booted, accepted login and returned guest-agent preflight results for Windows build 26300, PowerShell 5.1, Node v24.22.0, pnpm 11.10.0 and Vite+ 1.3.0. The host candidate worktree was clean at 6ea17522cb4648a6df730e90494b5721bc42a1ff.

The real Win11 x64 unsigned NSIS build of 6ea17522cb4648a6df730e90494b5721bc42a1ff succeeded with version 0.1.1-local.21273. A second unsigned NSIS package, version 0.1.2, reused that build with the supported --skip-build --mock-updates route. Both artifact commands exited 0. The original dependency installation succeeded; its first artifact attempt failed for missing toolchain prerequisites, which were then installed before the successful artifact retry.

The candidate app was installed and launched in the VM. Its real electron-updater flow found and downloaded 0.1.2 from the repository's scripts/mock-update-server.ts running on guest localhost port 31273. The old blockmap returned 404, so electron-updater fell back to a successful full installer download. The app's restart confirmation was driven through the UI. Logs record quitAndInstall and the actual NSIS installer launch. The app relaunched, and installed executable metadata and the uninstall registry changed from 0.1.1-local.21273 to 0.1.2. Build and updater logs, installer hashes and executable metadata are kept locally and available on request.

Unsigned build, localhost feed. The app asks to install update 0.1.2 and restart.

Unsigned build, localhost feed. The app relaunched after installation. This screen has no version label; the executable and registry receipts below establish 0.1.2.

Unsigned build, localhost feed. Rendering of saved before/after JSON receipts, not a terminal capture. FileVersion changes to 0.1.2, both executable SHA-256 values are shown, the registry reports 0.1.2, and Authenticode remains NotSigned.

This is unsigned build and local update evidence only. Both installers report NotSigned. The supported unsigned build's app-update.yml has no publisherName; pinned electron-updater 6.8.9 returns before calling its Windows signature verifier in that configuration. This run therefore did not exercise PowerShell signature verification, timeout or retry. The signed build's four Azure Trusted Signing settings were absent at preflight, so production signed signature-retry verification remains blocked on an authorized signing configuration and suitable signed test setup. No signing identity was generated, validation setting disabled or candidate source changed.

Not checked:

• A signed Windows update, PowerShell signature timeout/retry or endpoint security interaction. These remain blocked on a configured signing route and a suitable signed update test setup.
• Updates on macOS and Linux. The verifier-property check excludes them by design; they were not run.
• electron-updater versions beyond the pinned 6.8.9. Skip detection keys on its "Ignoring signature validation" warning text. A future rewording could let a skipped retry resolve null again, so upgrades need this revisited.

Credits

Thanks to the reporter of #20345 for the diagnosis, repeat-run timing and suggested retry option.

Production fix and tests written with Claude Opus 5.5 via claude_auto using the Claude Code harness; PR description and images by GPT-6.1 Sol using the Codex harness, both in T3 Code.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This production change wraps Windows update code-signature verification, retries timeout-killed PowerShell checks, and adds fail-closed handling for skipped validation. Because it changes a security-sensitive update-integrity boundary on an existing download path, human review is warranted despite the focused scope and targeted tests.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: c12d5339-874f-4216-a063-80fac6efba14

📥 Commits

Reviewing files that changed from the base of the PR and between b06b0df and 303eac0.

📒 Files selected for processing: 2

• apps/desktop/src/electron/ElectronUpdater.test.ts
• apps/desktop/src/electron/ElectronUpdater.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: b86854e0-9bad-4d76-a5fb-2a6b33d7dd2c

📥 Commits

Reviewing files that changed from the base of the PR and between 6ea1752 and b06b0df.

📒 Files selected for processing: 2

• apps/desktop/src/electron/ElectronUpdater.test.ts
• apps/desktop/src/electron/ElectronUpdater.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

The updater wraps the Windows NSIS signature verifier before downloading an update. If verification fails with a killed-process timeout, it retries once and checks whether the retry skipped signature validation. Tests cover retry limits and failure cases.

Changes: Windows signature verification retry

Layer / File(s)	Summary
Retry signature verification before download apps/desktop/src/electron/ElectronUpdater.ts, apps/desktop/src/electron/ElectronUpdater.test.ts	The updater installs a wrapper before downloading. It retries verification once after a killed-process timeout, forwards logger calls during retry, and restores the original logger. Tests cover invalid signatures, other errors, repeated timeouts, per-download retry limits, and a retry that returns null after an upstream capability-probe failure.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ElectronUpdater
  participant NSISSignatureVerifier
  participant UpdaterLogger
  ElectronUpdater->>NSISSignatureVerifier: Verify update signature
  NSISSignatureVerifier-->>ElectronUpdater: Reject with killed timeout
  ElectronUpdater->>NSISSignatureVerifier: Retry verification once
  NSISSignatureVerifier->>UpdaterLogger: Log skipped-validation warning when applicable
  NSISSignatureVerifier-->>ElectronUpdater: Return publisher, null, or failure
  ElectronUpdater->>UpdaterLogger: Check for skipped-validation warning

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to b06b0

No actionable merge-blocking issue was found in the Windows signature-check retry. Native Windows verification and human approval remain required before release.

Security architecture risk: 🔵 Low · up to b06b0

The retry is bounded and preserves the publisher and installer arguments. Invalid results, repeated timeouts, and detected skipped checks still fail. No introduced verification bypass was established, but interruption behavior and reliance on dependency warning text leave limited uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed trust decision concerns acceptance of downloaded Windows installers by the local desktop updater. The wrapper does not change publisher identities or installer paths and is not installed when the updater lacks the NSIS verification hook.

Trust Boundaries and Controls

• inferred — No retry-based signature bypass was established. Invalid results remain invalid, retry exceptions propagate, and a detected skipped check becomes a rejection. Earlier dependency source inspections identify both known skip warnings as matching the guard; installed dependency code was unavailable for fresh validation.

Resilience and Maintainability Implications

• inferred — Normal application serialization limits overlapping logger replacement. Interruption remains unresolved: DesktopUpdates restores state and releases its reservation, but the adapter supplies no explicit native cancellation, and logger restoration waits for the verifier promise to settle. Overlap or stranded logging state was not established as a reachable security failure.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description covers the problem, change, verification, limitations, and agent attribution in detail. However, the required Scope and approval section does not provide explicit maintainer approval o…	Add a link to an approval comment from a maintainer that explicitly approves the retry implementation and its scope. If the fix qualifies as an obvious bug exemption, explain why no prior implementation approval is required. Then update the…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: retrying a timed-out Windows update signature check once.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description covers the problem, change, verification, limitations, and agent attribution in detail. However, the required Scope and approval section does not provide explicit maintainer approval of the implementation direction and scope, and it states that no maintainer agreed to this implementation.

Resolution

Add a link to an approval comment from a maintainer that explicitly approves the retry implementation and its scope. If the fix qualifies as an obvious bug exemption, explain why no prior implementation approval is required. Then update the Scope and approval section accordingly.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[juliusmarminge]

Taking over this one as maintainer for batch desktop-app-06. I’ll preserve the contributor commits, merge current main, reduce the tests, and independently review the timeout retry and signature failure behavior. Real Windows updater verification and human approval remain required; this batch will not merge it.

[juliusmarminge]

Reverting this in #22222 together with the other desktop fixes the takeover campaign merged yesterday. Nothing was wrong with the review of this change; it was merged on CI and code audits only, and after #20907 from the same batch broke existing installs the maintainer decided that desktop changes do not ship without being run on a real desktop build. To bring it back: open a fresh PR from the original commit, run it on a real build that already has user state on each platform it touches, and put the before/after proof in the body.