diff --git a/app/Jobs/ServerLimitCheckJob.php b/app/Jobs/ServerLimitCheckJob.php index aa82c6dad..06e94fc93 100644 --- a/app/Jobs/ServerLimitCheckJob.php +++ b/app/Jobs/ServerLimitCheckJob.php @@ -38,7 +38,7 @@ public function handle() $server->forceDisableServer(); $this->team->notify(new ForceDisabled($server)); }); - } elseif ($number_of_servers_to_disable === 0) { + } elseif ($number_of_servers_to_disable <= 0) { $servers->each(function ($server) { if ($server->isForceDisabled()) { $server->forceEnableServer(); diff --git a/app/Livewire/Settings/Advanced.php b/app/Livewire/Settings/Advanced.php index 16361ce79..ad478273f 100644 --- a/app/Livewire/Settings/Advanced.php +++ b/app/Livewire/Settings/Advanced.php @@ -95,7 +95,9 @@ public function submit() // Check if it's valid CIDR notation if (str_contains($entry, '/')) { [$ip, $mask] = explode('/', $entry); - if (filter_var($ip, FILTER_VALIDATE_IP) && is_numeric($mask) && $mask >= 0 && $mask <= 32) { + $isIpv6 = filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false; + $maxMask = $isIpv6 ? 128 : 32; + if (filter_var($ip, FILTER_VALIDATE_IP) && is_numeric($mask) && $mask >= 0 && $mask <= $maxMask) { return $entry; } $invalidEntries[] = $entry; @@ -111,7 +113,7 @@ public function submit() $invalidEntries[] = $entry; return null; - })->filter()->unique(); + })->filter()->values()->all(); if (! empty($invalidEntries)) { $this->dispatch('error', 'Invalid IP addresses or subnets: '.implode(', ', $invalidEntries)); @@ -119,13 +121,15 @@ public function submit() return; } - if ($validEntries->isEmpty()) { + if (empty($validEntries)) { $this->dispatch('error', 'No valid IP addresses or subnets provided'); return; } - $this->allowed_ips = $validEntries->implode(','); + $validEntries = deduplicateAllowlist($validEntries); + + $this->allowed_ips = implode(',', $validEntries); } $this->instantSave(); diff --git a/app/Rules/ValidIpOrCidr.php b/app/Rules/ValidIpOrCidr.php index e172ffd1a..bd0bd2296 100644 --- a/app/Rules/ValidIpOrCidr.php +++ b/app/Rules/ValidIpOrCidr.php @@ -45,7 +45,10 @@ public function validate(string $attribute, mixed $value, Closure $fail): void [$ip, $mask] = $parts; - if (! filter_var($ip, FILTER_VALIDATE_IP) || ! is_numeric($mask) || $mask < 0 || $mask > 32) { + $isIpv6 = filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false; + $maxMask = $isIpv6 ? 128 : 32; + + if (! filter_var($ip, FILTER_VALIDATE_IP) || ! is_numeric($mask) || $mask < 0 || $mask > $maxMask) { $invalidEntries[] = $entry; } } else { diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index a1cecc879..3e993dbf3 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -1416,24 +1416,48 @@ function checkIPAgainstAllowlist($ip, $allowlist) } $mask = (int) $mask; + $isIpv6Subnet = filter_var($subnet, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false; + $maxMask = $isIpv6Subnet ? 128 : 32; - // Validate mask - if ($mask < 0 || $mask > 32) { + // Validate mask for address family + if ($mask < 0 || $mask > $maxMask) { continue; } - // Calculate network addresses - $ip_long = ip2long($ip); - $subnet_long = ip2long($subnet); + if ($isIpv6Subnet) { + // IPv6 CIDR matching using binary string comparison + $ipBin = inet_pton($ip); + $subnetBin = inet_pton($subnet); - if ($ip_long === false || $subnet_long === false) { - continue; - } + if ($ipBin === false || $subnetBin === false) { + continue; + } - $mask_long = ~((1 << (32 - $mask)) - 1); + // Build a 128-bit mask from $mask prefix bits + $maskBin = str_repeat("\xff", (int) ($mask / 8)); + $remainder = $mask % 8; + if ($remainder > 0) { + $maskBin .= chr(0xFF & (0xFF << (8 - $remainder))); + } + $maskBin = str_pad($maskBin, 16, "\x00"); - if (($ip_long & $mask_long) == ($subnet_long & $mask_long)) { - return true; + if (($ipBin & $maskBin) === ($subnetBin & $maskBin)) { + return true; + } + } else { + // IPv4 CIDR matching + $ip_long = ip2long($ip); + $subnet_long = ip2long($subnet); + + if ($ip_long === false || $subnet_long === false) { + continue; + } + + $mask_long = ~((1 << (32 - $mask)) - 1); + + if (($ip_long & $mask_long) == ($subnet_long & $mask_long)) { + return true; + } } } else { // Special case: 0.0.0.0 means allow all @@ -1451,6 +1475,67 @@ function checkIPAgainstAllowlist($ip, $allowlist) return false; } +function deduplicateAllowlist(array $entries): array +{ + if (count($entries) <= 1) { + return array_values($entries); + } + + // Normalize each entry into [original, ip, mask] + $parsed = []; + foreach ($entries as $entry) { + $entry = trim($entry); + if (empty($entry)) { + continue; + } + + if ($entry === '0.0.0.0') { + // Special case: bare 0.0.0.0 means "allow all" — treat as /0 + $parsed[] = ['original' => $entry, 'ip' => '0.0.0.0', 'mask' => 0]; + } elseif (str_contains($entry, '/')) { + [$ip, $mask] = explode('/', $entry); + $parsed[] = ['original' => $entry, 'ip' => $ip, 'mask' => (int) $mask]; + } else { + $ip = $entry; + $isIpv6 = filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false; + $parsed[] = ['original' => $entry, 'ip' => $ip, 'mask' => $isIpv6 ? 128 : 32]; + } + } + + $count = count($parsed); + $redundant = array_fill(0, $count, false); + + for ($i = 0; $i < $count; $i++) { + if ($redundant[$i]) { + continue; + } + + for ($j = 0; $j < $count; $j++) { + if ($i === $j || $redundant[$j]) { + continue; + } + + // Entry $j is redundant if its mask is narrower/equal (>=) than $i's mask + // AND $j's network IP falls within $i's CIDR range + if ($parsed[$j]['mask'] >= $parsed[$i]['mask']) { + $cidr = $parsed[$i]['ip'].'/'.$parsed[$i]['mask']; + if (checkIPAgainstAllowlist($parsed[$j]['ip'], [$cidr])) { + $redundant[$j] = true; + } + } + } + } + + $result = []; + for ($i = 0; $i < $count; $i++) { + if (! $redundant[$i]) { + $result[] = $parsed[$i]['original']; + } + } + + return $result; +} + function get_public_ips() { try { diff --git a/tests/Feature/ServerLimitCheckJobTest.php b/tests/Feature/ServerLimitCheckJobTest.php new file mode 100644 index 000000000..6b2c074be --- /dev/null +++ b/tests/Feature/ServerLimitCheckJobTest.php @@ -0,0 +1,83 @@ +set('constants.coolify.self_hosted', false); + + Notification::fake(); + + $this->team = Team::factory()->create(['custom_server_limit' => 5]); +}); + +function createServerForTeam(Team $team, bool $forceDisabled = false): Server +{ + $server = Server::factory()->create(['team_id' => $team->id]); + if ($forceDisabled) { + $server->settings()->update(['force_disabled' => true]); + } + + return $server->fresh(['settings']); +} + +it('re-enables force-disabled servers when under the limit', function () { + createServerForTeam($this->team); + $server2 = createServerForTeam($this->team, forceDisabled: true); + $server3 = createServerForTeam($this->team, forceDisabled: true); + + expect($server2->settings->force_disabled)->toBeTruthy(); + expect($server3->settings->force_disabled)->toBeTruthy(); + + // 3 servers, limit 5 → all should be re-enabled + ServerLimitCheckJob::dispatchSync($this->team); + + expect($server2->fresh()->settings->force_disabled)->toBeFalsy(); + expect($server3->fresh()->settings->force_disabled)->toBeFalsy(); +}); + +it('re-enables force-disabled servers when exactly at the limit', function () { + $this->team->update(['custom_server_limit' => 3]); + + createServerForTeam($this->team); + createServerForTeam($this->team); + $server3 = createServerForTeam($this->team, forceDisabled: true); + + // 3 servers, limit 3 → disabled one should be re-enabled + ServerLimitCheckJob::dispatchSync($this->team); + + expect($server3->fresh()->settings->force_disabled)->toBeFalsy(); +}); + +it('disables newest servers when over the limit', function () { + $this->team->update(['custom_server_limit' => 2]); + + $oldest = createServerForTeam($this->team); + sleep(1); + $middle = createServerForTeam($this->team); + sleep(1); + $newest = createServerForTeam($this->team); + + // 3 servers, limit 2 → newest 1 should be disabled + ServerLimitCheckJob::dispatchSync($this->team); + + expect($oldest->fresh()->settings->force_disabled)->toBeFalsy(); + expect($middle->fresh()->settings->force_disabled)->toBeFalsy(); + expect($newest->fresh()->settings->force_disabled)->toBeTruthy(); +}); + +it('does not change servers when under limit and none are force-disabled', function () { + $server1 = createServerForTeam($this->team); + $server2 = createServerForTeam($this->team); + + // 2 servers, limit 5 → nothing to do + ServerLimitCheckJob::dispatchSync($this->team); + + expect($server1->fresh()->settings->force_disabled)->toBeFalsy(); + expect($server2->fresh()->settings->force_disabled)->toBeFalsy(); +});