Skip to content

feat(api-service): improve url handling for webhook requests#10912

Merged
scopsy merged 3 commits intonextfrom
cursor/application-security-review-a728
Apr 29, 2026
Merged

feat(api-service): improve url handling for webhook requests#10912
scopsy merged 3 commits intonextfrom
cursor/application-security-review-a728

Conversation

@scopsy
Copy link
Copy Markdown
Contributor

@scopsy scopsy commented Apr 28, 2026

What changed? Why was the change needed?

This PR fixes two medium-severity Server-Side Request Forgery (SSRF) vulnerabilities. The change was needed to prevent attackers from using the inbound email reply webhook or the chat webhook provider to make unauthorized requests to internal or restricted network resources. SSRF validation is now applied to all outbound URLs in these flows.

What changed

This PR fixes two medium-severity Server-Side Request Forgery (SSRF) issues by normalizing and validating outbound webhook URLs before performing HTTP requests. The inbound email reply callback and chat webhook flows now normalize scheme-less URLs and run SSRF checks (including DNS resolution and IP classification) to block requests to localhost, metadata endpoints, or private/reserved IP ranges.

Affected areas

worker: The reply-to strategy now normalizes callback URLs and rejects or blocks requests with invalid formats or SSRF validation errors before calling out.

providers: The chat webhook provider now normalizes and validates target webhook URLs and uses the validated URL for outbound POSTs.

shared: Added SSRF utilities (normalizeOutboundHttpUrl, validateUrlSsrf) with DNS resolution caching and IP-range checks, and exported them via a new @novu/shared/utils/ssrf-url-validation subpath.

application-generic: Duplicated the SSRF utilities here to avoid package dependency/coupling issues that prevent consuming the shared implementation.

test: Updated inbound-email-parse fixtures/specs to use fully-qualified HTTPS webhook URLs so DNS resolution succeeds in CI.

Key technical decisions

  • SSRF validation logic is duplicated in packages/shared and libs/application-generic to avoid dependency/coupling problems.
  • A dedicated @novu/shared/utils/ssrf-url-validation export prevents Node.js-specific code (DNS/IP checks) from being pulled into browser bundles.
  • Added an LRU-backed DNS cache dependency to limit repeated lookups and improve performance.
  • Scheme-less webhook inputs are normalized to https://... to preserve backward compatibility while enforcing SSRF checks.

Testing

Updated unit/spec fixtures (inbound-email-parse) to use scheme-qualified HTTPS URLs; no new unit tests were added for the SSRF utilities in this PR. Manual/CI verification relies on the updated specs and existing test runs.

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

  • The validateUrlSsrf utility logic is duplicated in packages/shared and libs/application-generic due to package dependency constraints.
  • The @novu/shared/utils/ssrf-url-validation export is a dedicated entry to avoid pulling Node.js-specific code into browser bundles.

Slack Thread

Open in Web Open in Cursor 

Run validateUrlSsrf before reply-to parse webhooks and chat webhook POSTs.
Expose the helper from @novu/shared/utils/ssrf-url-validation for providers
and keep the same logic in application-generic for existing callers.

Co-authored-by: Dima Grossman <dima@grossman.io>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 28, 2026

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit b47f462
🔍 Latest deploy log https://app.netlify.com/projects/dashboard-v2-novu-staging/deploys/69f0ab0d69195b00091b610d

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

Hey there and thank you for opening this pull request! 👋

We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted.

Your PR title is: feat(api-service): improve url handling for webhook requests

Requirements:

  1. Follow the Conventional Commits specification
  2. As a team member, include Linear ticket ID at the end: fixes TICKET-ID or include it in your branch name

Expected format: feat(scope): Add fancy new feature fixes NOV-123

Details:

PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name

@scopsy scopsy changed the title Application security review feat(api-service): improve url handling for webhook requests Apr 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

Adds URL normalization and SSRF validation utilities and applies them to outbound HTTP flows (reply-callback and chat webhook), causing target URLs to be normalized and rejected when format, DNS resolution, or resolved IPs indicate SSRF risks.

Changes

