-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Support for Azure Trusted Signing in Electron Builder #8276
Comments
I guess a lot of those powershell commands could be copy-pasted/translated into electron-builder's signing workflow, but if any updates happen to that github action, they won't be propagated into electron-builder without someone opening an issue here. Would that be a problem? |
We're also interested in this. There is a nice guide for implementation of trusted EV code signing using Azure here: https://melatonin.dev/blog/code-signing-on-windows-with-azure-trusted-signing/ Basically, we'd need to call the action multiple times:
Currently, it seems that steps 1 and 2 are done in one action and therefore it's not possible to execute code signing after each of the steps using the GitHub action, right? |
From the github action code it looks like a powershell script that installs the TrustedSigning module and then invokes it with appropriate params is all that is required: Install-Module -Name TrustedSigning -RequiredVersion 0.4.1 -Force -Repository PSGallery
Invoke-TrustedSigning @params This should be possible to add to electron-builder with some extra env vars? |
I quickly put together a basic copy of the action code for an Basically, we skip the signing stage for the regular signing process. Instead, we leverage the Would you be willing to do some investigating from there? Not sure if the params need to be passed in as a var or as a literal string. I don't have any azure account to test this myself
|
Can confirm that this approach works! Here is what I ended up using:
...
afterSign: ./scripts/after-sign.js
win:
sign: ./scripts/nop.js
const { spawnSync } = require('node:child_process');
exports.default = async function sign(context) {
spawnSync(
'powershell.exe',
['Install-Module', '-Name', 'TrustedSigning', '-RequiredVersion', '0.4.1', '-Force', '-Repository', 'PSGallery'],
{ shell: true, stdio: 'inherit' },
);
const params = {
Endpoint: 'https://eus.codesigning.azure.net/',
CodeSigningAccountName: '<code signing account name>',
CertificateProfileName: '<certificate profile name>',
FilesFolder: context.appOutDir,
FilesFolderFilter: 'exe,dll',
FileDigest: 'SHA256',
TimestampRfc3161: 'http://timestamp.acs.microsoft.com',
TimestampDigest: 'SHA256',
};
spawnSync('powershell.exe', ['Invoke-TrustedSigning', params], { shell: true, stdio: 'inherit' });
};
exports.default = async function nop() {}; |
@MikeJerred Wouldn't your proposed solution result in only the packaged executables being signed, not the installer (e.g. NSIS executable file) that results from the packaging process? It seems that the Another problem I've spotted: When testing your script, I received the following output const paramsString = Object.keys(params).map(key => ` -${key} ${params[key]}`).join('');
spawnSync('powershell.exe', ['Invoke-TrustedSigning', paramsString], { shell: true, stdio: 'inherit' }); |
I just received confirmation from MS that I have approval for the trusted signing cert. Based on some of the above hacks, I may have to skip Windows signing and use a signtool manually since installers as well as app needs to be signed. |
Signing the installer after the electron-builder packing process during and extra build step results in wrong sha512 hashes in the resulting update YAML files. |
I've managed to get this signed with my own script during the build...all executables checkout out nicely. Gone are the days of EV certs. |
I'm looking into implementing this in electron-builder, but won't have a way to test (as I don't have any Azure account). So if anyone is willing, I'd be happy to supply a patch-package patch for testing out my implementation. What are the required params for
I also see this example configuration here: https://learn.microsoft.com/en-us/azure/trusted-signing/how-to-signing-integrations
Reason I ask is to see if there are any default values I can apply or using enums (for things like |
I am happy to help with testing. Those params (plus the azure auth env vars) were enough for the signing to complete without errors when I tried it. |
Okay, nvm, the patch is too large. My best recommendation is cloning electron-builder, pulling this PR #8458 via From there, the configuration is within electron-builder/packages/app-builder-lib/src/options/winOptions.ts Lines 178 to 202 in 0d24b78
|
Not sure if my local dev setup is correct because I get an error when doing this:
|
You'll either need to start with the base v25.0.5 installed in your package.json (as that includes most recent dependencies to be resolved during
|
That has fixed that issue, however it still doesn't work:
|
Looks like You can copy paste it manually in your node modules path or take the hard-copy approach (instead of using
|
@MikeJerred I'm thinking of releasing the refactored signing code as part of 25.0.7 with logging that azure signing is in Once released, I would like additional volunteers to test it though with |
You could also possibly release it on the beta release channel, if the changes are too impactful? Anyway, also switching here to Azure Trusted Signing, as our previous certificate was expired. |
Excellent! Thank you Previous logic is all in place for using It'll be default released to |
Alrighty. Beta signing implementation has been released in I'm expecting bug reports, so I also request patience as I get this implementation fully functional. 🙃 Also, not sure if the cmd line debug logs will need any info redacted before posting them here since it's a verbatim log of the powershell From my local testing, I got this working up until the point it does Logs below with
Implementation details: https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/codeSign/windowsSignAzureManager.ts Configuration details: electron-builder/packages/app-builder-lib/src/options/winOptions.ts Lines 88 to 91 in b3ce7f7
electron-builder/packages/app-builder-lib/src/options/winOptions.ts Lines 190 to 210 in b3ce7f7
|
I installed 25.1.0 but doing
The app-builder-lib in node_modules is v25.0.5, and it doesn't have a |
Thanks for checking. Hmmm, it sounds like it desynced the release versioning during the CI/CD. It's been acting finicky lately. Can you try force installing I'll look into the dependency resolution issue |
I added
|
Kk. I've redeployed the monorepo to resync all the workspace versions. Please try |
OK this version installs properly. It should also be noted that the docs are stating to use This is my
I also have set the 3 env vars |
@MikeJerred Can you please upload/send the logs for the azure signing steps?
Please make sure to redact any sensitive info from the logs if present. Also am happy to discuss further via discord (@onegoldfish) to streamline debugging/implementing this feature. |
This is the log, I redacted some long bits that I don't think are relevant:
|
Well that's super odd, it's hitting neither this line
nor
|
The code in my node_modules has those lines, it looks like I have the correct packages installed as far as I can see. Am I setting the debug flag correctly to enable the feature? |
Honestly, I'm confused because those are |
Also not working here. At least not signing. I see that this line is not returning valid data:
As when I set forceCodeSigning to Effective config being print:
Also, is it normal that I had to had to run it in a elevated cmd prompt? I got this error message: |
Great investigate work! Thank you :) FWIW, I'm honestly shocked that this was not caught in the code signing unit tests. I'm also struggling to reproduce this locally in my test project with config
Logs of successful build using signtool:
I'll see if I can mock a way to do the azure signing method without calling Invoke-TrustedSigning since I can't get an azure test account for free AFAIK, but at least it could test the initial logic in that electron-builder flow? I'll see what I can do |
Okay, I did a bit more refactoring and moved some of the signtool logic that was still in winPackager into the signtool manager class (#8524)
[EDIT], patch didn't work, removing from comment to reduce verbosity of this thread/GH issue |
Thanks for your changes @mmaietta , will try them out tonight.
But, there was probably no problem with normal signing. The problem is that electron-builder expects a code-signing file, as in your example as well, Anyway, I will test the patch provided. |
Great callout. I've added a unit test in the PR that throws Invalid Configuration when none of the required env var combinations are detected. I made the check occur after installing the nuget package provider and trusted signing module so that it is also covered in the CI tests. Good thing too, as GH runners use pwsh.exe, which differs from my VM of powershell.exe. There's some -Command differences between the two usages that I'm trying to iron out
|
OK with that patch I get this:
|
Yep, I discovered that as well. Got it working out locally, but haven't posted a patch for the updated code yet. Thanks for checking folks! |
Alright, got tests passing on GH runners covering invalid configuration + module installation in https://github.com/electron-userland/electron-builder/actions/runs/10998788786/job/30537499953?pr=8524
|
OK amazing, that is working perfectly now! |
Thanks for testing @MikeJerred ! Re:
|
Not sure what options you can put, or why someone would want to change it, so I would say put it as defaulted to SHA256 allowing overrides - as in your comment there. |
Merged PR with fixes #8524 Deploying the release now. It'll be in |
Already a step further in my case, it starts attempting signing now. But still get an error.
Must note I am trying this on a Windows on ARM (on a Mac with Parallels). So could be it influences some things... Update:
Powershell environment:
Update2: But then I get the message: Update3: After adding codeSigningAccountName (thus which is required), I get successfully to the signtool.exe execution. Also, might be very useful to add this resource to the documentation for Azure, as it is gold: https://melatonin.dev/blog/code-signing-on-windows-with-azure-trusted-signing/ |
@Bartel-C8 thank you for the deep debugging and report! Is this on local PC or a GH runner (or other platform)? I ask because my unit tests are not able to replicate the initialization setup error you're receiving on GH runner or on my local Parallels VM. (That being said, Parallels VM only needs to be set up once so there is the chance it already had the correct setup, but I don't recall ever setting Re: |
Released v25.1.6 with I'm going to close this issue since Azure Trusted Signing is now supported in electron-builder per the OP request and this thread is super long. If you encounter any issues from here, please open a new GH Issue and I'll be happy to take a look. |
I'm inquiring about the possibility of integrating Azure Trusted Signing within the Electron Builder workflow. Currently, my build process involves:
If you could provide guidance on how to integrate Azure Trusted Signing more seamlessly with Electron Builder or if there are plans to support this in the future, it would be greatly appreciated.
Thank you for your time and assistance.
The text was updated successfully, but these errors were encountered: