Merge pull request #152 from adcontextprotocol/EmmaLouise2018/fix-issue-151

fix: use default JSON separators for webhook HMAC signing

Author: EmmaLouise2018

src/adcp/client.py

@@ -1893,22 +1893,29 @@ async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
         await self.close()
 
     def _verify_webhook_signature(
-        self, payload: dict[str, Any], signature: str, timestamp: str
+        self,
+        payload: dict[str, Any],
+        signature: str,
+        timestamp: str,
+        raw_body: bytes | str | None = None,
     ) -> bool:
         """
         Verify HMAC-SHA256 signature of webhook payload.
 
         The verification algorithm matches get_adcp_signed_headers_for_webhook:
         1. Constructs message as "{timestamp}.{json_payload}"
-        2. JSON-serializes payload with compact separators
-        3. UTF-8 encodes the message
+        2. Uses raw HTTP body bytes when available (preserves sender's serialization)
+        3. Falls back to json.dumps() if raw_body not provided
         4. HMAC-SHA256 signs with the shared secret
         5. Compares against the provided signature (with "sha256=" prefix stripped)
 
         Args:
-            payload: Webhook payload dict
+            payload: Webhook payload dict (used as fallback if raw_body not provided)
             signature: Signature to verify (with or without "sha256=" prefix)
-            timestamp: ISO 8601 timestamp from X-AdCP-Timestamp header
+            timestamp: Unix timestamp in seconds from X-AdCP-Timestamp header
+            raw_body: Raw HTTP request body bytes. When provided, used directly
+                for signature verification to avoid cross-language serialization
+                mismatches. Strongly recommended for production use.
 
         Returns:
             True if signature is valid, False otherwise
@@ -1920,11 +1927,15 @@ def _verify_webhook_signature(
         if signature.startswith("sha256="):
             signature = signature[7:]
 
-        # Serialize payload to JSON with consistent formatting (matches signing)
-        payload_bytes = json.dumps(payload, separators=(",", ":"), sort_keys=False).encode("utf-8")
+        # Use raw body if available (avoids cross-language serialization mismatches),
+        # otherwise fall back to json.dumps() for backward compatibility
+        if raw_body is not None:
+            payload_str = raw_body.decode("utf-8") if isinstance(raw_body, bytes) else raw_body
+        else:
+            payload_str = json.dumps(payload)
 
-        # Construct signed message: timestamp.payload (matches get_adcp_signed_headers_for_webhook)
-        signed_message = f"{timestamp}.{payload_bytes.decode('utf-8')}"
+        # Construct signed message: timestamp.payload
+        signed_message = f"{timestamp}.{payload_str}"
 
         # Generate expected signature
         expected_signature = hmac.new(
@@ -2073,6 +2084,7 @@ async def _handle_mcp_webhook(
         operation_id: str,
         signature: str | None,
         timestamp: str | None = None,
+        raw_body: bytes | str | None = None,
     ) -> TaskResult[AdcpAsyncResponseData]:
         """
         Handle MCP webhook delivered via HTTP POST.
