fix: sign webhook HMACs over the exact transmitted bytes (#1441)

[pmezzich]

Summary

Closes #1441.

The AdCP legacy-HMAC contract is byte-equality: the signature covers {unix_timestamp}.{raw_http_body}. All three webhook senders signed one serialization and let the HTTP client re-serialize another (spaced vs compact separators, sorted vs insertion key order, ensure_ascii vs UTF-8, ISO vs unix timestamp) — so every emitted signature failed raw-body verification against a spec-compliant receiver. Sender and the in-repo receiver reference only agreed with each other because both re-serialized the same wrong way.

Fix

All three senders consolidate onto adcp.sign_legacy_webhook (in the pinned SDK since #1399), which returns (signed_headers, body_bytes). Each sender serializes once, before its retry loop, and POSTs those exact bytes (content=/data=, never json=):

• protocol_webhook_service (buyer push notifications — the primary live exposure): lib signer; the single serialization is also reused for payload-size metrics (was a third json.dumps).
• webhook_delivery_service (delivery reports): deletes the local _generate_hmac_signature (sorted + ISO); X-ADCP-Timestamp switches ISO→unix seconds per spec and the signature gains the sha256= prefix. BR-UC-004 feature wording updated accordingly.
• webhook_delivery (generic retry sender — latent, no live signed callers): lib signer; drops the non-spec X-Webhook-* header names. WebhookAuthenticator.sign_payload is removed (same sign-different-bytes defect); verify_signature stays (raw-body, spec-shaped).
• webhook_verification (receiver reference): raw str/bytes body is the contract; the dict path is kept only as a deprecated compact-form convenience (the old sort_keys dict path mirrored the sender bug); accepts unix timestamps (ISO kept for back-compat).

Tests grade the wire, not a re-serialization

• New tests/integration/test_webhook_wire_signature.py: real requests transport into a raw-capturing localhost receiver — HMAC recomputed over the received socket bytes for both the push-notification and generic senders, with non-ASCII / float / unsorted-key payloads (the exact divergence traps). Both in-repo verifiers must agree on the raw body.
• Every assertion that previously "verified" by re-running the sender's serialization now grades the transmitted bytes (mock kwargs carry data=/content= bytes); the BDD HMAC step recomputes over the raw sent body; header asserts are case-insensitive (X-AdCP-* casing from the lib).

Execution proof

• Full BR-UC-004 BDD: 390 passed / 510 xfailed / 0 failed (1h43m run).
• HMAC-signed webhook scenarios: PASSED per transport under -rxX (19.8s — real execution).
• Webhook/delivery unit + harness scope: 471 passed.
• Wire-signature suite: 2 passed against a live localhost receiver.

Compatibility note

Signature values change for identical payloads — deliberately: current signatures are already rejected by any spec-compliant verifier, so there is no working behavior to preserve. Receivers that copied the old verify-by-re-serialization pattern must switch to raw-body verification (the receiver reference in this PR shows the shape). Suggested for release notes.

[ChrisHuie]

Review — webhook HMAC byte-equality (#1441)

This is a well-executed, spec-correct fix. The {unix_timestamp}.{raw_http_body} scheme
with a sha256=-prefixed X-ADCP-Signature and a unix X-ADCP-Timestamp matches the AdCP
3.1.0-beta.3 Legacy-HMAC section verbatim, and the prior behavior (signing a re-serialization,
ISO timestamp) was genuinely off-spec — so this moves the code toward the spec, not away from
it. Consolidating the three senders onto sign_legacy_webhook and POSTing the signed bytes
verbatim (data=/content=, never json=) is the right structural fix. The new wire test is
a real oracle — reverting a sender to json=payload makes it fail with "HMAC does not verify
over the received raw bytes" (confirmed locally), so it grades the wire rather than
re-confirming a re-serialization. Both receiver-reference verifiers use hmac.compare_digest.

It can't merge yet (conflicts with current main), and there are a few in-scope improvements
plus some pre-existing gaps we'll track separately.

BLOCKER — merge state

The branch is CONFLICTING with current main (it predates the transport-aware BDD harness and
other merges). Conflicts: .duplication-baseline, the BR-UC-004 feature + uc004_delivery.py,
and test_delivery.py. Please rebase and re-run full CI (the current green was computed against
an older main).

• Resolving .duplication-baseline: current main lowered the ceiling to tests: 85 (this
  branch has 88). Re-run uv run python .pre-commit-hooks/check_code_duplication.py after
  rebasing and set the real value — don't keep 88.
• Keep the ISO→unix change in lockstep across all three places or a step will orphan: feature
  line 256 (unix timestamp), the then_timestamp_header parser (value.isdigit()), and
  webhook_delivery_service.py production (int(...timestamp())).

SHOULD-FIX (in scope)

1. Byte-equality has no single home. The compact body form is hand-rolled in the three
   unsigned sender branches (webhook_delivery.py:118, protocol_webhook_service.py:200,
   webhook_delivery_service.py:451) and the verifier dict path (webhook_verification.py:133),
   and it must stay byte-identical to what sign_legacy_webhook returns — but nothing enforces
   that (R0801 can't see one-liners). The HMAC-recompute test oracle is likewise copy-pasted
   across six test files. Please extract one _compact_webhook_body(payload) -> bytes for the
   senders + verifier (with a test asserting helper(p) == sign_legacy_webhook(secret, p)[1]),
   and one tests/helpers/assert_hmac_over_bytes(...) for the six test sites.
2. The third sender has no real-socket wire test. test_webhook_wire_signature.py covers
   protocol_webhook_service and webhook_delivery, but not webhook_delivery_service — the
   sender with the biggest change (ISO→unix, removed local signer, httpx content=). Please add
   a third case driving WebhookDeliveryService at the loopback receiver (it reads its config
   from the DB, so it needs integration_db + a seeded webhook_secret, or target
   _send_webhook_enhanced to keep it DB-free like the other two).
3. The feature file is generated. BR-UC-004-deliver-media-buy-metrics.feature carries
   # DO NOT EDIT -- re-run scripts/compile_bdd.py --merge, and line 256 was hand-edited
   (ISO→unix). It's coherent now, but a future compile_bdd.py --merge could revert it and
   orphan then_timestamp_header (→ the scenario silently xfails). Please make the wording
   change at the adcp-req source and recompile, or confirm merge-mode preserves the edit.
4. Add the spec citation to the PR description (repo Spec-Grounding Gate): the Legacy HMAC
   section of security.mdx + L3/webhooks.mdx at 3.1.0-beta.3, and note that this legacy path
   is ungraded by the webhook-emission storyboard (which grades RFC 9421 — see the note below).
5. Unsigned-path header parity. webhook_delivery_service.py's unsigned branch emits
   X-ADCP-Timestamp (all-caps) while senders 1/3 emit no timestamp on the unsigned path. An
   unsigned timestamp has no verification meaning — simplest to drop it there to match the other
   senders, and refresh the X-ADCP-Signature reference in the module docstring (line 4).

NIT

• protocol_webhook_service.py:195-198 passes headers=headers to sign_legacy_webhook (which
  returns a merged dict) and then also headers.update(signed_headers) — the merge runs twice.
  The other two senders just do .update(); drop headers=headers to match.
• The wire oracle doesn't assert Content-Type. Since moving off json= is what makes it
  non-automatic, add assert headers["content-type"] == "application/json" so a dropped
  setdefault reddens.
• TestWebhookAuthenticator now hosts three tests of the SDK sign_legacy_webhook (its
  sign_payload was deleted) — update the class docstring or split them out.

Pre-existing / out of scope (we'll file these separately — not blocking this PR)

• The spec makes RFC 9421 webhook signing the baseline-required default for sellers; this repo
  emits none, and this PR improves the deprecated legacy fallback (removed in AdCP 4.0). That's
  a fine incremental fix — flagging so it's on the radar; we'll open a follow-up for 9421 emission.
• The legacy scheme also mandates duplicate-object-key rejection on signer and verifier
  (json.loads currently collapses them).
• Two senders POST buyer/config URLs with no send-time SSRF check (only webhook_delivery.py
  validates).
• The weak-secret path sends unsigned when a too-short secret is configured.
• A fourth push-notification sender (order_approval_service.py) has no HMAC branch at all.

Re-review after the rebase + the in-scope items.