From 43a0c6a4d67c43e758a786db804512900caa5439 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 22:20:13 +0000 Subject: [PATCH] fix: Separate an unreadable webhook payload from a forged one SeamWebhook::verify raised Svix's WebhookVerificationException for a payload whose signature had already matched but whose body would not parse. The README maps that exception to a 400, so an unreadable delivery was answered with an error and Svix redelivered it on its full backoff schedule, while whoever read the logs saw a verification failure and went looking for a forgery. Raise InvalidWebhookPayloadError instead, a new SeamException, so the two cases can be answered differently: a signature that does not match may be forged, and a body that does not parse is genuinely from Seam and will not become readable however many times it arrives. Catch the parse failure explicitly rather than inferring it from a null event, and treat a correctly signed payload that is not an event the same way. It used to be returned as an Event with every field null, with nothing to tell the caller. Cast header names before lowercasing them, since an all-digit name arrives as an int key and would raise a TypeError. The parse path had no coverage at all. Add cases for malformed JSON, a non-object body, an empty body, and a signed non-event, along with the expired-timestamp and missing-header cases the suite was missing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HH3wdHh4Y6Wjyc5uHwk5iG --- README.md | 4 +- src/InvalidWebhookPayloadError.php | 10 +++++ src/SeamWebhook.php | 21 ++++++--- tests/SeamWebhookTest.php | 72 +++++++++++++++++++++++++++++- 4 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 src/InvalidWebhookPayloadError.php diff --git a/README.md b/README.md index 2145cf1f..08c2bc4f 100644 --- a/README.md +++ b/README.md @@ -331,7 +331,9 @@ try { $event = $webhook->verify($request_body, $request_headers); print $event->event_type; } catch (Svix\Exception\WebhookVerificationException $error) { - http_response_code(400); + http_response_code(401); +} catch (Seam\InvalidWebhookPayloadError $error) { + http_response_code(204); } ``` diff --git a/src/InvalidWebhookPayloadError.php b/src/InvalidWebhookPayloadError.php new file mode 100644 index 00000000..69df0d89 --- /dev/null +++ b/src/InvalidWebhookPayloadError.php @@ -0,0 +1,10 @@ + $headers The HTTP request headers. * * @throws \Svix\Exception\WebhookVerificationException When the signature does not match. + * @throws InvalidWebhookPayloadError When the signature matches but the body is not a Seam event. */ public function verify(string $payload, array $headers): Event { $normalized_headers = []; foreach ($headers as $name => $value) { - $normalized_headers[strtolower($name)] = $value; + $normalized_headers[strtolower((string) $name)] = $value; } $this->webhook->verify($payload, $normalized_headers); - $event = Event::from_json(json_decode($payload)); + $decoded = json_decode($payload); - if ($event === null) { - throw new WebhookVerificationException( + if (json_last_error() !== JSON_ERROR_NONE) { + throw new InvalidWebhookPayloadError( + "The verified webhook payload is not valid JSON: " . + json_last_error_msg(), + ); + } + + $event = Event::from_json($decoded); + + if ($event === null || $event->event_id === null) { + throw new InvalidWebhookPayloadError( "The verified webhook payload did not contain an event", ); } diff --git a/tests/SeamWebhookTest.php b/tests/SeamWebhookTest.php index be871513..f60e96fc 100644 --- a/tests/SeamWebhookTest.php +++ b/tests/SeamWebhookTest.php @@ -5,6 +5,8 @@ namespace Tests; use PHPUnit\Framework\TestCase; +use Seam\InvalidWebhookPayloadError; +use Seam\SeamException; use Seam\SeamWebhook; use Svix\Exception\WebhookVerificationException; use Svix\Webhook; @@ -28,10 +30,10 @@ private function payload(): string /** * @return array */ - private function signed_headers(string $payload): array + private function signed_headers(string $payload, ?int $at = null): array { $id = "msg_test"; - $timestamp = (string) time(); + $timestamp = (string) ($at ?? time()); $signature = (new Webhook(self::SECRET))->sign( $id, @@ -100,4 +102,70 @@ public function testVerifyRejectsTheWrongSecret(): void "whsec_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=", ))->verify($payload, $headers); } + + public function testVerifyRejectsAnExpiredTimestamp(): void + { + $payload = $this->payload(); + + $this->expectException(WebhookVerificationException::class); + + (new SeamWebhook(self::SECRET))->verify( + $payload, + $this->signed_headers($payload, time() - 3600), + ); + } + + /** + * @dataProvider missingHeaders + */ + public function testVerifyRejectsAMissingHeader(string $missing): void + { + $payload = $this->payload(); + $headers = $this->signed_headers($payload); + unset($headers[$missing]); + + $this->expectException(WebhookVerificationException::class); + + (new SeamWebhook(self::SECRET))->verify($payload, $headers); + } + + public static function missingHeaders(): array + { + return [ + "svix-id" => ["svix-id"], + "svix-timestamp" => ["svix-timestamp"], + "svix-signature" => ["svix-signature"], + ]; + } + + /** + * @dataProvider unreadablePayloads + */ + public function testVerifyDistinguishesAnUnreadablePayload( + string $payload, + ): void { + $headers = $this->signed_headers($payload); + + try { + (new SeamWebhook(self::SECRET))->verify($payload, $headers); + $this->fail("Expected InvalidWebhookPayloadError"); + } catch (InvalidWebhookPayloadError $error) { + $this->assertInstanceOf(SeamException::class, $error); + $this->assertNotInstanceOf( + WebhookVerificationException::class, + $error, + ); + } + } + + public static function unreadablePayloads(): array + { + return [ + "malformed json" => ["{not json"], + "json that is not an object" => ["[1, 2]"], + "json null" => ["null"], + "empty body" => [""], + "object that is not an event" => ['{"hello":"world"}'], + ]; + } }