Repository navigation
fix(webhooks): constrain subscription URLs against SSRF and add webhook unit tests (#280, #170) - #301
Open
ocakciomursel-pixel wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #280
Closes #170
This PR hardens the Webhook infrastructure against Server-Side Request Forgery (SSRF) across both subscription registration/update and outbound delivery (#280), and adds full unit test coverage for all services in
src/infrastructure/webhooks/services/(#170).1. Webhook SSRF & Outbound Delivery Hardening (#280)
WebhookUrlValidatorService(src/infrastructure/webhooks/services/webhook-url-validator.service.ts):https:by default; permitshttp:only inNODE_ENV=developmentor whenWEBHOOK_ALLOW_INSECURE_HTTP=trueoutside production. Rejectsfile:,gopher:,ftp:,data:, and URLs with embedded credentials.443, plus80whenhttp:is allowed) and optional custom ports viaWEBHOOK_ALLOWED_PORTS.localhost,*.localhost,*.local,*.internal,metadata.google.internal), obfuscated IPv4 encodings (short-form127.1, integer2130706433, hex0x7f000001, octal0177.0.0.1), and private/reserved IPv4 & IPv6 ranges (loopback127.0.0.0/8,::1/128, RFC 191810.0.0.0/8,172.16.0.0/12,192.168.0.0/16, link-local & cloud metadata169.254.0.0/16/169.254.169.254, CGNAT100.64.0.0/10, IPv4-mapped/translated IPv6::ffff:0:0/96, unique-localfc00::/7, link-localfe80::/10).dns.promises.lookup(..., { all: true, verbatim: true })and rejects endpoints if any resolvedA/AAAArecord falls in a blocked range.WebhookSubscriptionService):dto.urlon bothcreate()andupdate(), and avoids logging raw user-supplied URLs.WebhookDeliveryService,WebhookController):subscription.urlimmediately before each delivery attempt and pins the resolved IP on the outboundhttp.Agent/https.Agentlookupcallback to defeat DNS rebinding.maxRedirects: 0) and treats3xxresponses as delivery failures rather than following redirects to internal endpoints.subscription.rateLimitPerMinutesliding 60s window).retryDeadLetter/getMetricsto the authenticated user.2. Webhook Unit Test Suite (#170)
Added colocated unit specs under
src/infrastructure/webhooks/services/and wiredsrc/infrastructure/webhooksinto.github/workflows/build-check.yml:webhook-url-validator.service.spec.tswebhook-hmac.service.spec.tswebhook-subscription.service.spec.tswebhook-delivery.service.spec.tswebhook-event.service.spec.tswebhook-processor.service.spec.ts