fix: harden whatsapp command auth

This commit is contained in:
Peter Steinberger
2026-01-15 07:53:12 +00:00
parent 8c4b8f2c38
commit 8dacafce7f
7 changed files with 286 additions and 21 deletions

View File

@@ -55,6 +55,7 @@
- Telegram: let control commands bypass per-chat sequentialization; always allow abort triggers. - Telegram: let control commands bypass per-chat sequentialization; always allow abort triggers.
- Auto-reply: treat trailing `NO_REPLY` tokens as silent replies. - Auto-reply: treat trailing `NO_REPLY` tokens as silent replies.
- Config: prevent partial config writes from clobbering unrelated settings (base hash guard + merge patch for connection saves). - Config: prevent partial config writes from clobbering unrelated settings (base hash guard + merge patch for connection saves).
- WhatsApp: honor SenderE164 for owner command auth when SenderId is LID; let owner control commands bypass group mention gating.
- Sessions: return deep clones (`structuredClone`) so cached session entries can't be mutated. (#934) — thanks @ronak-guliani. - Sessions: return deep clones (`structuredClone`) so cached session entries can't be mutated. (#934) — thanks @ronak-guliani.
- Heartbeat: keep `updatedAt` monotonic when restoring heartbeat sessions. (#934) — thanks @ronak-guliani. - Heartbeat: keep `updatedAt` monotonic when restoring heartbeat sessions. (#934) — thanks @ronak-guliani.
- Agent: clear run context after CLI runs (`clearAgentRunContext`) to avoid runaway contexts. (#934) — thanks @ronak-guliani. - Agent: clear run context after CLI runs (`clearAgentRunContext`) to avoid runaway contexts. (#934) — thanks @ronak-guliani.

View File

@@ -91,6 +91,7 @@ Notes:
- `/verbose` is meant for debugging and extra visibility; keep it **off** in normal use. - `/verbose` is meant for debugging and extra visibility; keep it **off** in normal use.
- `/reasoning` (and `/verbose`) are risky in group settings: they may reveal internal reasoning or tool output you did not intend to expose. Prefer leaving them off, especially in group chats. - `/reasoning` (and `/verbose`) are risky in group settings: they may reveal internal reasoning or tool output you did not intend to expose. Prefer leaving them off, especially in group chats.
- **Fast path:** command-only messages from allowlisted senders are handled immediately (bypass queue + model). - **Fast path:** command-only messages from allowlisted senders are handled immediately (bypass queue + model).
- **Group mention gating:** command-only messages from allowlisted senders bypass mention requirements.
- **Inline shortcuts (allowlisted senders only):** certain commands also work when embedded in a normal message and are stripped before the model sees the remaining text. - **Inline shortcuts (allowlisted senders only):** certain commands also work when embedded in a normal message and are stripped before the model sees the remaining text.
- Example: `hey /status` triggers a status reply, and the remaining text continues through the normal flow. - Example: `hey /status` triggers a status reply, and the remaining text continues through the normal flow.
- Currently: `/help`, `/commands`, `/status` (`/usage`), `/whoami` (`/id`). - Currently: `/help`, `/commands`, `/status` (`/usage`), `/whoami` (`/id`).

View File

@@ -96,4 +96,27 @@ describe("resolveCommandAuthorization", () => {
expect(auth.senderId).toBe("+123"); expect(auth.senderId).toBe("+123");
expect(auth.isAuthorizedSender).toBe(true); expect(auth.isAuthorizedSender).toBe(true);
}); });
it("prefers SenderE164 when SenderId does not match allowFrom", () => {
const cfg = {
channels: { whatsapp: { allowFrom: ["+41796666864"] } },
} as ClawdbotConfig;
const ctx = {
Provider: "whatsapp",
Surface: "whatsapp",
From: "whatsapp:120363401234567890@g.us",
SenderId: "123@lid",
SenderE164: "+41796666864",
} as MsgContext;
const auth = resolveCommandAuthorization({
ctx,
cfg,
commandAuthorized: true,
});
expect(auth.senderId).toBe("+41796666864");
expect(auth.isAuthorizedSender).toBe(true);
});
}); });

View File

