AlexTorresDev / custom-electron-titlebar

Custom electon title bar inpire on VS Code title bar
MIT License
870 stars 149 forks source link

Titlebar does not work with nodeIntegration: false, contextIsolation: true, sandbox: true #136

Closed GintV closed 2 years ago

GintV commented 3 years ago

{ nodeIntegration: false, contextIsolation: true, sandbox: true } is recommended for security reasons and this package does not work with these options on. Any plans to make this package work with these options on?

raphael10-collab commented 3 years ago

I suppose I'm having the same problem:

I'm trying to use custom-electron-titlebar : https://github.com/AlexTorresSk/custom-electron-titlebar#readme . When I put in the renderer process :

import customTitlebar from 'custom-electron-titlebar'; 
        _getTitleBar() {
            return new customTitlebar.Titlebar({
                backgroundColor: customTitlebar.Color.fromHex('#333'),
            });
} 

I get the error require is not defined

image

When I move everything in the main process main.ts :

import customTitlebar from 'custom-electron-titlebar'
ipcMain.on("customTitlebar-from-A", (event, args) => {
  let titlebar = new customTitlebar.Titlebar({
    backgroundColor: customTitlebar.Color.fromHex('#333'),
  });
  WindowTypeA.webContents.send("customTitlebar-to-A", { titlebar });
 });

I get the error: navigator is not defined .

Searching I found this answer: https://stackoverflow.com/questions/59789539/electronjs-referenceerror-navigator-is-not-defined : "This can't be executed in main process. The main process is for managing the renderer process. There won't be any navigator at Electron main process. And navigator is property of browser. The renderer is responsible to render the code to browserWindow. So you can access the navigator of browserWindow at renderer not main. So please move this to your renderer where you'd like to customize the title bar. "

But how to keep customTitlebar within the renderer process, if I get the error require is not defined ?

  WindowTypeA = new BrowserWindow({
    width: 800,
    height: 600,
    titleBarStyle: "hidden",
    webPreferences: {
      nodeIntegration: false,
      enableRemoteModule: false,
      contextIsolation: true,
      nodeIntegrationInWorker: false,
      nodeIntegrationInSubFrames: false,
      webSecurity: true,
      webviewTag: false,
      preload: path.join(__dirname, "./preload/preload.js"), /* eng-disable PRELOAD_JS_CHECK */
    }
ericblade commented 3 years ago

It presently requires enableRemoteModule to work.

raphael10-collab commented 3 years ago

@ericblade from Official Electron Docs: https://www.electronjs.org/docs/all#default-changed-enableremotemodule-defaults-to-false

"In Electron 9, using the remote module without explicitly enabling it via the enableRemoteModule WebPreferences option began emitting a warning. In Electron 10, the remote module is now disabled by default. To use the remote module, enableRemoteModule: true must be specified in WebPreference.

We recommend moving away from the remote module.:

Electron’s ‘remote’ module considered harmful: https://medium.com/@nornagon/electrons-remote-module-considered-harmful-70d69500f31 "

Jeremy Apthorp: Remote Module Considered Harmful [CovalenceConf 2020]: https://www.youtube.com/watch?v=0lb7AxaucZI&list=PLhSaNgmcIodCuiyY5fHIH-dIm9FhrNj9X&index=10

timlg07 commented 3 years ago

@raphael10-collab The quoted text mainly means that you now have to explicitly enable the remote module to use it. Just change enableRemoteModule to true.

It of course also says that for security reasons, you should have it disabled. That's why this issue originally was created.
Currently, you need to enable the remote module tho, or else the custom title-bar won't work.

To avoid the problem with require, you should use a preload script.

raphael10-collab commented 3 years ago

I'm currently already using preload script within my Electron app for IPC safe communication between renderer process and main process. And for the same security reasons I need to keep enableRemoteModule to false

timlg07 commented 3 years ago

Then I am afraid you can't use this library as it requires you to set enableRemoteModule to true

ericblade commented 3 years ago

A couple of possibilities, if you don't mind not having access to the Menu, would be to either write a stub for Menu into the global space, that at least fills in the functions the library calls, so it doesn't just error, or fork it and make sure it doesn't break if Menu isn't available.

ericblade commented 3 years ago

... looking into this, there might be additional issues beyond simply the Menu not working, as it looks like it does a fair chunk of stuff using remote.getCurrentWindow() .. i'm having a look to see if it's even minimally functional without the remote module..

ericblade commented 3 years ago

alright, SO, it definitely relies on enableRemoteModule. If I understand everything correctly, you should be able to use that with contextIsolation true to get it to work inside your preload, where remote will be available, and the main renderer scripts, which it should not.

I believe there may be an issue with using it inside a preload, however.

Someone correct me if I'm wrong about the architecture.

In any case -- it would be worth doing a rearchitecture, looking forward to the security changes in 11, 12, 13, and future.. that provides a module to include in your main process that handles ipc messages from another module that is included in your preload.

So, ideally, if I'm understanding the electron architecture changes correctly, ideally, this module would provide two separate exports, something like 'custom-electron-titlebar/main' and 'custom-electron-titlebar/renderer'. The main would register a bunch of handlers with ipcMain, and the renderer would call them with ipcRenderer.

There's no possibility of my having time to handle something of that scope with code i barely have looked at, although this module is pretty small compared to the things that I normally work on.

It'd definitely be an undertaking worth spending some time on, though, and I'd definitely be willing to help, if anyone does want to do it. It'd also go a long way towards realizing some other enhancements that i proposed over in the Discussions board.