fix(feishu): enforce id-only allowlist matching

This commit is contained in:
Peter Steinberger
2026-02-22 18:54:24 +01:00
parent 3286791316
commit 4ed87a6672
4 changed files with 103 additions and 3 deletions

View File

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

View File

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

View File

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

View File

@@ -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<string | number>;
senderId: string;
senderIds?: Array<string | null | undefined>;
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<string | number>;
senderId: string;
senderIds?: Array<string | null | undefined>;
senderName?: string | null;
}): boolean {
const { groupPolicy } = params;