Cohort / File(s) Summary
Shared SSRF utilities
packages/shared/src/utils/ssrf-url-validation.ts, packages/shared/package.json
Adds normalizeOutboundHttpUrl and validateUrlSsrf, DNS resolution with LRU cache, private/reserved IP detection, package export entry, and lru-cache dependency.
Application helper shim
libs/application-generic/src/utils/ssrf-url-validation.ts
Adds normalizeOutboundHttpUrl helper that mirrors shared normalization behavior for outbound webhook-style inputs.
Worker: reply-to strategy
apps/worker/src/app/workflow/usecases/inbound-email-parse/strategies/reply-to.strategy.ts
Normalize compiled reply-callback URL, run validateUrlSsrf, abort with SSRF-specific error messages on failure, and use the validated URL for axios.post.
Chat webhook provider
packages/providers/src/lib/chat/chat-webhook/chat-webhook.provider.ts
Normalize and validate webhook targetUrl via normalizeOutboundHttpUrl/validateUrlSsrf before axios.post; throw detailed SSRF blocking errors when validation fails.
Tests updated
apps/worker/src/app/workflow/specs/inbound-email-parse.spec.ts
Fixtures and assertions updated to use fully-qualified webhook URLs (include scheme) for reply-callback expectations.

Sequence Diagram

