From a4d75ff0e28b8a9a5a9f4ea58ef32a87b7b4ec24 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 23 May 2026 13:06:36 +0200 Subject: [PATCH 1/2] fix(backups): validate S3 storage before backup scheduling Prevent scheduled database backups from enabling S3 uploads without a valid team-owned storage configuration, and preserve the previous S3 storage ID in missing-storage error messages. Add coverage for backup edit/create validation and S3 upload failure messaging. --- app/Jobs/DatabaseBackupJob.php | 4 +- app/Livewire/Project/Database/BackupEdit.php | 16 ++- .../Database/CreateScheduledBackup.php | 11 +- app/Models/S3Storage.php | 1 + .../ScheduledDatabaseBackupExecution.php | 1 + tests/Feature/BackupEditValidationTest.php | 85 +++++++++++++++ .../CreateScheduledBackupValidationTest.php | 102 ++++++++++++++++++ tests/Feature/DatabaseBackupJobTest.php | 42 ++++++++ 8 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 tests/Feature/BackupEditValidationTest.php create mode 100644 tests/Feature/CreateScheduledBackupValidationTest.php diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index bd31ab0c3..64e900b49 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -668,12 +668,14 @@ private function calculate_size() private function upload_to_s3(): void { if (is_null($this->s3)) { + $previousS3StorageId = $this->backup->s3_storage_id; + $this->backup->update([ 'save_s3' => false, 's3_storage_id' => null, ]); - throw new \Exception('S3 storage configuration is missing or has been deleted (S3 storage ID: '.($this->backup->s3_storage_id ?? 'null').'). S3 backup has been disabled for this schedule.'); + throw new \Exception('S3 storage configuration is missing or has been deleted (S3 storage ID: '.($previousS3StorageId ?? 'null').'). S3 backup has been disabled for this schedule.'); } try { diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index a18022882..ef106a65f 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Database; use App\Models\ScheduledDatabaseBackup; +use App\Models\ServiceDatabase; use Exception; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Attributes\Locked; @@ -144,7 +145,7 @@ public function delete($password, $selectedActions = []) try { $server = null; - if ($this->backup->database instanceof \App\Models\ServiceDatabase) { + if ($this->backup->database instanceof ServiceDatabase) { $server = $this->backup->database->service->destination->server; } elseif ($this->backup->database->destination && $this->backup->database->destination->server) { $server = $this->backup->database->destination->server; @@ -170,7 +171,7 @@ public function delete($password, $selectedActions = []) $this->backup->delete(); - if ($this->backup->database->getMorphClass() === \App\Models\ServiceDatabase::class) { + if ($this->backup->database->getMorphClass() === ServiceDatabase::class) { $serviceDatabase = $this->backup->database; return redirect()->route('project.service.database.backups', [ @@ -182,7 +183,7 @@ public function delete($password, $selectedActions = []) } else { return redirect()->route('project.database.backup.index', $this->parameters); } - } catch (\Exception $e) { + } catch (Exception $e) { $this->dispatch('error', 'Failed to delete backup: '.$e->getMessage()); return handleError($e, $this); @@ -207,6 +208,13 @@ private function customValidate() $this->backup->s3_storage_id = null; } + // S3 backup cannot be enabled without a valid S3 storage owned by the team + $availableS3Ids = collect($this->s3s)->pluck('id'); + if ($this->backup->save_s3 && ! $availableS3Ids->contains($this->backup->s3_storage_id)) { + $this->backup->save_s3 = $this->saveS3 = false; + $this->backup->s3_storage_id = $this->s3StorageId = null; + } + // Validate that disable_local_backup can only be true when S3 backup is enabled if ($this->backup->disable_local_backup && ! $this->backup->save_s3) { $this->backup->disable_local_backup = $this->disableLocalBackup = false; @@ -214,7 +222,7 @@ private function customValidate() $isValid = validate_cron_expression($this->backup->frequency); if (! $isValid) { - throw new \Exception('Invalid Cron / Human expression'); + throw new Exception('Invalid Cron / Human expression'); } $this->validate(); } diff --git a/app/Livewire/Project/Database/CreateScheduledBackup.php b/app/Livewire/Project/Database/CreateScheduledBackup.php index 7f807afe2..bb8b8b9c7 100644 --- a/app/Livewire/Project/Database/CreateScheduledBackup.php +++ b/app/Livewire/Project/Database/CreateScheduledBackup.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Database; use App\Models\ScheduledDatabaseBackup; +use App\Models\ServiceDatabase; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Livewire\Attributes\Locked; @@ -48,6 +49,14 @@ public function submit() $this->validate(); + if ($this->saveToS3) { + if (is_null($this->s3StorageId) || ! $this->definedS3s->contains('id', $this->s3StorageId)) { + $this->dispatch('error', 'Please select a valid S3 storage to enable S3 backups.'); + + return; + } + } + $isValid = validate_cron_expression($this->frequency); if (! $isValid) { $this->dispatch('error', 'Invalid Cron / Human expression.'); @@ -74,7 +83,7 @@ public function submit() } $databaseBackup = ScheduledDatabaseBackup::create($payload); - if ($this->database->getMorphClass() === \App\Models\ServiceDatabase::class) { + if ($this->database->getMorphClass() === ServiceDatabase::class) { $this->dispatch('refreshScheduledBackups', $databaseBackup->id); } else { $this->dispatch('refreshScheduledBackups'); diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 3f6ee51cc..fca74170d 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -15,6 +15,7 @@ class S3Storage extends BaseModel use HasFactory, HasSafeStringAttribute; protected $fillable = [ + 'team_id', 'name', 'description', 'region', diff --git a/app/Models/ScheduledDatabaseBackupExecution.php b/app/Models/ScheduledDatabaseBackupExecution.php index 51ad46de9..1d5f5f9ce 100644 --- a/app/Models/ScheduledDatabaseBackupExecution.php +++ b/app/Models/ScheduledDatabaseBackupExecution.php @@ -23,6 +23,7 @@ class ScheduledDatabaseBackupExecution extends BaseModel protected function casts(): array { return [ + 'size' => 'integer', 's3_uploaded' => 'boolean', 'local_storage_deleted' => 'boolean', 's3_storage_deleted' => 'boolean', diff --git a/tests/Feature/BackupEditValidationTest.php b/tests/Feature/BackupEditValidationTest.php new file mode 100644 index 000000000..8894f0f69 --- /dev/null +++ b/tests/Feature/BackupEditValidationTest.php @@ -0,0 +1,85 @@ +create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $database = StandalonePostgresql::create([ + 'name' => 'pg-backup-edit-validation', + 'image' => 'postgres:16-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'postgres', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + return ScheduledDatabaseBackup::create(array_merge([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => null, + 'database_type' => $database->getMorphClass(), + 'database_id' => $database->id, + 'team_id' => $team->id, + ], $overrides)); +} + +beforeEach(function () { + if (InstanceSettings::find(0) === null) { + $settings = new InstanceSettings; + $settings->id = 0; + $settings->save(); + } + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->user->teams()->attach($this->team, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); +}); + +it('disables S3 backup when saved without a selected S3 storage', function () { + $backup = createBackupForEditValidationTest($this->team); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + ->call('submit') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); +}); + +it('cascades to disabling local backup deletion when S3 is force-disabled', function () { + $backup = createBackupForEditValidationTest($this->team, [ + 'disable_local_backup' => true, + ]); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + ->call('submit') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); + expect($backup->disable_local_backup)->toBeFalsy(); +}); diff --git a/tests/Feature/CreateScheduledBackupValidationTest.php b/tests/Feature/CreateScheduledBackupValidationTest.php new file mode 100644 index 000000000..a167511e2 --- /dev/null +++ b/tests/Feature/CreateScheduledBackupValidationTest.php @@ -0,0 +1,102 @@ +create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + return StandalonePostgresql::create([ + 'name' => 'pg-scheduled-backup-validation', + 'image' => 'postgres:16-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'postgres', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); +} + +function createS3StorageForTeam(Team $team, string $name = 'Test S3'): S3Storage +{ + return S3Storage::create([ + 'name' => $name, + 'region' => 'us-east-1', + 'key' => 'test-key', + 'secret' => 'test-secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'https://s3.example.com', + 'is_usable' => true, + 'team_id' => $team->id, + ]); +} + +beforeEach(function () { + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->user->teams()->attach($this->team, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); +}); + +it('rejects enabling S3 backup without a selected S3 storage', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', null) + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + +it('rejects an S3 storage not owned by the current team', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + + $foreignS3 = createS3StorageForTeam(Team::factory()->create(), 'Foreign S3'); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $foreignS3->id) + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + +it('creates a scheduled backup with a valid team-owned S3 storage', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + $s3 = createS3StorageForTeam($this->team); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $s3->id) + ->call('submit') + ->assertDispatched('refreshScheduledBackups'); + + $backup = ScheduledDatabaseBackup::first(); + expect($backup)->not->toBeNull(); + expect($backup->save_s3)->toBeTruthy(); + expect($backup->s3_storage_id)->toBe($s3->id); +}); diff --git a/tests/Feature/DatabaseBackupJobTest.php b/tests/Feature/DatabaseBackupJobTest.php index 05cb21f12..2d549c1ca 100644 --- a/tests/Feature/DatabaseBackupJobTest.php +++ b/tests/Feature/DatabaseBackupJobTest.php @@ -66,6 +66,48 @@ expect($backup->s3_storage_id)->toBeNull(); }); +test('upload_to_s3 exception message reports the previous s3 storage id', function () { + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => 12345, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => Team::factory()->create()->id, + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + $reflection->getProperty('s3')->setValue($job, null); + + expect(fn () => $reflection->getMethod('upload_to_s3')->invoke($job)) + ->toThrow(Exception::class, 'S3 storage ID: 12345'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); +}); + +test('upload_to_s3 exception message reports null when no previous s3 storage id exists', function () { + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => null, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => Team::factory()->create()->id, + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + $reflection->getProperty('s3')->setValue($job, null); + + expect(fn () => $reflection->getMethod('upload_to_s3')->invoke($job)) + ->toThrow(Exception::class, 'S3 storage ID: null'); +}); + test('deleting s3 storage disables s3 on linked backups', function () { $team = Team::factory()->create(); From 33e172ac24aa43ae89f1f93783f87abd77e6673d Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 23 May 2026 21:06:22 +0200 Subject: [PATCH 2/2] fix(backups): revalidate S3 storage on scheduled backup submit Check the selected S3 storage against the database at submit time so stale Livewire state cannot schedule backups with storage that was reassigned or marked unusable after the component mounted. --- .../Database/CreateScheduledBackup.php | 9 ++++- .../CreateScheduledBackupValidationTest.php | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/Livewire/Project/Database/CreateScheduledBackup.php b/app/Livewire/Project/Database/CreateScheduledBackup.php index bb8b8b9c7..7384adcff 100644 --- a/app/Livewire/Project/Database/CreateScheduledBackup.php +++ b/app/Livewire/Project/Database/CreateScheduledBackup.php @@ -2,6 +2,7 @@ namespace App\Livewire\Project\Database; +use App\Models\S3Storage; use App\Models\ScheduledDatabaseBackup; use App\Models\ServiceDatabase; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; @@ -50,7 +51,13 @@ public function submit() $this->validate(); if ($this->saveToS3) { - if (is_null($this->s3StorageId) || ! $this->definedS3s->contains('id', $this->s3StorageId)) { + $s3StorageExists = ! is_null($this->s3StorageId) + && S3Storage::where('team_id', currentTeam()->id) + ->where('is_usable', true) + ->whereKey($this->s3StorageId) + ->exists(); + + if (! $s3StorageExists) { $this->dispatch('error', 'Please select a valid S3 storage to enable S3 backups.'); return; diff --git a/tests/Feature/CreateScheduledBackupValidationTest.php b/tests/Feature/CreateScheduledBackupValidationTest.php index a167511e2..4b5eec24c 100644 --- a/tests/Feature/CreateScheduledBackupValidationTest.php +++ b/tests/Feature/CreateScheduledBackupValidationTest.php @@ -84,6 +84,42 @@ function createS3StorageForTeam(Team $team, string $name = 'Test S3'): S3Storage expect(ScheduledDatabaseBackup::count())->toBe(0); }); +it('rejects an S3 storage that is reassigned after the component is mounted', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + $s3 = createS3StorageForTeam($this->team); + + $component = Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $s3->id); + + $s3->update(['team_id' => Team::factory()->create()->id]); + + $component + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + +it('rejects an S3 storage that becomes unusable after the component is mounted', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + $s3 = createS3StorageForTeam($this->team); + + $component = Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $s3->id); + + $s3->update(['is_usable' => false]); + + $component + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + it('creates a scheduled backup with a valid team-owned S3 storage', function () { $database = createDatabaseForScheduledBackupTest($this->team); $s3 = createS3StorageForTeam($this->team);