[PM-3756] Disable node integration and enable context isolation in desktop (#6975)

* Disable node integration and enable context isolation

* Review comments

* Log in renderer through IPC

* Missed imports

* Mock electron API

* resourcesPath is undefined in the preload, but process.windowsStore works correctly

* Replace fromBufferToUtf8 conditional implementation for the `buffer` package

The current non-node implementation is different than the node implementation,
as the non-node would break when the contents can't be parsed as a URI component.
Replacing the impl by the `buffer` package makes the result match in both environments.

* Fix lint

* Add some more tests

* Remove buffer from devDependencies

apps/desktop/src/app/accounts/settings.component.ts

@@ -584,7 +584,10 @@ export class SettingsComponent implements OnInit {
   }
 
   async saveBrowserIntegration() {
-    if (process.platform === "darwin" && !this.platformUtilsService.isMacAppStore()) {
+    if (
+      ipc.platform.deviceType === DeviceType.MacOsDesktop &&
+      !this.platformUtilsService.isMacAppStore()
+    ) {
       await this.dialogService.openSimpleDialog({
         title: { key: "browserIntegrationUnsupportedTitle" },
         content: { key: "browserIntegrationMasOnlyDesc" },
@@ -606,7 +609,7 @@ export class SettingsComponent implements OnInit {
 
       this.form.controls.enableBrowserIntegration.setValue(false);
       return;
-    } else if (process.platform == "linux") {
+    } else if (ipc.platform.deviceType === DeviceType.LinuxDesktop) {
       await this.dialogService.openSimpleDialog({
         title: { key: "browserIntegrationUnsupportedTitle" },
         content: { key: "browserIntegrationLinuxDesc" },

apps/desktop/src/app/main.ts

@@ -1,18 +1,12 @@
 import { enableProdMode } from "@angular/core";
 import { platformBrowserDynamic } from "@angular/platform-browser-dynamic";
 
-import { ipc } from "../preload";
-import { isDev } from "../utils";
-
-// Temporary polyfill for preload script
-(window as any).ipc = ipc;
-
 require("../scss/styles.scss");
 require("../scss/tailwind.css");
 
 import { AppModule } from "./app.module";
 
-if (!isDev()) {
+if (!ipc.platform.isDev) {
   enableProdMode();
 }
 

apps/desktop/src/app/services/services.module.ts

@@ -49,7 +49,7 @@ import { DialogService } from "@bitwarden/components";
 import { LoginGuard } from "../../auth/guards/login.guard";
 import { Account } from "../../models/account";
 import { ElectronCryptoService } from "../../platform/services/electron-crypto.service";
-import { ElectronLogService } from "../../platform/services/electron-log.service";
+import { ElectronLogRendererService } from "../../platform/services/electron-log.renderer.service";
 import { ElectronPlatformUtilsService } from "../../platform/services/electron-platform-utils.service";
 import { ElectronRendererMessagingService } from "../../platform/services/electron-renderer-messaging.service";
 import { ElectronRendererSecureStorageService } from "../../platform/services/electron-renderer-secure-storage.service";
@@ -91,7 +91,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK");
       provide: RELOAD_CALLBACK,
       useValue: null,
     },
-    { provide: LogServiceAbstraction, useClass: ElectronLogService, deps: [] },
+    { provide: LogServiceAbstraction, useClass: ElectronLogRendererService, deps: [] },
     {
       provide: PlatformUtilsServiceAbstraction,
       useClass: ElectronPlatformUtilsService,

apps/desktop/src/main.ts

@@ -30,14 +30,14 @@ import { Account } from "./models/account";
 import { BiometricsService, BiometricsServiceAbstraction } from "./platform/main/biometric/index";
 import { ClipboardMain } from "./platform/main/clipboard.main";
 import { DesktopCredentialStorageListener } from "./platform/main/desktop-credential-storage-listener";
-import { ElectronLogService } from "./platform/services/electron-log.service";
+import { ElectronLogMainService } from "./platform/services/electron-log.main.service";
 import { ElectronStateService } from "./platform/services/electron-state.service";
 import { ElectronStorageService } from "./platform/services/electron-storage.service";
 import { I18nMainService } from "./platform/services/i18n.main.service";
 import { ElectronMainMessagingService } from "./services/electron-main-messaging.service";
 
 export class Main {
-  logService: ElectronLogService;
+  logService: ElectronLogMainService;
   i18nService: I18nMainService;
   storageService: ElectronStorageService;
   memoryStorageService: MemoryStorageService;
@@ -89,8 +89,7 @@ export class Main {
       });
     }
 
-    this.logService = new ElectronLogService(null, app.getPath("userData"));
-    this.logService.init();
+    this.logService = new ElectronLogMainService(null, app.getPath("userData"));
     this.i18nService = new I18nMainService("en", "./locales/");
 
     const storageDefaults: any = {};

apps/desktop/src/main/window.main.ts

@@ -160,11 +160,11 @@ export class WindowMain {
       backgroundColor: await this.getBackgroundColor(),
       alwaysOnTop: this.enableAlwaysOnTop,
       webPreferences: {
-        // preload: path.join(__dirname, "preload.js"),
+        preload: path.join(__dirname, "preload.js"),
         spellcheck: false,
-        nodeIntegration: true,
+        nodeIntegration: false,
         backgroundThrottling: false,
-        contextIsolation: false,
+        contextIsolation: true,
         session: this.session,
       },
     });

apps/desktop/src/platform/preload.ts

@@ -1,7 +1,7 @@
 import { ipcRenderer } from "electron";
 
 import { DeviceType } from "@bitwarden/common/enums";
-import { ThemeType, KeySuffixOptions } from "@bitwarden/common/platform/enums";
+import { ThemeType, KeySuffixOptions, LogLevelType } from "@bitwarden/common/platform/enums";
 import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
 
 import {
@@ -11,7 +11,7 @@ import {
   UnencryptedMessageResponse,
 } from "../models/native-messaging";
 import { BiometricMessage, BiometricAction } from "../types/biometric-message";
-import { isDev, isWindowsStore } from "../utils";
+import { isDev, isMacAppStore, isWindowsStore } from "../utils";
 
 import { ClipboardWriteMessage } from "./types/clipboard";
 
@@ -82,8 +82,10 @@ export default {
   },
   deviceType: deviceType(),
   isDev: isDev(),
+  isMacAppStore: isMacAppStore(),
   isWindowsStore: isWindowsStore(),
   reloadProcess: () => ipcRenderer.send("reload-process"),
+  log: (level: LogLevelType, message: string) => ipcRenderer.invoke("ipc.log", { level, message }),
 
   openContextMenu: (
     menu: {

apps/desktop/src/platform/services/electron-log.main.service.ts

@@ -1,22 +1,20 @@
 import * as path from "path";
 
-import log from "electron-log";
+import { ipcMain } from "electron";
+import log from "electron-log/main";
 
 import { LogLevelType } from "@bitwarden/common/platform/enums/log-level-type.enum";
 import { ConsoleLogService as BaseLogService } from "@bitwarden/common/platform/services/console-log.service";
 
 import { isDev } from "../../utils";
 
-export class ElectronLogService extends BaseLogService {
+export class ElectronLogMainService extends BaseLogService {
   constructor(
     protected filter: (level: LogLevelType) => boolean = null,
     private logDir: string = null,
   ) {
     super(isDev(), filter);
-  }
 
-  // Initialize the log file transport. Only needs to be done once in the main process.
-  init() {
     if (log.transports == null) {
       return;
     }
@@ -26,6 +24,10 @@ export class ElectronLogService extends BaseLogService {
       log.transports.file.resolvePathFn = () => path.join(this.logDir, "app.log");
     }
     log.initialize();
+
+    ipcMain.handle("ipc.log", (_event, { level, message }) => {
+      this.write(level, message);
+    });
   }
 
   write(level: LogLevelType, message: string) {

apps/desktop/src/platform/services/electron-log.renderer.service.ts

@@ -0,0 +1,35 @@
+import { LogLevelType } from "@bitwarden/common/platform/enums/log-level-type.enum";
+import { ConsoleLogService as BaseLogService } from "@bitwarden/common/platform/services/console-log.service";
+
+export class ElectronLogRendererService extends BaseLogService {
+  constructor(protected filter: (level: LogLevelType) => boolean = null) {
+    super(ipc.platform.isDev, filter);
+  }
+
+  write(level: LogLevelType, message: string) {
+    if (this.filter != null && this.filter(level)) {
+      return;
+    }
+
+    /* eslint-disable no-console */
+    ipc.platform.log(level, message).catch((e) => console.log("Error logging", e));
+
+    /* eslint-disable no-console */
+    switch (level) {
+      case LogLevelType.Debug:
+        console.debug(message);
+        break;
+      case LogLevelType.Info:
+        console.info(message);
+        break;
+      case LogLevelType.Warning:
+        console.warn(message);
+        break;
+      case LogLevelType.Error:
+        console.error(message);
+        break;
+      default:
+        break;
+    }
+  }
+}

apps/desktop/src/platform/services/electron-log.service.spec.ts

@@ -1,9 +1,14 @@
-import { ElectronLogService } from "./electron-log.service";
+import { ElectronLogMainService } from "./electron-log.main.service";
 
-describe("ElectronLogService", () => {
+// Mock the use of the electron API to avoid errors
+jest.mock("electron", () => ({
+  ipcMain: { handle: jest.fn() },
+}));
+
+describe("ElectronLogMainService", () => {
   it("sets dev based on electron method", () => {
     process.env.ELECTRON_IS_DEV = "1";
-    const logService = new ElectronLogService();
+    const logService = new ElectronLogMainService();
     expect(logService).toEqual(expect.objectContaining({ isDev: true }) as any);
   });
 });

apps/desktop/src/platform/services/electron-platform-utils.service.ts

@@ -6,7 +6,6 @@ import {
   PlatformUtilsService,
 } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 
-import { isMacAppStore } from "../../utils";
 import { ClipboardWriteMessage } from "../types/clipboard";
 
 export class ElectronPlatformUtilsService implements PlatformUtilsService {
@@ -53,7 +52,7 @@ export class ElectronPlatformUtilsService implements PlatformUtilsService {
   }
 
   isMacAppStore(): boolean {
-    return isMacAppStore();
+    return ipc.platform.isMacAppStore;
   }
 
   isViewOpen(): Promise<boolean> {

apps/desktop/src/preload.ts

@@ -1,4 +1,5 @@
-// import { contextBridge } from "electron";
+import { contextBridge } from "electron";
+
 import auth from "./auth/preload";
 import platform from "./platform/preload";
 
@@ -18,4 +19,4 @@ export const ipc = {
   platform,
 };
 
-// contextBridge.exposeInMainWorld("ipc", ipc);
+contextBridge.exposeInMainWorld("ipc", ipc);

apps/desktop/src/utils.ts

@@ -55,7 +55,7 @@ export function isWindowsStore() {
   if (
     windows &&
     !windowsStore &&
-    process.resourcesPath.indexOf("8bitSolutionsLLC.bitwardendesktop_") > -1
+    process.resourcesPath?.indexOf("8bitSolutionsLLC.bitwardendesktop_") > -1
   ) {
     windowsStore = true;
   }

apps/desktop/webpack.renderer.js

@@ -54,6 +54,10 @@ const common = {
     extensions: [".tsx", ".ts", ".js"],
     symlinks: false,
     modules: [path.resolve("../../node_modules")],
+    fallback: {
+      path: require.resolve("path-browserify"),
+      fs: false,
+    },
   },
   output: {
     filename: "[name].js",
@@ -64,9 +68,7 @@ const common = {
 const renderer = {
   mode: NODE_ENV,
   devtool: "source-map",
-  // TODO: Replace this with web.
-  // target: "web",
-  target: "electron-renderer",
+  target: "web",
   node: {
     __dirname: false,
   },

libs/common/src/platform/misc/utils.spec.ts

@@ -258,6 +258,7 @@ describe("Utils Service", () => {
     });
   }
 
+  const asciiHelloWorld = "hello world";
   const asciiHelloWorldArray = [104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100];
   const b64HelloWorldString = "aGVsbG8gd29ybGQ=";
 
@@ -623,4 +624,59 @@ describe("Utils Service", () => {
       expect(Utils.daysRemaining(new Date(2024, 9, 2, 10))).toBe(366);
     });
   });
+
+  describe("fromBufferToUtf8(...)", () => {
+    const originalIsNode = Utils.isNode;
+
+    afterEach(() => {
+      Utils.isNode = originalIsNode;
+    });
+
+    runInBothEnvironments("should convert an ArrayBuffer to a utf8 string", () => {
+      const buffer = new Uint8Array(asciiHelloWorldArray).buffer;
+      const str = Utils.fromBufferToUtf8(buffer);
+      expect(str).toBe(asciiHelloWorld);
+    });
+
+    runInBothEnvironments("should handle an empty buffer", () => {
+      const buffer = new ArrayBuffer(0);
+      const str = Utils.fromBufferToUtf8(buffer);
+      expect(str).toBe("");
+    });
+
+    runInBothEnvironments("should convert a binary ArrayBuffer to a binary string", () => {
+      const cases = [
+        {
+          input: [
+            174, 21, 17, 79, 39, 130, 132, 173, 49, 180, 113, 118, 160, 15, 47, 99, 57, 208, 141,
+            187, 54, 194, 153, 12, 37, 130, 155, 213, 125, 196, 241, 101,
+          ],
+          output: "<22>O'<27><><EFBFBD>1<EFBFBD>qv<71>/c9Ѝ<39>6™%<25><><EFBFBD>}<7D><>e",
+        },
+        {
+          input: [
+            88, 17, 69, 41, 75, 69, 128, 225, 252, 219, 146, 72, 162, 14, 139, 120, 30, 239, 105,
+            229, 14, 131, 174, 119, 61, 88, 108, 135, 60, 88, 120, 145,
+          ],
+          output: "XE)KE<4B><45><EFBFBD>ےH<DB92><0E>x<1E>i<EFBFBD><0E><>w=Xl<58><Xx<58>",
+        },
+        {
+          input: [
+            121, 110, 81, 148, 48, 67, 209, 43, 3, 39, 143, 184, 237, 184, 213, 183, 84, 157, 47, 6,
+            31, 183, 99, 142, 155, 156, 192, 107, 118, 64, 176, 36,
+          ],
+          output: "ynQ<6E>0C<30>+'<27><><EFBFBD><EFBFBD>շT<D5B7>/<1F>c<EFBFBD><63><EFBFBD><EFBFBD>kv@<40>$",
+        },
+      ];
+
+      cases.forEach((c) => {
+        const buffer = new Uint8Array(c.input).buffer;
+        const str = Utils.fromBufferToUtf8(buffer);
+        // Match the expected output
+        expect(str).toBe(c.output);
+        // Make sure it matches with the Node.js Buffer output
+        expect(str).toBe(Buffer.from(buffer).toString("utf8"));
+      });
+    });
+  });
 });

libs/common/src/platform/misc/utils.ts

@@ -1,6 +1,7 @@
 /* eslint-disable no-useless-escape */
 import * as path from "path";
 
+import { Buffer as BufferLib } from "buffer/";
 import { Observable, of, switchMap } from "rxjs";
 import { getHostname, parse } from "tldts";
 import { Merge } from "type-fest";
@@ -145,13 +146,7 @@ export class Utils {
   }
 
   static fromBufferToUtf8(buffer: ArrayBuffer): string {
-    if (Utils.isNode) {
-      return Buffer.from(buffer).toString("utf8");
-    } else {
-      const bytes = new Uint8Array(buffer);
-      const encodedString = String.fromCharCode.apply(null, bytes);
-      return decodeURIComponent(escape(encodedString));
-    }
+    return BufferLib.from(buffer).toString("utf8");
   }
 
   static fromBufferToByteString(buffer: ArrayBuffer): string {

package.json

@@ -83,7 +83,6 @@
     "@webcomponents/custom-elements": "1.6.0",
     "autoprefixer": "10.4.16",
     "base64-loader": "1.0.0",
-    "buffer": "6.0.3",
     "chromatic": "10.0.0",
     "clean-webpack-plugin": "4.0.0",
     "concurrently": "8.2.2",
@@ -169,6 +168,7 @@
     "big-integer": "1.6.51",
     "bootstrap": "4.6.0",
     "braintree-web-drop-in": "1.42.0",
+    "buffer": "6.0.3",
     "bufferutil": "4.0.8",
     "chalk": "4.1.2",
     "commander": "11.1.0",