-
Notifications
You must be signed in to change notification settings - Fork 577
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
Improve Electron support by allowing renderer process "context isolation" and disabled "node integration" #6802
Comments
➤ PM Bot commented: Jira ticket: RJS-2868 |
Thanks for your question - this is something we've had extensive conversations about. I remember @takameyer especially advocating for this, so we definitely understand the need and problem. I've updated the title removing My current understanding of Electron's choice to enable context isolation by default is primarily for apps that allow third party code to execute in the renderer. As in, apps that have an extension architecture like VS Code (or the Atom editor). Other Electron apps, will also be executing third party code through the use of dependencies, but I'd consider that within your control and therefore a calculated risk and "safer" - the same way Node.js apps use third-party code with elevated privileges. The docs on "Context Isolation" does however mention:
I'd love if their recommendation was a bit more nuanced, talking about the tradeoffs involved. To me, disabling context isolation is perfectly safe if you're not planning on executing untrusted third-party code. I'd be happy to learn otherwise. Is it mainly because it's recommended that you request this or do you have other reasons that you need to disable context isolation that I might not be aware of? From my point of view, we have two options: Option A: Using Electron IPC primitivesFor us to support this, it would require that we implement a wrapper of our native module using IPC and we it's likely that we could leverage Electron contextBridge API for this. The main drawback of any Electron IPC however, is that underneath it use message passing and serialize data as it crosses the bridge:
Because of this, we'd have to either implement "shallow" variants of almost all of our classes (the same way we did for the React Native debugger, which we've since happily deleted as it was a huge hazzle for us to maintain) or we could to find (or build) and rely on a more generalized RPC solution that can handle control of instances of arbitrary classes, on top of the IPC primitives provided by Electron. Option B: Using WASM / browser binding in the Electron renderer processesWe're actively investing in stabilizing our browser support for the database running in browsers, via WebAssembly / WASM. This could be a viable alternative, with the main drawback that it could end up being hard to support sharing a database file between the renderer and main process. |
@kraenhansen Thanks for the quick reply. I have thought about your points when thinking about disabling, but it was mainly because of their recommendation, since it's pretty standard for you to have that enabled, and they seem to be pretty assertive about recommending to not disable it. Other than that, I understand your point, it would be awesome to not have to worry about that and just leave it enabled for that extra security, but it might not be a big problem in our case, since we do not execute untrusted party code. As for the options you mentioned, I have thought about option A, and also tried to implement a wrapper around IPC, but for the same reason you mentioned, it would be a LOT of work to get it done and maintain, due to their limitations on primitive values. Option B seems to be the best solution if we want to keep the default electron setting regarding node integration and context isolation (at least in our use case). Although it would be very nice to have that by default, I completely understand all your points and appreciate your detailed explanation. One question, I see you only mentioned |
Thanks for asking, I realize I missed clarify that part and after some further investigations, it turns out I learned something along the way. In the current state of the Realm JS SDK, the node integration is a must when importing If you disable node integration, the I realize we're not currently sharing these intricate details through our docs and I've alerted our docs team to help me get it published somehow. |
That's great, having |
I was thinking a bit more about this today, and I'll just add a few notes for myself (or whoever wants to pick this up). Once #6820 gets merged, I believe it would be ideal to implement an electron platform via the This could include detection of "realm" being required in an Electron renderer preload script:
|
@kraenhansen I tried it out a sample project using electron-react-boilerplate but couldn't get it working using the What I tried: Am I missing something? I never tried sending non-serializable data from main to renderer, so I'm not sure if my usage is correct. |
My hypothesis involved creating the |
Oh I see, I tried it out and was able to get the I still had some issues of using |
Problem
Hi, I'm wondering if there's any plan on supporting
@realm/react
usage in a Electron app. I know it's possible to use withnodeIntegration: true
andcontextIsolation: false
, but since enabling that is a security risk, I don't think it would be a good solution.Solution
Usage of
@react/realm
in the renderer process without the need to usenodeIntegration: true
andcontextIsolation: false
. Most likely by installing a renderer safe react library that would communicate with the main process to handle all updates.Alternatives
No response
How important is this improvement for you?
Would be a major improvement
Feature would mainly be used with
Atlas Device Sync
The text was updated successfully, but these errors were encountered: