diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index ec995cafb..fa6fa9d78 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -664,12 +664,17 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue // Build S3 path with optional prefix $s3Path = $bucket; if (filled($this->s3->path)) { - $pathPrefix = ltrim($this->s3->path, '/'); + // Strip leading/trailing slashes to avoid double slashes in path + $pathPrefix = trim($this->s3->path, '/'); $s3Path .= '/'.$pathPrefix; } $s3Path .= $this->backup_dir.'/'; - $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp $this->backup_location temporary/{$s3Path}"; + // Escape the paths to prevent command injection + $escapedBackupLocation = escapeshellarg($this->backup_location); + $escapedS3Path = escapeshellarg("temporary/{$s3Path}"); + + $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp {$escapedBackupLocation} {$escapedS3Path}"; instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true); $this->s3_uploaded = true; diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 3aae55966..65bb73e32 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -72,7 +72,11 @@ class S3Storage extends BaseModel return null; } - return str($value)->trim()->start('/')->value(); + // Trim whitespace and normalize: add leading slash, remove trailing slash + $path = str($value)->trim()->start('/')->rtrim('/')->value(); + + // Return null if only slashes remain after trimming + return $path === '/' ? null : $path; } ); } diff --git a/bootstrap/helpers/databases.php b/bootstrap/helpers/databases.php index 1d22c364b..47c47eb0a 100644 --- a/bootstrap/helpers/databases.php +++ b/bootstrap/helpers/databases.php @@ -211,7 +211,8 @@ function deleteBackupsS3(string|array|null $filenames, S3Storage $s3): void // Apply path prefix if configured if (filled($s3->path)) { - $pathPrefix = ltrim($s3->path, '/'); + // Strip leading/trailing slashes to avoid double slashes in path + $pathPrefix = trim($s3->path, '/'); $filenames = array_map(function ($filename) use ($pathPrefix) { // The filename is the backup path (e.g., /data/coolify/backups/databases/...) // We need to prepend the path prefix to match the S3 key structure diff --git a/tests/Unit/S3StorageTest.php b/tests/Unit/S3StorageTest.php index 614576819..128d3250e 100644 --- a/tests/Unit/S3StorageTest.php +++ b/tests/Unit/S3StorageTest.php @@ -99,9 +99,9 @@ test('S3Storage path attribute handles edge cases', function () { $s3Storage->path = 'path//to///backup'; expect($s3Storage->path)->toBe('/path//to///backup'); - // Path ending with slash + // Path ending with slash - trailing slashes are now stripped $s3Storage->path = 'backups/coolify/'; - expect($s3Storage->path)->toBe('/backups/coolify/'); + expect($s3Storage->path)->toBe('/backups/coolify'); // Only whitespace should return null $s3Storage->path = ' '; @@ -110,4 +110,8 @@ test('S3Storage path attribute handles edge cases', function () { // Path with dots (valid single dots) $s3Storage->path = 'path.with.dots'; expect($s3Storage->path)->toBe('/path.with.dots'); + + // Only slashes should return null + $s3Storage->path = '///'; + expect($s3Storage->path)->toBeNull(); });