fix(outbound): return error instead of silently redirecting to allowList[0] (#13578)

This commit is contained in:
Marcus Castro
2026-02-13 01:20:03 -03:00
committed by GitHub
parent a43136c85e
commit 39ee708df6
11 changed files with 392 additions and 74 deletions

View File

@@ -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.

View File

@@ -375,40 +375,23 @@ export const googlechatPlugin: ChannelPlugin<ResolvedGoogleChatAccount> = {
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",
"<spaces/{space}|users/{user}> or channels.googlechat.dm.allowFrom[0]",
),
error: missingTargetError("Google Chat", "<spaces/{space}|users/{user}>"),
};
}
return { ok: true, to: normalized };
}
if (allowList.length > 0) {
return { ok: true, to: allowList[0] };
}
return {
ok: false,
error: missingTargetError(
"Google Chat",
"<spaces/{space}|users/{user}> or channels.googlechat.dm.allowFrom[0]",
),
error: missingTargetError("Google Chat", "<spaces/{space}|users/{user}>"),
};
},
sendText: async ({ cfg, to, text, accountId, replyToId, threadId }) => {

View File

@@ -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();
});
});

View File

@@ -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",

View File

@@ -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", "<channel-name>"),
};
}
// 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", "<channel-name>"),
};
}
// 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",
"<channel-name> or channels.twitch.accounts.<account>.allowFrom[0]",
),
error: missingTargetError("Twitch", "<channel-name>"),
};
},

View File

@@ -301,15 +301,9 @@ export const whatsappPlugin: ChannelPlugin<ResolvedWhatsAppAccount> = {
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",
"<E.164|group JID> or channels.whatsapp.allowFrom[0]",
),
error: missingTargetError("WhatsApp", "<E.164|group JID>"),
};
}
if (isWhatsAppGroupJid(normalizedTo)) {
@@ -322,20 +316,16 @@ export const whatsappPlugin: ChannelPlugin<ResolvedWhatsAppAccount> = {
if (allowList.includes(normalizedTo)) {
return { ok: true, to: normalizedTo };
}
return { ok: true, to: allowList[0] };
return {
ok: false,
error: missingTargetError("WhatsApp", "<E.164|group JID>"),
};
}
return { ok: true, to: normalizedTo };
}
if (allowList.length > 0) {
return { ok: true, to: allowList[0] };
}
return {
ok: false,
error: missingTargetError(
"WhatsApp",
"<E.164|group JID> or channels.whatsapp.allowFrom[0]",
),
error: missingTargetError("WhatsApp", "<E.164|group JID>"),
};
},
sendText: async ({ to, text, accountId, deps, gifPlayback }) => {

View File

@@ -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();
});
});

View File

@@ -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",
});
});
});

View File

@@ -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",
"<E.164|group JID> or channels.whatsapp.allowFrom[0]",
),
error: missingTargetError("WhatsApp", "<E.164|group JID>"),
};
}
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", "<E.164|group JID>"),
};
}
return { ok: true, to: normalizedTo };
}
if (allowList.length > 0) {
return { ok: true, to: allowList[0] };
}
return {
ok: false,
error: missingTargetError("WhatsApp", "<E.164|group JID> or channels.whatsapp.allowFrom[0]"),
error: missingTargetError("WhatsApp", "<E.164|group JID>"),
};
},
sendText: async ({ to, text, accountId, deps, gifPlayback }) => {

View File

@@ -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,

View File

@@ -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",