feat(mac): ElectronAsarIntegrity in electron@15 #6511

indutny-signal · 2021-12-22T15:34:36Z

See: electron/electron#30667
Fix: #6506
Fix: #6507

changeset-bot · 2021-12-22T15:34:38Z

🦋 Changeset detected

Latest commit: 4068bd1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
app-builder-lib	Minor
dmg-builder	Minor
electron-builder-squirrel-windows	Minor
electron-builder	Minor
electron-forge-maker-appimage	Minor
electron-forge-maker-nsis-web	Minor
electron-forge-maker-nsis	Minor
electron-forge-maker-snap	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

indutny-signal · 2021-12-22T15:54:56Z

Sorry about the failing tests, I can't seem to be able to run them locally. Pushed an attempted fix.

indutny-signal · 2021-12-22T16:18:20Z

.changeset/nervous-pandas-film.md

@@ -0,0 +1,5 @@
+---
+"app-builder-lib": minor


Not sure if I made a right decision here. The scheme change below is a breaking change...

indutny-signal · 2021-12-22T16:51:21Z

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

      const isUnpacked = dirNode.unpacked || (this.unpackPattern != null && this.unpackPattern(file, stat))
-      this.fs.addFileNode(file, dirNode, newData == null ? stat.size : Buffer.byteLength(newData), isUnpacked, stat)
+      const integrity = newData === undefined ? await hashFile(file) : hashFileContents(newData)


This could potentially be optional since it is expensive to generate for the platforms that are not going to use it anyway.

Can you elaborate on this? I personally am in favor of defaulting to more security, i.e. asar integrity, and this will only impact the build-time though. Is this integrity check supposed to be platform-specific?

The integrity check so far only works on macOS. See fuses

mmaietta · 2021-12-28T17:50:03Z

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

    if (size > 4294967295) {
      throw new Error(`${file}: file size cannot be larger than 4.2GB`)
    }

    const node = new Node()
    node.size = size
+    node.integrity = integrity


Since integrity? is optional, should we first check it before setting it on Node()?

if (integrity) { node.integrity = integrity }

Similar to how node.unpacked is beneath an if-statement (as opposed to being node.unpacked = unpacked)

mmaietta · 2021-12-28T17:52:49Z

packages/app-builder-lib/src/electron/electronMac.ts

