From 59111e8cf35824b691790cc018d76d2c5a331793 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 22 May 2026 12:53:14 +0200 Subject: [PATCH] fix(destination): scope server and network selection to current team Resolve the server and network in Destination::addServer() and ::promote() through ownedByCurrentTeam() before use, authorize the update against the resource, and pass the validated IDs into attach()/detach()/update(). Errors are routed through handleError() to match the sibling removeServer() method. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/Livewire/Project/Shared/Destination.php | 38 ++++-- .../CrossTeamDestinationAttachTest.php | 124 ++++++++++++++++++ 2 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 tests/Feature/CrossTeamDestinationAttachTest.php diff --git a/app/Livewire/Project/Shared/Destination.php b/app/Livewire/Project/Shared/Destination.php index 363471760..d9e560074 100644 --- a/app/Livewire/Project/Shared/Destination.php +++ b/app/Livewire/Project/Shared/Destination.php @@ -110,15 +110,23 @@ public function redeploy(int $network_id, int $server_id) public function promote(int $network_id, int $server_id) { - $main_destination = $this->resource->destination; - $this->resource->update([ - 'destination_id' => $network_id, - 'destination_type' => StandaloneDocker::class, - ]); - $this->resource->additional_networks()->detach($network_id, ['server_id' => $server_id]); - $this->resource->additional_networks()->attach($main_destination->id, ['server_id' => $main_destination->server->id]); - $this->refreshServers(); - $this->resource->refresh(); + try { + $server = Server::ownedByCurrentTeam()->findOrFail($server_id); + $network = StandaloneDocker::ownedByCurrentTeam()->findOrFail($network_id); + $this->authorize('update', $this->resource); + + $main_destination = $this->resource->destination; + $this->resource->update([ + 'destination_id' => $network->id, + 'destination_type' => StandaloneDocker::class, + ]); + $this->resource->additional_networks()->detach($network->id, ['server_id' => $server->id]); + $this->resource->additional_networks()->attach($main_destination->id, ['server_id' => $main_destination->server->id]); + $this->refreshServers(); + $this->resource->refresh(); + } catch (\Exception $e) { + return handleError($e, $this); + } } public function refreshServers() @@ -130,8 +138,16 @@ public function refreshServers() public function addServer(int $network_id, int $server_id) { - $this->resource->additional_networks()->attach($network_id, ['server_id' => $server_id]); - $this->dispatch('refresh'); + try { + $server = Server::ownedByCurrentTeam()->findOrFail($server_id); + $network = StandaloneDocker::ownedByCurrentTeam()->findOrFail($network_id); + $this->authorize('update', $this->resource); + + $this->resource->additional_networks()->attach($network->id, ['server_id' => $server->id]); + $this->dispatch('refresh'); + } catch (\Exception $e) { + return handleError($e, $this); + } } public function removeServer(int $network_id, int $server_id, $password, $selectedActions = []) diff --git a/tests/Feature/CrossTeamDestinationAttachTest.php b/tests/Feature/CrossTeamDestinationAttachTest.php new file mode 100644 index 000000000..74c287107 --- /dev/null +++ b/tests/Feature/CrossTeamDestinationAttachTest.php @@ -0,0 +1,124 @@ + InstanceSettings::query()->create(['id' => 0])); + + // Attacker: Team A + $this->userA = User::factory()->create(); + $this->teamA = Team::factory()->create(); + $this->userA->teams()->attach($this->teamA, ['role' => 'owner']); + + $this->serverA = Server::factory()->create(['team_id' => $this->teamA->id]); + $this->projectA = Project::factory()->create(['team_id' => $this->teamA->id]); + $this->environmentA = Environment::factory()->create(['project_id' => $this->projectA->id]); + $this->destinationA = StandaloneDocker::factory()->create([ + 'server_id' => $this->serverA->id, + 'name' => 'dest-a-'.fake()->unique()->word(), + 'network' => 'coolify-a-'.fake()->unique()->word(), + ]); + + $this->applicationA = Application::factory()->create([ + 'environment_id' => $this->environmentA->id, + 'destination_id' => $this->destinationA->id, + 'destination_type' => StandaloneDocker::class, + ]); + + // A second usable destination on Team A's own server, used for positive-path tests. + $this->serverA2 = Server::factory()->create(['team_id' => $this->teamA->id]); + $this->destinationA2 = StandaloneDocker::factory()->create([ + 'server_id' => $this->serverA2->id, + 'name' => 'dest-a2-'.fake()->unique()->word(), + 'network' => 'coolify-a2-'.fake()->unique()->word(), + ]); + + // Victim: Team B + $this->userB = User::factory()->create(); + $this->teamB = Team::factory()->create(); + $this->userB->teams()->attach($this->teamB, ['role' => 'owner']); + + $this->serverB = Server::factory()->create(['team_id' => $this->teamB->id]); + $this->destinationB = StandaloneDocker::factory()->create([ + 'server_id' => $this->serverB->id, + 'name' => 'dest-b-'.fake()->unique()->word(), + 'network' => 'coolify-b-'.fake()->unique()->word(), + ]); + + // Act as attacker (Team A) + $this->actingAs($this->userA); + session(['currentTeam' => $this->teamA]); +}); + +describe('Destination::addServer GHSA-j395-3pqh-9r5g', function () { + test('cannot attach another team\'s server + network to own application', function () { + try { + Livewire::test(Destination::class, ['resource' => $this->applicationA]) + ->call('addServer', $this->destinationB->id, $this->serverB->id); + } catch (Throwable $e) { + // handleError on ModelNotFoundException calls abort(404); pivot assertion is source of truth. + } + + expect($this->applicationA->fresh()->additional_networks)->toHaveCount(0); + expect($this->applicationA->fresh()->additional_servers)->toHaveCount(0); + }); + + test('cannot attach own network paired with another team\'s server', function () { + try { + Livewire::test(Destination::class, ['resource' => $this->applicationA]) + ->call('addServer', $this->destinationA2->id, $this->serverB->id); + } catch (Throwable $e) { + } + + expect($this->applicationA->fresh()->additional_networks)->toHaveCount(0); + }); + + test('cannot attach another team\'s network paired with own server', function () { + try { + Livewire::test(Destination::class, ['resource' => $this->applicationA]) + ->call('addServer', $this->destinationB->id, $this->serverA2->id); + } catch (Throwable $e) { + } + + expect($this->applicationA->fresh()->additional_networks)->toHaveCount(0); + }); + + test('can attach own team\'s server + network to own application', function () { + Livewire::test(Destination::class, ['resource' => $this->applicationA]) + ->call('addServer', $this->destinationA2->id, $this->serverA2->id); + + $additional = $this->applicationA->fresh()->additional_networks; + expect($additional)->toHaveCount(1); + expect($additional->first()->id)->toBe($this->destinationA2->id); + expect($additional->first()->pivot->server_id)->toBe($this->serverA2->id); + }); +}); + +describe('Destination::promote GHSA-j395-3pqh-9r5g', function () { + test('cannot promote another team\'s network as the application\'s main destination', function () { + $originalDestinationId = $this->applicationA->destination_id; + + try { + Livewire::test(Destination::class, ['resource' => $this->applicationA]) + ->call('promote', $this->destinationB->id, $this->serverB->id); + } catch (Throwable $e) { + } + + expect($this->applicationA->fresh()->destination_id)->toBe($originalDestinationId); + }); +});