diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 54cbfebfcd..134b705bca 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2275,6 +2275,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->application_deployment_queue->addLogEntry('Custom healthcheck found in Dockerfile.'); } if ($this->container_name) { + $escapedContainerName = escapeshellarg($this->container_name); $counter = 1; $this->application_deployment_queue->addLogEntry('Waiting for healthcheck to pass on the new container.'); if ($this->full_healthcheck_url && ! $this->application->custom_healthcheck_found) { @@ -2290,13 +2291,13 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue while ($counter <= $this->application->health_check_retries) { $this->execute_remote_command( [ - "docker inspect --format='{{json .State.Health.Status}}' {$this->container_name}", + "docker inspect --format='{{json .State.Health.Status}}' {$escapedContainerName}", 'hidden' => true, 'save' => 'health_check', 'append' => false, ], [ - "docker inspect --format='{{json .State.Health.Log}}' {$this->container_name}", + "docker inspect --format='{{json .State.Health.Log}}' {$escapedContainerName}", 'hidden' => true, 'save' => 'health_check_logs', 'append' => false, @@ -2343,11 +2344,12 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue private function query_logs() { + $escapedContainerName = escapeshellarg($this->container_name); $this->application_deployment_queue->addLogEntry('----------------------------------------'); $this->application_deployment_queue->addLogEntry('Container logs:'); $this->execute_remote_command( [ - 'command' => "docker logs -n 100 {$this->container_name}", + 'command' => "docker logs -n 100 {$escapedContainerName}", 'type' => 'stderr', 'ignore_errors' => true, ], @@ -4273,11 +4275,11 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); if ($skipRemove) { $this->execute_remote_command( - [dockerStopCommand($timeout, $containerName, $this->server), 'hidden' => true, 'ignore_errors' => true] + [dockerStopCommand($timeout, escapeshellarg($containerName), $this->server), 'hidden' => true, 'ignore_errors' => true] ); } else { $this->execute_remote_command( - [dockerStopCommand($timeout, $containerName, $this->server), 'hidden' => true, 'ignore_errors' => true] + [dockerStopCommand($timeout, escapeshellarg($containerName), $this->server), 'hidden' => true, 'ignore_errors' => true] ); $this->removeContainerWithTimeout($containerName); } diff --git a/app/Livewire/Project/Application/Advanced.php b/app/Livewire/Project/Application/Advanced.php index 288014ae61..7161358ed0 100644 --- a/app/Livewire/Project/Application/Advanced.php +++ b/app/Livewire/Project/Application/Advanced.php @@ -4,6 +4,7 @@ namespace App\Livewire\Project\Application; use App\Models\Application; use App\Models\ApplicationSetting; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\ValidationException; @@ -67,7 +68,7 @@ class Advanced extends Component #[Validate(['boolean'])] public bool $isConsistentContainerNameEnabled = false; - #[Validate(['string', 'nullable'])] + #[Validate(['nullable', 'string', 'max:255', 'regex:'.ValidationPatterns::CONTAINER_NAME_PATTERN])] public ?string $customInternalName = null; #[Validate(['string', 'nullable', 'max:'.ApplicationSetting::MAX_CONTAINER_NAME_PREFIX_LENGTH])] diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index b32870a5f7..f616dd65fe 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -141,7 +141,7 @@ function sharedDataApplications() 'gpu_device_ids' => 'string|nullable', 'gpu_options' => 'string|nullable', 'is_consistent_container_name_enabled' => 'boolean', - 'custom_internal_name' => 'string|nullable', + 'custom_internal_name' => ['nullable', ...ValidationPatterns::containerNameRules()], 'custom_container_name_prefix' => 'string|nullable|max:'.ApplicationSetting::MAX_CONTAINER_NAME_PREFIX_LENGTH, 'preview_url_template' => 'string', 'max_restart_count' => 'integer|min:0', diff --git a/tests/Feature/Api/ApplicationSettingsApiTest.php b/tests/Feature/Api/ApplicationSettingsApiTest.php index 578b70acfe..3e94ffa69c 100644 --- a/tests/Feature/Api/ApplicationSettingsApiTest.php +++ b/tests/Feature/Api/ApplicationSettingsApiTest.php @@ -347,6 +347,59 @@ test('PATCH /api/v1/applications/{uuid} updates advanced application settings', } }); +test('PATCH /api/v1/applications/{uuid} accepts Docker-compatible custom internal names', function (string $name) { + $this->withHeaders(applicationSettingsApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'custom_internal_name' => $name, + ]) + ->assertOk(); + + expect($this->application->fresh()->settings->custom_internal_name)->toBe($name); +})->with([ + 'hyphens' => 'my-app-container', + 'uppercase, underscores, and dots' => 'My_App.v2', +]); + +test('PATCH /api/v1/applications/{uuid} rejects unsafe custom internal names', function (string $name) { + $this->withHeaders(applicationSettingsApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'custom_internal_name' => $name, + ]) + ->assertUnprocessable() + ->assertJsonValidationErrors('custom_internal_name'); + + expect($this->application->fresh()->settings->custom_internal_name)->toBeNull(); +})->with([ + 'semicolon' => 'app;id', + 'command substitution' => 'app$(id)', + 'backticks' => 'app`id`', + 'single quote' => "app'id", + 'double quote' => 'app"id', + 'space' => 'app name', + 'newline' => "app\nid", + 'option-like prefix' => '--help', + 'pipe' => 'app|id', + 'ampersand' => 'app&id', +]); + +test('application creation rejects an unsafe custom internal name', function () { + Queue::fake(); + + $this->withHeaders(applicationSettingsApiHeaders($this->bearerToken)) + ->postJson('/api/v1/applications/public', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'server_uuid' => $this->server->uuid, + 'git_repository' => 'https://gitlab.com/coolify/custom-name-test', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '3000', + 'custom_internal_name' => 'app$(id)', + ]) + ->assertUnprocessable() + ->assertJsonValidationErrors('custom_internal_name'); +}); + test('PATCH /api/v1/applications/{uuid} updates preview_url_template and max_restart_count', function () { $this->withHeaders(applicationSettingsApiHeaders($this->bearerToken)) ->patchJson("/api/v1/applications/{$this->application->uuid}", [ diff --git a/tests/Feature/Security/CommandInjectionSecurityTest.php b/tests/Feature/Security/CommandInjectionSecurityTest.php index 85ba2faf44..522638464b 100644 --- a/tests/Feature/Security/CommandInjectionSecurityTest.php +++ b/tests/Feature/Security/CommandInjectionSecurityTest.php @@ -1025,6 +1025,16 @@ describe('service application lifecycle command escaping', function () { ->and($source)->toContain('escapeshellarg($container_id)'); }); + test('application deployment shell commands escape the generated container name', function () { + $source = file_get_contents(app_path('Jobs/ApplicationDeploymentJob.php')); + + expect($source)->toContain('$escapedContainerName = escapeshellarg($this->container_name)') + ->and($source)->toContain("docker inspect --format='{{json .State.Health.Status}}' {\$escapedContainerName}") + ->and($source)->toContain("docker inspect --format='{{json .State.Health.Log}}' {\$escapedContainerName}") + ->and($source)->toContain('docker logs -n 100 {$escapedContainerName}') + ->and($source)->toContain('dockerStopCommand($timeout, escapeshellarg($containerName), $this->server)'); + }); + test('service application logs endpoint passes raw container name to docker helpers', function () { $source = file_get_contents(app_path('Http/Controllers/Api/ServiceApplicationsController.php'));