diff --git a/extensions/whatsapp/src/channel.ts b/extensions/whatsapp/src/channel.ts index 9d37fcf2a..93721b6d8 100644 --- a/extensions/whatsapp/src/channel.ts +++ b/extensions/whatsapp/src/channel.ts @@ -279,7 +279,7 @@ export const whatsappPlugin: ChannelPlugin = { chunkerMode: "text", textChunkLimit: 4000, pollMaxOptions: 12, - resolveTarget: ({ to, allowFrom, mode }) => { + resolveTarget: ({ to, allowFrom, mode, allowUnlisted }) => { const trimmed = to?.trim() ?? ""; const allowListRaw = (allowFrom ?? []).map((entry) => String(entry).trim()).filter(Boolean); const hasWildcard = allowListRaw.includes("*"); @@ -302,6 +302,7 @@ export const whatsappPlugin: ChannelPlugin = { ), }; } + // Groups are always allowed (no allowlist check needed) if (isWhatsAppGroupJid(normalizedTo)) { return { ok: true, to: normalizedTo }; } @@ -314,6 +315,28 @@ export const whatsappPlugin: ChannelPlugin = { } return { ok: true, to: allowList[0] }; } + // For explicit mode - validate allowlist unless override is set + if (mode === "explicit") { + // Wildcard or empty allowlist: allow any target + if (hasWildcard || allowList.length === 0) { + return { ok: true, to: normalizedTo }; + } + // Target is in allowlist: allow + if (allowList.includes(normalizedTo)) { + return { ok: true, to: normalizedTo }; + } + // Target not in allowlist: require explicit override + if (allowUnlisted) { + return { ok: true, to: normalizedTo }; + } + return { + ok: false, + error: new Error( + `Target ${normalizedTo} not in WhatsApp allowlist (channels.whatsapp.allowFrom). ` + + `Add the target to your allowlist, or use --allow-unlisted to override.`, + ), + }; + } return { ok: true, to: normalizedTo }; } diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index b5ef9711d..fdfb8287d 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -32,7 +32,12 @@ import { normalizeChatType } from "../../channels/chat-type.js"; import { stripMentions, stripStructuralPrefixes } from "./mentions.js"; import { formatInboundBodyWithSenderMeta } from "./inbound-sender-meta.js"; import { normalizeInboundTextNewlines } from "./inbound-text.js"; -import { normalizeSessionDeliveryFields } from "../../utils/delivery-context.js"; +import { + isSenderTrustedForDeliveryContext, + normalizeSessionDeliveryFields, +} from "../../utils/delivery-context.js"; +import { getChannelDock } from "../../channels/dock.js"; +import { normalizeAnyChannelId } from "../../channels/registry.js"; export type SessionInitResult = { sessionCtx: TemplateContext; @@ -233,12 +238,36 @@ export async function initSessionState(params: { } const baseEntry = !isNewSession && freshEntry ? entry : undefined; + + // FIX-3.2: Validate sender against allowlist before updating delivery context. + // This prevents routing replies to unknown recipients from untrusted senders. + const inboundChannel = normalizeAnyChannelId( + ctx.OriginatingChannel ?? ctx.Provider ?? ctx.Surface, + ); + const channelDock = inboundChannel ? getChannelDock(inboundChannel) : undefined; + const inboundAllowFrom = channelDock?.config?.resolveAllowFrom + ? channelDock.config.resolveAllowFrom({ cfg, accountId: ctx.AccountId ?? undefined }) + : undefined; + const senderForTrust = ctx.SenderE164 ?? ctx.SenderId ?? ctx.From ?? ctx.To; + const isTrustedSender = isSenderTrustedForDeliveryContext({ + sender: senderForTrust, + allowFrom: inboundAllowFrom, + }); + // Track the originating channel/to for announce routing (subagent announce-back). - const lastChannelRaw = (ctx.OriginatingChannel as string | undefined) || baseEntry?.lastChannel; - const lastToRaw = (ctx.OriginatingTo as string | undefined) || ctx.To || baseEntry?.lastTo; - const lastAccountIdRaw = (ctx.AccountId as string | undefined) || baseEntry?.lastAccountId; - const lastThreadIdRaw = - (ctx.MessageThreadId as string | number | undefined) || baseEntry?.lastThreadId; + // Only update from new inbound message if sender is trusted. + const lastChannelRaw = isTrustedSender + ? (ctx.OriginatingChannel as string | undefined) || baseEntry?.lastChannel + : baseEntry?.lastChannel; + const lastToRaw = isTrustedSender + ? (ctx.OriginatingTo as string | undefined) || ctx.To || baseEntry?.lastTo + : baseEntry?.lastTo; + const lastAccountIdRaw = isTrustedSender + ? (ctx.AccountId as string | undefined) || baseEntry?.lastAccountId + : baseEntry?.lastAccountId; + const lastThreadIdRaw = isTrustedSender + ? (ctx.MessageThreadId as string | number | undefined) || baseEntry?.lastThreadId + : baseEntry?.lastThreadId; const deliveryFields = normalizeSessionDeliveryFields({ deliveryContext: { channel: lastChannelRaw, diff --git a/src/channels/plugins/outbound/whatsapp.ts b/src/channels/plugins/outbound/whatsapp.ts index 303a015da..6017c908e 100644 --- a/src/channels/plugins/outbound/whatsapp.ts +++ b/src/channels/plugins/outbound/whatsapp.ts @@ -11,7 +11,7 @@ export const whatsappOutbound: ChannelOutboundAdapter = { chunkerMode: "text", textChunkLimit: 4000, pollMaxOptions: 12, - resolveTarget: ({ to, allowFrom, mode }) => { + resolveTarget: ({ to, allowFrom, mode, allowUnlisted }) => { const trimmed = to?.trim() ?? ""; const allowListRaw = (allowFrom ?? []).map((entry) => String(entry).trim()).filter(Boolean); const hasWildcard = allowListRaw.includes("*"); @@ -34,6 +34,7 @@ export const whatsappOutbound: ChannelOutboundAdapter = { ), }; } + // Groups are always allowed (no allowlist check needed) if (isWhatsAppGroupJid(normalizedTo)) { return { ok: true, to: normalizedTo }; } @@ -46,6 +47,28 @@ export const whatsappOutbound: ChannelOutboundAdapter = { } return { ok: true, to: allowList[0] }; } + // For explicit mode - validate allowlist unless override is set + if (mode === "explicit") { + // Wildcard or empty allowlist: allow any target + if (hasWildcard || allowList.length === 0) { + return { ok: true, to: normalizedTo }; + } + // Target is in allowlist: allow + if (allowList.includes(normalizedTo)) { + return { ok: true, to: normalizedTo }; + } + // Target not in allowlist: require explicit override + if (allowUnlisted) { + return { ok: true, to: normalizedTo }; + } + return { + ok: false, + error: new Error( + `Target ${normalizedTo} not in WhatsApp allowlist (channels.whatsapp.allowFrom). ` + + `Add the target to your allowlist, or use --allow-unlisted to override.`, + ), + }; + } return { ok: true, to: normalizedTo }; } diff --git a/src/channels/plugins/types.adapters.ts b/src/channels/plugins/types.adapters.ts index e0b3da23a..411402d10 100644 --- a/src/channels/plugins/types.adapters.ts +++ b/src/channels/plugins/types.adapters.ts @@ -98,6 +98,8 @@ export type ChannelOutboundAdapter = { allowFrom?: string[]; accountId?: string | null; mode?: ChannelOutboundTargetMode; + /** When true, allow explicit targets not in the allowlist (security bypass). */ + allowUnlisted?: boolean; }) => { ok: true; to: string } | { ok: false; error: Error }; sendPayload?: (ctx: ChannelOutboundPayloadContext) => Promise; sendText?: (ctx: ChannelOutboundContext) => Promise; diff --git a/src/cli/program/message/register.send.ts b/src/cli/program/message/register.send.ts index 4ab3a852f..eaba9a2f4 100644 --- a/src/cli/program/message/register.send.ts +++ b/src/cli/program/message/register.send.ts @@ -23,7 +23,12 @@ export function registerMessageSendCommand(message: Command, helpers: MessageCli .option("--reply-to ", "Reply-to message id") .option("--thread-id ", "Thread id (Telegram forum thread)") .option("--gif-playback", "Treat video media as GIF playback (WhatsApp only).", false) - .option("--silent", "Send message silently without notification (Telegram only)", false), + .option("--silent", "Send message silently without notification (Telegram only)", false) + .option( + "--allow-unlisted", + "Allow sending to targets not in the channel allowlist (WhatsApp security bypass).", + false, + ) ) .action(async (opts) => { await helpers.runMessageAction("send", opts); diff --git a/src/config/types.agent-defaults.ts b/src/config/types.agent-defaults.ts index 9c6ce0211..a5275ea53 100644 --- a/src/config/types.agent-defaults.ts +++ b/src/config/types.agent-defaults.ts @@ -193,6 +193,12 @@ export type AgentDefaultsConfig = { * Default: false (only the final heartbeat payload is delivered). */ includeReasoning?: boolean; + /** + * When true, require an explicit target for heartbeat delivery (no implicit routing). + * If no explicit target is provided, heartbeat delivery is skipped with reason "require-explicit". + * Default: false (backwards compatible; falls back to session-derived or "last" routing). + */ + requireExplicitTarget?: boolean; }; /** Max concurrent agent runs across all conversations. Default: 1 (sequential). */ maxConcurrent?: number; diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index 8f99ad791..c257c871d 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -674,6 +674,7 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise { beforeEach(() => { @@ -105,6 +109,178 @@ describe("resolveOutboundTarget", () => { expect(res.error.message).toContain("WebChat"); } }); + + describe("whatsapp explicit mode allowlist validation", () => { + it("rejects explicit target not in allowlist", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["+1555000001", "+1555000002"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555999999", + cfg, + mode: "explicit", + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error.message).toContain("not in WhatsApp allowlist"); + expect(res.error.message).toContain("--allow-unlisted"); + } + }); + + it("allows explicit target when in allowlist", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["+1555000001", "+1555000002"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555000001", + cfg, + mode: "explicit", + }); + expect(res).toEqual({ ok: true, to: "+1555000001" }); + }); + + it("allows explicit target with allowUnlisted override", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["+1555000001"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555999999", + cfg, + mode: "explicit", + allowUnlisted: true, + }); + expect(res).toEqual({ ok: true, to: "+1555999999" }); + }); + + it("allows explicit target when allowlist has wildcard", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["*", "+1555000001"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555999999", + cfg, + mode: "explicit", + }); + expect(res).toEqual({ ok: true, to: "+1555999999" }); + }); + + it("allows explicit target when allowlist is empty", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: [] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555999999", + cfg, + mode: "explicit", + }); + expect(res).toEqual({ ok: true, to: "+1555999999" }); + }); + + it("allows group targets regardless of allowlist", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["+1555000001"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "120363401234567890@g.us", + cfg, + mode: "explicit", + }); + expect(res).toEqual({ ok: true, to: "120363401234567890@g.us" }); + }); + + it("implicit mode still allows targets not in allowlist (fallback to first)", () => { + const cfg: ClawdbotConfig = { + channels: { whatsapp: { allowFrom: ["+1555000001", "+1555000002"] } }, + }; + const res = resolveOutboundTarget({ + channel: "whatsapp", + to: "+1555999999", + cfg, + mode: "implicit", + }); + // In implicit mode, unlisted targets fall back to the first allowFrom entry + expect(res).toEqual({ ok: true, to: "+1555000001" }); + }); + }); +}); + +describe("resolveHeartbeatDeliveryTarget", () => { + beforeEach(() => { + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "whatsapp", plugin: whatsappPlugin, source: "test" }, + { pluginId: "telegram", plugin: telegramPlugin, source: "test" }, + ]), + ); + }); + + it("returns none with reason require-explicit when requireExplicitTarget is true and no to is set", () => { + const cfg: ClawdbotConfig = { + agents: { + defaults: { + heartbeat: { + target: "whatsapp", + requireExplicitTarget: true, + // no `to` set + }, + }, + }, + channels: { whatsapp: { allowFrom: ["+1555000001"] } }, + }; + const result = resolveHeartbeatDeliveryTarget({ + cfg, + entry: { sessionId: "test", updatedAt: 1, lastChannel: "whatsapp", lastTo: "+1555000001" }, + }); + expect(result.channel).toBe("none"); + expect(result.reason).toBe("require-explicit"); + }); + + it("allows heartbeat delivery when requireExplicitTarget is true and to is set", () => { + const cfg: ClawdbotConfig = { + agents: { + defaults: { + heartbeat: { + target: "whatsapp", + requireExplicitTarget: true, + to: "+1555000001", + }, + }, + }, + channels: { whatsapp: { allowFrom: ["+1555000001"] } }, + }; + const result = resolveHeartbeatDeliveryTarget({ + cfg, + entry: { sessionId: "test", updatedAt: 1, lastChannel: "whatsapp", lastTo: "+1555000002" }, + }); + expect(result.channel).toBe("whatsapp"); + expect(result.to).toBe("+1555000001"); + }); + + it("allows implicit routing when requireExplicitTarget is false (default)", () => { + const cfg: ClawdbotConfig = { + agents: { + defaults: { + heartbeat: { + target: "last", + // requireExplicitTarget defaults to false + }, + }, + }, + channels: { whatsapp: { allowFrom: ["+1555000001"] } }, + }; + const result = resolveHeartbeatDeliveryTarget({ + cfg, + entry: { sessionId: "test", updatedAt: 1, lastChannel: "whatsapp", lastTo: "+1555000001" }, + }); + expect(result.channel).toBe("whatsapp"); + expect(result.to).toBe("+1555000001"); + }); }); describe("resolveSessionDeliveryTarget", () => { diff --git a/src/infra/outbound/targets.ts b/src/infra/outbound/targets.ts index 8b557c0a6..10efaf6ce 100644 --- a/src/infra/outbound/targets.ts +++ b/src/infra/outbound/targets.ts @@ -122,6 +122,8 @@ export function resolveOutboundTarget(params: { cfg?: MoltbotConfig; accountId?: string | null; mode?: ChannelOutboundTargetMode; + /** When true, allow explicit targets not in the allowlist (security bypass). */ + allowUnlisted?: boolean; }): OutboundTargetResolution { if (params.channel === INTERNAL_MESSAGE_CHANNEL) { return { @@ -157,6 +159,7 @@ export function resolveOutboundTarget(params: { allowFrom, accountId: params.accountId ?? undefined, mode: params.mode ?? "explicit", + allowUnlisted: params.allowUnlisted, }); } @@ -179,6 +182,8 @@ export function resolveHeartbeatDeliveryTarget(params: { const { cfg, entry } = params; const heartbeat = params.heartbeat ?? cfg.agents?.defaults?.heartbeat; const rawTarget = heartbeat?.target; + const requireExplicitTarget = heartbeat?.requireExplicitTarget ?? false; + const explicitTo = heartbeat?.to?.trim(); let target: HeartbeatTarget = "last"; if (rawTarget === "none" || rawTarget === "last") { target = rawTarget; @@ -198,10 +203,22 @@ export function resolveHeartbeatDeliveryTarget(params: { }; } + // When requireExplicitTarget is enabled, only allow delivery if an explicit target is set + if (requireExplicitTarget && !explicitTo) { + const base = resolveSessionDeliveryTarget({ entry }); + return { + channel: "none", + reason: "require-explicit", + accountId: undefined, + lastChannel: base.lastChannel, + lastAccountId: base.lastAccountId, + }; + } + const resolvedTarget = resolveSessionDeliveryTarget({ entry, requestedChannel: target === "last" ? "last" : target, - explicitTo: heartbeat?.to, + explicitTo, mode: "heartbeat", }); diff --git a/src/utils/delivery-context.test.ts b/src/utils/delivery-context.test.ts index 705e6d27f..45174590d 100644 --- a/src/utils/delivery-context.test.ts +++ b/src/utils/delivery-context.test.ts @@ -1,103 +1,92 @@ import { describe, expect, it } from "vitest"; +import { isSenderTrustedForDeliveryContext } from "./delivery-context.js"; -import { - deliveryContextKey, - deliveryContextFromSession, - mergeDeliveryContext, - normalizeDeliveryContext, - normalizeSessionDeliveryFields, -} from "./delivery-context.js"; +describe("isSenderTrustedForDeliveryContext", () => { + it("returns true when trustAll is set", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1999999999", + allowFrom: ["+1555000001"], + trustAll: true, + }); + expect(result).toBe(true); + }); -describe("delivery context helpers", () => { - it("normalizes channel/to/accountId and drops empty contexts", () => { + it("returns true when allowFrom is empty (open policy)", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1999999999", + allowFrom: [], + }); + expect(result).toBe(true); + }); + + it("returns true when allowFrom is undefined (open policy)", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1999999999", + }); + expect(result).toBe(true); + }); + + it("returns true when allowFrom has wildcard", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1999999999", + allowFrom: ["*", "+1555000001"], + }); + expect(result).toBe(true); + }); + + it("returns true when sender is in allowFrom", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1555000001", + allowFrom: ["+1555000001", "+1555000002"], + }); + expect(result).toBe(true); + }); + + it("returns false when sender is not in allowFrom", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "+1999999999", + allowFrom: ["+1555000001", "+1555000002"], + }); + expect(result).toBe(false); + }); + + it("returns false when sender is null/undefined", () => { expect( - normalizeDeliveryContext({ - channel: " whatsapp ", - to: " +1555 ", - accountId: " acct-1 ", + isSenderTrustedForDeliveryContext({ + sender: null, + allowFrom: ["+1555000001"], }), - ).toEqual({ - channel: "whatsapp", - to: "+1555", - accountId: "acct-1", - }); - - expect(normalizeDeliveryContext({ channel: " " })).toBeUndefined(); - }); - - it("merges primary values over fallback", () => { - const merged = mergeDeliveryContext( - { channel: "whatsapp", to: "channel:abc" }, - { channel: "slack", to: "channel:def", accountId: "acct" }, - ); - - expect(merged).toEqual({ - channel: "whatsapp", - to: "channel:abc", - accountId: "acct", - }); - }); - - it("builds stable keys only when channel and to are present", () => { - expect(deliveryContextKey({ channel: "whatsapp", to: "+1555" })).toBe("whatsapp|+1555||"); - expect(deliveryContextKey({ channel: "whatsapp" })).toBeUndefined(); - expect(deliveryContextKey({ channel: "whatsapp", to: "+1555", accountId: "acct-1" })).toBe( - "whatsapp|+1555|acct-1|", - ); - expect(deliveryContextKey({ channel: "slack", to: "channel:C1", threadId: "123.456" })).toBe( - "slack|channel:C1||123.456", - ); - }); - - it("derives delivery context from a session entry", () => { - expect( - deliveryContextFromSession({ - channel: "webchat", - lastChannel: " whatsapp ", - lastTo: " +1777 ", - lastAccountId: " acct-9 ", - }), - ).toEqual({ - channel: "whatsapp", - to: "+1777", - accountId: "acct-9", - }); + ).toBe(false); expect( - deliveryContextFromSession({ - channel: "telegram", - lastTo: " 123 ", - lastThreadId: " 999 ", + isSenderTrustedForDeliveryContext({ + sender: undefined, + allowFrom: ["+1555000001"], }), - ).toEqual({ - channel: "telegram", - to: "123", - accountId: undefined, - threadId: "999", - }); + ).toBe(false); }); - it("normalizes delivery fields and mirrors them on session entries", () => { - const normalized = normalizeSessionDeliveryFields({ - deliveryContext: { - channel: " Slack ", - to: " channel:1 ", - accountId: " acct-2 ", - threadId: " 444 ", - }, - lastChannel: " whatsapp ", - lastTo: " +1555 ", + it("returns false when sender is empty string", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: " ", + allowFrom: ["+1555000001"], }); + expect(result).toBe(false); + }); - expect(normalized.deliveryContext).toEqual({ - channel: "whatsapp", - to: "+1555", - accountId: "acct-2", - threadId: "444", + it("handles number entries in allowFrom", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: "123456789", + allowFrom: [123456789, "+1555000001"], }); - expect(normalized.lastChannel).toBe("whatsapp"); - expect(normalized.lastTo).toBe("+1555"); - expect(normalized.lastAccountId).toBe("acct-2"); - expect(normalized.lastThreadId).toBe("444"); + expect(result).toBe(true); + }); + + it("trims sender and allowFrom entries for matching", () => { + const result = isSenderTrustedForDeliveryContext({ + sender: " +1555000001 ", + allowFrom: [" +1555000001 "], + }); + expect(result).toBe(true); }); }); diff --git a/src/utils/delivery-context.ts b/src/utils/delivery-context.ts index 9f5803e17..44b7b22de 100644 --- a/src/utils/delivery-context.ts +++ b/src/utils/delivery-context.ts @@ -8,6 +8,44 @@ export type DeliveryContext = { threadId?: string | number; }; +export type DeliveryContextTrustParams = { + /** Sender identifier (e.g., E.164 phone number, user id). */ + sender?: string | null; + /** Allowlist entries for the channel. */ + allowFrom?: Array; + /** When true, trust any sender (no allowlist check). */ + trustAll?: boolean; +}; + +/** + * Validates whether a sender is trusted for updating the delivery context. + * Only messages from allowlisted senders (or when trustAll/wildcard is set) should + * update the delivery context to prevent routing replies to unknown recipients. + * + * @returns true if the sender is trusted, false otherwise + */ +export function isSenderTrustedForDeliveryContext(params: DeliveryContextTrustParams): boolean { + const { sender, allowFrom, trustAll } = params; + + // If trustAll is explicitly set, allow + if (trustAll) return true; + + // If no allowFrom list, allow (open policy) + if (!allowFrom || allowFrom.length === 0) return true; + + // Check for wildcard in allowlist + const hasWildcard = allowFrom.some((entry) => String(entry).trim() === "*"); + if (hasWildcard) return true; + + // No sender to check - don't trust + const senderNormalized = sender?.trim(); + if (!senderNormalized) return false; + + // Check if sender is in allowlist + const allowList = allowFrom.map((entry) => String(entry).trim()).filter(Boolean); + return allowList.includes(senderNormalized); +} + export type DeliveryContextSessionSource = { channel?: string; lastChannel?: string;