diff --git a/app/Livewire/Admin/Readiness.php b/app/Livewire/Admin/Readiness.php index 8840bbf..ac87858 100644 --- a/app/Livewire/Admin/Readiness.php +++ b/app/Livewire/Admin/Readiness.php @@ -54,12 +54,19 @@ class Readiness extends Component ]; /** - * A heartbeat older than this is not shown as fresh even where the check - * itself already says so — a display concern only. Kept in sync with - * App\Support\Readiness\OperationChecks::STALE_AFTER_MINUTES on purpose - * (see the docblock there for why five minutes), not re-derived from it: - * that constant is private, and duplicating the reasoning in a comment - * here is cheaper than making it public for one reader. + * Check::key → the App\Support\Settings row it is written under. + * + * Display only, and carries no threshold of its own: whether a heartbeat + * counts as fresh is decided exactly once, by + * App\Support\Readiness\OperationChecks::isFresh(), and shows up here + * only as the check's own `satisfied`/severity — this map exists solely + * so heartbeatDisplay() knows which stored value to format for which + * check. (A previous version of this comment claimed a staleness + * threshold was kept in sync with OperationChecks::STALE_AFTER_MINUTES — + * there is no such value here to keep in sync. CLAUDE.md's R19 names this + * exact shape of mistake — a call that reads as an assurance and is not + * one — as worse than no comment at all, because it stops the next + * reader from checking for themselves.) */ private const HEARTBEAT_KEYS = [ 'operation.scheduler' => 'heartbeat.scheduler', @@ -100,23 +107,21 @@ class Readiness extends Component /** * Where an operator goes to actually fix ONE entry. * - * Not always Admin\Integrations, and not always $check->tab handed - * straight through as its ?tab=: several of the five check groups - * predate the Integrations::TABS redesign and still carry a tab value - * from before it — 'hosts', 'plans', 'datacenters', 'mail', 'templates' - * and 'company' each name a DIFFERENT admin page entirely, and the - * leftover value 'integrations' is the exact bug Task 10's fix round - * already found and corrected for OperationChecks (it does not exist in - * Integrations::TABS, so a link built from it lands nowhere in - * particular — Integrations::mount() silently falls back to its own - * first tab). Rather than repeat that history by forwarding $check->tab - * unchecked, a value only ever becomes a ?tab= when it actually IS one - * of Integrations::TABS; every other known value gets its own real - * route, and anything unrecognised — including the eight checks in - * Onboarding/Provisioning/Delivery still carrying the pre-redesign - * 'integrations' value (Task 8's own deferred note; out of scope for - * this task to rename) — falls back to the Integrations page's own - * default tab rather than a dead link. + * Not always Admin\Integrations: six of the nine `tab` values used across + * the five check groups name a DIFFERENT admin page entirely — 'hosts', + * 'plans', 'datacenters', 'mail', 'templates' and 'company' are pages of + * their own, not Integrations tabs, and forwarding them as `?tab=` would + * either 404 or (since Integrations::mount() falls back silently to its + * own first tab for an unrecognised value) land quietly on the wrong + * section. Every OTHER `tab` value that exists in the five check group + * files today (checked at the source, Fix-Runde) really IS one of + * Integrations::TABS — nine of them were not, until that same Fix-Runde + * corrected them there rather than papering over it here (see + * BillingChecks/OnboardingChecks/ProvisioningChecks/DeliveryChecks). The + * `default` arm below is therefore a pure safety net for a value nobody + * currently produces, not a route any check relies on — kept so a future + * typo degrades to the Integrations landing tab instead of an + * UnhandledMatchError taking the whole page down. */ public function checkUrl(Check $check): string { diff --git a/app/Support/Readiness/DeliveryChecks.php b/app/Support/Readiness/DeliveryChecks.php index 772305b..82ae9c3 100644 --- a/app/Support/Readiness/DeliveryChecks.php +++ b/app/Support/Readiness/DeliveryChecks.php @@ -61,7 +61,10 @@ final class DeliveryChecks severity: Check::SEVERITY_WARNING, label: __('readiness.delivery.inbound_password'), breaks: __('readiness.delivery.inbound_password_breaks'), - tab: 'integrations', + // The inbound-mail card (host/port/user/folder + this + // password) is on the 'services' tab — 'integrations' is not + // a member of Integrations::TABS at all (Fix-Runde, Befund 1). + tab: 'services', satisfied: filled(app(SecretVault::class)->get('inbound_mail.password')), ), ]; diff --git a/app/Support/Readiness/OnboardingChecks.php b/app/Support/Readiness/OnboardingChecks.php index 5c99962..8339380 100644 --- a/app/Support/Readiness/OnboardingChecks.php +++ b/app/Support/Readiness/OnboardingChecks.php @@ -33,7 +33,13 @@ final class OnboardingChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.onboarding.ssh_key'), breaks: __('readiness.onboarding.ssh_key_breaks'), - tab: 'integrations', + // The SSH identity card lives on the 'platform' tab + // (integrations.blade.php: "own machines"), beside the + // public-key setting it pairs with — not 'integrations', + // which is not a member of Integrations::TABS at all and + // sent an operator clicking this exact blocking check to the + // wrong tab (Fix-Runde, Befund 1). + tab: 'platform', // EstablishSshTrust only deploys the PUBLIC half. Every step // after it (HostStep's own shell helper), and later every // Traefik write on a host already onboarded (SshTraefikWriter), @@ -46,7 +52,10 @@ final class OnboardingChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.onboarding.secrets_key'), breaks: __('readiness.onboarding.secrets_key_breaks'), - tab: 'integrations', + // SECRETS_KEY has no field anywhere on Integrations' 'services' + // or 'platform' tabs — the only place an operator can actually + // set it is the raw .env editor on the 'env' tab. + tab: 'env', // isUsable() rather than a bare filled() on the config value — // SecretCipher's own docblock warns that a second, shorter // rule that only checks emptiness lets a malformed-but-non- @@ -60,7 +69,10 @@ final class OnboardingChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.onboarding.vpn_config_key'), breaks: __('readiness.onboarding.vpn_config_key_breaks'), - tab: 'integrations', + // Same reasoning as onboarding.secrets_key just above: + // VPN_CONFIG_KEY has no console field either, only the raw + // .env editor. + tab: 'env', // Same reuse-not-reimplement reasoning as isUsable() above: // ConfigVault::available() is the one place that already knows // what counts as a usable key. @@ -72,7 +84,12 @@ final class OnboardingChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.onboarding.wg_hub'), breaks: __('readiness.onboarding.wg_hub_breaks'), - tab: 'integrations', + // Two of the three values this check reads (wgHubPubkey, + // wgEndpoint) are 'platform'-tab fields; only the third + // (subnet) is .env-only and cannot be changed from the + // console at all (see the docblock on satisfied: below). The + // tab an operator can actually act on is 'platform'. + tab: 'platform', // Two of three is no half a tunnel, it is none: ConfigureWireguard // writes the peer config from all three at once. Subnet is read // straight from config(), never through Settings/ProvisioningSettings diff --git a/app/Support/Readiness/ProvisioningChecks.php b/app/Support/Readiness/ProvisioningChecks.php index 2a3b841..5af85d4 100644 --- a/app/Support/Readiness/ProvisioningChecks.php +++ b/app/Support/Readiness/ProvisioningChecks.php @@ -26,7 +26,10 @@ final class ProvisioningChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.provisioning.dns_token'), breaks: __('readiness.provisioning.dns_token_breaks'), - tab: 'integrations', + // The DNS card (token + zone + Traefik path) is on the + // 'services' tab — 'integrations' is not a member of + // Integrations::TABS at all (Fix-Runde, Befund 1). + tab: 'services', satisfied: filled(app(SecretVault::class)->get('dns.token')), ), new Check( @@ -35,7 +38,8 @@ final class ProvisioningChecks severity: Check::SEVERITY_BLOCKING, label: __('readiness.provisioning.dns_zone'), breaks: __('readiness.provisioning.dns_zone_breaks'), - tab: 'integrations', + // Same DNS card as provisioning.dns_token just above. + tab: 'services', satisfied: filled(ProvisioningSettings::dnsZone()), ), new Check( @@ -44,7 +48,8 @@ final class ProvisioningChecks severity: Check::SEVERITY_WARNING, label: __('readiness.provisioning.traefik_path'), breaks: __('readiness.provisioning.traefik_path_breaks'), - tab: 'integrations', + // Same DNS card — traefikDynamicPath is a field on it. + tab: 'services', satisfied: filled(ProvisioningSettings::traefikDynamicPath()), ), new Check( @@ -93,7 +98,8 @@ final class ProvisioningChecks : Check::SEVERITY_WARNING, label: __('readiness.provisioning.monitoring_token'), breaks: __('readiness.provisioning.monitoring_token_breaks'), - tab: 'integrations', + // The monitoring card is on the 'services' tab too. + tab: 'services', satisfied: filled(app(SecretVault::class)->get('monitoring.token')), ), ]; diff --git a/resources/views/livewire/admin/readiness.blade.php b/resources/views/livewire/admin/readiness.blade.php index 0d1eb5e..0797b64 100644 --- a/resources/views/livewire/admin/readiness.blade.php +++ b/resources/views/livewire/admin/readiness.blade.php @@ -64,14 +64,20 @@
+ {{-- mount() only requires hosts.manage OR secrets.manage, so an + Admin (hosts.manage alone) can reach this page — but runCheck() + itself demands secrets.manage and 403s otherwise. A button that + is visible and throws when pressed is worse than no button. --}} @if (in_array($check->key, $runnable, true)) - - - {{ __('readiness.run_check') }} - + @can('secrets.manage') + + + {{ __('readiness.run_check') }} + + @endcan @endif 'services', + 'billing.webhook_secret' => 'env', + 'billing.company_details' => 'company', + 'billing.invoice_series' => 'company', + 'billing.catalogue_synced' => 'services', + 'onboarding.ssh_private_key' => 'platform', + 'onboarding.secrets_key' => 'env', + 'onboarding.vpn_config_key' => 'env', + 'onboarding.wg_hub' => 'platform', + 'onboarding.datacenter' => 'datacenters', + 'provisioning.dns_token' => 'services', + 'provisioning.dns_zone' => 'services', + 'provisioning.traefik_path' => 'services', + 'provisioning.usable_host' => 'hosts', + 'provisioning.vm_template' => 'plans', + 'provisioning.monitoring_token' => 'services', + 'delivery.mailer_not_log' => 'mail', + 'delivery.mailbox' => 'mail', + 'delivery.mail_templates' => 'templates', + 'delivery.inbound_mail_password' => 'services', + 'operation.scheduler' => 'env', + 'operation.queue_provisioning' => 'env', + ]; + + foreach (Readiness::all() as $check) { + // NICHT expect($expectedTab)->toHaveKey($check->key, $message) — genau + // dieselbe Falle wie bei toContain() weiter oben: toHaveKey()s zweites + // Argument ist der erwartete WERT des Schlüssels, keine Fehlermeldung. + expect(array_key_exists($check->key, $expectedTab))->toBeTrue( + "Neuer Prüfschlüssel [{$check->key}] hat keinen erwarteten tab-Wert in diesem Test — bitte nachtragen." + ); + + expect($check->tab)->toBe( + $expectedTab[$check->key], + "{$check->key} zeigt auf '{$check->tab}', sein Feld liegt aber unter '{$expectedTab[$check->key]}'." + ); + } +}); + +/** + * Fix-Runde, Befund 2: mount() lässt hosts.manage ODER secrets.manage durch, + * runCheck() verlangt aber secrets.manage allein. Ein Admin (nur + * hosts.manage) erreicht die Seite und sah bislang auch den Knopf — der beim + * Drücken 403 geworfen hätte. Ein sichtbarer Knopf, der beim Drücken + * scheitert, ist schlimmer als gar keiner. + */ +it('hides the check button from an operator who could not press it anyway', function () { + Livewire::actingAs(Operator::factory()->role('Admin')->create(), 'operator') + ->test(ReadinessPage::class) + ->assertDontSee(__('readiness.run_check')); +}); + +/** + * Fix-Runde, Befund 3: die eine Eigenschaft, wegen der diese ganze + * Konstruktion existiert — die vier RUNNABLE-Prüfungen laufen NUR auf + * Knopfdruck, weil DnsTokenCheck einen echten TXT-Eintrag in die + * Produktivzone schreibt — war bislang ungeprüft. + */ +it('sends nothing over the network on a plain page load', function () { + Http::fake(); + + Livewire::actingAs(Operator::factory()->role('Owner')->create(), 'operator') + ->test(ReadinessPage::class); + + Http::assertNothingSent(); +});