mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(slack): gate pin/reaction system events by sender auth
This commit is contained in:
@@ -22,7 +22,8 @@ Docs: https://docs.openclaw.ai
|
||||
- Models/Auth probes: map permanent auth failover reasons (`auth_permanent`, for example revoked keys) into probe auth status instead of `unknown`, so `openclaw models status --probe` reports actionable auth failures. (#25754) thanks @rrenamed.
|
||||
- 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/Exec approvals: bind `system.run` approval matching to exact argv identity and preserve argv whitespace in rendered command text, preventing trailing-space executable path swaps from reusing a mismatched approval. 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/Discord 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/Slack reactions + pins: gate `reaction_*` and `pin_*` system-event enqueue through shared sender authorization so DM `dmPolicy`/`allowFrom` and channel `users` allowlists are enforced consistently for non-message ingress, with regression coverage for denied/allowed sender paths. 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/Slack interactions: enforce channel/DM authorization and modal actor binding (`private_metadata.userId`) before enqueueing `block_action`/`view_submission`/`view_closed` system events, with regression coverage for unauthorized senders and missing/mismatched actor metadata. This 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.
|
||||
|
||||
170
src/slack/monitor/events/pins.test.ts
Normal file
170
src/slack/monitor/events/pins.test.ts
Normal file
@@ -0,0 +1,170 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { SlackMonitorContext } from "../context.js";
|
||||
import { registerSlackPinEvents } from "./pins.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 SlackPinHandler = (args: { event: Record<string, unknown>; body: unknown }) => Promise<void>;
|
||||
|
||||
function createPinContext(overrides?: {
|
||||
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
|
||||
allowFrom?: string[];
|
||||
channelType?: "im" | "channel";
|
||||
channelUsers?: string[];
|
||||
}) {
|
||||
let addedHandler: SlackPinHandler | null = null;
|
||||
let removedHandler: SlackPinHandler | null = null;
|
||||
const channelType = overrides?.channelType ?? "im";
|
||||
const app = {
|
||||
event: vi.fn((name: string, handler: SlackPinHandler) => {
|
||||
if (name === "pin_added") {
|
||||
addedHandler = handler;
|
||||
} else if (name === "pin_removed") {
|
||||
removedHandler = handler;
|
||||
}
|
||||
}),
|
||||
};
|
||||
const ctx = {
|
||||
app,
|
||||
runtime: { error: vi.fn() },
|
||||
dmEnabled: true,
|
||||
dmPolicy: overrides?.dmPolicy ?? "open",
|
||||
defaultRequireMention: true,
|
||||
channelsConfig: overrides?.channelUsers
|
||||
? {
|
||||
C1: {
|
||||
users: overrides.channelUsers,
|
||||
allow: true,
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
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;
|
||||
registerSlackPinEvents({ ctx });
|
||||
return {
|
||||
ctx,
|
||||
getAddedHandler: () => addedHandler,
|
||||
getRemovedHandler: () => removedHandler,
|
||||
};
|
||||
}
|
||||
|
||||
function makePinEvent(overrides?: { user?: string; channel?: string }) {
|
||||
return {
|
||||
type: "pin_added",
|
||||
user: overrides?.user ?? "U1",
|
||||
channel_id: overrides?.channel ?? "D1",
|
||||
event_ts: "123.456",
|
||||
item: {
|
||||
type: "message",
|
||||
message: {
|
||||
ts: "123.456",
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("registerSlackPinEvents", () => {
|
||||
it("enqueues DM pin system events when dmPolicy is open", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createPinContext({ dmPolicy: "open" });
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makePinEvent(),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks DM pin system events when dmPolicy is disabled", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createPinContext({ dmPolicy: "disabled" });
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makePinEvent(),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks DM pin system events for unauthorized senders in allowlist mode", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createPinContext({
|
||||
dmPolicy: "allowlist",
|
||||
allowFrom: ["U2"],
|
||||
});
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makePinEvent({ user: "U1" }),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows DM pin system events for authorized senders in allowlist mode", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createPinContext({
|
||||
dmPolicy: "allowlist",
|
||||
allowFrom: ["U1"],
|
||||
});
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makePinEvent({ user: "U1" }),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks channel pin events for users outside channel users allowlist", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createPinContext({
|
||||
dmPolicy: "open",
|
||||
channelType: "channel",
|
||||
channelUsers: ["U_OWNER"],
|
||||
});
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makePinEvent({ channel: "C1", user: "U_ATTACKER" }),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,7 @@
|
||||
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 { authorizeSlackSystemEventSender } from "../auth.js";
|
||||
import { resolveSlackChannelLabel } from "../channel-config.js";
|
||||
import type { SlackMonitorContext } from "../context.js";
|
||||
import type { SlackPinEvent } from "../types.js";
|
||||
@@ -22,19 +23,20 @@ async function handleSlackPinEvent(params: {
|
||||
|
||||
const payload = event as SlackPinEvent;
|
||||
const channelId = payload.channel_id;
|
||||
const channelInfo = channelId ? await ctx.resolveChannelName(channelId) : {};
|
||||
if (
|
||||
!ctx.isChannelAllowed({
|
||||
channelId,
|
||||
channelName: channelInfo?.name,
|
||||
channelType: channelInfo?.type,
|
||||
})
|
||||
) {
|
||||
const auth = await authorizeSlackSystemEventSender({
|
||||
ctx,
|
||||
senderId: payload.user,
|
||||
channelId,
|
||||
});
|
||||
if (!auth.allowed) {
|
||||
logVerbose(
|
||||
`slack: drop pin sender ${payload.user ?? "unknown"} channel=${channelId ?? "unknown"} reason=${auth.reason ?? "unauthorized"}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
const label = resolveSlackChannelLabel({
|
||||
channelId,
|
||||
channelName: channelInfo?.name,
|
||||
channelName: auth.channelName,
|
||||
});
|
||||
const userInfo = payload.user ? await ctx.resolveUserName(payload.user) : {};
|
||||
const userLabel = userInfo?.name ?? payload.user ?? "someone";
|
||||
@@ -42,7 +44,7 @@ async function handleSlackPinEvent(params: {
|
||||
const messageId = payload.item?.message?.ts ?? payload.event_ts;
|
||||
const sessionKey = ctx.resolveSlackSystemEventSessionKey({
|
||||
channelId,
|
||||
channelType: channelInfo?.type ?? undefined,
|
||||
channelType: auth.channelType,
|
||||
});
|
||||
enqueueSystemEvent(`Slack: ${userLabel} ${action} a ${itemType} in ${label}.`, {
|
||||
sessionKey,
|
||||
|
||||
@@ -22,6 +22,7 @@ function createReactionContext(overrides?: {
|
||||
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
|
||||
allowFrom?: string[];
|
||||
channelType?: "im" | "channel";
|
||||
channelUsers?: string[];
|
||||
}) {
|
||||
let addedHandler: SlackReactionHandler | null = null;
|
||||
let removedHandler: SlackReactionHandler | null = null;
|
||||
@@ -38,7 +39,17 @@ function createReactionContext(overrides?: {
|
||||
const ctx = {
|
||||
app,
|
||||
runtime: { error: vi.fn() },
|
||||
dmEnabled: true,
|
||||
dmPolicy: overrides?.dmPolicy ?? "open",
|
||||
defaultRequireMention: true,
|
||||
channelsConfig: overrides?.channelUsers
|
||||
? {
|
||||
C1: {
|
||||
users: overrides.channelUsers,
|
||||
allow: true,
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
groupPolicy: "open",
|
||||
allowFrom: overrides?.allowFrom ?? [],
|
||||
allowNameMatching: false,
|
||||
@@ -160,4 +171,23 @@ describe("registerSlackReactionEvents", () => {
|
||||
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks channel reaction events for users outside channel users allowlist", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
readAllowFromStoreMock.mockReset().mockResolvedValue([]);
|
||||
const { getAddedHandler } = createReactionContext({
|
||||
dmPolicy: "open",
|
||||
channelType: "channel",
|
||||
channelUsers: ["U_OWNER"],
|
||||
});
|
||||
const addedHandler = getAddedHandler();
|
||||
expect(addedHandler).toBeTruthy();
|
||||
|
||||
await addedHandler!({
|
||||
event: makeReactionEvent({ channel: "C1", user: "U_ATTACKER" }),
|
||||
body: {},
|
||||
});
|
||||
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
import type { SlackEventMiddlewareArgs } from "@slack/bolt";
|
||||
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 { authorizeSlackSystemEventSender } from "../auth.js";
|
||||
import { resolveSlackChannelLabel } from "../channel-config.js";
|
||||
import type { SlackMonitorContext } from "../context.js";
|
||||
import type { SlackReactionEvent } from "../types.js";
|
||||
@@ -18,50 +16,23 @@ export function registerSlackReactionEvents(params: { ctx: SlackMonitorContext }
|
||||
return;
|
||||
}
|
||||
|
||||
const channelInfo = item.channel ? await ctx.resolveChannelName(item.channel) : {};
|
||||
const channelType = channelInfo?.type;
|
||||
if (
|
||||
!ctx.isChannelAllowed({
|
||||
channelId: item.channel,
|
||||
channelName: channelInfo?.name,
|
||||
channelType,
|
||||
})
|
||||
) {
|
||||
const auth = await authorizeSlackSystemEventSender({
|
||||
ctx,
|
||||
senderId: event.user,
|
||||
channelId: item.channel,
|
||||
});
|
||||
if (!auth.allowed) {
|
||||
logVerbose(
|
||||
`slack: drop reaction sender ${event.user ?? "unknown"} channel=${item.channel ?? "unknown"} reason=${auth.reason ?? "unauthorized"}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const channelLabel = resolveSlackChannelLabel({
|
||||
channelId: item.channel,
|
||||
channelName: channelInfo?.name,
|
||||
channelName: auth.channelName,
|
||||
});
|
||||
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;
|
||||
@@ -70,7 +41,7 @@ export function registerSlackReactionEvents(params: { ctx: SlackMonitorContext }
|
||||
const text = authorLabel ? `${baseText} from ${authorLabel}` : baseText;
|
||||
const sessionKey = ctx.resolveSlackSystemEventSessionKey({
|
||||
channelId: item.channel,
|
||||
channelType,
|
||||
channelType: auth.channelType,
|
||||
});
|
||||
enqueueSystemEvent(text, {
|
||||
sessionKey,
|
||||
|
||||
Reference in New Issue
Block a user