From 0066b1c6a0bce4fc40bd48144fbe8dedb6e714e5 Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 27 Jul 2026 03:09:13 +0200 Subject: [PATCH] feat(security): a way back into the console that does not need the console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allowlist is managed in the console, which is fine until the address you manage it from changes — and then the page that would fix the problem is the page the problem blocks. Every gate needs a door that does not depend on itself, and on a server that door is a shell: php artisan clupilot:console-access show php artisan clupilot:console-access allow 203.0.113.7 php artisan clupilot:console-access open The address check moved to RestrictConsoleNetwork::isNetwork() so the command and the console apply the same rule. Codex caught the version that did not: a typo like 203.0.113.9/99 was stored, reported as success, and matched nothing — leaving whoever was recovering still locked out, now believing they were not. Co-Authored-By: Claude Opus 5 --- app/Console/Commands/ConsoleAccess.php | 103 ++++++++++++++++++ .../Middleware/RestrictConsoleNetwork.php | 25 +++++ app/Livewire/Admin/Settings.php | 18 +-- tests/Feature/Admin/ConsoleNetworkTest.php | 19 ++++ 4 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 app/Console/Commands/ConsoleAccess.php diff --git a/app/Console/Commands/ConsoleAccess.php b/app/Console/Commands/ConsoleAccess.php new file mode 100644 index 0000000..01a1b4b --- /dev/null +++ b/app/Console/Commands/ConsoleAccess.php @@ -0,0 +1,103 @@ +argument('action'); + $value = trim((string) $this->argument('value')); + + return match ($action) { + 'show' => $this->show(), + 'allow' => $this->allow($value), + 'deny' => $this->deny($value), + 'open' => $this->setRestricted(false), + 'close' => $this->setRestricted(true), + default => $this->refuse("Unknown action: {$action}"), + }; + } + + private function show(): int + { + $this->line(' restricted : '.(RestrictConsoleNetwork::isRestricted() ? 'yes' : 'no — anyone reaching the hostname gets in')); + $this->line(' always : '.implode(', ', (array) config('admin_access.trusted_ranges', [])).' (VPN, not removable)'); + + $own = (array) Settings::get('console.allowed_ips', []); + $this->line(' additional : '.($own === [] ? '(none)' : implode(', ', $own))); + + return self::SUCCESS; + } + + private function allow(string $value): int + { + if ($value === '') { + return $this->refuse('Give an address or range: clupilot:console-access allow 203.0.113.7'); + } + + // An entry that matches nothing would be stored, reported as success, + // and leave whoever is recovering still locked out. + if (! RestrictConsoleNetwork::isNetwork($value)) { + return $this->refuse("Not an address or a range: {$value}"); + } + + $list = (array) Settings::get('console.allowed_ips', []); + + if (! in_array($value, $list, true)) { + $list[] = $value; + Settings::set('console.allowed_ips', array_values($list)); + } + + $this->info("{$value} may now reach the console."); + + return $this->show(); + } + + private function deny(string $value): int + { + Settings::set('console.allowed_ips', array_values(array_filter( + (array) Settings::get('console.allowed_ips', []), + fn ($entry) => $entry !== $value, + ))); + + $this->info("{$value} removed."); + + return $this->show(); + } + + private function setRestricted(bool $on): int + { + // No lock-out check here on purpose: this command IS the recovery path, + // and it is only reachable by someone who already has the server. + Settings::set('console.network_restricted', $on); + $this->info($on ? 'Console restricted to the VPN and the listed addresses.' : 'Restriction lifted.'); + + return $this->show(); + } + + private function refuse(string $message): int + { + $this->error($message); + + return self::FAILURE; + } +} diff --git a/app/Http/Middleware/RestrictConsoleNetwork.php b/app/Http/Middleware/RestrictConsoleNetwork.php index e7570bb..b3d70c4 100644 --- a/app/Http/Middleware/RestrictConsoleNetwork.php +++ b/app/Http/Middleware/RestrictConsoleNetwork.php @@ -98,6 +98,31 @@ class RestrictConsoleNetwork return self::covers($ip, array_merge($vpn, $extra)); } + /** + * Whether a value is an address or a CIDR range at all. + * + * Shared by the console and the recovery command on purpose: an entry that + * matches nothing is stored happily and reports success, and the operator + * then switches the restriction on believing they are covered. That is the + * exact situation the recovery command exists to get out of. + */ + public static function isNetwork(string $value): bool + { + [$address, $prefix] = array_pad(explode('/', $value, 2), 2, null); + + if (filter_var($address, FILTER_VALIDATE_IP) === false) { + return false; + } + + if ($prefix === null) { + return true; + } + + $max = str_contains((string) $address, ':') ? 128 : 32; + + return ctype_digit((string) $prefix) && (int) $prefix >= 0 && (int) $prefix <= $max; + } + /** @param array $ranges */ private static function covers(string $ip, array $ranges): bool { diff --git a/app/Livewire/Admin/Settings.php b/app/Livewire/Admin/Settings.php index 3013611..9b82532 100644 --- a/app/Livewire/Admin/Settings.php +++ b/app/Livewire/Admin/Settings.php @@ -251,7 +251,7 @@ class Settings extends Component $value = trim($this->consoleIp); - if (! $this->looksLikeNetwork($value)) { + if (! \App\Http\Middleware\RestrictConsoleNetwork::isNetwork($value)) { $this->addError('consoleIp', __('admin_settings.console_ip_invalid')); return; @@ -321,22 +321,6 @@ class Settings extends Component $this->dispatch('notify', message: __($on ? 'admin_settings.console_locked' : 'admin_settings.console_unlocked')); } - private function looksLikeNetwork(string $value): bool - { - [$address, $prefix] = array_pad(explode('/', $value, 2), 2, null); - - if (filter_var($address, FILTER_VALIDATE_IP) === false) { - return false; - } - - if ($prefix === null) { - return true; - } - - $max = str_contains($address, ':') ? 128 : 32; - - return ctype_digit($prefix) && (int) $prefix >= 0 && (int) $prefix <= $max; - } public function render() { diff --git a/tests/Feature/Admin/ConsoleNetworkTest.php b/tests/Feature/Admin/ConsoleNetworkTest.php index 9150155..3c9e7c4 100644 --- a/tests/Feature/Admin/ConsoleNetworkTest.php +++ b/tests/Feature/Admin/ConsoleNetworkTest.php @@ -110,3 +110,22 @@ it('is re-checked on every Livewire action, not only on the page', function () { expect($persistent)->toContain(RestrictConsoleNetwork::class); }); + +it('offers a way back in that does not depend on the console', function () { + // Every gate needs a door that does not depend on itself: the list is + // managed from the console, and the address you manage it from can change. + Settings::set('console.network_restricted', true); + Settings::set('console.allowed_ips', []); + + $this->artisan('clupilot:console-access allow 203.0.113.42')->assertSuccessful(); + expect(Settings::get('console.allowed_ips'))->toBe(['203.0.113.42']) + ->and(RestrictConsoleNetwork::wouldStillAllow('203.0.113.42', ['203.0.113.42']))->toBeTrue(); + + $this->artisan('clupilot:console-access deny 203.0.113.42')->assertSuccessful(); + expect(Settings::get('console.allowed_ips'))->toBe([]); + + // And the restriction itself can be lifted from the shell — the only place + // that is reachable when the gate has shut on its own owner. + $this->artisan('clupilot:console-access open')->assertSuccessful(); + expect(RestrictConsoleNetwork::isRestricted())->toBeFalse(); +});