-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Summary
get_adcp_signed_headers_for_webhook() in src/adcp/webhooks.py computes the HMAC signature over compact JSON (no spaces), but callers send the HTTP body via requests.post(url, json=payload) which uses default JSON (with spaces). Receivers that verify against raw HTTP body bytes (per the AdCP spec) will always get a signature mismatch.
Root Cause
In get_adcp_signed_headers_for_webhook():
payload_bytes = json.dumps(payload_dict, separators=(",", ":"), sort_keys=False).encode("utf-8")
signed_message = f"{timestamp}.{payload_bytes.decode('utf-8')}"This signs over compact JSON: {"contextId":"msg_123","final":false}
But the caller sends the body with requests.post(url, json=payload), which internally uses json.dumps(payload) with default separators, producing: {"contextId": "msg_123", "final": false}
The receiver verifies HMAC(secret, "{timestamp}.{raw_http_body}") — since the raw body has spaces but the signature was computed without spaces, they never match.
Reference Implementation Mismatch
The official @adcp/client JS library (the reference implementation) uses JSON.stringify(payload) for signing, which produces default serialization with spaces — matching what goes on the wire.
From the JS test suite:
const message = `${timestamp}.${JSON.stringify(payload)}`;
const hmac = crypto.createHmac('sha256', webhookSecret);
hmac.update(message);
const signature = `sha256=${hmac.digest('hex')}`;JSON.stringify output: {"contextId": "msg_123", "final": false} (with spaces after : and ,)
The Python client should match this behavior.
Reproduction
import json, hashlib, hmac
payload = {"contextId": "msg_123", "final": False, "kind": "status-update"}
secret = "my-test-webhook-secret-that-is-32chars!"
timestamp = "1773185740"
# What get_adcp_signed_headers_for_webhook signs (compact):
compact = json.dumps(payload, separators=(",", ":"), sort_keys=False)
# '{"contextId":"msg_123","final":false,"kind":"status-update"}'
# What requests.post(json=payload) sends on the wire (default):
default = json.dumps(payload)
# '{"contextId": "msg_123", "final": false, "kind": "status-update"}'
sig_compact = hmac.new(secret.encode(), f"{timestamp}.{compact}".encode(), hashlib.sha256).hexdigest()
sig_default = hmac.new(secret.encode(), f"{timestamp}.{default}".encode(), hashlib.sha256).hexdigest()
print(f"Match: {sig_compact == sig_default}") # FalseSuggested Fix
# In get_adcp_signed_headers_for_webhook(), change:
payload_bytes = json.dumps(payload_dict, separators=(",", ":"), sort_keys=False).encode("utf-8")
# To:
payload_bytes = json.dumps(payload_dict).encode("utf-8")This aligns with the JS reference implementation's JSON.stringify() behavior.
Alternatively, the function's docstring could be updated to instruct callers to pre-serialize and send with data= instead of json=, but that's a larger change for all consumers.
Impact
All consumers of get_adcp_signed_headers_for_webhook() that use requests.post(json=...) or httpx.post(json=...) will have webhook HMAC validation failures on the receiver side. This is currently breaking create_media_buy, update_media_buy, and sync_creatives webhook callbacks.