Skip to content

Revert "feat(suite-desktop): turn on ASAR integrity check"#16972

Merged
peter-sanderson merged 1 commit intodevelopfrom
revert-16951-feat/ASAR-verification
Feb 13, 2025
Merged

Revert "feat(suite-desktop): turn on ASAR integrity check"#16972
peter-sanderson merged 1 commit intodevelopfrom
revert-16951-feat/ASAR-verification

Conversation

@Lemonexe
Copy link
Copy Markdown
Contributor

@Lemonexe Lemonexe commented Feb 13, 2025

Reverts #16951

It crashes MacOS build, after this revert it works again; need to investigate further 😞

@coderabbitai ignore

@Lemonexe Lemonexe marked this pull request as ready for review February 13, 2025 08:35
@Lemonexe Lemonexe requested a review from komret as a code owner February 13, 2025 08:35
@trezor trezor deleted a comment from coderabbitai Bot Feb 13, 2025
@Lemonexe Lemonexe added the no-project This label is used to specify that PR doesn't need to be added to a project label Feb 13, 2025
@peter-sanderson peter-sanderson merged commit 2691333 into develop Feb 13, 2025
@peter-sanderson peter-sanderson deleted the revert-16951-feat/ASAR-verification branch February 13, 2025 10:28
@Lemonexe
Copy link
Copy Markdown
Contributor Author

Lemonexe commented Feb 14, 2025

📝 research notes for future:

The problem is with electron, and not with electron-builder.

Proof: on macOS, it crashes just the same when you build Suite Desktop from develop, and on the built binary, turn on the fuses manually:

npx @electron/fuses write --app "./packages/suite-desktop/build-electron/mac-arm64/Trezor Suite.app/Contents/MacOS/Trezor Suite" EnableEmbeddedAsarIntegrityValidation=on OnlyLoadAppFromAsar=on

Alternative hypothesis (rejected) was that electron-builder messes up the binary when setting fuses during build.

Next steps?

  • turn on ASAR integrity only for Windows
    • doing it via config doesn't suffice (turns on for all supported platforms); have to do it ourselves via after pack hook
  • create minimal reproduction for the bug
    • if bug reproducible, make issue in Electron, else dig deeper why only in Trezor Suite

FYI useful for debug:

npx @electron/fuses read --app "./packages/suite-desktop/build-electron/mac-arm64/Trezor Suite.app/Contents/MacOS/Trezor Suite"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-project This label is used to specify that PR doesn't need to be added to a project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants