Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Sign with HSM #1498

Merged
merged 5 commits into from
Nov 6, 2023
Merged

chore: Sign with HSM #1498

merged 5 commits into from
Nov 6, 2023

Conversation

felixrieseberg
Copy link
Member

With this PR, we'll sign using Digicert Keylocker. I've already removed the old (now revoked) certificate and added the required environment variables.

Things to comment on in this PR:

  • @dsanders11 I saw that you added bash.exe as the default and I'm sorry to say that Windows subsystem stuff like msiexec are simply too hard to actually get right in bash. In bash, you wait for the installation of an MSI by looping through the MSDOS tasklist command and parsing the output, trying to figure out if it's still running. In Powershell, you just pipe to Wait-Process, which is a whole lot less brittle.

    I'm going with Powershell instead! We don't need to change our defaults, but here, I think it's the right choice for Windows.

  • SIGN_OR_DIE is now the default as signWithParams will throw if signing doesn't work. With it, I've replaced add-windows-cert.ts with a ps1 oneliner.

Confirmed that the builds work, too! Test run here: https://app.circleci.com/pipelines/github/electron/fiddle/1188/workflows/970eceda-bab0-47b0-b656-8cc3ca652704/jobs/4602/steps

@felixrieseberg felixrieseberg changed the title Sign with HSM chore: Sign with HSM Nov 6, 2023
@coveralls
Copy link

Coverage Status

coverage: 87.257%. remained the same
when pulling 5ec9c04 on felixr-windows-sign
into f60eb4e on main.

@felixrieseberg felixrieseberg merged commit c156099 into main Nov 6, 2023
12 of 13 checks passed
@felixrieseberg felixrieseberg deleted the felixr-windows-sign branch November 6, 2023 17:30
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.

3 participants