Matrix: harden account and direct target resolution

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,

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<ResolvedMatrixAccount> = {
       });
       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(),

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", () => {

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;
   }

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);
 

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", () => {

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<string, string>();
-function setDirectRoomCached(key: string, value: string): void {
+const directRoomCacheByClient = new WeakMap<MatrixClient, Map<string, string>>();
+
+function resolveDirectRoomCache(client: MatrixClient): Map<string, string> {
+  const existing = directRoomCacheByClient.get(client);
+  if (existing) {
+    return existing;
+  }
+  const created = new Map<string, string>();
+  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)`);
 }