diff --git a/app/Http/Controllers/Api/ServersController.php b/app/Http/Controllers/Api/ServersController.php index a29839d14..29c6b854a 100644 --- a/app/Http/Controllers/Api/ServersController.php +++ b/app/Http/Controllers/Api/ServersController.php @@ -290,9 +290,12 @@ public function domains_by_server(Request $request) } $uuid = $request->get('uuid'); if ($uuid) { - $domains = Application::getDomainsByUuid($uuid); + $application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $uuid)->first(); + if (! $application) { + return response()->json(['message' => 'Application not found.'], 404); + } - return response()->json(serializeApiResponse($domains)); + return response()->json(serializeApiResponse($application->fqdns)); } $projects = Project::where('team_id', $teamId)->get(); $domains = collect(); diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index b4d8d9204..121c8ee03 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -686,8 +686,6 @@ private function deploy_docker_compose_buildpack() // Inject build arguments after build subcommand if not using build secrets if (! $this->application->settings->use_build_secrets && $this->build_args instanceof \Illuminate\Support\Collection && $this->build_args->isNotEmpty()) { $build_args_string = $this->build_args->implode(' '); - // Escape single quotes for bash -c context used by executeInDocker - $build_args_string = str_replace("'", "'\\''", $build_args_string); // Inject build args right after 'build' subcommand (not at the end) $original_command = $build_command; @@ -699,9 +697,17 @@ private function deploy_docker_compose_buildpack() } } - $this->execute_remote_command( - [executeInDocker($this->deployment_uuid, "cd {$this->basedir} && {$build_command}"), 'hidden' => true], - ); + try { + $this->execute_remote_command( + [executeInDocker($this->deployment_uuid, "cd {$this->basedir} && {$build_command}"), 'hidden' => true], + ); + } catch (\RuntimeException $e) { + if (str_contains($e->getMessage(), "matching `'") || str_contains($e->getMessage(), 'unexpected EOF')) { + throw new DeploymentException("Custom build command failed due to shell syntax error. Please check your command for special characters (like unmatched quotes): {$this->docker_compose_custom_build_command}"); + } + + throw $e; + } } else { $command = "{$this->coolify_variables} docker compose"; // Prepend DOCKER_BUILDKIT=1 if BuildKit is supported @@ -718,8 +724,6 @@ private function deploy_docker_compose_buildpack() if (! $this->application->settings->use_build_secrets && $this->build_args instanceof \Illuminate\Support\Collection && $this->build_args->isNotEmpty()) { $build_args_string = $this->build_args->implode(' '); - // Escape single quotes for bash -c context used by executeInDocker - $build_args_string = str_replace("'", "'\\''", $build_args_string); $command .= " {$build_args_string}"; $this->application_deployment_queue->addLogEntry('Adding build arguments to Docker Compose build command.'); } @@ -765,9 +769,18 @@ private function deploy_docker_compose_buildpack() ); $this->write_deployment_configurations(); - $this->execute_remote_command( - [executeInDocker($this->deployment_uuid, "cd {$this->workdir} && {$start_command}"), 'hidden' => true], - ); + + try { + $this->execute_remote_command( + [executeInDocker($this->deployment_uuid, "cd {$this->workdir} && {$start_command}"), 'hidden' => true], + ); + } catch (\RuntimeException $e) { + if (str_contains($e->getMessage(), "matching `'") || str_contains($e->getMessage(), 'unexpected EOF')) { + throw new DeploymentException("Custom start command failed due to shell syntax error. Please check your command for special characters (like unmatched quotes): {$this->docker_compose_custom_start_command}"); + } + + throw $e; + } } else { $this->write_deployment_configurations(); $this->docker_compose_location = '/docker-compose.yaml'; diff --git a/app/Livewire/Project/Shared/ResourceOperations.php b/app/Livewire/Project/Shared/ResourceOperations.php index 4ba961dfd..e769e4bcb 100644 --- a/app/Livewire/Project/Shared/ResourceOperations.php +++ b/app/Livewire/Project/Shared/ResourceOperations.php @@ -49,9 +49,10 @@ public function cloneTo($destination_id) { $this->authorize('update', $this->resource); - $new_destination = StandaloneDocker::find($destination_id); + $teamScope = fn ($q) => $q->where('team_id', currentTeam()->id); + $new_destination = StandaloneDocker::whereHas('server', $teamScope)->find($destination_id); if (! $new_destination) { - $new_destination = SwarmDocker::find($destination_id); + $new_destination = SwarmDocker::whereHas('server', $teamScope)->find($destination_id); } if (! $new_destination) { return $this->addError('destination_id', 'Destination not found.'); @@ -352,7 +353,7 @@ public function moveTo($environment_id) { try { $this->authorize('update', $this->resource); - $new_environment = Environment::findOrFail($environment_id); + $new_environment = Environment::ownedByCurrentTeam()->findOrFail($environment_id); $this->resource->update([ 'environment_id' => $environment_id, ]); diff --git a/app/Models/Application.php b/app/Models/Application.php index c51ad4e81..a239c34db 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -1961,16 +1961,6 @@ public function parseHealthcheckFromDockerfile($dockerfile, bool $isInit = false } } - public static function getDomainsByUuid(string $uuid): array - { - $application = self::where('uuid', $uuid)->first(); - - if ($application) { - return $application->fqdns; - } - - return []; - } public function getLimits(): array { diff --git a/app/Policies/StandaloneDockerPolicy.php b/app/Policies/StandaloneDockerPolicy.php index 154648599..3e1f83d12 100644 --- a/app/Policies/StandaloneDockerPolicy.php +++ b/app/Policies/StandaloneDockerPolicy.php @@ -37,8 +37,7 @@ public function create(User $user): bool */ public function update(User $user, StandaloneDocker $standaloneDocker): bool { - // return $user->isAdmin() && $user->teams->contains('id', $standaloneDocker->server->team_id); - return true; + return $user->teams->contains('id', $standaloneDocker->server->team_id); } /** @@ -46,8 +45,7 @@ public function update(User $user, StandaloneDocker $standaloneDocker): bool */ public function delete(User $user, StandaloneDocker $standaloneDocker): bool { - // return $user->isAdmin() && $user->teams->contains('id', $standaloneDocker->server->team_id); - return true; + return $user->teams->contains('id', $standaloneDocker->server->team_id); } /** @@ -55,8 +53,7 @@ public function delete(User $user, StandaloneDocker $standaloneDocker): bool */ public function restore(User $user, StandaloneDocker $standaloneDocker): bool { - // return false; - return true; + return false; } /** @@ -64,7 +61,6 @@ public function restore(User $user, StandaloneDocker $standaloneDocker): bool */ public function forceDelete(User $user, StandaloneDocker $standaloneDocker): bool { - // return false; - return true; + return false; } } diff --git a/app/Policies/SwarmDockerPolicy.php b/app/Policies/SwarmDockerPolicy.php index 979bb5889..82a75910b 100644 --- a/app/Policies/SwarmDockerPolicy.php +++ b/app/Policies/SwarmDockerPolicy.php @@ -37,8 +37,7 @@ public function create(User $user): bool */ public function update(User $user, SwarmDocker $swarmDocker): bool { - // return $user->isAdmin() && $user->teams->contains('id', $swarmDocker->server->team_id); - return true; + return $user->teams->contains('id', $swarmDocker->server->team_id); } /** @@ -46,8 +45,7 @@ public function update(User $user, SwarmDocker $swarmDocker): bool */ public function delete(User $user, SwarmDocker $swarmDocker): bool { - // return $user->isAdmin() && $user->teams->contains('id', $swarmDocker->server->team_id); - return true; + return $user->teams->contains('id', $swarmDocker->server->team_id); } /** @@ -55,8 +53,7 @@ public function delete(User $user, SwarmDocker $swarmDocker): bool */ public function restore(User $user, SwarmDocker $swarmDocker): bool { - // return false; - return true; + return false; } /** @@ -64,7 +61,6 @@ public function restore(User $user, SwarmDocker $swarmDocker): bool */ public function forceDelete(User $user, SwarmDocker $swarmDocker): bool { - // return false; - return true; + return false; } } diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index 03c53989c..c522cd0ca 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -191,6 +191,10 @@ function clone_application(Application $source, $destination, array $overrides = $uuid = $overrides['uuid'] ?? (string) new Cuid2; $server = $destination->server; + if ($server->team_id !== currentTeam()->id) { + throw new \RuntimeException('Destination does not belong to the current team.'); + } + // Prepare name and URL $name = $overrides['name'] ?? 'clone-of-'.str($source->name)->limit(20).'-'.$uuid; $applicationSettings = $source->settings; diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 8212f9dc6..77e8b7b07 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -139,8 +139,9 @@ function checkMinimumDockerEngineVersion($dockerVersion) } function executeInDocker(string $containerId, string $command) { - return "docker exec {$containerId} bash -c '{$command}'"; - // return "docker exec {$this->deployment_uuid} bash -c '{$command} |& tee -a /proc/1/fd/1; [ \$PIPESTATUS -eq 0 ] || exit \$PIPESTATUS'"; + $escapedCommand = str_replace("'", "'\\''", $command); + + return "docker exec {$containerId} bash -c '{$escapedCommand}'"; } function getContainerStatus(Server $server, string $container_id, bool $all_data = false, bool $throwError = false) diff --git a/tests/Feature/DomainsByServerApiTest.php b/tests/Feature/DomainsByServerApiTest.php new file mode 100644 index 000000000..1e799bec5 --- /dev/null +++ b/tests/Feature/DomainsByServerApiTest.php @@ -0,0 +1,80 @@ +team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + + $this->token = $this->user->createToken('test-token', ['*'], $this->team->id); + $this->bearerToken = $this->token->plainTextToken; + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::factory()->create(['server_id' => $this->server->id]); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); +}); + +function authHeaders(): array +{ + return [ + 'Authorization' => 'Bearer '.test()->bearerToken, + ]; +} + +test('returns domains for own team application via uuid query param', function () { + $application = Application::factory()->create([ + 'fqdn' => 'https://my-app.example.com', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $response = $this->withHeaders(authHeaders()) + ->getJson("/api/v1/servers/{$this->server->uuid}/domains?uuid={$application->uuid}"); + + $response->assertOk(); + $response->assertJsonFragment(['my-app.example.com']); +}); + +test('returns 404 when application uuid belongs to another team', function () { + $otherTeam = Team::factory()->create(); + $otherUser = User::factory()->create(); + $otherTeam->members()->attach($otherUser->id, ['role' => 'owner']); + + $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherDestination = StandaloneDocker::factory()->create(['server_id' => $otherServer->id]); + $otherProject = Project::factory()->create(['team_id' => $otherTeam->id]); + $otherEnvironment = Environment::factory()->create(['project_id' => $otherProject->id]); + + $otherApplication = Application::factory()->create([ + 'fqdn' => 'https://secret-app.internal.company.com', + 'environment_id' => $otherEnvironment->id, + 'destination_id' => $otherDestination->id, + 'destination_type' => $otherDestination->getMorphClass(), + ]); + + $response = $this->withHeaders(authHeaders()) + ->getJson("/api/v1/servers/{$this->server->uuid}/domains?uuid={$otherApplication->uuid}"); + + $response->assertNotFound(); + $response->assertJson(['message' => 'Application not found.']); +}); + +test('returns 404 for nonexistent application uuid', function () { + $response = $this->withHeaders(authHeaders()) + ->getJson("/api/v1/servers/{$this->server->uuid}/domains?uuid=nonexistent-uuid"); + + $response->assertNotFound(); + $response->assertJson(['message' => 'Application not found.']); +}); diff --git a/tests/Feature/ResourceOperationsCrossTenantTest.php b/tests/Feature/ResourceOperationsCrossTenantTest.php new file mode 100644 index 000000000..056c7757c --- /dev/null +++ b/tests/Feature/ResourceOperationsCrossTenantTest.php @@ -0,0 +1,85 @@ +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->destinationA = StandaloneDocker::factory()->create(['server_id' => $this->serverA->id]); + $this->projectA = Project::factory()->create(['team_id' => $this->teamA->id]); + $this->environmentA = Environment::factory()->create(['project_id' => $this->projectA->id]); + + $this->applicationA = Application::factory()->create([ + 'environment_id' => $this->environmentA->id, + 'destination_id' => $this->destinationA->id, + 'destination_type' => $this->destinationA->getMorphClass(), + ]); + + // Team B (victim's team) + $this->teamB = Team::factory()->create(); + $this->serverB = Server::factory()->create(['team_id' => $this->teamB->id]); + $this->destinationB = StandaloneDocker::factory()->create(['server_id' => $this->serverB->id]); + $this->projectB = Project::factory()->create(['team_id' => $this->teamB->id]); + $this->environmentB = Environment::factory()->create(['project_id' => $this->projectB->id]); + + $this->actingAs($this->userA); + session(['currentTeam' => $this->teamA]); +}); + +test('cloneTo rejects destination belonging to another team', function () { + Livewire::test(ResourceOperations::class, ['resource' => $this->applicationA]) + ->call('cloneTo', $this->destinationB->id) + ->assertHasErrors('destination_id'); + + // Ensure no cross-tenant application was created + expect(Application::where('destination_id', $this->destinationB->id)->exists())->toBeFalse(); +}); + +test('cloneTo allows destination belonging to own team', function () { + $secondDestination = StandaloneDocker::factory()->create(['server_id' => $this->serverA->id]); + + Livewire::test(ResourceOperations::class, ['resource' => $this->applicationA]) + ->call('cloneTo', $secondDestination->id) + ->assertHasNoErrors('destination_id') + ->assertRedirect(); +}); + +test('moveTo rejects environment belonging to another team', function () { + Livewire::test(ResourceOperations::class, ['resource' => $this->applicationA]) + ->call('moveTo', $this->environmentB->id); + + // Resource should still be in original environment + $this->applicationA->refresh(); + expect($this->applicationA->environment_id)->toBe($this->environmentA->id); +}); + +test('moveTo allows environment belonging to own team', function () { + $secondEnvironment = Environment::factory()->create(['project_id' => $this->projectA->id]); + + Livewire::test(ResourceOperations::class, ['resource' => $this->applicationA]) + ->call('moveTo', $secondEnvironment->id) + ->assertRedirect(); + + $this->applicationA->refresh(); + expect($this->applicationA->environment_id)->toBe($secondEnvironment->id); +}); + +test('StandaloneDockerPolicy denies update for cross-team user', function () { + expect($this->userA->can('update', $this->destinationB))->toBeFalse(); +}); + +test('StandaloneDockerPolicy allows update for same-team user', function () { + expect($this->userA->can('update', $this->destinationA))->toBeTrue(); +}); diff --git a/tests/Unit/ExecuteInDockerEscapingTest.php b/tests/Unit/ExecuteInDockerEscapingTest.php new file mode 100644 index 000000000..14777ca1c --- /dev/null +++ b/tests/Unit/ExecuteInDockerEscapingTest.php @@ -0,0 +1,35 @@ +toBe("docker exec test-container bash -c 'ls -la /app'"); +}); + +it('escapes single quotes in command', function () { + $result = executeInDocker('test-container', "echo 'hello world'"); + + expect($result)->toBe("docker exec test-container bash -c 'echo '\\''hello world'\\'''"); +}); + +it('prevents command injection via single quote breakout', function () { + $malicious = "cd /dir && docker compose build'; id; #"; + $result = executeInDocker('test-container', $malicious); + + // The single quote in the malicious command should be escaped so it cannot break out of bash -c + // The raw unescaped pattern "build'; id;" must not appear — the quote must be escaped + expect($result)->not->toContain("build'; id;"); + expect($result)->toBe("docker exec test-container bash -c 'cd /dir && docker compose build'\\''; id; #'"); +}); + +it('handles empty command', function () { + $result = executeInDocker('test-container', ''); + + expect($result)->toBe("docker exec test-container bash -c ''"); +}); + +it('handles command with multiple single quotes', function () { + $result = executeInDocker('test-container', "echo 'a' && echo 'b'"); + + expect($result)->toBe("docker exec test-container bash -c 'echo '\\''a'\\'' && echo '\\''b'\\'''"); +});