Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: binary detection signing #5493

Merged
merged 5 commits into from Dec 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/app-builder-lib/electron-osx-sign/index.d.ts
Expand Up @@ -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
11 changes: 10 additions & 1 deletion packages/app-builder-lib/electron-osx-sign/index.js
Expand Up @@ -6,6 +6,7 @@

const sign = require('./sign')
const flat = require('./flat')
const util = require('./util')

/**
* This function is a normal callback implementation.
Expand Down Expand Up @@ -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
36 changes: 22 additions & 14 deletions packages/app-builder-lib/electron-osx-sign/sign.js
Expand Up @@ -5,7 +5,7 @@
'use strict'

const path = require('path')

const fs = require('fs')
const semver = require('semver')

const util = require('./util')
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down
57 changes: 32 additions & 25 deletions packages/app-builder-lib/electron-osx-sign/util.js
Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/app-builder-lib/package.json
Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions packages/app-builder-lib/src/asar/unpackDetector.ts
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
67 changes: 59 additions & 8 deletions yarn.lock
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down