mirror of
https://github.com/farcasclaudiu/openclaw.git
synced 2026-06-28 23:02:02 +03:00
fix(aa-01): apply security fix
Generated by staged fix workflow.
This commit is contained in:
committed by
Peter Steinberger
parent
78ec0a1edf
commit
f05553413d
@@ -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",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -581,12 +581,17 @@ export async function handleFeishuMessage(params: {
|
|||||||
0,
|
0,
|
||||||
feishuCfg?.historyLimit ?? cfg.messages?.groupChat?.historyLimit ?? DEFAULT_GROUP_HISTORY_LIMIT,
|
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) {
|
if (isGroup) {
|
||||||
const groupPolicy = feishuCfg?.groupPolicy ?? "open";
|
const groupPolicy = feishuCfg?.groupPolicy ?? "open";
|
||||||
const groupAllowFrom = feishuCfg?.groupAllowFrom ?? [];
|
const groupAllowFrom = feishuCfg?.groupAllowFrom ?? [];
|
||||||
// DEBUG: log(`feishu[${account.accountId}]: groupPolicy=${groupPolicy}`);
|
// 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)
|
// Check if this GROUP is allowed (groupAllowFrom contains group IDs like oc_xxx, not user IDs)
|
||||||
const groupAllowed = isFeishuGroupAllowed({
|
const groupAllowed = isFeishuGroupAllowed({
|
||||||
@@ -642,12 +647,9 @@ export async function handleFeishuMessage(params: {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
const dmPolicy = feishuCfg?.dmPolicy ?? "pairing";
|
|
||||||
const allowFrom = feishuCfg?.allowFrom ?? [];
|
|
||||||
|
|
||||||
if (dmPolicy === "allowlist") {
|
if (dmPolicy === "allowlist") {
|
||||||
const match = resolveFeishuAllowlistMatch({
|
const match = resolveFeishuAllowlistMatch({
|
||||||
allowFrom,
|
allowFrom: configAllowFrom,
|
||||||
senderId: ctx.senderOpenId,
|
senderId: ctx.senderOpenId,
|
||||||
});
|
});
|
||||||
if (!match.allowed) {
|
if (!match.allowed) {
|
||||||
@@ -659,6 +661,30 @@ export async function handleFeishuMessage(params: {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
const core = getFeishuRuntime();
|
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.
|
// 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.
|
// 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`,
|
MessageSid: `${ctx.messageId}:permission-error`,
|
||||||
Timestamp: Date.now(),
|
Timestamp: Date.now(),
|
||||||
WasMentioned: false,
|
WasMentioned: false,
|
||||||
CommandAuthorized: true,
|
CommandAuthorized: commandAuthorized,
|
||||||
OriginatingChannel: "feishu" as const,
|
OriginatingChannel: "feishu" as const,
|
||||||
OriginatingTo: feishuTo,
|
OriginatingTo: feishuTo,
|
||||||
});
|
});
|
||||||
@@ -903,7 +929,7 @@ export async function handleFeishuMessage(params: {
|
|||||||
ReplyToBody: quotedContent ?? undefined,
|
ReplyToBody: quotedContent ?? undefined,
|
||||||
Timestamp: Date.now(),
|
Timestamp: Date.now(),
|
||||||
WasMentioned: ctx.mentionedBot,
|
WasMentioned: ctx.mentionedBot,
|
||||||
CommandAuthorized: true,
|
CommandAuthorized: commandAuthorized,
|
||||||
OriginatingChannel: "feishu" as const,
|
OriginatingChannel: "feishu" as const,
|
||||||
OriginatingTo: feishuTo,
|
OriginatingTo: feishuTo,
|
||||||
...mediaPayload,
|
...mediaPayload,
|
||||||
|
|||||||
Reference in New Issue
Block a user