mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 07:31:24 +00:00
fix(slack): gate DM slash command authorization
This commit is contained in:
@@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Security/Google Chat: deprecate `users/<email>` allowlists (treat `users/...` as immutable user id only); keep raw email allowlists for usability. Thanks @vincentkoc.
|
- Security/Google Chat: deprecate `users/<email>` allowlists (treat `users/...` as immutable user id only); keep raw email allowlists for usability. Thanks @vincentkoc.
|
||||||
- Security/Google Chat: reject ambiguous shared-path webhook routing when multiple webhook targets verify successfully (prevents cross-account policy-context misrouting). Thanks @vincentkoc.
|
- Security/Google Chat: reject ambiguous shared-path webhook routing when multiple webhook targets verify successfully (prevents cross-account policy-context misrouting). Thanks @vincentkoc.
|
||||||
- Security/Browser: block cross-origin mutating requests to loopback browser control routes (CSRF hardening). Thanks @vincentkoc.
|
- Security/Browser: block cross-origin mutating requests to loopback browser control routes (CSRF hardening). Thanks @vincentkoc.
|
||||||
|
- Security/Slack: compute command authorization for DM slash commands even when `dmPolicy=open`, preventing unauthorized users from running privileged commands via DM. Thanks @christos-eth.
|
||||||
- Security/Nostr: require loopback source and block cross-origin profile mutation/import attempts. Thanks @vincentkoc.
|
- Security/Nostr: require loopback source and block cross-origin profile mutation/import attempts. Thanks @vincentkoc.
|
||||||
- Security/Archive: enforce archive extraction entry/size limits to prevent resource exhaustion from high-expansion ZIP/TAR archives. Thanks @vincentkoc.
|
- Security/Archive: enforce archive extraction entry/size limits to prevent resource exhaustion from high-expansion ZIP/TAR archives. Thanks @vincentkoc.
|
||||||
- Security/Media: reject oversized base64-backed input media before decoding to avoid large allocations. Thanks @vincentkoc.
|
- Security/Media: reject oversized base64-backed input media before decoding to avoid large allocations. Thanks @vincentkoc.
|
||||||
|
|||||||
@@ -267,6 +267,45 @@ describe("slack slash commands access groups", () => {
|
|||||||
expect(respond).not.toHaveBeenCalledWith(
|
expect(respond).not.toHaveBeenCalledWith(
|
||||||
expect.objectContaining({ text: "You are not authorized to use this command." }),
|
expect.objectContaining({ text: "You are not authorized to use this command." }),
|
||||||
);
|
);
|
||||||
|
const dispatchArg = dispatchMock.mock.calls[0]?.[0] as {
|
||||||
|
ctx?: { CommandAuthorized?: boolean };
|
||||||
|
};
|
||||||
|
expect(dispatchArg?.ctx?.CommandAuthorized).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("computes CommandAuthorized for DM slash commands when dmPolicy is open", async () => {
|
||||||
|
const { commands, ctx, account } = createHarness({
|
||||||
|
allowFrom: ["U_OWNER"],
|
||||||
|
channelId: "D999",
|
||||||
|
channelName: "directmessage",
|
||||||
|
resolveChannelName: async () => ({ name: "directmessage", type: "im" }),
|
||||||
|
});
|
||||||
|
registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
|
||||||
|
|
||||||
|
const handler = [...commands.values()][0];
|
||||||
|
if (!handler) {
|
||||||
|
throw new Error("Missing slash handler");
|
||||||
|
}
|
||||||
|
|
||||||
|
const respond = vi.fn().mockResolvedValue(undefined);
|
||||||
|
await handler({
|
||||||
|
command: {
|
||||||
|
user_id: "U_ATTACKER",
|
||||||
|
user_name: "Mallory",
|
||||||
|
channel_id: "D999",
|
||||||
|
channel_name: "directmessage",
|
||||||
|
text: "hello",
|
||||||
|
trigger_id: "t1",
|
||||||
|
},
|
||||||
|
ack: vi.fn().mockResolvedValue(undefined),
|
||||||
|
respond,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(dispatchMock).toHaveBeenCalledTimes(1);
|
||||||
|
const dispatchArg = dispatchMock.mock.calls[0]?.[0] as {
|
||||||
|
ctx?: { CommandAuthorized?: boolean };
|
||||||
|
};
|
||||||
|
expect(dispatchArg?.ctx?.CommandAuthorized).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("enforces access-group gating when lookup fails for private channels", async () => {
|
it("enforces access-group gating when lookup fails for private channels", async () => {
|
||||||
|
|||||||
@@ -204,7 +204,9 @@ export function registerSlackMonitorSlashCommands(params: {
|
|||||||
const effectiveAllowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]);
|
const effectiveAllowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]);
|
||||||
const effectiveAllowFromLower = normalizeAllowListLower(effectiveAllowFrom);
|
const effectiveAllowFromLower = normalizeAllowListLower(effectiveAllowFrom);
|
||||||
|
|
||||||
let commandAuthorized = true;
|
// Privileged command surface: compute CommandAuthorized, don't assume true.
|
||||||
|
// Keep this aligned with the Slack message path (message-handler/prepare.ts).
|
||||||
|
let commandAuthorized = false;
|
||||||
let channelConfig: SlackChannelConfigResolved | null = null;
|
let channelConfig: SlackChannelConfigResolved | null = null;
|
||||||
if (isDirectMessage) {
|
if (isDirectMessage) {
|
||||||
if (!ctx.dmEnabled || ctx.dmPolicy === "disabled") {
|
if (!ctx.dmEnabled || ctx.dmPolicy === "disabled") {
|
||||||
@@ -256,7 +258,6 @@ export function registerSlackMonitorSlashCommands(params: {
|
|||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
commandAuthorized = true;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -322,6 +323,13 @@ export function registerSlackMonitorSlashCommands(params: {
|
|||||||
id: command.user_id,
|
id: command.user_id,
|
||||||
name: senderName,
|
name: senderName,
|
||||||
}).allowed;
|
}).allowed;
|
||||||
|
// DMs: allow chatting in dmPolicy=open, but keep privileged command gating intact by setting
|
||||||
|
// CommandAuthorized based on allowlists/access-groups (downstream decides which commands need it).
|
||||||
|
commandAuthorized = resolveCommandAuthorizedFromAuthorizers({
|
||||||
|
useAccessGroups: ctx.useAccessGroups,
|
||||||
|
authorizers: [{ configured: effectiveAllowFromLower.length > 0, allowed: ownerAllowed }],
|
||||||
|
modeWhenAccessGroupsOff: "configured",
|
||||||
|
});
|
||||||
if (isRoomish) {
|
if (isRoomish) {
|
||||||
commandAuthorized = resolveCommandAuthorizedFromAuthorizers({
|
commandAuthorized = resolveCommandAuthorizedFromAuthorizers({
|
||||||
useAccessGroups: ctx.useAccessGroups,
|
useAccessGroups: ctx.useAccessGroups,
|
||||||
@@ -329,6 +337,7 @@ export function registerSlackMonitorSlashCommands(params: {
|
|||||||
{ configured: effectiveAllowFromLower.length > 0, allowed: ownerAllowed },
|
{ configured: effectiveAllowFromLower.length > 0, allowed: ownerAllowed },
|
||||||
{ configured: channelUsersAllowlistConfigured, allowed: channelUserAllowed },
|
{ configured: channelUsersAllowlistConfigured, allowed: channelUserAllowed },
|
||||||
],
|
],
|
||||||
|
modeWhenAccessGroupsOff: "configured",
|
||||||
});
|
});
|
||||||
if (ctx.useAccessGroups && !commandAuthorized) {
|
if (ctx.useAccessGroups && !commandAuthorized) {
|
||||||
await respond({
|
await respond({
|
||||||
|
|||||||
Reference in New Issue
Block a user