From 01031fc5f3d25422eec2f0376ea98475a7337212 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 12 Mar 2026 13:09:13 +0100 Subject: [PATCH] refactor: consolidate file path validation patterns and support scoped packages - Extract file path validation regex into ValidationPatterns::FILE_PATH_PATTERN constant - Add filePathRules() and filePathMessages() helper methods for reusable validation - Extend allowed characters from [a-zA-Z0-9._\-/] to [a-zA-Z0-9._\-/~@+] to support: - Scoped npm packages (@org/package) - Language-specific directories (c++, rust+) - Version markers (v1~, build~) - Replace duplicate inline regex patterns across multiple files - Add tests for paths with @ symbol and tilde/plus characters --- app/Jobs/ApplicationDeploymentJob.php | 2 +- app/Livewire/Project/Application/General.php | 12 +++---- .../Project/New/GithubPrivateRepository.php | 2 +- .../New/GithubPrivateRepositoryDeployKey.php | 4 +-- .../Project/New/PublicGitRepository.php | 4 +-- app/Support/ValidationPatterns.php | 26 +++++++++++++- bootstrap/helpers/api.php | 4 +-- .../Feature/CommandInjectionSecurityTest.php | 36 ++++++++++++++++++- 8 files changed, 74 insertions(+), 16 deletions(-) diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index c80d31ab3..b51d04fca 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -3929,7 +3929,7 @@ private function add_build_secrets_to_compose($composeFile) private function validatePathField(string $value, string $fieldName): string { - if (! preg_match('/^\/[a-zA-Z0-9._\-\/]+$/', $value)) { + if (! preg_match(\App\Support\ValidationPatterns::FILE_PATH_PATTERN, $value)) { throw new \RuntimeException("Invalid {$fieldName}: contains forbidden characters."); } if (str_contains($value, '..')) { diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 747536bcf..b3fe99806 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -73,7 +73,7 @@ class General extends Component #[Validate(['string', 'nullable'])] public ?string $dockerfile = null; - #[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'])] + #[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/~@+]+$/'])] public ?string $dockerfileLocation = null; #[Validate(['string', 'nullable'])] @@ -85,7 +85,7 @@ class General extends Component #[Validate(['string', 'nullable'])] public ?string $dockerRegistryImageTag = null; - #[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'])] + #[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/~@+]+$/'])] public ?string $dockerComposeLocation = null; #[Validate(['string', 'nullable'])] @@ -198,8 +198,8 @@ protected function rules(): array 'dockerfile' => 'nullable', 'dockerRegistryImageName' => 'nullable', 'dockerRegistryImageTag' => 'nullable', - 'dockerfileLocation' => ['nullable', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], - 'dockerComposeLocation' => ['nullable', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'dockerfileLocation' => ['nullable', 'regex:'.ValidationPatterns::FILE_PATH_PATTERN], + 'dockerComposeLocation' => ['nullable', 'regex:'.ValidationPatterns::FILE_PATH_PATTERN], 'dockerCompose' => 'nullable', 'dockerComposeRaw' => 'nullable', 'dockerfileTargetBuild' => 'nullable', @@ -231,8 +231,8 @@ protected function messages(): array return array_merge( ValidationPatterns::combinedMessages(), [ - 'dockerfileLocation.regex' => 'The Dockerfile location must be a valid path starting with / and containing only alphanumeric characters, dots, hyphens, and slashes.', - 'dockerComposeLocation.regex' => 'The Docker Compose location must be a valid path starting with / and containing only alphanumeric characters, dots, hyphens, and slashes.', + ...ValidationPatterns::filePathMessages('dockerfileLocation', 'Dockerfile'), + ...ValidationPatterns::filePathMessages('dockerComposeLocation', 'Docker Compose'), 'name.required' => 'The Name field is required.', 'gitRepository.required' => 'The Git Repository field is required.', 'gitBranch.required' => 'The Git Branch field is required.', diff --git a/app/Livewire/Project/New/GithubPrivateRepository.php b/app/Livewire/Project/New/GithubPrivateRepository.php index 1bb276b89..61ae0e151 100644 --- a/app/Livewire/Project/New/GithubPrivateRepository.php +++ b/app/Livewire/Project/New/GithubPrivateRepository.php @@ -168,7 +168,7 @@ public function submit() 'selected_repository_owner' => 'required|string|regex:/^[a-zA-Z0-9\-_]+$/', 'selected_repository_repo' => 'required|string|regex:/^[a-zA-Z0-9\-_\.]+$/', 'selected_branch_name' => ['required', 'string', new ValidGitBranch], - 'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), ]); if ($validator->fails()) { diff --git a/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php b/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php index f52c01e91..b9af20c76 100644 --- a/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php +++ b/app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php @@ -64,7 +64,7 @@ class GithubPrivateRepositoryDeployKey extends Component 'is_static' => 'required|boolean', 'publish_directory' => 'nullable|string', 'build_pack' => 'required|string', - 'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), ]; protected function rules() @@ -76,7 +76,7 @@ protected function rules() 'is_static' => 'required|boolean', 'publish_directory' => 'nullable|string', 'build_pack' => 'required|string', - 'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), ]; } diff --git a/app/Livewire/Project/New/PublicGitRepository.php b/app/Livewire/Project/New/PublicGitRepository.php index a08c448dd..cc2510a66 100644 --- a/app/Livewire/Project/New/PublicGitRepository.php +++ b/app/Livewire/Project/New/PublicGitRepository.php @@ -70,7 +70,7 @@ class PublicGitRepository extends Component 'publish_directory' => 'nullable|string', 'build_pack' => 'required|string', 'base_directory' => 'nullable|string', - 'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), ]; protected function rules() @@ -82,7 +82,7 @@ protected function rules() 'publish_directory' => 'nullable|string', 'build_pack' => 'required|string', 'base_directory' => 'nullable|string', - 'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'docker_compose_location' => \App\Support\ValidationPatterns::filePathRules(), 'git_branch' => ['required', 'string', new ValidGitBranch], ]; } diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index a2da0fc46..2ae1536da 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -17,6 +17,12 @@ class ValidationPatterns */ public const DESCRIPTION_PATTERN = '/^[\p{L}\p{M}\p{N}\s\-_.,!?()\'\"+=*@\/&]+$/u'; + /** + * Pattern for file paths (dockerfile location, docker compose location, etc.) + * Allows alphanumeric, dots, hyphens, underscores, slashes, @, ~, and + + */ + public const FILE_PATH_PATTERN = '/^\/[a-zA-Z0-9._\-\/~@+]+$/'; + /** * Get validation rules for name fields */ @@ -81,7 +87,25 @@ public static function descriptionMessages(): array ]; } - /** + /** + * Get validation rules for file path fields (dockerfile location, docker compose location) + */ + public static function filePathRules(int $maxLength = 255): array + { + return ['nullable', 'string', 'max:'.$maxLength, 'regex:'.self::FILE_PATH_PATTERN]; + } + + /** + * Get validation messages for file path fields + */ + public static function filePathMessages(string $field = 'dockerfileLocation', string $label = 'Dockerfile'): array + { + return [ + "{$field}.regex" => "The {$label} location must be a valid path starting with / and containing only alphanumeric characters, dots, hyphens, underscores, slashes, @, ~, and +.", + ]; + } + + /** * Get combined validation messages for both name and description fields */ public static function combinedMessages(): array diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 1b03a4d37..43c074cd1 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -134,8 +134,8 @@ function sharedDataApplications() 'manual_webhook_secret_gitlab' => 'string|nullable', 'manual_webhook_secret_bitbucket' => 'string|nullable', 'manual_webhook_secret_gitea' => 'string|nullable', - 'dockerfile_location' => ['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], - 'docker_compose_location' => ['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'], + 'dockerfile_location' => ['string', 'nullable', 'max:255', 'regex:'.\App\Support\ValidationPatterns::FILE_PATH_PATTERN], + 'docker_compose_location' => ['string', 'nullable', 'max:255', 'regex:'.\App\Support\ValidationPatterns::FILE_PATH_PATTERN], 'docker_compose' => 'string|nullable', 'docker_compose_domains' => 'array|nullable', 'docker_compose_custom_start_command' => 'string|nullable', diff --git a/tests/Feature/CommandInjectionSecurityTest.php b/tests/Feature/CommandInjectionSecurityTest.php index 47e9f3b35..7047e4bc6 100644 --- a/tests/Feature/CommandInjectionSecurityTest.php +++ b/tests/Feature/CommandInjectionSecurityTest.php @@ -91,6 +91,28 @@ ->toBe('/docker/Dockerfile.prod'); }); + test('allows path with @ symbol for scoped packages', function () { + $job = new ReflectionClass(ApplicationDeploymentJob::class); + $method = $job->getMethod('validatePathField'); + $method->setAccessible(true); + + $instance = $job->newInstanceWithoutConstructor(); + + expect($method->invoke($instance, '/packages/@intlayer/mcp/Dockerfile', 'dockerfile_location')) + ->toBe('/packages/@intlayer/mcp/Dockerfile'); + }); + + test('allows path with tilde and plus characters', function () { + $job = new ReflectionClass(ApplicationDeploymentJob::class); + $method = $job->getMethod('validatePathField'); + $method->setAccessible(true); + + $instance = $job->newInstanceWithoutConstructor(); + + expect($method->invoke($instance, '/build~v1/c++/Dockerfile', 'dockerfile_location')) + ->toBe('/build~v1/c++/Dockerfile'); + }); + test('allows valid compose file path', function () { $job = new ReflectionClass(ApplicationDeploymentJob::class); $method = $job->getMethod('validatePathField'); @@ -149,6 +171,18 @@ }); }); + test('dockerfile_location validation allows paths with @ for scoped packages', function () { + $rules = sharedDataApplications(); + + $validator = validator( + ['dockerfile_location' => '/packages/@intlayer/mcp/Dockerfile'], + ['dockerfile_location' => $rules['dockerfile_location']] + ); + + expect($validator->fails())->toBeFalse(); + }); +}); + describe('sharedDataApplications rules survive array_merge in controller', function () { test('docker_compose_location safe regex is not overridden by local rules', function () { $sharedRules = sharedDataApplications(); @@ -164,7 +198,7 @@ // The merged rules for docker_compose_location should be the safe regex, not just 'string' expect($merged['docker_compose_location'])->toBeArray(); - expect($merged['docker_compose_location'])->toContain('regex:/^\/[a-zA-Z0-9._\-\/]+$/'); + expect($merged['docker_compose_location'])->toContain('regex:'.\App\Support\ValidationPatterns::FILE_PATH_PATTERN); }); });