From cbed0e065c78ff5ad4bdaa677e94e54857cee095 Mon Sep 17 00:00:00 2001 From: Marcus Widing Date: Thu, 26 Feb 2026 22:28:39 +0100 Subject: [PATCH] fix: reject dmPolicy="allowlist" with empty allowFrom across all channels When dmPolicy is set to "allowlist" but allowFrom is missing or empty, all DMs are silently dropped because no sender can match the empty allowlist. This is a common pitfall after upgrades that change how allowlist files are handled (e.g., external allowlist-dm.json files being deprecated in favor of inline allowFrom arrays). Changes: - Add requireAllowlistAllowFrom schema refinement (zod-schema.core.ts) - Apply validation to all channel schemas: Telegram, Discord, Slack, Signal, IRC, iMessage, BlueBubbles, MS Teams, Google Chat, WhatsApp - Add detectEmptyAllowlistPolicy to doctor-config-flow.ts so "openclaw doctor" surfaces a clear warning with remediation steps - Add 12 test cases covering reject/accept for multiple channels Fixes #27892 --- src/commands/doctor-config-flow.ts | 76 +++++++++++ ...onfig.allowlist-requires-allowfrom.test.ts | 118 ++++++++++++++++++ src/config/zod-schema.core.ts | 26 ++++ src/config/zod-schema.providers-core.ts | 105 ++++++++++++++++ src/config/zod-schema.providers-whatsapp.ts | 36 ++++++ 5 files changed, 361 insertions(+) create mode 100644 src/config/config.allowlist-requires-allowfrom.test.ts diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index f7f53d29ae6..b875bc3a71a 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -1095,6 +1095,72 @@ function maybeRepairOpenPolicyAllowFrom(cfg: OpenClawConfig): { return { config: next, changes }; } +/** + * Scan all channel configs for dmPolicy="allowlist" without any allowFrom entries. + * This configuration causes all DMs to be silently dropped because no sender can + * match the empty allowlist. Common after upgrades that remove external allowlist + * file support. + */ +function detectEmptyAllowlistPolicy(cfg: OpenClawConfig): string[] { + const channels = cfg.channels; + if (!channels || typeof channels !== "object") { + return []; + } + + const warnings: string[] = []; + + const hasEntries = (list?: Array) => + Array.isArray(list) && list.map((v) => String(v).trim()).filter(Boolean).length > 0; + + const checkAccount = (account: Record, prefix: string) => { + const dmEntry = account.dm; + const dm = + dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry) + ? (dmEntry as Record) + : undefined; + const dmPolicy = + (account.dmPolicy as string | undefined) ?? (dm?.policy as string | undefined) ?? undefined; + + if (dmPolicy !== "allowlist") { + return; + } + + const topAllowFrom = account.allowFrom as Array | undefined; + const nestedAllowFrom = dm?.allowFrom as Array | undefined; + + if (hasEntries(topAllowFrom) || hasEntries(nestedAllowFrom)) { + return; + } + + warnings.push( + `- ${prefix}.dmPolicy is "allowlist" but allowFrom is empty — all DMs will be silently dropped. Add sender IDs to ${prefix}.allowFrom or change dmPolicy to "pairing".`, + ); + }; + + for (const [channelName, channelConfig] of Object.entries( + channels as Record>, + )) { + if (!channelConfig || typeof channelConfig !== "object") { + continue; + } + checkAccount(channelConfig, `channels.${channelName}`); + + const accounts = channelConfig.accounts; + if (accounts && typeof accounts === "object") { + for (const [accountId, account] of Object.entries( + accounts as Record>, + )) { + if (!account || typeof account !== "object") { + continue; + } + checkAccount(account, `channels.${channelName}.accounts.${accountId}`); + } + } + } + + return warnings; +} + type ExecSafeBinCoverageHit = { scopePath: string; bin: string; @@ -1551,6 +1617,11 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { cfg = allowFromRepair.config; } + const emptyAllowlistWarnings = detectEmptyAllowlistPolicy(candidate); + if (emptyAllowlistWarnings.length > 0) { + note(emptyAllowlistWarnings.join("\n"), "Doctor warnings"); + } + const toolsBySenderRepair = maybeRepairLegacyToolsBySenderKeys(candidate); if (toolsBySenderRepair.changes.length > 0) { note(toolsBySenderRepair.changes.join("\n"), "Doctor changes"); @@ -1603,6 +1674,11 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { ); } + const emptyAllowlistWarnings = detectEmptyAllowlistPolicy(candidate); + if (emptyAllowlistWarnings.length > 0) { + note(emptyAllowlistWarnings.join("\n"), "Doctor warnings"); + } + const toolsBySenderHits = scanLegacyToolsBySenderKeys(candidate); if (toolsBySenderHits.length > 0) { const sample = toolsBySenderHits[0]; diff --git a/src/config/config.allowlist-requires-allowfrom.test.ts b/src/config/config.allowlist-requires-allowfrom.test.ts new file mode 100644 index 00000000000..ab5b75fb957 --- /dev/null +++ b/src/config/config.allowlist-requires-allowfrom.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it } from "vitest"; +import { validateConfigObject } from "./config.js"; + +describe('dmPolicy="allowlist" requires non-empty allowFrom', () => { + it('rejects telegram dmPolicy="allowlist" without allowFrom', () => { + const res = validateConfigObject({ + channels: { telegram: { dmPolicy: "allowlist", botToken: "fake" } }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('rejects telegram dmPolicy="allowlist" with empty allowFrom', () => { + const res = validateConfigObject({ + channels: { telegram: { dmPolicy: "allowlist", allowFrom: [], botToken: "fake" } }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('accepts telegram dmPolicy="allowlist" with allowFrom entries', () => { + const res = validateConfigObject({ + channels: { telegram: { dmPolicy: "allowlist", allowFrom: ["12345"], botToken: "fake" } }, + }); + expect(res.ok).toBe(true); + }); + + it('accepts telegram dmPolicy="pairing" without allowFrom', () => { + const res = validateConfigObject({ + channels: { telegram: { dmPolicy: "pairing", botToken: "fake" } }, + }); + expect(res.ok).toBe(true); + }); + + it('rejects signal dmPolicy="allowlist" without allowFrom', () => { + const res = validateConfigObject({ + channels: { signal: { dmPolicy: "allowlist" } }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('accepts signal dmPolicy="allowlist" with allowFrom entries', () => { + const res = validateConfigObject({ + channels: { signal: { dmPolicy: "allowlist", allowFrom: ["+1234567890"] } }, + }); + expect(res.ok).toBe(true); + }); + + it('rejects discord dmPolicy="allowlist" without allowFrom', () => { + const res = validateConfigObject({ + channels: { discord: { dmPolicy: "allowlist" } }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('accepts discord dmPolicy="allowlist" with allowFrom entries', () => { + const res = validateConfigObject({ + channels: { discord: { dmPolicy: "allowlist", allowFrom: ["123456789"] } }, + }); + expect(res.ok).toBe(true); + }); + + it('rejects whatsapp dmPolicy="allowlist" without allowFrom', () => { + const res = validateConfigObject({ + channels: { whatsapp: { dmPolicy: "allowlist" } }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('accepts whatsapp dmPolicy="allowlist" with allowFrom entries', () => { + const res = validateConfigObject({ + channels: { whatsapp: { dmPolicy: "allowlist", allowFrom: ["+1234567890"] } }, + }); + expect(res.ok).toBe(true); + }); + + it('rejects telegram account dmPolicy="allowlist" without allowFrom', () => { + const res = validateConfigObject({ + channels: { + telegram: { + accounts: { + bot1: { dmPolicy: "allowlist", botToken: "fake" }, + }, + }, + }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true); + } + }); + + it('accepts telegram account dmPolicy="allowlist" with allowFrom entries', () => { + const res = validateConfigObject({ + channels: { + telegram: { + accounts: { + bot1: { dmPolicy: "allowlist", allowFrom: ["12345"], botToken: "fake" }, + }, + }, + }, + }); + expect(res.ok).toBe(true); + }); +}); diff --git a/src/config/zod-schema.core.ts b/src/config/zod-schema.core.ts index 201efe4aa96..711faf5e90c 100644 --- a/src/config/zod-schema.core.ts +++ b/src/config/zod-schema.core.ts @@ -511,6 +511,32 @@ export const requireOpenAllowFrom = (params: { }); }; +/** + * Validate that dmPolicy="allowlist" has a non-empty allowFrom array. + * Without this, all DMs are silently dropped because the allowlist is empty + * and no senders can match. + */ +export const requireAllowlistAllowFrom = (params: { + policy?: string; + allowFrom?: Array; + ctx: z.RefinementCtx; + path: Array; + message: string; +}) => { + if (params.policy !== "allowlist") { + return; + } + const allow = normalizeAllowFrom(params.allowFrom); + if (allow.length > 0) { + return; + } + params.ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: params.path, + message: params.message, + }); +}; + export const MSTeamsReplyStyleSchema = z.enum(["thread", "top-level"]); export const RetryConfigSchema = z diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index 8105d2fc77f..63e39ead5ff 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -29,6 +29,7 @@ import { ReplyToModeSchema, RetryConfigSchema, TtsConfigSchema, + requireAllowlistAllowFrom, requireOpenAllowFrom, } from "./zod-schema.core.js"; import { sensitive } from "./zod-schema.sensitive.js"; @@ -227,6 +228,14 @@ export const TelegramAccountSchema = TelegramAccountSchemaBase.superRefine((valu message: 'channels.telegram.dmPolicy="open" requires channels.telegram.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.telegram.dmPolicy="allowlist" requires channels.telegram.allowFrom to contain at least one sender ID', + }); validateTelegramCustomCommands(value, ctx); }); @@ -242,6 +251,14 @@ export const TelegramConfigSchema = TelegramAccountSchemaBase.extend({ message: 'channels.telegram.dmPolicy="open" requires channels.telegram.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.telegram.dmPolicy="allowlist" requires channels.telegram.allowFrom to contain at least one sender ID', + }); validateTelegramCustomCommands(value, ctx); const baseWebhookUrl = typeof value.webhookUrl === "string" ? value.webhookUrl.trim() : ""; @@ -508,6 +525,14 @@ export const DiscordAccountSchema = z message: 'channels.discord.dmPolicy="open" requires channels.discord.allowFrom (or channels.discord.dm.allowFrom) to include "*"', }); + requireAllowlistAllowFrom({ + policy: dmPolicy, + allowFrom, + ctx, + path: [...allowFromPath], + message: + 'channels.discord.dmPolicy="allowlist" requires channels.discord.allowFrom (or channels.discord.dm.allowFrom) to contain at least one sender ID', + }); }); export const DiscordConfigSchema = DiscordAccountSchema.extend({ @@ -530,6 +555,14 @@ export const GoogleChatDmSchema = z message: 'channels.googlechat.dm.policy="open" requires channels.googlechat.dm.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.policy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.googlechat.dm.policy="allowlist" requires channels.googlechat.dm.allowFrom to contain at least one sender ID', + }); }); export const GoogleChatGroupSchema = z @@ -718,6 +751,14 @@ export const SlackAccountSchema = z message: 'channels.slack.dmPolicy="open" requires channels.slack.allowFrom (or channels.slack.dm.allowFrom) to include "*"', }); + requireAllowlistAllowFrom({ + policy: dmPolicy, + allowFrom, + ctx, + path: [...allowFromPath], + message: + 'channels.slack.dmPolicy="allowlist" requires channels.slack.allowFrom (or channels.slack.dm.allowFrom) to contain at least one sender ID', + }); }); export const SlackConfigSchema = SlackAccountSchema.safeExtend({ @@ -814,6 +855,14 @@ export const SignalAccountSchema = SignalAccountSchemaBase.superRefine((value, c path: ["allowFrom"], message: 'channels.signal.dmPolicy="open" requires channels.signal.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.signal.dmPolicy="allowlist" requires channels.signal.allowFrom to contain at least one sender ID', + }); }); export const SignalConfigSchema = SignalAccountSchemaBase.extend({ @@ -826,6 +875,14 @@ export const SignalConfigSchema = SignalAccountSchemaBase.extend({ path: ["allowFrom"], message: 'channels.signal.dmPolicy="open" requires channels.signal.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.signal.dmPolicy="allowlist" requires channels.signal.allowFrom to contain at least one sender ID', + }); }); export const IrcGroupSchema = z @@ -898,6 +955,14 @@ function refineIrcAllowFromAndNickserv(value: IrcBaseConfig, ctx: z.RefinementCt path: ["allowFrom"], message: 'channels.irc.dmPolicy="open" requires channels.irc.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.irc.dmPolicy="allowlist" requires channels.irc.allowFrom to contain at least one sender ID', + }); if (value.nickserv?.register && !value.nickserv.registerEmail?.trim()) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -979,6 +1044,14 @@ export const IMessageAccountSchema = IMessageAccountSchemaBase.superRefine((valu message: 'channels.imessage.dmPolicy="open" requires channels.imessage.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.imessage.dmPolicy="allowlist" requires channels.imessage.allowFrom to contain at least one sender ID', + }); }); export const IMessageConfigSchema = IMessageAccountSchemaBase.extend({ @@ -992,6 +1065,14 @@ export const IMessageConfigSchema = IMessageAccountSchemaBase.extend({ message: 'channels.imessage.dmPolicy="open" requires channels.imessage.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.imessage.dmPolicy="allowlist" requires channels.imessage.allowFrom to contain at least one sender ID', + }); }); const BlueBubblesAllowFromEntry = z.union([z.string(), z.number()]); @@ -1059,6 +1140,14 @@ export const BlueBubblesAccountSchema = BlueBubblesAccountSchemaBase.superRefine path: ["allowFrom"], message: 'channels.bluebubbles.accounts.*.dmPolicy="open" requires allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.bluebubbles.accounts.*.dmPolicy="allowlist" requires allowFrom to contain at least one sender ID', + }); }); export const BlueBubblesConfigSchema = BlueBubblesAccountSchemaBase.extend({ @@ -1073,6 +1162,14 @@ export const BlueBubblesConfigSchema = BlueBubblesAccountSchemaBase.extend({ message: 'channels.bluebubbles.dmPolicy="open" requires channels.bluebubbles.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.bluebubbles.dmPolicy="allowlist" requires channels.bluebubbles.allowFrom to contain at least one sender ID', + }); }); export const MSTeamsChannelSchema = z @@ -1144,4 +1241,12 @@ export const MSTeamsConfigSchema = z message: 'channels.msteams.dmPolicy="open" requires channels.msteams.allowFrom to include "*"', }); + requireAllowlistAllowFrom({ + policy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + path: ["allowFrom"], + message: + 'channels.msteams.dmPolicy="allowlist" requires channels.msteams.allowFrom to contain at least one sender ID', + }); }); diff --git a/src/config/zod-schema.providers-whatsapp.ts b/src/config/zod-schema.providers-whatsapp.ts index 4387ed1abb5..ab5119a7786 100644 --- a/src/config/zod-schema.providers-whatsapp.ts +++ b/src/config/zod-schema.providers-whatsapp.ts @@ -80,6 +80,28 @@ function enforceOpenDmPolicyAllowFromStar(params: { }); } +function enforceAllowlistDmPolicyAllowFrom(params: { + dmPolicy: unknown; + allowFrom: unknown; + ctx: z.RefinementCtx; + message: string; +}) { + if (params.dmPolicy !== "allowlist") { + return; + } + const allow = (Array.isArray(params.allowFrom) ? params.allowFrom : []) + .map((v) => String(v).trim()) + .filter(Boolean); + if (allow.length > 0) { + return; + } + params.ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["allowFrom"], + message: params.message, + }); +} + export const WhatsAppAccountSchema = WhatsAppSharedSchema.extend({ name: z.string().optional(), enabled: z.boolean().optional(), @@ -95,6 +117,13 @@ export const WhatsAppAccountSchema = WhatsAppSharedSchema.extend({ ctx, message: 'channels.whatsapp.accounts.*.dmPolicy="open" requires allowFrom to include "*"', }); + enforceAllowlistDmPolicyAllowFrom({ + dmPolicy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + message: + 'channels.whatsapp.accounts.*.dmPolicy="allowlist" requires allowFrom to contain at least one sender ID', + }); }); export const WhatsAppConfigSchema = WhatsAppSharedSchema.extend({ @@ -118,4 +147,11 @@ export const WhatsAppConfigSchema = WhatsAppSharedSchema.extend({ message: 'channels.whatsapp.dmPolicy="open" requires channels.whatsapp.allowFrom to include "*"', }); + enforceAllowlistDmPolicyAllowFrom({ + dmPolicy: value.dmPolicy, + allowFrom: value.allowFrom, + ctx, + message: + 'channels.whatsapp.dmPolicy="allowlist" requires channels.whatsapp.allowFrom to contain at least one sender ID', + }); });