Fix #78: webhook delivery SSRF hardening #88

Merged

crueber merged 3 commits from fix/issue-78 into main

2026-09-05 01:49:54 +00:00

crueber commented

2026-09-05 01:39:35 +00:00

Owner

Fixes #78.

Scope mapping: the issue names internal/notify/webhooks.go + internal/repoimport/url.go, which exist only in unmerged feature worktrees, not on origin/main. The live SSRF surface on main is identical in shape: internal/events/sink.go used a default http.Client (up to 10 cross-scheme/scheme-downgrading redirects, Go forwarding X-Walgit-Signature + body cross-host) with no IP screening. This PR hardens that sink.

Fix (stdlib only, law 1; wire contract + admin-gating + delivery semantics unchanged):

No-redirect client: CheckRedirect refuses ALL redirects (not same-host-only — same-host still permits an https→http plaintext downgrade of secret+body). Any 3xx fails delivery, cursor untouched, replay next wake-up.
Delivery-time IP screening with pinned dialing (internal/events/ssrf.go, netip only): transport DialContext resolves, drops every non-public address, dials survivors — check and connect share one resolution (no TOCTOU); literal private IPs fail pre-SYN; unresolvable fails closed. Range table extends the repoimport isPrivateIP logic with 198.18/15 + TEST-NETs + reserved/multicast/unspecified.
Loopback stays allowed (operator-configured URL, dev/CI targets localhost, contract tests use httptest servers).
Doc Decisions entry in docs/go/09_events.md §4.2 + Decisions (law 12); no new goroutines/locks, no new concurrency hazard.

Tests: table-driven httptest — cross-host redirect reaches target 0 times (no signature/body leak); redirect-to-169.254 refused; same-host redirect refused; 9 private-literal URLs refused fast; dial-layer unit tests; 40-case blocked-IP table incl. mapped-v6. gofmt/vet clean, -race green, internal/events coverage 97.5% (gate ≥95%), e2e TestE2E_EventsWebhookDelivery passes against the real binary (loopback path intact).

Fixes #78. **Scope mapping:** the issue names internal/notify/webhooks.go + internal/repoimport/url.go, which exist only in unmerged feature worktrees, not on origin/main. The live SSRF surface on main is identical in shape: internal/events/sink.go used a default http.Client (up to 10 cross-scheme/scheme-downgrading redirects, Go forwarding X-Walgit-Signature + body cross-host) with no IP screening. This PR hardens that sink. **Fix (stdlib only, law 1; wire contract + admin-gating + delivery semantics unchanged):** - No-redirect client: CheckRedirect refuses ALL redirects (not same-host-only — same-host still permits an https→http plaintext downgrade of secret+body). Any 3xx fails delivery, cursor untouched, replay next wake-up. - Delivery-time IP screening with pinned dialing (internal/events/ssrf.go, netip only): transport DialContext resolves, drops every non-public address, dials survivors — check and connect share one resolution (no TOCTOU); literal private IPs fail pre-SYN; unresolvable fails closed. Range table extends the repoimport isPrivateIP logic with 198.18/15 + TEST-NETs + reserved/multicast/unspecified. - Loopback stays allowed (operator-configured URL, dev/CI targets localhost, contract tests use httptest servers). - Doc Decisions entry in docs/go/09_events.md §4.2 + Decisions (law 12); no new goroutines/locks, no new concurrency hazard. **Tests:** table-driven httptest — cross-host redirect reaches target 0 times (no signature/body leak); redirect-to-169.254 refused; same-host redirect refused; 9 private-literal URLs refused fast; dial-layer unit tests; 40-case blocked-IP table incl. mapped-v6. gofmt/vet clean, -race green, internal/events coverage 97.5% (gate ≥95%), e2e TestE2E_EventsWebhookDelivery passes against the real binary (loopback path intact).

crueber added 1 commit

2026-09-05 01:39:36 +00:00

Fix #78 : webhook delivery SSRF hardening (09 §4.2) 4b890027d3

No-redirect client (CheckRedirect refuses all) plus delivery-time
IP screening with pinned dialing; loopback stays allowed (dev/test
rule). Regression tests: cross-host redirect leaks nothing,
redirect-to-link-local refused, private literals refused.

crueber referenced this pull request

2026-09-05 01:39:38 +00:00

[omp major-3] Webhook SSRF: https validation bypassable via redirects #78

crueber referenced this pull request

2026-09-05 01:41:39 +00:00

[omp major-3] Webhook SSRF: https validation bypassable via redirects #78

crueber referenced this pull request from a commit

2026-09-05 01:47:11 +00:00

Fix #78 (rework): tenant webhook SSRF hardening (06 §5.3)

crueber added 1 commit

2026-09-05 01:47:11 +00:00

Fix #78 (rework): tenant webhook SSRF hardening (06 §5.3) 4293460713

PR #88 hardened the operator events sink but left the tenant path in
internal/notify/webhooks.go intact (default clients followed up to 10
cross-scheme redirects; Go forwards X-Walgit-Signature + body
cross-host; no IP screen on the https validation branch).

Rework per review: refuse-all CheckRedirect on BOTH hook lanes
(hookClient + hookClientInsecure), delivery-time private/loopback/
reserved screening with pinned DialContext on both transports via the
new shared stdlib-only internal/egress package (events ssrf.go keeps
its names as thin wrappers so the two sinks cannot drift apart).
Loopback stays allowed (dev rule). Refuse-all trade-off documented:
benign trailing-slash / http->https hops fail delivery (canonical URLs
required) rather than risk an https->http secret downgrade.

Tests: 302 cross-host + 307/308 body-preserving variants assert zero
target arrival and no signature leak; 13 private-literal https URLs
refused at delivery with cursor held; loopback delivers. Decisions
entry in 06_notifications.md (law 12); 09_events.md pointer updated.

crueber referenced this pull request

2026-09-05 01:47:22 +00:00

[omp major-3] Webhook SSRF: https validation bypassable via redirects #78

crueber added 1 commit

2026-09-05 01:48:58 +00:00