diff --git a/__test__/backup.test.ts b/__test__/backup.test.ts index 5695db0..788c578 100644 --- a/__test__/backup.test.ts +++ b/__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 () { diff --git a/__test__/help.test.ts b/__test__/help.test.ts index 8d364a0..69a9ce2 100644 --- a/__test__/help.test.ts +++ b/__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 + ); + } + ); }); diff --git a/src/Backup.ts b/src/Backup.ts index 36416a9..aeebbcc 100644 --- a/src/Backup.ts +++ b/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; + } } } diff --git a/src/helper.ts b/src/helper.ts index 3726fc2..45eba0c 100644 --- a/src/helper.ts +++ b/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); + } } diff --git a/src/locales/de_DE.json b/src/locales/de_DE.json index 1c41b6d..340eae5 100644 --- a/src/locales/de_DE.json +++ b/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!" } diff --git a/src/locales/en_US.json b/src/locales/en_US.json index 364533b..e5ed212 100644 --- a/src/locales/en_US.json +++ b/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!" }