From 6dd0eadadcc0debd8ab48240f1d1d9fcfb7c87ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20=C3=9CRG=C3=9CN?= Date: Thu, 23 Jul 2026 13:13:25 +0300 Subject: [PATCH 1/4] [Fix] Prevent backups:run crash when a server with backups is deleted --- app/Actions/Backup/ManageBackupFile.php | 23 ++++++++++- app/Console/Commands/RunBackupCommand.php | 2 + app/Models/Server.php | 5 +++ tests/Feature/BackupTest.php | 42 ++++++++++++++++++++ tests/Feature/ServerTest.php | 29 ++++++++++++++ tests/Unit/Commands/RunBackupCommandTest.php | 11 +++++ 6 files changed, 110 insertions(+), 2 deletions(-) diff --git a/app/Actions/Backup/ManageBackupFile.php b/app/Actions/Backup/ManageBackupFile.php index 9a522f741..f43262de1 100644 --- a/app/Actions/Backup/ManageBackupFile.php +++ b/app/Actions/Backup/ManageBackupFile.php @@ -8,7 +8,9 @@ use App\Http\Resources\BackupFileResource; use App\Jobs\Backup\DeleteFileJob; use App\Models\BackupFile; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; +use RuntimeException; use Symfony\Component\HttpFoundation\StreamedResponse; use Throwable; @@ -19,7 +21,12 @@ class ManageBackupFile */ public function download(BackupFile $file): StreamedResponse { - $file->backup->server->ssh()->download( + $server = $file->backup?->server; + if ($server === null) { + throw new RuntimeException('The backup server no longer exists.'); + } + + $server->ssh()->download( Storage::disk('tmp')->path(basename($file->path())), $file->path() ); @@ -29,12 +36,24 @@ public function download(BackupFile $file): StreamedResponse public function delete(BackupFile $file): void { + $projectId = $file->backup?->server?->project_id; + + if ($projectId === null) { + Log::warning('Deleting orphaned backup file without a server', [ + 'backup_file_id' => $file->id, + 'backup_id' => $file->backup_id, + ]); + $file->delete(); + + return; + } + $file->status = BackupFileStatus::DELETING; $file->message = null; $file->save(); SocketEvent::dispatch(new SocketEventDTO( - projectId: $file->backup->server->project_id, + projectId: $projectId, type: 'backup-file.updated', data: new BackupFileResource($file), )); diff --git a/app/Console/Commands/RunBackupCommand.php b/app/Console/Commands/RunBackupCommand.php index 861851994..68ab4cbc8 100644 --- a/app/Console/Commands/RunBackupCommand.php +++ b/app/Console/Commands/RunBackupCommand.php @@ -20,6 +20,8 @@ public function handle(): void Backup::query() ->where('enabled', true) ->whereNull('status') + ->whereHas('server') + ->with('server') ->chunkById(100, function ($backups) use (&$total): void { /** @var Backup $backup */ foreach ($backups as $backup) { diff --git a/app/Models/Server.php b/app/Models/Server.php index dd384153b..9bdfaa2e4 100755 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -144,6 +144,11 @@ public static function boot(): void $log->delete(); }); $server->services()->delete(); + $server->backups()->each(function ($backup): void { + /** @var Backup $backup */ + $backup->files()->delete(); + }); + $server->backups()->delete(); $server->databases()->delete(); $server->databaseUsers()->delete(); $server->firewallRules()->delete(); diff --git a/tests/Feature/BackupTest.php b/tests/Feature/BackupTest.php index 9e154cc09..b1b6b6c17 100644 --- a/tests/Feature/BackupTest.php +++ b/tests/Feature/BackupTest.php @@ -2,12 +2,14 @@ namespace Tests\Feature; +use App\Actions\Backup\ManageBackupFile; use App\Actions\Backup\RestoreBackup; use App\Actions\Backup\RunBackup; use App\Enums\BackupFileStatus; use App\Enums\BackupStatus; use App\Enums\BackupType; use App\Facades\SSH; +use App\Jobs\Backup\DeleteFileJob; use App\Models\Backup; use App\Models\BackupFile; use App\Models\Database; @@ -20,6 +22,7 @@ use Illuminate\Support\Facades\Http; use Illuminate\Validation\ValidationException; use PHPUnit\Framework\Attributes\DataProvider; +use RuntimeException; use Tests\TestCase; class BackupTest extends TestCase @@ -597,6 +600,45 @@ public function test_cannot_enable_a_backup_being_deleted(): void ]); } + public function test_delete_orphaned_backup_file_hard_deletes_without_dispatching_job(): void + { + Bus::fake(); + + $backup = Backup::factory()->create([ + 'type' => BackupType::DATABASE, + 'server_id' => 999999, + 'storage_id' => $this->storageProvider->id, + 'status' => null, + ]); + $file = BackupFile::factory()->create([ + 'backup_id' => $backup->id, + 'status' => BackupFileStatus::CREATED, + ]); + + app(ManageBackupFile::class)->delete($file); + + $this->assertDatabaseMissing('backup_files', ['id' => $file->id]); + Bus::assertNotDispatched(DeleteFileJob::class); + } + + public function test_download_orphaned_backup_file_throws(): void + { + $backup = Backup::factory()->create([ + 'type' => BackupType::DATABASE, + 'server_id' => 999999, + 'storage_id' => $this->storageProvider->id, + 'status' => null, + ]); + $file = BackupFile::factory()->create([ + 'backup_id' => $backup->id, + 'status' => BackupFileStatus::CREATED, + ]); + + $this->expectException(RuntimeException::class); + + app(ManageBackupFile::class)->download($file); + } + private function setupDatabase(string $database, string $version): void { $this->server->services()->where('type', 'database')->delete(); diff --git a/tests/Feature/ServerTest.php b/tests/Feature/ServerTest.php index a39dde986..84e6fbb6a 100644 --- a/tests/Feature/ServerTest.php +++ b/tests/Feature/ServerTest.php @@ -9,9 +9,13 @@ use App\Enums\UserRole; use App\Facades\Notifier; use App\Facades\SSH; +use App\Models\Backup; +use App\Models\BackupFile; +use App\Models\Database; use App\Models\Project; use App\Models\Server; use App\Models\ServerProvider; +use App\Models\StorageProvider; use App\Models\User; use App\NotificationChannels\Email\NotificationMail; use App\Notifications\ServerAutoUpdateCompleted; @@ -85,6 +89,31 @@ public function test_delete_server(): void ]); } + public function test_delete_server_cascades_backups(): void + { + $this->actingAs($this->user); + + SSH::fake(); + + $database = Database::factory()->create(['server_id' => $this->server->id]); + $storage = StorageProvider::factory()->dropbox()->create(['user_id' => $this->user->id]); + $backup = Backup::factory()->create([ + 'server_id' => $this->server->id, + 'database_id' => $database->id, + 'storage_id' => $storage->id, + 'keep_backups' => 10, + ]); + $file = BackupFile::factory()->create(['backup_id' => $backup->id]); + + $this->delete(route('servers.destroy', $this->server), [ + 'name' => $this->server->name, + ])->assertSessionDoesntHaveErrors(); + + $this->assertDatabaseMissing('servers', ['id' => $this->server->id]); + $this->assertDatabaseMissing('backups', ['id' => $backup->id]); + $this->assertDatabaseMissing('backup_files', ['id' => $file->id]); + } + public function test_cannot_delete_on_provider(): void { Mail::fake(); diff --git a/tests/Unit/Commands/RunBackupCommandTest.php b/tests/Unit/Commands/RunBackupCommandTest.php index 37fa63e40..2f26a3708 100644 --- a/tests/Unit/Commands/RunBackupCommandTest.php +++ b/tests/Unit/Commands/RunBackupCommandTest.php @@ -121,4 +121,15 @@ public function test_runs_enabled_backup_even_after_a_failed_run(): void $this->artisan('backups:run') ->expectsOutput('1 backups started'); } + + public function test_does_not_run_backups_whose_server_is_missing(): void + { + SSH::fake(); + Carbon::setTestNow('2026-06-19 10:00:00'); + + $this->createBackup(['interval' => '0 * * * *', 'server_id' => 999999]); + + $this->artisan('backups:run') + ->expectsOutput('0 backups started'); + } } From 90b5392205ed095b00ae75a6413fecaa7c6af642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20=C3=9CRG=C3=9CN?= Date: Thu, 23 Jul 2026 14:39:36 +0300 Subject: [PATCH 2/4] [Fix] Use nullable Server lookup in ManageBackupFile to satisfy PHPStan --- app/Actions/Backup/ManageBackupFile.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/Actions/Backup/ManageBackupFile.php b/app/Actions/Backup/ManageBackupFile.php index f43262de1..73648ed0c 100644 --- a/app/Actions/Backup/ManageBackupFile.php +++ b/app/Actions/Backup/ManageBackupFile.php @@ -8,6 +8,7 @@ use App\Http\Resources\BackupFileResource; use App\Jobs\Backup\DeleteFileJob; use App\Models\BackupFile; +use App\Models\Server; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use RuntimeException; @@ -21,7 +22,7 @@ class ManageBackupFile */ public function download(BackupFile $file): StreamedResponse { - $server = $file->backup?->server; + $server = Server::find($file->backup->server_id); if ($server === null) { throw new RuntimeException('The backup server no longer exists.'); } @@ -36,9 +37,9 @@ public function download(BackupFile $file): StreamedResponse public function delete(BackupFile $file): void { - $projectId = $file->backup?->server?->project_id; + $server = Server::find($file->backup->server_id); - if ($projectId === null) { + if ($server === null) { Log::warning('Deleting orphaned backup file without a server', [ 'backup_file_id' => $file->id, 'backup_id' => $file->backup_id, @@ -53,7 +54,7 @@ public function delete(BackupFile $file): void $file->save(); SocketEvent::dispatch(new SocketEventDTO( - projectId: $projectId, + projectId: $server->project_id, type: 'backup-file.updated', data: new BackupFileResource($file), )); From 00e8a3cbce1879418ad6f51b0207f18de6a0a94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20=C3=9CRG=C3=9CN?= Date: Thu, 23 Jul 2026 15:45:20 +0300 Subject: [PATCH 3/4] [Fix] Scope to already-orphaned backups; drop server-delete cascade (handled by #1198) --- app/Models/Server.php | 5 ---- tests/Feature/ServerTest.php | 29 -------------------- tests/Unit/Commands/RunBackupCommandTest.php | 7 ++++- 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/app/Models/Server.php b/app/Models/Server.php index 9bdfaa2e4..dd384153b 100755 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -144,11 +144,6 @@ public static function boot(): void $log->delete(); }); $server->services()->delete(); - $server->backups()->each(function ($backup): void { - /** @var Backup $backup */ - $backup->files()->delete(); - }); - $server->backups()->delete(); $server->databases()->delete(); $server->databaseUsers()->delete(); $server->firewallRules()->delete(); diff --git a/tests/Feature/ServerTest.php b/tests/Feature/ServerTest.php index 84e6fbb6a..a39dde986 100644 --- a/tests/Feature/ServerTest.php +++ b/tests/Feature/ServerTest.php @@ -9,13 +9,9 @@ use App\Enums\UserRole; use App\Facades\Notifier; use App\Facades\SSH; -use App\Models\Backup; -use App\Models\BackupFile; -use App\Models\Database; use App\Models\Project; use App\Models\Server; use App\Models\ServerProvider; -use App\Models\StorageProvider; use App\Models\User; use App\NotificationChannels\Email\NotificationMail; use App\Notifications\ServerAutoUpdateCompleted; @@ -89,31 +85,6 @@ public function test_delete_server(): void ]); } - public function test_delete_server_cascades_backups(): void - { - $this->actingAs($this->user); - - SSH::fake(); - - $database = Database::factory()->create(['server_id' => $this->server->id]); - $storage = StorageProvider::factory()->dropbox()->create(['user_id' => $this->user->id]); - $backup = Backup::factory()->create([ - 'server_id' => $this->server->id, - 'database_id' => $database->id, - 'storage_id' => $storage->id, - 'keep_backups' => 10, - ]); - $file = BackupFile::factory()->create(['backup_id' => $backup->id]); - - $this->delete(route('servers.destroy', $this->server), [ - 'name' => $this->server->name, - ])->assertSessionDoesntHaveErrors(); - - $this->assertDatabaseMissing('servers', ['id' => $this->server->id]); - $this->assertDatabaseMissing('backups', ['id' => $backup->id]); - $this->assertDatabaseMissing('backup_files', ['id' => $file->id]); - } - public function test_cannot_delete_on_provider(): void { Mail::fake(); diff --git a/tests/Unit/Commands/RunBackupCommandTest.php b/tests/Unit/Commands/RunBackupCommandTest.php index 2f26a3708..b681c4078 100644 --- a/tests/Unit/Commands/RunBackupCommandTest.php +++ b/tests/Unit/Commands/RunBackupCommandTest.php @@ -11,6 +11,7 @@ use App\Models\StorageProvider; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Http; use Tests\TestCase; @@ -125,11 +126,15 @@ public function test_runs_enabled_backup_even_after_a_failed_run(): void public function test_does_not_run_backups_whose_server_is_missing(): void { SSH::fake(); + Bus::fake(); Carbon::setTestNow('2026-06-19 10:00:00'); - $this->createBackup(['interval' => '0 * * * *', 'server_id' => 999999]); + $backup = $this->createBackup(['interval' => '0 * * * *', 'server_id' => 999999]); $this->artisan('backups:run') ->expectsOutput('0 backups started'); + + $this->assertDatabaseHas('backups', ['id' => $backup->id]); + Bus::assertNothingDispatched(); } } From db9aad5bf4b7326882ffdfcecd139b6ecc19725b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20=C3=9CRG=C3=9CN?= Date: Thu, 23 Jul 2026 22:15:01 +0300 Subject: [PATCH 4/4] Remove unused ManageBackupFile::download() and its test per review; strengthen orphan assertion --- app/Actions/Backup/ManageBackupFile.php | 22 -------------------- tests/Feature/BackupTest.php | 19 ----------------- tests/Unit/Commands/RunBackupCommandTest.php | 2 +- 3 files changed, 1 insertion(+), 42 deletions(-) diff --git a/app/Actions/Backup/ManageBackupFile.php b/app/Actions/Backup/ManageBackupFile.php index 73648ed0c..1b844e2e9 100644 --- a/app/Actions/Backup/ManageBackupFile.php +++ b/app/Actions/Backup/ManageBackupFile.php @@ -10,31 +10,9 @@ use App\Models\BackupFile; use App\Models\Server; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; -use RuntimeException; -use Symfony\Component\HttpFoundation\StreamedResponse; -use Throwable; class ManageBackupFile { - /** - * @throws Throwable - */ - public function download(BackupFile $file): StreamedResponse - { - $server = Server::find($file->backup->server_id); - if ($server === null) { - throw new RuntimeException('The backup server no longer exists.'); - } - - $server->ssh()->download( - Storage::disk('tmp')->path(basename($file->path())), - $file->path() - ); - - return Storage::disk('tmp')->download(basename($file->path())); - } - public function delete(BackupFile $file): void { $server = Server::find($file->backup->server_id); diff --git a/tests/Feature/BackupTest.php b/tests/Feature/BackupTest.php index 8ef719d2a..715dc57b2 100644 --- a/tests/Feature/BackupTest.php +++ b/tests/Feature/BackupTest.php @@ -28,7 +28,6 @@ use Illuminate\Validation\ValidationException; use Inertia\Testing\AssertableInertia; use PHPUnit\Framework\Attributes\DataProvider; -use RuntimeException; use Tests\TestCase; class BackupTest extends TestCase @@ -627,24 +626,6 @@ public function test_delete_orphaned_backup_file_hard_deletes_without_dispatchin Bus::assertNotDispatched(DeleteFileJob::class); } - public function test_download_orphaned_backup_file_throws(): void - { - $backup = Backup::factory()->create([ - 'type' => BackupType::DATABASE, - 'server_id' => 999999, - 'storage_id' => $this->storageProvider->id, - 'status' => null, - ]); - $file = BackupFile::factory()->create([ - 'backup_id' => $backup->id, - 'status' => BackupFileStatus::CREATED, - ]); - - $this->expectException(RuntimeException::class); - - app(ManageBackupFile::class)->download($file); - } - public function test_see_global_backups_list_scoped_to_current_project(): void { $this->actingAs($this->user); diff --git a/tests/Unit/Commands/RunBackupCommandTest.php b/tests/Unit/Commands/RunBackupCommandTest.php index b681c4078..0d1da6fff 100644 --- a/tests/Unit/Commands/RunBackupCommandTest.php +++ b/tests/Unit/Commands/RunBackupCommandTest.php @@ -134,7 +134,7 @@ public function test_does_not_run_backups_whose_server_is_missing(): void $this->artisan('backups:run') ->expectsOutput('0 backups started'); - $this->assertDatabaseHas('backups', ['id' => $backup->id]); + $this->assertDatabaseHas('backups', ['id' => $backup->id, 'server_id' => 999999]); Bus::assertNothingDispatched(); } }