handle file links, fix web links, security

src/electron/index.js

@@ -8,6 +8,7 @@ const {
 } = require("electron");
 const path = require("path");
 const fs = require("fs");
+const url = require("url");
 const { initUserFilesDir } = require("./userFilesInit");
 const settings = require("./settings");
 const migrate = require("./migrations");
@@ -70,44 +71,113 @@ try {
   );
 }
 
-/**
- * Open browser windows on link-click, an event triggered by renderer process.
- * @param {String} link
- */
-ipcMain.on("link-click", (_, link) => {
-  // This presents a security challenge: see https://github.com/cloverich/chronicles/issues/51
-  // https://www.electronjs.org/docs/latest/tutorial/security#15-do-not-use-openexternal-with-untrusted-content
-  shell.openExternal(link);
-});
-
-// Allow files to load if using the "chronicles" protocol
+// Allow files in <img> and <video> tags to load using the "chronicles://" protocol
 // https://www.electronjs.org/docs/api/protocol
 app.whenReady().then(() => {
+  // todo: registerFileProtocol is deprecated; using the new protocol method works,
+  // but videos don't seek properly.
   protocol.registerFileProtocol("chronicles", (request, callback) => {
-    // strip the leading chronicles://
-    const url = decodeURI(request.url.substr(13));
-
-    // add the USER_FILES directory
-    // todo: cache this value. The backend API updates it after start-up, otherwise all updates
-    // happen through main. Refactor so backend api calls through here, or move default user files setup
-    // logic into main, then cache the value
-    const absoluteUrl = path.join(settings.get("USER_FILES_DIR"), url);
-
-    // NOTE: If the file does not exist... ELECTRON WILL MAKE AN HTTP REQUEST WITH THE FULL URL???
-    // Seems like... odd fallback behavior.
-    // This isn't performant but is for my sanity.
-    // todo: Upgrade libraries, see if this behavior is fixed or can be disabled somehow?
-    if (!fs.existsSync(absoluteUrl)) {
-      console.warn(
-        "chronicles:// file handler could not find file:",
-        absoluteUrl,
-        "Maybe you need to set the USER_FILES or update broken file links?",
-      );
+    callback({ path: validateChroniclesUrl(request.url) });
+  });
+});
+
+/**
+ * Convert a "chronicles://" URL to an absolute file path.
+ *
+ * This function is used by the "chronicles://" protocol handler to resolve file paths
+ * to the user files directory. It also performs some basic security checks. Insecure
+ * or missing files resolve to null; in testing, passing null to the protocol handler
+ * results in 404's in img and video tags.
+ *
+ * @param {string} chroniclesUrl The "chronicles://" URL to convert.
+ */
+function validateChroniclesUrl(chroniclesUrl) {
+  // strip the leading chronicles://, then convert to absolute url
+  const url = decodeURI(chroniclesUrl.slice("chronicles://".length));
+  const baseDir = settings.get("USER_FILES_DIR");
+  const absPath = path.join(baseDir, url);
+  const normalizedPath = path.normalize(absPath);
+
+  if (!isPathWithinDirectory(normalizedPath, baseDir)) {
+    console.warn(
+      "chronicles:// file handler blocked access to file outside of user files directory:",
+      normalizedPath,
+    );
+
+    return null;
+  }
+
+  if (!fs.existsSync(normalizedPath)) {
+    console.warn(
+      "chronicles:// file handler could not find file:",
+      normalizedPath,
+      "Maybe you need to set the USER_FILES or update broken file links?",
+    );
+
+    return null;
+  }
+
+  return normalizedPath;
+}
+
+// Checks if the resolved path is within the specified directory
+function isPathWithinDirectory(resolvedPath, directory) {
+  const relative = path.relative(directory, resolvedPath);
+  return !relative.startsWith("..") && !path.isAbsolute(relative);
+}
+
+/**
+ * Checks if a URL is safe for opening in external applications.
+ * @param {string} urlString The URL to check.
+ * @returns {boolean} True if the URL is considered safe, false otherwise.
+ */
+function isSafeForExternalOpen(urlString) {
+  try {
+    const parsedUrl = new url.URL(urlString);
+
+    const allowedProtocols = ["http:", "https:", "mailto:"];
+    if (allowedProtocols.includes(parsedUrl.protocol)) {
+      return true;
+    }
+  } catch (error) {
+    console.error("isSafeForExternalOpen - Error parsing URL:", error);
+    return false;
+  }
+
+  return false;
+}
+
+/**
+ * Handle opening web and file links in system default applications.
+ * @param {string} url
+ */
+function handleLinkClick(url) {
+  if (url.startsWith("chronicles://")) {
+    const sanitized = validateChroniclesUrl(url);
+    if (sanitized) {
+      shell.showItemInFolder(sanitized);
+    } else {
+      console.warn("Blocked file navigation:", url);
     }
+  } else if (isSafeForExternalOpen(url)) {
+    shell.openExternal(url);
+  } else {
+    console.warn("Blocked navigation to:", url);
+  }
+}
 
-    // todo: santize URL: Prevent ../, ensure it points to USER_FILES directory
-    // https://github.com/cloverich/chronicles/issues/53
-    callback({ path: absoluteUrl });
+app.on("web-contents-created", (event, contents) => {
+  contents.on("will-navigate", (event, navigationUrl) => {
+    // prevent navigation from the main window
+    event.preventDefault();
+    handleLinkClick(navigationUrl);
+  });
+
+  contents.setWindowOpenHandler(({ url }) => {
+    handleLinkClick(url);
+    // Prevent the creation of new windows
+    // https://www.electronjs.org/docs/latest/tutorial/security#14-disable-or-limit-creation-of-new-windows
+    return { action: "deny" };
   });
 });
 
@@ -122,18 +192,9 @@ function createWindow() {
     width,
     height: 600,
     webPreferences: {
-      // This is needed to load electron modules
-      // Loading the electron requiring scripts in a preload
-      // script, then disabling nodeIntegration, would be
-      // more secure
-      // NOTE: Right now, loading local images also requires this.
-      // Loading images via the API would resolve this issue.
-      // ...or using a custom protocol
-      // https://github.com/electron/electron/issues/23393
       nodeIntegration: false,
       sandbox: false,
       contextIsolation: true,
-      // preload: app.isPackaged ? path.join(__dirname, 'preload.js') : path.join(__dirname, '../preload.js'),
       preload: path.join(__dirname, "preload.bundle.js"),
     },
   });
