From 259f551fc3b7dca87c1b7ee90b437fb84f71e2e6 Mon Sep 17 00:00:00 2001 From: Furox88 Date: Fri, 11 Sep 2026 16:28:24 +0300 Subject: [PATCH] fix: harden volume backup/restore shell quoting Validate docker volume names and pass backup filenames into tar via positional args so user-controlled values cannot break out of bash -c. Co-authored-by: Cursor --- .../utils/issue-416-path-safety.test.ts | 27 +++++++++++++++++++ .../server/src/utils/volume-backups/backup.ts | 17 ++++++++---- .../src/utils/volume-backups/restore.ts | 25 +++++++++++------ 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/apps/dokploy/__test__/utils/issue-416-path-safety.test.ts b/apps/dokploy/__test__/utils/issue-416-path-safety.test.ts index 705756cdc..a8e53fb6c 100644 --- a/apps/dokploy/__test__/utils/issue-416-path-safety.test.ts +++ b/apps/dokploy/__test__/utils/issue-416-path-safety.test.ts @@ -5,6 +5,10 @@ import { assertSafeRclonePath, getRclonePathAndFlags, } from "@dokploy/server/utils/backups/utils"; +import { + normalizeDockerVolumeName, + normalizeVolumeBackupFilePath, +} from "@dokploy/server/utils/volume-backups/restore"; import { describe, expect, test } from "vitest"; const destination = (overrides: Record = {}) => @@ -108,3 +112,26 @@ describe("issue #416 credential redaction", () => { expect(redacted).toContain('--sftp-key-file-pass="[REDACTED]"'); }); }); + + +describe("issue #416 volume name and backup path shell safety", () => { + test("accepts valid docker volume names", () => { + expect(normalizeDockerVolumeName("app_data")).toBe("app_data"); + expect(normalizeDockerVolumeName("App.Data-1")).toBe("App.Data-1"); + }); + + test.each(["", "../evil", "vol;rm -rf /", "vol$(id)", "-sneaky"])( + "rejects unsafe docker volume name %s", + (value) => { + expect(() => normalizeDockerVolumeName(value)).toThrow( + "Invalid docker volume name", + ); + }, + ); + + test("normalizes generated backup file names", () => { + expect( + normalizeVolumeBackupFilePath("app_data-2026-01-01T00-00-00.tar"), + ).toBe("app_data-2026-01-01T00-00-00.tar"); + }); +}); diff --git a/packages/server/src/utils/volume-backups/backup.ts b/packages/server/src/utils/volume-backups/backup.ts index eb1b6c4ce..73c17d2a1 100644 --- a/packages/server/src/utils/volume-backups/backup.ts +++ b/packages/server/src/utils/volume-backups/backup.ts @@ -9,6 +9,10 @@ import { getRclonePathAndFlags, normalizeS3Path, } from "../backups/utils"; +import { + normalizeDockerVolumeName, + normalizeVolumeBackupFilePath, +} from "./restore"; interface RestartSafeBackupCommandOptions { stopCommand: string; @@ -75,7 +79,10 @@ export const backupVolume = async ( volumeBackup.application?.serverId || volumeBackup.compose?.serverId; const { VOLUME_BACKUPS_PATH, VOLUME_BACKUP_LOCK_PATH } = paths(!!serverId); const appName = getVolumeServiceAppName(volumeBackup); - const backupFileName = `${volumeName}-${getBackupTimestamp()}.tar`; + const safeVolumeName = normalizeDockerVolumeName(volumeName); + const backupFileName = normalizeVolumeBackupFilePath( + `${safeVolumeName}-${getBackupTimestamp()}.tar`, + ); const destinationPath = `${appName}/${normalizeS3Path(prefix || "")}${backupFileName}`; const { flags: rcloneFlags, path: rcloneDestination } = await getRclonePathAndFlags(destination, destinationPath); @@ -86,16 +93,16 @@ export const backupVolume = async ( const backupCommand = ` set -e - echo "Volume name: ${volumeName}" - echo "Backup file name: ${backupFileName}" + echo "Volume name:" ${quote([safeVolumeName])} + echo "Backup file name:" ${quote([backupFileName])} echo "Turning off volume backup: ${turnOff ? "Yes" : "No"}" echo "Starting volume backup" echo "Dir: ${volumeBackupPath}" docker run --rm \ - -v ${volumeName}:/volume_data \ + -v ${quote([safeVolumeName])}:/volume_data \ -v ${quote([volumeBackupPath])}:/backup \ ubuntu \ - bash -c "cd /volume_data && tar cvf /backup/${backupFileName} ." + bash -c 'cd /volume_data && tar cvf "/backup/$1" .' -- ${quote([backupFileName])} echo "Volume backup done ✅" `; diff --git a/packages/server/src/utils/volume-backups/restore.ts b/packages/server/src/utils/volume-backups/restore.ts index 4872876e3..ec8cc6456 100644 --- a/packages/server/src/utils/volume-backups/restore.ts +++ b/packages/server/src/utils/volume-backups/restore.ts @@ -10,6 +10,14 @@ import { const UNSAFE_BACKUP_PATH_CHARS = /[\0\r\n;&|`$<>]/; +export const normalizeDockerVolumeName = (value: string) => { + const normalized = value.trim(); + if (!normalized || !/^[A-Za-z0-9][A-Za-z0-9_.-]*$/.test(normalized)) { + throw new Error("Invalid docker volume name"); + } + return normalized; +}; + export const normalizeVolumeBackupFilePath = (value: string) => { const normalized = value.trim().replace(/\\/g, "/"); if ( @@ -39,7 +47,8 @@ export const restoreVolume = async ( ) => { const destination = await findDestinationById(destinationId); const { VOLUME_BACKUPS_PATH } = paths(!!serverId); - const volumeBackupPath = path.join(VOLUME_BACKUPS_PATH, volumeName); + const safeVolumeName = normalizeDockerVolumeName(volumeName); + const volumeBackupPath = path.join(VOLUME_BACKUPS_PATH, safeVolumeName); const safeBackupFileName = normalizeVolumeBackupFilePath(backupFileName); const { flags: rcloneFlags, path: backupPath } = await getRclonePathAndFlags( destination, @@ -57,7 +66,7 @@ export const restoreVolume = async ( // Base restore command that creates the volume and restores data const baseRestoreCommand = ` set -e - echo "Volume name: ${volumeName}" + echo "Volume name: ${safeVolumeName}" echo "Backup file name:" ${quote([safeBackupFileName])} echo "Volume backup path: ${volumeBackupPath}" echo "Downloading backup from destination..." @@ -66,7 +75,7 @@ export const restoreVolume = async ( echo "Download completed ✅" echo "Creating new volume and restoring data..." docker run --rm \ - -v ${volumeName}:/volume_data \ + -v ${quote([safeVolumeName])}:/volume_data \ -v ${quote([volumeBackupPath])}:/backup \ ubuntu \ bash -c 'cd /volume_data && tar xvf "/backup/$1" .' -- ${quote([safeBackupFileName])} @@ -76,7 +85,7 @@ export const restoreVolume = async ( // Function to check if volume exists and get containers using it const checkVolumeCommand = ` # Check if volume exists - VOLUME_EXISTS=$(docker volume ls -q --filter name="^${volumeName}$" | wc -l) + VOLUME_EXISTS=$(docker volume ls -q --filter name="^${safeVolumeName}$" | wc -l) echo "Volume exists: $VOLUME_EXISTS" if [ "$VOLUME_EXISTS" = "0" ]; then @@ -86,18 +95,18 @@ export const restoreVolume = async ( echo "Volume exists, checking for containers using it (including stopped ones)..." # Get ALL containers (running and stopped) using this volume - much simpler with native filter! - CONTAINERS_USING_VOLUME=$(docker ps -a --filter "volume=${volumeName}" --format "{{.ID}}|{{.Names}}|{{.State}}|{{.Labels}}") + CONTAINERS_USING_VOLUME=$(docker ps -a --filter "volume=${safeVolumeName}" --format "{{.ID}}|{{.Names}}|{{.State}}|{{.Labels}}") if [ -z "$CONTAINERS_USING_VOLUME" ]; then echo "Volume exists but no containers are using it" echo "Removing existing volume and proceeding with restore" - docker volume rm ${volumeName} --force + docker volume rm ${quote([safeVolumeName])} --force ${baseRestoreCommand} else echo "" echo "⚠️ WARNING: Cannot restore volume as it is currently in use!" echo "" - echo "📋 The following containers are using volume '${volumeName}':" + echo "📋 The following containers are using volume '${safeVolumeName}':" echo "" echo "$CONTAINERS_USING_VOLUME" | while IFS='|' read container_id container_name container_state labels; do @@ -120,7 +129,7 @@ export const restoreVolume = async ( echo "" echo "🔧 To restore this volume, please:" echo " 1. Stop all containers/services using this volume" - echo " 2. Remove the existing volume: docker volume rm ${volumeName}" + echo " 2. Remove the existing volume: docker volume rm ${safeVolumeName}" echo " 3. Run the restore operation again" echo "" echo "❌ Volume restore aborted - volume is in use"