sequenceDiagram
    participant Component as Component (Worker / ChatProvider)
    participant Normalizer as normalizeOutboundHttpUrl
    participant Validator as validateUrlSsrf
    participant DNSCache as LRU Cache
    participant DNSResolver as DNS Resolver
    participant IPChecker as IP Range Validator

    Component->>Normalizer: normalizeOutboundHttpUrl(rawUrl)
    alt Invalid format
        Normalizer-->>Component: null
        Component-->>Component: Abort with "Invalid URL format" SSRF error
    else Valid format
        Normalizer-->>Component: normalizedUrl
        Component->>Validator: validateUrlSsrf(normalizedUrl)
        Validator->>Validator: parse URL & check host blocklist
        Validator->>DNSCache: lookup(hostname)
        alt Cache hit
            DNSCache-->>Validator: cached IPs
        else Cache miss
            Validator->>DNSResolver: resolve(hostname)
            DNSResolver-->>Validator: IPs
            Validator->>DNSCache: store(hostname, IPs)
        end
        Validator->>IPChecker: check(IPs)
        alt Private/reserved IP found
            IPChecker-->>Validator: violation
            Validator-->>Component: Error (SSRF blocked)
        else All public IPs
            IPChecker-->>Validator: ok
            Validator-->>Component: null (allowed)
            Component->>Component: axios.post(validatedUrl, ...)
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title uses a valid Conventional Commits format with type 'feat' and scope 'api-service', but 'api-service' is not in the allowed scopes list; the changes actually span multiple packages (shared, application-generic, worker, providers) which should use comma-separated scopes or be categorized differently. Update the title to use valid scopes from the requirements (e.g., 'feat(shared,application-generic,worker,providers): improve url handling for webhook requests') and consider whether the scope accurately reflects the primary changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/shared/src/utils/ssrf-url-validation.ts`:
- Around line 36-74: validateUrlSsrf currently only checks a preflight DNS
lookup which can be rebinding-bypassed; after resolving in validateUrlSsrf
(where DNS_CACHE and isPrivateIp are used) pin the validated address(es) for the
actual HTTP request by returning or storing the resolved IP(s) and ensure the
outbound client uses that IP with the original hostname set in the Host header
(or use the client's custom DNS lookup hook to re-check and enforce isPrivateIp
during connect), so change validateUrlSsrf to return the safe resolved IP(s) (or
attach them to the request context) and update the HTTP client to connect to
those IPs while preserving the original hostname.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ffa49bf8-5e40-4138-ba59-18b04559fca9

📥 Commits

Reviewing files that changed from the base of the PR and between c3c40db and 4d56d9d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • apps/worker/src/app/workflow/usecases/inbound-email-parse/strategies/reply-to.strategy.ts
  • libs/application-generic/src/utils/ssrf-url-validation.ts
  • packages/providers/src/lib/chat/chat-webhook/chat-webhook.provider.ts
  • packages/shared/package.json
  • packages/shared/src/utils/ssrf-url-validation.ts

Comment thread packages/shared/src/utils/ssrf-url-validation.ts
… SSRF

Align outbound URLs with axios (prepend https when missing) so existing
templates keep working. Update inbound-email-parse spec to use https://example.com
so DNS resolution succeeds in CI.

Co-authored-by: Dima Grossman <dima@grossman.io>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/shared/src/utils/ssrf-url-validation.ts (1)

95-110: ⚠️ Potential issue | 🔴 Critical

Preflight DNS validation is still vulnerable to DNS rebinding.

This validates one DNS answer, but outbound clients can resolve again at connect time and hit a different (private) IP. Please enforce the private-IP check in the HTTP client lookup hook or pin the validated IP for the actual connection.

#!/bin/bash
set -euo pipefail

echo "Find validateUrlSsrf call sites:"
rg -n --type=ts -C4 '\bvalidateUrlSsrf\s*\('

echo
echo "Find outbound HTTP calls (axios/request wrappers):"
rg -n --type=ts -C6 '\baxios\.(request|get|post|put|patch|delete)\s*\(|\bfetch\s*\('

echo
echo "Check for DNS pinning / custom lookup hooks / custom agents:"
rg -n --type=ts -C4 '\blookup\s*:|\bhttpAgent\b|\bhttpsAgent\b|\bAgent\('

If call sites do not bind connect-time resolution to validated addresses (or re-check inside lookup), this issue is confirmed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/utils/ssrf-url-validation.ts` around lines 95 - 110, The
current validateUrlSsrf flow (DNS_CACHE, hostname resolution using
dns.promises.lookup and isPrivateIp) only checks preflight answers and is
vulnerable to DNS rebinding; update callers of validateUrlSsrf (or the HTTP
client wrappers) to either pin the resolved IPs for the actual connection or
perform the private-IP check again in the HTTP client's lookup/agent hook (e.g.,
custom lookup passed to axios/http(s).Agent or fetch wrapper) so that the
connection uses the validated address list (or rejects connections to any
address that fails isPrivateIp) instead of performing a fresh, unchecked
resolution at connect time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/shared/src/utils/ssrf-url-validation.ts`:
- Around line 49-65: The IPv6 private/reserved checks in isPrivateIp are
incomplete—update the privateRanges array (in function isPrivateIp) to block the
full fc00::/7 and fe80::/10 ranges and their IPv4-mapped forms by adding regexes
that match both fc00..fdff (e.g. /^f[cd]/i or more precise /^f[cd][0-9a-f]/i)
and fe80–febf (e.g. /^fe(?:8|9|a|b)/i), and also add corresponding ::ffff
variants like /^::ffff:f[cd]/i and /^::ffff:fe(?:8|9|a|b)/i so private IPv6
addresses cannot bypass the filter.

---

Duplicate comments:
In `@packages/shared/src/utils/ssrf-url-validation.ts`:
- Around line 95-110: The current validateUrlSsrf flow (DNS_CACHE, hostname
resolution using dns.promises.lookup and isPrivateIp) only checks preflight
answers and is vulnerable to DNS rebinding; update callers of validateUrlSsrf
(or the HTTP client wrappers) to either pin the resolved IPs for the actual
connection or perform the private-IP check again in the HTTP client's
lookup/agent hook (e.g., custom lookup passed to axios/http(s).Agent or fetch
wrapper) so that the connection uses the validated address list (or rejects
connections to any address that fails isPrivateIp) instead of performing a
fresh, unchecked resolution at connect time.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d910ee6f-9b82-4002-9829-6785b549366e

📥 Commits

Reviewing files that changed from the base of the PR and between 4d56d9d and e789348.

📒 Files selected for processing (5)
  • apps/worker/src/app/workflow/specs/inbound-email-parse.spec.ts
  • apps/worker/src/app/workflow/usecases/inbound-email-parse/strategies/reply-to.strategy.ts
  • libs/application-generic/src/utils/ssrf-url-validation.ts
  • packages/providers/src/lib/chat/chat-webhook/chat-webhook.provider.ts
  • packages/shared/src/utils/ssrf-url-validation.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/worker/src/app/workflow/specs/inbound-email-parse.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • libs/application-generic/src/utils/ssrf-url-validation.ts
  • apps/worker/src/app/workflow/usecases/inbound-email-parse/strategies/reply-to.strategy.ts
  • packages/providers/src/lib/chat/chat-webhook/chat-webhook.provider.ts

Comment thread packages/shared/src/utils/ssrf-url-validation.ts
Co-authored-by: Dima Grossman <dima@grossman.io>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
packages/shared/src/utils/ssrf-url-validation.ts (2)

93-97: Consider adding Alibaba Cloud metadata IP to blocked ranges.

The current blocklist covers major cloud provider metadata endpoints. However, Alibaba Cloud uses 100.100.100.200 for its metadata service, which isn't currently caught by isPrivateIp. If Alibaba Cloud is a deployment target, consider adding /^100\.100\.100\.200$/ to the private ranges.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/utils/ssrf-url-validation.ts` around lines 93 - 97, The
check currently blocks common metadata hostnames but misses Alibaba Cloud's
metadata IP; update the private-IP detection used by isPrivateIp by adding a
literal regex for 100.100.100.200 (e.g. /^100\.100\.100\.200$/) to the
array/object of private ranges that isPrivateIp consults (or, if you prefer,
also add '100.100.100.200' to blockedHostnames), so requests to that Alibaba
metadata endpoint are treated as private and rejected.

