Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/webhook-ssrf-dns-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperdx/api': patch
---

fix: Block generic and incident.io webhooks whose hostname resolves to a private or reserved address. The check runs on the address actually dialed, so it also covers DNS rebinding. Self-hosted setups that send webhooks to internal hostnames must add them to `WEBHOOK_HOSTNAME_ALLOWLIST`.
7 changes: 7 additions & 0 deletions packages/api/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,10 @@ global.fetch = jest.fn().mockResolvedValue({
text: jest.fn().mockResolvedValue(''),
json: jest.fn().mockResolvedValue({}),
} as any);

// Webhook delivery uses undici's fetch; route it to the global mock so tests
// stub and assert on `global.fetch` as before.
jest.mock('undici', () => ({
...jest.requireActual('undici'),
fetch: (...args: Parameters<typeof fetch>) => global.fetch(...args),
}));
1 change: 1 addition & 0 deletions packages/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"protobufjs": "^7.6.5",
"serialize-error": "^8.1.0",
"sqlstring": "^2.3.3",
"undici": "^7.29.0",
"uuid": "^8.3.2",
"zod": "3.25",
"zod-express-middleware": "^1.4.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5037,6 +5037,7 @@ describe('checkAlerts', () => {
'Idempotency-Key': expect.any(String),
}),
signal: expect.any(AbortSignal),
dispatcher: expect.anything(),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
DEFAULT_INCIDENT_IO_WEBHOOK_BODY,
WebhookService,
} from '@hyperdx/common-utils/dist/types';
import dns from 'dns';
import { ObjectId } from 'mongodb';

import { AlertState } from '@/models/alert';
Expand All @@ -12,6 +13,7 @@ import {
handleSendGenericWebhook,
} from '@/tasks/checkAlerts/transports/generic';
import type { Message } from '@/tasks/checkAlerts/transports/types';
import { WebhookUrlValidationError } from '@/utils/validators';

const message: Message = {
hdxLink: 'https://example.test/alert',
Expand Down Expand Up @@ -91,6 +93,48 @@ describe('handleSendGenericWebhook — per-attempt timeout', () => {
}, 2000); // short test-level timeout: a regression here should fail fast, not stall on Jest's default 30s
});

describe('handleSendGenericWebhook — DNS resolution', () => {
afterEach(() => jest.restoreAllMocks());

it('blocks a public hostname that resolves to loopback, without retrying', async () => {
// The real undici fetch + Agent pair that production uses.
jest
.mocked(global.fetch)
.mockImplementationOnce(jest.requireActual('undici').fetch);
const lookupSpy = jest
.spyOn(dns, 'lookup')
.mockImplementation((...args: any[]) =>
args[2](null, [{ address: '127.0.0.1', family: 4 }]),
);
const loopbackChannel: any = {
type: 'webhook',
channel: { ...webhook, url: 'http://127-0-0-1.nip.io/' },
};

await expect(
handleSendGenericWebhook(loopbackChannel, message),
).rejects.toThrow(WebhookUrlValidationError);
expect(lookupSpy).toHaveBeenCalledTimes(1);
}, 2000);

it('unwraps a block nested deeper in the cause chain, without retrying', async () => {
const blocked = new WebhookUrlValidationError('blocked');
jest
.mocked(global.fetch)
.mockClear()
.mockRejectedValueOnce(
new TypeError('fetch failed', {
cause: new Error('connect failed', { cause: blocked }),
}),
);

await expect(handleSendGenericWebhook(channel, message)).rejects.toBe(
blocked,
);
expect(global.fetch).toHaveBeenCalledTimes(1);
}, 2000);
});

