From e84a0dde175e0cbec3480c70de4e970a95152e1a Mon Sep 17 00:00:00 2001 From: velanir-ai-manager Date: Thu, 9 Jul 2026 04:20:54 -0700 Subject: [PATCH] fix(msteams): surface quoted message body in Teams quote replies (#101856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(msteams): surface quoted message body in Teams quote replies Teams sends the quoted text of a 1:1 DM quote-reply in

(a truncated snippet), not

which the parser matched. The match failed, so quoteInfo was undefined and nothing was surfaced to the agent. Fix 1 (primary): extractMSTeamsQuoteInfo now accepts copy OR preview (prefers copy, falls back to preview) and captures the blockquote itemid as the quoted message id. Fix 2 (enhancement): when the quoted message id is known, fetch the full text via the app-only Graph endpoint GET /chats/{chatId}/messages/{id} (permitted with Chat.Read.All, unlike the delegated /me/chats listing) and use it as the quote body. Restricted to chats (DM + group) whose Graph chat id is a 19: id; any failure degrades to the truncated preview so message handling never breaks. Co-Authored-By: Claude Opus 4.8 * fix(msteams): address review — DM-only full-text fetch, drop unsupported $select, fix mocks Addresses ClawSweeper review on the quote-reply PR: - Security (P1): restrict the app-only Graph full-text quote fetch to 1:1 DMs. Previously it ran for any non-channel chat, so in a group an allowlisted sender could quote a non-allowlisted member and the fetched full body would bypass the supplemental-quote visibility allowlist. Group/channel quotes keep the (now-surfaced) truncated preview from fix 1. - Graph contract (P2): the get-chatMessage endpoint does not support OData query params; drop the ?$select=id,body that tenants enforcing the contract would reject (which would silently fall back to the preview). - Tests (P1): add fetchChatMessageText to the two vi.mock(../graph-thread.js) factories prod now touches, and add a group-chat regression test proving the Graph full-text fetch does not fire for group quotes. Co-Authored-By: Claude Opus 4.8 * docs(msteams): add redacted real Teams quote-reply proof screenshots Real-behavior evidence for the quote-reply fix (personal names + avatars redacted): a 1:1 DM where the bot now reads back the full quoted message. Co-Authored-By: Claude Opus 4.8 * docs(msteams): remove committed proof screenshots from branch Proof media should live as external PR artifacts, not in the product tree. Co-Authored-By: Claude Opus 4.8 * test(msteams): bound and prove quote enrichment * test(msteams): use valid open DM fixture --------- Co-authored-by: Yash Inani Co-authored-by: Claude Opus 4.8 Co-authored-by: Peter Steinberger --- extensions/msteams/src/graph-thread.test.ts | 48 ++++++++++++ extensions/msteams/src/graph-thread.ts | 30 ++++++++ extensions/msteams/src/graph.test.ts | 32 ++++++-- extensions/msteams/src/graph.ts | 3 + extensions/msteams/src/inbound.test.ts | 67 ++++++++++++++++ extensions/msteams/src/inbound.ts | 22 +++++- .../message-handler.authz.test.ts | 77 +++++++++++++++++++ .../message-handler.thread-parent.test.ts | 2 + .../src/monitor-handler/message-handler.ts | 26 ++++++- 9 files changed, 297 insertions(+), 10 deletions(-) diff --git a/extensions/msteams/src/graph-thread.test.ts b/extensions/msteams/src/graph-thread.test.ts index 77164b1c4697..d936d01546fa 100644 --- a/extensions/msteams/src/graph-thread.test.ts +++ b/extensions/msteams/src/graph-thread.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { _teamGroupIdCacheForTest, fetchChannelMessage, + fetchChatMessageText, fetchThreadReplies, formatThreadContext, resolveTeamGroupId, @@ -161,6 +162,53 @@ describe("fetchChannelMessage", () => { }); }); +describe("fetchChatMessageText", () => { + beforeEach(() => { + vi.mocked(fetchGraphJson).mockReset(); + }); + + it("fetches the chat message and strips HTML body to plain text", async () => { + vi.mocked(fetchGraphJson).mockResolvedValueOnce({ + id: "1783379480258", + body: { + content: "

San Francisco right now: Bot full text

", + contentType: "html", + }, + } as never); + + const result = await fetchChatMessageText("tok", "19:chat@thread.v2", "1783379480258"); + + expect(result).toBe("San Francisco right now: @Bot full text"); + expect(fetchGraphJson).toHaveBeenCalledWith({ + token: "tok", + path: "/chats/19%3Achat%40thread.v2/messages/1783379480258", + }); + }); + + it("returns trimmed plain text when body is not HTML", async () => { + vi.mocked(fetchGraphJson).mockResolvedValueOnce({ + body: { content: " plain body ", contentType: "text" }, + } as never); + + const result = await fetchChatMessageText("tok", "19:chat", "m-1"); + expect(result).toBe("plain body"); + }); + + it("returns undefined on fetch error", async () => { + vi.mocked(fetchGraphJson).mockRejectedValueOnce(new Error("not found") as never); + + const result = await fetchChatMessageText("tok", "19:chat", "m-1"); + expect(result).toBeUndefined(); + }); + + it("returns undefined when the message has no body", async () => { + vi.mocked(fetchGraphJson).mockResolvedValueOnce({} as never); + + const result = await fetchChatMessageText("tok", "19:chat", "m-1"); + expect(result).toBeUndefined(); + }); +}); + describe("fetchThreadReplies", () => { beforeEach(() => { vi.mocked(fetchGraphJson).mockReset(); diff --git a/extensions/msteams/src/graph-thread.ts b/extensions/msteams/src/graph-thread.ts index 4254af7698f2..1f731259b4b9 100644 --- a/extensions/msteams/src/graph-thread.ts +++ b/extensions/msteams/src/graph-thread.ts @@ -112,6 +112,36 @@ export async function fetchChannelMessage( } } +/** + * Fetch a single chat message's full text via Graph and return plain text. + * + * Used to recover the complete quoted message for Teams quote replies: the + * inbound blockquote only carries a Teams-truncated `preview` snippet. The + * app-only `GET /chats/{chatId}/messages/{messageId}` endpoint IS permitted + * with the `Chat.Read.All` application permission (unlike the delegated + * `/me/chats` listing used by `resolveGraphChatId`, which 400s app-only). + * + * Returns undefined on any failure so callers degrade to the truncated preview. + */ +export async function fetchChatMessageText( + token: string, + chatId: string, + messageId: string, +): Promise { + // The get-chatMessage endpoint does not support OData query params (e.g. + // `$select`); tenants that enforce the documented contract reject the request, + // which would silently fall back to the truncated preview. Request it plainly. + const path = `/chats/${encodeURIComponent(chatId)}/messages/${encodeURIComponent(messageId)}`; + try { + const msg = await fetchGraphJson({ token, path }); + const raw = msg.body?.content ?? ""; + const text = msg.body?.contentType === "html" ? stripHtmlFromTeamsMessage(raw) : raw.trim(); + return text || undefined; + } catch { + return undefined; + } +} + /** * Fetch thread replies for a channel message, ordered chronologically. * diff --git a/extensions/msteams/src/graph.test.ts b/extensions/msteams/src/graph.test.ts index 62674b34bd22..d9fce5b0a4bb 100644 --- a/extensions/msteams/src/graph.test.ts +++ b/extensions/msteams/src/graph.test.ts @@ -4,12 +4,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const { loadMSTeamsSdkWithAuthMock, createMSTeamsTokenProviderMock, + fetchWithSsrFGuardMock, readAccessTokenMock, resolveMSTeamsCredentialsMock, } = vi.hoisted(() => { return { loadMSTeamsSdkWithAuthMock: vi.fn(), createMSTeamsTokenProviderMock: vi.fn(), + fetchWithSsrFGuardMock: vi.fn( + async (params: { url: string; init?: RequestInit; timeoutMs?: number }) => ({ + response: await globalThis.fetch(params.url, params.init), + finalUrl: params.url, + release: async () => undefined, + }), + ), readAccessTokenMock: vi.fn(), resolveMSTeamsCredentialsMock: vi.fn(), }; @@ -32,11 +40,7 @@ vi.mock("../runtime-api.js", async (importOriginal) => { const original = await importOriginal(); return { ...original, - fetchWithSsrFGuard: async (params: { url: string; init?: RequestInit }) => ({ - response: await globalThis.fetch(params.url, params.init), - finalUrl: params.url, - release: async () => undefined, - }), + fetchWithSsrFGuard: fetchWithSsrFGuardMock, }; }); @@ -45,6 +49,7 @@ import { deleteGraphRequest, escapeOData, fetchAllGraphPages, + fetchGraphAbsoluteUrl, fetchGraphJson, listChannelsForTeam, listTeamsByName, @@ -231,6 +236,10 @@ describe("msteams graph helpers", () => { expect(fetchCallUrl(0)).toBe("https://graph.microsoft.com/v1.0/groups?$select=id"); expect(fetchCallHeader(0, "Authorization")).toBe(`Bearer ${graphToken}`); expect(fetchCallHeader(0, "ConsistencyLevel")).toBe("eventual"); + expect(fetchWithSsrFGuardMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ timeoutMs: 30_000 }), + ); mockTextFetchResponse("forbidden", { status: 403 }); @@ -270,6 +279,19 @@ describe("msteams graph helpers", () => { expect(arrayBuffer).not.toHaveBeenCalled(); }); + it("bounds absolute Graph pagination requests", async () => { + mockGraphCollection(groupOne); + + await fetchGraphAbsoluteUrl({ + token: graphToken, + url: "https://graph.microsoft.com/v1.0/groups?$skiptoken=next", + }); + + expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith( + expect.objectContaining({ timeoutMs: 30_000 }), + ); + }); + it("posts Graph JSON to v1 and beta roots and treats empty mutation responses as undefined", async () => { mockFetch(async (input) => { if (requestUrl(input).startsWith("https://graph.microsoft.com/beta")) { diff --git a/extensions/msteams/src/graph.ts b/extensions/msteams/src/graph.ts index dce49865cd7c..4fca2491c685 100644 --- a/extensions/msteams/src/graph.ts +++ b/extensions/msteams/src/graph.ts @@ -11,6 +11,7 @@ import { resolveDelegatedAccessToken, resolveMSTeamsCredentials } from "./token. import { buildUserAgent } from "./user-agent.js"; const GRAPH_BETA = "https://graph.microsoft.com/beta"; +const GRAPH_REQUEST_TIMEOUT_MS = 30_000; export type GraphUser = { id?: string; @@ -65,6 +66,7 @@ async function requestGraph(params: { body: hasBody ? JSON.stringify(params.body) : undefined, }, auditContext: "msteams.graph", + timeoutMs: GRAPH_REQUEST_TIMEOUT_MS, }); let releaseInFinally = true; try { @@ -130,6 +132,7 @@ export async function fetchGraphAbsoluteUrl(params: { }, }, auditContext: "msteams.graph.absolute", + timeoutMs: GRAPH_REQUEST_TIMEOUT_MS, }); try { if (!response.ok) { diff --git a/extensions/msteams/src/inbound.test.ts b/extensions/msteams/src/inbound.test.ts index f1a40ff59093..de2407d869d1 100644 --- a/extensions/msteams/src/inbound.test.ts +++ b/extensions/msteams/src/inbound.test.ts @@ -218,5 +218,72 @@ describe("msteams inbound", () => { ]); expect(result).toEqual({ sender: "Alice", body: "Hello world" }); }); + + it("parses body from itemprop='preview' when 'copy' is absent", () => { + const result = extractMSTeamsQuoteInfo([ + { + contentType: "text/html", + content: + '
' + + 'Frank' + + '

truncated snippet…

' + + "
", + }, + ]); + expect(result?.body).toBe("truncated snippet…"); + expect(result?.sender).toBe("Frank"); + }); + + it("prefers 'copy' over 'preview' when both are present", () => { + const result = extractMSTeamsQuoteInfo([ + { + contentType: "text/html", + content: + '
' + + 'Grace' + + '

short…

' + + '

the full text

' + + "
", + }, + ]); + expect(result?.body).toBe("the full text"); + }); + + it("captures the blockquote itemid as the quoted message id", () => { + const result = extractMSTeamsQuoteInfo([ + { + contentType: "text/html", + content: + '
' + + 'Heidi' + + '

San Francisco right now…

' + + "
", + }, + ]); + expect(result).toEqual({ + sender: "Heidi", + body: "San Francisco right now…", + id: "1783379480258", + }); + }); + + it("parses a real Teams quote-reply payload (preview + itemid)", () => { + const result = extractMSTeamsQuoteInfo([ + { + contentType: "text/html", + content: + '
' + + 'Display Name' + + '' + + '

San Francisco right now ... Today\'s range: 54-64 °F (avg…

' + + "
\n

what abt not?

", + }, + ]); + expect(result).toEqual({ + sender: "Display Name", + body: "San Francisco right now ... Today's range: 54-64 °F (avg…", + id: "1783379480258", + }); + }); }); }); diff --git a/extensions/msteams/src/inbound.ts b/extensions/msteams/src/inbound.ts index 4eb2312a6332..c7aed94133ad 100644 --- a/extensions/msteams/src/inbound.ts +++ b/extensions/msteams/src/inbound.ts @@ -2,6 +2,12 @@ type MSTeamsQuoteInfo = { sender: string; body: string; + /** + * The quoted message's Teams id (the blockquote `itemid`). Present when Teams + * includes it; used to fetch the complete message text via Graph because the + * inbound blockquote only carries a truncated `preview` snippet. + */ + id?: string; }; /** @@ -64,12 +70,22 @@ export function extractMSTeamsQuoteInfo( const senderMatch = /]*itemprop=["']mri["'][^>]*>(.*?)<\/strong>/i.exec(content); const sender = senderMatch?.[1] ? htmlToPlainText(senderMatch[1]) : undefined; - // Extract body from

. - const bodyMatch = /]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content); + // Extract body from

(full quoted text) and fall back to + //

— the truncated snippet Teams actually sends for + // quote replies. Prefer `copy` when both are present. + const copyMatch = /]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content); + const bodyMatch = + copyMatch ?? /]*itemprop=["']preview["'][^>]*>(.*?)<\/p>/is.exec(content); const body = bodyMatch?.[1] ? htmlToPlainText(bodyMatch[1]) : undefined; + // Capture the blockquote `itemid` (the quoted message's Teams id) so callers + // can fetch the complete message text via Graph when only a preview snippet + // is available. + const idMatch = /]*\bitemid=["']([^"']+)["'][^>]*>/is.exec(content); + const id = idMatch?.[1]?.trim() || undefined; + if (body) { - return { sender: sender ?? "unknown", body }; + return { sender: sender ?? "unknown", body, ...(id ? { id } : {}) }; } } return undefined; diff --git a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts index 6bf510180a07..b5c0301a6902 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts @@ -39,6 +39,9 @@ const graphThreadMockState = vi.hoisted(() => ({ limit?: number, ) => Promise >(async () => []), + fetchChatMessageText: vi.fn< + (token: string, chatId: string, messageId: string) => Promise + >(async () => undefined), })); vi.mock("../graph-thread.js", () => { @@ -78,6 +81,7 @@ vi.mock("../graph-thread.js", () => { resolveTeamGroupId: graphThreadMockState.resolveTeamGroupId, fetchChannelMessage: graphThreadMockState.fetchChannelMessage, fetchThreadReplies: graphThreadMockState.fetchThreadReplies, + fetchChatMessageText: graphThreadMockState.fetchChatMessageText, }; }); @@ -120,6 +124,7 @@ describe("msteams monitor handler authz", () => { graphThreadMockState.resolveTeamGroupId.mockClear(); graphThreadMockState.fetchChannelMessage.mockReset(); graphThreadMockState.fetchThreadReplies.mockReset(); + graphThreadMockState.fetchChatMessageText.mockClear(); // Parent-context LRU + per-session dedupe are module-level; clear between // cases so stale parent fetches from earlier tests don't bleed in. resetThreadParentContextCachesForTest(); @@ -991,4 +996,76 @@ describe("msteams monitor handler authz", () => { expect(ctx.SupplementalContext).toEqual({}); expect(ctx.BodyForAgent).toBe("Current message"); }); + + it("does not fetch full quote text via Graph for group-chat quote replies", async () => { + resetThreadMocks(); + const { deps } = createDeps({ + channels: { msteams: { groupPolicy: "open", requireMention: false } }, + } as OpenClawConfig); + const handler = createMSTeamsMessageHandler(deps); + await handler( + createMessageActivity({ + id: "grp-quote-1", + text: "what about this?", + from: { id: "attacker-id", aadObjectId: "attacker-aad", name: "Attacker" }, + conversation: { id: "19:group@thread.tacv2", conversationType: "groupChat" }, + attachments: [ + { + contentType: "text/html", + content: + '

' + + 'Victim' + + '

secret snippet…

', + }, + ], + }), + ); + + // The group quote IS surfaced from the inbound preview (fix 1), proving the + // quote path ran — but the app-only Graph full-text fetch must NOT fire in a + // group chat: the fetched body would bypass the supplemental-quote visibility + // allowlist. Only 1:1 DMs may fetch full text. + const ctx = recordFromMockCall(firstSettledDispatch().ctxPayload); + expect(ctx.SupplementalContext).toMatchObject({ quote: { body: "secret snippet…" } }); + expect(graphThreadMockState.fetchChatMessageText).not.toHaveBeenCalled(); + }); + + it("replaces a DM quote preview with the complete Graph message", async () => { + resetThreadMocks(); + graphThreadMockState.fetchChatMessageText.mockResolvedValueOnce("complete quoted message"); + const { deps } = createDeps({ + channels: { msteams: { dmPolicy: "open", allowFrom: ["*"] } }, + } as OpenClawConfig); + const handler = createMSTeamsMessageHandler(deps); + + await handler( + createMessageActivity({ + id: "dm-quote-1", + text: "what about this?", + from: { id: "user-id", aadObjectId: "user-aad", name: "User" }, + conversation: { id: "19:dm@thread.v2", conversationType: "personal" }, + attachments: [ + { + contentType: "text/html", + content: + '
' + + 'Bot' + + '

truncated preview…

', + }, + ], + }), + ); + + expect(deps.tokenProvider.getAccessToken).toHaveBeenCalledWith("https://graph.microsoft.com"); + expect(graphThreadMockState.fetchChatMessageText).toHaveBeenCalledWith( + "token", + "19:dm@thread.v2", + "message-1", + ); + expect(recordFromMockCall(firstSettledDispatch().ctxPayload).SupplementalContext).toMatchObject( + { + quote: { id: "message-1", body: "complete quoted message", sender: "Bot" }, + }, + ); + }); }); diff --git a/extensions/msteams/src/monitor-handler/message-handler.thread-parent.test.ts b/extensions/msteams/src/monitor-handler/message-handler.thread-parent.test.ts index c0f5ecc063df..45f8852eec09 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.thread-parent.test.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.thread-parent.test.ts @@ -14,6 +14,7 @@ import { const runtimeApiMockState = getRuntimeApiMockState(); const fetchChannelMessageMock = vi.hoisted(() => vi.fn()); const fetchThreadRepliesMock = vi.hoisted(() => vi.fn(async () => [])); +const fetchChatMessageTextMock = vi.hoisted(() => vi.fn(async () => undefined)); const resolveTeamGroupIdMock = vi.hoisted(() => vi.fn(async () => "group-1")); vi.mock("../graph-thread.js", () => { @@ -34,6 +35,7 @@ vi.mock("../graph-thread.js", () => { resolveTeamGroupId: resolveTeamGroupIdMock, fetchChannelMessage: fetchChannelMessageMock, fetchThreadReplies: fetchThreadRepliesMock, + fetchChatMessageText: fetchChatMessageTextMock, }; }); diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index c015a0e2b8c9..6728731c9d35 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.ts @@ -33,6 +33,7 @@ import { tryNormalizeBotFrameworkServiceUrl } from "../bot-framework-service-url import type { StoredConversationReference } from "../conversation-store.js"; import { formatUnknownError } from "../errors.js"; import { + fetchChatMessageText, fetchThreadReplies, formatThreadContext, resolveTeamGroupId, @@ -589,6 +590,27 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { } } + // The inbound Teams blockquote only carries a truncated `preview` snippet for + // quote replies. When we have the quoted message id, fetch the complete text + // via the app-only `GET /chats/{chatId}/messages/{id}` endpoint (allowed with + // Chat.Read.All). Restricted to 1:1 DMs on purpose: in a group chat an + // allowlisted sender could quote a non-allowlisted member, and the fetched + // full body would bypass the supplemental-quote visibility allowlist applied + // below. DMs have only two participants, so there is no third-party exposure. + // Group/channel quotes keep the (now-surfaced) truncated preview from fix 1. + // Any failure degrades to that preview, so message handling never breaks. + let quoteBodyFull: string | undefined; + if (quoteInfo?.id && isDirectMessage && graphConversationId.startsWith("19:")) { + try { + const graphToken = await tokenProvider.getAccessToken("https://graph.microsoft.com"); + quoteBodyFull = await fetchChatMessageText(graphToken, graphConversationId, quoteInfo.id); + } catch (err) { + log.debug?.("failed to fetch full quoted message text", { + error: formatUnknownError(err), + }); + } + } + const mediaList = await resolveMSTeamsInboundMedia({ attachments, htmlSummary: htmlSummary ?? undefined, @@ -776,8 +798,8 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { supplemental: { quote: quoteInfo ? { - id: activity.replyToId ?? undefined, - body: quoteInfo.body, + id: quoteInfo.id ?? activity.replyToId ?? undefined, + body: quoteBodyFull ?? quoteInfo.body, sender: quoteInfo.sender, senderAllowed: quoteSenderAllowed, isQuote: true,