diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d01f95cc8..936ff55f561 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting. - Hooks/Cron: suppress duplicate main-session events for delivered hook turns and mark `SILENT_REPLY_TOKEN` (`NO_REPLY`) early exits as delivered to prevent hook context pollution. (#20678) Thanks @JonathanWorks. - Providers/OpenRouter: inject `cache_control` on system prompts for OpenRouter Anthropic models to improve prompt-cache reuse. (#17473) Thanks @rrenamed. - Installer/Smoke tests: remove legacy `OPENCLAW_USE_GUM` overrides from docker install-smoke runs so tests exercise installer auto TTY detection behavior directly. diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 2cf30c44067..ba48abb60cb 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -522,6 +522,7 @@ export async function handleFeishuMessage(params: { let ctx = parseFeishuMessageEvent(event, botOpenId); const isGroup = ctx.chatType === "group"; + const senderUserId = event.sender.sender_id.user_id?.trim() || undefined; // Resolve sender display name (best-effort) so the agent can attribute messages correctly. const senderResult = await resolveFeishuSenderName({ @@ -601,6 +602,7 @@ export async function handleFeishuMessage(params: { groupPolicy: "allowlist", allowFrom: senderAllowFrom, senderId: ctx.senderOpenId, + senderIds: [senderUserId], senderName: ctx.senderName, }); if (!senderAllowed) { @@ -653,6 +655,7 @@ export async function handleFeishuMessage(params: { const dmAllowed = resolveFeishuAllowlistMatch({ allowFrom: effectiveDmAllowFrom, senderId: ctx.senderOpenId, + senderIds: [senderUserId], senderName: ctx.senderName, }).allowed; @@ -694,6 +697,7 @@ export async function handleFeishuMessage(params: { const senderAllowedForCommands = resolveFeishuAllowlistMatch({ allowFrom: commandAllowFrom, senderId: ctx.senderOpenId, + senderIds: [senderUserId], senderName: ctx.senderName, }).allowed; const commandAuthorized = shouldComputeCommandAuthorized diff --git a/extensions/feishu/src/policy.test.ts b/extensions/feishu/src/policy.test.ts new file mode 100644 index 00000000000..8e7d24ba67b --- /dev/null +++ b/extensions/feishu/src/policy.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; +import { isFeishuGroupAllowed, resolveFeishuAllowlistMatch } from "./policy.js"; + +describe("feishu policy", () => { + describe("resolveFeishuAllowlistMatch", () => { + it("allows wildcard", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["*"], + senderId: "ou-attacker", + }), + ).toEqual({ allowed: true, matchKey: "*", matchSource: "wildcard" }); + }); + + it("matches normalized ID entries", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["feishu:user:OU_ALLOWED"], + senderId: "ou_allowed", + }), + ).toEqual({ allowed: true, matchKey: "ou_allowed", matchSource: "id" }); + }); + + it("supports user_id as an additional immutable sender candidate", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["on_user_123"], + senderId: "ou_other", + senderIds: ["on_user_123"], + }), + ).toEqual({ allowed: true, matchKey: "on_user_123", matchSource: "id" }); + }); + + it("does not authorize based on display-name collision", () => { + const victimOpenId = "ou_4f4ec5aa111122223333444455556666"; + + expect( + resolveFeishuAllowlistMatch({ + allowFrom: [victimOpenId], + senderId: "ou_attacker_real_open_id", + senderIds: ["on_attacker_user_id"], + senderName: victimOpenId, + }), + ).toEqual({ allowed: false }); + }); + }); + + describe("isFeishuGroupAllowed", () => { + it("matches group IDs with chat: prefix", () => { + expect( + isFeishuGroupAllowed({ + groupPolicy: "allowlist", + allowFrom: ["chat:oc_group_123"], + senderId: "oc_group_123", + }), + ).toBe(true); + }); + }); +}); diff --git a/extensions/feishu/src/policy.ts b/extensions/feishu/src/policy.ts index 89e12ba859e..6ddac42d0e6 100644 --- a/extensions/feishu/src/policy.ts +++ b/extensions/feishu/src/policy.ts @@ -3,17 +3,52 @@ import type { ChannelGroupContext, GroupToolPolicyConfig, } from "openclaw/plugin-sdk"; -import { resolveAllowlistMatchSimple } from "openclaw/plugin-sdk"; +import { normalizeFeishuTarget } from "./targets.js"; import type { FeishuConfig, FeishuGroupConfig } from "./types.js"; -export type FeishuAllowlistMatch = AllowlistMatch<"wildcard" | "id" | "name">; +export type FeishuAllowlistMatch = AllowlistMatch<"wildcard" | "id">; + +function normalizeFeishuAllowEntry(raw: string): string { + const trimmed = raw.trim(); + if (!trimmed) { + return ""; + } + if (trimmed === "*") { + return "*"; + } + const withoutProviderPrefix = trimmed.replace(/^feishu:/i, ""); + const normalized = normalizeFeishuTarget(withoutProviderPrefix) ?? withoutProviderPrefix; + return normalized.trim().toLowerCase(); +} export function resolveFeishuAllowlistMatch(params: { allowFrom: Array; senderId: string; + senderIds?: Array; senderName?: string | null; }): FeishuAllowlistMatch { - return resolveAllowlistMatchSimple(params); + const allowFrom = params.allowFrom + .map((entry) => normalizeFeishuAllowEntry(String(entry))) + .filter(Boolean); + if (allowFrom.length === 0) { + return { allowed: false }; + } + if (allowFrom.includes("*")) { + return { allowed: true, matchKey: "*", matchSource: "wildcard" }; + } + + // Feishu allowlists are ID-based; mutable display names must never grant access. + const senderCandidates = [params.senderId, ...(params.senderIds ?? [])] + .map((entry) => normalizeFeishuAllowEntry(String(entry ?? ""))) + .filter(Boolean); + + for (const senderId of senderCandidates) { + if (allowFrom.includes(senderId)) { + return { allowed: true, matchKey: senderId, matchSource: "id" }; + } + } + + return { allowed: false }; } export function resolveFeishuGroupConfig(params: { @@ -56,6 +91,7 @@ export function isFeishuGroupAllowed(params: { groupPolicy: "open" | "allowlist" | "disabled"; allowFrom: Array; senderId: string; + senderIds?: Array; senderName?: string | null; }): boolean { const { groupPolicy } = params;