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.

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

Electron 10 and contextIsolation #693

Open
Coding-Kiwi opened this issue Aug 28, 2020 · 32 comments
Open

Electron 10 and contextIsolation #693

Coding-Kiwi opened this issue Aug 28, 2020 · 32 comments

Comments

@Coding-Kiwi
Copy link

Coding-Kiwi commented Aug 28, 2020

Spectron has not been updated to work with Electron 10 in a timely manner, and it seems like it may soon lose its ability to work with Electron at all. The only way to use spectron in the future is in completely security-disabled environments which then do not show the real-world case of an app. Currently the problems are:

  • electron 10
  • contextIsolation: true
  • enableRemoteModule: false

contextIsolation:

Every single application should have context isolation enabled and from Electron 12 it will be enabled by default.

From Electron Docs

enableRemoteModule

In Electron 10, the remote module is now disabled by default

From Breaking Changes

I think it should be time to come up with a solution. I think in order to make spectron work with electron 10 and contextIsolation, the api needs a proper contextbridge script in the preload.js

@Coding-Kiwi Coding-Kiwi changed the title Electron 10,contextIsolation Electron 10 and contextIsolation Aug 28, 2020
@FrancescoBorzi
Copy link

Confirmed. It's failing for me with:

Error: Failed to create session.
session not created: This version of ChromeDriver only supports Chrome version 83

@Coding-Kiwi
Copy link
Author

For me it just opens tons of electron and cmd windows, I have to abort it to make it stop, no error messages

@FrancescoBorzi
Copy link

For me it just opens tons of electron and cmd windows, I have to abort it to make it stop, no error messages

For me also opens tons of electron windows, and in the terminal I got that error

@AMDZEN3
Copy link

AMDZEN3 commented Sep 4, 2020

Just chiming in that I'm having the same issue with the hundreds of windows opening. After upgrading to 10 I had to set enableRemoteModule: true in my webPreferences. In case that's important.

@FrancescoBorzi
Copy link

