mirror of
https://fastgit.cc/github.com/openclaw/openclaw
synced 2026-05-01 06:36:23 +08:00
fix(slack): gate bot room relays on owner presence
This commit is contained in:
@@ -8,6 +8,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
- Agents/subagents: bound automatic orphan recovery with persisted recovery attempts and a wedged-session tombstone, and teach task maintenance/doctor to reconcile those sessions so restart loops no longer require manual `sessions.json` surgery. Fixes #74864. Thanks @solosage1.
|
||||
- Gateway/startup: skip pre-bind web-fetch provider discovery for credential-free `tools.web.fetch` config, so Docker/Kubernetes gateways bind even when optional fetch limits are present. Fixes #74896. Thanks @KoykL.
|
||||
- Slack: require bot-authored room messages with `allowBots=true` to come from an explicitly channel-allowlisted bot or from a room where an explicit Slack owner is present, so broad bot relays cannot run unattended. Fixes #59284. Thanks @andrewhong-translucent.
|
||||
- CLI/progress: suppress nested progress spinners and line clears while TUI input owns raw stdin, so Crestodian `/status` no longer disturbs the active input row. (#75003) Thanks @velvet-shark.
|
||||
- Models/OpenAI Codex: restore `openai-codex/gpt-5.4-mini` for ChatGPT/Codex OAuth PI runs after live OAuth proof, and align the manifest, forward-compat metadata, docs, and regression tests so stale cron and heartbeat configs resolve again. Fixes #74451. Thanks @0xCyda, @hclsys, and @Marvae.
|
||||
- Telegram: use durable message edits for streaming previews instead of native draft state, so generated replies no longer flicker through draft-to-message transitions that look like duplicates. (#75073) Thanks @obviyus.
|
||||
|
||||
@@ -582,6 +582,8 @@ Current Slack message actions include `send`, `upload-file`, `download-file`, `r
|
||||
- `toolsBySender` key format: `id:`, `e164:`, `username:`, `name:`, or `"*"` wildcard
|
||||
(legacy unprefixed keys still map to `id:` only)
|
||||
|
||||
`allowBots` is conservative for channels and private channels: bot-authored room messages are accepted only when the sending bot is explicitly listed in that room's `users` allowlist, or when at least one explicit Slack owner ID from `channels.slack.allowFrom` is currently a room member. Wildcards and display-name owner entries do not satisfy owner presence. Owner presence uses Slack `conversations.members`; make sure the app has the matching read scope for the room type (`channels:read` for public channels, `groups:read` for private channels). If the member lookup fails, OpenClaw drops the bot-authored room message.
|
||||
|
||||
</Tab>
|
||||
</Tabs>
|
||||
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
|
||||
import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { readStoreAllowFromForDmPolicy } from "openclaw/plugin-sdk/security-runtime";
|
||||
import {
|
||||
allowListMatches,
|
||||
normalizeAllowList,
|
||||
normalizeAllowListLower,
|
||||
normalizeSlackAllowOwnerEntry,
|
||||
resolveSlackAllowListMatch,
|
||||
resolveSlackUserAllowed,
|
||||
} from "./allow-list.js";
|
||||
@@ -24,8 +27,20 @@ type SlackAllowFromCacheState = {
|
||||
pairingPending?: Promise<ResolvedAllowFromLists>;
|
||||
};
|
||||
|
||||
type SlackChannelMembersCacheEntry = {
|
||||
expiresAtMs: number;
|
||||
members?: Set<string>;
|
||||
pending?: Promise<Set<string>>;
|
||||
};
|
||||
|
||||
let slackAllowFromCache = new WeakMap<SlackMonitorContext, SlackAllowFromCacheState>();
|
||||
let slackChannelMembersCache = new WeakMap<
|
||||
SlackMonitorContext,
|
||||
Map<string, SlackChannelMembersCacheEntry>
|
||||
>();
|
||||
const DEFAULT_PAIRING_ALLOW_FROM_CACHE_TTL_MS = 5000;
|
||||
const DEFAULT_CHANNEL_MEMBERS_CACHE_TTL_MS = 60_000;
|
||||
const CHANNEL_MEMBERS_CACHE_MAX = 512;
|
||||
|
||||
function getPairingAllowFromCacheTtlMs(): number {
|
||||
const raw = process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS?.trim();
|
||||
@@ -39,6 +54,18 @@ function getPairingAllowFromCacheTtlMs(): number {
|
||||
return Math.max(0, Math.floor(parsed));
|
||||
}
|
||||
|
||||
function getChannelMembersCacheTtlMs(): number {
|
||||
const raw = process.env.OPENCLAW_SLACK_CHANNEL_MEMBERS_CACHE_TTL_MS?.trim();
|
||||
if (!raw) {
|
||||
return DEFAULT_CHANNEL_MEMBERS_CACHE_TTL_MS;
|
||||
}
|
||||
const parsed = Number(raw);
|
||||
if (!Number.isFinite(parsed)) {
|
||||
return DEFAULT_CHANNEL_MEMBERS_CACHE_TTL_MS;
|
||||
}
|
||||
return Math.max(0, Math.floor(parsed));
|
||||
}
|
||||
|
||||
function getAllowFromCacheState(ctx: SlackMonitorContext): SlackAllowFromCacheState {
|
||||
const existing = slackAllowFromCache.get(ctx);
|
||||
if (existing) {
|
||||
@@ -49,6 +76,28 @@ function getAllowFromCacheState(ctx: SlackMonitorContext): SlackAllowFromCacheSt
|
||||
return next;
|
||||
}
|
||||
|
||||
function getChannelMembersCache(
|
||||
ctx: SlackMonitorContext,
|
||||
): Map<string, SlackChannelMembersCacheEntry> {
|
||||
const existing = slackChannelMembersCache.get(ctx);
|
||||
if (existing) {
|
||||
return existing;
|
||||
}
|
||||
const next = new Map<string, SlackChannelMembersCacheEntry>();
|
||||
slackChannelMembersCache.set(ctx, next);
|
||||
return next;
|
||||
}
|
||||
|
||||
function pruneChannelMembersCache(cache: Map<string, SlackChannelMembersCacheEntry>): void {
|
||||
while (cache.size > CHANNEL_MEMBERS_CACHE_MAX) {
|
||||
const oldest = cache.keys().next();
|
||||
if (oldest.done) {
|
||||
return;
|
||||
}
|
||||
cache.delete(oldest.value);
|
||||
}
|
||||
}
|
||||
|
||||
function buildBaseAllowFrom(ctx: SlackMonitorContext): ResolvedAllowFromLists {
|
||||
const allowFrom = normalizeAllowList(ctx.allowFrom);
|
||||
return {
|
||||
@@ -131,6 +180,10 @@ export async function resolveSlackEffectiveAllowFrom(
|
||||
|
||||
export function clearSlackAllowFromCacheForTest(): void {
|
||||
slackAllowFromCache = new WeakMap<SlackMonitorContext, SlackAllowFromCacheState>();
|
||||
slackChannelMembersCache = new WeakMap<
|
||||
SlackMonitorContext,
|
||||
Map<string, SlackChannelMembersCacheEntry>
|
||||
>();
|
||||
}
|
||||
|
||||
export function isSlackSenderAllowListed(params: {
|
||||
@@ -151,6 +204,128 @@ export function isSlackSenderAllowListed(params: {
|
||||
);
|
||||
}
|
||||
|
||||
async function fetchSlackChannelMemberIds(
|
||||
ctx: SlackMonitorContext,
|
||||
channelId: string,
|
||||
): Promise<Set<string>> {
|
||||
const members = new Set<string>();
|
||||
let cursor: string | undefined;
|
||||
do {
|
||||
const response = await ctx.app.client.conversations.members({
|
||||
token: ctx.botToken,
|
||||
channel: channelId,
|
||||
limit: 999,
|
||||
...(cursor ? { cursor } : {}),
|
||||
});
|
||||
for (const member of normalizeAllowListLower(response.members)) {
|
||||
members.add(member);
|
||||
}
|
||||
const nextCursor = response.response_metadata?.next_cursor?.trim();
|
||||
cursor = nextCursor ? nextCursor : undefined;
|
||||
} while (cursor);
|
||||
return members;
|
||||
}
|
||||
|
||||
async function resolveSlackChannelMemberIds(
|
||||
ctx: SlackMonitorContext,
|
||||
channelId: string,
|
||||
): Promise<Set<string>> {
|
||||
const cache = getChannelMembersCache(ctx);
|
||||
const key = `${ctx.accountId}:${channelId}`;
|
||||
const ttlMs = getChannelMembersCacheTtlMs();
|
||||
const nowMs = Date.now();
|
||||
const cached = cache.get(key);
|
||||
if (ttlMs > 0 && cached?.members && cached.expiresAtMs >= nowMs) {
|
||||
return cached.members;
|
||||
}
|
||||
if (cached?.pending) {
|
||||
return await cached.pending;
|
||||
}
|
||||
|
||||
const pending = fetchSlackChannelMemberIds(ctx, channelId);
|
||||
cache.set(key, {
|
||||
expiresAtMs: ttlMs > 0 ? nowMs + ttlMs : 0,
|
||||
pending,
|
||||
});
|
||||
pruneChannelMembersCache(cache);
|
||||
try {
|
||||
const members = await pending;
|
||||
if (ttlMs > 0) {
|
||||
cache.set(key, {
|
||||
expiresAtMs: Date.now() + ttlMs,
|
||||
members,
|
||||
});
|
||||
pruneChannelMembersCache(cache);
|
||||
} else {
|
||||
cache.delete(key);
|
||||
}
|
||||
return members;
|
||||
} finally {
|
||||
const latest = cache.get(key);
|
||||
if (latest?.pending === pending) {
|
||||
cache.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function resolveExplicitSlackOwnerIds(allowFromLower: string[]): string[] {
|
||||
const ownerIds = new Set<string>();
|
||||
for (const entry of allowFromLower) {
|
||||
const ownerId = normalizeSlackAllowOwnerEntry(entry);
|
||||
if (ownerId) {
|
||||
ownerIds.add(ownerId);
|
||||
}
|
||||
}
|
||||
return [...ownerIds];
|
||||
}
|
||||
|
||||
export async function authorizeSlackBotRoomMessage(params: {
|
||||
ctx: SlackMonitorContext;
|
||||
channelId: string;
|
||||
senderId: string;
|
||||
senderName?: string;
|
||||
channelUsers?: Array<string | number>;
|
||||
allowFromLower: string[];
|
||||
}): Promise<boolean> {
|
||||
const channelUserAllowList = normalizeAllowListLower(params.channelUsers).filter(
|
||||
(entry) => entry !== "*",
|
||||
);
|
||||
if (
|
||||
channelUserAllowList.length > 0 &&
|
||||
allowListMatches({
|
||||
allowList: channelUserAllowList,
|
||||
id: params.senderId,
|
||||
name: params.senderName,
|
||||
allowNameMatching: params.ctx.allowNameMatching,
|
||||
})
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const explicitOwnerIds = resolveExplicitSlackOwnerIds(params.allowFromLower);
|
||||
if (explicitOwnerIds.length === 0) {
|
||||
logVerbose(
|
||||
`slack: drop bot message ${params.senderId} in ${params.channelId} (no explicit owner id for presence check)`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
const channelMemberIds = await resolveSlackChannelMemberIds(params.ctx, params.channelId);
|
||||
if (explicitOwnerIds.some((ownerId) => channelMemberIds.has(ownerId))) {
|
||||
return true;
|
||||
}
|
||||
logVerbose(
|
||||
`slack: drop bot message ${params.senderId} in ${params.channelId} (no owner present)`,
|
||||
);
|
||||
} catch (error) {
|
||||
logVerbose(
|
||||
`slack: drop bot message ${params.senderId} in ${params.channelId} (owner presence lookup failed: ${formatErrorMessage(error)})`,
|
||||
);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export type SlackSystemEventAuthResult = {
|
||||
allowed: boolean;
|
||||
reason?:
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
recordSlackThreadParticipation,
|
||||
} from "../../sent-thread-cache.js";
|
||||
import type { SlackMessageEvent } from "../../types.js";
|
||||
import { clearSlackAllowFromCacheForTest } from "../auth.js";
|
||||
import type { SlackMonitorContext } from "../context.js";
|
||||
import { resetSlackThreadStarterCacheForTest } from "../thread.js";
|
||||
import { resolveSlackMessageContent } from "./prepare-content.js";
|
||||
@@ -37,6 +38,7 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
||||
beforeEach(() => {
|
||||
resetSlackThreadStarterCacheForTest();
|
||||
clearSlackThreadParticipationCache();
|
||||
clearSlackAllowFromCacheForTest();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
@@ -86,6 +88,37 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
||||
} as SlackMessageEvent;
|
||||
}
|
||||
|
||||
function createBotRoomMessage(overrides: Partial<SlackMessageEvent> = {}): SlackMessageEvent {
|
||||
return createSlackMessage({
|
||||
channel: "C123",
|
||||
channel_type: "channel",
|
||||
user: undefined,
|
||||
bot_id: "B0AGV8EQYA3",
|
||||
subtype: "bot_message",
|
||||
username: "deploy-bot",
|
||||
text: "Readiness probe failed",
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
function createOwnerScopedBotRoomCtx(params: { members: string[] }) {
|
||||
const members = vi.fn().mockResolvedValue({
|
||||
members: params.members,
|
||||
response_metadata: { next_cursor: "" },
|
||||
});
|
||||
const slackCtx = createInboundSlackCtx({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: { enabled: true },
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
appClient: { conversations: { members } } as unknown as App["client"],
|
||||
defaultRequireMention: false,
|
||||
});
|
||||
slackCtx.allowFrom = ["UOWNER"];
|
||||
return { slackCtx, members };
|
||||
}
|
||||
|
||||
async function prepareMessageWith(
|
||||
ctx: SlackMonitorContext,
|
||||
account: ResolvedSlackAccount,
|
||||
@@ -424,6 +457,83 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
||||
expect(prepared!.ctxPayload.RawBody).toContain("Readiness probe failed");
|
||||
});
|
||||
|
||||
it("drops bot-authored room messages when allowBots is true but no owner is present (#59284)", async () => {
|
||||
const { slackCtx, members } = createOwnerScopedBotRoomCtx({ members: ["UOTHER"] });
|
||||
|
||||
const prepared = await prepareMessageWith(
|
||||
slackCtx,
|
||||
createSlackAccount({ allowBots: true }),
|
||||
createBotRoomMessage(),
|
||||
);
|
||||
|
||||
expect(prepared).toBeNull();
|
||||
expect(members).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ token: "token", channel: "C123", limit: 999 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("allows bot-authored room messages when an explicit owner is present (#59284)", async () => {
|
||||
const { slackCtx, members } = createOwnerScopedBotRoomCtx({ members: ["UOWNER"] });
|
||||
|
||||
const prepared = await prepareMessageWith(
|
||||
slackCtx,
|
||||
createSlackAccount({ allowBots: true }),
|
||||
createBotRoomMessage(),
|
||||
);
|
||||
|
||||
expect(prepared).toBeTruthy();
|
||||
expect(prepared!.ctxPayload.RawBody).toContain("Readiness probe failed");
|
||||
expect(members).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("allows bot-authored room messages when the bot is explicitly channel-allowlisted (#59284)", async () => {
|
||||
const members = vi.fn();
|
||||
const slackCtx = createInboundSlackCtx({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: { enabled: true },
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
appClient: { conversations: { members } } as unknown as App["client"],
|
||||
defaultRequireMention: false,
|
||||
channelsConfig: {
|
||||
C123: { users: ["B0AGV8EQYA3"] },
|
||||
},
|
||||
});
|
||||
|
||||
const prepared = await prepareMessageWith(
|
||||
slackCtx,
|
||||
createSlackAccount({ allowBots: true }),
|
||||
createBotRoomMessage(),
|
||||
);
|
||||
|
||||
expect(prepared).toBeTruthy();
|
||||
expect(prepared!.ctxPayload.RawBody).toContain("Readiness probe failed");
|
||||
expect(members).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("drops bot-authored room messages when owner presence lookup fails (#59284)", async () => {
|
||||
const members = vi.fn().mockRejectedValue(new Error("missing_scope"));
|
||||
const slackCtx = createInboundSlackCtx({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: { enabled: true },
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
appClient: { conversations: { members } } as unknown as App["client"],
|
||||
defaultRequireMention: false,
|
||||
});
|
||||
slackCtx.allowFrom = ["UOWNER"];
|
||||
|
||||
const prepared = await prepareMessageWith(
|
||||
slackCtx,
|
||||
createSlackAccount({ allowBots: true }),
|
||||
createBotRoomMessage(),
|
||||
);
|
||||
|
||||
expect(prepared).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps channel metadata out of GroupSystemPrompt", async () => {
|
||||
const slackCtx = createInboundSlackCtx({
|
||||
cfg: {
|
||||
|
||||
@@ -41,7 +41,7 @@ import {
|
||||
resolveSlackAllowListMatch,
|
||||
resolveSlackUserAllowed,
|
||||
} from "../allow-list.js";
|
||||
import { resolveSlackEffectiveAllowFrom } from "../auth.js";
|
||||
import { authorizeSlackBotRoomMessage, resolveSlackEffectiveAllowFrom } from "../auth.js";
|
||||
import { resolveSlackChannelConfig } from "../channel-config.js";
|
||||
import { stripSlackMentionsForCommandDetection } from "../commands.js";
|
||||
import {
|
||||
@@ -271,6 +271,7 @@ export async function prepareSlackMessage(params: {
|
||||
isRoom,
|
||||
isRoomish,
|
||||
channelConfig,
|
||||
allowBots,
|
||||
isBotMessage,
|
||||
} = conversation;
|
||||
const authorization = await authorizeSlackInboundMessage({
|
||||
@@ -394,6 +395,21 @@ export async function prepareSlackMessage(params: {
|
||||
logVerbose(`Blocked unauthorized slack sender ${senderId} (not in channel users)`);
|
||||
return null;
|
||||
}
|
||||
if (
|
||||
isRoom &&
|
||||
isBotMessage &&
|
||||
allowBots &&
|
||||
!(await authorizeSlackBotRoomMessage({
|
||||
ctx,
|
||||
channelId: message.channel,
|
||||
senderId,
|
||||
senderName: senderNameForAuth,
|
||||
channelUsers: channelConfig?.users,
|
||||
allowFromLower,
|
||||
}))
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const allowTextCommands = shouldHandleTextCommands({
|
||||
cfg,
|
||||
|
||||
Reference in New Issue
Block a user