@@ -214,7 +214,7 @@ export async function createMacApp(packager: MacPackager, appOutDir: string, asa
  }

  if (asarIntegrity != null) {
-    appPlist.AsarIntegrity = JSON.stringify(asarIntegrity)
+    appPlist.ElectronAsarIntegrity = asarIntegrity


Will this break anything for electron pre-15?

There is no mention of AsarIntegrity in electron 14-x-y, 13-x-y, 12-x-y, and 11-x-y so it is safe to say that it wasn't used at all.

indutny-signal · 2022-01-03T18:46:11Z

Thanks for the feedback! I believe everything is addressed now.

netlify · 2022-01-03T18:46:46Z

✔️ Deploy Preview for car-park-attendant-cleat-11576 ready!

🔨 Explore the source changes: ce472cb

🔍 Inspect the deploy log: https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/61d5f6ed4818ce0007208974

😎 Browse the preview: https://deploy-preview-6511--car-park-attendant-cleat-11576.netlify.app

mmaietta · 2022-01-05T17:17:58Z

Can you please take a look at the failing unit tests?

 FAIL  src/globTest.ts (25.929 s)
  ✕ unpackDir one (2465 ms)
  ✕ unpackDir (2133 ms)
  ✕ asarUnpack and files ignore (2191 ms)
  ✓ link (2195 ms)
  ✕ outside link (2169 ms)
  ✓ failed peer dep (8396 ms)
  ✓ ignore node_modules (3003 ms)
  ✕ asarUnpack node_modules (2862 ms)

It looks like the snapshots just need to be updated to include the new integrity field, this should do the trick:
TEST_FILES=globTest UPDATE_SNAPSHOT=true pnpm test
Or perhaps mirror BuildTest where you use delete value.integrity? Either way should work.

Also, since this schema has a breaking change, I suggest we incorporate this into a v23.0.0-alpha branch that I'm currently working on. Can we retarget this PR to that branch?

indutny-signal · 2022-01-05T18:14:56Z

@mmaietta sorry about that. I didn't get a chance to look into it yesterday, and thanks so much for the command to update the snapshots. Pushed the fix!

indutny-signal · 2022-01-05T18:29:35Z

FWIW, I couldn't make pnpm test pass without failures on my macbook. Maybe the issue is that I have Windows 11 Parallels VM installed, but there are always failures that seem unrelated to my changes.

indutny-signal · 2022-01-05T18:44:04Z

Looks like my fix didn't quite work and the better approach is to delete the integrity for these tests as well (as you said it).

See: electron/electron#30667 Fix: electron-userland#6506 Fix: electron-userland#6507

indutny-signal · 2022-01-05T20:20:47Z

Alright. Rebased over the alpha branch and hopefully fixed the tests. Thanks for your patience!

indutny-signal · 2022-01-07T20:33:44Z

Awesome. Thanks for merging it!

* Adding INPUTxxx and OUTPUTxxx CHARSETS to makensis Fixes: #4898 #6232 #6259 * Adding additional details to error console logging * Breaking change: Removing Bintray support since it was sunset. Ref: https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/ * fix: force strip path separators for backslashes on Windows * fix: Force authentication for local Mac Squirrel update server * Breaking Change: Fail-fast for signature verification failures. Adding `-LiteralPath` to update file for injected wildcards * Adding changeset and eslint * Fix error: `-OUTPUTCHARSET is disabled for non Win32 platforms.` * feat(mac): ElectronAsarIntegrity in electron@15 (#6511) * feat(mac): ElectronAsarIntegrity in electron@15 See: electron/electron#30667 Fix: #6506 Fix: #6507 * fix(msi): MSI fails to install when deployed machine-wide via GPO (#6514) * fix(msi): MSI fails to install when deployed machine-wide via GPO * Disable advertised shortcuts, since MSIs with advertised Start Menu shortcuts that have a Shortcut Property fails to install when deployed machine-wide via GPO but works fine in all other contexts. This might be a bug in Windows or a misdiagnosis; see #6508 for more details. Closes #6508 * BREAKING CHANGE: Admins using advertisement must apply an MST to re-enable it. See #6508. * Don't set GitHub Releases draft title since it automatically pulls it from tag name. Fixes #3683 * feat(snap): add lzo to Snap compression options (also as new default) (#6201) * feat(msi): add fileAssociation support for MSI target (#6530) * fix(win): iconId sometimes containing invalid characters, and iconId config option being ignored. * fix(msi): change the fallback value for generated MSI Ids to a unique string for the product. * BREAKING CHANGE: remove MSI option `iconId` * fix: stabilizing tests by moving updater tests to its own node to explicitly segment env.___TOKEN integration tests from other standard unit tests * chore: synchronizing docs and schema plus prettier * Adding changset to set as alpha * Updating changeset documentation * feat(msi): support assisted installer for MSI target (#6550) * Add basic support for assisted installer, with UI to choose between per-user and per-machine. Supported config settings: runAfterFinish, perMachine, oneClick. Not supported: license (EULA), allowToChangeInstallationDirectory, etc. Also prevent oneClick's runAfterFinish from executing when installed silently. Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Co-authored-by: Alex Plumley <alex+git@aplum.ca> Co-authored-by: Mike Maietta <mmaietta@outlook.com> Co-authored-by: Omer Akram <om26er@gmail.com> Co-authored-by: Maximilian Federle <max.federle@gmail.com>

mmaietta · 2022-01-17T16:38:13Z

Published in 23.0.0-alpha.0. Enjoy!

indutny-signal · 2022-01-18T02:24:12Z

Awesome. Thanks!

indutny-signal · 2022-03-03T18:11:18Z

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

@@ -2,6 +2,20 @@ import { createFromBuffer } from "chromium-pickle-js"
 import { close, open, read, readFile, Stats } from "fs-extra"
 import * as path from "path"

+/** @internal */
+export interface ReadAsarHeader {


@mmaietta I don't think this, below, and AsarFilesystem should be internal. I get following TS errors when importing electron-builder:

node_modules/app-builder-lib/out/asar/asar.d.ts:1:66 - error TS2304: Cannot find name 'ReadAsarHeader'. 1 export declare function readAsarHeader(archive: string): Promise<ReadAsarHeader>; ~~~~~~~~~~~~~~ node_modules/app-builder-lib/out/asar/asar.d.ts:2:60 - error TS2304: Cannot find name 'AsarFilesystem'. 2 export declare function readAsar(archive: string): Promise<AsarFilesystem>; ~~~~~~~~~~~~~~ node_modules/app-builder-lib/out/asar/integrity.d.ts:2:10 - error TS2305: Module '"./asar"' has no exported member 'NodeIntegrity'. 2 import { NodeIntegrity } from "./asar"; ~~~~~~~~~~~~~ [10:10:15 AM] Found 3 errors. Watching for file changes.

I have no qualms with making them public. Can you open a PR for it?

Sure thing. Done: #6692 Thank you!

btw, I'm happy to report that we have tested 23.0.1 on our app and both asar integrity and merging ASARs appears to be working great! Thanks!

github-actions bot added the app-builder-lib label Dec 22, 2021

indutny-signal force-pushed the feature/asar-integrity branch from 3f913df to 7d7bedc Compare December 22, 2021 15:36

indutny-signal force-pushed the feature/asar-integrity branch from 7d7bedc to 44bc253 Compare December 22, 2021 15:54

indutny-signal commented Dec 22, 2021

View reviewed changes

mmaietta reviewed Dec 28, 2021

View reviewed changes

indutny-signal requested a review from mmaietta January 3, 2022 18:45

indutny-signal force-pushed the feature/asar-integrity branch from 6a4c827 to ce472cb Compare January 5, 2022 19:52

indutny-signal added 3 commits January 5, 2022 12:20

feat(mac): ElectronAsarIntegrity in electron@15

ccc2e47

See: electron/electron#30667 Fix: electron-userland#6506 Fix: electron-userland#6507

add conditional check for node.integrity

383897f

Fix tests

4068bd1

indutny-signal changed the base branch from master to v23.0.0-alpha January 5, 2022 20:20

indutny-signal force-pushed the feature/asar-integrity branch from ce472cb to 4068bd1 Compare January 5, 2022 20:21

mmaietta merged commit 475f083 into electron-userland:v23.0.0-alpha Jan 7, 2022

indutny-signal deleted the feature/asar-integrity branch January 7, 2022 20:33

indutny-signal commented Mar 3, 2022

View reviewed changes

indutny-signal mentioned this pull request Mar 3, 2022

fix(app-builder-lib): export missing TS types #6692

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(mac): ElectronAsarIntegrity in electron@15 #6511

feat(mac): ElectronAsarIntegrity in electron@15 #6511

indutny-signal commented Dec 22, 2021

changeset-bot bot commented Dec 22, 2021 •

edited

Loading

indutny-signal commented Dec 22, 2021

indutny-signal Dec 22, 2021

indutny-signal Dec 22, 2021

mmaietta Dec 28, 2021

indutny-signal Jan 3, 2022

mmaietta Dec 28, 2021

indutny-signal Jan 3, 2022

mmaietta Dec 28, 2021

indutny-signal Jan 3, 2022

indutny-signal commented Jan 3, 2022

netlify bot commented Jan 3, 2022 •

edited

Loading

mmaietta commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 7, 2022

mmaietta commented Jan 17, 2022

indutny-signal commented Jan 18, 2022

indutny-signal Mar 3, 2022

mmaietta Mar 3, 2022

indutny-signal Mar 3, 2022

indutny-signal Mar 3, 2022

feat(mac): ElectronAsarIntegrity in electron@15 #6511

feat(mac): ElectronAsarIntegrity in electron@15 #6511

Conversation

indutny-signal commented Dec 22, 2021

changeset-bot bot commented Dec 22, 2021 • edited Loading

🦋 Changeset detected

indutny-signal commented Dec 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indutny-signal commented Jan 3, 2022

netlify bot commented Jan 3, 2022 • edited Loading

mmaietta commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 5, 2022

indutny-signal commented Jan 7, 2022

mmaietta commented Jan 17, 2022

indutny-signal commented Jan 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Dec 22, 2021 •

edited

Loading

netlify bot commented Jan 3, 2022 •

edited

Loading