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

rfc: Change Electron Fuse Security Defaults #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MarshallOfSound
Copy link
Member

We've talked about this a bit internally, but I think it would be good to formalize this as a proposal. This is less API discussion and more "proposing a breaking change" but I still felt it fits the RFC process a bit better than discussing in a PR somewhere.

@MarshallOfSound MarshallOfSound requested a review from a team as a code owner June 24, 2024 21:07

## Unresolved questions

N/A
Copy link
Member

Choose a reason for hiding this comment

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

Rewording from a Slack message I made in #wg-releases back in February when these changes were first discussed. I've been intending to create an RFC describing this possible implementation, but haven't had the time. I'd still like to raise these concerns:

Historically the defaults for fuses have been permissive, and they’ve been flipped to more restrictive at packaging time. With the discussed changing of these defaults, this will no longer be true, and I don’t think we have a good DX for how app developers flip it back to true in development if their app needs that functionality.

Additionally, some fuses make sense to only flip at packaging time (onlyLoadAppFromAsar, nodeCliInspect, etc) but I think some would be beneficial to also flip in dev mode so that you have better consistency between dev and prod (runAsNode, grantFileProtocolExtraPrivileges, etc) and don’t get surprised by behavior differences.

Basically, I think it should be more straight forward for app developers to flip fuses in development. It’s technically possible at the moment by rolling your own using the @electron/fuses CLI, but it would be awkward DX and I don’t think we should push that to app developers to figure out if we could just make it supported functionality.

One possible implementation, for the sake of further illustration (would be fleshed out in a separate RFC):

  • Conceptually support a config.@electron/fuses key in package.json which defines boolean values for fuses, similar to how we have config.forge for Forge
  • Extend @electron/fuses CLI to add a new command (and expose it programmatically as well), say @electron/fuses apply which would read the config out of your app’s package.json and apply it to the Electron executable in node_modules
    • On first run it would create node_modules/electron/original_fuses.json (similar to how we dump the executable path into path.txt) so that the default state for any given fuse can always be restored by the CLI
  • Update npm/cli.js in the e/e repo so that before spawning Electron we apply fuses from the config (or reset them back to defaults if the developer has removed the config)

We could then also leverage config.@electron/fuses in Fiddle to support flipping fuses there. When uploading a fiddle to a Gist it would include that config and Fiddle could read it back on load and flip the fuses accordingly (requires some @electron/fiddle-core changes to ensure flipped fuses don’t bleed between runs).

@erickzhao erickzhao added the pending-review Waiting for reviewers to give feedback on the PR specifications label Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-review Waiting for reviewers to give feedback on the PR specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants