diff --git a/app/Actions/Database/StartDatabaseImport.php b/app/Actions/Database/StartDatabaseImport.php index 5689882c2b..daa74c98bd 100644 --- a/app/Actions/Database/StartDatabaseImport.php +++ b/app/Actions/Database/StartDatabaseImport.php @@ -301,6 +301,7 @@ class StartDatabaseImport .' -e S3_ENDPOINT='.escapeshellarg((string) $storage->endpoint) .' -e S3_ACCESS_KEY='.escapeshellarg((string) $storage->key) .' -e S3_SECRET_KEY='.escapeshellarg((string) $storage->secret) + .' '.$storage->minioClientRegionOption() .' '.$image.' sleep 86400', ], $server); } catch (Throwable) { diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index eb63bdee94..20264d61b1 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -914,7 +914,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue // Remove a leftover helper in the same batch, so a replayed batch does not fail with a name conflict. $commands[] = "docker rm -f backup-of-{$this->backup_log_uuid} >/dev/null 2>&1 || true"; $mount = escapeshellarg($this->backupMountSource().':'.$this->backup_location.':ro'); - $commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v {$mount} {$fullImageName}"; + $commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm {$s3->minioClientRegionOption()} -v {$mount} {$fullImageName}"; // Escape S3 credentials to prevent command injection $escapedEndpoint = escapeshellarg($endpoint); diff --git a/app/Jobs/VolumeBackupJob.php b/app/Jobs/VolumeBackupJob.php index 8fc34ca252..d3079fe096 100644 --- a/app/Jobs/VolumeBackupJob.php +++ b/app/Jobs/VolumeBackupJob.php @@ -437,7 +437,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue try { instant_remote_process([ 'docker rm -f '.escapeshellarg($containerName).' >/dev/null 2>&1 || true', - 'docker run -d --name '.escapeshellarg($containerName).' --rm -v ' + 'docker run -d --name '.escapeshellarg($containerName).' --rm '.$s3->minioClientRegionOption().' -v ' .escapeshellarg($backupLocation.':'.$backupLocation.':ro').' '.escapeshellarg($image), 'docker exec '.escapeshellarg($containerName).' mc alias set'.$resolveOptions.' temporary ' .escapeshellarg($s3->endpoint).' '.escapeshellarg($s3->key).' '.escapeshellarg($s3->secret), @@ -476,6 +476,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue .' | sed -n '.escapeshellarg('s/.*"size":\([0-9][0-9]*\).*/\1/p'); return 'docker run --rm --name '.escapeshellarg($containerName) + .' '.$s3->minioClientRegionOption() .' -v '.escapeshellarg($source.':/volume:ro') .' '.escapeshellarg($image) .' sh -c '.escapeshellarg($streamScript); diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 3ee7d5f269..9857b4faf2 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -273,6 +273,15 @@ class S3Storage extends BaseModel ]); } + /** + * The `docker run` option that makes the MinIO client sign requests for the storage region, as filesystem() does. + * Without it, mc signs for us-east-1, and providers that enforce the region reject the requests. + */ + public function minioClientRegionOption(): string + { + return '-e '.escapeshellarg('MC_REGION='.$this->region); + } + /** * The bundled MinIO container is a trusted internal S3 target, not a user-supplied webhook destination. * diff --git a/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php b/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php index 0191aa09d7..7b6a26bd22 100644 --- a/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php +++ b/tests/Feature/DatabaseBackupMultipleS3DestinationsTest.php @@ -14,6 +14,7 @@ use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\Team; use App\Models\User; +use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Event; @@ -82,6 +83,31 @@ function fakeMultiS3Deletes(): Collection * @return array{0: ScheduledDatabaseBackupExecution, 1: Collection} */ function runMultiS3BackupJob(array $failingStorages): array +{ + $backup = createMultiS3Backup(); + $commands = collect(); + Process::fake(function ($process) use ($commands) { + $command = is_array($process->command) ? implode(' ', $process->command) : $process->command; + $commands->push($command); + + return Process::result(output: str_contains($command, 'du -b') ? '128' : ''); + }); + + $job = Mockery::mock(DatabaseBackupJob::class.'[upload_to_s3]', [$backup])->shouldAllowMockingProtectedMethods(); + $job->shouldReceive('upload_to_s3')->andReturnUsing(function (S3Storage $storage) use ($failingStorages) { + if (in_array($storage->name, $failingStorages, true)) { + throw new RuntimeException('access denied'); + } + }); + $job->handle(); + + return [ScheduledDatabaseBackupExecution::query()->sole(), $commands]; +} + +/** + * Creates an S3-only Postgres backup schedule with the destinations "First" and "Second". + */ +function createMultiS3Backup(): ScheduledDatabaseBackup { $team = Team::factory()->create(); $server = Server::factory()->create([ @@ -107,23 +133,8 @@ function runMultiS3BackupJob(array $failingStorages): array 'team_id' => $team->id, ])->save(); $backup->syncS3Storages([$first->id, $second->id]); - $commands = collect(); - Process::fake(function ($process) use ($commands) { - $command = is_array($process->command) ? implode(' ', $process->command) : $process->command; - $commands->push($command); - return Process::result(output: str_contains($command, 'du -b') ? '128' : ''); - }); - - $job = Mockery::mock(DatabaseBackupJob::class.'[upload_to_s3]', [$backup->fresh()])->shouldAllowMockingProtectedMethods(); - $job->shouldReceive('upload_to_s3')->andReturnUsing(function (S3Storage $storage) use ($failingStorages) { - if (in_array($storage->name, $failingStorages, true)) { - throw new RuntimeException('access denied'); - } - }); - $job->handle(); - - return [ScheduledDatabaseBackupExecution::query()->sole(), $commands]; + return $backup->fresh(); } it('applies S3 retention to each destination separately', function () { @@ -301,6 +312,26 @@ describe('backup job', function () { 'failed upload' => [['First']], ]); + it('signs each upload with the region of its destination', function () { + $backup = createMultiS3Backup(); + S3Storage::query()->update(['endpoint' => 'https://8.8.8.8']); + S3Storage::query()->where('name', 'First')->update(['region' => 'ru-central-1']); + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->shouldReceive('files')->andReturn([]); + $sshDisk = Storage::disk('ssh-keys'); + Storage::shouldReceive('disk')->with('ssh-keys')->andReturn($sshDisk); + Storage::shouldReceive('build')->andReturn($disk); + Process::fake(fn ($process) => Process::result(output: str_contains($process->command, 'du -b') ? '128' : '')); + + (new DatabaseBackupJob($backup))->handle(); + + expect(ScheduledDatabaseBackupExecution::query()->sole()->s3_uploaded)->toBeTrue(); + Process::assertRan(fn ($process) => str_contains($process->command, "-e 'MC_REGION=ru-central-1'") + && str_contains($process->command, 'temporary/first-bucket/')); + Process::assertRan(fn ($process) => str_contains($process->command, "-e 'MC_REGION=us-east-1'") + && str_contains($process->command, 'temporary/second-bucket/')); + }); + it('keeps the local file and names the failed destination when one upload fails', function () { [$execution, $commands] = runMultiS3BackupJob(['Second']); diff --git a/tests/Feature/StartDatabaseImportS3CredentialsTest.php b/tests/Feature/StartDatabaseImportS3CredentialsTest.php index 8a45132d69..b66750e6f7 100644 --- a/tests/Feature/StartDatabaseImportS3CredentialsTest.php +++ b/tests/Feature/StartDatabaseImportS3CredentialsTest.php @@ -157,3 +157,35 @@ test('s3 import activity stores the cleanup names and paths but no credential va Queue::assertPushed(CoolifyTask::class, fn (CoolifyTask $job) => $job->call_event_data === $stored); }); + +test('s3 import helper signs requests with the storage region', function () { + $storage = S3Storage::create([ + 'name' => 'Import S3', + 'region' => 'ru-central-1', + 'key' => 'key', + 'secret' => 'secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'https://8.8.8.8', + 'is_usable' => true, + 'team_id' => $this->team->id, + ]); + + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->shouldReceive('exists')->andReturn(true); + $disk->shouldReceive('size')->andReturn(1024); + $filesystem = Mockery::mock(FilesystemManager::class, [app()])->makePartial(); + $filesystem->shouldReceive('build')->andReturn($disk); + Storage::swap($filesystem); + + Process::fake(); + Queue::fake(); + + app(StartDatabaseImport::class)->handle( + $this->database, + new DatabaseImportSource('s3', path: 'backups/restore.sql', s3StorageUuid: $storage->uuid), + $this->team->id, + ); + + Process::assertRan(fn ($process) => str_contains($process->command, "-e 'MC_REGION=ru-central-1'") + && str_contains($process->command, '-e S3_ENDPOINT=')); +}); diff --git a/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php b/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php index 3eadab443a..4f95a3fe66 100644 --- a/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php +++ b/tests/Feature/VolumeBackupMultipleS3DestinationsTest.php @@ -116,6 +116,23 @@ it('archives locally and uploads to every destination instead of streaming', fun ]); }); +it('signs each upload with the region of its destination', function () { + [$backup, $first] = createMultiDestinationVolumeBackup(); + $first->update(['region' => 'ru-central-1']); + fakeMultiDestinationS3Disks(); + Process::fake([ + '*du -b*' => '128', + '*' => '', + ]); + + (new VolumeBackupJob($backup))->handle(); + + Process::assertRan(fn ($process) => str_contains($process->command, "-e 'MC_REGION=ru-central-1'") + && str_contains($process->command, 'temporary/first-bucket/')); + Process::assertRan(fn ($process) => str_contains($process->command, "-e 'MC_REGION=us-east-1'") + && str_contains($process->command, 'temporary/second-bucket/')); +}); + it('keeps the local archive and cleans the partial upload when one destination fails', function () { [$backup, $first, $second] = createMultiDestinationVolumeBackup(['disable_local_backup' => true]); fakeMultiDestinationS3Disks(); diff --git a/tests/Feature/VolumeBackupTest.php b/tests/Feature/VolumeBackupTest.php index 47450f2151..9b82d22785 100644 --- a/tests/Feature/VolumeBackupTest.php +++ b/tests/Feature/VolumeBackupTest.php @@ -1976,7 +1976,7 @@ it('streams S3-only volume backups with Content-MD5 on Object Lock buckets and w [$application, $volume] = createVolumeBackupApplication($team); $s3Storage = S3Storage::create([ 'name' => 'Streaming destination', - 'region' => 'us-east-1', + 'region' => 'ru-central-1', 'key' => 'key', 'secret' => 'secret', 'bucket' => 'bucket', @@ -2008,6 +2008,7 @@ it('streams S3-only volume backups with Content-MD5 on Object Lock buckets and w $expectedFilename = 'volume-app-data-1786795200.tar.gz'; Process::assertRan(fn ($process) => str_contains($process->command, 'tar -I') && str_contains($process->command, 'mc pipe') + && str_contains($process->command, "-e 'MC_REGION=ru-central-1'") && ! str_contains($process->command, '--quiet') && str_contains($process->command, 'lock=$(mc retention info --default --json --resolve') && str_contains($process->command, '*) checksum=--checksum=MD5 ;; *) checksum= ;; esac && (compressor=')