From fffcecb15205c0fe212b383f7df57bf77bc1b6d2 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 12:12:24 +0200 Subject: [PATCH] fix(engine-b): address Codex round 3 (webhook email/replay/rotation, db secret) - Require a real customer email (no unknown@ fallback merging customers). - Stripe signature: enforce 5-min replay tolerance + accept any rotating v1. - DeployApplicationStack no longer persists the DB password in the run context. Co-Authored-By: Claude Opus 4.8 --- .../Controllers/StripeWebhookController.php | 38 +++++++++++++++---- .../Steps/Customer/DeployApplicationStack.php | 6 +-- .../Provisioning/CustomerStepsTest.php | 17 ++++++--- .../Provisioning/StripeWebhookTest.php | 14 ++++++- 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/app/Http/Controllers/StripeWebhookController.php b/app/Http/Controllers/StripeWebhookController.php index cdf6b18..99979f6 100644 --- a/app/Http/Controllers/StripeWebhookController.php +++ b/app/Http/Controllers/StripeWebhookController.php @@ -38,9 +38,16 @@ class StripeWebhookController extends Controller $meta = $object['metadata'] ?? []; + // A real email is required — never merge unrelated customers under a + // manufactured address or send credentials into the void. + $email = $object['customer_details']['email'] ?? $object['customer_email'] ?? ($meta['email'] ?? null); + if (blank($email)) { + return response()->json(['ignored' => 'no_customer_email']); + } + $action->fromStripeEvent([ 'id' => (string) ($event['id'] ?? ''), - 'email' => $object['customer_details']['email'] ?? $object['customer_email'] ?? ($meta['email'] ?? 'unknown@example.com'), + 'email' => $email, 'name' => $object['customer_details']['name'] ?? ($meta['name'] ?? null), 'stripe_customer_id' => $object['customer'] ?? null, 'plan' => $meta['plan'] ?? 'start', @@ -52,6 +59,9 @@ class StripeWebhookController extends Controller return response()->json(['ok' => true]); } + /** Stripe's default replay tolerance, in seconds. */ + private const SIGNATURE_TOLERANCE = 300; + /** Verify Stripe's `t=…,v1=…` signature: HMAC-SHA256 of "{t}.{payload}". */ private function signatureValid(string $payload, string $header, string $secret): bool { @@ -59,20 +69,34 @@ class StripeWebhookController extends Controller return false; } - $parts = []; + $timestamp = null; + $signatures = []; foreach (explode(',', $header) as $segment) { [$key, $value] = array_pad(explode('=', $segment, 2), 2, ''); - $parts[$key] = $value; + if ($key === 't') { + $timestamp = $value; + } elseif ($key === 'v1') { + $signatures[] = $value; // keep every v1 (secret rotation sends several) + } } - $timestamp = $parts['t'] ?? null; - $signature = $parts['v1'] ?? null; - if ($timestamp === null || $signature === null) { + if ($timestamp === null || $signatures === []) { + return false; + } + + // Reject replays outside the tolerance window. + if (abs(time() - (int) $timestamp) > self::SIGNATURE_TOLERANCE) { return false; } $expected = hash_hmac('sha256', $timestamp.'.'.$payload, $secret); - return hash_equals($expected, $signature); + foreach ($signatures as $signature) { + if (hash_equals($expected, $signature)) { + return true; + } + } + + return false; } } diff --git a/app/Provisioning/Steps/Customer/DeployApplicationStack.php b/app/Provisioning/Steps/Customer/DeployApplicationStack.php index b42d217..b939fb9 100644 --- a/app/Provisioning/Steps/Customer/DeployApplicationStack.php +++ b/app/Provisioning/Steps/Customer/DeployApplicationStack.php @@ -28,10 +28,10 @@ class DeployApplicationStack extends CustomerStep $vmid = (int) $run->context('vmid'); $pve = $this->pve->forHost($instance->host); - // Deploy once (guarded); the generated DB password is transient. + // Deploy once (guarded). The generated DB password is written straight + // into the guest's .env and never persisted in the run context. if (! $run->context('stack_deployed')) { $dbPassword = Str::random(28); - $run->mergeContext(['db_password' => $dbPassword]); $this->guest($pve, $run, 'install -d /opt/nextcloud && printf %s '.escapeshellarg('MYSQL_PASSWORD='.$dbPassword."\n"). @@ -48,8 +48,6 @@ class DeployApplicationStack extends CustomerStep return StepResult::poll(15, 'waiting for application stack'); } - $run->forgetContext('db_password'); // scrub the secret once the stack is up - return StepResult::advance(); } } diff --git a/tests/Feature/Provisioning/CustomerStepsTest.php b/tests/Feature/Provisioning/CustomerStepsTest.php index b8a89ff..3dfacd3 100644 --- a/tests/Feature/Provisioning/CustomerStepsTest.php +++ b/tests/Feature/Provisioning/CustomerStepsTest.php @@ -187,18 +187,23 @@ it('configures the guest network, captures the ip, and firewall', function () { }); // 8. DeployApplicationStack -it('deploys the stack, polling until healthy, and scrubs the db password', function () { +it('deploys the stack, polling until healthy, without persisting the db password', function () { $s = fakeServices(); ['run' => $run] = reservedRun(); - // not healthy yet → poll, secret still present + // not healthy yet → poll; the db password is never in the run context expect(app(DeployApplicationStack::class)->execute($run)->type)->toBe('poll') - ->and($run->fresh()->context('db_password'))->not->toBeNull(); + ->and($run->fresh()->context('db_password'))->toBeNull() + ->and($run->fresh()->context('stack_deployed'))->toBeTrue() + ->and($s['pve']->guestRan('docker compose up'))->toBeTrue(); - // healthy → advance, secret scrubbed + // healthy → advance $s['pve']->guestScript('status.php', 0, 'ok'); - expect(app(DeployApplicationStack::class)->execute($run->fresh())->type)->toBe('advance') - ->and($run->fresh()->context('db_password'))->toBeNull(); + expect(app(DeployApplicationStack::class)->execute($run->fresh())->type)->toBe('advance'); + + // the persisted context never contains a plaintext DB credential + $raw = \DB::table('provisioning_runs')->where('id', $run->id)->value('context'); + expect($raw)->not->toContain('MYSQL_PASSWORD'); }); // 9. ConfigureNextcloud diff --git a/tests/Feature/Provisioning/StripeWebhookTest.php b/tests/Feature/Provisioning/StripeWebhookTest.php index 3b98504..29d5ddb 100644 --- a/tests/Feature/Provisioning/StripeWebhookTest.php +++ b/tests/Feature/Provisioning/StripeWebhookTest.php @@ -54,6 +54,18 @@ it('ignores unrelated event types', function () { Queue::assertNothingPushed(); }); +it('ignores a paid checkout without a customer email', function () { + Queue::fake(); + + $event = stripeEvent('evt_noemail'); + unset($event['data']['object']['customer_details'], $event['data']['object']['customer_email'], $event['data']['object']['metadata']['email']); + + $this->postJson(route('webhooks.stripe'), $event)->assertOk()->assertJson(['ignored' => 'no_customer_email']); + + expect(Order::query()->where('stripe_event_id', 'evt_noemail')->exists())->toBeFalse(); + Queue::assertNothingPushed(); +}); + it('ignores a completed but unpaid checkout session', function () { Queue::fake(); @@ -78,7 +90,7 @@ it('accepts a valid signature', function () { config()->set('services.stripe.webhook_secret', 'whsec_test'); $payload = json_encode(stripeEvent('evt_ok')); - $timestamp = 1700000000; + $timestamp = time(); // within Stripe's replay tolerance $signature = hash_hmac('sha256', $timestamp.'.'.$payload, 'whsec_test'); $this->call(