Webhooks: tighten pre-auth body handling (#46802)

vincentkoc · web-flow · commit 5e78c8bc95d8 · 2026-03-15T09:45:18.000-07:00
* Webhooks: tighten pre-auth body handling

* Webhooks: clean up request body guards
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai
 - Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146)
 - Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts.
 - Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`.
+- Webhooks/runtime: move auth earlier and tighten pre-auth body limits and timeouts across bundled webhook handlers, including slow-body handling for Mattermost slash commands. Thanks @vincentkoc.
 - Subagents/follow-ups: require the same controller ownership checks for `/subagents send` as other control actions, so leaf sessions cannot message nested child runs they do not control. Thanks @vincentkoc.
 - Inbound policy hardening: tighten callback and webhook sender checks across Mattermost and Google Chat, match Nextcloud Talk rooms by stable room token, and treat explicit empty Twitch allowlists as deny-all. (#46787) Thanks @zpbrent, @ijxpwastaken and @vincentkoc.
 - macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. (#46790) Thanks @vincentkoc.
diff --git a/extensions/googlechat/src/monitor-webhook.ts b/extensions/googlechat/src/monitor-webhook.ts
@@ -21,6 +21,9 @@ function extractBearerToken(header: unknown): string {
     : "";
 }
 
+const ADD_ON_PREAUTH_MAX_BYTES = 16 * 1024;
+const ADD_ON_PREAUTH_TIMEOUT_MS = 3_000;
+
 type ParsedGoogleChatInboundPayload =
   | { ok: true; event: GoogleChatEvent; addOnBearerToken: string }
   | { ok: false };
@@ -112,6 +115,12 @@ export function createGoogleChatWebhookRequestHandler(params: {
             req,
             res,
             profile,
+            ...(profile === "pre-auth"
+              ? {
+                  maxBytes: ADD_ON_PREAUTH_MAX_BYTES,
+                  timeoutMs: ADD_ON_PREAUTH_TIMEOUT_MS,
+                }
+              : {}),
             emptyObjectOnEmpty: false,
             invalidJsonMessage: "invalid payload",
           });
diff --git a/extensions/mattermost/src/mattermost/slash-http.test.ts b/extensions/mattermost/src/mattermost/slash-http.test.ts
@@ -1,14 +1,15 @@
 import type { IncomingMessage, ServerResponse } from "node:http";
 import { PassThrough } from "node:stream";
 import type { OpenClawConfig, RuntimeEnv } from "openclaw/plugin-sdk/mattermost";
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import type { ResolvedMattermostAccount } from "./accounts.js";
 import { createSlashCommandHttpHandler } from "./slash-http.js";
 
 function createRequest(params: {
   method?: string;
   body?: string;
   contentType?: string;
+  autoEnd?: boolean;
 }): IncomingMessage {
   const req = new PassThrough();
   const incoming = req as unknown as IncomingMessage;
@@ -20,7 +21,9 @@ function createRequest(params: {
     if (params.body) {
       req.write(params.body);
     }
-    req.end();
+    if (params.autoEnd !== false) {
+      req.end();
+    }
   });
   return incoming;
 }
@@ -128,4 +131,27 @@ describe("slash-http", () => {
     expect(response.res.statusCode).toBe(401);
     expect(response.getBody()).toContain("Unauthorized: invalid command token.");
   });
+
+  it("returns 408 when the request body stalls", async () => {
+    vi.useFakeTimers();
+    try {
+      const handler = createSlashCommandHttpHandler({
+        account: accountFixture,
+        cfg: {} as OpenClawConfig,
+        runtime: {} as RuntimeEnv,
+        commandTokens: new Set(["valid-token"]),
+      });
+      const req = createRequest({ autoEnd: false });
+      const response = createResponse();
+      const pending = handler(req, response.res);
+
+      await vi.advanceTimersByTimeAsync(5_000);
+      await pending;
+
+      expect(response.res.statusCode).toBe(408);
+      expect(response.getBody()).toBe("Request body timeout");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
 });
diff --git a/extensions/mattermost/src/mattermost/slash-http.ts b/extensions/mattermost/src/mattermost/slash-http.ts
@@ -10,7 +10,9 @@ import {
   buildModelsProviderData,
   createReplyPrefixOptions,
   createTypingCallbacks,
+  isRequestBodyLimitError,
   logTypingFailure,
+  readRequestBodyWithLimit,
   type OpenClawConfig,
   type ReplyPayload,
   type RuntimeEnv,
@@ -54,24 +56,16 @@ type SlashHttpHandlerParams = {
   log?: (msg: string) => void;
 };
 
+const MAX_BODY_BYTES = 64 * 1024;
+const BODY_READ_TIMEOUT_MS = 5_000;
+
 /**
  * Read the full request body as a string.
  */
 function readBody(req: IncomingMessage, maxBytes: number): Promise<string> {
-  return new Promise((resolve, reject) => {
-    const chunks: Buffer[] = [];
-    let size = 0;
-    req.on("data", (chunk: Buffer) => {
-      size += chunk.length;
-      if (size > maxBytes) {
-        req.destroy();
-        reject(new Error("Request body too large"));
-        return;
-      }
-      chunks.push(chunk);
-    });
-    req.on("end", () => resolve(Buffer.concat(chunks).toString("utf8")));
-    req.on("error", reject);
+  return readRequestBodyWithLimit(req, {
+    maxBytes,
+    timeoutMs: BODY_READ_TIMEOUT_MS,
   });
 }
 
@@ -215,8 +209,6 @@ async function authorizeSlashInvocation(params: {
 export function createSlashCommandHttpHandler(params: SlashHttpHandlerParams) {
   const { account, cfg, runtime, commandTokens, triggerMap, log } = params;
 
-  const MAX_BODY_BYTES = 64 * 1024; // 64KB
-
   return async (req: IncomingMessage, res: ServerResponse): Promise<void> => {
     if (req.method !== "POST") {
       res.statusCode = 405;
@@ -228,7 +220,12 @@ export function createSlashCommandHttpHandler(params: SlashHttpHandlerParams) {
     let body: string;
     try {
       body = await readBody(req, MAX_BODY_BYTES);
-    } catch {
+    } catch (error) {
+      if (isRequestBodyLimitError(error, "REQUEST_BODY_TIMEOUT")) {
+        res.statusCode = 408;
+        res.end("Request body timeout");
+        return;
+      }
       res.statusCode = 413;
       res.end("Payload Too Large");
       return;
diff --git a/extensions/msteams/src/monitor.ts b/extensions/msteams/src/monitor.ts
@@ -269,6 +269,7 @@ export async function monitorMSTeamsProvider(
 
   // Create Express server
   const expressApp = express.default();
+  expressApp.use(authorizeJWT(authConfig));
   expressApp.use(express.json({ limit: MSTEAMS_WEBHOOK_MAX_BODY_BYTES }));
   expressApp.use((err: unknown, _req: Request, res: Response, next: (err?: unknown) => void) => {
     if (err && typeof err === "object" && "status" in err && err.status === 413) {
@@ -277,7 +278,6 @@ export async function monitorMSTeamsProvider(
     }
     next(err);
   });
-  expressApp.use(authorizeJWT(authConfig));
 
   // Set up the messages endpoint - use configured path and /api/messages as fallback
   const configuredPath = msteamsCfg.webhook?.path ?? "/api/messages";
diff --git a/extensions/nextcloud-talk/src/monitor.ts b/extensions/nextcloud-talk/src/monitor.ts
@@ -25,6 +25,8 @@ const DEFAULT_WEBHOOK_HOST = "0.0.0.0";
 const DEFAULT_WEBHOOK_PATH = "/nextcloud-talk-webhook";
 const DEFAULT_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024;
 const DEFAULT_WEBHOOK_BODY_TIMEOUT_MS = 30_000;
+const PREAUTH_WEBHOOK_MAX_BODY_BYTES = 64 * 1024;
+const PREAUTH_WEBHOOK_BODY_TIMEOUT_MS = 5_000;
 const HEALTH_PATH = "/healthz";
 const WEBHOOK_ERRORS = {
   missingSignatureHeaders: "Missing signature headers",
@@ -171,8 +173,10 @@ export function readNextcloudTalkWebhookBody(
   maxBodyBytes: number,
 ): Promise<string> {
   return readRequestBodyWithLimit(req, {
-    maxBytes: maxBodyBytes,
-    timeoutMs: DEFAULT_WEBHOOK_BODY_TIMEOUT_MS,
+    // This read happens before signature verification, so keep the unauthenticated
+    // body budget bounded even if the operator-configured post-parse limit is larger.
+    maxBytes: Math.min(maxBodyBytes, PREAUTH_WEBHOOK_MAX_BODY_BYTES),
+    timeoutMs: PREAUTH_WEBHOOK_BODY_TIMEOUT_MS,
   });
 }
 
diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts
@@ -16,6 +16,8 @@ import type { SynologyWebhookPayload, ResolvedSynologyChatAccount } from "./type
 
 // One rate limiter per account, created lazily
 const rateLimiters = new Map<string, RateLimiter>();
+const PREAUTH_MAX_BODY_BYTES = 64 * 1024;
+const PREAUTH_BODY_TIMEOUT_MS = 5_000;
 
 function getRateLimiter(account: ResolvedSynologyChatAccount): RateLimiter {
   let rl = rateLimiters.get(account.accountId);
@@ -49,8 +51,8 @@ async function readBody(req: IncomingMessage): Promise<
 > {
   try {
     const body = await readRequestBodyWithLimit(req, {
-      maxBytes: 1_048_576,
-      timeoutMs: 30_000,
+      maxBytes: PREAUTH_MAX_BODY_BYTES,
+      timeoutMs: PREAUTH_BODY_TIMEOUT_MS,
     });
     return { ok: true, body };
   } catch (err) {
diff --git a/src/plugin-sdk/mattermost.ts b/src/plugin-sdk/mattermost.ts
@@ -104,3 +104,4 @@ export { buildAgentMediaPayload } from "./agent-media-payload.js";
 export { getAgentScopedMediaLocalRoots } from "../media/local-roots.js";
 export { loadOutboundMediaFromUrl } from "./outbound-media.js";
 export { createScopedPairingAccess } from "./pairing-access.js";
+export { isRequestBodyLimitError, readRequestBodyWithLimit } from "../infra/http-body.js";
