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.

Sign up for GitHub

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

Jump to bottom

feat: add new fuse to treat file: identically to browsers #40372

Merged
merged 1 commit into from Nov 9, 2023
Merged

feat: add new fuse to treat file: identically to browsers #40372

merged 1 commit into from Nov 9, 2023

Conversation

MarshallOfSound
Copy link
Member

@MarshallOfSound MarshallOfSound commented Oct 30, 2023 •
edited

A long standing gap between Electron and "The Web" has been are special treatment of file:// URLs. This was handy during the early days but nowadays we have a good stable custom protocol API that should supercede the usage of a "privileged" file:// protocol.

This PR does a few things in one go:

  • Documents that folks should prefer custom protocols instead of using the file:// protocol
  • Fixes the protocol docs to securely serve files within a folder
  • Adds a fuse that allows folks to opt in to the web standard behavior for the file:// protocol

Notes: Added new Electron Fuse that opts the file:// protocol into more secure and restrictive behaviour that matches Chromium

@MarshallOfSound MarshallOfSound added semver/minor backwards-compatible functionality no-backport labels Oct 30, 2023
@electron-cation electron-cation bot added api-review/requested 🗳 new-pr 🌱 PR opened in the last 24 hours labels Oct 30, 2023
@MarshallOfSound MarshallOfSound force-pushed the add-fuse-to-limit-file-permissions branch from 9c3b23d to 0e8324f Compare October 30, 2023 18:50
erickzhao
erickzhao reviewed Oct 30, 2023
View reviewed changes
docs/tutorial/fuses.md
**Default:** Enabled
**@electron/fuses:** `FuseV1Options.GrantFileProtocolExtraPrivileges`

The grantFileProtocolExtraPrivileges fuse changes whether pages loaded from the `file://` protocol are given privileges beyond what they would receive in a traditional web browser. This behavior was core to Electron apps in original versions of Electron but is no longer required as apps should be [serving local files from custom protocols](./security.md#18-avoid-usage-of-the-file-protocol-and-prefer-usage-of-custom-protocols) now instead. If you aren't serving pages from `file://` you should disable this fuse.
Copy link
Member

Choose a reason for hiding this comment

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

This behavior was core to Electron apps in original versions of Electron but is no longer required as apps should be serving local files from custom protocols now instead.

Is there a specific major version that we want to mention?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, this has been doable with custom protocols for long enough we don't need to specifically say when

erickzhao
erickzhao approved these changes Oct 30, 2023
View reviewed changes
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.

API LGTM

@miniak
Copy link
Contributor

miniak commented Oct 31, 2023

API LGTM

jkleinsc
jkleinsc reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

API LGTM

@MarshallOfSound MarshallOfSound merged commit d504d15 into main Nov 9, 2023
18 checks passed
@MarshallOfSound MarshallOfSound deleted the add-fuse-to-limit-file-permissions branch November 9, 2023 18:23
@release-clerk Release Clerk
Copy link

release-clerk bot commented Nov 9, 2023

Release Notes Persisted

Added new Electron Fuse that opts the file:// protocol into more secure and restrictive behaviour that matches Chromium

@derekcicerone
Copy link

Using a custom protocol to improve security is a great idea. Unfortunately we found that it breaks the React & Redux devtools. If anyone has a workaround please let me know. Without support for those tools we had to rollback our use of a custom protocol.

@PalmerAL PalmerAL mentioned this pull request Dec 10, 2023
Merged
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
@MarshallOfSound
feat: add new fuse to treat file: identically to browsers (electron#4…
2713446 
…0372)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc left review comments

@miniak miniak miniak approved these changes

@VerteDinde VerteDinde VerteDinde approved these changes

@erickzhao erickzhao erickzhao approved these changes

@codebytere codebytere Awaiting requested review from codebytere

Assignees
No one assigned
Labels
api-review/approved ✅ no-backport semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@MarshallOfSound @miniak @derekcicerone @jkleinsc @VerteDinde @erickzhao