From a1c44d28fcb83e35dcd04b8b7731986adeed84cf Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Mon, 13 Apr 2026 15:59:07 -0700 Subject: [PATCH] Feishu: tighten allowlist target canonicalization (#66021) * fix(feishu): tighten allowlist id matching * fix(feishu): address review follow-ups * changelog: note Feishu allowlist canonicalization tightening (#66021) * fix(feishu): collapse typed wildcard allowlist aliases to bare wildcard Previously normalizeFeishuTarget folded chat:* / user:* / open_id:* / dm:* / group:* / channel:* down to '*', so those entries acted as allow-all. The new typed canonicalization was producing literal keys (chat:*, user:*, ...) that never matched any sender, silently flipping those configs from allow-all to deny-all. Restore the prior behavior by collapsing a wildcard value to '*' inside canonicalizeFeishuAllowlistKey. --------- Co-authored-by: Devin Robison --- CHANGELOG.md | 1 + extensions/feishu/src/monitor.account.ts | 8 +- .../src/monitor.reaction.lifecycle.test.ts | 3 +- extensions/feishu/src/policy.test.ts | 81 +++++++++++++++- extensions/feishu/src/policy.ts | 96 ++++++++++++++++--- 5 files changed, 167 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38f06436ae9..3dd23b7e538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Docs: https://docs.openclaw.ai - Memory/QMD: stop treating legacy lowercase `memory.md` as a second default root collection, so QMD recall no longer searches phantom `memory-alt-*` collections and builtin/QMD root-memory fallback stays aligned. (#66141) Thanks @mbelinky. - Agents/OpenAI: map `minimal` thinking to OpenAI's supported `low` reasoning effort for GPT-5.4 requests, so embedded runs stop failing request validation. - Voice-call/media-stream: resolve the source IP from trusted forwarding headers for per-IP pending-connection limits when `webhookSecurity.trustForwardingHeaders` and `trustedProxyIPs` are configured, and reserve `maxConnections` capacity for in-flight WebSocket upgrades so concurrent handshakes can no longer momentarily exceed the operator-set cap. (#66027) Thanks @eleqtrizit. +- Feishu/allowlist: canonicalize allowlist entries by explicit `user`/`chat` kind, strip repeated `feishu:`/`lark:` provider prefixes, and stop folding opaque Feishu IDs to lowercase, so allowlist matching no longer crosses user/chat namespaces or widens to case-insensitive ID matches the operator did not intend. (#66021) Thanks @eleqtrizit. ## 2026.4.12 diff --git a/extensions/feishu/src/monitor.account.ts b/extensions/feishu/src/monitor.account.ts index 4c8f84c25da..ff28b958547 100644 --- a/extensions/feishu/src/monitor.account.ts +++ b/extensions/feishu/src/monitor.account.ts @@ -62,7 +62,7 @@ export type FeishuReactionCreatedEvent = { chat_type?: string; reaction_type?: { emoji_type?: string }; operator_type?: string; - user_id?: { open_id?: string }; + user_id?: { open_id?: string; user_id?: string }; action_time?: string; }; @@ -100,6 +100,7 @@ export async function resolveReactionSyntheticEvent( const emoji = event.reaction_type?.emoji_type; const messageId = event.message_id; const senderId = event.user_id?.open_id; + const senderUserId = event.user_id?.user_id; if (!emoji || !messageId || !senderId) { return null; } @@ -154,7 +155,10 @@ export async function resolveReactionSyntheticEvent( const syntheticChatType: FeishuChatType = resolvedChatType; return { sender: { - sender_id: { open_id: senderId }, + sender_id: { + open_id: senderId, + ...(senderUserId ? { user_id: senderUserId } : {}), + }, sender_type: "user", }, message: { diff --git a/extensions/feishu/src/monitor.reaction.lifecycle.test.ts b/extensions/feishu/src/monitor.reaction.lifecycle.test.ts index 2648ff1b8de..e449946715b 100644 --- a/extensions/feishu/src/monitor.reaction.lifecycle.test.ts +++ b/extensions/feishu/src/monitor.reaction.lifecycle.test.ts @@ -24,7 +24,7 @@ describe("Feishu reaction lifecycle", () => { const result = await resolveReactionSyntheticEvent({ cfg, accountId: "default", - event: makeReactionEvent(), + event: makeReactionEvent({ user_id: { open_id: "ou_user1", user_id: "on_user1" } }), botOpenId: "ou_bot", fetchMessage: async () => ({ messageId: "om_msg1", @@ -38,6 +38,7 @@ describe("Feishu reaction lifecycle", () => { uuid: () => "fixed-uuid", }); + expect(result?.sender.sender_id).toEqual({ open_id: "ou_user1", user_id: "on_user1" }); expect(result?.message.content).toBe('{"text":"[reacted with THUMBSUP to message om_msg1]"}'); }); diff --git a/extensions/feishu/src/policy.test.ts b/extensions/feishu/src/policy.test.ts index 47d4b83b36d..622dedc0a41 100644 --- a/extensions/feishu/src/policy.test.ts +++ b/extensions/feishu/src/policy.test.ts @@ -151,13 +151,68 @@ describe("resolveFeishuAllowlistMatch", () => { ).toEqual({ allowed: true, matchKey: "*", matchSource: "wildcard" }); }); + it("allows provider-prefixed wildcard entries", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["feishu:*", "lark:*"], + senderId: "ou_anyone", + }), + ).toEqual({ allowed: true, matchKey: "*", matchSource: "wildcard" }); + }); + + it("treats typed wildcard aliases as bare wildcards", () => { + for (const wildcard of [ + "chat:*", + "group:*", + "channel:*", + "user:*", + "dm:*", + "open_id:*", + "feishu:user:*", + ]) { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: [wildcard], + senderId: "ou_anyone", + }), + ).toEqual({ allowed: true, matchKey: "*", matchSource: "wildcard" }); + } + }); + it("matches normalized ID entries", () => { expect( resolveFeishuAllowlistMatch({ - allowFrom: ["feishu:user:OU_ALLOWED"], - senderId: "ou_allowed", + allowFrom: ["feishu:user:ou_ALLOWED"], + senderId: "ou_ALLOWED", }), - ).toEqual({ allowed: true, matchKey: "ou_allowed", matchSource: "id" }); + ).toEqual({ allowed: true, matchKey: "user:ou_ALLOWED", matchSource: "id" }); + }); + + it("accepts repeated provider prefixes for legacy allowlist entries", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["feishu:feishu:user:ou_ALLOWED"], + senderId: "ou_ALLOWED", + }), + ).toEqual({ allowed: true, matchKey: "user:ou_ALLOWED", matchSource: "id" }); + }); + + it("does not fold opaque IDs to lowercase", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["user:OU_ALLOWED"], + senderId: "ou_ALLOWED", + }), + ).toEqual({ allowed: false }); + }); + + it("keeps user and chat allowlist namespaces distinct", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["user:oc_group_123"], + senderId: "oc_group_123", + }), + ).toEqual({ allowed: false }); }); it("supports user_id as an additional immutable sender candidate", () => { @@ -167,7 +222,25 @@ describe("resolveFeishuAllowlistMatch", () => { senderId: "ou_other", senderIds: ["on_user_123"], }), - ).toEqual({ allowed: true, matchKey: "on_user_123", matchSource: "id" }); + ).toEqual({ allowed: true, matchKey: "user:on_user_123", matchSource: "id" }); + }); + + it("auto-detects bare open_id entries as user allowlist matches", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["ou_BARE"], + senderId: "ou_BARE", + }), + ).toEqual({ allowed: true, matchKey: "user:ou_BARE", matchSource: "id" }); + }); + + it("auto-detects bare chat_id entries as chat allowlist matches", () => { + expect( + resolveFeishuAllowlistMatch({ + allowFrom: ["oc_group_123"], + senderId: "oc_group_123", + }), + ).toEqual({ allowed: true, matchKey: "chat:oc_group_123", matchSource: "id" }); }); it("does not authorize based on display-name collision", () => { diff --git a/extensions/feishu/src/policy.ts b/extensions/feishu/src/policy.ts index 2df21d5ee4e..80237249986 100644 --- a/extensions/feishu/src/policy.ts +++ b/extensions/feishu/src/policy.ts @@ -5,12 +5,36 @@ import { import type { OpenClawConfig } from "openclaw/plugin-sdk/core"; import { evaluateSenderGroupAccessForPolicy } from "openclaw/plugin-sdk/group-access"; import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/text-runtime"; -import type { AllowlistMatch, ChannelGroupContext, GroupToolPolicyConfig } from "../runtime-api.js"; -import { normalizeFeishuTarget } from "./targets.js"; -import type { FeishuConfig, FeishuGroupConfig } from "./types.js"; +import type { AllowlistMatch, ChannelGroupContext } from "../runtime-api.js"; +import { detectIdType } from "./targets.js"; +import type { FeishuConfig } from "./types.js"; export type FeishuAllowlistMatch = AllowlistMatch<"wildcard" | "id">; +const FEISHU_PROVIDER_PREFIX_RE = /^(feishu|lark):/i; + +function stripRepeatedFeishuProviderPrefixes(raw: string): string { + let normalized = raw.trim(); + while (FEISHU_PROVIDER_PREFIX_RE.test(normalized)) { + normalized = normalized.replace(FEISHU_PROVIDER_PREFIX_RE, "").trim(); + } + return normalized; +} + +function canonicalizeFeishuAllowlistKey(params: { kind: "chat" | "user"; value: string }): string { + const value = params.value.trim(); + if (!value) { + return ""; + } + // A typed wildcard (`chat:*`, `user:*`, `open_id:*`, `dm:*`, `group:*`, + // `channel:*`) collapses to the bare wildcard so it keeps matching across + // both kinds, preserving the prior `normalizeFeishuTarget`-based behavior. + if (value === "*") { + return "*"; + } + return `${params.kind}:${value}`; +} + function normalizeFeishuAllowEntry(raw: string): string { const trimmed = raw.trim(); if (!trimmed) { @@ -19,9 +43,56 @@ function normalizeFeishuAllowEntry(raw: string): string { if (trimmed === "*") { return "*"; } - const withoutProviderPrefix = trimmed.replace(/^feishu:/i, ""); - const normalized = normalizeFeishuTarget(withoutProviderPrefix) ?? withoutProviderPrefix; - return normalizeOptionalLowercaseString(normalized) ?? ""; + + const withoutProviderPrefix = stripRepeatedFeishuProviderPrefixes(trimmed); + if (withoutProviderPrefix === "*") { + return "*"; + } + const lowered = normalizeOptionalLowercaseString(withoutProviderPrefix) ?? ""; + if (!lowered) { + return ""; + } + // Lowercase for prefix detection only; preserve the original ID casing in the + // canonicalized key. Sender candidates pass through this same path so allowlist + // entries and runtime IDs stay normalized symmetrically. + if ( + lowered.startsWith("chat:") || + lowered.startsWith("group:") || + lowered.startsWith("channel:") + ) { + return canonicalizeFeishuAllowlistKey({ + kind: "chat", + value: withoutProviderPrefix.slice(withoutProviderPrefix.indexOf(":") + 1), + }); + } + if (lowered.startsWith("user:") || lowered.startsWith("dm:")) { + return canonicalizeFeishuAllowlistKey({ + kind: "user", + value: withoutProviderPrefix.slice(withoutProviderPrefix.indexOf(":") + 1), + }); + } + if (lowered.startsWith("open_id:")) { + return canonicalizeFeishuAllowlistKey({ + kind: "user", + value: withoutProviderPrefix.slice(withoutProviderPrefix.indexOf(":") + 1), + }); + } + + const detectedType = detectIdType(withoutProviderPrefix); + if (detectedType === "chat_id") { + return canonicalizeFeishuAllowlistKey({ + kind: "chat", + value: withoutProviderPrefix, + }); + } + if (detectedType === "open_id" || detectedType === "user_id") { + return canonicalizeFeishuAllowlistKey({ + kind: "user", + value: withoutProviderPrefix, + }); + } + + return ""; } export function resolveFeishuAllowlistMatch(params: { @@ -54,10 +125,7 @@ export function resolveFeishuAllowlistMatch(params: { return { allowed: false }; } -export function resolveFeishuGroupConfig(params: { - cfg?: FeishuConfig; - groupId?: string | null; -}): FeishuGroupConfig | undefined { +export function resolveFeishuGroupConfig(params: { cfg?: FeishuConfig; groupId?: string | null }) { const groups = params.cfg?.groups ?? {}; const wildcard = groups["*"]; const groupId = params.groupId?.trim(); @@ -80,10 +148,8 @@ export function resolveFeishuGroupConfig(params: { return wildcard; } -export function resolveFeishuGroupToolPolicy( - params: ChannelGroupContext, -): GroupToolPolicyConfig | undefined { - const cfg = params.cfg.channels?.feishu as FeishuConfig | undefined; +export function resolveFeishuGroupToolPolicy(params: ChannelGroupContext) { + const cfg = params.cfg.channels?.feishu; if (!cfg) { return undefined; } @@ -127,7 +193,7 @@ export function resolveFeishuReplyPolicy(params: { return { requireMention: false }; } - const feishuCfg = params.cfg.channels?.feishu as FeishuConfig | undefined; + const feishuCfg = params.cfg.channels?.feishu; const resolvedCfg = resolveMergedAccountConfig({ channelConfig: feishuCfg, accounts: feishuCfg?.accounts as Record> | undefined,