[HARD] Design and ship end-to-end SSRF hardening for outbound webhook delivery (redirects, DNS rebinding, IP-literal encodings)

[Emmyt24]

Description

octo's webhook system (crates/webhooks/src/lib.rs) delivers signed payloads to operator-registered URLs, gated by is_safe_url's string-based host checks. Taken together, the smaller tickets on this tracker around redirect-following (reqwest's default policy follows redirects, bypassing the registration-time check entirely) and IP-literal-encoding bypasses (decimal/octal/hex/IPv6 forms of blocked addresses) point at a single underlying gap: is_safe_url validates a URL string once, at registration time, rather than validating the resolved destination at delivery time, every time. This ticket asks for the complete, defensible design and implementation — not just patching the two known gaps individually, but reasoning about the whole class of TOCTOU (time-of-check-to-time-of-use) SSRF risk in this delivery path, including DNS rebinding (a hostname that resolves to a public IP at registration time but a private IP at delivery time).

Requirements and Context

• Write a short design section in the PR description covering: (1) the redirect-following fix (disable auto-redirect, treat 3xx as a failed delivery), (2) proper IP-literal parsing (parse the host as an actual std::net::IpAddr where possible instead of string-prefix matching, covering decimal/octal/hex forms by normalizing before parsing, and all of IPv4 private/link-local/CGNAT plus IPv6 loopback/link-local/ULA), and (3) a decision on DNS-rebinding: at minimum, re-resolve and re-validate the destination IP immediately before each delivery attempt (not just at registration), and state clearly whether you consider connect-time-of-check-to-time-of-use fully closed by this or whether a residual window remains (a fully bulletproof fix might require a custom reqwest connector that validates the resolved socket address at the TCP-connect layer — decide how far to take this and justify the stopping point).
• Preserve the existing OCTO_ALLOW_LOCAL_WEBHOOKS dev/test escape hatch for all new checks.
• This should subsume and close the narrower redirect-following and IP-encoding tickets if they haven't already been merged individually — coordinate via the issue tracker; if those smaller PRs land first, this ticket becomes the integration/verification pass plus the DNS-rebinding piece specifically.

Suggested Execution

Branch: hard/webhooks/full-ssrf-hardening

Implement Changes

• Rework is_safe_url (or replace it with a richer validation module) in crates/webhooks/src/lib.rs to parse hosts as real IpAddrs wherever the host is IP-literal, and to resolve hostnames and validate the resolved address(es).
• Set reqwest::redirect::Policy::none() on the delivery client.
• If pursuing connect-time validation, consider reqwest's custom-resolver or a hyper connector hook — this is genuinely the hard part of this ticket, and a well-reasoned partial solution with clearly documented residual risk is an acceptable outcome if fully closing the TOCTOU window turns out to require disproportionate complexity; say so explicitly.

Test and Commit

• A comprehensive table-driven corpus covering every encoding form (decimal, octal, hex, IPv6 loopback/link-local/ULA/mapped) proven blocked.
• A redirect-to-internal-target test proving the redirect is never followed.
• A DNS-rebinding simulation (e.g. a test DNS resolver stub, or a documented manual verification if a stub is impractical in this test harness) demonstrating the delivery-time re-validation actually catches a hostname that changed resolution between registration and delivery.
• Full regression pass of every existing is_safe_url test.
• Run cargo test -p octo-webhooks && cargo build --release locally before committing.

Example Commit Message

fix(webhooks): close redirect, IP-encoding, and DNS-rebinding SSRF gaps in webhook delivery

Replaces string-based URL validation with real IP-address parsing and
delivery-time re-validation, disables automatic redirect-following, and adds
a comprehensive encoding-bypass and DNS-rebinding test corpus. Documents any
remaining residual risk explicitly rather than overclaiming full closure.

Guidelines

• This is a security-sensitive change touching a shared crate (octo-webhooks) consumed by both octo-api and octo-ingest — run the full workspace test suite, not just this crate's, before submitting.
• Be honest in the PR description about what this closes vs. what residual risk (if any) remains — overclaiming security guarantees is worse than clearly scoping them.
• Reference this issue with Closes #<issue-number> in the PR description.

Submission Requirements (Hard Issue)

This is one of the hard/complex issues on this tracker. It touches more than one crate (or backend + frontend), so review will look for a design paragraph in the PR description explaining the approach you chose and any tradeoffs. In addition to the Guidelines above, your PR description must include:

• A screenshot of the full test suite passing locally (e.g. the tail of cargo test --workspace or npm run build && npx vitest run showing a green/passing summary).
• A screenshot of the feature building successfully end-to-end (e.g. cargo build --release for Rust changes, or npm run build for frontend changes) with no errors.
• PRs missing either screenshot will be asked to add them before review starts.