From 0732e74553b68fbd6707a22685541bc563bfd95a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:15:30 +0200 Subject: [PATCH] fix(server-transfer): require deploy permission for import checks Require write and deploy abilities on the import check endpoint, and clarify the target token permissions in transfer messages and UI. --- .../ServerTransfer/ServerTransferMigrator.php | 4 ++-- .../views/livewire/server/transfer.blade.php | 2 +- routes/api.php | 2 +- tests/Feature/Api/ServerTransferApiTest.php | 19 +++++++++++++------ .../ServerTransferMigratorTest.php | 2 +- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/Services/ServerTransfer/ServerTransferMigrator.php b/app/Services/ServerTransfer/ServerTransferMigrator.php index bb51b563f5..1c8922c8d5 100644 --- a/app/Services/ServerTransfer/ServerTransferMigrator.php +++ b/app/Services/ServerTransfer/ServerTransferMigrator.php @@ -598,7 +598,7 @@ class ServerTransferMigrator $token = trim(substr($token, 7)); } if ($token === '') { - throw new RuntimeException('A target instance API token is required (root or write + create servers).'); + throw new RuntimeException('A target instance API token is required (root or write + deploy).'); } return $token; @@ -697,7 +697,7 @@ class ServerTransferMigrator $gates = [ [404, 'the target instance supports server transfer', 'The target instance does not support server transfer. Upgrade it to Coolify '.config('constants.coolify.version').' or newer.'], [401, 'the target instance accepts the API token', 'The target instance does not accept the API token. Create a new token on the target instance and paste it here.'], - [403, 'the API token can create servers on the target instance', 'Use a token with the write permission from an admin or owner. Target response: '.$message], + [403, 'the API token can create servers on the target instance', 'Use a token with the root permission, or the write and deploy permissions, from an admin or owner. Target response: '.$message], ]; foreach ($gates as [$failStatus, $label, $failMessage]) { if ($status === $failStatus) { diff --git a/resources/views/livewire/server/transfer.blade.php b/resources/views/livewire/server/transfer.blade.php index a28757a968..900e04f8be 100644 --- a/resources/views/livewire/server/transfer.blade.php +++ b/resources/views/livewire/server/transfer.blade.php @@ -94,7 +94,7 @@ helper="Base URL of the other Coolify instance (no trailing path). It must use https." /> + helper="Create a token on the target with root (or write + deploy). It is only used for this request." />
middleware(['api.ability:deploy']); Route::post('/servers', [ServersController::class, 'create_server'])->middleware(['api.ability:write']); - Route::get('/servers/import/check', [ServerTransferController::class, 'check'])->middleware(['api.ability:write']); + Route::get('/servers/import/check', [ServerTransferController::class, 'check'])->middleware(['api.ability:write', 'api.ability:deploy']); Route::post('/servers/import', [ServerTransferController::class, 'import'])->middleware(['api.ability:write', 'api.ability:deploy']); Route::get('/servers/{uuid}/transfer', [ServerTransferController::class, 'status'])->middleware(['api.ability:read,write']); Route::post('/servers/{uuid}/transfer/complete', [ServerTransferController::class, 'complete'])->middleware(['api.ability:write']); diff --git a/tests/Feature/Api/ServerTransferApiTest.php b/tests/Feature/Api/ServerTransferApiTest.php index 134802cd09..c1d9aa536a 100644 --- a/tests/Feature/Api/ServerTransferApiTest.php +++ b/tests/Feature/Api/ServerTransferApiTest.php @@ -46,6 +46,7 @@ beforeEach(function () { $this->sensitiveToken = $this->user->createToken('transfer-sensitive', ['*', 'read:sensitive'])->plainTextToken; $this->readToken = $this->user->createToken('transfer-read', ['read'])->plainTextToken; $this->writeToken = $this->user->createToken('transfer-write', ['read', 'write'])->plainTextToken; + $this->importToken = $this->user->createToken('transfer-import', ['read', 'write', 'deploy'])->plainTextToken; $this->privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]); $this->server = Server::factory()->create([ @@ -394,7 +395,7 @@ describe('GET /api/v1/servers/import/check', function () { $listener = stream_socket_server('tcp://127.0.0.1:0'); $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); - $this->withHeaders(transferHeaders($this->writeToken)) + $this->withHeaders(transferHeaders($this->importToken)) ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}") ->assertOk() ->assertJsonPath('checks', fn ($checks) => collect($checks)->every(fn ($check) => $check['ok'] === true) @@ -408,14 +409,14 @@ describe('GET /api/v1/servers/import/check', function () { $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); fclose($listener); - $this->withHeaders(transferHeaders($this->writeToken)) + $this->withHeaders(transferHeaders($this->importToken)) ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}") ->assertStatus(422) ->assertJsonPath('message', fn ($message) => str_contains($message, 'cannot reach the server at 127.0.0.1')); }); test('rejects a target that is not an IP address or hostname before it opens a socket', function (string $ip) { - $this->withHeaders(transferHeaders($this->writeToken)) + $this->withHeaders(transferHeaders($this->importToken)) ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip='.urlencode($ip)) ->assertStatus(422) ->assertJsonValidationErrors('ip') @@ -423,7 +424,7 @@ describe('GET /api/v1/servers/import/check', function () { })->with(['tcp://127.0.0.1', 'unix:///var/run/docker.sock', 'ssl://example.com']); test('rejects another transfer bundle version', function () { - $this->withHeaders(transferHeaders($this->writeToken)) + $this->withHeaders(transferHeaders($this->importToken)) ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&schema_version=999') ->assertStatus(422) ->assertJsonPath('message', fn ($message) => str_contains($message, 'Upgrade both instances')); @@ -434,7 +435,7 @@ describe('GET /api/v1/servers/import/check', function () { $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); $this->server->forceFill(['ip' => '127.0.0.1'])->save(); - $checks = $this->withHeaders(transferHeaders($this->writeToken)) + $checks = $this->withHeaders(transferHeaders($this->importToken)) ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}") ->assertStatus(422) ->assertJsonPath('message', fn ($message) => str_contains($message, 'already exists on this instance')) @@ -455,6 +456,12 @@ describe('GET /api/v1/servers/import/check', function () { ->assertForbidden(); }); + test('rejects a token without the deploy permission that the import needs', function () { + $this->withHeaders(transferHeaders($this->writeToken)) + ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.66.0.99') + ->assertForbidden(); + }); + test('rejects an invalid token', function () { $this->withHeaders(transferHeaders('1|not-a-real-token')) ->getJson('/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=10.66.0.99') @@ -467,7 +474,7 @@ test('the import check lists the system-wide git sources that this instance does $listener = stream_socket_server('tcp://127.0.0.1:0'); $port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1); - $this->withHeaders(transferHeaders($this->writeToken)) + $this->withHeaders(transferHeaders($this->importToken)) ->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}&system_github_apps[]={$present->uuid}&system_github_apps[]=missing-uuid&system_gitlab_apps[]=other-missing") ->assertOk() ->assertJsonPath('missing_system_sources.github', ['missing-uuid']) diff --git a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php index 1abdd22c38..914b416170 100644 --- a/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php +++ b/tests/Unit/ServerTransfer/ServerTransferMigratorTest.php @@ -351,7 +351,7 @@ test('start checks the target before it queues the transfer', function (int $sta && $request->hasHeader('Authorization', 'Bearer wrong-token')); })->with([ 'invalid token' => [401, 'does not accept the API token'], - 'token without write permission' => [403, 'write permission'], + 'token without the write and deploy permissions' => [403, 'write and deploy permissions'], 'conflict on the target' => [422, 'already exists on this instance'], 'target without server transfer' => [404, 'does not support server transfer'], ]);