fix(webhooks): Close the SSRF gap where reqwest follows redirects into private address ranges

[Emmyt24]

Description

WebhookSender in crates/webhooks/src/lib.rs constructs its reqwest::Client with only a timeout configured (reqwest::Client::builder().timeout(Duration::from_secs(10)).build()), which means it uses reqwest's default redirect policy — following up to 10 redirects. is_safe_url validates the registered URL string at registration time (create_webhook) and again is not re-checked per-delivery, so a webhook endpoint that is safe at registration time (a public URL) can later respond with a 3xx redirecting the actual request to http://169.254.169.254/... or http://localhost/..., and deliver_with_retry would follow it without any re-validation — a classic SSRF-via-redirect gap.

Requirements and Context

• Change the reqwest::Client used for webhook delivery to use reqwest::redirect::Policy::none(), so redirects are never automatically followed.
• Decide and implement the desired behavior when a webhook endpoint responds with a 3xx: treat it as a failed delivery attempt (log the response code as today, do not follow it) — this is the safer default and requires no additional validation logic.
• This is one of the tracked hard/complex issues on this board (see the corresponding hard-issue ticket for the full cross-cutting SSRF hardening effort, which also covers IP-literal encodings) — this specific ticket is scoped narrowly to the redirect-following gap only, so keep the change minimal and focused.

Suggested Execution

Branch: fix/webhooks/no-redirect-follow-ssrf

Implement Changes

• Update WebhookSender::new in crates/webhooks/src/lib.rs to set .redirect(reqwest::redirect::Policy::none()) on the client builder.
• Confirm deliver_with_retry's existing status-code handling (r.status().is_success()) already treats a 3xx as non-success without any change needed, now that it will actually be surfaced as such instead of silently followed.

Test and Commit

• webhook_delivery_does_not_follow_a_redirect_response (mock endpoint returns a 302 pointing at a second mock endpoint; assert the second endpoint never receives a request, and the delivery is logged as failed with the 302's code).
• Run cargo test -p octo-webhooks locally before committing.

Example Commit Message

fix(webhooks): disable HTTP redirect-following for webhook delivery to close an SSRF gap

WebhookSender's HTTP client used reqwest's default redirect policy (follow
up to 10 redirects), so a registered public URL that later returned a 3xx
could redirect delivery into a private/internal target that is_safe_url's
registration-time check would never see. Disables redirect-following entirely
for webhook delivery.

Guidelines

• Do not attempt to build a 'safe redirect' allowlist mechanism in this PR — the correct fix is to simply never follow redirects for outbound webhook delivery; this keeps the change small and easy to review.
• Reference this issue with Closes #<issue-number> in the PR description.

[YfengJ]

Hi maintainers, I would like to be considered for this FWC26 issue.

If assigned, I would keep the change narrowly scoped to the redirect-following SSRF gap: set the webhook delivery reqwest client to redirect::Policy::none(), preserve the existing status-code failure path for 3xx responses, and add a focused cargo test -p octo-webhooks case proving a 302 target is not requested. I will wait for explicit maintainer or GrantFox assignment before cloning, coding, or opening a PR.

[ALLEN-AYODEJI]

Hello Maintainer!

I would really love to work on this, as i have done something very similar to this!