@@ -61,35 +61,49 @@ function normalizeAllowFromEntry(params: {
cfg: ClawdbotConfig; cfg: ClawdbotConfig;
accountId?: string | null; accountId?: string | null;
value: string; value: string;
}): string | undefined { }): string[] {
return formatAllowFromList({ const normalized = formatAllowFromList({
dock: params.dock, dock: params.dock,
cfg: params.cfg, cfg: params.cfg,
accountId: params.accountId, accountId: params.accountId,
allowFrom: [params.value], allowFrom: [params.value],
})[0]; });
return normalized.filter((entry) => entry.trim().length > 0);
} }
function resolveSenderId(params: { function resolveSenderCandidates(params: {
dock?: ChannelDock; dock?: ChannelDock;
providerId?: ChannelId;
cfg: ClawdbotConfig; cfg: ClawdbotConfig;
accountId?: string | null; accountId?: string | null;
senderId?: string | null; senderId?: string | null;
senderE164?: string | null; senderE164?: string | null;
from?: string | null; from?: string | null;
}): string | undefined { }): string[] {
const { dock, cfg, accountId } = params; const { dock, cfg, accountId } = params;
const candidates: string[] = [];
const senderCandidates = [params.senderId, params.senderE164, params.from] const pushCandidate = (value?: string | null) => {
.map((value) => (value ?? "").trim()) const trimmed = (value ?? "").trim();
.filter(Boolean); if (!trimmed) return;
candidates.push(trimmed);
for (const sender of senderCandidates) { };
const normalized = normalizeAllowFromEntry({ dock, cfg, accountId, value: sender }); if (params.providerId === "whatsapp") {
if (normalized) return normalized; pushCandidate(params.senderE164);
pushCandidate(params.senderId);
} else {
pushCandidate(params.senderId);
pushCandidate(params.senderE164);
} }
pushCandidate(params.from);
return undefined; const normalized: string[] = [];
for (const sender of candidates) {
const entries = normalizeAllowFromEntry({ dock, cfg, accountId, value: sender });
for (const entry of entries) {
if (!normalized.includes(entry)) normalized.push(entry);
}
}
return normalized;
} }
export function resolveCommandAuthorization(params: { export function resolveCommandAuthorization(params: {
@@ -122,25 +136,30 @@ export function resolveCommandAuthorization(params: {
accountId: ctx.AccountId, accountId: ctx.AccountId,
value: to, value: to,
}); });
if (normalizedTo) ownerCandidates.push(normalizedTo); if (normalizedTo.length > 0) ownerCandidates.push(...normalizedTo);
} }
const ownerList = ownerCandidates; const ownerList = Array.from(new Set(ownerCandidates));
const senderId = resolveSenderId({ const senderCandidates = resolveSenderCandidates({
dock, dock,
providerId,
cfg, cfg,
accountId: ctx.AccountId, accountId: ctx.AccountId,
senderId: ctx.SenderId, senderId: ctx.SenderId,
senderE164: ctx.SenderE164, senderE164: ctx.SenderE164,
from, from,
}); });
const matchedSender = ownerList.length
? senderCandidates.find((candidate) => ownerList.includes(candidate))
: undefined;
const senderId = matchedSender ?? senderCandidates[0];
const enforceOwner = Boolean(dock?.commands?.enforceOwnerForCommands); const enforceOwner = Boolean(dock?.commands?.enforceOwnerForCommands);
const isOwner = const isOwner =
!enforceOwner || !enforceOwner ||
allowAll || allowAll ||
ownerList.length === 0 || ownerList.length === 0 ||
(senderId ? ownerList.includes(senderId) : false); Boolean(matchedSender);
const isAuthorizedSender = commandAuthorized && isOwner; const isAuthorizedSender = commandAuthorized && isOwner;
return { return {

View File

@@ -167,4 +167,87 @@ describe("initSessionState reset triggers in WhatsApp groups", () => {
expect(result.sessionId).not.toBe(existingSessionId); expect(result.sessionId).not.toBe(existingSessionId);
expect(result.bodyStripped).toBe(""); expect(result.bodyStripped).toBe("");
}); });
it("Reset trigger /new works when SenderId is LID but SenderE164 is authorized", async () => {
const storePath = await createStorePath("clawdbot-group-reset-lid-");
const sessionKey = "agent:main:whatsapp:group:120363406150318674@g.us";
const existingSessionId = "existing-session-123";
await seedSessionStore({
storePath,
sessionKey,
sessionId: existingSessionId,
});
const cfg = makeCfg({
storePath,
allowFrom: ["+41796666864"],
});
const groupMessageCtx = {
Body: `[WhatsApp 120363406150318674@g.us 2026-01-13T07:45Z] Owner: /new\n[from: Owner (+41796666864)]`,
RawBody: "/new",
CommandBody: "/new",
From: "120363406150318674@g.us",
To: "+41779241027",
ChatType: "group",
SessionKey: sessionKey,
Provider: "whatsapp",
Surface: "whatsapp",
SenderName: "Owner",
SenderE164: "+41796666864",
SenderId: "123@lid",
};
const result = await initSessionState({
ctx: groupMessageCtx,
cfg,
commandAuthorized: true,
});
expect(result.triggerBodyNormalized).toBe("/new");
expect(result.isNewSession).toBe(true);
expect(result.sessionId).not.toBe(existingSessionId);
expect(result.bodyStripped).toBe("");
});
it("Reset trigger /new blocked when SenderId is LID but SenderE164 is unauthorized", async () => {
const storePath = await createStorePath("clawdbot-group-reset-lid-unauth-");
const sessionKey = "agent:main:whatsapp:group:120363406150318674@g.us";
const existingSessionId = "existing-session-123";
await seedSessionStore({
storePath,
sessionKey,
sessionId: existingSessionId,
});
const cfg = makeCfg({
storePath,
allowFrom: ["+41796666864"],
});
const groupMessageCtx = {
Body: `[WhatsApp 120363406150318674@g.us 2026-01-13T07:45Z] Other: /new\n[from: Other (+1555123456)]`,
RawBody: "/new",
CommandBody: "/new",
From: "120363406150318674@g.us",
To: "+41779241027",
ChatType: "group",
SessionKey: sessionKey,
Provider: "whatsapp",
Surface: "whatsapp",
SenderName: "Other",
SenderE164: "+1555123456",
SenderId: "123@lid",
};
const result = await initSessionState({
ctx: groupMessageCtx,
cfg,
commandAuthorized: true,
});
expect(result.triggerBodyNormalized).toBe("/new");
expect(result.sessionId).toBe(existingSessionId);
expect(result.isNewSession).toBe(false);
});
}); });

