Refuse to overwrite any directory containing the home directory or th…

…e profile directory

__test__/backup.test.ts

@@ -1,6 +1,7 @@
 import { Backup } from "../src/Backup";
 import * as fs from "fs-extra";
 import * as path from "path";
+import * as os from "os";
 import { when } from "jest-when";
 import { sevenZip } from "../src/sevenZip";
 import joplin from "api";
@@ -34,6 +35,7 @@ const spyOnGlobalValue = jest.spyOn(joplin.settings, "globalValue");
 const spyOnSettingsSetValue = jest
   .spyOn(joplin.settings, "setValue")
   .mockImplementation();
+const homeDirMock = jest.spyOn(os, "homedir");
 
 async function createTestStructure() {
   const test = await getTestPaths();
@@ -180,7 +182,7 @@ describe("Backup", function () {
     });
 
     it(`relative paths`, async () => {
-      const backupPath = "../";
+      const backupPath = "../foo";
       /* prettier-ignore */
       when(spyOnsSettingsValue)
       .calledWith("path").mockImplementation(() => Promise.resolve(backupPath));
@@ -192,6 +194,29 @@ describe("Backup", function () {
       expect(backup.log.error).toHaveBeenCalledTimes(0);
       expect(backup.log.warn).toHaveBeenCalledTimes(0);
     });
+
+    it.each([
+      os.homedir(),
+      path.dirname(os.homedir()),
+      path.join(os.homedir(), "Desktop"),
+      path.join(os.homedir(), "Documents"),
+
+      // Avoid including system-specific paths here. For example,
+      // testing with "C:\Windows" fails on POSIX systems because it is interpreted
+      // as a relative path.
+    ])(
+      "should not allow backup path (%s) to be an important system directory",
+      async (path) => {
+        when(spyOnsSettingsValue)
+          .calledWith("path")
+          .mockImplementation(() => Promise.resolve(path));
+        backup.createSubfolder = false;
+
+        await backup.loadBackupPath();
+
+        expect(backup.backupBasePath).toBe(null);
+      }
+    );
   });
 
   describe("Div", function () {

__test__/help.test.ts

@@ -1,3 +1,4 @@
+import * as path from "path";
 import { helper } from "../src/helper";
 
 describe("Test helper", function () {
@@ -146,4 +147,43 @@ describe("Test helper", function () {
       ).toBe(testCase.expected);
     }
   });
+
+  test.each([
+    // Equality
+    ["/tmp/this/is/a/test", "/tmp/this/is/a/test", true],
+    ["/tmp/test", "/tmp/test///", true],
+
+    // Subdirectories
+    ["/tmp", "/tmp/test", true],
+    ["/tmp/", "/tmp/test", true],
+    ["/tmp/", "/tmp/..test", true],
+    ["/tmp/test", "/tmp/", false],
+
+    // Different directories
+    ["/tmp/", "/tmp/../test", false],
+    ["/tmp/te", "/tmp/test", false],
+    ["a", "/a", false],
+    ["/a/b", "/b/c", false],
+  ])(
+    "isSubdirectoryOrEqual with POSIX paths (is %s the parent of %s?)",
+    (path1, path2, expected) => {
+      expect(helper.isSubdirectoryOrEqual(path1, path2, path.posix)).toBe(
+        expected
+      );
+    }
+  );
+
+  test.each([
+    ["C:\\Users\\User\\", "C:\\Users\\User\\", true],
+    ["D:\\Users\\User\\", "C:\\Users\\User\\", false],
+    ["C:\\Users\\Userr\\", "C:\\Users\\User\\", false],
+    ["C:\\Users\\User\\", "C:\\Users\\User\\.config\\joplin-desktop", true],
+  ])(
+    "isSubdirectoryOrEqual with Windows paths (is %s the parent of %s?)",
+    (path1, path2, expected) => {
+      expect(helper.isSubdirectoryOrEqual(path1, path2, path.win32)).toBe(
+        expected
+      );
+    }
+  );
 });

src/Backup.ts

@@ -4,6 +4,7 @@ import joplin from "api";
 import * as path from "path";
 import backupLogging from "electron-log";
 import * as fs from "fs-extra";
+import * as os from "os";
 import { sevenZip } from "./sevenZip";
 import * as moment from "moment";
 import { helper } from "./helper";
@@ -288,11 +289,31 @@ class Backup {
       }
     }
 
