From 57b4865ab31c2ca79370d6b98840915be397c41e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 14 Jan 2026 23:24:21 +0000 Subject: [PATCH] fix(whatsapp): normalize user JIDs for group allowlists (#838) Thanks @peschee. Co-authored-by: Peter Siska <63866+peschee@users.noreply.github.com> --- CHANGELOG.md | 1 + src/auto-reply/command-auth.test.ts | 27 ++- src/auto-reply/command-auth.ts | 45 +++-- .../reply/session-reset-group.test.ts | 172 ++++++++++++++++++ src/whatsapp/normalize.test.ts | 35 +++- src/whatsapp/normalize.ts | 31 ++++ 6 files changed, 297 insertions(+), 14 deletions(-) create mode 100644 src/auto-reply/reply/session-reset-group.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eaeaf382..96d64e8c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Slack: drop Socket Mode events with mismatched `api_app_id`/`team_id`. (#889) — thanks @roshanasingh4. - Discord: isolate autoThread thread context. (#856) — thanks @davidguttman. - WhatsApp: fix context isolation using wrong ID (was bot's number, now conversation ID). (#911) — thanks @tristanmanchester. +- WhatsApp: normalize user JIDs with device suffix for allowlist checks in groups. (#838) — thanks @peschee. ## 2026.1.13 diff --git a/src/auto-reply/command-auth.test.ts b/src/auto-reply/command-auth.test.ts index a09be0139..abc770eb2 100644 --- a/src/auto-reply/command-auth.test.ts +++ b/src/auto-reply/command-auth.test.ts @@ -30,7 +30,7 @@ describe("resolveCommandAuthorization", () => { it("falls back from whitespace SenderId to SenderE164", () => { const cfg = { - whatsapp: { allowFrom: ["+123"] }, + channels: { whatsapp: { allowFrom: ["+123"] } }, } as ClawdbotConfig; const ctx = { @@ -53,7 +53,7 @@ describe("resolveCommandAuthorization", () => { it("falls back to From when SenderId and SenderE164 are whitespace", () => { const cfg = { - whatsapp: { allowFrom: ["+999"] }, + channels: { whatsapp: { allowFrom: ["+999"] } }, } as ClawdbotConfig; const ctx = { @@ -73,4 +73,27 @@ describe("resolveCommandAuthorization", () => { expect(auth.senderId).toBe("+999"); expect(auth.isAuthorizedSender).toBe(true); }); + + it("falls back from un-normalizable SenderId to SenderE164", () => { + const cfg = { + channels: { whatsapp: { allowFrom: ["+123"] } }, + } as ClawdbotConfig; + + const ctx = { + Provider: "whatsapp", + Surface: "whatsapp", + From: "whatsapp:+999", + SenderId: "wat", + SenderE164: "+123", + } as MsgContext; + + const auth = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + expect(auth.senderId).toBe("+123"); + expect(auth.isAuthorizedSender).toBe(true); + }); }); diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 39c752bc5..f6b903e08 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -14,6 +14,31 @@ export type CommandAuthorization = { to?: string; }; +function resolveSenderId(params: { + dock?: ChannelDock; + cfg: ClawdbotConfig; + accountId?: string | null; + ctx: MsgContext; +}): string | undefined { + const { dock, cfg, accountId, ctx } = params; + + const senderCandidates = [ctx.SenderId, ctx.SenderE164, ctx.From] + .map((value) => (value ?? "").trim()) + .filter(Boolean); + + for (const sender of senderCandidates) { + const normalized = formatAllowFromList({ + dock, + cfg, + accountId, + allowFrom: [sender], + })[0]; + if (normalized) return normalized; + } + + return undefined; +} + function resolveProviderFromContext(ctx: MsgContext, cfg: ClawdbotConfig): ChannelId | undefined { const direct = normalizeChannelId(ctx.Provider) ?? @@ -90,17 +115,15 @@ export function resolveCommandAuthorization(params: { } const ownerList = ownerCandidates; - const senderIdCandidate = ctx.SenderId?.trim() ?? ""; - const senderE164Candidate = ctx.SenderE164?.trim() ?? ""; - const senderRaw = senderIdCandidate || senderE164Candidate || from; - const senderId = senderRaw - ? formatAllowFromList({ - dock, - cfg, - accountId: ctx.AccountId, - allowFrom: [senderRaw], - })[0] - : undefined; + const senderId = resolveSenderId({ + dock, + cfg, + accountId: ctx.AccountId, + ctx: { + ...ctx, + From: from, + }, + }); const enforceOwner = Boolean(dock?.commands?.enforceOwnerForCommands); const isOwner = diff --git a/src/auto-reply/reply/session-reset-group.test.ts b/src/auto-reply/reply/session-reset-group.test.ts new file mode 100644 index 000000000..049928ec8 --- /dev/null +++ b/src/auto-reply/reply/session-reset-group.test.ts @@ -0,0 +1,172 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import type { ClawdbotConfig } from "../../config/config.js"; +import { initSessionState } from "./session.js"; + +describe("initSessionState reset triggers in WhatsApp groups", () => { + async function createStorePath(prefix: string): Promise { + const root = await fs.mkdtemp( + path.join(os.tmpdir(), prefix), + ); + return path.join(root, "sessions.json"); + } + + async function seedSessionStore(params: { + storePath: string; + sessionKey: string; + sessionId: string; + }): Promise { + const { saveSessionStore } = await import("../../config/sessions.js"); + await saveSessionStore(params.storePath, { + [params.sessionKey]: { + sessionId: params.sessionId, + updatedAt: Date.now(), + }, + }); + } + + function makeCfg(params: { storePath: string; allowFrom: string[] }): ClawdbotConfig { + return { + session: { store: params.storePath, idleMinutes: 999 }, + channels: { + whatsapp: { + allowFrom: params.allowFrom, + groupPolicy: "open", + }, + }, + } as ClawdbotConfig; + } + + it("Reset trigger /new works for authorized sender in WhatsApp group", async () => { + const storePath = await createStorePath("clawdbot-group-reset-"); + 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"], + }); + + // Group message context matching what WhatsApp handler creates + const groupMessageCtx = { + Body: `[Chat messages since your last reply - for context]\\n[WhatsApp 120363406150318674@g.us 2026-01-13T07:45Z] Someone: hello\\n\\n[Current message - respond to this]\\n[WhatsApp 120363406150318674@g.us 2026-01-13T07:45Z] Peschiño: /new\\n[from: Peschiño (+41796666864)]`, + RawBody: "/new", + CommandBody: "/new", + From: "120363406150318674@g.us", + To: "+41779241027", + ChatType: "group", + SessionKey: sessionKey, + Provider: "whatsapp", + Surface: "whatsapp", + SenderName: "Peschiño", + SenderE164: "+41796666864", + SenderId: "41796666864:0@s.whatsapp.net", + }; + + const result = await initSessionState({ + ctx: groupMessageCtx, + cfg, + commandAuthorized: true, + }); + + // The reset should be detected + 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 for unauthorized sender in existing session", async () => { + const storePath = await createStorePath("clawdbot-group-reset-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"], + }); + + // Group message from different sender (not in allowFrom) + const groupMessageCtx = { + Body: `[Context]\\n[WhatsApp ...] OtherPerson: /new\\n[from: OtherPerson (+1555123456)]`, + RawBody: "/new", + CommandBody: "/new", + From: "120363406150318674@g.us", + To: "+41779241027", + ChatType: "group", + SessionKey: sessionKey, + Provider: "whatsapp", + Surface: "whatsapp", + SenderName: "OtherPerson", + SenderE164: "+1555123456", // Different sender (not authorized) + SenderId: "1555123456:0@s.whatsapp.net", + }; + + const result = await initSessionState({ + ctx: groupMessageCtx, + cfg, + commandAuthorized: true, + }); + + // Reset should NOT be triggered for unauthorized sender - session ID should stay the same + expect(result.triggerBodyNormalized).toBe("/new"); + expect(result.sessionId).toBe(existingSessionId); // Session should NOT change + expect(result.isNewSession).toBe(false); + }); + + it("Reset trigger works when RawBody is clean but Body has wrapped context", async () => { + const storePath = await createStorePath("clawdbot-group-rawbody-"); + const sessionKey = "agent:main:whatsapp:group:G1"; + const existingSessionId = "existing-session-123"; + await seedSessionStore({ + storePath, + sessionKey, + sessionId: existingSessionId, + }); + + const cfg = makeCfg({ + storePath, + allowFrom: ["*"], + }); + + const groupMessageCtx = { + // Body is wrapped with context prefixes + Body: `[WhatsApp 120363406150318674@g.us 2026-01-13T07:45Z] Jake: /new\n[from: Jake (+1222)]`, + // RawBody is clean + RawBody: "/new", + CommandBody: "/new", + From: "120363406150318674@g.us", + To: "+1111", + ChatType: "group", + SessionKey: sessionKey, + Provider: "whatsapp", + SenderE164: "+1222", + }; + + 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(""); + }); +}); diff --git a/src/whatsapp/normalize.test.ts b/src/whatsapp/normalize.test.ts index 28b3b34c6..3936b4098 100644 --- a/src/whatsapp/normalize.test.ts +++ b/src/whatsapp/normalize.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { isWhatsAppGroupJid, normalizeWhatsAppTarget } from "./normalize.js"; +import { + isWhatsAppGroupJid, + isWhatsAppUserJid, + normalizeWhatsAppTarget, +} from "./normalize.js"; describe("normalizeWhatsAppTarget", () => { it("preserves group JIDs", () => { @@ -24,6 +28,25 @@ describe("normalizeWhatsAppTarget", () => { expect(normalizeWhatsAppTarget("1555123@s.whatsapp.net")).toBe("+1555123"); }); + it("normalizes user JIDs with device suffix to E.164", () => { + // This is the bug fix: JIDs like "41796666864:0@s.whatsapp.net" should + // normalize to "+41796666864", not "+417966668640" (extra digit from ":0") + expect(normalizeWhatsAppTarget("41796666864:0@s.whatsapp.net")).toBe( + "+41796666864", + ); + expect(normalizeWhatsAppTarget("1234567890:123@s.whatsapp.net")).toBe( + "+1234567890", + ); + // Without device suffix still works + expect(normalizeWhatsAppTarget("41796666864@s.whatsapp.net")).toBe( + "+41796666864", + ); + }); + + it("normalizes LID JIDs to E.164", () => { + expect(normalizeWhatsAppTarget("123456789@lid")).toBe("+123456789"); + }); + it("rejects invalid targets", () => { expect(normalizeWhatsAppTarget("wat")).toBeNull(); expect(normalizeWhatsAppTarget("whatsapp:")).toBeNull(); @@ -37,6 +60,16 @@ describe("normalizeWhatsAppTarget", () => { }); }); +describe("isWhatsAppUserJid", () => { + it("detects user JIDs with various formats", () => { + expect(isWhatsAppUserJid("41796666864:0@s.whatsapp.net")).toBe(true); + expect(isWhatsAppUserJid("1234567890@s.whatsapp.net")).toBe(true); + expect(isWhatsAppUserJid("123456789@lid")).toBe(true); + expect(isWhatsAppUserJid("123456789-987654321@g.us")).toBe(false); + expect(isWhatsAppUserJid("+1555123")).toBe(false); + }); +}); + describe("isWhatsAppGroupJid", () => { it("detects group JIDs with or without prefixes", () => { expect(isWhatsAppGroupJid("120363401234567890@g.us")).toBe(true); diff --git a/src/whatsapp/normalize.ts b/src/whatsapp/normalize.ts index 8fa18677b..a970fd298 100644 --- a/src/whatsapp/normalize.ts +++ b/src/whatsapp/normalize.ts @@ -1,5 +1,8 @@ import { normalizeE164 } from "../utils.js"; +const WHATSAPP_USER_JID_RE = /^(\d+)(?::\d+)?@s\.whatsapp\.net$/i; +const WHATSAPP_LID_RE = /^(\d+)@lid$/i; + function stripWhatsAppTargetPrefixes(value: string): string { let candidate = value.trim(); for (;;) { @@ -21,6 +24,27 @@ export function isWhatsAppGroupJid(value: string): boolean { return /^[0-9]+(-[0-9]+)*$/.test(localPart); } +/** + * Check if value looks like a WhatsApp user JID (e.g. "41796666864:0@s.whatsapp.net" or "123@lid"). + */ +export function isWhatsAppUserJid(value: string): boolean { + const candidate = stripWhatsAppTargetPrefixes(value); + return WHATSAPP_USER_JID_RE.test(candidate) || WHATSAPP_LID_RE.test(candidate); +} + +/** + * Extract the phone number from a WhatsApp user JID. + * "41796666864:0@s.whatsapp.net" -> "41796666864" + * "123456@lid" -> "123456" + */ +function extractUserJidPhone(jid: string): string | null { + const userMatch = jid.match(WHATSAPP_USER_JID_RE); + if (userMatch) return userMatch[1]; + const lidMatch = jid.match(WHATSAPP_LID_RE); + if (lidMatch) return lidMatch[1]; + return null; +} + export function normalizeWhatsAppTarget(value: string): string | null { const candidate = stripWhatsAppTargetPrefixes(value); if (!candidate) return null; @@ -28,6 +52,13 @@ export function normalizeWhatsAppTarget(value: string): string | null { const localPart = candidate.slice(0, candidate.length - "@g.us".length); return `${localPart}@g.us`; } + // Handle user JIDs (e.g. "41796666864:0@s.whatsapp.net") + if (isWhatsAppUserJid(candidate)) { + const phone = extractUserJidPhone(candidate); + if (!phone) return null; + const normalized = normalizeE164(phone); + return normalized.length > 1 ? normalized : null; + } const normalized = normalizeE164(candidate); return normalized.length > 1 ? normalized : null; }