diff --git a/CHANGELOG.md b/CHANGELOG.md index 593b2f1839..c8f65a6f23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Docs: https://docs.openclaw.ai - Onboarding/CLI: restore terminal state without resuming paused `stdin`, so onboarding exits cleanly after choosing Web UI and the installer returns instead of appearing stuck. - macOS Voice Wake: fix a crash in trigger trimming for CJK/Unicode transcripts by matching and slicing on original-string ranges instead of transformed-string indices. (#11052) Thanks @Flash-LHR. - Heartbeat: prevent scheduler silent-death races during runner reloads, preserve retry cooldown backoff under wake bursts, and prioritize user/action wake causes over interval/retry reasons when coalescing. (#15108) Thanks @joeykrug. +- Outbound targets: fail closed for WhatsApp/Twitch/Google Chat fallback paths so invalid or missing targets are dropped instead of rerouted, and align resolver hints with strict target requirements. (#13578) Thanks @mcaxtr. - Exec/Allowlist: allow multiline heredoc bodies (`<<`, `<<-`) while keeping multiline non-heredoc shell commands blocked, so exec approval parsing permits heredoc input safely without allowing general newline command chaining. (#13811) Thanks @mcaxtr. - Docs/Mermaid: remove hardcoded Mermaid init theme blocks from four docs diagrams so dark mode inherits readable theme defaults. (#15157) Thanks @heytulsiprasad. - Outbound/Threading: pass `replyTo` and `threadId` from `message send` tool actions through the core outbound send path to channel adapters, preserving thread/reply routing. (#14948) Thanks @mcaxtr. diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index 50c8046400..79918b9494 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -375,40 +375,23 @@ export const googlechatPlugin: ChannelPlugin = { chunker: (text, limit) => getGoogleChatRuntime().channel.text.chunkMarkdownText(text, limit), chunkerMode: "markdown", textChunkLimit: 4000, - resolveTarget: ({ to, allowFrom, mode }) => { + resolveTarget: ({ to }) => { const trimmed = to?.trim() ?? ""; - const allowListRaw = (allowFrom ?? []).map((entry) => String(entry).trim()).filter(Boolean); - const allowList = allowListRaw - .filter((entry) => entry !== "*") - .map((entry) => normalizeGoogleChatTarget(entry)) - .filter((entry): entry is string => Boolean(entry)); if (trimmed) { const normalized = normalizeGoogleChatTarget(trimmed); if (!normalized) { - if ((mode === "implicit" || mode === "heartbeat") && allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError( - "Google Chat", - " or channels.googlechat.dm.allowFrom[0]", - ), + error: missingTargetError("Google Chat", ""), }; } return { ok: true, to: normalized }; } - if (allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError( - "Google Chat", - " or channels.googlechat.dm.allowFrom[0]", - ), + error: missingTargetError("Google Chat", ""), }; }, sendText: async ({ cfg, to, text, accountId, replyToId, threadId }) => { diff --git a/extensions/googlechat/src/resolve-target.test.ts b/extensions/googlechat/src/resolve-target.test.ts new file mode 100644 index 0000000000..1631972bc6 --- /dev/null +++ b/extensions/googlechat/src/resolve-target.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("openclaw/plugin-sdk", () => ({ + getChatChannelMeta: () => ({ id: "googlechat", label: "Google Chat" }), + missingTargetError: (provider: string, hint: string) => + new Error(`Delivering to ${provider} requires target ${hint}`), + GoogleChatConfigSchema: {}, + DEFAULT_ACCOUNT_ID: "default", + PAIRING_APPROVED_MESSAGE: "Approved", + applyAccountNameToChannelSection: vi.fn(), + buildChannelConfigSchema: vi.fn(), + deleteAccountFromConfigSection: vi.fn(), + formatPairingApproveHint: vi.fn(), + migrateBaseNameToDefaultAccount: vi.fn(), + normalizeAccountId: vi.fn(), + resolveChannelMediaMaxBytes: vi.fn(), + resolveGoogleChatGroupRequireMention: vi.fn(), + setAccountEnabledInConfigSection: vi.fn(), +})); + +vi.mock("./accounts.js", () => ({ + listGoogleChatAccountIds: vi.fn(), + resolveDefaultGoogleChatAccountId: vi.fn(), + resolveGoogleChatAccount: vi.fn(), +})); + +vi.mock("./actions.js", () => ({ + googlechatMessageActions: [], +})); + +vi.mock("./api.js", () => ({ + sendGoogleChatMessage: vi.fn(), + uploadGoogleChatAttachment: vi.fn(), + probeGoogleChat: vi.fn(), +})); + +vi.mock("./monitor.js", () => ({ + resolveGoogleChatWebhookPath: vi.fn(), + startGoogleChatMonitor: vi.fn(), +})); + +vi.mock("./onboarding.js", () => ({ + googlechatOnboardingAdapter: {}, +})); + +vi.mock("./runtime.js", () => ({ + getGoogleChatRuntime: vi.fn(() => ({ + channel: { + text: { chunkMarkdownText: vi.fn() }, + }, + })), +})); + +vi.mock("./targets.js", () => ({ + normalizeGoogleChatTarget: (raw?: string | null) => { + if (!raw?.trim()) return undefined; + if (raw === "invalid-target") return undefined; + const trimmed = raw.trim().replace(/^(googlechat|google-chat|gchat):/i, ""); + if (trimmed.startsWith("spaces/")) return trimmed; + if (trimmed.includes("@")) return `users/${trimmed.toLowerCase()}`; + return `users/${trimmed}`; + }, + isGoogleChatUserTarget: (value: string) => value.startsWith("users/"), + isGoogleChatSpaceTarget: (value: string) => value.startsWith("spaces/"), + resolveGoogleChatOutboundSpace: vi.fn(), +})); + +import { googlechatPlugin } from "./channel.js"; + +const resolveTarget = googlechatPlugin.outbound!.resolveTarget!; + +describe("googlechat resolveTarget", () => { + it("should resolve valid target", () => { + const result = resolveTarget({ + to: "spaces/AAA", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("spaces/AAA"); + }); + + it("should resolve email target", () => { + const result = resolveTarget({ + to: "user@example.com", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("users/user@example.com"); + }); + + it("should error on normalization failure with allowlist (implicit mode)", () => { + const result = resolveTarget({ + to: "invalid-target", + mode: "implicit", + allowFrom: ["spaces/BBB"], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should error when no target provided with allowlist", () => { + const result = resolveTarget({ + to: undefined, + mode: "implicit", + allowFrom: ["spaces/BBB"], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should error when no target and no allowlist", () => { + const result = resolveTarget({ + to: undefined, + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should handle whitespace-only target", () => { + const result = resolveTarget({ + to: " ", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); +}); diff --git a/extensions/twitch/src/outbound.test.ts b/extensions/twitch/src/outbound.test.ts index 7c871e3c77..a807b1a873 100644 --- a/extensions/twitch/src/outbound.test.ts +++ b/extensions/twitch/src/outbound.test.ts @@ -108,15 +108,15 @@ describe("outbound", () => { expect(result.to).toBe("allowed"); }); - it("should fallback to first allowlist entry when target not in list", () => { + it("should error when target not in allowlist (implicit mode)", () => { const result = twitchOutbound.resolveTarget({ to: "#notallowed", mode: "implicit", allowFrom: ["#primary", "#secondary"], }); - expect(result.ok).toBe(true); - expect(result.to).toBe("primary"); + expect(result.ok).toBe(false); + expect(result.error).toContain("Twitch"); }); it("should accept any target when allowlist is empty", () => { @@ -130,15 +130,15 @@ describe("outbound", () => { expect(result.to).toBe("anychannel"); }); - it("should use first allowlist entry when no target provided", () => { + it("should error when no target provided with allowlist", () => { const result = twitchOutbound.resolveTarget({ to: undefined, mode: "implicit", allowFrom: ["#fallback", "#other"], }); - expect(result.ok).toBe(true); - expect(result.to).toBe("fallback"); + expect(result.ok).toBe(false); + expect(result.error).toContain("Twitch"); }); it("should return error when no target and no allowlist", () => { @@ -163,6 +163,17 @@ describe("outbound", () => { expect(result.error).toContain("Missing target"); }); + it("should error when target normalizes to empty string", () => { + const result = twitchOutbound.resolveTarget({ + to: "#", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(false); + expect(result.error).toContain("Twitch"); + }); + it("should filter wildcard from allowlist when checking membership", () => { const result = twitchOutbound.resolveTarget({ to: "#mychannel", diff --git a/extensions/twitch/src/outbound.ts b/extensions/twitch/src/outbound.ts index 8a1c75f5dd..6ada089faf 100644 --- a/extensions/twitch/src/outbound.ts +++ b/extensions/twitch/src/outbound.ts @@ -54,6 +54,12 @@ export const twitchOutbound: ChannelOutboundAdapter = { // If target is provided, normalize and validate it if (trimmed) { const normalizedTo = normalizeTwitchChannel(trimmed); + if (!normalizedTo) { + return { + ok: false, + error: missingTargetError("Twitch", ""), + }; + } // For implicit/heartbeat modes with allowList, check against allowlist if (mode === "implicit" || mode === "heartbeat") { @@ -63,26 +69,22 @@ export const twitchOutbound: ChannelOutboundAdapter = { if (allowList.includes(normalizedTo)) { return { ok: true, to: normalizedTo }; } - // Fallback to first allowFrom entry - return { ok: true, to: allowList[0] }; + return { + ok: false, + error: missingTargetError("Twitch", ""), + }; } // For explicit mode, accept any valid channel name return { ok: true, to: normalizedTo }; } - // No target provided, use allowFrom fallback - if (allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } + // No target provided - error // No target and no allowFrom - error return { ok: false, - error: missingTargetError( - "Twitch", - " or channels.twitch.accounts..allowFrom[0]", - ), + error: missingTargetError("Twitch", ""), }; }, diff --git a/extensions/whatsapp/src/channel.ts b/extensions/whatsapp/src/channel.ts index f9f2b757a2..95406ba92e 100644 --- a/extensions/whatsapp/src/channel.ts +++ b/extensions/whatsapp/src/channel.ts @@ -301,15 +301,9 @@ export const whatsappPlugin: ChannelPlugin = { if (trimmed) { const normalizedTo = normalizeWhatsAppTarget(trimmed); if (!normalizedTo) { - if ((mode === "implicit" || mode === "heartbeat") && allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError( - "WhatsApp", - " or channels.whatsapp.allowFrom[0]", - ), + error: missingTargetError("WhatsApp", ""), }; } if (isWhatsAppGroupJid(normalizedTo)) { @@ -322,20 +316,16 @@ export const whatsappPlugin: ChannelPlugin = { if (allowList.includes(normalizedTo)) { return { ok: true, to: normalizedTo }; } - return { ok: true, to: allowList[0] }; + return { + ok: false, + error: missingTargetError("WhatsApp", ""), + }; } return { ok: true, to: normalizedTo }; } - - if (allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError( - "WhatsApp", - " or channels.whatsapp.allowFrom[0]", - ), + error: missingTargetError("WhatsApp", ""), }; }, sendText: async ({ to, text, accountId, deps, gifPlayback }) => { diff --git a/extensions/whatsapp/src/resolve-target.test.ts b/extensions/whatsapp/src/resolve-target.test.ts new file mode 100644 index 0000000000..afa20b9136 --- /dev/null +++ b/extensions/whatsapp/src/resolve-target.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("openclaw/plugin-sdk", () => ({ + getChatChannelMeta: () => ({ id: "whatsapp", label: "WhatsApp" }), + normalizeWhatsAppTarget: (value: string) => { + if (value === "invalid-target") return null; + // Simulate E.164 normalization: strip leading + and whatsapp: prefix + const stripped = value.replace(/^whatsapp:/i, "").replace(/^\+/, ""); + return stripped.includes("@g.us") ? stripped : `${stripped}@s.whatsapp.net`; + }, + isWhatsAppGroupJid: (value: string) => value.endsWith("@g.us"), + missingTargetError: (provider: string, hint: string) => + new Error(`Delivering to ${provider} requires target ${hint}`), + WhatsAppConfigSchema: {}, + whatsappOnboardingAdapter: {}, + resolveWhatsAppHeartbeatRecipients: vi.fn(), + buildChannelConfigSchema: vi.fn(), + collectWhatsAppStatusIssues: vi.fn(), + createActionGate: vi.fn(), + DEFAULT_ACCOUNT_ID: "default", + escapeRegExp: vi.fn(), + formatPairingApproveHint: vi.fn(), + listWhatsAppAccountIds: vi.fn(), + listWhatsAppDirectoryGroupsFromConfig: vi.fn(), + listWhatsAppDirectoryPeersFromConfig: vi.fn(), + looksLikeWhatsAppTargetId: vi.fn(), + migrateBaseNameToDefaultAccount: vi.fn(), + normalizeAccountId: vi.fn(), + normalizeE164: vi.fn(), + normalizeWhatsAppMessagingTarget: vi.fn(), + readStringParam: vi.fn(), + resolveDefaultWhatsAppAccountId: vi.fn(), + resolveWhatsAppAccount: vi.fn(), + resolveWhatsAppGroupRequireMention: vi.fn(), + resolveWhatsAppGroupToolPolicy: vi.fn(), + applyAccountNameToChannelSection: vi.fn(), +})); + +vi.mock("./runtime.js", () => ({ + getWhatsAppRuntime: vi.fn(() => ({ + channel: { + text: { chunkText: vi.fn() }, + whatsapp: { + sendMessageWhatsApp: vi.fn(), + createLoginTool: vi.fn(), + }, + }, + })), +})); + +import { whatsappPlugin } from "./channel.js"; + +const resolveTarget = whatsappPlugin.outbound!.resolveTarget!; + +describe("whatsapp resolveTarget", () => { + it("should resolve valid target in explicit mode", () => { + const result = resolveTarget({ + to: "5511999999999", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("5511999999999@s.whatsapp.net"); + }); + + it("should resolve target in implicit mode with wildcard", () => { + const result = resolveTarget({ + to: "5511999999999", + mode: "implicit", + allowFrom: ["*"], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("5511999999999@s.whatsapp.net"); + }); + + it("should resolve target in implicit mode when in allowlist", () => { + const result = resolveTarget({ + to: "5511999999999", + mode: "implicit", + allowFrom: ["5511999999999"], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("5511999999999@s.whatsapp.net"); + }); + + it("should allow group JID regardless of allowlist", () => { + const result = resolveTarget({ + to: "120363123456789@g.us", + mode: "implicit", + allowFrom: ["5511999999999"], + }); + + expect(result.ok).toBe(true); + expect(result.to).toBe("120363123456789@g.us"); + }); + + it("should error when target not in allowlist (implicit mode)", () => { + const result = resolveTarget({ + to: "5511888888888", + mode: "implicit", + allowFrom: ["5511999999999", "5511777777777"], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should error on normalization failure with allowlist (implicit mode)", () => { + const result = resolveTarget({ + to: "invalid-target", + mode: "implicit", + allowFrom: ["5511999999999"], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should error when no target provided with allowlist", () => { + const result = resolveTarget({ + to: undefined, + mode: "implicit", + allowFrom: ["5511999999999"], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should error when no target and no allowlist", () => { + const result = resolveTarget({ + to: undefined, + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("should handle whitespace-only target", () => { + const result = resolveTarget({ + to: " ", + mode: "explicit", + allowFrom: [], + }); + + expect(result.ok).toBe(false); + expect(result.error).toBeDefined(); + }); +}); diff --git a/src/channels/plugins/outbound/whatsapp.test.ts b/src/channels/plugins/outbound/whatsapp.test.ts new file mode 100644 index 0000000000..7922ed0079 --- /dev/null +++ b/src/channels/plugins/outbound/whatsapp.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { whatsappOutbound } from "./whatsapp.js"; + +describe("whatsappOutbound.resolveTarget", () => { + it("returns error when no target is provided even with allowFrom", () => { + const result = whatsappOutbound.resolveTarget?.({ + to: undefined, + allowFrom: ["+15551234567"], + mode: "implicit", + }); + + expect(result).toEqual({ + ok: false, + error: expect.any(Error), + }); + }); + + it("returns error when implicit target is not in allowFrom", () => { + const result = whatsappOutbound.resolveTarget?.({ + to: "+15550000000", + allowFrom: ["+15551234567"], + mode: "implicit", + }); + + expect(result).toEqual({ + ok: false, + error: expect.any(Error), + }); + }); + + it("keeps group JID targets even when allowFrom does not contain them", () => { + const result = whatsappOutbound.resolveTarget?.({ + to: "120363401234567890@g.us", + allowFrom: ["+15551234567"], + mode: "implicit", + }); + + expect(result).toEqual({ + ok: true, + to: "120363401234567890@g.us", + }); + }); +}); diff --git a/src/channels/plugins/outbound/whatsapp.ts b/src/channels/plugins/outbound/whatsapp.ts index cf1f7b3ab7..cbb66935a1 100644 --- a/src/channels/plugins/outbound/whatsapp.ts +++ b/src/channels/plugins/outbound/whatsapp.ts @@ -23,15 +23,9 @@ export const whatsappOutbound: ChannelOutboundAdapter = { if (trimmed) { const normalizedTo = normalizeWhatsAppTarget(trimmed); if (!normalizedTo) { - if ((mode === "implicit" || mode === "heartbeat") && allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError( - "WhatsApp", - " or channels.whatsapp.allowFrom[0]", - ), + error: missingTargetError("WhatsApp", ""), }; } if (isWhatsAppGroupJid(normalizedTo)) { @@ -44,17 +38,17 @@ export const whatsappOutbound: ChannelOutboundAdapter = { if (allowList.includes(normalizedTo)) { return { ok: true, to: normalizedTo }; } - return { ok: true, to: allowList[0] }; + return { + ok: false, + error: missingTargetError("WhatsApp", ""), + }; } return { ok: true, to: normalizedTo }; } - if (allowList.length > 0) { - return { ok: true, to: allowList[0] }; - } return { ok: false, - error: missingTargetError("WhatsApp", " or channels.whatsapp.allowFrom[0]"), + error: missingTargetError("WhatsApp", ""), }; }, sendText: async ({ to, text, accountId, deps, gifPlayback }) => { diff --git a/src/infra/heartbeat-runner.returns-default-unset.test.ts b/src/infra/heartbeat-runner.returns-default-unset.test.ts index 25cde979c7..a5eeaf3207 100644 --- a/src/infra/heartbeat-runner.returns-default-unset.test.ts +++ b/src/infra/heartbeat-runner.returns-default-unset.test.ts @@ -198,7 +198,7 @@ describe("resolveHeartbeatDeliveryTarget", () => { }); }); - it("applies allowFrom fallback for WhatsApp targets", () => { + it("rejects WhatsApp target not in allowFrom (no silent fallback)", () => { const cfg: OpenClawConfig = { agents: { defaults: { heartbeat: { target: "whatsapp", to: "+1999" } } }, channels: { whatsapp: { allowFrom: ["+1555", "+1666"] } }, @@ -209,9 +209,8 @@ describe("resolveHeartbeatDeliveryTarget", () => { lastTo: "+1222", }; expect(resolveHeartbeatDeliveryTarget({ cfg, entry })).toEqual({ - channel: "whatsapp", - to: "+1555", - reason: "allowFrom-fallback", + channel: "none", + reason: "no-target", accountId: undefined, lastChannel: "whatsapp", lastAccountId: undefined, diff --git a/src/infra/outbound/targets.test.ts b/src/infra/outbound/targets.test.ts index a8b45af313..ff9dee1613 100644 --- a/src/infra/outbound/targets.test.ts +++ b/src/infra/outbound/targets.test.ts @@ -16,7 +16,7 @@ describe("resolveOutboundTarget", () => { ); }); - it("falls back to whatsapp allowFrom via config", () => { + it("rejects whatsapp with empty target even when allowFrom configured", () => { const cfg: OpenClawConfig = { channels: { whatsapp: { allowFrom: ["+1555"] } }, }; @@ -26,7 +26,10 @@ describe("resolveOutboundTarget", () => { cfg, mode: "explicit", }); - expect(res).toEqual({ ok: true, to: "+1555" }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.error.message).toContain("WhatsApp"); + } }); it.each([ @@ -49,18 +52,18 @@ describe("resolveOutboundTarget", () => { expected: { ok: true as const, to: "120363401234567890@g.us" }, }, { - name: "falls back to whatsapp allowFrom", + name: "rejects whatsapp with empty target and allowFrom (no silent fallback)", input: { channel: "whatsapp" as const, to: "", allowFrom: ["+1555"] }, - expected: { ok: true as const, to: "+1555" }, + expectedErrorIncludes: "WhatsApp", }, { - name: "normalizes whatsapp allowFrom fallback targets", + name: "rejects whatsapp with empty target and prefixed allowFrom (no silent fallback)", input: { channel: "whatsapp" as const, to: "", allowFrom: ["whatsapp:(555) 123-4567"], }, - expected: { ok: true as const, to: "+5551234567" }, + expectedErrorIncludes: "WhatsApp", }, { name: "rejects invalid whatsapp target",