fix(security): gate webhook creation through the public-host validator (#81)#92
Conversation
#81) Webhook creation only scheme-checked the URL, so an authenticated repo owner could register a hook pointed at loopback, private, or metadata endpoints and make the node POST to them on repo events. Route the URL through is_public_http_url, the same validator the peer announce path already uses. Wiring it up surfaced a bypass in that shared validator: it folded IPv4-mapped and compatible IPv6 but not 6to4 or NAT64, so [2002:7f00:1::] (127.0.0.1) slipped through. Fold 6to4 and the NAT64 well-known prefix, reject the NAT64 local-use prefix and the rest of 64:ff9b::/32 outright (the embedded v4 offset varies by prefix length, so a decode would be error-prone), and reject 0.0.0.0/8. The change only ever rejects more, so the peer announce handler, the DB-boundary writer, and the boot-time prune inherit the same hardening. Closes #81.
📝 WalkthroughWalkthrough
ChangesSSRF Hardening: IPv6 Transition Encoding and Webhook URL Gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gitlawb-node/src/api/webhooks.rs`:
- Around line 43-52: The webhook URL validation at registration time only checks
the hostname string, not the actual resolved IP address, which allows
DNS-rebinding attacks where a public hostname resolves to a loopback or private
IP. Add IP address validation at webhook delivery time in the
`client.post(&hook.url)` call in crates/gitlawb-node/src/webhooks.rs around line
85. Before posting the request, resolve the webhook URL's hostname and validate
that the resolved socket address passes the same IP range checks as
`is_public_http_url` (rejecting loopback and private IP ranges). This ensures
that even if DNS resolves to a private address at delivery time, the request is
blocked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: baa9d9df-531c-4309-a8e0-0bdab3469ffc
📒 Files selected for processing (2)
crates/gitlawb-node/src/api/peers.rscrates/gitlawb-node/src/api/webhooks.rs
| // Gate the target through the same hardened public-host validator the peer | ||
| // announce path uses, so an authenticated owner cannot register a webhook | ||
| // that makes the node POST to loopback/private/link-local/metadata | ||
| // endpoints (SSRF). Delivery runs on the shared no-redirect client | ||
| // (main.rs), which closes the 3xx-to-internal bounce. | ||
| if !crate::api::peers::is_public_http_url(&req.url) { | ||
| return Err(AppError::BadRequest( | ||
| "webhook URL must be http:// or https://".into(), | ||
| "webhook URL must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(), | ||
| )); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate webhook delivery code and inspect the reqwest client construction.
fd -t f -e rs | xargs rg -nP -C3 '(redirect|Policy::none|resolve\(|dns_resolver|deliver|webhook)' 2>/dev/null
# Specifically look for any post-resolution IP re-validation in delivery.
rg -nP -C4 'is_public_http_url|to_socket_addrs|lookup_host|resolve' crates/gitlawb-node/src 2>/dev/nullRepository: Gitlawb/node
Length of output: 50369
🏁 Script executed:
# Find is_public_http_url implementation to understand what it validates
rg -A20 'fn is_public_http_url' crates/gitlawb-node/src 2>/dev/null
# Check if there's any custom DNS resolver or socket filtering
rg -n 'dns_resolver|custom_resolver|socket_addr|to_socket_addrs|0\.0\.0\.0|127\.0\.0\.1|private_ip|is_private' crates/gitlawb-node/src/webhooks.rs 2>/dev/null
# Search for any IP validation in webhooks or related code
rg -n 'IpAddr|ip_addr|resolve|lookup' crates/gitlawb-node/src/webhooks.rs 2>/dev/nullRepository: Gitlawb/node
Length of output: 1661
🏁 Script executed:
# Get the complete is_public_http_url function
rg -A30 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs
# Check the full webhooks.rs delivery code for any IP validation
cat -n crates/gitlawb-node/src/webhooks.rs | head -130Repository: Gitlawb/node
Length of output: 5941
🏁 Script executed:
# Get the complete is_public_http_url function - need more lines
rg -A50 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs | head -60Repository: Gitlawb/node
Length of output: 2323
🏁 Script executed:
# Get the complete is_public_http_url function
rg -A80 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs | head -100Repository: Gitlawb/node
Length of output: 3548
Webhook delivery does not validate resolved IP addresses; DNS-rebinding SSRF remains exploitable.
is_public_http_url validates only the URL's hostname literals and suffix patterns (rejecting localhost, .local, .internal), and rejects IP literals in loopback/private ranges. However, a public hostname (e.g., webhook.attacker.example) that resolves to 127.0.0.1 or a private IP passes this gate, since DNS resolution happens at delivery time.
The webhook delivery code in crates/gitlawb-node/src/webhooks.rs line 85 (client.post(&hook.url)) performs no validation of the resolved address. The HTTP client (main.rs) uses Policy::none() to prevent 3xx redirects but has no custom DNS resolver, socket filter, or connect-time IP guards. Add IP address validation at delivery time to validate the resolved socket address matches the URL's validation requirements.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/gitlawb-node/src/api/webhooks.rs` around lines 43 - 52, The webhook
URL validation at registration time only checks the hostname string, not the
actual resolved IP address, which allows DNS-rebinding attacks where a public
hostname resolves to a loopback or private IP. Add IP address validation at
webhook delivery time in the `client.post(&hook.url)` call in
crates/gitlawb-node/src/webhooks.rs around line 85. Before posting the request,
resolve the webhook URL's hostname and validate that the resolved socket address
passes the same IP range checks as `is_public_http_url` (rejecting loopback and
private IP ranges). This ensures that even if DNS resolves to a private address
at delivery time, the request is blocked.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
No findings.
I completed the no-findings challenge gate. The one potential bypass I investigated — IPv6 literals with zone IDs such as http://[::1%25eth0]/ — is blocked at reqwest::Url::parse (returns InvalidIpv6Address), so it cannot reach the validator. Other checked classes (handler wiring regression, boot-time prune over-deletion, NAT64 well-known prefix misidentification) also held up.
@kevincodex1 LGTM
3 participants
Webhook creation accepted any http(s) URL behind a scheme check and never re-validated the host, so any authenticated repo owner could register a webhook at
http://127.0.0.1:<port>/orhttp://169.254.169.254/latest/meta-data/and turn the node's on-event POST into an SSRF probe against loopback, private, or cloud-metadata endpoints. This routes the registration URL throughis_public_http_url, the hardened validator the peer announce path already uses, so the two SSRF surfaces share one definition of "public".The other two prongs the issue raised are already closed on main: the owner check is present (
require_repo_owner, full-and-short DID match), and delivery runs on the shared no-redirect client from #78, so a public URL that 302s inward is not followed.The validator gap this surfaced
Wiring the validator into webhooks exposed a real bypass in it: it folded IPv4-mapped and IPv4-compatible IPv6 but not 6to4 or NAT64, so
http://[2002:7f00:1::]/(which decodes to 127.0.0.1) sailed past. The webhook gate is only as strong as the validator behind it, so this hardens the shared function rather than the webhook path alone:2002::/16) and the NAT64 well-known prefix (64:ff9b::/96) down to their embedded v4 and re-run the range checks.64:ff9b:1::/48, RFC 8215) and the rest of64:ff9b::/32outright. The embedded v4 sits at a prefix-length-dependent offset (RFC 6052 §2.2), so attempting to decode every variant is error-prone; refusing the block is the safe call, and a NAT64 literal is never a legitimate webhook or peer target.0.0.0.0/8"this host" block.The change is tightening-only (it never accepts an address it previously rejected), so all four callers of the validator move in the same direction: the webhook gate, the peer announce handler, the DB-boundary
upsert_peerwriter, and the boot-time prune. A 6to4 or NAT64 literal that wraps a genuinely public v4 still passes, so no legitimate peer or webhook is rejected or pruned.Tests
Extended the peer validator's reject and accept sets for the 6to4 / NAT64 / local-use /
0.0.0.0/8cases, and added a webhook-site regression test pinning issue #81's exact exploit URLs so the gate cannot regress to the old scheme-only check.cargo test -p gitlawb-nodeis green (172 tests), clippy clean.Out of scope
Noted for separate follow-up, not addressed here: DNS rebinding between registration and delivery (accepted residual, same posture as the peer path), and three pre-existing issues this review surfaced: the gossip task building a redirect-following client, unauthenticated
list_webhooksexposing webhook target URLs, and the validator's lack of a port restriction.Closes #81.
Summary by CodeRabbit
Bug Fixes
Tests