Skip to content

feat: add integrity digest plugin#4159

Open
nmggithub wants to merge 4 commits intoelectron:nextfrom
nmggithub:integrity-digest-plugin
Open

feat: add integrity digest plugin#4159
nmggithub wants to merge 4 commits intoelectron:nextfrom
nmggithub:integrity-digest-plugin

Conversation

@nmggithub
Copy link

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

This follows up electron/asar#380, which implements a management API for electron/electron#48587.

Some additional things we might want to do:

  1. Add the plugin to https://github.com/electron/forge/blob/next/packages/api/core/src/api/init-scripts/init-npm.ts
  2. Add the plugin to the default templates.
  3. Anything else beyond the plugin simply existing.

@nmggithub nmggithub requested a review from a team as a code owner March 5, 2026 23:14
@nmggithub nmggithub force-pushed the integrity-digest-plugin branch from c369190 to a456367 Compare March 5, 2026 23:23
@socket-security
Copy link

socket-security bot commented Mar 5, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​electron/​asar@​4.1.010010010083100

View full report

@erickzhao erickzhao added the next label Mar 5, 2026
Copy link
Member

@erickzhao erickzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugin code seems pretty straightforward but we need to update some config stuff on the library side (^:

nmggithub and others added 3 commits March 13, 2026 02:37
Co-authored-by: Erick Zhao <erick@hotmail.ca>
…plugin

Co-authored-by: Erick Zhao <erick@hotmail.ca>
Co-authored-by: Erick Zhao <erick@hotmail.ca>
@nmggithub nmggithub requested a review from erickzhao March 13, 2026 06:38
@nmggithub
Copy link
Author

Thanks for the comments @erickzhao !

What are your thoughts on these three line items from my original post?

  1. Add the plugin to https://github.com/electron/forge/blob/next/packages/api/core/src/api/init-scripts/init-npm.ts
  2. Add the plugin to the default templates.
  3. Anything else beyond the plugin simply existing.

@erickzhao
Copy link
Member

@nmggithub I think it'd be good to be able to choose a set of additional plugins to install to each template via the @inquirer/prompts CLI. I can open a separate PR to tweak the init UX such that interactive mode selectively chooses prompts instead of being disabled every time we pass any flags in.

(I think for example auto-unpack-natives doesn't need to be added by default anymore because of this upstream change in Packager 19 (electron/packager#1841)).

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be a plugin, I think this should Just Happen by default in @electron/packager in the exact same way as the integrity stuff happens today? Why would this be different 🤔

@MarshallOfSound
Copy link
Member

Also maybe I missed a reference, can someone link me the context for why these APIs are living in @electron/asar as opposed to the packaging tools.

The separation of concerns here as I understand it is:

  • @electron/asar is responsible for creating and extracting ASAR files
  • @electron/packager is responsible for generating a packaged electron app with all relevant metadata, signatures, configuration and tweaks
  • @electron/forge wires everything else together

I'm confused as to why the digest management things are living in ASAR only to then immediately be called exclusively from other packages? That seems very antithetical to the goals of dependency reduction and keeping a clean ecosystem. (Context is I noticed this because @electron/asar started depending on "plist" and IMO there's no reason for that)

If the goal here is to have a single reference implementation of ASAR digest for macOS imo that reference implementation should be in @electron/packager not in asar, not in forge and not in it's own package

@nmggithub
Copy link
Author

nmggithub commented Mar 23, 2026

@MarshallOfSound I reached for @electron/asar as I wanted to break this feature down similarly to @electron/fuses:

  • a CLI that modifies the executable,
  • an API that can then be used by @electron/packager, and
  • then a @electron/forge plugin

I completely understand how putting the CLI implementation changes the scope of @electron/asar. It does feel a bit strange, but I wasn't sure how else to achieve the above.

If the goal here is to have a single reference implementation of ASAR digest for macOS imo that reference implementation should be in @electron/packager not in asar, not in forge and not in it's own package

I think this makes sense within the scope of our own packages. However, it severely limits anyone who doesn't use Forge. Other systems, like Electron Builder are able to use things like Fuses because we happen to expose them as an API. That may not have been an explicit goal, but it does currently happen and I find it a compelling reason to also include an API here as well.

If that API lived in @electron/packager, then systems like Electron Builder importing it would likely be redundant, since it would involve depending on a package that contains logic it already has.

On a side note, if you have strong opinions, in general, about where things should live, then you should probably also take a look at electron/asar#410.

Edit: one important thing to note is that the CLI in @electron/asar has been documented in our Electron 41 release blog post, so that might make it a bit difficult to pivot from the current implementation.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants