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

* Webhooks: tighten pre-auth body handling

* Webhooks: clean up request body guards
This commit is contained in:
Vincent Koc 2026-03-15 09:45:18 -07:00 committed by GitHub
parent 7679eb3752
commit 5e78c8bc95
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 64 additions and 24 deletions

View File

@ -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.

View File

@ -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",
});

View File

@ -1,7 +1,7 @@
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";
@ -9,6 +9,7 @@ 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();
}
});
});

View File

@ -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;

View File

@ -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";

View File

@ -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,
});
}

View File

@ -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) {

View File

@ -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";