Skip to content

fix(webhook): block SSRF to internal addresses in callback URL validation#43

Merged
ogazboiz merged 1 commit into
LabsCrypt:mainfrom
System625:fix/issue-3-webhook-ssrf-guard
Jul 1, 2026
Merged

fix(webhook): block SSRF to internal addresses in callback URL validation#43
ogazboiz merged 1 commit into
LabsCrypt:mainfrom
System625:fix/issue-3-webhook-ssrf-guard

Conversation

@System625

Copy link
Copy Markdown
Contributor

Summary

Closes #3. Webhook callback URL registration previously only checked that the
URL parsed and used http/https, letting an authenticated caller point the
indexer at internal services or cloud metadata endpoints (SSRF).

Changes

  • Add src/utils/ssrfGuard.ts with assertCallbackUrlAllowed() — validates the
    scheme, resolves the host via DNS, and rejects any address in a
    loopback, link-local (incl. 169.254.169.254), RFC1918, CGNAT, or other
    reserved range (IPv4 and IPv6, including IPv4-mapped).
  • Enforce the guard at registration time in indexerController.ts (returns
    400 on a blocked URL).
  • Re-validate the resolved address at delivery time in webhookService.ts
    (postWebhook), defending against DNS rebinding on both initial dispatch and
    retries.
  • Document the allowed/blocked policy in docs/webhook-url-policy.md.

Tests

  • New src/utils/__tests__/ssrfGuard.test.ts covers 127.0.0.1,
    169.254.169.254, a 10.x host, IPv6, public addresses, and multi-address
    resolution.
  • Existing webhook delivery tests mock the guard to avoid real DNS.
  • npm test -- ssrfGuard webhookService → 33/33 passing; npm run lint → 0
    errors.

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSRF guard is solid. DNS resolution at both registration and delivery time covers the DNS rebinding vector. Comprehensive IP range checks for IPv4 and IPv6 (including IPv4-mapped). Tests cover the key scenarios. Docs are clear. LGTM.

@ogazboiz

Copy link
Copy Markdown
Contributor

This PR has been approved but has merge conflicts with the base branch. Please rebase your branch on the latest main and push to resolve the conflicts. Once clean, we can merge.

@System625 System625 force-pushed the fix/issue-3-webhook-ssrf-guard branch from 347bbd6 to 235f7aa Compare June 30, 2026 08:57
@System625

Copy link
Copy Markdown
Contributor Author

Hi @ogazboiz I've rebased on the latest main and resolved the conflict in indexerController.ts. All good to merge

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-confirming after your rebase: the SSRF guard is still solid (DNS resolution at both registration and delivery, full IPv4/IPv6 range checks incl IPv4-mapped) and the indexerController conflict resolution is clean. note: the repo's ci is currently red for reasons unrelated to your pr (a separate lint/typecheck/test mess on main from earlier bad merges, i'm cleaning it up separately) so i'm merging on the approved logic + mergeable state. thanks for the solid work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Backend] Webhook subscriptions accept any URL, enabling SSRF to internal services

2 participants