From c0c5a0ebdfdf24089c973b1a1de74dd3bd19da13 Mon Sep 17 00:00:00 2001 From: Narciso Date: Wed, 7 Oct 2026 09:33:34 -0400 Subject: [PATCH] feat(backups): tighten custom name validation and rotate by server modtime --- apps/dokploy/__test__/utils/backups.test.ts | 242 +++++------------- .../database/backups/handle-backup.tsx | 7 +- packages/server/src/db/schema/backups.ts | 4 +- packages/server/src/utils/backups/index.ts | 4 +- packages/server/src/utils/backups/utils.ts | 23 +- packages/server/src/utils/restore/utils.ts | 2 +- 6 files changed, 81 insertions(+), 201 deletions(-) diff --git a/apps/dokploy/__test__/utils/backups.test.ts b/apps/dokploy/__test__/utils/backups.test.ts index 84e004d95..4fd789ac3 100644 --- a/apps/dokploy/__test__/utils/backups.test.ts +++ b/apps/dokploy/__test__/utils/backups.test.ts @@ -1,13 +1,13 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import type { Destination } from "@dokploy/server/services/destination"; +import { apiCreateBackup } from "@dokploy/server/db/schema"; import { keepLatestNBackups } from "@dokploy/server/utils/backups"; import { getBackupFileName, normalizeS3Path, - sanitizeBackupCustomName, } from "@dokploy/server/utils/backups/utils"; +import { getRestoreCommand } from "@dokploy/server/utils/restore/utils"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; const { findDestinationByIdMock } = vi.hoisted(() => ({ @@ -77,43 +77,31 @@ describe("normalizeS3Path", () => { }); }); -describe("sanitizeBackupCustomName", () => { - test("should return an empty string for empty, null or undefined input", () => { - expect(sanitizeBackupCustomName("")).toBe(""); - expect(sanitizeBackupCustomName(null)).toBe(""); - expect(sanitizeBackupCustomName(undefined)).toBe(""); +describe("customName", () => { + const base = { + destinationId: "dest-1", + prefix: "/", + database: "db", + schedule: "0 0 * * *", + backupType: "database" as const, + databaseType: "postgres" as const, + }; + + test("rejects a name that starts with a hyphen", () => { + expect( + apiCreateBackup.safeParse({ ...base, customName: "-rf" }).success, + ).toBe(false); }); - test("should trim whitespace", () => { - expect(sanitizeBackupCustomName(" my-backup ")).toBe("my-backup"); - }); - - test("should preserve already-safe characters", () => { - expect(sanitizeBackupCustomName("my-backup_v1.2")).toBe("my-backup_v1.2"); - }); - - test("should replace unsafe characters with a hyphen", () => { - expect(sanitizeBackupCustomName("my backup")).toBe("my-backup"); - expect(sanitizeBackupCustomName("my/backup")).toBe("my-backup"); - expect(sanitizeBackupCustomName("my@backup!")).toBe("my-backup"); - expect(sanitizeBackupCustomName("café ☕")).toBe("caf"); - }); - - test("should collapse consecutive unsafe characters into a single hyphen", () => { - expect(sanitizeBackupCustomName("my backup")).toBe("my-backup"); - expect(sanitizeBackupCustomName("my///backup")).toBe("my-backup"); - }); - - test("should strip leading and trailing hyphens produced by sanitization", () => { - expect(sanitizeBackupCustomName("/my-backup/")).toBe("my-backup"); - expect(sanitizeBackupCustomName("!!!")).toBe(""); - expect(sanitizeBackupCustomName(" ")).toBe(""); + test("accepts dots in the name", () => { + expect( + apiCreateBackup.safeParse({ ...base, customName: "daily.db" }).success, + ).toBe(true); }); }); describe("getBackupFileName", () => { beforeEach(() => { - // Freeze time so the generated timestamp segment is deterministic. vi.useFakeTimers(); vi.setSystemTime(new Date(Date.UTC(2026, 7, 4, 3, 27, 47, 369))); }); @@ -122,193 +110,103 @@ describe("getBackupFileName", () => { vi.useRealTimers(); }); - const expectedTimestamp = "2026-08-04T03-27-47-369Z"; - - test("should return only the timestamp when there is no custom name or fixed prefix", () => { + test("timestamp only", () => { expect(getBackupFileName(undefined, "sql.gz")).toBe( - `${expectedTimestamp}.sql.gz`, + "2026-08-04T03-27-47-369Z.sql.gz", ); - expect(getBackupFileName(null, "sql.gz")).toBe( - `${expectedTimestamp}.sql.gz`, - ); - expect(getBackupFileName("", "sql.gz")).toBe(`${expectedTimestamp}.sql.gz`); }); - test("should prepend the sanitized custom name to the timestamp", () => { + test("custom name before the timestamp", () => { expect(getBackupFileName("my-backup", "sql.gz")).toBe( - `my-backup-${expectedTimestamp}.sql.gz`, - ); - expect(getBackupFileName("My Backup!", "bson.gz")).toBe( - `My-Backup-${expectedTimestamp}.bson.gz`, + "my-backup-2026-08-04T03-27-47-369Z.sql.gz", ); }); - test("should fall back to the timestamp-only name when the custom name sanitizes to empty", () => { - expect(getBackupFileName("!!!", "sql.gz")).toBe( - `${expectedTimestamp}.sql.gz`, - ); - }); - - test("should use the fixed prefix alone when there is no custom name (web-server default)", () => { - expect(getBackupFileName(undefined, "zip", "webserver-backup")).toBe( - `webserver-backup-${expectedTimestamp}.zip`, - ); - expect(getBackupFileName("", "zip", "webserver-backup")).toBe( - `webserver-backup-${expectedTimestamp}.zip`, - ); - }); - - test("should combine the fixed prefix and the custom name when both are present", () => { + test("fixed prefix, custom name and timestamp", () => { expect(getBackupFileName("dokploy-local", "zip", "webserver-backup")).toBe( - `webserver-backup-dokploy-local-${expectedTimestamp}.zip`, + "webserver-backup-dokploy-local-2026-08-04T03-27-47-369Z.zip", ); }); }); +describe("mongo restore", () => { + test("strips only the trailing .gz", () => { + const cmd = getRestoreCommand({ + appName: "app", + type: "mongo", + restoreType: "database", + credentials: { database: "db" }, + rcloneCommand: "rclone copy src", + backupFile: "app/db.gz-2026-08-04T03-27-47-369Z.bson.gz", + }); + expect(cmd).toContain("< db.gz-2026-08-04T03-27-47-369Z.bson &&"); + }); +}); + describe("keepLatestNBackups", () => { - // Drives the real, unmodified keepLatestNBackups: no shell fragment is - // re-typed here, so this can't silently drift from what index.ts runs. - // The only two externals stubbed are the DB lookup (findDestinationById) - // and the `rclone` binary itself, via a fake executable put on PATH; the - // rest of the pipeline (execAsync, sort/tail/cut/xargs) runs for real. let stubDir: string; - let lsfFixtureFile: string; let deletedFile: string; let originalPath: string | undefined; - let consoleErrorSpy: ReturnType; - - const fakeDestination: Destination = { - destinationId: "dest-1", - name: "test-destination", - provider: null, - accessKey: "AK", - secretAccessKey: "SK", - bucket: "test-bucket", - region: "us-east-1", - endpoint: "https://s3.example.com", - additionalFlags: null, - organizationId: "org-1", - serverId: null, - createdAt: new Date().toISOString(), - } as unknown as Destination; - - const runKeepLatestNBackups = async ( - keepLatestCount: number, - lsfLines: string[], - ) => { - writeFileSync(lsfFixtureFile, `${lsfLines.join("\n")}\n`); - findDestinationByIdMock.mockResolvedValue(fakeDestination); - - await keepLatestNBackups({ - backupId: "backup-1", - destinationId: "dest-1", - prefix: "/", - appName: "test-app", - databaseType: "postgres", - keepLatestCount, - // biome-ignore lint/suspicious/noExplicitAny: minimal fixture, not the full drizzle relation shape - } as any); - - expect(consoleErrorSpy).not.toHaveBeenCalled(); - - try { - return readFileSync(deletedFile, "utf-8") - .trim() - .split("\n") - .filter(Boolean); - } catch { - return []; - } - }; beforeEach(() => { stubDir = mkdtempSync(join(tmpdir(), "rclone-stub-")); - lsfFixtureFile = join(stubDir, "lsf-fixture.txt"); deletedFile = join(stubDir, "deleted.txt"); - - // A fake `rclone`, close enough to the real CLI to catch a regression - // in how index.ts calls it: - // - "lsf" only emits the ";" fixture when invoked with - // `--format tp` (the real flag rclone needs to produce that shape) — - // without it, it emits bare paths, just like the real binary would. - // - "delete" records the exact path it was asked to remove instead of - // touching real S3. writeFileSync( join(stubDir, "rclone"), [ "#!/bin/bash", 'if [ "$1" = "lsf" ]; then', - ' if printf "%s\\n" "$@" | grep -qx "tp"; then', - ` cat "${lsfFixtureFile}"`, - " else", - ` cut -d';' -f2- "${lsfFixtureFile}"`, - " fi", + ' printf "%s\\n" "$@" | grep -qx -- "--use-server-modtime" || exit 1', + ' printf "%s\\n" "$@" | grep -qx "tp" || exit 1', + ' echo "2026-08-04 03:27:00;2026-08-04T03-27-00-000Z.sql.gz"', + ' echo "2026-08-04 03:28:00;dokploy-local-2026-08-04T03-28-00-000Z.sql.gz"', + ' echo "2026-08-04 03:29:00;2026-08-04T03-29-00-000Z.sql.gz"', + ' echo "2026-08-04 03:30:00;dokploy-local-2026-08-04T03-30-00-000Z.sql.gz"', 'elif [ "$1" = "delete" ]; then', ` echo "\${@: -1}" >> "${deletedFile}"`, "fi", ].join("\n"), { mode: 0o755 }, ); - originalPath = process.env.PATH; process.env.PATH = `${stubDir}:${originalPath ?? ""}`; - findDestinationByIdMock.mockReset(); - consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + findDestinationByIdMock.mockResolvedValue({ + bucket: "test-bucket", + accessKey: "AK", + secretAccessKey: "SK", + region: "us-east-1", + endpoint: "https://s3.example.com", + }); }); afterEach(() => { process.env.PATH = originalPath; - consoleErrorSpy.mockRestore(); rmSync(stubDir, { recursive: true, force: true }); }); - // Deliberately mixes a plain timestamp filename with a customName-prefixed - // one so filename-lexicographic order ("2..." < "dokploy-local-...") - // disagrees with chronological order — the exact scenario a customName - // introduces for a pre-existing schedule once it's edited to add/remove/ - // change the custom name. - const lsfLines = [ - "2026-08-04 03:27:00;2026-08-04T03-27-00-000Z.sql.gz", - "2026-08-04 03:28:00;dokploy-local-2026-08-04T03-28-00-000Z.sql.gz", - "2026-08-04 03:29:00;2026-08-04T03-29-00-000Z.sql.gz", - "2026-08-04 03:30:00;dokploy-local-2026-08-04T03-30-00-000Z.sql.gz", - ]; - - test("deletes the chronologically oldest backups, not the lexicographically last ones", async () => { - const deleted = await runKeepLatestNBackups(2, lsfLines); - const deletedFilenames = deleted.map((path) => path.split("/").at(-1)); - - // A naive filename-only sort would have kept the two files whose names - // sort last ("dokploy-local-...-03-30" and "dokploy-local-...-03-28") - // and deleted "...03-27" and "...03-29" instead — deleting the - // second-*newest* backup. The real, modtime-based pipeline must - // instead delete exactly the two oldest by time, with a well-formed - // path (no stray modtime/separator leaking through). - expect(deletedFilenames.sort()).toEqual( - [ - "2026-08-04T03-27-00-000Z.sql.gz", - "dokploy-local-2026-08-04T03-28-00-000Z.sql.gz", - ].sort(), - ); - }); - - test("deletes nothing when keepLatestCount covers the whole list", async () => { - const deleted = await runKeepLatestNBackups(4, lsfLines); - expect(deleted).toEqual([]); - }); - - test("does nothing (and never calls rclone) when keepLatestCount is 0", async () => { + test("deletes the oldest by modtime, not by name", async () => { + const consoleErrorSpy = vi + .spyOn(console, "error") + .mockImplementation(() => {}); await keepLatestNBackups({ backupId: "backup-1", destinationId: "dest-1", prefix: "/", appName: "test-app", databaseType: "postgres", - keepLatestCount: 0, - // biome-ignore lint/suspicious/noExplicitAny: minimal fixture, not the full drizzle relation shape - } as any); + keepLatestCount: 2, + } as never); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + consoleErrorSpy.mockRestore(); - expect(findDestinationByIdMock).not.toHaveBeenCalled(); - expect(() => readFileSync(deletedFile, "utf-8")).toThrow(); + const deleted = readFileSync(deletedFile, "utf-8") + .trim() + .split("\n") + .map((path) => path.split("/").at(-1)) + .sort(); + expect(deleted).toEqual([ + "2026-08-04T03-27-00-000Z.sql.gz", + "dokploy-local-2026-08-04T03-28-00-000Z.sql.gz", + ]); }); }); diff --git a/apps/dokploy/components/dashboard/database/backups/handle-backup.tsx b/apps/dokploy/components/dashboard/database/backups/handle-backup.tsx index 65f1bb241..c1fe7bf84 100644 --- a/apps/dokploy/components/dashboard/database/backups/handle-backup.tsx +++ b/apps/dokploy/components/dashboard/database/backups/handle-backup.tsx @@ -83,8 +83,8 @@ const Schema = z .trim() .max(100, "Custom name must be 100 characters or less") .regex( - /^[a-zA-Z0-9._-]*$/, - "Only letters, numbers, dots, hyphens and underscores are allowed", + /^([a-zA-Z0-9][a-zA-Z0-9._-]*)?$/, + "Must start with a letter or number; only letters, numbers, dots, hyphens and underscores are allowed", ) .optional(), enabled: z.boolean(), @@ -652,7 +652,8 @@ export const HandleBackup = ({ timestamp, e.g. "my-backup-2026-08-03T20-58-22-123Z.sql.gz". Only letters, numbers, dots, hyphens and underscores are - allowed. + allowed. "Keep the latest" counts every backup in the + same destination folder, regardless of its custom name. diff --git a/packages/server/src/db/schema/backups.ts b/packages/server/src/db/schema/backups.ts index d32a0f783..bbfe774d5 100644 --- a/packages/server/src/db/schema/backups.ts +++ b/packages/server/src/db/schema/backups.ts @@ -145,8 +145,8 @@ const customNameSchema = z .trim() .max(100, "Custom name must be 100 characters or less") .regex( - /^[a-zA-Z0-9._-]*$/, - "Only letters, numbers, dots, hyphens and underscores are allowed", + /^([a-zA-Z0-9][a-zA-Z0-9._-]*)?$/, + "Must start with a letter or number; only letters, numbers, dots, hyphens and underscores are allowed", ) .optional(); diff --git a/packages/server/src/utils/backups/index.ts b/packages/server/src/utils/backups/index.ts index 8bb737890..f146ccd98 100644 --- a/packages/server/src/utils/backups/index.ts +++ b/packages/server/src/utils/backups/index.ts @@ -139,8 +139,8 @@ export const keepLatestNBackups = async ( const backupFilesPath = `:s3:${destination.bucket}/${appName}/${normalizeS3Path(backup.prefix)}`; // --include "*.bson.gz" or "*.sql.gz" or "*.zip" ensures nothing else other than the dokploy backup files are touched by rclone - // --format "tp" prefixes each line with the real modification time so sorting reflects actual backup age instead of filename order - const rcloneList = `rclone lsf ${rcloneFlags.join(" ")} --include "*${backup.databaseType === "web-server" ? ".zip" : ".{sql.gz,bson.gz}"}" --format "tp" ${backupFilesPath}`; + // sort by server mtime: custom names break lexical order + const rcloneList = `rclone lsf ${rcloneFlags.join(" ")} --include "*${backup.databaseType === "web-server" ? ".zip" : ".{sql.gz,bson.gz}"}" --format "tp" --use-server-modtime ${backupFilesPath}`; // when we pipe the above command with this one, we only get the list of files we want to delete const sortAndPickUnwantedBackups = `sort -r | tail -n +$((${backup.keepLatestCount}+1)) | cut -d';' -f2- | xargs -I{}`; // this command deletes the files diff --git a/packages/server/src/utils/backups/utils.ts b/packages/server/src/utils/backups/utils.ts index 5934d2d2a..16d87fe4a 100644 --- a/packages/server/src/utils/backups/utils.ts +++ b/packages/server/src/utils/backups/utils.ts @@ -61,31 +61,12 @@ export const removeScheduleBackup = (backupId: string) => { export const getBackupTimestamp = () => new Date().toISOString().replace(/[:.]/g, "-"); -const UNSAFE_CUSTOM_NAME_CHARS = /[^a-zA-Z0-9._-]+/g; - -export const sanitizeBackupCustomName = ( - customName?: string | null, -): string => { - if (!customName) return ""; - return customName - .trim() - .replace(UNSAFE_CUSTOM_NAME_CHARS, "-") - .replace(/^-+|-+$/g, ""); -}; - export const getBackupFileName = ( customName: string | null | undefined, extension: string, fixedPrefix?: string, -): string => { - const timestamp = getBackupTimestamp(); - const parts = [fixedPrefix, sanitizeBackupCustomName(customName)].filter( - Boolean, - ); - return parts.length - ? `${parts.join("-")}-${timestamp}.${extension}` - : `${timestamp}.${extension}`; -}; +) => + `${[fixedPrefix, customName, getBackupTimestamp()].filter(Boolean).join("-")}.${extension}`; export const normalizeS3Path = (prefix: string) => { // Trim whitespace and remove leading/trailing slashes diff --git a/packages/server/src/utils/restore/utils.ts b/packages/server/src/utils/restore/utils.ts index c232b6956..1c9b6b989 100644 --- a/packages/server/src/utils/restore/utils.ts +++ b/packages/server/src/utils/restore/utils.ts @@ -90,7 +90,7 @@ const getMongoSpecificCommand = ( ): string => { const tempDir = "/tmp/dokploy-restore"; const fileName = backupFile.split("/").pop() || "backup.sql.gz"; - const decompressedName = fileName.replace(".gz", ""); + const decompressedName = fileName.replace(/\.gz$/, ""); return ` rm -rf ${tempDir} && \ mkdir -p ${tempDir} && \