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', + }); });