diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b86f1696..318e540a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,8 @@ - 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. +- Security: lock down slash/control commands to sender allowlists across Discord/Slack/Telegram/Signal/iMessage/WhatsApp (+ plugin channels like Matrix/Teams) and add stable `clawdbot security audit` checkIds for Slack/Discord command allowlists. +- CLI: speed up `clawdbot sandbox-explain` by avoiding heavy plugin imports when normalizing channel ids. - 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/extensions/matrix/src/matrix/monitor/index.ts b/extensions/matrix/src/matrix/monitor/index.ts index e79b337ca..dc97c2f5f 100644 --- a/extensions/matrix/src/matrix/monitor/index.ts +++ b/extensions/matrix/src/matrix/monitor/index.ts @@ -15,6 +15,7 @@ import { } from "../../../../../src/auto-reply/reply/mentions.js"; import { createReplyDispatcherWithTyping } from "../../../../../src/auto-reply/reply/reply-dispatcher.js"; import type { ReplyPayload } from "../../../../../src/auto-reply/types.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../../../../src/channels/command-gating.js"; import { loadConfig } from "../../../../../src/config/config.js"; import { resolveStorePath, updateLastRoute } from "../../../../../src/config/sessions.js"; import { danger, logVerbose, shouldLogVerbose } from "../../../../../src/globals.js"; @@ -294,17 +295,26 @@ export async function monitorMatrixProvider(opts: MonitorMatrixOpts = {}): Promi text: bodyText, mentionRegexes, }); - const commandAuthorized = - (!allowlistOnly && effectiveAllowFrom.length === 0) || - resolveMatrixAllowListMatches({ - allowList: effectiveAllowFrom, - userId: senderId, - userName: senderName, - }); const allowTextCommands = shouldHandleTextCommands({ cfg, surface: "matrix", }); + const useAccessGroups = cfg.commands?.useAccessGroups !== false; + const senderAllowedForCommands = resolveMatrixAllowListMatches({ + allowList: effectiveAllowFrom, + userId: senderId, + userName: senderName, + }); + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands }, + ], + }); + if (isRoom && allowTextCommands && hasControlCommand(bodyText, cfg) && !commandAuthorized) { + logVerbose(`matrix: drop control command from unauthorized sender ${senderId}`); + return; + } const shouldRequireMention = isRoom ? roomConfigInfo.config?.autoReply === true ? false diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index d35f69058..6c4080755 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.ts @@ -14,6 +14,7 @@ import { type HistoryEntry, } from "../../../../src/auto-reply/reply/history.js"; import { resolveMentionGating } from "../../../../src/channels/mention-gating.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../../../src/channels/command-gating.js"; import { danger, logVerbose, shouldLogVerbose } from "../../../../src/globals.js"; import { enqueueSystemEvent } from "../../../../src/infra/system-events.js"; import { @@ -125,11 +126,14 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { const senderName = from.name ?? from.id; const senderId = from.aadObjectId ?? from.id; const storedAllowFrom = await readChannelAllowFromStore("msteams").catch(() => []); + const useAccessGroups = cfg.commands?.useAccessGroups !== false; // Check DM policy for direct messages. + const dmAllowFrom = msteamsCfg?.allowFrom ?? []; + const effectiveDmAllowFrom = [...dmAllowFrom.map((v) => String(v)), ...storedAllowFrom]; if (isDirectMessage && msteamsCfg) { const dmPolicy = msteamsCfg.dmPolicy ?? "pairing"; - const allowFrom = msteamsCfg.allowFrom ?? []; + const allowFrom = dmAllowFrom; if (dmPolicy === "disabled") { log.debug("dropping dm (dms disabled)"); @@ -172,13 +176,18 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { } } - if (!isDirectMessage && msteamsCfg) { - const groupPolicy = msteamsCfg.groupPolicy ?? "allowlist"; - const groupAllowFrom = - msteamsCfg.groupAllowFrom ?? - (msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0 ? msteamsCfg.allowFrom : []); - const effectiveGroupAllowFrom = [...groupAllowFrom.map((v) => String(v)), ...storedAllowFrom]; + const groupPolicy = !isDirectMessage && msteamsCfg ? (msteamsCfg.groupPolicy ?? "allowlist") : "disabled"; + const groupAllowFrom = + !isDirectMessage && msteamsCfg + ? (msteamsCfg.groupAllowFrom ?? + (msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0 ? msteamsCfg.allowFrom : [])) + : []; + const effectiveGroupAllowFrom = + !isDirectMessage && msteamsCfg + ? [...groupAllowFrom.map((v) => String(v)), ...storedAllowFrom] + : []; + if (!isDirectMessage && msteamsCfg) { if (groupPolicy === "disabled") { log.debug("dropping group message (groupPolicy: disabled)", { conversationId, @@ -209,6 +218,30 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { } } + const ownerAllowedForCommands = isMSTeamsGroupAllowed({ + groupPolicy: "allowlist", + allowFrom: effectiveDmAllowFrom, + senderId, + senderName, + }); + const groupAllowedForCommands = isMSTeamsGroupAllowed({ + groupPolicy: "allowlist", + allowFrom: effectiveGroupAllowFrom, + senderId, + senderName, + }); + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: effectiveDmAllowFrom.length > 0, allowed: ownerAllowedForCommands }, + { configured: effectiveGroupAllowFrom.length > 0, allowed: groupAllowedForCommands }, + ], + }); + if (hasControlCommand(text, cfg) && !commandAuthorized) { + logVerbose(`msteams: drop control command from unauthorized sender ${senderId}`); + return; + } + // Build conversation reference for proactive replies. const agent = activity.recipient; const teamId = activity.channelData?.team?.id; @@ -400,7 +433,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { MessageSid: activity.id, Timestamp: timestamp?.getTime() ?? Date.now(), WasMentioned: isDirectMessage || params.wasMentioned || params.implicitMention, - CommandAuthorized: true, + CommandAuthorized: commandAuthorized, OriginatingChannel: "msteams" as const, OriginatingTo: teamsTo, ...mediaPayload, diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 08fcf50e0..81d624aa7 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -1,7 +1,7 @@ import type { ChannelDock } from "../channels/dock.js"; import { getChannelDock, listChannelDocks } from "../channels/dock.js"; import type { ChannelId } from "../channels/plugins/types.js"; -import { normalizeChannelId } from "../channels/plugins/index.js"; +import { normalizeAnyChannelId } from "../channels/registry.js"; import type { ClawdbotConfig } from "../config/config.js"; import type { MsgContext } from "./templating.js"; @@ -16,15 +16,15 @@ export type CommandAuthorization = { function resolveProviderFromContext(ctx: MsgContext, cfg: ClawdbotConfig): ChannelId | undefined { const direct = - normalizeChannelId(ctx.Provider) ?? - normalizeChannelId(ctx.Surface) ?? - normalizeChannelId(ctx.OriginatingChannel); + normalizeAnyChannelId(ctx.Provider) ?? + normalizeAnyChannelId(ctx.Surface) ?? + normalizeAnyChannelId(ctx.OriginatingChannel); if (direct) return direct; const candidates = [ctx.From, ctx.To] .filter((value): value is string => Boolean(value?.trim())) .flatMap((value) => value.split(":").map((part) => part.trim())); for (const candidate of candidates) { - const normalized = normalizeChannelId(candidate); + const normalized = normalizeAnyChannelId(candidate); if (normalized) return normalized; } const configured = listChannelDocks() diff --git a/src/auto-reply/reply/abort.ts b/src/auto-reply/reply/abort.ts index 81e87b7d3..087d7ea78 100644 --- a/src/auto-reply/reply/abort.ts +++ b/src/auto-reply/reply/abort.ts @@ -119,14 +119,6 @@ export async function tryFastAbortFromMessage(params: { cfg: ClawdbotConfig; }): Promise<{ handled: boolean; aborted: boolean; stoppedSubagents?: number }> { const { ctx, cfg } = params; - const commandAuthorized = ctx.CommandAuthorized ?? true; - const auth = resolveCommandAuthorization({ - ctx, - cfg, - commandAuthorized, - }); - if (!auth.isAuthorizedSender) return { handled: false, aborted: false }; - const targetKey = resolveAbortTargetKey(ctx); const agentId = resolveSessionAgentId({ sessionKey: targetKey ?? ctx.SessionKey ?? "", @@ -140,6 +132,14 @@ export async function tryFastAbortFromMessage(params: { const abortRequested = normalized === "/stop" || isAbortTrigger(stripped); if (!abortRequested) return { handled: false, aborted: false }; + const commandAuthorized = ctx.CommandAuthorized ?? true; + const auth = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized, + }); + if (!auth.isAuthorizedSender) return { handled: false, aborted: false }; + const abortKey = targetKey ?? auth.from ?? auth.to; const requesterSessionKey = targetKey ?? ctx.SessionKey ?? abortKey; diff --git a/src/channels/command-gating.test.ts b/src/channels/command-gating.test.ts new file mode 100644 index 000000000..03918589c --- /dev/null +++ b/src/channels/command-gating.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; + +import { resolveCommandAuthorizedFromAuthorizers } from "./command-gating.js"; + +describe("resolveCommandAuthorizedFromAuthorizers", () => { + it("denies when useAccessGroups is enabled and no authorizer is configured", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: true, + authorizers: [{ configured: false, allowed: true }], + }), + ).toBe(false); + }); + + it("allows when useAccessGroups is enabled and any configured authorizer allows", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: true, + authorizers: [ + { configured: true, allowed: false }, + { configured: true, allowed: true }, + ], + }), + ).toBe(true); + }); + + it("allows when useAccessGroups is disabled (default)", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: false, + authorizers: [{ configured: true, allowed: false }], + }), + ).toBe(true); + }); + + it("honors modeWhenAccessGroupsOff=deny", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: false, + authorizers: [{ configured: false, allowed: true }], + modeWhenAccessGroupsOff: "deny", + }), + ).toBe(false); + }); + + it("honors modeWhenAccessGroupsOff=configured (allow when none configured)", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: false, + authorizers: [{ configured: false, allowed: false }], + modeWhenAccessGroupsOff: "configured", + }), + ).toBe(true); + }); + + it("honors modeWhenAccessGroupsOff=configured (enforce when configured)", () => { + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: false, + authorizers: [{ configured: true, allowed: false }], + modeWhenAccessGroupsOff: "configured", + }), + ).toBe(false); + expect( + resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: false, + authorizers: [{ configured: true, allowed: true }], + modeWhenAccessGroupsOff: "configured", + }), + ).toBe(true); + }); +}); + diff --git a/src/channels/command-gating.ts b/src/channels/command-gating.ts new file mode 100644 index 000000000..574819abc --- /dev/null +++ b/src/channels/command-gating.ts @@ -0,0 +1,24 @@ +export type CommandAuthorizer = { + configured: boolean; + allowed: boolean; +}; + +export type CommandGatingModeWhenAccessGroupsOff = "allow" | "deny" | "configured"; + +export function resolveCommandAuthorizedFromAuthorizers(params: { + useAccessGroups: boolean; + authorizers: CommandAuthorizer[]; + modeWhenAccessGroupsOff?: CommandGatingModeWhenAccessGroupsOff; +}): boolean { + const { useAccessGroups, authorizers } = params; + const mode = params.modeWhenAccessGroupsOff ?? "allow"; + if (!useAccessGroups) { + if (mode === "allow") return true; + if (mode === "deny") return false; + const anyConfigured = authorizers.some((entry) => entry.configured); + if (!anyConfigured) return true; + return authorizers.some((entry) => entry.configured && entry.allowed); + } + return authorizers.some((entry) => entry.configured && entry.allowed); +} + diff --git a/src/channels/plugins/discord.ts b/src/channels/plugins/discord.ts index 91eb7ea9a..8ed09e9e0 100644 --- a/src/channels/plugins/discord.ts +++ b/src/channels/plugins/discord.ts @@ -14,7 +14,6 @@ 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"; @@ -120,7 +119,7 @@ export const discordPlugin: ChannelPlugin = { normalizeEntry: (raw) => raw.replace(/^(discord|user):/i, "").replace(/^<@!?(\d+)>$/, "$1"), }; }, - collectWarnings: ({ cfg, account }) => { + collectWarnings: ({ account }) => { const warnings: string[] = []; const groupPolicy = account.config.groupPolicy ?? "allowlist"; const guildEntries = account.config.guilds ?? {}; @@ -139,33 +138,6 @@ export const discordPlugin: ChannelPlugin = { } } - 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; }, }, diff --git a/src/channels/plugins/slack.ts b/src/channels/plugins/slack.ts index 5ebdf7d81..6efdf6429 100644 --- a/src/channels/plugins/slack.ts +++ b/src/channels/plugins/slack.ts @@ -13,7 +13,6 @@ 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,13 +134,11 @@ export const slackPlugin: ChannelPlugin = { normalizeEntry: (raw) => raw.replace(/^(slack|user):/i, ""), }; }, - collectWarnings: ({ cfg, account }) => { + collectWarnings: ({ account }) => { const warnings: string[] = []; const groupPolicy = account.config.groupPolicy ?? "allowlist"; const channelAllowlistConfigured = Boolean(account.config.channels) && Object.keys(account.config.channels ?? {}).length > 0; - const roomAccessPossible = - groupPolicy === "open" || (groupPolicy === "allowlist" && channelAllowlistConfigured); if (groupPolicy === "open") { if (channelAllowlistConfigured) { @@ -155,30 +152,6 @@ export const slackPlugin: ChannelPlugin = { } } - 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; }, }, diff --git a/src/channels/registry.ts b/src/channels/registry.ts index dffd15f60..9324a31f4 100644 --- a/src/channels/registry.ts +++ b/src/channels/registry.ts @@ -1,4 +1,6 @@ import type { ChannelMeta } from "./plugins/types.js"; +import type { ChannelId } from "./plugins/types.js"; +import { getActivePluginRegistry } from "../plugins/runtime.js"; // Channel docking: add new channels here (order + meta + aliases), then // register the plugin in src/channels/plugins/index.ts and keep protocol IDs in sync. @@ -111,6 +113,29 @@ export function normalizeChannelId(raw?: string | null): ChatChannelId | null { return normalizeChatChannelId(raw); } +// Normalizes core chat channels plus any *already-loaded* plugin channels. +// +// Keep this light: we do not import core channel plugins here (those are "heavy" and can pull in +// monitors, web login, etc). If plugins are not loaded (e.g. in many tests), only core channel IDs +// resolve. +export function normalizeAnyChannelId(raw?: string | null): ChannelId | null { + const core = normalizeChatChannelId(raw); + if (core) return core; + + const key = normalizeChannelKey(raw); + if (!key) return null; + + const registry = getActivePluginRegistry(); + if (!registry) return null; + + const hit = registry.channels.find((entry) => { + const id = String(entry.plugin.id ?? "").trim().toLowerCase(); + if (id && id === key) return true; + return (entry.plugin.meta.aliases ?? []).some((alias) => alias.trim().toLowerCase() === key); + }); + return (hit?.plugin.id as ChannelId | undefined) ?? null; +} + export function formatChannelPrimerLine(meta: ChatChannelMeta): string { return `${meta.label}: ${meta.blurb}`; } diff --git a/src/commands/sandbox-explain.ts b/src/commands/sandbox-explain.ts index 178e00f78..daaa9e483 100644 --- a/src/commands/sandbox-explain.ts +++ b/src/commands/sandbox-explain.ts @@ -3,7 +3,7 @@ import { resolveSandboxConfigForAgent, resolveSandboxToolPolicyForAgent, } from "../agents/sandbox.js"; -import { normalizeChannelId } from "../channels/plugins/index.js"; +import { normalizeAnyChannelId } from "../channels/registry.js"; import type { ClawdbotConfig } from "../config/config.js"; import { loadConfig } from "../config/config.js"; import { @@ -67,7 +67,7 @@ function inferProviderFromSessionKey(params: { const candidate = parts[0]?.trim().toLowerCase(); if (!candidate) return undefined; if (candidate === INTERNAL_MESSAGE_CHANNEL) return INTERNAL_MESSAGE_CHANNEL; - return normalizeChannelId(candidate) ?? undefined; + return normalizeAnyChannelId(candidate) ?? undefined; } function resolveActiveChannel(params: { @@ -98,7 +98,7 @@ function resolveActiveChannel(params: { .trim() .toLowerCase(); if (candidate === INTERNAL_MESSAGE_CHANNEL) return INTERNAL_MESSAGE_CHANNEL; - const normalized = normalizeChannelId(candidate); + const normalized = normalizeAnyChannelId(candidate); if (normalized) return normalized; return inferProviderFromSessionKey({ cfg: params.cfg, diff --git a/src/discord/monitor/message-handler.preflight.ts b/src/discord/monitor/message-handler.preflight.ts index 7d4ef2efc..ad8d2afc7 100644 --- a/src/discord/monitor/message-handler.preflight.ts +++ b/src/discord/monitor/message-handler.preflight.ts @@ -16,6 +16,7 @@ import { import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { resolveMentionGating } from "../../channels/mention-gating.js"; import { sendMessageDiscord } from "../send.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { allowListMatches, isDiscordGroupAllowedByPolicy, @@ -318,30 +319,38 @@ export async function preflightDiscordMessage( surface: "discord", }); - if (!isDirectMessage) { - const ownerAllowList = normalizeDiscordAllowList(params.allowFrom, ["discord:", "user:"]); - const ownerOk = ownerAllowList - ? allowListMatches(ownerAllowList, { + 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; + 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; + const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; + commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: ownerAllowList != null, allowed: ownerOk }, + { configured: Array.isArray(channelUsers) && channelUsers.length > 0, allowed: usersOk }, + ], + modeWhenAccessGroupsOff: "configured", + }); - if (allowTextCommands && hasControlCommand(baseText, params.cfg) && !commandAuthorized) { - logVerbose(`Blocked discord control command from unauthorized sender ${author.id}`); - return null; + if (allowTextCommands && hasControlCommand(baseText, params.cfg) && !commandAuthorized) { + logVerbose(`Blocked discord control command from unauthorized sender ${author.id}`); + return null; } } diff --git a/src/discord/monitor/native-command.ts b/src/discord/monitor/native-command.ts index f89def249..bce122345 100644 --- a/src/discord/monitor/native-command.ts +++ b/src/discord/monitor/native-command.ts @@ -41,6 +41,7 @@ import { import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { loadWebMedia } from "../../web/media.js"; import { chunkDiscordText } from "../chunk.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { allowListMatches, isDiscordGroupAllowedByPolicy, @@ -529,7 +530,17 @@ async function dispatchDiscordCommandInteraction(params: { userTag: formatDiscordUserTag(user), }) : false; - commandAuthorized = useAccessGroups ? ownerOk || userOk : hasUserAllowlist ? userOk : true; + const authorizers = useAccessGroups + ? [ + { configured: ownerAllowList != null, allowed: ownerOk }, + { configured: hasUserAllowlist, allowed: userOk }, + ] + : [{ configured: hasUserAllowlist, allowed: userOk }]; + commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers, + modeWhenAccessGroupsOff: "configured", + }); if (!commandAuthorized) { await respond("You are not authorized to use this command.", { ephemeral: true }); return; diff --git a/src/imessage/monitor/monitor-provider.ts b/src/imessage/monitor/monitor-provider.ts index eb0c77f8d..099b0779e 100644 --- a/src/imessage/monitor/monitor-provider.ts +++ b/src/imessage/monitor/monitor-provider.ts @@ -43,6 +43,7 @@ import { } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { truncateUtf16Safe } from "../../utils.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { resolveIMessageAccount } from "../accounts.js"; import { createIMessageRpcClient } from "../client.js"; import { probeIMessage } from "../probe.js"; @@ -322,8 +323,19 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P overrideOrder: "before-config", }); const canDetectMention = mentionRegexes.length > 0; - const commandAuthorized = isGroup - ? effectiveGroupAllowFrom.length > 0 + const useAccessGroups = cfg.commands?.useAccessGroups !== false; + const ownerAllowedForCommands = + effectiveDmAllowFrom.length > 0 + ? isAllowedIMessageSender({ + allowFrom: effectiveDmAllowFrom, + sender, + chatId: chatId ?? undefined, + chatGuid, + chatIdentifier, + }) + : false; + const groupAllowedForCommands = + effectiveGroupAllowFrom.length > 0 ? isAllowedIMessageSender({ allowFrom: effectiveGroupAllowFrom, sender, @@ -331,8 +343,20 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P chatGuid, chatIdentifier, }) - : true + : false; + const commandAuthorized = isGroup + ? resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: effectiveDmAllowFrom.length > 0, allowed: ownerAllowedForCommands }, + { configured: effectiveGroupAllowFrom.length > 0, allowed: groupAllowedForCommands }, + ], + }) : dmAuthorized; + if (isGroup && hasControlCommand(messageText, cfg) && !commandAuthorized) { + logVerbose(`imessage: drop control command from unauthorized sender ${sender}`); + return; + } const shouldBypassMention = isGroup && requireMention && diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index b9d230d10..85701403d 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -226,56 +226,171 @@ 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 prevStateDir = process.env.CLAWDBOT_STATE_DIR; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-discord-")); + process.env.CLAWDBOT_STATE_DIR = tmp; + await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 }); + try { + 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 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"); + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.discord.commands.native.no_allowlists", + severity: "warn", + }), + ]), + ); + } finally { + if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = prevStateDir; + } + }); + + it("flags Discord slash commands when access-group enforcement is disabled and no users allowlist exists", async () => { + const prevStateDir = process.env.CLAWDBOT_STATE_DIR; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-discord-open-")); + process.env.CLAWDBOT_STATE_DIR = tmp; + await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 }); + try { + const cfg: ClawdbotConfig = { + commands: { useAccessGroups: false }, + 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], + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.discord.commands.native.unrestricted", + severity: "critical", + }), + ]), + ); + } finally { + if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = prevStateDir; + } }); 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 prevStateDir = process.env.CLAWDBOT_STATE_DIR; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-slack-")); + process.env.CLAWDBOT_STATE_DIR = tmp; + await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 }); + try { + 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 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"); + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.slack.commands.slash.no_allowlists", + severity: "warn", + }), + ]), + ); + } finally { + if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = prevStateDir; + } + }); + + it("flags Slack slash commands when access-group enforcement is disabled", async () => { + const prevStateDir = process.env.CLAWDBOT_STATE_DIR; + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-slack-open-")); + process.env.CLAWDBOT_STATE_DIR = tmp; + await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 }); + try { + const cfg: ClawdbotConfig = { + commands: { useAccessGroups: false }, + 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], + }); + + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.slack.commands.slash.useAccessGroups_off", + severity: "critical", + }), + ]), + ); + } finally { + if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = prevStateDir; + } }); it("flags Telegram group commands without a sender allowlist", async () => { diff --git a/src/security/audit.ts b/src/security/audit.ts index 8d6d3b445..17c130e1d 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -20,7 +20,7 @@ import { readConfigSnapshotForAudit, } from "./audit-extra.js"; import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; -import { resolveNativeSkillsEnabled } from "../config/commands.js"; +import { resolveNativeCommandsEnabled, resolveNativeSkillsEnabled } from "../config/commands.js"; import { formatOctal, isGroupReadable, @@ -381,6 +381,13 @@ async function collectChannelSecurityFindings(params: { }): Promise { const findings: SecurityAuditFinding[] = []; + const coerceNativeSetting = (value: unknown): boolean | "auto" | undefined => { + if (value === true) return true; + if (value === false) return false; + if (value === "auto") return "auto"; + return undefined; + }; + const warnDmPolicy = async (input: { label: string; provider: ChannelId; @@ -465,6 +472,140 @@ async function collectChannelSecurityFindings(params: { : true; if (!configured) continue; + if (plugin.id === "discord") { + const discordCfg = + (account as { config?: Record } | null)?.config ?? ({} as Record< + string, + unknown + >); + const nativeEnabled = resolveNativeCommandsEnabled({ + providerId: "discord", + providerSetting: coerceNativeSetting( + (discordCfg.commands as { native?: unknown } | undefined)?.native, + ), + globalSetting: params.cfg.commands?.native, + }); + const nativeSkillsEnabled = resolveNativeSkillsEnabled({ + providerId: "discord", + providerSetting: coerceNativeSetting( + (discordCfg.commands as { nativeSkills?: unknown } | undefined)?.nativeSkills, + ), + globalSetting: params.cfg.commands?.nativeSkills, + }); + const slashEnabled = nativeEnabled || nativeSkillsEnabled; + if (slashEnabled) { + const groupPolicy = (discordCfg.groupPolicy as string | undefined) ?? "allowlist"; + const guildEntries = (discordCfg.guilds as Record | undefined) ?? {}; + const guildsConfigured = Object.keys(guildEntries).length > 0; + const hasAnyUserAllowlist = Object.values(guildEntries).some((guild) => { + if (!guild || typeof guild !== "object") return false; + const g = guild as Record; + if (Array.isArray(g.users) && g.users.length > 0) return true; + const channels = g.channels; + if (!channels || typeof channels !== "object") return false; + return Object.values(channels as Record).some((channel) => { + if (!channel || typeof channel !== "object") return false; + const c = channel as Record; + return Array.isArray(c.users) && c.users.length > 0; + }); + }); + const dmAllowFromRaw = (discordCfg.dm as { allowFrom?: unknown } | undefined)?.allowFrom; + const dmAllowFrom = Array.isArray(dmAllowFromRaw) ? dmAllowFromRaw : []; + const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []); + const ownerAllowFromConfigured = + normalizeAllowFromList([...dmAllowFrom, ...storeAllowFrom]).length > 0; + + const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; + if (!useAccessGroups && groupPolicy !== "disabled" && guildsConfigured && !hasAnyUserAllowlist) { + findings.push({ + checkId: "channels.discord.commands.native.unrestricted", + severity: "critical", + title: "Discord slash commands are unrestricted", + detail: + 'commands.useAccessGroups=false disables sender allowlists for Discord slash commands unless a per-guild/channel users allowlist is configured; with no users allowlist, any user in allowed guild channels can invoke /… commands.', + remediation: + 'Set commands.useAccessGroups=true (recommended), or configure channels.discord.guilds..users (or channels.discord.guilds..channels..users).', + }); + } else if ( + useAccessGroups && + groupPolicy !== "disabled" && + guildsConfigured && + !ownerAllowFromConfigured && + !hasAnyUserAllowlist + ) { + findings.push({ + checkId: "channels.discord.commands.native.no_allowlists", + severity: "warn", + title: "Discord slash commands have no allowlists", + detail: + "Discord slash commands are enabled, but neither an owner allowFrom list nor any per-guild/channel users allowlist is configured; /… commands will be rejected for everyone.", + remediation: + 'Add your user id to channels.discord.dm.allowFrom (or approve yourself via pairing), or configure channels.discord.guilds..users.', + }); + } + } + } + + if (plugin.id === "slack") { + const slackCfg = + (account as { config?: Record; dm?: Record } | null) + ?.config ?? ({} as Record); + const nativeEnabled = resolveNativeCommandsEnabled({ + providerId: "slack", + providerSetting: coerceNativeSetting( + (slackCfg.commands as { native?: unknown } | undefined)?.native, + ), + globalSetting: params.cfg.commands?.native, + }); + const nativeSkillsEnabled = resolveNativeSkillsEnabled({ + providerId: "slack", + providerSetting: coerceNativeSetting( + (slackCfg.commands as { nativeSkills?: unknown } | undefined)?.nativeSkills, + ), + globalSetting: params.cfg.commands?.nativeSkills, + }); + const slashCommandEnabled = + nativeEnabled || + nativeSkillsEnabled || + ((slackCfg.slashCommand as { enabled?: unknown } | undefined)?.enabled === true); + if (slashCommandEnabled) { + const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; + if (!useAccessGroups) { + findings.push({ + checkId: "channels.slack.commands.slash.useAccessGroups_off", + severity: "critical", + title: "Slack slash commands bypass access groups", + detail: + "Slack slash/native commands are enabled while commands.useAccessGroups=false; this can allow unrestricted /… command execution from channels/users you didn't explicitly authorize.", + remediation: "Set commands.useAccessGroups=true (recommended).", + }); + } else { + const dmAllowFromRaw = (account as { dm?: { allowFrom?: unknown } } | null)?.dm?.allowFrom; + const dmAllowFrom = Array.isArray(dmAllowFromRaw) ? dmAllowFromRaw : []; + const storeAllowFrom = await readChannelAllowFromStore("slack").catch(() => []); + const ownerAllowFromConfigured = + normalizeAllowFromList([...dmAllowFrom, ...storeAllowFrom]).length > 0; + const channels = (slackCfg.channels as Record | undefined) ?? {}; + const hasAnyChannelUsersAllowlist = Object.values(channels).some((value) => { + if (!value || typeof value !== "object") return false; + const channel = value as Record; + return Array.isArray(channel.users) && channel.users.length > 0; + }); + if (!ownerAllowFromConfigured && !hasAnyChannelUsersAllowlist) { + findings.push({ + checkId: "channels.slack.commands.slash.no_allowlists", + severity: "warn", + title: "Slack slash commands have no allowlists", + detail: + "Slack slash/native commands are enabled, but neither an owner allowFrom list nor any channels..users allowlist is configured; /… commands will be rejected for everyone.", + remediation: + "Approve yourself via pairing (recommended), or set channels.slack.dm.allowFrom and/or channels.slack.channels..users.", + }); + } + } + } + } + const dmPolicy = plugin.security.resolveDmPolicy?.({ cfg: params.cfg, accountId: defaultAccountId, diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts index 218268215..754fd3584 100644 --- a/src/signal/monitor/event-handler.ts +++ b/src/signal/monitor/event-handler.ts @@ -31,6 +31,7 @@ import { } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { normalizeE164 } from "../../utils.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { formatSignalPairingIdLine, formatSignalSenderDisplay, @@ -400,11 +401,22 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { } } + const useAccessGroups = deps.cfg.commands?.useAccessGroups !== false; + const ownerAllowedForCommands = isSignalSenderAllowed(sender, effectiveDmAllow); + const groupAllowedForCommands = isSignalSenderAllowed(sender, effectiveGroupAllow); const commandAuthorized = isGroup - ? effectiveGroupAllow.length > 0 - ? isSignalSenderAllowed(sender, effectiveGroupAllow) - : true + ? resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: effectiveDmAllow.length > 0, allowed: ownerAllowedForCommands }, + { configured: effectiveGroupAllow.length > 0, allowed: groupAllowedForCommands }, + ], + }) : dmAllowed; + if (isGroup && hasControlCommand(messageText, deps.cfg) && !commandAuthorized) { + logVerbose(`signal: drop control command from unauthorized sender ${senderDisplay}`); + return; + } let mediaPath: string | undefined; let mediaType: string | undefined; diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 322776ccf..6ba4b2b1d 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -19,6 +19,7 @@ import { resolveAgentRoute } from "../../../routing/resolve-route.js"; import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; import { resolveMentionGating } from "../../../channels/mention-gating.js"; import { resolveConversationLabel } from "../../../channels/conversation-label.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../../channels/command-gating.js"; import type { ResolvedSlackAccount } from "../../accounts.js"; import { reactSlackMessage } from "../../actions.js"; @@ -238,7 +239,13 @@ export async function prepareSlackMessage(params: { userName: senderName, }) : false; - const commandAuthorized = ownerAuthorized || channelCommandAuthorized; + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: ctx.useAccessGroups, + authorizers: [ + { configured: allowFromLower.length > 0, allowed: ownerAuthorized }, + { configured: channelUsersAllowlistConfigured, allowed: channelCommandAuthorized }, + ], + }); if ( allowTextCommands && diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index dc09b5e1e..b84da205d 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -20,6 +20,7 @@ import { } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { resolveConversationLabel } from "../../channels/conversation-label.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import type { ResolvedSlackAccount } from "../accounts.js"; @@ -293,15 +294,21 @@ export function registerSlackMonitorSlashCommands(params: { 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; + commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: ctx.useAccessGroups, + authorizers: [ + { configured: effectiveAllowFromLower.length > 0, allowed: ownerAllowed }, + { configured: channelUsersAllowlistConfigured, allowed: channelUserAllowed }, + ], + }); + if (ctx.useAccessGroups && !commandAuthorized) { + await respond({ + text: "You are not authorized to use this command.", + response_type: "ephemeral", + }); + return; + } } if (commandDefinition && supportsInteractiveArgMenus) { diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index dc614fc16..5d068df4c 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -1,4 +1,5 @@ -// @ts-nocheck +import type { Bot } from "grammy"; + import { resolveAckReaction } from "../agents/identity.js"; import { hasControlCommand } from "../auto-reply/command-detection.js"; import { normalizeCommandBody } from "../auto-reply/commands-registry.js"; @@ -6,15 +7,19 @@ import { formatInboundEnvelope } from "../auto-reply/envelope.js"; import { buildPendingHistoryContextFromMap, recordPendingHistoryEntry, + type HistoryEntry, } from "../auto-reply/reply/history.js"; import { finalizeInboundContext } from "../auto-reply/reply/inbound-context.js"; import { buildMentionRegexes, matchesMentionPatterns } from "../auto-reply/reply/mentions.js"; import { formatLocationText, toLocationContext } from "../channels/location.js"; import { resolveStorePath, updateLastRoute } from "../config/sessions.js"; +import type { ClawdbotConfig } from "../config/config.js"; +import type { DmPolicy, TelegramGroupConfig, TelegramTopicConfig } from "../config/types.js"; import { logVerbose, shouldLogVerbose } from "../globals.js"; import { recordChannelActivity } from "../infra/channel-activity.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; import { resolveMentionGating } from "../channels/mention-gating.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../channels/command-gating.js"; import { buildGroupLabel, buildSenderLabel, @@ -29,6 +34,52 @@ import { } from "./bot/helpers.js"; import { firstDefined, isSenderAllowed, normalizeAllowFrom } from "./bot-access.js"; import { upsertTelegramPairingRequest } from "./pairing-store.js"; +import type { TelegramContext } from "./bot/types.js"; + +type TelegramMediaRef = { path: string; contentType?: string }; + +type TelegramMessageContextOptions = { + forceWasMentioned?: boolean; + messageIdOverride?: string; +}; + +type TelegramLogger = { + info: (obj: Record, msg: string) => void; +}; + +type ResolveTelegramGroupConfig = ( + chatId: string | number, + messageThreadId?: number, +) => { groupConfig?: TelegramGroupConfig; topicConfig?: TelegramTopicConfig }; + +type ResolveGroupActivation = (params: { + chatId: string | number; + agentId?: string; + messageThreadId?: number; + sessionKey?: string; +}) => boolean | undefined; + +type ResolveGroupRequireMention = (chatId: string | number) => boolean; + +type BuildTelegramMessageContextParams = { + primaryCtx: TelegramContext; + allMedia: TelegramMediaRef[]; + storeAllowFrom: string[]; + options?: TelegramMessageContextOptions; + bot: Bot; + cfg: ClawdbotConfig; + account: { accountId: string }; + historyLimit: number; + groupHistories: Map; + dmPolicy: DmPolicy; + allowFrom?: Array; + groupAllowFrom?: Array; + ackReactionScope: "off" | "group-mentions" | "group-all" | "direct" | "all"; + logger: TelegramLogger; + resolveGroupActivation: ResolveGroupActivation; + resolveGroupRequireMention: ResolveGroupRequireMention; + resolveTelegramGroupConfig: ResolveTelegramGroupConfig; +}; export const buildTelegramMessageContext = async ({ primaryCtx, @@ -48,7 +99,7 @@ export const buildTelegramMessageContext = async ({ resolveGroupActivation, resolveGroupRequireMention, resolveTelegramGroupConfig, -}) => { +}: BuildTelegramMessageContextParams) => { const msg = primaryCtx.message; recordChannelActivity({ channel: "telegram", @@ -205,8 +256,10 @@ export const buildTelegramMessageContext = async ({ senderUsername, }); const useAccessGroups = cfg.commands?.useAccessGroups !== false; - const commandAuthorized = - useAccessGroups && !allowForCommands.hasEntries ? false : senderAllowedForCommands; + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [{ configured: allowForCommands.hasEntries, allowed: senderAllowedForCommands }], + }); const historyKey = isGroup ? buildTelegramGroupPeerId(chatId, resolvedThreadId) : undefined; let placeholder = ""; @@ -227,7 +280,7 @@ export const buildTelegramMessageContext = async ({ bodyText = `${allMedia.length > 1 ? ` (${allMedia.length} images)` : ""}`; } const computedWasMentioned = - (Boolean(botUsername) && hasBotMention(msg, botUsername)) || + (botUsername ? hasBotMention(msg, botUsername) : false) || matchesMentionPatterns(msg.text ?? msg.caption ?? "", mentionRegexes); const wasMentioned = options?.forceWasMentioned === true ? true : computedWasMentioned; const hasAnyMention = (msg.entities ?? msg.caption_entities ?? []).some( diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index 4499e2b29..413fa95af 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -1,4 +1,4 @@ -// @ts-nocheck +import type { Bot, Context } from "grammy"; import { resolveEffectiveMessagesConfig } from "../agents/identity.js"; import { @@ -16,6 +16,16 @@ import { dispatchReplyWithBufferedBlockDispatcher } from "../auto-reply/reply/pr import { finalizeInboundContext } from "../auto-reply/reply/inbound-context.js"; import { danger, logVerbose } from "../globals.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; +import { resolveCommandAuthorizedFromAuthorizers } from "../channels/command-gating.js"; +import type { ChannelGroupPolicy } from "../config/group-policy.js"; +import type { + ReplyToMode, + TelegramAccountConfig, + TelegramGroupConfig, + TelegramTopicConfig, +} from "../config/types.js"; +import type { ClawdbotConfig } from "../config/config.js"; +import type { RuntimeEnv } from "../runtime.js"; import { deliverReplies } from "./bot/delivery.js"; import { buildInlineKeyboard } from "./send.js"; import { @@ -27,6 +37,31 @@ import { import { firstDefined, isSenderAllowed, normalizeAllowFrom } from "./bot-access.js"; import { readTelegramAllowFromStore } from "./pairing-store.js"; +type TelegramNativeCommandContext = Context & { match?: string }; + +type RegisterTelegramNativeCommandsParams = { + bot: Bot; + cfg: ClawdbotConfig; + runtime: RuntimeEnv; + accountId: string; + telegramCfg: TelegramAccountConfig; + allowFrom?: Array; + groupAllowFrom?: Array; + replyToMode: ReplyToMode; + textLimit: number; + useAccessGroups: boolean; + nativeEnabled: boolean; + nativeSkillsEnabled: boolean; + nativeDisabledExplicit: boolean; + resolveGroupPolicy: (chatId: string | number) => ChannelGroupPolicy; + resolveTelegramGroupConfig: ( + chatId: string | number, + messageThreadId?: number, + ) => { groupConfig?: TelegramGroupConfig; topicConfig?: TelegramTopicConfig }; + shouldSkipUpdate: (ctx: unknown) => boolean; + opts: { token: string }; +}; + export const registerTelegramNativeCommands = ({ bot, cfg, @@ -45,7 +80,7 @@ export const registerTelegramNativeCommands = ({ resolveTelegramGroupConfig, shouldSkipUpdate, opts, -}) => { +}: RegisterTelegramNativeCommandsParams) => { const skillCommands = nativeEnabled && nativeSkillsEnabled ? listSkillCommandsForAgents({ cfg }) : []; const nativeCommands = nativeEnabled @@ -74,24 +109,15 @@ export const registerTelegramNativeCommands = ({ ]; if (allCommands.length > 0) { - const api = bot.api as unknown as { - setMyCommands?: ( - commands: Array<{ command: string; description: string }>, - ) => Promise; - }; - if (typeof api.setMyCommands === "function") { - api.setMyCommands(allCommands).catch((err) => { + bot.api.setMyCommands(allCommands).catch((err) => { runtime.error?.(danger(`telegram setMyCommands failed: ${String(err)}`)); }); - } else { - logVerbose("telegram: setMyCommands unavailable; skipping registration"); - } if (typeof (bot as unknown as { command?: unknown }).command !== "function") { logVerbose("telegram: bot.command unavailable; skipping native handlers"); } else { for (const command of nativeCommands) { - bot.command(command.name, async (ctx) => { + bot.command(command.name, async (ctx: TelegramNativeCommandContext) => { const msg = ctx.message; if (!msg) return; if (shouldSkipUpdate(ctx)) return; @@ -172,18 +198,17 @@ export const registerTelegramNativeCommands = ({ : []; const senderId = msg.from?.id ? String(msg.from.id) : ""; const senderUsername = msg.from?.username ?? ""; - 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; + const dmAllow = normalizeAllowFrom(allowFromList); + const senderAllowed = isSenderAllowed({ + allow: dmAllow, + senderId, + senderUsername, + }); + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [{ configured: dmAllow.hasEntries, allowed: senderAllowed }], + modeWhenAccessGroupsOff: "configured", + }); if (!commandAuthorized) { await bot.api.sendMessage(chatId, "You are not authorized to use this command."); return; @@ -208,7 +233,7 @@ export const registerTelegramNativeCommands = ({ cfg, }) : null; - if (menu) { + if (menu && commandDefinition) { const title = menu.title ?? `Choose ${menu.arg.description || menu.arg.name} for /${commandDefinition.nativeName ?? commandDefinition.key}.`; @@ -316,15 +341,8 @@ export const registerTelegramNativeCommands = ({ } } } else if (nativeDisabledExplicit) { - const api = bot.api as unknown as { - setMyCommands?: (commands: []) => Promise; - }; - if (typeof api.setMyCommands === "function") { - api.setMyCommands([]).catch((err) => { - runtime.error?.(danger(`telegram clear commands failed: ${String(err)}`)); - }); - } else { - logVerbose("telegram: setMyCommands unavailable; skipping clear"); - } + bot.api.setMyCommands([]).catch((err) => { + runtime.error?.(danger(`telegram clear commands failed: ${String(err)}`)); + }); } }; diff --git a/src/web/auto-reply/monitor/process-message.ts b/src/web/auto-reply/monitor/process-message.ts index 731c8e57b..5b70b64f8 100644 --- a/src/web/auto-reply/monitor/process-message.ts +++ b/src/web/auto-reply/monitor/process-message.ts @@ -16,11 +16,13 @@ import { import { dispatchReplyWithBufferedBlockDispatcher } from "../../../auto-reply/reply/provider-dispatcher.js"; import type { getReplyFromConfig } from "../../../auto-reply/reply.js"; import type { ReplyPayload } from "../../../auto-reply/types.js"; +import { isControlCommandMessage } from "../../../auto-reply/command-detection.js"; import { finalizeInboundContext } from "../../../auto-reply/reply/inbound-context.js"; import { toLocationContext } from "../../../channels/location.js"; import type { loadConfig } from "../../../config/config.js"; import { logVerbose, shouldLogVerbose } from "../../../globals.js"; import type { getChildLogger } from "../../../logging.js"; +import { readChannelAllowFromStore } from "../../../pairing/pairing-store.js"; import type { resolveAgentRoute } from "../../../routing/resolve-route.js"; import { jidToE164, normalizeE164 } from "../../../utils.js"; import { newConnectionId } from "../../reconnect.js"; @@ -42,6 +44,47 @@ export type GroupHistoryEntry = { senderJid?: string; }; +function normalizeAllowFromE164(values: Array | undefined): string[] { + const list = Array.isArray(values) ? values : []; + return list + .map((entry) => String(entry).trim()) + .filter((entry) => entry && entry !== "*") + .map((entry) => normalizeE164(entry)) + .filter((entry): entry is string => Boolean(entry)); +} + +async function resolveWhatsAppCommandAuthorized(params: { + cfg: ReturnType; + msg: WebInboundMsg; +}): Promise { + const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; + if (!useAccessGroups) return true; + + const isGroup = params.msg.chatType === "group"; + const senderE164 = normalizeE164( + isGroup ? (params.msg.senderE164 ?? "") : (params.msg.senderE164 ?? params.msg.from ?? ""), + ); + if (!senderE164) return false; + + const configuredAllowFrom = params.cfg.channels?.whatsapp?.allowFrom ?? []; + const configuredGroupAllowFrom = + params.cfg.channels?.whatsapp?.groupAllowFrom ?? + (configuredAllowFrom.length > 0 ? configuredAllowFrom : undefined); + + if (isGroup) { + if (!configuredGroupAllowFrom || configuredGroupAllowFrom.length === 0) return false; + if (configuredGroupAllowFrom.some((v) => String(v).trim() === "*")) return true; + return normalizeAllowFromE164(configuredGroupAllowFrom).includes(senderE164); + } + + const storeAllowFrom = await readChannelAllowFromStore("whatsapp").catch(() => []); + const combinedAllowFrom = Array.from(new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom])); + const allowFrom = + combinedAllowFrom.length > 0 ? combinedAllowFrom : params.msg.selfE164 ? [params.msg.selfE164] : []; + if (allowFrom.some((v) => String(v).trim() === "*")) return true; + return normalizeAllowFromE164(allowFrom).includes(senderE164); +} + export async function processMessage(params: { cfg: ReturnType; msg: WebInboundMsg; @@ -180,6 +223,9 @@ export async function processMessage(params: { const textLimit = params.maxMediaTextChunkLimit ?? resolveTextChunkLimit(params.cfg, "whatsapp"); let didLogHeartbeatStrip = false; let didSendReply = false; + const commandAuthorized = isControlCommandMessage(params.msg.body, params.cfg) + ? await resolveWhatsAppCommandAuthorized({ cfg: params.cfg, msg: params.msg }) + : undefined; const configuredResponsePrefix = params.cfg.messages?.responsePrefix; const resolvedMessages = resolveEffectiveMessagesConfig(params.cfg, params.route.agentId); const isSelfChat = @@ -224,6 +270,7 @@ export async function processMessage(params: { SenderName: params.msg.senderName, SenderId: params.msg.senderJid?.trim() || params.msg.senderE164, SenderE164: params.msg.senderE164, + CommandAuthorized: commandAuthorized, WasMentioned: params.msg.wasMentioned, ...(params.msg.location ? toLocationContext(params.msg.location) : {}), Provider: "whatsapp",