mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 15:30:39 +00:00
fix(security): harden nextcloud-talk webhook replay handling
This commit is contained in:
@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
||||
- Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
|
||||
- Security/Nextcloud Talk: stop treating DM pairing-store entries as group allowlist senders, so group authorization remains bounded to configured group allowlists. (#26116) Thanks @bmendonca3.
|
||||
- Security/IRC: keep pairing-store approvals DM-only and out of IRC group allowlist authorization, with policy regression tests for allowlist resolution. (#26112) Thanks @bmendonca3.
|
||||
|
||||
91
extensions/nextcloud-talk/src/monitor.backend.test.ts
Normal file
91
extensions/nextcloud-talk/src/monitor.backend.test.ts
Normal file
@@ -0,0 +1,91 @@
|
||||
import { type AddressInfo } from "node:net";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createNextcloudTalkWebhookServer } from "./monitor.js";
|
||||
import { generateNextcloudTalkSignature } from "./signature.js";
|
||||
|
||||
type WebhookHarness = {
|
||||
webhookUrl: string;
|
||||
stop: () => Promise<void>;
|
||||
};
|
||||
|
||||
const cleanupFns: Array<() => Promise<void>> = [];
|
||||
|
||||
afterEach(async () => {
|
||||
while (cleanupFns.length > 0) {
|
||||
const cleanup = cleanupFns.pop();
|
||||
if (cleanup) {
|
||||
await cleanup();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
async function startWebhookServer(params: {
|
||||
path: string;
|
||||
isBackendAllowed: (backend: string) => boolean;
|
||||
onMessage: () => void | Promise<void>;
|
||||
}): Promise<WebhookHarness> {
|
||||
const { server, start } = createNextcloudTalkWebhookServer({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
path: params.path,
|
||||
secret: "nextcloud-secret",
|
||||
isBackendAllowed: params.isBackendAllowed,
|
||||
onMessage: params.onMessage,
|
||||
});
|
||||
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<void>((resolve) => {
|
||||
server.close(() => resolve());
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
describe("createNextcloudTalkWebhookServer backend allowlist", () => {
|
||||
it("rejects requests from unexpected backend origins", async () => {
|
||||
const onMessage = vi.fn(async () => {});
|
||||
const harness = await startWebhookServer({
|
||||
path: "/nextcloud-backend-check",
|
||||
isBackendAllowed: (backend) => backend === "https://nextcloud.expected",
|
||||
onMessage,
|
||||
});
|
||||
cleanupFns.push(harness.stop);
|
||||
|
||||
const payload = {
|
||||
type: "Create",
|
||||
actor: { type: "Person", id: "alice", name: "Alice" },
|
||||
object: {
|
||||
type: "Note",
|
||||
id: "msg-1",
|
||||
name: "hello",
|
||||
content: "hello",
|
||||
mediaType: "text/plain",
|
||||
},
|
||||
target: { type: "Collection", id: "room-1", name: "Room 1" },
|
||||
};
|
||||
const body = JSON.stringify(payload);
|
||||
const { random, signature } = generateNextcloudTalkSignature({
|
||||
body,
|
||||
secret: "nextcloud-secret",
|
||||
});
|
||||
const response = await fetch(harness.webhookUrl, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"content-type": "application/json",
|
||||
"x-nextcloud-talk-random": random,
|
||||
"x-nextcloud-talk-signature": signature,
|
||||
"x-nextcloud-talk-backend": "https://nextcloud.unexpected",
|
||||
},
|
||||
body,
|
||||
});
|
||||
|
||||
expect(response.status).toBe(401);
|
||||
expect(await response.json()).toEqual({ error: "Invalid backend" });
|
||||
expect(onMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
115
extensions/nextcloud-talk/src/monitor.replay.test.ts
Normal file
115
extensions/nextcloud-talk/src/monitor.replay.test.ts
Normal file
@@ -0,0 +1,115 @@
|
||||
import { type AddressInfo } from "node:net";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createNextcloudTalkWebhookServer } from "./monitor.js";
|
||||
import { generateNextcloudTalkSignature } from "./signature.js";
|
||||
|
||||
type WebhookHarness = {
|
||||
webhookUrl: string;
|
||||
stop: () => Promise<void>;
|
||||
};
|
||||
|
||||
const cleanupFns: Array<() => Promise<void>> = [];
|
||||
|
||||
afterEach(async () => {
|
||||
while (cleanupFns.length > 0) {
|
||||
const cleanup = cleanupFns.pop();
|
||||
if (cleanup) {
|
||||
await cleanup();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
async function startWebhookServer(params: {
|
||||
path: string;
|
||||
shouldProcessMessage?: (
|
||||
message: Parameters<
|
||||
NonNullable<Parameters<typeof createNextcloudTalkWebhookServer>[0]["onMessage"]>
|
||||
>[0],
|
||||
) => boolean | Promise<boolean>;
|
||||
onMessage: (message: { messageId: string }) => void | Promise<void>;
|
||||
}): Promise<WebhookHarness> {
|
||||
const { server, start } = createNextcloudTalkWebhookServer({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
path: params.path,
|
||||
secret: "nextcloud-secret",
|
||||
shouldProcessMessage: params.shouldProcessMessage,
|
||||
onMessage: params.onMessage,
|
||||
});
|
||||
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<void>((resolve) => {
|
||||
server.close(() => resolve());
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
function createSignedRequest(body: string): { random: string; signature: string } {
|
||||
return generateNextcloudTalkSignature({
|
||||
body,
|
||||
secret: "nextcloud-secret",
|
||||
});
|
||||
}
|
||||
|
||||
describe("createNextcloudTalkWebhookServer replay handling", () => {
|
||||
it("acknowledges replayed requests and skips onMessage side effects", async () => {
|
||||
const seen = new Set<string>();
|
||||
const onMessage = vi.fn(async () => {});
|
||||
const shouldProcessMessage = vi.fn(async (message: { messageId: string }) => {
|
||||
if (seen.has(message.messageId)) {
|
||||
return false;
|
||||
}
|
||||
seen.add(message.messageId);
|
||||
return true;
|
||||
});
|
||||
const harness = await startWebhookServer({
|
||||
path: "/nextcloud-replay",
|
||||
shouldProcessMessage,
|
||||
onMessage,
|
||||
});
|
||||
cleanupFns.push(harness.stop);
|
||||
|
||||
const payload = {
|
||||
type: "Create",
|
||||
actor: { type: "Person", id: "alice", name: "Alice" },
|
||||
object: {
|
||||
type: "Note",
|
||||
id: "msg-1",
|
||||
name: "hello",
|
||||
content: "hello",
|
||||
mediaType: "text/plain",
|
||||
},
|
||||
target: { type: "Collection", id: "room-1", name: "Room 1" },
|
||||
};
|
||||
const body = JSON.stringify(payload);
|
||||
const { random, signature } = createSignedRequest(body);
|
||||
const headers = {
|
||||
"content-type": "application/json",
|
||||
"x-nextcloud-talk-random": random,
|
||||
"x-nextcloud-talk-signature": signature,
|
||||
"x-nextcloud-talk-backend": "https://nextcloud.example",
|
||||
};
|
||||
|
||||
const first = await fetch(harness.webhookUrl, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body,
|
||||
});
|
||||
const second = await fetch(harness.webhookUrl, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body,
|
||||
});
|
||||
|
||||
expect(first.status).toBe(200);
|
||||
expect(second.status).toBe(200);
|
||||
expect(shouldProcessMessage).toHaveBeenCalledTimes(2);
|
||||
expect(onMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http";
|
||||
import os from "node:os";
|
||||
import {
|
||||
createLoggerBackedRuntime,
|
||||
type RuntimeEnv,
|
||||
@@ -8,6 +9,7 @@ import {
|
||||
} from "openclaw/plugin-sdk";
|
||||
import { resolveNextcloudTalkAccount } from "./accounts.js";
|
||||
import { handleNextcloudTalkInbound } from "./inbound.js";
|
||||
import { createNextcloudTalkReplayGuard } from "./replay-guard.js";
|
||||
import { getNextcloudTalkRuntime } from "./runtime.js";
|
||||
import { extractNextcloudTalkHeaders, verifyNextcloudTalkSignature } from "./signature.js";
|
||||
import type {
|
||||
@@ -31,6 +33,14 @@ function formatError(err: unknown): string {
|
||||
return typeof err === "string" ? err : JSON.stringify(err);
|
||||
}
|
||||
|
||||
function normalizeOrigin(value: string): string | null {
|
||||
try {
|
||||
return new URL(value).origin.toLowerCase();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function parseWebhookPayload(body: string): NextcloudTalkWebhookPayload | null {
|
||||
try {
|
||||
const data = JSON.parse(body);
|
||||
@@ -93,6 +103,8 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe
|
||||
? Math.floor(opts.maxBodyBytes)
|
||||
: DEFAULT_WEBHOOK_MAX_BODY_BYTES;
|
||||
const readBody = opts.readBody ?? readNextcloudTalkWebhookBody;
|
||||
const isBackendAllowed = opts.isBackendAllowed;
|
||||
const shouldProcessMessage = opts.shouldProcessMessage;
|
||||
|
||||
const server = createServer(async (req: IncomingMessage, res: ServerResponse) => {
|
||||
if (req.url === HEALTH_PATH) {
|
||||
@@ -116,6 +128,11 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe
|
||||
res.end(JSON.stringify({ error: "Missing signature headers" }));
|
||||
return;
|
||||
}
|
||||
if (isBackendAllowed && !isBackendAllowed(headers.backend)) {
|
||||
res.writeHead(401, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ error: "Invalid backend" }));
|
||||
return;
|
||||
}
|
||||
|
||||
const body = await readBody(req, maxBodyBytes);
|
||||
|
||||
@@ -146,6 +163,14 @@ export function createNextcloudTalkWebhookServer(opts: NextcloudTalkWebhookServe
|
||||
}
|
||||
|
||||
const message = payloadToInboundMessage(payload);
|
||||
if (shouldProcessMessage) {
|
||||
const shouldProcess = await shouldProcessMessage(message);
|
||||
if (!shouldProcess) {
|
||||
res.writeHead(200);
|
||||
res.end();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
res.writeHead(200);
|
||||
res.end();
|
||||
@@ -233,12 +258,41 @@ export async function monitorNextcloudTalkProvider(
|
||||
channel: "nextcloud-talk",
|
||||
accountId: account.accountId,
|
||||
});
|
||||
const expectedBackendOrigin = normalizeOrigin(account.baseUrl);
|
||||
const replayGuard = createNextcloudTalkReplayGuard({
|
||||
stateDir: core.state.resolveStateDir(process.env, os.homedir),
|
||||
onDiskError: (error) => {
|
||||
logger.warn(
|
||||
`[nextcloud-talk:${account.accountId}] replay guard disk error: ${String(error)}`,
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
const { start, stop } = createNextcloudTalkWebhookServer({
|
||||
port,
|
||||
host,
|
||||
path,
|
||||
secret: account.secret,
|
||||
isBackendAllowed: (backend) => {
|
||||
if (!expectedBackendOrigin) {
|
||||
return true;
|
||||
}
|
||||
const backendOrigin = normalizeOrigin(backend);
|
||||
return backendOrigin === expectedBackendOrigin;
|
||||
},
|
||||
shouldProcessMessage: async (message) => {
|
||||
const shouldProcess = await replayGuard.shouldProcessMessage({
|
||||
accountId: account.accountId,
|
||||
roomToken: message.roomToken,
|
||||
messageId: message.messageId,
|
||||
});
|
||||
if (!shouldProcess) {
|
||||
logger.warn(
|
||||
`[nextcloud-talk:${account.accountId}] replayed webhook ignored room=${message.roomToken} messageId=${message.messageId}`,
|
||||
);
|
||||
}
|
||||
return shouldProcess;
|
||||
},
|
||||
onMessage: async (message) => {
|
||||
core.channel.activity.record({
|
||||
channel: "nextcloud-talk",
|
||||
|
||||
70
extensions/nextcloud-talk/src/replay-guard.test.ts
Normal file
70
extensions/nextcloud-talk/src/replay-guard.test.ts
Normal file
@@ -0,0 +1,70 @@
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createNextcloudTalkReplayGuard } from "./replay-guard.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
while (tempDirs.length > 0) {
|
||||
const dir = tempDirs.pop();
|
||||
if (dir) {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
async function makeTempDir(): Promise<string> {
|
||||
const dir = await mkdtemp(path.join(os.tmpdir(), "nextcloud-talk-replay-"));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
describe("createNextcloudTalkReplayGuard", () => {
|
||||
it("persists replay decisions across guard instances", async () => {
|
||||
const stateDir = await makeTempDir();
|
||||
|
||||
const firstGuard = createNextcloudTalkReplayGuard({ stateDir });
|
||||
const firstAttempt = await firstGuard.shouldProcessMessage({
|
||||
accountId: "account-a",
|
||||
roomToken: "room-1",
|
||||
messageId: "msg-1",
|
||||
});
|
||||
const replayAttempt = await firstGuard.shouldProcessMessage({
|
||||
accountId: "account-a",
|
||||
roomToken: "room-1",
|
||||
messageId: "msg-1",
|
||||
});
|
||||
|
||||
const secondGuard = createNextcloudTalkReplayGuard({ stateDir });
|
||||
const restartReplayAttempt = await secondGuard.shouldProcessMessage({
|
||||
accountId: "account-a",
|
||||
roomToken: "room-1",
|
||||
messageId: "msg-1",
|
||||
});
|
||||
|
||||
expect(firstAttempt).toBe(true);
|
||||
expect(replayAttempt).toBe(false);
|
||||
expect(restartReplayAttempt).toBe(false);
|
||||
});
|
||||
|
||||
it("scopes replay state by account namespace", async () => {
|
||||
const stateDir = await makeTempDir();
|
||||
const guard = createNextcloudTalkReplayGuard({ stateDir });
|
||||
|
||||
const accountAFirst = await guard.shouldProcessMessage({
|
||||
accountId: "account-a",
|
||||
roomToken: "room-1",
|
||||
messageId: "msg-9",
|
||||
});
|
||||
const accountBFirst = await guard.shouldProcessMessage({
|
||||
accountId: "account-b",
|
||||
roomToken: "room-1",
|
||||
messageId: "msg-9",
|
||||
});
|
||||
|
||||
expect(accountAFirst).toBe(true);
|
||||
expect(accountBFirst).toBe(true);
|
||||
});
|
||||
});
|
||||
65
extensions/nextcloud-talk/src/replay-guard.ts
Normal file
65
extensions/nextcloud-talk/src/replay-guard.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
import path from "node:path";
|
||||
import { createPersistentDedupe } from "openclaw/plugin-sdk";
|
||||
|
||||
const DEFAULT_REPLAY_TTL_MS = 24 * 60 * 60 * 1000;
|
||||
const DEFAULT_MEMORY_MAX_SIZE = 1_000;
|
||||
const DEFAULT_FILE_MAX_ENTRIES = 10_000;
|
||||
|
||||
function sanitizeSegment(value: string): string {
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) {
|
||||
return "default";
|
||||
}
|
||||
return trimmed.replace(/[^a-zA-Z0-9_-]/g, "_");
|
||||
}
|
||||
|
||||
function buildReplayKey(params: { roomToken: string; messageId: string }): string | null {
|
||||
const roomToken = params.roomToken.trim();
|
||||
const messageId = params.messageId.trim();
|
||||
if (!roomToken || !messageId) {
|
||||
return null;
|
||||
}
|
||||
return `${roomToken}:${messageId}`;
|
||||
}
|
||||
|
||||
export type NextcloudTalkReplayGuardOptions = {
|
||||
stateDir: string;
|
||||
ttlMs?: number;
|
||||
memoryMaxSize?: number;
|
||||
fileMaxEntries?: number;
|
||||
onDiskError?: (error: unknown) => void;
|
||||
};
|
||||
|
||||
export type NextcloudTalkReplayGuard = {
|
||||
shouldProcessMessage: (params: {
|
||||
accountId: string;
|
||||
roomToken: string;
|
||||
messageId: string;
|
||||
}) => Promise<boolean>;
|
||||
};
|
||||
|
||||
export function createNextcloudTalkReplayGuard(
|
||||
options: NextcloudTalkReplayGuardOptions,
|
||||
): NextcloudTalkReplayGuard {
|
||||
const stateDir = options.stateDir.trim();
|
||||
const persistentDedupe = createPersistentDedupe({
|
||||
ttlMs: options.ttlMs ?? DEFAULT_REPLAY_TTL_MS,
|
||||
memoryMaxSize: options.memoryMaxSize ?? DEFAULT_MEMORY_MAX_SIZE,
|
||||
fileMaxEntries: options.fileMaxEntries ?? DEFAULT_FILE_MAX_ENTRIES,
|
||||
resolveFilePath: (namespace) =>
|
||||
path.join(stateDir, "nextcloud-talk", "replay-dedupe", `${sanitizeSegment(namespace)}.json`),
|
||||
});
|
||||
|
||||
return {
|
||||
shouldProcessMessage: async ({ accountId, roomToken, messageId }) => {
|
||||
const replayKey = buildReplayKey({ roomToken, messageId });
|
||||
if (!replayKey) {
|
||||
return true;
|
||||
}
|
||||
return await persistentDedupe.checkAndRecord(replayKey, {
|
||||
namespace: accountId,
|
||||
onDiskError: options.onDiskError,
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -170,6 +170,8 @@ export type NextcloudTalkWebhookServerOptions = {
|
||||
secret: string;
|
||||
maxBodyBytes?: number;
|
||||
readBody?: (req: import("node:http").IncomingMessage, maxBodyBytes: number) => Promise<string>;
|
||||
isBackendAllowed?: (backend: string) => boolean;
|
||||
shouldProcessMessage?: (message: NextcloudTalkInboundMessage) => boolean | Promise<boolean>;
|
||||
onMessage: (message: NextcloudTalkInboundMessage) => void | Promise<void>;
|
||||
onError?: (error: Error) => void;
|
||||
abortSignal?: AbortSignal;
|
||||
|
||||
Reference in New Issue
Block a user