From d1126c02a9d88d0537566208105005de81afb210 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 13 May 2026 11:59:45 +0200 Subject: [PATCH 1/3] fix(deployments): filter generated compose service env vars Exclude generated Docker Compose SERVICE_FQDN, SERVICE_URL, and SERVICE_NAME variables from runtime, build-time, and build arg environments so stale stored values cannot override generated service names for preview deployments. --- app/Jobs/ApplicationDeploymentJob.php | 37 ++++-- ...ationDeploymentControlVarFilteringTest.php | 121 ++++++++++++++++++ 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 2e43456b8..ac24f3cc6 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -1293,12 +1293,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $sorted_environment_variables_preview = $this->application->runtime_environment_variables_preview->sortBy('id'); } if ($this->build_pack === 'dockercompose') { - $sorted_environment_variables = $sorted_environment_variables->filter(function ($env) { - return ! str($env->key)->startsWith('SERVICE_FQDN_') && ! str($env->key)->startsWith('SERVICE_URL_') && ! str($env->key)->startsWith('SERVICE_NAME_'); - }); - $sorted_environment_variables_preview = $sorted_environment_variables_preview->filter(function ($env) { - return ! str($env->key)->startsWith('SERVICE_FQDN_') && ! str($env->key)->startsWith('SERVICE_URL_') && ! str($env->key)->startsWith('SERVICE_NAME_'); - }); + $sorted_environment_variables = $sorted_environment_variables->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); + $sorted_environment_variables_preview = $sorted_environment_variables_preview->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); } $ports = $this->application->main_port(); $coolify_envs = $this->generate_coolify_env_variables(); @@ -1451,6 +1447,15 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return $envs; } + private function isGeneratedDockerComposeEnvironmentVariable(EnvironmentVariable $environmentVariable): bool + { + $key = str($environmentVariable->key); + + return $key->startsWith('SERVICE_FQDN_') + || $key->startsWith('SERVICE_URL_') + || $key->startsWith('SERVICE_NAME_'); + } + private function save_runtime_environment_variables() { // This method saves the .env file with ALL runtime variables @@ -1666,11 +1671,9 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ->orderBy($this->application->settings->is_env_sorting_enabled ? 'key' : 'id') ->get(); - // For Docker Compose, filter out SERVICE_FQDN and SERVICE_URL as we generate these + // For Docker Compose, filter out generated SERVICE_* variables as we generate these if ($this->build_pack === 'dockercompose') { - $sorted_environment_variables = $sorted_environment_variables->filter(function ($env) { - return ! str($env->key)->startsWith('SERVICE_FQDN_') && ! str($env->key)->startsWith('SERVICE_URL_'); - }); + $sorted_environment_variables = $sorted_environment_variables->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); } foreach ($sorted_environment_variables as $env) { @@ -1719,11 +1722,9 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ->orderBy($this->application->settings->is_env_sorting_enabled ? 'key' : 'id') ->get(); - // For Docker Compose, filter out SERVICE_FQDN and SERVICE_URL as we generate these with PR-specific values + // For Docker Compose, filter out generated SERVICE_* variables as we generate these with PR-specific values if ($this->build_pack === 'dockercompose') { - $sorted_environment_variables = $sorted_environment_variables->filter(function ($env) { - return ! str($env->key)->startsWith('SERVICE_FQDN_') && ! str($env->key)->startsWith('SERVICE_URL_'); - }); + $sorted_environment_variables = $sorted_environment_variables->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); } foreach ($sorted_environment_variables as $env) { @@ -3019,6 +3020,10 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->where('is_buildtime', true) ->get(); + if ($this->build_pack === 'dockercompose') { + $envs = $envs->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); + } + foreach ($envs as $env) { $resolvedValue = $env->getResolvedValueWithServer($this->mainServer); if (! is_null($resolvedValue)) { @@ -3031,6 +3036,10 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); ->where('is_buildtime', true) ->get(); + if ($this->build_pack === 'dockercompose') { + $envs = $envs->reject(fn (EnvironmentVariable $env) => $this->isGeneratedDockerComposeEnvironmentVariable($env)); + } + foreach ($envs as $env) { $resolvedValue = $env->getResolvedValueWithServer($this->mainServer); if (! is_null($resolvedValue)) { diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index d728f5ad7..83244c567 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -205,6 +205,127 @@ it('filters buildpack control vars from preview build-time env files', function expect($buildtimeEnvs->contains(fn (string $env) => str($env)->startsWith('RAILPACK_NODE_VERSION=')))->toBeFalse(); }); +it('does not let preview docker compose service names override generated build-time service names', function () { + $compose = <<<'YAML' +services: + app: + image: nginx + postgresapp: + image: postgres:16-alpine +YAML; + + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => $compose, + 'docker_compose' => $compose, + 'docker_compose_domains' => '[]', + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SERVICE_NAME_POSTGRESAPP', + 'value' => '', + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SERVICE_URL_APP', + 'value' => '', + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server, [ + 'pull_request_id' => 241, + ]); + + /** @var Collection $buildtimeEnvs */ + $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'); + $envString = $buildtimeEnvs->implode("\n"); + + expect($envString)->toContain("SERVICE_NAME_POSTGRESAPP='postgresapp-pr-241'"); + expect($envString)->not->toContain('SERVICE_NAME_POSTGRESAPP=""'); + expect($envString)->not->toContain('SERVICE_URL_APP='); +}); + +it('does not let production docker compose service names override generated build-time service names', function () { + $compose = <<<'YAML' +services: + app: + image: nginx + postgresapp: + image: postgres:16-alpine +YAML; + + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => $compose, + 'docker_compose' => $compose, + 'docker_compose_domains' => '[]', + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SERVICE_NAME_POSTGRESAPP', + 'value' => 'stale-postgresapp', + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server); + + /** @var Collection $buildtimeEnvs */ + $buildtimeEnvs = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'); + $envString = $buildtimeEnvs->implode("\n"); + + expect($envString)->toContain("SERVICE_NAME_POSTGRESAPP='postgresapp'"); + expect($envString)->not->toContain('stale-postgresapp'); +}); + +it('filters docker compose generated service variables from build args', function () { + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockercompose', + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'APP_ENV', + 'value' => 'production', + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SERVICE_NAME_POSTGRESAPP', + 'value' => '', + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + createApplicationEnvironmentVariable($application, [ + 'key' => 'SERVICE_URL_APP', + 'value' => 'https://preview.example.com', + 'is_preview' => true, + 'is_runtime' => true, + 'is_buildtime' => true, + ]); + + [$job, $reflection] = makeControlVarFilteringJob($application, $server, [ + 'pull_request_id' => 241, + ]); + + invokeDeploymentJobMethod($job, $reflection, 'generate_env_variables'); + + /** @var Collection $envArgs */ + $envArgs = readDeploymentJobProperty($job, $reflection, 'env_args'); + + expect($envArgs->get('APP_ENV'))->toBe('production'); + expect($envArgs->has('SERVICE_NAME_POSTGRESAPP'))->toBeFalse(); + expect($envArgs->has('SERVICE_URL_APP'))->toBeFalse(); +}); + it('filters buildpack control vars from preview runtime env fallback', function () { [$application, $server] = makeDeploymentControlVarFixture(); 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 2/3] 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 @@ 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 { 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 @@ 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(); } 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 @@ 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'); 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 @@ 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(); 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 3/3] 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 @@ class CreateScheduledBackup extends Component $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 @@ it('rejects an S3 storage not owned by the current team', function () { 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);