-    if (path.normalize(profileDir) === this.backupBasePath) {
-      this.backupBasePath = null;
-      await this.showError(
-        i18n.__("msg.error.backupPathJoplinDir", path.normalize(profileDir))
-      );
+    // Creating a backup can overwrite the backup directory. Thus,
+    // we mark several system and user directories as not-overwritable.
+    const systemDirectories = [
+      profileDir,
+      os.homedir(),
+
+      path.join(os.homedir(), "Desktop"),
+      path.join(os.homedir(), "Documents"),
+      path.join(os.homedir(), "Downloads"),
+      path.join(os.homedir(), "Pictures"),
+    ];
+
+    if (os.platform() === "win32") {
+      systemDirectories.push("C:\\Windows");
+    }
+
+    for (const systemDirectory of systemDirectories) {
+      if (helper.isSubdirectoryOrEqual(this.backupBasePath, systemDirectory)) {
+        const invalidBackupPath = this.backupBasePath;
+        this.backupBasePath = null;
+        await this.showError(
+          i18n.__("msg.error.backupPathContainsImportantDir", invalidBackupPath)
+        );
+        break;
+      }
     }
   }
 

src/helper.ts

@@ -1,4 +1,5 @@
 import joplin from "api";
+import * as path from "path";
 
 export namespace helper {
   export async function validFileName(fileName: string) {
@@ -65,4 +66,28 @@ export namespace helper {
 
     return result;
   }
+
+  // Doesn't resolve simlinks
+  // See https://stackoverflow.com/questions/44892672/how-to-check-if-two-paths-are-the-same-in-npm
+  // for possible alternative implementations.
+  export function isSubdirectoryOrEqual(
+    parent: string,
+    possibleChild: string,
+
+    // Testing only
+    pathModule: typeof path = path
+  ) {
+    // Appending path.sep to handle this case:
+    //   parent: /a/b/test
+    //   possibleChild: /a/b/test2
+    // "/a/b/test2".startsWith("/a/b/test") -> true, but
+    // "/a/b/test2/".startsWith("/a/b/test/") -> false
+    //
+    // Note that .resolve removes trailing slashes.
+    //
+    const normalizedParent = pathModule.resolve(parent) + pathModule.sep;
+    const normalizedChild = pathModule.resolve(possibleChild) + pathModule.sep;
+
+    return normalizedChild.startsWith(normalizedParent);
+  }
 }

src/locales/de_DE.json

@@ -13,7 +13,6 @@
       "Backup": "Backup Fehler für %s: %s",
       "fileCopy": "Fehler beim kopieren von Datei/Ordner in %s: %s",
       "deleteFile": "Fehler beim löschen von Datei/Ordner in %s: %s",
-      "backupPathJoplinDir": "Als Sicherungs Pfad wurde das Joplin profile Verzeichniss (%s) ohne Unterordner ausgewählt, dies ist nicht erlaubt!",
       "BackupSetNotSupportedChars": "Der Name des Backup-Sets enthält nicht zulässige Zeichen ( %s )!",
       "passwordDoubleQuotes": "Das Passwort enthält \" (Doppelte Anführungszeichen), diese sind wegen eines Bugs nicht erlaubt. Der Passwortschutz für die Backups wurde deaktivert!"
     }

src/locales/en_US.json

@@ -13,7 +13,7 @@
       "Backup": "Backup error for %s: %s",
       "fileCopy": "Error on file/folder copy in %s: %s",
       "deleteFile": "Error on file/folder delete in %s: %s",
-      "backupPathJoplinDir": "The backup path is the Joplin profile directory (%s) without subfolders, this is not allowed!",
+      "backupPathContainsImportantDir": "The backup path is or contains an important directory (%s) that could be overwritten by a backup. Without enabling the subfolder setting, this is not allowed!",
       "BackupSetNotSupportedChars": "Backup set name does contain not allowed characters ( %s )!",
       "passwordDoubleQuotes": "Password contains \" (double quotes), these are not allowed because of a bug. Password protection for the backup is deactivated!"
     }