mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 11:48:38 +00:00
fix: harden msteams mentions and fallback links (#15436) (thanks @hyojin)
This commit is contained in:
@@ -7,6 +7,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
### Fixes
|
### 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.
|
- 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.
|
- 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.
|
- 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.
|
- Sandbox: pass configured `sandbox.docker.env` variables to sandbox containers at `docker create` time. (#15138) Thanks @stevebot-alive.
|
||||||
|
|||||||
@@ -121,6 +121,18 @@ describe("parseMentions", () => {
|
|||||||
expect(result.entities[0]?.mentioned.id).toBe("28:abc-123");
|
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)", () => {
|
it("accepts AAD object IDs (UUIDs)", () => {
|
||||||
const result = parseMentions("@[User](a1b2c3d4-e5f6-7890-abcd-ef1234567890)");
|
const result = parseMentions("@[User](a1b2c3d4-e5f6-7890-abcd-ef1234567890)");
|
||||||
expect(result.entities).toHaveLength(1);
|
expect(result.entities).toHaveLength(1);
|
||||||
|
|||||||
@@ -25,17 +25,17 @@ export type MentionInfo = {
|
|||||||
/**
|
/**
|
||||||
* Check whether an ID looks like a valid Teams user/bot identifier.
|
* Check whether an ID looks like a valid Teams user/bot identifier.
|
||||||
* Accepts:
|
* 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"
|
* - AAD object IDs (UUIDs): "d5318c29-33ac-4e6b-bd42-57b8b793908f"
|
||||||
*
|
*
|
||||||
* This prevents false positives from text like `@[表示名](ユーザーID)`
|
* Keep this permissive enough for real Teams IDs while still rejecting
|
||||||
* that appears in code snippets or documentation within messages.
|
* documentation placeholders like `@[表示名](ユーザーID)`.
|
||||||
*/
|
*/
|
||||||
const TEAMS_ID_PATTERN =
|
const TEAMS_BOT_ID_PATTERN = /^\d+:[a-z0-9._=-]+(?::[a-z0-9._=-]+)*$/i;
|
||||||
/^(?:\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 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 {
|
function isValidTeamsId(id: string): boolean {
|
||||||
return TEAMS_ID_PATTERN.test(id);
|
return TEAMS_BOT_ID_PATTERN.test(id) || AAD_OBJECT_ID_PATTERN.test(id);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -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 { 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";
|
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<typeof import("./graph-upload.js")>("./graph-upload.js");
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
uploadAndShareOneDrive: graphUploadMockState.uploadAndShareOneDrive,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
import {
|
import {
|
||||||
type MSTeamsAdapter,
|
type MSTeamsAdapter,
|
||||||
renderReplyPayloadsToMessages,
|
renderReplyPayloadsToMessages,
|
||||||
@@ -36,6 +51,13 @@ const runtimeStub = {
|
|||||||
describe("msteams messenger", () => {
|
describe("msteams messenger", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
setMSTeamsRuntime(runtimeStub);
|
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", () => {
|
describe("renderReplyPayloadsToMessages", () => {
|
||||||
@@ -153,6 +175,64 @@ describe("msteams messenger", () => {
|
|||||||
expect(ref.conversation?.id).toBe("19:abc@thread.tacv2");
|
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 <at>John</at>");
|
||||||
|
expect(sent[0]?.text).toContain(
|
||||||
|
"📎 [upload.txt](https://onedrive.example.com/share/item123)",
|
||||||
|
);
|
||||||
|
expect(sent[0]?.entities).toEqual([
|
||||||
|
{
|
||||||
|
type: "mention",
|
||||||
|
text: "<at>John</at>",
|
||||||
|
mentioned: {
|
||||||
|
id: "29:08q2j2o3jc09au90eucae",
|
||||||
|
name: "John",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
} finally {
|
||||||
|
await rm(tmpDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it("retries thread sends on throttling (429)", async () => {
|
it("retries thread sends on throttling (429)", async () => {
|
||||||
const attempts: string[] = [];
|
const attempts: string[] = [];
|
||||||
const retryEvents: Array<{ nextAttempt: number; delayMs: number }> = [];
|
const retryEvents: Array<{ nextAttempt: number; delayMs: number }> = [];
|
||||||
|
|||||||
@@ -358,7 +358,8 @@ async function buildActivity(
|
|||||||
|
|
||||||
// Bot Framework doesn't support "reference" attachment type for sending
|
// Bot Framework doesn't support "reference" attachment type for sending
|
||||||
const fileLink = `📎 [${uploaded.name}](${uploaded.shareUrl})`;
|
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;
|
return activity;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user