From 738af608b747f8448802d9ef277d091b6c8b0bf6 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Thu, 12 Mar 2026 09:10:50 +0000 Subject: [PATCH] Matrix: dedupe strict DM trust checks --- docs/channels/matrix.md | 5 +- docs/concepts/session.md | 2 + extensions/matrix/src/matrix/direct-room.ts | 66 +++++++++++++++++ .../matrix/src/matrix/monitor/direct.test.ts | 71 ++++++++++++------- .../matrix/src/matrix/monitor/direct.ts | 30 ++++---- .../matrix/monitor/handler.test-helpers.ts | 1 + .../matrix/src/matrix/monitor/handler.ts | 4 +- extensions/matrix/src/matrix/monitor/index.ts | 5 ++ .../src/matrix/monitor/verification-events.ts | 29 ++------ extensions/matrix/src/matrix/send/targets.ts | 30 ++------ 10 files changed, 152 insertions(+), 91 deletions(-) create mode 100644 extensions/matrix/src/matrix/direct-room.ts diff --git a/docs/channels/matrix.md b/docs/channels/matrix.md index a7c797e88b5..ea3ddc2ecf1 100644 --- a/docs/channels/matrix.md +++ b/docs/channels/matrix.md @@ -61,6 +61,7 @@ Wizard behavior that matters: - When you add another Matrix account interactively, the entered account name is normalized into the account ID used in config and env vars. For example, `Ops Bot` becomes `ops-bot`. - DM allowlist prompts accept full `@user:server` values immediately. Display names only work when live directory lookup finds one exact match; otherwise the wizard asks you to retry with a full Matrix ID. - Room allowlist prompts accept room IDs and aliases directly. They can also resolve joined-room names live, but unresolved names are only kept as typed during setup and are ignored later by runtime allowlist resolution. Prefer `!room:server` or `#alias:server`. +- Runtime room/session identity uses the stable Matrix room ID. Room-declared aliases are only used as lookup inputs, not as the long-term session key or group channel identifier. - To resolve room names before saving them, use `openclaw channels resolve --channel matrix "Project Room"`. Minimal token-based setup: @@ -575,10 +576,10 @@ Live directory lookup uses the logged-in Matrix account: - `reactionNotifications`: inbound reaction notification mode (`own`, `off`). - `mediaMaxMb`: outbound media size cap in MB. - `autoJoin`: invite auto-join policy (`always`, `allowlist`, `off`). Default: `off`. -- `autoJoinAllowlist`: rooms/aliases allowed when `autoJoin` is `allowlist`. +- `autoJoinAllowlist`: rooms/aliases allowed when `autoJoin` is `allowlist`. Alias entries are resolved to room IDs during invite handling; OpenClaw does not trust alias state claimed by the invited room. - `dm`: DM policy block (`enabled`, `policy`, `allowFrom`). - `dm.allowFrom` entries should be full Matrix user IDs unless you already resolved them through live directory lookup. - `accounts`: named per-account overrides. Top-level `channels.matrix` values act as defaults for these entries. -- `groups`: per-room policy map. Prefer room IDs or aliases; unresolved room names are ignored at runtime. +- `groups`: per-room policy map. Prefer room IDs or aliases; unresolved room names are ignored at runtime. Session/group metadata uses the stable room ID after resolution. - `rooms`: legacy alias for `groups`. - `actions`: per-action tool gating (`messages`, `reactions`, `pins`, `memberInfo`, `channelInfo`, `verification`). diff --git a/docs/concepts/session.md b/docs/concepts/session.md index 6c9010d2c11..71a557de606 100644 --- a/docs/concepts/session.md +++ b/docs/concepts/session.md @@ -308,3 +308,5 @@ Each session entry records where it came from (best-effort) in `origin`: `GroupSubject`, `GroupChannel`, `GroupSpace`, and `SenderName` in the inbound context and calling `recordSessionMetaFromInbound` (or passing the same context to `updateLastRoute`). + `GroupChannel` should carry the stable provider-side channel identity when one + exists. For example, Matrix now uses the room ID instead of room-declared aliases. diff --git a/extensions/matrix/src/matrix/direct-room.ts b/extensions/matrix/src/matrix/direct-room.ts new file mode 100644 index 00000000000..a25004dbeb1 --- /dev/null +++ b/extensions/matrix/src/matrix/direct-room.ts @@ -0,0 +1,66 @@ +import type { MatrixClient } from "./sdk.js"; + +function trimMaybeString(value: unknown): string | null { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +export function normalizeJoinedMatrixMembers(joinedMembers: unknown): string[] { + if (!Array.isArray(joinedMembers)) { + return []; + } + return joinedMembers + .map((entry) => trimMaybeString(entry)) + .filter((entry): entry is string => Boolean(entry)); +} + +export function isStrictDirectMembership(params: { + selfUserId?: string | null; + remoteUserId?: string | null; + joinedMembers?: readonly string[] | null; +}): boolean { + const selfUserId = trimMaybeString(params.selfUserId); + const remoteUserId = trimMaybeString(params.remoteUserId); + const joinedMembers = params.joinedMembers ?? []; + return Boolean( + selfUserId && + remoteUserId && + joinedMembers.length === 2 && + joinedMembers.includes(selfUserId) && + joinedMembers.includes(remoteUserId), + ); +} + +export async function readJoinedMatrixMembers( + client: MatrixClient, + roomId: string, +): Promise { + try { + return normalizeJoinedMatrixMembers(await client.getJoinedRoomMembers(roomId)); + } catch { + return null; + } +} + +export async function isStrictDirectRoom(params: { + client: MatrixClient; + roomId: string; + remoteUserId: string; + selfUserId?: string | null; +}): Promise { + const selfUserId = + trimMaybeString(params.selfUserId) ?? + trimMaybeString(await params.client.getUserId().catch(() => null)); + if (!selfUserId) { + return false; + } + const joinedMembers = await readJoinedMatrixMembers(params.client, params.roomId); + return isStrictDirectMembership({ + selfUserId, + remoteUserId: params.remoteUserId, + joinedMembers, + }); +} diff --git a/extensions/matrix/src/matrix/monitor/direct.test.ts b/extensions/matrix/src/matrix/monitor/direct.test.ts index 5ed4a67d687..e7250683a97 100644 --- a/extensions/matrix/src/matrix/monitor/direct.test.ts +++ b/extensions/matrix/src/matrix/monitor/direct.test.ts @@ -1,13 +1,8 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { MatrixClient } from "../sdk.js"; import { createDirectRoomTracker } from "./direct.js"; -function createMockClient(params: { - isDm?: boolean; - senderDirect?: boolean; - selfDirect?: boolean; - members?: string[]; -}) { +function createMockClient(params: { isDm?: boolean; members?: string[] }) { let members = params.members ?? ["@alice:example.org", "@bot:example.org"]; return { dms: { @@ -16,24 +11,24 @@ function createMockClient(params: { }, getUserId: vi.fn().mockResolvedValue("@bot:example.org"), getJoinedRoomMembers: vi.fn().mockImplementation(async () => members), - getRoomStateEvent: vi - .fn() - .mockImplementation(async (_roomId: string, eventType: string, stateKey: string) => { - if (stateKey === "@alice:example.org") { - return { is_direct: params.senderDirect === true }; - } - if (stateKey === "@bot:example.org") { - return { is_direct: params.selfDirect === true }; - } - return {}; - }), __setMembers(next: string[]) { members = next; }, - } as unknown as MatrixClient; + } as unknown as MatrixClient & { + dms: { + update: ReturnType; + isDm: ReturnType; + }; + getJoinedRoomMembers: ReturnType; + __setMembers: (members: string[]) => void; + }; } describe("createDirectRoomTracker", () => { + afterEach(() => { + vi.useRealTimers(); + }); + it("treats m.direct rooms as DMs", async () => { const tracker = createDirectRoomTracker(createMockClient({ isDm: true })); await expect( @@ -112,11 +107,7 @@ describe("createDirectRoomTracker", () => { }), ).resolves.toBe(true); - (client as MatrixClient & { __setMembers: (members: string[]) => void }).__setMembers([ - "@alice:example.org", - "@bot:example.org", - "@mallory:example.org", - ]); + client.__setMembers(["@alice:example.org", "@bot:example.org", "@mallory:example.org"]); tracker.invalidateRoom("!room:example.org"); @@ -129,7 +120,7 @@ describe("createDirectRoomTracker", () => { }); it("still recognizes exact 2-member rooms when member state also claims is_direct", async () => { - const tracker = createDirectRoomTracker(createMockClient({ senderDirect: true })); + const tracker = createDirectRoomTracker(createMockClient({})); await expect( tracker.isDirectMessage({ roomId: "!room:example.org", @@ -141,7 +132,6 @@ describe("createDirectRoomTracker", () => { it("ignores member-state is_direct when the room is not a strict DM", async () => { const tracker = createDirectRoomTracker( createMockClient({ - senderDirect: true, members: ["@alice:example.org", "@bot:example.org", "@observer:example.org"], }), ); @@ -171,4 +161,33 @@ describe("createDirectRoomTracker", () => { expect(client.getJoinedRoomMembers).toHaveBeenCalledTimes(1026); }); + + it("refreshes dm and membership caches after the ttl expires", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-12T10:00:00Z")); + const client = createMockClient({ isDm: true }); + const tracker = createDirectRoomTracker(client); + + await tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }); + await tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }); + + expect(client.dms.update).toHaveBeenCalledTimes(1); + expect(client.getJoinedRoomMembers).toHaveBeenCalledTimes(1); + + vi.setSystemTime(new Date("2026-03-12T10:00:31Z")); + + await tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }); + + expect(client.dms.update).toHaveBeenCalledTimes(2); + expect(client.getJoinedRoomMembers).toHaveBeenCalledTimes(2); + }); }); diff --git a/extensions/matrix/src/matrix/monitor/direct.ts b/extensions/matrix/src/matrix/monitor/direct.ts index adc4f74a85c..c40967a05d6 100644 --- a/extensions/matrix/src/matrix/monitor/direct.ts +++ b/extensions/matrix/src/matrix/monitor/direct.ts @@ -1,3 +1,4 @@ +import { isStrictDirectMembership, readJoinedMatrixMembers } from "../direct-room.js"; import type { MatrixClient } from "../sdk.js"; type DirectMessageCheck = { @@ -61,11 +62,10 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr return cached.members; } try { - const members = await client.getJoinedRoomMembers(roomId); - const normalized = members - .filter((entry): entry is string => typeof entry === "string") - .map((entry) => entry.trim()) - .filter(Boolean); + const normalized = await readJoinedMatrixMembers(client, roomId); + if (!normalized) { + throw new Error("membership unavailable"); + } rememberBounded(joinedMembersCache, roomId, { members: normalized, ts: now }); return normalized; } catch (err) { @@ -88,11 +88,11 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr if (client.dms.isDm(roomId)) { const directViaAccountData = Boolean( - selfUserId && - senderId?.trim() && - joinedMembers?.length === 2 && - joinedMembers.includes(selfUserId) && - joinedMembers.includes(senderId.trim()), + isStrictDirectMembership({ + selfUserId, + remoteUserId: senderId, + joinedMembers, + }), ); if (directViaAccountData) { log(`matrix: dm detected via m.direct room=${roomId}`); @@ -102,11 +102,11 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr } if ( - selfUserId && - senderId?.trim() && - joinedMembers?.length === 2 && - joinedMembers.includes(selfUserId) && - joinedMembers.includes(senderId.trim()) + isStrictDirectMembership({ + selfUserId, + remoteUserId: senderId, + joinedMembers, + }) ) { log(`matrix: dm detected via exact 2-member room room=${roomId}`); return true; diff --git a/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts b/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts index 7a548755e5b..3517f00ac33 100644 --- a/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts +++ b/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts @@ -177,6 +177,7 @@ export function createMatrixHandlerTestHarness( }, getRoomInfo: options.getRoomInfo ?? (async () => ({ altAliases: [] })), getMemberDisplayName: options.getMemberDisplayName ?? (async () => "sender"), + needsRoomAliasesForConfig: false, }); return { diff --git a/extensions/matrix/src/matrix/monitor/handler.ts b/extensions/matrix/src/matrix/monitor/handler.ts index 17e90b6fd53..4d11647123a 100644 --- a/extensions/matrix/src/matrix/monitor/handler.ts +++ b/extensions/matrix/src/matrix/monitor/handler.ts @@ -79,6 +79,7 @@ export type MatrixMonitorHandlerParams = { opts?: { includeAliases?: boolean }, ) => Promise<{ name?: string; canonicalAlias?: string; altAliases: string[] }>; getMemberDisplayName: (roomId: string, userId: string) => Promise; + needsRoomAliasesForConfig: boolean; }; function resolveMatrixMentionPrecheckText(params: { @@ -126,6 +127,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam directTracker, getRoomInfo, getMemberDisplayName, + needsRoomAliasesForConfig, } = params; let cachedStoreAllowFrom: { value: string[]; @@ -263,7 +265,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam } const roomInfoForConfig = - isRoom && roomsConfig && Object.keys(roomsConfig).some((key) => key.trim().startsWith("#")) + isRoom && needsRoomAliasesForConfig ? await getRoomInfo(roomId, { includeAliases: true }) : undefined; const roomAliasesForConfig = roomInfoForConfig diff --git a/extensions/matrix/src/matrix/monitor/index.ts b/extensions/matrix/src/matrix/monitor/index.ts index 49ea6f99376..1a24c92afb5 100644 --- a/extensions/matrix/src/matrix/monitor/index.ts +++ b/extensions/matrix/src/matrix/monitor/index.ts @@ -83,6 +83,7 @@ export async function monitorMatrixProvider(opts: MonitorMatrixOpts = {}): Promi let allowFrom: string[] = (accountConfig.dm?.allowFrom ?? []).map(String); let groupAllowFrom: string[] = (accountConfig.groupAllowFrom ?? []).map(String); let roomsConfig = accountConfig.groups ?? accountConfig.rooms; + let needsRoomAliasesForConfig = false; ({ allowFrom, groupAllowFrom, roomsConfig } = await resolveMatrixMonitorConfig({ cfg, @@ -92,6 +93,9 @@ export async function monitorMatrixProvider(opts: MonitorMatrixOpts = {}): Promi roomsConfig, runtime, })); + needsRoomAliasesForConfig = Boolean( + roomsConfig && Object.keys(roomsConfig).some((key) => key.trim().startsWith("#")), + ); cfg = { ...cfg, @@ -193,6 +197,7 @@ export async function monitorMatrixProvider(opts: MonitorMatrixOpts = {}): Promi directTracker, getRoomInfo, getMemberDisplayName, + needsRoomAliasesForConfig, }); const threadBindingManager = await createMatrixThreadBindingManager({ diff --git a/extensions/matrix/src/matrix/monitor/verification-events.ts b/extensions/matrix/src/matrix/monitor/verification-events.ts index 0d31f17d2e4..fe6588773cc 100644 --- a/extensions/matrix/src/matrix/monitor/verification-events.ts +++ b/extensions/matrix/src/matrix/monitor/verification-events.ts @@ -1,3 +1,4 @@ +import { isStrictDirectRoom } from "../direct-room.js"; import type { MatrixClient } from "../sdk.js"; import type { MatrixRawEvent } from "./types.js"; import { EventType } from "./types.js"; @@ -161,26 +162,6 @@ function resolveSummaryRecency(summary: MatrixVerificationSummaryLike): number { return Number.isFinite(ts) ? ts : 0; } -async function isStrictDirectVerificationRoom(params: { - client: MatrixClient; - roomId: string; - senderId: string; -}): Promise { - const selfUserId = trimMaybeString(await params.client.getUserId().catch(() => null)); - if (!selfUserId) { - return false; - } - const joinedMembers = await params.client.getJoinedRoomMembers(params.roomId).catch(() => null); - if (!Array.isArray(joinedMembers) || joinedMembers.length !== 2) { - return false; - } - const normalizedMembers = joinedMembers - .filter((entry): entry is string => typeof entry === "string") - .map((entry) => entry.trim()) - .filter(Boolean); - return normalizedMembers.includes(selfUserId) && normalizedMembers.includes(params.senderId); -} - async function resolveVerificationSummaryForSignal( client: MatrixClient, params: { @@ -212,10 +193,10 @@ async function resolveVerificationSummaryForSignal( // spoofed verification event in an unrelated room can leak the current SAS // prompt into that room. if ( - !(await isStrictDirectVerificationRoom({ + !(await isStrictDirectRoom({ client, roomId: params.roomId, - senderId: params.senderId, + remoteUserId: params.senderId, })) ) { return null; @@ -285,10 +266,10 @@ 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({ + const shouldRouteInRoom = await isStrictDirectRoom({ client: params.client, roomId, - senderId, + remoteUserId: senderId, }); if (!shouldRouteInRoom) { params.logVerboseMessage( diff --git a/extensions/matrix/src/matrix/send/targets.ts b/extensions/matrix/src/matrix/send/targets.ts index e71da63117c..e71e6d958a4 100644 --- a/extensions/matrix/src/matrix/send/targets.ts +++ b/extensions/matrix/src/matrix/send/targets.ts @@ -1,3 +1,4 @@ +import { isStrictDirectRoom } from "../direct-room.js"; import type { MatrixClient } from "../sdk.js"; import { isMatrixQualifiedUserId, normalizeMatrixResolvableTarget } from "../target-ids.js"; import { EventType, type MatrixDirectAccountData } from "./types.js"; @@ -43,26 +44,6 @@ function setDirectRoomCached(client: MatrixClient, key: string, value: string): } } -async function isStrictDirectRoom( - client: MatrixClient, - roomId: string, - remoteUserId: string, - selfUserId: string | null, -): Promise { - if (!selfUserId) { - return false; - } - let members: string[]; - try { - members = await client.getJoinedRoomMembers(roomId); - } catch { - return false; - } - return ( - members.length === 2 && members.includes(remoteUserId.trim()) && members.includes(selfUserId) - ); -} - async function persistDirectRoom( client: MatrixClient, userId: string, @@ -101,7 +82,10 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis const directRoomCache = resolveDirectRoomCache(client); const cached = directRoomCache.get(trimmed); - if (cached && (await isStrictDirectRoom(client, cached, trimmed, selfUserId))) { + if ( + cached && + (await isStrictDirectRoom({ client, roomId: cached, remoteUserId: trimmed, selfUserId })) + ) { return cached; } if (cached) { @@ -116,7 +100,7 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis >; const list = Array.isArray(directContent?.[trimmed]) ? directContent[trimmed] : []; for (const roomId of list) { - if (await isStrictDirectRoom(client, roomId, trimmed, selfUserId)) { + if (await isStrictDirectRoom({ client, roomId, remoteUserId: trimmed, selfUserId })) { setDirectRoomCached(client, trimmed, roomId); return roomId; } @@ -130,7 +114,7 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis try { const rooms = await client.getJoinedRooms(); for (const roomId of rooms) { - if (await isStrictDirectRoom(client, roomId, trimmed, selfUserId)) { + if (await isStrictDirectRoom({ client, roomId, remoteUserId: trimmed, selfUserId })) { setDirectRoomCached(client, trimmed, roomId); await persistDirectRoom(client, trimmed, roomId); return roomId;