fix(backups): correctly resolve paths when folder name matches volume name (#576)

When a selected subfolder had the exact same name as its parent volume
mount, the `path.relative` check falsely identified the subfolder path
as an absolute path already inside the volume. This caused the entire
volume root to be backed up instead of the intended subfolder.

Closes #250

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved handling of backup patterns when folder names match volume names.
  * Enhanced error handling for snapshot deletion operations.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

app/server/modules/backups/__tests__/backups.patterns.test.ts

@@ -53,20 +53,23 @@ describe("executeBackup - include / exclude patterns", () => {
 		);
 	});
 
-	test("should not join with volume path if pattern already starts with it", async () => {
+	test("should handle the case where a subfolder has the exact same name as the volume name", async () => {
 		// arrange
-		const volume = await createTestVolume();
+		const volumeName = "SyncFolder";
+		const volume = await createTestVolume({
+			name: volumeName,
+			type: "directory",
+			config: { backend: "directory", path: `/${volumeName}` },
+		});
 		const volumePath = getVolumePath(volume);
 		const repository = await createTestRepository();
 
-		const alreadyJoinedInclude = path.join(volumePath, "already/joined");
-		const alreadyJoinedExclude = path.join(volumePath, "already/excluded");
+		const selectedPath = `/${volumeName}`; // Selection of the folder inside the volume
 
 		const schedule = await createTestBackupSchedule({
 			volumeId: volume.id,
 			repositoryId: repository.id,
-			includePatterns: [alreadyJoinedInclude],
-			excludePatterns: [alreadyJoinedExclude],
+			includePatterns: [selectedPath],
 		});
 
 		// act
@@ -77,8 +80,8 @@ describe("executeBackup - include / exclude patterns", () => {
 			expect.anything(),
 			volumePath,
 			expect.objectContaining({
-				include: [alreadyJoinedInclude],
-				exclude: [alreadyJoinedExclude],
+				// Should produce /SyncFolder/SyncFolder and not just /SyncFolder
+				include: [path.join(volumePath, volumeName)],
 			}),
 		);
 	});
@@ -89,14 +92,13 @@ describe("executeBackup - include / exclude patterns", () => {
 		const volumePath = getVolumePath(volume);
 		const repository = await createTestRepository();
 
-		const alreadyJoinedInclude = path.join(volumePath, "already/joined");
 		const relativeInclude = "relative/include";
 		const anchoredInclude = "/anchored/include";
 
 		const schedule = await createTestBackupSchedule({
 			volumeId: volume.id,
 			repositoryId: repository.id,
-			includePatterns: [alreadyJoinedInclude, relativeInclude, anchoredInclude],
+			includePatterns: [relativeInclude, anchoredInclude],
 		});
 
 		// act
@@ -107,7 +109,7 @@ describe("executeBackup - include / exclude patterns", () => {
 			expect.anything(),
 			volumePath,
 			expect.objectContaining({
-				include: [alreadyJoinedInclude, relativeInclude, path.join(volumePath, "anchored/include")],
+				include: [relativeInclude, path.join(volumePath, "anchored/include")],
 			}),
 		);
 	});

app/server/modules/backups/backup.helpers.ts

@@ -23,10 +23,7 @@ export const processPattern = (pattern: string, volumePath: string) => {
 	const isNegated = pattern.startsWith("!");
 	const p = isNegated ? pattern.slice(1) : pattern;
 
-	const relative = path.relative(volumePath, p);
-	const isInsideVolume = !relative.startsWith("..") && !path.isAbsolute(relative);
-
-	if (isInsideVolume || !p.startsWith("/")) {
+	if (!p.startsWith("/")) {
 		return pattern;
 	}
 

app/server/modules/repositories/__tests__/repositories.controller.test.ts

@@ -1,4 +1,4 @@
-import { test, describe, expect } from "bun:test";
+import { test, describe, expect, spyOn } from "bun:test";
 import crypto from "node:crypto";
 import { createApp } from "~/server/app";
 import { db } from "~/server/db/db";
@@ -240,4 +240,31 @@ describe("repositories updates", () => {
 
 		expect(res.status).toBe(400);
 	});
+
+	describe("delete snapshot", () => {
+		test("should return 500 when restic deleteSnapshot throws ResticError", async () => {
+			const { token, organizationId } = await createTestSession();
+			const repository = await createRepositoryRecord(organizationId);
+
+			const { restic } = await import("~/server/utils/restic");
+			const { ResticError } = await import("~/server/utils/errors");
+
+			const deleteSnapshotSpy = spyOn(restic, "deleteSnapshot").mockImplementation(async () => {
+				throw new ResticError(1, "Fatal: unexpected HTTP response (403): 403 Forbidden");
+			});
+
+			try {
+				const res = await app.request(`/api/v1/repositories/${repository.shortId}/snapshots/snap123`, {
+					method: "DELETE",
+					headers: getAuthHeaders(token),
+				});
+
+				expect(res.status).toBe(500);
+				const body = await res.json();
+				expect(body.message).toContain("Command failed");
+			} finally {
+				deleteSnapshotSpy.mockRestore();
+			}
+		});
+	});
 });

app/server/modules/repositories/__tests__/repositories.service.test.ts

@@ -12,8 +12,28 @@ import { restic } from "~/server/utils/restic";
 import { createTestSession } from "~/test/helpers/auth";
 import { createTestBackupSchedule } from "~/test/helpers/backup";
 import { cache, cacheKeys } from "~/server/utils/cache";
+import { ResticError } from "~/server/utils/errors";
 import { repositoriesService } from "../repositories.service";
 
+const createTestRepository = async (organizationId: string) => {
+	const id = randomUUID();
+	const shortId = generateShortId();
+	const [repository] = await db
+		.insert(repositoriesTable)
+		.values({
+			id,
+			shortId,
+			name: `Test-${randomUUID()}`,
+			type: "local",
+			config: { backend: "local", path: "/tmp" },
+			compressionMode: "auto",
+			status: "healthy",
+			organizationId,
+		})
+		.returning();
+	return repository;
+};
+
 describe("repositoriesService.createRepository", () => {
 	const initMock = mock(() => Promise.resolve({ success: true, error: null }));
 
@@ -371,3 +391,24 @@ describe("repositoriesService.getRetentionCategories", () => {
 		expect(forgetSpy).toHaveBeenCalledTimes(2);
 	});
 });
+
+describe("repositoriesService.deleteSnapshot", () => {
+	afterEach(() => {
+		mock.restore();
+	});
+
+	test("should throw original error when restic deleteSnapshot fails", async () => {
+		const { organizationId, user } = await createTestSession();
+		const repository = await createTestRepository(organizationId);
+
+		spyOn(restic, "deleteSnapshot").mockImplementation(async () => {
+			throw new ResticError(1, "Fatal: unexpected HTTP response (403): 403 Forbidden");
+		});
+
+		await expect(
+			withContext({ organizationId, userId: user.id }, () =>
+				repositoriesService.deleteSnapshot(repository.shortId, "snap123"),
+			),
+		).rejects.toThrow("Fatal: unexpected HTTP response");
+	});
+});