refactor(security): centralize trusted sender checks for discord moderation

This commit is contained in:
Peter Steinberger
2026-02-19 15:39:21 +01:00
parent 81b19aaa1a
commit c9dee59266
11 changed files with 292 additions and 145 deletions

View File

@@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai
- Security/Discord: enforce trusted-sender guild permission checks for moderation actions (`timeout`, `kick`, `ban`) and ignore untrusted `senderUserId` params to prevent privilege escalation in tool-driven flows. Thanks @aether-ai-agent for reporting.
- Security/ACP+Exec: add `openclaw acp --token-file/--password-file` secret-file support (with inline secret flag warnings), redact ACP working-directory prefixes to `~` home-relative paths, constrain exec script preflight file inspection to the effective `workdir` boundary, and add security-audit warnings when `tools.exec.host="sandbox"` is configured while sandbox mode is off.
- Security/Plugins/Hooks: enforce runtime/package path containment with realpath checks so `openclaw.extensions`, `openclaw.hooks`, and hook handler modules cannot escape their trusted roots via traversal or symlinks.
- Security/Discord: centralize trusted sender checks for moderation actions in message-action dispatch, share moderation command parsing across handlers, and clarify permission helpers with explicit any/all semantics.
- Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage.
- Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift.
- Security/Plugins: harden plugin discovery by blocking unsafe candidates (root escapes, world-writable paths, suspicious ownership), add startup warnings when `plugins.allow` is empty with discoverable non-bundled plugins, and warn on loaded plugins without install/load-path provenance.

View File

@@ -0,0 +1,48 @@
import { PermissionFlagsBits } from "discord-api-types/v10";
import { readNumberParam, readStringParam } from "./common.js";
export type DiscordModerationAction = "timeout" | "kick" | "ban";
export type DiscordModerationCommand = {
action: DiscordModerationAction;
guildId: string;
userId: string;
durationMinutes?: number;
until?: string;
reason?: string;
deleteMessageDays?: number;
};
const moderationPermissions: Record<DiscordModerationAction, bigint> = {
timeout: PermissionFlagsBits.ModerateMembers,
kick: PermissionFlagsBits.KickMembers,
ban: PermissionFlagsBits.BanMembers,
};
export function isDiscordModerationAction(action: string): action is DiscordModerationAction {
return action === "timeout" || action === "kick" || action === "ban";
}
export function requiredGuildPermissionForModerationAction(
action: DiscordModerationAction,
): bigint {
return moderationPermissions[action];
}
export function readDiscordModerationCommand(
action: string,
params: Record<string, unknown>,
): DiscordModerationCommand {
if (!isDiscordModerationAction(action)) {
throw new Error(`Unsupported Discord moderation action: ${action}`);
}
return {
action,
guildId: readStringParam(params, "guildId", { required: true }),
userId: readStringParam(params, "userId", { required: true }),
durationMinutes: readNumberParam(params, "durationMinutes", { integer: true }),
until: readStringParam(params, "until"),
reason: readStringParam(params, "reason"),
deleteMessageDays: readNumberParam(params, "deleteMessageDays", { integer: true }),
};
}

View File

@@ -7,10 +7,10 @@ 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),
hasAnyGuildPermissionDiscord: vi.fn(async () => false),
}));
const { banMemberDiscord, kickMemberDiscord, timeoutMemberDiscord, hasGuildPermissionDiscord } =
const { banMemberDiscord, kickMemberDiscord, timeoutMemberDiscord, hasAnyGuildPermissionDiscord } =
discordSendMocks;
vi.mock("../../discord/send.js", () => ({
@@ -21,7 +21,7 @@ const enableAllActions = (_key: keyof DiscordActionConfig, _defaultValue = true)
describe("discord moderation sender authorization", () => {
it("rejects ban when sender lacks BAN_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
hasAnyGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
@@ -35,7 +35,7 @@ describe("discord moderation sender authorization", () => {
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
expect(hasAnyGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.BanMembers],
@@ -45,7 +45,7 @@ describe("discord moderation sender authorization", () => {
});
it("rejects kick when sender lacks KICK_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
hasAnyGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
@@ -59,7 +59,7 @@ describe("discord moderation sender authorization", () => {
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
expect(hasAnyGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.KickMembers],
@@ -69,7 +69,7 @@ describe("discord moderation sender authorization", () => {
});
it("rejects timeout when sender lacks MODERATE_MEMBERS", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(false);
hasAnyGuildPermissionDiscord.mockResolvedValueOnce(false);
await expect(
handleDiscordModerationAction(
@@ -84,7 +84,7 @@ describe("discord moderation sender authorization", () => {
),
).rejects.toThrow("required permissions");
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
expect(hasAnyGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.ModerateMembers],
@@ -94,7 +94,7 @@ describe("discord moderation sender authorization", () => {
});
it("executes moderation action when sender has required permission", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(true);
hasAnyGuildPermissionDiscord.mockResolvedValueOnce(true);
kickMemberDiscord.mockResolvedValueOnce({ ok: true });
await handleDiscordModerationAction(
@@ -108,7 +108,7 @@ describe("discord moderation sender authorization", () => {
enableAllActions,
);
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
expect(hasAnyGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.KickMembers],
@@ -122,7 +122,7 @@ describe("discord moderation sender authorization", () => {
});
it("forwards accountId into permission check and moderation execution", async () => {
hasGuildPermissionDiscord.mockResolvedValueOnce(true);
hasAnyGuildPermissionDiscord.mockResolvedValueOnce(true);
timeoutMemberDiscord.mockResolvedValueOnce({ id: "user-1" });
await handleDiscordModerationAction(
@@ -137,7 +137,7 @@ describe("discord moderation sender authorization", () => {
enableAllActions,
);
expect(hasGuildPermissionDiscord).toHaveBeenCalledWith(
expect(hasAnyGuildPermissionDiscord).toHaveBeenCalledWith(
"guild-1",
"sender-1",
[PermissionFlagsBits.ModerateMembers],

View File

@@ -1,28 +1,32 @@
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
import { PermissionFlagsBits } from "discord-api-types/v10";
import type { DiscordActionConfig } from "../../config/config.js";
import {
banMemberDiscord,
hasGuildPermissionDiscord,
hasAnyGuildPermissionDiscord,
kickMemberDiscord,
timeoutMemberDiscord,
} from "../../discord/send.js";
import { type ActionGate, jsonResult, readStringParam } from "./common.js";
import {
isDiscordModerationAction,
readDiscordModerationCommand,
requiredGuildPermissionForModerationAction,
} from "./discord-actions-moderation-shared.js";
async function verifySenderModerationPermission(params: {
guildId: string;
senderUserId?: string;
requiredPermissions: bigint[];
requiredPermission: bigint;
accountId?: string;
}) {
// CLI/manual flows may not have sender context; enforce only when present.
if (!params.senderUserId) {
return;
}
const hasPermission = await hasGuildPermissionDiscord(
const hasPermission = await hasAnyGuildPermissionDiscord(
params.guildId,
params.senderUserId,
params.requiredPermissions,
[params.requiredPermission],
params.accountId ? { accountId: params.accountId } : undefined,
);
if (!hasPermission) {
@@ -35,117 +39,82 @@ export async function handleDiscordModerationAction(
params: Record<string, unknown>,
isActionEnabled: ActionGate<DiscordActionConfig>,
): Promise<AgentToolResult<unknown>> {
if (!isDiscordModerationAction(action)) {
throw new Error(`Unknown action: ${action}`);
}
if (!isActionEnabled("moderation", false)) {
throw new Error("Discord moderation is disabled.");
}
const command = readDiscordModerationCommand(action, params);
const accountId = readStringParam(params, "accountId");
const senderUserId = readStringParam(params, "senderUserId");
switch (action) {
await verifySenderModerationPermission({
guildId: command.guildId,
senderUserId,
requiredPermission: requiredGuildPermissionForModerationAction(command.action),
accountId,
});
switch (command.action) {
case "timeout": {
if (!isActionEnabled("moderation", false)) {
throw new Error("Discord moderation is disabled.");
}
const guildId = readStringParam(params, "guildId", {
required: true,
});
const userId = readStringParam(params, "userId", {
required: true,
});
const durationMinutes =
typeof params.durationMinutes === "number" && Number.isFinite(params.durationMinutes)
? params.durationMinutes
: undefined;
const until = readStringParam(params, "until");
const reason = readStringParam(params, "reason");
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.ModerateMembers],
accountId,
});
const member = accountId
? await timeoutMemberDiscord(
{
guildId,
userId,
durationMinutes,
until,
reason,
guildId: command.guildId,
userId: command.userId,
durationMinutes: command.durationMinutes,
until: command.until,
reason: command.reason,
},
{ accountId },
)
: await timeoutMemberDiscord({
guildId,
userId,
durationMinutes,
until,
reason,
guildId: command.guildId,
userId: command.userId,
durationMinutes: command.durationMinutes,
until: command.until,
reason: command.reason,
});
return jsonResult({ ok: true, member });
}
case "kick": {
if (!isActionEnabled("moderation", false)) {
throw new Error("Discord moderation is disabled.");
}
const guildId = readStringParam(params, "guildId", {
required: true,
});
const userId = readStringParam(params, "userId", {
required: true,
});
const reason = readStringParam(params, "reason");
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.KickMembers],
accountId,
});
if (accountId) {
await kickMemberDiscord({ guildId, userId, reason }, { accountId });
await kickMemberDiscord(
{
guildId: command.guildId,
userId: command.userId,
reason: command.reason,
},
{ accountId },
);
} else {
await kickMemberDiscord({ guildId, userId, reason });
await kickMemberDiscord({
guildId: command.guildId,
userId: command.userId,
reason: command.reason,
});
}
return jsonResult({ ok: true });
}
case "ban": {
if (!isActionEnabled("moderation", false)) {
throw new Error("Discord moderation is disabled.");
}
const guildId = readStringParam(params, "guildId", {
required: true,
});
const userId = readStringParam(params, "userId", {
required: true,
});
const reason = readStringParam(params, "reason");
const deleteMessageDays =
typeof params.deleteMessageDays === "number" && Number.isFinite(params.deleteMessageDays)
? params.deleteMessageDays
: undefined;
await verifySenderModerationPermission({
guildId,
senderUserId,
requiredPermissions: [PermissionFlagsBits.BanMembers],
accountId,
});
if (accountId) {
await banMemberDiscord(
{
guildId,
userId,
reason,
deleteMessageDays,
guildId: command.guildId,
userId: command.userId,
reason: command.reason,
deleteMessageDays: command.deleteMessageDays,
},
{ accountId },
);
} else {
await banMemberDiscord({
guildId,
userId,
reason,
deleteMessageDays,
guildId: command.guildId,
userId: command.userId,
reason: command.reason,
deleteMessageDays: command.deleteMessageDays,
});
}
return jsonResult({ ok: true });
}
default:
throw new Error(`Unknown action: ${action}`);
}
}

View File

@@ -379,21 +379,6 @@ describe("handleDiscordMessageAction", () => {
expect.any(Object),
);
});
it("rejects moderation when trusted sender id is missing in Discord tool context", async () => {
await expect(
handleDiscordMessageAction({
action: "kick",
params: {
guildId: "guild-1",
userId: "user-2",
},
cfg: {} as OpenClawConfig,
toolContext: { currentChannelProvider: "discord" },
}),
).rejects.toThrow("Sender user ID required for Discord moderation actions.");
expect(handleDiscordAction).not.toHaveBeenCalled();
});
});
describe("telegramMessageActions", () => {

View File

@@ -5,11 +5,15 @@ import {
readStringArrayParam,
readStringParam,
} from "../../../../agents/tools/common.js";
import {
isDiscordModerationAction,
readDiscordModerationCommand,
} from "../../../../agents/tools/discord-actions-moderation-shared.js";
import { handleDiscordAction } from "../../../../agents/tools/discord-actions.js";
type Ctx = Pick<
ChannelMessageActionContext,
"action" | "params" | "cfg" | "accountId" | "requesterSenderId" | "toolContext"
"action" | "params" | "cfg" | "accountId" | "requesterSenderId"
>;
export async function tryHandleDiscordMessageActionGuildAdmin(params: {
@@ -345,35 +349,25 @@ export async function tryHandleDiscordMessageActionGuildAdmin(params: {
);
}
if (action === "timeout" || action === "kick" || action === "ban") {
const guildId = readStringParam(actionParams, "guildId", {
required: true,
});
const userId = readStringParam(actionParams, "userId", { required: true });
const durationMinutes = readNumberParam(actionParams, "durationMin", {
integer: true,
});
const until = readStringParam(actionParams, "until");
const reason = readStringParam(actionParams, "reason");
const deleteMessageDays = readNumberParam(actionParams, "deleteDays", {
integer: true,
if (isDiscordModerationAction(action)) {
const moderation = readDiscordModerationCommand(action, {
...actionParams,
durationMinutes: readNumberParam(actionParams, "durationMin", { integer: true }),
deleteMessageDays: readNumberParam(actionParams, "deleteDays", {
integer: true,
}),
});
const senderUserId = ctx.requesterSenderId?.trim() || undefined;
// In channel/tool flows, require trusted sender identity for moderation authorization.
if (ctx.toolContext?.currentChannelProvider === "discord" && !senderUserId) {
throw new Error("Sender user ID required for Discord moderation actions.");
}
const discordAction = action;
return await handleDiscordAction(
{
action: discordAction,
action: moderation.action,
accountId: accountId ?? undefined,
guildId,
userId,
durationMinutes,
until,
reason,
deleteMessageDays,
guildId: moderation.guildId,
userId: moderation.userId,
durationMinutes: moderation.durationMinutes,
until: moderation.until,
reason: moderation.reason,
deleteMessageDays: moderation.deleteMessageDays,
senderUserId,
},
cfg,

View File

@@ -0,0 +1,82 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import type { ChannelPlugin } from "./types.js";
import { jsonResult } from "../../agents/tools/common.js";
import { setActivePluginRegistry } from "../../plugins/runtime.js";
import { createTestRegistry } from "../../test-utils/channel-plugins.js";
import { dispatchChannelMessageAction } from "./message-actions.js";
const handleAction = vi.fn(async () => jsonResult({ ok: true }));
const emptyRegistry = createTestRegistry([]);
const discordPlugin: ChannelPlugin = {
id: "discord",
meta: {
id: "discord",
label: "Discord",
selectionLabel: "Discord",
docsPath: "/channels/discord",
blurb: "Discord test plugin.",
},
capabilities: { chatTypes: ["direct", "group"] },
config: {
listAccountIds: () => ["default"],
resolveAccount: () => ({}),
},
actions: {
listActions: () => ["kick"],
supportsAction: ({ action }) => action === "kick",
handleAction,
},
};
describe("dispatchChannelMessageAction trusted sender guard", () => {
beforeEach(() => {
handleAction.mockClear();
setActivePluginRegistry(
createTestRegistry([{ pluginId: "discord", source: "test", plugin: discordPlugin }]),
);
});
afterEach(() => {
setActivePluginRegistry(emptyRegistry);
});
it("rejects privileged discord moderation action without trusted sender in tool context", async () => {
await expect(
dispatchChannelMessageAction({
channel: "discord",
action: "kick",
cfg: {} as OpenClawConfig,
params: { guildId: "g1", userId: "u1" },
toolContext: { currentChannelProvider: "discord" },
}),
).rejects.toThrow("Trusted sender identity is required for discord:kick");
expect(handleAction).not.toHaveBeenCalled();
});
it("allows privileged discord moderation action with trusted sender in tool context", async () => {
await dispatchChannelMessageAction({
channel: "discord",
action: "kick",
cfg: {} as OpenClawConfig,
params: { guildId: "g1", userId: "u1" },
requesterSenderId: "trusted-user",
toolContext: { currentChannelProvider: "discord" },
});
expect(handleAction).toHaveBeenCalledOnce();
});
it("does not require trusted sender without tool context", async () => {
await dispatchChannelMessageAction({
channel: "discord",
action: "kick",
cfg: {} as OpenClawConfig,
params: { guildId: "g1", userId: "u1" },
});
expect(handleAction).toHaveBeenCalledOnce();
});
});

View File

@@ -1,7 +1,18 @@
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
import type { OpenClawConfig } from "../../config/config.js";
import { getChannelPlugin, listChannelPlugins } from "./index.js";
import type { ChannelMessageActionContext, ChannelMessageActionName } from "./types.js";
import { getChannelPlugin, listChannelPlugins } from "./index.js";
const trustedRequesterRequiredByChannel: Readonly<
Partial<Record<string, ReadonlySet<ChannelMessageActionName>>>
> = {
discord: new Set<ChannelMessageActionName>(["timeout", "kick", "ban"]),
};
function requiresTrustedRequesterSender(ctx: ChannelMessageActionContext): boolean {
const actions = trustedRequesterRequiredByChannel[ctx.channel];
return Boolean(actions?.has(ctx.action) && ctx.toolContext);
}
export function listChannelMessageActions(cfg: OpenClawConfig): ChannelMessageActionName[] {
const actions = new Set<ChannelMessageActionName>(["send", "broadcast"]);
@@ -60,6 +71,11 @@ export function supportsChannelMessageCardsForChannel(params: {
export async function dispatchChannelMessageAction(
ctx: ChannelMessageActionContext,
): Promise<AgentToolResult<unknown> | null> {
if (requiresTrustedRequesterSender(ctx) && !ctx.requesterSenderId?.trim()) {
throw new Error(
`Trusted sender identity is required for ${ctx.channel}:${ctx.action} in tool-driven contexts.`,
);
}
const plugin = getChannelPlugin(ctx.channel);
if (!plugin?.actions?.handleAction) {
return null;

View File

@@ -3,7 +3,8 @@ import { PermissionFlagsBits, Routes } from "discord-api-types/v10";
import { describe, expect, it, vi } from "vitest";
import {
fetchMemberGuildPermissionsDiscord,
hasGuildPermissionDiscord,
hasAllGuildPermissionsDiscord,
hasAnyGuildPermissionDiscord,
} from "./send.permissions.js";
const mockRest = vi.hoisted(() => ({
@@ -54,7 +55,7 @@ describe("discord guild permission authorization", () => {
});
});
describe("hasGuildPermissionDiscord", () => {
describe("hasAnyGuildPermissionDiscord", () => {
it("returns true when user has required permission", async () => {
mockRest.get.mockImplementation(async (route: string) => {
if (route === Routes.guild("guild-1")) {
@@ -72,7 +73,7 @@ describe("discord guild permission authorization", () => {
throw new Error(`Unexpected route: ${route}`);
});
const result = await hasGuildPermissionDiscord("guild-1", "user-1", [
const result = await hasAnyGuildPermissionDiscord("guild-1", "user-1", [
PermissionFlagsBits.KickMembers,
]);
expect(result).toBe(true);
@@ -98,7 +99,7 @@ describe("discord guild permission authorization", () => {
throw new Error(`Unexpected route: ${route}`);
});
const result = await hasGuildPermissionDiscord("guild-1", "user-1", [
const result = await hasAnyGuildPermissionDiscord("guild-1", "user-1", [
PermissionFlagsBits.KickMembers,
]);
expect(result).toBe(true);
@@ -118,11 +119,37 @@ describe("discord guild permission authorization", () => {
throw new Error(`Unexpected route: ${route}`);
});
const result = await hasGuildPermissionDiscord("guild-1", "user-1", [
const result = await hasAnyGuildPermissionDiscord("guild-1", "user-1", [
PermissionFlagsBits.BanMembers,
PermissionFlagsBits.KickMembers,
]);
expect(result).toBe(false);
});
});
describe("hasAllGuildPermissionsDiscord", () => {
it("returns false when user has only one of multiple required permissions", async () => {
mockRest.get.mockImplementation(async (route: string) => {
if (route === Routes.guild("guild-1")) {
return {
id: "guild-1",
roles: [
{ id: "guild-1", permissions: "0" },
{ id: "role-mod", permissions: PermissionFlagsBits.KickMembers.toString() },
],
};
}
if (route === Routes.guildMember("guild-1", "user-1")) {
return { id: "user-1", roles: ["role-mod"] };
}
throw new Error(`Unexpected route: ${route}`);
});
const result = await hasAllGuildPermissionsDiscord("guild-1", "user-1", [
PermissionFlagsBits.KickMembers,
PermissionFlagsBits.BanMembers,
]);
expect(result).toBe(false);
});
});
});

View File

@@ -90,7 +90,7 @@ export async function fetchMemberGuildPermissionsDiscord(
/**
* Returns true when the user has ADMINISTRATOR or any required permission bit.
*/
export async function hasGuildPermissionDiscord(
export async function hasAnyGuildPermissionDiscord(
guildId: string,
userId: string,
requiredPermissions: bigint[],
@@ -106,6 +106,30 @@ export async function hasGuildPermissionDiscord(
return requiredPermissions.some((permission) => hasPermissionBit(permissions, permission));
}
/**
* Returns true when the user has ADMINISTRATOR or all required permission bits.
*/
export async function hasAllGuildPermissionsDiscord(
guildId: string,
userId: string,
requiredPermissions: bigint[],
opts: DiscordReactOpts = {},
): Promise<boolean> {
const permissions = await fetchMemberGuildPermissionsDiscord(guildId, userId, opts);
if (permissions === null) {
return false;
}
if (hasAdministrator(permissions)) {
return true;
}
return requiredPermissions.every((permission) => hasPermissionBit(permissions, permission));
}
/**
* @deprecated Prefer hasAnyGuildPermissionDiscord or hasAllGuildPermissionsDiscord for clarity.
*/
export const hasGuildPermissionDiscord = hasAnyGuildPermissionDiscord;
export async function fetchChannelPermissionsDiscord(
channelId: string,
opts: DiscordReactOpts = {},

View File

@@ -46,8 +46,9 @@ export {
export { sendDiscordComponentMessage } from "./send.components.js";
export {
fetchChannelPermissionsDiscord,
hasAllGuildPermissionsDiscord,
hasAnyGuildPermissionDiscord,
fetchMemberGuildPermissionsDiscord,
hasGuildPermissionDiscord,
} from "./send.permissions.js";
export {
fetchReactionsDiscord,