fix: azure signing logic should not have logic flow dependent on custom signtool hook (#8524)

Author: mmaietta

.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

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
   }

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) {

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

packages/app-builder-lib/src/winPackager.ts

@@ -122,56 +122,19 @@ export class WinPackager extends PlatformPackager<WindowsConfiguration> {
     )
   }
 
-  async sign(file: string, logMessagePrefix?: string): Promise<boolean> {
+  async sign(file: string): Promise<boolean> {
     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) {

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"`;

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",
+        },
+      },
+    },
+  })
+)