From 106d60551932af3db2ad08c4de822951b4c26017 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 15:08:48 +0100 Subject: [PATCH] fix: harden msteams mentions and fallback links (#15436) (thanks @hyojin) --- CHANGELOG.md | 1 + extensions/msteams/src/mentions.test.ts | 12 ++++ extensions/msteams/src/mentions.ts | 12 ++-- extensions/msteams/src/messenger.test.ts | 82 +++++++++++++++++++++++- extensions/msteams/src/messenger.ts | 3 +- 5 files changed, 102 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fd76c619bc..b6d6de7fbd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Security/Gateway + ACP: block high-risk tools (`sessions_spawn`, `sessions_send`, `gateway`, `whatsapp_login`) from HTTP `/tools/invoke` by default with `gateway.tools.{allow,deny}` overrides, and harden ACP permission selection to fail closed when tool identity/options are ambiguous while supporting `allow_always`/`reject_always`. (#15390) Thanks @aether-ai-agent. +- MS Teams: preserve parsed mention entities/text when appending OneDrive fallback file links, and accept broader real-world Teams mention ID formats (`29:...`, `8:orgid:...`) while still rejecting placeholder patterns. (#15436) Thanks @hyojin. - Security/Audit: distinguish external webhooks (`hooks.enabled`) from internal hooks (`hooks.internal.enabled`) in attack-surface summaries to avoid false exposure signals when only internal hooks are enabled. (#13474) Thanks @mcaxtr. - Auto-reply/Threading: auto-inject implicit reply threading so `replyToMode` works without requiring model-emitted `[[reply_to_current]]`, while preserving `replyToMode: "off"` behavior for implicit Slack replies and keeping block-streaming chunk coalescing stable under `replyToMode: "first"`. (#14976) Thanks @Diaspar4u. - Sandbox: pass configured `sandbox.docker.env` variables to sandbox containers at `docker create` time. (#15138) Thanks @stevebot-alive. diff --git a/extensions/msteams/src/mentions.test.ts b/extensions/msteams/src/mentions.test.ts index 13d9e02e745..bddb1383887 100644 --- a/extensions/msteams/src/mentions.test.ts +++ b/extensions/msteams/src/mentions.test.ts @@ -121,6 +121,18 @@ describe("parseMentions", () => { expect(result.entities[0]?.mentioned.id).toBe("28:abc-123"); }); + it("accepts Bot Framework IDs with non-hex payloads (29:xxx)", () => { + const result = parseMentions("@[Bot](29:08q2j2o3jc09au90eucae)"); + expect(result.entities).toHaveLength(1); + expect(result.entities[0]?.mentioned.id).toBe("29:08q2j2o3jc09au90eucae"); + }); + + it("accepts org-scoped IDs with extra segments (8:orgid:...)", () => { + const result = parseMentions("@[User](8:orgid:2d8c2d2c-1111-2222-3333-444444444444)"); + expect(result.entities).toHaveLength(1); + expect(result.entities[0]?.mentioned.id).toBe("8:orgid:2d8c2d2c-1111-2222-3333-444444444444"); + }); + it("accepts AAD object IDs (UUIDs)", () => { const result = parseMentions("@[User](a1b2c3d4-e5f6-7890-abcd-ef1234567890)"); expect(result.entities).toHaveLength(1); diff --git a/extensions/msteams/src/mentions.ts b/extensions/msteams/src/mentions.ts index fc63093b000..eda07f13fda 100644 --- a/extensions/msteams/src/mentions.ts +++ b/extensions/msteams/src/mentions.ts @@ -25,17 +25,17 @@ export type MentionInfo = { /** * Check whether an ID looks like a valid Teams user/bot identifier. * Accepts: - * - Bot Framework IDs: "28:xxx..." or "29:xxx..." + * - Bot Framework IDs: "28:xxx..." / "29:xxx..." / "8:orgid:..." * - AAD object IDs (UUIDs): "d5318c29-33ac-4e6b-bd42-57b8b793908f" * - * This prevents false positives from text like `@[表示名](ユーザーID)` - * that appears in code snippets or documentation within messages. + * Keep this permissive enough for real Teams IDs while still rejecting + * documentation placeholders like `@[表示名](ユーザーID)`. */ -const TEAMS_ID_PATTERN = - /^(?:\d+:[a-f0-9-]+|[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})$/i; +const TEAMS_BOT_ID_PATTERN = /^\d+:[a-z0-9._=-]+(?::[a-z0-9._=-]+)*$/i; +const AAD_OBJECT_ID_PATTERN = /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i; function isValidTeamsId(id: string): boolean { - return TEAMS_ID_PATTERN.test(id); + return TEAMS_BOT_ID_PATTERN.test(id) || AAD_OBJECT_ID_PATTERN.test(id); } /** diff --git a/extensions/msteams/src/messenger.test.ts b/extensions/msteams/src/messenger.test.ts index bd49e4e8161..9ff3c0d2868 100644 --- a/extensions/msteams/src/messenger.test.ts +++ b/extensions/msteams/src/messenger.test.ts @@ -1,6 +1,21 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { SILENT_REPLY_TOKEN, type PluginRuntime } from "openclaw/plugin-sdk"; -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { StoredConversationReference } from "./conversation-store.js"; +const graphUploadMockState = vi.hoisted(() => ({ + uploadAndShareOneDrive: vi.fn(), +})); + +vi.mock("./graph-upload.js", async () => { + const actual = await vi.importActual("./graph-upload.js"); + return { + ...actual, + uploadAndShareOneDrive: graphUploadMockState.uploadAndShareOneDrive, + }; +}); + import { type MSTeamsAdapter, renderReplyPayloadsToMessages, @@ -36,6 +51,13 @@ const runtimeStub = { describe("msteams messenger", () => { beforeEach(() => { setMSTeamsRuntime(runtimeStub); + graphUploadMockState.uploadAndShareOneDrive.mockReset(); + graphUploadMockState.uploadAndShareOneDrive.mockResolvedValue({ + itemId: "item123", + webUrl: "https://onedrive.example.com/item123", + shareUrl: "https://onedrive.example.com/share/item123", + name: "upload.txt", + }); }); describe("renderReplyPayloadsToMessages", () => { @@ -153,6 +175,64 @@ describe("msteams messenger", () => { expect(ref.conversation?.id).toBe("19:abc@thread.tacv2"); }); + it("preserves parsed mentions when appending OneDrive fallback file links", async () => { + const tmpDir = await mkdtemp(path.join(os.tmpdir(), "msteams-mention-")); + const localFile = path.join(tmpDir, "note.txt"); + await writeFile(localFile, "hello"); + + try { + const sent: Array<{ text?: string; entities?: unknown[] }> = []; + const ctx = { + sendActivity: async (activity: unknown) => { + sent.push(activity as { text?: string; entities?: unknown[] }); + return { id: "id:one" }; + }, + }; + + const adapter: MSTeamsAdapter = { + continueConversation: async () => {}, + }; + + const ids = await sendMSTeamsMessages({ + replyStyle: "thread", + adapter, + appId: "app123", + conversationRef: { + ...baseRef, + conversation: { + ...baseRef.conversation, + conversationType: "channel", + }, + }, + context: ctx, + messages: [{ text: "Hello @[John](29:08q2j2o3jc09au90eucae)", mediaUrl: localFile }], + tokenProvider: { + getAccessToken: async () => "token", + }, + }); + + expect(ids).toEqual(["id:one"]); + expect(graphUploadMockState.uploadAndShareOneDrive).toHaveBeenCalledOnce(); + expect(sent).toHaveLength(1); + expect(sent[0]?.text).toContain("Hello John"); + expect(sent[0]?.text).toContain( + "📎 [upload.txt](https://onedrive.example.com/share/item123)", + ); + expect(sent[0]?.entities).toEqual([ + { + type: "mention", + text: "John", + mentioned: { + id: "29:08q2j2o3jc09au90eucae", + name: "John", + }, + }, + ]); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } + }); + it("retries thread sends on throttling (429)", async () => { const attempts: string[] = []; const retryEvents: Array<{ nextAttempt: number; delayMs: number }> = []; diff --git a/extensions/msteams/src/messenger.ts b/extensions/msteams/src/messenger.ts index e29c620b202..ff6f1e58485 100644 --- a/extensions/msteams/src/messenger.ts +++ b/extensions/msteams/src/messenger.ts @@ -358,7 +358,8 @@ async function buildActivity( // Bot Framework doesn't support "reference" attachment type for sending const fileLink = `📎 [${uploaded.name}](${uploaded.shareUrl})`; - activity.text = msg.text ? `${msg.text}\n\n${fileLink}` : fileLink; + const existingText = typeof activity.text === "string" ? activity.text : undefined; + activity.text = existingText ? `${existingText}\n\n${fileLink}` : fileLink; return activity; }