fix: harden discord and slack reaction ingress authorization

This commit is contained in:
Peter Steinberger
2026-02-26 01:26:37 +01:00
parent c736f11a16
commit aedf62ac7e
8 changed files with 483 additions and 5 deletions

View File

@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
- Slack/Session threads: prevent oversized parent-session inheritance from silently bricking new thread sessions, surface embedded context-overflow empty-result failures to users, and add configurable `session.parentForkMaxTokens` (default `100000`, `0` disables). (#26912) Thanks @markshields-tl.
- Security/Signal: enforce DM/group authorization before reaction-only notification enqueue so unauthorized senders can no longer inject Signal reaction system events under `dmPolicy`/`groupPolicy`; reaction notifications now require channel access checks first. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Security/Discord + Slack reactions: enforce DM policy/allowlist authorization before reaction-event system enqueue in direct messages; Discord reaction handling now also honors DM/group-DM enablement and guild `groupPolicy` channel gating to keep reaction ingress aligned with normal message preflight. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Security/Telegram reactions: enforce `dmPolicy`/`allowFrom` and group allowlist authorization on `message_reaction` events before enqueueing reaction system events, preventing unauthorized reaction-triggered input in DMs and groups; ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.

View File

@@ -1,5 +1,5 @@
import { ChannelType, type Guild } from "@buape/carbon";
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { typedCases } from "../test-utils/typed-cases.js";
import {
allowListMatches,
@@ -20,6 +20,12 @@ import {
} from "./monitor.js";
import { DiscordMessageListener, DiscordReactionListener } from "./monitor/listeners.js";
const readAllowFromStoreMock = vi.hoisted(() => vi.fn());
vi.mock("../pairing/pairing-store.js", () => ({
readChannelAllowFromStore: (...args: unknown[]) => readAllowFromStoreMock(...args),
}));
const fakeGuild = (id: string, name: string) => ({ id, name }) as Guild;
const makeEntries = (
@@ -899,6 +905,12 @@ function makeReactionClient(options?: {
function makeReactionListenerParams(overrides?: {
botUserId?: string;
dmEnabled?: boolean;
groupDmEnabled?: boolean;
groupDmChannels?: string[];
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
allowFrom?: string[];
groupPolicy?: "open" | "allowlist" | "disabled";
allowNameMatching?: boolean;
guildEntries?: Record<string, DiscordGuildEntryResolved>;
}) {
@@ -907,6 +919,12 @@ function makeReactionListenerParams(overrides?: {
accountId: "acc-1",
runtime: {} as import("../runtime.js").RuntimeEnv,
botUserId: overrides?.botUserId ?? "bot-1",
dmEnabled: overrides?.dmEnabled ?? true,
groupDmEnabled: overrides?.groupDmEnabled ?? true,
groupDmChannels: overrides?.groupDmChannels ?? [],
dmPolicy: overrides?.dmPolicy ?? "open",
allowFrom: overrides?.allowFrom ?? [],
groupPolicy: overrides?.groupPolicy ?? "open",
allowNameMatching: overrides?.allowNameMatching ?? false,
guildEntries: overrides?.guildEntries,
logger: {
@@ -919,6 +937,12 @@ function makeReactionListenerParams(overrides?: {
}
describe("discord DM reaction handling", () => {
beforeEach(() => {
enqueueSystemEventSpy.mockClear();
resolveAgentRouteMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
});
it("processes DM reactions with or without guild allowlists", async () => {
const cases = [
{ name: "no guild allowlist", guildEntries: undefined },
@@ -952,9 +976,77 @@ describe("discord DM reaction handling", () => {
}
});
it("blocks DM reactions when dmPolicy is disabled", async () => {
const data = makeReactionEvent({ botAsAuthor: true });
const client = makeReactionClient({ channelType: ChannelType.DM });
const listener = new DiscordReactionListener(
makeReactionListenerParams({ dmPolicy: "disabled" }),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
});
it("blocks DM reactions for unauthorized sender in allowlist mode", async () => {
const data = makeReactionEvent({ botAsAuthor: true, userId: "user-1" });
const client = makeReactionClient({ channelType: ChannelType.DM });
const listener = new DiscordReactionListener(
makeReactionListenerParams({
dmPolicy: "allowlist",
allowFrom: ["user:user-2"],
}),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
});
it("allows DM reactions for authorized sender in allowlist mode", async () => {
const data = makeReactionEvent({ botAsAuthor: true, userId: "user-1" });
const client = makeReactionClient({ channelType: ChannelType.DM });
const listener = new DiscordReactionListener(
makeReactionListenerParams({
dmPolicy: "allowlist",
allowFrom: ["user:user-1"],
}),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).toHaveBeenCalledOnce();
});
it("blocks group DM reactions when group DMs are disabled", async () => {
const data = makeReactionEvent({ botAsAuthor: true });
const client = makeReactionClient({ channelType: ChannelType.GroupDM });
const listener = new DiscordReactionListener(
makeReactionListenerParams({ groupDmEnabled: false }),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
});
it("blocks guild reactions when groupPolicy is disabled", async () => {
const data = makeReactionEvent({
guildId: "guild-123",
botAsAuthor: true,
guild: { id: "guild-123", name: "Guild" },
});
const client = makeReactionClient({ channelType: ChannelType.GuildText });
const listener = new DiscordReactionListener(
makeReactionListenerParams({ groupPolicy: "disabled" }),
);
await listener.handle(data, client);
expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
});
it("still processes guild reactions (no regression)", async () => {
enqueueSystemEventSpy.mockClear();
resolveAgentRouteMock.mockClear();
resolveAgentRouteMock.mockReturnValueOnce({
agentId: "default",
channel: "discord",

View File

@@ -7,14 +7,20 @@ import {
PresenceUpdateListener,
type User,
} from "@buape/carbon";
import { danger } from "../../globals.js";
import { danger, logVerbose } from "../../globals.js";
import { formatDurationSeconds } from "../../infra/format-time/format-duration.ts";
import { enqueueSystemEvent } from "../../infra/system-events.js";
import { createSubsystemLogger } from "../../logging/subsystem.js";
import { readChannelAllowFromStore } from "../../pairing/pairing-store.js";
import { resolveAgentRoute } from "../../routing/resolve-route.js";
import { resolveDmGroupAccessWithLists } from "../../security/dm-policy-shared.js";
import {
isDiscordGroupAllowedByPolicy,
normalizeDiscordAllowList,
normalizeDiscordSlug,
resolveDiscordAllowListMatch,
resolveDiscordChannelConfigWithFallback,
resolveGroupDmAllow,
resolveDiscordGuildEntry,
shouldEmitDiscordReactionNotification,
} from "./allow-list.js";
@@ -37,6 +43,12 @@ type DiscordReactionListenerParams = {
accountId: string;
runtime: RuntimeEnv;
botUserId?: string;
dmEnabled: boolean;
groupDmEnabled: boolean;
groupDmChannels: string[];
dmPolicy: "open" | "pairing" | "allowlist" | "disabled";
allowFrom: string[];
groupPolicy: "open" | "allowlist" | "disabled";
allowNameMatching: boolean;
guildEntries?: Record<string, import("./allow-list.js").DiscordGuildEntryResolved>;
logger: Logger;
@@ -179,6 +191,12 @@ async function runDiscordReactionHandler(params: {
cfg: params.handlerParams.cfg,
accountId: params.handlerParams.accountId,
botUserId: params.handlerParams.botUserId,
dmEnabled: params.handlerParams.dmEnabled,
groupDmEnabled: params.handlerParams.groupDmEnabled,
groupDmChannels: params.handlerParams.groupDmChannels,
dmPolicy: params.handlerParams.dmPolicy,
allowFrom: params.handlerParams.allowFrom,
groupPolicy: params.handlerParams.groupPolicy,
allowNameMatching: params.handlerParams.allowNameMatching,
guildEntries: params.handlerParams.guildEntries,
logger: params.handlerParams.logger,
@@ -193,6 +211,12 @@ async function handleDiscordReactionEvent(params: {
cfg: LoadedConfig;
accountId: string;
botUserId?: string;
dmEnabled: boolean;
groupDmEnabled: boolean;
groupDmChannels: string[];
dmPolicy: "open" | "pairing" | "allowlist" | "disabled";
allowFrom: string[];
groupPolicy: "open" | "allowlist" | "disabled";
allowNameMatching: boolean;
guildEntries?: Record<string, import("./allow-list.js").DiscordGuildEntryResolved>;
logger: Logger;
@@ -236,6 +260,12 @@ async function handleDiscordReactionEvent(params: {
channelType === ChannelType.PublicThread ||
channelType === ChannelType.PrivateThread ||
channelType === ChannelType.AnnouncementThread;
if (isDirectMessage && !params.dmEnabled) {
return;
}
if (isGroupDm && !params.groupDmEnabled) {
return;
}
let parentId = "parentId" in channel ? (channel.parentId ?? undefined) : undefined;
let parentName: string | undefined;
let parentSlug = "";
@@ -264,6 +294,45 @@ async function handleDiscordReactionEvent(params: {
reactionBase = { baseText, contextKey };
return reactionBase;
};
const isDirectReactionAuthorized = async () => {
if (!isDirectMessage) {
return true;
}
const storeAllowFrom =
params.dmPolicy === "allowlist"
? []
: await readChannelAllowFromStore("discord").catch(() => []);
const access = resolveDmGroupAccessWithLists({
isGroup: false,
dmPolicy: params.dmPolicy,
groupPolicy: params.groupPolicy,
allowFrom: params.allowFrom,
groupAllowFrom: [],
storeAllowFrom,
isSenderAllowed: (allowEntries) => {
const allowList = normalizeDiscordAllowList(allowEntries, ["discord:", "user:", "pk:"]);
const allowMatch = allowList
? resolveDiscordAllowListMatch({
allowList,
candidate: {
id: user.id,
name: user.username,
tag: formatDiscordUserTag(user),
},
allowNameMatching: params.allowNameMatching,
})
: { allowed: false };
return allowMatch.allowed;
},
});
if (access.decision !== "allow") {
logVerbose(
`discord reaction blocked sender=${user.id} (dmPolicy=${params.dmPolicy}, decision=${access.decision}, reason=${access.reason})`,
);
return false;
}
return true;
};
const emitReaction = (text: string, parentPeerId?: string) => {
const { contextKey } = resolveReactionBase();
const route = resolveAgentRoute({
@@ -322,6 +391,44 @@ async function handleDiscordReactionEvent(params: {
parentSlug,
scope: "thread",
});
const isGuildReactionAllowed = (channelConfig: { allowed?: boolean } | null) => {
if (!isGuildMessage) {
return true;
}
const channelAllowlistConfigured =
Boolean(guildInfo?.channels) && Object.keys(guildInfo?.channels ?? {}).length > 0;
const channelAllowed = channelConfig?.allowed !== false;
if (
!isDiscordGroupAllowedByPolicy({
groupPolicy: params.groupPolicy,
guildAllowlisted: Boolean(guildInfo),
channelAllowlistConfigured,
channelAllowed,
})
) {
return false;
}
if (channelConfig?.allowed === false) {
return false;
}
return true;
};
if (!(await isDirectReactionAuthorized())) {
return;
}
if (
isGroupDm &&
!resolveGroupDmAllow({
channels: params.groupDmChannels,
channelId: data.channel_id,
channelName,
channelSlug,
})
) {
return;
}
// Parallelize async operations for thread channels
if (isThreadChannel) {
@@ -370,6 +477,9 @@ async function handleDiscordReactionEvent(params: {
if (channelConfig?.allowed === false) {
return;
}
if (!isGuildReactionAllowed(channelConfig)) {
return;
}
const messageAuthorId = message?.author?.id ?? undefined;
if (!shouldNotifyReaction({ mode: reactionMode, messageAuthorId })) {
@@ -394,6 +504,9 @@ async function handleDiscordReactionEvent(params: {
if (channelConfig?.allowed === false) {
return;
}
if (!isGuildReactionAllowed(channelConfig)) {
return;
}
const reactionMode = guildInfo?.reactionNotifications ?? "own";

View File

@@ -561,6 +561,12 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) {
accountId: account.accountId,
runtime,
botUserId,
dmEnabled,
groupDmEnabled,
groupDmChannels: groupDmChannels ?? [],
dmPolicy,
allowFrom: allowFrom ?? [],
groupPolicy,
allowNameMatching: isDangerousNameMatchingEnabled(discordCfg),
guildEntries,
logger,
@@ -573,6 +579,12 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) {
accountId: account.accountId,
runtime,
botUserId,
dmEnabled,
groupDmEnabled,
groupDmChannels: groupDmChannels ?? [],
dmPolicy,
allowFrom: allowFrom ?? [],
groupPolicy,
allowNameMatching: isDangerousNameMatchingEnabled(discordCfg),
guildEntries,
logger,

View File

@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
resolveDmAllowState,
resolveDmGroupAccessDecision,
resolveDmGroupAccessWithLists,
resolveEffectiveAllowFromLists,
} from "./dm-policy-shared.js";
@@ -75,6 +76,37 @@ describe("security/dm-policy-shared", () => {
expect(lists.effectiveGroupAllowFrom).toEqual(["+1111", "+2222"]);
});
it("resolves access + effective allowlists in one shared call", () => {
const resolved = resolveDmGroupAccessWithLists({
isGroup: false,
dmPolicy: "pairing",
groupPolicy: "allowlist",
allowFrom: ["owner"],
groupAllowFrom: ["group:room"],
storeAllowFrom: ["paired-user"],
isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"),
});
expect(resolved.decision).toBe("allow");
expect(resolved.reason).toBe("dmPolicy=pairing (allowlisted)");
expect(resolved.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
expect(resolved.effectiveGroupAllowFrom).toEqual(["group:room", "paired-user"]);
});
it("keeps allowlist mode strict in shared resolver (no pairing-store fallback)", () => {
const resolved = resolveDmGroupAccessWithLists({
isGroup: false,
dmPolicy: "allowlist",
groupPolicy: "allowlist",
allowFrom: ["owner"],
groupAllowFrom: [],
storeAllowFrom: ["paired-user"],
isSenderAllowed: () => false,
});
expect(resolved.decision).toBe("block");
expect(resolved.reason).toBe("dmPolicy=allowlist (not allowlisted)");
expect(resolved.effectiveAllowFrom).toEqual(["owner"]);
});
const channels = [
"bluebubbles",
"imessage",

View File

@@ -77,6 +77,41 @@ export function resolveDmGroupAccessDecision(params: {
return { decision: "block", reason: `dmPolicy=${dmPolicy} (not allowlisted)` };
}
export function resolveDmGroupAccessWithLists(params: {
isGroup: boolean;
dmPolicy?: string | null;
groupPolicy?: string | null;
allowFrom?: Array<string | number> | null;
groupAllowFrom?: Array<string | number> | null;
storeAllowFrom?: Array<string | number> | null;
isSenderAllowed: (allowFrom: string[]) => boolean;
}): {
decision: DmGroupAccessDecision;
reason: string;
effectiveAllowFrom: string[];
effectiveGroupAllowFrom: string[];
} {
const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveEffectiveAllowFromLists({
allowFrom: params.allowFrom,
groupAllowFrom: params.groupAllowFrom,
storeAllowFrom: params.storeAllowFrom,
dmPolicy: params.dmPolicy,
});
const access = resolveDmGroupAccessDecision({
isGroup: params.isGroup,
dmPolicy: params.dmPolicy,
groupPolicy: params.groupPolicy,
effectiveAllowFrom,
effectiveGroupAllowFrom,
isSenderAllowed: params.isSenderAllowed,
});
return {
...access,
effectiveAllowFrom,
effectiveGroupAllowFrom,
};
}
export async function resolveDmAllowState(params: {
provider: ChannelId;
allowFrom?: Array<string | number> | null;

View File

@@ -0,0 +1,163 @@
import { describe, expect, it, vi } from "vitest";
import type { SlackMonitorContext } from "../context.js";
import { registerSlackReactionEvents } from "./reactions.js";
const enqueueSystemEventMock = vi.fn();
const readAllowFromStoreMock = vi.fn();
vi.mock("../../../infra/system-events.js", () => ({
enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args),
}));
vi.mock("../../../pairing/pairing-store.js", () => ({
readChannelAllowFromStore: (...args: unknown[]) => readAllowFromStoreMock(...args),
}));
type SlackReactionHandler = (args: {
event: Record<string, unknown>;
body: unknown;
}) => Promise<void>;
function createReactionContext(overrides?: {
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
allowFrom?: string[];
channelType?: "im" | "channel";
}) {
let addedHandler: SlackReactionHandler | null = null;
let removedHandler: SlackReactionHandler | null = null;
const channelType = overrides?.channelType ?? "im";
const app = {
event: vi.fn((name: string, handler: SlackReactionHandler) => {
if (name === "reaction_added") {
addedHandler = handler;
} else if (name === "reaction_removed") {
removedHandler = handler;
}
}),
};
const ctx = {
app,
runtime: { error: vi.fn() },
dmPolicy: overrides?.dmPolicy ?? "open",
groupPolicy: "open",
allowFrom: overrides?.allowFrom ?? [],
allowNameMatching: false,
shouldDropMismatchedSlackEvent: vi.fn().mockReturnValue(false),
isChannelAllowed: vi.fn().mockReturnValue(true),
resolveChannelName: vi.fn().mockResolvedValue({
name: channelType === "im" ? "direct" : "general",
type: channelType,
}),
resolveUserName: vi.fn().mockResolvedValue({ name: "alice" }),
resolveSlackSystemEventSessionKey: vi.fn().mockReturnValue("agent:main:main"),
} as unknown as SlackMonitorContext;
registerSlackReactionEvents({ ctx });
return {
ctx,
getAddedHandler: () => addedHandler,
getRemovedHandler: () => removedHandler,
};
}
function makeReactionEvent(overrides?: { user?: string; channel?: string }) {
return {
type: "reaction_added",
user: overrides?.user ?? "U1",
reaction: "thumbsup",
item: {
type: "message",
channel: overrides?.channel ?? "D1",
ts: "123.456",
},
item_user: "UBOT",
};
}
describe("registerSlackReactionEvents", () => {
it("enqueues DM reaction system events when dmPolicy is open", async () => {
enqueueSystemEventMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
const { getAddedHandler } = createReactionContext({ dmPolicy: "open" });
const addedHandler = getAddedHandler();
expect(addedHandler).toBeTruthy();
await addedHandler!({
event: makeReactionEvent(),
body: {},
});
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
});
it("blocks DM reaction system events when dmPolicy is disabled", async () => {
enqueueSystemEventMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
const { getAddedHandler } = createReactionContext({ dmPolicy: "disabled" });
const addedHandler = getAddedHandler();
expect(addedHandler).toBeTruthy();
await addedHandler!({
event: makeReactionEvent(),
body: {},
});
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
});
it("blocks DM reaction system events for unauthorized senders in allowlist mode", async () => {
enqueueSystemEventMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
const { getAddedHandler } = createReactionContext({
dmPolicy: "allowlist",
allowFrom: ["U2"],
});
const addedHandler = getAddedHandler();
expect(addedHandler).toBeTruthy();
await addedHandler!({
event: makeReactionEvent({ user: "U1" }),
body: {},
});
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
});
it("allows DM reaction system events for authorized senders in allowlist mode", async () => {
enqueueSystemEventMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
const { getAddedHandler } = createReactionContext({
dmPolicy: "allowlist",
allowFrom: ["U1"],
});
const addedHandler = getAddedHandler();
expect(addedHandler).toBeTruthy();
await addedHandler!({
event: makeReactionEvent({ user: "U1" }),
body: {},
});
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
});
it("enqueues channel reaction events regardless of dmPolicy", async () => {
enqueueSystemEventMock.mockClear();
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
const { getRemovedHandler } = createReactionContext({
dmPolicy: "disabled",
channelType: "channel",
});
const removedHandler = getRemovedHandler();
expect(removedHandler).toBeTruthy();
await removedHandler!({
event: {
...makeReactionEvent({ channel: "C1" }),
type: "reaction_removed",
},
body: {},
});
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
});
});

View File

@@ -1,6 +1,9 @@
import type { SlackEventMiddlewareArgs } from "@slack/bolt";
import { danger } from "../../../globals.js";
import { danger, logVerbose } from "../../../globals.js";
import { enqueueSystemEvent } from "../../../infra/system-events.js";
import { resolveDmGroupAccessWithLists } from "../../../security/dm-policy-shared.js";
import { resolveSlackAllowListMatch } from "../allow-list.js";
import { resolveSlackEffectiveAllowFrom } from "../auth.js";
import { resolveSlackChannelLabel } from "../channel-config.js";
import type { SlackMonitorContext } from "../context.js";
import type { SlackReactionEvent } from "../types.js";
@@ -32,6 +35,33 @@ export function registerSlackReactionEvents(params: { ctx: SlackMonitorContext }
channelName: channelInfo?.name,
});
const actorInfo = event.user ? await ctx.resolveUserName(event.user) : undefined;
if (channelType === "im") {
if (!event.user) {
return;
}
const { allowFromLower } = await resolveSlackEffectiveAllowFrom(ctx);
const access = resolveDmGroupAccessWithLists({
isGroup: false,
dmPolicy: ctx.dmPolicy,
groupPolicy: ctx.groupPolicy,
allowFrom: allowFromLower,
groupAllowFrom: [],
storeAllowFrom: [],
isSenderAllowed: (allowList) =>
resolveSlackAllowListMatch({
allowList,
id: event.user,
name: actorInfo?.name,
allowNameMatching: ctx.allowNameMatching,
}).allowed,
});
if (access.decision !== "allow") {
logVerbose(
`slack: drop reaction sender ${event.user} (dmPolicy=${ctx.dmPolicy}, decision=${access.decision}, reason=${access.reason})`,
);
return;
}
}
const actorLabel = actorInfo?.name ?? event.user;
const emojiLabel = event.reaction ?? "emoji";
const authorInfo = event.item_user ? await ctx.resolveUserName(event.item_user) : undefined;