rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: improve application URL handling

Browse source
This commit is contained in:
Andras Bacsai 2026-07-02 16:52:07 +02:00
parent 438eeefa73
commit bbff70c8d0
6 changed files with 306 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
app/Http/Controllers/Api/ApplicationsController.php
View file

@ -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'])) {

25
app/Http/Controllers/Api/ServicesController.php
View file

@ -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;
}

32
app/Jobs/ApplicationDeploymentJob.php
View file

@ -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);
}

156
app/Support/ValidationPatterns.php
View file

@ -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<int, string>
*/
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<int, string>
*/
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
*/

2
bootstrap/helpers/api.php
View file

@ -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(),

103
tests/Feature/Security/CommandInjectionSecurityTest.php
View file

@ -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' ");
});
});