fix(security): enforce trusted sender auth for discord moderation

This commit is contained in:
Peter Steinberger
2026-02-19 15:18:00 +01:00
parent baa335f258
commit 775816035e
15 changed files with 498 additions and 22 deletions

View File

@@ -1,12 +1,12 @@
import type { OpenClawConfig } from "../config/config.js";
import { resolvePluginTools } from "../plugins/tools.js";
import type { GatewayMessageChannel } from "../utils/message-channel.js";
import { resolveSessionAgentId } from "./agent-scope.js";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
import type { AnyAgentTool } from "./tools/common.js";
import { resolvePluginTools } from "../plugins/tools.js";
import { resolveSessionAgentId } from "./agent-scope.js";
import { createAgentsListTool } from "./tools/agents-list-tool.js";
import { createBrowserTool } from "./tools/browser-tool.js";
import { createCanvasTool } from "./tools/canvas-tool.js";
import type { AnyAgentTool } from "./tools/common.js";
import { createCronTool } from "./tools/cron-tool.js";
import { createGatewayTool } from "./tools/gateway-tool.js";
import { createImageTool } from "./tools/image-tool.js";
@@ -61,6 +61,8 @@ export function createOpenClawTools(options?: {
requireExplicitMessageTarget?: boolean;
/** If true, omit the message tool from the tool list. */
disableMessageTool?: boolean;
/** Trusted sender id from inbound context (not tool args). */
requesterSenderId?: string | null;
/** Whether the requesting sender is an owner. */
senderIsOwner?: boolean;
}): AnyAgentTool[] {
@@ -98,6 +100,7 @@ export function createOpenClawTools(options?: {
hasRepliedRef: options?.hasRepliedRef,
sandboxRoot: options?.sandboxRoot,
requireExplicitTarget: options?.requireExplicitMessageTarget,
requesterSenderId: options?.requesterSenderId ?? undefined,
});
const tools: AnyAgentTool[] = [
createBrowserTool({

View File

@@ -7,6 +7,9 @@ import {
} from "@mariozechner/pi-coding-agent";
import type { OpenClawConfig } from "../config/config.js";
import type { ToolLoopDetectionConfig } from "../config/types.tools.js";
import type { ModelAuthMode } from "./model-auth.js";
import type { AnyAgentTool } from "./pi-tools.types.js";
import type { SandboxContext } from "./sandbox.js";
import { logWarn } from "../logger.js";
import { getPluginToolMeta } from "../plugins/tools.js";
import { isSubagentSessionKey } from "../routing/session-key.js";
@@ -21,7 +24,6 @@ import {
} from "./bash-tools.js";
import { listChannelAgentTools } from "./channel-tools.js";
import { resolveImageSanitizationLimits } from "./image-sanitization.js";
import type { ModelAuthMode } from "./model-auth.js";
import { createOpenClawTools } from "./openclaw-tools.js";
import { wrapToolWithAbortSignal } from "./pi-tools.abort.js";
import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js";
@@ -44,8 +46,6 @@ import {
wrapToolParamNormalization,
} from "./pi-tools.read.js";
import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js";
import type { AnyAgentTool } from "./pi-tools.types.js";
import type { SandboxContext } from "./sandbox.js";
import { getSubagentDepthFromSessionStore } from "./subagent-depth.js";
import {
applyToolPolicyPipeline,
@@ -455,6 +455,7 @@ export function createOpenClawCodingTools(options?: {
requireExplicitMessageTarget: options?.requireExplicitMessageTarget,
disableMessageTool: options?.disableMessageTool,
requesterAgentIdOverride: agentId,
requesterSenderId: options?.senderId,
senderIsOwner: options?.senderIsOwner,
}),
];

View File

@@ -0,0 +1,157 @@
import { PermissionFlagsBits } from "discord-api-types/v10";
import { describe, expect, it, vi } from "vitest";
import type { DiscordActionConfig } from "../../config/config.js";
import { handleDiscordModerationAction } from "./discord-actions-moderation.js";
const discordSendMocks = vi.hoisted(() => ({
banMemberDiscord: vi.fn(async () => ({ ok: true })),
kickMemberDiscord: vi.fn(async () => ({ ok: true })),
timeoutMemberDiscord: vi.fn(async () => ({ id: "user-1" })),
hasGuildPermissionDiscord: vi.fn(async () => false),
}));
const { banMemberDiscord, kickMemberDiscord, timeoutMemberDiscord, hasGuildPermissionDiscord } =
discordSendMocks;
vi.mock("../../discord/send.js", () => ({
...discordSendMocks,
}));
const enableAllActions = (_key: keyof DiscordActionConfig, _defaultValue = true) => true;
describe("discord moderation sender authorization", () => {
it("rejects ban when sender lacks BAN_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
"ban",
{
guildId: "guild-1",
userId: "user-1",
senderUserId: "sender-1",
},
enableAllActions,
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.BanMembers],
undefined,
);
expect(banMemberDiscord).not.toHaveBeenCalled();
});
it("rejects kick when sender lacks KICK_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
"kick",
{
guildId: "guild-1",
userId: "user-1",
senderUserId: "sender-1",
},
enableAllActions,
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.KickMembers],
undefined,
);
expect(kickMemberDiscord).not.toHaveBeenCalled();
});
it("rejects timeout when sender lacks MODERATE_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
"timeout",
{
guildId: "guild-1",
userId: "user-1",
senderUserId: "sender-1",
durationMinutes: 60,
},
enableAllActions,
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.ModerateMembers],
undefined,
);
expect(timeoutMemberDiscord).not.toHaveBeenCalled();
});
it("executes moderation action when sender has required permission", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(true);
kickMemberDiscord.mockResolvedValueOnce({ ok: true });
await handleDiscordModerationAction(
"kick",
{
guildId: "guild-1",
userId: "user-1",
senderUserId: "sender-1",
reason: "rule violation",
},
enableAllActions,
);
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.KickMembers],
undefined,
);
expect(kickMemberDiscord).toHaveBeenCalledWith({
guildId: "guild-1",
userId: "user-1",
reason: "rule violation",
});
});
it("forwards accountId into permission check and moderation execution", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(true);
timeoutMemberDiscord.mockResolvedValueOnce({ id: "user-1" });
await handleDiscordModerationAction(
"timeout",
{
guildId: "guild-1",
userId: "user-1",
senderUserId: "sender-1",
accountId: "ops",
durationMinutes: 5,
},
enableAllActions,
);
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.ModerateMembers],
{ accountId: "ops" },
);
expect(timeoutMemberDiscord).toHaveBeenCalledWith(
{
guildId: "guild-1",
userId: "user-1",
durationMinutes: 5,
until: undefined,
reason: undefined,
},
{ accountId: "ops" },
);
});
});

