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.
This commit is contained in:
Andras Bacsai
2026-05-23 13:06:36 +02:00
parent a49bc5dd14
commit a4d75ff0e2
8 changed files with 256 additions and 6 deletions
+3 -1
View File
@@ -668,12 +668,14 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
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 {
+12 -4
View File
@@ -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 @@ class BackupEdit extends Component
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 @@ class BackupEdit extends Component
$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 @@ class BackupEdit extends Component
} 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 @@ class BackupEdit extends Component
$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 @@ class BackupEdit extends Component
$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();
}
@@ -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 @@ class CreateScheduledBackup extends Component
$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 @@ class CreateScheduledBackup extends Component
}
$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');
+1
View File
@@ -15,6 +15,7 @@ class S3Storage extends BaseModel
use HasFactory, HasSafeStringAttribute;
protected $fillable = [
'team_id',
'name',
'description',
'region',
@@ -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',
@@ -0,0 +1,85 @@
<?php
use App\Livewire\Project\Database\BackupEdit;
use App\Models\Environment;
use App\Models\InstanceSettings;
use App\Models\Project;
use App\Models\ScheduledDatabaseBackup;
use App\Models\Server;
use App\Models\StandaloneDocker;
use App\Models\StandalonePostgresql;
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Livewire\Livewire;
uses(RefreshDatabase::class);
function createBackupForEditValidationTest(Team $team, array $overrides = []): ScheduledDatabaseBackup
{
$server = Server::factory()->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();
});
@@ -0,0 +1,102 @@
<?php
use App\Livewire\Project\Database\CreateScheduledBackup;
use App\Models\Environment;
use App\Models\Project;
use App\Models\S3Storage;
use App\Models\ScheduledDatabaseBackup;
use App\Models\Server;
use App\Models\StandaloneDocker;
use App\Models\StandalonePostgresql;
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Livewire\Livewire;
uses(RefreshDatabase::class);
function createDatabaseForScheduledBackupTest(Team $team): StandalonePostgresql
{
$server = Server::factory()->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);
});
+42
View File
@@ -66,6 +66,48 @@ test('upload_to_s3 throws exception and disables s3 when storage is null', funct
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();