From d00d6876f552d94f50296090da8cf7a18b1bf9fc Mon Sep 17 00:00:00 2001 From: Coy Geek <65363919+coygeek@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:37:51 -0800 Subject: [PATCH] fix(aa-01): apply security fix Generated by staged fix workflow. --- extensions/feishu/src/bot.test.ts | 105 ++++++++++++++++++++++++++++++ extensions/feishu/src/bot.ts | 40 ++++++++++-- 2 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 extensions/feishu/src/bot.test.ts diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts new file mode 100644 index 0000000000..26c52e83e8 --- /dev/null +++ b/extensions/feishu/src/bot.test.ts @@ -0,0 +1,105 @@ +import type { ClawdbotConfig, PluginRuntime, RuntimeEnv } from "openclaw/plugin-sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { FeishuMessageEvent } from "./bot.js"; +import { handleFeishuMessage } from "./bot.js"; +import { setFeishuRuntime } from "./runtime.js"; + +const { mockCreateFeishuReplyDispatcher } = vi.hoisted(() => ({ + mockCreateFeishuReplyDispatcher: vi.fn(() => ({ + dispatcher: vi.fn(), + replyOptions: {}, + markDispatchIdle: vi.fn(), + })), +})); + +vi.mock("./reply-dispatcher.js", () => ({ + createFeishuReplyDispatcher: mockCreateFeishuReplyDispatcher, +})); + +describe("handleFeishuMessage command authorization", () => { + const mockFinalizeInboundContext = vi.fn((ctx: unknown) => ctx); + const mockDispatchReplyFromConfig = vi + .fn() + .mockResolvedValue({ queuedFinal: false, counts: { final: 1 } }); + const mockResolveCommandAuthorizedFromAuthorizers = vi.fn(() => false); + const mockShouldComputeCommandAuthorized = vi.fn(() => true); + const mockReadAllowFromStore = vi.fn().mockResolvedValue([]); + + beforeEach(() => { + vi.clearAllMocks(); + setFeishuRuntime({ + system: { + enqueueSystemEvent: vi.fn(), + }, + channel: { + routing: { + resolveAgentRoute: vi.fn(() => ({ + agentId: "main", + accountId: "default", + sessionKey: "agent:main:feishu:dm:ou-attacker", + matchedBy: "default", + })), + }, + reply: { + resolveEnvelopeFormatOptions: vi.fn(() => ({ template: "channel+name+time" })), + formatAgentEnvelope: vi.fn((params: { body: string }) => params.body), + finalizeInboundContext: mockFinalizeInboundContext, + dispatchReplyFromConfig: mockDispatchReplyFromConfig, + }, + commands: { + shouldComputeCommandAuthorized: mockShouldComputeCommandAuthorized, + resolveCommandAuthorizedFromAuthorizers: mockResolveCommandAuthorizedFromAuthorizers, + }, + pairing: { + readAllowFromStore: mockReadAllowFromStore, + }, + }, + } as unknown as PluginRuntime); + }); + + it("uses authorizer resolution instead of hardcoded CommandAuthorized=true", async () => { + const cfg: ClawdbotConfig = { + commands: { useAccessGroups: true }, + channels: { + feishu: { + dmPolicy: "pairing", + allowFrom: ["ou-admin"], + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { + sender_id: { + open_id: "ou-attacker", + }, + }, + message: { + message_id: "msg-auth-bypass-regression", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "/status" }), + }, + }; + + await handleFeishuMessage({ + cfg, + event, + runtime: { log: vi.fn(), error: vi.fn() } as RuntimeEnv, + }); + + expect(mockResolveCommandAuthorizedFromAuthorizers).toHaveBeenCalledWith({ + useAccessGroups: true, + authorizers: [{ configured: true, allowed: false }], + }); + expect(mockFinalizeInboundContext).toHaveBeenCalledTimes(1); + expect(mockFinalizeInboundContext).toHaveBeenCalledWith( + expect.objectContaining({ + CommandAuthorized: false, + SenderId: "ou-attacker", + Surface: "feishu", + }), + ); + }); +}); diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 6266dc289b..eed6f5bc52 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -581,12 +581,17 @@ export async function handleFeishuMessage(params: { 0, feishuCfg?.historyLimit ?? cfg.messages?.groupChat?.historyLimit ?? DEFAULT_GROUP_HISTORY_LIMIT, ); + const groupConfig = isGroup + ? resolveFeishuGroupConfig({ cfg: feishuCfg, groupId: ctx.chatId }) + : undefined; + const dmPolicy = feishuCfg?.dmPolicy ?? "pairing"; + const configAllowFrom = feishuCfg?.allowFrom ?? []; + const useAccessGroups = cfg.commands?.useAccessGroups !== false; if (isGroup) { const groupPolicy = feishuCfg?.groupPolicy ?? "open"; const groupAllowFrom = feishuCfg?.groupAllowFrom ?? []; // DEBUG: log(`feishu[${account.accountId}]: groupPolicy=${groupPolicy}`); - const groupConfig = resolveFeishuGroupConfig({ cfg: feishuCfg, groupId: ctx.chatId }); // Check if this GROUP is allowed (groupAllowFrom contains group IDs like oc_xxx, not user IDs) const groupAllowed = isFeishuGroupAllowed({ @@ -642,12 +647,9 @@ export async function handleFeishuMessage(params: { return; } } else { - const dmPolicy = feishuCfg?.dmPolicy ?? "pairing"; - const allowFrom = feishuCfg?.allowFrom ?? []; - if (dmPolicy === "allowlist") { const match = resolveFeishuAllowlistMatch({ - allowFrom, + allowFrom: configAllowFrom, senderId: ctx.senderOpenId, }); if (!match.allowed) { @@ -659,6 +661,30 @@ export async function handleFeishuMessage(params: { try { const core = getFeishuRuntime(); + const shouldComputeCommandAuthorized = core.channel.commands.shouldComputeCommandAuthorized( + ctx.content, + cfg, + ); + const storeAllowFrom = + !isGroup && shouldComputeCommandAuthorized + ? await core.channel.pairing.readAllowFromStore("feishu").catch(() => []) + : []; + const commandAllowFrom = isGroup + ? (groupConfig?.allowFrom ?? []) + : [...configAllowFrom, ...storeAllowFrom]; + const senderAllowedForCommands = resolveFeishuAllowlistMatch({ + allowFrom: commandAllowFrom, + senderId: ctx.senderOpenId, + senderName: ctx.senderName, + }).allowed; + const commandAuthorized = shouldComputeCommandAuthorized + ? core.channel.commands.resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: commandAllowFrom.length > 0, allowed: senderAllowedForCommands }, + ], + }) + : undefined; // In group chats, the session is scoped to the group, but the *speaker* is the sender. // Using a group-scoped From causes the agent to treat different users as the same person. @@ -815,7 +841,7 @@ export async function handleFeishuMessage(params: { MessageSid: `${ctx.messageId}:permission-error`, Timestamp: Date.now(), WasMentioned: false, - CommandAuthorized: true, + CommandAuthorized: commandAuthorized, OriginatingChannel: "feishu" as const, OriginatingTo: feishuTo, }); @@ -903,7 +929,7 @@ export async function handleFeishuMessage(params: { ReplyToBody: quotedContent ?? undefined, Timestamp: Date.now(), WasMentioned: ctx.mentionedBot, - CommandAuthorized: true, + CommandAuthorized: commandAuthorized, OriginatingChannel: "feishu" as const, OriginatingTo: feishuTo, ...mediaPayload,