diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index ca1daef72..5c186af70 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -3,11 +3,14 @@ namespace App\Livewire\Project\Application; use App\Actions\Application\GenerateConfig; +use App\Jobs\ApplicationDeploymentJob; use App\Models\Application; use App\Support\ValidationPatterns; +use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Livewire\Component; +use Livewire\Features\SupportEvents\Event; use Spatie\Url\Url; use Visus\Cuid2\Cuid2; @@ -194,9 +197,9 @@ protected function messages(): array 'baseDirectory.regex' => 'The base directory must be a valid path starting with / and containing only safe characters.', 'publishDirectory.regex' => 'The publish directory must be a valid path starting with / and containing only safe characters.', 'dockerfileTargetBuild.regex' => 'The Dockerfile target build must contain only alphanumeric characters, dots, hyphens, and underscores.', - 'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.', - 'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.', - 'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.', + 'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.', + 'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.', + 'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Shell operators like ;, |, $, and backticks are not allowed.', 'preDeploymentCommandContainer.regex' => 'The pre-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.', 'postDeploymentCommandContainer.regex' => 'The post-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.', 'name.required' => 'The Name field is required.', @@ -288,7 +291,7 @@ public function mount() $this->authorize('update', $this->application); $this->application->fqdn = null; $this->application->settings->save(); - } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + } catch (AuthorizationException $e) { // User doesn't have update permission, just continue without saving } } @@ -309,7 +312,7 @@ public function mount() $this->customLabels = str(implode('|coolify|', generateLabelsApplication($this->application)))->replace('|coolify|', "\n"); $this->application->custom_labels = base64_encode($this->customLabels); $this->application->save(); - } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + } catch (AuthorizationException $e) { // User doesn't have update permission, just use existing labels // $this->customLabels = str(implode('|coolify|', generateLabelsApplication($this->application)))->replace('|coolify|', "\n"); } @@ -321,7 +324,7 @@ public function mount() $this->authorize('update', $this->application); $this->initLoadingCompose = true; $this->dispatch('info', 'Loading docker compose file.'); - } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + } catch (AuthorizationException $e) { // User doesn't have update permission, skip loading compose file } } @@ -587,7 +590,7 @@ public function updatedBuildPack() // Check if user has permission to update try { $this->authorize('update', $this->application); - } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + } catch (AuthorizationException $e) { // User doesn't have permission, revert the change and return $this->application->refresh(); $this->syncData(); @@ -612,7 +615,7 @@ public function updatedBuildPack() $this->fqdn = null; $this->application->fqdn = null; $this->application->settings->save(); - } catch (\Illuminate\Auth\Access\AuthorizationException $e) { + } catch (AuthorizationException $e) { // User doesn't have update permission, just continue without saving } } @@ -809,7 +812,7 @@ public function submit($showToaster = true) restoreBaseDirectory: $oldBaseDirectory, restoreDockerComposeLocation: $oldDockerComposeLocation ); - if ($compose_return instanceof \Livewire\Features\SupportEvents\Event) { + if ($compose_return instanceof Event) { // Validation failed - restore original values to component properties $this->baseDirectory = $oldBaseDirectory; $this->dockerComposeLocation = $oldDockerComposeLocation; @@ -939,7 +942,7 @@ public function getDockerComposeBuildCommandPreviewProperty(): string $command = injectDockerComposeFlags( $this->dockerComposeCustomBuildCommand, ".{$normalizedBase}{$this->dockerComposeLocation}", - \App\Jobs\ApplicationDeploymentJob::BUILD_TIME_ENV_PATH + ApplicationDeploymentJob::BUILD_TIME_ENV_PATH ); // Inject build args if not using build secrets diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index bc19d52a5..27789b506 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -37,11 +37,13 @@ class ValidationPatterns /** * Pattern for shell-safe command strings (docker compose commands, docker run options) - * Blocks dangerous shell metacharacters: ; & | ` $ ( ) > < newlines and carriage returns - * Also blocks backslashes, single quotes, and double quotes to prevent escape-sequence attacks + * Blocks dangerous shell metacharacters: ; | ` $ ( ) > < newlines and carriage returns + * Allows & for command chaining (&&) which is common in multi-step build commands + * Allows double quotes for build args with spaces (e.g. --build-arg KEY="value") + * Blocks backslashes and single quotes to prevent escape-sequence attacks * Uses [ \t] instead of \s to explicitly exclude \n and \r (which act as command separators) */ - public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~]+$/'; + public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~&"]+$/'; /** * Pattern for Docker container names diff --git a/tests/Feature/CommandInjectionSecurityTest.php b/tests/Feature/CommandInjectionSecurityTest.php index 12a24f42c..cfa363e79 100644 --- a/tests/Feature/CommandInjectionSecurityTest.php +++ b/tests/Feature/CommandInjectionSecurityTest.php @@ -1,6 +1,7 @@ toBeArray(); - expect($merged['docker_compose_location'])->toContain('regex:'.\App\Support\ValidationPatterns::FILE_PATH_PATTERN); + expect($merged['docker_compose_location'])->toContain('regex:'.ValidationPatterns::FILE_PATH_PATTERN); }); }); @@ -285,7 +286,7 @@ $job = new ReflectionClass(ApplicationDeploymentJob::class); // Test that validateShellSafeCommand is also available as a pattern - $pattern = \App\Support\ValidationPatterns::DOCKER_TARGET_PATTERN; + $pattern = ValidationPatterns::DOCKER_TARGET_PATTERN; expect(preg_match($pattern, 'production'))->toBe(1); expect(preg_match($pattern, 'build; env'))->toBe(0); expect(preg_match($pattern, 'target`whoami`'))->toBe(0); @@ -364,15 +365,15 @@ expect($validator->fails())->toBeTrue(); }); - test('rejects ampersand chaining in docker_compose_custom_start_command', function () { + test('allows ampersand chaining in docker_compose_custom_start_command', function () { $rules = sharedDataApplications(); $validator = validator( - ['docker_compose_custom_start_command' => 'docker compose up && rm -rf /'], + ['docker_compose_custom_start_command' => 'docker compose up && docker compose logs'], ['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']] ); - expect($validator->fails())->toBeTrue(); + expect($validator->fails())->toBeFalse(); }); test('rejects command substitution in docker_compose_custom_build_command', function () { @@ -399,6 +400,7 @@ 'docker compose build', 'docker compose up -d --build', 'docker compose -f custom.yml build --no-cache', + 'docker compose build && docker tag registry.example.com/app:beta localhost:5000/app:beta && docker push localhost:5000/app:beta', ]); test('rejects backslash in docker_compose_custom_start_command', function () { @@ -423,15 +425,15 @@ expect($validator->fails())->toBeTrue(); }); - test('rejects double quotes in docker_compose_custom_start_command', function () { + test('allows double quotes in docker_compose_custom_start_command', function () { $rules = sharedDataApplications(); $validator = validator( - ['docker_compose_custom_start_command' => 'docker compose up -d --build "malicious"'], + ['docker_compose_custom_start_command' => 'docker compose up -d --build --build-arg VERSION="1.0.0"'], ['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']] ); - expect($validator->fails())->toBeTrue(); + expect($validator->fails())->toBeFalse(); }); test('rejects newline injection in docker_compose_custom_start_command', function () { @@ -564,7 +566,7 @@ expect($merged)->toHaveKey('dockerfile_target_build'); expect($merged['dockerfile_target_build'])->toBeArray(); - expect($merged['dockerfile_target_build'])->toContain('regex:'.\App\Support\ValidationPatterns::DOCKER_TARGET_PATTERN); + expect($merged['dockerfile_target_build'])->toContain('regex:'.ValidationPatterns::DOCKER_TARGET_PATTERN); }); }); @@ -582,7 +584,7 @@ $merged = array_merge($sharedRules, $localRules); expect($merged['docker_compose_custom_start_command'])->toBeArray(); - expect($merged['docker_compose_custom_start_command'])->toContain('regex:'.\App\Support\ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN); + expect($merged['docker_compose_custom_start_command'])->toContain('regex:'.ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN); }); test('docker_compose_custom_build_command safe regex is not overridden by local rules', function () { @@ -595,7 +597,7 @@ $merged = array_merge($sharedRules, $localRules); expect($merged['docker_compose_custom_build_command'])->toBeArray(); - expect($merged['docker_compose_custom_build_command'])->toContain('regex:'.\App\Support\ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN); + expect($merged['docker_compose_custom_build_command'])->toContain('regex:'.ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN); }); });