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

Add context isolation option to windows and webview tags #8348

Merged
merged 44 commits into from Jan 17, 2017

Conversation

Projects
None yet
6 participants
@kevinsawicki
Contributor

kevinsawicki commented Jan 5, 2017

This pull request adds support for running the preload script and Electron APIs in a separate, isolated JavaScript context from the main JavaScript context of the loaded page.

This ensures the loaded page can't tamper with any JavaScript built-ins (such as Array.prototype.push, JSON.parse, etc.) that the preload script and Electron APIs make use of.

The preload script still has full access to the DOM, document, and window globals via secure proxies that prevent leakage across the contexts. This is provided by reusing Chrome's built-in support for content scripts.

This option is completely opt-in and no existing behavior is changed.

Example

Shown below is an application that loads a page (possibly remote/untrusted) but wants to open any clicked links in an external browser using Electron's shell API.

This example also shows how variables can be injected into the loaded page and how the preload script can listen for messages from the page using window.postMessage.

main.js

const {app, BrowserWindow} = require('electron')
const path = require('path')

let window

app.once('ready', () => {
  window = new BrowserWindow({
    webPreferences: {
      contextIsolation: true,
      preload: path.join(__dirname, 'preload.js')
    }
  })
  window.loadURL(`file://${__dirname}/index.html`)
})

preload.js

const {shell, webFrame} = require('electron')
const path = require('path')

// Set a variable in the page before it loads
webFrame.executeJavaScript('window.foo = "foo";')

// The loaded page will not be able to access this, it is only available
// in this context
window.bar = 'bar'

document.addEventListener('DOMContentLoaded', () => {
  // Will log out 'undefined' since window.foo is only available in the main
  // context
  console.log(window.foo)

  // Will log out 'bar' since window.bar is available in this context
  console.log(window.bar)

  document.body.addEventListener('click', (event) => {
    // Open clicked links externally
    if (event.target.tagName === 'A') {
      shell.openExternal(event.target.href)
      event.preventDefault()
    }
  })
})

window.addEventListener('message', (event) => {
  // Beep when the page requests it
  if (event.data === 'beep') {
    shell.beep()
  }
})

index.html

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>A Remote Page</title>
    <script>
      // Will log out 'foo' since window.foo is only available in this context
      console.log(window.foo)

      // Will log out 'undefined' since window.bar is only available in the isolated
      // context
      console.log(window.bar)
    </script>
  </head>
  <body>
    <a href="https://github.com">Click me</a>
    <button onclick="window.postMessage('beep', '*')">Beep!</button>
  </body>
</html>

Depends on electron/libchromiumcontent#251

/cc @electron/maintainers

@frozeman

This comment has been minimized.

Show comment
Hide comment
@frozeman

frozeman commented Jan 5, 2017

👍

@enlight

This comment has been minimized.

Show comment
Hide comment
@enlight

enlight Jan 6, 2017

Contributor

The security guide should also be updated to mention this feature.

Contributor

enlight commented Jan 6, 2017

The security guide should also be updated to mention this feature.

@kevinsawicki kevinsawicki merged commit feac868 into master Jan 17, 2017

8 of 9 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
electron-linux-arm Build #5193662 succeeded in 64s
Details
electron-linux-ia32 Build #5193663 succeeded in 58s
Details
electron-linux-x64 Build #5193664 succeeded in 129s
Details
electron-mas-x64 Build #3176 succeeded in 8 min 38 sec
Details
electron-osx-x64 Build #3186 succeeded in 8 min 56 sec
Details
electron-win-ia32 Build #2217 succeeded in 12 min
Details
electron-win-x64 Build #2205 succeeded in 13 min
Details

@kevinsawicki kevinsawicki deleted the isolated-world branch Jan 17, 2017

@zeke

This comment has been minimized.

Show comment
Hide comment
@zeke
Member

zeke commented Jan 17, 2017

Bravo, @kevinsawicki!

@kevinsawicki kevinsawicki referenced this pull request Jan 17, 2017

Merged

Use gn chrome54 #8406

@bundyo

This comment has been minimized.

Show comment
Hide comment
@bundyo

bundyo Jan 20, 2017

Contributor

Can we now get webviews with nodeIntegration off? :)

Contributor

bundyo commented Jan 20, 2017

Can we now get webviews with nodeIntegration off? :)

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Jan 20, 2017

Member

@bundyo You always disable nodeIntegration on a webview? Or do you mean use a webview from within a BrowserWindow which has nodeIntegration disabled?

Member

MarshallOfSound commented Jan 20, 2017

@bundyo You always disable nodeIntegration on a webview? Or do you mean use a webview from within a BrowserWindow which has nodeIntegration disabled?

@bundyo

This comment has been minimized.

Show comment
Hide comment
@bundyo

bundyo Jan 20, 2017

Contributor

The second. Our use case is that we don't want any node modules required by mistake in the renderer (and we also load some external content), so we preload them with nodeIntegration off. However, we do need a webview that has to communicate with the main process too. If we use an iframe, we should make an additional proxy from postMessage to ipc.

Contributor

bundyo commented Jan 20, 2017

The second. Our use case is that we don't want any node modules required by mistake in the renderer (and we also load some external content), so we preload them with nodeIntegration off. However, we do need a webview that has to communicate with the main process too. If we use an iframe, we should make an additional proxy from postMessage to ipc.

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Jan 20, 2017

Member

@bundyo Your use case sounds a bit unusual, however it is still not possible to load a webview in a webContents with nodeIntegration disabled.

However as a potential solution to your exact use case, have you considered simply using two WebViews? I.e.

Main Process
  |
  | - BrowserWindow
             | - WebView
             | - WebView
Member

MarshallOfSound commented Jan 20, 2017

@bundyo Your use case sounds a bit unusual, however it is still not possible to load a webview in a webContents with nodeIntegration disabled.

However as a potential solution to your exact use case, have you considered simply using two WebViews? I.e.

Main Process
  |
  | - BrowserWindow
             | - WebView
             | - WebView
@bundyo

This comment has been minimized.

Show comment
Hide comment
@bundyo

bundyo Jan 20, 2017

Contributor

This actually sounds okay :) Thanks, I'll try it.

Contributor

bundyo commented Jan 20, 2017

This actually sounds okay :) Thanks, I'll try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment