fix(security): SSRF guard at webhook delivery time + shared predicate (LC-152) #186
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/lc-152-webhook-delivery-ssrf"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Closes the webhook delivery-time SSRF gap from the LC-148 audit (findings S6 + S7).
Problem
Outgoing-webhook and slash-command webhook URLs were IP-validated only at creation, as a string parse: it rejects IP literals +
localhostbut lets any hostname through ("admin-trusted; full DNS checks out of scope for v1"). At delivery time the host was fetched with no re-validation, so a URL whose hostname resolves to an internal / cloud-metadata address (169.254.169.254) was fetched on every matching event with HMAC-signed payloads. The slash IPv6 guard also missed ULAfc00::/7, and neither client disabled redirect-following (a 3xx to an internal host bypassed the check).Fix
is_globally_routable+host_resolves_public) into a newcrate::ssrfmodule so the unfurler and both webhook deliverers enforce one policy. The unit tests moved with it;unfurlkeeps its per-hop redirect re-validation, now calling the shared helper.run_delivery_tick): resolvest.urland rejects non-http(s) schemes + any host resolving to a non-public address before connecting; a blocked delivery is marked failed terminally (not retried - it won't become public later).run_webhook): resolves the host and rejects the same;webhook_url_ok's IP-literal branch now defers to the shared predicate, so it gainsfc00::/7+ CGNAT + benchmark coverage.redirect::Policy::none().Test
ssrf.rs): v4/v6 accept/reject matrix +host_resolves_publicrejecting private/loopback/metadata IP-literal URLs (offline-safe).outgoing_webhooks::delivery_blocks_non_public_url_and_is_terminal: a webhook with a loopback URL is blocked at delivery and not retried.run_delivery_tickgains a#[doc(hidden)] run_delivery_tick_uncheckedtest seam so the existing delivery-path tests can target their loopback receiver (which the guard, correctly for production, now rejects).just check/just test/just test-saasgreen (theroutes_uploadsupload-pipeline flake under concurrent-binary load is pre-existing and passes in isolation; this PR touches no upload code).Part of the LC-159 post-audit story.
🤖 Generated with Claude Code
Outgoing and slash-command webhook URLs were IP-validated only at creation, as a string parse that rejects IP literals + localhost but lets any hostname through ("admin-trusted; full DNS checks out of scope"). At delivery time the host was fetched with no re-validation, so a URL whose hostname resolves to an internal or cloud-metadata address (169.254.169.254) was fetched on every matching event with signed payloads (audit S6). The slash IPv6 check also missed ULA fc00::/7, and neither client disabled redirect-following, so a 3xx to an internal host bypassed the check (S7). Extracts the unfurl SSRF predicate (is_globally_routable + host_resolves_public) into a shared crate::ssrf module so the unfurler and both webhook deliverers enforce one policy. Adds, at delivery time: - outgoing::run_delivery_tick resolves t.url and rejects non-http(s) schemes + any host resolving to a non-public address before connecting; a blocked delivery is marked failed terminally (not retried). - slash::run_webhook resolves the host and rejects the same; webhook_url_ok's IP-literal branch now defers to the shared predicate (so it gains fc00::/7 + CGNAT + benchmark coverage). - Both webhook reqwest clients set redirect::Policy::none() so a redirect cannot escape the resolved-host check. unfurl keeps its per-hop redirect re-validation, now calling the shared host_resolves_public. The SSRF predicate unit tests moved to ssrf.rs. run_delivery_tick gains a #[doc(hidden)] run_delivery_tick_unchecked test seam so the existing delivery-path tests can target their loopback receiver (which the guard, correctly for production, now rejects). New test: a webhook with a loopback URL is blocked at delivery and not retried. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>