From ecb14e6cd52e928620a8a29b51d375981268b911 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:51:22 +0100 Subject: [PATCH] fix(server): allow servers with the same IP and different SSH ports Closes #7573 Co-Authored-By: Claude Opus 5.5 --- .../Api/ServerTransferController.php | 2 +- .../Controllers/Api/ServersController.php | 12 +- app/Livewire/Boarding/Index.php | 5 +- app/Livewire/Server/New/ByIp.php | 5 +- app/Livewire/Server/Show.php | 13 +- app/Models/Server.php | 13 ++ .../ServerTransfer/ServerTransferImporter.php | 19 +-- tests/Feature/Api/ServerTransferApiTest.php | 18 ++- .../Feature/ResourceAccessConsistencyTest.php | 2 +- .../Feature/Server/ServerIpUniquenessTest.php | 127 +++++++++++++++++- .../Server/ServerIpv6NormalizationTest.php | 2 +- .../Feature/ServerUsernameValidationTest.php | 2 +- 12 files changed, 186 insertions(+), 34 deletions(-) diff --git a/app/Http/Controllers/Api/ServerTransferController.php b/app/Http/Controllers/Api/ServerTransferController.php index 0dc0229a30..6d7e366f03 100644 --- a/app/Http/Controllers/Api/ServerTransferController.php +++ b/app/Http/Controllers/Api/ServerTransferController.php @@ -249,7 +249,7 @@ class ServerTransferController extends Controller 'message' => $schemaVersion === ServerTransferBundle::SCHEMA_VERSION ? null : 'This instance reads transfer bundle version '.ServerTransferBundle::SCHEMA_VERSION.", but the source sends version {$schemaVersion}. Upgrade both instances to the same Coolify version.", ], - ...collect($this->importer->importChecks((string) $request->query('server_uuid'), $ip, $teamId)) + ...collect($this->importer->importChecks((string) $request->query('server_uuid'), $ip, $teamId, serverPort: $port)) ->map(fn (array $check) => collect($check)->only(['label', 'ok', 'message'])->all()) ->all(), [ diff --git a/app/Http/Controllers/Api/ServersController.php b/app/Http/Controllers/Api/ServersController.php index 60ccf064a2..ce4b5f8f68 100644 --- a/app/Http/Controllers/Api/ServersController.php +++ b/app/Http/Controllers/Api/ServersController.php @@ -620,9 +620,8 @@ class ServersController extends Controller if (! $privateKey) { return response()->json(['message' => 'Private key not found.'], 404); } - $foundServer = ModelsServer::whereIp(ModelsServer::normalizeIp($request->ip))->first(); - if ($foundServer) { - return response()->json(['message' => 'A server with this IP/Domain is already in use.'], 400); + if (ModelsServer::sshEndpointInUse($request->ip, $request->port)) { + return response()->json(['message' => 'A server with this IP/Domain and SSH port is already in use.'], 400); } if ($request->instant_validate) { ModelsServer::throttleValidation($teamId, $request->ip); @@ -812,6 +811,13 @@ class ServersController extends Controller 'errors' => $connectionErrors, ], 422); } + $newIp = $request->filled('ip') ? $request->input('ip') : $server->getRawOriginal('ip'); + $newPort = $request->filled('port') ? (int) $request->input('port') : (int) $server->port; + $endpointChanged = ModelsServer::normalizeIp($newIp) !== ModelsServer::normalizeIp($server->getRawOriginal('ip')) + || $newPort !== (int) $server->port; + if ($endpointChanged && ModelsServer::sshEndpointInUse($newIp, $newPort, $server->id)) { + return response()->json(['message' => 'A server with this IP/Domain and SSH port is already in use.'], 400); + } if ($request->instant_validate) { ModelsServer::throttleValidation($teamId, $request->ip ?? $server->ip); } diff --git a/app/Livewire/Boarding/Index.php b/app/Livewire/Boarding/Index.php index a875c383e8..885c3c22e2 100644 --- a/app/Livewire/Boarding/Index.php +++ b/app/Livewire/Boarding/Index.php @@ -302,9 +302,8 @@ class Index extends Component $this->validate(); - $foundServer = Server::whereIp(Server::normalizeIp($this->remoteServerHost))->first(); - if ($foundServer) { - return $this->dispatch('error', 'A server with this IP/Domain already exists.'); + if (Server::sshEndpointInUse($this->remoteServerHost, $this->remoteServerPort)) { + return $this->dispatch('error', 'A server with this IP/Domain and SSH port already exists.'); } $privateKeyId = $this->createdPrivateKey?->id ?? $this->selectedExistingPrivateKey; $this->createdPrivateKey = PrivateKey::ownedByCurrentTeam(['team_id'])->findOrFail($privateKeyId); diff --git a/app/Livewire/Server/New/ByIp.php b/app/Livewire/Server/New/ByIp.php index d3bb69ae64..44985ecdd2 100644 --- a/app/Livewire/Server/New/ByIp.php +++ b/app/Livewire/Server/New/ByIp.php @@ -144,9 +144,8 @@ class ByIp extends Component try { $this->authorize('create', Server::class); $this->ip = Server::normalizeIp($this->ip); - $foundServer = Server::whereIp($this->ip)->first(); - if ($foundServer) { - return $this->dispatch('error', 'A server with this IP/Domain already exists.'); + if (Server::sshEndpointInUse($this->ip, $this->port)) { + return $this->dispatch('error', 'A server with this IP/Domain and SSH port already exists.'); } if (is_null($this->private_key_id)) { diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index bf5d519018..c8080ac36e 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -250,14 +250,13 @@ class Show extends Component $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) - ->where('id', '!=', $this->server->id) - ->first() : null; - if ($foundServer) { + // Check only a changed IP or port, so that an old duplicate does not block saving other fields. + $endpointChanged = $this->ip !== Server::normalizeIp($this->server->getRawOriginal('ip')) + || (int) $this->port !== (int) $this->server->port; + if ($endpointChanged && Server::sshEndpointInUse($this->ip, $this->port, $this->server->id)) { $this->ip = $this->server->ip; - throw new \Exception('A server with this IP/Domain already exists.'); + $this->port = $this->server->port; + throw new \Exception('A server with this IP/Domain and SSH port already exists.'); } $this->server->name = $this->name; diff --git a/app/Models/Server.php b/app/Models/Server.php index 4e07d9818c..d46a100965 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -2055,6 +2055,19 @@ $siteAddress { { return Attribute::make( get: function () { + /** + * Whether a server of any team already uses this SSH endpoint. The port is part of the endpoint, + * so servers behind one NAT address with different forwarded SSH ports are different servers. + */ + public static function sshEndpointInUse(?string $ip, int|string|null $port, ?int $exceptServerId = null): bool + { + return self::query() + ->where('ip', self::normalizeIp($ip)) + ->where('port', (int) ($port ?? 22)) + ->when($exceptServerId !== null, fn ($query) => $query->whereKeyNot($exceptServerId)) + ->exists(); + } + if (isDev()) { return '127.0.0.1'; } diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 0746c3f39e..7a9816bce7 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -201,14 +201,15 @@ class ServerTransferImporter $serverUuid = (string) data_get($bundle, 'server.uuid'); $serverIp = (string) data_get($bundle, 'server.ip'); + $serverPort = (int) data_get($bundle, 'server.port', 22); // A server that comes back replaces the archived copy that this instance kept from the first transfer. $archive = $preserveUuids ? $this->replaceableArchive($serverUuid, $teamId) : null; $uuidConflict = $archive === null && $preserveUuids && Server::withTrashed()->where('uuid', $serverUuid)->exists(); - $ipConflict = Server::where('ip', Server::normalizeIp($serverIp))->when($archive, fn ($query) => $query->whereKeyNot($archive->id))->exists(); + $ipConflict = Server::sshEndpointInUse($serverIp, $serverPort, $archive?->id); if (! $dryRun) { - $this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids); + $this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids, $serverPort); // The import validates the server over SSH, so it uses the same Cloud limit as a new server. Server::throttleValidation($teamId, $serverIp); } @@ -217,7 +218,7 @@ class ServerTransferImporter $warnings[] = "A server with UUID {$serverUuid} already exists on this instance."; } if ($ipConflict) { - $warnings[] = "A server with IP/domain {$serverIp} already exists on this instance."; + $warnings[] = "A server with IP/domain {$serverIp} and SSH port {$serverPort} already exists on this instance."; } $created = [ @@ -503,7 +504,7 @@ class ServerTransferImporter * * @return list */ - public function importChecks(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true): array + public function importChecks(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true, int $serverPort = 22): array { $team = Team::find($teamId); $archive = $preserveUuids ? $this->replaceableArchive($serverUuid, $teamId) : null; @@ -518,7 +519,7 @@ class ServerTransferImporter default => "A server with UUID {$serverUuid} already exists on this instance. Delete/transfer it first, or import with preserve_uuids=false.", }; - $ipExists = Server::where('ip', Server::normalizeIp($serverIp))->when($archive, fn ($query) => $query->whereKeyNot($archive->id))->exists(); + $ipExists = Server::sshEndpointInUse($serverIp, $serverPort, $archive?->id); return [ [ @@ -535,9 +536,9 @@ class ServerTransferImporter ], [ 'field' => 'server.ip', - 'label' => 'no server with the same IP exists', + 'label' => 'no server with the same IP and SSH port exists', 'ok' => ! $ipExists, - 'message' => $ipExists ? "A server with IP/domain {$serverIp} already exists on this instance. Remove it first." : null, + 'message' => $ipExists ? "A server with IP/domain {$serverIp} and SSH port {$serverPort} already exists on this instance. Remove it first." : null, ], ]; } @@ -572,9 +573,9 @@ class ServerTransferImporter /** * @throws ValidationException with the first failed import check */ - public function assertCanImport(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true): void + public function assertCanImport(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true, int $serverPort = 22): void { - foreach ($this->importChecks($serverUuid, $serverIp, $teamId, $preserveUuids) as $check) { + foreach ($this->importChecks($serverUuid, $serverIp, $teamId, $preserveUuids, $serverPort) as $check) { if (! $check['ok']) { throw ValidationException::withMessages([$check['field'] => [$check['message']]]); } diff --git a/tests/Feature/Api/ServerTransferApiTest.php b/tests/Feature/Api/ServerTransferApiTest.php index ee10ca3192..4558e060ed 100644 --- a/tests/Feature/Api/ServerTransferApiTest.php +++ b/tests/Feature/Api/ServerTransferApiTest.php @@ -634,7 +634,7 @@ describe('GET /api/v1/servers/import/check', function () { test('lists every check with its result when a server already exists on this instance', function () { $listener = stream_socket_server('tcp://127.0.0.1:0'); $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); - $this->server->forceFill(['ip' => '127.0.0.1'])->save(); + $this->server->forceFill(['ip' => '127.0.0.1', 'port' => $port])->save(); $checks = $this->withHeaders(transferHeaders($this->importToken)) ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}") @@ -645,12 +645,26 @@ describe('GET /api/v1/servers/import/check', function () { expect(collect($checks)->pluck('ok', 'label')->all())->toMatchArray([ 'the transfer bundle version is supported' => true, 'no server with the same UUID exists' => true, - 'no server with the same IP exists' => false, + 'no server with the same IP and SSH port exists' => false, "the target can reach the SSH port of the server (127.0.0.1:{$port})" => true, ]); fclose($listener); }); + test('accepts a server with the IP of an existing server and a different SSH port', function () { + $listener = stream_socket_server('tcp://127.0.0.1:0'); + $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); + $this->server->forceFill(['ip' => '127.0.0.1', 'port' => 22])->save(); + + $checks = $this->withHeaders(transferHeaders($this->importToken)) + ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}") + ->assertOk() + ->json('checks'); + + expect(collect($checks)->pluck('ok', 'label')->get('no server with the same IP and SSH port exists'))->toBeTrue(); + fclose($listener); + }); + test('on Cloud, does not probe private, loopback, or link-local addresses', function (string $ip) { config()->set('constants.coolify.self_hosted', false); diff --git a/tests/Feature/ResourceAccessConsistencyTest.php b/tests/Feature/ResourceAccessConsistencyTest.php index 2511e14342..406186fafd 100644 --- a/tests/Feature/ResourceAccessConsistencyTest.php +++ b/tests/Feature/ResourceAccessConsistencyTest.php @@ -68,7 +68,7 @@ test('server creation returns a consistent duplicate address response', function $ownResponse->assertBadRequest(); $otherResponse->assertBadRequest(); expect($ownResponse->json('message')) - ->toBe('A server with this IP/Domain is already in use.') + ->toBe('A server with this IP/Domain and SSH port is already in use.') ->toBe($otherResponse->json('message')); }); diff --git a/tests/Feature/Server/ServerIpUniquenessTest.php b/tests/Feature/Server/ServerIpUniquenessTest.php index 1c54fe0f10..82195c8b77 100644 --- a/tests/Feature/Server/ServerIpUniquenessTest.php +++ b/tests/Feature/Server/ServerIpUniquenessTest.php @@ -1,18 +1,22 @@ 0]); + Process::fake(); + InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]); $this->user = User::factory()->create(); $this->team = Team::factory()->create(); @@ -23,6 +27,10 @@ beforeEach(function () { $this->privateKey = PrivateKey::factory()->create([ 'team_id' => $this->team->id, ]); + + $token = $this->user->createToken('write-token', ['write']); + $token->accessToken->forceFill(['team_id' => $this->team->id])->save(); + $this->token = $token->plainTextToken; }); it('detects duplicate ip within the same team', function () { @@ -66,7 +74,7 @@ it('rejects a duplicate ip from the same team when adding a server', function () ->set('ip', '1.2.3.4') ->set('private_key_id', $this->privateKey->id) ->call('submit') - ->assertDispatched('error', 'A server with this IP/Domain already exists.'); + ->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.'); expect(Server::where('ip', '1.2.3.4')->count())->toBe(1); }); @@ -86,7 +94,7 @@ it('rejects a duplicate ip from another team without revealing the owner team', ->set('ip', '5.6.7.8') ->set('private_key_id', $this->privateKey->id) ->call('submit') - ->assertDispatched('error', 'A server with this IP/Domain already exists.') + ->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.') ->assertNotDispatched('error', 'A server with this IP/Domain is already in use by another team.'); expect(Server::where('ip', '5.6.7.8')->count())->toBe(1); @@ -116,3 +124,116 @@ it('enforces global uniqueness not just team-scoped', function () { ->first(); expect($teamScopedServer)->toBeNull(); }); + +it('adds a server with the IP of another server and a different SSH port', function () { + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'port' => 22, + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + Livewire::test(ByIp::class, [ + 'private_keys' => collect([$this->privateKey]), + 'limit_reached' => false, + ]) + ->set('name', 'nat-vm-2') + ->set('ip', '1.2.3.4') + ->set('port', 2222) + ->set('private_key_id', $this->privateKey->id) + ->call('submit') + ->assertNotDispatched('error'); + + expect(Server::where('ip', '1.2.3.4')->orderBy('port')->pluck('port')->all())->toBe([22, 2222]); +}); + +it('rejects a server with the IP and SSH port of a server of another team', function () { + Server::factory()->create([ + 'ip' => '5.6.7.8', + 'port' => 2222, + 'team_id' => Team::factory()->create()->id, + ]); + + Livewire::test(ByIp::class, [ + 'private_keys' => collect([$this->privateKey]), + 'limit_reached' => false, + ]) + ->set('ip', '5.6.7.8') + ->set('port', 2222) + ->set('private_key_id', $this->privateKey->id) + ->call('submit') + ->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.'); + + expect(Server::where('ip', '5.6.7.8')->count())->toBe(1); +}); + +it('rejects changing the SSH port of a server to the endpoint of another server', function () { + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'port' => 2222, + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + $server = Server::factory()->create([ + 'ip' => '1.2.3.4', + 'port' => 22, + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + Livewire::test(Show::class, ['server_uuid' => $server->uuid]) + ->set('port', '2222') + ->call('submit') + ->assertDispatched('error') + ->assertNotDispatched('success'); + + expect($server->fresh()->port)->toBe(22); +}); + +it('creates a server through the API with the IP of another server and a different SSH port', function () { + Auth::forgetGuards(); + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'port' => 22, + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + $payload = [ + 'name' => 'nat-vm', + 'ip' => '1.2.3.4', + 'private_key_uuid' => $this->privateKey->uuid, + ]; + + $this->withToken($this->token)->postJson('/api/v1/servers', $payload) + ->assertBadRequest() + ->assertJsonPath('message', 'A server with this IP/Domain and SSH port is already in use.'); + $this->withToken($this->token)->postJson('/api/v1/servers', [...$payload, 'port' => 2222]) + ->assertCreated(); + + expect(Server::where('ip', '1.2.3.4')->orderBy('port')->pluck('port')->all())->toBe([22, 2222]); +}); + +it('rejects an API update to the IP and SSH port of another server', function () { + Auth::forgetGuards(); + Server::factory()->create([ + 'ip' => '1.2.3.4', + 'port' => 2222, + 'team_id' => Team::factory()->create()->id, + ]); + $server = Server::factory()->create([ + 'ip' => '9.9.9.9', + 'port' => 22, + 'team_id' => $this->team->id, + 'private_key_id' => $this->privateKey->id, + ]); + + $this->withToken($this->token)->patchJson('/api/v1/servers/'.$server->uuid, ['ip' => '1.2.3.4', 'port' => 2222]) + ->assertBadRequest() + ->assertJsonPath('message', 'A server with this IP/Domain and SSH port is already in use.'); + expect($server->fresh()->only(['ip', 'port']))->toBe(['ip' => '9.9.9.9', 'port' => 22]); + + $this->withToken($this->token)->patchJson('/api/v1/servers/'.$server->uuid, ['ip' => '1.2.3.4']) + ->assertCreated(); + expect($server->fresh()->ip)->toBe('1.2.3.4'); +}); diff --git a/tests/Feature/Server/ServerIpv6NormalizationTest.php b/tests/Feature/Server/ServerIpv6NormalizationTest.php index 3397f98b65..9c2e560d57 100644 --- a/tests/Feature/Server/ServerIpv6NormalizationTest.php +++ b/tests/Feature/Server/ServerIpv6NormalizationTest.php @@ -89,7 +89,7 @@ it('treats IPv6 notations as the same IP when adding a server', function () { ->set('ip', '2001:0DB8::1') ->set('private_key_id', $this->privateKey->id) ->call('submit') - ->assertDispatched('error', 'A server with this IP/Domain already exists.'); + ->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.'); expect(Server::query()->where('ip', '2001:db8::1')->count())->toBe(1); }); diff --git a/tests/Feature/ServerUsernameValidationTest.php b/tests/Feature/ServerUsernameValidationTest.php index d3b4343740..5997a5a9ad 100644 --- a/tests/Feature/ServerUsernameValidationTest.php +++ b/tests/Feature/ServerUsernameValidationTest.php @@ -191,6 +191,6 @@ it('does not disclose another team through the duplicate server IP message', fun Livewire::test(ByIp::class, ['private_keys' => collect([$this->privateKey])]) ->set('ip', $ip) ->call('submit') - ->assertDispatched('error', 'A server with this IP/Domain already exists.'); + ->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.'); } });