fix: Separate an unreadable webhook payload from a forged one

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);
 }
 ```
 

src/InvalidWebhookPayloadError.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Seam;
+
+/**
+ * Error thrown when a webhook payload passes signature verification but does
+ * not carry a readable Seam event.
+ */
+class InvalidWebhookPayloadError extends \UnexpectedValueException implements
+    SeamException {}

src/SeamWebhook.php

@@ -3,7 +3,6 @@
 namespace Seam;
 
 use Seam\Resources\Event;
-use Svix\Exception\WebhookVerificationException;
 use Svix\Webhook;
 
 /**
@@ -13,6 +12,8 @@
  * resource returned by the API.
  *
  * Verification failures raise Svix\Exception\WebhookVerificationException.
+ * A verified payload that is not a readable event raises
+ * InvalidWebhookPayloadError.
  */
 class SeamWebhook
 {
@@ -30,20 +31,30 @@ public function __construct(string $secret)
      * @param array<string, string> $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",
             );
         }

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<string, string>
      */
-    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"}'],
+        ];
+    }
 }