fix: blend node and blink code generation policy when both are loaded (#36567)

patches/chromium/.patches

@@ -123,4 +123,5 @@ build_only_use_the_mas_build_config_in_the_required_components.patch
 fix_tray_icon_gone_on_lock_screen.patch
 chore_introduce_blocking_api_for_electron.patch
 chore_patch_out_partition_attribute_dcheck_for_webviews.patch
+expose_v8initializer_codegenerationcheckcallbackinmainthread.patch
 chore_patch_out_profile_methods_in_profile_selections_cc.patch

patches/chromium/expose_v8initializer_codegenerationcheckcallbackinmainthread.patch

@@ -0,0 +1,38 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Rose <japthorp@slack-corp.com>
+Date: Mon, 5 Dec 2022 14:27:20 -0800
+Subject: expose V8Initializer::CodeGenerationCheckCallbackInMainThread
+
+This is needed to blend Blink and Node's policy for code generation policy.
+
+diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+index 7f2f8101c043a66a8c771bc21833ec541ea55101..d151ab58afb6014d4bf63a1a1464caecdce69715 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
+@@ -443,8 +443,9 @@ TrustedTypesCodeGenerationCheck(v8::Local<v8::Context> context,
+   return {true, V8String(context->GetIsolate(), stringified_source)};
+ }
+ 
+-static v8::ModifyCodeGenerationFromStringsResult
+-CodeGenerationCheckCallbackInMainThread(v8::Local<v8::Context> context,
++// static
++v8::ModifyCodeGenerationFromStringsResult
++V8Initializer::CodeGenerationCheckCallbackInMainThread(v8::Local<v8::Context> context,
+                                         v8::Local<v8::Value> source,
+                                         bool is_code_like) {
+   // The TC39 "Dynamic Code Brand Check" feature is currently behind a flag.
+diff --git a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
+index 39d1351a4a8bbc95097640be54affec426abe73e..30a36cf16d4a8f4692ec6a13be1217212390172a 100644
+--- a/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
++++ b/third_party/blink/renderer/bindings/core/v8/v8_initializer.h
+@@ -69,6 +69,10 @@ class CORE_EXPORT V8Initializer {
+                                          v8::Local<v8::Value>);
+   static void MessageHandlerInWorker(v8::Local<v8::Message>,
+                                      v8::Local<v8::Value>);
++  static v8::ModifyCodeGenerationFromStringsResult
++  CodeGenerationCheckCallbackInMainThread(v8::Local<v8::Context> context,
++                                          v8::Local<v8::Value> source,
++                                          bool is_code_like);
+   static bool WasmCodeGenerationCheckCallbackInMainThread(
+       v8::Local<v8::Context> context,
+       v8::Local<v8::String> source);

patches/node/.patches

@@ -52,3 +52,4 @@ fix_prevent_changing_functiontemplateinfo_after_publish.patch
 chore_add_missing_algorithm_include.patch
 fix_tolocalstring_unicode_mismatch.patch
 enable_crashpad_linux_node_processes.patch
+allow_embedder_to_control_codegenerationfromstringscallback.patch

patches/node/allow_embedder_to_control_codegenerationfromstringscallback.patch

@@ -0,0 +1,44 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Rose <japthorp@slack-corp.com>
+Date: Mon, 5 Dec 2022 14:28:40 -0800
+Subject: allow embedder to control CodeGenerationFromStringsCallback
+
+This is needed to blend Blink and Node's code generation policy.
+
+This should be upstreamed.
+
+diff --git a/src/api/environment.cc b/src/api/environment.cc
+index b6981c37d5b286e22f24d11751eb05f72ca27619..c4a3a54753767e7686a0e32996bcda0c90f659a7 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -259,11 +259,15 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+   isolate->SetMicrotasksPolicy(s.policy);
+ 
++  // Allow the embedder first chance at policy decisions.
++  // This is particularly important for embedders that combine Node and Blink,
++  // as Blink must be able to make Content Security Policy-based decisions.
+   auto* allow_wasm_codegen_cb = s.allow_wasm_code_generation_callback ?
+     s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
+   isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
+-  isolate->SetModifyCodeGenerationFromStringsCallback(
+-      ModifyCodeGenerationFromStrings);
++  auto* modify_code_generation_from_strings_callback = s.modify_code_generation_from_strings_callback ?
++    s.modify_code_generation_from_strings_callback : ModifyCodeGenerationFromStrings;
++  isolate->SetModifyCodeGenerationFromStringsCallback(modify_code_generation_from_strings_callback);
+ 
+   Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
+   if (per_process::cli_options->get_per_isolate_options()
+diff --git a/src/node.h b/src/node.h
+index bee494f9da8470530ee9ec58958f8f2c7ce6a302..387ce8e5edf6524db170e2d46ef501a1cd956d98 100644
+--- a/src/node.h
++++ b/src/node.h
+@@ -461,6 +461,8 @@ struct IsolateSettings {
+   v8::PromiseRejectCallback promise_reject_callback = nullptr;
+   v8::AllowWasmCodeGenerationCallback
+       allow_wasm_code_generation_callback = nullptr;
++  v8::ModifyCodeGenerationFromStringsCallback2
++      modify_code_generation_from_strings_callback = nullptr;
+ };
+ 
+ // Overriding IsolateSettings may produce unexpected behavior

shell/common/node_bindings.cc

@@ -158,19 +158,35 @@ void V8FatalErrorCallback(const char* location, const char* message) {
 }
 
 bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
-                                     v8::Local<v8::String>) {
+                                     v8::Local<v8::String> source) {
   // If we're running with contextIsolation enabled in the renderer process,
   // fall back to Blink's logic.
-  v8::Isolate* isolate = context->GetIsolate();
-  if (node::Environment::GetCurrent(isolate) == nullptr) {
+  if (node::Environment::GetCurrent(context) == nullptr) {
     if (gin_helper::Locker::IsBrowserProcess())
       return false;
     return blink::V8Initializer::WasmCodeGenerationCheckCallbackInMainThread(
-        context, v8::String::Empty(isolate));
+        context, source);
   }
 
-  return node::AllowWasmCodeGenerationCallback(context,
-                                               v8::String::Empty(isolate));
+  return node::AllowWasmCodeGenerationCallback(context, source);
+}
+
+v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Value> source,
+    bool is_code_like) {
+  // If we're running with contextIsolation enabled in the renderer process,
+  // fall back to Blink's logic.
+  if (node::Environment::GetCurrent(context) == nullptr) {
+    if (gin_helper::Locker::IsBrowserProcess()) {
+      NOTREACHED();
+      return {false, {}};
+    }
+    return blink::V8Initializer::CodeGenerationCheckCallbackInMainThread(
+        context, source, is_code_like);
+  }
+
+  return node::ModifyCodeGenerationFromStrings(context, source, is_code_like);
 }
 
 void ErrorMessageListener(v8::Local<v8::Message> message,
@@ -543,6 +559,8 @@ node::Environment* NodeBindings::CreateEnvironment(
   // Use a custom callback here to allow us to leverage Blink's logic in the
   // renderer process.
   is.allow_wasm_code_generation_callback = AllowWasmCodeGenerationCallback;
+  is.modify_code_generation_from_strings_callback =
+      ModifyCodeGenerationFromStrings;
 
   if (browser_env_ == BrowserEnvironment::kBrowser ||
       browser_env_ == BrowserEnvironment::kUtility) {

spec/chromium-spec.ts

@@ -355,6 +355,30 @@ describe('web security', () => {
     });
   });
 
+  describe('csp in sandbox: false', () => {
+    it('is correctly applied', async () => {
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: { sandbox: false }
+      });
+      w.loadURL(`data:text/html,<head>
+          <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self' 'unsafe-inline'">
+        </head>
+        <script>
+          try {
+            // We use console.log here because it is easier than making a
+            // preload script, and the behavior under test changes when
+            // contextIsolation: false
+            console.log(eval('failure'))
+          } catch (e) {
+            console.log('success')
+          }
+        </script>`);
+      const [,, message] = await emittedOnce(w.webContents, 'console-message');
+      expect(message).to.equal('success');
+    });
+  });
+
   it('does not crash when multiple WebContent are created with web security disabled', () => {
     const options = { show: false, webPreferences: { webSecurity: false } };
     const w1 = new BrowserWindow(options);