vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-03-12 07:20:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(discord): enforce users/roles allowlist in reaction ingress

Browse Source 
References GHSA-9vvh-2768-c8vp.
This commit is contained in:
Robin Waslander
2026-03-12 02:48:17 +01:00
parent 980619b9be
commit 487a3ba8ce
3 changed files with 164 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
src/discord/monitor.test.ts
View File

@@ -38,6 +38,7 @@ const makeEntries = (
requireMention: value.requireMention,
reactionNotifications: value.reactionNotifications,
users: value.users,
roles: value.roles,
channels: value.channels,
};
}
@@ -730,6 +731,17 @@ describe("discord reaction notification gating", () => {
},
expected: true,
},
{
name: "all mode blocks non-allowlisted guild member",
input: {
mode: "all" as const,
botId: "bot-1",
messageAuthorId: "user-1",
userId: "user-2",
guildInfo: { users: ["trusted-user"] },
},
expected: false,
},
{
name: "own mode with bot-authored message",
input: {
@@ -750,6 +762,17 @@ describe("discord reaction notification gating", () => {
},
expected: false,
},
{
name: "own mode still blocks member outside users allowlist",
input: {
mode: "own" as const,
botId: "bot-1",
messageAuthorId: "bot-1",
userId: "user-3",
guildInfo: { users: ["trusted-user"] },
},
expected: false,
},
{
name: "allowlist mode without match",
input: {
@@ -769,7 +792,7 @@ describe("discord reaction notification gating", () => {
messageAuthorId: "user-1",
userId: "123",
userName: "steipete",
allowlist: ["123", "other"] as string[],
guildInfo: { users: ["123", "other"] },
},
expected: true,
},
@@ -781,7 +804,7 @@ describe("discord reaction notification gating", () => {
messageAuthorId: "user-1",
userId: "999",
userName: "trusted-user",
allowlist: ["trusted-user"] as string[],
guildInfo: { users: ["trusted-user"] },
},
expected: false,
},
@@ -793,21 +816,29 @@ describe("discord reaction notification gating", () => {
messageAuthorId: "user-1",
userId: "999",
userName: "trusted-user",
allowlist: ["trusted-user"] as string[],
guildInfo: { users: ["trusted-user"] },
allowNameMatching: true,
},
expected: true,
},
{
name: "allowlist mode matches allowed role",
input: {
mode: "allowlist" as const,
botId: "bot-1",
messageAuthorId: "user-1",
userId: "999",
guildInfo: { roles: ["role:trusted-role"] },
memberRoleIds: ["trusted-role"],
},
expected: true,
},
]);
for (const testCase of cases) {
expect(
shouldEmitDiscordReactionNotification({
...testCase.input,
allowlist:
"allowlist" in testCase.input && testCase.input.allowlist
? [...testCase.input.allowlist]
: undefined,
}),
testCase.name,
).toBe(testCase.expected);
@@ -863,6 +894,7 @@ function makeReactionEvent(overrides?: {
messageAuthorId?: string;
messageFetch?: ReturnType<typeof vi.fn>;
guild?: { name?: string; id?: string };
memberRoleIds?: string[];
}) {
const userId = overrides?.userId ?? "user-1";
const messageId = overrides?.messageId ?? "msg-1";
@@ -882,6 +914,7 @@ function makeReactionEvent(overrides?: {
message_id: messageId,
emoji: { name: overrides?.emojiName ?? "👍", id: null },
guild: overrides?.guild,
rawMember: overrides?.memberRoleIds ? { roles: overrides.memberRoleIds } : undefined,
user: {
id: userId,
bot: false,
@@ -1059,7 +1092,31 @@ describe("discord DM reaction handling", () => {
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
});
it("still processes guild reactions (no regression)", async () => {
it("blocks guild reactions for sender outside users allowlist", async () => {
const data = makeReactionEvent({
guildId: "guild-123",
userId: "attacker-user",
botAsAuthor: true,
guild: { id: "guild-123", name: "Test Guild" },
});
const client = makeReactionClient({ channelType: ChannelType.GuildText });
const listener = new DiscordReactionListener(
makeReactionListenerParams({
guildEntries: makeEntries({
"guild-123": {
users: ["user:trusted-user"],
},
}),
}),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
expect(resolveAgentRouteMock).not.toHaveBeenCalled();
});
it("allows guild reactions for sender in channel role allowlist override", async () => {
resolveAgentRouteMock.mockReturnValueOnce({
agentId: "default",
channel: "discord",
@@ -1069,11 +1126,27 @@ describe("discord DM reaction handling", () => {
const data = makeReactionEvent({
guildId: "guild-123",
userId: "member-user",
botAsAuthor: true,
guild: { name: "Test Guild" },
guild: { id: "guild-123", name: "Test Guild" },
memberRoleIds: ["trusted-role"],
});
const client = makeReactionClient({ channelType: ChannelType.GuildText });
const listener = new DiscordReactionListener(makeReactionListenerParams());
const listener = new DiscordReactionListener(
makeReactionListenerParams({
guildEntries: makeEntries({
"guild-123": {
roles: ["role:blocked-role"],
channels: {
"channel-1": {
allow: true,
roles: ["role:trusted-role"],
},
},
},
}),
}),
);
await listener.handle(data, client);

38
src/discord/monitor/allow-list.ts
View File

@@ -556,6 +556,9 @@ export function shouldEmitDiscordReactionNotification(params: {
userId: string;
userName?: string;
userTag?: string;
channelConfig?: DiscordChannelConfigResolved | null;
guildInfo?: DiscordGuildEntryResolved | null;
memberRoleIds?: string[];
allowlist?: string[];
allowNameMatching?: boolean;
}) {
@@ -563,26 +566,31 @@ export function shouldEmitDiscordReactionNotification(params: {
if (mode === "off") {
return false;
}
const accessGuildInfo =
params.guildInfo ??
(params.allowlist ? ({ users: params.allowlist } satisfies DiscordGuildEntryResolved) : null);
const { hasAccessRestrictions, memberAllowed } = resolveDiscordMemberAccessState({
channelConfig: params.channelConfig,
guildInfo: accessGuildInfo,
memberRoleIds: params.memberRoleIds ?? [],
sender: {
id: params.userId,
name: params.userName,
tag: params.userTag,
},
allowNameMatching: params.allowNameMatching,
});
if (mode === "allowlist") {
return hasAccessRestrictions && memberAllowed;
}
if (hasAccessRestrictions && !memberAllowed) {
return false;
}
if (mode === "all") {
return true;
}
if (mode === "own") {
return Boolean(params.botId && params.messageAuthorId === params.botId);
}
if (mode === "allowlist") {
const list = normalizeDiscordAllowList(params.allowlist, ["discord:", "user:", "pk:"]);
if (!list) {
return false;
}
return allowListMatches(
list,
{
id: params.userId,
name: params.userName,
tag: params.userTag,
},
{ allowNameMatching: params.allowNameMatching },
);
}
return false;
}

84
src/discord/monitor/listeners.ts
View File

@@ -24,6 +24,7 @@ import {
normalizeDiscordSlug,
resolveDiscordAllowListMatch,
resolveDiscordChannelConfigWithFallback,
resolveDiscordMemberAccessState,
resolveGroupDmAllow,
resolveDiscordGuildEntry,
shouldEmitDiscordReactionNotification,
@@ -294,6 +295,7 @@ async function runDiscordReactionHandler(params: {
type DiscordReactionIngressAuthorizationParams = {
accountId: string;
user: User;
memberRoleIds: string[];
isDirectMessage: boolean;
isGroupDm: boolean;
isGuildMessage: boolean;
@@ -308,7 +310,7 @@ type DiscordReactionIngressAuthorizationParams = {
groupPolicy: "open" | "allowlist" | "disabled";
allowNameMatching: boolean;
guildInfo: import("./allow-list.js").DiscordGuildEntryResolved | null;
channelConfig?: { allowed?: boolean } | null;
channelConfig?: import("./allow-list.js").DiscordChannelConfigResolved | null;
};
async function authorizeDiscordReactionIngress(
@@ -383,6 +385,20 @@ async function authorizeDiscordReactionIngress(
if (params.channelConfig?.allowed === false) {
return { allowed: false, reason: "guild-channel-denied" };
}
const { hasAccessRestrictions, memberAllowed } = resolveDiscordMemberAccessState({
channelConfig: params.channelConfig,
guildInfo: params.guildInfo,
memberRoleIds: params.memberRoleIds,
sender: {
id: params.user.id,
name: params.user.username,
tag: formatDiscordUserTag(params.user),
},
allowNameMatching: params.allowNameMatching,
});
if (hasAccessRestrictions && !memberAllowed) {
return { allowed: false, reason: "guild-member-denied" };
}
return { allowed: true };
}
@@ -434,9 +450,13 @@ async function handleDiscordReactionEvent(
channelType === ChannelType.PublicThread ||
channelType === ChannelType.PrivateThread ||
channelType === ChannelType.AnnouncementThread;
const memberRoleIds = Array.isArray(data.rawMember?.roles)
? data.rawMember.roles.map((roleId: string) => String(roleId))
: [];
const reactionIngressBase: Omit<DiscordReactionIngressAuthorizationParams, "channelConfig"> = {
accountId: params.accountId,
user,
memberRoleIds,
isDirectMessage,
isGroupDm,
isGuildMessage,
@@ -452,17 +472,18 @@ async function handleDiscordReactionEvent(
allowNameMatching: params.allowNameMatching,
guildInfo,
};
const ingressAccess = await authorizeDiscordReactionIngress(reactionIngressBase);
if (!ingressAccess.allowed) {
logVerbose(`discord reaction blocked sender=${user.id} (reason=${ingressAccess.reason})`);
return;
// Guild reactions need resolved channel/thread config before member access
// can mirror the normal message preflight path.
if (!isGuildMessage) {
const ingressAccess = await authorizeDiscordReactionIngress(reactionIngressBase);
if (!ingressAccess.allowed) {
logVerbose(`discord reaction blocked sender=${user.id} (reason=${ingressAccess.reason})`);
return;
}
}
let parentId = "parentId" in channel ? (channel.parentId ?? undefined) : undefined;
let parentName: string | undefined;
let parentSlug = "";
const memberRoleIds = Array.isArray(data.rawMember?.roles)
? data.rawMember.roles.map((roleId: string) => String(roleId))
: [];
let reactionBase: { baseText: string; contextKey: string } | null = null;
const resolveReactionBase = () => {
if (reactionBase) {
@@ -507,6 +528,7 @@ async function handleDiscordReactionEvent(
const shouldNotifyReaction = (options: {
mode: "off" | "own" | "all" | "allowlist";
messageAuthorId?: string;
channelConfig?: ReturnType<typeof resolveDiscordChannelConfigWithFallback>;
}) =>
shouldEmitDiscordReactionNotification({
mode: options.mode,
@@ -515,7 +537,9 @@ async function handleDiscordReactionEvent(
userId: user.id,
userName: user.username,
userTag: formatDiscordUserTag(user),
allowlist: guildInfo?.users,
channelConfig: options.channelConfig,
guildInfo,
memberRoleIds,
allowNameMatching: params.allowNameMatching,
});
const emitReactionWithAuthor = (message: { author?: User } | null) => {
@@ -550,10 +574,12 @@ async function handleDiscordReactionEvent(
...reactionIngressBase,
channelConfig,
});
const authorizeThreadChannelAccess = async (channelInfo: { parentId?: string } | null) => {
const resolveThreadChannelAccess = async (channelInfo: { parentId?: string } | null) => {
parentId = channelInfo?.parentId;
await loadThreadParentInfo();
return await authorizeReactionIngressForChannel(resolveThreadChannelConfig());
const channelConfig = resolveThreadChannelConfig();
const access = await authorizeReactionIngressForChannel(channelConfig);
return { access, channelConfig };
};
// Parallelize async operations for thread channels
@@ -572,16 +598,18 @@ async function handleDiscordReactionEvent(
// Fast path: for "all" and "allowlist" modes, we don't need to fetch the message
if (reactionMode === "all" || reactionMode === "allowlist") {
const channelInfo = await channelInfoPromise;
const threadAccess = await authorizeThreadChannelAccess(channelInfo);
const { access: threadAccess, channelConfig: threadChannelConfig } =
await resolveThreadChannelAccess(channelInfo);
if (!threadAccess.allowed) {
return;
}
// For allowlist mode, check if user is in allowlist first
if (reactionMode === "allowlist") {
if (!shouldNotifyReaction({ mode: reactionMode })) {
return;
}
if (
!shouldNotifyReaction({
mode: reactionMode,
channelConfig: threadChannelConfig,
})
) {
return;
}
const { baseText } = resolveReactionBase();
@@ -593,13 +621,20 @@ async function handleDiscordReactionEvent(
const messagePromise = data.message.fetch().catch(() => null);
const [channelInfo, message] = await Promise.all([channelInfoPromise, messagePromise]);
const threadAccess = await authorizeThreadChannelAccess(channelInfo);
const { access: threadAccess, channelConfig: threadChannelConfig } =
await resolveThreadChannelAccess(channelInfo);
if (!threadAccess.allowed) {
return;
}
const messageAuthorId = message?.author?.id ?? undefined;
if (!shouldNotifyReaction({ mode: reactionMode, messageAuthorId })) {
if (
!shouldNotifyReaction({
mode: reactionMode,
messageAuthorId,
channelConfig: threadChannelConfig,
})
) {
return;
}
@@ -634,11 +669,8 @@ async function handleDiscordReactionEvent(
// Fast path: for "all" and "allowlist" modes, we don't need to fetch the message
if (reactionMode === "all" || reactionMode === "allowlist") {
// For allowlist mode, check if user is in allowlist first
if (reactionMode === "allowlist") {
if (!shouldNotifyReaction({ mode: reactionMode })) {
return;
}
if (!shouldNotifyReaction({ mode: reactionMode, channelConfig })) {
return;
}
const { baseText } = resolveReactionBase();
@@ -649,7 +681,7 @@ async function handleDiscordReactionEvent(
// For "own" mode, we need to fetch the message to check the author
const message = await data.message.fetch().catch(() => null);
const messageAuthorId = message?.author?.id ?? undefined;
if (!shouldNotifyReaction({ mode: reactionMode, messageAuthorId })) {
if (!shouldNotifyReaction({ mode: reactionMode, messageAuthorId, channelConfig })) {
return;
}