49-72: Consider blocking additional reserved/documentation ranges.

The current list covers the most critical private ranges. For more comprehensive SSRF protection, consider also blocking:

  • 255.255.255.255 (broadcast)
  • 100.64.0.0/10 (Carrier-grade NAT)
  • Documentation ranges: 192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24, 2001:db8::/32

These are lower priority since they're less likely to be exploitable, but could be relevant depending on your infrastructure.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/utils/ssrf-url-validation.ts` around lines 49 - 72,
Update isPrivateIp to also block IPv4 broadcast, CGNAT and documentation ranges
by adding regex entries to the privateRanges array: add /^255\.255\.255\.255$/
to catch broadcast, add a regex for
100\.6[4-9]\.|100\.(7[0-9]|[89][0-9]|1[0-9]{2})?/... (i.e., 100.64.0.0/10) to
cover 100.64.0.0–100.127.255.255, add /^192\.0\.2\./, /^198\.51\.100\./,
/^203\.0\.113\./ for the IANA documentation IPv4 blocks, and add an IPv6 regex
for /^2001:db8:/i (2001:db8::/32); ensure these new regex patterns follow the
same case-insensitive style and are included in the same privateRanges used by
isPrivateIp.
libs/application-generic/src/utils/ssrf-url-validation.ts (1)

4-4: Consider automating sync verification between duplicated files.

Since this file must stay in sync with packages/shared/src/utils/ssrf-url-validation.ts due to dependency constraints, consider adding a CI check or pre-commit hook that compares the two files and fails if they diverge. This prevents security-relevant drift.

A simple approach:

diff packages/shared/src/utils/ssrf-url-validation.ts libs/application-generic/src/utils/ssrf-url-validation.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/application-generic/src/utils/ssrf-url-validation.ts` at line 4, The
duplicated ssrf-url-validation.ts file can drift; add an automated sync
verification by creating a script (e.g., verify-ssrf-sync) that diffs the two
ssrf-url-validation.ts copies and exits non-zero on any difference, then wire
that script into CI (and optionally a pre-commit hook) so the CI job runs the
script and fails the build when the files diverge; update project scripts
(package.json) and the CI workflow to call verify-ssrf-sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@libs/application-generic/src/utils/ssrf-url-validation.ts`:
- Line 4: The duplicated ssrf-url-validation.ts file can drift; add an automated
sync verification by creating a script (e.g., verify-ssrf-sync) that diffs the
two ssrf-url-validation.ts copies and exits non-zero on any difference, then
wire that script into CI (and optionally a pre-commit hook) so the CI job runs
the script and fails the build when the files diverge; update project scripts
(package.json) and the CI workflow to call verify-ssrf-sync.

In `@packages/shared/src/utils/ssrf-url-validation.ts`:
- Around line 93-97: The check currently blocks common metadata hostnames but
misses Alibaba Cloud's metadata IP; update the private-IP detection used by
isPrivateIp by adding a literal regex for 100.100.100.200 (e.g.
/^100\.100\.100\.200$/) to the array/object of private ranges that isPrivateIp
consults (or, if you prefer, also add '100.100.100.200' to blockedHostnames), so
requests to that Alibaba metadata endpoint are treated as private and rejected.
- Around line 49-72: Update isPrivateIp to also block IPv4 broadcast, CGNAT and
documentation ranges by adding regex entries to the privateRanges array: add
/^255\.255\.255\.255$/ to catch broadcast, add a regex for
100\.6[4-9]\.|100\.(7[0-9]|[89][0-9]|1[0-9]{2})?/... (i.e., 100.64.0.0/10) to
cover 100.64.0.0–100.127.255.255, add /^192\.0\.2\./, /^198\.51\.100\./,
/^203\.0\.113\./ for the IANA documentation IPv4 blocks, and add an IPv6 regex
for /^2001:db8:/i (2001:db8::/32); ensure these new regex patterns follow the
same case-insensitive style and are included in the same privateRanges used by
isPrivateIp.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a274a93d-a859-4310-ad9e-fd64cdd76982

📥 Commits

Reviewing files that changed from the base of the PR and between e789348 and b47f462.

📒 Files selected for processing (2)
  • libs/application-generic/src/utils/ssrf-url-validation.ts
  • packages/shared/src/utils/ssrf-url-validation.ts

@scopsy scopsy merged commit 9a17eef into next Apr 29, 2026
37 checks passed
@scopsy scopsy deleted the cursor/application-security-review-a728 branch April 29, 2026 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants