Wire SSRF guard into legacy webhooks.deliver()

[bokelley]

Summary

adcp.webhooks.deliver() (the deprecated AdCP 3.x HMAC-SHA256 path) constructs an unpinned httpx.AsyncClient(timeout=...) and POSTs to a buyer-controlled URL when no operator client is supplied. No SSRF guard.

Source: src/adcp/webhooks.py:867-872. Surfaced by security-reviewer on PR #297 as L4 — explicitly deferred from the prep PR scope.

Why deferred

deliver() is the AdCP 3.x deprecated path; emits DeprecationWarning already; new traffic goes through WebhookSender. PR #297 was scoped to RFC 9421 sender hardening.

Proposed fix

Mirror the WebhookSender._send_bytes pattern:

• When the operator supplies a client, trust them.
• When the sender owns the client, build a per-request AsyncIpPinnedTransport via build_async_ip_pinned_transport(url, ...) and construct httpx.AsyncClient(transport=transport, follow_redirects=False, trust_env=False).
• Same allow_private_destinations and allowed_destination_ports kwargs on the public surface.

Risk

Adopters on the legacy deliver() path posting to private/internal endpoints (dev/test fixtures) will start getting SSRFValidationError. One-kwarg opt-out via allow_private=True (or whatever name fits the existing deliver() surface).

References

• Code: src/adcp/webhooks.py:867-872
• Pattern to mirror: src/adcp/webhook_sender.py:_send_bytes (post PR feat(signing): close 4 SSRF gaps and add opt-in port hardening (foundation audit) #297)
• Sec audit reference: .context/foundations-audit/FINDINGS.md

[bokelley]

Triage

Classification: Bug (security — SSRF guard absent on legacy webhook path)
Bucket(s): signing / client (no verified repo labels for either)
Status: deferred
Blocked-on: #297 — resurfaces on merge

What the code shows:

• deliver() lines 861–868 constructs a bare httpx.AsyncClient(timeout=…) when client=None. No SSRF transport is wired.
• build_async_ip_pinned_transport already lives on main (src/adcp/signing/ip_pinned_transport.py:310), so the fix is technically implementable right now without waiting for feat(signing): close 4 SSRF gaps and add opt-in port hardening (foundation audit) #297.
• The existing docstring acknowledges the gap (line 769): "does NOT block private / link-local destinations."
• The security finding was already captured as L4 in the PR feat(signing): close 4 SSRF gaps and add opt-in port hardening (foundation audit) #297 review panel, with explicit scope-defer. Approach as described (per-request pinned transport on the owned-client path; operator-supplied clients trusted) is consistent with how WebhookSender._send_bytes is being fixed in feat(signing): close 4 SSRF gaps and add opt-in port hardening (foundation audit) #297.

My take: This is a strong fold candidate for PR #297 — same author, same SSRF hardening effort, PR still iterating. The deliver() patch is small: swap the bare AsyncClient on the owned-client path for one backed by build_async_ip_pinned_transport, add allow_private_destinations and allowed_destination_ports kwargs to deliver()'s public surface, and update the security note in the docstring. Tracking here so the gap has a named home and resurfaces if #297 ships without it.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01SaZb7fWVnax15UzFjAWCUs

---

Generated by Claude Code

[bokelley]

Triage

Classification: Bug (security gap)
Bucket(s): signing
Status: ready-for-human

What the experts said:

• security-reviewer: Flag — SSRF risk is real despite deprecation. deliver() validates scheme and control characters but does no post-DNS-resolution check; DNS rebinding and private-range POSTs (e.g. https://10.0.0.1/internal) are unblocked. Deprecated paths are often the longest-lived in operator deployments. IP-pinned transport (build_async_ip_pinned_transport + follow_redirects=False + trust_env=False) is the correct and sufficient mitigation, proven in revocation_fetcher.py:291-302. However: adding allow_private=True to the public deliver() signature is not recommended — it permanently widens the API contract and creates a footgun for LLM-generated or config-driven callers. Callers needing internal destinations should supply their own client= kwarg with a custom transport.

My take: The mitigation pattern is clear and well-precedented in this codebase. The open design question is whether to add the escape kwarg to the public signature or require callers to use client=. This is a policy call on the public API surface that needs explicit sign-off before shipping. Flagging for @bokelley.

---

Triaged by Claude Code. Session: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

---

Generated by Claude Code