From a1e3484fbad1b00238aabe8a92cc7ce3f642e85c Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Thu, 30 Jul 2026 12:13:55 +0200
Subject: [PATCH] Verify webhooks against the secret of the mode we are in

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .env.example                                  |  4 ++
 .../Controllers/StripeWebhookController.php   |  3 +-
 app/Support/StripeWebhookSecret.php           | 33 +++++++++++++
 config/services.php                           |  5 ++
 phpunit.xml                                   | 10 ++--
 .../Feature/StripeWebhookSecretByModeTest.php | 46 +++++++++++++++++++
 6 files changed, 97 insertions(+), 4 deletions(-)
 create mode 100644 app/Support/StripeWebhookSecret.php
 create mode 100644 tests/Feature/StripeWebhookSecretByModeTest.php

diff --git a/.env.example b/.env.example
index 7318d2d..b7dd576 100644
--- a/.env.example
+++ b/.env.example
@@ -94,7 +94,11 @@ MAIL_FROM_NAME=CluPilot
 # ── Stripe (payments) ────────────────────────────────────────────────────
 # WEBHOOK_SECRET is READ BY CODE today (signature check on /webhooks/stripe).
 # Outside local/testing a missing secret makes the webhook fail closed.
+# Stripe hands out a SEPARATE signing secret per operating mode (test/live) —
+# fill in the one matching each. Which one applies follows platform.mode
+# (App\Support\StripeWebhookSecret), not this file.
 STRIPE_WEBHOOK_SECRET=
+STRIPE_WEBHOOK_SECRET_TEST=
 # Not read yet — needed once the Checkout flow is built (currently the portal
 # only records a purchase intent):
 STRIPE_KEY=
diff --git a/app/Http/Controllers/StripeWebhookController.php b/app/Http/Controllers/StripeWebhookController.php
index fabe785..b1f4c19 100644
--- a/app/Http/Controllers/StripeWebhookController.php
+++ b/app/Http/Controllers/StripeWebhookController.php
@@ -4,6 +4,7 @@ namespace App\Http\Controllers;
 
 use App\Actions\ApplyStripeBillingEvent;
 use App\Actions\StartCustomerProvisioning;
