mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(server-transfer): harden import on Cloud and stop archives from using server slots
- Rate limit the import check probe and refuse private, loopback, and link-local targets on Cloud. - Rate limit the SSH validation that an import starts, like other server validations. - Count only managed servers for the server limit, the limit check job, and the subscription page. Archives of transferred servers are left out. - Do not show the name of another team's archived server in the import check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
30d840f1c6
commit
95a3a22fdf
@@ -6,13 +6,16 @@ use App\Http\Controllers\Controller;
|
||||
use App\Models\GithubApp;
|
||||
use App\Models\GitlabApp;
|
||||
use App\Models\Server;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Rules\ValidServerIp;
|
||||
use App\Services\ServerTransfer\ServerTransferBundle;
|
||||
use App\Services\ServerTransfer\ServerTransferClaimer;
|
||||
use App\Services\ServerTransfer\ServerTransferImporter;
|
||||
use App\Services\ServerTransfer\ServerTransferMigrator;
|
||||
use Illuminate\Http\Exceptions\ThrottleRequestsException;
|
||||
use Illuminate\Http\JsonResponse;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
use Illuminate\Validation\ValidationException;
|
||||
use OpenApi\Attributes as OA;
|
||||
use Throwable;
|
||||
@@ -217,7 +220,12 @@ class ServerTransferController extends Controller
|
||||
|
||||
// The target connects to the server over SSH when it takes over management, so an unreachable
|
||||
// server would leave an imported server here that this instance cannot manage.
|
||||
$connection = @fsockopen($ip, $port, timeout: 5);
|
||||
// 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.
|
||||
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;
|
||||
if ($connection !== false) {
|
||||
fclose($connection);
|
||||
}
|
||||
@@ -235,8 +243,11 @@ class ServerTransferController extends Controller
|
||||
[
|
||||
'label' => "the target can reach the SSH port of the server ({$ip}:{$port})",
|
||||
'ok' => $connection !== false,
|
||||
'message' => $connection !== false ? null
|
||||
: "This instance cannot reach the server at {$ip}:{$port}. Allow SSH from this instance to the server, then transfer again.",
|
||||
'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.",
|
||||
default => "This instance cannot reach the server at {$ip}:{$port}. Allow SSH from this instance to the server, then transfer again.",
|
||||
},
|
||||
],
|
||||
];
|
||||
|
||||
@@ -319,6 +330,8 @@ class ServerTransferController extends Controller
|
||||
preserveUuids: $request->boolean('preserve_uuids', true),
|
||||
adoptMode: $request->boolean('adopt_mode', true),
|
||||
);
|
||||
} catch (ThrottleRequestsException $e) {
|
||||
throw $e;
|
||||
} catch (Throwable $e) {
|
||||
$payload = ['message' => $e->getMessage()];
|
||||
if ($e instanceof ValidationException) {
|
||||
|
||||
@@ -31,7 +31,7 @@ class ServerLimitCheckJob implements ShouldBeEncrypted, ShouldQueue
|
||||
public function handle()
|
||||
{
|
||||
try {
|
||||
$servers = $this->team->servers;
|
||||
$servers = $this->team->managedServers;
|
||||
$servers_count = $servers->count();
|
||||
$number_of_servers_to_disable = $servers_count - $this->team->limits;
|
||||
if ($number_of_servers_to_disable > 0) {
|
||||
|
||||
@@ -1333,6 +1333,16 @@ $siteAddress {
|
||||
return $this->proxy->modelScope();
|
||||
}
|
||||
|
||||
/**
|
||||
* Servers that this instance manages. A transferred server is only an archive and does not use a server slot.
|
||||
*/
|
||||
public function scopeNotTransferredAway(Builder $query): Builder
|
||||
{
|
||||
return $query->where(fn (Builder $query) => $query
|
||||
->whereNull('server_metadata->transfer->status')
|
||||
->orWhere('server_metadata->transfer->status', '!=', 'transferred'));
|
||||
}
|
||||
|
||||
public function scopeWhereProxyType(Builder $query, string $proxyType): Builder
|
||||
{
|
||||
return $query->where('proxy->type', $proxyType);
|
||||
|
||||
+10
-2
@@ -158,7 +158,7 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen
|
||||
return true;
|
||||
}
|
||||
$serverLimit = Team::serverLimit($team);
|
||||
$servers = $team->servers()->count();
|
||||
$servers = $team->managedServers()->count();
|
||||
|
||||
return $servers >= $serverLimit;
|
||||
}
|
||||
@@ -192,7 +192,7 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen
|
||||
|
||||
public function serverOverflow()
|
||||
{
|
||||
if (Team::serverLimit($this) < $this->servers->count()) {
|
||||
if (Team::serverLimit($this) < $this->managedServers()->count()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -392,6 +392,14 @@ class Team extends Model implements SendsDiscord, SendsEmail, SendsPushover, Sen
|
||||
return $this->hasMany(Server::class);
|
||||
}
|
||||
|
||||
/**
|
||||
* Servers that use a server slot. Archives of transferred servers are left out.
|
||||
*/
|
||||
public function managedServers()
|
||||
{
|
||||
return $this->servers()->notTransferredAway();
|
||||
}
|
||||
|
||||
public function usesSwarm(): bool
|
||||
{
|
||||
return $this->servers()
|
||||
|
||||
@@ -206,6 +206,8 @@ class ServerTransferImporter
|
||||
|
||||
if (! $dryRun) {
|
||||
$this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids);
|
||||
// The import validates the server over SSH, so it uses the same Cloud limit as a new server.
|
||||
Server::throttleValidation($teamId, $serverIp);
|
||||
}
|
||||
|
||||
if ($uuidConflict) {
|
||||
@@ -509,7 +511,7 @@ class ServerTransferImporter
|
||||
$uuidMessage = match (true) {
|
||||
$existingServer === null => null,
|
||||
$existingServer->trashed() => "The server with UUID {$serverUuid} is still being deleted on this instance. Wait a minute, then transfer again.",
|
||||
$existingServer->isTransferredAway() => "Another team of this instance has the archived copy of server {$existingServer->name}. Transfer the server into that team, or delete the archive there first.",
|
||||
$existingServer->isTransferredAway() => 'Another team of this instance has the archived copy of this server. Transfer the server into that team, or delete the archive there first.',
|
||||
default => "A server with UUID {$serverUuid} already exists on this instance. Delete/transfer it first, or import with preserve_uuids=false.",
|
||||
};
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
<div wire:init="loadRefundEligibility" class="application-settings-workspace flex flex-col gap-6" x-data="{
|
||||
qty: {{ $quantity }},
|
||||
get current() { return $wire.server_limits; },
|
||||
activeServers: {{ currentTeam()->servers->count() }},
|
||||
activeServers: {{ currentTeam()->managedServers()->count() }},
|
||||
preview: @js($pricePreview),
|
||||
loading: false,
|
||||
showModal: false,
|
||||
|
||||
@@ -241,6 +241,23 @@ describe('POST /api/v1/servers/import', function () {
|
||||
->assertJsonValidationErrors(['preserve_uuids']);
|
||||
});
|
||||
|
||||
test('on Cloud, rate limits the SSH validation that the import starts', function () {
|
||||
config()->set('constants.coolify.self_hosted', false);
|
||||
$this->team->update(['custom_server_limit' => 10]);
|
||||
InstanceSettings::findOrFail(0)->forceFill(['server_validation_max_failures_per_ip' => 1])->save();
|
||||
Server::throttleValidation($this->team->id, '10.66.0.20');
|
||||
|
||||
$export = transferExportForApiTest($this->server);
|
||||
$this->server->forceDelete();
|
||||
|
||||
$this->withHeaders(transferHeaders($this->sensitiveToken))
|
||||
->postJson('/api/v1/servers/import', ['bundle' => $export])
|
||||
->assertStatus(429);
|
||||
|
||||
expect(Server::where('uuid', $export['server']['uuid'])->exists())->toBeFalse();
|
||||
Queue::assertNotPushed(ValidateAndInstallServerJob::class);
|
||||
});
|
||||
|
||||
test('rejects import when the destination team is at its cloud server limit', function () {
|
||||
config()->set('constants.coolify.self_hosted', false);
|
||||
$this->team->update(['custom_server_limit' => 1]);
|
||||
@@ -475,6 +492,30 @@ describe('GET /api/v1/servers/import/check', function () {
|
||||
fclose($listener);
|
||||
});
|
||||
|
||||
test('on Cloud, does not probe private, loopback, or link-local addresses', function (string $ip) {
|
||||
config()->set('constants.coolify.self_hosted', false);
|
||||
|
||||
$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'));
|
||||
})->with(['127.0.0.1', '10.0.0.5', '169.254.169.254', '::1', 'localhost']);
|
||||
|
||||
test('on Cloud, rate limits the reachability probe', function () {
|
||||
config()->set('constants.coolify.self_hosted', false);
|
||||
InstanceSettings::findOrFail(0)->forceFill(['server_validation_max_failures_per_team' => 2])->save();
|
||||
|
||||
foreach (['10.0.0.5', '10.0.0.6'] as $ip) {
|
||||
$this->withHeaders(transferHeaders($this->importToken))
|
||||
->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip={$ip}")
|
||||
->assertStatus(422);
|
||||
}
|
||||
|
||||
$this->withHeaders(transferHeaders($this->importToken))
|
||||
->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.0.0.7')
|
||||
->assertStatus(429);
|
||||
});
|
||||
|
||||
test('rejects a token without the write permission', function () {
|
||||
$this->withHeaders(transferHeaders($this->readToken))
|
||||
->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.66.0.99')
|
||||
|
||||
@@ -81,3 +81,23 @@ it('does not change servers when under limit and none are force-disabled', funct
|
||||
expect($server1->fresh()->settings->force_disabled)->toBeFalsy();
|
||||
expect($server2->fresh()->settings->force_disabled)->toBeFalsy();
|
||||
});
|
||||
|
||||
it('does not count archives of transferred servers against the limit', function () {
|
||||
$this->team->update(['custom_server_limit' => 1]);
|
||||
|
||||
$live = createServerForTeam($this->team);
|
||||
sleep(1);
|
||||
$archive = createServerForTeam($this->team);
|
||||
$archive->forceFill(['server_metadata' => ['transfer' => ['status' => 'transferred']]])->save();
|
||||
|
||||
ServerLimitCheckJob::dispatchSync($this->team);
|
||||
|
||||
expect($live->fresh()->settings->force_disabled)->toBeFalsy()
|
||||
->and(Team::serverLimitReached($this->team))->toBeTrue()
|
||||
->and($this->team->serverOverflow())->toBeFalse();
|
||||
Notification::assertNothingSent();
|
||||
|
||||
$live->forceFill(['server_metadata' => ['transfer' => ['status' => 'transferred']]])->save();
|
||||
|
||||
expect(Team::serverLimitReached($this->team))->toBeFalse();
|
||||
});
|
||||
|
||||
@@ -525,7 +525,10 @@ test('the archive of another team is not replaced', function () {
|
||||
$otherTeam = Team::factory()->create();
|
||||
|
||||
expect(fn () => $this->importer->import($bundle, teamId: $otherTeam->id))
|
||||
->toThrow(ValidationException::class, 'Another team of this instance has the archived copy');
|
||||
->toThrow(ValidationException::class, 'Another team of this instance has the archived copy of this server.');
|
||||
|
||||
$uuidCheck = collect($this->importer->importChecks($this->server->uuid, '203.0.113.50', $otherTeam->id))->firstWhere('field', 'server.uuid');
|
||||
expect($uuidCheck['message'])->not->toContain($this->server->name);
|
||||
});
|
||||
|
||||
test('import rolls back all created rows when a later resource fails', function () {
|
||||
|
||||
Reference in New Issue
Block a user