Skip to content

desktop: keep the mac signing cert off the windows build - #1797

Merged
RhysSullivan merged 1 commit into
mainfrom
fix/windows-updater-signing-scope
Aug 28, 2026
Merged

desktop: keep the mac signing cert off the windows build#1797
RhysSullivan merged 1 commit into
mainfrom
fix/windows-updater-signing-scope

Conversation

@RhysSullivan

Copy link
Copy Markdown
Collaborator

Release-pipeline change — needs explicit review before merge; next Windows release after merge should be manually verified to auto-update.

Windows desktop auto-updates fail on every release with:

New version is not signed by the application owner: publisherNames: Developer ID Application: ...

An Apple certificate name has no business appearing in a Windows Authenticode check. It gets there because the publish workflow leaks the macOS signing certificate into the Windows build.

Cause

CSC_LINK reads like a macOS variable, but in electron-builder it is the cross-platform certificate variable. WinPackager falls back to it whenever WIN_CSC_LINK is unset:

// app-builder-lib/out/platformPackager.js
getCscLink(extraEnvName) {
  const envValue = chooseNotNull(extraEnvName == null ? null : process.env[extraEnvName], process.env.CSC_LINK);
  return chooseNotNull(chooseNotNull(this.info.config.cscLink, this.platformSpecificBuildOptions.cscLink), envValue);
}

publish-desktop.yml set CSC_LINK / CSC_KEY_PASSWORD in the Build desktop distributables step unconditionally, and the build matrix includes a windows-latest leg. So the Windows leg received the Apple Developer ID .p12.

From there the failure is fully determined, and it does not depend on whether signing itself succeeded:

  1. win.verifyUpdateCodeSignature defaults to true, so WinPackager.isForceCodeSigningVerification is true.
  2. PublishManager.getAppUpdatePublishConfiguration therefore asks for a publisher name and writes it into app-update.yml.
  3. WindowsSignToolManager.computedPublisherName has no explicit publisherName configured, so it falls through to the certificate's subject Common Name — parsed out of whatever .p12 was supplied, with no check that it is a Windows code-signing certificate. That yields Developer ID Application: ....
  4. On the client, NsisUpdater.doDownloadUpdate calls verifySignature, which shells out to Get-AuthenticodeSignature. The installer either carries no signature or one chaining to Apple's CA, which Windows does not trust for code signing. data.Status !== 0, the name comparison is never reached, and the updater throws ERR_UPDATER_INVALID_SIGNATURE with the expected publisher name in the message.

Worth noting for anyone reading the error: the name printed after publisherNames: is what the updater expected, not what it found. It reads like a mismatch when the real problem is that there is no trusted signature at all. The observed certificate is in the raw info: JSON that follows.

forceCodeSigning defaults to false, so a failed Windows signing attempt never failed the build — the release shipped an installer that could not update itself.

The fix

Two changes, both making the build honest about the fact that we have no Windows certificate.

1. .github/workflows/publish-desktop.yml — scope the certificate variables to the mac legs:

CSC_LINK: ${{ matrix.platform == 'mac' && secrets.CSC_LINK || '' }}
CSC_KEY_PASSWORD: ${{ matrix.platform == 'mac' && secrets.CSC_KEY_PASSWORD || '' }}

Empty string is the documented "no certificate" value, not a hack — WindowsSignToolManager short-circuits on it explicitly:

const cscLink = this.packager.getCscLink("WIN_CSC_LINK");
if (cscLink == null || cscLink === "") {
  return Promise.resolve(null);
}

This is also already the fork/local behaviour today, where the secrets are unset and evaluate to ''. The mac legs are unaffected: on matrix.platform == 'mac' the expression yields exactly the previous value.

APPLE_API_KEY / APPLE_API_KEY_ID / APPLE_API_ISSUER are deliberately left unscoped — they are only read by notarytool on darwin and are inert elsewhere. Scoping them would have been diff noise in a release workflow.

2. apps/desktop/electron-builder.config.ts — state the invariant in config:

win: {
  target: ["nsis"],
  verifyUpdateCodeSignature: false,
},

Change 1 alone fixes the bug: with no certificate, no publisherName is written, and NsisUpdater.verifySignature returns null early and skips verification entirely. Change 2 is the durable guard — it stops the manifest from ever claiming a publisher we cannot back up, so a future edit that reintroduces a certificate variable globally cannot silently break updates again.

Two details that make this safe rather than a blunt "turn off security":

  • It does not disable signing. isForceCodeSigningVerification is read in exactly one place in app-builder-libPublishManager.js:204, computing the updater manifest. Nothing in the signing path consults it.
  • Absent and empty are not equivalent here. publisherName: [] would fail every update ([] == null is false, the match loop never runs); omitting the key is what skips verification. This change omits the key.

Verification

This cannot be end-to-end verified locally, and I want to be plain about that. Proving the fix requires building a signed-manifest NSIS installer on a Windows runner, publishing it to a real GitHub release, and having a previously-installed Windows client download and accept it. No part of that is reproducible on this machine, and no test in this repo covers the release pipeline.

