Replace deprecated file protocol registration (actualbudget#3475)

* replace deprecated file handler in electron * release notes * types eh * types
MikesGlitch · Sep 21, 2024 · ec55e8d · ec55e8d
1 parent a1bc66b
commit ec55e8d
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 22 deletions.
diff --git a/packages/desktop-electron/index.ts b/packages/desktop-electron/index.ts
@@ -3,6 +3,7 @@ import Module from 'module';
 import path from 'path';
 
 import {
+  net,
   app,
   ipcMain,
   BrowserWindow,
@@ -17,8 +18,6 @@ import {
   SaveDialogOptions,
 } from 'electron';
 import isDev from 'electron-is-dev';
-// @ts-strict-ignore
-import fetch from 'node-fetch';
 import promiseRetry from 'promise-retry';
 
 import { getMenu } from './menu';
@@ -37,8 +36,6 @@ protocol.registerSchemesAsPrivileged([
   { scheme: 'app', privileges: { standard: true } },
 ]);
 
-global.fetch = fetch;
-
 if (!isDev || !process.env.ACTUAL_DOCUMENT_DIR) {
   process.env.ACTUAL_DOCUMENT_DIR = app.getPath('documents');
 }
@@ -63,13 +60,13 @@ function createBackgroundProcess() {
     isDev ? { execArgv: ['--inspect'], stdio: 'pipe' } : { stdio: 'pipe' },
   );
 
-  serverProcess.stdout.on('data', (chunk: Buffer) => {
+  serverProcess.stdout?.on('data', (chunk: Buffer) => {
     // Send the Server console.log messages to the main browser window
     clientWin?.webContents.executeJavaScript(`
       console.info('Server Log:', ${JSON.stringify(chunk.toString('utf8'))})`);
   });
 
-  serverProcess.stderr.on('data', (chunk: Buffer) => {
+  serverProcess.stderr?.on('data', (chunk: Buffer) => {
     // Send the Server console.error messages out to the main browser window
     clientWin?.webContents.executeJavaScript(`
       console.error('Server Log:', ${JSON.stringify(chunk.toString('utf8'))})`);
@@ -111,6 +108,7 @@ async function createWindow() {
       preload: __dirname + '/preload.js',
     },
   });
+
   win.setBackgroundColor('#E8ECF0');
 
   if (isDev) {
@@ -220,34 +218,49 @@ app.on('ready', async () => {
   // Install an `app://` protocol that always returns the base HTML
   // file no matter what URL it is. This allows us to use react-router
   // on the frontend
-  protocol.registerFileProtocol('app', (request, callback) => {
+  protocol.handle('app', request => {
     if (request.method !== 'GET') {
-      callback({ error: -322 }); // METHOD_NOT_SUPPORTED from chromium/src/net/base/net_error_list.h
-      return null;
+      return new Response(null, {
+        status: 405,
+        statusText: 'Method Not Allowed',
+      });
     }
 
     const parsedUrl = new URL(request.url);
     if (parsedUrl.protocol !== 'app:') {
-      callback({ error: -302 }); // UNKNOWN_URL_SCHEME
-      return;
+      return new Response(null, {
+        status: 404,
+        statusText: 'Unknown URL Scheme',
+      });
     }
 
     if (parsedUrl.host !== 'actual') {
-      callback({ error: -105 }); // NAME_NOT_RESOLVED
-      return;
+      return new Response(null, {
+        status: 404,
+        statusText: 'Host Not Resolved',
+      });
     }
 
     const pathname = parsedUrl.pathname;
 
+    let filePath = path.normalize(`${__dirname}/client-build/index.html`); // default web path
+
     if (pathname.startsWith('/static')) {
-      callback({
-        path: path.normalize(`${__dirname}/client-build${pathname}`),
-      });
-    } else {
-      callback({
-        path: path.normalize(`${__dirname}/client-build/index.html`),
-      });
+      // static assets
+      filePath = path.normalize(`${__dirname}/client-build${pathname}`);
+      const resolvedPath = path.resolve(filePath);
+      const clientBuildPath = path.resolve(__dirname, 'client-build');
+
+      // Ensure filePath is within client-build directory - prevents directory traversal vulnerability
+      if (!resolvedPath.startsWith(clientBuildPath)) {
+        return new Response(null, {
+          status: 403,
+          statusText: 'Forbidden',
+        });
+      }
     }
+
+    return net.fetch(`file:///${filePath}`);
   });
 
   if (process.argv[1] !== '--server') {
@@ -298,8 +311,11 @@ ipcMain.on('get-bootstrap-data', event => {
 });
 
 ipcMain.handle('restart-server', () => {
-  serverProcess.kill();
-  serverProcess = null;
+  if (serverProcess) {
+    serverProcess.kill();
+    serverProcess = null;
+  }
+
   createBackgroundProcess();
 });
 

diff --git a/upcoming-release-notes/3475.md b/upcoming-release-notes/3475.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [MikesGlitch]
+---
+
+Replacing the deprecated Electron file handler protocol with the new version