starred/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2026-09-26 01:10:30 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(backups): improve backup command handling

Browse Source
This commit is contained in:
Andras Bacsai
2026-09-24 11:44:53 +02:00
parent 5561247669
commit 26248ca161
7 changed files with 139 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Jobs/DatabaseBackupJob.php
+57 -40
View File
@@ -21,6 +21,7 @@ use App\Rules\SafeWebhookUrl;
use App\Services\ScheduledJobDeliveryService;
use App\Support\BackupCompression;
use App\Support\ClickhouseBackupCommand;
use App\Support\ValidationPatterns;
use Carbon\Carbon;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldBeEncrypted;
@@ -143,10 +144,14 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
$databaseType = $this->database->databaseType();
$serviceUuid = $this->database->service->uuid;
$serviceName = str($this->database->service->name)->slug();
$this->container_name = "{$this->database->name}-$serviceUuid";
if (! ValidationPatterns::isValidContainerName($this->container_name)) {
throw new \Exception('Invalid database container name.');
}
$this->directory_name = $serviceName.'-'.str($this->database->name)->slug().'-'.$serviceUuid;
$escapedContainerName = escapeshellarg($this->container_name);
if (str($databaseType)->contains('postgres')) {
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env | grep POSTGRES_";
$commands[] = "docker exec {$escapedContainerName} env | grep POSTGRES_";
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
@@ -175,9 +180,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
$this->postgres_password = str($this->postgres_password)->after('POSTGRES_PASSWORD=')->value();
}
} elseif (str($databaseType)->contains('mysql')) {
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env | grep MYSQL_";
$commands[] = "docker exec {$escapedContainerName} env | grep MYSQL_";
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
@@ -198,9 +201,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
throw new \Exception('MYSQL_DATABASE not found');
}
} elseif (str($databaseType)->contains('mariadb')) {
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env";
$commands[] = "docker exec {$escapedContainerName} env";
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
$rootPassword = $envs->filter(function ($env) {
@@ -236,13 +237,11 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
}
} elseif (str($databaseType)->contains('mongo')) {
$databasesToBackup = ['*'];
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
// Try to extract MongoDB credentials from environment variables
try {
$commands = [];
$commands[] = "docker exec $this->container_name env | grep MONGO_INITDB_";
$commands[] = "docker exec {$escapedContainerName} env | grep MONGO_INITDB_";
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
if (filled($envs)) {
@@ -317,7 +316,8 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
// Step 1: Create local backup
try {
if (str($databaseType)->contains('postgres')) {
$this->backup_file = "/pg-dump-$database-".Carbon::now()->timestamp.'.dmp';
$fileDatabaseName = $this->backupFilenamePart($database);
$this->backup_file = "/pg-dump-$fileDatabaseName-".Carbon::now()->timestamp.'.dmp';
if ($this->backup->dump_all) {
$this->backup_file = '/pg-dump-all-'.Carbon::now()->timestamp.'.gz';
}
@@ -342,7 +342,8 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
$databaseName = $database;
}
}
$this->backup_file = "/mongo-dump-$databaseName-".Carbon::now()->timestamp.'.tar.gz';
$fileDatabaseName = $this->backupFilenamePart($databaseName);
$this->backup_file = "/mongo-dump-$fileDatabaseName-".Carbon::now()->timestamp.'.tar.gz';
$this->backup_location = $this->backup_dir.$this->backup_file;
$this->backup_log = ScheduledDatabaseBackupExecution::create([
'uuid' => $this->backup_log_uuid,
@@ -354,7 +355,8 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
BackupCreated::dispatch($this->team->id);
$this->backup_standalone_mongodb($database);
} elseif (str($databaseType)->contains('mysql')) {
$this->backup_file = "/mysql-dump-$database-".Carbon::now()->timestamp.'.dmp';
$fileDatabaseName = $this->backupFilenamePart($database);
$this->backup_file = "/mysql-dump-$fileDatabaseName-".Carbon::now()->timestamp.'.dmp';
if ($this->backup->dump_all) {
$this->backup_file = '/mysql-dump-all-'.Carbon::now()->timestamp.'.gz';
}
@@ -369,7 +371,8 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
BackupCreated::dispatch($this->team->id);
$this->backup_standalone_mysql($database);
} elseif (str($databaseType)->contains('mariadb')) {
$this->backup_file = "/mariadb-dump-$database-".Carbon::now()->timestamp.'.dmp';
$fileDatabaseName = $this->backupFilenamePart($database);
$this->backup_file = "/mariadb-dump-$fileDatabaseName-".Carbon::now()->timestamp.'.dmp';
if ($this->backup->dump_all) {
$this->backup_file = '/mariadb-dump-all-'.Carbon::now()->timestamp.'.gz';
}
@@ -521,6 +524,11 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
}
}
private function backupFilenamePart(string $database): string
{
return preg_replace('/[^A-Za-z0-9_.-]/', '-', $database);
}
private function backup_standalone_mongodb(string $databaseWithCollections): void
{
try {
@@ -540,12 +548,15 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
}
Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]);
$escapedUrl = escapeshellarg($url);
$escapedContainerName = escapeshellarg($this->container_name);
$escapedBackupDir = escapeshellarg($this->backup_dir);
$escapedBackupLocation = escapeshellarg($this->backup_location);
if ($databaseWithCollections === 'all') {
$commands[] = 'mkdir -p '.$this->backup_dir;
$commands[] = 'mkdir -p '.$escapedBackupDir;
if (str($this->database->image)->startsWith('mongo:4')) {
$commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --uri=$escapedUrl --gzip --archive > {$escapedBackupLocation}";
} else {
$commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --gzip --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --authenticationDatabase=admin --uri=$escapedUrl --gzip --archive > {$escapedBackupLocation}";
}
} else {
if (str($databaseWithCollections)->contains(':')) {
@@ -555,7 +566,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
$databaseName = $databaseWithCollections;
$collectionsToExclude = collect();
}
$commands[] = 'mkdir -p '.$this->backup_dir;
$commands[] = 'mkdir -p '.$escapedBackupDir;
// Validate and escape database name to prevent command injection
validateShellSafePath($databaseName, 'database name');
@@ -563,9 +574,9 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
if ($collectionsToExclude->count() === 0) {
if (str($this->database->image)->startsWith('mongo:4')) {
$commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --uri=$escapedUrl --gzip --archive > {$escapedBackupLocation}";
} else {
$commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --archive > {$escapedBackupLocation}";
}
} else {
// Validate and escape each collection name
@@ -577,9 +588,9 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
});
if (str($this->database->image)->startsWith('mongo:4')) {
$commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --uri=$escapedUrl --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > {$escapedBackupLocation}";
} else {
$commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > {$escapedBackupLocation}";
}
}
}
@@ -621,20 +632,22 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
private function backup_standalone_postgresql(string $database): void
{
try {
$commands[] = 'mkdir -p '.$this->backup_dir;
$commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
$backupCommand = 'docker exec';
if ($this->postgres_password) {
$backupCommand .= ' -e PGPASSWORD='.escapeshellarg($this->postgres_password);
}
$escapedUsername = escapeshellarg($this->database->postgres_user);
$escapedContainerName = escapeshellarg($this->container_name);
$escapedBackupLocation = escapeshellarg($this->backup_location);
if ($this->backup->dump_all) {
$backupCommand .= " $this->container_name pg_dumpall --username $escapedUsername";
$backupCommand = $this->buildCompressedDumpCommand($backupCommand).' > '.escapeshellarg($this->backup_location);
$backupCommand .= " {$escapedContainerName} pg_dumpall --username $escapedUsername";
$backupCommand = $this->buildCompressedDumpCommand($backupCommand).' > '.$escapedBackupLocation;
} else {
// Validate and escape database name to prevent command injection
validateShellSafePath($database, 'database name');
$escapedDatabase = escapeshellarg($database);
$backupCommand .= " $this->container_name pg_dump --format=custom --no-acl --no-owner --username $escapedUsername $escapedDatabase > $this->backup_location";
$backupCommand .= " {$escapedContainerName} pg_dump --format=custom --no-acl --no-owner --username $escapedUsername $escapedDatabase > {$escapedBackupLocation}";
}
$commands[] = $backupCommand;
@@ -652,16 +665,18 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
private function backup_standalone_mysql(string $database): void
{
try {
$commands[] = 'mkdir -p '.$this->backup_dir;
$commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
$escapedPassword = escapeshellarg($this->database->mysql_root_password);
$escapedContainerName = escapeshellarg($this->container_name);
$escapedBackupLocation = escapeshellarg($this->backup_location);
if ($this->backup->dump_all) {
$dumpCommand = "docker exec $this->container_name mysqldump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false";
$commands[] = $this->buildCompressedDumpCommand($dumpCommand).' > '.escapeshellarg($this->backup_location);
$dumpCommand = "docker exec {$escapedContainerName} mysqldump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false";
$commands[] = $this->buildCompressedDumpCommand($dumpCommand).' > '.$escapedBackupLocation;
} else {
// Validate and escape database name to prevent command injection
validateShellSafePath($database, 'database name');
$escapedDatabase = escapeshellarg($database);
$commands[] = "docker exec $this->container_name mysqldump -u root -p$escapedPassword $escapedDatabase > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mysqldump -u root -p$escapedPassword $escapedDatabase > {$escapedBackupLocation}";
}
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
@@ -677,16 +692,18 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
private function backup_standalone_mariadb(string $database): void
{
try {
$commands[] = 'mkdir -p '.$this->backup_dir;
$commands[] = 'mkdir -p '.escapeshellarg($this->backup_dir);
$escapedPassword = escapeshellarg($this->database->mariadb_root_password);
$escapedContainerName = escapeshellarg($this->container_name);
$escapedBackupLocation = escapeshellarg($this->backup_location);
if ($this->backup->dump_all) {
$dumpCommand = "docker exec $this->container_name mariadb-dump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false";
$commands[] = $this->buildCompressedDumpCommand($dumpCommand).' > '.escapeshellarg($this->backup_location);
$dumpCommand = "docker exec {$escapedContainerName} mariadb-dump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false";
$commands[] = $this->buildCompressedDumpCommand($dumpCommand).' > '.$escapedBackupLocation;
} else {
// Validate and escape database name to prevent command injection
validateShellSafePath($database, 'database name');
$escapedDatabase = escapeshellarg($database);
$commands[] = "docker exec $this->container_name mariadb-dump -u root -p$escapedPassword $escapedDatabase > $this->backup_location";
$commands[] = "docker exec {$escapedContainerName} mariadb-dump -u root -p$escapedPassword $escapedDatabase > {$escapedBackupLocation}";
}
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
@@ -745,7 +762,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
private function calculate_size()
{
return instant_remote_process(["du -b $this->backup_location | cut -f1"], $this->server, false, false, null, disableMultiplexing: true);
return instant_remote_process(['du -b '.escapeshellarg($this->backup_location).' | cut -f1'], $this->server, false, false, null, disableMultiplexing: true);
}
private function upload_to_s3(): void
@@ -785,14 +802,14 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
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 {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
} 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 {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
}
} else {
$commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}";
$backup_location_from = $this->backup_location;
}
$mount = escapeshellarg($backup_location_from.':'.$this->backup_location.':ro');
$commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v {$mount} {$fullImageName}";
// Escape S3 credentials to prevent command injection
$escapedEndpoint = escapeshellarg($endpoint);
app/Models/ServiceApplication.php
+1 -1
View File
@@ -88,7 +88,7 @@ class ServiceApplication extends BaseModel
public function restart()
{
$container_id = $this->name.'-'.$this->service->uuid;
instant_remote_process(["docker restart {$container_id}"], $this->service->server);
instant_remote_process(['docker restart '.escapeshellarg($container_id)], $this->service->server);
}
public static function ownedByCurrentTeamAPI(int $teamId)
app/Models/ServiceDatabase.php
+1 -1
View File
@@ -84,7 +84,7 @@ class ServiceDatabase extends BaseModel
public function restart()
{
$container_id = $this->name.'-'.$this->service->uuid;
remote_process(["docker restart {$container_id}"], $this->service->server);
remote_process(['docker restart '.escapeshellarg($container_id)], $this->service->server);
}
public function isRunning()
app/Support/ValidationPatterns.php
+1 -1
View File
@@ -84,7 +84,7 @@ class ValidationPatterns
* Pattern for Docker container names
* Must start with alphanumeric, followed by alphanumeric, dots, hyphens, or underscores
*/
public const CONTAINER_NAME_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/';
public const CONTAINER_NAME_PATTERN = '/\A[a-zA-Z0-9][a-zA-Z0-9._-]*\z/';
/**
* Pattern for Docker network names
tests/Feature/CreateScheduledBackupValidationTest.php
+27
View File
@@ -1,5 +1,6 @@
<?php
use App\Jobs\DatabaseBackupJob;
use App\Livewire\Project\Database\CreateScheduledBackup;
use App\Models\Environment;
use App\Models\Project;
@@ -93,6 +94,32 @@ it('creates a service database backup without S3 and opens its configuration', f
->and($backup->s3_storage_id)->toBeNull();
});
it('rejects an unsupported service database name during backup', function () {
$service = Service::factory()->create([
'server_id' => $this->server->id,
'destination_id' => $this->destination->id,
'destination_type' => $this->destination->getMorphClass(),
'environment_id' => $this->environment->id,
]);
$database = ServiceDatabase::create([
'service_id' => $service->id,
'name' => 'postgres test',
'image' => 'postgres:16-alpine',
'custom_type' => 'postgresql',
'status' => 'running',
]);
$backup = ScheduledDatabaseBackup::create([
'frequency' => '0 0 * * *',
'save_s3' => false,
'database_type' => ServiceDatabase::class,
'database_id' => $database->id,
'team_id' => $this->team->id,
]);
expect(fn () => (new DatabaseBackupJob($backup))->handle())
->toThrow(Exception::class, 'Invalid database container name.');
});
it('selects a service database when creating a backup from the unified backups page', function () {
$service = Service::factory()->create([
'server_id' => $this->server->id,
tests/Unit/DatabaseBackupSecurityTest.php
+42
View File
@@ -1,5 +1,8 @@
<?php
use App\Jobs\DatabaseBackupJob;
use App\Support\ValidationPatterns;
/**
* Database Backup Security Tests
*
@@ -222,3 +225,42 @@ test('escapeshellarg on mongodb URI prevents shell breakout', function () {
// No unescaped double quotes that could break the command
expect(substr_count($command, "'"))->toBeGreaterThanOrEqual(2);
});
test('backup shell commands do not interpolate raw container names or backup paths', function () {
$source = file_get_contents(__DIR__.'/../../app/Jobs/DatabaseBackupJob.php');
preg_match_all('/^.*(?:docker exec|mkdir -p |du -b |docker run -d).*$/m', $source, $matches);
expect($matches[0])->not->toBeEmpty();
foreach ($matches[0] as $command) {
$quotedArgumentsRemoved = preg_replace('/escapeshellarg\(\$this->(?:container_name|backup_dir|backup_location)\)/', '', $command);
expect($quotedArgumentsRemoved)->not->toMatch('/\$this->(?:container_name|backup_dir|backup_location)\b/');
}
});
test('backup job accepts supported service container names only', function () {
expect(ValidationPatterns::isValidContainerName('db-name_service.1-uuid'))->toBeTrue()
->and(ValidationPatterns::isValidContainerName('db name-uuid'))->toBeFalse()
->and(ValidationPatterns::isValidContainerName("db\nname-uuid"))->toBeFalse()
->and(ValidationPatterns::isValidContainerName("db-uuid\n"))->toBeFalse();
$source = file_get_contents(__DIR__.'/../../app/Jobs/DatabaseBackupJob.php');
expect($source)->toContain('ValidationPatterns::isValidContainerName($this->container_name)');
});
test('service restart commands do not interpolate raw container names', function () {
foreach (['ServiceDatabase.php', 'ServiceApplication.php'] as $model) {
$source = file_get_contents(__DIR__.'/../../app/Models/'.$model);
expect($source)->not->toContain('docker restart {$container_id}');
}
});
test('backup filenames remove shell and path separators from database names', function () {
$job = (new ReflectionClass(DatabaseBackupJob::class))->newInstanceWithoutConstructor();
$filenamePart = (new ReflectionClass($job))->getMethod('backupFilenamePart');
expect($filenamePart->invoke($job, 'safe_db-1'))->toBe('safe_db-1')
->and($filenamePart->invoke($job, '../db name'))->toBe('..-db-name');
$source = file_get_contents(__DIR__.'/../../app/Jobs/DatabaseBackupJob.php');
expect(substr_count($source, '$this->backupFilenamePart('))->toBe(4);
});
tests/Unit/ParseCommandsByLineForSudoTest.php
+10
View File
@@ -43,6 +43,16 @@ test('preserves command substitutions inside database and volume backup scripts'
->not->toContain('$(sudo if');
});
test('preserves quoted backup container and file arguments for a non-root server', function () {
$container = escapeshellarg('db-name-uuid');
$path = escapeshellarg('/backups/db-name.dump');
$command = "docker exec {$container} pg_dump --username 'postgres' 'app' > {$path}";
$result = parseCommandsByLineForSudo(collect([$command]), $this->server);
expect($result)->toBe(['sudo '.$command]);
});
test('wraps complex Docker install command with multiple fallbacks', function () {
$commands = collect([
'curl --max-time 300 https://releases.rancher.com/install-docker/27.3.sh | sh || curl https://get.docker.com | sh -s -- --version 27.3',