@jkleinsc can you please have a look at this? we are still unable to use Electron 10 because of this :(

@jkleinsc
Copy link

Spectron unfortunately is written to use the remote module and setting contextIsolation: true and enableRemoteModule: false will make spectron unusable. As @AMDZEN3 mentioned you will need to set enableRemoteModule: true in the webPreferences setting in order to use Spectron.

Long term Spectron needs to be rewritten to not use the remote module.

@baparham
Copy link

@jkleinsc this workaround doesn't work to get Spectron running with electron 10 as there are a few other blockers, like the chromium webdriver version mismatch. Even with enableRemoteModule: True, and manually modifying the dependency chain of electron to use the proper chrome webdriver version, spectron still opens hundreds of windows and fails with errors.

Is there a plan to create a version of spectron (e.g. 12) that officially supports electron 10, at least until the remote module is completely deprecated?

@Coding-Kiwi
Copy link
Author

As mentioned in the issue description I think we need something that communicates between main and renderer through a preload contextbridge. The solution to this cant be "yeah just disable every suggested security feature and you can test your code"

I switched so using puppeteer instead.

@Nawv
Copy link

Nawv commented Sep 23, 2020

@jkleinsc I'm also seeing this issue with Electron 10 opening multiple windows before failing, with the same error message about the ChromeDriver mismatch. This is with contextIsolation: false & enableRemoteModule: true.

@jkleinsc
Copy link

@Nawv can you try the newly released v12.0.0 version?

@baparham
Copy link

Looks good so far, thanks John!

@Nawv
Copy link

Nawv commented Sep 24, 2020

@jkleinsc Yes, that's working for me - cheers!

@baparham
Copy link

@Coding-Kiwi this issue is still not really resolved yet though, right? I believe that spectron still fails when contextIsolation: true is set.

@Coding-Kiwi
Copy link
Author

@baparham Yes, I created this issue because we need a longterm solution. I really appreciate the 12.0.0 release but in the context of the issue, it is a make-shift solution just to get things running kinda. I understand that making spectron work without the remote module might require a major rework but I'd like this issue open so people who are experiencing issues with contextIsolation find this.

@FrancescoBorzi
Copy link

@baparham Yes, I created this issue because we need a longterm solution. I really appreciate the 12.0.0 release but in the context of the issue, it is a make-shift solution just to get things running kinda. I understand that making spectron work without the remote module might require a major rework but I'd like this issue open so people who are experiencing issues with contextIsolation find this.

I agree with @Coding-Kiwi about the original issue being not solved yet. However, it's nice to have a workaround allowing to use Electron 10. So thanks @jkleinsc

@NoneOfMaster
Copy link

Spectron unfortunately is written to use the remote module and setting contextIsolation: true and enableRemoteModule: false will make spectron unusable.

^ Since a developer following the latest Electron security practices during app development (and/or just having default settings) then attempting to use Spectron will run into this issue and need to find this thread for the solution, I wonder if it is something that should be addressed in the README.

My approach after following the README's Node Integration section was to reuse NODE_ENV to set web preferences:

// main.js

const TESTING = process.env.NODE_ENV === 'test';

const envBasedWebPreferences = {
  enableRemoteModule: TESTING,
  contextIsolation: !TESTING,
};

// any new windows
new BrowserWindow({
  ...,
  webPreferences: {
    ...envBasedWebPreferences,
    ...,
  }
})

@JulienBier
Copy link

I tried all the solutions exposed here (including @NoneOfMaster suggestion) to use Spectron with contextIsolation: true but none of them work.
Here is my setup:

  • electron 11.1.0
  • spectron 13.0.0
  • browerWindow settings:
webPreferences: {
	nodeIntegration: false, // tried with true
	contextIsolation: true,
	enableRemoteModule: false // tried with true
},

In Spectron, window.require is undefined.

Does anyone managed to run Spectron when contextIsolation is set to true?

@baparham
Copy link

@JulienBier I was under the impression that @NoneOfMaster suggestion was to set contextIsolation: false for spectron testing purposes and default back to true for his non-testing environment.

@cawa-93
Copy link

cawa-93 commented Dec 17, 2020

Spectron unfortunately is written to use the remote module and setting contextIsolation: true and enableRemoteModule: false will make spectron unusable.

^ Since a developer following the latest Electron security practices during app development (and/or just having default settings) then attempting to use Spectron will run into this issue and need to find this thread for the solution, I wonder if it is something that should be addressed in the README.

My approach after following the README's Node Integration section was to reuse NODE_ENV to set web preferences:

// main.js

const TESTING = process.env.NODE_ENV === 'test';

const envBasedWebPreferences = {
  enableRemoteModule: TESTING,
  contextIsolation: !TESTING,
};

// any new windows
new BrowserWindow({
  ...,
  webPreferences: {
    ...envBasedWebPreferences,
    ...,
  }
})

This will not work for several reasons:

  • Most likely, your application in production state uses a preload script that run electron.contextBridge.exposeInMainWorld( ... )
  • contextBridge API can only be used when contextIsolation is enabled.
  • With contextBridge: false You will get an error in the rendering and the tests will fail.
  • With contextBridge: true Tests can't work.

@NoneOfMaster
Copy link

  • Most likely, your application in production state uses a preload script that run electron.contextBridge.exposeInMainWorld( ... )
  • contextBridge API can only be used when contextIsolation is enabled.
  • With contextBridge: false You will get an error in the rendering and the tests will fail.
  • With contextBridge: true Tests can't work.

This is right (and really helpful). I ran into it after writing a few more tests. So far, a conditional preload as follows, along with the main.js above seems to be working:

// preload.js
const { contextBridge, ipcRenderer } = require('electron');

const api = {
  // github.com/electron/electron/issues/21437#issuecomment-573522360
  ...safeApiMethods,
};

if (process.env.NODE_ENV === 'test') {
  window.electronRequire = require; // github.com/electron-userland/spectron#node-integration
  window.api = api;
} else {
  contextBridge.exposeInMainWorld('api', api);
}

@JulienBier
Copy link

Thanks for the help. I managed to get it to work by declaring the api in windows when running the e2e.
This is not ideal because the tested code is slightly different in production but it is better than nothing.

@cawa-93
Copy link

cawa-93 commented Dec 19, 2020

  • Most likely, your application in production state uses a preload script that run electron.contextBridge.exposeInMainWorld( ... )
  • contextBridge API can only be used when contextIsolation is enabled.
  • With contextBridge: false You will get an error in the rendering and the tests will fail.
  • With contextBridge: true Tests can't work.

This is right (and really helpful). I ran into it after writing a few more tests. So far, a conditional preload as follows, along with the main.js above seems to be working:

// preload.js
const { contextBridge, ipcRenderer } = require('electron');

const api = {
  // github.com/electron/electron/issues/21437#issuecomment-573522360
  ...safeApiMethods,
};

if (process.env.NODE_ENV === 'test') {
  window.electronRequire = require; // github.com/electron-userland/spectron#node-integration
  window.api = api;
} else {
  contextBridge.exposeInMainWorld('api', api);
}

This is a good idea. However, one thing needs to be added.

exposeInMainWorld freezes any data before transmitting it to the main world. So to keep your identity in test mode, you need to do the same.

Summarizing all the above, we have the following solution (my example uses TypeScript):

// main.ts

webPreferences: {
  preload:  'path/to/preload.ts',
  contextIsolation: !isTestMode,  // Spectron tests can't work with contextIsolation: true
  enableRemoteModule: isTestMode, // Spectron tests can't work with enableRemoteModule: false
},
// preload.ts

/**
 * @see https://github.com/electron/electron/issues/21437#issuecomment-573522360
 */
const api = {
  doThing: () => ipcRenderer.send('do-a-thing'),
  ... safeApiMethods
} as const


/**
 * If contextIsolated enabled use contextBridge
 * Else use fallback
 *
 * Note: Spectron tests can't work in isolated context
 * @see https://github.com/electron-userland/spectron/issues/693#issuecomment-748482545
 */
if (process.contextIsolated) {
  /**
   * The "Main World" is the JavaScript context that your main renderer code runs in.
   * By default, the page you load in your renderer executes code in this world.
   *
   * @see https://www.electronjs.org/docs/api/context-bridge
   */
  contextBridge.exposeInMainWorld('electron', api)
} else {
  /**
   * Recursively Object.freeze() on objects and functions
   * @see https://github.com/substack/deep-freeze
   * @param o Object on which to lock the attributes
   */
  function deepFreeze<T extends Record<string, any>>(o: T): Readonly<T> {
    Object.freeze(o)

    Object.getOwnPropertyNames(o).forEach(prop => {
      if (o.hasOwnProperty(prop)
        && o[prop] !== null
        && (typeof o[prop] === 'object' || typeof o[prop] === 'function')
        && !Object.isFrozen(o[prop])) {
        deepFreeze(o[prop])
      }
    })

    return o
  }

  deepFreeze(api)

  // @ts-expect-error https://github.com/electron-userland/spectron#node-integration
  window.electronRequire = require

  // @ts-expect-error https://github.com/electron-userland/spectron/issues/693#issuecomment-747872160
  window.electron = api
}

UPD
In Electron v13 was provided process.contextIsolated


UPD 2
Use playwright instead.

@lacymorrow
Copy link

lacymorrow commented Jan 26, 2021

Edit: This no longer works in Electron versions above 11

I couldn't read process.env.NODE_ENV from the preload.js but I found I could access a property through contextBridge.internalContextBridge.contextIsolationEnabled:

// Spectron issue: https://github.com/electron-userland/spectron/issues/693
if ( contextBridge.internalContextBridge && contextBridge.internalContextBridge.contextIsolationEnabled ) {

	/**
	   * The "Main World" is the JavaScript context that your main renderer code runs in.
	   * By default, the page you load in your renderer executes code in this world.
	   *
	   * @see https://www.electronjs.org/docs/api/context-bridge
	   */
	contextBridge.exposeInMainWorld( 'electron', api )

} else {
	// ...
	
	// deepFreeze from https://github.com/electron-userland/spectron/issues/693#issuecomment-748482545
	window.electron = deepFreeze( api )
	window.testing = true
	// Github.com/electron-userland/spectron#node-integration
	window.electronRequire = require

}

@cawa-93
Copy link

cawa-93 commented Jan 27, 2021

@lacymorrow contextBridge.internalContextBridge.contextIsolationEnabled looks like a good solution. However, I do not understand why this thing is not documented and is not in the electron type definitions? Is it safe to use this in production?

@lacymorrow
Copy link

@cawa-93 I have no idea why it's not documented, maybe is supposed to be internal but it looks like it exactly reflects the value passed to window.webPreferences: https://github.com/electron/electron/blob/7672aa95258bafce9abb2e38b1cf4a4d20ff32d0/lib/renderer/api/context-bridge.ts#L4

@codethread
Copy link

as a small note, for anyone using webpack to transpile their typescript I had to alter require to stop webpack trying to convert it to it's own _webpack_require function (which was causing me issues.).

window.electronRequire = require

became -->

window.electronRequire = eval('require');

Very good chance it's my poor webpack config knowledge 😅 and there's probably a better fix, but hope it helps someone.

@gtritchie
Copy link

https://www.electronjs.org/docs/tutorial/using-selenium-and-webdriver#setting-up-spectron says: "Spectron is the officially supported ChromeDriver testing framework for Electron."

It seems like there should be a prominent warning that Spectron has significant issues with recent versions of Electron (i.e. this issue). Not sure what process is for requesting a docs change (where to open an issue).

@zhex900
Copy link

zhex900 commented Jul 29, 2021

  • Most likely, your application in production state uses a preload script that run electron.contextBridge.exposeInMainWorld( ... )
  • contextBridge API can only be used when contextIsolation is enabled.
  • With contextBridge: false You will get an error in the rendering and the tests will fail.
  • With contextBridge: true Tests can't work.

This is right (and really helpful). I ran into it after writing a few more tests. So far, a conditional preload as follows, along with the main.js above seems to be working:

// preload.js
const { contextBridge, ipcRenderer } = require('electron');

const api = {
  // github.com/electron/electron/issues/21437#issuecomment-573522360
  ...safeApiMethods,
};

if (process.env.NODE_ENV === 'test') {
  window.electronRequire = require; // github.com/electron-userland/spectron#node-integration
  window.api = api;
} else {
  contextBridge.exposeInMainWorld('api', api);
}

This is a good idea. However, one thing needs to be added.

exposeInMainWorld freezes any data before transmitting it to the main world. So to keep your identity in test mode, you need to do the same.

Summarizing all the above, we have the following solution (my example uses TypeScript):

// main.ts

webPreferences: {
  preload:  'path/to/preload.ts',
  contextIsolation: !isTestMode,  // Spectron tests can't work with contextIsolation: true
  enableRemoteModule: isTestMode, // Spectron tests can't work with enableRemoteModule: false
},
// preload.ts

/**
 * @see https://github.com/electron/electron/issues/21437#issuecomment-573522360
 */
const api = {
  doThing: () => ipcRenderer.send('do-a-thing'),
  ... safeApiMethods
} as const

if (!isTestMode) {
  /**
   * The "Main World" is the JavaScript context that your main renderer code runs in.
   * By default, the page you load in your renderer executes code in this world.
   *
   * @see https://www.electronjs.org/docs/api/context-bridge
   */
  contextBridge.exposeInMainWorld('electron', api)
} else {
  /**
   * Recursively Object.freeze() on objects and functions
   * @see https://github.com/substack/deep-freeze
   * @param o Object on which to lock the attributes
   */
  function deepFreeze<T extends Record<string, any>>(o: T): Readonly<T> {
    Object.freeze(o)

    Object.getOwnPropertyNames(o).forEach(prop => {
      if (o.hasOwnProperty(prop)
        && o[prop] !== null
        && (typeof o[prop] === 'object' || typeof o[prop] === 'function')
        && !Object.isFrozen(o[prop])) {
        deepFreeze(o[prop])
      }
    })

    return o
  }

  deepFreeze(api)

  // @ts-expect-error https://github.com/electron-userland/spectron#node-integration
  window.electronRequire = require

  // @ts-expect-error https://github.com/electron-userland/spectron/issues/693#issuecomment-747872160
  window.electron = api
}

@cawa-93 Why do you need to freeze the API deepFreeze(api) ?

@cawa-93
Copy link

cawa-93 commented Jul 29, 2021

@zhex900 From contextBridge.exposeInMainWorld docs:

Function values are proxied to the other context and all other values are copied and frozen. Any data / primitives sent in the API become immutable and updates on either side of the bridge do not result in an update on the other side.

So, if we simulate contextBridge.exposeInMainWorld behaviour with window.electron = api then, I think, We should also freeze all data.

@johns10
Copy link

johns10 commented Jul 31, 2021

Thank ya'll so much. I've literally been on this ALL DAY. I'm updating the readme with this info on PR#1018.

@oscartbeaumont
Copy link

I have started working on a project called Spectroscope as an alternative to Spectron that doesn't require disabling all the security features and jumping through so many hoops to run your Electron tests while maintaining a secure application. Spectroscope is not compatible with the Spectron API but I'm sure if someone wanted to fix Spectron they could refer to Spectroscope for inspiration on how to.

@ghost
Copy link

ghost commented Oct 5, 2021

Could this be implemented in the https://github.com/electron/chromedriver repository?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests