mirror of https://github.com/openclaw/openclaw.git
Webhooks: tighten pre-auth body handling
This commit is contained in:
parent
db20141993
commit
0ba757fc41
|
|
@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Configure/startup: move outbound send-deps resolution into a lightweight helper so `openclaw configure` no longer stalls after the banner while eagerly loading channel plugins. (#46301) thanks @scoootscooob.
|
||||
- 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)
|
||||
- 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.
|
||||
|
||||
### Fixes
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -228,7 +222,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;
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
|
|
@ -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,8 @@ export function readNextcloudTalkWebhookBody(
|
|||
maxBodyBytes: number,
|
||||
): Promise<string> {
|
||||
return readRequestBodyWithLimit(req, {
|
||||
maxBytes: maxBodyBytes,
|
||||
timeoutMs: DEFAULT_WEBHOOK_BODY_TIMEOUT_MS,
|
||||
maxBytes: Math.min(maxBodyBytes, PREAUTH_WEBHOOK_MAX_BODY_BYTES),
|
||||
timeoutMs: PREAUTH_WEBHOOK_BODY_TIMEOUT_MS,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
Loading…
Reference in New Issue