@@ -2082,7 +2094,8 @@ async def _handle_mcp_webhook(
             task_type: Task type from application routing
             operation_id: Operation identifier from application routing
             signature: Optional HMAC-SHA256 signature for verification (X-AdCP-Signature header)
-            timestamp: Optional timestamp for signature verification (X-AdCP-Timestamp header)
+            timestamp: Optional Unix timestamp for signature verification (X-AdCP-Timestamp header)
+            raw_body: Optional raw HTTP request body for signature verification
 
         Returns:
             TaskResult with parsed task-specific response data
@@ -2097,7 +2110,7 @@ async def _handle_mcp_webhook(
         if (
             signature
             and timestamp
-            and not self._verify_webhook_signature(payload, signature, timestamp)
+            and not self._verify_webhook_signature(payload, signature, timestamp, raw_body)
         ):
             logger.warning(
                 f"Webhook signature verification failed for agent {self.agent_config.id}"
@@ -2283,6 +2296,7 @@ async def handle_webhook(
         operation_id: str,
         signature: str | None = None,
         timestamp: str | None = None,
+        raw_body: bytes | str | None = None,
     ) -> TaskResult[AdcpAsyncResponseData]:
         """
         Handle incoming webhook and return typed result.
@@ -2310,8 +2324,12 @@ async def handle_webhook(
                 Used to correlate webhook notifications with original task submission.
             signature: Optional HMAC-SHA256 signature for MCP webhook verification
                 (X-AdCP-Signature header). Ignored for A2A webhooks.
-            timestamp: Optional timestamp for MCP webhook signature verification
-                (X-AdCP-Timestamp header). Required when signature is provided.
+            timestamp: Optional Unix timestamp (seconds) for MCP webhook signature
+                verification (X-AdCP-Timestamp header). Required when signature is provided.
+            raw_body: Optional raw HTTP request body bytes for signature verification.
+                When provided, used directly instead of re-serializing the payload,
+                avoiding cross-language JSON serialization mismatches. Strongly
+                recommended for production use.
 
         Returns:
             TaskResult with parsed task-specific response data. The structure
@@ -2330,11 +2348,13 @@ async def handle_webhook(
             MCP webhook (HTTP endpoint):
             >>> @app.post("/webhook/{task_type}/{agent_id}/{operation_id}")
             >>> async def webhook_handler(task_type: str, operation_id: str, request: Request):
-            >>>     payload = await request.json()
+            >>>     raw_body = await request.body()
+            >>>     payload = json.loads(raw_body)
             >>>     signature = request.headers.get("X-AdCP-Signature")
             >>>     timestamp = request.headers.get("X-AdCP-Timestamp")
             >>>     result = await client.handle_webhook(
-            >>>         payload, task_type, operation_id, signature, timestamp
+            >>>         payload, task_type, operation_id, signature, timestamp,
+            >>>         raw_body=raw_body,
             >>>     )
             >>>     if result.success:
             >>>         print(f"Task completed: {result.data}")
@@ -2368,7 +2388,7 @@ async def handle_webhook(
         else:
             # MCP webhook (dict payload)
             return await self._handle_mcp_webhook(
-                payload, task_type, operation_id, signature, timestamp
+                payload, task_type, operation_id, signature, timestamp, raw_body
             )
 
 

src/adcp/webhooks.py

@@ -125,7 +125,10 @@ def create_mcp_webhook_payload(
 
 
 def get_adcp_signed_headers_for_webhook(
-    headers: dict[str, Any], secret: str, timestamp: str, payload: dict[str, Any] | AdCPBaseModel
+    headers: dict[str, Any],
+    secret: str,
+    timestamp: str | int | None,
+    payload: dict[str, Any] | AdCPBaseModel,
 ) -> dict[str, Any]:
     """
     Generate AdCP-compliant signed headers for webhook delivery.
@@ -136,28 +139,29 @@ def get_adcp_signed_headers_for_webhook(
 
     The function adds two headers to the provided headers dict:
     - X-AdCP-Signature: HMAC-SHA256 signature in format "sha256=<hex_digest>"
-    - X-AdCP-Timestamp: ISO 8601 timestamp used in signature generation
+    - X-AdCP-Timestamp: Unix timestamp in seconds
 
     The signing algorithm:
     1. Constructs message as "{timestamp}.{json_payload}"
-    2. JSON-serializes payload with compact separators (no sorted keys for performance)
+    2. JSON-serializes payload with default separators (matches wire format from json= kwarg)
     3. UTF-8 encodes the message
     4. HMAC-SHA256 signs with the shared secret
     5. Hex-encodes and prefixes with "sha256="
 
     Args:
         headers: Existing headers dictionary to add signature headers to
         secret: Shared secret key for HMAC signing
-        timestamp: ISO 8601 timestamp string (e.g., "2025-01-15T10:00:00Z")
+        timestamp: Unix timestamp in seconds (str or int). If None, uses current time.
         payload: Webhook payload (dict or Pydantic model - will be JSON-serialized)
 
     Returns:
         The modified headers dictionary with signature headers added
 
     Examples:
         Sign and send an MCP webhook:
-        >>> from adcp.webhooks import create_mcp_webhook_payload get_adcp_signed_headers_for_webhook
-        >>> from datetime import datetime, timezone
+        >>> import time
+        >>> from adcp.webhooks import create_mcp_webhook_payload
+        >>> from adcp.webhooks import get_adcp_signed_headers_for_webhook
         >>>
         >>> payload = create_mcp_webhook_payload(
         ...     task_id="task_123",
@@ -166,9 +170,9 @@ def get_adcp_signed_headers_for_webhook(
         ...     result={"products": [...]}
         ... )
         >>> headers = {"Content-Type": "application/json"}
-        >>> timestamp = datetime.now(timezone.utc).isoformat()
         >>> signed_headers = get_adcp_signed_headers_for_webhook(
-        ...     headers, secret="my-webhook-secret", timestamp=timestamp, payload=payload
+        ...     headers, secret="my-webhook-secret", timestamp=str(int(time.time())),
+        ...     payload=payload,
         ... )
         >>>
         >>> # Send webhook with signed headers
@@ -184,35 +188,31 @@ def get_adcp_signed_headers_for_webhook(
         {
             "Content-Type": "application/json",
             "X-AdCP-Signature": "sha256=a1b2c3...",
-            "X-AdCP-Timestamp": "2025-01-15T10:00:00Z"
+            "X-AdCP-Timestamp": "1773185740"
         }
-
-        Sign with Pydantic model directly:
-        >>> from adcp import GetMediaBuyDeliveryResponse
-        >>> from datetime import datetime, timezone
-        >>>
-        >>> response: GetMediaBuyDeliveryResponse = ...  # From API call
-        >>> headers = {"Content-Type": "application/json"}
-        >>> timestamp = datetime.now(timezone.utc).isoformat()
-        >>> signed_headers = get_adcp_signed_headers_for_webhook(
-        ...     headers, secret="my-webhook-secret", timestamp=timestamp, payload=response
-        ... )
-        >>> # Pydantic model is automatically converted to dict for signing
     """
+    # Default to current Unix time if not provided
+    if timestamp is None:
+        import time
+
+        timestamp = str(int(time.time()))
+    else:
+        timestamp = str(timestamp)
+
     # Convert Pydantic model to dict if needed
     # All AdCP types inherit from AdCPBaseModel (Pydantic BaseModel)
     if hasattr(payload, "model_dump"):
         payload_dict = payload.model_dump(mode="json")
     else:
         payload_dict = payload
 
-    # Serialize payload to JSON with consistent formatting
-    # Note: sort_keys=False for performance (key order doesn't affect signature)
-    payload_bytes = json.dumps(payload_dict, separators=(",", ":"), sort_keys=False).encode("utf-8")
+    # Serialize payload to JSON with default formatting (matches what json= kwarg sends on the wire)
+    # This aligns with the JS reference implementation's JSON.stringify() behavior
+    payload_json = json.dumps(payload_dict)
 
     # Construct signed message: timestamp.payload
     # Including timestamp prevents replay attacks
-    signed_message = f"{timestamp}.{payload_bytes.decode('utf-8')}"
+    signed_message = f"{timestamp}.{payload_json}"
 
     # Generate HMAC-SHA256 signature over timestamp + payload
     signature_hex = hmac.new(

tests/fixtures/webhook-hmac-sha256.json

@@ -0,0 +1,68 @@
+{
+  "version": 1,
+  "algorithm": "HMAC-SHA256",
+  "secret": "test-secret-key-minimum-32-characters-long",
+  "description": "Test vectors from adcontextprotocol/adcp PR #1383",
+  "vectors": [
+    {
+      "description": "compact JSON (JS-style JSON.stringify)",
+      "timestamp": 1700000000,
+      "raw_body": "{\"event\":\"creative.status_changed\",\"creative_id\":\"creative_123\",\"status\":\"approved\"}",
+      "expected_signature": "sha256=c4faf82609efe07621706df0d28c801de2b5145f427e129f243a3839df891a4e"
+    },
+    {
+      "description": "spaced JSON (Python-style json.dumps with default separators)",
+      "timestamp": 1700000000,
+      "raw_body": "{\"event\": \"creative.status_changed\", \"creative_id\": \"creative_123\", \"status\": \"approved\"}",
+      "expected_signature": "sha256=4acce503547a93922a2b41c32f5f0e646b71a36572fd1536d3d7fcd88a4e5c5f"
+    },
+    {
+      "description": "empty object",
+      "timestamp": 1700000000,
+      "raw_body": "{}",
+      "expected_signature": "sha256=fc66235ab6cf0a5927d76d88194036fa99c7e08c75d55c9de5008288d448f1a0"
+    },
+    {
+      "description": "nested objects and arrays",
+      "timestamp": 1700000000,
+      "raw_body": "{\"task_id\":\"task_456\",\"operation_id\":\"op_789\",\"result\":{\"media_buy_id\":\"mb_001\",\"packages\":[{\"package_id\":\"pkg_1\"},{\"package_id\":\"pkg_2\"}]}}",
+      "expected_signature": "sha256=a90052e145bd73ba69a236748df05a3887ef9e73ddd429ef179bdd498ddb97ba"
+    },
+    {
+      "description": "unicode characters (literal UTF-8, not escaped)",
+      "timestamp": 1700000000,
+      "raw_body": "{\"brand_name\":\"Café Münchën\",\"tagline\":\"日本語テスト\"}",
+      "expected_signature": "sha256=4383aa943264c461c5b9796734fdd9ae51934ecbdf7d38fcf94d330bfa590576"
+    },
+    {
+      "description": "pretty-printed JSON (multiline with indentation)",
+      "timestamp": 1700000000,
+      "raw_body": "{\n  \"status\": \"completed\",\n  \"result\": {\n    \"id\": \"mb_001\"\n  }\n}",
+      "expected_signature": "sha256=ad4858d6a7a38207ee178502b4bffc700080258a433e127919b445b68794f085"
+    },
+    {
+      "description": "numeric values, booleans, and null",
+      "timestamp": 1700000000,
+      "raw_body": "{\"price\":19.99,\"count\":1000,\"active\":true,\"discount\":null}",
+      "expected_signature": "sha256=12d4173bebd369c066880bd8f12952c4c1f6f48addbc1dc5267d8ba8de205a4f"
+    },
+    {
+      "description": "empty body",
+      "timestamp": 1700000000,
+      "raw_body": "",
+      "expected_signature": "sha256=9ab3f90245d5919d344a849a4a1b0ec20b75fcf8f29d817e63b23b54fce52294"
+    },
+    {
+      "description": "timestamp zero",
+      "timestamp": 0,
+      "raw_body": "{\"event\":\"test\"}",
+      "expected_signature": "sha256=446cc9dbe11ee98af9445a27dfcf9d52530c874583e5750d295bad336a406c3c"
+    },
+    {
+      "description": "large timestamp (year 2040)",
+      "timestamp": 2208988800,
+      "raw_body": "{\"event\":\"test\"}",
+      "expected_signature": "sha256=a0fdee5e93b2ac2efdf8d3d22b7a03ae8e6df157b493d0140f7902ef32f6be60"
+    }
+  ]
+}