View File

@@ -169,6 +169,144 @@ describe("web auto-reply", () => {
expect(payload.Body).toContain("[from: Bob (+222)]"); expect(payload.Body).toContain("[from: Bob (+222)]");
}); });
it("bypasses mention gating for owner /new in group chats", async () => {
const sendMedia = vi.fn();
const reply = vi.fn().mockResolvedValue(undefined);
const sendComposing = vi.fn();
const resolver = vi.fn().mockResolvedValue({ text: "ok" });
let capturedOnMessage:
| ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
| undefined;
const listenerFactory = async (opts: {
onMessage: (msg: import("./inbound.js").WebInboundMessage) => Promise<void>;
}) => {
capturedOnMessage = opts.onMessage;
return { close: vi.fn() };
};
setLoadConfigMock(() => ({
channels: {
whatsapp: {
allowFrom: ["+111"],
},
},
}));
await monitorWebChannel(false, listenerFactory, false, resolver);
expect(capturedOnMessage).toBeDefined();
await capturedOnMessage?.({
body: "/new",
from: "123@g.us",
conversationId: "123@g.us",
chatId: "123@g.us",
chatType: "group",
to: "+2",
id: "g-new",
senderE164: "+111",
senderName: "Owner",
selfE164: "+999",
sendComposing,
reply,
sendMedia,
});
expect(resolver).toHaveBeenCalledTimes(1);
});
it("does not bypass mention gating for non-owner /new in group chats", async () => {
const sendMedia = vi.fn();
const reply = vi.fn().mockResolvedValue(undefined);
const sendComposing = vi.fn();
const resolver = vi.fn().mockResolvedValue({ text: "ok" });
let capturedOnMessage:
| ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
| undefined;
const listenerFactory = async (opts: {
onMessage: (msg: import("./inbound.js").WebInboundMessage) => Promise<void>;
}) => {
capturedOnMessage = opts.onMessage;
return { close: vi.fn() };
};
setLoadConfigMock(() => ({
channels: {
whatsapp: {
allowFrom: ["+999"],
},
},
}));
await monitorWebChannel(false, listenerFactory, false, resolver);
expect(capturedOnMessage).toBeDefined();
await capturedOnMessage?.({
body: "/new",
from: "123@g.us",
conversationId: "123@g.us",
chatId: "123@g.us",
chatType: "group",
to: "+2",
id: "g-new-unauth",
senderE164: "+111",
senderName: "NotOwner",
selfE164: "+999",
sendComposing,
reply,
sendMedia,
});
expect(resolver).not.toHaveBeenCalled();
});
it("bypasses mention gating for owner /status in group chats", async () => {
const sendMedia = vi.fn();
const reply = vi.fn().mockResolvedValue(undefined);
const sendComposing = vi.fn();
const resolver = vi.fn().mockResolvedValue({ text: "ok" });
let capturedOnMessage:
| ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
| undefined;
const listenerFactory = async (opts: {
onMessage: (msg: import("./inbound.js").WebInboundMessage) => Promise<void>;
}) => {
capturedOnMessage = opts.onMessage;
return { close: vi.fn() };
};
setLoadConfigMock(() => ({
channels: {
whatsapp: {
allowFrom: ["+111"],
},
},
}));
await monitorWebChannel(false, listenerFactory, false, resolver);
expect(capturedOnMessage).toBeDefined();
await capturedOnMessage?.({
body: "/status",
from: "123@g.us",
conversationId: "123@g.us",
chatId: "123@g.us",
chatType: "group",
to: "+2",
id: "g-status",
senderE164: "+111",
senderName: "Owner",
selfE164: "+999",
sendComposing,
reply,
sendMedia,
});
expect(resolver).toHaveBeenCalledTimes(1);
});
it("passes conversation id through as From for group replies", async () => { it("passes conversation id through as From for group replies", async () => {
const sendMedia = vi.fn(); const sendMedia = vi.fn();
const reply = vi.fn().mockResolvedValue(undefined); const reply = vi.fn().mockResolvedValue(undefined);

View File

@@ -1,10 +1,11 @@
import { hasControlCommand } from "../../../auto-reply/command-detection.js";
import { parseActivationCommand } from "../../../auto-reply/group-activation.js"; import { parseActivationCommand } from "../../../auto-reply/group-activation.js";
import type { loadConfig } from "../../../config/config.js"; import type { loadConfig } from "../../../config/config.js";
import { normalizeE164 } from "../../../utils.js"; import { normalizeE164 } from "../../../utils.js";
import type { MentionConfig } from "../mentions.js"; import type { MentionConfig } from "../mentions.js";
import { buildMentionConfig, debugMention, resolveOwnerList } from "../mentions.js"; import { buildMentionConfig, debugMention, resolveOwnerList } from "../mentions.js";
import type { WebInboundMsg } from "../types.js"; import type { WebInboundMsg } from "../types.js";
import { isStatusCommand, stripMentionsForCommand } from "./commands.js"; import { stripMentionsForCommand } from "./commands.js";
import { resolveGroupActivationFor, resolveGroupPolicyFor } from "./group-activation.js"; import { resolveGroupActivationFor, resolveGroupPolicyFor } from "./group-activation.js";
import { noteGroupMember } from "./group-members.js"; import { noteGroupMember } from "./group-members.js";
@@ -59,8 +60,7 @@ export function applyGroupGating(params: {
); );
const activationCommand = parseActivationCommand(commandBody); const activationCommand = parseActivationCommand(commandBody);
const owner = isOwnerSender(params.baseMentionConfig, params.msg); const owner = isOwnerSender(params.baseMentionConfig, params.msg);
const statusCommand = isStatusCommand(commandBody); const shouldBypassMention = owner && hasControlCommand(commandBody, params.cfg);
const shouldBypassMention = owner && (activationCommand.hasCommand || statusCommand);
if (activationCommand.hasCommand && !owner) { if (activationCommand.hasCommand && !owner) {
params.logVerbose(`Ignoring /activation from non-owner in group ${params.conversationId}`); params.logVerbose(`Ignoring /activation from non-owner in group ${params.conversationId}`);