diff --git a/app/Helpers/SshMultiplexingHelper.php b/app/Helpers/SshMultiplexingHelper.php index 2138c5d131..77738670b2 100644 --- a/app/Helpers/SshMultiplexingHelper.php +++ b/app/Helpers/SshMultiplexingHelper.php @@ -69,7 +69,8 @@ class SshMultiplexingHelper */ private static function assertNotDisabled(Server $server): void { - if ($server->settings->force_disabled) { + // Read the saved flag too: a job that loaded the server before a transfer finished must stop. + if ($server->settings->force_disabled || ($server->exists && $server->settings()->value('force_disabled'))) { throw new \RuntimeException('Server is disabled.'); } } @@ -84,7 +85,7 @@ class SshMultiplexingHelper $serverInterval = config('constants.ssh.server_interval'); $muxPersistTime = config('constants.ssh.mux_persist_time'); - $establishCommand = "ssh -fN -o ControlMaster=auto -o ControlPath=$muxSocket -o ControlPersist={$muxPersistTime} "; + $establishCommand = 'ssh -fN -o ControlMaster=auto -o ControlPath='.escapeshellarg($muxSocket)." -o ControlPersist={$muxPersistTime} "; if (data_get($server, 'settings.is_cloudflare_tunnel')) { $establishCommand .= ' -o ProxyCommand="cloudflared access ssh --hostname %h" '; @@ -301,7 +302,7 @@ class SshMultiplexingHelper $muxSocket = $sshConfig['muxFilename']; $healthCheckTimeout = config('constants.ssh.mux_health_check_timeout'); - $healthCommand = "timeout $healthCheckTimeout ssh -o ControlMaster=auto -o ControlPath=$muxSocket "; + $healthCommand = "timeout $healthCheckTimeout ssh -o ControlMaster=auto -o ControlPath=".escapeshellarg($muxSocket).' '; if (data_get($server, 'settings.is_cloudflare_tunnel')) { $healthCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" '; } @@ -366,7 +367,7 @@ class SshMultiplexingHelper private static function muxControlCommand(Server $server, string $operation): string { - $command = "ssh -O {$operation} -o ControlPath=".self::muxSocket($server).' '; + $command = "ssh -O {$operation} -o ControlPath=".escapeshellarg(self::muxSocket($server)).' '; if (data_get($server, 'settings.is_cloudflare_tunnel')) { $command .= '-o ProxyCommand="cloudflared access ssh --hostname %h" '; } @@ -377,7 +378,7 @@ class SshMultiplexingHelper private static function multiplexingOptions(Server $server): string { return '-o ControlMaster=auto ' - .'-o ControlPath='.self::muxSocket($server).' ' + .'-o ControlPath='.escapeshellarg(self::muxSocket($server)).' ' .'-o ControlPersist='.config('constants.ssh.mux_persist_time').' '; } @@ -433,7 +434,7 @@ class SshMultiplexingHelper private static function getCommonSshOptions(Server $server, string $sshKeyLocation, int $connectionTimeout, int $serverInterval, bool $isScp = false): string { - $options = "-i {$sshKeyLocation} " + $options = '-i '.escapeshellarg($sshKeyLocation).' ' .'-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null ' .'-o PasswordAuthentication=no ' ."-o ConnectTimeout=$connectionTimeout " diff --git a/app/Http/Controllers/Api/ServerTransferController.php b/app/Http/Controllers/Api/ServerTransferController.php index 6cbdde8d1b..a818ae26cc 100644 --- a/app/Http/Controllers/Api/ServerTransferController.php +++ b/app/Http/Controllers/Api/ServerTransferController.php @@ -138,7 +138,7 @@ class ServerTransferController extends Controller new OA\Response(response: 404, ref: '#/components/responses/404'), ] )] - public function status(string $uuid): JsonResponse + public function status(Request $request, string $uuid): JsonResponse { $teamId = getTeamIdFromToken(); if (is_null($teamId)) { @@ -152,12 +152,18 @@ class ServerTransferController extends Controller $this->authorize('view', $server); + // The log and the error of a transfer can name hosts and credentials problems, so members see only the state. + $migration = data_get($server->server_metadata, 'migration'); + if (is_array($migration) && ! $request->user()->can('update', $server)) { + $migration = collect($migration)->except(['log', 'error'])->all(); + } + return response()->json([ 'server_uuid' => $server->uuid, 'transfer' => collect((array) data_get($server->server_metadata, 'transfer', [])) ->only(['status', 'export_id', 'imported_at', 'claimed_at', 'transferred_at', 'target_instance_url']) ->all(), - 'migration' => data_get($server->server_metadata, 'migration'), + 'migration' => $migration, ]); } @@ -222,10 +228,16 @@ class ServerTransferController extends Controller // server would leave an imported server here that this instance cannot manage. // On Cloud the probe is rate limited and cannot reach private, loopback, or link-local addresses, // so users cannot scan ports of Cloud's own network. + // A host name is resolved once and the probe connects to that address, so DNS cannot change it in between. Server::throttleValidation($teamId, $ip); - $probeHost = filter_var(Server::normalizeIp($ip), FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? '['.Server::normalizeIp($ip).']' : $ip; - $probeAllowed = ! isCloud() || Validator::make(['ip' => "http://{$probeHost}:{$port}"], ['ip' => [new SafeWebhookUrl]])->passes(); - $connection = $probeAllowed ? @fsockopen($ip, $port, timeout: 5) : false; + $probeIp = Server::normalizeIp($ip); + if (isCloud() && ! filter_var($probeIp, FILTER_VALIDATE_IP)) { + $probeIp = gethostbyname($probeIp); + } + $probeHost = filter_var($probeIp, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? "[{$probeIp}]" : $probeIp; + $probeAllowed = ! isCloud() || (filter_var($probeIp, FILTER_VALIDATE_IP) + && Validator::make(['ip' => "http://{$probeHost}:{$port}"], ['ip' => [new SafeWebhookUrl]])->passes()); + $connection = $probeAllowed ? @fsockopen($probeHost, $port, timeout: 5) : false; if ($connection !== false) { fclose($connection); } @@ -245,7 +257,7 @@ class ServerTransferController extends Controller 'ok' => $connection !== false, 'message' => match (true) { $connection !== false => null, - ! $probeAllowed => "This instance cannot connect to a private, loopback, or link-local address ({$ip}). Use the public IP address of the server.", + ! $probeAllowed => "This instance cannot connect to {$ip}: it is a private, loopback, or link-local address, or it does not resolve to a public IPv4 address. Use the public IP address of the server.", default => "This instance cannot reach the server at {$ip}:{$port}. Allow SSH from this instance to the server, then transfer again.", }, ], @@ -399,11 +411,23 @@ class ServerTransferController extends Controller ], 422); } + // The UI prefills this URL for a transfer back, which sends a team token to it. + $targetInstanceUrl = $request->input('target_instance_url'); + if (filled($targetInstanceUrl)) { + try { + $targetInstanceUrl = ServerTransferMigrator::validateInstanceUrl($targetInstanceUrl); + } catch (ValidationException $e) { + return response()->json(['message' => 'Validation failed.', 'errors' => ['target_instance_url' => $e->validator->errors()->all()]], 422); + } catch (Throwable $e) { + return response()->json(['message' => 'Validation failed.', 'errors' => ['target_instance_url' => [$e->getMessage()]]], 422); + } + } + try { $result = $this->claimer->markTransferred( $server, exportId: $request->input('export_id'), - targetInstanceUrl: $request->input('target_instance_url'), + targetInstanceUrl: filled($targetInstanceUrl) ? $targetInstanceUrl : null, ); } catch (Throwable $e) { return response()->json(['message' => $e->getMessage()], 422); diff --git a/app/Http/Controllers/Api/ServersController.php b/app/Http/Controllers/Api/ServersController.php index 9be3a088b0..557843f396 100644 --- a/app/Http/Controllers/Api/ServersController.php +++ b/app/Http/Controllers/Api/ServersController.php @@ -53,6 +53,27 @@ class ServersController extends Controller ->put('is_build_server', $settings->effectiveServerRole() === ServerRole::BUILD); } + /** + * Another instance manages a transferred server. Changing its host or credentials would let + * "Enable management" connect to a different host, so these fields stay as they are. + * + * @return array> + */ + private function transferredServerConnectionChanges(Request $request, ModelsServer $server): array + { + $changed = [ + 'ip' => $request->filled('ip') && ModelsServer::normalizeIp($request->input('ip')) !== ModelsServer::normalizeIp($server->getRawOriginal('ip')), + 'user' => $request->filled('user') && (string) $request->input('user') !== (string) $server->user, + 'port' => $request->filled('port') && (int) $request->input('port') !== (int) $server->port, + 'private_key_uuid' => $request->filled('private_key_uuid') && $request->input('private_key_uuid') !== $server->privateKey?->uuid, + ]; + + return collect($changed) + ->filter() + ->map(fn () => ['This field cannot be changed on a transferred server.']) + ->all(); + } + /** * Rejects requests where the deprecated is_build_server flag disagrees with server_role. */ @@ -775,6 +796,12 @@ class ServersController extends Controller return response()->json(['message' => 'Server not found.'], 404); } $this->authorize('update', $server); + if ($server->isTransferredAway() && ($connectionErrors = $this->transferredServerConnectionChanges($request, $server))) { + return response()->json([ + 'message' => 'Validation failed.', + 'errors' => $connectionErrors, + ], 422); + } if ($request->instant_validate) { ModelsServer::throttleValidation($teamId, $request->ip ?? $server->ip); } diff --git a/app/Jobs/CleanupStaleMultiplexedConnections.php b/app/Jobs/CleanupStaleMultiplexedConnections.php index 3162ae9c41..b03b3eab49 100644 --- a/app/Jobs/CleanupStaleMultiplexedConnections.php +++ b/app/Jobs/CleanupStaleMultiplexedConnections.php @@ -227,7 +227,7 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue } $muxSocket = "/var/www/html/storage/app/ssh/mux/{$muxFile}"; - $checkCommand = "ssh -O check -o ControlPath={$muxSocket} {$server->user}@{$server->ip} 2>/dev/null"; + $checkCommand = 'ssh -O check -o ControlPath='.escapeshellarg($muxSocket).' '.escapeshellarg($server->user).'@'.escapeshellarg($server->ip).' 2>/dev/null'; $checkProcess = Process::run($checkCommand); if ($checkProcess->exitCode() !== 0) { @@ -272,7 +272,7 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue } $muxSocket = "/var/www/html/storage/app/ssh/mux/{$muxFile}"; - $checkProcess = Process::run("ssh -O check -o ControlPath={$muxSocket} localhost"); + $checkProcess = Process::run('ssh -O check -o ControlPath='.escapeshellarg($muxSocket).' localhost'); $pid = preg_match('/pid=(\d+)/', $checkProcess->output().$checkProcess->errorOutput(), $matches) ? $matches[1] : null; @@ -281,7 +281,7 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue SshMultiplexingHelper::markMuxProcessAsRetiring($pid, $muxSocket); } - $closeCommand = "ssh -O stop -o ControlPath={$muxSocket} localhost 2>/dev/null"; + $closeCommand = 'ssh -O stop -o ControlPath='.escapeshellarg($muxSocket).' localhost 2>/dev/null'; $stopProcess = Process::run($closeCommand); if ($pid !== null && ! $stopProcess->successful()) { SshMultiplexingHelper::unmarkMuxProcessAsRetiring($pid, $muxSocket); diff --git a/app/Listeners/CloudflareTunnelChangedNotification.php b/app/Listeners/CloudflareTunnelChangedNotification.php index 3cc44c2f82..c1804f0955 100644 --- a/app/Listeners/CloudflareTunnelChangedNotification.php +++ b/app/Listeners/CloudflareTunnelChangedNotification.php @@ -19,6 +19,9 @@ class CloudflareTunnelChangedNotification $ssh_domain = data_get($event, 'data.ssh_domain'); $this->server = Server::where('id', $server_id)->firstOrFail(); + if ($this->server->isTransferredAway()) { + return; + } // Check if cloudflare tunnel is running (container is healthy) - try 3 times with 5 second intervals $cloudflareHealthy = false; diff --git a/app/Livewire/Server/CloudflareTunnel.php b/app/Livewire/Server/CloudflareTunnel.php index b40b6b5628..29d52b5a2c 100644 --- a/app/Livewire/Server/CloudflareTunnel.php +++ b/app/Livewire/Server/CloudflareTunnel.php @@ -115,6 +115,7 @@ class CloudflareTunnel extends Component { try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $this->isCloudflareTunnelsEnabled = true; $this->server->settings->is_cloudflare_tunnel = true; $this->server->settings->save(); @@ -130,6 +131,7 @@ class CloudflareTunnel extends Component { try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $this->ssh_domain = $this->normalizeSshDomain($this->ssh_domain); $activity = ConfigureCloudflared::run($this->server, $this->cloudflare_token, $this->ssh_domain); auditLog('ui.server.cloudflare_tunnel.configuration_started', $this->auditContext([ diff --git a/app/Livewire/Server/Delete.php b/app/Livewire/Server/Delete.php index 2b5051c65e..ce9313a9f9 100644 --- a/app/Livewire/Server/Delete.php +++ b/app/Livewire/Server/Delete.php @@ -70,11 +70,16 @@ class Delete extends Component return; } + $resources = $this->server->definedResources(); if ($isArchive) { - app(ServerTransferClaimer::class)->forgetDnsRecords($this->server); + $claimer = app(ServerTransferClaimer::class); + $claimer->forgetDnsRecords($this->server); + // Applications that use the server only as an additional destination stay; they lose this server. + $claimer->detachAdditionalApplications($this->server); + $resources = $claimer->archiveResources($this->server); } if ($this->force_delete_resources) { - foreach ($this->server->definedResources() as $resource) { + foreach ($resources as $resource) { DeleteResourceJob::dispatch($resource, deleteFromCoolifyOnly: $isArchive); } } diff --git a/app/Livewire/Server/PrivateKey/Show.php b/app/Livewire/Server/PrivateKey/Show.php index 0673ac2aad..72b9d31b02 100644 --- a/app/Livewire/Server/PrivateKey/Show.php +++ b/app/Livewire/Server/PrivateKey/Show.php @@ -46,6 +46,11 @@ class Show extends Component return; } $this->authorize('update', $this->server); + if ($this->server->isTransferredAway()) { + $this->dispatch('error', 'The private key of a transferred server cannot be changed.'); + + return; + } try { Server::throttleValidation($this->server->team_id, $this->server->ip); } catch (ThrottleRequestsException $e) { @@ -101,6 +106,7 @@ class Show extends Component public function checkConnection() { try { + $this->authorize('update', $this->server); Server::throttleValidation($this->server->team_id, $this->server->ip); ['uptime' => $uptime, 'error' => $error] = $this->server->validateConnection(); if ($uptime) { diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index 42facdc9f9..796ddcc82c 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -7,6 +7,7 @@ use App\Enums\ServerRole; use App\Events\ServerReachabilityChanged; use App\Models\CloudProviderToken; use App\Models\Server; +use App\Models\Team; use App\Rules\ValidServerIp; use App\Services\DigitalOceanService; use App\Services\HetznerService; @@ -251,6 +252,12 @@ class Show extends Component if ($toModel) { $this->validate(); $this->ip = Server::normalizeIp($this->ip); + if ($this->server->isTransferredAway() && $this->connectionFieldsChanged()) { + $this->ip = $this->server->ip; + $this->user = $this->server->user; + $this->port = $this->server->port; + throw new \Exception('The IP address, SSH user, and SSH port of a transferred server cannot be changed.'); + } // Check only a changed IP, so that an old duplicate does not block saving other fields. $ipChanged = $this->ip !== Server::normalizeIp($this->server->getRawOriginal('ip')); $foundServer = $ipChanged ? Server::where('ip', $this->ip) @@ -322,6 +329,13 @@ class Show extends Component } } + private function connectionFieldsChanged(): bool + { + return $this->ip !== Server::normalizeIp($this->server->getRawOriginal('ip')) + || (string) $this->user !== (string) $this->server->user + || (int) $this->port !== (int) $this->server->port; + } + /** * Server columns are audited by the model. Settings live on ServerSetting, so record their names here. * @@ -406,13 +420,16 @@ class Show extends Component return; } - if ($this->server->team->serverOverflow()) { + // The archive does not use a server slot, so taking it back needs a free slot. + if (Team::serverLimitReached($this->server->team)) { $this->dispatch('error', 'Your team is over its server limit. Upgrade your subscription or remove a server first.'); return; } try { + // The claim connects to the host over SSH, so it uses the same Cloud limit as a validation. + Server::throttleValidation($this->server->team_id, $this->server->ip); $claimer->claim($this->server); } catch (\Throwable $e) { handleError($e, $this); @@ -595,6 +612,9 @@ class Show extends Component { try { $this->authorize('view', $this->server); + if ($this->server->isTransferredAway()) { + return; + } if (! $this->server->hetzner_server_id || ! $this->server->cloudProviderToken) { $this->dispatch('error', 'This server is not associated with a Hetzner Cloud server or token.'); @@ -644,6 +664,9 @@ class Show extends Component { try { $this->authorize('view', $this->server); + if ($this->server->isTransferredAway()) { + return; + } if (! $this->server->vultr_instance_id || ! $this->server->cloudProviderToken) { $this->dispatch('error', 'This server is not associated with a Vultr instance or token.'); @@ -666,6 +689,9 @@ class Show extends Component { try { $this->authorize('view', $this->server); + if ($this->server->isTransferredAway()) { + return; + } if (! $this->server->digitalocean_droplet_id || ! $this->server->cloudProviderToken) { $this->dispatch('error', 'This server is not associated with a DigitalOcean droplet or token.'); @@ -688,6 +714,9 @@ class Show extends Component { try { $this->authorize('view', $this->server); + if ($this->server->isTransferredAway()) { + return null; + } if (! $this->server->hostinger_virtual_machine_id || ! $this->server->cloudProviderToken) { $this->dispatch('error', 'This server is not associated with a Hostinger VPS or token.'); @@ -918,6 +947,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHetznerTokens->firstWhere('id', $this->selectedHetznerTokenId); if (! $token) { @@ -959,6 +989,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHetznerTokens->firstWhere('id', $this->selectedHetznerTokenId); if (! $token) { @@ -990,6 +1021,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHetznerTokens->firstWhere('id', $this->selectedHetznerTokenId); if (! $token) { @@ -1046,6 +1078,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableDigitalOceanTokens->firstWhere('id', $this->selectedDigitalOceanTokenId); if (! $token) { @@ -1087,6 +1120,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableDigitalOceanTokens->firstWhere('id', $this->selectedDigitalOceanTokenId); if (! $token) { @@ -1118,6 +1152,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableDigitalOceanTokens->firstWhere('id', $this->selectedDigitalOceanTokenId); if (! $token) { @@ -1177,6 +1212,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHostingerTokens->firstWhere('id', $this->selectedHostingerTokenId); if (! $token) { @@ -1217,6 +1253,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHostingerTokens->firstWhere('id', $this->selectedHostingerTokenId); if (! $token) { @@ -1248,6 +1285,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableHostingerTokens->firstWhere('id', $this->selectedHostingerTokenId); if (! $token) { @@ -1307,6 +1345,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableVultrTokens->firstWhere('id', $this->selectedVultrTokenId); if (! $token) { @@ -1348,6 +1387,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableVultrTokens->firstWhere('id', $this->selectedVultrTokenId); if (! $token) { @@ -1379,6 +1419,7 @@ class Show extends Component try { $this->authorize('update', $this->server); + $this->server->assertNotTransferredAway(); $token = $this->availableVultrTokens->firstWhere('id', $this->selectedVultrTokenId); if (! $token) { diff --git a/app/Livewire/Server/Transfer.php b/app/Livewire/Server/Transfer.php index 22bb6d0685..fb0a0c2af3 100644 --- a/app/Livewire/Server/Transfer.php +++ b/app/Livewire/Server/Transfer.php @@ -38,7 +38,12 @@ class Transfer extends Component // The transfer exports every credential of the server, so only admins and owners may open it. $this->authorize('update', $this->server); $this->exportId = data_get($this->server->server_metadata, 'transfer.export_id'); - $this->managerUrl = (string) data_get($this->server->server_metadata, 'transfer.target_instance_url', ''); + // Transfer back sends a new team token to this URL, so it is prefilled only when this instance chose it. + $managerUrl = data_get($this->server->server_metadata, 'transfer.target_instance_url'); + if (is_string($managerUrl) && ! data_get($this->server->server_metadata, 'transfer.claimed_by_marker') + && ServerTransferMigrator::isValidInstanceUrl($managerUrl)) { + $this->managerUrl = $managerUrl; + } $this->thisInstanceUrl = rtrim((string) (instanceSettings()->fqdn ?: config('app.url')), '/'); } catch (Throwable $e) { handleError($e, $this); @@ -77,8 +82,19 @@ class Transfer extends Component return array_values(array_filter((array) data_get($this->migration, 'checklist', []), 'is_string')); } + /** + * The link to the server on the target. The target sends it, so only an https link is shown. + */ + public function getTargetServerUrlProperty(): ?string + { + $url = data_get($this->migration, 'target_server_url'); + + return ServerTransferMigrator::isLinkableUrl($url) ? $url : null; + } + public function refreshMigration(): void { + $this->authorize('update', $this->server); $this->server->refresh(); $this->lastWarnings = array_values((array) data_get($this->migration, 'warnings', $this->lastWarnings)); } @@ -105,12 +121,14 @@ class Transfer extends Component $this->server->refresh(); $this->lastWarnings = []; - $this->targetToken = ''; $this->dispatch('success', 'All checks passed. The transfer started.'); } catch (Throwable $e) { // A failed check is saved in the transfer log, so show it. $this->server->refresh(); handleError($e, $this); + } finally { + // Public properties are sent to the browser with every response. + $this->targetToken = ''; } } @@ -127,10 +145,11 @@ class Transfer extends Component ]); $this->server->refresh(); - $this->managerToken = ''; $this->dispatch('success', 'The other instance started the transfer back.'); } catch (Throwable $e) { handleError($e, $this); + } finally { + $this->managerToken = ''; } } diff --git a/app/Models/Server.php b/app/Models/Server.php index b8b56389ec..f07f881f03 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -413,6 +413,10 @@ class Server extends BaseModel */ public function backfillPlaceholderIp(?string $ip): bool { + if ($this->isTransferredAway()) { + return false; + } + $ip = blank($ip) ? null : self::normalizeIp($ip); if (self::isPlaceholderIp($ip)) { return false; @@ -457,6 +461,11 @@ class Server extends BaseModel public function refreshHetznerState(): ?string { + // Another instance manages the cloud server of an archive; keep the stored status. + if ($this->isTransferredAway()) { + return $this->hetzner_server_status; + } + if (! $this->hetzner_server_id || ! $this->cloudProviderToken || $this->cloudProviderToken->provider !== 'hetzner') { return $this->hetzner_server_status; } @@ -478,6 +487,11 @@ class Server extends BaseModel public function refreshVultrState(): ?string { + // Another instance manages the cloud server of an archive; keep the stored status. + if ($this->isTransferredAway()) { + return $this->vultr_instance_status; + } + if (! $this->vultr_instance_id || ! $this->cloudProviderToken) { return null; } @@ -514,6 +528,11 @@ class Server extends BaseModel public function refreshDigitalOceanState(): ?string { + // Another instance manages the cloud server of an archive; keep the stored status. + if ($this->isTransferredAway()) { + return $this->digitalocean_droplet_status; + } + if (! $this->digitalocean_droplet_id || ! $this->cloudProviderToken || $this->cloudProviderToken->provider !== 'digitalocean') { return $this->digitalocean_droplet_status; } @@ -555,6 +574,11 @@ class Server extends BaseModel public function refreshHostingerState(): ?string { + // Another instance manages the cloud server of an archive; keep the stored status. + if ($this->isTransferredAway()) { + return $this->hostinger_virtual_machine_status; + } + if (! $this->hostinger_virtual_machine_id || ! $this->cloudProviderToken || $this->cloudProviderToken->provider !== 'hostinger') { return $this->hostinger_virtual_machine_status; } diff --git a/app/Services/ProjectIconStorageService.php b/app/Services/ProjectIconStorageService.php index 0c6d314816..2a0b3dba51 100644 --- a/app/Services/ProjectIconStorageService.php +++ b/app/Services/ProjectIconStorageService.php @@ -22,6 +22,10 @@ class ProjectIconStorageService extends AvatarStorageService $storageType = $settings->avatar_storage_type === 's3' && $settings->avatar_s3_storage_id ? 's3' : 'local'; $s3StorageId = $storageType === 's3' ? $settings->avatar_s3_storage_id : null; $disk = $this->disk($storageType, $s3StorageId); + // The uuid is a path segment, so it must not leave the project-icons directory. + if (preg_match('/\A[a-zA-Z0-9_-]+\z/', (string) $project->uuid) !== 1) { + throw new RuntimeException('Unable to store the project icon: the project UUID is not valid.'); + } $path = "project-icons/{$project->uuid}/icon.jpg"; if (! $disk->put($path, $contents)) { diff --git a/app/Services/ServerTransfer/ServerTransferBundle.php b/app/Services/ServerTransfer/ServerTransferBundle.php index aa5936d14e..349afa96d0 100644 --- a/app/Services/ServerTransfer/ServerTransferBundle.php +++ b/app/Services/ServerTransfer/ServerTransferBundle.php @@ -2,12 +2,60 @@ namespace App\Services\ServerTransfer; +use App\Support\ValidationPatterns; +use Closure; +use Illuminate\Support\Facades\Validator; use Illuminate\Validation\ValidationException; +use Throwable; class ServerTransferBundle { public const SCHEMA_VERSION = 1; + /** + * Every `uuid`, `*_uuid`, and `*_uuids` value in a bundle. Coolify ids are random lowercase + * letters and digits (new_public_id) or RFC 4122 UUIDs. The ids go into file paths and local + * shell commands (SSH key file, SSH mux socket, project icon directory). + */ + public const ID_PATTERN = '/\A[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}\z/'; + + /** + * The most items a bundle can have for each list key, summed over the whole bundle. + * + * @var array + */ + public const MAX_ITEMS = [ + 'projects' => 1000, + 'environments' => 5000, + 'applications' => 5000, + 'databases' => 5000, + 'services' => 2000, + 'previews' => 10000, + 'environment_variables' => 50000, + 'environment_variables_preview' => 50000, + 'shared_environment_variables' => 20000, + 'persistent_storages' => 20000, + 'file_storages' => 20000, + 'scheduled_tasks' => 10000, + 'scheduled_backups' => 10000, + 'volume_backups' => 10000, + 'tags' => 20000, + 'resource_links' => 10000, + 'destinations' => 1000, + 'private_keys' => 1000, + 'github_apps' => 1000, + 'gitlab_apps' => 1000, + 'gitea_apps' => 1000, + 's3_storages' => 1000, + 'cloud_provider_tokens' => 1000, + 'integration_tokens' => 1000, + 'dns_zones' => 1000, + 'managed_dns_records' => 10000, + 'ssl_certificates' => 10000, + 'warnings' => 1000, + 'post_transfer_checklist' => 1000, + ]; + /** * @param array $payload */ @@ -76,6 +124,8 @@ class ServerTransferBundle $errors[] = 'Missing projects array.'; } + $errors = array_merge($errors, self::contentErrors($bundle)); + return [ 'valid' => $errors === [], 'errors' => $errors, @@ -95,4 +145,272 @@ class ServerTransferBundle ]); } } + + /** + * Checks the values of a bundle before the import writes anything. A bundle comes from the + * API, so each value gets the validation that the normal API applies to it. + * + * @param array $bundle + * @return list + */ + public static function contentErrors(array $bundle): array + { + $errors = []; + $counts = []; + self::walk($bundle, '', $errors, $counts); + + foreach ($counts as $key => $count) { + if ($count > self::MAX_ITEMS[$key]) { + $errors[] = "The bundle has {$count} {$key}. A bundle can have at most ".self::MAX_ITEMS[$key]." {$key}."; + } + } + + // Stop before the field checks: a bundle with a bad id or too many items is not imported. + if ($errors !== []) { + return $errors; + } + + foreach (self::items(data_get($bundle, 'destinations')) as $i => $destination) { + self::check($errors, "destinations.{$i}.network", $destination['network'] ?? null, ['nullable', 'string', 'max:255', 'regex:'.ValidationPatterns::DOCKER_NETWORK_PATTERN]); + } + + // Only http and https URLs, as for a git source made with the API. The import does not resolve the host: + // the target may not reach the git host yet, and SafeExternalUrl checks the host on every request. + $gitSourceUrl = function (string $attribute, mixed $value, Closure $fail): void { + if (is_string($value) && ! isValidDomainUrl($value)) { + $fail('The :attribute must use the http or https scheme.'); + } + }; + foreach (['github_apps', 'gitlab_apps', 'gitea_apps'] as $key) { + foreach (self::items(data_get($bundle, $key)) as $i => $source) { + foreach (['html_url', 'api_url'] as $field) { + self::check($errors, "{$key}.{$i}.{$field}", $source[$field] ?? null, ['nullable', 'string', 'url', $gitSourceUrl]); + } + } + } + + foreach (self::items(data_get($bundle, 'ssl_certificates')) as $i => $certificate) { + self::checkPath($errors, "ssl_certificates.{$i}.configuration_dir", $certificate['configuration_dir'] ?? null); + self::checkPath($errors, "ssl_certificates.{$i}.mount_path", $certificate['mount_path'] ?? null); + } + + foreach (self::items(data_get($bundle, 'volume_backups')) as $i => $backup) { + self::checkCron($errors, "volume_backups.{$i}.frequency", $backup['frequency'] ?? null); + } + + foreach (self::items(data_get($bundle, 'projects')) as $p => $project) { + foreach (self::items($project['environments'] ?? null) as $e => $environment) { + $at = "projects.{$p}.environments.{$e}"; + + foreach (self::items($environment['applications'] ?? null) as $a => $application) { + self::checkDomains($errors, "{$at}.applications.{$a}.fqdn", data_get($application, 'attributes.fqdn')); + self::checkStorages($errors, "{$at}.applications.{$a}", $application); + self::checkScheduledTasks($errors, "{$at}.applications.{$a}", $application); + foreach (self::items($application['previews'] ?? null) as $r => $preview) { + self::checkDomains($errors, "{$at}.applications.{$a}.previews.{$r}.fqdn", $preview['fqdn'] ?? null); + self::checkStorages($errors, "{$at}.applications.{$a}.previews.{$r}", $preview); + } + } + + foreach (self::items($environment['databases'] ?? null) as $d => $database) { + self::checkDatabaseAttributes($errors, "{$at}.databases.{$d}", (array) ($database['attributes'] ?? [])); + self::checkStorages($errors, "{$at}.databases.{$d}", $database); + self::checkScheduledBackups($errors, "{$at}.databases.{$d}", $database); + } + + foreach (self::items($environment['services'] ?? null) as $s => $service) { + self::checkScheduledTasks($errors, "{$at}.services.{$s}", $service); + foreach (self::items($service['applications'] ?? null) as $a => $serviceApplication) { + self::checkDomains($errors, "{$at}.services.{$s}.applications.{$a}.fqdn", data_get($serviceApplication, 'attributes.fqdn', $serviceApplication['fqdn'] ?? null)); + self::checkStorages($errors, "{$at}.services.{$s}.applications.{$a}", $serviceApplication); + } + foreach (self::items($service['databases'] ?? null) as $d => $serviceDatabase) { + self::checkStorages($errors, "{$at}.services.{$s}.databases.{$d}", $serviceDatabase); + self::checkScheduledBackups($errors, "{$at}.services.{$s}.databases.{$d}", $serviceDatabase); + } + } + } + } + + return $errors; + } + + /** + * Checks every id in the bundle and counts the items of every list key in MAX_ITEMS. + * + * @param list $errors + * @param array $counts + */ + private static function walk(array $node, string $path, array &$errors, array &$counts): void + { + foreach ($node as $key => $value) { + $at = $path === '' ? (string) $key : "{$path}.{$key}"; + + if (is_string($key)) { + if ($key === 'uuid' || str_ends_with($key, '_uuid')) { + self::checkId($errors, $at, $value); + } elseif (str_ends_with($key, '_uuids')) { + foreach (is_array($value) ? $value : [$value] as $i => $id) { + self::checkId($errors, "{$at}.{$i}", $id); + } + + continue; + } + + if (isset(self::MAX_ITEMS[$key]) && is_array($value)) { + $counts[$key] = ($counts[$key] ?? 0) + (array_is_list($value) + ? count($value) + : array_sum(array_map(fn ($list) => is_array($list) ? count($list) : 0, $value))); + } + } + + if (is_array($value)) { + self::walk($value, $at, $errors, $counts); + } + } + } + + /** + * @param list $errors + */ + private static function checkId(array &$errors, string $at, mixed $value): void + { + if ($value === null || $value === '') { + return; + } + + if (! is_string($value) || preg_match(self::ID_PATTERN, $value) !== 1) { + $errors[] = "{$at} is not a valid id. An id may only contain letters, digits, hyphens, and underscores (at most 64 characters)."; + } + } + + /** + * @return array> + */ + private static function items(mixed $value): array + { + return is_array($value) ? array_filter($value, 'is_array') : []; + } + + /** + * @param list $errors + * @param array $rules + */ + private static function check(array &$errors, string $at, mixed $value, array $rules): void + { + $validator = Validator::make(['value' => $value], ['value' => $rules], [], ['value' => $at]); + foreach ($validator->errors()->all() as $message) { + $errors[] = $message; + } + } + + /** + * @param list $errors + */ + private static function checkDomains(array &$errors, string $at, mixed $value): void + { + foreach (ValidationPatterns::validateApplicationDomains($value) as $message) { + $errors[] = "{$at}: {$message}"; + } + } + + /** + * @param list $errors + */ + private static function checkPath(array &$errors, string $at, mixed $value): void + { + if ($value === null || $value === '') { + return; + } + if (! is_string($value)) { + $errors[] = "{$at} must be a string."; + + return; + } + + try { + validateShellSafePath($value, $at); + } catch (Throwable $e) { + $errors[] = $e->getMessage(); + } + } + + /** + * @param list $errors + */ + private static function checkCron(array &$errors, string $at, mixed $value): void + { + if ($value === null) { + return; + } + + try { + $valid = is_string($value) && validate_cron_expression($value); + } catch (Throwable) { + $valid = false; + } + if (! $valid) { + $errors[] = "{$at} is not a valid cron expression or frequency."; + } + } + + /** + * @param list $errors + * @param array $resource + */ + private static function checkStorages(array &$errors, string $at, array $resource): void + { + foreach (self::items($resource['persistent_storages'] ?? null) as $i => $volume) { + self::checkPath($errors, "{$at}.persistent_storages.{$i}.mount_path", $volume['mount_path'] ?? null); + self::checkPath($errors, "{$at}.persistent_storages.{$i}.host_path", $volume['host_path'] ?? null); + } + foreach (self::items($resource['file_storages'] ?? null) as $i => $storage) { + self::checkPath($errors, "{$at}.file_storages.{$i}.fs_path", $storage['fs_path'] ?? null); + self::checkPath($errors, "{$at}.file_storages.{$i}.mount_path", $storage['mount_path'] ?? null); + } + } + + /** + * The same checks as the scheduled task API. + * + * @param list $errors + * @param array $resource + */ + private static function checkScheduledTasks(array &$errors, string $at, array $resource): void + { + foreach (self::items($resource['scheduled_tasks'] ?? null) as $i => $task) { + self::check($errors, "{$at}.scheduled_tasks.{$i}.command", $task['command'] ?? null, ['required', 'string']); + self::check($errors, "{$at}.scheduled_tasks.{$i}.container", $task['container'] ?? null, ['nullable', ...ValidationPatterns::containerNameRules()]); + self::checkCron($errors, "{$at}.scheduled_tasks.{$i}.frequency", $task['frequency'] ?? ''); + } + } + + /** + * @param list $errors + * @param array $database + */ + private static function checkScheduledBackups(array &$errors, string $at, array $database): void + { + foreach (self::items($database['scheduled_backups'] ?? null) as $i => $backup) { + self::checkCron($errors, "{$at}.scheduled_backups.{$i}.frequency", $backup['frequency'] ?? null); + self::checkPath($errors, "{$at}.scheduled_backups.{$i}.databases_to_backup", $backup['databases_to_backup'] ?? null); + } + } + + /** + * The port checks of the database API. Names and passwords are not checked: older databases + * have values that the current patterns refuse, and the values stay inside the containers. + * + * @param list $errors + * @param array $attributes + */ + private static function checkDatabaseAttributes(array &$errors, string $at, array $attributes): void + { + if (array_key_exists('ports_mappings', $attributes)) { + self::check($errors, "{$at}.ports_mappings", $attributes['ports_mappings'], ValidationPatterns::portMappingRules()); + } + if (array_key_exists('public_port', $attributes)) { + self::check($errors, "{$at}.public_port", $attributes['public_port'], ['nullable', 'integer', 'between:1024,65535']); + } + } } diff --git a/app/Services/ServerTransfer/ServerTransferClaimer.php b/app/Services/ServerTransfer/ServerTransferClaimer.php index a2eca37bf6..9a3862f7df 100644 --- a/app/Services/ServerTransfer/ServerTransferClaimer.php +++ b/app/Services/ServerTransfer/ServerTransferClaimer.php @@ -4,13 +4,16 @@ namespace App\Services\ServerTransfer; use App\Enums\GithubRunnerStatus; use App\Jobs\DeleteResourceJob; +use App\Models\Application; use App\Models\GithubRunnerExecution; use App\Models\ManagedDnsRecord; use App\Models\Server; +use App\Models\Team; use App\Notifications\Internal\GeneralNotification; use App\Services\Dns\CloudflareDnsProvider; use App\Services\Dns\ManagedDnsRecordCleanup; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Carbon; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; @@ -67,13 +70,16 @@ class ServerTransferClaimer $server = Server::query()->with('settings')->lockForUpdate()->findOrFail($server->id); // A disabled server refuses SSH, so the owner marker could not be written. - if ($server->team->serverOverflow()) { + // An archive does not use a server slot, so taking it back needs a free slot. + $overLimit = $server->isTransferredAway() ? Team::serverLimitReached($server->team) : $server->team->serverOverflow(); + if ($overLimit) { throw new RuntimeException('Your team is over its server limit. Upgrade your subscription or remove a server first.'); } if ($server->settings) { $server->settings->sentinel_custom_url = $instanceUrl; - $server->settings->ensureValidSentinelToken(); + // A new token: the instance that managed the server before keeps a copy of the old one. + $server->settings->generateSentinelToken(save: false); $server->settings->is_sentinel_enabled = true; $server->settings->force_disabled = false; $server->settings->save(); @@ -85,6 +91,7 @@ class ServerTransferClaimer 'claimed_at' => $claim['claimed_at'], 'claim' => $claim, ]); + unset($metadata['transfer']['owner_marker_conflict'], $metadata['transfer']['claimed_by_marker']); $server->server_metadata = $metadata; $server->save(); @@ -107,13 +114,13 @@ class ServerTransferClaimer * * @return array{server_uuid: string, message: string} */ - public function markTransferred(Server $server, ?string $exportId = null, ?string $targetInstanceUrl = null): array + public function markTransferred(Server $server, ?string $exportId = null, ?string $targetInstanceUrl = null, bool $fromOwnerMarker = false): array { if ($server->id === 0) { throw new RuntimeException('Cannot transfer the Coolify host itself.'); } - $result = DB::transaction(function () use ($server, $exportId, $targetInstanceUrl) { + $result = DB::transaction(function () use ($server, $exportId, $targetInstanceUrl, $fromOwnerMarker) { $server = Server::query()->with('settings')->lockForUpdate()->findOrFail($server->id); if ($server->settings) { @@ -127,9 +134,11 @@ class ServerTransferClaimer 'status' => 'transferred', 'export_id' => $exportId ?? data_get($metadata, 'transfer.export_id'), 'target_instance_url' => $targetInstanceUrl, + // Anyone who can write files on the host can write the owner marker, so its URL is not trusted. + 'claimed_by_marker' => $fromOwnerMarker, 'transferred_at' => now()->toIso8601String(), ]); - unset($metadata['transfer']['finish'], $metadata['transfer']['taken_back_at']); + unset($metadata['transfer']['finish'], $metadata['transfer']['taken_back_at'], $metadata['transfer']['owner_marker_conflict']); $server->server_metadata = $metadata; $server->save(); @@ -176,12 +185,49 @@ class ServerTransferClaimer } $this->forgetDnsRecords($server); - foreach ($server->definedResources() as $resource) { + $this->detachAdditionalApplications($server); + foreach ($this->archiveResources($server) as $resource) { (new DeleteResourceJob($resource, deleteFromCoolifyOnly: true))->handle(); } $server->forceDelete(); } + /** + * The resources that the archive of a transferred server deletes. An application that has the server only as an + * additional destination runs on another server of this instance, so it is not one of them. + * + * @return Collection + */ + public function archiveResources(Server $server): Collection + { + return $this->primaryApplications($server) + ->concat($server->databases()) + ->concat($server->services()->get()); + } + + /** + * Remove the server from applications that run on it only as an additional destination. + */ + public function detachAdditionalApplications(Server $server): void + { + $primaryIds = $this->primaryApplications($server)->pluck('id'); + DB::table('additional_destinations') + ->where('server_id', $server->id) + ->whereNotIn('application_id', $primaryIds) + ->delete(); + } + + /** + * @return Collection + */ + private function primaryApplications(Server $server): Collection + { + return $server->applications() + ->filter(fn (Application $application) => (int) data_get($application, 'destination.server_id') === (int) $server->id) + ->unique('id') + ->values(); + } + /** * @return Collection */ @@ -189,7 +235,7 @@ class ServerTransferClaimer { // The server itself owns the DNS record of its Cloudflare tunnel. return collect([$server]) - ->concat($server->applications()) + ->concat($this->primaryApplications($server)) ->concat($server->services()->with('applications')->get()->flatMap->applications); } @@ -236,10 +282,16 @@ class ServerTransferClaimer } /** - * Stop managing a claimed server when its owner marker names a claim of another instance. + * Stop managing a claimed server when its owner marker names a newer claim of another instance. * A server that was added here without a claim ignores the marker: a marker can be left over on a * host that an old instance deleted, and the next claim overwrites it. * + * Anyone who can write files on the host can write the marker, so it is not proof of a claim. A marker that is + * not newer than the claim of this instance, or that names a URL that this instance would not send credentials + * to, does not release the server: this instance keeps managing it and saves the conflict, which the UI shows. + * A forged marker with a newer time and a valid URL still releases the server; it can be taken back with + * Enable management. + * * @return bool true when this instance released the server */ public function releaseIfClaimedElsewhere(Server $server, ?string $markerJson): bool @@ -255,13 +307,25 @@ class ServerTransferClaimer } if ($markerClaimId === data_get($server->server_metadata, 'transfer.claim.claim_id')) { + if (data_get($server->server_metadata, 'transfer.owner_marker_conflict') !== null) { + $this->recordOwnerMarkerConflict($server, null, null); + } + return false; } + // Any http(s) URL: a default self-hosted target uses http://IP:8000. The URL is not trusted + // (claimed_by_marker), so it is never prefilled for a transfer back. $owner = data_get($marker, 'instance_url'); - $owner = is_string($owner) && $owner !== '' ? $owner : null; + $owner = is_string($owner) && filter_var($owner, FILTER_VALIDATE_URL) && in_array(strtolower((string) parse_url($owner, PHP_URL_SCHEME)), ['http', 'https'], true) + ? rtrim(trim($owner), '/') : null; + if ($owner === null || ! $this->isNewerThanOwnClaim($server, data_get($marker, 'claimed_at'))) { + $this->recordOwnerMarkerConflict($server, $markerClaimId, data_get($marker, 'instance_url')); - $this->markTransferred($server, targetInstanceUrl: $owner); + return false; + } + + $this->markTransferred($server, targetInstanceUrl: $owner, fromOwnerMarker: true); Log::warning('Server is claimed by another Coolify instance; management stopped here', [ 'server_uuid' => $server->uuid, @@ -275,6 +339,50 @@ class ServerTransferClaimer return true; } + private function isNewerThanOwnClaim(Server $server, mixed $markerClaimedAt): bool + { + $ownClaimedAt = data_get($server->server_metadata, 'transfer.claim.claimed_at'); + if (! is_string($markerClaimedAt) || ! is_string($ownClaimedAt)) { + return false; + } + + try { + return Carbon::parse($markerClaimedAt)->isAfter(Carbon::parse($ownClaimedAt)); + } catch (Throwable) { + return false; + } + } + + /** + * Save the untrusted marker of another claim, or remove the saved one when $markerClaimId is null. + */ + private function recordOwnerMarkerConflict(Server $server, ?string $markerClaimId, mixed $instanceUrl): void + { + if ($markerClaimId !== null && data_get($server->server_metadata, 'transfer.owner_marker_conflict.claim_id') === $markerClaimId) { + return; + } + + if ($markerClaimId !== null) { + Log::warning('The owner marker on the server names another claim, but it is not trusted; this instance keeps managing the server', [ + 'server_uuid' => $server->uuid, + 'marker_claim_id' => $markerClaimId, + ]); + } + + DB::transaction(function () use ($server, $markerClaimId, $instanceUrl) { + $locked = Server::query()->lockForUpdate()->findOrFail($server->id); + $metadata = $locked->server_metadata ?? []; + $metadata['transfer']['owner_marker_conflict'] = $markerClaimId === null ? null : [ + 'claim_id' => $markerClaimId, + 'instance_url' => is_string($instanceUrl) ? mb_substr($instanceUrl, 0, 255) : null, + 'seen_at' => now()->toIso8601String(), + ]; + $locked->server_metadata = $metadata; + $locked->save(); + $server->server_metadata = $metadata; + }); + } + /** * @param array $claim */ diff --git a/app/Services/ServerTransfer/ServerTransferExporter.php b/app/Services/ServerTransfer/ServerTransferExporter.php index 8b45a1fbdc..04181d22d3 100644 --- a/app/Services/ServerTransfer/ServerTransferExporter.php +++ b/app/Services/ServerTransfer/ServerTransferExporter.php @@ -564,6 +564,8 @@ class ServerTransferExporter $steps[] = 'After the GitHub App sends webhooks to {target_url}, open this server there, check the runner requirements, and enable GitHub runners. ' .'Enabling them stops runners that another Coolify instance started on this server.'; } + $steps[] = 'This instance keeps a copy of the SSH key of the server. On {target_url}, add a new SSH key to the server and select it there, ' + .'then remove the old public key from ~/.ssh/authorized_keys on the server.'; return $steps; } diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 3dd43f8d7c..9cdb066eee 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -181,7 +181,11 @@ class ServerTransferImporter bool $preserveUuids = true, bool $adoptMode = true, ): array { - ServerTransferBundle::assertValid($bundle); + // Checks every id, list size, and imported value before anything is written. + $validation = ServerTransferBundle::validate($bundle); + if (! $validation['valid']) { + throw ValidationException::withMessages(['bundle' => $validation['errors']]); + } $this->assertValidServerPayload((array) data_get($bundle, 'server', [])); if ($adoptMode && ! $preserveUuids) { @@ -190,7 +194,6 @@ class ServerTransferImporter ]); } - $validation = ServerTransferBundle::validate($bundle); $warnings = $validation['warnings']; if (is_array(data_get($bundle, 'warnings'))) { $warnings = array_values(array_unique(array_merge($warnings, $bundle['warnings']))); @@ -504,8 +507,8 @@ class ServerTransferImporter { $team = Team::find($teamId); $archive = $preserveUuids ? $this->replaceableArchive($serverUuid, $teamId) : null; - // The archive is replaced, so the import does not add a server. - $limitReached = ! $team || ($archive === null && Team::serverLimitReached($team)); + // An archive does not use a server slot, so replacing it also needs a free slot. + $limitReached = ! $team || Team::serverLimitReached($team); $existingServer = $preserveUuids && $archive === null ? Server::withTrashed()->where('uuid', $serverUuid)->first() : null; $uuidMessage = match (true) { @@ -592,7 +595,7 @@ class ServerTransferImporter if ($adoptMode) { throw ValidationException::withMessages([ - 'bundle' => ["The {$label} UUID {$uuid} is already used on this instance. Delete that {$label} permanently first, because adopt mode needs the same UUID to keep the running containers."], + 'bundle' => ["The {$label} UUID {$uuid} cannot be used on this instance. Adopt mode needs the same UUID to keep the running containers. If you deleted the {$label} on this instance, delete it permanently first."], ]); } @@ -2273,7 +2276,7 @@ class ServerTransferImporter $preview = ApplicationPreview::withTrashed()->where('uuid', $uuid)->first(); if ($preview && ! $this->previewBelongsToTeam($preview, $teamId)) { throw ValidationException::withMessages([ - 'previews' => ['An imported preview UUID conflicts with a preview outside the target team.'], + 'previews' => ['An imported preview UUID cannot be used on this instance.'], ]); } if (! $preview) { diff --git a/app/Services/ServerTransfer/ServerTransferMigrator.php b/app/Services/ServerTransfer/ServerTransferMigrator.php index 3166df1d24..0f81e423f2 100644 --- a/app/Services/ServerTransfer/ServerTransferMigrator.php +++ b/app/Services/ServerTransfer/ServerTransferMigrator.php @@ -10,6 +10,9 @@ use App\Models\Environment; use App\Models\GithubApp; use App\Models\GitlabApp; use App\Models\Project; +use App\Models\ScheduledDatabaseBackupExecution; +use App\Models\ScheduledTaskExecution; +use App\Models\ScheduledVolumeBackupExecution; use App\Models\Server; use App\Models\Service; use App\Models\User; @@ -25,6 +28,7 @@ use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Validator; use Illuminate\Support\Sleep; use Illuminate\Support\Str; +use Illuminate\Validation\ValidationException; use RuntimeException; use Throwable; @@ -48,6 +52,9 @@ class ServerTransferMigrator */ public const MIGRATION_TIMEOUT_SECONDS = 1500; + /** Lifetime of the token that a transfer back gives the other instance: the job timeout plus time in the queue. */ + public const TRANSFER_BACK_TOKEN_SECONDS = 3600; + /** Longer than the job timeout, so a running state older than this one is stale. */ public const STALE_MIGRATION_SECONDS = 1800; @@ -304,6 +311,32 @@ class ServerTransferMigrator 'This server was already transferred to another instance.'), $this->check('no deployment is queued or running on the server', $activeDeployments === 0, "{$activeDeployments} deployment(s) on this server are queued or running. Wait until they finish or cancel them, then transfer the server."), + ...array_map(fn (array $job) => $this->check("no {$job['label']} is running on the server", $job['count'] === 0, + "{$job['count']} {$job['label']}(s) on this server are running. Wait until they finish, then transfer the server."), $this->runningJobs($server)), + ]; + } + + /** + * Backups and scheduled tasks that run on the server. A transfer during a run leaves the run without an owner. + * A run that has been 'running' for a day is left over from a stopped worker, so it does not block. + * + * @return list + */ + private function runningJobs(Server $server): array + { + $onServer = fn ($schedule) => $schedule && (int) $schedule->server()?->id === (int) $server->id; + $running = fn (string $model, string $schedule) => $model::query() + ->where('status', 'running') + ->where('created_at', '>', now()->subDay()) + ->with($schedule) + ->get() + ->filter(fn (Model $execution) => $onServer($execution->{$schedule})) + ->count(); + + return [ + ['label' => 'database backup', 'count' => $running(ScheduledDatabaseBackupExecution::class, 'scheduledDatabaseBackup')], + ['label' => 'volume backup', 'count' => $running(ScheduledVolumeBackupExecution::class, 'scheduledVolumeBackup')], + ['label' => 'scheduled task', 'count' => $running(ScheduledTaskExecution::class, 'scheduledTask')], ]; } @@ -484,7 +517,8 @@ class ServerTransferMigrator 'complete' => $complete, 'warnings' => $warnings, 'checklist' => $checklist, - 'target_server_url' => is_string(data_get($importBody, 'server_url')) ? data_get($importBody, 'server_url') : null, + // The target sends this URL, and the UI renders it as a link. + 'target_server_url' => self::isLinkableUrl(data_get($importBody, 'server_url')) ? data_get($importBody, 'server_url') : null, 'message' => 'Server migrated to '.$targetUrl.'. Automations disabled on this instance.', ]; } @@ -523,8 +557,9 @@ class ServerTransferMigrator throw new RuntimeException('Enter the URL at which the other instance reaches this instance.'); } - // The import checks and the import need write and deploy. - $importToken = $user->createToken("transfer back {$archive->uuid}", ['read', 'write', 'deploy'], now()->addHours(3), $archive->team_id); + // The import checks, the import, and the status probes need only write and deploy. The claim after the + // import deletes the token, and the expiry covers a failed import: the transfer job runs at most 25 minutes. + $importToken = $user->createToken(self::transferBackTokenName($archive->uuid), ['write', 'deploy'], now()->addSeconds(self::TRANSFER_BACK_TOKEN_SECONDS), $archive->team_id); $migrateUrl = $managerUrl.'/api/v1/servers/'.rawurlencode($archive->uuid).'/migrate'; try { @@ -555,37 +590,80 @@ class ServerTransferMigrator }); } - private function normalizeTargetUrl(string $targetUrl): string + /** + * The rules for the URL of another Coolify instance that this instance sends credentials to: https and a public + * address, or a local address in development. Every stored or prefilled instance URL must pass them. + * + * @throws RuntimeException|ValidationException + */ + public static function validateInstanceUrl(string $url): string { - $targetUrl = rtrim(trim($targetUrl), '/'); - if ($targetUrl === '' || ! filter_var($targetUrl, FILTER_VALIDATE_URL)) { + $url = rtrim(trim($url), '/'); + if ($url === '' || ! filter_var($url, FILTER_VALIDATE_URL)) { throw new RuntimeException('A valid target instance URL is required (e.g. https://coolify.example.com).'); } - if ($this->isLocalDevelopmentTarget($targetUrl)) { - if (file_exists('/.dockerenv') || is_file('/run/.containerenv')) { - return (string) preg_replace( - '#^(https?://)(localhost|127\.0\.0\.1)(?=[:/]|$)#i', - '$1host.docker.internal', - $targetUrl, - ); - } - - return $targetUrl; + if (self::isLocalDevelopmentTarget($url)) { + return $url; } - if (strtolower((string) parse_url($targetUrl, PHP_URL_SCHEME)) !== 'https') { + if (strtolower((string) parse_url($url, PHP_URL_SCHEME)) !== 'https') { throw new RuntimeException('The target instance URL must use https, because the transfer sends credentials.'); } - Validator::make(['target_url' => $targetUrl], [ + Validator::make(['target_url' => $url], [ 'target_url' => ['required', new SafeWebhookUrl], ])->validate(); + return $url; + } + + public static function isValidInstanceUrl(?string $url): bool + { + try { + self::validateInstanceUrl((string) $url); + + return true; + } catch (RuntimeException|ValidationException) { + return false; + } + } + + /** + * A URL from another instance that the UI may render as a link: https, or http in development. + */ + public static function isLinkableUrl(mixed $url): bool + { + if (! is_string($url) || ! filter_var($url, FILTER_VALIDATE_URL)) { + return false; + } + + $scheme = strtolower((string) parse_url($url, PHP_URL_SCHEME)); + + return $scheme === 'https' || ($scheme === 'http' && isDev()); + } + + public static function transferBackTokenName(string $serverUuid): string + { + return "transfer back {$serverUuid}"; + } + + private function normalizeTargetUrl(string $targetUrl): string + { + $targetUrl = self::validateInstanceUrl($targetUrl); + + if (self::isLocalDevelopmentTarget($targetUrl) && (file_exists('/.dockerenv') || is_file('/run/.containerenv'))) { + return (string) preg_replace( + '#^(https?://)(localhost|127\.0\.0\.1)(?=[:/]|$)#i', + '$1host.docker.internal', + $targetUrl, + ); + } + return $targetUrl; } - private function isLocalDevelopmentTarget(string $url): bool + private static function isLocalDevelopmentTarget(string $url): bool { return isDev() && in_array(strtolower((string) parse_url($url, PHP_URL_SCHEME)), ['http', 'https'], true) @@ -801,7 +879,7 @@ class ServerTransferMigrator private function targetRequest(string $url, string $token, int $timeout): PendingRequest { return Http::timeout($timeout) - ->withOptions($this->isLocalDevelopmentTarget($url) + ->withOptions(self::isLocalDevelopmentTarget($url) ? ['allow_redirects' => false] : SafeWebhookUrl::httpClientOptions($url)) ->acceptJson() diff --git a/app/Services/ServerTransfer/TransferFinishChecklist.php b/app/Services/ServerTransfer/TransferFinishChecklist.php index ee8d93dd79..d894b49cb3 100644 --- a/app/Services/ServerTransfer/TransferFinishChecklist.php +++ b/app/Services/ServerTransfer/TransferFinishChecklist.php @@ -48,14 +48,14 @@ class TransferFinishChecklist ]; private const GROUP_OF_TYPE = [ - 'server_connection' => 'server', 'sentinel' => 'server', 's3_storage' => 'server', + 'server_connection' => 'server', 'sentinel' => 'server', 'previous_ssh_key' => 'server', 's3_storage' => 'server', 'github_app_webhook' => 'git', 'github_app_urls' => 'git', 'gitlab_webhook' => 'git', 'manual_webhook' => 'git', 'git_source' => 'git', 'github_runners' => 'git', 'dns_record' => 'dns', 'domain' => 'dns', ]; /** Types that the user can only mark as done. */ - private const MANUAL_TYPES = ['github_app_urls']; + private const MANUAL_TYPES = ['github_app_urls', 'previous_ssh_key']; /** Types that Coolify can fix with one click. */ private const FIX_LABELS = [ @@ -145,11 +145,16 @@ class TransferFinishChecklist { if (in_array($item['type'], ['github_app_webhook', 'github_app_urls'], true)) { $githubApp = GithubApp::where('uuid', $item['subject'])->first(); + $url = $githubApp?->settingsUrl(); - return $githubApp ? [$githubApp->settingsUrl(), 'Open on GitHub'] : null; + // The html_url of a GitHub App comes from the bundle of another instance, so only web links are shown. + return $url && in_array(strtolower((string) parse_url($url, PHP_URL_SCHEME)), ['http', 'https'], true) ? [$url, 'Open on GitHub'] : null; } - return filled($item['link'] ?? null) ? [$item['link'], $item['link_label'] ?? 'Open'] : null; + $link = $item['link'] ?? null; + + // Saved links are paths of this instance. + return is_string($link) && preg_match('#^/(?![/\\\\])#', $link) ? [$link, $item['link_label'] ?? 'Open'] : null; } public function fixLabel(array $item): ?string @@ -228,6 +233,10 @@ class TransferFinishChecklist $items = [ $this->item('server_connection', 'server', 'Connection to the server'), $this->item('sentinel', 'server', 'Sentinel sends its data to this instance'), + // The SSH key moved with the server, so the previous instance still has a copy of it. + $this->item('previous_ssh_key', 'server', 'Remove the SSH key of the previous instance from the server', + 'The previous instance keeps a copy of the SSH key. Add a new key of this instance to the server and select it here, then remove the old public key from ~/.ssh/authorized_keys on the server.', + [], route('server.private-key', ['server_uuid' => $server->uuid], false), 'Open private key'), ]; $githubApps = $applications->map(fn (Application $application) => $application->source) diff --git a/resources/views/livewire/server/private-key/show.blade.php b/resources/views/livewire/server/private-key/show.blade.php index 72e98917e9..0fac1b1f7a 100644 --- a/resources/views/livewire/server/private-key/show.blade.php +++ b/resources/views/livewire/server/private-key/show.blade.php @@ -89,7 +89,7 @@ @click.prevent="copyPublicKeyToClipboard({{ Js::from($privateKey->public_key) }})"> Copy public key - @if (data_get($server, 'privateKey.uuid') !== $privateKey->uuid) + @if (data_get($server, 'privateKey.uuid') !== $privateKey->uuid && ! $server->isTransferredAway()) Use this key diff --git a/resources/views/livewire/server/show.blade.php b/resources/views/livewire/server/show.blade.php index de4c475729..bbd3c5935b 100644 --- a/resources/views/livewire/server/show.blade.php +++ b/resources/views/livewire/server/show.blade.php @@ -222,11 +222,11 @@ + required :disabled="$isValidating || $server->isTransferredAway()" /> + required :disabled="$isValidating || $server->isTransferredAway()" /> + label="SSH port" required :disabled="$isValidating || $server->isTransferredAway()" />
diff --git a/resources/views/livewire/server/transfer-finish.blade.php b/resources/views/livewire/server/transfer-finish.blade.php index aaeb37869a..daacb6c40f 100644 --- a/resources/views/livewire/server/transfer-finish.blade.php +++ b/resources/views/livewire/server/transfer-finish.blade.php @@ -87,11 +87,11 @@ class="listbox-panel top-full! right-0! left-auto! z-[90]! mt-1! w-40! min-w-40!"> @unless ($manualOnly) + wire:click="checkStep({{ \Illuminate\Support\Js::from($step['key']) }})" @click="menu = false">Check now @endunless @if (! $manualOnly || $link) + wire:click="markStepDone({{ \Illuminate\Support\Js::from($step['key']) }})" @click="menu = false">Mark as done @endif @if ($fixLabel && $link) @endif @if ($fixLabel) - {{ $fixLabel }} + {{ $fixLabel }} @elseif ($link) {{ $linkLabel }} @elseif ($manualOnly) - Mark as done + Mark as done @endif
@endcan diff --git a/resources/views/livewire/server/transfer.blade.php b/resources/views/livewire/server/transfer.blade.php index 900e04f8be..488d1b1bc3 100644 --- a/resources/views/livewire/server/transfer.blade.php +++ b/resources/views/livewire/server/transfer.blade.php @@ -34,6 +34,14 @@
+ @if (! $server->isTransferredAway() && ($markerConflict = data_get($server->server_metadata, 'transfer.owner_marker_conflict'))) + + The owner file on the server names a claim of + {{ data_get($markerConflict, 'instance_url') ?: 'an unknown instance' }}, but it is older than the + claim of this instance or its URL is not valid, so this instance keeps managing the server. + If you did not transfer the server, check who can write files on it. + + @endif @if ($server->isTransferredAway())

Transfer back

@@ -51,11 +59,18 @@ placeholder="Paste token from that instance" autocomplete="off" helper="A token with root, or with read:sensitive and write, from an admin or owner of that instance." /> + helper="The URL at which the other instance reaches this instance. This instance gives it a token that expires in 1 hour." />
+ @php + // A static string: x-text shows the typed host as text, also after the modal was rendered. + $transferBackWarning = 'This instance sends a new API token of your team to ' + .'. ' + .'Continue only if you trust that instance.'; + @endphp The target shows these steps on the server page, checks them where it can, and keeps their state. - @if ($targetServerUrl = data_get($migration, 'target_server_url')) + @if ($targetServerUrl = $this->targetServerUrl) Open the server on the target. @endif diff --git a/tests/Feature/Api/ServerTransferApiTest.php b/tests/Feature/Api/ServerTransferApiTest.php index 7435085821..8debea9663 100644 --- a/tests/Feature/Api/ServerTransferApiTest.php +++ b/tests/Feature/Api/ServerTransferApiTest.php @@ -280,6 +280,72 @@ describe('POST /api/v1/servers/import', function () { }); }); +describe('POST /api/v1/servers/import with an unsafe bundle', function () { + test('rejects the whole bundle before it writes anything', function (Closure $tamper, string $error) { + $export = transferExportForApiTest($this->server); + $this->application->forceDelete(); + $this->server->forceDelete(); + $tamper($export); + $counts = fn () => [Server::withTrashed()->count(), PrivateKey::count(), Project::count(), Application::withTrashed()->count(), GithubApp::count()]; + $before = $counts(); + + $response = $this->withHeaders(transferHeaders($this->importToken)) + ->postJson('/api/v1/servers/import', ['bundle' => $export]) + ->assertStatus(422); + + expect(collect($response->json('errors.bundle'))->contains(fn (string $message) => str_contains($message, $error)))->toBeTrue((string) $response->getContent()) + ->and($counts())->toBe($before); + Queue::assertNotPushed(ValidateAndInstallServerJob::class); + Process::assertNothingRan(); + })->with([ + 'server uuid with shell syntax' => [function (array &$bundle) { + $bundle['server']['uuid'] = 'x;id;#'; + }, 'server.uuid is not a valid id'], + 'private key uuid with command substitution' => [function (array &$bundle) { + $bundle['private_key']['uuid'] = 'a $(id)'; + }, 'private_key.uuid is not a valid id'], + 'project uuid with path traversal' => [function (array &$bundle) { + $bundle['projects'][0]['uuid'] = '../x'; + }, 'projects.0.uuid is not a valid id'], + 'reference uuid with path traversal' => [function (array &$bundle) { + $bundle['projects'][0]['environments'][0]['applications'][0]['destination_uuid'] = '../../etc'; + }, 'destination_uuid is not a valid id'], + 'javascript git source url' => [function (array &$bundle) { + $bundle['github_apps'] = [['uuid' => 'evil-app', 'name' => 'evil', 'html_url' => 'javascript:alert(1)', 'api_url' => 'https://api.github.com']]; + }, 'github_apps.0.html_url'], + 'invalid application fqdn' => [function (array &$bundle) { + $bundle['projects'][0]['environments'][0]['applications'][0]['attributes']['fqdn'] = 'javascript:alert(document.cookie)'; + }, 'applications.0.fqdn'], + 'invalid scheduled task cron' => [function (array &$bundle) { + $bundle['projects'][0]['environments'][0]['applications'][0]['scheduled_tasks'] = [['name' => 'task', 'command' => 'echo hi', 'frequency' => 'every full moon']]; + }, 'scheduled_tasks.0.frequency is not a valid cron expression'], + 'storage path with shell syntax' => [function (array &$bundle) { + $bundle['projects'][0]['environments'][0]['applications'][0]['persistent_storages'] = [['name' => 'data', 'mount_path' => '/data;id', 'host_path' => null]]; + }, 'persistent_storages.0.mount_path'], + 'destination network with shell syntax' => [function (array &$bundle) { + $bundle['destinations'][0]['network'] = 'coolify;id'; + }, 'destinations.0.network'], + 'too many warnings' => [function (array &$bundle) { + $bundle['warnings'] = array_fill(0, 1001, 'x'); + }, 'A bundle can have at most 1000 warnings'], + ]); + + test('a uuid that another team uses does not say that the other team has it', function () { + $export = transferExportForApiTest($this->server); + $appUuid = $this->application->uuid; + $this->server->forceDelete(); + $this->application->forceFill(['environment_id' => Environment::factory()->create([ + 'project_id' => Project::factory()->create(['team_id' => Team::factory()->create()->id])->id, + ])->id])->saveQuietly(); + + $response = $this->withHeaders(transferHeaders($this->importToken)) + ->postJson('/api/v1/servers/import', ['bundle' => $export]) + ->assertStatus(422); + + expect($response->json('errors.bundle.0'))->toBe("The application UUID {$appUuid} cannot be used on this instance. Adopt mode needs the same UUID to keep the running containers. If you deleted the application on this instance, delete it permanently first."); + }); +}); + describe('POST /api/v1/servers/import claim', function () { test('import takes management of the server and rebinds Sentinel', function () { $export = transferExportForApiTest($this->server); @@ -320,7 +386,7 @@ describe('POST /api/v1/servers/{uuid}/transfer/complete', function () { $response = $this->withHeaders(transferHeaders($this->sensitiveToken)) ->postJson("/api/v1/servers/{$this->server->uuid}/transfer/complete", [ 'export_id' => 'exp-123', - 'target_instance_url' => 'https://coolify-b.test', + 'target_instance_url' => 'https://8.8.8.8', ]); $response->assertOk() @@ -330,7 +396,50 @@ describe('POST /api/v1/servers/{uuid}/transfer/complete', function () { expect($server->settings->force_disabled)->toBeTrue() ->and(data_get($server->server_metadata, 'transfer.status'))->toBe('transferred') ->and(data_get($server->server_metadata, 'transfer.export_id'))->toBe('exp-123') - ->and(data_get($server->server_metadata, 'transfer.target_instance_url'))->toBe('https://coolify-b.test'); + ->and(data_get($server->server_metadata, 'transfer.target_instance_url'))->toBe('https://8.8.8.8'); + }); +}); + +describe('POST /api/v1/servers/{uuid}/transfer/complete with an unsafe URL', function () { + test('rejects a target instance URL that a transfer back must not send a token to', function (string $url) { + $this->withHeaders(transferHeaders($this->sensitiveToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/transfer/complete", ['target_instance_url' => $url]) + ->assertStatus(422) + ->assertJsonValidationErrors(['target_instance_url']); + + expect($this->server->fresh()->isTransferredAway())->toBeFalse(); + })->with([ + 'javascript' => 'javascript:alert(1)', + 'plain http' => 'http://8.8.8.8', + 'private address' => 'https://10.0.0.5', + ]); +}); + +describe('GET /api/v1/servers/{uuid}/transfer for a member', function () { + test('shows the transfer state, but not its log and error', function () { + $member = User::factory()->create(); + $this->team->members()->attach($member->id, ['role' => 'member']); + $memberToken = $member->createToken('member-read', ['read'], teamId: $this->team->id)->plainTextToken; + $this->server->forceFill(['server_metadata' => ['migration' => [ + 'status' => 'failed', + 'target_url' => 'https://8.8.8.8', + 'error' => 'secret error', + 'log' => [['at' => now()->toIso8601String(), 'level' => 'error', 'message' => 'secret log']], + ]]])->save(); + + $this->withHeaders(transferHeaders($memberToken)) + ->getJson("/api/v1/servers/{$this->server->uuid}/transfer") + ->assertOk() + ->assertJsonPath('migration.status', 'failed') + ->assertJsonMissingPath('migration.log') + ->assertJsonMissingPath('migration.error'); + + app('auth')->forgetGuards(); + $this->withHeaders(transferHeaders($this->sensitiveToken)) + ->getJson("/api/v1/servers/{$this->server->uuid}/transfer") + ->assertOk() + ->assertJsonPath('migration.error', 'secret error') + ->assertJsonPath('migration.log.0.message', 'secret log'); }); }); @@ -341,7 +450,7 @@ describe('full transfer flow A to B on same process', function () { $this->withHeaders(transferHeaders($this->sensitiveToken)) ->postJson("/api/v1/servers/{$this->server->uuid}/transfer/complete", [ 'export_id' => $export['export_id'], - 'target_instance_url' => 'https://coolify-b.test', + 'target_instance_url' => 'https://8.8.8.8', ]) ->assertOk(); @@ -498,7 +607,7 @@ describe('GET /api/v1/servers/import/check', function () { $this->withHeaders(transferHeaders($this->importToken)) ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip='.urlencode($ip).'&port=22') ->assertStatus(422) - ->assertJsonPath('message', fn ($message) => str_contains($message, 'cannot connect to a private, loopback, or link-local address')); + ->assertJsonPath('message', fn ($message) => str_contains($message, 'is a private, loopback, or link-local address')); })->with(['127.0.0.1', '10.0.0.5', '169.254.169.254', '::1', 'localhost']); test('on Cloud, rate limits the reachability probe', function () { diff --git a/tests/Feature/Livewire/ServerTransferUiTest.php b/tests/Feature/Livewire/ServerTransferUiTest.php index 614e92de9f..67516af896 100644 --- a/tests/Feature/Livewire/ServerTransferUiTest.php +++ b/tests/Feature/Livewire/ServerTransferUiTest.php @@ -315,3 +315,91 @@ test('team members cannot start a transfer back', function () { Http::assertNothingSent(); }); + +test('the transfer log cannot be polled after the user lost the admin role', function () { + $component = Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]); + $member = User::factory()->create(); + $this->team->members()->attach($member->id, ['role' => 'member']); + $this->actingAs($member); + + $component->call('refreshMigration')->assertForbidden(); +}); + +test('the tokens are not kept in the component after a failed action', function () { + Queue::fake(); + Http::fake(['*' => Http::response(['message' => 'Unauthenticated.'], 401)]); + + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->set('targetUrl', 'https://8.8.8.8') + ->set('targetToken', 'secret-target-token') + ->call('migrateServer') + ->assertDispatched('error') + ->assertSet('targetToken', ''); + + app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'https://8.8.8.8'); + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->set('managerToken', 'secret-manager-token') + ->call('transferBack') + ->assertDispatched('error') + ->assertSet('managerToken', ''); +}); + +test('the URL of an owner marker is not prefilled for a transfer back', function () { + $this->server->forceFill(['server_metadata' => ['transfer' => ['status' => 'claimed', 'claim' => ['claim_id' => 'own', 'claimed_at' => now()->subDay()->toIso8601String()]]]])->save(); + app(ServerTransferClaimer::class)->releaseIfClaimedElsewhere($this->server->fresh(), json_encode([ + 'claim_id' => 'other', 'instance_url' => 'https://8.8.8.8', 'claimed_at' => now()->toIso8601String(), + ])); + expect($this->server->fresh()->isTransferredAway())->toBeTrue(); + + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->assertSet('managerUrl', '') + ->assertSee('data-testid="transfer-back-host"', escape: false); +}); + +test('a stored target URL that is not a safe instance URL is not prefilled for a transfer back', function () { + app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'http://10.0.0.5:8000'); + + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->assertSet('managerUrl', ''); +}); + +test('the transfer page does not link to a server URL that is not https', function () { + $this->server->forceFill(['server_metadata' => ['migration' => [ + 'status' => 'succeeded', + 'target_url' => 'https://8.8.8.8', + 'checklist' => ['A step.'], + 'target_server_url' => 'javascript:alert(document.cookie)', + ]]])->save(); + + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->assertSee('Finish the transfer on the target') + ->assertDontSee('javascript:alert', escape: false); +}); + +test('the transfer page warns about an untrusted owner marker', function () { + $this->server->forceFill(['server_metadata' => ['transfer' => ['status' => 'claimed', 'claim' => ['claim_id' => 'own', 'claimed_at' => now()->toIso8601String()]]]])->save(); + app(ServerTransferClaimer::class)->releaseIfClaimedElsewhere($this->server->fresh(), json_encode([ + 'claim_id' => 'forged', 'instance_url' => 'https://evil.example', 'claimed_at' => now()->subDay()->toIso8601String(), + ])); + + Livewire::test(Transfer::class, ['server_uuid' => $this->server->uuid]) + ->assertSee('Another instance may claim this server') + ->assertSee('https://evil.example'); +}); + +test('deleting an archive keeps applications that use the server only as an additional destination', function () { + Queue::fake(); + $otherServer = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $this->privateKey->id, 'ip' => '10.77.0.11']); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->firstOrFail(); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $sharedApplication = Application::factory()->create(['destination_id' => $otherDestination->id, 'destination_type' => $otherDestination->getMorphClass()]); + $sharedApplication->additional_servers()->attach($this->server->id, ['standalone_docker_id' => $destination->id]); + $application = Application::factory()->create(['destination_id' => $destination->id, 'destination_type' => $destination->getMorphClass()]); + app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'https://coolify-b.test'); + + Livewire::test(Delete::class, ['server_uuid' => $this->server->uuid])->call('delete', $this->userPassword); + + Queue::assertPushed(DeleteResourceJob::class, fn (DeleteResourceJob $job) => $job->resource->is($application)); + Queue::assertNotPushed(DeleteResourceJob::class, fn (DeleteResourceJob $job) => $job->resource->is($sharedApplication)); + expect($sharedApplication->additional_servers()->count())->toBe(0); +}); diff --git a/tests/Feature/ServerConnectionCheckIsolationTest.php b/tests/Feature/ServerConnectionCheckIsolationTest.php index 9d5b0b572b..ee308cb52f 100644 --- a/tests/Feature/ServerConnectionCheckIsolationTest.php +++ b/tests/Feature/ServerConnectionCheckIsolationTest.php @@ -245,14 +245,14 @@ it('gives the job enough time for every connection check step', function () { ->and($lock->expiresAfter)->toBeGreaterThan($job->timeout); }); -it('stops managing a server when its owner marker names another instance', function () { +it('stops managing a server when its owner marker names a newer claim of another instance', function () { $server = createServerForConnectionIsolationTest([ 'ip' => '203.0.113.10', - 'server_metadata' => ['transfer' => ['status' => 'claimed', 'claim' => ['claim_id' => 'claim-of-this-instance']]], + 'server_metadata' => ['transfer' => ['status' => 'claimed', 'claim' => ['claim_id' => 'claim-of-this-instance', 'claimed_at' => '2026-01-01T00:00:00+00:00']]], ]); $server->settings->update(['is_reachable' => true, 'is_usable' => true]); Process::fake([ - '*ls -la /*' => Process::result(output: '{"claim_id":"claim-of-instance-b","instance_url":"https://coolify-b.test"}', exitCode: 0), + '*ls -la /*' => Process::result(output: '{"claim_id":"claim-of-instance-b","instance_url":"https://8.8.8.8","claimed_at":"2026-02-01T00:00:00+00:00"}', exitCode: 0), '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), ]); @@ -260,7 +260,7 @@ it('stops managing a server when its owner marker names another instance', funct $server->refresh(); expect($server->isTransferredAway())->toBeTrue() - ->and(data_get($server->server_metadata, 'transfer.target_instance_url'))->toBe('https://coolify-b.test') + ->and(data_get($server->server_metadata, 'transfer.target_instance_url'))->toBe('https://8.8.8.8') ->and((bool) $server->settings->force_disabled)->toBeTrue(); Process::assertDidntRun(fn ($process) => str_contains($process->command, 'docker version')); }); diff --git a/tests/Feature/SshMultiplexingLockTest.php b/tests/Feature/SshMultiplexingLockTest.php index a489af4955..9e712bbf28 100644 --- a/tests/Feature/SshMultiplexingLockTest.php +++ b/tests/Feature/SshMultiplexingLockTest.php @@ -151,7 +151,7 @@ it('adds mux options to ssh commands only after the explicit master is ready', f expect($command) ->toContain('-o ControlMaster=auto') - ->toContain("-o ControlPath=/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}") + ->toContain("-o ControlPath='/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}'") ->toContain('-o ControlPersist=3600') ->toContain("'sh -c \"if command -v bash >/dev/null 2>&1; then exec bash -se; else exec sh -se; fi\"' << \\") ->not->toContain('<< $delimiter'); @@ -159,6 +159,37 @@ it('adds mux options to ssh commands only after the explicit master is ready', f Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -fN ')); }); +it('quotes the mux socket and key paths in every local ssh command', function () { + config(['constants.ssh.mux_enabled' => true]); + $server = makeMuxServer(); + // Ids with shell syntax, as an import bundle could have had them before ids were validated. + $server->forceFill(['uuid' => 'x;touch /tmp/pwn;#'])->saveQuietly(); + $server->privateKey->forceFill(['uuid' => 'a $(id)'])->saveQuietly(); + $server = Server::find($server->id); + Storage::disk('ssh-keys')->put('ssh_key@a $(id)', $server->privateKey->private_key); + + Process::fake([ + '*-O check*' => Process::result(exitCode: 1), + '*-fN *' => Process::result(exitCode: 0), + ]); + + $socket = escapeshellarg('/var/www/html/storage/app/ssh/mux/mux_x;touch /tmp/pwn;#'); + $key = escapeshellarg($server->privateKey->getKeyLocation()); + + $sshCommand = SshMultiplexingHelper::generateSshCommand($server, 'echo ok'); + $scpCommand = SshMultiplexingHelper::generateScpCommand($server, '/tmp/source', '/tmp/dest'); + + foreach ([$sshCommand, $scpCommand] as $command) { + expect($command)->toContain("-o ControlPath={$socket} ")->toContain("-i {$key} "); + } + Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -fN ') + && str_contains($process->command, "-o ControlPath={$socket} ") + && str_contains($process->command, "-i {$key} ")); + Process::assertRan(fn ($process) => str_contains($process->command, "ssh -O check -o ControlPath={$socket} ")); + Process::assertDidntRun(fn ($process) => str_contains($process->command, 'mux_x;touch /tmp/pwn;# ') + || str_contains($process->command, '-i /') || str_contains($process->command, 'ControlPath=/')); +}); + it('can generate terminal ssh commands without a hard command timeout', function () { config(['constants.ssh.mux_enabled' => false]); $server = makeMuxServer(); @@ -200,7 +231,7 @@ it('adds mux options to scp commands only after the explicit master is ready', f expect($command) ->toContain('-o ControlMaster=auto') - ->toContain("-o ControlPath=/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}") + ->toContain("-o ControlPath='/var/www/html/storage/app/ssh/mux/mux_{$server->uuid}'") ->toContain('-o ControlPersist=3600'); Process::assertRan(fn ($process) => str_contains($process->command, 'ssh -fN ')); diff --git a/tests/Unit/ServerTransfer/ServerTransferBundleTest.php b/tests/Unit/ServerTransfer/ServerTransferBundleTest.php index cd4d74019b..aa27c86120 100644 --- a/tests/Unit/ServerTransfer/ServerTransferBundleTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferBundleTest.php @@ -1,6 +1,7 @@ toBeTrue(); }); + +test('validate accepts Coolify ids and legacy RFC 4122 uuids', function () { + $bundle = bundleTestMinimalBundle(); + $bundle['server']['uuid'] = new_public_id(); + $bundle['private_key']['uuid'] = (string) Str::uuid(); + $bundle['volume_backups'] = [['uuid' => 'abc_DEF-123', 's3_storage_uuids' => [(string) Str::uuid()], 'frequency' => 'daily']]; + + expect(ServerTransferBundle::validate($bundle)['errors'])->toBe([]); +}); + +test('validate rejects an unsafe id in an id list', function () { + $bundle = bundleTestMinimalBundle(); + $bundle['volume_backups'] = [['s3_storage_uuids' => ['ok', '../../x']]]; + + expect(ServerTransferBundle::validate($bundle)['errors'])->toContain('volume_backups.0.s3_storage_uuids.1 is not a valid id. An id may only contain letters, digits, hyphens, and underscores (at most 64 characters).'); +}); diff --git a/tests/Unit/ServerTransfer/ServerTransferClaimerTest.php b/tests/Unit/ServerTransfer/ServerTransferClaimerTest.php index c5c07e6195..0388f3b867 100644 --- a/tests/Unit/ServerTransfer/ServerTransferClaimerTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferClaimerTest.php @@ -96,6 +96,8 @@ test('claim writes the owner marker with its claim id on the host', function () }); test('claim changes nothing when the owner marker cannot be written', function () { + // The new Sentinel token restarts Sentinel; this test is about the marker. + Queue::fake(); Process::fake(['*' => Process::result(errorOutput: 'Permission denied', exitCode: 1)]); $sentinelUrlBefore = $this->server->settings->sentinel_custom_url; @@ -107,18 +109,20 @@ test('claim changes nothing when the owner marker cannot be written', function ( ->and($this->server->settings->sentinel_custom_url)->toBe($sentinelUrlBefore); }); -test('a server is released when its owner marker names the claim of another instance', function () { +test('a server is released when its owner marker names a newer claim of another instance', function () { app(ServerTransferClaimer::class)->claim($this->server); $released = app(ServerTransferClaimer::class)->releaseIfClaimedElsewhere($this->server->fresh(), json_encode([ 'claim_id' => 'claim-of-instance-b', - 'instance_url' => 'https://coolify-b.test', + 'instance_url' => 'https://8.8.8.8', + 'claimed_at' => now()->addMinute()->toIso8601String(), ])); $this->server->refresh(); expect($released)->toBeTrue() ->and($this->server->isTransferredAway())->toBeTrue() - ->and(data_get($this->server->server_metadata, 'transfer.target_instance_url'))->toBe('https://coolify-b.test') + ->and(data_get($this->server->server_metadata, 'transfer.target_instance_url'))->toBe('https://8.8.8.8') + ->and(data_get($this->server->server_metadata, 'transfer.claimed_by_marker'))->toBeTrue() ->and((bool) $this->server->settings->force_disabled)->toBeTrue() ->and((bool) $this->server->settings->is_sentinel_enabled)->toBeFalse(); }); @@ -288,3 +292,67 @@ test('a GitHub webhook does not queue a runner for a transferred server', functi expect(GithubRunnerExecution::count())->toBe(0); Queue::assertPushed(ProvisionGithubRunnerJob::class, 1); }); + +test('an owner marker that is not newer than the own claim, or has no http(s) URL, does not release the server', function (array $marker) { + app(ServerTransferClaimer::class)->claim($this->server); + $marker = ['claim_id' => 'forged-claim', 'instance_url' => 'https://8.8.8.8', 'claimed_at' => now()->addMinute()->toIso8601String(), ...$marker]; + + $released = app(ServerTransferClaimer::class)->releaseIfClaimedElsewhere($this->server->fresh(), json_encode($marker)); + + $this->server->refresh(); + expect($released)->toBeFalse() + ->and($this->server->isTransferredAway())->toBeFalse() + ->and((bool) $this->server->settings->force_disabled)->toBeFalse() + ->and(data_get($this->server->server_metadata, 'transfer.owner_marker_conflict.claim_id'))->toBe('forged-claim'); +})->with([ + 'older claim' => [['claimed_at' => now()->subDay()->toIso8601String()]], + 'no claim time' => [['claimed_at' => null]], + 'javascript URL' => [['instance_url' => 'javascript:alert(1)']], + 'no URL' => [['instance_url' => null]], +]); + +test('a newer owner marker of a self-hosted target with a plain http URL releases the server, and the URL stays untrusted', function () { + app(ServerTransferClaimer::class)->claim($this->server); + $marker = ['claim_id' => 'target-claim', 'instance_url' => 'http://10.0.0.5:8000', 'claimed_at' => now()->addMinute()->toIso8601String()]; + + $released = app(ServerTransferClaimer::class)->releaseIfClaimedElsewhere($this->server->fresh(), json_encode($marker)); + + $this->server->refresh(); + expect($released)->toBeTrue() + ->and($this->server->isTransferredAway())->toBeTrue() + ->and(data_get($this->server->server_metadata, 'transfer.target_instance_url'))->toBe('http://10.0.0.5:8000') + ->and(data_get($this->server->server_metadata, 'transfer.claimed_by_marker'))->toBeTrue(); +}); + +test('claim gives Sentinel a new token', function () { + $tokenBefore = $this->server->settings->sentinel_token; + + app(ServerTransferClaimer::class)->claim($this->server); + + expect($this->server->fresh()->settings->sentinel_token)->not->toBe($tokenBefore)->not->toBeEmpty(); +}); + +test('applications that use the server only as an additional destination keep their DNS records and survive the archive', function () { + Http::fake(); + Queue::fake(); + $otherServer = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $this->privateKey->id, 'ip' => '10.77.0.11']); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->firstOrFail(); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $application = Application::factory()->create(['destination_id' => $otherDestination->id, 'destination_type' => $otherDestination->getMorphClass()]); + $application->additional_servers()->attach($this->server->id, ['standalone_docker_id' => $destination->id]); + $token = IntegrationToken::factory()->for($this->team)->create(['provider' => 'cloudflare']); + $record = ManagedDnsRecord::factory()->owned()->create(['dns_provider_zone_id' => DnsProviderZone::factory()->create(['integration_token_id' => $token->id])->id]); + $record->addReference($application); + $claimer = app(ServerTransferClaimer::class); + + $claimer->markTransferred($this->server, targetInstanceUrl: 'https://coolify-b.test'); + expect($record->fresh()->owned)->toBeTrue() + ->and(data_get($this->server->fresh()->server_metadata, 'transfer.parked_dns_record_ids'))->toBe([]); + + $claimer->deleteArchive($this->server->fresh()); + + expect(Application::find($application->id))->not->toBeNull() + ->and($application->additional_servers()->count())->toBe(0) + ->and($record->fresh()->owned)->toBeTrue() + ->and(Server::withTrashed()->find($this->server->id))->toBeNull(); +}); diff --git a/tests/Unit/ServerTransfer/ServerTransferCoverageTest.php b/tests/Unit/ServerTransfer/ServerTransferCoverageTest.php index 29ddb8df11..d2443b5ea9 100644 --- a/tests/Unit/ServerTransfer/ServerTransferCoverageTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferCoverageTest.php @@ -29,7 +29,15 @@ use App\Models\ServiceApplication; use App\Models\ServiceDatabase; use App\Models\SharedEnvironmentVariable; use App\Models\SslCertificate; +use App\Models\StandaloneClickhouse; use App\Models\StandaloneDocker; +use App\Models\StandaloneDragonfly; +use App\Models\StandaloneKeydb; +use App\Models\StandaloneMariadb; +use App\Models\StandaloneMongodb; +use App\Models\StandaloneMysql; +use App\Models\StandalonePostgresql; +use App\Models\StandaloneRedis; use App\Models\SwarmDocker; use App\Models\Tag; use App\Models\Team; @@ -232,6 +240,21 @@ test('every exported value arrives unchanged on the target', function () { ServiceApplication::class => ['fqdn' => 'https://fuzz-service.example.test', 'domain_port_overrides' => null, 'noindex_domains' => []], GithubRunnerConfig::class => ['labels' => ['fuzz-label'], 'max_runners' => 5, 'runner_image' => 'ghcr.io/fuzz/runner:2', 'cpu_limit' => '1.5', 'memory_limit' => '2g', 'capacity_wait_timeout' => 33, 'idle_timeout' => 44, 'job_timeout' => 55], LocalFileVolume::class => ['fs_path' => '/data/coolify/applications/fuzz/app.conf', 'mount_path' => '/etc/fuzz/app.conf', 'chown' => '1000:1000', 'chmod' => '0640'], + // The import validates these values like the API does. + GithubApp::class => ['html_url' => 'https://github.fuzz.example.test', 'api_url' => 'https://api.github.fuzz.example.test'], + GitlabApp::class => ['html_url' => 'https://gitlab.fuzz.example.test', 'api_url' => 'https://gitlab.fuzz.example.test/api/v4'], + GiteaApp::class => ['html_url' => 'https://gitea.fuzz.example.test', 'api_url' => 'https://gitea.fuzz.example.test/api/v1'], + ScheduledTask::class => ['frequency' => '*/7 * * * *'], + ScheduledDatabaseBackup::class => ['frequency' => '0 3 * * *'], + ScheduledVolumeBackup::class => ['frequency' => '0 4 * * *'], + StandalonePostgresql::class => ['postgres_user' => 'fuzz_user', 'postgres_db' => 'fuzz_db', 'ports_mappings' => '15432:5432', 'public_port' => 15432], + StandaloneMongodb::class => ['mongo_initdb_root_username' => 'fuzz_user', 'mongo_initdb_database' => 'fuzz_db', 'ports_mappings' => '17017:27017', 'public_port' => 17017], + StandaloneMysql::class => ['mysql_user' => 'fuzz_user', 'mysql_database' => 'fuzz_db', 'ports_mappings' => '13306:3306', 'public_port' => 13306], + StandaloneMariadb::class => ['mariadb_user' => 'fuzz_user', 'mariadb_database' => 'fuzz_db', 'ports_mappings' => '13307:3306', 'public_port' => 13307], + StandaloneClickhouse::class => ['clickhouse_admin_user' => 'fuzz_user', 'ports_mappings' => '18123:8123', 'public_port' => 18123], + StandaloneRedis::class => ['ports_mappings' => '16379:6379', 'public_port' => 16379], + StandaloneKeydb::class => ['ports_mappings' => '16380:6379', 'public_port' => 16380], + StandaloneDragonfly::class => ['ports_mappings' => '16381:6379', 'public_port' => 16381], ]; foreach (array_keys(ServerTransferCoverage::TRANSFERRED) as $class) { $class::query()->withoutGlobalScopes()->get() diff --git a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php index b322aa3ff6..63f416eb6b 100644 --- a/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferExporterImporterTest.php @@ -487,6 +487,8 @@ test('the patch check frequency moves with the server and the last patch check r }); test('a transfer back replaces the archive of the same team in the import transaction', function () { + // The claim restarts Sentinel with its new token; this test is about the resource containers. + Queue::fake(); $bundle = $this->exporter->export($this->server); $bundle['server']['name'] = 'changed-on-the-other-instance'; app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'https://coolify-b.test'); @@ -506,6 +508,17 @@ test('a transfer back replaces the archive of the same team in the import transa Process::assertDidntRun(fn ($process) => str_contains($process->command, 'docker rm') || str_contains($process->command, 'docker stop')); }); +test('the import check of a transfer back needs a free server slot, because the archive does not use one', function () { + config()->set('constants.coolify.self_hosted', false); + $this->team->update(['custom_server_limit' => 1]); + app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'https://coolify-b.test'); + Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $this->server->private_key_id, 'ip' => '203.0.113.77']); + + $limitCheck = collect($this->importer->importChecks($this->server->uuid, $this->server->ip, $this->team->id))->firstWhere('field', 'server'); + + expect($limitCheck['ok'])->toBeFalse(); +}); + test('a failed transfer back keeps the archive', function () { $bundle = $this->exporter->export($this->server); $bundle['projects'][0]['environments'][0]['databases'][] = [ @@ -1340,7 +1353,7 @@ test('adopt mode stops the import when a resource uuid is already used', functio $this->server->forceDelete(); expect(fn () => $this->importer->import($bundle, teamId: $this->team->id, adoptMode: true)) - ->toThrow(ValidationException::class, "The application UUID {$appUuid} is already used"); + ->toThrow(ValidationException::class, "The application UUID {$appUuid} cannot be used on this instance."); expect(Server::where('uuid', $bundle['server']['uuid'])->exists())->toBeFalse(); }); @@ -1850,6 +1863,7 @@ test('the GitHub runner config moves disabled with its GitHub App and the bundle 'On GitHub, open the settings of GitHub App Runner App. Set the Webhook URL to {target_url}/webhooks/source/github/events, the Setup URL to {target_url}/webhooks/source/github/install, and the Callback URL to {target_url}/login/github/app.', 'In GitLab, open Settings > Webhooks of every project that uses GitLab App Team GitLab, and set the webhook URL to {target_url}/webhooks/source/gitlab/events.', 'After the GitHub App sends webhooks to {target_url}, open this server there, check the runner requirements, and enable GitHub runners. Enabling them stops runners that another Coolify instance started on this server.', + 'This instance keeps a copy of the SSH key of the server. On {target_url}, add a new SSH key to the server and select it there, then remove the old public key from ~/.ssh/authorized_keys on the server.', ]) ->and(implode(' ', $bundle['warnings']))->not->toContain('webhook URL') ->and(implode(' ', $bundle['warnings']))->not->toContain('runner'); diff --git a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php index 2c94b5cb1e..c2aecbc444 100644 --- a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php @@ -8,8 +8,13 @@ use App\Models\GithubApp; use App\Models\InstanceSettings; use App\Models\PrivateKey; use App\Models\Project; +use App\Models\ScheduledDatabaseBackup; +use App\Models\ScheduledDatabaseBackupExecution; +use App\Models\ScheduledTask; +use App\Models\ScheduledTaskExecution; use App\Models\Server; use App\Models\StandaloneDocker; +use App\Models\StandalonePostgresql; use App\Models\Team; use App\Models\User; use App\Services\ServerTransfer\ServerTransferClaimer; @@ -418,7 +423,7 @@ test('start runs every check and logs which are OK and which failed', function ( ], 422)]); expect(fn () => app(ServerTransferMigrator::class)->start($this->server, 'https://8.8.8.8', 'token')) - ->toThrow(RuntimeException::class, '2 of 10 checks failed'); + ->toThrow(RuntimeException::class, '2 of 13 checks failed'); $log = collect(data_get($this->server->fresh()->server_metadata, 'migration.log')); expect($log->where('level', 'success')->pluck('message'))->toContain('OK: the target instance accepts the API token.') @@ -707,3 +712,60 @@ test('start notes system-wide git sources that the target does not have', functi 'the target has no matching App' => [true], 'the target has the App' => [false], ]); + +test('a transfer back token has only the import abilities, expires within an hour, and survives the claim', function () { + $user = User::factory()->create(); + $this->team->members()->attach($user->id, ['role' => 'owner']); + app(ServerTransferClaimer::class)->markTransferred($this->server, targetInstanceUrl: 'https://8.8.8.8'); + Http::fake(['https://8.8.8.8/api/v1/servers/*/migrate' => Http::response(['migration' => ['status' => 'running']], 202)]); + + app(ServerTransferMigrator::class)->requestTransferBack($this->server->fresh(), $user, 'https://8.8.8.8', 'token-of-b', 'https://coolify-a.test'); + + $token = $user->tokens()->sole(); + expect($token->abilities)->toBe(['write', 'deploy']) + ->and($token->expires_at->lte(now()->addHour()))->toBeTrue(); + + // The other instance still checks the import status with the token after the import claims the server. + Queue::fake(); + app(ServerTransferClaimer::class)->claim($this->server->fresh()); + + expect($user->tokens()->count())->toBe(1); +}); + +test('start fails while a backup or a scheduled task runs on the server', function (string $kind) { + Queue::fake(); + Http::fake(['*/api/v1/servers/import/check*' => Http::response(['message' => 'ok', 'checks' => []])]); + $application = Application::where('name', 'app-on-server')->sole(); + if ($kind === 'database backup') { + $database = StandalonePostgresql::withoutEvents(fn () => StandalonePostgresql::forceCreate([ + 'uuid' => new_public_id(), 'name' => 'busy-db', 'postgres_password' => 'secret', + 'environment_id' => $application->environment_id, 'destination_id' => $application->destination_id, 'destination_type' => $application->destination_type, + ])); + $backup = ScheduledDatabaseBackup::create([ + 'enabled' => true, 'frequency' => '0 0 * * *', 'save_s3' => false, 'team_id' => $this->team->id, + 'database_type' => $database->getMorphClass(), 'database_id' => $database->id, + ]); + ScheduledDatabaseBackupExecution::create(['scheduled_database_backup_id' => $backup->id, 'status' => 'running']); + } else { + $task = ScheduledTask::factory()->create(['team_id' => $this->team->id, 'application_id' => $application->id]); + ScheduledTaskExecution::create(['scheduled_task_id' => $task->id, 'status' => 'running']); + } + + expect(fn () => app(ServerTransferMigrator::class)->start($this->server, 'https://8.8.8.8', 'token')) + ->toThrow(RuntimeException::class, "1 {$kind}(s) on this server are running"); + Queue::assertNotPushed(MigrateServerToInstanceJob::class); +})->with(['database backup', 'scheduled task']); + +test('migrate keeps only an https link to the server on the target', function (string $serverUrl, ?string $kept) { + Http::fake(['https://8.8.8.8/api/v1/servers/import' => Http::response([ + 'server_uuid' => $this->server->uuid, 'claimed' => true, 'warnings' => [], 'server_url' => $serverUrl, + ], 201)]); + + $result = app(ServerTransferMigrator::class)->migrate($this->server, 'https://8.8.8.8', 'token'); + + expect($result['target_server_url'])->toBe($kept); +})->with([ + 'https' => ['https://coolify-b.example.com/server/abc', 'https://coolify-b.example.com/server/abc'], + 'javascript' => ['javascript:alert(document.cookie)', null], + 'plain http' => ['http://coolify-b.example.com/server/abc', null], +]); diff --git a/tests/Unit/ServerTransfer/TransferFinishChecklistTest.php b/tests/Unit/ServerTransfer/TransferFinishChecklistTest.php index 61d912da4f..ad6491152f 100644 --- a/tests/Unit/ServerTransfer/TransferFinishChecklistTest.php +++ b/tests/Unit/ServerTransfer/TransferFinishChecklistTest.php @@ -153,7 +153,7 @@ test('I finished closes the list and keeps the open steps as skipped', function $finish = $this->checklist->of($this->server->fresh()); expect($finish['finished_by'])->toBe($this->user->id) - ->and($finish['skipped'])->toBe(['sentinel:server']) + ->and($finish['skipped'])->toBe(['sentinel:server', 'previous_ssh_key:server']) ->and($this->checklist->isOpen($this->server->fresh()))->toBeFalse(); }); @@ -162,14 +162,15 @@ test('the panel shows the open steps by group, collapses done steps, and finishe Livewire::test(TransferFinish::class, ['server' => $this->server]) ->assertSee('Finish the transfer') - ->assertSee('0 of 2 done.') + ->assertSee('0 of 3 done.') ->assertSee('Server') ->assertSee('Restart Sentinel') ->call('markStepDone', 'sentinel:server') - ->assertSee('1 of 2 done.') + ->assertSee('1 of 3 done.') ->assertSee('1 step is OK or done') ->assertSee('Finish with open steps') ->call('markStepDone', 'server_connection:server') + ->call('markStepDone', 'previous_ssh_key:server') ->assertDontSee('Finish with open steps') ->call('finishTransfer') ->assertDontSee('Finish the transfer'); @@ -257,12 +258,12 @@ test('restarting Sentinel waits for its data instead of failing the step at once test('a changed step is broadcast to the team, and the open panel shows it without a reload', function () { Event::fake([TransferFinishChanged::class]); $this->checklist->start($this->server); - $component = Livewire::test(TransferFinish::class, ['server' => $this->server])->assertSee('0 of 2 done.'); + $component = Livewire::test(TransferFinish::class, ['server' => $this->server])->assertSee('0 of 3 done.'); $this->checklist->markDone($this->server->fresh(), 'sentinel:server'); Event::assertDispatched(TransferFinishChanged::class, fn ($event) => $event->serverUuid === $this->server->uuid && $event->teamId === $this->team->id); - $component->call('handleTransferFinishChanged', ['serverUuid' => $this->server->uuid])->assertSee('1 of 2 done.'); + $component->call('handleTransferFinishChanged', ['serverUuid' => $this->server->uuid])->assertSee('1 of 3 done.'); }); test('the GitHub App steps link to the App settings at GitHub', function () { @@ -276,3 +277,38 @@ test('the GitHub App steps link to the App settings at GitHub', function () { ->assertSeeHtml('href="https://github.com/organizations/acme/settings/apps/deploy-bot" target="_blank"') ->assertSee('Open on GitHub'); }); + +test('a step key is passed to Livewire as a JavaScript string, so a quote in a domain cannot run script', function () { + // parse_url keeps quotes in a host, so a domain step can have one in its key. + $host = (string) parse_url("https://x'-alert(1)-'.example.test", PHP_URL_HOST); + $items = $this->checklist->start($this->server); + $metadata = $this->server->fresh()->server_metadata; + $metadata['transfer']['finish']['items'][] = [...$items[0], 'key' => "domain:{$host}", 'type' => 'domain', 'subject' => $host, 'label' => "Domain {$host}"]; + $this->server->forceFill(['server_metadata' => $metadata])->save(); + + $html = Livewire::test(TransferFinish::class, ['server' => $this->server->fresh()])->html(); + + expect($html)->toContain("checkStep('domain:x\\u0027-alert(1)-\\u0027.example.test')") + ->and($html)->not->toContain("Step('domain:x'"); +}); + +test('the SSH key of the previous instance is a manual step', function () { + $item = collect($this->checklist->start($this->server))->firstWhere('key', 'previous_ssh_key:server'); + + expect($item)->not->toBeNull() + ->and($this->checklist->canCheck($item))->toBeFalse() + ->and($this->checklist->linkOf($item))->toBe(["/server/{$this->server->uuid}/private-key", 'Open private key']); + + $this->checklist->markDone($this->server, 'previous_ssh_key:server'); + expect(finishTestItem($this->server, 'previous_ssh_key:server')['status'])->toBe('done'); +}); + +test('a GitHub App link that is not a web URL, or a saved link to another host, is not shown', function () { + finishTestApplication($this, ['source_type' => GithubApp::class, 'source_id' => $this->githubApp->id]); + $items = collect($this->checklist->start($this->server)); + $this->githubApp->forceFill(['html_url' => 'javascript:alert(1)//'])->save(); + + expect($this->checklist->linkOf($items->firstWhere('key', "github_app_webhook:{$this->githubApp->uuid}")))->toBeNull() + ->and($this->checklist->linkOf(['type' => 'git_source', 'link' => '//evil.example/x']))->toBeNull() + ->and($this->checklist->linkOf(['type' => 'git_source', 'link' => 'javascript:alert(1)']))->toBeNull(); +}); diff --git a/tests/Unit/ServerTransfer/TransferredServerHostFieldsTest.php b/tests/Unit/ServerTransfer/TransferredServerHostFieldsTest.php new file mode 100644 index 0000000000..d38e951947 --- /dev/null +++ b/tests/Unit/ServerTransfer/TransferredServerHostFieldsTest.php @@ -0,0 +1,251 @@ +set('app.maintenance.store', 'array'); + InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true, 'instance_timezone' => 'UTC', 'fqdn' => 'https://coolify.test']); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + $this->privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]); + $this->otherKey = PrivateKey::factory()->create(['team_id' => $this->team->id, 'private_key' => generateSSHKey('ed25519')['private']]); + $this->cloudToken = CloudProviderToken::factory()->create(['team_id' => $this->team->id, 'provider' => 'hetzner']); + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + 'ip' => '203.0.113.10', + 'user' => 'root', + 'port' => 22, + 'hetzner_server_id' => 4242, + 'cloud_provider_token_id' => $this->cloudToken->id, + ]); + + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); +}); + +function archiveServerForHostFields(Server $server): Server +{ + app(ServerTransferClaimer::class)->markTransferred($server, targetInstanceUrl: 'https://target.test'); + + return Server::query()->with('settings')->findOrFail($server->id); +} + +function patchServerHostFields(object $test, Team $team, User $user, Server $server, array $payload): TestResponse +{ + app('auth')->forgetGuards(); + $token = $user->createToken('write-token', ['write']); + $token->accessToken->forceFill(['team_id' => $team->id])->save(); + + return $test->withHeaders(['Authorization' => 'Bearer '.$token->plainTextToken, 'Accept' => 'application/json']) + ->patchJson('/api/v1/servers/'.$server->uuid, $payload); +} + +test('the server page refuses a new host for an archive', function (string $field, mixed $value) { + $server = archiveServerForHostFields($this->server); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->set($field, $value) + ->call('submit') + ->assertDispatched('error') + ->assertNotDispatched('success'); + + $server->refresh(); + expect($server->ip)->toBe('203.0.113.10') + ->and($server->user)->toBe('root') + ->and((int) $server->port)->toBe(22); +})->with([ + 'ip' => ['ip', '198.51.100.9'], + 'user' => ['user', 'admin'], + 'port' => ['port', 2222], +]); + +test('the server page still saves the name of an archive', function () { + $server = archiveServerForHostFields($this->server); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->set('name', 'old-host') + ->call('submit') + ->assertDispatched('success'); + + expect($server->fresh()->name)->toBe('old-host'); +}); + +test('the server page still saves a new host for a managed server', function () { + Livewire::test(Show::class, ['server_uuid' => $this->server->uuid]) + ->set('ip', '198.51.100.9') + ->set('port', 2222) + ->call('submit') + ->assertDispatched('success'); + + expect($this->server->fresh()->ip)->toBe('198.51.100.9') + ->and((int) $this->server->fresh()->port)->toBe(2222); +}); + +test('the private key page refuses a new key for an archive', function () { + $server = archiveServerForHostFields($this->server); + + Livewire::test(PrivateKeyShow::class, ['server_uuid' => $server->uuid]) + ->call('setPrivateKey', $this->otherKey->id) + ->assertDispatched('error') + ->assertNotDispatched('success'); + + expect($server->fresh()->private_key_id)->toBe($this->privateKey->id); + Process::assertNothingRan(); +}); + +test('the API refuses a new host or key for an archive', function (string $field) { + $server = archiveServerForHostFields($this->server); + $payload = [ + 'ip' => '198.51.100.9', + 'user' => 'admin', + 'port' => 2222, + 'private_key_uuid' => $this->otherKey->uuid, + ]; + + patchServerHostFields($this, $this->team, $this->user, $server, [$field => $payload[$field]]) + ->assertStatus(422) + ->assertJsonValidationErrors([$field]); + + $server->refresh(); + expect($server->ip)->toBe('203.0.113.10') + ->and($server->user)->toBe('root') + ->and((int) $server->port)->toBe(22) + ->and($server->private_key_id)->toBe($this->privateKey->id); +})->with(['ip', 'user', 'port', 'private_key_uuid']); + +test('the API accepts unchanged host fields and other changes for an archive', function () { + $server = archiveServerForHostFields($this->server); + + patchServerHostFields($this, $this->team, $this->user, $server, [ + 'name' => 'old-host', + 'ip' => '203.0.113.10', + 'user' => 'root', + 'port' => 22, + 'private_key_uuid' => $this->privateKey->uuid, + ])->assertCreated(); + + expect($server->fresh()->name)->toBe('old-host'); +}); + +test('the API still changes the host of a managed server', function () { + patchServerHostFields($this, $this->team, $this->user, $this->server, ['ip' => '198.51.100.9', 'port' => 2222]) + ->assertCreated(); + + expect($this->server->fresh()->ip)->toBe('198.51.100.9') + ->and((int) $this->server->fresh()->port)->toBe(2222); +}); + +test('enable management uses the server validation limit on Cloud', function () { + config()->set('constants.coolify.self_hosted', false); + instanceSettings()->forceFill(['server_validation_max_failures_per_team' => 1])->save(); + $this->team->forceFill(['custom_server_limit' => 5])->save(); + $server = archiveServerForHostFields($this->server); + Server::throttleValidation($this->team->id, '192.0.2.1'); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->call('enableManagement') + ->assertDispatched('error') + ->assertNotDispatched('success'); + + $server->refresh(); + expect($server->isTransferredAway())->toBeTrue() + ->and((bool) $server->settings->force_disabled)->toBeTrue(); + Process::assertNothingRan(); +}); + +test('the cloud provider state of an archive is not refreshed', function () { + Http::fake(); + $this->server->forceFill([ + 'ip' => '1.2.3.4', + 'hetzner_server_status' => 'running', + 'vultr_instance_id' => 'vultr-1', + 'vultr_instance_status' => 'active', + 'digitalocean_droplet_id' => 77, + 'digitalocean_droplet_status' => 'active', + 'hostinger_virtual_machine_id' => 88, + 'hostinger_virtual_machine_status' => 'running', + ])->save(); + $server = archiveServerForHostFields($this->server); + + expect($server->refreshHetznerState())->toBe('running') + ->and($server->refreshVultrState())->toBe('active') + ->and($server->refreshDigitalOceanState())->toBe('active') + ->and($server->refreshHostingerState())->toBe('running') + ->and($server->backfillPlaceholderIp('198.51.100.9'))->toBeFalse() + ->and($server->fresh()->ip)->toBe('1.2.3.4'); + Http::assertNothingSent(); +}); + +test('a member cannot make the server page call the cloud provider for an archive', function () { + Http::fake(); + $member = User::factory()->create(); + $this->team->members()->attach($member->id, ['role' => 'member']); + $this->server->forceFill(['vultr_instance_id' => 'vultr-1', 'digitalocean_droplet_id' => 77, 'hostinger_virtual_machine_id' => 88])->save(); + $server = archiveServerForHostFields($this->server); + $this->actingAs($member); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->call('checkHetznerServerStatus', true) + ->call('checkVultrInstanceStatus', true) + ->call('checkDigitalOceanDropletStatus', true) + ->call('checkHostingerVirtualMachineStatus', true); + + Http::assertNothingSent(); +}); + +test('the server page does not search or link a cloud server for an archive', function () { + Http::fake(); + $server = archiveServerForHostFields($this->server); + $server->forceFill(['hetzner_server_id' => null, 'cloud_provider_token_id' => null])->save(); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->set('selectedHetznerTokenId', $this->cloudToken->id) + ->call('searchHetznerServer') + ->set('manualHetznerServerId', 9999) + ->call('searchHetznerServerById') + ->set('matchedHetznerServer', ['id' => 9999]) + ->call('linkToHetzner'); + + expect($server->fresh()->hetzner_server_id)->toBeNull(); + Http::assertNothingSent(); +}); + +test('the Cloudflare tunnel of an archive cannot be turned on', function () { + $archive = archiveServerForHostFields($this->server); + + Livewire::test(CloudflareTunnel::class, ['server_uuid' => $archive->uuid]) + ->call('manualCloudflareConfig') + ->assertDispatched('error'); + + expect((bool) $archive->settings->fresh()->is_cloudflare_tunnel)->toBeFalse(); +});