+use App\Support\StripeWebhookSecret;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 
@@ -20,7 +21,7 @@ class StripeWebhookController extends Controller
         ApplyStripeBillingEvent $billing,
     ): JsonResponse {
         $payload = $request->getContent();
-        $secret = (string) config('services.stripe.webhook_secret');
+        $secret = StripeWebhookSecret::current();
 
         if (blank($secret)) {
             // Fail closed: an unconfigured secret must not authorize provisioning
diff --git a/app/Support/StripeWebhookSecret.php b/app/Support/StripeWebhookSecret.php
new file mode 100644
index 0000000..e1c7acb
--- /dev/null
+++ b/app/Support/StripeWebhookSecret.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace App\Support;
+
+/**
+ * Der Signaturschlüssel des aktiven Betriebsmodus.
+ *
+ * Beide Werte bleiben in der `.env`, nicht im Tresor (siehe Kopfkommentar von
+ * App\Services\Secrets\SecretVault): dieser Wert wird bei jedem eingehenden
+ * Zahlungsereignis gelesen, und ein Datenbankproblem würde die
+ * Signaturprüfung still fehlschlagen lassen — die eine Fehlerart, die laut
+ * bleiben muss.
+ *
+ * Die AUSWAHL fragt allerdings die Datenbank (OperatingMode::current()), was
+ * diese Abhängigkeit auf einem Umweg wieder einführt. Entschärft durch die
+ * Richtung des Ausfalls: `Settings::get()` liefert bei unerreichbarer Tabelle
+ * seinen Vorgabewert `live`, also wird der Live-Schlüssel benutzt. Ein echtes
+ * Zahlungsereignis wird weiter korrekt geprüft, ein Testereignis scheitert
+ * laut. Andersherum wäre es die gefährliche Richtung.
+ *
+ * Eigene Klasse statt einer privaten Methode auf dem Controller: die Auswahl
+ * selbst ist der Prüfgegenstand (siehe StripeWebhookSecretByModeTest), und ein
+ * Test soll dafür nicht per Reflection ins Controller-Innere greifen müssen.
+ */
+final class StripeWebhookSecret
+{
+    public static function current(): string
+    {
+        return OperatingMode::current()->isTest()
+            ? (string) config('services.stripe.webhook_secret_test')
+            : (string) config('services.stripe.webhook_secret');
+    }
+}
diff --git a/config/services.php b/config/services.php
index 84462ab..e8cec2a 100644
--- a/config/services.php
+++ b/config/services.php
@@ -37,6 +37,11 @@ return [
 
     'stripe' => [
         'webhook_secret' => env('STRIPE_WEBHOOK_SECRET'),
+        // Stripe issues a SEPARATE signing secret per operating mode. Which of
+        // the two applies is decided by App\Support\StripeWebhookSecret, not
+        // here — see its header comment for why that lookup still touches the
+        // database despite both values living in the .env.
+        'webhook_secret_test' => env('STRIPE_WEBHOOK_SECRET_TEST'),
         // Secret key for the REST API. Blank means "not connected": the
         // catalogue sync says so and does nothing, rather than half-creating
         // products against an account that is not there.
diff --git a/phpunit.xml b/phpunit.xml
index bbd3a88..9a33a84 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -30,11 +30,15 @@
         <!-- Fixed test key: storing a VPN config must be exercised, and a
              random one would make a decryption failure irreproducible. -->
         <env name="VPN_CONFIG_KEY" value="Y2x1cGlsb3QtdGVzdC1vbmx5LXZwbi1rZXktMzJieXQ=" force="true"/>
-        <!-- Blank so the suite never picks up the operator's real webhook secret
+        <!-- Blank so the suite never picks up the operator's real webhook secrets
              from .env: with one set, every unsigned test payload is rejected as
-             an invalid signature. The two tests that DO exercise verification
-             set the secret themselves. -->
+             an invalid signature. The tests that DO exercise verification set
+             the secret(s) themselves. _TEST is the live secret's sibling
+             (StripeWebhookSecret picks between them by operating mode) — same
+             variable class, same risk, so it gets the same treatment even
+             though no current test happens to need it. -->
         <env name="STRIPE_WEBHOOK_SECRET" value="" force="true"/>
+        <env name="STRIPE_WEBHOOK_SECRET_TEST" value="" force="true"/>
         <!-- Pinned so the suite stops depending on the operator's .env. It did:
              HostStepsTest asserted `fsn-01.node.clupilot.com` and passed only
              because this server's .env carried the wrong zone. Correcting the
diff --git a/tests/Feature/StripeWebhookSecretByModeTest.php b/tests/Feature/StripeWebhookSecretByModeTest.php
new file mode 100644
index 0000000..0d2717e
--- /dev/null
+++ b/tests/Feature/StripeWebhookSecretByModeTest.php
@@ -0,0 +1,46 @@
+<?php
+
+use App\Support\OperatingMode;
+use App\Support\StripeWebhookSecret;
+use Illuminate\Support\Facades\Schema;
+
+/**
+ * Zwei Signaturschlüssel, weil Stripe je Modus einen eigenen vergibt.
+ *
+ * Der dritte Test ist der eigentliche Grund für diese Datei: die Auswahl liest
+ * den Modus aus der Datenbank — aus genau der Quelle, die für diesen Wert
+ * bewusst vermieden wurde. Fällt sie aus, muss die Auswahl auf LIVE fallen. Ein
+ * echtes Zahlungsereignis wird dann weiter korrekt geprüft; ein Testereignis
+ * scheitert laut. Die umgekehrte Vorgabe wäre die gefährliche.
+ *
+ * Ruft App\Support\StripeWebhookSecret::current() direkt auf statt über
+ * Reflection eine private Controller-Methode: die Auswahl sitzt in einer
+ * eigenen, öffentlich aufrufbaren Klasse (Begründung in deren Kopfkommentar),
+ * gerade damit dieser Test ohne ReflectionMethod auskommt.
+ */
+beforeEach(function () {
+    config()->set('services.stripe.webhook_secret', 'whsec_live');
+    config()->set('services.stripe.webhook_secret_test', 'whsec_test');
+});
+
+it('uses the live signing secret in live mode', function () {
+    OperatingMode::set(OperatingMode::Live);
+
+    expect(StripeWebhookSecret::current())->toBe('whsec_live');
+});
+
+it('uses the test signing secret in test mode', function () {
+    OperatingMode::set(OperatingMode::Test);
+
+    expect(StripeWebhookSecret::current())->toBe('whsec_test');
+});
+
+it('uses the live secret when the settings table cannot be read', function () {
+    OperatingMode::set(OperatingMode::Test);
+
+    // Der Ausfall, gegen den der Wert ursprünglich aus der Datenbank
+    // herausgehalten wurde.
+    Schema::drop('app_settings');
+
+    expect(StripeWebhookSecret::current())->toBe('whsec_live');
+});