@@ -164,9 +225,6 @@ app.on("activate", () => {
   }
 });
 
-// In this file you can include the rest of your app's specific main process
-// code. You can also put them in separate files and require them here.
-
 // Preferences in UI allows user to specify database file
 ipcMain.on("select-database-file", async (event, arg) => {
   if (!mainWindow) {

src/hooks/images.tsx

@@ -68,9 +68,18 @@ export function isImageUrl(filepath: string) {
   return imageExtensions.includes(extension.toLowerCase());
 }
 
+/**
+ * Does the URL end with a known video extension?
+ */
+export function isVideoUrl(filepath: string) {
+  const extension = filepath.split(".").pop();
+  if (!extension) return false;
+  return videoExtensions.includes(extension.toLowerCase());
+}
+
 // Copied from this repo: https://github.com/arthurvr/image-extensions
 // Which is an npm package that is just a json file
-const imageExtensions = [
+export const imageExtensions = [
   "ase",
   "art",
   "bmp",

src/preload/index.ts

@@ -1,8 +1,6 @@
 import { contextBridge } from "electron";
 import { create } from "./client";
-
-import { listenLinks } from "./utils.electron";
-listenLinks();
+import "./utils.electron";
 
 contextBridge.exposeInMainWorld("chronicles", {
   createClient: create,

src/preload/utils.electron.tsx

@@ -7,42 +7,3 @@ window.onauxclick = (e) => {
   if (e.button !== 1) return;
   ipcRenderer.send("inspect-element", { x: e.x, y: e.y });
 };
-
-/**
- * NOTE: Copied from chronicles4
- *
- * Most display items are rendered via markdown. There is no built-in way
- * to convert <a> tags to use target='_blank', but this is the only way to get
- * a chrome app to actually open a new link. Below is a hack to treat all links
- * not starting with a # (ex: #/internal/app/link) as an external link request,
- * and window.open defaults to opening a new window / tab
- */
-function externalLinksListener(event: any) {
-  // hopefully only relevent on <a href="..."> elements
-  if (event.target.tagName !== "A") return;
-
-  const link = event.target.getAttribute("href");
-  // don't navigate the main window to an external url
-  event.preventDefault();
-
-  // if its not a reference link, open an external browser:
-  if (link && link.indexOf("#") !== 0) {
-    ipcRenderer.send("link-click", link);
-  }
-}
-
-/**
- * Listen for link click's and open in an external browser.
- * Otherwise, links open in _this_ window o.0
- */
-export function listenLinks() {
-  document.addEventListener("click", externalLinksListener);
-  document.addEventListener("auxclick", externalLinksListener);
-
-  // Return a function for removing the listener.. I used to use this in hot-reloading.
-  // Might use it again...
-  return () => {
-    document.removeEventListener("click", externalLinksListener);
-    document.removeEventListener("auxclick", externalLinksListener);
-  };
-}

src/views/edit/editor/index.tsx

@@ -82,6 +82,7 @@ import {
 } from "./elements";
 
 import { ELEMENT_VIDEO, createVideoPlugin } from "./plugins/createVideoPlugin";
+import { createFilesPlugin } from "./plugins/createFilesPlugin";
 
 import { uploadImage } from "../../../hooks/images";
 
@@ -109,8 +110,11 @@ export default observer((props: Props) => {
       createBasicMarksPlugin(),
 
       createLinkPlugin({
-        // Without the toolbar, links are not really useable by default.
+        // Without the toolbar, links cannot be easily edited
         renderAfterEditable: LinkFloatingToolbar as RenderAfterEditable,
+        options: {
+          allowedSchemes: ["http", "https", "mailto", "chronicles"],
+        },
       }),
 
       createListPlugin(),
@@ -129,9 +133,10 @@ export default observer((props: Props) => {
       // I'm unsure how to trigger the logic, probabzly via toolbar or shortcut.
       // createMediaEmbedPlugin(),
 
-      // NOTE: This plugin MUST come after createImagePlugin, otherwise createImagePlugin swallows
+      // NOTE: These plugins MUST come after createImagePlugin, otherwise createImagePlugin swallows
       // dropped video files and this won't be called.
       createVideoPlugin(),
+      createFilesPlugin(),
 
       // I believe this is so hitting backspace lets you select and delete the
       // image

src/views/edit/editor/plugins/createFilesPlugin.tsx

@@ -0,0 +1,59 @@
+import {
+  PlateEditor,
+  WithPlatePlugin,
+  createPluginFactory,
+  insertNode,
+  ELEMENT_LINK,
+} from "@udecode/plate";
+
+import { isVideoUrl, isImageUrl } from "../../../../hooks/images";
+
+// Ideally this is injected
+import { IClient } from "../../../../hooks/useClient";
+const client: IClient = (window as any).chronicles.createClient();
+
+const ELEMENT_FILE = "file";
+
+/**
+ * createFilesPlugin handles copying drag and dropped (non-video, non-image) files, then injecting
+ * a link element with a reference to its chronicles:// url.
+ */
+export const createFilesPlugin = createPluginFactory({
+  key: ELEMENT_FILE,
+
+  isLeaf: true,
+
+  withOverrides: (editor: PlateEditor, _: WithPlatePlugin) => {
+    // store reference to original insertData function; we'll call it later
+    const insertData = editor.insertData;
+
+    // override insertData function to handle files
+    editor.insertData = (dataTransfer: DataTransfer) => {
+      const text = dataTransfer.getData("text/plain");
+      const { files } = dataTransfer;
+
+      if (!text && files && files.length > 0) {
+        for (const file of files) {
+          // Treat anything not image / video as a file
+          if (isImageUrl(file.name) || isVideoUrl(file.name)) {
+            // If it's not a file, delegate to the next plugin.
+            insertData(dataTransfer);
+          } else {
+            client.files.uploadFile(file).then((json: any) => {
+              insertNode(editor, {
+                type: ELEMENT_LINK,
+                url: `chronicles://${json.filename}`,
+                children: [{ text: `File: ${json.filename}` }],
+              });
+            });
+          }
+        }
+      } else {
+        // If it's not a file, delegate to the next plugin.
+        insertData(dataTransfer);
+      }
+    };
+
+    return editor;
+  },
+});

src/views/edit/editor/plugins/createVideoPlugin.tsx

@@ -1,13 +1,10 @@
-import React from "react";
 import {
   PlateEditor,
-  PlateRenderElementProps,
-  PlateElement,
   WithPlatePlugin,
   createPluginFactory,
   insertNode,
 } from "@udecode/plate";
-import { videoExtensions } from "../../../../hooks/images";
+import { isVideoUrl } from "../../../../hooks/images";
 // Ideally this is injected
 import { IClient } from "../../../../hooks/useClient";
 
@@ -46,7 +43,7 @@ export const createVideoPlugin = createPluginFactory({
           if (mime == "video") {
             // The slate-mdast parser / serializer relies on a whitelist of
             // extensions to parse and serialize the video element correctly.
-            if (!videoExtensions.includes(extension || "")) {
+            if (!isVideoUrl(file.name)) {
               console.error("Unsupported video extension:", extension);
               continue;
             }