fix(msteams): pin reply target at inbound time to prevent DM/channel leak (#54520) (#62716)

extensions/msteams/src/conversation-store-helpers.test.ts

@@ -0,0 +1,202 @@
+import { describe, expect, it } from "vitest";
+import { findPreferredDmConversationByUserId } from "./conversation-store-helpers.js";
+import type { MSTeamsConversationStoreEntry } from "./conversation-store.js";
+
+function entry(params: {
+  conversationId: string;
+  userId?: string;
+  aadObjectId?: string;
+  conversationType?: string;
+  lastSeenAt?: string;
+}): MSTeamsConversationStoreEntry {
+  return {
+    conversationId: params.conversationId,
+    reference: {
+      user: {
+        id: params.userId ?? "user-1",
+        aadObjectId: params.aadObjectId ?? "aad-1",
+      },
+      conversation: {
+        id: params.conversationId,
+        conversationType: params.conversationType,
+      },
+      lastSeenAt: params.lastSeenAt,
+    },
+  };
+}
+
+describe("findPreferredDmConversationByUserId", () => {
+  it("returns null for empty id", () => {
+    expect(findPreferredDmConversationByUserId([], "  ")).toBeNull();
+  });
+
+  it("returns null when no entries match", () => {
+    const entries = [entry({ conversationId: "conv-1", aadObjectId: "other-user" })];
+    expect(findPreferredDmConversationByUserId(entries, "aad-1")).toBeNull();
+  });
+
+  it("returns a personal DM conversation by aadObjectId", () => {
+    const entries = [
+      entry({
+        conversationId: "dm-conv",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("dm-conv");
+  });
+
+  it("returns a personal DM conversation by user.id", () => {
+    const entries = [
+      entry({
+        conversationId: "dm-conv",
+        userId: "user-target",
+        aadObjectId: "other",
+        conversationType: "personal",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "user-target");
+    expect(result?.conversationId).toBe("dm-conv");
+  });
+
+  it("does NOT return a channel conversation for a user lookup (#54520)", () => {
+    // This is the core bug: user sends messages in both a DM and a channel.
+    // The channel conversation also carries the user's aadObjectId.
+    // findPreferredDmByUserId must NOT return the channel conversation.
+    const entries = [
+      entry({
+        conversationId: "19:channel@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "channel",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result).toBeNull();
+  });
+
+  it("does NOT return a groupChat conversation for a user lookup (#54520)", () => {
+    const entries = [
+      entry({
+        conversationId: "19:group@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "groupChat",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result).toBeNull();
+  });
+
+  it("prefers personal DM over channel even when channel is more recent (#54520)", () => {
+    // Reproduces the exact race: channel message arrives after DM, but the
+    // DM conversation should still be returned.
+    const entries = [
+      entry({
+        conversationId: "dm-conv",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+        lastSeenAt: "2026-03-25T20:00:00.000Z",
+      }),
+      entry({
+        conversationId: "19:channel@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "channel",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("dm-conv");
+  });
+
+  it("prefers personal DM over groupChat even when groupChat is more recent", () => {
+    const entries = [
+      entry({
+        conversationId: "dm-conv",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+        lastSeenAt: "2026-03-25T20:00:00.000Z",
+      }),
+      entry({
+        conversationId: "19:group@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "groupChat",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("dm-conv");
+  });
+
+  it("prefers the freshest personal DM when multiple exist", () => {
+    const entries = [
+      entry({
+        conversationId: "dm-old",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+        lastSeenAt: "2026-03-25T20:00:00.000Z",
+      }),
+      entry({
+        conversationId: "dm-new",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("dm-new");
+  });
+
+  it("falls back to unknown-type entries when no personal conversations exist", () => {
+    // Legacy entries without conversationType should still be usable as a
+    // fallback to avoid breaking existing deployments.
+    const entries = [
+      entry({
+        conversationId: "legacy-conv",
+        aadObjectId: "aad-target",
+        // No conversationType set (legacy entry)
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("legacy-conv");
+  });
+
+  it("prefers personal over unknown-type entries", () => {
+    const entries = [
+      entry({
+        conversationId: "legacy-conv",
+        aadObjectId: "aad-target",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+        // No conversationType
+      }),
+      entry({
+        conversationId: "dm-conv",
+        aadObjectId: "aad-target",
+        conversationType: "personal",
+        lastSeenAt: "2026-03-25T20:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result?.conversationId).toBe("dm-conv");
+  });
+
+  it("does NOT fall back to channel/group when no personal or unknown entries exist", () => {
+    const entries = [
+      entry({
+        conversationId: "19:channel@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "channel",
+        lastSeenAt: "2026-03-25T21:00:00.000Z",
+      }),
+      entry({
+        conversationId: "19:group@thread.tacv2",
+        aadObjectId: "aad-target",
+        conversationType: "groupChat",
+        lastSeenAt: "2026-03-25T20:00:00.000Z",
+      }),
+    ];
+    const result = findPreferredDmConversationByUserId(entries, "aad-target");
+    expect(result).toBeNull();
+  });
+});

extensions/msteams/src/conversation-store-helpers.ts

@@ -51,30 +51,45 @@ export function findPreferredDmConversationByUserId(
     return null;
   }
 
-  const matches: MSTeamsConversationStoreEntry[] = [];
+  // Partition user matches into DM-safe and non-DM buckets.
+  // Channel and group conversations also carry the sender's aadObjectId, but
+  // returning one of those when the caller asked for a user-targeted DM would
+  // leak the reply into a shared channel -- the root cause of #54520.
+  const personalMatches: MSTeamsConversationStoreEntry[] = [];
+  const unknownTypeMatches: MSTeamsConversationStoreEntry[] = [];
   for (const entry of entries) {
-    if (entry.reference.user?.aadObjectId === target || entry.reference.user?.id === target) {
-      matches.push(entry);
+    if (entry.reference.user?.aadObjectId !== target && entry.reference.user?.id !== target) {
+      continue;
+    }
+    const convType = normalizeLowercaseStringOrEmpty(
+      entry.reference.conversation?.conversationType ?? "",
+    );
+    if (convType === "personal") {
+      personalMatches.push(entry);
+    } else if (convType === "channel" || convType === "groupchat") {
+      // Explicitly skip channel/group conversations -- these must never be
+      // returned for a user-targeted DM lookup.
+    } else {
+      // Legacy entries without conversationType are ambiguous. Include them
+      // as a fallback but rank below confirmed personal conversations.
+      unknownTypeMatches.push(entry);
     }
   }
 
-  if (matches.length === 0) {
+  // Prefer confirmed personal DMs, fall back to unknown-type entries.
+  const candidates = personalMatches.length > 0 ? personalMatches : unknownTypeMatches;
+  if (candidates.length === 0) {
     return null;
   }
 
-  matches.sort((a, b) => {
-    const aType = normalizeLowercaseStringOrEmpty(a.reference.conversation?.conversationType ?? "");
-    const bType = normalizeLowercaseStringOrEmpty(b.reference.conversation?.conversationType ?? "");
-    const aPersonal = aType === "personal" ? 1 : 0;
-    const bPersonal = bType === "personal" ? 1 : 0;
-    if (aPersonal !== bPersonal) {
-      return bPersonal - aPersonal;
-    }
-    return (
-      (parseStoredConversationTimestamp(b.reference.lastSeenAt) ?? 0) -
-      (parseStoredConversationTimestamp(a.reference.lastSeenAt) ?? 0)
+  // When multiple candidates exist, prefer the most recently seen one.
+  if (candidates.length > 1) {
+    candidates.sort(
+      (a, b) =>
+        (parseStoredConversationTimestamp(b.reference.lastSeenAt) ?? 0) -
+        (parseStoredConversationTimestamp(a.reference.lastSeenAt) ?? 0),
     );
-  });
+  }
 
-  return matches[0] ?? null;
+  return candidates[0] ?? null;
 }

extensions/msteams/src/send-context.ts

@@ -130,6 +130,21 @@ export async function resolveMSTeamsSendContext(params: {
   }
 
   const { conversationId, ref } = found;
+
+  // Safety check: when the caller targeted a specific user (DM), verify the
+  // resolved conversation is actually a personal DM.  Without this guard a
+  // stale or mismatched conversation store could route a private DM reply
+  // into a shared channel or group chat -- see #54520.
+  if (recipient.type === "user") {
+    const resolvedType = normalizeLowercaseStringOrEmpty(ref.conversation?.conversationType ?? "");
+    if (resolvedType && resolvedType !== "personal") {
+      throw new Error(
+        `Conversation reference for user:${recipient.id} resolved to a ${resolvedType} ` +
+          `conversation (${conversationId}) instead of a personal DM. ` +
+          `The bot must receive a DM from this user before it can send proactively.`,
+      );
+    }
+  }
   const core = getMSTeamsRuntime();
   const log = core.logging.getChildLogger({ name: "msteams:send" });