diff --git a/extensions/matrix/src/channel.directory.test.ts b/extensions/matrix/src/channel.directory.test.ts index d8b1190e362..7b27a0cf2f8 100644 --- a/extensions/matrix/src/channel.directory.test.ts +++ b/extensions/matrix/src/channel.directory.test.ts @@ -438,6 +438,74 @@ describe("matrix directory", () => { } }); + it("clears stored auth fields when switching a Matrix account to env-backed auth", () => { + const envKeys = { + MATRIX_OPS_HOMESERVER: process.env.MATRIX_OPS_HOMESERVER, + MATRIX_OPS_ACCESS_TOKEN: process.env.MATRIX_OPS_ACCESS_TOKEN, + MATRIX_OPS_DEVICE_ID: process.env.MATRIX_OPS_DEVICE_ID, + MATRIX_OPS_DEVICE_NAME: process.env.MATRIX_OPS_DEVICE_NAME, + }; + process.env.MATRIX_OPS_HOMESERVER = "https://ops.env.example.org"; + process.env.MATRIX_OPS_ACCESS_TOKEN = "ops-env-token"; + process.env.MATRIX_OPS_DEVICE_ID = "OPSENVDEVICE"; + process.env.MATRIX_OPS_DEVICE_NAME = "Ops Env Device"; + + try { + const cfg = { + channels: { + matrix: { + accounts: { + ops: { + homeserver: "https://ops.inline.example.org", + userId: "@ops:inline.example.org", + accessToken: "ops-inline-token", + password: "ops-inline-password", // pragma: allowlist secret + deviceId: "OPSINLINEDEVICE", + deviceName: "Ops Inline Device", + encryption: true, + }, + }, + }, + }, + } as unknown as CoreConfig; + + const updated = matrixPlugin.setup!.applyAccountConfig({ + cfg, + accountId: "ops", + input: { + useEnv: true, + name: "Ops", + }, + }) as CoreConfig; + + expect(updated.channels?.["matrix"]?.accounts?.ops).toMatchObject({ + name: "Ops", + enabled: true, + encryption: true, + }); + expect(updated.channels?.["matrix"]?.accounts?.ops?.homeserver).toBeUndefined(); + expect(updated.channels?.["matrix"]?.accounts?.ops?.userId).toBeUndefined(); + expect(updated.channels?.["matrix"]?.accounts?.ops?.accessToken).toBeUndefined(); + expect(updated.channels?.["matrix"]?.accounts?.ops?.password).toBeUndefined(); + expect(updated.channels?.["matrix"]?.accounts?.ops?.deviceId).toBeUndefined(); + expect(updated.channels?.["matrix"]?.accounts?.ops?.deviceName).toBeUndefined(); + expect(resolveMatrixConfigForAccount(updated, "ops", process.env)).toMatchObject({ + homeserver: "https://ops.env.example.org", + accessToken: "ops-env-token", + deviceId: "OPSENVDEVICE", + deviceName: "Ops Env Device", + }); + } finally { + for (const [key, value] of Object.entries(envKeys)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + } + }); + it("resolves account id from input name when explicit account id is missing", () => { const accountId = matrixPlugin.setup!.resolveAccountId?.({ cfg: {} as CoreConfig, diff --git a/extensions/matrix/src/channel.ts b/extensions/matrix/src/channel.ts index 909211742bd..465aaa8c6bb 100644 --- a/extensions/matrix/src/channel.ts +++ b/extensions/matrix/src/channel.ts @@ -6,6 +6,7 @@ import { formatPairingApproveHint, moveSingleAccountChannelSectionToDefaultAccount, normalizeAccountId, + normalizeSecretInputString, PAIRING_APPROVED_MESSAGE, resolveAllowlistProviderRuntimeGroupPolicy, resolveDefaultGroupPolicy, @@ -331,16 +332,18 @@ export const matrixPlugin: ChannelPlugin = { }); const next = namedConfig as CoreConfig; if (input.useEnv) { - return setAccountEnabledInConfigSection({ - cfg: next as CoreConfig, - sectionKey: "matrix", - accountId, + return updateMatrixAccountConfig(next, accountId, { enabled: true, - allowTopLevel: true, - }) as CoreConfig; + homeserver: null, + userId: null, + accessToken: null, + password: null, + deviceId: null, + deviceName: null, + }); } const accessToken = input.accessToken?.trim(); - const password = input.password?.trim(); + const password = normalizeSecretInputString(input.password); const userId = input.userId?.trim(); return updateMatrixAccountConfig(next as CoreConfig, accountId, { homeserver: input.homeserver?.trim(), diff --git a/extensions/matrix/src/matrix/client.test.ts b/extensions/matrix/src/matrix/client.test.ts index d67eb1e917d..b3092f60a24 100644 --- a/extensions/matrix/src/matrix/client.test.ts +++ b/extensions/matrix/src/matrix/client.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { CoreConfig } from "../types.js"; import { + resolveImplicitMatrixAccountId, resolveMatrixAuth, resolveMatrixAuthContext, resolveMatrixConfig, @@ -128,6 +129,23 @@ describe("resolveMatrixConfig", () => { encryption: true, }); }); + + it("ignores typoed defaultAccount values that do not map to a real Matrix account", () => { + const cfg = { + channels: { + matrix: { + defaultAccount: "ops", + homeserver: "https://legacy.example.org", + accessToken: "legacy-token", + }, + }, + } as CoreConfig; + + expect(resolveImplicitMatrixAccountId(cfg, {} as NodeJS.ProcessEnv)).toBeNull(); + expect(resolveMatrixAuthContext({ cfg, env: {} as NodeJS.ProcessEnv }).accountId).toBe( + "default", + ); + }); }); describe("resolveMatrixAuth", () => { diff --git a/extensions/matrix/src/matrix/client/config.ts b/extensions/matrix/src/matrix/client/config.ts index c4712f1316a..cb2283375fa 100644 --- a/extensions/matrix/src/matrix/client/config.ts +++ b/extensions/matrix/src/matrix/client/config.ts @@ -288,15 +288,15 @@ export function resolveImplicitMatrixAccountId( cfg: CoreConfig, env: NodeJS.ProcessEnv = process.env, ): string | null { + const accountIds = listNormalizedMatrixAccountIds(cfg); const configuredDefault = normalizeOptionalAccountId(cfg.channels?.matrix?.defaultAccount); - if (configuredDefault) { + if (configuredDefault && accountIds.includes(configuredDefault)) { const resolved = resolveMatrixConfigForAccount(cfg, configuredDefault, env); if (hasMatrixAuthInputs(resolved)) { return configuredDefault; } } - const accountIds = listNormalizedMatrixAccountIds(cfg); if (accountIds.length === 0) { return null; } diff --git a/extensions/matrix/src/matrix/config-update.ts b/extensions/matrix/src/matrix/config-update.ts index dc6d194ea6f..27035bfbe41 100644 --- a/extensions/matrix/src/matrix/config-update.ts +++ b/extensions/matrix/src/matrix/config-update.ts @@ -9,6 +9,7 @@ export type MatrixAccountPatch = { userId?: string | null; accessToken?: string | null; password?: string | null; + deviceId?: string | null; deviceName?: string | null; avatarUrl?: string | null; encryption?: boolean | null; @@ -138,6 +139,7 @@ export function updateMatrixAccountConfig( applyNullableStringField(nextAccount, "userId", patch.userId); applyNullableStringField(nextAccount, "accessToken", patch.accessToken); applyNullableStringField(nextAccount, "password", patch.password); + applyNullableStringField(nextAccount, "deviceId", patch.deviceId); applyNullableStringField(nextAccount, "deviceName", patch.deviceName); applyNullableStringField(nextAccount, "avatarUrl", patch.avatarUrl); diff --git a/extensions/matrix/src/matrix/send/targets.test.ts b/extensions/matrix/src/matrix/send/targets.test.ts index 9a60393b7f8..d53f15ed0d9 100644 --- a/extensions/matrix/src/matrix/send/targets.test.ts +++ b/extensions/matrix/src/matrix/send/targets.test.ts @@ -72,7 +72,7 @@ describe("resolveMatrixRoomId", () => { expect(setAccountData).toHaveBeenCalled(); }); - it("allows larger rooms when no 1:1 match exists", async () => { + it("does not fall back to larger shared rooms for direct-user sends", async () => { const userId = "@group:example.org"; const roomId = "!group:example.org"; const client = { @@ -84,9 +84,11 @@ describe("resolveMatrixRoomId", () => { setAccountData: vi.fn().mockResolvedValue(undefined), } as unknown as MatrixClient; - const resolved = await resolveMatrixRoomId(client, userId); - - expect(resolved).toBe(roomId); + await expect(resolveMatrixRoomId(client, userId)).rejects.toThrow( + `No direct room found for ${userId} (m.direct missing)`, + ); + // oxlint-disable-next-line typescript/unbound-method + expect(client.setAccountData).not.toHaveBeenCalled(); }); it("accepts nested Matrix user target prefixes", async () => { @@ -108,6 +110,36 @@ describe("resolveMatrixRoomId", () => { // oxlint-disable-next-line typescript/unbound-method expect(client.resolveRoom).not.toHaveBeenCalled(); }); + + it("scopes direct-room cache per Matrix client", async () => { + const userId = "@shared:example.org"; + const clientA = { + getAccountData: vi.fn().mockResolvedValue({ + [userId]: ["!room-a:example.org"], + }), + getJoinedRooms: vi.fn(), + getJoinedRoomMembers: vi.fn(), + setAccountData: vi.fn(), + resolveRoom: vi.fn(), + } as unknown as MatrixClient; + const clientB = { + getAccountData: vi.fn().mockResolvedValue({ + [userId]: ["!room-b:example.org"], + }), + getJoinedRooms: vi.fn(), + getJoinedRoomMembers: vi.fn(), + setAccountData: vi.fn(), + resolveRoom: vi.fn(), + } as unknown as MatrixClient; + + await expect(resolveMatrixRoomId(clientA, userId)).resolves.toBe("!room-a:example.org"); + await expect(resolveMatrixRoomId(clientB, userId)).resolves.toBe("!room-b:example.org"); + + // oxlint-disable-next-line typescript/unbound-method + expect(clientA.getAccountData).toHaveBeenCalledTimes(1); + // oxlint-disable-next-line typescript/unbound-method + expect(clientB.getAccountData).toHaveBeenCalledTimes(1); + }); }); describe("normalizeThreadId", () => { diff --git a/extensions/matrix/src/matrix/send/targets.ts b/extensions/matrix/src/matrix/send/targets.ts index c459a066be4..66498f993e3 100644 --- a/extensions/matrix/src/matrix/send/targets.ts +++ b/extensions/matrix/src/matrix/send/targets.ts @@ -20,8 +20,20 @@ export function normalizeThreadId(raw?: string | number | null): string | null { // Size-capped to prevent unbounded growth (#4948) const MAX_DIRECT_ROOM_CACHE_SIZE = 1024; -const directRoomCache = new Map(); -function setDirectRoomCached(key: string, value: string): void { +const directRoomCacheByClient = new WeakMap>(); + +function resolveDirectRoomCache(client: MatrixClient): Map { + const existing = directRoomCacheByClient.get(client); + if (existing) { + return existing; + } + const created = new Map(); + directRoomCacheByClient.set(client, created); + return created; +} + +function setDirectRoomCached(client: MatrixClient, key: string, value: string): void { + const directRoomCache = resolveDirectRoomCache(client); directRoomCache.set(key, value); if (directRoomCache.size > MAX_DIRECT_ROOM_CACHE_SIZE) { const oldest = directRoomCache.keys().next().value; @@ -66,6 +78,7 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis throw new Error(`Matrix user IDs must be fully qualified (got "${trimmed}")`); } + const directRoomCache = resolveDirectRoomCache(client); const cached = directRoomCache.get(trimmed); if (cached) { return cached; @@ -79,16 +92,15 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis >; const list = Array.isArray(directContent?.[trimmed]) ? directContent[trimmed] : []; if (list && list.length > 0) { - setDirectRoomCached(trimmed, list[0]); + setDirectRoomCached(client, trimmed, list[0]); return list[0]; } } catch { // Ignore and fall back. } - // 2) Fallback: look for an existing joined room that looks like a 1:1 with the user. + // 2) Fallback: look for an existing joined room that is actually a 1:1 with the user. // Many clients only maintain m.direct for *their own* account data, so relying on it is brittle. - let fallbackRoom: string | null = null; try { const rooms = await client.getJoinedRooms(); for (const roomId of rooms) { @@ -101,26 +113,16 @@ async function resolveDirectRoomId(client: MatrixClient, userId: string): Promis if (!members.includes(trimmed)) { continue; } - // Prefer classic 1:1 rooms, but allow larger rooms if requested. if (members.length === 2) { - setDirectRoomCached(trimmed, roomId); + setDirectRoomCached(client, trimmed, roomId); await persistDirectRoom(client, trimmed, roomId); return roomId; } - if (!fallbackRoom) { - fallbackRoom = roomId; - } } } catch { // Ignore and fall back. } - if (fallbackRoom) { - setDirectRoomCached(trimmed, fallbackRoom); - await persistDirectRoom(client, trimmed, fallbackRoom); - return fallbackRoom; - } - throw new Error(`No direct room found for ${trimmed} (m.direct missing)`); }