From 8a563d603b70ef6338915f0527bee87282c3bad5 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Tue, 31 Mar 2026 09:09:03 -0700 Subject: [PATCH] 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 --- .../monitor/handler.body-for-agent.test.ts | 83 +++++++++++++++++++ .../matrix/src/matrix/monitor/handler.ts | 62 +++++++++++--- .../src/matrix/monitor/reply-context.test.ts | 3 + .../src/matrix/monitor/reply-context.ts | 2 + .../src/matrix/monitor/thread-context.test.ts | 2 + .../src/matrix/monitor/thread-context.ts | 2 + 6 files changed, 144 insertions(+), 10 deletions(-) diff --git a/extensions/matrix/src/matrix/monitor/handler.body-for-agent.test.ts b/extensions/matrix/src/matrix/monitor/handler.body-for-agent.test.ts index fea06d85522..4308f80ab43 100644 --- a/extensions/matrix/src/matrix/monitor/handler.body-for-agent.test.ts +++ b/extensions/matrix/src/matrix/monitor/handler.body-for-agent.test.ts @@ -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(); + }); }); diff --git a/extensions/matrix/src/matrix/monitor/handler.ts b/extensions/matrix/src/matrix/monitor/handler.ts index d2fa1203660..5e2ec946137 100644 --- a/extensions/matrix/src/matrix/monitor/handler.ts +++ b/extensions/matrix/src/matrix/monitor/handler.ts @@ -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> | 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); diff --git a/extensions/matrix/src/matrix/monitor/reply-context.test.ts b/extensions/matrix/src/matrix/monitor/reply-context.test.ts index 9c5b4a69eff..a2e3fc1a833 100644 --- a/extensions/matrix/src/matrix/monitor/reply-context.test.ts +++ b/extensions/matrix/src/matrix/monitor/reply-context.test.ts @@ -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", }); }); diff --git a/extensions/matrix/src/matrix/monitor/reply-context.ts b/extensions/matrix/src/matrix/monitor/reply-context.ts index 5ae84c1965d..b96a3b001d5 100644 --- a/extensions/matrix/src/matrix/monitor/reply-context.ts +++ b/extensions/matrix/src/matrix/monitor/reply-context.ts @@ -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, }); }; } diff --git a/extensions/matrix/src/matrix/monitor/thread-context.test.ts b/extensions/matrix/src/matrix/monitor/thread-context.test.ts index bfd8e8178bd..54c94e494f8 100644 --- a/extensions/matrix/src/matrix/monitor/thread-context.test.ts +++ b/extensions/matrix/src/matrix/monitor/thread-context.test.ts @@ -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", }); diff --git a/extensions/matrix/src/matrix/monitor/thread-context.ts b/extensions/matrix/src/matrix/monitor/thread-context.ts index df4a5a89b0c..9e7e321af65 100644 --- a/extensions/matrix/src/matrix/monitor/thread-context.ts +++ b/extensions/matrix/src/matrix/monitor/thread-context.ts @@ -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, });