What I did validate:

  • .github/workflows/publish-desktop.yml parses as YAML, and the Build desktop distributables env block resolves as intended across all four matrix legs.
  • actionlint on the changed workflow reports no new findings. The one finding it does report — the blacksmith-4vcpu-ubuntu-2404 custom runner label — is present identically on main and is unrelated.
  • bun run typecheck passes in apps/desktop, confirming verifyUpdateCodeSignature is a real field on WindowsConfiguration in the pinned version rather than a silently-ignored key. This matters: win.publisherName from older electron-builder majors is not a valid key in the installed version and would have been silently dropped.
  • The mechanism above was read out of the actually-installed dependencies rather than the docs — electron-updater 6.8.3 and app-builder-lib 26.8.1 as pinned in the lockfile. Every quoted snippet is from node_modules, cross-checked against upstream master.

The real verification is the next Windows release. After merging, publish a release, install it on Windows, then publish a subsequent one and confirm the client updates itself. Until that happens this fix is reasoned, not demonstrated.

Long-term: get a real Windows certificate

Shipping unsigned Windows installers is the honest description of where we are, not a good end state. Unsigned builds still trigger SmartScreen warnings on download and install, and unverified updates mean a compromised release asset would be accepted by clients. Signature verification is genuinely worth having; we just cannot claim it without a certificate.

The recommended path as of now is Azure Trusted Signing (recently rebranded Azure Artifact Signing). Microsoft runs the identity validation and holds the key material, so there is no EV hardware token or HSM to buy and manage, and no three-year certificate to rotate by hand. It is markedly cheaper and less operationally painful than a traditional EV certificate.

What adopting it would take:

  • An Azure subscription with a Trusted Signing account, plus organisation identity validation (a business-verification step with lead time — worth starting early).
  • A certificate profile, and an Entra ID app registration granted the "Trusted Signing Certificate Profile Signer" role.
  • Repo secrets AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, scoped to the Windows leg exactly as the Apple secrets are now scoped to mac.
  • Config on win.azureSignOptions: endpoint (region-matched), codeSigningAccountName, certificateProfileName, and publisherName.
  • Removing verifyUpdateCodeSignature: false in the same change, or verification stays off for a signed build.

Two sharp edges to plan around when we do it:

  • On the Azure path publisherName is required and has no certificate-derived fallback — WindowsSignAzureManager.computedPublisherName returns null when it is unset, which silently disables update verification rather than erroring. It must be set explicitly and must match the certificate subject.
  • The pinned app-builder-lib 26.8.1 implements Azure signing by installing a PowerShell module (TrustedSigning) and calling Invoke-TrustedSigning, so it needs the Windows runner and build-time network access to Azure. Upstream has since moved to a signtool /dlib approach and calls the PowerShell route deprecated, so an electron-builder bump is worth pairing with this work.

A traditional OV/EV certificate from a CA is the alternative. It is more expensive, requires managing a hardware token in CI, and for a fresh OV certificate SmartScreen reputation still has to be earned from zero — Trusted Signing is the better trade.

Residual risks

  • Unsigned Windows builds remain unsigned. This change makes the pipeline honest, it does not make it secure. SmartScreen warnings persist, and update-payload authenticity now rests on HTTPS and the GitHub release plus the SHA512 in latest.yml, not on a code signature.
  • verifyUpdateCodeSignature: false is a footgun if forgotten. If a Windows certificate is added later and this line is left behind, builds will be signed but updates will not be verified — failing open, silently. The inline comment flags it; the failure mode is no worse than today's post-fix state, which is why it is preferred over leaving the manifest able to claim a publisher we cannot honour.
  • Clients already on a broken version are not rescued by this. Existing Windows installs are stuck on whatever they have; the fix only applies from the next release forward, and those users may need a manual reinstall. Worth confirming how many are affected before deciding whether an announcement is needed.
  • Unverified for the mac legs beyond reading the expression. I did not run a mac release. The expression is value-identical on matrix.platform == 'mac', but a release-pipeline reviewer should confirm the mac signing and notarization steps still behave as expected on the next release.

CSC_LINK is electron-builder cross-platform, not mac-only: WinPackager falls
back to it when WIN_CSC_LINK is unset. Setting it unconditionally handed the
Apple Developer ID cert to the windows leg, which wrote its subject CN into
app-update.yml as publisherName. Every Windows client then rejected the update
it downloaded because the installer carries no signature Windows trusts.

Scope CSC_LINK/CSC_KEY_PASSWORD to the mac legs, and set
win.verifyUpdateCodeSignature so the updater manifest stops claiming a
publisher the build cannot back up.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing c11bb87 Commit Preview URL

Branch Preview URL
Aug 28 2026, 03:34 AM

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Torn down — the PR is closed.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud c11bb87 Aug 28 2026, 03:35 AM

@pkg-pr-new

pkg-pr-new Bot commented Aug 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1797

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1797

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1797

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1797

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1797

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1797

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1797

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1797

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1797

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1797

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1797

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1797

executor

npm i https://pkg.pr.new/executor@1797

commit: c11bb87

@RhysSullivan
RhysSullivan marked this pull request as ready for review August 28, 2026 05:51
@RhysSullivan
RhysSullivan merged commit a3a73e0 into main Aug 28, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant