fix(msteams): surface quoted message body in Teams quote replies (#101856)

* fix(msteams): surface quoted message body in Teams quote replies

Teams sends the quoted text of a 1:1 DM quote-reply in <p itemprop="preview">
(a truncated snippet), not <p itemprop="copy"> 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* test(msteams): bound and prove quote enrichment

* test(msteams): use valid open DM fixture

---------

Co-authored-by: Yash Inani <yashinani@Yashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
velanir-ai-manager 2026-07-09 04:20:54 -07:00 committed by GitHub
parent ac71074807
commit e84a0dde17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 297 additions and 10 deletions

View file

@ -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: "<p>San Francisco right now: <at>Bot</at> full text</p>",
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();

View file

@ -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<string | undefined> {
// 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<GraphThreadMessage>({ 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.
*

View file

@ -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<typeof import("../runtime-api.js")>();
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")) {

View file

@ -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<T>(params: {
},
},
auditContext: "msteams.graph.absolute",
timeoutMs: GRAPH_REQUEST_TIMEOUT_MS,
});
try {
if (!response.ok) {

View file

@ -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:
'<blockquote itemtype="http://schema.skype.com/Reply" itemscope>' +
'<strong itemprop="mri">Frank</strong>' +
'<p itemprop="preview">truncated snippet…</p>' +
"</blockquote>",
},
]);
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:
'<blockquote itemtype="http://schema.skype.com/Reply" itemscope>' +
'<strong itemprop="mri">Grace</strong>' +
'<p itemprop="preview">short…</p>' +
'<p itemprop="copy">the full text</p>' +
"</blockquote>",
},
]);
expect(result?.body).toBe("the full text");
});
it("captures the blockquote itemid as the quoted message id", () => {
const result = extractMSTeamsQuoteInfo([
{
contentType: "text/html",
content:
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="1783379480258">' +
'<strong itemprop="mri">Heidi</strong>' +
'<p itemprop="preview">San Francisco right now…</p>' +
"</blockquote>",
},
]);
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:
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="1783379480258">' +
'<strong itemprop="mri" itemid="28:abc">Display Name</strong>' +
'<span itemprop="time" itemid="1783379480258"></span>' +
'<p itemprop="preview">San Francisco right now ... Today\'s range: 54-64 °F (avg…</p>' +
"</blockquote>\n<p>what abt not?</p>",
},
]);
expect(result).toEqual({
sender: "Display Name",
body: "San Francisco right now ... Today's range: 54-64 °F (avg…",
id: "1783379480258",
});
});
});
});

View file

@ -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 = /<strong[^>]*itemprop=["']mri["'][^>]*>(.*?)<\/strong>/i.exec(content);
const sender = senderMatch?.[1] ? htmlToPlainText(senderMatch[1]) : undefined;
// Extract body from <p itemprop="copy">.
const bodyMatch = /<p[^>]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content);
// Extract body from <p itemprop="copy"> (full quoted text) and fall back to
// <p itemprop="preview"> — the truncated snippet Teams actually sends for
// quote replies. Prefer `copy` when both are present.
const copyMatch = /<p[^>]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content);
const bodyMatch =
copyMatch ?? /<p[^>]*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 = /<blockquote[^>]*\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;

View file

@ -39,6 +39,9 @@ const graphThreadMockState = vi.hoisted(() => ({
limit?: number,
) => Promise<GraphThreadMessage[]>
>(async () => []),
fetchChatMessageText: vi.fn<
(token: string, chatId: string, messageId: string) => Promise<string | undefined>
>(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:
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="1783379480258">' +
'<strong itemprop="mri">Victim</strong>' +
'<p itemprop="preview">secret snippet…</p></blockquote>',
},
],
}),
);
// 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:
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="message-1">' +
'<strong itemprop="mri">Bot</strong>' +
'<p itemprop="preview">truncated preview…</p></blockquote>',
},
],
}),
);
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" },
},
);
});
});

View file

@ -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,
};
});

View file

@ -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,