From 461d14557acc7dac0f3e9cefb5059deb456df603 Mon Sep 17 00:00:00 2001 From: Brian Mendonca Date: Tue, 24 Feb 2026 21:36:04 -0700 Subject: [PATCH] security(nextcloud-talk): reject unsigned webhooks before body read --- .../src/monitor.auth-order.test.ts | 73 +++++++++++++++++++ extensions/nextcloud-talk/src/monitor.ts | 5 +- extensions/nextcloud-talk/src/types.ts | 1 + 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 extensions/nextcloud-talk/src/monitor.auth-order.test.ts diff --git a/extensions/nextcloud-talk/src/monitor.auth-order.test.ts b/extensions/nextcloud-talk/src/monitor.auth-order.test.ts new file mode 100644 index 00000000000..f2b4b65054d --- /dev/null +++ b/extensions/nextcloud-talk/src/monitor.auth-order.test.ts @@ -0,0 +1,73 @@ +import { type AddressInfo } from "node:net"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createNextcloudTalkWebhookServer } from "./monitor.js"; + +type WebhookHarness = { + webhookUrl: string; + stop: () => Promise; +}; + +const cleanupFns: Array<() => Promise> = []; + +afterEach(async () => { + while (cleanupFns.length > 0) { + const cleanup = cleanupFns.pop(); + if (cleanup) { + await cleanup(); + } + } +}); + +async function startWebhookServer(params: { + path: string; + maxBodyBytes: number; + readBody?: (req: import("node:http").IncomingMessage, maxBodyBytes: number) => Promise; +}): Promise { + const { server, start } = createNextcloudTalkWebhookServer({ + port: 0, + host: "127.0.0.1", + path: params.path, + secret: "nextcloud-secret", + maxBodyBytes: params.maxBodyBytes, + readBody: params.readBody, + onMessage: vi.fn(), + }); + await start(); + const address = server.address() as AddressInfo | null; + if (!address) { + throw new Error("missing server address"); + } + return { + webhookUrl: `http://127.0.0.1:${address.port}${params.path}`, + stop: () => + new Promise((resolve) => { + server.close(() => resolve()); + }), + }; +} + +describe("createNextcloudTalkWebhookServer auth order", () => { + it("rejects missing signature headers before reading request body", async () => { + const readBody = vi.fn(async () => { + throw new Error("should not be called for missing signature headers"); + }); + const harness = await startWebhookServer({ + path: "/nextcloud-auth-order", + maxBodyBytes: 128, + readBody, + }); + cleanupFns.push(harness.stop); + + const response = await fetch(harness.webhookUrl, { + method: "POST", + headers: { + "content-type": "application/json", + }, + body: "{}", + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: "Missing signature headers" }); + expect(readBody).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/nextcloud-talk/src/monitor.ts b/extensions/nextcloud-talk/src/monitor.ts index b7daac4d07c..4b68a3c4d0b 100644 --- a/extensions/nextcloud-talk/src/monitor.ts +++ b/extensions/nextcloud-talk/src/monitor.ts @@ -92,6 +92,7 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe opts.maxBodyBytes > 0 ? Math.floor(opts.maxBodyBytes) : DEFAULT_WEBHOOK_MAX_BODY_BYTES; + const readBody = opts.readBody ?? readNextcloudTalkWebhookBody; const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { if (req.url === HEALTH_PATH) { @@ -107,8 +108,6 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe } try { - const body = await readNextcloudTalkWebhookBody(req, maxBodyBytes); - const headers = extractNextcloudTalkHeaders( req.headers as Record, ); @@ -118,6 +117,8 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe return; } + const body = await readBody(req, maxBodyBytes); + const isValid = verifyNextcloudTalkSignature({ signature: headers.signature, random: headers.random, diff --git a/extensions/nextcloud-talk/src/types.ts b/extensions/nextcloud-talk/src/types.ts index ecdbe8437ae..a9fe49be36d 100644 --- a/extensions/nextcloud-talk/src/types.ts +++ b/extensions/nextcloud-talk/src/types.ts @@ -169,6 +169,7 @@ export type NextcloudTalkWebhookServerOptions = { path: string; secret: string; maxBodyBytes?: number; + readBody?: (req: import("node:http").IncomingMessage, maxBodyBytes: number) => Promise; onMessage: (message: NextcloudTalkInboundMessage) => void | Promise; onError?: (error: Error) => void; abortSignal?: AbortSignal;