mirror of
https://github.com/Dokploy/dokploy.git
synced 2026-09-12 19:51:00 +05:00
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 <cursoragent@cursor.com>
This commit is contained in:
parent
f95ef6b94d
commit
259f551fc3
@ -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<string, unknown> = {}) =>
|
||||
@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
@ -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 ✅"
|
||||
`;
|
||||
|
||||
|
||||
@ -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"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user