fix: ensure the sandboxed preloads globals do not leak (#17712)

BUILD.gn

@@ -10,6 +10,7 @@ import("//tools/grit/repack.gni")
 import("//tools/v8_context_snapshot/v8_context_snapshot.gni")
 import("//v8/snapshot_toolchain.gni")
 import("build/asar.gni")
+import("build/js_wrap.gni")
 import("build/npm.gni")
 import("build/tsc.gni")
 import("buildflags/buildflags.gni")
@@ -70,7 +71,7 @@ npm_action("build_electron_definitions") {
   ]
 }
 
-npm_action("atom_browserify_sandbox") {
+npm_action("atom_browserify_sandbox_unwrapped") {
   script = "browserify"
   deps = [
     ":build_electron_definitions",
@@ -79,7 +80,7 @@ npm_action("atom_browserify_sandbox") {
   inputs = auto_filenames.sandbox_browserify_deps
 
   outputs = [
-    "$target_gen_dir/js2c/preload_bundle.js",
+    "$target_gen_dir/js2c/preload_bundle_unwrapped.js",
   ]
 
   args = [
@@ -94,12 +95,14 @@ npm_action("atom_browserify_sandbox") {
     "-p",
     "tsconfig.electron.json",
     "]",
+    "--standalone",
+    "sandboxed_preload",
     "-o",
     rebase_path(outputs[0]),
   ]
 }
 
-npm_action("atom_browserify_isolated") {
+npm_action("atom_browserify_isolated_unwrapped") {
   script = "browserify"
   deps = [
     ":build_electron_definitions",
@@ -108,7 +111,7 @@ npm_action("atom_browserify_isolated") {
   inputs = auto_filenames.isolated_browserify_deps
 
   outputs = [
-    "$target_gen_dir/js2c/isolated_bundle.js",
+    "$target_gen_dir/js2c/isolated_bundle_unwrapped.js",
   ]
 
   args = [
@@ -121,12 +124,14 @@ npm_action("atom_browserify_isolated") {
     "-p",
     "tsconfig.electron.json",
     "]",
+    "--standalone",
+    "isolated_preload",
     "-o",
     rebase_path(outputs[0]),
   ]
 }
 
-npm_action("atom_browserify_content_script") {
+npm_action("atom_browserify_content_script_unwrapped") {
   script = "browserify"
   deps = [
     ":build_electron_definitions",
@@ -135,7 +140,7 @@ npm_action("atom_browserify_content_script") {
   inputs = auto_filenames.context_script_browserify_deps
 
   outputs = [
-    "$target_gen_dir/js2c/content_script_bundle.js",
+    "$target_gen_dir/js2c/content_script_bundle_unwrapped.js",
   ]
 
   args = [
@@ -148,11 +153,55 @@ npm_action("atom_browserify_content_script") {
     "-p",
     "tsconfig.electron.json",
     "]",
+    "--standalone",
+    "content_script_preload",
     "-o",
     rebase_path(outputs[0]),
   ]
 }
 
+js_wrap("atom_browserify_content_script") {
+  deps = [
+    ":atom_browserify_content_script_unwrapped",
+  ]
+
+  inputs = [
+    "$target_gen_dir/js2c/content_script_bundle_unwrapped.js",
+  ]
+
+  outputs = [
+    "$target_gen_dir/js2c/content_script_bundle.js",
+  ]
+}
+
+js_wrap("atom_browserify_isolated") {
+  deps = [
+    ":atom_browserify_isolated_unwrapped",
+  ]
+
+  inputs = [
+    "$target_gen_dir/js2c/isolated_bundle_unwrapped.js",
+  ]
+
+  outputs = [
+    "$target_gen_dir/js2c/isolated_bundle.js",
+  ]
+}
+
+js_wrap("atom_browserify_sandbox") {
+  deps = [
+    ":atom_browserify_sandbox_unwrapped",
+  ]
+
+  inputs = [
+    "$target_gen_dir/js2c/preload_bundle_unwrapped.js",
+  ]
+
+  outputs = [
+    "$target_gen_dir/js2c/preload_bundle.js",
+  ]
+}
+
 copy("atom_js2c_copy") {
   sources = [
     "lib/common/asar.js",

build/js_wrap.gni

@@ -0,0 +1,19 @@
+template("js_wrap") {
+  assert(defined(invoker.inputs), "Need input JS script")
+  assert(defined(invoker.outputs), "Need output JS script")
+
+  action(target_name) {
+    forward_variables_from(invoker,
+                           [
+                             "deps",
+                             "public_deps",
+                             "sources",
+                             "inputs",
+                             "outputs",
+                           ])
+
+    script = "//electron/build/js_wrap.py"
+    args = [ "--in" ] + rebase_path(invoker.inputs) + [ "--out" ] +
+           rebase_path(invoker.outputs)
+  }
+}

build/js_wrap.py

@@ -0,0 +1,19 @@
+import sys
+
+in_start = sys.argv.index("--in") + 1
+out_start = sys.argv.index("--out") + 1
+
+in_bundles = sys.argv[in_start:out_start - 1]
+out_bundles = sys.argv[out_start:]
+
+if len(in_bundles) is not len(out_bundles):
+  print("--out and --in must provide the same number of arguments")
+  sys.exit(1)
+
+for i in range(len(in_bundles)):
+  in_bundle = in_bundles[i]
+  out_path = out_bundles[i]
+  with open(in_bundle, 'r') as f:
+    lines = ["(function(){var exports={},module={exports};"] + f.readlines() + ["})();"]
+    with open(out_path, 'w') as out_f:
+      out_f.writelines(lines)

spec/api-browser-window-spec.js

@@ -1357,6 +1357,48 @@ describe('BrowserWindow module', () => {
     afterEach(() => { ipcMain.removeAllListeners('answer') })
 
     describe('"preload" option', () => {
+      const doesNotLeakSpec = (name, webPrefs) => {
+        it(name, async function () {
+          w.destroy()
+          w = new BrowserWindow({
+            webPreferences: {
+              ...webPrefs,
+              preload: path.resolve(fixtures, 'module', 'empty.js')
+            },
+            show: false
+          })
+          const leakResult = emittedOnce(ipcMain, 'leak-result')
+          w.loadFile(path.join(fixtures, 'api', 'no-leak.html'))
+          const [, result] = await leakResult
+          console.log(result)
+          expect(result).to.have.property('require', 'undefined')
+          expect(result).to.have.property('exports', 'undefined')
+          expect(result).to.have.property('windowExports', 'undefined')
+          expect(result).to.have.property('windowPreload', 'undefined')
+          expect(result).to.have.property('windowRequire', 'undefined')
+        })
+      }
+      doesNotLeakSpec('does not leak require', {
+        nodeIntegration: false,
+        sandbox: false,
+        contextIsolation: false
+      })
+      doesNotLeakSpec('does not leak require when sandbox is enabled', {
+        nodeIntegration: false,
+        sandbox: true,
+        contextIsolation: false
+      })
+      doesNotLeakSpec('does not leak require when context isolation is enabled', {
+        nodeIntegration: false,
+        sandbox: false,
+        contextIsolation: true
+      })
+      doesNotLeakSpec('does not leak require when context isolation and sandbox are enabled', {
+        nodeIntegration: false,
+        sandbox: true,
+        contextIsolation: true
+      })
+
       it('loads the script before other scripts in window', async () => {
         const preload = path.join(fixtures, 'module', 'set-global.js')
         w.destroy()

spec/fixtures/api/no-leak.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <title>Document</title>
+</head>
+<body>
+  <script>
+  window.postMessage({
+    require: typeof require,
+    exports: typeof exports,
+    windowRequire: typeof window.require,
+    windowExports: typeof window.exports,
+    windowPreload: typeof window.sandboxed_preload,
+  })
+  </script>
+</body>
+</html>

spec/fixtures/module/empty.js

@@ -0,0 +1,5 @@
+const { ipcRenderer } = require('electron')
+
+window.addEventListener('message', (event) => {
+  ipcRenderer.send('leak-result', event.data)
+})