From 5e24a6b5c2e7668c63966bf0908b51d36ef5026d Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Sun, 20 Dec 2020 17:24:38 -0800 Subject: [PATCH 1/4] Check if each binary path exists, otherwise resolve all binaries from the artifact's app Contents path --- .../app-builder-lib/electron-osx-sign/sign.js | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/app-builder-lib/electron-osx-sign/sign.js b/packages/app-builder-lib/electron-osx-sign/sign.js index adedad3ef85..3504721eae0 100644 --- a/packages/app-builder-lib/electron-osx-sign/sign.js +++ b/packages/app-builder-lib/electron-osx-sign/sign.js @@ -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, From f8bc3ec31c905ec019f6f0c2b9b14d4a7d7c5237 Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Sun, 20 Dec 2020 15:36:12 -0800 Subject: [PATCH 2/4] #5465: Switching from `isBinary` to `istextorbinary`. Exposing `getFilePathIfBinarySync` to app-builder-lib to consolidate binary detection logic --- .../electron-osx-sign/index.d.ts | 2 + .../electron-osx-sign/index.js | 11 ++- .../app-builder-lib/electron-osx-sign/sign.js | 2 +- .../app-builder-lib/electron-osx-sign/util.js | 14 ++-- packages/app-builder-lib/package.json | 2 +- .../src/asar/unpackDetector.ts | 6 +- yarn.lock | 67 ++++++++++++++++--- 7 files changed, 80 insertions(+), 24 deletions(-) diff --git a/packages/app-builder-lib/electron-osx-sign/index.d.ts b/packages/app-builder-lib/electron-osx-sign/index.d.ts index 9988c64f681..326909a5371 100644 --- a/packages/app-builder-lib/electron-osx-sign/index.d.ts +++ b/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; + +export function getFilePathIfBinarySync(filePath: string): string | undefined \ No newline at end of file diff --git a/packages/app-builder-lib/electron-osx-sign/index.js b/packages/app-builder-lib/electron-osx-sign/index.js index ead61a49156..f3c4172bc76 100644 --- a/packages/app-builder-lib/electron-osx-sign/index.js +++ b/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 diff --git a/packages/app-builder-lib/electron-osx-sign/sign.js b/packages/app-builder-lib/electron-osx-sign/sign.js index 3504721eae0..78fcb910a1a 100644 --- a/packages/app-builder-lib/electron-osx-sign/sign.js +++ b/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') diff --git a/packages/app-builder-lib/electron-osx-sign/util.js b/packages/app-builder-lib/electron-osx-sign/util.js index 30203aa0c14..56b539953f2 100644 --- a/packages/app-builder-lib/electron-osx-sign/util.js +++ b/packages/app-builder-lib/electron-osx-sign/util.js @@ -132,7 +132,7 @@ 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. @@ -140,11 +140,8 @@ const isBinaryFile = require("isbinaryfile").isBinaryFile; * @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) { + return isBinaryFile(filePath, fs.readFileSync(filePath)) ? filePath : undefined } /** @@ -215,10 +212,7 @@ module.exports.walkAsync = async function (dirPath) { 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) - } + return getFilePathIfBinarySync(filePath) } } else if (stat.isDirectory() && !stat.isSymbolicLink()) { const result = await _walkAsync(filePath) diff --git a/packages/app-builder-lib/package.json b/packages/app-builder-lib/package.json index 3e956665ec0..650971282f1 100644 --- a/packages/app-builder-lib/package.json +++ b/packages/app-builder-lib/package.json @@ -55,7 +55,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", diff --git a/packages/app-builder-lib/src/asar/unpackDetector.ts b/packages/app-builder-lib/src/asar/unpackDetector.ts index c9c9c784e5f..4e8a61360e6 100644 --- a/packages/app-builder-lib/src/asar/unpackDetector.ts +++ b/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>, 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) { diff --git a/yarn.lock b/yarn.lock index 1646c350152..f213eddac04 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2712,7 +2712,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 @@ -3249,6 +3249,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" @@ -4394,6 +4401,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" @@ -4581,6 +4598,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" @@ -6086,13 +6110,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" @@ -6174,6 +6191,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" @@ -9527,6 +9555,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" @@ -10086,6 +10121,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.11": version: 2.6.12 resolution: "vue-template-compiler@npm:2.6.12" From e10081977b415700ce60b2d20667608456e50e5e Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Mon, 21 Dec 2020 14:19:18 -0800 Subject: [PATCH 3/4] Consolidating binary path detection with extension overrides. --- .../app-builder-lib/electron-osx-sign/util.js | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/app-builder-lib/electron-osx-sign/util.js b/packages/app-builder-lib/electron-osx-sign/util.js index 56b539953f2..21d1d073843 100644 --- a/packages/app-builder-lib/electron-osx-sign/util.js +++ b/packages/app-builder-lib/electron-osx-sign/util.js @@ -135,13 +135,30 @@ var detectElectronPlatformAsync = module.exports.detectElectronPlatformAsync = f 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 getFilePathIfBinarySync = module.exports.getFilePathIfBinarySync = function (filePath) { - return isBinaryFile(filePath, fs.readFileSync(filePath)) ? filePath : undefined + 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 } /** @@ -198,21 +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: - return getFilePathIfBinarySync(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) From d6255e56d10ae74b599cb40259d680a41b2fcd9e Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Tue, 22 Dec 2020 11:41:04 -0800 Subject: [PATCH 4/4] Update lockfile --- yarn.lock | 67 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/yarn.lock b/yarn.lock index a23ff62d22a..568939d07a0 100644 --- a/yarn.lock +++ b/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"