Skip to content

fix(webhooks): reject internal and metadata addresses in webhook delivery - #467

Open
polylane[bot] wants to merge 1 commit into
mainfrom
polylane/autofix/4p0jdtvo2jc3
Open

polylane[bot] wants to merge 1 commit into
mainfrom
polylane/autofix/4p0jdtvo2jc3

Conversation

@polylane

@polylane polylane Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes: Webhook delivery calls tenant URLs without an SSRF check

Any team member could save a webhook destination on an internal address. The delivery worker sent the signed payload to that address, and the tenant could read the response back from the webhook call record. Once this merges, an internal, loopback, link-local, or metadata destination is no longer usable for a webhook.

flowchart LR
  A["Tenant saves a webhook URL"] --> B["createWebhook / updateWebhook"]
  B --> C["assertWebhookUrlAllowed"]
  C -->|blocked| D["BAD_REQUEST to the tenant"]
  C -->|allowed| E["db.webhook.url stored"]
  E --> F["Worker: postWebhook"]
  F --> G["assertWebhookUrlAllowed"]
  G -->|blocked| H["failed call, target not contacted"]
  G -->|allowed| I["fetch POST to the public endpoint"]
Loading
Detection evidence

Affected: int_13ee3c91c001c2c8xfdj06a9

Why this fix

A unit test drives the real delivery worker at a loopback webhook target, and the worker contacted it, while the repository docs already require a publicly accessible endpoint. The worker now resolves the host and refuses a private, loopback, link-local, or metadata address before it sends, and the save path refuses the same addresses so the tenant gets a clear error instead of a silently failing webhook. The residual risk is a DNS answer that changes between the save check and the send check, because the resolved address is not pinned.

Fix walkthrough

Watch the fix walkthrough

Watch the fix explained (2:33). Recorded for commit 1358dc55696f684d948c8e55b429249404c0dcce.

Out of scope
  • SesSettingsService.createSesSetting fetches an admin-supplied URL without this guard. Its caller is the platform-admin procedure, a different boundary, and it is recorded on the incident timeline.
  • Pinning the resolved address against a DNS answer that changes between the check and the request.
Causal chain
  • Signal (alert): Tenant-controlled webhook URL fetched server-side without SSRF protection in useSend
  • Surfacing site: usesend/useSend at apps/web/src/server/service/webhook-service.ts#postWebhook
  • Mechanism: postWebhook calls fetch with the URL stored in db.webhook, and no code checks the scheme, the host, or the resolved address. The URL comes from any team member, not an admin.
  • Producer: useSend webhook delivery worker (postWebhook), at usesend/useSend:apps/web/src/server/service/webhook-service.ts#postWebhook
  • Trigger: A team member saves a webhook URL with an internal or link-local host, then sends a test event or a real event.
  • What happens to the failed unit today: postWebhook sets redirect: "manual", so a redirect to an internal host is not followed. BullMQ retries a failed call up to six times with exponential backoff and disables the webhook after thirty consecutive failures.
  • Blast radius: unknown other resource(s), unknown other tenant(s); data at risk: Internal service responses, and any cloud instance metadata that the deployment network exposes to the worker.
  • Producer evidence:
    • Reproducer in apps/web/src/server/service/webhook-url-guard.unit.test.ts: a webhook URL of http://127.0.0.1:/internal made the captured worker handler contact a loopback server once before the change and zero times after it.
    • apps/web/src/server/service/webhook-service.ts#postWebhook calls fetch(params.url, { method: "POST", redirect: "manual", signal }) with no address check.
    • apps/web/src/server/api/routers/webhook.ts validates the URL with z.string().url() only, which accepts http://169.254.169.254/latest/meta-data/.
    • apps/web/src/server/api/trpc.ts teamProcedure requires team membership only, so any team member can set the URL.
4 files changed (+624/-0)
  • apps/web/src/server/service/webhook-service.ts: modified, +26/-0
  • apps/web/src/server/service/webhook-service.unit.test.ts: modified, +6/-0
  • apps/web/src/server/service/webhook-url-guard.ts: added, +250/-0
  • apps/web/src/server/service/webhook-url-guard.unit.test.ts: added, +342/-0

Repository lint: pnpm run lint (declared in package.json) fails over the whole repository (the declared target takes no file arguments), so the failure is not attributed to this change and did not block it; run it before merging.

