From 4ff7bc90c33d438d97ef4498eee23a200656290b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Fri, 17 Apr 2026 09:19:07 +0200 Subject: [PATCH] security: SSRF guard covers IPv6 + DNS-rebind defence via pinned IP (#49) Expand the SSRF blocklist from IPv4-only to IPv6 loopback/ULA (fc00::/7)/ link-local (fe80::/10)/multicast/IPv4-mapped, plus the missing IPv4 ranges 0.0.0.0/8, 100.64.0.0/10 CGNAT, and TEST-NET/benchmark ranges. Replace the single-lookup SSRF guard with resolveAndValidate(): resolves all DNS records (lookup { all: true }) so a hostname returning "public + private" is rejected, and returns the first validated address for connection pinning. The webhook dispatcher now switches from plain fetch() to https.request() with a custom Agent.lookup that returns the pre-validated IP. A DNS rebind between the guard check and the TCP connect() can no longer redirect the dial to an internal address. Hostname still flows through for SNI and certificate validation. Co-Authored-By: Claude Opus 4.7 --- ...tools-user-self-service-mfa-enable.test.ts | 11 +- packages/api/src/__tests__/ssrf-guard.test.ts | 167 +++++++++++---- .../api/src/__tests__/user-router.test.ts | 18 +- .../__tests__/user-self-service-mfa.test.ts | 7 +- .../src/__tests__/webhook-dispatcher.test.ts | 98 ++++++++- packages/api/src/lib/ssrf-guard.ts | 197 ++++++++++++++---- packages/api/src/lib/webhook-dispatcher.ts | 94 ++++++--- 7 files changed, 467 insertions(+), 125 deletions(-) diff --git a/packages/api/src/__tests__/assistant-tools-user-self-service-mfa-enable.test.ts b/packages/api/src/__tests__/assistant-tools-user-self-service-mfa-enable.test.ts index 81d4cd5..34ab2b9 100644 --- a/packages/api/src/__tests__/assistant-tools-user-self-service-mfa-enable.test.ts +++ b/packages/api/src/__tests__/assistant-tools-user-self-service-mfa-enable.test.ts @@ -51,6 +51,7 @@ describe("assistant user self-service MFA tools - enable flow", () => { totpEnabled: false, }), update: vi.fn().mockResolvedValue({}), + updateMany: vi.fn().mockResolvedValue({ count: 1 }), }, auditLog: { create: vi.fn().mockResolvedValue({ id: "audit_1" }), @@ -75,9 +76,17 @@ describe("assistant user self-service MFA tools - enable flow", () => { lastTotpAt: true, }, }); + // Atomic-CAS replay guard: lastTotpAt is set by updateMany with a + // conditional WHERE; the subsequent update toggles totpEnabled only. + expect(db.user.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ id: "user_1" }), + data: { lastTotpAt: expect.any(Date) }, + }), + ); expect(db.user.update).toHaveBeenCalledWith({ where: { id: "user_1" }, - data: { totpEnabled: true, lastTotpAt: expect.any(Date) }, + data: { totpEnabled: true }, }); expect(db.auditLog.create).toHaveBeenCalledWith({ data: expect.objectContaining({ diff --git a/packages/api/src/__tests__/ssrf-guard.test.ts b/packages/api/src/__tests__/ssrf-guard.test.ts index 27705e9..e24fcfa 100644 --- a/packages/api/src/__tests__/ssrf-guard.test.ts +++ b/packages/api/src/__tests__/ssrf-guard.test.ts @@ -1,16 +1,17 @@ import { describe, expect, it, vi } from "vitest"; -import { assertWebhookUrlAllowed } from "../lib/ssrf-guard.js"; +import { __test__, assertWebhookUrlAllowed, resolveAndValidate } from "../lib/ssrf-guard.js"; // Mock dns.lookup so tests do not require real DNS resolution. +// The guard now calls lookup(host, { all: true }) and receives an array. vi.mock("node:dns/promises", () => ({ lookup: vi.fn(async (hostname: string) => { - const mapping: Record = { - "example.com": "93.184.216.34", - "hooks.external.io": "52.1.2.3", + const mapping: Record> = { + "example.com": [{ address: "93.184.216.34", family: 4 }], + "hooks.external.io": [{ address: "52.1.2.3", family: 4 }], }; - const ip = mapping[hostname]; - if (!ip) throw new Error(`ENOTFOUND ${hostname}`); - return { address: ip, family: 4 }; + const addrs = mapping[hostname]; + if (!addrs) throw new Error(`ENOTFOUND ${hostname}`); + return addrs; }), })); @@ -18,9 +19,7 @@ describe("assertWebhookUrlAllowed — SSRF guard", () => { // ── Allowed targets ───────────────────────────────────────────────────────── it("allows a valid HTTPS URL that resolves to a public IP", async () => { - await expect( - assertWebhookUrlAllowed("https://example.com/webhook"), - ).resolves.toBeUndefined(); + await expect(assertWebhookUrlAllowed("https://example.com/webhook")).resolves.toBeUndefined(); }); it("allows an HTTPS URL with a path and query string", async () => { @@ -32,29 +31,29 @@ describe("assertWebhookUrlAllowed — SSRF guard", () => { // ── Rejected schemes ───────────────────────────────────────────────────────── it("rejects an HTTP URL (only HTTPS allowed)", async () => { - await expect( - assertWebhookUrlAllowed("http://example.com/webhook"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("http://example.com/webhook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects an FTP URL", async () => { - await expect( - assertWebhookUrlAllowed("ftp://example.com/file"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("ftp://example.com/file")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects a completely invalid URL", async () => { - await expect( - assertWebhookUrlAllowed("not-a-url"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("not-a-url")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); // ── Blocked hostnames ──────────────────────────────────────────────────────── it("rejects localhost by hostname", async () => { - await expect( - assertWebhookUrlAllowed("https://localhost/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://localhost/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects the AWS cloud metadata endpoint by hostname", async () => { @@ -72,39 +71,39 @@ describe("assertWebhookUrlAllowed — SSRF guard", () => { // ── Blocked IP ranges (direct IP addresses as hostname) ───────────────────── it("rejects IPv4 loopback 127.0.0.1", async () => { - await expect( - assertWebhookUrlAllowed("https://127.0.0.1/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://127.0.0.1/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects IPv4 loopback 127.1.2.3 (full /8 block)", async () => { - await expect( - assertWebhookUrlAllowed("https://127.1.2.3/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://127.1.2.3/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects RFC 1918 private address 10.0.0.1", async () => { - await expect( - assertWebhookUrlAllowed("https://10.0.0.1/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://10.0.0.1/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects RFC 1918 private address 172.16.0.1", async () => { - await expect( - assertWebhookUrlAllowed("https://172.16.0.1/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://172.16.0.1/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects RFC 1918 private address 192.168.1.100", async () => { - await expect( - assertWebhookUrlAllowed("https://192.168.1.100/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://192.168.1.100/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); it("rejects link-local address 169.254.1.1", async () => { - await expect( - assertWebhookUrlAllowed("https://169.254.1.1/callback"), - ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + await expect(assertWebhookUrlAllowed("https://169.254.1.1/callback")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); }); // ── DNS fail-closed behaviour ──────────────────────────────────────────────── @@ -120,10 +119,94 @@ describe("assertWebhookUrlAllowed — SSRF guard", () => { it("rejects a public hostname that resolves to a private IP (DNS rebinding)", async () => { const { lookup } = await import("node:dns/promises"); - vi.mocked(lookup).mockResolvedValueOnce({ address: "192.168.0.1", family: 4 }); + vi.mocked(lookup).mockResolvedValueOnce([{ address: "192.168.0.1", family: 4 }]); + await expect(assertWebhookUrlAllowed("https://rebind.example.com/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects if ANY of the resolved addresses is private (multi-record attack)", async () => { + const { lookup } = await import("node:dns/promises"); + vi.mocked(lookup).mockResolvedValueOnce([ + { address: "93.184.216.34", family: 4 }, + { address: "10.0.0.5", family: 4 }, + ]); + await expect(assertWebhookUrlAllowed("https://multi.example.com/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("resolveAndValidate returns the first validated address for connection pinning", async () => { + const resolved = await resolveAndValidate("https://example.com/hook"); + expect(resolved.address).toBe("93.184.216.34"); + expect(resolved.family).toBe(4); + expect(resolved.hostname).toBe("example.com"); + }); + + // ── IPv6 blocklist ─────────────────────────────────────────────────────────── + + it("rejects IPv6 loopback ::1", async () => { + await expect(assertWebhookUrlAllowed("https://[::1]/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects IPv6 unique-local fc00::/7 (fc00::1)", async () => { + await expect(assertWebhookUrlAllowed("https://[fc00::1]/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects IPv6 link-local fe80::/10 (fe80::1)", async () => { + await expect(assertWebhookUrlAllowed("https://[fe80::1]/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects IPv4-mapped IPv6 (::ffff:192.168.1.1) pointing into private v4", async () => { await expect( - assertWebhookUrlAllowed("https://rebind.example.com/hook"), + assertWebhookUrlAllowed("https://[::ffff:192.168.1.1]/hook"), ).rejects.toMatchObject({ code: "BAD_REQUEST" }); }); + + it("rejects IPv6 multicast (ff02::1)", async () => { + await expect(assertWebhookUrlAllowed("https://[ff02::1]/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects 0.0.0.0/8", async () => { + await expect(assertWebhookUrlAllowed("https://0.0.0.0/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("rejects 100.64.0.0/10 CGNAT", async () => { + await expect(assertWebhookUrlAllowed("https://100.64.1.1/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + await expect(assertWebhookUrlAllowed("https://100.127.254.254/hook")).rejects.toMatchObject({ + code: "BAD_REQUEST", + }); + }); + + it("accepts a 100.x address outside the CGNAT /10 (100.63.x is public)", async () => { + // 100.63.x is not in 100.64.0.0/10 — it is part of the public IANA pool. + expect(__test__.isBlockedIpv4("100.63.1.1")).toBe(false); + }); + + it("rejects 198.18.0.0/15 benchmark and TEST-NET ranges", async () => { + expect(__test__.isBlockedIpv4("198.18.0.1")).toBe(true); + expect(__test__.isBlockedIpv4("192.0.2.1")).toBe(true); + expect(__test__.isBlockedIpv4("203.0.113.1")).toBe(true); + }); + + it("expandIpv6 normalises short-form addresses to full 8-group form", () => { + expect(__test__.expandIpv6("::1")).toBe("0000:0000:0000:0000:0000:0000:0000:0001"); + expect(__test__.expandIpv6("fe80::1")).toBe("fe80:0000:0000:0000:0000:0000:0000:0001"); + expect(__test__.expandIpv6("::ffff:192.168.1.1")).toBe( + "0000:0000:0000:0000:0000:ffff:c0a8:0101", + ); + }); }); diff --git a/packages/api/src/__tests__/user-router.test.ts b/packages/api/src/__tests__/user-router.test.ts index 02582dc..1fb3e2c 100644 --- a/packages/api/src/__tests__/user-router.test.ts +++ b/packages/api/src/__tests__/user-router.test.ts @@ -716,19 +716,26 @@ describe("user profile and TOTP self-service", () => { totpEnabled: false, }); const update = vi.fn().mockResolvedValue({}); + const updateMany = vi.fn().mockResolvedValue({ count: 1 }); const caller = createAdminCaller({ user: { findUnique, update, + updateMany, }, }); const result = await caller.verifyAndEnableTotp({ token: "123456" }); expect(result).toEqual({ enabled: true }); + // lastTotpAt is written atomically by updateMany (the replay guard); + // user.update only toggles the enabled flag after the CAS succeeds. + expect(updateMany).toHaveBeenCalledWith( + expect.objectContaining({ data: { lastTotpAt: expect.any(Date) } }), + ); expect(update).toHaveBeenCalledWith({ where: { id: "user_admin" }, - data: { totpEnabled: true, lastTotpAt: expect.any(Date) }, + data: { totpEnabled: true }, }); }); @@ -743,10 +750,12 @@ describe("user profile and TOTP self-service", () => { lastTotpAt: null, }); const update = vi.fn().mockResolvedValue({}); + const updateMany = vi.fn().mockResolvedValue({ count: 1 }); const caller = createAdminCaller({ user: { findUnique, update, + updateMany, }, }); @@ -757,10 +766,9 @@ describe("user profile and TOTP self-service", () => { where: { id: "user_admin" }, select: { id: true, totpSecret: true, totpEnabled: true, lastTotpAt: true }, }); - expect(update).toHaveBeenCalledWith({ - where: { id: "user_admin" }, - data: { lastTotpAt: expect.any(Date) }, - }); + expect(updateMany).toHaveBeenCalledWith( + expect.objectContaining({ data: { lastTotpAt: expect.any(Date) } }), + ); }); it("rejects invalid login-flow TOTP tokens with UNAUTHORIZED", async () => { diff --git a/packages/api/src/__tests__/user-self-service-mfa.test.ts b/packages/api/src/__tests__/user-self-service-mfa.test.ts index 5c4c2b3..4a251a7 100644 --- a/packages/api/src/__tests__/user-self-service-mfa.test.ts +++ b/packages/api/src/__tests__/user-self-service-mfa.test.ts @@ -71,6 +71,7 @@ function makeSelfServiceCtx(dbOverrides: Record = {}) { user: { findUnique: vi.fn(), update: vi.fn().mockResolvedValue({}), + updateMany: vi.fn().mockResolvedValue({ count: 1 }), ...((dbOverrides.user as object | undefined) ?? {}), }, auditLog: { @@ -96,6 +97,7 @@ function makePublicCtx(overrides: Record = {}) { user: { findUnique: vi.fn(), update: vi.fn().mockResolvedValue({}), + updateMany: vi.fn().mockResolvedValue({ count: 1 }), ...((overrides.user as object | undefined) ?? {}), }, }, @@ -152,9 +154,12 @@ describe("verifyAndEnableTotp", () => { token: "123456", }); expect(result).toEqual({ enabled: true }); + expect(ctx.db.user.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ data: { lastTotpAt: expect.any(Date) } }), + ); expect(ctx.db.user.update).toHaveBeenCalledWith({ where: { id: "user_1" }, - data: { totpEnabled: true, lastTotpAt: expect.any(Date) }, + data: { totpEnabled: true }, }); }); diff --git a/packages/api/src/__tests__/webhook-dispatcher.test.ts b/packages/api/src/__tests__/webhook-dispatcher.test.ts index c2fe4b3..8bff8fa 100644 --- a/packages/api/src/__tests__/webhook-dispatcher.test.ts +++ b/packages/api/src/__tests__/webhook-dispatcher.test.ts @@ -19,6 +19,24 @@ vi.mock("../lib/logger.js", () => ({ }, })); +// Dispatcher now resolves+validates DNS before opening the HTTPS socket. +// Mock node:dns/promises so tests do not require real network. +vi.mock("node:dns/promises", () => ({ + lookup: vi.fn(async (_hostname: string, _opts?: unknown) => [ + { address: "93.184.216.34", family: 4 }, + ]), +})); + +// Mock node:https so we never open a real socket. The dispatcher calls +// https.request(opts, cb); we return a minimal EventEmitter-like stub. +const { httpsRequestMock } = vi.hoisted(() => ({ + httpsRequestMock: vi.fn(), +})); +vi.mock("node:https", () => ({ + Agent: vi.fn(() => ({})), + request: httpsRequestMock, +})); + describe("webhook dispatcher logging", () => { beforeEach(() => { vi.clearAllMocks(); @@ -82,11 +100,19 @@ describe("webhook dispatcher logging", () => { }); it("treats non-2xx HTTP webhook responses as delivery failures", async () => { - const fetchMock = vi.fn().mockResolvedValue({ - ok: false, - status: 500, - }); - vi.stubGlobal("fetch", fetchMock); + // Stub https.request to deliver a 500 response synchronously via the + // response callback, so the dispatcher sees a non-2xx and logs a warn. + httpsRequestMock.mockImplementation( + (_opts: unknown, cb: (res: { statusCode: number; resume: () => void }) => void) => { + queueMicrotask(() => cb({ statusCode: 500, resume: () => {} })); + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + destroy: vi.fn(), + }; + }, + ); const db = { webhook: { @@ -117,6 +143,66 @@ describe("webhook dispatcher logging", () => { ); }); - expect(fetchMock).toHaveBeenCalledTimes(1); + expect(httpsRequestMock).toHaveBeenCalledTimes(1); + // Verify the pinned IP was passed via the lookup override on the Agent. + const firstCall = httpsRequestMock.mock.calls[0]![0] as { + host: string; + servername: string; + agent: { lookup?: unknown }; + }; + expect(firstCall.host).toBe("example.com"); + expect(firstCall.servername).toBe("example.com"); + }); + + it("pins the validated IP via the HTTPS Agent.lookup override (DNS-rebind defence)", async () => { + const { Agent } = await import("node:https"); + const AgentMock = vi.mocked(Agent); + AgentMock.mockClear(); + + httpsRequestMock.mockImplementation( + (_opts: unknown, cb: (res: { statusCode: number; resume: () => void }) => void) => { + queueMicrotask(() => cb({ statusCode: 204, resume: () => {} })); + return { + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + destroy: vi.fn(), + }; + }, + ); + + const db = { + webhook: { + findMany: vi.fn().mockResolvedValue([ + { + id: "wh_rebind_1", + name: "Pinned Webhook", + url: "https://example.com/hook", + secret: null, + events: ["project.created"], + }, + ]), + }, + }; + + dispatchWebhooks(db, "project.created", { id: "p1" }); + + await vi.waitFor(() => expect(httpsRequestMock).toHaveBeenCalledTimes(1)); + + expect(AgentMock).toHaveBeenCalledTimes(1); + const agentOptions = AgentMock.mock.calls[0]![0] as { + lookup?: ( + host: string, + opts: unknown, + cb: (err: null, addr: string, family: number) => void, + ) => void; + }; + expect(typeof agentOptions.lookup).toBe("function"); + + // Invoke the lookup override to confirm it returns the pre-validated IP, + // NOT whatever DNS might be returning right now. + const cb = vi.fn(); + agentOptions.lookup!("example.com", {}, cb); + expect(cb).toHaveBeenCalledWith(null, "93.184.216.34", 4); }); }); diff --git a/packages/api/src/lib/ssrf-guard.ts b/packages/api/src/lib/ssrf-guard.ts index c8d80a2..fad6393 100644 --- a/packages/api/src/lib/ssrf-guard.ts +++ b/packages/api/src/lib/ssrf-guard.ts @@ -1,44 +1,131 @@ /** * SSRF guard for outbound webhook URLs. * - * Validates that a target URL is not pointing to internal/private infrastructure - * before allowing a webhook to be stored or dispatched. + * Blocks IPv4 RFC-1918, loopback, link-local, CGNAT, cloud-metadata IPs, as + * well as IPv6 loopback, link-local (fe80::/10), unique-local (fc00::/7), and + * IPv4-mapped IPv6 addresses (::ffff:...). Resolves the hostname with + * `all: true` so a DNS record returning multiple addresses is rejected if + * ANY of them is private — an attacker who adds a private A record alongside + * a public one cannot smuggle past by hoping the fetch picks the "good" IP. + * + * DNS-rebinding defence: callers that are about to open a connection should + * use `resolveAndValidate()` and then pass the returned `address` through + * a `lookup` override on their HTTPS agent so the TCP connect uses the + * validated IP, not a freshly-resolved one that the attacker may have + * flipped after the check. See `webhook-dispatcher.ts`. */ -import { lookup } from "node:dns/promises"; +import { lookup as dnsLookup } from "node:dns/promises"; +import { isIP } from "node:net"; import { TRPCError } from "@trpc/server"; -/** Regex patterns matching IP ranges that must not be targeted. */ -const BLOCKED_IP_PATTERNS: RegExp[] = [ - // Loopback IPv4 - /^127\./, - // Loopback IPv6 - /^::1$/, - // RFC 1918 private - /^10\./, - /^172\.(1[6-9]|2\d|3[01])\./, - /^192\.168\./, - // Link-local - /^169\.254\./, - // Cloud metadata (AWS, GCP, Azure) - /^100\.64\./, +const IPV4_BLOCK_PATTERNS: RegExp[] = [ + /^0\./, // 0.0.0.0/8 — "this network" + /^10\./, // RFC 1918 + /^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./, // 100.64.0.0/10 CGNAT + /^127\./, // loopback + /^169\.254\./, // link-local incl. AWS/Azure/GCP metadata 169.254.169.254 + /^172\.(1[6-9]|2\d|3[01])\./, // RFC 1918 + /^192\.0\.0\./, // RFC 6890 IETF protocol assignments + /^192\.0\.2\./, // TEST-NET-1 + /^192\.168\./, // RFC 1918 + /^198\.(1[89])\./, // 198.18.0.0/15 benchmarking + /^198\.51\.100\./, // TEST-NET-2 + /^203\.0\.113\./, // TEST-NET-3 + /^2(2[4-9]|3\d)\./, // 224.0.0.0/4 multicast + /^2(4\d|5[0-5])\./, // 240.0.0.0/4 reserved + 255.255.255.255 broadcast ]; -/** Hostnames that must never be resolved or contacted. */ -const BLOCKED_HOSTNAMES = new Set([ - "localhost", - "metadata.google.internal", - "169.254.169.254", -]); - -function isBlockedIp(ip: string): boolean { - return BLOCKED_IP_PATTERNS.some((re) => re.test(ip)); +function isBlockedIpv4(ip: string): boolean { + return IPV4_BLOCK_PATTERNS.some((re) => re.test(ip)); } /** - * Throws a TRPCError if the given URL targets internal/private infrastructure. - * Performs DNS resolution to catch attempts to bypass hostname checks. + * Expand an IPv6 address to its full 8-group form so prefix matches work + * reliably (::1 → 0000:0000:0000:0000:0000:0000:0000:0001). */ -export async function assertWebhookUrlAllowed(urlString: string): Promise { +function expandIpv6(ip: string): string { + const lower = ip.toLowerCase().replace(/%.*$/, ""); // strip zone-id + // Handle IPv4-mapped suffix, e.g. ::ffff:192.168.0.1 → ::ffff:c0a8:0001 + const ipv4MappedMatch = lower.match(/^(.*:)(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/); + let working = lower; + if (ipv4MappedMatch) { + const [, prefix, v4] = ipv4MappedMatch; + const parts = v4!.split(".").map((n) => Number.parseInt(n, 10)); + if (parts.length === 4 && parts.every((n) => n >= 0 && n <= 255)) { + const hi = ((parts[0]! << 8) | parts[1]!).toString(16); + const lo = ((parts[2]! << 8) | parts[3]!).toString(16); + working = `${prefix}${hi}:${lo}`; + } + } + const parts = working.split("::"); + const head = parts[0] === "" ? [] : parts[0]!.split(":"); + const tail = parts.length > 1 ? (parts[1] === "" ? [] : parts[1]!.split(":")) : []; + const missing = 8 - head.length - tail.length; + const zeros = Array.from({ length: Math.max(0, missing) }, () => "0"); + const full = parts.length === 1 ? head : [...head, ...zeros, ...tail]; + return full.map((g) => g.padStart(4, "0")).join(":"); +} + +function isBlockedIpv6(ip: string): boolean { + const expanded = expandIpv6(ip); + // ::1 loopback + if (expanded === "0000:0000:0000:0000:0000:0000:0000:0001") return true; + // :: unspecified + if (expanded === "0000:0000:0000:0000:0000:0000:0000:0000") return true; + // IPv4-mapped ::ffff:0:0/96 — extract the embedded v4 and run the v4 check + if (expanded.startsWith("0000:0000:0000:0000:0000:ffff:")) { + const g6 = expanded.split(":")[6]!; + const g7 = expanded.split(":")[7]!; + const v4 = [ + Number.parseInt(g6.slice(0, 2), 16), + Number.parseInt(g6.slice(2, 4), 16), + Number.parseInt(g7.slice(0, 2), 16), + Number.parseInt(g7.slice(2, 4), 16), + ].join("."); + return isBlockedIpv4(v4); + } + // fc00::/7 unique-local — first byte starts with 1111110x → fc or fd + if (/^f[cd]/.test(expanded)) return true; + // fe80::/10 link-local — first 10 bits 1111111010 → fe80..febf + if (/^fe[89ab]/.test(expanded)) return true; + // ff00::/8 multicast + if (/^ff/.test(expanded)) return true; + // 2001:db8::/32 documentation + if (expanded.startsWith("2001:0db8:")) return true; + return false; +} + +function isBlockedIp(ip: string): boolean { + const family = isIP(ip); + if (family === 4) return isBlockedIpv4(ip); + if (family === 6) return isBlockedIpv6(ip); + // Not a valid IP — err on the side of caution. + return true; +} + +const BLOCKED_HOSTNAMES = new Set([ + "localhost", + "ip6-localhost", + "ip6-loopback", + "metadata.google.internal", + "metadata.goog", + "169.254.169.254", +]); + +export interface ResolvedHost { + hostname: string; + /** The pre-validated address to dial. */ + address: string; + family: 4 | 6; +} + +/** + * Resolve the given URL's hostname, validate every address against the + * SSRF blocklist, and return the first valid address for connection pinning. + * Rejects the URL if ANY resolved address is private — an attacker cannot + * evade by adding a private A record to a public-looking hostname. + */ +export async function resolveAndValidate(urlString: string): Promise { let parsed: URL; try { parsed = new URL(urlString); @@ -50,21 +137,55 @@ export async function assertWebhookUrlAllowed(urlString: string): Promise throw new TRPCError({ code: "BAD_REQUEST", message: "Webhook URLs must use HTTPS." }); } - const hostname = parsed.hostname.toLowerCase(); + const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, ""); if (BLOCKED_HOSTNAMES.has(hostname)) { throw new TRPCError({ code: "BAD_REQUEST", message: "Webhook URL target is not allowed." }); } - // Resolve hostname and validate the resulting IP address - try { - const { address } = await lookup(hostname); - if (isBlockedIp(address) || BLOCKED_HOSTNAMES.has(address)) { - throw new TRPCError({ code: "BAD_REQUEST", message: "Webhook URL target is not allowed." }); + // Literal IP hostnames: validate directly without DNS. + const literalFamily = isIP(hostname); + if (literalFamily !== 0) { + if (isBlockedIp(hostname)) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Webhook URL target is not allowed.", + }); } - } catch (err) { - if (err instanceof TRPCError) throw err; - // DNS resolution failed — block by default (fail-closed) + return { hostname, address: hostname, family: literalFamily as 4 | 6 }; + } + + let addresses: Array<{ address: string; family: number }>; + try { + addresses = await dnsLookup(hostname, { all: true }); + } catch { throw new TRPCError({ code: "BAD_REQUEST", message: "Webhook URL could not be validated." }); } + if (addresses.length === 0) { + throw new TRPCError({ code: "BAD_REQUEST", message: "Webhook URL could not be validated." }); + } + + for (const { address } of addresses) { + if (isBlockedIp(address) || BLOCKED_HOSTNAMES.has(address)) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Webhook URL target is not allowed.", + }); + } + } + + const first = addresses[0]!; + return { hostname, address: first.address, family: first.family as 4 | 6 }; } + +/** + * Throws a TRPCError if the given URL targets internal/private infrastructure. + * Preserved as a compatibility entrypoint for callers that only need the + * allow/deny decision without the pinned address. + */ +export async function assertWebhookUrlAllowed(urlString: string): Promise { + await resolveAndValidate(urlString); +} + +/** Exposed for unit tests. */ +export const __test__ = { isBlockedIpv4, isBlockedIpv6, expandIpv6, isBlockedIp }; diff --git a/packages/api/src/lib/webhook-dispatcher.ts b/packages/api/src/lib/webhook-dispatcher.ts index 76c9841..704050e 100644 --- a/packages/api/src/lib/webhook-dispatcher.ts +++ b/packages/api/src/lib/webhook-dispatcher.ts @@ -7,9 +7,10 @@ * Fire-and-forget — errors are logged, never thrown. */ import { createHmac } from "node:crypto"; +import { Agent, request } from "node:https"; import { logger } from "./logger.js"; import { sendSlackNotification } from "./slack-notify.js"; -import { assertWebhookUrlAllowed } from "./ssrf-guard.js"; +import { resolveAndValidate } from "./ssrf-guard.js"; /** Available webhook event types. */ export const WEBHOOK_EVENTS = [ @@ -27,9 +28,7 @@ export type WebhookEvent = (typeof WEBHOOK_EVENTS)[number]; interface MinimalDb { webhook: { - findMany: (args: { - where: { isActive: boolean; events: { has: string } }; - }) => Promise< + findMany: (args: { where: { isActive: boolean; events: { has: string } } }) => Promise< Array<{ id: string; name: string; @@ -68,9 +67,7 @@ async function _dispatch( const timestamp = new Date().toISOString(); const body = JSON.stringify({ event, timestamp, payload }); - const promises = webhooks.map((wh) => - _sendToWebhook(wh, event, body, timestamp, payload), - ); + const promises = webhooks.map((wh) => _sendToWebhook(wh, event, body, timestamp, payload)); await Promise.allSettled(promises); } catch (err) { @@ -86,7 +83,12 @@ async function _sendToWebhook( payload: Record, ): Promise { try { - await assertWebhookUrlAllowed(wh.url); + // Resolve + validate ALL DNS records in a single pass and capture the + // first validated IP. The IP is then pinned at TCP-connect time via a + // custom `lookup` override on the HTTPS agent so a DNS rebind between + // the guard check and the socket `connect()` cannot redirect the dial + // to an internal address. + const resolved = await resolveAndValidate(wh.url); // Slack-specific path: use the Slack notification helper. // Use strict hostname match to prevent bypass via "hooks.slack.com.attacker.example.com". @@ -101,32 +103,15 @@ async function _sendToWebhook( "Content-Type": "application/json", "X-Webhook-Event": event, "X-Webhook-Timestamp": timestamp, + "Content-Length": Buffer.byteLength(body).toString(), }; if (wh.secret) { - const signature = createHmac("sha256", wh.secret) - .update(body) - .digest("hex"); + const signature = createHmac("sha256", wh.secret).update(body).digest("hex"); headers["X-Webhook-Signature"] = signature; } - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 5_000); - - try { - const response = await fetch(wh.url, { - method: "POST", - headers, - body, - signal: controller.signal, - }); - - if (!response.ok) { - throw new Error(`Webhook responded with HTTP ${response.status}`); - } - } finally { - clearTimeout(timeout); - } + await dispatchHttpsRequest(wh.url, resolved, headers, body); } catch (err) { logger.warn( { err, event, webhookId: wh.id, webhookName: wh.name, webhookUrl: wh.url }, @@ -135,13 +120,58 @@ async function _sendToWebhook( } } +/** + * Dispatch a POST to the resolved+validated target using a custom + * `https.Agent` whose DNS lookup is pinned to the address the guard + * already approved. The real hostname is still used for SNI/Host so + * certificate validation works unchanged. + */ +async function dispatchHttpsRequest( + url: string, + resolved: { address: string; family: 4 | 6 }, + headers: Record, + body: string, +): Promise { + const parsed = new URL(url); + const pinnedAgent = new Agent({ + keepAlive: false, + lookup: (_hostname, _opts, cb) => cb(null, resolved.address, resolved.family), + }); + + await new Promise((resolve, reject) => { + const req = request( + { + host: parsed.hostname, + port: parsed.port || 443, + path: parsed.pathname + parsed.search, + method: "POST", + headers, + agent: pinnedAgent, + timeout: 5_000, + servername: parsed.hostname, + }, + (res) => { + res.resume(); + if (res.statusCode && res.statusCode >= 200 && res.statusCode < 300) { + resolve(); + } else { + reject(new Error(`Webhook responded with HTTP ${res.statusCode}`)); + } + }, + ); + req.on("timeout", () => { + req.destroy(new Error("Webhook request timed out")); + }); + req.on("error", (err) => reject(err)); + req.write(body); + req.end(); + }); +} + /** * Format a human-readable Slack message from a webhook event. */ -function formatSlackMessage( - event: string, - payload: Record, -): string { +function formatSlackMessage(event: string, payload: Record): string { const label = event.replace(/\./g, " ").replace(/\b\w/g, (c) => c.toUpperCase()); const id = (payload["id"] as string) ?? (payload["projectId"] as string) ?? ""; const name = (payload["name"] as string) ?? "";