diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 5fc9f6cd8..c319fe074 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -666,7 +666,21 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $escapedSecret = escapeshellarg($secret); $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}/"; + + // Build S3 path with optional prefix + $s3Path = $bucket; + if (filled($this->s3->path)) { + // Strip leading/trailing slashes to avoid double slashes in path + $pathPrefix = trim($this->s3->path, '/'); + $s3Path .= '/'.$pathPrefix; + } + $s3Path .= $this->backup_dir.'/'; + + // 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/Livewire/Storage/Form.php b/app/Livewire/Storage/Form.php index 4dc0b6ae2..898258a5d 100644 --- a/app/Livewire/Storage/Form.php +++ b/app/Livewire/Storage/Form.php @@ -29,6 +29,8 @@ class Form extends Component public string $secret; + public ?string $path = null; + public ?bool $isUsable = null; protected function rules(): array @@ -42,6 +44,7 @@ class Form extends Component 'secret' => 'required|max:255', 'bucket' => 'required|max:255', 'endpoint' => 'required|url|max:255', + 'path' => ['nullable', 'max:255', 'regex:/^[a-zA-Z0-9\/\-_\.]*$/', 'not_regex:/\.\./'], ]; } @@ -61,6 +64,9 @@ class Form extends Component 'endpoint.required' => 'The Endpoint field is required.', 'endpoint.url' => 'The Endpoint must be a valid URL.', 'endpoint.max' => 'The Endpoint may not be greater than 255 characters.', + 'path.max' => 'The Path Prefix may not be greater than 255 characters.', + 'path.regex' => 'The Path Prefix may only contain letters, numbers, slashes (/), dashes (-), underscores (_), and dots (.).', + 'path.not_regex' => 'The Path Prefix may not contain directory traversal sequences (..).', ] ); } @@ -74,6 +80,7 @@ class Form extends Component 'secret' => 'Secret', 'bucket' => 'Bucket', 'endpoint' => 'Endpoint', + 'path' => 'Path Prefix', ]; /** @@ -92,6 +99,7 @@ class Form extends Component $this->storage->region = $this->region; $this->storage->key = $this->key; $this->storage->secret = $this->secret; + $this->storage->path = $this->path; $this->storage->is_usable = $this->isUsable; } else { // Sync FROM model (on load/refresh) @@ -102,6 +110,7 @@ class Form extends Component $this->region = $this->storage->region; $this->key = $this->storage->key; $this->secret = $this->storage->secret; + $this->path = $this->storage->path; $this->isUsable = $this->storage->is_usable; } } 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 5df36db33..47c47eb0a 100644 --- a/bootstrap/helpers/databases.php +++ b/bootstrap/helpers/databases.php @@ -209,6 +209,19 @@ function deleteBackupsS3(string|array|null $filenames, S3Storage $s3): void $filenames = [$filenames]; } + // Apply path prefix if configured + if (filled($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 + $cleanFilename = ltrim($filename, '/'); + + return $pathPrefix.'/'.$cleanFilename; + }, $filenames); + } + $disk = Storage::build([ 'driver' => 's3', 'key' => $s3->key, diff --git a/database/migrations/2025_12_26_000001_add_path_to_s3_storages_table.php b/database/migrations/2025_12_26_000001_add_path_to_s3_storages_table.php new file mode 100644 index 000000000..92aedb977 --- /dev/null +++ b/database/migrations/2025_12_26_000001_add_path_to_s3_storages_table.php @@ -0,0 +1,28 @@ +string('path')->nullable()->after('endpoint'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('s3_storages', function (Blueprint $table) { + $table->dropColumn('path'); + }); + } +}; diff --git a/resources/views/livewire/storage/form.blade.php b/resources/views/livewire/storage/form.blade.php index 850d7735f..146c272e3 100644 --- a/resources/views/livewire/storage/form.blade.php +++ b/resources/views/livewire/storage/form.blade.php @@ -40,6 +40,11 @@ +
+ +
diff --git a/tests/Unit/S3StorageTest.php b/tests/Unit/S3StorageTest.php index 6709f381d..128d3250e 100644 --- a/tests/Unit/S3StorageTest.php +++ b/tests/Unit/S3StorageTest.php @@ -51,3 +51,67 @@ test('S3Storage model is guarded correctly', function () { // The model should have $guarded = [] which means everything is fillable expect($s3Storage->getGuarded())->toBe([]); }); + +test('S3Storage path attribute normalizes path correctly', function () { + $s3Storage = new S3Storage; + + // Path should be normalized to start with / + $s3Storage->path = 'backups/coolify'; + expect($s3Storage->path)->toBe('/backups/coolify'); + + // Path with leading slash should remain unchanged + $s3Storage->path = '/backups/coolify'; + expect($s3Storage->path)->toBe('/backups/coolify'); + + // Empty path should return null + $s3Storage->path = ''; + expect($s3Storage->path)->toBeNull(); + + // Null path should return null + $s3Storage->path = null; + expect($s3Storage->path)->toBeNull(); + + // Path with whitespace should be trimmed + $s3Storage->path = ' backups/coolify '; + expect($s3Storage->path)->toBe('/backups/coolify'); +}); + +test('S3Storage path attribute handles various path formats', function () { + $s3Storage = new S3Storage; + + // Simple path + $s3Storage->path = 'instance-1'; + expect($s3Storage->path)->toBe('/instance-1'); + + // Nested path + $s3Storage->path = 'production/backups/db'; + expect($s3Storage->path)->toBe('/production/backups/db'); + + // Path with special characters + $s3Storage->path = 'my-instance_2024.backups'; + expect($s3Storage->path)->toBe('/my-instance_2024.backups'); +}); + +test('S3Storage path attribute handles edge cases', function () { + $s3Storage = new S3Storage; + + // Multiple consecutive slashes are preserved (validation should catch this) + $s3Storage->path = 'path//to///backup'; + expect($s3Storage->path)->toBe('/path//to///backup'); + + // Path ending with slash - trailing slashes are now stripped + $s3Storage->path = 'backups/coolify/'; + expect($s3Storage->path)->toBe('/backups/coolify'); + + // Only whitespace should return null + $s3Storage->path = ' '; + expect($s3Storage->path)->toBeNull(); + + // 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(); +});