diff --git a/CHANGELOG.md b/CHANGELOG.md index 17cf60ba5..20f15edcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ - Sessions: reset `compactionCount` on `/new` and `/reset`, and preserve `sessions.json` file mode (0600). - Sessions: repair orphaned user turns before embedded prompts. - Channels: treat replies to the bot as implicit mentions across supported channels. +- Security: lock down slash/control commands to sender allowlists across Discord/Slack/Telegram and extend `clawdbot security audit` coverage for missing allowlists and extensions. - Browser: remote profile tab operations prefer persistent Playwright and avoid silent HTTP fallbacks. (#1057) — thanks @mukhtharcm. - Browser: remote profile tab ops follow-up: shared Playwright loader, Playwright-based focus, and more coverage (incl. opt-in live Browserless test). (follow-up to #1057) — thanks @mukhtharcm. - WhatsApp: scope self-chat response prefix; inject pending-only group history and clear after any processed message. diff --git a/src/channels/plugins/discord.ts b/src/channels/plugins/discord.ts index 0a65cc007..97a90561b 100644 --- a/src/channels/plugins/discord.ts +++ b/src/channels/plugins/discord.ts @@ -14,6 +14,7 @@ import { sendMessageDiscord, sendPollDiscord, } from "../../discord/send.js"; +import { resolveNativeCommandsEnabled } from "../../config/commands.js"; import { shouldLogVerbose } from "../../globals.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../routing/session-key.js"; import { getChatChannelMeta } from "../registry.js"; @@ -117,19 +118,53 @@ export const discordPlugin: ChannelPlugin = { normalizeEntry: (raw) => raw.replace(/^(discord|user):/i, "").replace(/^<@!?(\d+)>$/, "$1"), }; }, - collectWarnings: ({ account }) => { + collectWarnings: ({ cfg, account }) => { + const warnings: string[] = []; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - if (groupPolicy !== "open") return []; - const channelAllowlistConfigured = - Boolean(account.config.guilds) && Object.keys(account.config.guilds ?? {}).length > 0; - if (channelAllowlistConfigured) { - return [ - `- Discord guilds: groupPolicy="open" allows any channel not explicitly denied to trigger (mention-gated). Set channels.discord.groupPolicy="allowlist" and configure channels.discord.guilds..channels.`, - ]; + const guildEntries = account.config.guilds ?? {}; + const guildsConfigured = Object.keys(guildEntries).length > 0; + const channelAllowlistConfigured = guildsConfigured; + + if (groupPolicy === "open") { + if (channelAllowlistConfigured) { + warnings.push( + `- Discord guilds: groupPolicy="open" allows any channel not explicitly denied to trigger (mention-gated). Set channels.discord.groupPolicy="allowlist" and configure channels.discord.guilds..channels.`, + ); + } else { + warnings.push( + `- Discord guilds: groupPolicy="open" with no guild/channel allowlist; any channel can trigger (mention-gated). Set channels.discord.groupPolicy="allowlist" and configure channels.discord.guilds..channels.`, + ); + } } - return [ - `- Discord guilds: groupPolicy="open" with no guild/channel allowlist; any channel can trigger (mention-gated). Set channels.discord.groupPolicy="allowlist" and configure channels.discord.guilds..channels.`, - ]; + + const nativeCommandsEnabled = resolveNativeCommandsEnabled({ + providerId: "discord", + providerSetting: account.config.commands?.native, + globalSetting: cfg.commands?.native, + }); + if (nativeCommandsEnabled && guildsConfigured) { + const hasAnyUserAllowlist = Object.values(guildEntries).some((guild) => { + if (!guild || typeof guild !== "object") return false; + if (Array.isArray(guild.users) && guild.users.length > 0) return true; + const channels = guild.channels; + if (!channels || typeof channels !== "object") return false; + return Object.values(channels).some( + (channel) => + Boolean(channel) && + typeof channel === "object" && + Array.isArray(channel.users) && + channel.users.length > 0, + ); + }); + + if (!hasAnyUserAllowlist) { + warnings.push( + `- Discord slash commands: no users allowlist configured; this allows any user in allowed guild channels to invoke /… commands. Set channels.discord.guilds..users (or channels.discord.guilds..channels..users).`, + ); + } + } + + return warnings; }, }, groups: { diff --git a/src/channels/plugins/slack.ts b/src/channels/plugins/slack.ts index 9bf3db690..90b37e385 100644 --- a/src/channels/plugins/slack.ts +++ b/src/channels/plugins/slack.ts @@ -13,6 +13,7 @@ import { probeSlack } from "../../slack/probe.js"; import { sendMessageSlack } from "../../slack/send.js"; import { getChatChannelMeta } from "../registry.js"; import { SlackConfigSchema } from "../../config/zod-schema.providers-core.js"; +import { resolveNativeCommandsEnabled } from "../../config/commands.js"; import { buildChannelConfigSchema } from "./config-schema.js"; import { deleteAccountFromConfigSection, @@ -135,19 +136,51 @@ export const slackPlugin: ChannelPlugin = { normalizeEntry: (raw) => raw.replace(/^(slack|user):/i, ""), }; }, - collectWarnings: ({ account }) => { + collectWarnings: ({ cfg, account }) => { + const warnings: string[] = []; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - if (groupPolicy !== "open") return []; const channelAllowlistConfigured = Boolean(account.config.channels) && Object.keys(account.config.channels ?? {}).length > 0; - if (channelAllowlistConfigured) { - return [ - `- Slack channels: groupPolicy="open" allows any channel not explicitly denied to trigger (mention-gated). Set channels.slack.groupPolicy="allowlist" and configure channels.slack.channels.`, - ]; + const roomAccessPossible = + groupPolicy === "open" || (groupPolicy === "allowlist" && channelAllowlistConfigured); + + if (groupPolicy === "open") { + if (channelAllowlistConfigured) { + warnings.push( + `- Slack channels: groupPolicy="open" allows any channel not explicitly denied to trigger (mention-gated). Set channels.slack.groupPolicy="allowlist" and configure channels.slack.channels.`, + ); + } else { + warnings.push( + `- Slack channels: groupPolicy="open" with no channel allowlist; any channel can trigger (mention-gated). Set channels.slack.groupPolicy="allowlist" and configure channels.slack.channels.`, + ); + } } - return [ - `- Slack channels: groupPolicy="open" with no channel allowlist; any channel can trigger (mention-gated). Set channels.slack.groupPolicy="allowlist" and configure channels.slack.channels.`, - ]; + + const nativeEnabled = resolveNativeCommandsEnabled({ + providerId: "slack", + providerSetting: account.config.commands?.native, + globalSetting: cfg.commands?.native, + }); + const slashCommandEnabled = nativeEnabled || account.config.slashCommand?.enabled === true; + + if (slashCommandEnabled && roomAccessPossible) { + const hasAnyUserAllowlist = Object.values(account.config.channels ?? {}).some( + (channel) => Array.isArray(channel.users) && channel.users.length > 0, + ); + if (!hasAnyUserAllowlist) { + warnings.push( + `- Slack slash commands: no channel users allowlist configured; this allows any user in allowed channels to invoke /… commands (including skill commands). Set channels.slack.channels..users.`, + ); + } + } + + if (slashCommandEnabled && cfg.commands?.useAccessGroups === false) { + warnings.push( + `- Slack slash commands: commands.useAccessGroups=false disables channel allowlist gating; this allows any channel to invoke /… commands (including skill commands). Set commands.useAccessGroups=true and configure channels.slack.groupPolicy/channels.`, + ); + } + + return warnings; }, }, groups: { diff --git a/src/discord/monitor/message-handler.preflight.ts b/src/discord/monitor/message-handler.preflight.ts index 68f9c3eea..7d4ef2efc 100644 --- a/src/discord/monitor/message-handler.preflight.ts +++ b/src/discord/monitor/message-handler.preflight.ts @@ -22,7 +22,6 @@ import { normalizeDiscordAllowList, normalizeDiscordSlug, resolveDiscordChannelConfig, - resolveDiscordCommandAuthorized, resolveDiscordGuildEntry, resolveDiscordShouldRequireMention, resolveDiscordUserAllowed, @@ -314,18 +313,38 @@ export async function preflightDiscordMessage( (message.mentionedUsers?.length ?? 0) > 0 || (message.mentionedRoles?.length ?? 0) > 0), ); - if (!isDirectMessage) { - commandAuthorized = resolveDiscordCommandAuthorized({ - isDirectMessage, - allowFrom: params.allowFrom, - guildInfo, - author, - }); - } const allowTextCommands = shouldHandleTextCommands({ cfg: params.cfg, surface: "discord", }); + + if (!isDirectMessage) { + const ownerAllowList = normalizeDiscordAllowList(params.allowFrom, ["discord:", "user:"]); + const ownerOk = ownerAllowList + ? allowListMatches(ownerAllowList, { + id: author.id, + name: author.username, + tag: formatDiscordUserTag(author), + }) + : false; + const channelUsers = channelConfig?.users ?? guildInfo?.users; + const usersOk = + Array.isArray(channelUsers) && channelUsers.length > 0 + ? resolveDiscordUserAllowed({ + allowList: channelUsers, + userId: author.id, + userName: author.username, + userTag: formatDiscordUserTag(author), + }) + : false; + commandAuthorized = ownerOk || usersOk; + + if (allowTextCommands && hasControlCommand(baseText, params.cfg) && !commandAuthorized) { + logVerbose(`Blocked discord control command from unauthorized sender ${author.id}`); + return null; + } + } + const shouldBypassMention = allowTextCommands && isGuildMessage && diff --git a/src/discord/monitor/native-command.ts b/src/discord/monitor/native-command.ts index 0e99eeed4..f89def249 100644 --- a/src/discord/monitor/native-command.ts +++ b/src/discord/monitor/native-command.ts @@ -423,6 +423,18 @@ async function dispatchDiscordCommandInteraction(params: { const isGroupDm = channelType === ChannelType.GroupDM; const channelName = channel && "name" in channel ? (channel.name as string) : undefined; const channelSlug = channelName ? normalizeDiscordSlug(channelName) : ""; + const ownerAllowList = normalizeDiscordAllowList(discordConfig?.dm?.allowFrom ?? [], [ + "discord:", + "user:", + ]); + const ownerOk = + ownerAllowList && user + ? allowListMatches(ownerAllowList, { + id: user.id, + name: user.username, + tag: formatDiscordUserTag(user), + }) + : false; const guildInfo = resolveDiscordGuildEntry({ guild: interaction.guild ?? undefined, guildEntries: discordConfig?.guilds, @@ -508,17 +520,19 @@ async function dispatchDiscordCommandInteraction(params: { } if (!isDirectMessage) { const channelUsers = channelConfig?.users ?? guildInfo?.users; - if (Array.isArray(channelUsers) && channelUsers.length > 0) { - const userOk = resolveDiscordUserAllowed({ - allowList: channelUsers, - userId: user.id, - userName: user.username, - userTag: formatDiscordUserTag(user), - }); - if (!userOk) { - await respond("You are not authorized to use this command."); - return; - } + const hasUserAllowlist = Array.isArray(channelUsers) && channelUsers.length > 0; + const userOk = hasUserAllowlist + ? resolveDiscordUserAllowed({ + allowList: channelUsers, + userId: user.id, + userName: user.username, + userTag: formatDiscordUserTag(user), + }) + : false; + commandAuthorized = useAccessGroups ? ownerOk || userOk : hasUserAllowlist ? userOk : true; + if (!commandAuthorized) { + await respond("You are not authorized to use this command.", { ephemeral: true }); + return; } } if (isGroupDm && discordConfig?.dm?.groupEnabled === false) { diff --git a/src/security/audit-extra.ts b/src/security/audit-extra.ts index 48023d718..db33aa1e1 100644 --- a/src/security/audit-extra.ts +++ b/src/security/audit-extra.ts @@ -5,6 +5,7 @@ import JSON5 from "json5"; import type { ClawdbotConfig, ConfigFileSnapshot } from "../config/config.js"; import { createConfigIO } from "../config/config.js"; +import { resolveNativeSkillsEnabled } from "../config/commands.js"; import { resolveOAuthDir } from "../config/paths.js"; import { resolveDefaultAgentId } from "../agents/agent-scope.js"; import { INCLUDE_KEY, MAX_INCLUDE_DEPTH } from "../config/includes.js"; @@ -380,11 +381,70 @@ export async function collectPluginsTrustFindings(params: { const allow = params.cfg.plugins?.allow; const allowConfigured = Array.isArray(allow) && allow.length > 0; if (!allowConfigured) { + const hasString = (value: unknown) => typeof value === "string" && value.trim().length > 0; + const hasAccountStringKey = (account: unknown, key: string) => + Boolean(account && typeof account === "object" && hasString((account as Record)[key])); + + const discordConfigured = + hasString(params.cfg.channels?.discord?.token) || + Boolean( + params.cfg.channels?.discord?.accounts && + Object.values(params.cfg.channels.discord.accounts).some((a) => hasAccountStringKey(a, "token")), + ) || + hasString(process.env.DISCORD_BOT_TOKEN); + + const telegramConfigured = + hasString(params.cfg.channels?.telegram?.botToken) || + hasString(params.cfg.channels?.telegram?.tokenFile) || + Boolean( + params.cfg.channels?.telegram?.accounts && + Object.values(params.cfg.channels.telegram.accounts).some( + (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "tokenFile"), + ), + ) || + hasString(process.env.TELEGRAM_BOT_TOKEN); + + const slackConfigured = + hasString(params.cfg.channels?.slack?.botToken) || + hasString(params.cfg.channels?.slack?.appToken) || + Boolean( + params.cfg.channels?.slack?.accounts && + Object.values(params.cfg.channels.slack.accounts).some( + (a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "appToken"), + ), + ) || + hasString(process.env.SLACK_BOT_TOKEN) || + hasString(process.env.SLACK_APP_TOKEN); + + const skillCommandsLikelyExposed = + (discordConfigured && + resolveNativeSkillsEnabled({ + providerId: "discord", + providerSetting: params.cfg.channels?.discord?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, + })) || + (telegramConfigured && + resolveNativeSkillsEnabled({ + providerId: "telegram", + providerSetting: params.cfg.channels?.telegram?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, + })) || + (slackConfigured && + resolveNativeSkillsEnabled({ + providerId: "slack", + providerSetting: params.cfg.channels?.slack?.commands?.nativeSkills, + globalSetting: params.cfg.commands?.nativeSkills, + })); + findings.push({ checkId: "plugins.extensions_no_allowlist", - severity: "warn", + severity: skillCommandsLikelyExposed ? "critical" : "warn", title: "Extensions exist but plugins.allow is not set", - detail: `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).`, + detail: + `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).` + + (skillCommandsLikelyExposed + ? "\nNative skill commands are enabled on at least one configured chat surface; treat unpinned/unallowlisted extensions as high risk." + : ""), remediation: "Set plugins.allow to an explicit list of plugin ids you trust.", }); } diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 4220c51c1..b9d230d10 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -3,6 +3,9 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { ClawdbotConfig } from "../config/config.js"; import type { ChannelPlugin } from "../channels/plugins/types.js"; import { runSecurityAudit } from "./audit.js"; +import { discordPlugin } from "../channels/plugins/discord.js"; +import { slackPlugin } from "../channels/plugins/slack.js"; +import { telegramPlugin } from "../channels/plugins/telegram.js"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -222,6 +225,97 @@ describe("security audit", () => { ); }); + it("flags Discord native commands without a guild user allowlist", async () => { + const cfg: ClawdbotConfig = { + channels: { + discord: { + enabled: true, + token: "t", + groupPolicy: "allowlist", + guilds: { + "123": { + channels: { + general: { allow: true }, + }, + }, + }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: true, + plugins: [discordPlugin], + }); + + const finding = res.findings.find((f) => f.detail.includes("Discord slash commands")); + expect(finding?.severity).toBe("critical"); + }); + + it("flags Slack slash commands without a channel users allowlist", async () => { + const cfg: ClawdbotConfig = { + channels: { + slack: { + enabled: true, + botToken: "xoxb-test", + appToken: "xapp-test", + groupPolicy: "open", + slashCommand: { enabled: true }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: true, + plugins: [slackPlugin], + }); + + const finding = res.findings.find((f) => f.detail.includes("Slack slash commands")); + expect(finding?.severity).toBe("critical"); + }); + + it("flags Telegram group commands without a sender allowlist", async () => { + const prevStateDir = process.env.CLAWDBOT_STATE_DIR; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-telegram-")); + process.env.CLAWDBOT_STATE_DIR = tmp; + await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 }); + try { + const cfg: ClawdbotConfig = { + channels: { + telegram: { + enabled: true, + botToken: "t", + groupPolicy: "allowlist", + groups: { "-100123": {} }, + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: true, + plugins: [telegramPlugin], + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.telegram.groups.allowFrom.missing", + severity: "critical", + }), + ]), + ); + } finally { + if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = prevStateDir; + } + }); + it("adds a warning when deep probe fails", async () => { const cfg: ClawdbotConfig = { gateway: { mode: "local" } }; @@ -380,6 +474,14 @@ describe("security audit", () => { }); it("flags extensions without plugins.allow", async () => { + const prevDiscordToken = process.env.DISCORD_BOT_TOKEN; + const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; + const prevSlackBotToken = process.env.SLACK_BOT_TOKEN; + const prevSlackAppToken = process.env.SLACK_APP_TOKEN; + delete process.env.DISCORD_BOT_TOKEN; + delete process.env.TELEGRAM_BOT_TOKEN; + delete process.env.SLACK_BOT_TOKEN; + delete process.env.SLACK_APP_TOKEN; const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-")); const stateDir = path.join(tmp, "state"); await fs.mkdir(path.join(stateDir, "extensions", "some-plugin"), { @@ -387,20 +489,69 @@ describe("security audit", () => { mode: 0o700, }); - const cfg: ClawdbotConfig = {}; - const res = await runSecurityAudit({ - config: cfg, - includeFilesystem: true, - includeChannelSecurity: false, - stateDir, - configPath: path.join(stateDir, "clawdbot.json"), + try { + const cfg: ClawdbotConfig = {}; + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "clawdbot.json"), + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ checkId: "plugins.extensions_no_allowlist", severity: "warn" }), + ]), + ); + } finally { + if (prevDiscordToken == null) delete process.env.DISCORD_BOT_TOKEN; + else process.env.DISCORD_BOT_TOKEN = prevDiscordToken; + if (prevTelegramToken == null) delete process.env.TELEGRAM_BOT_TOKEN; + else process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; + if (prevSlackBotToken == null) delete process.env.SLACK_BOT_TOKEN; + else process.env.SLACK_BOT_TOKEN = prevSlackBotToken; + if (prevSlackAppToken == null) delete process.env.SLACK_APP_TOKEN; + else process.env.SLACK_APP_TOKEN = prevSlackAppToken; + } + }); + + it("flags unallowlisted extensions as critical when native skill commands are exposed", async () => { + const prevDiscordToken = process.env.DISCORD_BOT_TOKEN; + delete process.env.DISCORD_BOT_TOKEN; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-")); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(path.join(stateDir, "extensions", "some-plugin"), { + recursive: true, + mode: 0o700, }); - expect(res.findings).toEqual( - expect.arrayContaining([ - expect.objectContaining({ checkId: "plugins.extensions_no_allowlist", severity: "warn" }), - ]), - ); + try { + const cfg: ClawdbotConfig = { + channels: { + discord: { enabled: true, token: "t" }, + }, + }; + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath: path.join(stateDir, "clawdbot.json"), + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "plugins.extensions_no_allowlist", + severity: "critical", + }), + ]), + ); + } finally { + if (prevDiscordToken == null) delete process.env.DISCORD_BOT_TOKEN; + else process.env.DISCORD_BOT_TOKEN = prevDiscordToken; + } }); it("flags open groupPolicy when tools.elevated is enabled", async () => { diff --git a/src/security/audit.ts b/src/security/audit.ts index 6e8897980..f0547633c 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -20,6 +20,7 @@ import { readConfigSnapshotForAudit, } from "./audit-extra.js"; import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; +import { resolveNativeSkillsEnabled } from "../config/commands.js"; import { formatOctal, isGroupReadable, @@ -498,6 +499,81 @@ async function collectChannelSecurityFindings(params: { }); } } + + if (plugin.id === "telegram") { + const allowTextCommands = params.cfg.commands?.text !== false; + if (!allowTextCommands) continue; + + const telegramCfg = + (account as { config?: Record } | null)?.config ?? ({} as Record< + string, + unknown + >); + const groupPolicy = (telegramCfg.groupPolicy as string | undefined) ?? "allowlist"; + const groups = telegramCfg.groups as Record | undefined; + const groupsConfigured = Boolean(groups) && Object.keys(groups ?? {}).length > 0; + const groupAccessPossible = + groupPolicy === "open" || (groupPolicy === "allowlist" && groupsConfigured); + if (!groupAccessPossible) continue; + + const storeAllowFrom = await readChannelAllowFromStore("telegram").catch(() => []); + const storeHasWildcard = storeAllowFrom.some((v) => String(v).trim() === "*"); + const groupAllowFrom = Array.isArray(telegramCfg.groupAllowFrom) ? telegramCfg.groupAllowFrom : []; + const groupAllowFromHasWildcard = groupAllowFrom.some((v) => String(v).trim() === "*"); + const anyGroupOverride = Boolean( + groups && + Object.values(groups).some((value) => { + if (!value || typeof value !== "object") return false; + const group = value as Record; + const allowFrom = Array.isArray(group.allowFrom) ? group.allowFrom : []; + if (allowFrom.length > 0) return true; + const topics = group.topics; + if (!topics || typeof topics !== "object") return false; + return Object.values(topics as Record).some((topicValue) => { + if (!topicValue || typeof topicValue !== "object") return false; + const topic = topicValue as Record; + const topicAllow = Array.isArray(topic.allowFrom) ? topic.allowFrom : []; + return topicAllow.length > 0; + }); + }), + ); + + const hasAnySenderAllowlist = + storeAllowFrom.length > 0 || groupAllowFrom.length > 0 || anyGroupOverride; + + if (storeHasWildcard || groupAllowFromHasWildcard) { + findings.push({ + checkId: "channels.telegram.groups.allowFrom.wildcard", + severity: "critical", + title: "Telegram group allowlist contains wildcard", + detail: + 'Telegram group sender allowlist contains "*", which allows any group member to run /… commands and control directives.', + remediation: + 'Remove "*" from channels.telegram.groupAllowFrom and pairing store; prefer explicit user ids/usernames.', + }); + continue; + } + + if (!hasAnySenderAllowlist) { + const providerSetting = (telegramCfg.commands as { nativeSkills?: unknown } | undefined) + ?.nativeSkills as any; + const skillsEnabled = resolveNativeSkillsEnabled({ + providerId: "telegram", + providerSetting, + globalSetting: params.cfg.commands?.nativeSkills, + }); + findings.push({ + checkId: "channels.telegram.groups.allowFrom.missing", + severity: "critical", + title: "Telegram group commands have no sender allowlist", + detail: + `Telegram group access is enabled but no sender allowlist is configured; this allows any group member to invoke /… commands` + + (skillsEnabled ? " (including skill commands)." : "."), + remediation: + "Approve yourself via pairing (recommended), or set channels.telegram.groupAllowFrom (or per-group groups..allowFrom).", + }); + } + } } return findings; diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index c79792fb2..8a5a5186e 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -26,7 +26,7 @@ import { sendMessageSlack } from "../../send.js"; import type { SlackMessageEvent } from "../../types.js"; import { allowListMatches, resolveSlackUserAllowed } from "../allow-list.js"; -import { isSlackSenderAllowListed, resolveSlackEffectiveAllowFrom } from "../auth.js"; +import { resolveSlackEffectiveAllowFrom } from "../auth.js"; import { resolveSlackChannelConfig } from "../channel-config.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "../context.js"; import { resolveSlackMedia, resolveSlackThreadStarter } from "../media.js"; @@ -217,18 +217,34 @@ export async function prepareSlackMessage(params: { return null; } - const commandAuthorized = - isSlackSenderAllowListed({ - allowListLower: allowFromLower, - senderId, - senderName, - }) && channelUserAuthorized; - const hasAnyMention = /<@[^>]+>/.test(message.text ?? ""); const allowTextCommands = shouldHandleTextCommands({ cfg, surface: "slack", }); + + const ownerAuthorized = allowListMatches({ + allowList: allowFromLower, + id: senderId, + name: senderName, + }); + const channelUsersAllowlistConfigured = + isRoom && Array.isArray(channelConfig?.users) && channelConfig.users.length > 0; + const channelCommandAuthorized = + isRoom && channelUsersAllowlistConfigured + ? resolveSlackUserAllowed({ + allowList: channelConfig?.users, + userId: senderId, + userName: senderName, + }) + : false; + const commandAuthorized = ownerAuthorized || channelCommandAuthorized; + + if (allowTextCommands && isRoomish && hasControlCommand(message.text ?? "", cfg) && !commandAuthorized) { + logVerbose(`Blocked slack control command from unauthorized sender ${senderId}`); + return null; + } + const shouldRequireMention = isRoom ? (channelConfig?.requireMention ?? ctx.defaultRequireMention) : false; diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 1b4123eac..dc09b5e1e 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -167,6 +167,7 @@ export function registerSlackMonitorSlashCommands(params: { const isDirectMessage = channelType === "im"; const isGroupDm = channelType === "mpim"; const isRoom = channelType === "channel" || channelType === "group"; + const isRoomish = isRoom || isGroupDm; if ( !ctx.isChannelAllowed({ @@ -270,14 +271,16 @@ export function registerSlackMonitorSlashCommands(params: { const sender = await ctx.resolveUserName(command.user_id); const senderName = sender?.name ?? command.user_name ?? command.user_id; - const channelUserAllowed = isRoom + const channelUsersAllowlistConfigured = + isRoom && Array.isArray(channelConfig?.users) && channelConfig.users.length > 0; + const channelUserAllowed = channelUsersAllowlistConfigured ? resolveSlackUserAllowed({ allowList: channelConfig?.users, userId: command.user_id, userName: senderName, }) - : true; - if (isRoom && !channelUserAllowed) { + : false; + if (channelUsersAllowlistConfigured && !channelUserAllowed) { await respond({ text: "You are not authorized to use this command here.", response_type: "ephemeral", @@ -285,6 +288,22 @@ export function registerSlackMonitorSlashCommands(params: { return; } + const ownerAllowed = allowListMatches({ + allowList: effectiveAllowFromLower, + id: command.user_id, + name: senderName, + }); + if (isRoomish && ctx.useAccessGroups && !(ownerAllowed || channelUserAllowed)) { + await respond({ + text: "You are not authorized to use this command.", + response_type: "ephemeral", + }); + return; + } + if (isRoomish) { + commandAuthorized = ctx.useAccessGroups ? ownerAllowed || channelUserAllowed : true; + } + if (commandDefinition && supportsInteractiveArgMenus) { const menu = resolveCommandArgMenu({ command: commandDefinition, @@ -313,7 +332,6 @@ export function registerSlackMonitorSlashCommands(params: { const channelName = channelInfo?.name; const roomLabel = channelName ? `#${channelName}` : `#${command.channel_id}`; - const isRoomish = isRoom || isGroupDm; const route = resolveAgentRoute({ cfg, channel: "slack", diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index 90a92e6c9..dc614fc16 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -198,11 +198,15 @@ export const buildTelegramMessageContext = async ({ return null; } } - const commandAuthorized = isSenderAllowed({ - allow: isGroup ? effectiveGroupAllow : effectiveDmAllow, + const allowForCommands = isGroup ? effectiveGroupAllow : effectiveDmAllow; + const senderAllowedForCommands = isSenderAllowed({ + allow: allowForCommands, senderId, senderUsername, }); + const useAccessGroups = cfg.commands?.useAccessGroups !== false; + const commandAuthorized = + useAccessGroups && !allowForCommands.hasEntries ? false : senderAllowedForCommands; const historyKey = isGroup ? buildTelegramGroupPeerId(chatId, resolvedThreadId) : undefined; let placeholder = ""; @@ -229,6 +233,14 @@ export const buildTelegramMessageContext = async ({ const hasAnyMention = (msg.entities ?? msg.caption_entities ?? []).some( (ent) => ent.type === "mention", ); + if ( + isGroup && + hasControlCommand(msg.text ?? msg.caption ?? "", cfg, { botUsername }) && + !commandAuthorized + ) { + logVerbose(`telegram: drop control command from unauthorized sender ${senderId ?? "unknown"}`); + return null; + } const activationOverride = resolveGroupActivation({ chatId, messageThreadId: resolvedThreadId, diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index 5d5a1d4bf..adf7eddf1 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -172,17 +172,18 @@ export const registerTelegramNativeCommands = ({ : []; const senderId = msg.from?.id ? String(msg.from.id) : ""; const senderUsername = msg.from?.username ?? ""; - const commandAuthorized = - allowFromList.length === 0 || - allowFromList.includes("*") || - (senderId && allowFromList.includes(senderId)) || - (senderId && allowFromList.includes(`telegram:${senderId}`)) || - (senderUsername && - allowFromList.some( - (entry) => - entry.toLowerCase() === senderUsername.toLowerCase() || - entry.toLowerCase() === `@${senderUsername.toLowerCase()}`, - )); + const allowFromConfigured = allowFromList.length > 0; + const commandAuthorized = allowFromConfigured + ? allowFromList.includes("*") || + (senderId && allowFromList.includes(senderId)) || + (senderId && allowFromList.includes(`telegram:${senderId}`)) || + (senderUsername && + allowFromList.some( + (entry) => + entry.toLowerCase() === senderUsername.toLowerCase() || + entry.toLowerCase() === `@${senderUsername.toLowerCase()}`, + )) + : !useAccessGroups; if (!commandAuthorized) { await bot.api.sendMessage(chatId, "You are not authorized to use this command."); return; diff --git a/src/telegram/bot.create-telegram-bot.applies-topic-skill-filters-system-prompts.test.ts b/src/telegram/bot.create-telegram-bot.applies-topic-skill-filters-system-prompts.test.ts index 788e642e2..2afe8cd1c 100644 --- a/src/telegram/bot.create-telegram-bot.applies-topic-skill-filters-system-prompts.test.ts +++ b/src/telegram/bot.create-telegram-bot.applies-topic-skill-filters-system-prompts.test.ts @@ -296,9 +296,11 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ commands: { native: true }, - telegram: { - dmPolicy: "open", - allowFrom: ["*"], + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + }, }, }); diff --git a/src/telegram/bot.test.ts b/src/telegram/bot.test.ts index cd7f3a551..ad4deb039 100644 --- a/src/telegram/bot.test.ts +++ b/src/telegram/bot.test.ts @@ -2064,9 +2064,11 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ commands: { native: true }, - telegram: { - dmPolicy: "open", - allowFrom: ["*"], + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + }, }, });