fix: add URL validation to webhook endpoints (#26593)
Validates webhook URLs on create and update: - HTTPS required (HTTP allowed for self-hosted and E2E) - Blocks private IP ranges and localhost - Blocks cloud metadata endpoints Existing webhooks are preserved: validation only applies when URL is created or changed. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent
c57b517555
commit
cb36fc201f
+5
@@ -1,6 +1,7 @@
|
||||
import { OrganizationsWebhooksRepository } from "@/modules/organizations/webhooks/organizations-webhooks.repository";
|
||||
import { UpdateWebhookInputDto } from "@/modules/webhooks/inputs/webhook.input";
|
||||
import { PipedInputWebhookType } from "@/modules/webhooks/pipes/WebhookInputPipe";
|
||||
import { validateWebhookUrl, validateWebhookUrlIfChanged } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
import { ConflictException, Injectable, NotFoundException } from "@nestjs/common";
|
||||
|
||||
@@ -12,6 +13,8 @@ export class OrganizationsWebhooksService {
|
||||
) {}
|
||||
|
||||
async createWebhook(orgId: number, body: PipedInputWebhookType) {
|
||||
validateWebhookUrl(body.subscriberUrl);
|
||||
|
||||
const existingWebhook = await this.organizationsWebhooksRepository.findWebhookByUrl(
|
||||
orgId,
|
||||
body.subscriberUrl
|
||||
@@ -40,6 +43,8 @@ export class OrganizationsWebhooksService {
|
||||
}
|
||||
|
||||
async updateWebhook(webhookId: string, body: UpdateWebhookInputDto) {
|
||||
const existingSubscriberUrl = await this.webhooksRepository.getWebhookSubscriberUrl(webhookId);
|
||||
validateWebhookUrlIfChanged(body.subscriberUrl, existingSubscriberUrl);
|
||||
return this.webhooksRepository.updateWebhook(webhookId, body);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { PipedInputWebhookType } from "@/modules/webhooks/pipes/WebhookInputPipe";
|
||||
import { validateWebhookUrl } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
import { BadRequestException, ConflictException, Injectable } from "@nestjs/common";
|
||||
|
||||
@@ -9,6 +10,8 @@ export class EventTypeWebhooksService {
|
||||
constructor(private readonly webhooksRepository: WebhooksRepository) {}
|
||||
|
||||
async createEventTypeWebhook(eventTypeId: number, body: PipedInputWebhookType) {
|
||||
validateWebhookUrl(body.subscriberUrl);
|
||||
|
||||
if (body.eventTriggers.includes(WebhookTriggerEvents.DELEGATION_CREDENTIAL_ERROR)) {
|
||||
throw new BadRequestException(
|
||||
"DELEGATION_CREDENTIAL_ERROR trigger is only available for organization webhooks"
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { PipedInputWebhookType } from "@/modules/webhooks/pipes/WebhookInputPipe";
|
||||
import { validateWebhookUrl } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
import { ConflictException, Injectable } from "@nestjs/common";
|
||||
|
||||
@@ -7,6 +8,8 @@ export class OAuthClientWebhooksService {
|
||||
constructor(private readonly webhooksRepository: WebhooksRepository) {}
|
||||
|
||||
async createOAuthClientWebhook(platformOAuthClientId: string, body: PipedInputWebhookType) {
|
||||
validateWebhookUrl(body.subscriberUrl);
|
||||
|
||||
const existingWebhook = await this.webhooksRepository.getOAuthClientWebhookByUrl(
|
||||
platformOAuthClientId,
|
||||
body.subscriberUrl
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { WebhookTriggerEvents } from "@calcom/prisma/enums";
|
||||
import { BadRequestException, ConflictException, Injectable } from "@nestjs/common";
|
||||
import type { PipedInputWebhookType } from "@/modules/webhooks/pipes/WebhookInputPipe";
|
||||
import { validateWebhookUrl } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
|
||||
@Injectable()
|
||||
@@ -8,6 +9,8 @@ export class TeamEventTypeWebhooksService {
|
||||
constructor(private readonly webhooksRepository: WebhooksRepository) {}
|
||||
|
||||
async createTeamEventTypeWebhook(eventTypeId: number, body: PipedInputWebhookType) {
|
||||
validateWebhookUrl(body.subscriberUrl);
|
||||
|
||||
if (body.eventTriggers.includes(WebhookTriggerEvents.DELEGATION_CREDENTIAL_ERROR)) {
|
||||
throw new BadRequestException(
|
||||
"DELEGATION_CREDENTIAL_ERROR trigger is only available for organization webhooks"
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { PipedInputWebhookType } from "@/modules/webhooks/pipes/WebhookInputPipe";
|
||||
import { validateWebhookUrl } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
import { BadRequestException, ConflictException, Injectable } from "@nestjs/common";
|
||||
|
||||
@@ -9,6 +10,8 @@ export class UserWebhooksService {
|
||||
constructor(private readonly webhooksRepository: WebhooksRepository) {}
|
||||
|
||||
async createUserWebhook(userId: number, body: PipedInputWebhookType) {
|
||||
validateWebhookUrl(body.subscriberUrl);
|
||||
|
||||
if (body.eventTriggers.includes(WebhookTriggerEvents.DELEGATION_CREDENTIAL_ERROR)) {
|
||||
throw new BadRequestException(
|
||||
"DELEGATION_CREDENTIAL_ERROR trigger is only available for organization webhooks"
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { UpdateWebhookInputDto } from "@/modules/webhooks/inputs/webhook.input";
|
||||
import { validateWebhookUrlIfChanged } from "@/modules/webhooks/utils/validate-webhook-url";
|
||||
import { WebhooksRepository } from "@/modules/webhooks/webhooks.repository";
|
||||
import { Injectable, NotFoundException } from "@nestjs/common";
|
||||
|
||||
@@ -7,6 +8,8 @@ export class WebhooksService {
|
||||
constructor(private readonly webhooksRepository: WebhooksRepository) {}
|
||||
|
||||
async updateWebhook(webhookId: string, body: UpdateWebhookInputDto) {
|
||||
const existingSubscriberUrl = await this.webhooksRepository.getWebhookSubscriberUrl(webhookId);
|
||||
validateWebhookUrlIfChanged(body.subscriberUrl, existingSubscriberUrl);
|
||||
return this.webhooksRepository.updateWebhook(webhookId, body);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { BadRequestException } from "@nestjs/common";
|
||||
|
||||
// Mock the platform-libraries module
|
||||
const mockValidateUrlForSSRFSync: jest.Mock = jest.fn();
|
||||
jest.mock("@calcom/platform-libraries", () => ({
|
||||
validateUrlForSSRFSync: (url: string) => mockValidateUrlForSSRFSync(url),
|
||||
}));
|
||||
|
||||
import { validateWebhookUrl, validateWebhookUrlIfChanged } from "./validate-webhook-url";
|
||||
|
||||
describe("validateWebhookUrl", () => {
|
||||
beforeEach(() => {
|
||||
mockValidateUrlForSSRFSync.mockReset();
|
||||
});
|
||||
|
||||
it("does not throw when URL is valid", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: true });
|
||||
|
||||
expect(() => validateWebhookUrl("https://example.com/webhook")).not.toThrow();
|
||||
expect(mockValidateUrlForSSRFSync).toHaveBeenCalledWith("https://example.com/webhook");
|
||||
});
|
||||
|
||||
it("throws BadRequestException when URL is invalid", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: false, error: "Private IP address" });
|
||||
|
||||
expect(() => validateWebhookUrl("https://127.0.0.1/webhook")).toThrow(BadRequestException);
|
||||
expect(() => validateWebhookUrl("https://127.0.0.1/webhook")).toThrow(
|
||||
"Webhook URL is not allowed: Private IP address"
|
||||
);
|
||||
});
|
||||
|
||||
it("includes error message from validation result", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: false, error: "Blocked hostname" });
|
||||
|
||||
expect(() => validateWebhookUrl("https://localhost/webhook")).toThrow(
|
||||
"Webhook URL is not allowed: Blocked hostname"
|
||||
);
|
||||
});
|
||||
|
||||
it("handles validation result without error message", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: false });
|
||||
|
||||
expect(() => validateWebhookUrl("invalid-url")).toThrow(
|
||||
"Webhook URL is not allowed: undefined"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateWebhookUrlIfChanged", () => {
|
||||
beforeEach(() => {
|
||||
mockValidateUrlForSSRFSync.mockReset();
|
||||
});
|
||||
|
||||
it("validates URL when it is different from existing", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: true });
|
||||
|
||||
validateWebhookUrlIfChanged("https://new.com/webhook", "https://old.com/webhook");
|
||||
|
||||
expect(mockValidateUrlForSSRFSync).toHaveBeenCalledWith("https://new.com/webhook");
|
||||
});
|
||||
|
||||
it("throws when new URL is different and invalid", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: false, error: "Only HTTPS URLs are allowed" });
|
||||
|
||||
expect(() =>
|
||||
validateWebhookUrlIfChanged("http://new.com/webhook", "https://old.com/webhook")
|
||||
).toThrow(BadRequestException);
|
||||
});
|
||||
|
||||
it("does not validate when URL is unchanged", () => {
|
||||
validateWebhookUrlIfChanged("https://same.com/webhook", "https://same.com/webhook");
|
||||
|
||||
expect(mockValidateUrlForSSRFSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not validate when new URL is undefined", () => {
|
||||
validateWebhookUrlIfChanged(undefined, "https://existing.com/webhook");
|
||||
|
||||
expect(mockValidateUrlForSSRFSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("validates when existing URL is undefined and new URL is provided", () => {
|
||||
mockValidateUrlForSSRFSync.mockReturnValue({ isValid: true });
|
||||
|
||||
validateWebhookUrlIfChanged("https://new.com/webhook", undefined);
|
||||
|
||||
expect(mockValidateUrlForSSRFSync).toHaveBeenCalledWith("https://new.com/webhook");
|
||||
});
|
||||
|
||||
it("does not validate when both URLs are undefined", () => {
|
||||
validateWebhookUrlIfChanged(undefined, undefined);
|
||||
|
||||
expect(mockValidateUrlForSSRFSync).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,18 @@
|
||||
import { BadRequestException } from "@nestjs/common";
|
||||
import { validateUrlForSSRFSync } from "@calcom/platform-libraries";
|
||||
|
||||
export function validateWebhookUrl(subscriberUrl: string): void {
|
||||
const validation = validateUrlForSSRFSync(subscriberUrl);
|
||||
if (!validation.isValid) {
|
||||
throw new BadRequestException(`Webhook URL is not allowed: ${validation.error}`);
|
||||
}
|
||||
}
|
||||
|
||||
export function validateWebhookUrlIfChanged(
|
||||
newSubscriberUrl: string | undefined,
|
||||
existingSubscriberUrl: string | undefined
|
||||
): void {
|
||||
if (newSubscriberUrl && newSubscriberUrl !== existingSubscriberUrl) {
|
||||
validateWebhookUrl(newSubscriberUrl);
|
||||
}
|
||||
}
|
||||
@@ -49,6 +49,14 @@ export class WebhooksRepository {
|
||||
});
|
||||
}
|
||||
|
||||
async getWebhookSubscriberUrl(webhookId: string): Promise<string | undefined> {
|
||||
const webhook = await this.dbRead.prisma.webhook.findFirst({
|
||||
where: { id: webhookId },
|
||||
select: { subscriberUrl: true },
|
||||
});
|
||||
return webhook?.subscriberUrl ?? undefined;
|
||||
}
|
||||
|
||||
async getUserWebhooksPaginated(userId: number, skip: number, take: number) {
|
||||
return this.dbRead.prisma.webhook.findMany({
|
||||
where: { userId },
|
||||
|
||||
Reference in New Issue
Block a user