From 09e40d0b74e64f00ee1ed7e755f4b664155c19a3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 3 Aug 2026 07:57:23 -0700 Subject: [PATCH] fix(whatsapp): honor disabled self-chat without widening group access (#118711) --- extensions/whatsapp/src/inbound-policy.ts | 28 ++--- .../src/inbound/access-control.test.ts | 118 +++++++++++++++++- .../whatsapp/src/inbound/access-control.ts | 6 +- .../src/monitor-inbox.access-and-echo.test.ts | 40 ++++++ 4 files changed, 165 insertions(+), 27 deletions(-) diff --git a/extensions/whatsapp/src/inbound-policy.ts b/extensions/whatsapp/src/inbound-policy.ts index 7fe0f9e7df28..ad6e296f16ff 100644 --- a/extensions/whatsapp/src/inbound-policy.ts +++ b/extensions/whatsapp/src/inbound-policy.ts @@ -41,17 +41,6 @@ function normalizeWhatsAppIngressPhone(value: string): string | null { return normalizeE164(trimmed); } -function maybeSamePhoneDmAllowFrom(params: { - isGroup: boolean; - policy: ResolvedWhatsAppInboundPolicy; - dmSenderId?: string | null; -}): string[] { - if (params.isGroup || !params.dmSenderId || !params.policy.isSamePhone(params.dmSenderId)) { - return []; - } - return [params.dmSenderId]; -} - function buildResolvedWhatsAppGroupConfig(params: { groupPolicy: GroupPolicy; groups: ResolvedWhatsAppAccount["groups"]; @@ -131,15 +120,8 @@ export async function resolveWhatsAppIngressAccess(params: { isGroup: boolean; conversationId: string; senderId?: string | null; - dmSenderId?: string | null; includeCommand?: boolean; }) { - const samePhoneDmAllowFrom = maybeSamePhoneDmAllowFrom({ - isGroup: params.isGroup, - policy: params.policy, - dmSenderId: params.dmSenderId, - }); - const dmAllowFrom = [...params.policy.dmAllowFrom, ...samePhoneDmAllowFrom]; return await resolveStableChannelMessageIngress({ channelId: "whatsapp", accountId: params.policy.account.accountId, @@ -163,7 +145,14 @@ export async function resolveWhatsAppIngressAccess(params: { groupAllowFromFallbackToAllowFrom: false, }, providerMissingFallbackApplied: params.policy.providerMissingFallbackApplied, - allowFrom: dmAllowFrom, + // Keep implicit self access direct-only; groups reuse this list for command ownership. + allowFrom: + !params.isGroup && + params.policy.account.selfChatMode !== false && + params.senderId && + params.policy.isSamePhone(params.senderId) + ? [...params.policy.dmAllowFrom, params.senderId] + : params.policy.dmAllowFrom, groupAllowFrom: params.policy.groupAllowFrom, command: params.includeCommand === true ? {} : undefined, }); @@ -203,7 +192,6 @@ export async function resolveWhatsAppCommandAuthorized(params: { isGroup, conversationId: admission.conversation.id, senderId: isGroup ? groupSender : dmSender, - dmSenderId: dmSender, includeCommand: true, }); return access.commandAccess.authorized; diff --git a/extensions/whatsapp/src/inbound/access-control.test.ts b/extensions/whatsapp/src/inbound/access-control.test.ts index d2101d4def20..fcb5bd140bda 100644 --- a/extensions/whatsapp/src/inbound/access-control.test.ts +++ b/extensions/whatsapp/src/inbound/access-control.test.ts @@ -414,12 +414,100 @@ describe("WhatsApp dmPolicy precedence", () => { expect(readAllowFromStoreMock).not.toHaveBeenCalled(); }); - it("always allows same-phone DMs even when allowFrom is restrictive", async () => { + it.each([ + { name: "omitted", selfChatMode: undefined }, + { name: "enabled", selfChatMode: true }, + ])("allows same-phone fromMe DMs when self-chat mode is $name", async ({ selfChatMode }) => { const cfg = { channels: { whatsapp: { dmPolicy: "pairing", allowFrom: ["+15550001111"], + ...(selfChatMode === undefined ? {} : { selfChatMode }), + }, + }, + }; + setAccessControlTestConfig(cfg); + + const result = await checkInboundAccessControl({ + cfg: getAccessControlTestConfig() as never, + accountId: "default", + from: "+15550009999", + selfE164: "+15550009999", + senderE164: "+15550009999", + group: false, + pushName: "Owner", + isFromMe: true, + sock: { sendMessage: sendMessageMock }, + remoteJid: "15550009999@s.whatsapp.net", + }); + const commandAuthorized = await checkCommandAuthorizedForDm({ + cfg, + accountId: "default", + from: "+15550009999", + senderE164: "+15550009999", + selfE164: "+15550009999", + }); + + expect(result.allowed).toBe(true); + expect(commandAuthorized).toBe(true); + expect(upsertPairingRequestMock).not.toHaveBeenCalled(); + expect(sendMessageMock).not.toHaveBeenCalled(); + }); + + it.each([ + { + name: "the default account", + accountId: "default", + whatsapp: { + dmPolicy: "pairing", + allowFrom: ["+15550009999"], + selfChatMode: false, + }, + }, + { + name: "a named account overriding enabled channel self-chat", + accountId: "work", + whatsapp: { + dmPolicy: "pairing", + selfChatMode: true, + accounts: { + work: { + allowFrom: ["+15550009999"], + selfChatMode: false, + }, + }, + }, + }, + ])("blocks allowlisted same-phone fromMe DMs for $name", async ({ accountId, whatsapp }) => { + setAccessControlTestConfig({ channels: { whatsapp } }); + + const result = await checkInboundAccessControl({ + cfg: getAccessControlTestConfig() as never, + accountId, + from: "+15550009999", + selfE164: "+15550009999", + senderE164: "+15550009999", + group: false, + pushName: "Owner", + isFromMe: true, + sock: { sendMessage: sendMessageMock }, + remoteJid: "15550009999@s.whatsapp.net", + }); + + expectSilentlyBlocked(result); + expect(result.shouldMarkRead).toBe(false); + expect(result.isSelfChat).toBe(false); + expect(result.resolvedAccountId).toBe(accountId); + }); + + it("does not implicitly allow the linked phone when self-chat is disabled", async () => { + const cfg = { + channels: { + whatsapp: { + dmPolicy: "pairing", + allowFrom: ["+15550001111"], + selfChatMode: false, }, }, }; @@ -445,10 +533,30 @@ describe("WhatsApp dmPolicy precedence", () => { selfE164: "+15550009999", }); - expect(result.allowed).toBe(true); - expect(commandAuthorized).toBe(true); - expect(upsertPairingRequestMock).not.toHaveBeenCalled(); - expect(sendMessageMock).not.toHaveBeenCalled(); + expectSilentlyBlocked(result); + expect(commandAuthorized).toBe(false); + }); + + it("does not grant group command ownership through implicit linked-phone access", async () => { + const cfg = { + channels: { + whatsapp: { + dmPolicy: "pairing", + groupPolicy: "open", + allowFrom: ["+15550001111"], + }, + }, + }; + setAccessControlTestConfig(cfg); + + expect( + await checkCommandAuthorizedForGroup({ + cfg, + accountId: "default", + senderE164: "+15550009999", + selfE164: "+15550009999", + }), + ).toBe(false); }); it("allows DMs from generic message sender access groups", async () => { diff --git a/extensions/whatsapp/src/inbound/access-control.ts b/extensions/whatsapp/src/inbound/access-control.ts index 0cb5876d20e3..81f3d61db14c 100644 --- a/extensions/whatsapp/src/inbound/access-control.ts +++ b/extensions/whatsapp/src/inbound/access-control.ts @@ -101,7 +101,6 @@ export async function checkInboundAccessControl(params: { isGroup: params.group, conversationId, senderId: accessSenderId, - dmSenderId: params.from, }); const { senderAccess } = access; if (params.group && senderAccess.decision !== "allow") { @@ -123,7 +122,10 @@ export async function checkInboundAccessControl(params: { // DM access control (secure defaults): "pairing" (default) / "allowlist" / "open" / "disabled". if (!params.group) { - if (params.isFromMe && !policy.isSamePhone(params.from)) { + if ( + params.isFromMe && + (policy.account.selfChatMode === false || !policy.isSamePhone(params.from)) + ) { logWhatsAppVerbose(params.verbose, "Skipping outbound DM (fromMe); no pairing reply needed."); return blockedInboundAccess(policy); } diff --git a/extensions/whatsapp/src/monitor-inbox.access-and-echo.test.ts b/extensions/whatsapp/src/monitor-inbox.access-and-echo.test.ts index 1629e1de7440..8d40e3ab1176 100644 --- a/extensions/whatsapp/src/monitor-inbox.access-and-echo.test.ts +++ b/extensions/whatsapp/src/monitor-inbox.access-and-echo.test.ts @@ -170,6 +170,45 @@ describe("web monitor inbox", () => { await listener.close(); }); + it("blocks allowlisted same-phone fromMe DMs when self-chat mode is disabled", async () => { + mockLoadConfig.mockReturnValue({ + channels: { + whatsapp: { + dmPolicy: "pairing", + allowFrom: ["+123"], + selfChatMode: false, + }, + }, + messages: DEFAULT_MESSAGES_CFG, + }); + const { onMessage, listener, sock } = await openInboxMonitor(); + + try { + sock.ev.emit("messages.upsert", { + type: "notify", + messages: [ + { + key: { + id: "self-disabled", + fromMe: true, + remoteJid: "123@s.whatsapp.net", + }, + message: { conversation: "disabled self-chat" }, + messageTimestamp: nowSeconds(), + }, + ], + }); + await settleInboundWork(); + + expect(onMessage).not.toHaveBeenCalled(); + expect(upsertPairingRequestMock).not.toHaveBeenCalled(); + expect(sock.sendMessage).not.toHaveBeenCalled(); + expect(sock.readMessages).not.toHaveBeenCalled(); + } finally { + await listener.close(); + } + }); + it("locks down when no config is present (pairing for unknown senders)", async () => { // No config file => locked-down defaults apply (pairing for unknown senders) mockLoadConfig.mockReturnValue({}); @@ -289,6 +328,7 @@ describe("web monitor inbox", () => { whatsapp: { groupPolicy: "open", allowFrom: ["+123"], + selfChatMode: false, }, }, messages: DEFAULT_MESSAGES_CFG,