Fix round: CGNAT, guarded write/delete, and a silent host is not a missing template
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feature/betriebsmodus
nexxo
parent
2eff3a5a3c
commit
8321e825eb
|
|
@ -56,27 +56,52 @@ final class DnsTokenCheck
|
|||
// The actual point of this whole check. A read-only token gets this
|
||||
// far and looks in the console like a working one; it fails only when
|
||||
// a customer VM needs its A record — after payment.
|
||||
$probe = Http::withHeaders(['Auth-API-Token' => $token])->acceptJson()->timeout(15)
|
||||
->post('https://dns.hetzner.com/api/v1/records', [
|
||||
'zone_id' => $zoneId,
|
||||
'type' => 'TXT',
|
||||
'name' => '_clupilot-write-probe-'.Str::lower(Str::random(12)),
|
||||
'value' => 'clupilot readiness probe',
|
||||
'ttl' => 60,
|
||||
]);
|
||||
//
|
||||
// Guarded the same way as the GET above: this call is just as capable
|
||||
// of hitting a broken network as the first one, and "the network
|
||||
// died mid-write" must stay distinct from "the token cannot write"
|
||||
// (read_only, below) — both are failures, but only one of them says
|
||||
// anything about the token.
|
||||
try {
|
||||
$probe = Http::withHeaders(['Auth-API-Token' => $token])->acceptJson()->timeout(15)
|
||||
->post('https://dns.hetzner.com/api/v1/records', [
|
||||
'zone_id' => $zoneId,
|
||||
'type' => 'TXT',
|
||||
'name' => '_clupilot-write-probe-'.Str::lower(Str::random(12)),
|
||||
'value' => 'clupilot readiness probe',
|
||||
'ttl' => 60,
|
||||
]);
|
||||
} catch (Throwable) {
|
||||
return ['ok' => false, 'reason' => 'unreachable'];
|
||||
}
|
||||
|
||||
if (! $probe->successful()) {
|
||||
return ['ok' => false, 'reason' => 'read_only', 'status' => $probe->status()];
|
||||
}
|
||||
|
||||
// Always clean up, even when the delete itself fails: a leftover TXT
|
||||
// record with this name is harmless, but has to be named in the
|
||||
// result so someone can remove it by hand.
|
||||
// The write already proved the token can write — that is the whole
|
||||
// point of this check, and nothing past this line changes that
|
||||
// verdict. Always attempt to clean up, and guard the attempt itself:
|
||||
// a delete that throws is handled exactly like one that comes back
|
||||
// non-2xx, because both leave the same TXT record sitting in the
|
||||
// zone. Named in the result either way, so someone can remove it by
|
||||
// hand instead of a record nobody knows exists.
|
||||
$recordId = $probe->json('record.id');
|
||||
$removed = Http::withHeaders(['Auth-API-Token' => $token])->acceptJson()->timeout(15)
|
||||
->delete('https://dns.hetzner.com/api/v1/records/'.$recordId)
|
||||
->successful();
|
||||
|
||||
return ['ok' => true, 'reason' => 'writable', 'probe_removed' => $removed];
|
||||
try {
|
||||
$removed = Http::withHeaders(['Auth-API-Token' => $token])->acceptJson()->timeout(15)
|
||||
->delete('https://dns.hetzner.com/api/v1/records/'.$recordId)
|
||||
->successful();
|
||||
} catch (Throwable) {
|
||||
$removed = false;
|
||||
}
|
||||
|
||||
$result = ['ok' => true, 'reason' => 'writable', 'probe_removed' => $removed];
|
||||
|
||||
if (! $removed) {
|
||||
$result['leftover_record_id'] = $recordId;
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,9 +43,11 @@ final class VmTemplateCheck
|
|||
}
|
||||
|
||||
$missing = [];
|
||||
$unverifiable = [];
|
||||
|
||||
foreach ($required as $vmid) {
|
||||
$found = false;
|
||||
$aHostFailedToAnswer = false;
|
||||
|
||||
foreach ($hosts as $host) {
|
||||
try {
|
||||
|
|
@ -55,18 +57,48 @@ final class VmTemplateCheck
|
|||
}
|
||||
} catch (Throwable) {
|
||||
// A host that is not answering right now is not proof of
|
||||
// a missing template. Keep looking at the others.
|
||||
// a missing template — it might be sitting on exactly
|
||||
// this host. Keep looking at the others; a later "yes"
|
||||
// still wins outright (see $found above).
|
||||
$aHostFailedToAnswer = true;
|
||||
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
if (! $found) {
|
||||
if ($found) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Two different verdicts, not one: every host answered "no" is a
|
||||
// confirmed gap (build the template); some host never answered
|
||||
// is merely unverified (ask again once it is back). Telling an
|
||||
// operator to build something that may already exist on a host
|
||||
// that is simply not responding right now sends them to fix the
|
||||
// wrong problem.
|
||||
if ($aHostFailedToAnswer) {
|
||||
$unverifiable[] = $vmid;
|
||||
} else {
|
||||
$missing[] = $vmid;
|
||||
}
|
||||
}
|
||||
|
||||
return $missing === []
|
||||
? ['ok' => true, 'reason' => 'present']
|
||||
: ['ok' => false, 'reason' => 'template_missing', 'vmids' => $missing];
|
||||
if ($missing !== []) {
|
||||
$result = ['ok' => false, 'reason' => 'template_missing', 'vmids' => $missing];
|
||||
|
||||
if ($unverifiable !== []) {
|
||||
// Named separately, never merged into vmids: one is
|
||||
// confirmed and actionable, the other merely unanswered.
|
||||
$result['unverifiable'] = $unverifiable;
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
if ($unverifiable !== []) {
|
||||
return ['ok' => false, 'reason' => 'hosts_unreachable', 'vmids' => $unverifiable];
|
||||
}
|
||||
|
||||
return ['ok' => true, 'reason' => 'present'];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,18 +22,51 @@ use Illuminate\Support\Str;
|
|||
final class WireguardEndpointCheck
|
||||
{
|
||||
/**
|
||||
* RFC 5737 "TEST-NET" ranges: documentation and example addresses, never
|
||||
* routed on the real internet. PHP's own FILTER_FLAG_NO_RES_RANGE does
|
||||
* NOT reject these (verified: 203.0.113.11 passes filter_var with that
|
||||
* flag) — exactly the range a seed value or a copy-pasted example lands
|
||||
* in, which is precisely the failure mode this check exists to catch.
|
||||
* IPv4 ranges FILTER_FLAG_NO_PRIV_RANGE|FILTER_FLAG_NO_RES_RANGE does NOT
|
||||
* reject, checked one by one against this PHP build (`php -r
|
||||
* 'var_dump(filter_var($ip, FILTER_VALIDATE_IP,
|
||||
* FILTER_FLAG_NO_PRIV_RANGE|FILTER_FLAG_NO_RES_RANGE));'`) so the next
|
||||
* reader does not have to repeat the research:
|
||||
*
|
||||
* Included below, because each is a shape a real installation can
|
||||
* actually end up with:
|
||||
* - 192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24 (RFC 5737 "TEST-NET"):
|
||||
* documentation/example addresses — seed data and copy-pasted examples
|
||||
* land here. This was the original gap (Task 9 review).
|
||||
* - 100.64.0.0/10 (RFC 6598, Carrier-Grade NAT): an ISP hands this out to
|
||||
* a real customer connection; it is exactly as unreachable from the
|
||||
* outside as RFC 1918, just not covered by NO_PRIV_RANGE. Confirmed
|
||||
* passing filter_var's flags before this fix (fix-round finding).
|
||||
* - 224.0.0.0/4 (multicast): not a shape a *mistake* produces, but never
|
||||
* a single host's own reachable address either — no WireGuard peer's
|
||||
* endpoint can legitimately be a multicast address, so it belongs in
|
||||
* the same bucket as the others rather than being let through by
|
||||
* omission.
|
||||
*
|
||||
* Already rejected by the filter_var flags above WITHOUT this list, so
|
||||
* deliberately NOT duplicated here (verified empirically, not assumed):
|
||||
* 0.0.0.0/8, 127.0.0.0/8 (loopback), 169.254.0.0/16 (link-local),
|
||||
* 240.0.0.0/4 (reserved — this range also covers the 255.255.255.255
|
||||
* broadcast address, confirmed separately).
|
||||
*
|
||||
* Deliberately left UNCHECKED, a considered omission and not an
|
||||
* oversight: 192.0.0.0/24 (IETF protocol assignments), 192.88.99.0/24
|
||||
* (deprecated 6to4 relay anycast), 198.18.0.0/15 (RFC 2544 benchmark
|
||||
* testing). All three pass filter_var's flags today, but none of them is
|
||||
* a shape an operator's real configuration plausibly lands in by
|
||||
* accident or by a real provider's assignment — unlike the TEST-NET
|
||||
* ranges (copy-pasted examples) and CGNAT (an ISP genuinely assigns it).
|
||||
* Adding checks for ranges nobody will ever hit is effort spent on the
|
||||
* task's shape, not its risk (R22).
|
||||
*
|
||||
* @var array<int, string>
|
||||
*/
|
||||
private const DOCUMENTATION_RANGES = [
|
||||
'192.0.2.0/24', // TEST-NET-1
|
||||
'198.51.100.0/24', // TEST-NET-2
|
||||
'203.0.113.0/24', // TEST-NET-3
|
||||
private const NON_PUBLIC_RANGES = [
|
||||
'192.0.2.0/24', // TEST-NET-1 (RFC 5737)
|
||||
'198.51.100.0/24', // TEST-NET-2 (RFC 5737)
|
||||
'203.0.113.0/24', // TEST-NET-3 (RFC 5737)
|
||||
'100.64.0.0/10', // Carrier-Grade NAT (RFC 6598)
|
||||
'224.0.0.0/4', // Multicast
|
||||
];
|
||||
|
||||
/** @return array<string, mixed> */
|
||||
|
|
@ -63,14 +96,14 @@ final class WireguardEndpointCheck
|
|||
// at one host on 2026-07-30.
|
||||
$public = filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE);
|
||||
|
||||
if ($public === false || $this->isDocumentationAddress($host)) {
|
||||
if ($public === false || $this->isKnownNonPublicRange($host)) {
|
||||
return ['ok' => false, 'reason' => 'not_public', 'address' => $host];
|
||||
}
|
||||
|
||||
return ['ok' => true, 'reason' => 'public', 'address' => $host];
|
||||
}
|
||||
|
||||
private function isDocumentationAddress(string $ip): bool
|
||||
private function isKnownNonPublicRange(string $ip): bool
|
||||
{
|
||||
if (! filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
|
||||
return false;
|
||||
|
|
@ -78,7 +111,7 @@ final class WireguardEndpointCheck
|
|||
|
||||
$long = ip2long($ip);
|
||||
|
||||
foreach (self::DOCUMENTATION_RANGES as $cidr) {
|
||||
foreach (self::NON_PUBLIC_RANGES as $cidr) {
|
||||
[$subnet, $bits] = explode('/', $cidr);
|
||||
$mask = -1 << (32 - (int) $bits);
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ use App\Services\Proxmox\ProxmoxClient;
|
|||
use App\Services\Proxmox\VmTemplateCheck;
|
||||
use App\Services\Vpn\WireguardEndpointCheck;
|
||||
use App\Support\Settings;
|
||||
use Illuminate\Http\Client\ConnectionException;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Illuminate\Support\Str;
|
||||
|
||||
|
|
@ -68,6 +69,27 @@ it('rejects a blank endpoint', function () {
|
|||
expect((new WireguardEndpointCheck)->run('')['reason'])->toBe('missing');
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 1): FILTER_FLAG_NO_PRIV_RANGE|FILTER_FLAG_NO_RES_RANGE
|
||||
* does not reject RFC 6598 Carrier-Grade NAT — verified live against
|
||||
* `100.64.0.5:51820` before this fix, which came back `ok: true`. A host
|
||||
* behind CGNAT is exactly as unreachable from outside as one behind RFC 1918,
|
||||
* it is just an ISP handing the address out instead of an operator.
|
||||
*/
|
||||
it('rejects a carrier-grade NAT address', function () {
|
||||
Http::fake();
|
||||
expect((new WireguardEndpointCheck)->run('100.64.0.5:51820')['ok'])->toBeFalse();
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 1, vollständiger Durchgang): kein Mehrfach-Empfänger ist
|
||||
* je der eigene Endpunkt eines einzelnen WireGuard-Peers.
|
||||
*/
|
||||
it('rejects a multicast address', function () {
|
||||
Http::fake();
|
||||
expect((new WireguardEndpointCheck)->run('224.1.1.1:51820')['ok'])->toBeFalse();
|
||||
});
|
||||
|
||||
// --- DnsTokenCheck --------------------------------------------------------
|
||||
//
|
||||
// SICHERHEITSAUFLAGE: jeder dieser Tests benutzt Http::fake() und darf unter
|
||||
|
|
@ -141,6 +163,53 @@ it('tells a read-only token apart from one that can write', function () {
|
|||
Http::assertNotSent(fn ($request) => $request->method() === 'DELETE');
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 2): Das GET auf /zones ist gegen Throwable abgesichert
|
||||
* (Reason "unreachable"), der Schreibversuch danach war es nicht — ein
|
||||
* Netzwerkfehler genau dort hätte die Ausnahme aus run() herausgeworfen, auf
|
||||
* einer Seite, deren einziger Zweck ist, ruhig zu berichten. "Netz kaputt"
|
||||
* bleibt dabei von "Token darf nicht schreiben" (read_only, 403 s.o.) und von
|
||||
* "Zone falsch benannt" getrennt — dieselbe Reason wie beim GET, weil es
|
||||
* dieselbe Fehlerklasse ist, nur an einer anderen Stelle der Kette.
|
||||
*/
|
||||
it('reports unreachable when the write attempt itself cannot reach the network', function () {
|
||||
Http::fake([
|
||||
'dns.hetzner.com/api/v1/zones' => Http::response(['zones' => [['id' => 'zone-1', 'name' => 'probe.example']]]),
|
||||
'dns.hetzner.com/api/v1/records' => function () {
|
||||
throw new ConnectionException('Could not resolve host');
|
||||
},
|
||||
]);
|
||||
|
||||
$result = (new DnsTokenCheck)->run('rw-token');
|
||||
|
||||
expect($result['ok'])->toBeFalse()
|
||||
->and($result['reason'])->toBe('unreachable');
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 2, Sonderfall Aufräumen): der Schreibversuch selbst hat
|
||||
* bereits bewiesen, dass der Token schreiben darf — das ist der Zweck der
|
||||
* ganzen Prüfung, und ein kaputtes Netz beim Löschen ändert daran nichts.
|
||||
* `probe_removed` muss trotzdem false sein, und der liegengebliebene Eintrag
|
||||
* muss benennbar bleiben, sonst weiß niemand, dass er in der Zone liegt.
|
||||
*/
|
||||
it('still proves write access when the cleanup delete cannot reach the network', function () {
|
||||
Http::fake([
|
||||
'dns.hetzner.com/api/v1/zones' => Http::response(['zones' => [['id' => 'zone-1', 'name' => 'probe.example']]]),
|
||||
'dns.hetzner.com/api/v1/records' => Http::response(['record' => ['id' => 'rec-orphaned']]),
|
||||
'dns.hetzner.com/api/v1/records/*' => function () {
|
||||
throw new ConnectionException('Could not resolve host');
|
||||
},
|
||||
]);
|
||||
|
||||
$result = (new DnsTokenCheck)->run('rw-token');
|
||||
|
||||
expect($result['ok'])->toBeTrue()
|
||||
->and($result['reason'])->toBe('writable')
|
||||
->and($result['probe_removed'])->toBeFalse()
|
||||
->and($result['leftover_record_id'])->toBe('rec-orphaned');
|
||||
});
|
||||
|
||||
it('does not attempt anything without a token or a zone', function () {
|
||||
// Zurückgesetzt statt sich auf den .env-Wert dieser Installation zu
|
||||
// verlassen — genau die Falle, die phpunit.xml zu CLUPILOT_DNS_ZONE
|
||||
|
|
@ -219,6 +288,80 @@ it('names the missing vmid when the template is not on any active host', functio
|
|||
->and($result['vmids'])->toContain(4002);
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 3): ein Host, der gerade nicht antwortet, ist kein
|
||||
* Beweis dafür, dass die Vorlage fehlt — sie könnte genau auf diesem Host
|
||||
* liegen. Der bestehende FakeProxmoxClient kann das nicht abbilden (jede
|
||||
* Antwort ist entweder true oder false, nie eine Ausnahme), deshalb eine
|
||||
* anonyme Unterklasse nach dem Muster aus TrafficTest.php
|
||||
* ($flaky/$blind), die vmExists() gezielt scheitern lässt.
|
||||
*/
|
||||
it('does not confuse a silent host with a confirmed-missing template', function () {
|
||||
Http::fake();
|
||||
|
||||
$silent = new class extends FakeProxmoxClient
|
||||
{
|
||||
public function vmExists(string $node, int $vmid): bool
|
||||
{
|
||||
throw new RuntimeException('proxmox unreachable');
|
||||
}
|
||||
};
|
||||
app()->instance(ProxmoxClient::class, $silent);
|
||||
|
||||
Host::factory()->create(['status' => 'active', 'api_token_ref' => 'ref-1', 'node' => 'pve']);
|
||||
makePublishedVersion(4004);
|
||||
|
||||
$result = app(VmTemplateCheck::class)->run();
|
||||
|
||||
// "build the template" and "your hosts are not answering" are two
|
||||
// different instructions — a reason of 'template_missing' here would
|
||||
// send an operator to build a template that might already exist.
|
||||
expect($result['ok'])->toBeFalse()
|
||||
->and($result['reason'])->toBe('hosts_unreachable')
|
||||
->and($result['reason'])->not->toBe('template_missing')
|
||||
->and($result['vmids'])->toContain(4004);
|
||||
});
|
||||
|
||||
/**
|
||||
* Fix-Runde (Befund 3, Gegenprobe): ein Host, der antwortet und die Vorlage
|
||||
* bestätigt, darf das Ergebnis nicht kippen lassen, nur weil ein ANDERER
|
||||
* Host in derselben Runde schweigt — "gefunden" sticht "still", und die
|
||||
* Schleife bricht bei einem Treffer ab, statt den stillen Host danach noch
|
||||
* als Beweis für irgendetwas zu werten.
|
||||
*
|
||||
* Eine einzige ProxmoxClient-Instanz bedient in VmTemplateCheck ALLE Hosts
|
||||
* (`forHost($host)` wird pro Host auf demselben `$this->pve` aufgerufen), also
|
||||
* unterscheidet dieser Fake anhand dessen, welcher Host zuletzt über
|
||||
* `forHost()` hereinkam — genau das Feld, das `FakeProxmoxClient::forHost()`
|
||||
* ohnehin schon merkt.
|
||||
*/
|
||||
it('still confirms a template that a working host has, despite a silent one', function () {
|
||||
Http::fake();
|
||||
|
||||
$mixed = new class extends FakeProxmoxClient
|
||||
{
|
||||
public function vmExists(string $node, int $vmid): bool
|
||||
{
|
||||
if ($this->host?->node === 'silent-node') {
|
||||
throw new RuntimeException('proxmox unreachable');
|
||||
}
|
||||
|
||||
return parent::vmExists($node, $vmid);
|
||||
}
|
||||
};
|
||||
$mixed->clonedVmids = [4005]; // what the WORKING host actually has
|
||||
app()->instance(ProxmoxClient::class, $mixed);
|
||||
|
||||
Host::factory()->create(['status' => 'active', 'api_token_ref' => 'ref-1', 'node' => 'silent-node']);
|
||||
Host::factory()->create(['status' => 'active', 'api_token_ref' => 'ref-2', 'node' => 'working-node']);
|
||||
makePublishedVersion(4005);
|
||||
|
||||
$result = app(VmTemplateCheck::class)->run();
|
||||
|
||||
expect($result['ok'])->toBeTrue()
|
||||
->and($result['reason'])->toBe('present');
|
||||
});
|
||||
|
||||
it('reports no usable host when nothing is left to ask', function () {
|
||||
Http::fake();
|
||||
app()->instance(ProxmoxClient::class, new FakeProxmoxClient);
|
||||
|
|
|
|||
Loading…
Reference in New Issue