search

electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.72k stars 1.74k forks source link

Azure Trusted Signing Adds Path to Filenames Without Whitespaces #8612

Closed olivereisenhut closed 1 month ago

olivereisenhut commented 1 month ago

I think we experiencing a side-effect of #8600.

Our bundle folder (\dist) looks somewhat like this:

builder-debug.yml
bulder-effective-config.yaml
'${appName} ${appVersion}.msi'
'${appName} Setup ${appVersion}.exe'
'${appName} Setup ${appVersion}.exe.blockmap'
${appName}-${appVersion}-win.zip
latest.yml

Before applying the latest alpha (26.0.0-alpha.3) the only file that was uploaded was ${appName}-${appVersion}-win.zip. Now, after applying 26.0.0-alpha.3, all the files with spaces in them are uploaded correctly, although the ones that don't have spaces have the absolute path prefixed in the filename.

We now have the following files uploaded to Github:

${appName} ${appVersion}.msi -> correct
${appName} Setup ${appVersion}.exe -> correct
${appName} Setup ${appVersion}.exe.blockmap -> correct
D.a.${repoName}.${repoName}.dist.${appName}-${appVersion}-win.zip
D.a.${repoName}.${repoName}.dist.latest.yml

Builder output from our CI, this is why I now it's the absolute path:

uploading       file=D:\a\${repoName}\${repoName}\dist\${appName}-${appVersion}-win.zip provider=github
mmaietta commented 1 month ago

@olivereisenhut was this working on a previous alpha release of electron-builder? Trying to discern where the issue is coming from and git diff it.

olivereisenhut commented 1 month ago

I guess not:

We're currently using a different signing method and trying to migrate to Azure Trusted Signing, so we've never had it in a working state.

mmaietta commented 1 month ago

Out of curiosity, @MikeJerred have you run into this issue yet? I'm not sure how PR https://github.com/electron-userland/electron-builder/pull/8606 would cause this to appear as it only changes the Files path, no logic in the publisher was changed.

MikeJerred commented 1 month ago

Out of curiosity, @MikeJerred have you run into this issue yet? I'm not sure how PR #8606 would cause this to appear as it only changes the Files path, no logic in the publisher was changed.

I have not yet tried to publish so didn't run into this (yet)

mmaietta commented 1 month ago

@olivereisenhut does this issue occur for you when using certificates with signtool? (Even if self-signed cert) Trying to discern if there was a change in the publisher logic versus just the signing file path PR. I don't see this issue occurring in CI tests for signtool, but the CI tests are unable to cover the azure implementation

olivereisenhut commented 1 month ago

Ahh right, good point. I built with the latest alpha (26.0.0-alpha.3) and signtool and the same thing happens. Building with the latest stable (25.1.8) works as expected.

I'll try building with the previous alpha versions to narrow it down further.

olivereisenhut commented 1 month ago

Alright, I built all the alpha version with signtool, here are the results: 26.0.0-alpha.0 -> works 26.0.0-alpha.1 -> works 26.0.0-alpha.2 -> works 26.0.0-alpha.3 -> adds the path as prefix

olivereisenhut commented 1 month ago

I think I have found the problem. I created a PR for it. #8631 After applying this PR, the files will upload correctly.

mmaietta commented 1 month ago

Wow, very nice find! Merging now, will get it released asap

mmaietta commented 1 month ago

Released in v26.0.0-alpha.4

mmaietta commented 1 month ago

@olivereisenhut do you happen to have a production azure-trusted-signed exe you could send me (or download via GH releases)? I'd like to see if I can implement signature verification for the auto-update flow, or determine if azure trusted signing even can support that flow.

olivereisenhut commented 1 month ago

Hey,

We're going to test our current build on Monday. Though, I quickly checked our app-update.yml, which still contains the publisher name. We kept the publisherName in our build config by accident.

mmaietta commented 1 month ago

Perfect, thanks for confirming the workaround. Auto-updates should function as expected with publisherName in the build config