From f046a4dda62b59b41f9f5b78938747400ea4f969 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Thu, 11 Dec 2025 14:50:06 +0000 Subject: [PATCH] Fixes! --- app/Jobs/DatabaseBackupJob.php | 30 ++++++--- app/Jobs/PgBackrestRestoreJob.php | 64 +++++++++++--------- app/Livewire/Project/Database/BackupEdit.php | 4 +- 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index ba53effa7..7a46e1d02 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -636,31 +636,38 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue } $fullImageName = $this->getFullImageName(); + $escapedNetwork = escapeshellarg($network); + $escapedContainerName = escapeshellarg("backup-of-{$this->backup_log_uuid}"); + $escapedImageName = escapeshellarg($fullImageName); $containerExists = instant_remote_process(["docker ps -a -q -f name=backup-of-{$this->backup_log_uuid}"], $this->server, false, false, null, disableMultiplexing: true); if (filled($containerExists)) { - instant_remote_process(["docker rm -f backup-of-{$this->backup_log_uuid}"], $this->server, false, false, null, disableMultiplexing: true); + instant_remote_process(["docker rm -f {$escapedContainerName}"], $this->server, false, false, null, disableMultiplexing: true); } if (isDev()) { if ($this->database->name === 'coolify-db') { $backup_location_from = '/var/lib/docker/volumes/coolify_dev_backups_data/_data/coolify/coolify-db-'.$this->server->ip.$this->backup_file; - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; + $escapedVolumeMount = escapeshellarg($backup_location_from.':'.$this->backup_location.':ro'); + $commands[] = "docker run -d --network {$escapedNetwork} --name {$escapedContainerName} --rm -v {$escapedVolumeMount} {$escapedImageName}"; } else { $backup_location_from = '/var/lib/docker/volumes/coolify_dev_backups_data/_data/databases/'.str($this->team->name)->slug().'-'.$this->team->id.'/'.$this->directory_name.$this->backup_file; - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; + $escapedVolumeMount = escapeshellarg($backup_location_from.':'.$this->backup_location.':ro'); + $commands[] = "docker run -d --network {$escapedNetwork} --name {$escapedContainerName} --rm -v {$escapedVolumeMount} {$escapedImageName}"; } } else { - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}"; + $escapedVolumeMount = escapeshellarg($this->backup_location.':'.$this->backup_location.':ro'); + $commands[] = "docker run -d --network {$escapedNetwork} --name {$escapedContainerName} --rm -v {$escapedVolumeMount} {$escapedImageName}"; } - // Escape S3 credentials to prevent command injection $escapedEndpoint = escapeshellarg($endpoint); $escapedKey = escapeshellarg($key); $escapedSecret = escapeshellarg($secret); + $escapedBucketPath = escapeshellarg("temporary/{$bucket}{$this->backup_dir}/"); + $escapedBackupLocation = escapeshellarg($this->backup_location); - $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc alias set temporary {$escapedEndpoint} {$escapedKey} {$escapedSecret}"; - $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp $this->backup_location temporary/$bucket{$this->backup_dir}/"; + $commands[] = "docker exec {$escapedContainerName} mc alias set temporary {$escapedEndpoint} {$escapedKey} {$escapedSecret}"; + $commands[] = "docker exec {$escapedContainerName} mc cp {$escapedBackupLocation} {$escapedBucketPath}"; instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true); $this->s3_uploaded = true; @@ -669,7 +676,8 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $this->add_to_error_output($e->getMessage()); throw $e; } finally { - $command = "docker rm -f backup-of-{$this->backup_log_uuid}"; + $escapedContainerNameForCleanup = escapeshellarg("backup-of-{$this->backup_log_uuid}"); + $command = "docker rm -f {$escapedContainerNameForCleanup}"; instant_remote_process([$command], $this->server, true, false, null, disableMultiplexing: true); } } @@ -709,9 +717,11 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $s3EnvVars = PgBackrestService::buildS3EnvVars($this->backup); $dockerEnvArgs = PgBackrestService::buildDockerEnvArgs($s3EnvVars); $fixPermsCmd = 'chown -R postgres:postgres /var/lib/pgbackrest /tmp/pgbackrest /var/log/pgbackrest 2>/dev/null || true'; - $escapedBackupCmd = escapeshellarg($backupCmdWithWait); + $escapedInnerCmd = str_replace("'", "'\"'\"'", $backupCmdWithWait); + $fullScript = "{$fixPermsCmd}; su postgres -c '{$escapedInnerCmd}' 2>&1; echo \"EXIT_CODE:\$?\""; + $escapedScript = escapeshellarg($fullScript); $containerName = escapeshellarg($this->container_name); - $backupFullCmd = "docker exec{$dockerEnvArgs} {$containerName} sh -c '{$fixPermsCmd}; su postgres -c {$escapedBackupCmd} 2>&1; echo \"EXIT_CODE:\$?\"'"; + $backupFullCmd = "docker exec{$dockerEnvArgs} {$containerName} sh -c {$escapedScript}"; $rawOutput = instant_remote_process([$backupFullCmd], $this->server, false, false, $this->timeout, disableMultiplexing: true); $rawOutput = trim($rawOutput) ?: ''; diff --git a/app/Jobs/PgBackrestRestoreJob.php b/app/Jobs/PgBackrestRestoreJob.php index 602cb261c..9bb6e0dc3 100644 --- a/app/Jobs/PgBackrestRestoreJob.php +++ b/app/Jobs/PgBackrestRestoreJob.php @@ -195,10 +195,10 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $repoVolume = $this->database->pgbackrestRepoVolume(); if ($repoVolume) { $repoMount = $repoVolume->host_path ?: $repoVolume->name; - $mounts[] = "-v {$repoMount}:/var/lib/pgbackrest"; + $mounts[] = '-v '.escapeshellarg($repoMount.':/var/lib/pgbackrest'); } - $mounts[] = "-v {$configDir}:/etc/pgbackrest:ro"; + $mounts[] = '-v '.escapeshellarg($configDir.':/etc/pgbackrest:ro'); $s3EnvVars = PgBackrestService::buildS3EnvVars($backup); foreach ($s3EnvVars as $key => $value) { @@ -209,13 +209,13 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $sidecarName = 'pgbackrest-info-'.$this->database->uuid.'-'.time(); $cmd = sprintf( - 'docker run --rm --name %s --network %s %s %s %s sh -c \'%s\' 2>&1', - $sidecarName, - $network, + 'docker run --rm --name %s --network %s %s %s %s sh -c %s 2>&1', + escapeshellarg($sidecarName), + escapeshellarg($network), implode(' ', $envPieces), implode(' ', $mounts), - $this->getSidecarImage(), - $this->getInstallAndRunCommand($infoCmd) + escapeshellarg($this->getSidecarImage()), + escapeshellarg($this->getInstallAndRunCommand($infoCmd)) ); $output = instant_remote_process([$cmd], $server, false, false, 120, disableMultiplexing: true); @@ -249,7 +249,8 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $volumeName = $pgdataVolume->host_path ?: $pgdataVolume->name; $backupVolumeName = "{$pgdataVolume->name}_backup_{$timestamp}"; - $checkCmd = "docker run --rm -v {$volumeName}:/data alpine sh -c 'test -n \"$(ls -A /data 2>/dev/null)\" && echo OK || echo EMPTY'"; + $sourceMount = escapeshellarg($volumeName.':/data'); + $checkCmd = 'docker run --rm -v '.$sourceMount." alpine sh -c 'test -n \"\$(ls -A /data 2>/dev/null)\" && echo OK || echo EMPTY'"; $checkResult = instant_remote_process([$checkCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($checkResult) === 'EMPTY') { @@ -258,12 +259,14 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue return null; } - instant_remote_process(["docker volume create {$backupVolumeName}"], $server, false, false, 30, disableMultiplexing: true); + instant_remote_process(['docker volume create '.escapeshellarg($backupVolumeName)], $server, false, false, 30, disableMultiplexing: true); - $copyCmd = "docker run --rm -v {$volumeName}:/source:ro -v {$backupVolumeName}:/backup alpine sh -c 'cp -a /source/. /backup/'"; - instant_remote_process([$copyCmd], $server, true, false, 300, disableMultiplexing: true); + $sourceReadOnlyMount = escapeshellarg($volumeName.':/source:ro'); + $backupMount = escapeshellarg($backupVolumeName.':/backup'); + $copyCmd = 'docker run --rm -v '.$sourceReadOnlyMount.' -v '.$backupMount." alpine sh -c 'cp -a /source/. /backup/'"; + instant_remote_process([$copyCmd], $server, true, false, $this->timeout, disableMultiplexing: true); - $verifyCmd = "docker run --rm -v {$backupVolumeName}:/backup alpine sh -c 'test -n \"$(ls -A /backup 2>/dev/null)\" && echo OK'"; + $verifyCmd = 'docker run --rm -v '.$backupMount." alpine sh -c 'test -n \"\$(ls -A /backup 2>/dev/null)\" && echo OK'"; $result = instant_remote_process([$verifyCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($result) !== 'OK') { @@ -291,11 +294,13 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $this->restore->appendLog('Recovering PGDATA from backup...'); - $clearCmd = "docker run --rm -v {$volumeName}:/data alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true'"; + $dataMount = escapeshellarg($volumeName.':/data'); + $clearCmd = 'docker run --rm -v '.$dataMount." alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true'"; instant_remote_process([$clearCmd], $server, false, false, 60, disableMultiplexing: true); - $copyCmd = "docker run --rm -v {$backupVolumeName}:/source:ro -v {$volumeName}:/data alpine sh -c 'cp -a /source/. /data/'"; - instant_remote_process([$copyCmd], $server, true, false, 300, disableMultiplexing: true); + $sourceMount = escapeshellarg($backupVolumeName.':/source:ro'); + $copyCmd = 'docker run --rm -v '.$sourceMount.' -v '.$dataMount." alpine sh -c 'cp -a /source/. /data/'"; + instant_remote_process([$copyCmd], $server, true, false, $this->timeout, disableMultiplexing: true); $this->restore->appendLog('PGDATA recovered from backup.'); } catch (Throwable $e) { @@ -308,7 +313,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue try { $server = $this->database->destination->server; - instant_remote_process(["docker volume rm {$backupVolumeName} 2>/dev/null || true"], $server, false, false, 60, disableMultiplexing: true); + instant_remote_process(['docker volume rm '.escapeshellarg($backupVolumeName).' 2>/dev/null || true'], $server, false, false, 60, disableMultiplexing: true); $this->restore->appendLog('Temporary backup volume removed.'); } catch (Throwable $e) { @@ -327,8 +332,9 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; - $rmCmd = "docker run --rm -v {$mount}:/data alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true'"; - instant_remote_process([$rmCmd], $server, false, false, 300, disableMultiplexing: true); + $dataMount = escapeshellarg($mount.':/data'); + $rmCmd = 'docker run --rm -v '.$dataMount." alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true'"; + instant_remote_process([$rmCmd], $server, false, false, $this->timeout, disableMultiplexing: true); $this->restore->appendLog('PGDATA directory cleared.'); } @@ -344,8 +350,8 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $server = $this->database->destination->server; $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; - // Verify PG_VERSION exists in restored data - $checkCmd = "docker run --rm -v {$mount}:/data alpine test -f /data/PG_VERSION && echo 'OK' || echo 'FAIL'"; + $dataMount = escapeshellarg($mount.':/data'); + $checkCmd = 'docker run --rm -v '.$dataMount." alpine test -f /data/PG_VERSION && echo 'OK' || echo 'FAIL'"; $result = instant_remote_process([$checkCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($result) !== 'OK') { @@ -375,16 +381,16 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $pgdataMount = $pgdataVolume->host_path ?: $pgdataVolume->name; $mounts = []; - $mounts[] = "-v {$pgdataMount}:".PgBackrestService::PGDATA_PATH; + $mounts[] = '-v '.escapeshellarg($pgdataMount.':'.PgBackrestService::PGDATA_PATH); if ($repoVolume) { $repoMount = $repoVolume->host_path ?: $repoVolume->name; - $mounts[] = "-v {$repoMount}:/var/lib/pgbackrest"; + $mounts[] = '-v '.escapeshellarg($repoMount.':/var/lib/pgbackrest'); } - $mounts[] = "-v {$configDir}:/etc/pgbackrest:ro"; + $mounts[] = '-v '.escapeshellarg($configDir.':/etc/pgbackrest:ro'); - $envPieces = ['-e PGBACKREST_PG1_PATH='.PgBackrestService::PGDATA_PATH]; + $envPieces = ['-e PGBACKREST_PG1_PATH='.escapeshellarg(PgBackrestService::PGDATA_PATH)]; $s3EnvVars = PgBackrestService::buildS3EnvVars($backup); foreach ($s3EnvVars as $key => $value) { @@ -403,13 +409,13 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd); $cmd = sprintf( - 'docker run --rm --name %s --network %s %s %s %s sh -c \'%s\' 2>&1', - $sidecarName, - $network, + 'docker run --rm --name %s --network %s %s %s %s sh -c %s 2>&1', + escapeshellarg($sidecarName), + escapeshellarg($network), implode(' ', $envPieces), implode(' ', $mounts), - $this->getSidecarImage(), - $fullRestoreScript + escapeshellarg($this->getSidecarImage()), + escapeshellarg($fullRestoreScript) ); $output = instant_remote_process([$cmd], $server, true, false, $this->timeout, disableMultiplexing: true); diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index 6cede88dc..8ffdf7f93 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -168,7 +168,9 @@ class BackupEdit extends Component if ($this->engine === 'pgbackrest') { DB::transaction(function () { $this->backup->save_s3 = $this->saveS3; - $this->backup->disable_local_backup = $this->saveS3 && $this->disableLocalBackup; + $computedDisableLocal = $this->saveS3 && $this->disableLocalBackup; + $this->disableLocalBackup = $computedDisableLocal; + $this->backup->disable_local_backup = $computedDisableLocal; $this->backup->pgbackrest_backup_type = $this->pgbackrestBackupType; $this->backup->pgbackrest_compress_type = $this->pgbackrestCompressType;