chore: make nodeIntegration / webviewTag defaults false

atom/browser/web_contents_preferences.cc

@@ -99,9 +99,9 @@ WebContentsPreferences::WebContentsPreferences(
   // Set WebPreferences defaults onto the JS object
   SetDefaultBoolIfUndefined(options::kPlugins, false);
   SetDefaultBoolIfUndefined(options::kExperimentalFeatures, false);
-  bool node = SetDefaultBoolIfUndefined(options::kNodeIntegration, true);
+  SetDefaultBoolIfUndefined(options::kNodeIntegration, false);
   SetDefaultBoolIfUndefined(options::kNodeIntegrationInWorker, false);
-  SetDefaultBoolIfUndefined(options::kWebviewTag, node);
+  SetDefaultBoolIfUndefined(options::kWebviewTag, false);
   SetDefaultBoolIfUndefined(options::kSandbox, false);
   SetDefaultBoolIfUndefined(options::kNativeWindowOpen, false);
   SetDefaultBoolIfUndefined(options::kContextIsolation, false);
@@ -228,19 +228,16 @@ void WebContentsPreferences::AppendCommandLineSwitches(
         ::switches::kEnableExperimentalWebPlatformFeatures);
 
   // Check if we have node integration specified.
-  bool enable_node_integration = IsEnabled(options::kNodeIntegration, true);
-  command_line->AppendSwitchASCII(switches::kNodeIntegration,
-                                  enable_node_integration ? "true" : "false");
+  if (IsEnabled(options::kNodeIntegration))
+    command_line->AppendSwitch(switches::kNodeIntegration);
 
   // Whether to enable node integration in Worker.
   if (IsEnabled(options::kNodeIntegrationInWorker))
     command_line->AppendSwitch(switches::kNodeIntegrationInWorker);
 
   // Check if webview tag creation is enabled, default to nodeIntegration value.
-  // TODO(kevinsawicki): Default to false in 2.0
-  bool webview_tag = IsEnabled(options::kWebviewTag, enable_node_integration);
-  command_line->AppendSwitchASCII(switches::kWebviewTag,
-                                  webview_tag ? "true" : "false");
+  if (IsEnabled(options::kWebviewTag))
+    command_line->AppendSwitch(switches::kWebviewTag);
 
   // If the `sandbox` option was passed to the BrowserWindow's webPreferences,
   // pass `--enable-sandbox` to the renderer so it won't have any node.js

atom/renderer/atom_renderer_client.h

@@ -35,13 +35,6 @@ class AtomRendererClient : public RendererClientBase {
                                content::RenderFrame* render_frame) override;
 
  private:
-  enum NodeIntegration {
-    ALL,
-    EXCEPT_IFRAME,
-    MANUAL_ENABLE_IFRAME,
-    DISABLE,
-  };
-
   // content::ContentRendererClient:
   void RenderThreadStarted() override;
   void RenderFrameCreated(content::RenderFrame*) override;

filenames.gni

@@ -65,6 +65,7 @@ filenames = {
     "lib/common/web-view-methods.js",
     "lib/renderer/callbacks-registry.js",
     "lib/renderer/chrome-api.js",
+    "lib/renderer/command-line.js",
     "lib/renderer/content-scripts-injector.js",
     "lib/renderer/init.js",
     "lib/renderer/inspector.js",

lib/browser/guest-view-manager.js

@@ -202,7 +202,7 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
 
   const webPreferences = {
     guestInstanceId: guestInstanceId,
-    nodeIntegration: params.nodeintegration != null ? params.nodeintegration : false,
+    nodeIntegration: params.nodeintegration,
     enableRemoteModule: params.enableremotemodule,
     plugins: params.plugins,
     zoomFactor: embedder._getZoomFactor(),
@@ -237,7 +237,6 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
     ['contextIsolation', true],
     ['javascript', false],
     ['nativeWindowOpen', true],
-    ['nodeIntegration', false],
     ['enableRemoteModule', false],
     ['sandbox', true]
   ])

lib/browser/guest-window-manager.js

@@ -13,10 +13,8 @@ const inheritedWebPreferences = new Map([
   ['contextIsolation', true],
   ['javascript', false],
   ['nativeWindowOpen', true],
-  ['nodeIntegration', false],
   ['enableRemoteModule', false],
-  ['sandbox', true],
-  ['webviewTag', false]
+  ['sandbox', true]
 ])
 
 // Copy attribute of |parent| to |child| if it is not defined in |child|.

lib/renderer/command-line.js

@@ -0,0 +1,14 @@
+'use strict'
+
+exports.hasSwitch = function (name) {
+  return process.argv.includes(`--${name}`)
+}
+
+exports.getSwitchValue = function (name, defaultValue, converter = value => value) {
+  for (const arg of process.argv) {
+    if (arg.indexOf(`--${name}=`) === 0) {
+      return converter(arg.substr(arg.indexOf('=') + 1))
+    }
+  }
+  return defaultValue
+}

lib/renderer/init.js

@@ -31,67 +31,37 @@ const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
 require('@electron/internal/renderer/web-frame-init')()
 
 // Process command line arguments.
-let nodeIntegration = false
-let webviewTag = false
-let contextIsolation = false
-let preloadScript = null
-let preloadScripts = []
-let isBackgroundPage = false
-let appPath = null
-let guestInstanceId = null
-let openerId = null
-for (const arg of process.argv) {
-  if (arg.indexOf('--guest-instance-id=') === 0) {
-    // This is a guest web view.
-    guestInstanceId = parseInt(arg.substr(arg.indexOf('=') + 1))
-  } else if (arg.indexOf('--opener-id=') === 0) {
-    // This is a guest BrowserWindow.
-    openerId = parseInt(arg.substr(arg.indexOf('=') + 1))
-  } else if (arg.indexOf('--node-integration=') === 0) {
-    nodeIntegration = arg.substr(arg.indexOf('=') + 1) === 'true'
-  } else if (arg.indexOf('--preload=') === 0) {
-    preloadScript = arg.substr(arg.indexOf('=') + 1)
-  } else if (arg === '--background-page') {
-    isBackgroundPage = true
-  } else if (arg.indexOf('--app-path=') === 0) {
-    appPath = arg.substr(arg.indexOf('=') + 1)
-  } else if (arg.indexOf('--webview-tag=') === 0) {
-    webviewTag = arg.substr(arg.indexOf('=') + 1) === 'true'
-  } else if (arg === '--context-isolation') {
-    contextIsolation = true
-  } else if (arg.indexOf('--preload-scripts') === 0) {
-    preloadScripts = arg.substr(arg.indexOf('=') + 1).split(path.delimiter)
-  }
-}
+const { hasSwitch, getSwitchValue } = require('@electron/internal/renderer/command-line')
 
-const hiddenPage = process.argv.includes('--hidden-page')
-const usesNativeWindowOpen = process.argv.includes('--native-window-open')
+const contextIsolation = hasSwitch('context-isolation')
+const nodeIntegration = hasSwitch('node-integration')
+const webviewTag = hasSwitch('webview-tag')
+const isHiddenPage = hasSwitch('hidden-page')
+const isBackgroundPage = hasSwitch('background-page')
+const usesNativeWindowOpen = hasSwitch('native-window-open')
+
+const preloadScript = getSwitchValue('preload', null)
+const preloadScripts = getSwitchValue('preload-scripts', [], value => value.split(path.delimiter))
+const appPath = getSwitchValue('app-path', null)
+const guestInstanceId = getSwitchValue('guest-instance-id', null, value => parseInt(value))
+const openerId = getSwitchValue('opener-id', null, value => parseInt(value))
 
 // The webContents preload script is loaded after the session preload scripts.
 if (preloadScript) {
   preloadScripts.push(preloadScript)
 }
 
-// Pass the arguments to isolatedWorld.
-if (contextIsolation) {
-  const isolatedWorldArgs = { ipcRenderer, guestInstanceId, hiddenPage, openerId, usesNativeWindowOpen }
-  v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
-}
-
 if (window.location.protocol === 'chrome-devtools:') {
   // Override some inspector APIs.
   require('@electron/internal/renderer/inspector')
-  nodeIntegration = false
 } else if (window.location.protocol === 'chrome-extension:') {
   // Add implementations of chrome API.
   require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
-  nodeIntegration = false
 } else if (window.location.protocol === 'chrome:') {
   // Disable node integration for chrome UI scheme.
-  nodeIntegration = false
 } else {
   // Override default web functions.
-  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNativeWindowOpen)
+  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
 
   // Inject content scripts.
   require('@electron/internal/renderer/content-scripts-injector')
@@ -107,6 +77,12 @@ if (window.location.protocol === 'chrome-devtools:') {
   }
 }
 
+// Pass the arguments to isolatedWorld.
+if (contextIsolation) {
+  const isolatedWorldArgs = { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen }
+  v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
+}
+
 if (nodeIntegration) {
   // Export node bindings to global.
   global.require = require

spec/api-app-spec.js

@@ -32,7 +32,10 @@ describe('electron module', () => {
       window = new BrowserWindow({
         show: false,
         width: 400,
-        height: 400
+        height: 400,
+        webPreferences: {
+          nodeIntegration: true
+        }
       })
     })
 
@@ -298,7 +301,12 @@ describe('app module', () => {
         password: 'electron'
       }
 
-      w = new BrowserWindow({ show: false })
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true
+        }
+      })
 
       w.webContents.on('did-finish-load', () => {
         expect(w.webContents.getTitle()).to.equal('authorized')
@@ -375,7 +383,12 @@ describe('app module', () => {
         expect(webContents).to.equal(w.webContents)
         done()
       })
-      w = new BrowserWindow({ show: false })
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true
+        }
+      })
       w.loadURL('about:blank')
       w.webContents.executeJavaScript(`require('electron').desktopCapturer.getSources({ types: ['screen'] }, () => {})`)
     })
@@ -386,7 +399,12 @@ describe('app module', () => {
         expect(moduleName).to.equal('test')
         done()
       })
-      w = new BrowserWindow({ show: false })
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true
+        }
+      })
       w.loadURL('about:blank')
       w.webContents.executeJavaScript(`require('electron').remote.require('test')`)
     })
@@ -397,7 +415,12 @@ describe('app module', () => {
         expect(globalName).to.equal('test')
         done()
       })
-      w = new BrowserWindow({ show: false })
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true
+        }
+      })
       w.loadURL('about:blank')
       w.webContents.executeJavaScript(`require('electron').remote.getGlobal('test')`)
     })
@@ -590,6 +613,7 @@ describe('app module', () => {
       w = new BrowserWindow({
         show: false,
         webPreferences: {
+          nodeIntegration: true,
           partition: 'empty-certificate'
         }
       })