Enable Sandboxing in BrowserWindow

[BrettCleary]

Problem description

No response

Feature description

We should consider enabling sandboxing in the renderer process in Electron (https://www.electronjs.org/docs/latest/tutorial/sandbox). It's default enabled in electron v20+ (https://www.electronjs.org/blog/electron-20-0#default-changed-renderers-without-nodeintegration-true-are-sandboxed-by-default) and helps prevent "cross-site scripting to content injection to man-in-the-middle attacks on remotely loaded websites, just to name a few".

Since we're navigating to external websites in the renderer thread (epic and gog store), those sites might be able to utilize those exploits and potentially get file system/nodejs library access. Html5/webgl games running in the electron renderer process might be able to do the same in the future.

To do this, we'll have to use a preload script to expose the main process's interface to the renderer process and refactor any renderer process code that uses node into the main process.

I'll fork and create a branch from beta to implement for review.

Alternatives

No response

Additional information

No response

[imLinguin]

Good idea, although in terms of loading other sites we use <webview> tag, which runs in yet another process separately from the renderer. There were some additions to that when fixing recent Epic login issue on the main branch. More security won't hurt 🙃

[BrettCleary]

Good idea, although in terms of loading other sites we use <webview> tag, which runs in yet another process separately from the renderer. There were some additions to that when fixing recent Epic login issue on the main branch. More security won't hurt 🙃

Ahh that's good to know! Agreed. More security won't hurt!

Do you recommend I branch/PR from/to main or beta to implement this?

[Nocccer]

Good idea, although in terms of loading other sites we use <webview> tag, which runs in yet another process separately from the renderer. There were some additions to that when fixing recent Epic login issue on the main branch. More security won't hurt 🙃

Ahh that's good to know! Agreed. More security won't hurt!

Do you recommend I branch/PR from/to main or beta to implement this?

Beta would be good to avoid conflicts. We switched to vite in beta and also changed the structure overall.

[flavioislima]

We are already on electron 20, btw.
@BrettCleary the PR should be opened against main imo. We keep beta for big features and refactors.

[BrettCleary]

We are already on electron 20, btw. @BrettCleary the PR should be opened against main imo. We keep beta for big features and refactors.

Ah I've been trying to work from main but ran into some issues referencing types from both the frontend and backend from the preload script since the backend directory wasn't under src. It's turning out to be a pretty significant refactor too since I'm having to change every ipcRenderer.send/invoke method calls. If beta is the folder structure that HGL is planning to use going forward, I think working from that will reduce work required to merge in the future.

We'll still have to enable sandboxing in the Browser Window since it's currently disabled due to nodeIntegration being true and contextIsolation being false. By my comment, I just meant that Electron heavily recommends its usage.

[Nocccer]

We are already on electron 20, btw. @BrettCleary the PR should be opened against main imo. We keep beta for big features and refactors.

Ah I've been trying to work from main but ran into some issues referencing types from both the frontend and backend from the preload script since the backend directory wasn't under src. It's turning out to be a pretty significant refactor too since I'm having to change every ipcRenderer.send/invoke method calls. If beta is the folder structure that HGL is planning to use going forward, I think working from that will reduce work required to merge in the future.

We'll still have to enable sandboxing in the Browser Window since it's currently disabled due to nodeIntegration being true and contextIsolation being false. By my comment, I just meant that Electron heavily recommends its usage.

For me it sounds like a big pr so developing agains beta is my choice. There we have backend and frontend in the sec directory.

[flavioislima]

Hmm. Maybe we should bring the vite changes to the main branch. It looks stable to me. Will try to do something about it soon.

[flavioislima]

Implemented