From bbff70c8d0c60ef4f49ed22fca8e1750b1d07fe9 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:52:07 +0200 Subject: [PATCH 1/3] fix: improve application URL handling --- .../Api/ApplicationsController.php | 67 ++------ .../Controllers/Api/ServicesController.php | 25 +-- app/Jobs/ApplicationDeploymentJob.php | 32 +++- app/Support/ValidationPatterns.php | 156 ++++++++++++++++++ bootstrap/helpers/api.php | 2 +- .../Security/CommandInjectionSecurityTest.php | 103 +++++++++++- 6 files changed, 306 insertions(+), 79 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 9570026c1..d125c01ec 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -952,6 +952,10 @@ private function create_application(Request $request, $type) } $serverUuid = $request->server_uuid; $fqdn = $request->domains; + if ($request->has('domains') && is_string($request->domains)) { + $fqdn = ValidationPatterns::normalizeApplicationDomains($request->domains); + $request->offsetSet('domains', $fqdn); + } $autogenerateDomain = $request->boolean('autogenerate_domain', true); $instantDeploy = $request->instant_deploy; $githubAppUuid = $request->github_app_uuid; @@ -1031,7 +1035,7 @@ private function create_application(Request $request, $type) 'docker_compose_domains' => 'array|nullable', 'docker_compose_domains.*' => 'array:name,domain', 'docker_compose_domains.*.name' => 'string|required', - 'docker_compose_domains.*.domain' => 'string|nullable', + 'docker_compose_domains.*.domain' => ValidationPatterns::applicationDomainRules(), ]; // ports_exposes is not required for dockercompose if ($request->build_pack === 'dockercompose') { @@ -1239,7 +1243,7 @@ private function create_application(Request $request, $type) 'docker_compose_domains' => 'array|nullable', 'docker_compose_domains.*' => 'array:name,domain', 'docker_compose_domains.*.name' => 'string|required', - 'docker_compose_domains.*.domain' => 'string|nullable', + 'docker_compose_domains.*.domain' => ValidationPatterns::applicationDomainRules(), ]; $validationRules = array_merge(sharedDataApplications(), $validationRules); $validationMessages = [ @@ -1479,7 +1483,7 @@ private function create_application(Request $request, $type) 'docker_compose_domains' => 'array|nullable', 'docker_compose_domains.*' => 'array:name,domain', 'docker_compose_domains.*.name' => 'string|required', - 'docker_compose_domains.*.domain' => 'string|nullable', + 'docker_compose_domains.*.domain' => ValidationPatterns::applicationDomainRules(), ]; $validationRules = array_merge(sharedDataApplications(), $validationRules); @@ -2378,7 +2382,7 @@ public function update_by_uuid(Request $request) 'docker_compose_domains' => 'array|nullable', 'docker_compose_domains.*' => 'array:name,domain', 'docker_compose_domains.*.name' => 'string|required', - 'docker_compose_domains.*.domain' => 'string|nullable', + 'docker_compose_domains.*.domain' => ValidationPatterns::applicationDomainRules(), 'custom_nginx_configuration' => 'string|nullable', 'is_http_basic_auth_enabled' => 'boolean|nullable', 'http_basic_auth_username' => 'string', @@ -2482,29 +2486,7 @@ public function update_by_uuid(Request $request) $requestHasDomains = $request->has('domains'); if ($requestHasDomains && $server->isProxyShouldRun()) { $uuid = $request->uuid; - $urls = $request->domains; - $urls = str($urls)->replaceStart(',', '')->replaceEnd(',', '')->trim(); - $errors = []; - $urls = str($urls)->trim()->explode(',')->map(function ($url) use (&$errors) { - $url = trim($url); - - // If "domains" is empty clear all URLs from the fqdn column - if (blank($url)) { - return null; - } - - if (! filter_var($url, FILTER_VALIDATE_URL)) { - $errors[] = 'Invalid URL: '.$url; - - return $url; - } - $scheme = parse_url($url, PHP_URL_SCHEME) ?? ''; - if (! in_array(strtolower($scheme), ['http', 'https'])) { - $errors[] = "Invalid URL scheme: {$scheme} for URL: {$url}. Only http and https are supported."; - } - - return str($url)->lower(); - }); + $errors = ValidationPatterns::validateApplicationDomains($request->domains); if (count($errors) > 0) { return response()->json([ @@ -2512,6 +2494,9 @@ public function update_by_uuid(Request $request) 'errors' => $errors, ], 422); } + $domains = ValidationPatterns::normalizeApplicationDomains($request->domains); + $request->offsetSet('domains', $domains); + $urls = collect(ValidationPatterns::applicationDomainList($domains)); // Check for domain conflicts $result = checkIfDomainIsAlreadyUsedViaAPI($urls, $teamId, $uuid); if (isset($result['error'])) { @@ -3871,36 +3856,16 @@ private function validateDataApplications(Request $request, Server $server) } if ($request->has('domains') && $server->isProxyShouldRun()) { $uuid = $request->uuid; - $urls = $request->domains; - $urls = str($urls)->replaceEnd(',', '')->trim(); - $urls = str($urls)->replaceStart(',', '')->trim(); - $errors = []; - $urls = str($urls)->trim()->explode(',')->map(function ($url) use (&$errors) { - $url = trim($url); - - // If "domains" is empty clear all URLs from the fqdn column - if (blank($url)) { - return null; - } - - if (! filter_var($url, FILTER_VALIDATE_URL)) { - $errors[] = 'Invalid URL: '.$url; - - return str($url)->lower(); - } - $scheme = parse_url($url, PHP_URL_SCHEME) ?? ''; - if (! in_array(strtolower($scheme), ['http', 'https'])) { - $errors[] = "Invalid URL scheme: {$scheme} for URL: {$url}. Only http and https are supported."; - } - - return str($url)->lower(); - }); + $errors = ValidationPatterns::validateApplicationDomains($request->domains); if (count($errors) > 0) { return response()->json([ 'message' => 'Validation failed.', 'errors' => $errors, ], 422); } + $normalizedDomains = ValidationPatterns::normalizeApplicationDomains($request->domains); + $request->offsetSet('domains', $normalizedDomains); + $urls = collect(ValidationPatterns::applicationDomainList($normalizedDomains)); // Check for domain conflicts $result = checkIfDomainIsAlreadyUsedViaAPI($urls, $teamId, $uuid); if (isset($result['error'])) { diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 6c121bcf8..97fd41c5c 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -60,19 +60,10 @@ private function applyServiceUrls(Service $service, array $urlsArray, string $te return str($urlValue)->replaceStart(',', '')->replaceEnd(',', '')->trim()->explode(',')->map(fn ($url) => trim($url))->filter(); }); - $urls = $urls->map(function ($url) use (&$errors) { - if (! filter_var($url, FILTER_VALIDATE_URL)) { - $errors[] = "Invalid URL: {$url}"; - - return $url; - } - $scheme = parse_url($url, PHP_URL_SCHEME) ?? ''; - if (! in_array(strtolower($scheme), ['http', 'https'])) { - $errors[] = "Invalid URL scheme: {$scheme} for URL: {$url}. Only http and https are supported."; - } - - return $url; - }); + $errors = ValidationPatterns::validateApplicationDomains($urls->implode(',')); + $urls = collect(ValidationPatterns::applicationDomainList( + ValidationPatterns::normalizeApplicationDomains($urls->implode(',')) + )); $duplicates = $urls->duplicates()->unique()->values(); if ($duplicates->isNotEmpty() && ! $forceDomainOverride) { @@ -101,10 +92,10 @@ private function applyServiceUrls(Service $service, array $urlsArray, string $te } if (filled($containerUrls)) { - $containerUrls = str($containerUrls)->replaceStart(',', '')->replaceEnd(',', '')->trim(); - $containerUrls = str($containerUrls)->explode(',')->map(fn ($url) => str(trim($url))->lower()); + $containerUrls = ValidationPatterns::normalizeApplicationDomains($containerUrls); + $containerUrlCollection = collect(ValidationPatterns::applicationDomainList($containerUrls)); - $result = checkIfDomainIsAlreadyUsedViaAPI($containerUrls, $teamId, $application->uuid); + $result = checkIfDomainIsAlreadyUsedViaAPI($containerUrlCollection, $teamId, $application->uuid); if (isset($result['error'])) { $errors[] = $result['error']; @@ -116,8 +107,6 @@ private function applyServiceUrls(Service $service, array $urlsArray, string $te return; } - - $containerUrls = $containerUrls->filter(fn ($u) => filled($u))->unique()->implode(','); } else { $containerUrls = null; } diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 545735cf6..7427df54e 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2229,7 +2229,7 @@ private function set_coolify_variables() // Only include SOURCE_COMMIT in build context if enabled in settings if ($this->application->settings->include_source_commit_in_build) { - $this->coolify_variables .= "SOURCE_COMMIT={$this->commit} "; + $this->coolify_variables .= 'SOURCE_COMMIT='.escapeShellValue($this->commit).' '; } if ($this->pull_request_id === 0) { $fqdn = $this->application->fqdn; @@ -2241,17 +2241,33 @@ private function set_coolify_variables() $fqdn = $url->getHost(); $url = $url->withHost($fqdn)->withPort(null)->__toString(); if ((int) $this->application->compose_parsing_version >= 3) { - $this->coolify_variables .= "COOLIFY_URL={$url} "; - $this->coolify_variables .= "COOLIFY_FQDN={$fqdn} "; + $this->coolify_variables .= 'COOLIFY_URL='.escapeShellValue($url).' '; + $this->coolify_variables .= 'COOLIFY_FQDN='.escapeShellValue($fqdn).' '; } else { - $this->coolify_variables .= "COOLIFY_URL={$fqdn} "; - $this->coolify_variables .= "COOLIFY_FQDN={$url} "; + $this->coolify_variables .= 'COOLIFY_URL='.escapeShellValue($fqdn).' '; + $this->coolify_variables .= 'COOLIFY_FQDN='.escapeShellValue($url).' '; } } if (isset($this->application->git_branch)) { $this->coolify_variables .= 'COOLIFY_BRANCH='.escapeShellValue($this->application->git_branch).' '; } - $this->coolify_variables .= "COOLIFY_RESOURCE_UUID={$this->application->uuid} "; + $this->coolify_variables .= 'COOLIFY_RESOURCE_UUID='.escapeShellValue($this->application->uuid).' '; + } + + private function shellAssignmentForDockerfileArg(string $assignment): string + { + [$key, $value] = array_pad(explode('=', $assignment, 2), 2, null); + + if ($value === null) { + return $assignment; + } + + if (str_starts_with($value, "'") && str_ends_with($value, "'")) { + $value = substr($value, 1, -1); + $value = str_replace("'\\''", "'", $value); + } + + return "{$key}={$value}"; } private function gitLsRemoteCommand(string $lsRemoteRef, ?string $identityFile = null): string @@ -4220,7 +4236,7 @@ private function add_build_env_variables_to_dockerfile() $coolify_vars = collect(explode(' ', trim($this->coolify_variables))) ->filter() ->map(function ($var) { - return "ARG {$var}"; + return 'ARG '.$this->shellAssignmentForDockerfileArg($var); }); $argsToInsert = $argsToInsert->merge($coolify_vars); } @@ -4242,7 +4258,7 @@ private function add_build_env_variables_to_dockerfile() $coolify_vars = collect(explode(' ', trim($this->coolify_variables))) ->filter() ->map(function ($var) { - return "ARG {$var}"; + return 'ARG '.$this->shellAssignmentForDockerfileArg($var); }); $argsToInsert = $argsToInsert->merge($coolify_vars); } diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index d781c7416..f88f78c34 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -108,6 +108,12 @@ class ValidationPatterns */ public const ENVIRONMENT_VARIABLE_KEY_PATTERN = '/\A[A-Za-z_][A-Za-z0-9_.]*\z/u'; + /** + * Characters that are valid in some URL positions but unsafe for values + * that are later reused in shell assignment contexts. + */ + public const APPLICATION_DOMAIN_FORBIDDEN_PATTERN = '/[`$;&|<>()\\\\\r\n]/'; + /** * Pattern for SQL-safe unquoted database identifiers (usernames, database names). * Allows letters, digits, underscore; first char must be letter or underscore. @@ -511,6 +517,156 @@ public static function shellSafeCommandRules(int $maxLength = 1000): array return ['nullable', 'string', 'max:'.$maxLength, 'regex:'.self::SHELL_SAFE_COMMAND_PATTERN]; } + /** + * Get validation rules for comma-separated application URL fields. + */ + public static function applicationDomainRules(int $maxLength = 2048): array + { + return [ + 'nullable', + 'string', + 'max:'.$maxLength, + function (string $attribute, mixed $value, \Closure $fail): void { + foreach (self::validateApplicationDomains($value) as $error) { + $fail($error); + } + }, + ]; + } + + /** + * Validate a comma-separated list of application URLs. + * + * @return array + */ + public static function validateApplicationDomains(mixed $value): array + { + if (blank($value)) { + return []; + } + + if (! is_string($value)) { + return ['The domains field must be a string.']; + } + + $errors = []; + foreach (self::applicationDomainList($value) as $url) { + if (preg_match(self::APPLICATION_DOMAIN_FORBIDDEN_PATTERN, $url) === 1) { + $errors[] = "Invalid URL: {$url}"; + + continue; + } + + if (! filter_var($url, FILTER_VALIDATE_URL)) { + $errors[] = "Invalid URL: {$url}"; + + continue; + } + + $scheme = parse_url($url, PHP_URL_SCHEME) ?? ''; + if (! in_array(strtolower($scheme), ['http', 'https'], true)) { + $errors[] = "Invalid URL scheme: {$scheme} for URL: {$url}. Only http and https are supported."; + + continue; + } + + if (blank(parse_url($url, PHP_URL_HOST))) { + $errors[] = "Invalid URL: {$url}"; + } + } + + return $errors; + } + + /** + * Normalize a comma-separated application URL list for storage. + */ + public static function normalizeApplicationDomains(?string $value): ?string + { + $urls = self::applicationDomainList($value); + + if ($urls === []) { + return null; + } + + return collect($urls) + ->map(fn (string $url) => self::normalizeApplicationDomainUrl($url)) + ->implode(','); + } + + /** + * Normalize URL components that are case-insensitive while preserving + * case-sensitive path, query, and fragment components. + */ + private static function normalizeApplicationDomainUrl(string $url): string + { + $components = parse_url($url); + + if ($components === false) { + return $url; + } + + $normalized = ''; + + if (isset($components['scheme'])) { + $normalized .= strtolower($components['scheme']).'://'; + } + + if (isset($components['user'])) { + $normalized .= $components['user']; + + if (isset($components['pass'])) { + $normalized .= ':'.$components['pass']; + } + + $normalized .= '@'; + } + + if (isset($components['host'])) { + $normalized .= strtolower($components['host']); + } + + if (isset($components['port'])) { + $normalized .= ':'.$components['port']; + } + + if (isset($components['path'])) { + $normalized .= $components['path']; + } + + if (array_key_exists('query', $components)) { + $normalized .= '?'.$components['query']; + } + + if (array_key_exists('fragment', $components)) { + $normalized .= '#'.$components['fragment']; + } + + return $normalized; + } + + /** + * Split a comma-separated application URL list into trimmed URL strings. + * + * @return array + */ + public static function applicationDomainList(?string $value): array + { + if (blank($value)) { + return []; + } + + return str($value) + ->replaceStart(',', '') + ->replaceEnd(',', '') + ->trim() + ->explode(',') + ->map(fn (string $url) => trim($url)) + ->filter(fn (string $url) => filled($url)) + ->values() + ->all(); + } + /** * Get validation rules for Docker volume name fields */ diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 6a288a064..200bc6856 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -98,7 +98,7 @@ function sharedDataApplications() 'is_auto_deploy_enabled' => 'boolean', 'is_force_https_enabled' => 'boolean', 'static_image' => Rule::enum(StaticImageTypes::class), - 'domains' => 'string|nullable', + 'domains' => ValidationPatterns::applicationDomainRules(), 'redirect' => Rule::enum(RedirectTypes::class), 'git_commit_sha' => ['string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], 'docker_registry_image_name' => ValidationPatterns::dockerImageNameRules(), diff --git a/tests/Feature/Security/CommandInjectionSecurityTest.php b/tests/Feature/Security/CommandInjectionSecurityTest.php index 5fa5c08d2..45101635b 100644 --- a/tests/Feature/Security/CommandInjectionSecurityTest.php +++ b/tests/Feature/Security/CommandInjectionSecurityTest.php @@ -130,6 +130,69 @@ }); describe('API validation rules for path fields', function () { + test('domains validation rejects command injection payloads', function (string $payload) { + $rules = sharedDataApplications(); + + $validator = validator( + ['domains' => $payload], + ['domains' => $rules['domains']] + ); + + expect($validator->fails())->toBeTrue(); + })->with([ + 'host command substitution' => 'http://$(whoami).example.com', + 'path command substitution' => 'http://example.com/$(whoami)', + 'query command substitution' => 'http://example.com/path?next=$(id)', + 'host backtick substitution' => 'http://`whoami`.example.com', + 'path backtick substitution' => 'http://example.com/`whoami`', + 'semicolon command separator' => 'http://example.com/path;id', + 'newline injection' => "http://example.com\nwhoami", + 'carriage return injection' => "http://example.com\rwhoami", + 'pipe injection' => 'http://example.com/path|id', + ]); + + test('domains validation rejects non http schemes', function () { + $rules = sharedDataApplications(); + + $validator = validator( + ['domains' => 'ftp://example.com'], + ['domains' => $rules['domains']] + ); + + expect($validator->fails())->toBeTrue(); + }); + + test('domains validation allows comma separated http and https urls', function () { + $rules = sharedDataApplications(); + + $validator = validator( + ['domains' => 'https://app.example.com,http://api.example.com/path'], + ['domains' => $rules['domains']] + ); + + expect($validator->fails())->toBeFalse(); + }); + + test('docker compose service domains validation rejects command injection payloads', function () { + $rules = [ + 'docker_compose_domains' => 'array|nullable', + 'docker_compose_domains.*' => 'array:name,domain', + 'docker_compose_domains.*.name' => 'string|required', + 'docker_compose_domains.*.domain' => ValidationPatterns::applicationDomainRules(), + ]; + + $validator = validator( + [ + 'docker_compose_domains' => [ + ['name' => 'app', 'domain' => 'https://app.example.com/$(whoami)'], + ], + ], + $rules + ); + + expect($validator->fails())->toBeTrue(); + }); + test('git_branch validation rejects shell metacharacters', function (string $branch) { $rules = sharedDataApplications(); @@ -276,7 +339,45 @@ expect($coolifyVariables->getValue($instance)) ->toContain("COOLIFY_BRANCH='main`id`' ") - ->toContain('COOLIFY_RESOURCE_UUID=app-uuid '); + ->toContain("COOLIFY_RESOURCE_UUID='app-uuid' "); + }); + + test('coolify url and fqdn shell assignments are quoted', function () { + $job = new ReflectionClass(ApplicationDeploymentJob::class); + $instance = $job->newInstanceWithoutConstructor(); + + $application = new Application; + $application->uuid = 'app-uuid'; + $application->git_branch = 'main'; + $application->fqdn = 'https://app.example.com/path'; + $application->compose_parsing_version = '3'; + + $settings = new ApplicationSetting; + $settings->include_source_commit_in_build = true; + $application->setRelation('settings', $settings); + + foreach ([ + 'application' => $application, + 'commit' => 'HEAD$(id)', + '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("SOURCE_COMMIT='HEAD$(id)' ") + ->toContain("COOLIFY_URL='https://app.example.com/path' ") + ->toContain("COOLIFY_FQDN='app.example.com' ") + ->toContain("COOLIFY_RESOURCE_UUID='app-uuid' "); }); }); From cd95ac3d0dfe818b3955aca819345c61ada3a2cb Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:56:49 +0200 Subject: [PATCH 2/3] test: cover case-sensitive application URL paths --- .../ServicesControllerUrlDuplicateTest.php | 37 +++++++++++++++++++ tests/Unit/ValidationPatternsTest.php | 7 ++++ 2 files changed, 44 insertions(+) create mode 100644 tests/Unit/ServicesControllerUrlDuplicateTest.php diff --git a/tests/Unit/ServicesControllerUrlDuplicateTest.php b/tests/Unit/ServicesControllerUrlDuplicateTest.php new file mode 100644 index 000000000..8f3e4f9c4 --- /dev/null +++ b/tests/Unit/ServicesControllerUrlDuplicateTest.php @@ -0,0 +1,37 @@ +invoke($controller, $service, [ + ['name' => 'web', 'url' => 'https://example.com/Route'], + ['name' => 'api', 'url' => 'HTTPS://EXAMPLE.COM/route'], + ], '1'); + + expect($result['errors'] ?? [])->toBe([ + "Service container with 'web' not found.", + "Service container with 'api' not found.", + ]); +}); diff --git a/tests/Unit/ValidationPatternsTest.php b/tests/Unit/ValidationPatternsTest.php index 2b5763177..cb9d0144d 100644 --- a/tests/Unit/ValidationPatternsTest.php +++ b/tests/Unit/ValidationPatternsTest.php @@ -180,3 +180,10 @@ expect($environmentVariable->key)->toBe('APP_ENV'); }); + +it('normalizes application domain scheme and host without lowercasing path query or fragment', function () { + $domains = ' HTTPS://EXAMPLE.COM/MixedCase/Path?Token=ABC#Fragment, http://Sub.EXAMPLE.com/Api/V1 '; + + expect(ValidationPatterns::normalizeApplicationDomains($domains)) + ->toBe('https://example.com/MixedCase/Path?Token=ABC#Fragment,http://sub.example.com/Api/V1'); +}); From c6c7ec1c317883fc0967f911ac4d6d47f83ab069 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:47:04 +0200 Subject: [PATCH 3/3] fix(security): validate application domains safely Use shared domain validation and normalization for application, service, and preview domains so unsafe host input is rejected consistently. Cover command-substitution payloads in application domain tests. --- app/Livewire/Project/Application/General.php | 18 ++++------ app/Livewire/Project/Application/Previews.php | 9 +++-- .../Project/Application/PreviewsCompose.php | 12 ++++++- app/Livewire/Project/Service/EditDomain.php | 23 ++++++------- app/Livewire/Project/Service/Index.php | 14 +++----- ...neralDockerRegistryImageValidationTest.php | 34 +++++++++++++++++++ 6 files changed, 72 insertions(+), 38 deletions(-) diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 7af0a275d..289c8eeb0 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -12,7 +12,6 @@ use Illuminate\Support\Collection; use Livewire\Component; use Livewire\Features\SupportEvents\Event; -use Spatie\Url\Url; class General extends Component { @@ -142,7 +141,8 @@ protected function rules(): array return [ 'name' => ValidationPatterns::nameRules(), 'description' => ValidationPatterns::descriptionRules(), - 'fqdn' => 'nullable', + 'fqdn' => ValidationPatterns::applicationDomainRules(), + 'parsedServiceDomains.*.domain' => ValidationPatterns::applicationDomainRules(), 'gitRepository' => 'required', 'gitBranch' => ['required', 'string', new ValidGitBranch], 'gitCommitSha' => ['nullable', 'string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], @@ -771,16 +771,7 @@ public function submit($showToaster = true) $oldBaseDirectory = $this->application->base_directory; // Process FQDN with intermediate variable to avoid Collection/string confusion - $this->fqdn = str($this->fqdn)->replaceEnd(',', '')->trim()->toString(); - $this->fqdn = str($this->fqdn)->replaceStart(',', '')->trim()->toString(); - $domains = str($this->fqdn)->trim()->explode(',')->map(function ($domain) { - $domain = trim($domain); - Url::fromString($domain, ['http', 'https']); - - return str($domain)->lower(); - }); - - $this->fqdn = $domains->unique()->implode(','); + $this->fqdn = ValidationPatterns::normalizeApplicationDomains($this->fqdn); $warning = sslipDomainWarning($this->fqdn); if ($warning) { $this->dispatch('warning', __('warning.sslipdomain')); @@ -863,6 +854,9 @@ public function submit($showToaster = true) } } if ($this->buildPack === 'dockercompose') { + foreach ($this->parsedServiceDomains as $serviceName => $service) { + $this->parsedServiceDomains[$serviceName]['domain'] = ValidationPatterns::normalizeApplicationDomains(data_get($service, 'domain')); + } $this->application->docker_compose_domains = json_encode($this->parsedServiceDomains); if ($this->application->isDirty('docker_compose_domains')) { foreach ($this->parsedServiceDomains as $service) { diff --git a/app/Livewire/Project/Application/Previews.php b/app/Livewire/Project/Application/Previews.php index 74b2ebce8..338f102b5 100644 --- a/app/Livewire/Project/Application/Previews.php +++ b/app/Livewire/Project/Application/Previews.php @@ -6,6 +6,7 @@ use App\Jobs\DeleteResourceJob; use App\Models\Application; use App\Models\ApplicationPreview; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Livewire\Component; @@ -117,12 +118,14 @@ public function save_preview($preview_id) }); if ($previewKey !== false && isset($this->previewFqdns[$previewKey])) { + $this->validate([ + "previewFqdns.{$previewKey}" => ValidationPatterns::applicationDomainRules(), + ]); + $fqdn = $this->previewFqdns[$previewKey]; if (! empty($fqdn)) { - $fqdn = str($fqdn)->replaceEnd(',', '')->trim(); - $fqdn = str($fqdn)->replaceStart(',', '')->trim(); - $fqdn = str($fqdn)->trim()->lower(); + $fqdn = ValidationPatterns::normalizeApplicationDomains($fqdn); $this->previewFqdns[$previewKey] = $fqdn; if (! validateDNSEntry($fqdn, $this->application->destination->server)) { diff --git a/app/Livewire/Project/Application/PreviewsCompose.php b/app/Livewire/Project/Application/PreviewsCompose.php index e8da3b45c..48392a742 100644 --- a/app/Livewire/Project/Application/PreviewsCompose.php +++ b/app/Livewire/Project/Application/PreviewsCompose.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Application; use App\Models\ApplicationPreview; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Component; use Spatie\Url\Url; @@ -33,6 +34,11 @@ public function save() { try { $this->authorize('update', $this->preview->application); + $this->validate([ + 'domain' => ValidationPatterns::applicationDomainRules(), + ]); + + $this->domain = ValidationPatterns::normalizeApplicationDomains($this->domain); $docker_compose_domains = data_get($this->preview, 'docker_compose_domains'); $docker_compose_domains = json_decode($docker_compose_domains, true) ?: []; @@ -73,9 +79,13 @@ public function generate() $preview_fqdn = str_replace('{{pr_id}}', $this->preview->pull_request_id, $preview_fqdn); $preview_fqdn = str($generated_fqdn)->before('://').'://'.$preview_fqdn; } else { + foreach (ValidationPatterns::validateApplicationDomains($domain_string) as $error) { + throw new \InvalidArgumentException($error); + } + // Use the existing domain from the main application // Handle multiple domains separated by commas - $domain_list = explode(',', $domain_string); + $domain_list = ValidationPatterns::applicationDomainList($domain_string); $preview_fqdns = []; $template = $this->preview->application->preview_url_template; $random = new_public_id(); diff --git a/app/Livewire/Project/Service/EditDomain.php b/app/Livewire/Project/Service/EditDomain.php index 7158b6e40..96fe6a62c 100644 --- a/app/Livewire/Project/Service/EditDomain.php +++ b/app/Livewire/Project/Service/EditDomain.php @@ -3,10 +3,10 @@ namespace App\Livewire\Project\Service; use App\Models\ServiceApplication; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Attributes\Validate; use Livewire\Component; -use Spatie\Url\Url; class EditDomain extends Component { @@ -28,12 +28,15 @@ class EditDomain extends Component public $requiredPort = null; - #[Validate(['nullable'])] + #[Validate] public ?string $fqdn = null; - protected $rules = [ - 'fqdn' => 'nullable', - ]; + protected function rules(): array + { + return [ + 'fqdn' => ValidationPatterns::applicationDomainRules(), + ]; + } public function mount() { @@ -82,15 +85,9 @@ public function submit() { try { $this->authorize('update', $this->application); - $this->fqdn = str($this->fqdn)->replaceEnd(',', '')->trim()->toString(); - $this->fqdn = str($this->fqdn)->replaceStart(',', '')->trim()->toString(); - $domains = str($this->fqdn)->trim()->explode(',')->map(function ($domain) { - $domain = trim($domain); - Url::fromString($domain, ['http', 'https']); + $this->validate(); - return str($domain)->lower(); - }); - $this->fqdn = $domains->unique()->implode(','); + $this->fqdn = ValidationPatterns::normalizeApplicationDomains($this->fqdn); $warning = sslipDomainWarning($this->fqdn); if ($warning) { $this->dispatch('warning', __('warning.sslipdomain')); diff --git a/app/Livewire/Project/Service/Index.php b/app/Livewire/Project/Service/Index.php index 12c0edbca..7249c8133 100644 --- a/app/Livewire/Project/Service/Index.php +++ b/app/Livewire/Project/Service/Index.php @@ -8,11 +8,11 @@ use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Livewire\Component; -use Spatie\Url\Url; class Index extends Component { @@ -480,15 +480,11 @@ public function submitApplication() { try { $this->authorize('update', $this->serviceApplication); - $this->fqdn = str($this->fqdn)->replaceEnd(',', '')->trim()->toString(); - $this->fqdn = str($this->fqdn)->replaceStart(',', '')->trim()->toString(); - $domains = str($this->fqdn)->trim()->explode(',')->map(function ($domain) { - $domain = trim($domain); - Url::fromString($domain, ['http', 'https']); + $this->validate([ + 'fqdn' => ValidationPatterns::applicationDomainRules(), + ]); - return str($domain)->lower(); - }); - $this->fqdn = $domains->unique()->implode(','); + $this->fqdn = ValidationPatterns::normalizeApplicationDomains($this->fqdn); $warning = sslipDomainWarning($this->fqdn); if ($warning) { $this->dispatch('warning', __('warning.sslipdomain')); diff --git a/tests/Feature/Livewire/ApplicationGeneralDockerRegistryImageValidationTest.php b/tests/Feature/Livewire/ApplicationGeneralDockerRegistryImageValidationTest.php index 34c889837..3ed6b8544 100644 --- a/tests/Feature/Livewire/ApplicationGeneralDockerRegistryImageValidationTest.php +++ b/tests/Feature/Livewire/ApplicationGeneralDockerRegistryImageValidationTest.php @@ -2,6 +2,40 @@ use App\Livewire\Project\Application\General; +it('uses safe domain validation rules in the application general form', function () { + $component = new General; + $method = new ReflectionMethod($component, 'rules'); + $rules = $method->invoke($component); + + $validator = validator([ + 'fqdn' => 'http://$(whoami).example.com', + ], [ + 'fqdn' => $rules['fqdn'], + ]); + + expect($validator->fails())->toBeTrue() + ->and($validator->errors()->has('fqdn'))->toBeTrue(); +}); + +it('uses safe docker compose service domain validation rules in the application general form', function () { + $component = new General; + $method = new ReflectionMethod($component, 'rules'); + $rules = $method->invoke($component); + + $validator = validator([ + 'parsedServiceDomains' => [ + 'app' => [ + 'domain' => 'http://$(whoami).example.com', + ], + ], + ], [ + 'parsedServiceDomains.*.domain' => $rules['parsedServiceDomains.*.domain'], + ]); + + expect($validator->fails())->toBeTrue() + ->and($validator->errors()->has('parsedServiceDomains.app.domain'))->toBeTrue(); +}); + it('uses safe docker registry image validation rules in the application general form', function () { $component = new General; $method = new ReflectionMethod($component, 'rules');