From f6742e909ad452f5d63f347599c31fbf6d2aea99 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Thu, 12 Mar 2026 04:40:55 +0000 Subject: [PATCH] Matrix: tighten direct room trust --- .../matrix/src/matrix/monitor/direct.test.ts | 15 +++++++++++ .../matrix/src/matrix/monitor/direct.ts | 27 ++++++++++++------- .../matrix/src/matrix/monitor/events.test.ts | 15 +++-------- .../src/matrix/monitor/verification-events.ts | 11 ++++++++ 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/extensions/matrix/src/matrix/monitor/direct.test.ts b/extensions/matrix/src/matrix/monitor/direct.test.ts index dfd272cdbc5..9e8fcb2a030 100644 --- a/extensions/matrix/src/matrix/monitor/direct.test.ts +++ b/extensions/matrix/src/matrix/monitor/direct.test.ts @@ -41,6 +41,21 @@ describe("createDirectRoomTracker", () => { ).resolves.toBe(true); }); + it("does not trust stale m.direct classifications for shared rooms", async () => { + const tracker = createDirectRoomTracker( + createMockClient({ + isDm: true, + members: ["@alice:example.org", "@bot:example.org", "@extra:example.org"], + }), + ); + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(false); + }); + it("classifies 2-member rooms as DMs when direct metadata is missing", async () => { const client = createMockClient({ isDm: false }); const tracker = createDirectRoomTracker(client); diff --git a/extensions/matrix/src/matrix/monitor/direct.ts b/extensions/matrix/src/matrix/monitor/direct.ts index 2a2ae2a9769..38518673655 100644 --- a/extensions/matrix/src/matrix/monitor/direct.ts +++ b/extensions/matrix/src/matrix/monitor/direct.ts @@ -67,21 +67,30 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr isDirectMessage: async (params: DirectMessageCheck): Promise => { const { roomId, senderId } = params; await refreshDmCache(); - - if (client.dms.isDm(roomId)) { - log(`matrix: dm detected via m.direct room=${roomId}`); - return true; - } - const selfUserId = params.selfUserId ?? (await ensureSelfUserId()); const joinedMembers = await resolveJoinedMembers(roomId); - const normalizedSenderId = senderId?.trim(); + + if (client.dms.isDm(roomId)) { + const directViaAccountData = Boolean( + selfUserId && + senderId?.trim() && + joinedMembers?.length === 2 && + joinedMembers.includes(selfUserId) && + joinedMembers.includes(senderId.trim()), + ); + if (directViaAccountData) { + log(`matrix: dm detected via m.direct room=${roomId}`); + return true; + } + log(`matrix: ignoring stale m.direct classification room=${roomId}`); + } + if ( selfUserId && - normalizedSenderId && + senderId?.trim() && joinedMembers?.length === 2 && joinedMembers.includes(selfUserId) && - joinedMembers.includes(normalizedSenderId) + joinedMembers.includes(senderId.trim()) ) { log(`matrix: dm detected via exact 2-member room room=${roomId}`); return true; diff --git a/extensions/matrix/src/matrix/monitor/events.test.ts b/extensions/matrix/src/matrix/monitor/events.test.ts index b144721f8d2..af352953d43 100644 --- a/extensions/matrix/src/matrix/monitor/events.test.ts +++ b/extensions/matrix/src/matrix/monitor/events.test.ts @@ -47,7 +47,8 @@ function createHarness(params?: { sendMessage, getUserId: vi.fn(async () => params?.selfUserId ?? "@bot:example.org"), getJoinedRoomMembers: vi.fn( - async (roomId: string) => params?.joinedMembersByRoom?.[roomId] ?? [], + async (roomId: string) => + params?.joinedMembersByRoom?.[roomId] ?? ["@bot:example.org", "@alice:example.org"], ), ...(params?.cryptoAvailable === false ? {} @@ -203,7 +204,7 @@ describe("registerMatrixMonitorEvents verification routing", () => { }); }); - it("does not leak SAS details into unrelated non-DM rooms when flow ids do not match", async () => { + it("ignores verification notices in unrelated non-DM rooms", async () => { const { sendMessage, roomEventListener } = createHarness({ joinedMembersByRoom: { "!group:example.org": ["@alice:example.org", "@bot:example.org", "@ops:example.org"], @@ -237,16 +238,8 @@ describe("registerMatrixMonitorEvents verification routing", () => { }); await vi.waitFor(() => { - expect(sendMessage).toHaveBeenCalledTimes(1); + expect(sendMessage).toHaveBeenCalledTimes(0); }); - expect(getSentNoticeBody(sendMessage, 0)).toContain( - "Matrix verification started with @alice:example.org.", - ); - expect( - (sendMessage.mock.calls as unknown[][]).some((call) => - String((call[1] as { body?: string } | undefined)?.body ?? "").includes("SAS emoji:"), - ), - ).toBe(false); }); it("does not emit duplicate SAS notices for the same verification payload", async () => { diff --git a/extensions/matrix/src/matrix/monitor/verification-events.ts b/extensions/matrix/src/matrix/monitor/verification-events.ts index 776cec17fe5..0d31f17d2e4 100644 --- a/extensions/matrix/src/matrix/monitor/verification-events.ts +++ b/extensions/matrix/src/matrix/monitor/verification-events.ts @@ -285,6 +285,17 @@ export function createMatrixVerificationEventRouter(params: { const flowId = signal.flowId; const sourceEventId = trimMaybeString(event?.event_id); const sourceFingerprint = sourceEventId ?? `${senderId}:${event.type}:${flowId ?? "none"}`; + const shouldRouteInRoom = await isStrictDirectVerificationRoom({ + client: params.client, + roomId, + senderId, + }); + if (!shouldRouteInRoom) { + params.logVerboseMessage( + `matrix: ignoring verification event outside strict DM room=${roomId} sender=${senderId}`, + ); + return; + } if (!trackBounded(routedVerificationEvents, sourceFingerprint)) { return; }