From 62fd74dcfa13a564706141d08e5d0dea11ecf33b Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Mon, 23 Sep 2024 11:26:24 -0700 Subject: [PATCH] fix: azure signing logic should not have logic flow dependent on custom signtool hook (#8524) --- .changeset/tiny-knives-behave.md | 5 ++ .../src/codeSign/windowsSignAzureManager.ts | 25 +++++++--- .../src/codeSign/windowsSignToolManager.ts | 27 +++++++++- .../src/targets/nsis/NsisTarget.ts | 2 +- packages/app-builder-lib/src/winPackager.ts | 49 +++---------------- .../snapshots/windows/winCodeSignTest.js.snap | 2 + test/src/windows/winCodeSignTest.ts | 18 ++++++- 7 files changed, 74 insertions(+), 54 deletions(-) create mode 100644 .changeset/tiny-knives-behave.md diff --git a/.changeset/tiny-knives-behave.md b/.changeset/tiny-knives-behave.md new file mode 100644 index 00000000000..d65bcbafae8 --- /dev/null +++ b/.changeset/tiny-knives-behave.md @@ -0,0 +1,5 @@ +--- +"app-builder-lib": patch +--- + +fix: moving cscInfo logic into signtoolManager to distinguish the logic between custom sign, csc info, and azure signing diff --git a/packages/app-builder-lib/src/codeSign/windowsSignAzureManager.ts b/packages/app-builder-lib/src/codeSign/windowsSignAzureManager.ts index 044749d86e2..321c72286c7 100644 --- a/packages/app-builder-lib/src/codeSign/windowsSignAzureManager.ts +++ b/packages/app-builder-lib/src/codeSign/windowsSignAzureManager.ts @@ -10,9 +10,15 @@ export class WindowsSignAzureManager { const vm = await this.packager.vm.value const ps = await getPSCmd(vm) - log.debug(null, "installing required package provider (NuGet) and module (TrustedSigning) with scope CurrentUser") - await vm.exec(ps, ["Install-PackageProvider", "-Name", "NuGet", "-MinimumVersion", "2.8.5.201", "-Force", "-Scope", "CurrentUser"]) - await vm.exec(ps, ["Install-Module", "-Name", "TrustedSigning", "-RequiredVersion", "0.4.1", "-Force", "-Repository", "PSGallery", "-Scope", "CurrentUser"]) + log.info(null, "installing required module (TrustedSigning) with scope CurrentUser") + try { + await vm.exec(ps, ["-NoProfile", "-NonInteractive", "-Command", "Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force -Scope CurrentUser"]) + } catch (error: any) { + // Might not be needed, seems GH runners already have NuGet set up. + // Logging to debug just in case users run into this. If NuGet isn't present, Install-Module -Name TrustedSigning will fail, so we'll get the logs at that point + log.debug({ message: error.message || error.stack }, "unable to install PackageProvider Nuget. Might be a false alarm though as some systems already have it installed") + } + await vm.exec(ps, ["-NoProfile", "-NonInteractive", "-Command", "Install-Module -Name TrustedSigning -RequiredVersion 0.4.1 -Force -Repository PSGallery -Scope CurrentUser"]) // Preemptively check env vars once during initialization // Options: https://learn.microsoft.com/en-us/dotnet/api/azure.identity.environmentcredential?view=azure-dotnet#definition @@ -75,15 +81,18 @@ export class WindowsSignAzureManager { const { endpoint, certificateProfileName, ...extraSigningArgs }: WindowsAzureSigningConfiguration = options.options.azureSignOptions! const params = { - ...extraSigningArgs, + FileDigest: "SHA256", + ...extraSigningArgs, // allows overriding FileDigest if provided in config Endpoint: endpoint, CertificateProfileName: certificateProfileName, Files: options.path, } - const paramsString = Object.entries(params).reduce((res, [field, value]) => { - return [...res, `-${field}`, value] - }, [] as string[]) - await vm.exec(ps, ["Invoke-TrustedSigning", ...paramsString]) + const paramsString = Object.entries(params) + .reduce((res, [field, value]) => { + return [...res, `-${field}`, value] + }, [] as string[]) + .join(" ") + await vm.exec(ps, ["-NoProfile", "-NonInteractive", "-Command", `Invoke-TrustedSigning ${paramsString}`]) return true } diff --git a/packages/app-builder-lib/src/codeSign/windowsSignToolManager.ts b/packages/app-builder-lib/src/codeSign/windowsSignToolManager.ts index 93f3920b0d1..d81ac53b138 100644 --- a/packages/app-builder-lib/src/codeSign/windowsSignToolManager.ts +++ b/packages/app-builder-lib/src/codeSign/windowsSignToolManager.ts @@ -174,11 +174,36 @@ export class WindowsSignToolManager { hashes = Array.isArray(hashes) ? hashes : [hashes] } - const cscInfo = await this.cscInfo.value const name = this.packager.appInfo.productName const site = await this.packager.appInfo.computePackageUrl() const customSign = await resolveFunction(this.packager.appInfo.type, chooseNotNull(options.options.signtoolOptions?.sign, options.options.sign), "sign") + + const cscInfo = await this.cscInfo.value + if (cscInfo) { + let logInfo: any = { + file: log.filePath(options.path), + } + if ("file" in cscInfo) { + logInfo = { + ...logInfo, + certificateFile: cscInfo.file, + } + } else { + logInfo = { + ...logInfo, + subject: cscInfo.subject, + thumbprint: cscInfo.thumbprint, + store: cscInfo.store, + user: cscInfo.isLocalMachineStore ? "local machine" : "current user", + } + } + log.info(logInfo, "signing") + } else if (!customSign) { + log.error({ signHook: customSign, cscInfo }, "no signing info identified, signing is skipped") + return false + } + const executor = customSign || ((config: CustomWindowsSignTaskConfiguration, packager: WinPackager) => this.doSign(config, packager)) let isNest = false for (const hash of hashes) { diff --git a/packages/app-builder-lib/src/targets/nsis/NsisTarget.ts b/packages/app-builder-lib/src/targets/nsis/NsisTarget.ts index db818983798..d1abbe7ffaa 100644 --- a/packages/app-builder-lib/src/targets/nsis/NsisTarget.ts +++ b/packages/app-builder-lib/src/targets/nsis/NsisTarget.ts @@ -403,7 +403,7 @@ export class NsisTarget extends Target { } else { await execWine(installerPath, null, [], { env: { __COMPAT_LAYER: "RunAsInvoker" } }) } - await packager.sign(uninstallerPath, "signing NSIS uninstaller") + await packager.sign(uninstallerPath) delete defines.BUILD_UNINSTALLER // platform-specific path, not wine diff --git a/packages/app-builder-lib/src/winPackager.ts b/packages/app-builder-lib/src/winPackager.ts index 32cadcc3dcf..467e05b4d00 100644 --- a/packages/app-builder-lib/src/winPackager.ts +++ b/packages/app-builder-lib/src/winPackager.ts @@ -122,56 +122,19 @@ export class WinPackager extends PlatformPackager { ) } - async sign(file: string, logMessagePrefix?: string): Promise { + async sign(file: string): Promise { const signOptions: WindowsSignOptions = { path: file, options: this.platformSpecificBuildOptions, } - const cscInfo = await (await this.signtoolManager.value).cscInfo.value - if (cscInfo == null) { - if (chooseNotNull(this.platformSpecificBuildOptions.signtoolOptions?.sign, this.platformSpecificBuildOptions.sign) != null) { - return signWindows(signOptions, this) - } else if (this.forceCodeSigning) { - throw new InvalidConfigurationError( - `App is not signed and "forceCodeSigning" is set to true, please ensure that code signing configuration is correct, please see https://electron.build/code-signing` - ) - } - return false - } - - if (logMessagePrefix == null) { - logMessagePrefix = "signing" - } - - if ("file" in cscInfo) { - log.info( - { - file: log.filePath(file), - certificateFile: cscInfo.file, - }, - logMessagePrefix - ) - } else { - const info = cscInfo - log.info( - { - file: log.filePath(file), - subject: info.subject, - thumbprint: info.thumbprint, - store: info.store, - user: info.isLocalMachineStore ? "local machine" : "current user", - }, - logMessagePrefix + const didSignSuccessfully = await this.doSign(signOptions) + if (!didSignSuccessfully && this.forceCodeSigning) { + throw new InvalidConfigurationError( + `App is not signed and "forceCodeSigning" is set to true, please ensure that code signing configuration is correct, please see https://electron.build/code-signing` ) } - - return this.doSign({ - ...signOptions, - options: { - ...this.platformSpecificBuildOptions, - }, - }) + return didSignSuccessfully } private async doSign(options: WindowsSignOptions) { diff --git a/test/snapshots/windows/winCodeSignTest.js.snap b/test/snapshots/windows/winCodeSignTest.js.snap index 304ccebb1a3..0620c5af5c3 100644 --- a/test/snapshots/windows/winCodeSignTest.js.snap +++ b/test/snapshots/windows/winCodeSignTest.js.snap @@ -1,5 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`azure signing without credentials 1`] = `"ERR_ELECTRON_BUILDER_INVALID_CONFIGURATION"`; + exports[`electronDist 1`] = `"ENOENT"`; exports[`forceCodeSigning 1`] = `"ERR_ELECTRON_BUILDER_INVALID_CONFIGURATION"`; diff --git a/test/src/windows/winCodeSignTest.ts b/test/src/windows/winCodeSignTest.ts index 39166d23620..97711a77b2e 100644 --- a/test/src/windows/winCodeSignTest.ts +++ b/test/src/windows/winCodeSignTest.ts @@ -106,9 +106,25 @@ test.ifAll.ifNotCiMac( test.ifAll.ifNotCiMac( "electronDist", appThrows({ - targets: Platform.WINDOWS.createTarget(DIR_TARGET), + targets: windowsDirTarget, config: { electronDist: "foo", }, }) ) + +test.ifAll.ifNotCiMac( + "azure signing without credentials", + appThrows({ + targets: windowsDirTarget, + config: { + forceCodeSigning: true, + win: { + azureSignOptions: { + endpoint: "https://weu.codesigning.azure.net/", + certificateProfileName: "profilenamehere", + }, + }, + }, + }) +)