Skip to content

ci(app): electron builder Windows signing workaround#18479

Merged
y3rsh merged 6 commits intochore_release-8.5.0from
work-aroundapp-build-both-as-release
Jun 2, 2025
Merged

ci(app): electron builder Windows signing workaround#18479
y3rsh merged 6 commits intochore_release-8.5.0from
work-aroundapp-build-both-as-release

Conversation

@y3rsh
Copy link
Copy Markdown
Member

@y3rsh y3rsh commented Jun 2, 2025

Overview

There’s an issue in Electron Builder/TrustedSigning that causes an error when installing the required tooling for code signing. Windows signing fails. I don't know how the build I did against edge PR ever even worked.
electron-userland/electron-builder#9076

@y3rsh is ok with this pain using a beta and does not want to fall back to using DigiCert. BUT it will require diligence.

Solution

The work around comes from
electron-userland/electron-builder#9076 (comment)

  1. Before the electron builder runs, install TrustedSigning at the specific version Electron Builder wants
  2. Invoke this tool on a single dummy.exe just like Electron Builder will be invoking it on our files.
  3. The dependencies needed for this type of signing are automatically installed.
  4. The signing fails as expected but all the tools for signing are ready for electron builder in the next step, sidestepping the issue.

Test Plan and Hands on Testing

Review requests

I considered directly trying to install the dependencies... but I think it seems best to leave that to TrustedSigning. So the invoke is necessary.

        Build tools package installed: False
	Trusted signing package installed: False
	Sign CLI package installed: False
	Installing required dependencies.
		Found existing package source: https://www.nuget.org/api/v2/
		Installing package: Microsoft.Windows.SDK.BuildTools 10.0.22621.3233
		Installing package: Microsoft.Trusted.Signing.Client 1.0.53
		Installing package: sign 0.9.1-beta.2[44]

@y3rsh y3rsh self-assigned this Jun 2, 2025
@y3rsh y3rsh requested a review from a team as a code owner June 2, 2025 13:16
Copy link
Copy Markdown
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Hopefully it's smooth sailing ⛵

@y3rsh y3rsh merged commit 26c3bf0 into chore_release-8.5.0 Jun 2, 2025
19 checks passed
@y3rsh y3rsh deleted the work-aroundapp-build-both-as-release branch June 2, 2025 14:53
sfoster1 added a commit that referenced this pull request Jun 30, 2025
sfoster1 added a commit that referenced this pull request Jul 1, 2025
This reverts #18450 and #18479 to revert to digicert signing for our
windows builds.

The digicert certificate has the Common Name "Opentrons Labworks Inc."
and the ATS cert has the common name "OPENTRONS LABWORKS INC.". These
were both determined automatically by the CA from our identity
submissions, as is apparently required in the code signing cert baseline
requirements. Why are they different? A mystery for the ages.

In either case, electron-updater requires that _if_ you specify a
`publisherName` in your `app-update.yml` (which we do specifically on
windows, since it is generated from our electron-builder config and on
windows we set it because nsis packager wants it for doing signing in
the first place) _then_ the autoupdate package that will be installed
must have a CSC CN exactly matching an entry in `publisherName` or the
update will fail. Therefore updates in between <=8.4.1 and >=8.5.0 would
fail if we switched to ATS.

Instead, we'll switch back to digicert for now; we'll build the new CN
into our publisher names; and then whenever we're confident enough
people are on >=8.5.0 and therefore have the new publisher names, we'll
switch over again (we can't switch immediately because we don't do
incremental updates, just full overwrites, so the intermediate update
state would go away).

This is upsetting.

## Testing
- [x] the signing has to work again, which is never guaranteed given the
shonky state of dco integration
- [x] we should make sure we can update from 8.4.1 to this by making
sure the CN of the digital signature on the build from this pr is
exactly `Opentrons Labworks Inc.` (and updating to it in the resulting
alpha)
- [x] we should make sure we can update from this to something signed
with the new cert (by mucking around with the latest-alpha or something?
or just checking that the app-update.yml in the app's install directory
has both names)

Supercedes #18785 for build branch name reasons.
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.

2 participants