From 2bb07bbe9e75ba50b27703ff14e9894f92bdd376 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:13:04 +0200 Subject: [PATCH] fix: validate application branch updates --- app/Livewire/Project/Application/General.php | 3 +- app/Livewire/Project/Application/Source.php | 3 +- bootstrap/helpers/api.php | 30 +++++++------ .../Feature/CommandInjectionSecurityTest.php | 44 +++++++++++++++++++ 4 files changed, 64 insertions(+), 16 deletions(-) diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 258b54eed..69240337f 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -5,6 +5,7 @@ namespace App\Livewire\Project\Application; use App\Actions\Application\GenerateConfig; use App\Jobs\ApplicationDeploymentJob; use App\Models\Application; +use App\Rules\ValidGitBranch; use App\Support\ValidationPatterns; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; @@ -144,7 +145,7 @@ class General extends Component 'description' => ValidationPatterns::descriptionRules(), 'fqdn' => 'nullable', 'gitRepository' => 'required', - 'gitBranch' => 'required', + 'gitBranch' => ['required', 'string', new ValidGitBranch], 'gitCommitSha' => ['nullable', 'string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], 'installCommand' => ValidationPatterns::shellSafeCommandRules(), 'buildCommand' => ValidationPatterns::shellSafeCommandRules(), diff --git a/app/Livewire/Project/Application/Source.php b/app/Livewire/Project/Application/Source.php index f14689ee0..3ee5919fe 100644 --- a/app/Livewire/Project/Application/Source.php +++ b/app/Livewire/Project/Application/Source.php @@ -6,6 +6,7 @@ use App\Models\Application; use App\Models\GithubApp; use App\Models\GitlabApp; use App\Models\PrivateKey; +use App\Rules\ValidGitBranch; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Attributes\Locked; use Livewire\Attributes\Validate; @@ -29,7 +30,7 @@ class Source extends Component #[Validate(['required', 'string'])] public string $gitRepository; - #[Validate(['required', 'string'])] + #[Validate(['required', 'string', new ValidGitBranch])] public string $gitBranch; #[Validate(['nullable', 'string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'])] 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..558a8bd4f 100644 --- a/tests/Feature/CommandInjectionSecurityTest.php +++ b/tests/Feature/CommandInjectionSecurityTest.php @@ -1,6 +1,7 @@ toContain('regex:'.ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN); }); }); + +describe('git_branch validation rules survive array_merge in controller', function () { + test('git_branch uses ValidGitBranch in shared application rules', function () { + $rules = sharedDataApplications(); + + expect($rules['git_branch'])->toBeArray(); + expect(collect($rules['git_branch'])->contains(fn ($rule) => $rule instanceof ValidGitBranch))->toBeTrue(); + }); + + test('git_branch rejects shell metacharacter payloads', function (string $payload) { + $rules = sharedDataApplications(); + + $validator = validator( + ['git_branch' => $payload], + ['git_branch' => $rules['git_branch']] + ); + + expect($validator->fails())->toBeTrue(); + })->with([ + 'semicolon command separator' => 'main;touch /tmp/pwned;#', + 'command substitution' => 'main$(touch /tmp/pwned)', + 'backtick substitution' => 'main`touch /tmp/pwned`', + 'pipe operator' => 'main|id', + 'newline injection' => "main\ntouch /tmp/pwned", + 'redirect operator' => 'main>/tmp/pwned', + 'single quote breakout' => "main';id;#", + ]); + + test('git_branch accepts 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/my-branch', + 'release_1.2.3', + ]); +});