mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 03:45:46 -06:00
fix(nextcloud-talk): dispose webhook auth rate limiter on monitor stop (#126908)
* fix(nextcloud-talk): dispose webhook auth rate limiter on monitor stop The monitor created its webhook auth rate limiter with a prune interval but never called dispose(), leaking one interval plus entry maps per stop/start cycle (boot, config hot reload, health-monitor restart). dispose() is idempotent and callers own the timer lifecycle, matching how core gateway callers release their limiters on shutdown. * fix(nextcloud-talk): release webhook limiter in all lifecycle owners Preserve the contributor stop-time limiter disposal while routing the shared webhook harness and both direct-listener fixtures through the same canonical stop owner, including failure cleanup. Co-authored-by: wangmiao0668000666 <wang.miao86@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
committed by
GitHub
parent
ec091478fc
commit
c61bc221b0
@@ -0,0 +1,44 @@
|
||||
// Nextcloud Talk plugin module implements monitor limiter lifecycle behavior.
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createNextcloudTalkWebhookServer } from "./monitor.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
describe("Nextcloud Talk webhook auth rate limiter lifecycle", () => {
|
||||
it("releases the limiter prune timer on stop", async () => {
|
||||
vi.useFakeTimers();
|
||||
const baselineTimerCount = vi.getTimerCount();
|
||||
const handle = createNextcloudTalkWebhookServer({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
path: "/w",
|
||||
secret: "s",
|
||||
onWebhook: async () => "ignored",
|
||||
});
|
||||
expect(vi.getTimerCount()).toBe(baselineTimerCount + 1);
|
||||
|
||||
await handle.stop();
|
||||
|
||||
expect(vi.getTimerCount()).toBe(baselineTimerCount);
|
||||
});
|
||||
|
||||
it("keeps stop idempotent for the limiter timer", async () => {
|
||||
vi.useFakeTimers();
|
||||
const baselineTimerCount = vi.getTimerCount();
|
||||
const handle = createNextcloudTalkWebhookServer({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
path: "/w",
|
||||
secret: "s",
|
||||
onWebhook: async () => "ignored",
|
||||
});
|
||||
expect(vi.getTimerCount()).toBe(baselineTimerCount + 1);
|
||||
|
||||
await handle.stop();
|
||||
await handle.stop();
|
||||
|
||||
expect(vi.getTimerCount()).toBe(baselineTimerCount);
|
||||
});
|
||||
});
|
||||
@@ -64,7 +64,7 @@ async function invokeWebhookServerRequest(params: {
|
||||
headers: Record<string, string>;
|
||||
maxBodyBytes: number;
|
||||
}) {
|
||||
const { server } = createNextcloudTalkWebhookServer({
|
||||
const { server, stop } = createNextcloudTalkWebhookServer({
|
||||
host: "127.0.0.1",
|
||||
port: 0,
|
||||
path: "/nextcloud-body-limit",
|
||||
@@ -72,19 +72,23 @@ async function invokeWebhookServerRequest(params: {
|
||||
maxBodyBytes: params.maxBodyBytes,
|
||||
onMessage: vi.fn(),
|
||||
});
|
||||
const listener = server.listeners("request")[0] as
|
||||
| ((req: IncomingMessage, res: ServerResponse) => void)
|
||||
| undefined;
|
||||
if (!listener) {
|
||||
throw new Error("expected Nextcloud Talk request listener");
|
||||
try {
|
||||
const listener = server.listeners("request")[0] as
|
||||
| ((req: IncomingMessage, res: ServerResponse) => void)
|
||||
| undefined;
|
||||
if (!listener) {
|
||||
throw new Error("expected Nextcloud Talk request listener");
|
||||
}
|
||||
return await invokeWebhookRequestListener({
|
||||
listener,
|
||||
path: "/nextcloud-body-limit",
|
||||
body: params.body,
|
||||
headers: params.headers,
|
||||
remoteAddress: "127.0.0.1",
|
||||
});
|
||||
} finally {
|
||||
await stop();
|
||||
}
|
||||
return await invokeWebhookRequestListener({
|
||||
listener,
|
||||
path: "/nextcloud-body-limit",
|
||||
body: params.body,
|
||||
headers: params.headers,
|
||||
remoteAddress: "127.0.0.1",
|
||||
});
|
||||
}
|
||||
|
||||
describe("createNextcloudTalkWebhookServer auth order", () => {
|
||||
@@ -386,7 +390,7 @@ describe("createNextcloudTalkWebhookServer auth rate limiting", () => {
|
||||
|
||||
it("keeps unattributed trusted proxies in separate socket buckets", async () => {
|
||||
const path = "/nextcloud-auth-rate-limit-proxy-fallback";
|
||||
const { server } = createNextcloudTalkWebhookServer({
|
||||
const { server, stop } = createNextcloudTalkWebhookServer({
|
||||
host: "127.0.0.1",
|
||||
port: 0,
|
||||
path,
|
||||
@@ -395,33 +399,37 @@ describe("createNextcloudTalkWebhookServer auth rate limiting", () => {
|
||||
trustedProxies: ["127.0.0.0/8"],
|
||||
onMessage: vi.fn(),
|
||||
});
|
||||
const listener = server.listeners("request")[0] as
|
||||
| ((req: IncomingMessage, res: ServerResponse) => void)
|
||||
| undefined;
|
||||
if (!listener) {
|
||||
throw new Error("expected Nextcloud Talk request listener");
|
||||
try {
|
||||
const listener = server.listeners("request")[0] as
|
||||
| ((req: IncomingMessage, res: ServerResponse) => void)
|
||||
| undefined;
|
||||
if (!listener) {
|
||||
throw new Error("expected Nextcloud Talk request listener");
|
||||
}
|
||||
const { body, headers } = createSignedCreateMessageRequest();
|
||||
const invalidHeaders = {
|
||||
...headers,
|
||||
"x-nextcloud-talk-signature": "invalid-signature",
|
||||
};
|
||||
const invoke = (remoteAddress: string, requestHeaders: Record<string, string>) =>
|
||||
invokeWebhookRequestListener({
|
||||
listener,
|
||||
path,
|
||||
body,
|
||||
headers: requestHeaders,
|
||||
remoteAddress,
|
||||
});
|
||||
|
||||
const firstAttack = await invoke("127.0.0.2", invalidHeaders);
|
||||
const blockedAttack = await invoke("127.0.0.2", invalidHeaders);
|
||||
const legitimateDelivery = await invoke("127.0.0.3", headers);
|
||||
|
||||
expect(firstAttack.status).toBe(401);
|
||||
expect(blockedAttack.status).toBe(429);
|
||||
expect(legitimateDelivery.status).toBe(200);
|
||||
} finally {
|
||||
await stop();
|
||||
}
|
||||
const { body, headers } = createSignedCreateMessageRequest();
|
||||
const invalidHeaders = {
|
||||
...headers,
|
||||
"x-nextcloud-talk-signature": "invalid-signature",
|
||||
};
|
||||
const invoke = (remoteAddress: string, requestHeaders: Record<string, string>) =>
|
||||
invokeWebhookRequestListener({
|
||||
listener,
|
||||
path,
|
||||
body,
|
||||
headers: requestHeaders,
|
||||
remoteAddress,
|
||||
});
|
||||
|
||||
const firstAttack = await invoke("127.0.0.2", invalidHeaders);
|
||||
const blockedAttack = await invoke("127.0.0.2", invalidHeaders);
|
||||
const legitimateDelivery = await invoke("127.0.0.3", headers);
|
||||
|
||||
expect(firstAttack.status).toBe(401);
|
||||
expect(blockedAttack.status).toBe(429);
|
||||
expect(legitimateDelivery.status).toBe(200);
|
||||
});
|
||||
|
||||
it("does not rate limit valid signed webhook bursts from the same source", async () => {
|
||||
|
||||
@@ -61,7 +61,7 @@ export async function startWebhookServer(
|
||||
const host = params.host ?? "127.0.0.1";
|
||||
const port = params.port ?? 0;
|
||||
const secret = params.secret ?? "nextcloud-secret";
|
||||
const { server, start } = createNextcloudTalkWebhookServer({
|
||||
const { server, start, stop } = createNextcloudTalkWebhookServer({
|
||||
...params,
|
||||
port,
|
||||
host,
|
||||
@@ -75,10 +75,7 @@ export async function startWebhookServer(
|
||||
|
||||
const harness: WebhookHarness = {
|
||||
webhookUrl: `http://${host}:${address.port}${params.path}`,
|
||||
stop: () =>
|
||||
new Promise<void>((resolve) => {
|
||||
server.close(() => resolve());
|
||||
}),
|
||||
stop,
|
||||
};
|
||||
cleanupFns.push(harness.stop);
|
||||
return harness;
|
||||
|
||||
@@ -230,6 +230,7 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe
|
||||
const stop = async () => {
|
||||
stopRequested = true;
|
||||
await closeIfListening();
|
||||
webhookAuthRateLimiter.dispose();
|
||||
};
|
||||
|
||||
const start = (): Promise<void> => {
|
||||
|
||||
Reference in New Issue
Block a user