fix(win): Sign all your asar unpacked binaries

Close #3997

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

@@ -1,8 +1,7 @@
 import BluebirdPromise from "bluebird-lst"
 import { Arch, asArray, InvalidConfigurationError, log, use } from "builder-util"
 import { parseDn } from "builder-util-runtime"
-import { CopyFileTransformer, FileTransformer } from "builder-util/out/fs"
-import { orIfFileNotExist } from "builder-util/out/promise"
+import { CopyFileTransformer, FileTransformer, walk } from "builder-util/out/fs"
 import { createHash } from "crypto"
 import { readdir } from "fs-extra-p"
 import isCI from "is-ci"
@@ -372,13 +371,8 @@ export class WinPackager extends PlatformPackager<WindowsConfiguration> {
     }
 
     const outResourcesDir = path.join(packContext.appOutDir, "resources", "app.asar.unpacked")
-    await BluebirdPromise.map(orIfFileNotExist(readdir(outResourcesDir), []), (file: string): any => {
-      if (file.endsWith(".exe") || file.endsWith(".dll")) {
-        return this.sign(path.join(outResourcesDir, file))
-      }
-      else {
-        return null
-      }
-    })
+    // noinspection JSUnusedLocalSymbols
+    const fileToSign = await walk(outResourcesDir, (file, stat) => stat.isDirectory() || file.endsWith(".exe") || file.endsWith(".dll"))
+    await BluebirdPromise.map(fileToSign, file => this.sign(file), {concurrency: 4})
   }
 }

packages/builder-util/src/fs.ts

@@ -3,7 +3,7 @@ import { access, chmod, copyFile as _nodeCopyFile, createReadStream, createWrite
 import * as path from "path"
 import Mode from "stat-mode"
 import { log } from "./log"
-import { orNullIfFileNotExist } from "./promise"
+import { orIfFileNotExist, orNullIfFileNotExist } from "./promise"
 
 export const MAX_FILE_REQUESTS = 8
 export const CONCURRENCY = {concurrency: MAX_FILE_REQUESTS}
@@ -65,7 +65,7 @@ export async function walk(initialDirPath: string, filter?: Filter | null, consu
       }
     }
 
-    const childNames = await readdir(dirPath)
+    const childNames = await orIfFileNotExist(readdir(dirPath), [])
     childNames.sort()
 
     let nodeModuleContent: Array<string> | null = null

test/out/windows/__snapshots__/winCodeSignTest.js.snap

@@ -1,5 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`electronDist 1`] = `"ENOENT"`;
+
+exports[`forceCodeSigning 1`] = `"ERR_ELECTRON_BUILDER_INVALID_CONFIGURATION"`;
+
 exports[`parseDn 1`] = `
 Map {
   "CN" => "7digital Limited",
@@ -8,7 +12,3 @@ Map {
   "C" => "GB",
 }
 `;
-
-exports[`sign electronDist 1`] = `"ENOENT"`;
-
-exports[`sign forceCodeSigning 1`] = `"ERR_ELECTRON_BUILDER_INVALID_CONFIGURATION"`;

test/src/helpers/fileAssert.ts

@@ -55,7 +55,7 @@ class Assertions {
     }
   }
 
-  async throws() {
+  async throws(customErrorAssert?: (error: Error) => void) {
     let actualError: Error | null = null
     let result: any
     try {
@@ -85,7 +85,12 @@ class Assertions {
       m = m.replace(/'(C:)?(\/|\\)[^']+(\/|\\)([^'\/\\]+)'/g, `'<path>/$4'`)
     }
     try {
-      expect(m).toMatchSnapshot()
+      if (customErrorAssert == null) {
+        expect(m).toMatchSnapshot()
+      }
+      else {
+        customErrorAssert(actualError!!)
+      }
     }
     catch (matchError) {
       throw new Error(matchError + " " + actualError)

test/src/helpers/packTester.ts

@@ -59,8 +59,8 @@ export interface PackedContext {
   readonly tmpDir: TmpDir
 }
 
-export function appThrows(packagerOptions: PackagerOptions, checkOptions: AssertPackOptions = {}) {
-  return () => assertThat(assertPack("test-app-one", packagerOptions, checkOptions)).throws()
+export function appThrows(packagerOptions: PackagerOptions, checkOptions: AssertPackOptions = {}, customErrorAssert?: (error: Error) => void) {
+  return () => assertThat(assertPack("test-app-one", packagerOptions, checkOptions)).throws(customErrorAssert)
 }
 
 export function appTwoThrows(packagerOptions: PackagerOptions, checkOptions: AssertPackOptions = {}) {

test/src/windows/winCodeSignTest.ts

@@ -1,4 +1,5 @@
 import { DIR_TARGET, Platform } from "electron-builder"
+import { outputFile } from "fs-extra-p"
 import * as path from "path"
 import { CheckingWinPackager } from "../helpers/CheckingPackager"
 import { app, appThrows } from "../helpers/packTester"
@@ -11,61 +12,72 @@ test("parseDn", () => {
   expect(safeLoad("publisherName:\n  - 7digital Limited")).toMatchObject({publisherName: ["7digital Limited"]})
 })
 
-describe.ifAll("sign", () => {
-  const windowsDirTarget = Platform.WINDOWS.createTarget(["dir"])
+const windowsDirTarget = Platform.WINDOWS.createTarget(["dir"])
 
-  function testCustomSign(sign: any) {
-    return app({
-      targets: Platform.WINDOWS.createTarget(DIR_TARGET),
-      platformPackagerFactory: (packager, platform) => new CheckingWinPackager(packager),
-      config: {
-        win: {
-          certificatePassword: "pass",
-          certificateFile: "secretFile",
-          sign,
-          signingHashAlgorithms: ["sha256"],
-          // to be sure that sign code will be executed
-          forceCodeSigning: true,
-        }
-      },
-    })
+test("sign nested asar unpacked executables", appThrows({
+  targets: Platform.WINDOWS.createTarget(DIR_TARGET),
+  config: {
+    publish: "never",
+    asarUnpack: ["assets"],
   }
+}, {
+  signedWin: true,
+  projectDirCreated: async projectDir => {
+    await outputFile(path.join(projectDir, "assets", "nested", "nested", "file.exe"), "invalid PE file")
+  },
+}, error => expect(error.message).toContain("Unrecognized file type")))
 
-  test.ifNotCiMac("certificateFile/password - sign as function", testCustomSign(require("../helpers/customWindowsSign").default))
-  test.ifNotCiMac("certificateFile/password - sign as path", testCustomSign(path.join(__dirname, "../helpers/customWindowsSign")))
-
-  test.ifNotCiMac("custom sign if no code sign info", () => {
-    let called = false
-    return app({
-      targets: Platform.WINDOWS.createTarget(DIR_TARGET),
-      platformPackagerFactory: (packager, platform) => new CheckingWinPackager(packager),
-      config: {
-        win: {
-          // to be sure that sign code will be executed
-          forceCodeSigning: true,
-          sign: async () => {
-            called = true
-          },
-        },
-      },
-    }, {
-      packed: async () => {
-        expect(called).toBe(true)
+function testCustomSign(sign: any) {
+  return app({
+    targets: Platform.WINDOWS.createTarget(DIR_TARGET),
+    platformPackagerFactory: (packager, platform) => new CheckingWinPackager(packager),
+    config: {
+      win: {
+        certificatePassword: "pass",
+        certificateFile: "secretFile",
+        sign,
+        signingHashAlgorithms: ["sha256"],
+        // to be sure that sign code will be executed
+        forceCodeSigning: true,
       }
-    })()
+    },
   })
+}
 
-  test.ifNotCiMac("forceCodeSigning", appThrows({
-    targets: windowsDirTarget,
-    config: {
-      forceCodeSigning: true,
-    }
-  }))
+test.ifAll.ifNotCiMac("certificateFile/password - sign as function", testCustomSign(require("../helpers/customWindowsSign").default))
+test.ifAll.ifNotCiMac("certificateFile/password - sign as path", testCustomSign(path.join(__dirname, "../helpers/customWindowsSign")))
 
-  test.ifNotCiMac("electronDist", appThrows({
+test.ifAll.ifNotCiMac("custom sign if no code sign info", () => {
+  let called = false
+  return app({
     targets: Platform.WINDOWS.createTarget(DIR_TARGET),
+    platformPackagerFactory: (packager, platform) => new CheckingWinPackager(packager),
     config: {
-      electronDist: "foo",
+      win: {
+        // to be sure that sign code will be executed
+        forceCodeSigning: true,
+        sign: async () => {
+          called = true
+        },
+      },
+    },
+  }, {
+    packed: async () => {
+      expect(called).toBe(true)
     }
-  }))
-})
+  })()
+})
+
+test.ifAll.ifNotCiMac("forceCodeSigning", appThrows({
+  targets: windowsDirTarget,
+  config: {
+    forceCodeSigning: true,
+  }
+}))
+
+test.ifAll.ifNotCiMac("electronDist", appThrows({
+  targets: Platform.WINDOWS.createTarget(DIR_TARGET),
+  config: {
+    electronDist: "foo",
+  }
+}))

test/src/windows/winPackagerTest.ts

@@ -1,5 +1,6 @@
 import { Platform, DIR_TARGET } from "electron-builder"
-import { remove, rename, unlink, writeFile } from "fs-extra-p"
+import { remove, writeFile } from "fs-extra-p"
+import { promises as fs } from "fs"
 import * as path from "path"
 import { CheckingWinPackager } from "../helpers/CheckingPackager"
 import { app, appThrows, assertPack, platform } from "../helpers/packTester"
@@ -27,14 +28,14 @@ test.ifNotCiMac.ifAll("zip artifactName", app({
 }))
 
 test.ifNotCiMac("icon < 256", appThrows(platform(Platform.WINDOWS), {
-  projectDirCreated: projectDir => rename(path.join(projectDir, "build", "incorrect.ico"), path.join(projectDir, "build", "icon.ico"))
+  projectDirCreated: projectDir => fs.rename(path.join(projectDir, "build", "incorrect.ico"), path.join(projectDir, "build", "icon.ico"))
 }))
 
 test.ifNotCiMac("icon not an image", appThrows(platform(Platform.WINDOWS), {
   projectDirCreated: async projectDir => {
     const file = path.join(projectDir, "build", "icon.ico")
     // because we use hardlinks
-    await unlink(file)
+    await fs.unlink(file)
     await writeFile(file, "foo")
   }
 }))
@@ -50,7 +51,7 @@ test.ifMac("custom icon", () => {
       },
     },
   }, {
-    projectDirCreated: projectDir => rename(path.join(projectDir, "build", "icon.ico"), path.join(projectDir, "customIcon.ico")),
+    projectDirCreated: projectDir => fs.rename(path.join(projectDir, "build", "icon.ico"), path.join(projectDir, "customIcon.ico")),
     packed: async context => {
       expect(await platformPackager!!.getIconPath()).toEqual(path.join(context.projectDir, "customIcon.ico"))
     },
@@ -69,7 +70,7 @@ test.ifAll("win icon from icns", () => {
     platformPackagerFactory: packager => platformPackager = new CheckingWinPackager(packager)
   }, {
     projectDirCreated: projectDir => Promise.all([
-      unlink(path.join(projectDir, "build", "icon.ico")),
+      fs.unlink(path.join(projectDir, "build", "icon.ico")),
       remove(path.join(projectDir, "build", "icons")),
     ]),
     packed: async () => {