refactor: clean up the default app implementation (#14719)

* Disable nodeIntegration
* Enable contextIsolation
* Re-implement the CSP security check to handle running in
contextIsolation
* Disable bad DCHECKS for the promise helper
* Remove the unused "-d" flag for the electron binary
* Added a way to hide the default help output for electron devs who
don't want to see it every time

atom/common/promise_util.cc

@@ -11,7 +11,6 @@ namespace atom {
 namespace util {
 
 Promise::Promise(v8::Isolate* isolate) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   isolate_ = isolate;
   resolver_.Reset(isolate, v8::Promise::Resolver::New(isolate));
 }

atom/common/promise_util.h

@@ -52,7 +52,6 @@ class Promise {
 
  private:
   v8::Local<v8::Promise::Resolver> GetInner() const {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     return resolver_.Get(isolate());
   }
 

default_app/default_app.js

@@ -8,28 +8,32 @@ app.on('window-all-closed', () => {
   app.quit()
 })
 
-exports.load = (appUrl) => {
-  app.on('ready', () => {
-    const options = {
-      width: 900,
-      height: 600,
-      autoHideMenuBar: true,
-      backgroundColor: '#FFFFFF',
-      webPreferences: {
-        nodeIntegrationInWorker: true
-      },
-      useContentSize: true,
-      show: false
-    }
-    if (process.platform === 'linux') {
-      options.icon = path.join(__dirname, 'icon.png')
-    }
-
-    mainWindow = new BrowserWindow(options)
-
-    mainWindow.on('ready-to-show', () => mainWindow.show())
-
-    mainWindow.loadURL(appUrl)
-    mainWindow.focus()
-  })
+exports.load = async (appUrl) => {
+  await app.whenReady()
+
+  const options = {
+    width: 900,
+    height: 600,
+    autoHideMenuBar: true,
+    backgroundColor: '#FFFFFF',
+    webPreferences: {
+      contextIsolation: true,
+      nodeIntegration: false,
+      preload: path.resolve(__dirname, 'renderer.js'),
+      webviewTag: false
+    },
+    useContentSize: true,
+    show: false
+  }
+
+  if (process.platform === 'linux') {
+    options.icon = path.join(__dirname, 'icon.png')
+  }
+
+  mainWindow = new BrowserWindow(options)
+
+  mainWindow.on('ready-to-show', () => mainWindow.show())
+
+  mainWindow.loadURL(appUrl)
+  mainWindow.focus()
 }

default_app/index.html

@@ -83,8 +83,6 @@
       </div>
     </div>
   </nav>
-
-  <script src="./renderer.js"></script>
 </body>
 
 </html>

default_app/main.js

@@ -1,50 +1,62 @@
-const { app, dialog, shell, Menu } = require('electron')
+const { app, dialog } = require('electron')
 
 const fs = require('fs')
 const Module = require('module')
 const path = require('path')
 const url = require('url')
 
+const { setDefaultApplicationMenu } = require('./menu')
+
 // Parse command line options.
 const argv = process.argv.slice(1)
 
 const option = {
   file: null,
-  help: null,
-  default: null,
+  noHelp: Boolean(process.env.ELECTRON_NO_HELP),
   version: null,
   webdriver: null,
   modules: []
 }
 
-for (let i = 0; i < argv.length; i++) {
-  if (argv[i] === '--version' || argv[i] === '-v') {
+let nextArgIsRequire = false
+
+for (const arg of argv) {
+  if (nextArgIsRequire) {
+    option.modules.push(arg)
+    nextArgIsRequire = false
+    continue
+  } else if (arg === '--version' || arg === '-v') {
     option.version = true
     break
-  } else if (argv[i].match(/^--app=/)) {
-    option.file = argv[i].split('=')[1]
+  } else if (arg.match(/^--app=/)) {
+    option.file = arg.split('=')[1]
     break
-  } else if (argv[i] === '--default' || argv[i] === '-d') {
-    option.default = true
-    break
-  } else if (argv[i] === '--interactive' || argv[i] === '-i' || argv[i] === '-repl') {
+  } else if (arg === '--interactive' || arg === '-i' || arg === '-repl') {
     option.interactive = true
-  } else if (argv[i] === '--test-type=webdriver') {
+  } else if (arg === '--test-type=webdriver') {
     option.webdriver = true
-  } else if (argv[i] === '--require' || argv[i] === '-r') {
-    option.modules.push(argv[++i])
+  } else if (arg === '--require' || arg === '-r') {
+    nextArgIsRequire = true
     continue
-  } else if (argv[i] === '--abi' || argv[i] === '-a') {
+  } else if (arg === '--abi' || arg === '-a') {
     option.abi = true
     continue
-  } else if (argv[i][0] === '-') {
+  } else if (arg === '--no-help') {
+    option.noHelp = true
+    continue
+  } else if (arg[0] === '-') {
     continue
   } else {
-    option.file = argv[i]
+    option.file = arg
     break
   }
 }
 
+if (nextArgIsRequire) {
+  console.error('Invalid Usage: --require [file]\n\n"file" is required')
+  process.exit(1)
+}
+
 // Quit when all windows are closed and no other one is listening to this.
 app.on('window-all-closed', () => {
   if (app.listeners('window-all-closed').length === 1 && !option.interactive) {
@@ -54,201 +66,21 @@ app.on('window-all-closed', () => {
 
 // Create default menu.
 app.once('ready', () => {
-  if (Menu.getApplicationMenu()) return
-
-  const template = [
-    {
-      label: 'Edit',
-      submenu: [
-        {
-          role: 'undo'
-        },
-        {
-          role: 'redo'
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'cut'
-        },
-        {
-          role: 'copy'
-        },
-        {
-          role: 'paste'
-        },
-        {
-          role: 'pasteandmatchstyle'
-        },
-        {
-          role: 'delete'
-        },
-        {
-          role: 'selectall'
-        }
-      ]
-    },
-    {
-      label: 'View',
-      submenu: [
-        {
-          role: 'reload'
-        },
-        {
-          role: 'forcereload'
-        },
-        {
-          role: 'toggledevtools'
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'resetzoom'
-        },
-        {
-          role: 'zoomin'
-        },
-        {
-          role: 'zoomout'
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'togglefullscreen'
-        }
-      ]
-    },
-    {
-      role: 'window',
-      submenu: [
-        {
-          role: 'minimize'
-        },
-        {
-          role: 'close'
-        }
-      ]
-    },
-    {
-      role: 'help',
-      submenu: [
-        {
-          label: 'Learn More',
-          click () {
-            shell.openExternal('https://electronjs.org')
-          }
-        },
-        {
-          label: 'Documentation',
-          click () {
-            shell.openExternal(
-              `https://github.com/electron/electron/tree/v${process.versions.electron}/docs#readme`
-            )
-          }
-        },
-        {
-          label: 'Community Discussions',
-          click () {
-            shell.openExternal('https://discuss.atom.io/c/electron')
-          }
-        },
-        {
-          label: 'Search Issues',
-          click () {
-            shell.openExternal('https://github.com/electron/electron/issues')
-          }
-        }
-      ]
-    }
-  ]
-
-  if (process.platform === 'darwin') {
-    template.unshift({
-      label: 'Electron',
-      submenu: [
-        {
-          role: 'about'
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'services',
-          submenu: []
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'hide'
-        },
-        {
-          role: 'hideothers'
-        },
-        {
-          role: 'unhide'
-        },
-        {
-          type: 'separator'
-        },
-        {
-          role: 'quit'
-        }
-      ]
-    })
-    template[1].submenu.push({
-      type: 'separator'
-    }, {
-      label: 'Speech',
-      submenu: [
-        {
-          role: 'startspeaking'
-        },
-        {
-          role: 'stopspeaking'
-        }
-      ]
-    })
-    template[3].submenu = [
-      {
-        role: 'close'
-      },
-      {
-        role: 'minimize'
-      },
-      {
-        role: 'zoom'
-      },
-      {
-        type: 'separator'
-      },
-      {
-        role: 'front'
-      }
-    ]
-  } else {
-    template.unshift({
-      label: 'File',
-      submenu: [{
-        role: 'quit'
-      }]
-    })
-  }
-
-  const menu = Menu.buildFromTemplate(template)
-  Menu.setApplicationMenu(menu)
+  setDefaultApplicationMenu()
 })
 
+// Set up preload modules
 if (option.modules.length > 0) {
   Module._preloadModules(option.modules)
 }
 
 function loadApplicationPackage (packagePath) {
   // Add a flag indicating app is started from default app.
-  process.defaultApp = true
+  Object.defineProperty(process, 'defaultApp', {
+    configurable: false,
+    enumerable: true,
+    value: true
+  })
 
   try {
     // Override app name and version.
@@ -333,31 +165,34 @@ if (option.file && !option.webdriver) {
 } else if (option.abi) {
   console.log(process.versions.modules)
   process.exit(0)
-} else if (option.default) {
-  const indexPath = path.join(__dirname, '/index.html')
-  loadApplicationByUrl(`file://${indexPath}`)
 } else if (option.interactive) {
   startRepl()
 } else {
-  const welcomeMessage = `
-  Electron ${process.versions.electron} - Build cross platform desktop apps with JavaScript, HTML, and CSS
-  Usage: electron [options] [path]
-
-  A path to an Electron app may be specified. It must be one of the following:
-    - index.js file.
-    - Folder containing a package.json file.
-    - Folder containing an index.js file.
-    - .html/.htm file.
-    - http://, https://, or file:// URL.
-
-  Options:
-    -d, --default         Run the default bundled Electron app.
-    -i, --interactive     Open a REPL to the main process.
-    -r, --require         Module to preload (option can be repeated).
-    -v, --version         Print the version.
-    -a, --abi             Print the Node ABI version.`
+  if (!option.noHelp) {
+    const welcomeMessage = `
+Electron ${process.versions.electron} - Build cross platform desktop apps with JavaScript, HTML, and CSS
+Usage: electron [options] [path]
+
+A path to an Electron app may be specified. It must be one of the following:
+  - index.js file.
+  - Folder containing a package.json file.
+  - Folder containing an index.js file.
+  - .html/.htm file.
+  - http://, https://, or file:// URL.
+
+Options:
+  -i, --interactive     Open a REPL to the main process.
+  -r, --require         Module to preload (option can be repeated).
+  -v, --version         Print the version.
+  -a, --abi             Print the Node ABI version.`
+
+    console.log(welcomeMessage)
+  }
 
-  console.log(welcomeMessage)
   const indexPath = path.join(__dirname, '/index.html')
-  loadApplicationByUrl(`file://${indexPath}`)
+  loadApplicationByUrl(url.format({
+    protocol: 'file:',
+    slashes: true,
+    pathname: indexPath
+  }))
 }

default_app/menu.js

@@ -0,0 +1,194 @@
+const { shell, Menu } = require('electron')
+
+const setDefaultApplicationMenu = () => {
+  if (Menu.getApplicationMenu()) return
+
+  const template = [
+    {
+      label: 'Edit',
+      submenu: [
+        {
+          role: 'undo'
+        },
+        {
+          role: 'redo'
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'cut'
+        },
+        {
+          role: 'copy'
+        },
+        {
+          role: 'paste'
+        },
+        {
+          role: 'pasteandmatchstyle'
+        },
+        {
+          role: 'delete'
+        },
+        {
+          role: 'selectall'
+        }
+      ]
+    },
+    {
+      label: 'View',
+      submenu: [
+        {
+          role: 'reload'
+        },
+        {
+          role: 'forcereload'
+        },
+        {
+          role: 'toggledevtools'
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'resetzoom'
+        },
+        {
+          role: 'zoomin'
+        },
+        {
+          role: 'zoomout'
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'togglefullscreen'
+        }
+      ]
+    },
+    {
+      role: 'window',
+      submenu: [
+        {
+          role: 'minimize'
+        },
+        {
+          role: 'close'
+        }
+      ]
+    },
+    {
+      role: 'help',
+      submenu: [
+        {
+          label: 'Learn More',
+          click () {
+            shell.openExternal('https://electronjs.org')
+          }
+        },
+        {
+          label: 'Documentation',
+          click () {
+            shell.openExternal(
+              `https://github.com/electron/electron/tree/v${process.versions.electron}/docs#readme`
+            )
+          }
+        },
+        {
+          label: 'Community Discussions',
+          click () {
+            shell.openExternal('https://discuss.atom.io/c/electron')
+          }
+        },
+        {
+          label: 'Search Issues',
+          click () {
+            shell.openExternal('https://github.com/electron/electron/issues')
+          }
+        }
+      ]
+    }
+  ]
+
+  if (process.platform === 'darwin') {
+    template.unshift({
+      label: 'Electron',
+      submenu: [
+        {
+          role: 'about'
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'services',
+          submenu: []
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'hide'
+        },
+        {
+          role: 'hideothers'
+        },
+        {
+          role: 'unhide'
+        },
+        {
+          type: 'separator'
+        },
+        {
+          role: 'quit'
+        }
+      ]
+    })
+    template[1].submenu.push({
+      type: 'separator'
+    }, {
+      label: 'Speech',
+      submenu: [
+        {
+          role: 'startspeaking'
+        },
+        {
+          role: 'stopspeaking'
+        }
+      ]
+    })
+    template[3].submenu = [
+      {
+        role: 'close'
+      },
+      {
+        role: 'minimize'
+      },
+      {
+        role: 'zoom'
+      },
+      {
+        type: 'separator'
+      },
+      {
+        role: 'front'
+      }
+    ]
+  } else {
+    template.unshift({
+      label: 'File',
+      submenu: [{
+        role: 'quit'
+      }]
+    })
+  }
+
+  const menu = Menu.buildFromTemplate(template)
+  Menu.setApplicationMenu(menu)
+}
+
+module.exports = {
+  setDefaultApplicationMenu
+}

default_app/renderer.js

@@ -2,51 +2,64 @@ const { remote, shell } = require('electron')
 const fs = require('fs')
 const path = require('path')
 const URL = require('url')
-const electronPath = path.relative(process.cwd(), remote.process.execPath)
-
-Array.from(document.querySelectorAll('a[href]')).forEach(link => {
-  // safely add `?utm_source=default_app
-  let url = URL.parse(link.getAttribute('href'), true)
-  url.query = Object.assign(url.query, { utm_source: 'default_app' })
-  url = URL.format(url)
-
-  link.addEventListener('click', (e) => {
-    e.preventDefault()
-    shell.openExternal(url)
-  })
-})
-
-document.querySelector('.electron-version').innerText = `Electron v${process.versions.electron}`
-document.querySelector('.chrome-version').innerText = `Chromium v${process.versions.chrome}`
-document.querySelector('.node-version').innerText = `Node v${process.versions.node}`
-document.querySelector('.v8-version').innerText = `v8 v${process.versions.v8}`
-document.querySelector('.command-example').innerText = `${electronPath} path-to-app`
-
-function getOcticonSvg (name) {
-  const octiconPath = path.resolve(__dirname, 'node_modules', 'octicons', 'build', 'svg', `${name}.svg`)
-  if (fs.existsSync(octiconPath)) {
-    const content = fs.readFileSync(octiconPath, 'utf8')
-    const div = document.createElement('div')
-    div.innerHTML = content
-    return div
+
+function initialize () {
+  // Find the shortest path to the electron binary
+  const absoluteElectronPath = remote.process.execPath
+  const relativeElectronPath = path.relative(process.cwd(), absoluteElectronPath)
+  const electronPath = absoluteElectronPath.length < relativeElectronPath.length
+    ? absoluteElectronPath
+    : relativeElectronPath
+
+  for (const link of document.querySelectorAll('a[href]')) {
+    // safely add `?utm_source=default_app
+    const parsedUrl = URL.parse(link.getAttribute('href'), true)
+    parsedUrl.query = { ...parsedUrl.query, utm_source: 'default_app' }
+    const url = URL.format(parsedUrl)
+
+    const openLinkExternally = (e) => {
+      e.preventDefault()
+      shell.openExternal(url)
+    }
+
+    link.addEventListener('click', openLinkExternally)
+    link.addEventListener('auxclick', openLinkExternally)
   }
-  return null
-}
 
-function loadSVG (element) {
-  for (const cssClass of element.classList) {
-    if (cssClass.startsWith('octicon-')) {
-      const icon = getOcticonSvg(cssClass.substr(8))
-      if (icon) {
-        icon.classList = element.classList
-        element.parentNode.insertBefore(icon, element)
-        element.remove()
-        break
+  document.querySelector('.electron-version').innerText = `Electron v${process.versions.electron}`
+  document.querySelector('.chrome-version').innerText = `Chromium v${process.versions.chrome}`
+  document.querySelector('.node-version').innerText = `Node v${process.versions.node}`
+  document.querySelector('.v8-version').innerText = `v8 v${process.versions.v8}`
+  document.querySelector('.command-example').innerText = `${electronPath} path-to-app`
+
+  function getOcticonSvg (name) {
+    const octiconPath = path.resolve(__dirname, 'node_modules', 'octicons', 'build', 'svg', `${name}.svg`)
+    if (fs.existsSync(octiconPath)) {
+      const content = fs.readFileSync(octiconPath, 'utf8')
+      const div = document.createElement('div')
+      div.innerHTML = content
+      return div
+    }
+    return null
+  }
+
+  function loadSVG (element) {
+    for (const cssClass of element.classList) {
+      if (cssClass.startsWith('octicon-')) {
+        const icon = getOcticonSvg(cssClass.substr(8))
+        if (icon) {
+          icon.classList = element.classList
+          element.parentNode.insertBefore(icon, element)
+          element.remove()
+          break
+        }
       }
     }
   }
-}
 
-for (const element of document.querySelectorAll('.octicon')) {
-  loadSVG(element)
+  for (const element of document.querySelectorAll('.octicon')) {
+    loadSVG(element)
+  }
 }
+
+window.addEventListener('load', initialize)

filenames.gni

@@ -91,6 +91,7 @@ filenames = {
     "default_app/icon.png",
     "default_app/index.html",
     "default_app/main.js",
+    "default_app/menu.js",
     "default_app/package.json",
     "default_app/renderer.js",
     "default_app/styles.css",

lib/renderer/security-warnings.js

@@ -61,13 +61,18 @@ const getIsRemoteProtocol = function () {
  * @returns {boolean} Is a CSP with `unsafe-eval` set?
  */
 const isUnsafeEvalEnabled = function () {
-  try {
-    // eslint-disable-next-line
-    new Function('');
-    return true
-  } catch (error) {
-    return false
-  }
+  const { webFrame } = require('electron')
+
+  return new Promise((resolve) => {
+    webFrame.executeJavaScript(`(${(() => {
+      try {
+        new Function('') // eslint-disable-line no-new,no-new-func
+      } catch (err) {
+        return false
+      }
+      return true
+    }).toString()})()`, resolve)
+  })
 }
 
 /**
@@ -176,14 +181,16 @@ module.exports = {
    * Logs a warning message about unset or insecure CSP
    */
   warnAboutInsecureCSP: () => {
-    if (isUnsafeEvalEnabled()) {
+    isUnsafeEvalEnabled().then((enabled) => {
+      if (!enabled) return
+
       const warning = `This renderer process has either no Content Security
       Policy set or a policy with "unsafe-eval" enabled. This exposes users of
       this app to unnecessary security risks.\n ${moreInformation}`
 
       console.warn('%cElectron Security Warning (Insecure Content-Security-Policy)',
         'font-weight: bold;', warning)
-    }
+    })
   },
 
   /**

package.json

@@ -71,5 +71,5 @@
     "replacements": {
       "@electron/internal/(.+)": "./lib/$1"
     }
-}
+  }
 }