feat(webhook): harden signed callback transport and bind DNS validation to dial authority

[andrexibiza]

Part of #84834 — Webhook Feature Package callback transport. Related #4386 / #73828.

Current ownership

This PR owns the outbound callback transport primitive, not the runtime completion-hook integration. The branch has been semantically compressed onto current main so it no longer carries campaign receipts or contributor-map churn.

Exact topology:

• base: f43eabee5f36e11448086ee8ee17c499958e81bf
• head: bb27b3a39e93f66540c416bc5e73508441f098ad
• commits: 1
• files: 2

Runtime completion ownership remains separate: #85645 owns the true-end-of-run completion seam; #85640 is the terminal integration lane that composes callback transport with the final webhook package. This PR does not claim to close #4386/#73828 by itself.

Transport contract

• callback envelopes are HMAC-SHA256 signed when a secret is configured;
• malformed envelopes are rejected before DNS/network/retry work;
• callback URLs must be public HTTP(S), contain no userinfo, and every resolved address must pass the private/loopback/link-local/metadata guard;
• DNS is resolved once per delivery generation and the socket dials that exact validated address; HTTPS still uses the configured hostname for SNI/certificate verification and preserves the original Host authority;
• redirects are terminal failures and are never followed;
• 4xx responses are not retried; transient failures/5xx use a bounded retry budget;
• the synchronous transport is wrapped by deliver_callback_async() via asyncio.to_thread, so gateway callers do not block the event loop during DNS/connect/TLS/response I/O or backoff.

Review fixes

The previous "per-address resolution check" was not sufficient: validation and urllib connect performed different DNS lookups. That TOCTOU/rebinding class is now closed by binding validation output to the actual dial target. The blocking-I/O and malformed-envelope findings are also covered by focused regressions.

Exact-head verification

At bb27b3a39e93f66540c416bc5e73508441f098ad:

• CI 32391837152 — success
• Docker 32391835648 — success
• Nix 32391835383 — success

These receipts are attached to the one-commit replacement head itself; no historical checks are inherited.

[spfcraze]

This was generated by AI during triage.

Summary:
The SSRF check in deliver_callback and the connection it opens are two separate DNS lookups, so a rebinding answer can pass validate_callback_url and the POST still lands on a loopback/private destination — the body's "DNS rebinding-safe" claim is a stronger guarantee than the mechanism provides.

Problems:

• _is_private_host (webhook_callbacks.py:46) resolves the host with socket.getaddrinfo and returns only a boolean; deliver_callback (line 126) then opens the request with _opener.open(req), and urllib's HTTPConnection resolves the hostname again at connect time. The resolved address from the check is discarded before the connection opens, so the two lookups are unrelated.
• A resolver that returns a public address to the guard's lookups and 127.0.0.1 to the connect-time lookup makes validate_callback_url return ok=True while deliver_callback POSTs to a loopback listener and returns True.

Solution:
Resolve once and connect to the validated address (preserving SNI/Host for the intended hostname) so the connection re-resolves to the checked destination. As a wording that matches the mechanism: "DNS rebinding is mitigated by a per-address resolution check".

Evidence

no deterministic fact backs this claim — model belief, not executed or read evidence

---

Checked against bc282d2 — the PR head when this was written — and 8c8d55b, main at the same moment.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

feat(webhook): add SSRF-guarded signed completion callbacks (Webhook Feature Package)

1. DNS rebinding is not actually closed — _is_private_host resolves the hostname once and checks every returned address, but urllib will re-resolve the hostname again at connect time (TOCTOU): an attacker-controlled DNS that answers the check with a public IP and the connect with 127.0.0.1 bypasses the guard. The docstring claims rebinding is "mitigated by resolving the host and checking every resolved address before connecting", but the check is not bound to the resolved addresses. A real fix pins the resolved IP at connect time (custom socket/opener with Host header), or at least the residual risk should be acknowledged in the docstring. (gateway/platforms/webhook_callbacks.py _is_private_host, deliver_callback)
2. Blocking I/O in an async path — deliver_callback uses urllib.request plus blocking time.sleep for backoff. If this runs on the webhook completion path (asyncio), it stalls the event loop for up to timeout + 1s per attempt. Prefer asyncio/to_thread, or clearly document that callers must invoke it off the loop.
3. Retry semantics mismatch with "fire-and-forget" — CALLBACK_MAX_ATTEMPTS=2 with time.sleep(CALLBACK_BACKOFF_SECONDS * attempt) means each call blocks ~1s before returning; for fire-and-forget delivery, consider an async retry with the loop instead of sleeping inline.
4. Unvalidated envelope — deliver_callback(url, secret, {}, ...) on a public URL raises KeyError on envelope["execution_id"] (caught by the broad except Exception, then retried pointlessly). Validate the envelope shape before the retry loop. Minor.

[andrexibiza]

Published a replacement head for this review train: bb27b3a39e93f66540c416bc5e73508441f098ad, rebased directly onto current main as a one-commit/two-file transport object.

The DNS-rebinding finding was correct and is fixed at the authority boundary rather than by wording: resolution now produces the exact address the socket dials. HTTPS still verifies/SNIs against the configured hostname and preserves the original Host authority, so there is no second DNS lookup between SSRF validation and connect. Any unsafe answer fails the entire resolution set closed.

The same head also adds deliver_callback_async() via asyncio.to_thread so DNS/connect/TLS/response I/O and retry backoff stay off the gateway event loop, treats 3xx as terminal without following redirects, and rejects malformed envelopes before DNS/retry work.

I also narrowed the PR claim: this is the callback transport authority only. Runtime true-completion wiring remains with #85645/#85640 rather than introducing a second completion owner here. Fresh exact-head CI/Docker/Nix is running; no historical green is inherited.

[andrexibiza]

@teknium1 maintainer review requested on exact head bb27b3a39e93f66540c416bc5e73508441f098ad.

Revalidated now: open/non-draft/mergeable; exact-head CI 32391837152, Docker 32391835648, and Nix 32391835383 are all successful.

This lane should remain the callback transport authority only: DNS validation is bound to the dial target, redirects fail closed, response/retry work is bounded, and gateway use is async-wrapped. #85645 owns true completion timing; #85640 owns final integration. Please review/merge without folding those distinct authorities back together.