chore: improve deployment input handling

2026-06-14 03:19:51 +00:00 · 2026-06-01 15:15:38 +02:00
parent 4d0be415c8
commit 5e4873322e
3 changed files with 126 additions and 17 deletions
@@ -2232,11 +2232,22 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
            }
        }
        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 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                    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 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
        } 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',
                ],
@@ -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',
    ];
@@ -1,6 +1,8 @@
 <?php

 use App\Jobs\ApplicationDeploymentJob;
+use App\Models\Application;
+use App\Models\ApplicationSetting;
 use App\Support\ValidationPatterns;

 describe('deployment job path field validation', function () {
@@ -127,6 +129,38 @@ describe('deployment job path field validation', function () {
 });

 describe('API validation rules for path fields', function () {
+    test('git_branch validation rejects shell metacharacters', function (string $branch) {
+        $rules = sharedDataApplications();
+
+        $validator = validator(
+            ['git_branch' => $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('API validation rules for path fields', function () {
    });
 });

+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();