diff --git a/app/Actions/Database/StartDatabaseImport.php b/app/Actions/Database/StartDatabaseImport.php index f8c3c861ba..b4b12ec517 100644 --- a/app/Actions/Database/StartDatabaseImport.php +++ b/app/Actions/Database/StartDatabaseImport.php @@ -159,10 +159,10 @@ class StartDatabaseImport throw new DatabaseImportException('The S3 backup was not found or exceeds the 10 GiB limit.'); } $helper = "s3-restore-{$operation}"; - $this->startS3Helper($storage, $server, $helper, $network); + $resolveOptions = $this->startS3Helper($storage, $server, $helper, $network); $sourceArg = escapeshellarg("s3temp/{$storage->bucket}/{$key}"); $commandList = [ - 'docker exec '.escapeshellarg($helper).' mc cp '.$sourceArg.' /tmp/restore', + 'docker exec '.escapeshellarg($helper).' mc cp'.$resolveOptions.' '.$sourceArg.' /tmp/restore', ...$this->streamFromHelper($helper, $container, $containerPath), 'docker rm -f '.escapeshellarg($helper).' 2>/dev/null || true', ]; @@ -294,10 +294,12 @@ class StartDatabaseImport * Starts the S3 restore helper and sets its `s3temp` mc alias. The S3 keys go to mc through the * stdin of a separate SSH process, so they are not in the arguments of any process on Coolify or * on the server. The helper is removed when either step fails. + * + * @return string The mc `--resolve` option that pins the endpoint host. */ - private function startS3Helper(S3Storage $storage, Server $server, string $helper, string $network): void + private function startS3Helper(S3Storage $storage, Server $server, string $helper, string $network): string { - $image = escapeshellarg(coolifyHelperImage().':'.getHelperVersion()); + $image = coolifyHelperImage().':'.getHelperVersion(); try { instant_remote_process([ @@ -305,9 +307,12 @@ class StartDatabaseImport 'docker run -d --network '.escapeshellarg($network) .' --name '.escapeshellarg($helper) .' '.$storage->minioClientRegionOption() - .' '.$image.' sleep 86400', + .' '.escapeshellarg($image).' sleep 86400', ], $server); - $this->setMinioClientAlias($storage, $server, $helper); + $resolveOptions = $storage->minioClientResolveOptions($server, $image, $network); + $this->setMinioClientAlias($storage, $server, $helper, $resolveOptions); + + return $resolveOptions; } catch (Throwable) { instant_remote_process(['docker rm -f '.escapeshellarg($helper).' 2>/dev/null || true'], $server, throwError: false); @@ -315,10 +320,10 @@ class StartDatabaseImport } } - private function setMinioClientAlias(S3Storage $storage, Server $server, string $helper): void + private function setMinioClientAlias(S3Storage $storage, Server $server, string $helper, string $resolveOptions): void { $remoteCommand = ($server->isNonRoot() ? 'sudo ' : '').'docker exec -i '.escapeshellarg($helper) - .' mc alias set'.$storage->minioClientAliasOptions().' s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null'; + .' mc alias set'.$resolveOptions.$storage->minioClientAliasOptions().' s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null'; $timeout = (int) config('constants.ssh.command_timeout'); SshRetryHandler::retry( diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 02094a9dc2..6ffcda6ed4 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -20,7 +20,6 @@ use App\Models\Team; use App\Notifications\Database\BackupFailed; use App\Notifications\Database\BackupSuccess; use App\Notifications\Database\BackupSuccessWithS3Warning; -use App\Rules\SafeWebhookUrl; use App\Services\ScheduledJobDeliveryService; use App\Services\Security\SensitiveDataRedactor; use App\Support\BackupCompression; @@ -934,10 +933,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $escapedBackupLocation = escapeshellarg($this->backup_location); $escapedS3Destination = escapeshellarg("temporary/{$bucket}{$this->backup_dir}/"); - $resolveOptions = collect(SafeWebhookUrl::minioClientResolveOptions($endpoint, $s3->trustedInternalHosts())) - ->map(fn (string $resolveOption): string => '--resolve '.escapeshellarg($resolveOption)) - ->implode(' '); - $resolveOptions = $resolveOptions === '' ? '' : ' '.$resolveOptions; + $resolveOptions = $s3->minioClientResolveOptions($this->server, $fullImageName, $network); instant_remote_process($commands, $this->server, true, false, $this->commandTimeout(), disableMultiplexing: true); $this->setMinioClientAlias($this->server, "backup-of-{$this->backup_log_uuid}", $s3, $resolveOptions); diff --git a/app/Jobs/VolumeBackupJob.php b/app/Jobs/VolumeBackupJob.php index 42f209d1e5..90f75c8c6b 100644 --- a/app/Jobs/VolumeBackupJob.php +++ b/app/Jobs/VolumeBackupJob.php @@ -13,7 +13,6 @@ use App\Models\Server; use App\Models\VolumeBackupS3Replica; use App\Notifications\VolumeBackup\BackupFailed; use App\Notifications\VolumeBackup\BackupSuccess; -use App\Rules\SafeWebhookUrl; use App\Services\ScheduledJobDeliveryService; use App\Services\Security\SensitiveDataRedactor; use App\Support\BackupCompression; @@ -104,7 +103,10 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue $archiveScript = self::archiveScript($compressionCpuPercentage); if ($streamToS3) { $this->execution->update(['local_storage_deleted' => true]); - $archiveCommand = $this->streamToS3Command($destinations->first(), $archiveScript, $backupLocation, $containerName); + $streamS3 = $destinations->first(); + $streamS3->testConnection(shouldSave: true); + $streamResolveOptions = $streamS3->minioClientResolveOptions($server, $image); + $archiveCommand = $this->streamToS3Command($streamS3, $archiveScript, $backupLocation, $containerName, $streamResolveOptions); $this->execution->update(['s3_cleanup_pending' => true]); } else { $archiveCommand = 'docker run --rm --name '.escapeshellarg($containerName) @@ -128,7 +130,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue try { if ($streamToS3) { - $this->startStreamHelper($destinations->first(), $verifySourceCommand, $source, $containerName, $image, $server); + $this->startStreamHelper($streamS3, $verifySourceCommand, $source, $containerName, $image, $server, $streamResolveOptions); } $archiveOutput = instant_remote_process(array_filter([ $verifySourceCommand, @@ -440,7 +442,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue $s3->testConnection(shouldSave: true); $containerName = 'volume-upload-'.$this->execution->uuid; $image = coolifyHelperImage().':'.getHelperVersion(); - $resolveOptions = $this->minioClientResolveOptions($s3); + $resolveOptions = $s3->minioClientResolveOptions($server, $image); try { instant_remote_process([ @@ -467,7 +469,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue * Starts the helper container that streams the volume to S3 and sets its mc alias. The archive then * runs in it with `docker exec`, so the S3 keys are never part of a command line. */ - private function startStreamHelper(S3Storage $s3, string $verifySourceCommand, string $source, string $containerName, string $image, Server $server): void + private function startStreamHelper(S3Storage $s3, string $verifySourceCommand, string $source, string $containerName, string $image, Server $server, string $resolveOptions): void { instant_remote_process([ $verifySourceCommand, @@ -477,7 +479,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue .' -v '.escapeshellarg($source.':/volume:ro') .' '.escapeshellarg($image).' >/dev/null', ], $server, timeout: $this->timeout, disableMultiplexing: true); - $this->setMinioClientAlias($server, $containerName, $s3, $this->minioClientResolveOptions($s3)); + $this->setMinioClientAlias($server, $containerName, $s3, $resolveOptions); } /** @@ -506,19 +508,8 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue ); } - private function minioClientResolveOptions(S3Storage $s3): string + private function streamToS3Command(S3Storage $s3, string $archiveScript, string $backupLocation, string $containerName, string $resolveOptions): string { - $resolveOptions = collect(SafeWebhookUrl::minioClientResolveOptions($s3->endpoint, $s3->trustedInternalHosts())) - ->map(fn (string $option): string => '--resolve '.escapeshellarg($option)) - ->implode(' '); - - return $resolveOptions === '' ? '' : ' '.$resolveOptions; - } - - private function streamToS3Command(S3Storage $s3, string $archiveScript, string $backupLocation, string $containerName): string - { - $s3->testConnection(shouldSave: true); - $resolveOptions = $this->minioClientResolveOptions($s3); $destination = 'temporary/'.$s3->bucket.$backupLocation; // `mc pipe --quiet` panics in recent mc releases, so keep its progress bar out of the error output instead. // Each progress frame starts with "\r", so printing the last field per line keeps only the final frame and real errors. diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 84bf072dc1..eabe8f322c 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -44,6 +44,11 @@ class S3Storage extends BaseModel */ private const REMOTE_COMMAND_TIMEOUT_EXIT_CODE = 124; + /** + * How long the helper container waits for a TCP connection to one IP of the endpoint. + */ + private const RESOLVE_PROBE_TIMEOUT_SECONDS = 5; + protected $fillable = [ 'team_id', 'name', @@ -369,6 +374,43 @@ class S3Storage extends BaseModel return ' --api S3v4'.($this->usesPathStyleEndpoint() ? '' : ' --path off'); } + /** + * The mc `--resolve` option that pins the endpoint host to one of its validated IPs. mc keeps one IP per host, + * so when the host has more than one IP, a helper container on the upload network connects to each IP, IPv4 + * first, and the first IP that connects is used. A server without IPv4 then uses an IPv6 address of a + * dual-stack endpoint. When no IP connects, the first IP is used, so mc reports the real connection error. + */ + public function minioClientResolveOptions(Server $server, string $image, ?string $network = null): string + { + $entries = SafeWebhookUrl::minioClientResolveOptions($this->endpoint, $this->trustedInternalHosts()); + if ($entries === []) { + return ''; + } + + $entry = $entries[0]; + if (count($entries) > 1) { + $ips = array_map(fn (string $entry): string => str($entry)->afterLast('=')->toString(), $entries); + $port = (int) str($entries[0])->beforeLast('=')->afterLast(':')->toString(); + $probe = 'for ip in '.implode(' ', array_map('escapeshellarg', $ips)).'; do' + .' if timeout '.self::RESOLVE_PROBE_TIMEOUT_SECONDS.' bash -c "exec 3<>/dev/tcp/$ip/'.$port.'" 2>/dev/null; then echo "$ip"; exit 0; fi;' + .' done'; + $reachableIp = trim((string) instant_remote_process( + ['docker run --rm'.($network === null ? '' : ' --network '.escapeshellarg($network)) + .' '.escapeshellarg($image).' bash -c '.escapeshellarg($probe)], + $server, + throwError: false, + timeout: count($ips) * self::RESOLVE_PROBE_TIMEOUT_SECONDS + 60, + disableMultiplexing: true, + )); + $reachableIndex = array_search($reachableIp, $ips, true); + if ($reachableIndex !== false) { + $entry = $entries[$reachableIndex]; + } + } + + return ' --resolve '.escapeshellarg($entry); + } + /** * Runs an upload of a local file with the MinIO client again when it fails, for example after a short network * outage. A timeout is not retried, because another full attempt can pass the timeout of the backup job. diff --git a/app/Rules/SafeWebhookUrl.php b/app/Rules/SafeWebhookUrl.php index c872dde82b..23010dc6d5 100644 --- a/app/Rules/SafeWebhookUrl.php +++ b/app/Rules/SafeWebhookUrl.php @@ -194,7 +194,7 @@ class SafeWebhookUrl implements ValidationRule } /** - * Build mc --resolve mappings that pin the endpoint host for S3 backups. + * Build mc --resolve mappings, IPv4 first, that pin the endpoint host for S3 backups. * * @return array */ @@ -206,17 +206,17 @@ class SafeWebhookUrl implements ValidationRule return []; } - // The MinIO client keeps one --resolve IP per host and parses it with Go's - // netip.ParseAddr, which rejects bracketed IPv6. Emit a single entry, preferring - // IPv4 for reachability, without brackets. - $preferredIps = array_values(array_filter( - $target['ips'], - fn (string $ip): bool => filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false, - )); + // mc parses --resolve values with Go's netip.ParseAddr, which rejects bracketed IPv6. + // mc also keeps one IP per host, so S3Storage::minioClientResolveOptions() picks one entry. + $ips = $target['ips']; + usort($ips, fn (string $a, string $b): int => self::isIpv6($a) <=> self::isIpv6($b)); - $ip = $preferredIps[0] ?? $target['ips'][0]; + return array_map(fn (string $ip): string => sprintf('%s:%d=%s', $target['host'], $target['port'], $ip), $ips); + } - return [sprintf('%s:%d=%s', $target['host'], $target['port'], $ip)]; + private static function isIpv6(string $ip): bool + { + return filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false; } public static function redactedUrlForLog(string $url): string diff --git a/tests/Feature/S3StorageMinioClientResolveTest.php b/tests/Feature/S3StorageMinioClientResolveTest.php new file mode 100644 index 0000000000..69fe5e0967 --- /dev/null +++ b/tests/Feature/S3StorageMinioClientResolveTest.php @@ -0,0 +1,89 @@ + InstanceSettings::query()->updateOrCreate(['id' => 0], [ + 'webhook_allowed_internal_hosts' => ['localhost'], + 'webhook_allow_localhost' => true, + ])); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +/** + * @return array{0: Server, 1: S3Storage} + */ +function minioResolveTestSetup(string $endpoint, string $sshUser = 'root'): array +{ + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'user' => $sshUser, + 'ip' => '203.0.113.10', + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $storage = S3Storage::create([ + 'name' => 'Destination', + 'region' => 'us-east-1', + 'key' => 'key', + 'secret' => 'secret', + 'bucket' => 'bucket', + 'endpoint' => $endpoint, + 'team_id' => $team->id, + 'is_usable' => true, + ]); + + return [$server, $storage]; +} + +it('pins the IPv6 address of a dual-stack S3 host when only IPv6 connects from the upload network', function () { + [$server, $storage] = minioResolveTestSetup('http://localhost:9000'); + fakeRemoteProcesses(['*/dev/tcp/*' => '::1', '*' => '']); + + $options = $storage->minioClientResolveOptions($server, 'ghcr.io/coollabsio/coolify-helper:1.0.0', 'coolify'); + + expect($options)->toBe(" --resolve 'localhost:9000=::1'"); + Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "docker run --rm --network 'coolify' 'ghcr.io/coollabsio/coolify-helper:1.0.0' bash -c") + && str_contains(remoteProcessScript($process), "for ip in '\\''127.0.0.1'\\'' '\\''::1'\\''; do") + && str_contains(remoteProcessScript($process), '/dev/tcp/$ip/9000')); +}); + +it('pins the IPv4 address when it connects or when no address connects', function (string $probeOutput) { + [$server, $storage] = minioResolveTestSetup('http://localhost:9000'); + fakeRemoteProcesses(['*/dev/tcp/*' => $probeOutput, '*' => '']); + + expect($storage->minioClientResolveOptions($server, 'helper:1.0.0'))->toBe(" --resolve 'localhost:9000=127.0.0.1'"); +})->with([ + 'IPv4 connects' => '127.0.0.1', + 'no address connects' => '', + 'unexpected output' => '10.0.0.1', +]); + +it('runs the probe with sudo on servers with a non-root SSH user', function () { + [$server, $storage] = minioResolveTestSetup('http://localhost:9000', 'deploy'); + fakeRemoteProcesses(['*/dev/tcp/*' => '::1', '*' => '']); + + expect($storage->minioClientResolveOptions($server, 'helper:1.0.0'))->toBe(" --resolve 'localhost:9000=::1'"); + Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "sudo docker run --rm 'helper:1.0.0' bash -c") + && str_contains(remoteProcessScript($process), 'if timeout 5 bash -c "exec 3<>/dev/tcp/$ip/9000" 2>/dev/null; then echo "$ip"; exit 0; fi; done')); +}); + +it('does not pin or probe an S3 endpoint that is an IP address', function () { + [$server, $storage] = minioResolveTestSetup('https://8.8.8.8'); + Process::fake(); + + expect($storage->minioClientResolveOptions($server, 'helper:1.0.0'))->toBe(''); + Process::assertNothingRan(); +}); diff --git a/tests/Feature/StartDatabaseImportS3CredentialsTest.php b/tests/Feature/StartDatabaseImportS3CredentialsTest.php index 15b0041535..045d5c1db9 100644 --- a/tests/Feature/StartDatabaseImportS3CredentialsTest.php +++ b/tests/Feature/StartDatabaseImportS3CredentialsTest.php @@ -282,6 +282,43 @@ function importS3ArgvTestRunAliasProcess(array $aliasProcess): array } } +it('pins the reachable IP of a dual-stack S3 host for the restore mc commands', function () { + InstanceSettings::query()->whereKey(0)->update([ + 'webhook_allowed_internal_hosts' => ['localhost'], + 'webhook_allow_localhost' => true, + ]); + $storage = S3Storage::create([ + 'name' => 'Import S3', + 'region' => 'us-east-1', + 'key' => 'key', + 'secret' => 'secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'http://localhost:9000', + 'is_usable' => true, + 'team_id' => $this->team->id, + ]); + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->shouldReceive('exists')->once()->andReturn(true); + $disk->shouldReceive('size')->once()->andReturn(1024); + $filesystem = Mockery::mock(FilesystemManager::class, [app()])->makePartial(); + $filesystem->shouldReceive('build')->once()->andReturn($disk); + Storage::swap($filesystem); + fakeRemoteProcesses(['*/dev/tcp/*' => '::1', '*' => '']); + Queue::fake(); + + $activity = 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(remoteProcessScript($process), "docker run --rm --network 'coolify'") + && str_contains(remoteProcessScript($process), '/dev/tcp/$ip/9000')); + Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "mc alias set --resolve '\\''localhost:9000=::1'\\'' --api S3v4 s3temp")); + expect((string) RemoteProcessCommand::read($activity)) + ->toContain("mc cp --resolve 'localhost:9000=::1' ".escapeshellarg('s3temp/test-bucket/backups/restore.sql')); +}); + it('passes the S3 keys of an import to mc on stdin, not in a command line', function (string $sshUser) { [$processes, $activity, $error] = importS3ArgvTestRun($this, $sshUser); diff --git a/tests/Unit/SafeWebhookUrlTest.php b/tests/Unit/SafeWebhookUrlTest.php index 83d8c9557c..abd7e85937 100644 --- a/tests/Unit/SafeWebhookUrlTest.php +++ b/tests/Unit/SafeWebhookUrlTest.php @@ -278,17 +278,16 @@ it('builds MinIO client resolve options for S3 backup uploads', function () { expect($options)->toContain('localhost:9000=127.0.0.1'); }); -it('collapses dual-stack S3 hosts to a single bracket-free IPv4 resolve entry', function () { +it('lists every S3 host IP as a bracket-free resolve entry, IPv4 first', function () { InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate(['id' => 0], [ 'webhook_allowed_internal_hosts' => ['localhost'], 'webhook_allow_localhost' => true, ])); - // localhost resolves to both 127.0.0.1 and ::1. The MinIO client keeps one IP per - // host and rejects bracketed IPv6, so exactly one bracket-free IPv4 entry is produced. + // mc rejects bracketed IPv6, and S3Storage::minioClientResolveOptions() picks the reachable entry. $options = SafeWebhookUrl::minioClientResolveOptions('http://localhost:9000'); - expect($options)->toBe(['localhost:9000=127.0.0.1']); + expect($options)->toBe(['localhost:9000=127.0.0.1', 'localhost:9000=::1']); }); it('rejects trailing-dot hostnames to avoid DNS pinning mismatch', function () {