refactor(dns-validation): rename DNS validation functions for consistency and clarity, and remove unused code

This commit is contained in:
Andras Bacsai
2025-09-09 09:00:35 +02:00
parent ccc9ceb734
commit a7671ed379
6 changed files with 38 additions and 110 deletions

View File

@@ -28,7 +28,7 @@ class ApiAllowed
$allowedIps = array_map('trim', $allowedIps);
$allowedIps = array_filter($allowedIps); // Remove empty entries
if (! empty($allowedIps) && ! check_ip_against_allowlist($request->ip(), $allowedIps)) {
if (! empty($allowedIps) && ! checkIPAgainstAllowlist($request->ip(), $allowedIps)) {
return response()->json(['success' => true, 'message' => 'You are not allowed to access the API.'], 403);
}
}

View File

@@ -487,7 +487,7 @@ class General extends Component
$domains = str($this->application->fqdn)->trim()->explode(',');
if ($this->application->additional_servers->count() === 0) {
foreach ($domains as $domain) {
if (! validate_dns_entry($domain, $this->application->destination->server)) {
if (! validateDNSEntry($domain, $this->application->destination->server)) {
$showToaster && $this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.<br><br>$domain->{$this->application->destination->server->ip}<br><br>Check this <a target='_blank' class='underline dark:text-white' href='https://coolify.io/docs/knowledge-base/dns-configuration'>documentation</a> for further help.");
}
}
@@ -615,7 +615,7 @@ class General extends Component
foreach ($this->parsedServiceDomains as $service) {
$domain = data_get($service, 'domain');
if ($domain) {
if (! validate_dns_entry($domain, $this->application->destination->server)) {
if (! validateDNSEntry($domain, $this->application->destination->server)) {
$showToaster && $this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.<br><br>$domain->{$this->application->destination->server->ip}<br><br>Check this <a target='_blank' class='underline dark:text-white' href='https://coolify.io/docs/knowledge-base/dns-configuration'>documentation</a> for further help.");
}
}

View File

@@ -77,7 +77,7 @@ class Previews extends Component
$preview->fqdn = str($preview->fqdn)->replaceEnd(',', '')->trim();
$preview->fqdn = str($preview->fqdn)->replaceStart(',', '')->trim();
$preview->fqdn = str($preview->fqdn)->trim()->lower();
if (! validate_dns_entry($preview->fqdn, $this->application->destination->server)) {
if (! validateDNSEntry($preview->fqdn, $this->application->destination->server)) {
$this->dispatch('error', 'Validating DNS failed.', "Make sure you have added the DNS records correctly.<br><br>$preview->fqdn->{$this->application->destination->server->ip}<br><br>Check this <a target='_blank' class='underline dark:text-white' href='https://coolify.io/docs/knowledge-base/dns-configuration'>documentation</a> for further help.");
$success = false;
}

View File

@@ -115,7 +115,7 @@ class Index extends Component
$this->validate();
if ($this->settings->is_dns_validation_enabled && $this->fqdn) {
if (! validate_dns_entry($this->fqdn, $this->server)) {
if (! validateDNSEntry($this->fqdn, $this->server)) {
$this->dispatch('error', "Validating DNS failed.<br><br>Make sure you have added the DNS records correctly.<br><br>{$this->fqdn}->{$this->server->ip}<br><br>Check this <a target='_blank' class='underline dark:text-white' href='https://coolify.io/docs/knowledge-base/dns-configuration'>documentation</a> for further help.");
$error_show = true;
}

View File

@@ -961,7 +961,7 @@ function getRealtime()
}
}
function validate_dns_entry(string $fqdn, Server $server)
function validateDNSEntry(string $fqdn, Server $server)
{
// https://www.cloudflare.com/ips-v4/#
$cloudflare_ips = collect(['173.245.48.0/20', '103.21.244.0/22', '103.22.200.0/22', '103.31.4.0/22', '141.101.64.0/18', '108.162.192.0/18', '190.93.240.0/20', '188.114.96.0/20', '197.234.240.0/22', '198.41.128.0/17', '162.158.0.0/15', '104.16.0.0/13', '172.64.0.0/13', '131.0.72.0/22']);
@@ -994,7 +994,7 @@ function validate_dns_entry(string $fqdn, Server $server)
} else {
foreach ($results as $result) {
if ($result->getType() == $type) {
if (ip_match($result->getData(), $cloudflare_ips->toArray(), $match)) {
if (ipMatch($result->getData(), $cloudflare_ips->toArray(), $match)) {
$found_matching_ip = true;
break;
}
@@ -1012,7 +1012,7 @@ function validate_dns_entry(string $fqdn, Server $server)
return $found_matching_ip;
}
function ip_match($ip, $cidrs, &$match = null)
function ipMatch($ip, $cidrs, &$match = null)
{
foreach ((array) $cidrs as $cidr) {
[$subnet, $mask] = explode('/', $cidr);
@@ -1026,7 +1026,7 @@ function ip_match($ip, $cidrs, &$match = null)
return false;
}
function check_ip_against_allowlist($ip, $allowlist)
function checkIPAgainstAllowlist($ip, $allowlist)
{
if (empty($allowlist)) {
return false;
@@ -1084,78 +1084,6 @@ function check_ip_against_allowlist($ip, $allowlist)
return false;
}
function parseCommandsByLineForSudo(Collection $commands, Server $server): array
{
$commands = $commands->map(function ($line) {
if (
! str(trim($line))->startsWith([
'cd',
'command',
'echo',
'true',
'if',
'fi',
])
) {
return "sudo $line";
}
if (str(trim($line))->startsWith('if')) {
return str_replace('if', 'if sudo', $line);
}
return $line;
});
$commands = $commands->map(function ($line) use ($server) {
if (Str::startsWith($line, 'sudo mkdir -p')) {
return "$line && sudo chown -R $server->user:$server->user ".Str::after($line, 'sudo mkdir -p').' && sudo chmod -R o-rwx '.Str::after($line, 'sudo mkdir -p');
}
return $line;
});
$commands = $commands->map(function ($line) {
$line = str($line);
if (str($line)->contains('$(')) {
$line = $line->replace('$(', '$(sudo ');
}
if (str($line)->contains('||')) {
$line = $line->replace('||', '|| sudo');
}
if (str($line)->contains('&&')) {
$line = $line->replace('&&', '&& sudo');
}
if (str($line)->contains(' | ')) {
$line = $line->replace(' | ', ' | sudo ');
}
return $line->value();
});
return $commands->toArray();
}
function parseLineForSudo(string $command, Server $server): string
{
if (! str($command)->startSwith('cd') && ! str($command)->startSwith('command')) {
$command = "sudo $command";
}
if (Str::startsWith($command, 'sudo mkdir -p')) {
$command = "$command && sudo chown -R $server->user:$server->user ".Str::after($command, 'sudo mkdir -p').' && sudo chmod -R o-rwx '.Str::after($command, 'sudo mkdir -p');
}
if (str($command)->contains('$(') || str($command)->contains('`')) {
$command = str($command)->replace('$(', '$(sudo ')->replace('`', '`sudo ')->value();
}
if (str($command)->contains('||')) {
$command = str($command)->replace('||', '|| sudo ')->value();
}
if (str($command)->contains('&&')) {
$command = str($command)->replace('&&', '&& sudo ')->value();
}
return $command;
}
function get_public_ips()
{
try {

View File

@@ -8,7 +8,7 @@ test('IP allowlist with single IPs', function () {
];
foreach ($testCases as $case) {
$result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
$result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
expect($result)->toBe($case['expected']);
}
});
@@ -24,7 +24,7 @@ test('IP allowlist with CIDR notation', function () {
];
foreach ($testCases as $case) {
$result = check_ip_against_allowlist($case['ip'], $case['allowlist']);
$result = checkIPAgainstAllowlist($case['ip'], $case['allowlist']);
expect($result)->toBe($case['expected']);
}
});
@@ -40,16 +40,16 @@ test('IP allowlist with 0.0.0.0 allows all', function () {
// Test 0.0.0.0 without subnet
foreach ($testIps as $ip) {
$result = check_ip_against_allowlist($ip, ['0.0.0.0']);
$result = checkIPAgainstAllowlist($ip, ['0.0.0.0']);
expect($result)->toBeTrue();
}
// Test 0.0.0.0 with any subnet notation - should still allow all
foreach ($testIps as $ip) {
expect(check_ip_against_allowlist($ip, ['0.0.0.0/0']))->toBeTrue();
expect(check_ip_against_allowlist($ip, ['0.0.0.0/8']))->toBeTrue();
expect(check_ip_against_allowlist($ip, ['0.0.0.0/24']))->toBeTrue();
expect(check_ip_against_allowlist($ip, ['0.0.0.0/32']))->toBeTrue();
expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/0']))->toBeTrue();
expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/8']))->toBeTrue();
expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/24']))->toBeTrue();
expect(checkIPAgainstAllowlist($ip, ['0.0.0.0/32']))->toBeTrue();
}
});
@@ -66,44 +66,44 @@ test('IP allowlist with mixed entries', function () {
];
foreach ($testCases as $case) {
$result = check_ip_against_allowlist($case['ip'], $allowlist);
$result = checkIPAgainstAllowlist($case['ip'], $allowlist);
expect($result)->toBe($case['expected']);
}
});
test('IP allowlist handles empty and invalid entries', function () {
// Empty allowlist blocks all
expect(check_ip_against_allowlist('192.168.1.1', []))->toBeFalse();
expect(check_ip_against_allowlist('192.168.1.1', ['']))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.1', []))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.1', ['']))->toBeFalse();
// Handles spaces
expect(check_ip_against_allowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
expect(check_ip_against_allowlist('10.0.0.5', [' 10.0.0.0/8 ']))->toBeTrue();
expect(checkIPAgainstAllowlist('192.168.1.100', [' 192.168.1.100 ']))->toBeTrue();
expect(checkIPAgainstAllowlist('10.0.0.5', [' 10.0.0.0/8 ']))->toBeTrue();
// Invalid entries are skipped
expect(check_ip_against_allowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/-1']))->toBeFalse(); // Invalid mask
expect(checkIPAgainstAllowlist('192.168.1.1', ['invalid.ip']))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/33']))->toBeFalse(); // Invalid mask
expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/-1']))->toBeFalse(); // Invalid mask
});
test('IP allowlist with various subnet sizes', function () {
// /32 - single host
expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.1/32']))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.1/32']))->toBeTrue();
expect(checkIPAgainstAllowlist('192.168.1.2', ['192.168.1.1/32']))->toBeFalse();
// /31 - point-to-point link
expect(check_ip_against_allowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
expect(check_ip_against_allowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
expect(check_ip_against_allowlist('192.168.1.2', ['192.168.1.0/31']))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.0', ['192.168.1.0/31']))->toBeTrue();
expect(checkIPAgainstAllowlist('192.168.1.1', ['192.168.1.0/31']))->toBeTrue();
expect(checkIPAgainstAllowlist('192.168.1.2', ['192.168.1.0/31']))->toBeFalse();
// /16 - class B
expect(check_ip_against_allowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
expect(check_ip_against_allowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
expect(check_ip_against_allowlist('172.17.0.1', ['172.16.0.0/16']))->toBeFalse();
expect(checkIPAgainstAllowlist('172.16.0.1', ['172.16.0.0/16']))->toBeTrue();
expect(checkIPAgainstAllowlist('172.16.255.255', ['172.16.0.0/16']))->toBeTrue();
expect(checkIPAgainstAllowlist('172.17.0.1', ['172.16.0.0/16']))->toBeFalse();
// /0 - all addresses
expect(check_ip_against_allowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
expect(check_ip_against_allowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
expect(checkIPAgainstAllowlist('1.1.1.1', ['0.0.0.0/0']))->toBeTrue();
expect(checkIPAgainstAllowlist('255.255.255.255', ['0.0.0.0/0']))->toBeTrue();
});
test('IP allowlist comma-separated string input', function () {
@@ -111,10 +111,10 @@ test('IP allowlist comma-separated string input', function () {
$allowlistString = '192.168.1.100,10.0.0.0/8,172.16.0.0/16';
$allowlist = explode(',', $allowlistString);
expect(check_ip_against_allowlist('192.168.1.100', $allowlist))->toBeTrue();
expect(check_ip_against_allowlist('10.5.5.5', $allowlist))->toBeTrue();
expect(check_ip_against_allowlist('172.16.10.10', $allowlist))->toBeTrue();
expect(check_ip_against_allowlist('8.8.8.8', $allowlist))->toBeFalse();
expect(checkIPAgainstAllowlist('192.168.1.100', $allowlist))->toBeTrue();
expect(checkIPAgainstAllowlist('10.5.5.5', $allowlist))->toBeTrue();
expect(checkIPAgainstAllowlist('172.16.10.10', $allowlist))->toBeTrue();
expect(checkIPAgainstAllowlist('8.8.8.8', $allowlist))->toBeFalse();
});
test('ValidIpOrCidr validation rule', function () {