Lint output
r read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
@usesend/ui:lint: $ eslint . --max-warnings 0
usesend-js:lint: cache miss, executing 2b48c7c6e12d1991
usesend-js:lint: [WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
usesend-js:lint: $ eslint . --max-warnings 0
usesend-js:lint: sh: 1: eslint: not found
usesend-js:lint: [ELIFECYCLE] Command failed.
@usesend/ui:lint: [ELIFECYCLE] Command failed.

 Tasks:    1 successful, 3 total
Cached:    0 cached, 3 total
  Time:    3.988s 
Failed:    usesend-js#lint

[ELIFECYCLE] Command failed with exit code 1.
[WARN] The "pnpm" field in package.json is no longer read by pnpm. The following keys were ignored: "pnpm.overrides". See https://pnpm.io/settings for the new home of each setting.
$ turbo lint

Attention:
Turborepo now collects completely anonymous telemetry regarding usage.
This information is used to shape the Turborepo roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://turborepo.dev/docs/telemetry

• turbo 2.10.4
usesend-js#lint:  ERROR  command (/workspace/usesend/useSend/packages/sdk) /root/.local/share/pnpm/.tools/pnpm/11.11.0/bin/pnpm run lint exited (1)
 ERROR  run failed: command  exited (1)

View thread View autofix


Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.


Summary by cubic

Rejects webhook destinations that resolve to internal, loopback, link-local, or cloud metadata addresses so the delivery worker no longer sends signed payloads to tenant controlled internal hosts.

  • Validates the webhook URL on create and update, returning a BAD_REQUEST to the tenant instead of storing a silently failing webhook.
  • Re-checks the resolved address in the delivery worker right before sending, and fails the call without contacting the target.
  • Uses a new assertWebhookUrlAllowed guard that resolves hostnames and rejects if any returned address is non-public.
  • The residual risk is a DNS answer that changes after the check, since the resolved address is not pinned.

Written for commit 1358dc5. Summary will update on new commits.

View guided diff Turn on auto-fix

…very

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot added polylane severity:medium Polylane autofix severity: medium labels Oct 16, 2026
@polylane
polylane Bot requested a review from KMKoushik October 16, 2026 05:40
@vercel

vercel Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
unsend-marketing Ready Ready Preview Oct 16, 2026 5:41am UTC

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f39359b5-6e02-4bdb-968c-38583f5b85ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying usesend with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1358dc5
Status: ✅  Deploy successful!
Preview URL: https://86e96378.usesend.pages.dev
Branch Preview URL: https://polylane-autofix-4p0jdtvo2jc.usesend.pages.dev

View logs

@polylane polylane Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested observability improvements for code this pull request changes.

try {
await assertWebhookUrlAllowed(url);
} catch (error) {
if (error instanceof WebhookUrlNotAllowedError) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Log each refused non-public webhook URL so an operator can count SSRF attempts at the save path.

Suggested change
if (error instanceof WebhookUrlNotAllowedError) {
if (error instanceof WebhookUrlNotAllowedError) {
logger.warn(
{ reason: error.message },
"[WebhookService]: Rejected non-public webhook URL",
);

@polylane

polylane Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Tip

Expected to resolve the linked issue.

Linked issue: Webhook delivery calls tenant URLs without an SSRF check

Checked the unsend production service. 330 webhook deliveries in 72h, all HTTP 200, no failures. The guard adds one DNS resolve before the existing fetch, and CPU stayed under 0.05 vCPU of a 32 vCPU limit.

View the full analysis →

Also considered · 1 plausible, 3 refuted
  • Refuted · The guard's DNS resolve turns a transient resolver failure into a failed delivery · The guard adds no new failure mode: the fetch below it performs the same hostname resolution and fails the same way.
  • Refuted · One extra DNS resolve per delivery regresses webhook throughput at concurrency 25 · Deliveries run at about 4.6 per hour (330 in 72h).
  • Plausible · A stored webhook that points at a non-public address stops delivering and auto-disables · unlikely, low
  • Refuted · The new node:dns import breaks the Next.js build or an edge route · No runtime = "edge" declaration exists anywhere in apps/web/src, and every importer of webhook-service is under src/server.

unsend · CPU usage

Analysed against 1 Service and 1 repository

View in Polylane Disable reviews

Polylane analysed 1358dc5 for production impact. You can ask follow-ups by mentioning @polylane in a comment.

Rate this review: 👍 helpful · 👎 not helpful · 😕 confusing

This branch was successfully deployed

1 active deployment
Preview — 1358dc55 Deployed Oct 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

polylane severity:medium Polylane autofix severity: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant