fix: binary detection signing (#5493)

* Check if each binary path exists, otherwise resolve all binaries from the artifact's app Contents path

* #5465: Switching from `isBinary` to `istextorbinary`. Exposing `getFilePathIfBinarySync` to app-builder-lib to consolidate binary detection logic

* Consolidating binary path detection with extension overrides.

* Update lockfile

Co-authored-by: Mike Maietta <michaelmaietta@upwork.com>

packages/app-builder-lib/electron-osx-sign/index.d.ts

@@ -37,3 +37,5 @@ interface FlatOptions extends BaseSignOptions {
 export function flat(opts: FlatOptions, callback: (error: Error) => void): void;
 
 export function flatAsync(opts: FlatOptions): Promise<any>;
+
+export function getFilePathIfBinarySync(filePath: string): string | undefined

packages/app-builder-lib/electron-osx-sign/index.js

@@ -6,6 +6,7 @@
 
 const sign = require('./sign')
 const flat = require('./flat')
+const util = require('./util')
 
 /**
  * This function is a normal callback implementation.
@@ -42,6 +43,14 @@ module.exports.flat = flat.flat
  * This function is exported and returns a promise flattening the application.
  * @function
  * @param {Object} opts - Options.
- * @returns {Promise} Promise.
+ * @returns {Promise} - Promise.
  */
 module.exports.flatAsync = flat.flatAsync
+
+/**
+ * This function is exported and returns a filepath if detected to be a binary (sync).
+ * @function
+ * @param {string} filepath - Filepath
+ * @returns {string | undefined} - Filepath if binary, otherwise undefined
+ */
+module.exports.getFilePathIfBinarySync = util.getFilePathIfBinarySync

packages/app-builder-lib/electron-osx-sign/sign.js

@@ -5,7 +5,7 @@
 'use strict'
 
 const path = require('path')
-
+const fs = require('fs')
 const semver = require('semver')
 
 const util = require('./util')
@@ -117,19 +117,9 @@ async function verifySignApplicationAsync (opts) {
  * @returns {Promise} Promise.
  */
 function signApplicationAsync (opts) {
-  return walkAsync(getAppContentsPath(opts))
+  const appContentsPath = getAppContentsPath(opts)
+  return walkAsync(appContentsPath)
     .then(async function (childPaths) {
-      /**
-       * Sort the child paths by how deep they are in the file tree.  Some arcane apple
-       * logic expects the deeper files to be signed first otherwise strange errors get
-       * thrown our way
-       */
-      childPaths = childPaths.sort((a, b) => {
-        const aDepth = a.split(path.sep).length
-        const bDepth = b.split(path.sep).length
-        return bDepth - aDepth
-      })
-
       function ignoreFilePath (opts, filePath) {
         if (opts.ignore) {
           return opts.ignore.some(function (ignore) {
@@ -142,7 +132,25 @@ function signApplicationAsync (opts) {
         return false
       }
 
-      if (opts.binaries) childPaths = childPaths.concat(opts.binaries)
+      if (opts.binaries) {
+        // Accept absolute paths for external binaries, else resolve relative paths from the artifact's app Contents path.
+        const userDefinedBinaries = opts.binaries.map(function (destination) { return fs.existsSync(destination) ? destination : path.resolve(appContentsPath, destination)})
+        // Insert at front to prioritize signing. We still sort by depth next
+        childPaths = userDefinedBinaries.concat(childPaths)
+        debuglog('Signing addtional user-defined binaries: ' + JSON.stringify(userDefinedBinaries, null, 1))
+      }
+
+      /**
+       * Sort the child paths by how deep they are in the file tree.  Some arcane apple
+       * logic expects the deeper files to be signed first otherwise strange errors get
+       * thrown our way
+       */
+      childPaths = childPaths.sort((a, b) => {
+        const aDepth = a.split(path.sep).length
+        const bDepth = b.split(path.sep).length
+        return bDepth - aDepth
+      })
+
 
       const args = [
         '--sign', opts.identity.hash || opts.identity.name,

packages/app-builder-lib/electron-osx-sign/util.js

@@ -132,19 +132,33 @@ var detectElectronPlatformAsync = module.exports.detectElectronPlatformAsync = f
   })
 }
 
-const isBinaryFile = require("isbinaryfile").isBinaryFile;
+const isBinaryFile = require("istextorbinary").isBinary;
 
 /**
- * This function returns a promise resolving the file path if file binary.
+ * This function returns a promise resolving the file path if file binary. We check filepath extension first to reduce overhead of reading the file to a buffer for it to scan start/mid/end encoding of the file
  * @function
  * @param {string} filePath - Path to file.
  * @returns {Promise} Promise resolving file path or undefined.
  */
-const getFilePathIfBinaryAsync = module.exports.getFilePathIfBinaryAsync = function (filePath) {
-  return isBinaryFile(filePath)
-    .then(function (isBinary) {
-      return isBinary ? filePath : undefined
-    })
+const getFilePathIfBinarySync = module.exports.getFilePathIfBinarySync = function (filePath) {
+  const forceAllowedExts = [
+    '.dylib', // Dynamic library
+    '.node' // Native node addon
+  ]
+  const name = path.basename(filePath)
+  const ext = path.extname(filePath)
+
+  // Sometimes .node is the base name, not as a file extension
+  // https://github.com/electron/electron-osx-sign/pull/169
+  if (forceAllowedExts.includes(ext) || forceAllowedExts.includes(name)) {
+    return filePath
+  }
+  // Still consider the file as binary if extension is empty or seems invalid
+  // https://github.com/electron-userland/electron-builder/issues/5465
+  if ((ext === '' || ext.indexOf(' ') >= 0) && name[0] !== '.') {
+    return (isBinaryFile(filePath, null) || isBinaryFile(null, fs.readFileSync(filePath))) ? filePath : undefined
+  }
+  return undefined
 }
 
 /**
@@ -201,24 +215,17 @@ module.exports.walkAsync = async function (dirPath) {
       let filePath = path.join(dirPath, name)
       const stat = await fs.lstat(filePath)
       if (stat.isFile()) {
-        switch (path.extname(filePath)) {
-          case '': // Binary
-            if (path.basename(filePath)[0] !== '.') {
-              return getFilePathIfBinaryAsync(filePath)
-            } // Else reject hidden file
-            break
-          case '.dylib': // Dynamic library
-          case '.node': // Native node addon
-            return filePath
-          case '.cstemp': // Temporary file generated from past codesign
-            debuglog('Removing... ' + filePath)
-            await fs.unlink(filePath)
-            return
-          default:
-            if (path.extname(filePath).indexOf(' ') >= 0) {
-              // Still consider the file as binary if extension seems invalid
-              return getFilePathIfBinaryAsync(filePath)
-            }
+        const forceRemoveExts = [
+          '.cstemp' // Temporary file generated from past codesign
+        ]
+        if (forceRemoveExts.includes(path.extname(filePath))) {
+          debuglog('Removing... ' + filePath)
+          await fs.unlink(filePath)
+          return
+        }
+        const binaryPath = getFilePathIfBinarySync(filePath)
+        if (binaryPath) {
+          return binaryPath
         }
       } else if (stat.isDirectory() && !stat.isSymbolicLink()) {
         const result = await _walkAsync(filePath)

packages/app-builder-lib/package.json

@@ -56,7 +56,7 @@
     "fs-extra": "^9.0.1",
     "hosted-git-info": "^3.0.7",
     "is-ci": "^2.0.0",
-    "isbinaryfile": "^4.0.6",
+    "istextorbinary": "^5.12.0",
     "js-yaml": "^3.14.1",
     "lazy-val": "^1.0.4",
     "minimatch": "^3.0.4",

packages/app-builder-lib/src/asar/unpackDetector.ts

@@ -2,10 +2,10 @@ import BluebirdPromise from "bluebird-lst"
 import { log } from "builder-util"
 import { CONCURRENCY } from "builder-util/out/fs"
 import { ensureDir } from "fs-extra"
-import { isBinaryFile } from "isbinaryfile"
 import * as path from "path"
 import { NODE_MODULES_PATTERN } from "../fileTransformer"
 import { getDestinationPath, ResolvedFileSet } from "../util/appFileCopier"
+import { getFilePathIfBinarySync } from "../../electron-osx-sign";
 
 function addValue(map: Map<string, Array<string>>, key: string, value: string) {
   let list = map.get(key)
@@ -85,8 +85,8 @@ export async function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDir
     if (moduleName ===  "ffprobe-static" || moduleName === "ffmpeg-static" || isLibOrExe(file)) {
       shouldUnpack = true
     }
-    else if (!file.includes(".", nextSlashIndex) && path.extname(file) === "") {
-      shouldUnpack = await isBinaryFile(file)
+    else if (!file.includes(".", nextSlashIndex)) {
+      shouldUnpack = !!getFilePathIfBinarySync(file)
     }
 
     if (!shouldUnpack) {

yarn.lock

@@ -2769,7 +2769,7 @@ __metadata:
     fs-extra: ^9.0.1
     hosted-git-info: ^3.0.7
     is-ci: ^2.0.0
-    isbinaryfile: ^4.0.6
+    istextorbinary: ^5.12.0
     js-yaml: ^3.14.1
     lazy-val: ^1.0.4
     minimatch: ^3.0.4
@@ -3324,6 +3324,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"binaryextensions@npm:^4.15.0":
+  version: 4.15.0
+  resolution: "binaryextensions@npm:4.15.0"
+  checksum: c594a31c1699460a9786cd3fa57cef73eab9635dc6eba586738ea7f012dcc92d10d00f4ce60c42d2de21b0931b4fdb6b560d8095e515f69e3ebe757f8dd60be1
+  languageName: node
+  linkType: hard
+
 "bl@npm:^4.0.3":
   version: 4.0.3
   resolution: "bl@npm:4.0.3"
@@ -4523,6 +4530,16 @@ __metadata:
   languageName: node
   linkType: hard
 
+"editions@npm:^6.1.0":
+  version: 6.1.0
+  resolution: "editions@npm:6.1.0"
+  dependencies:
+    errlop: ^4.0.0
+    version-range: ^1.0.0
+  checksum: 9d9b9520da8e3fc7ea1aae75642504cf26cb5634e9c74a85d0ee03fc9f29cb7987824903c18d1cbec958507cbbe079077b2e72c3037342d7a598536b39a8292a
+  languageName: node
+  linkType: hard
+
 "ejs@npm:^3.1.5":
   version: 3.1.5
   resolution: "ejs@npm:3.1.5"
@@ -4710,6 +4727,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"errlop@npm:^4.0.0":
+  version: 4.1.0
+  resolution: "errlop@npm:4.1.0"
+  checksum: 358fa5d7502b3e2337aec03a0fd81c9fb07d6e8b762872b766abc768d8961794efb1f4ab396974ead2a09ce8538cb972c3326c8ed307b5bc99ae5d0af0da1c1c
+  languageName: node
+  linkType: hard
+
 "error-ex@npm:^1.3.1":
   version: 1.3.2
   resolution: "error-ex@npm:1.3.2"
@@ -6221,13 +6245,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"isbinaryfile@npm:^4.0.6":
-  version: 4.0.6
-  resolution: "isbinaryfile@npm:4.0.6"
-  checksum: 23d94f36fbf9898c7095d29aa1e4f4b1afad77fac6e1e987f32e89036fa45d7365f3a55990de7d99b76ca591b683640efa594f1fa19b03e3d712c4f2a690efa0
-  languageName: node
-  linkType: hard
-
 "isexe@npm:^2.0.0":
   version: 2.0.0
   resolution: "isexe@npm:2.0.0"
@@ -6309,6 +6326,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"istextorbinary@npm:^5.12.0":
+  version: 5.12.0
+  resolution: "istextorbinary@npm:5.12.0"
+  dependencies:
+    binaryextensions: ^4.15.0
+    editions: ^6.1.0
+    textextensions: ^5.11.0
+  checksum: c30ebde873fe5f9496b4f6bbf871bf864ee848448fa4d2dc41b03408b5aa89c276f6431945f080e4a02fb7b4b53b3f3aa26d8e9d83b171f94e237071e2f4f1eb
+  languageName: node
+  linkType: hard
+
 "jake@npm:^10.6.1":
   version: 10.8.2
   resolution: "jake@npm:10.8.2"
@@ -9682,6 +9710,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"textextensions@npm:^5.11.0":
+  version: 5.12.0
+  resolution: "textextensions@npm:5.12.0"
+  checksum: fca9fed6c8a0d338f6300e368070883ef73004c013bbb056d56f7f00f19133016d6ccfe094b1126b66fc942c32c81f53531a4122f7a6abeab870079b8c1e857f
+  languageName: node
+  linkType: hard
+
 "throat@npm:^5.0.0":
   version: 5.0.0
   resolution: "throat@npm:5.0.0"
@@ -10241,6 +10276,22 @@ typescript@~4.1.3:
   languageName: node
   linkType: hard
 
+"version-compare@npm:^1.0.0":
+  version: 1.1.0
+  resolution: "version-compare@npm:1.1.0"
+  checksum: ef8802dbbf9c3ff1ce201189018aa26fc53d6615d68d3976959e5236a09057f4d6bb0f792ad76e98eb96099ee1dc21a6cca3e109f20ff1a84465be5a77826ac6
+  languageName: node
+  linkType: hard
+
+"version-range@npm:^1.0.0":
+  version: 1.1.0
+  resolution: "version-range@npm:1.1.0"
+  dependencies:
+    version-compare: ^1.0.0
+  checksum: 7a18b8eeaf9c4984c7d8938ebcf02973e840e4c07b8d3ce9f3ab993bff53466612c321a6fe8330484988b852ad469a0722252a91fbedb445cd15c40e74eaba5a
+  languageName: node
+  linkType: hard
+
 "vue-template-compiler@npm:^2.6.12":
   version: 2.6.12
   resolution: "vue-template-compiler@npm:2.6.12"