From f73dbdbaea12d0a04c55b1729af1fe961f382862 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 18 Jan 2026 01:21:27 +0000 Subject: [PATCH] refactor: unify channel config matching and gating Co-authored-by: thewilloftheshadow --- CHANGELOG.md | 1 + docs/channels/discord.md | 2 +- docs/channels/telegram.md | 1 + extensions/msteams/src/policy.test.ts | 2 + extensions/msteams/src/policy.ts | 4 + src/channels/channel-config.test.ts | 47 +++++++++- src/channels/channel-config.ts | 58 +++++++++++- src/channels/command-gating.test.ts | 25 ++++- src/channels/command-gating.ts | 16 ++++ src/channels/mention-gating.test.ts | 34 ++++++- src/channels/mention-gating.ts | 39 ++++++++ src/channels/plugins/channel-config.ts | 8 +- src/channels/plugins/index.ts | 2 + src/channels/plugins/status-issues/discord.ts | 12 +-- src/channels/plugins/status-issues/shared.ts | 24 +++++ .../plugins/status-issues/telegram.ts | 12 +-- src/discord/monitor.test.ts | 1 + src/discord/monitor/allow-list.ts | 93 +++++++++++-------- .../monitor/message-handler.preflight.ts | 30 +++--- src/slack/monitor/channel-config.test.ts | 12 +++ src/slack/monitor/channel-config.ts | 17 ++-- src/slack/monitor/message-handler/prepare.ts | 34 +++---- src/telegram/bot-message-context.ts | 33 ++++--- src/telegram/bot.test.ts | 43 +++++++++ 24 files changed, 430 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7fabfac9..d72ef81f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.clawd.bot ### Changes - Tools: allow `sessions_spawn` to override thinking level for sub-agent runs. +- Channels: unify thread/topic allowlist matching + command/mention gating helpers across core providers. - Models: add Qwen Portal OAuth provider support. (#1120) — thanks @mukhtharcm. ### Fixes diff --git a/docs/channels/discord.md b/docs/channels/discord.md index f24ad6971..a16b6ed04 100644 --- a/docs/channels/discord.md +++ b/docs/channels/discord.md @@ -175,7 +175,7 @@ Notes: - `agents.list[].groupChat.mentionPatterns` (or `messages.groupChat.mentionPatterns`) also count as mentions for guild messages. - Multi-agent override: set per-agent patterns on `agents.list[].groupChat.mentionPatterns`. - If `channels` is present, any channel not listed is denied by default. -- Threads inherit parent channel config (allowlist, `requireMention`, skills, prompts, etc.) unless you add the thread id explicitly. +- Threads inherit parent channel config (allowlist, `requireMention`, skills, prompts, etc.) unless you add the thread channel id explicitly. - Bot-authored messages are ignored by default; set `channels.discord.allowBots=true` to allow them (own messages remain filtered). - Warning: If you allow replies to other bots (`channels.discord.allowBots=true`), prevent bot-to-bot reply loops with `requireMention`, `channels.discord.guilds.*.channels..users` allowlists, and/or clear guardrails in `AGENTS.md` and `SOUL.md`. diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index be5866a22..7f655b1c9 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -152,6 +152,7 @@ By default, the bot only responds to mentions in groups (`@botname` or patterns ``` **Important:** Setting `channels.telegram.groups` creates an **allowlist** - only listed groups (or `"*"`) will be accepted. +Forum topics inherit their parent group config (allowFrom, requireMention, skills, prompts) unless you add per-topic overrides under `channels.telegram.groups..topics.`. To allow all groups with always-respond: ```json5 diff --git a/extensions/msteams/src/policy.test.ts b/extensions/msteams/src/policy.test.ts index ecd2c9dfe..0401a9a50 100644 --- a/extensions/msteams/src/policy.test.ts +++ b/extensions/msteams/src/policy.test.ts @@ -31,6 +31,8 @@ describe("msteams policy", () => { expect(res.channelConfig?.requireMention).toBe(true); expect(res.allowlistConfigured).toBe(true); expect(res.allowed).toBe(true); + expect(res.channelMatchKey).toBe("chan456"); + expect(res.channelMatchSource).toBe("direct"); }); it("returns undefined configs when teamId is missing", () => { diff --git a/extensions/msteams/src/policy.ts b/extensions/msteams/src/policy.ts index 99563befd..166103655 100644 --- a/extensions/msteams/src/policy.ts +++ b/extensions/msteams/src/policy.ts @@ -13,6 +13,8 @@ export type MSTeamsResolvedRouteConfig = { allowed: boolean; teamKey?: string; channelKey?: string; + channelMatchKey?: string; + channelMatchSource?: "direct" | "wildcard"; }; export function resolveMSTeamsRouteConfig(params: { @@ -75,6 +77,8 @@ export function resolveMSTeamsRouteConfig(params: { allowed, teamKey, channelKey, + channelMatchKey: channelKey, + channelMatchSource: channelKey ? (channelKey === "*" ? "wildcard" : "direct") : undefined, }; } diff --git a/src/channels/channel-config.test.ts b/src/channels/channel-config.test.ts index c3618c8ef..a42483404 100644 --- a/src/channels/channel-config.test.ts +++ b/src/channels/channel-config.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { buildChannelKeyCandidates, resolveChannelEntryMatch } from "./channel-config.js"; +import { + buildChannelKeyCandidates, + resolveChannelEntryMatch, + resolveChannelEntryMatchWithFallback, +} from "./channel-config.js"; describe("buildChannelKeyCandidates", () => { it("dedupes and trims keys", () => { @@ -22,3 +26,44 @@ describe("resolveChannelEntryMatch", () => { expect(match.wildcardKey).toBe("*"); }); }); + +describe("resolveChannelEntryMatchWithFallback", () => { + it("prefers direct matches over parent and wildcard", () => { + const entries = { a: { allow: true }, parent: { allow: false }, "*": { allow: false } }; + const match = resolveChannelEntryMatchWithFallback({ + entries, + keys: ["a"], + parentKeys: ["parent"], + wildcardKey: "*", + }); + expect(match.entry).toBe(entries.a); + expect(match.matchSource).toBe("direct"); + expect(match.matchKey).toBe("a"); + }); + + it("falls back to parent when direct misses", () => { + const entries = { parent: { allow: false }, "*": { allow: true } }; + const match = resolveChannelEntryMatchWithFallback({ + entries, + keys: ["missing"], + parentKeys: ["parent"], + wildcardKey: "*", + }); + expect(match.entry).toBe(entries.parent); + expect(match.matchSource).toBe("parent"); + expect(match.matchKey).toBe("parent"); + }); + + it("falls back to wildcard when no direct or parent match", () => { + const entries = { "*": { allow: true } }; + const match = resolveChannelEntryMatchWithFallback({ + entries, + keys: ["missing"], + parentKeys: ["still-missing"], + wildcardKey: "*", + }); + expect(match.entry).toBe(entries["*"]); + expect(match.matchSource).toBe("wildcard"); + expect(match.matchKey).toBe("*"); + }); +}); diff --git a/src/channels/channel-config.ts b/src/channels/channel-config.ts index 4dfb65272..d4f9f516a 100644 --- a/src/channels/channel-config.ts +++ b/src/channels/channel-config.ts @@ -1,11 +1,22 @@ +export type ChannelMatchSource = "direct" | "parent" | "wildcard"; + +export function buildChannelKeyCandidates( + ...keys: Array +): string[] { export type ChannelEntryMatch = { entry?: T; key?: string; wildcardEntry?: T; wildcardKey?: string; + parentEntry?: T; + parentKey?: string; + matchKey?: string; + matchSource?: ChannelMatchSource; }; -export function buildChannelKeyCandidates(...keys: Array): string[] { +export function buildChannelKeyCandidates( + ...keys: Array +): string[] { const seen = new Set(); const candidates: string[] = []; for (const key of keys) { @@ -37,3 +48,48 @@ export function resolveChannelEntryMatch(params: { } return match; } + +export function resolveChannelEntryMatchWithFallback(params: { + entries?: Record; + keys: string[]; + parentKeys?: string[]; + wildcardKey?: string; +}): ChannelEntryMatch { + const direct = resolveChannelEntryMatch({ + entries: params.entries, + keys: params.keys, + wildcardKey: params.wildcardKey, + }); + + if (direct.entry && direct.key) { + return { ...direct, matchKey: direct.key, matchSource: "direct" }; + } + + const parentKeys = params.parentKeys ?? []; + if (parentKeys.length > 0) { + const parent = resolveChannelEntryMatch({ entries: params.entries, keys: parentKeys }); + if (parent.entry && parent.key) { + return { + ...direct, + entry: parent.entry, + key: parent.key, + parentEntry: parent.entry, + parentKey: parent.key, + matchKey: parent.key, + matchSource: "parent", + }; + } + } + + if (direct.wildcardEntry && direct.wildcardKey) { + return { + ...direct, + entry: direct.wildcardEntry, + key: direct.wildcardKey, + matchKey: direct.wildcardKey, + matchSource: "wildcard", + }; + } + + return direct; +} diff --git a/src/channels/command-gating.test.ts b/src/channels/command-gating.test.ts index 00aea968d..6c220e2fd 100644 --- a/src/channels/command-gating.test.ts +++ b/src/channels/command-gating.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { resolveCommandAuthorizedFromAuthorizers } from "./command-gating.js"; +import { resolveCommandAuthorizedFromAuthorizers, resolveControlCommandGate } from "./command-gating.js"; describe("resolveCommandAuthorizedFromAuthorizers", () => { it("denies when useAccessGroups is enabled and no authorizer is configured", () => { @@ -70,3 +70,26 @@ describe("resolveCommandAuthorizedFromAuthorizers", () => { ).toBe(true); }); }); + +describe("resolveControlCommandGate", () => { + it("blocks control commands when unauthorized", () => { + const result = resolveControlCommandGate({ + useAccessGroups: true, + authorizers: [{ configured: true, allowed: false }], + allowTextCommands: true, + hasControlCommand: true, + }); + expect(result.commandAuthorized).toBe(false); + expect(result.shouldBlock).toBe(true); + }); + + it("does not block when control commands are disabled", () => { + const result = resolveControlCommandGate({ + useAccessGroups: true, + authorizers: [{ configured: true, allowed: false }], + allowTextCommands: false, + hasControlCommand: true, + }); + expect(result.shouldBlock).toBe(false); + }); +}); diff --git a/src/channels/command-gating.ts b/src/channels/command-gating.ts index d6690c667..803fecc87 100644 --- a/src/channels/command-gating.ts +++ b/src/channels/command-gating.ts @@ -21,3 +21,19 @@ export function resolveCommandAuthorizedFromAuthorizers(params: { } return authorizers.some((entry) => entry.configured && entry.allowed); } + +export function resolveControlCommandGate(params: { + useAccessGroups: boolean; + authorizers: CommandAuthorizer[]; + allowTextCommands: boolean; + hasControlCommand: boolean; + modeWhenAccessGroupsOff?: CommandGatingModeWhenAccessGroupsOff; +}): { commandAuthorized: boolean; shouldBlock: boolean } { + const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: params.useAccessGroups, + authorizers: params.authorizers, + modeWhenAccessGroupsOff: params.modeWhenAccessGroupsOff, + }); + const shouldBlock = params.allowTextCommands && params.hasControlCommand && !commandAuthorized; + return { commandAuthorized, shouldBlock }; +} diff --git a/src/channels/mention-gating.test.ts b/src/channels/mention-gating.test.ts index d962c4f67..5c205b312 100644 --- a/src/channels/mention-gating.test.ts +++ b/src/channels/mention-gating.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { resolveMentionGating } from "./mention-gating.js"; +import { resolveMentionGating, resolveMentionGatingWithBypass } from "./mention-gating.js"; describe("resolveMentionGating", () => { it("combines explicit, implicit, and bypass mentions", () => { @@ -36,3 +36,35 @@ describe("resolveMentionGating", () => { expect(res.shouldSkip).toBe(false); }); }); + +describe("resolveMentionGatingWithBypass", () => { + it("enables bypass when control commands are authorized", () => { + const res = resolveMentionGatingWithBypass({ + isGroup: true, + requireMention: true, + canDetectMention: true, + wasMentioned: false, + hasAnyMention: false, + allowTextCommands: true, + hasControlCommand: true, + commandAuthorized: true, + }); + expect(res.shouldBypassMention).toBe(true); + expect(res.shouldSkip).toBe(false); + }); + + it("does not bypass when control commands are not authorized", () => { + const res = resolveMentionGatingWithBypass({ + isGroup: true, + requireMention: true, + canDetectMention: true, + wasMentioned: false, + hasAnyMention: false, + allowTextCommands: true, + hasControlCommand: true, + commandAuthorized: false, + }); + expect(res.shouldBypassMention).toBe(false); + expect(res.shouldSkip).toBe(true); + }); +}); diff --git a/src/channels/mention-gating.ts b/src/channels/mention-gating.ts index a2cc5a4d8..4be89785e 100644 --- a/src/channels/mention-gating.ts +++ b/src/channels/mention-gating.ts @@ -11,6 +11,22 @@ export type MentionGateResult = { shouldSkip: boolean; }; +export type MentionGateWithBypassParams = { + isGroup: boolean; + requireMention: boolean; + canDetectMention: boolean; + wasMentioned: boolean; + implicitMention?: boolean; + hasAnyMention?: boolean; + allowTextCommands: boolean; + hasControlCommand: boolean; + commandAuthorized: boolean; +}; + +export type MentionGateWithBypassResult = MentionGateResult & { + shouldBypassMention: boolean; +}; + export function resolveMentionGating(params: MentionGateParams): MentionGateResult { const implicit = params.implicitMention === true; const bypass = params.shouldBypassMention === true; @@ -18,3 +34,26 @@ export function resolveMentionGating(params: MentionGateParams): MentionGateResu const shouldSkip = params.requireMention && params.canDetectMention && !effectiveWasMentioned; return { effectiveWasMentioned, shouldSkip }; } + +export function resolveMentionGatingWithBypass( + params: MentionGateWithBypassParams, +): MentionGateWithBypassResult { + const shouldBypassMention = + params.isGroup && + params.requireMention && + !params.wasMentioned && + !(params.hasAnyMention ?? false) && + params.allowTextCommands && + params.commandAuthorized && + params.hasControlCommand; + return { + ...resolveMentionGating({ + requireMention: params.requireMention, + canDetectMention: params.canDetectMention, + wasMentioned: params.wasMentioned, + implicitMention: params.implicitMention, + shouldBypassMention, + }), + shouldBypassMention, + }; +} diff --git a/src/channels/plugins/channel-config.ts b/src/channels/plugins/channel-config.ts index a489f9708..ae6ee2c65 100644 --- a/src/channels/plugins/channel-config.ts +++ b/src/channels/plugins/channel-config.ts @@ -1,2 +1,6 @@ -export type { ChannelEntryMatch } from "../channel-config.js"; -export { buildChannelKeyCandidates, resolveChannelEntryMatch } from "../channel-config.js"; +export type { ChannelEntryMatch, ChannelMatchSource } from "../channel-config.js"; +export { + buildChannelKeyCandidates, + resolveChannelEntryMatch, + resolveChannelEntryMatchWithFallback, +} from "../channel-config.js"; diff --git a/src/channels/plugins/index.ts b/src/channels/plugins/index.ts index 59f8d9b58..c44313df5 100644 --- a/src/channels/plugins/index.ts +++ b/src/channels/plugins/index.ts @@ -87,6 +87,8 @@ export { export { buildChannelKeyCandidates, resolveChannelEntryMatch, + resolveChannelEntryMatchWithFallback, type ChannelEntryMatch, + type ChannelMatchSource, } from "./channel-config.js"; export type { ChannelId, ChannelPlugin } from "./types.js"; diff --git a/src/channels/plugins/status-issues/discord.ts b/src/channels/plugins/status-issues/discord.ts index 7a6cb1df7..85a1e84b7 100644 --- a/src/channels/plugins/status-issues/discord.ts +++ b/src/channels/plugins/status-issues/discord.ts @@ -1,5 +1,5 @@ import type { ChannelAccountSnapshot, ChannelStatusIssue } from "../types.js"; -import { asString, isRecord } from "./shared.js"; +import { appendMatchMetadata, asString, isRecord } from "./shared.js"; type DiscordIntentSummary = { messageContent?: "enabled" | "limited" | "disabled"; @@ -128,15 +128,15 @@ export function collectDiscordStatusIssues( if (channel.ok === true) continue; const missing = channel.missing?.length ? ` missing ${channel.missing.join(", ")}` : ""; const error = channel.error ? `: ${channel.error}` : ""; - const matchMeta = - channel.matchKey || channel.matchSource - ? ` (matchKey=${channel.matchKey ?? "none"} matchSource=${channel.matchSource ?? "none"})` - : ""; + const baseMessage = `Channel ${channel.channelId} permission check failed.${missing}${error}`; issues.push({ channel: "discord", accountId, kind: "permissions", - message: `Channel ${channel.channelId} permission check failed.${missing}${error}${matchMeta}`, + message: appendMatchMetadata(baseMessage, { + matchKey: channel.matchKey, + matchSource: channel.matchSource, + }), fix: "Ensure the bot role can view + send in this channel (and that channel overrides don't deny it).", }); } diff --git a/src/channels/plugins/status-issues/shared.ts b/src/channels/plugins/status-issues/shared.ts index 59d26448b..a3e5e1fa9 100644 --- a/src/channels/plugins/status-issues/shared.ts +++ b/src/channels/plugins/status-issues/shared.ts @@ -5,3 +5,27 @@ export function asString(value: unknown): string | undefined { export function isRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } + +export function formatMatchMetadata(params: { + matchKey?: unknown; + matchSource?: unknown; +}): string | undefined { + const matchKey = + typeof params.matchKey === "string" + ? params.matchKey + : typeof params.matchKey === "number" + ? String(params.matchKey) + : undefined; + const matchSource = asString(params.matchSource); + const parts = [matchKey ? `matchKey=${matchKey}` : null, matchSource ? `matchSource=${matchSource}` : null] + .filter((entry): entry is string => Boolean(entry)); + return parts.length > 0 ? parts.join(" ") : undefined; +} + +export function appendMatchMetadata( + message: string, + params: { matchKey?: unknown; matchSource?: unknown }, +): string { + const meta = formatMatchMetadata(params); + return meta ? `${message} (${meta})` : message; +} diff --git a/src/channels/plugins/status-issues/telegram.ts b/src/channels/plugins/status-issues/telegram.ts index 30b68a987..5d340a264 100644 --- a/src/channels/plugins/status-issues/telegram.ts +++ b/src/channels/plugins/status-issues/telegram.ts @@ -1,5 +1,5 @@ import type { ChannelAccountSnapshot, ChannelStatusIssue } from "../types.js"; -import { asString, isRecord } from "./shared.js"; +import { appendMatchMetadata, asString, isRecord } from "./shared.js"; type TelegramAccountStatus = { accountId?: unknown; @@ -111,15 +111,15 @@ export function collectTelegramStatusIssues( if (group.ok === true) continue; const status = group.status ? ` status=${group.status}` : ""; const err = group.error ? `: ${group.error}` : ""; - const matchMeta = - group.matchKey || group.matchSource - ? ` (matchKey=${group.matchKey ?? "none"} matchSource=${group.matchSource ?? "none"})` - : ""; + const baseMessage = `Group ${group.chatId} not reachable by bot.${status}${err}`; issues.push({ channel: "telegram", accountId, kind: "runtime", - message: `Group ${group.chatId} not reachable by bot.${status}${err}${matchMeta}`, + message: appendMatchMetadata(baseMessage, { + matchKey: group.matchKey, + matchSource: group.matchSource, + }), fix: "Invite the bot to the group, then DM the bot once (/start) and restart the gateway.", }); } diff --git a/src/discord/monitor.test.ts b/src/discord/monitor.test.ts index 4dd2cbfba..55f0af637 100644 --- a/src/discord/monitor.test.ts +++ b/src/discord/monitor.test.ts @@ -268,6 +268,7 @@ describe("discord mention gating", () => { scope: "thread", }); expect(channelConfig?.matchSource).toBe("parent"); + expect(channelConfig?.matchKey).toBe("parent-1"); expect( resolveDiscordShouldRequireMention({ isGuildMessage: true, diff --git a/src/discord/monitor/allow-list.ts b/src/discord/monitor/allow-list.ts index 49be63db2..7e00a984e 100644 --- a/src/discord/monitor/allow-list.ts +++ b/src/discord/monitor/allow-list.ts @@ -2,7 +2,7 @@ import type { Guild, User } from "@buape/carbon"; import { buildChannelKeyCandidates, - resolveChannelEntryMatch, + resolveChannelEntryMatchWithFallback, } from "../../channels/channel-config.js"; import { formatDiscordUserTag } from "./format.js"; @@ -178,40 +178,47 @@ type DiscordChannelLookup = { slug?: string; }; type DiscordChannelScope = "channel" | "thread"; -type DiscordChannelMatch = { - entry: DiscordChannelEntry; - key: string; -}; -function resolveDiscordChannelEntry( - channels: NonNullable, +function buildDiscordChannelKeys( params: DiscordChannelLookup & { allowNameMatch?: boolean }, -): DiscordChannelMatch | null { +): string[] { const allowNameMatch = params.allowNameMatch !== false; - const keys = buildChannelKeyCandidates( + return buildChannelKeyCandidates( params.id, allowNameMatch ? params.slug : undefined, allowNameMatch ? params.name : undefined, ); - const { entry, key } = resolveChannelEntryMatch({ entries: channels, keys }); - if (!entry || !key) return null; - return { entry, key }; +} + +function resolveDiscordChannelEntryMatch( + channels: NonNullable, + params: DiscordChannelLookup & { allowNameMatch?: boolean }, + parentParams?: DiscordChannelLookup, +) { + const keys = buildDiscordChannelKeys(params); + const parentKeys = parentParams ? buildDiscordChannelKeys(parentParams) : undefined; + return resolveChannelEntryMatchWithFallback({ + entries: channels, + keys, + parentKeys, + }); } function resolveDiscordChannelConfigEntry( - match: DiscordChannelMatch, + entry: DiscordChannelEntry, + matchKey: string | undefined, matchSource: "direct" | "parent", ): DiscordChannelConfigResolved { const resolved: DiscordChannelConfigResolved = { - allowed: match.entry.allow !== false, - requireMention: match.entry.requireMention, - skills: match.entry.skills, - enabled: match.entry.enabled, - users: match.entry.users, - systemPrompt: match.entry.systemPrompt, - autoThread: match.entry.autoThread, + allowed: entry.allow !== false, + requireMention: entry.requireMention, + skills: entry.skills, + enabled: entry.enabled, + users: entry.users, + systemPrompt: entry.systemPrompt, + autoThread: entry.autoThread, }; - if (match.key) resolved.matchKey = match.key; + if (matchKey) resolved.matchKey = matchKey; resolved.matchSource = matchSource; return resolved; } @@ -225,13 +232,13 @@ export function resolveDiscordChannelConfig(params: { const { guildInfo, channelId, channelName, channelSlug } = params; const channels = guildInfo?.channels; if (!channels) return null; - const entry = resolveDiscordChannelEntry(channels, { + const match = resolveDiscordChannelEntryMatch(channels, { id: channelId, name: channelName, slug: channelSlug, }); - if (!entry) return { allowed: false }; - return resolveDiscordChannelConfigEntry(entry, "direct"); + if (!match.entry || !match.matchKey) return { allowed: false }; + return resolveDiscordChannelConfigEntry(match.entry, match.matchKey, "direct"); } export function resolveDiscordChannelConfigWithFallback(params: { @@ -256,21 +263,29 @@ export function resolveDiscordChannelConfigWithFallback(params: { } = params; const channels = guildInfo?.channels; if (!channels) return null; - const entry = resolveDiscordChannelEntry(channels, { - id: channelId, - name: channelName, - slug: channelSlug, - allowNameMatch: scope !== "thread", - }); - if (entry) return resolveDiscordChannelConfigEntry(entry, "direct"); - if (parentId || parentName || parentSlug) { - const resolvedParentSlug = parentSlug ?? (parentName ? normalizeDiscordSlug(parentName) : ""); - const parentEntry = resolveDiscordChannelEntry(channels, { - id: parentId ?? "", - name: parentName, - slug: resolvedParentSlug, - }); - if (parentEntry) return resolveDiscordChannelConfigEntry(parentEntry, "parent"); + const resolvedParentSlug = parentSlug ?? (parentName ? normalizeDiscordSlug(parentName) : ""); + const match = resolveDiscordChannelEntryMatch( + channels, + { + id: channelId, + name: channelName, + slug: channelSlug, + allowNameMatch: scope !== "thread", + }, + parentId || parentName || parentSlug + ? { + id: parentId ?? "", + name: parentName, + slug: resolvedParentSlug, + } + : undefined, + ); + if (match.entry && match.matchKey && match.matchSource) { + return resolveDiscordChannelConfigEntry( + match.entry, + match.matchKey, + match.matchSource === "parent" ? "parent" : "direct", + ); } return { allowed: false }; } diff --git a/src/discord/monitor/message-handler.preflight.ts b/src/discord/monitor/message-handler.preflight.ts index 1dc7e797b..070a8ef5b 100644 --- a/src/discord/monitor/message-handler.preflight.ts +++ b/src/discord/monitor/message-handler.preflight.ts @@ -14,9 +14,9 @@ import { upsertChannelPairingRequest, } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; -import { resolveMentionGating } from "../../channels/mention-gating.js"; +import { resolveMentionGatingWithBypass } from "../../channels/mention-gating.js"; import { sendMessageDiscord } from "../send.js"; -import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; +import { resolveControlCommandGate } from "../../channels/command-gating.js"; import { allowListMatches, isDiscordGroupAllowedByPolicy, @@ -347,6 +347,7 @@ export async function preflightDiscordMessage( cfg: params.cfg, surface: "discord", }); + const hasControlCommandInMessage = hasControlCommand(baseText, params.cfg); if (!isDirectMessage) { const ownerAllowList = normalizeDiscordAllowList(params.allowFrom, ["discord:", "user:"]); @@ -368,36 +369,35 @@ export async function preflightDiscordMessage( }) : false; const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; - commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + const commandGate = resolveControlCommandGate({ useAccessGroups, authorizers: [ { configured: ownerAllowList != null, allowed: ownerOk }, { configured: Array.isArray(channelUsers) && channelUsers.length > 0, allowed: usersOk }, ], modeWhenAccessGroupsOff: "configured", + allowTextCommands, + hasControlCommand: hasControlCommandInMessage, }); + commandAuthorized = commandGate.commandAuthorized; - if (allowTextCommands && hasControlCommand(baseText, params.cfg) && !commandAuthorized) { + if (commandGate.shouldBlock) { logVerbose(`Blocked discord control command from unauthorized sender ${author.id}`); return null; } } - const shouldBypassMention = - allowTextCommands && - isGuildMessage && - shouldRequireMention && - !wasMentioned && - !hasAnyMention && - commandAuthorized && - hasControlCommand(baseText, params.cfg); const canDetectMention = Boolean(botId) || mentionRegexes.length > 0; - const mentionGate = resolveMentionGating({ + const mentionGate = resolveMentionGatingWithBypass({ + isGroup: isGuildMessage, requireMention: Boolean(shouldRequireMention), canDetectMention, wasMentioned, implicitMention, - shouldBypassMention, + hasAnyMention, + allowTextCommands, + hasControlCommand: hasControlCommandInMessage, + commandAuthorized, }); const effectiveWasMentioned = mentionGate.effectiveWasMentioned; if (isGuildMessage && shouldRequireMention) { @@ -504,7 +504,7 @@ export async function preflightDiscordMessage( shouldRequireMention, hasAnyMention, allowTextCommands, - shouldBypassMention, + shouldBypassMention: mentionGate.shouldBypassMention, effectiveWasMentioned, canDetectMention, historyEntry, diff --git a/src/slack/monitor/channel-config.test.ts b/src/slack/monitor/channel-config.test.ts index aa59a6971..d090d8ac5 100644 --- a/src/slack/monitor/channel-config.test.ts +++ b/src/slack/monitor/channel-config.test.ts @@ -42,4 +42,16 @@ describe("resolveSlackChannelConfig", () => { matchSource: "wildcard", }); }); + + it("uses direct match metadata when channel config exists", () => { + const res = resolveSlackChannelConfig({ + channelId: "C1", + channels: { C1: { allow: true, requireMention: false } }, + defaultRequireMention: true, + }); + expect(res).toMatchObject({ + matchKey: "C1", + matchSource: "direct", + }); + }); }); diff --git a/src/slack/monitor/channel-config.ts b/src/slack/monitor/channel-config.ts index bea5f430e..2b9a31291 100644 --- a/src/slack/monitor/channel-config.ts +++ b/src/slack/monitor/channel-config.ts @@ -2,7 +2,7 @@ import type { SlackReactionNotificationMode } from "../../config/config.js"; import type { SlackMessageEvent } from "../types.js"; import { buildChannelKeyCandidates, - resolveChannelEntryMatch, + resolveChannelEntryMatchWithFallback, } from "../../channels/channel-config.js"; import { allowListMatches, normalizeAllowListLower, normalizeSlackSlug } from "./allow-list.js"; @@ -91,10 +91,10 @@ export function resolveSlackChannelConfig(params: { ); const { entry: matched, - key: matchedKey, wildcardEntry: fallback, - wildcardKey, - } = resolveChannelEntryMatch({ + matchKey, + matchSource, + } = resolveChannelEntryMatchWithFallback({ entries, keys: candidates, wildcardKey: "*", @@ -127,12 +127,9 @@ export function resolveSlackChannelConfig(params: { skills, systemPrompt, }; - if (matchedKey) { - result.matchKey = matchedKey; - result.matchSource = "direct"; - } else if (wildcardKey && fallback) { - result.matchKey = wildcardKey; - result.matchSource = "wildcard"; + if (matchKey) result.matchKey = matchKey; + if (matchSource === "direct" || matchSource === "wildcard") { + result.matchSource = matchSource; } return result; } diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 1b8b8615c..5f424f84c 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -18,9 +18,9 @@ import { buildPairingReply } from "../../../pairing/pairing-messages.js"; import { upsertChannelPairingRequest } from "../../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../../routing/resolve-route.js"; import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; -import { resolveMentionGating } from "../../../channels/mention-gating.js"; +import { resolveMentionGatingWithBypass } from "../../../channels/mention-gating.js"; import { resolveConversationLabel } from "../../../channels/conversation-label.js"; -import { resolveCommandAuthorizedFromAuthorizers } from "../../../channels/command-gating.js"; +import { resolveControlCommandGate } from "../../../channels/command-gating.js"; import type { ResolvedSlackAccount } from "../../accounts.js"; import { reactSlackMessage } from "../../actions.js"; @@ -229,6 +229,7 @@ export async function prepareSlackMessage(params: { cfg, surface: "slack", }); + const hasControlCommandInMessage = hasControlCommand(message.text ?? "", cfg); const ownerAuthorized = resolveSlackAllowListMatch({ allowList: allowFromLower, @@ -245,20 +246,18 @@ export async function prepareSlackMessage(params: { userName: senderName, }) : false; - const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + const commandGate = resolveControlCommandGate({ useAccessGroups: ctx.useAccessGroups, authorizers: [ { configured: allowFromLower.length > 0, allowed: ownerAuthorized }, { configured: channelUsersAllowlistConfigured, allowed: channelCommandAuthorized }, ], + allowTextCommands, + hasControlCommand: hasControlCommandInMessage, }); + const commandAuthorized = commandGate.commandAuthorized; - if ( - allowTextCommands && - isRoomish && - hasControlCommand(message.text ?? "", cfg) && - !commandAuthorized - ) { + if (isRoomish && commandGate.shouldBlock) { logVerbose(`Blocked slack control command from unauthorized sender ${senderId}`); return null; } @@ -268,22 +267,17 @@ export async function prepareSlackMessage(params: { : false; // Allow "control commands" to bypass mention gating if sender is authorized. - const shouldBypassMention = - allowTextCommands && - isRoom && - shouldRequireMention && - !wasMentioned && - !hasAnyMention && - commandAuthorized && - hasControlCommand(message.text ?? "", cfg); - const canDetectMention = Boolean(ctx.botUserId) || mentionRegexes.length > 0; - const mentionGate = resolveMentionGating({ + const mentionGate = resolveMentionGatingWithBypass({ + isGroup: isRoom, requireMention: Boolean(shouldRequireMention), canDetectMention, wasMentioned, implicitMention, - shouldBypassMention, + hasAnyMention, + allowTextCommands, + hasControlCommand: hasControlCommandInMessage, + commandAuthorized, }); const effectiveWasMentioned = mentionGate.effectiveWasMentioned; if (isRoom && shouldRequireMention && mentionGate.shouldSkip) { diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index 53a22702e..286354c4f 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -18,8 +18,8 @@ import type { DmPolicy, TelegramGroupConfig, TelegramTopicConfig } from "../conf 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 { resolveMentionGatingWithBypass } from "../channels/mention-gating.js"; +import { resolveControlCommandGate } from "../channels/command-gating.js"; import { buildGroupLabel, buildSenderLabel, @@ -269,10 +269,16 @@ export const buildTelegramMessageContext = async ({ senderUsername, }); const useAccessGroups = cfg.commands?.useAccessGroups !== false; - const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({ + const hasControlCommandInMessage = hasControlCommand(msg.text ?? msg.caption ?? "", cfg, { + botUsername, + }); + const commandGate = resolveControlCommandGate({ useAccessGroups, authorizers: [{ configured: allowForCommands.hasEntries, allowed: senderAllowedForCommands }], + allowTextCommands: true, + hasControlCommand: hasControlCommandInMessage, }); + const commandAuthorized = commandGate.commandAuthorized; const historyKey = isGroup ? buildTelegramGroupPeerId(chatId, resolvedThreadId) : undefined; let placeholder = ""; @@ -300,11 +306,7 @@ 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 - ) { + if (isGroup && commandGate.shouldBlock) { logVerbose(`telegram: drop control command from unauthorized sender ${senderId ?? "unknown"}`); return null; } @@ -325,20 +327,17 @@ export const buildTelegramMessageContext = async ({ const botId = primaryCtx.me?.id; const replyFromId = msg.reply_to_message?.from?.id; const implicitMention = botId != null && replyFromId === botId; - const shouldBypassMention = - isGroup && - requireMention && - !wasMentioned && - !hasAnyMention && - commandAuthorized && - hasControlCommand(msg.text ?? msg.caption ?? "", cfg, { botUsername }); const canDetectMention = Boolean(botUsername) || mentionRegexes.length > 0; - const mentionGate = resolveMentionGating({ + const mentionGate = resolveMentionGatingWithBypass({ + isGroup, requireMention: Boolean(requireMention), canDetectMention, wasMentioned, implicitMention: isGroup && Boolean(requireMention) && implicitMention, - shouldBypassMention, + hasAnyMention, + allowTextCommands: true, + hasControlCommand: hasControlCommandInMessage, + commandAuthorized, }); const effectiveWasMentioned = mentionGate.effectiveWasMentioned; if (isGroup && requireMention && canDetectMention) { diff --git a/src/telegram/bot.test.ts b/src/telegram/bot.test.ts index f934197e6..c0d81a8f8 100644 --- a/src/telegram/bot.test.ts +++ b/src/telegram/bot.test.ts @@ -1203,6 +1203,49 @@ describe("createTelegramBot", () => { expect(replySpy).toHaveBeenCalledTimes(1); }); + it("prefers topic allowFrom over group allowFrom", async () => { + onSpy.mockReset(); + const replySpy = replyModule.__replySpy as unknown as ReturnType; + replySpy.mockReset(); + loadConfig.mockReturnValue({ + channels: { + telegram: { + groupPolicy: "allowlist", + groups: { + "-1001234567890": { + allowFrom: ["123456789"], + topics: { + "99": { allowFrom: ["999999999"] }, + }, + }, + }, + }, + }, + }); + + createTelegramBot({ token: "tok" }); + const handler = getOnHandler("message") as (ctx: Record) => Promise; + + await handler({ + message: { + chat: { + id: -1001234567890, + type: "supergroup", + title: "Forum Group", + is_forum: true, + }, + from: { id: 123456789, username: "testuser" }, + text: "hello", + date: 1736380800, + message_thread_id: 99, + }, + me: { username: "clawdbot_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + + expect(replySpy).toHaveBeenCalledTimes(0); + }); + it("honors groups default when no explicit group override exists", async () => { onSpy.mockReset(); const replySpy = replyModule.__replySpy as unknown as ReturnType;