mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 00:13:28 +00:00
fix(slack): gate interactive system events by sender auth
This commit is contained in:
@@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- 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/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/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/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.
|
- 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.
|
- 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.
|
||||||
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ describe("parseSlackModalPrivateMetadata", () => {
|
|||||||
sessionKey: "agent:main:slack:channel:C1",
|
sessionKey: "agent:main:slack:channel:C1",
|
||||||
channelId: "D123",
|
channelId: "D123",
|
||||||
channelType: "im",
|
channelType: "im",
|
||||||
|
userId: "U123",
|
||||||
ignored: "x",
|
ignored: "x",
|
||||||
}),
|
}),
|
||||||
),
|
),
|
||||||
@@ -25,6 +26,7 @@ describe("parseSlackModalPrivateMetadata", () => {
|
|||||||
sessionKey: "agent:main:slack:channel:C1",
|
sessionKey: "agent:main:slack:channel:C1",
|
||||||
channelId: "D123",
|
channelId: "D123",
|
||||||
channelType: "im",
|
channelType: "im",
|
||||||
|
userId: "U123",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -37,11 +39,13 @@ describe("encodeSlackModalPrivateMetadata", () => {
|
|||||||
sessionKey: "agent:main:slack:channel:C1",
|
sessionKey: "agent:main:slack:channel:C1",
|
||||||
channelId: "",
|
channelId: "",
|
||||||
channelType: "im",
|
channelType: "im",
|
||||||
|
userId: "U123",
|
||||||
}),
|
}),
|
||||||
),
|
),
|
||||||
).toEqual({
|
).toEqual({
|
||||||
sessionKey: "agent:main:slack:channel:C1",
|
sessionKey: "agent:main:slack:channel:C1",
|
||||||
channelType: "im",
|
channelType: "im",
|
||||||
|
userId: "U123",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ export type SlackModalPrivateMetadata = {
|
|||||||
sessionKey?: string;
|
sessionKey?: string;
|
||||||
channelId?: string;
|
channelId?: string;
|
||||||
channelType?: string;
|
channelType?: string;
|
||||||
|
userId?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
const SLACK_PRIVATE_METADATA_MAX = 3000;
|
const SLACK_PRIVATE_METADATA_MAX = 3000;
|
||||||
@@ -20,6 +21,7 @@ export function parseSlackModalPrivateMetadata(raw: unknown): SlackModalPrivateM
|
|||||||
sessionKey: normalizeString(parsed.sessionKey),
|
sessionKey: normalizeString(parsed.sessionKey),
|
||||||
channelId: normalizeString(parsed.channelId),
|
channelId: normalizeString(parsed.channelId),
|
||||||
channelType: normalizeString(parsed.channelType),
|
channelType: normalizeString(parsed.channelType),
|
||||||
|
userId: normalizeString(parsed.userId),
|
||||||
};
|
};
|
||||||
} catch {
|
} catch {
|
||||||
return {};
|
return {};
|
||||||
@@ -31,6 +33,7 @@ export function encodeSlackModalPrivateMetadata(input: SlackModalPrivateMetadata
|
|||||||
...(input.sessionKey ? { sessionKey: input.sessionKey } : {}),
|
...(input.sessionKey ? { sessionKey: input.sessionKey } : {}),
|
||||||
...(input.channelId ? { channelId: input.channelId } : {}),
|
...(input.channelId ? { channelId: input.channelId } : {}),
|
||||||
...(input.channelType ? { channelType: input.channelType } : {}),
|
...(input.channelType ? { channelType: input.channelType } : {}),
|
||||||
|
...(input.userId ? { userId: input.userId } : {}),
|
||||||
};
|
};
|
||||||
const encoded = JSON.stringify(payload);
|
const encoded = JSON.stringify(payload);
|
||||||
if (encoded.length > SLACK_PRIVATE_METADATA_MAX) {
|
if (encoded.length > SLACK_PRIVATE_METADATA_MAX) {
|
||||||
|
|||||||
@@ -1,6 +1,12 @@
|
|||||||
import { readChannelAllowFromStore } from "../../pairing/pairing-store.js";
|
import { readChannelAllowFromStore } from "../../pairing/pairing-store.js";
|
||||||
import { allowListMatches, normalizeAllowList, normalizeAllowListLower } from "./allow-list.js";
|
import {
|
||||||
import type { SlackMonitorContext } from "./context.js";
|
allowListMatches,
|
||||||
|
normalizeAllowList,
|
||||||
|
normalizeAllowListLower,
|
||||||
|
resolveSlackUserAllowed,
|
||||||
|
} from "./allow-list.js";
|
||||||
|
import { resolveSlackChannelConfig } from "./channel-config.js";
|
||||||
|
import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js";
|
||||||
|
|
||||||
export async function resolveSlackEffectiveAllowFrom(ctx: SlackMonitorContext) {
|
export async function resolveSlackEffectiveAllowFrom(ctx: SlackMonitorContext) {
|
||||||
const storeAllowFrom =
|
const storeAllowFrom =
|
||||||
@@ -27,3 +33,137 @@ export function isSlackSenderAllowListed(params: {
|
|||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type SlackSystemEventAuthResult = {
|
||||||
|
allowed: boolean;
|
||||||
|
reason?:
|
||||||
|
| "missing-sender"
|
||||||
|
| "sender-mismatch"
|
||||||
|
| "channel-not-allowed"
|
||||||
|
| "dm-disabled"
|
||||||
|
| "sender-not-allowlisted"
|
||||||
|
| "sender-not-channel-allowed";
|
||||||
|
channelType?: "im" | "mpim" | "channel" | "group";
|
||||||
|
channelName?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function authorizeSlackSystemEventSender(params: {
|
||||||
|
ctx: SlackMonitorContext;
|
||||||
|
senderId?: string;
|
||||||
|
channelId?: string;
|
||||||
|
channelType?: string | null;
|
||||||
|
expectedSenderId?: string;
|
||||||
|
}): Promise<SlackSystemEventAuthResult> {
|
||||||
|
const senderId = params.senderId?.trim();
|
||||||
|
if (!senderId) {
|
||||||
|
return { allowed: false, reason: "missing-sender" };
|
||||||
|
}
|
||||||
|
|
||||||
|
const expectedSenderId = params.expectedSenderId?.trim();
|
||||||
|
if (expectedSenderId && expectedSenderId !== senderId) {
|
||||||
|
return { allowed: false, reason: "sender-mismatch" };
|
||||||
|
}
|
||||||
|
|
||||||
|
const channelId = params.channelId?.trim();
|
||||||
|
let channelType = normalizeSlackChannelType(params.channelType, channelId);
|
||||||
|
let channelName: string | undefined;
|
||||||
|
if (channelId) {
|
||||||
|
const info: {
|
||||||
|
name?: string;
|
||||||
|
type?: "im" | "mpim" | "channel" | "group";
|
||||||
|
} = await params.ctx.resolveChannelName(channelId).catch(() => ({}));
|
||||||
|
channelName = info.name;
|
||||||
|
channelType = normalizeSlackChannelType(params.channelType ?? info.type, channelId);
|
||||||
|
if (
|
||||||
|
!params.ctx.isChannelAllowed({
|
||||||
|
channelId,
|
||||||
|
channelName,
|
||||||
|
channelType,
|
||||||
|
})
|
||||||
|
) {
|
||||||
|
return {
|
||||||
|
allowed: false,
|
||||||
|
reason: "channel-not-allowed",
|
||||||
|
channelType,
|
||||||
|
channelName,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const senderInfo: { name?: string } = await params.ctx
|
||||||
|
.resolveUserName(senderId)
|
||||||
|
.catch(() => ({}));
|
||||||
|
const senderName = senderInfo.name;
|
||||||
|
|
||||||
|
const resolveAllowFromLower = async () =>
|
||||||
|
(await resolveSlackEffectiveAllowFrom(params.ctx)).allowFromLower;
|
||||||
|
|
||||||
|
if (channelType === "im") {
|
||||||
|
if (!params.ctx.dmEnabled || params.ctx.dmPolicy === "disabled") {
|
||||||
|
return { allowed: false, reason: "dm-disabled", channelType, channelName };
|
||||||
|
}
|
||||||
|
if (params.ctx.dmPolicy !== "open") {
|
||||||
|
const allowFromLower = await resolveAllowFromLower();
|
||||||
|
const senderAllowListed = isSlackSenderAllowListed({
|
||||||
|
allowListLower: allowFromLower,
|
||||||
|
senderId,
|
||||||
|
senderName,
|
||||||
|
allowNameMatching: params.ctx.allowNameMatching,
|
||||||
|
});
|
||||||
|
if (!senderAllowListed) {
|
||||||
|
return {
|
||||||
|
allowed: false,
|
||||||
|
reason: "sender-not-allowlisted",
|
||||||
|
channelType,
|
||||||
|
channelName,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else if (!channelId) {
|
||||||
|
// No channel context. Apply allowFrom if configured so we fail closed
|
||||||
|
// for privileged interactive events when owner allowlist is present.
|
||||||
|
const allowFromLower = await resolveAllowFromLower();
|
||||||
|
if (allowFromLower.length > 0) {
|
||||||
|
const senderAllowListed = isSlackSenderAllowListed({
|
||||||
|
allowListLower: allowFromLower,
|
||||||
|
senderId,
|
||||||
|
senderName,
|
||||||
|
allowNameMatching: params.ctx.allowNameMatching,
|
||||||
|
});
|
||||||
|
if (!senderAllowListed) {
|
||||||
|
return { allowed: false, reason: "sender-not-allowlisted" };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
const channelConfig = resolveSlackChannelConfig({
|
||||||
|
channelId,
|
||||||
|
channelName,
|
||||||
|
channels: params.ctx.channelsConfig,
|
||||||
|
defaultRequireMention: params.ctx.defaultRequireMention,
|
||||||
|
});
|
||||||
|
const channelUsersAllowlistConfigured =
|
||||||
|
Array.isArray(channelConfig?.users) && channelConfig.users.length > 0;
|
||||||
|
if (channelUsersAllowlistConfigured) {
|
||||||
|
const channelUserAllowed = resolveSlackUserAllowed({
|
||||||
|
allowList: channelConfig?.users,
|
||||||
|
userId: senderId,
|
||||||
|
userName: senderName,
|
||||||
|
allowNameMatching: params.ctx.allowNameMatching,
|
||||||
|
});
|
||||||
|
if (!channelUserAllowed) {
|
||||||
|
return {
|
||||||
|
allowed: false,
|
||||||
|
reason: "sender-not-channel-allowed",
|
||||||
|
channelType,
|
||||||
|
channelName,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
allowed: true,
|
||||||
|
channelType,
|
||||||
|
channelName,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ type RegisteredViewHandler = (args: {
|
|||||||
view?: {
|
view?: {
|
||||||
id?: string;
|
id?: string;
|
||||||
callback_id?: string;
|
callback_id?: string;
|
||||||
|
private_metadata?: string;
|
||||||
root_view_id?: string;
|
root_view_id?: string;
|
||||||
previous_view_id?: string;
|
previous_view_id?: string;
|
||||||
external_id?: string;
|
external_id?: string;
|
||||||
@@ -58,7 +59,23 @@ type RegisteredViewClosedHandler = (args: {
|
|||||||
};
|
};
|
||||||
}) => Promise<void>;
|
}) => Promise<void>;
|
||||||
|
|
||||||
function createContext() {
|
function createContext(overrides?: {
|
||||||
|
dmEnabled?: boolean;
|
||||||
|
dmPolicy?: "open" | "allowlist" | "pairing" | "disabled";
|
||||||
|
allowFrom?: string[];
|
||||||
|
allowNameMatching?: boolean;
|
||||||
|
channelsConfig?: Record<string, { users?: string[] }>;
|
||||||
|
isChannelAllowed?: (params: {
|
||||||
|
channelId?: string;
|
||||||
|
channelName?: string;
|
||||||
|
channelType?: "im" | "mpim" | "channel" | "group";
|
||||||
|
}) => boolean;
|
||||||
|
resolveUserName?: (userId: string) => Promise<{ name?: string }>;
|
||||||
|
resolveChannelName?: (channelId: string) => Promise<{
|
||||||
|
name?: string;
|
||||||
|
type?: "im" | "mpim" | "channel" | "group";
|
||||||
|
}>;
|
||||||
|
}) {
|
||||||
let handler: RegisteredHandler | null = null;
|
let handler: RegisteredHandler | null = null;
|
||||||
let viewHandler: RegisteredViewHandler | null = null;
|
let viewHandler: RegisteredViewHandler | null = null;
|
||||||
let viewClosedHandler: RegisteredViewClosedHandler | null = null;
|
let viewClosedHandler: RegisteredViewClosedHandler | null = null;
|
||||||
@@ -80,9 +97,40 @@ function createContext() {
|
|||||||
};
|
};
|
||||||
const runtimeLog = vi.fn();
|
const runtimeLog = vi.fn();
|
||||||
const resolveSessionKey = vi.fn().mockReturnValue("agent:ops:slack:channel:C1");
|
const resolveSessionKey = vi.fn().mockReturnValue("agent:ops:slack:channel:C1");
|
||||||
|
const isChannelAllowed = vi
|
||||||
|
.fn<
|
||||||
|
(params: {
|
||||||
|
channelId?: string;
|
||||||
|
channelName?: string;
|
||||||
|
channelType?: "im" | "mpim" | "channel" | "group";
|
||||||
|
}) => boolean
|
||||||
|
>()
|
||||||
|
.mockImplementation((params) => overrides?.isChannelAllowed?.(params) ?? true);
|
||||||
|
const resolveUserName = vi
|
||||||
|
.fn<(userId: string) => Promise<{ name?: string }>>()
|
||||||
|
.mockImplementation((userId) => overrides?.resolveUserName?.(userId) ?? Promise.resolve({}));
|
||||||
|
const resolveChannelName = vi
|
||||||
|
.fn<
|
||||||
|
(channelId: string) => Promise<{
|
||||||
|
name?: string;
|
||||||
|
type?: "im" | "mpim" | "channel" | "group";
|
||||||
|
}>
|
||||||
|
>()
|
||||||
|
.mockImplementation(
|
||||||
|
(channelId) => overrides?.resolveChannelName?.(channelId) ?? Promise.resolve({}),
|
||||||
|
);
|
||||||
const ctx = {
|
const ctx = {
|
||||||
app,
|
app,
|
||||||
runtime: { log: runtimeLog },
|
runtime: { log: runtimeLog },
|
||||||
|
dmEnabled: overrides?.dmEnabled ?? true,
|
||||||
|
dmPolicy: overrides?.dmPolicy ?? ("open" as const),
|
||||||
|
allowFrom: overrides?.allowFrom ?? [],
|
||||||
|
allowNameMatching: overrides?.allowNameMatching ?? false,
|
||||||
|
channelsConfig: overrides?.channelsConfig ?? {},
|
||||||
|
defaultRequireMention: true,
|
||||||
|
isChannelAllowed,
|
||||||
|
resolveUserName,
|
||||||
|
resolveChannelName,
|
||||||
resolveSlackSystemEventSessionKey: resolveSessionKey,
|
resolveSlackSystemEventSessionKey: resolveSessionKey,
|
||||||
};
|
};
|
||||||
return {
|
return {
|
||||||
@@ -90,6 +138,9 @@ function createContext() {
|
|||||||
app,
|
app,
|
||||||
runtimeLog,
|
runtimeLog,
|
||||||
resolveSessionKey,
|
resolveSessionKey,
|
||||||
|
isChannelAllowed,
|
||||||
|
resolveUserName,
|
||||||
|
resolveChannelName,
|
||||||
getHandler: () => handler,
|
getHandler: () => handler,
|
||||||
getViewHandler: () => viewHandler,
|
getViewHandler: () => viewHandler,
|
||||||
getViewClosedHandler: () => viewClosedHandler,
|
getViewClosedHandler: () => viewClosedHandler,
|
||||||
@@ -168,7 +219,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
});
|
});
|
||||||
expect(resolveSessionKey).toHaveBeenCalledWith({
|
expect(resolveSessionKey).toHaveBeenCalledWith({
|
||||||
channelId: "C1",
|
channelId: "C1",
|
||||||
channelType: undefined,
|
channelType: "channel",
|
||||||
});
|
});
|
||||||
expect(app.client.chat.update).toHaveBeenCalledTimes(1);
|
expect(app.client.chat.update).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
@@ -228,6 +279,85 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("blocks block actions from users outside configured channel users allowlist", async () => {
|
||||||
|
enqueueSystemEventMock.mockClear();
|
||||||
|
const { ctx, app, getHandler } = createContext({
|
||||||
|
channelsConfig: {
|
||||||
|
C1: { users: ["U_ALLOWED"] },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||||
|
const handler = getHandler();
|
||||||
|
expect(handler).toBeTruthy();
|
||||||
|
|
||||||
|
const ack = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const respond = vi.fn().mockResolvedValue(undefined);
|
||||||
|
await handler!({
|
||||||
|
ack,
|
||||||
|
respond,
|
||||||
|
body: {
|
||||||
|
user: { id: "U_DENIED" },
|
||||||
|
channel: { id: "C1" },
|
||||||
|
message: {
|
||||||
|
ts: "201.202",
|
||||||
|
blocks: [{ type: "actions", block_id: "verify_block", elements: [] }],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
action: {
|
||||||
|
type: "button",
|
||||||
|
action_id: "openclaw:verify",
|
||||||
|
block_id: "verify_block",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(ack).toHaveBeenCalled();
|
||||||
|
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||||
|
expect(app.client.chat.update).not.toHaveBeenCalled();
|
||||||
|
expect(respond).toHaveBeenCalledWith({
|
||||||
|
text: "You are not authorized to use this control.",
|
||||||
|
response_type: "ephemeral",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("blocks DM block actions when sender is not in allowFrom", async () => {
|
||||||
|
enqueueSystemEventMock.mockClear();
|
||||||
|
const { ctx, app, getHandler } = createContext({
|
||||||
|
dmPolicy: "allowlist",
|
||||||
|
allowFrom: ["U_OWNER"],
|
||||||
|
});
|
||||||
|
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||||
|
const handler = getHandler();
|
||||||
|
expect(handler).toBeTruthy();
|
||||||
|
|
||||||
|
const ack = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const respond = vi.fn().mockResolvedValue(undefined);
|
||||||
|
await handler!({
|
||||||
|
ack,
|
||||||
|
respond,
|
||||||
|
body: {
|
||||||
|
user: { id: "U_ATTACKER" },
|
||||||
|
channel: { id: "D222" },
|
||||||
|
message: {
|
||||||
|
ts: "301.302",
|
||||||
|
blocks: [{ type: "actions", block_id: "verify_block", elements: [] }],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
action: {
|
||||||
|
type: "button",
|
||||||
|
action_id: "openclaw:verify",
|
||||||
|
block_id: "verify_block",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(ack).toHaveBeenCalled();
|
||||||
|
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||||
|
expect(app.client.chat.update).not.toHaveBeenCalled();
|
||||||
|
expect(respond).toHaveBeenCalledWith({
|
||||||
|
text: "You are not authorized to use this control.",
|
||||||
|
response_type: "ephemeral",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("ignores malformed action payloads after ack and logs warning", async () => {
|
it("ignores malformed action payloads after ack and logs warning", async () => {
|
||||||
enqueueSystemEventMock.mockClear();
|
enqueueSystemEventMock.mockClear();
|
||||||
const { ctx, app, getHandler, runtimeLog } = createContext();
|
const { ctx, app, getHandler, runtimeLog } = createContext();
|
||||||
@@ -338,7 +468,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
expect(ack).toHaveBeenCalled();
|
expect(ack).toHaveBeenCalled();
|
||||||
expect(resolveSessionKey).toHaveBeenCalledWith({
|
expect(resolveSessionKey).toHaveBeenCalledWith({
|
||||||
channelId: "C222",
|
channelId: "C222",
|
||||||
channelType: undefined,
|
channelType: "channel",
|
||||||
});
|
});
|
||||||
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
||||||
const [eventText] = enqueueSystemEventMock.mock.calls[0] as [string];
|
const [eventText] = enqueueSystemEventMock.mock.calls[0] as [string];
|
||||||
@@ -697,7 +827,11 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
previous_view_id: "VPREV",
|
previous_view_id: "VPREV",
|
||||||
external_id: "deploy-ext-1",
|
external_id: "deploy-ext-1",
|
||||||
hash: "view-hash-1",
|
hash: "view-hash-1",
|
||||||
private_metadata: JSON.stringify({ channelId: "D123", channelType: "im" }),
|
private_metadata: JSON.stringify({
|
||||||
|
channelId: "D123",
|
||||||
|
channelType: "im",
|
||||||
|
userId: "U777",
|
||||||
|
}),
|
||||||
state: {
|
state: {
|
||||||
values: {
|
values: {
|
||||||
env_block: {
|
env_block: {
|
||||||
@@ -771,6 +905,59 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("blocks modal events when private metadata userId does not match submitter", async () => {
|
||||||
|
enqueueSystemEventMock.mockClear();
|
||||||
|
const { ctx, getViewHandler } = createContext();
|
||||||
|
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||||
|
const viewHandler = getViewHandler();
|
||||||
|
expect(viewHandler).toBeTruthy();
|
||||||
|
|
||||||
|
const ack = vi.fn().mockResolvedValue(undefined);
|
||||||
|
await viewHandler!({
|
||||||
|
ack,
|
||||||
|
body: {
|
||||||
|
user: { id: "U222" },
|
||||||
|
view: {
|
||||||
|
callback_id: "openclaw:deploy_form",
|
||||||
|
private_metadata: JSON.stringify({
|
||||||
|
channelId: "D123",
|
||||||
|
channelType: "im",
|
||||||
|
userId: "U111",
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
expect(ack).toHaveBeenCalled();
|
||||||
|
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("blocks modal events when private metadata is missing userId", async () => {
|
||||||
|
enqueueSystemEventMock.mockClear();
|
||||||
|
const { ctx, getViewHandler } = createContext();
|
||||||
|
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||||
|
const viewHandler = getViewHandler();
|
||||||
|
expect(viewHandler).toBeTruthy();
|
||||||
|
|
||||||
|
const ack = vi.fn().mockResolvedValue(undefined);
|
||||||
|
await viewHandler!({
|
||||||
|
ack,
|
||||||
|
body: {
|
||||||
|
user: { id: "U222" },
|
||||||
|
view: {
|
||||||
|
callback_id: "openclaw:deploy_form",
|
||||||
|
private_metadata: JSON.stringify({
|
||||||
|
channelId: "D123",
|
||||||
|
channelType: "im",
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
expect(ack).toHaveBeenCalled();
|
||||||
|
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("captures modal input labels and picker values across block types", async () => {
|
it("captures modal input labels and picker values across block types", async () => {
|
||||||
enqueueSystemEventMock.mockClear();
|
enqueueSystemEventMock.mockClear();
|
||||||
const { ctx, getViewHandler } = createContext();
|
const { ctx, getViewHandler } = createContext();
|
||||||
@@ -786,6 +973,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
view: {
|
view: {
|
||||||
id: "V400",
|
id: "V400",
|
||||||
callback_id: "openclaw:routing_form",
|
callback_id: "openclaw:routing_form",
|
||||||
|
private_metadata: JSON.stringify({ userId: "U444" }),
|
||||||
state: {
|
state: {
|
||||||
values: {
|
values: {
|
||||||
env_block: {
|
env_block: {
|
||||||
@@ -1001,6 +1189,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
view: {
|
view: {
|
||||||
id: "V555",
|
id: "V555",
|
||||||
callback_id: "openclaw:long_richtext",
|
callback_id: "openclaw:long_richtext",
|
||||||
|
private_metadata: JSON.stringify({ userId: "U555" }),
|
||||||
state: {
|
state: {
|
||||||
values: {
|
values: {
|
||||||
richtext_block: {
|
richtext_block: {
|
||||||
@@ -1054,7 +1243,10 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
previous_view_id: "VPREV900",
|
previous_view_id: "VPREV900",
|
||||||
external_id: "deploy-ext-900",
|
external_id: "deploy-ext-900",
|
||||||
hash: "view-hash-900",
|
hash: "view-hash-900",
|
||||||
private_metadata: JSON.stringify({ sessionKey: "agent:main:slack:channel:C99" }),
|
private_metadata: JSON.stringify({
|
||||||
|
sessionKey: "agent:main:slack:channel:C99",
|
||||||
|
userId: "U900",
|
||||||
|
}),
|
||||||
state: {
|
state: {
|
||||||
values: {
|
values: {
|
||||||
env_block: {
|
env_block: {
|
||||||
@@ -1101,7 +1293,10 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
viewId: "V900",
|
viewId: "V900",
|
||||||
userId: "U900",
|
userId: "U900",
|
||||||
isCleared: true,
|
isCleared: true,
|
||||||
privateMetadata: JSON.stringify({ sessionKey: "agent:main:slack:channel:C99" }),
|
privateMetadata: JSON.stringify({
|
||||||
|
sessionKey: "agent:main:slack:channel:C99",
|
||||||
|
userId: "U900",
|
||||||
|
}),
|
||||||
rootViewId: "VROOT900",
|
rootViewId: "VROOT900",
|
||||||
previousViewId: "VPREV900",
|
previousViewId: "VPREV900",
|
||||||
externalId: "deploy-ext-900",
|
externalId: "deploy-ext-900",
|
||||||
@@ -1131,6 +1326,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
view: {
|
view: {
|
||||||
id: "V901",
|
id: "V901",
|
||||||
callback_id: "openclaw:deploy_form",
|
callback_id: "openclaw:deploy_form",
|
||||||
|
private_metadata: JSON.stringify({ userId: "U901" }),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import type { SlackActionMiddlewareArgs } from "@slack/bolt";
|
|||||||
import type { Block, KnownBlock } from "@slack/web-api";
|
import type { Block, KnownBlock } from "@slack/web-api";
|
||||||
import { enqueueSystemEvent } from "../../../infra/system-events.js";
|
import { enqueueSystemEvent } from "../../../infra/system-events.js";
|
||||||
import { parseSlackModalPrivateMetadata } from "../../modal-metadata.js";
|
import { parseSlackModalPrivateMetadata } from "../../modal-metadata.js";
|
||||||
|
import { authorizeSlackSystemEventSender } from "../auth.js";
|
||||||
import type { SlackMonitorContext } from "../context.js";
|
import type { SlackMonitorContext } from "../context.js";
|
||||||
import { escapeSlackMrkdwn } from "../mrkdwn.js";
|
import { escapeSlackMrkdwn } from "../mrkdwn.js";
|
||||||
|
|
||||||
@@ -78,6 +79,7 @@ type SlackModalBody = {
|
|||||||
type SlackModalEventBase = {
|
type SlackModalEventBase = {
|
||||||
callbackId: string;
|
callbackId: string;
|
||||||
userId: string;
|
userId: string;
|
||||||
|
expectedUserId?: string;
|
||||||
viewId?: string;
|
viewId?: string;
|
||||||
sessionRouting: ReturnType<typeof resolveModalSessionRouting>;
|
sessionRouting: ReturnType<typeof resolveModalSessionRouting>;
|
||||||
payload: {
|
payload: {
|
||||||
@@ -366,11 +368,15 @@ function summarizeViewState(values: unknown): ModalInputSummary[] {
|
|||||||
|
|
||||||
function resolveModalSessionRouting(params: {
|
function resolveModalSessionRouting(params: {
|
||||||
ctx: SlackMonitorContext;
|
ctx: SlackMonitorContext;
|
||||||
privateMetadata: unknown;
|
metadata: ReturnType<typeof parseSlackModalPrivateMetadata>;
|
||||||
}): { sessionKey: string; channelId?: string; channelType?: string } {
|
}): { sessionKey: string; channelId?: string; channelType?: string } {
|
||||||
const metadata = parseSlackModalPrivateMetadata(params.privateMetadata);
|
const metadata = params.metadata;
|
||||||
if (metadata.sessionKey) {
|
if (metadata.sessionKey) {
|
||||||
return { sessionKey: metadata.sessionKey };
|
return {
|
||||||
|
sessionKey: metadata.sessionKey,
|
||||||
|
channelId: metadata.channelId,
|
||||||
|
channelType: metadata.channelType,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
if (metadata.channelId) {
|
if (metadata.channelId) {
|
||||||
return {
|
return {
|
||||||
@@ -416,17 +422,19 @@ function resolveSlackModalEventBase(params: {
|
|||||||
ctx: SlackMonitorContext;
|
ctx: SlackMonitorContext;
|
||||||
body: SlackModalBody;
|
body: SlackModalBody;
|
||||||
}): SlackModalEventBase {
|
}): SlackModalEventBase {
|
||||||
|
const metadata = parseSlackModalPrivateMetadata(params.body.view?.private_metadata);
|
||||||
const callbackId = params.body.view?.callback_id ?? "unknown";
|
const callbackId = params.body.view?.callback_id ?? "unknown";
|
||||||
const userId = params.body.user?.id ?? "unknown";
|
const userId = params.body.user?.id ?? "unknown";
|
||||||
const viewId = params.body.view?.id;
|
const viewId = params.body.view?.id;
|
||||||
const inputs = summarizeViewState(params.body.view?.state?.values);
|
const inputs = summarizeViewState(params.body.view?.state?.values);
|
||||||
const sessionRouting = resolveModalSessionRouting({
|
const sessionRouting = resolveModalSessionRouting({
|
||||||
ctx: params.ctx,
|
ctx: params.ctx,
|
||||||
privateMetadata: params.body.view?.private_metadata,
|
metadata,
|
||||||
});
|
});
|
||||||
return {
|
return {
|
||||||
callbackId,
|
callbackId,
|
||||||
userId,
|
userId,
|
||||||
|
expectedUserId: metadata.userId,
|
||||||
viewId,
|
viewId,
|
||||||
sessionRouting,
|
sessionRouting,
|
||||||
payload: {
|
payload: {
|
||||||
@@ -449,16 +457,17 @@ function resolveSlackModalEventBase(params: {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
function emitSlackModalLifecycleEvent(params: {
|
async function emitSlackModalLifecycleEvent(params: {
|
||||||
ctx: SlackMonitorContext;
|
ctx: SlackMonitorContext;
|
||||||
body: SlackModalBody;
|
body: SlackModalBody;
|
||||||
interactionType: SlackModalInteractionKind;
|
interactionType: SlackModalInteractionKind;
|
||||||
contextPrefix: "slack:interaction:view" | "slack:interaction:view-closed";
|
contextPrefix: "slack:interaction:view" | "slack:interaction:view-closed";
|
||||||
}): void {
|
}): Promise<void> {
|
||||||
const { callbackId, userId, viewId, sessionRouting, payload } = resolveSlackModalEventBase({
|
const { callbackId, userId, expectedUserId, viewId, sessionRouting, payload } =
|
||||||
ctx: params.ctx,
|
resolveSlackModalEventBase({
|
||||||
body: params.body,
|
ctx: params.ctx,
|
||||||
});
|
body: params.body,
|
||||||
|
});
|
||||||
const isViewClosed = params.interactionType === "view_closed";
|
const isViewClosed = params.interactionType === "view_closed";
|
||||||
const isCleared = params.body.is_cleared === true;
|
const isCleared = params.body.is_cleared === true;
|
||||||
const eventPayload = isViewClosed
|
const eventPayload = isViewClosed
|
||||||
@@ -482,6 +491,27 @@ function emitSlackModalLifecycleEvent(params: {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!expectedUserId) {
|
||||||
|
params.ctx.runtime.log?.(
|
||||||
|
`slack:interaction drop modal callback=${callbackId} user=${userId} reason=missing-expected-user`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const auth = await authorizeSlackSystemEventSender({
|
||||||
|
ctx: params.ctx,
|
||||||
|
senderId: userId,
|
||||||
|
channelId: sessionRouting.channelId,
|
||||||
|
channelType: sessionRouting.channelType,
|
||||||
|
expectedSenderId: expectedUserId,
|
||||||
|
});
|
||||||
|
if (!auth.allowed) {
|
||||||
|
params.ctx.runtime.log?.(
|
||||||
|
`slack:interaction drop modal callback=${callbackId} user=${userId} reason=${auth.reason ?? "unauthorized"}`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
enqueueSystemEvent(`Slack interaction: ${JSON.stringify(eventPayload)}`, {
|
enqueueSystemEvent(`Slack interaction: ${JSON.stringify(eventPayload)}`, {
|
||||||
sessionKey: sessionRouting.sessionKey,
|
sessionKey: sessionRouting.sessionKey,
|
||||||
contextKey: [params.contextPrefix, callbackId, viewId, userId].filter(Boolean).join(":"),
|
contextKey: [params.contextPrefix, callbackId, viewId, userId].filter(Boolean).join(":"),
|
||||||
@@ -497,7 +527,7 @@ function registerModalLifecycleHandler(params: {
|
|||||||
}) {
|
}) {
|
||||||
params.register(params.matcher, async ({ ack, body }: SlackModalEventHandlerArgs) => {
|
params.register(params.matcher, async ({ ack, body }: SlackModalEventHandlerArgs) => {
|
||||||
await ack();
|
await ack();
|
||||||
emitSlackModalLifecycleEvent({
|
await emitSlackModalLifecycleEvent({
|
||||||
ctx: params.ctx,
|
ctx: params.ctx,
|
||||||
body: body as SlackModalBody,
|
body: body as SlackModalBody,
|
||||||
interactionType: params.interactionType,
|
interactionType: params.interactionType,
|
||||||
@@ -557,6 +587,27 @@ export function registerSlackInteractionEvents(params: { ctx: SlackMonitorContex
|
|||||||
const channelId = typedBody.channel?.id ?? typedBody.container?.channel_id;
|
const channelId = typedBody.channel?.id ?? typedBody.container?.channel_id;
|
||||||
const messageTs = typedBody.message?.ts ?? typedBody.container?.message_ts;
|
const messageTs = typedBody.message?.ts ?? typedBody.container?.message_ts;
|
||||||
const threadTs = typedBody.container?.thread_ts;
|
const threadTs = typedBody.container?.thread_ts;
|
||||||
|
const auth = await authorizeSlackSystemEventSender({
|
||||||
|
ctx,
|
||||||
|
senderId: userId,
|
||||||
|
channelId,
|
||||||
|
});
|
||||||
|
if (!auth.allowed) {
|
||||||
|
ctx.runtime.log?.(
|
||||||
|
`slack:interaction drop action=${actionId} user=${userId} channel=${channelId ?? "unknown"} reason=${auth.reason ?? "unauthorized"}`,
|
||||||
|
);
|
||||||
|
if (respond) {
|
||||||
|
try {
|
||||||
|
await respond({
|
||||||
|
text: "You are not authorized to use this control.",
|
||||||
|
response_type: "ephemeral",
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
// Best-effort feedback only.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
const actionSummary = summarizeAction(typedAction);
|
const actionSummary = summarizeAction(typedAction);
|
||||||
const eventPayload: InteractionSummary = {
|
const eventPayload: InteractionSummary = {
|
||||||
interactionType: "block_action",
|
interactionType: "block_action",
|
||||||
@@ -581,7 +632,7 @@ export function registerSlackInteractionEvents(params: { ctx: SlackMonitorContex
|
|||||||
// Pass undefined (not "unknown") to allow proper main session fallback
|
// Pass undefined (not "unknown") to allow proper main session fallback
|
||||||
const sessionKey = ctx.resolveSlackSystemEventSessionKey({
|
const sessionKey = ctx.resolveSlackSystemEventSessionKey({
|
||||||
channelId: channelId,
|
channelId: channelId,
|
||||||
channelType: undefined,
|
channelType: auth.channelType,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Build context key - only include defined values to avoid "unknown" noise
|
// Build context key - only include defined values to avoid "unknown" noise
|
||||||
|
|||||||
Reference in New Issue
Block a user