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 <drobison@nvidia.com>
This commit is contained in:
Agustin Rivera
2026-04-13 15:59:07 -07:00
committed by GitHub
parent 692438cbb2
commit a1c44d28fc
5 changed files with 167 additions and 22 deletions

View File

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

View File

@@ -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: {

View File

@@ -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]"}');
});

View File

@@ -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", () => {

View File

@@ -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<FeishuConfig>({
channelConfig: feishuCfg,
accounts: feishuCfg?.accounts as Record<string, Partial<FeishuConfig>> | undefined,