Skip to content
Merged
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
11 changes: 8 additions & 3 deletions apps/web/app/api/(internal)/pipeline/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { sendTelemetryEvents } from "@/app/api/(internal)/pipeline/lib/telemetry
import { ZPipelineInput } from "@/app/api/(internal)/pipeline/types/pipelines";
import { responses } from "@/app/lib/api/response";
import { transformErrorToDetails } from "@/app/lib/api/validator";
import { CRON_SECRET, POSTHOG_KEY } from "@/lib/constants";
import { CRON_SECRET, DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS, POSTHOG_KEY } from "@/lib/constants";
import { generateStandardWebhookSignature } from "@/lib/crypto";
import { getIntegrations } from "@/lib/integration/service";
import { getOrganizationByEnvironmentId } from "@/lib/organization/service";
Expand Down Expand Up @@ -91,10 +91,15 @@ export const POST = async (request: Request) => {
const webhooks: Webhook[] = await getWebhooksForPipeline(environmentId, event, surveyId);
// Prepare webhook and email promises

// Fetch with timeout of 5 seconds to prevent hanging
// Fetch with timeout of 5 seconds to prevent hanging.
// `redirect: "manual"` blocks SSRF via redirect — webhook URLs are validated against private/internal
// ranges before delivery, but redirect targets would otherwise bypass that check. Gated on the same
// env var as `validateWebhookUrl`: self-hosters who opted into trusting internal URLs also get the
// pre-patch redirect-follow behavior for consistency.
const redirectMode: RequestRedirect = DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS ? "follow" : "manual";
const fetchWithTimeout = (url: string, options: RequestInit, timeout: number = 5000): Promise<Response> => {
return Promise.race([
fetch(url, options),
fetch(url, { ...options, redirect: redirectMode }),
new Promise<never>((_, reject) => setTimeout(() => reject(new Error("Timeout")), timeout)),
]);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const AddWebhookModal = ({
url: testEndpointInput,
secret: webhookSecret,
});

if (!testEndpointActionResult?.data) {
const errorMessage = getFormattedErrorMessage(testEndpointActionResult);
throw new Error(errorMessage);
Expand Down
39 changes: 39 additions & 0 deletions apps/web/modules/integrations/webhooks/lib/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ vi.mock("@formbricks/database", () => ({
},
}));

const constantsMock = vi.hoisted(() => ({ dangerouslyAllow: false }));

vi.mock("@/lib/constants", () => ({
get DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS() {
return constantsMock.dangerouslyAllow;
},
}));

vi.mock("@/lib/crypto", () => ({
generateStandardWebhookSignature: vi.fn(() => "signed-payload"),
generateWebhookSecret: vi.fn(() => "generated-secret"),
Expand All @@ -41,6 +49,7 @@ vi.mock("uuid", () => ({
describe("testEndpoint", () => {
beforeEach(() => {
vi.resetAllMocks();
constantsMock.dangerouslyAllow = false;
vi.mocked(generateStandardWebhookSignature).mockReturnValue("signed-payload");
vi.mocked(validateWebhookUrl).mockResolvedValue(undefined);
vi.mocked(getTranslate).mockResolvedValue((key: string) => key);
Expand Down Expand Up @@ -76,6 +85,36 @@ describe("testEndpoint", () => {
expect(getTranslate).toHaveBeenCalled();
});

test.each([301, 302, 303, 307, 308])(
"rejects %s redirects to prevent SSRF via redirect",
async (statusCode) => {
const fetchMock = vi.fn(async () => ({ status: statusCode }));
vi.stubGlobal("fetch", fetchMock);

await expect(testEndpoint("https://example.com/webhook")).rejects.toThrow(
"Webhook endpoint returned a redirect, which is not allowed"
);

expect(fetchMock).toHaveBeenCalledWith(
"https://example.com/webhook",
expect.objectContaining({ redirect: "manual" })
);
}
);

test("follows redirects when DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS is enabled", async () => {
constantsMock.dangerouslyAllow = true;
const fetchMock = vi.fn(async () => ({ status: 200 }));
vi.stubGlobal("fetch", fetchMock);

await expect(testEndpoint("https://example.com/webhook")).resolves.toBe(true);

expect(fetchMock).toHaveBeenCalledWith(
"https://example.com/webhook",
expect.objectContaining({ redirect: "follow" })
);
});

test("allows non-blocked non-2xx statuses", async () => {
vi.stubGlobal(
"fetch",
Expand Down
17 changes: 17 additions & 0 deletions apps/web/modules/integrations/webhooks/lib/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
ResourceNotFoundError,
UnknownError,
} from "@formbricks/types/errors";
import { DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS } from "@/lib/constants";
import { generateStandardWebhookSignature, generateWebhookSecret } from "@/lib/crypto";
import { validateInputs } from "@/lib/utils/validate";
import { validateWebhookUrl } from "@/lib/utils/validate-webhook-url";
Expand Down Expand Up @@ -191,13 +192,29 @@ export const testEndpoint = async (url: string, secret?: string): Promise<boolea
);
}

// `redirect: "manual"` prevents SSRF via redirect — validateWebhookUrl only checks the
// initial URL, so following 30x to a private/internal host (e.g. cloud metadata) would bypass it.
// Gated on the same env var as validateWebhookUrl: self-hosters who opted into trusting internal
// URLs also get the pre-patch redirect-follow behavior for consistency.
const redirectMode: RequestRedirect = DANGEROUSLY_ALLOW_WEBHOOK_INTERNAL_URLS ? "follow" : "manual";
const response = await fetch(url, {
method: "POST",
body,
headers: requestHeaders,
signal: controller.signal,
redirect: redirectMode,
});

const statusCode = response.status;

// With `redirect: "manual"`, Node's undici returns the actual 30x response (not the spec's
// opaqueredirect filter). Treat any 30x as a redirect rejection so users get a clear error
// instead of a misleading success. With `redirect: "follow"`, fetch returns the final 2xx/4xx/5xx
// and this branch is unreachable.
if (statusCode >= 300 && statusCode < 400) {
throw new InvalidInputError("Webhook endpoint returned a redirect, which is not allowed");
}

const errorMessage = await getWebhookTestErrorMessage(statusCode);

if (errorMessage) {
Expand Down
Loading