describe('buildWebhookTemplateVariables', () => {
it('exposes the enriched variables alongside the original set', () => {
const vars = buildWebhookTemplateVariables({
Expand Down
11 changes: 11 additions & 0 deletions packages/api/src/tasks/checkAlerts/transports/generic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Handlebars from 'handlebars';
import _ from 'lodash';
import { performance } from 'perf_hooks';
import { serializeError } from 'serialize-error';
import { Agent, fetch } from 'undici';

import { IWebhook } from '@/models/webhook';
import {
Expand All @@ -24,9 +25,12 @@ import logger from '@/utils/logger';
import { withRetry } from '@/utils/retry';
import {
validateWebhookUrl,
webhookLookup,
WebhookUrlValidationError,
} from '@/utils/validators';

const webhookDispatcher = new Agent({ connect: { lookup: webhookLookup } });

// Webhook delivery is the last (and most failure-prone) hop of an alert. It
// happens in the background task, so failures only show up in logs today.
// `service` and `outcome` are bounded enums (see agent_docs/observability.md).
Expand Down Expand Up @@ -261,6 +265,13 @@ const sendGenericWebhook = async (
headers,
body,
signal: attemptSignal,
dispatcher: webhookDispatcher,
}).catch(e => {
// Surface a connect-time block as the validation error (a 400), not "fetch failed".
for (let c = e; c; c = c.cause) {
Comment thread
knudtty marked this conversation as resolved.
Resolved
if (c instanceof WebhookUrlValidationError) throw c;
}
throw e;
});

// Disallow redirects to avoid redirect-based SSRF.
Expand Down
120 changes: 100 additions & 20 deletions packages/api/src/utils/__tests__/validators.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { WebhookService } from '@hyperdx/common-utils/dist/types';
import dns from 'dns';

import { createHandlebarsWithHelpers } from '@/tasks/checkAlerts/transports';
import * as validators from '@/utils/validators';
Expand Down Expand Up @@ -98,6 +99,10 @@ describe('validators', () => {
['mapped public address', '::ffff:8.8.8.8', false],
['mapped private address in hexadecimal form', '::ffff:ac10:1', true],
['mapped public address in hexadecimal form', '::ffff:808:808', false],
['NAT64 loopback address', '64:ff9b::7f00:1', true],
['NAT64 private address in dotted form', '64:ff9b::10.0.0.1', true],
['NAT64 public address', '64:ff9b::808:808', false],
['local-use NAT64 address', '64:ff9b:1::808:808', true],
['private IPv4 address with a CIDR suffix', '127.0.0.1/8', true],
['scoped IPv6 link-local address', 'fe80::1%lo0', true],
])('%s (%s)', (_description, ip, expected) => {
Expand Down Expand Up @@ -128,28 +133,28 @@ describe('validators', () => {
});
});

describe('validateWebhookUrl', () => {
const loadValidatorsWithConfig = (config: {
clickhouseHost?: string;
hostnameAllowlist?: string;
}): typeof validators => {
let isolatedValidators: typeof validators | undefined;

jest.isolateModules(() => {
jest.doMock('@/config', () => ({
CLICKHOUSE_HOST: config.clickhouseHost,
MONGO_URI: undefined,
WEBHOOK_HOSTNAME_ALLOWLIST: config.hostnameAllowlist ?? '',
}));

// eslint-disable-next-line @typescript-eslint/no-require-imports, n/no-missing-require
isolatedValidators = require('@/utils/validators');
});
jest.dontMock('@/config');
const loadValidatorsWithConfig = (config: {
clickhouseHost?: string;
hostnameAllowlist?: string;
}): typeof validators => {
let isolatedValidators: typeof validators | undefined;

jest.isolateModules(() => {
jest.doMock('@/config', () => ({
CLICKHOUSE_HOST: config.clickhouseHost,
MONGO_URI: undefined,
WEBHOOK_HOSTNAME_ALLOWLIST: config.hostnameAllowlist ?? '',
}));

// eslint-disable-next-line @typescript-eslint/no-require-imports, n/no-missing-require
isolatedValidators = require('@/utils/validators');
});
jest.dontMock('@/config');

return isolatedValidators!;
};
return isolatedValidators!;
};

describe('validateWebhookUrl', () => {
it.each([
[
'generic webhook',
Expand Down Expand Up @@ -322,6 +327,81 @@ describe('validators', () => {
});
});

describe('webhookLookup', () => {
const lookup = (
v: typeof validators,
hostname: string,
all = false,
): Promise<unknown> =>
new Promise((resolve, reject) =>
v.webhookLookup(hostname, { all }, (err, address) =>
err ? reject(err) : resolve(address),
),
);

const mockLookup = (err: Error | null, addresses: dns.LookupAddress[]) =>
jest
.spyOn(dns, 'lookup')
.mockImplementation((...args: any[]) => args[2](err, addresses));

afterEach(() => jest.restoreAllMocks());

it('blocks a hostname that resolves to a private address', async () => {
await expect(lookup(validators, 'localhost')).rejects.toThrow(
validators.WebhookUrlValidationError,
);
});

it('returns public addresses in the shape the caller asked for', async () => {
mockLookup(null, [{ address: '93.184.216.34', family: 4 }]);

await expect(lookup(validators, 'example.com')).resolves.toBe(
'93.184.216.34',
);
await expect(lookup(validators, 'example.com', true)).resolves.toEqual([
{ address: '93.184.216.34', family: 4 },
]);
});

it.each([
[
'one private record among public ones',
[
{ address: '93.184.216.34', family: 4 },
{ address: '10.0.0.1', family: 4 },
],
],
['an IPv6 loopback record', [{ address: '::1', family: 6 }]],
[
'an IPv4-mapped private record',
[{ address: '::ffff:192.168.1.1', family: 6 }],
],
['a NAT64 loopback record', [{ address: '64:ff9b::7f00:1', family: 6 }]],
])('blocks %s', async (_name, addresses) => {
mockLookup(null, addresses);
await expect(lookup(validators, 'example.com', true)).rejects.toThrow(
validators.WebhookUrlValidationError,
);
});

it('passes through a DNS error unchanged', async () => {
const dnsError = Object.assign(new Error('getaddrinfo ENOTFOUND'), {
code: 'ENOTFOUND',
});
mockLookup(dnsError, []);
await expect(lookup(validators, 'nope.example')).rejects.toBe(dnsError);
});

it('allows an allowlisted hostname that resolves privately', async () => {
const isolatedValidators = loadValidatorsWithConfig({
hostnameAllowlist: 'localhost',
});
await expect(lookup(isolatedValidators, 'localhost')).resolves.toEqual(
expect.any(String),
);
});
});

describe('getWebhookConfigErrors', () => {
const BROKEN_BODY = '{"text": "{{#if title}}"}';

Expand Down
48 changes: 42 additions & 6 deletions packages/api/src/utils/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
PASSWORD_SPECIAL_CHARS,
passwordValidators,
} from '@hyperdx/common-utils/dist/validation';
import dns from 'dns';
import { Address4, Address6 } from 'ip-address';
import type { LookupFunction } from 'net';
import { z } from 'zod';

import * as config from '@/config';
Expand All @@ -32,6 +34,8 @@ const IPV4_COMPATIBLE_SUBNET = new Address6('::/96');
const IPV4_MAPPED_SUBNET = new Address6('::ffff:0:0/96');
const IPV6_LINK_LOCAL_SUBNET = new Address6('fe80::/10');
const IPV6_DISCARD_ONLY_SUBNET = new Address6('100::/64');
const NAT64_WELL_KNOWN_SUBNET = new Address6('64:ff9b::/96'); // RFC 6052
const NAT64_LOCAL_USE_SUBNET = new Address6('64:ff9b:1::/48'); // RFC 8215

// Returns true for private/reserved IPv4 and IPv6 addresses that must not be
// reachable from outbound requests (SSRF protection).
Expand All @@ -50,10 +54,12 @@ export function isPrivateIp(ip: string): boolean {
}
if (Address6.isValid(ip)) {
const addr = new Address6(ip);
// Check the embedded address in mapped and deprecated compatible forms.
// Check the embedded address in mapped, deprecated compatible, and NAT64
// forms; a NAT64 gateway forwards 64:ff9b::7f00:1 to 127.0.0.1.
if (
addr.isInSubnet(IPV4_MAPPED_SUBNET) ||
addr.isInSubnet(IPV4_COMPATIBLE_SUBNET)
addr.isInSubnet(IPV4_COMPATIBLE_SUBNET) ||
addr.isInSubnet(NAT64_WELL_KNOWN_SUBNET)
) {
const v4 = addr.to4();
if (v4 && Address4.isValid(v4.address)) {
Expand All @@ -68,7 +74,9 @@ export function isPrivateIp(ip: string): boolean {
addr.isMulticast() ||
addr.isUnspecified() ||
addr.isDocumentation() ||
addr.isInSubnet(IPV6_DISCARD_ONLY_SUBNET)
addr.isInSubnet(IPV6_DISCARD_ONLY_SUBNET) ||
// Operator-defined prefixes embed the IPv4 at varying offsets; block all.
addr.isInSubnet(NAT64_LOCAL_USE_SUBNET)
);
}
return false;
Expand Down Expand Up @@ -114,6 +122,9 @@ const BLOCKED_WEBHOOK_HOSTS = (() => {
})();

export class WebhookUrlValidationError extends Error {
// A 4xx status makes withRetry give up instead of retrying a blocked URL.
readonly status = 400;

constructor(message: string) {
super(message);
this.name = 'WebhookUrlValidationError';
Expand Down Expand Up @@ -163,12 +174,37 @@ export function validateWebhookUrl(
hostname === 'localhost' || hostname.endsWith('.localhost');
const isAllowlistedHostname = isAllowlistedWebhookHostname(hostname);
if (!isAllowlistedHostname && (isLocalhost || isPrivateIp(hostname))) {
throw new WebhookUrlValidationError(
`Webhook URL resolves to a private or reserved address.`,
);
throw new WebhookUrlValidationError(PRIVATE_ADDRESS_MESSAGE);
}
}

const PRIVATE_ADDRESS_MESSAGE =
'Webhook URL resolves to a private or reserved address.';

// validateWebhookUrl only sees the hostname, so a public name pointing at a
// private IP (e.g. 127-0-0-1.nip.io) gets through it. Checking at connect time
// also covers DNS rebinding, since the checked address is the one dialed.
// Node skips lookup for IP literals; validateWebhookUrl covers those.
export const webhookLookup: LookupFunction = (hostname, options, callback) => {
dns.lookup(hostname, { ...options, all: true }, (err, addresses) => {
if (err) return callback(err, []);
const blocked =
!isAllowlistedWebhookHostname(normalizeWebhookHostname(hostname)) &&
addresses.some(
({ address }) =>
isPrivateIp(address) && !isAllowlistedWebhookHostname(address),
);
if (blocked) {
return callback(
new WebhookUrlValidationError(PRIVATE_ADDRESS_MESSAGE),
[],
);
}
if (options.all) return callback(null, addresses);
callback(null, addresses[0].address, addresses[0].family);
});
};

/** A 400-worthy webhook config error whose message is safe to return. */
export const isWebhookValidationError = (
err: unknown,
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4817,6 +4817,7 @@ __metadata:
tsc-alias: "npm:^1.8.8"
tsconfig-paths: "npm:^4.2.0"
typescript: "npm:^6.0.3"
undici: "npm:^7.29.0"
uuid: "npm:^8.3.2"
zod: "npm:3.25"
zod-express-middleware: "npm:^1.4.0"
Expand Down
Loading