View File

@@ -1,14 +1,42 @@
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
import { PermissionFlagsBits } from "discord-api-types/v10";
import type { DiscordActionConfig } from "../../config/config.js";
import { banMemberDiscord, kickMemberDiscord, timeoutMemberDiscord } from "../../discord/send.js";
import {
banMemberDiscord,
hasGuildPermissionDiscord,
kickMemberDiscord,
timeoutMemberDiscord,
} from "../../discord/send.js";
import { type ActionGate, jsonResult, readStringParam } from "./common.js";
async function verifySenderModerationPermission(params: {
guildId: string;
senderUserId?: string;
requiredPermissions: bigint[];
accountId?: string;
}) {
// CLI/manual flows may not have sender context; enforce only when present.
if (!params.senderUserId) {
return;
}
const hasPermission = await hasGuildPermissionDiscord(
params.guildId,
params.senderUserId,
params.requiredPermissions,
params.accountId ? { accountId: params.accountId } : undefined,
);
if (!hasPermission) {
throw new Error("Sender does not have required permissions for this moderation action.");
}
}
export async function handleDiscordModerationAction(
action: string,
params: Record<string, unknown>,
isActionEnabled: ActionGate<DiscordActionConfig>,
): Promise<AgentToolResult<unknown>> {
const accountId = readStringParam(params, "accountId");
const senderUserId = readStringParam(params, "senderUserId");
switch (action) {
case "timeout": {
if (!isActionEnabled("moderation", false)) {
@@ -26,6 +54,12 @@ export async function handleDiscordModerationAction(
: undefined;
const until = readStringParam(params, "until");
const reason = readStringParam(params, "reason");
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.ModerateMembers],
accountId,
});
const member = accountId
? await timeoutMemberDiscord(
{
@@ -57,6 +91,12 @@ export async function handleDiscordModerationAction(
required: true,
});
const reason = readStringParam(params, "reason");
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.KickMembers],
accountId,
});
if (accountId) {
await kickMemberDiscord({ guildId, userId, reason }, { accountId });
} else {
@@ -79,6 +119,12 @@ export async function handleDiscordModerationAction(
typeof params.deleteMessageDays === "number" && Number.isFinite(params.deleteMessageDays)
? params.deleteMessageDays
: undefined;
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.BanMembers],
accountId,
});
if (accountId) {
await banMemberDiscord(
{

View File

@@ -329,4 +329,22 @@ describe("message tool sandbox passthrough", () => {
const call = mocks.runMessageAction.mock.calls[0]?.[0];
expect(call?.sandboxRoot).toBeUndefined();
});
it("forwards trusted requesterSenderId to runMessageAction", async () => {
mockSendResult({ to: "discord:123" });
const tool = createMessageTool({
config: {} as never,
requesterSenderId: "1234567890",
});
await tool.execute("1", {
action: "send",
target: "discord:123",
message: "hi",
});
const call = mocks.runMessageAction.mock.calls[0]?.[0];
expect(call?.requesterSenderId).toBe("1234567890");
});
});

View File

@@ -1,4 +1,6 @@
import { Type } from "@sinclair/typebox";
import type { OpenClawConfig } from "../../config/config.js";
import type { AnyAgentTool } from "./common.js";
import { BLUEBUBBLES_GROUP_ACTIONS } from "../../channels/plugins/bluebubbles-actions.js";
import {
listChannelMessageActions,
@@ -11,7 +13,6 @@ import {
CHANNEL_MESSAGE_ACTION_NAMES,
type ChannelMessageActionName,
} from "../../channels/plugins/types.js";
import type { OpenClawConfig } from "../../config/config.js";
import { loadConfig } from "../../config/config.js";
import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../gateway/protocol/client-info.js";
import { getToolResult, runMessageAction } from "../../infra/outbound/message-action-runner.js";
@@ -22,7 +23,6 @@ import { normalizeMessageChannel } from "../../utils/message-channel.js";
import { resolveSessionAgentId } from "../agent-scope.js";
import { listChannelSupportedActions } from "../channel-tools.js";
import { channelTargetSchema, channelTargetsSchema, stringEnum } from "../schema/typebox.js";
import type { AnyAgentTool } from "./common.js";
import { jsonResult, readNumberParam, readStringParam } from "./common.js";
import { resolveGatewayOptions } from "./gateway.js";
@@ -429,6 +429,7 @@ type MessageToolOptions = {
hasRepliedRef?: { value: boolean };
sandboxRoot?: string;
requireExplicitTarget?: boolean;
requesterSenderId?: string;
};
function resolveMessageToolSchemaActions(params: {
@@ -656,6 +657,7 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
action,
params,
defaultAccountId: accountId ?? undefined,
requesterSenderId: options?.requesterSenderId,
gateway,
toolContext,
sessionKey: options?.agentSessionKey,