brrd / electron-tabs

Tab component for Electron
MIT License
697 stars 130 forks source link

How we could use the safer `BrowserWindow` instead of `webviewTag` ? #152

Closed raphael10-collab closed 2 years ago

raphael10-collab commented 2 years ago

From the electron docs https://www.electronjs.org/docs/v14-x-y/api/browser-window :

webviewTag Boolean (optional) - Whether to enable the [<webview> tag](https://www.electronjs.org/docs/v14-x-y/api/webview-tag). Defaults to false. Note: The preload script configured for the <webview> will have node integration enabled when it is executed so you should ensure remote/untrusted content is not able to create a <webview> tag with a possibly malicious preload script. You can use the will-attach-webview event on [webContents](https://www.electronjs.org/docs/v14-x-y/api/web-contents) to strip away the preload script and to validate or alter the <webview>'s initial setting

Based on what we read in the electron docs, webviewTag set to true doesn't seem to be secure and safe.

I tried to use BrowserView instead, but didn't succeed in showing the webpages up:

demo/app.js :

const electron = require("electron");
const app = electron.app;

app.on("ready", function () {
  const mainWindow = new electron.BrowserWindow({
    webPreferences: {
      //webviewTag: true
    }

  });

  const bw = new electron.BrowserView()
  mainWindow.setBrowserView(bw)

  mainWindow.loadURL("file://" + __dirname + "/electron-tabs.html");
  mainWindow.on("ready-to-show", function () {
    mainWindow.show();
    mainWindow.focus();
  });
});

image

How we could use then the safer BrowserWindow instead of webviewTag ?

brrd commented 2 years ago

Based on what we read in the electron docs, webviewTag set to true doesn't seem to be secure and safe.

Can you give the exact reference please?

As far as I know webview are perfectly safe if you keep nodeIntegration and contextIsolation options to their default values and if you use a preload script to handle the communication between processes. If your renderer process loads untrusted contents (in tabs for example) then you also have to block webview's preload script injection with the will-attach-webview event, as explained in the text you quoted. I don't see any security breach here.

The BrowserView VS webview debate is not about security but stability and performance. See: https://slack.engineering/growing-pains-migrating-slacks-desktop-app-to-browserview/

How we could use then the safer BrowserWindow instead of webviewTag ?

I guess you meant BrowserView? I may be wrong, but I think we can't switch to BrowserView unless rewriting the whole electron-tab library. BrowserView is not a part of the DOM, so we can not manage it with a web component as we do in electron-tabs. So if you want to use BrowserView for your tabs I'm afraid you will have to write your own implementation from scratch.

brrd commented 2 years ago

I'm closing this issue. I would reopen it if you demonstrate that webview is intrinsically insecure.

raphael10-collab commented 2 years ago

@brrd I actually was misleaded by the electron docs: webview is not itself insecure but contains bugs, which do not seems to be all resolved... or are they?

https://github.com/electron/electron/issues/18187#issuecomment-490361263

brrd commented 2 years ago

This question is out of the scope of electron-tabs. You should probably ask the Electron community.

raphael10-collab commented 2 years ago

@brrd Hi Thomas!

I asked the Electron community this question: "For how long chrome.webvieTag will be available in Electron, even if is already "deprecated" in Google Chrome docs ?"

https://github.com/electron/electron/issues/34356

This is the interesting answer I got:

`"Hey @raphael10-collab, thanks for asking this! The honest answer to your question is "we don't know." As of this time, there aren't any plans to remove webviewTag on Electron's side. However, webviewTag is is part of the deprecated Chrome Apps platform - as such, Chrome could decide to remove it or drop support at any time. According to this timeline outlined by Chrome itself, Chrome is planning to end support for Chrome Apps for all customers next month, June 2022: https://blog.chromium.org/2020/01/moving-forward-from-chrome-apps.html This isn't a guarantee that the element will be removed then, but I think it should be seen as a warning.

My recommendation to you would be to remove any chrome.webviewTag dependencies in your apps as soon as possible, while the API is deprecated, but still available. Chrome included a migration guide for appis using the Chrome Apps platform API here: https://developer.chrome.com/docs/apps/migration/

I hope this helps you! Since this issue doesn't represent a task needing to be completed, I'm going to close it, but please feel free to ask in our Discord if you have more questions - we have many active help channels and mentors, as well as fellow devs, who can help you out."`

brrd commented 2 years ago

Thank you for the information.

I just rewrote electron-tabs a few days ago using web components. Moving to BrowserView would require a new rewriting of the whole library which I'm not willing to do right now (electron-tabs is a volunteer project, I can't spend my full time on it unfortunately).

We obviously don't know anything about Chrome short term plans, so let's wait and see until we get some new information. The current API is still perfectly working with current versions of Electron, which is the important thing IMO.