mirror of https://github.com/openclaw/openclaw.git
fix(matrix): filter fetched room context by sender allowlist (#58376)
* fix(matrix): filter fetched room context by sender allowlist * style(matrix): normalize reply context guard formatting * fix(matrix): drop raw ids from allowlist context logs
This commit is contained in:
parent
6c679e5f04
commit
8a563d603b
|
|
@ -267,4 +267,87 @@ describe("createMatrixRoomMessageHandler inbound body formatting", () => {
|
|||
expect(getEvent).toHaveBeenCalledTimes(1);
|
||||
expect(getMemberDisplayName).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("drops thread and reply context fetched from non-allowlisted room senders", async () => {
|
||||
const { handler, finalizeInboundContext } = createMatrixHandlerTestHarness({
|
||||
client: {
|
||||
getEvent: async () =>
|
||||
createMatrixTextMessageEvent({
|
||||
eventId: "$thread-root",
|
||||
sender: "@mallory:example.org",
|
||||
body: "Malicious root topic",
|
||||
}),
|
||||
},
|
||||
isDirectMessage: false,
|
||||
groupPolicy: "allowlist",
|
||||
groupAllowFrom: ["@alice:example.org"],
|
||||
roomsConfig: { "*": {} },
|
||||
getMemberDisplayName: async (_roomId, userId) =>
|
||||
userId === "@alice:example.org" ? "Alice" : "Mallory",
|
||||
});
|
||||
|
||||
await handler(
|
||||
"!room:example.org",
|
||||
createMatrixTextMessageEvent({
|
||||
eventId: "$reply1",
|
||||
sender: "@alice:example.org",
|
||||
body: "@room follow up",
|
||||
relatesTo: {
|
||||
rel_type: "m.thread",
|
||||
event_id: "$thread-root",
|
||||
"m.in_reply_to": { event_id: "$thread-root" },
|
||||
},
|
||||
mentions: { room: true },
|
||||
}),
|
||||
);
|
||||
|
||||
const finalized = vi.mocked(finalizeInboundContext).mock.calls.at(-1)?.[0] as {
|
||||
ReplyToBody?: string;
|
||||
ReplyToSender?: string;
|
||||
ThreadStarterBody?: string;
|
||||
};
|
||||
expect(finalized.ThreadStarterBody).toBeUndefined();
|
||||
expect(finalized.ReplyToBody).toBeUndefined();
|
||||
expect(finalized.ReplyToSender).toBeUndefined();
|
||||
});
|
||||
|
||||
it("drops quoted reply context fetched from non-allowlisted room senders", async () => {
|
||||
const { handler, finalizeInboundContext } = createMatrixHandlerTestHarness({
|
||||
client: {
|
||||
getEvent: async () =>
|
||||
createMatrixTextMessageEvent({
|
||||
eventId: "$quoted",
|
||||
sender: "@mallory:example.org",
|
||||
body: "Quoted payload",
|
||||
}),
|
||||
},
|
||||
isDirectMessage: false,
|
||||
groupPolicy: "allowlist",
|
||||
groupAllowFrom: ["@alice:example.org"],
|
||||
roomsConfig: { "*": {} },
|
||||
replyToMode: "all",
|
||||
getMemberDisplayName: async (_roomId, userId) =>
|
||||
userId === "@alice:example.org" ? "Alice" : "Mallory",
|
||||
});
|
||||
|
||||
await handler(
|
||||
"!room:example.org",
|
||||
createMatrixTextMessageEvent({
|
||||
eventId: "$reply1",
|
||||
sender: "@alice:example.org",
|
||||
body: "@room follow up",
|
||||
relatesTo: {
|
||||
"m.in_reply_to": { event_id: "$quoted" },
|
||||
},
|
||||
mentions: { room: true },
|
||||
}),
|
||||
);
|
||||
|
||||
const finalized = vi.mocked(finalizeInboundContext).mock.calls.at(-1)?.[0] as {
|
||||
ReplyToBody?: string;
|
||||
ReplyToSender?: string;
|
||||
};
|
||||
expect(finalized.ReplyToBody).toBeUndefined();
|
||||
expect(finalized.ReplyToSender).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import {
|
|||
} from "../send.js";
|
||||
import { resolveMatrixMonitorAccessState } from "./access-state.js";
|
||||
import { resolveMatrixAckReactionConfig } from "./ack-config.js";
|
||||
import { resolveMatrixAllowListMatch } from "./allowlist.js";
|
||||
import type { MatrixInboundEventDeduper } from "./inbound-dedupe.js";
|
||||
import { resolveMatrixLocation, type MatrixLocationPayload } from "./location.js";
|
||||
import { downloadMatrixMedia } from "./media.js";
|
||||
|
|
@ -471,6 +472,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
|
|||
isRoom,
|
||||
});
|
||||
const {
|
||||
effectiveGroupAllowFrom,
|
||||
effectiveRoomUsers,
|
||||
groupAllowConfigured,
|
||||
directAllowMatch,
|
||||
|
|
@ -860,6 +862,8 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
|
|||
triggerSnapshot,
|
||||
threadRootId: _threadRootId,
|
||||
thread,
|
||||
effectiveGroupAllowFrom,
|
||||
effectiveRoomUsers,
|
||||
};
|
||||
};
|
||||
const ingressResult =
|
||||
|
|
@ -910,24 +914,62 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
|
|||
triggerSnapshot,
|
||||
threadRootId: _threadRootId,
|
||||
thread,
|
||||
effectiveGroupAllowFrom,
|
||||
effectiveRoomUsers,
|
||||
} = resolvedIngressResult;
|
||||
|
||||
// Keep the per-room ingress gate focused on ordering-sensitive state updates.
|
||||
// Prompt/session enrichment below can run concurrently after the history snapshot is fixed.
|
||||
const replyToEventId = resolveMatrixReplyToEventId(event.content as RoomMessageEventContent);
|
||||
const threadTarget = thread.threadId;
|
||||
const threadContext = _threadRootId
|
||||
const shouldIncludeRoomContextSender = (contextSenderId?: string): boolean => {
|
||||
if (!isRoom || !contextSenderId) {
|
||||
return true;
|
||||
}
|
||||
if (effectiveRoomUsers.length > 0) {
|
||||
return resolveMatrixAllowListMatch({
|
||||
allowList: effectiveRoomUsers,
|
||||
userId: contextSenderId,
|
||||
}).allowed;
|
||||
}
|
||||
if (groupPolicy === "allowlist" && effectiveGroupAllowFrom.length > 0) {
|
||||
return resolveMatrixAllowListMatch({
|
||||
allowList: effectiveGroupAllowFrom,
|
||||
userId: contextSenderId,
|
||||
}).allowed;
|
||||
}
|
||||
return true;
|
||||
};
|
||||
let threadContext = _threadRootId
|
||||
? await resolveThreadContext({ roomId, threadRootId: _threadRootId })
|
||||
: undefined;
|
||||
const replyContext =
|
||||
replyToEventId && replyToEventId === _threadRootId && threadContext?.summary
|
||||
? {
|
||||
replyToBody: threadContext.summary,
|
||||
replyToSender: threadContext.senderLabel,
|
||||
}
|
||||
: replyToEventId
|
||||
? await resolveReplyContext({ roomId, eventId: replyToEventId })
|
||||
: undefined;
|
||||
let threadContextBlockedByAllowlist = false;
|
||||
if (threadContext?.senderId && !shouldIncludeRoomContextSender(threadContext.senderId)) {
|
||||
logVerboseMessage("matrix: drop thread root context (sender allowlist)");
|
||||
threadContextBlockedByAllowlist = true;
|
||||
threadContext = undefined;
|
||||
}
|
||||
let replyContext: Awaited<ReturnType<typeof resolveReplyContext>> | undefined;
|
||||
if (replyToEventId && replyToEventId === _threadRootId && threadContextBlockedByAllowlist) {
|
||||
replyContext = undefined;
|
||||
} else if (replyToEventId && replyToEventId === _threadRootId && threadContext?.summary) {
|
||||
replyContext = {
|
||||
replyToBody: threadContext.summary,
|
||||
replyToSender: threadContext.senderLabel,
|
||||
replyToSenderId: threadContext.senderId,
|
||||
};
|
||||
} else {
|
||||
replyContext = replyToEventId
|
||||
? await resolveReplyContext({ roomId, eventId: replyToEventId })
|
||||
: undefined;
|
||||
}
|
||||
if (
|
||||
replyContext?.replyToSenderId &&
|
||||
!shouldIncludeRoomContextSender(replyContext.replyToSenderId)
|
||||
) {
|
||||
logVerboseMessage("matrix: drop reply context (sender allowlist)");
|
||||
replyContext = undefined;
|
||||
}
|
||||
const roomInfo = isRoom ? await getRoomInfo(roomId) : undefined;
|
||||
const roomName = roomInfo?.name;
|
||||
const envelopeFrom = isDirectMessage ? senderName : (roomName ?? roomId);
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ describe("matrix reply context", () => {
|
|||
expect(result).toEqual({
|
||||
replyToBody: "This is the original message",
|
||||
replyToSender: "Alice",
|
||||
replyToSenderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
// Second call should use cache
|
||||
|
|
@ -198,6 +199,7 @@ describe("matrix reply context", () => {
|
|||
expect(second).toEqual({
|
||||
replyToBody: "Recovered message",
|
||||
replyToSender: "Bob",
|
||||
replyToSenderId: "@bob:example.org",
|
||||
});
|
||||
|
||||
expect(getEvent).toHaveBeenCalledTimes(2);
|
||||
|
|
@ -231,6 +233,7 @@ describe("matrix reply context", () => {
|
|||
expect(result).toEqual({
|
||||
replyToBody: "Hello",
|
||||
replyToSender: "@charlie:example.org",
|
||||
replyToSenderId: "@charlie:example.org",
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ const MAX_REPLY_BODY_LENGTH = 500;
|
|||
export type MatrixReplyContext = {
|
||||
replyToBody?: string;
|
||||
replyToSender?: string;
|
||||
replyToSenderId?: string;
|
||||
};
|
||||
|
||||
function truncateReplyBody(value: string): string {
|
||||
|
|
@ -85,6 +86,7 @@ export function createMatrixReplyContextResolver(params: {
|
|||
return remember(cacheKey, {
|
||||
replyToBody,
|
||||
replyToSender: senderName ?? senderId,
|
||||
replyToSenderId: senderId,
|
||||
});
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ describe("matrix thread context", () => {
|
|||
}),
|
||||
).resolves.toEqual({
|
||||
threadStarterBody: "Matrix thread root $root from Alice:\nRoot topic",
|
||||
senderId: "@alice:example.org",
|
||||
senderLabel: "Alice",
|
||||
summary: "Root topic",
|
||||
});
|
||||
|
|
@ -115,6 +116,7 @@ describe("matrix thread context", () => {
|
|||
}),
|
||||
).resolves.toEqual({
|
||||
threadStarterBody: "Matrix thread root $root from Alice:\nRecovered topic",
|
||||
senderId: "@alice:example.org",
|
||||
senderLabel: "Alice",
|
||||
summary: "Recovered topic",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ const MAX_THREAD_STARTER_BODY_LENGTH = 500;
|
|||
|
||||
type MatrixThreadContext = {
|
||||
threadStarterBody?: string;
|
||||
senderId?: string;
|
||||
senderLabel?: string;
|
||||
summary?: string;
|
||||
};
|
||||
|
|
@ -99,6 +100,7 @@ export function createMatrixThreadContextResolver(params: {
|
|||
senderName,
|
||||
summary,
|
||||
}),
|
||||
senderId,
|
||||
senderLabel,
|
||||
summary,
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue