From db52ad0f07ae1b6d17721bd9ed6bb73665346cf1 Mon Sep 17 00:00:00 2001 From: Keira Voss Date: Mon, 27 Apr 2026 17:51:49 +0800 Subject: [PATCH] fix(whatsapp): gate owner-typed forwards on customer chatId allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The opt-in WHATSAPP_FORWARD_OWNER_MESSAGES path in bot mode marks fromMe inbound messages as fromOwner: true and forwards them to the Python adapter so plugins can detect "owner just typed in this chat" and trigger handover / sliding TTL flows. The previous implementation bypassed the allowlist for that path: the existing allowlist gate at the bottom of the dispatch loop is guarded by !msg.key.fromMe, so any chat the operator happened to reply to was forwarded — even ones not on WHATSAPP_ALLOWED_USERS. Concretely, on a deployment with a single allowlisted customer, an owner reply in any other chat would still wake Hermes and let the gateway-policy plugin's owner-implicit branch create a stray handover row keyed by the non-allowlisted chatId. Fix: extract the bot-mode fromMe gate into a small pure helper (`owner_message_gate.js`) that returns one of {drop_echo, drop_disabled, drop_allowlist, forward_owner, pass} so the new allowlist branch can be unit-tested without spinning up Baileys. The check runs against the customer chatId (not senderId, which is the owner's own number/LID and won't be on the allowlist by construction). matchesAllowedUser already short-circuits true on an empty allowlist or "*", so deployments without an allowlist see no behavior change. Self-chat mode is untouched — its existing isSelfChat pin is the correct guard there. Tests: scripts/whatsapp-bridge/owner_message_gate.test.mjs covers echo drop, disabled drop, the new allowlist drop, the forward path, the open-allowlist short-circuit, and the precedence of echo/disabled checks over the allowlist check (so logs stay honest). --- scripts/whatsapp-bridge/bridge.js | 29 +++- scripts/whatsapp-bridge/owner_message_gate.js | 56 ++++++++ .../owner_message_gate.test.mjs | 126 ++++++++++++++++++ 3 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 scripts/whatsapp-bridge/owner_message_gate.js create mode 100644 scripts/whatsapp-bridge/owner_message_gate.test.mjs diff --git a/scripts/whatsapp-bridge/bridge.js b/scripts/whatsapp-bridge/bridge.js index b3b256d76c8..2d372bd344f 100644 --- a/scripts/whatsapp-bridge/bridge.js +++ b/scripts/whatsapp-bridge/bridge.js @@ -31,6 +31,7 @@ import { tmpdir } from 'os'; import qrcode from 'qrcode-terminal'; import { matchesAllowedUser, parseAllowedUsers } from './allowlist.js'; import { createOutboundIdTracker } from './outbound_ids.js'; +import { classifyOwnerMessageGate } from './owner_message_gate.js'; // Parse CLI args const args = process.argv.slice(2); @@ -334,9 +335,31 @@ async function startSocket() { // // We always drop (a). We drop (b) too unless the operator opts in // via WHATSAPP_FORWARD_OWNER_MESSAGES so existing deployments see - // no behavior change. - if (recentlySentIds.has(msg.key.id)) continue; - if (!FORWARD_OWNER_MESSAGES) continue; + // no behavior change. When opted in, we still gate on the + // customer chatId allowlist — without that gate, any contact + // the owner replied to would leak into Hermes and trigger + // implicit handover. See `owner_message_gate.js`. + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: FORWARD_OWNER_MESSAGES, + recentlySent: recentlySentIds, + allowlistMatches: (id) => matchesAllowedUser(id, ALLOWED_USERS, SESSION_DIR), + messageId: msg.key.id, + chatId, + }); + if (decision.action === 'drop_echo') continue; + if (decision.action === 'drop_disabled') continue; + if (decision.action === 'drop_allowlist') { + try { + console.log(JSON.stringify({ + event: 'ignored', + reason: 'allowlist_mismatch_owner_chat', + chatId, + senderId, + })); + } catch {} + continue; + } fromOwner = true; } else { // Self-chat mode: only allow messages in the user's own self-chat. diff --git a/scripts/whatsapp-bridge/owner_message_gate.js b/scripts/whatsapp-bridge/owner_message_gate.js new file mode 100644 index 00000000000..52f3fc6bb7f --- /dev/null +++ b/scripts/whatsapp-bridge/owner_message_gate.js @@ -0,0 +1,56 @@ +/** + * Pure classifier for the WhatsApp bridge's bot-mode dispatch loop. + * + * Centralises the "should this fromMe message be forwarded as fromOwner?" + * decision so the gate can be unit-tested without spinning up Baileys or + * the Express server. + * + * Lives next to `outbound_ids.js` rather than inline in `bridge.js` + * because the previous implementation accidentally bypassed the + * customer-side allowlist when forwarding owner-typed messages — see + * the regression test in `owner_message_gate.test.mjs`. + * + * Caller responsibilities: + * - Only invoke in bot mode. Self-chat mode has its own self-chat + * pinning logic and must not delegate here. + * - Pre-filter group / status JIDs (the gate doesn't know about them). + * - On `drop_allowlist`, log the rejection so operators can audit + * accidental allowlist mismatches. + * + * Returned actions: + * - 'pass' : non-fromMe, fall through to existing handling + * - 'drop_echo' : fromMe and matches a recently-sent /send id + * - 'drop_disabled' : fromMe but operator hasn't opted into forwarding + * - 'drop_allowlist' : fromMe and the *customer chatId* isn't on the + * allowlist (owner-typed reply to a stranger) + * - 'forward_owner' : fromMe, owner-typed, allowlisted — forward with + * fromOwner: true + */ + +export function classifyOwnerMessageGate({ + fromMe, + fromOwnerEnabled, + recentlySent, + allowlistMatches, + messageId, + chatId, +}) { + if (!fromMe) { + return { action: 'pass' }; + } + if (recentlySent && recentlySent.has(messageId)) { + return { action: 'drop_echo' }; + } + if (!fromOwnerEnabled) { + return { action: 'drop_disabled' }; + } + // Allowlist gate: check the *customer* chatId, not the sender. The + // sender is the owner's own number/LID and won't be on the allowlist + // by construction. Without this check, any contact the owner happens + // to reply to leaks into Hermes and triggers implicit handover in the + // gateway-policy plugin. + if (typeof allowlistMatches === 'function' && !allowlistMatches(chatId)) { + return { action: 'drop_allowlist' }; + } + return { action: 'forward_owner' }; +} diff --git a/scripts/whatsapp-bridge/owner_message_gate.test.mjs b/scripts/whatsapp-bridge/owner_message_gate.test.mjs new file mode 100644 index 00000000000..dc3820d0e6b --- /dev/null +++ b/scripts/whatsapp-bridge/owner_message_gate.test.mjs @@ -0,0 +1,126 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +import { classifyOwnerMessageGate } from './owner_message_gate.js'; + +function makeRecentlySent(ids = []) { + const set = new Set(ids); + return { has: (id) => set.has(id) }; +} + +function makeAllowlist(allowedChatIds) { + if (allowedChatIds === '*') { + return () => true; + } + const set = new Set(allowedChatIds); + return (id) => set.has(id); +} + +test('non-fromMe messages always pass through', () => { + const decision = classifyOwnerMessageGate({ + fromMe: false, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(), + allowlistMatches: makeAllowlist([]), + messageId: 'M1', + chatId: '6281234567890@s.whatsapp.net', + }); + assert.deepEqual(decision, { action: 'pass' }); +}); + +test('fromMe echo of our own /send is dropped', () => { + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(['M-OWN-1']), + allowlistMatches: makeAllowlist('*'), + messageId: 'M-OWN-1', + chatId: '6281234567890@s.whatsapp.net', + }); + assert.deepEqual(decision, { action: 'drop_echo' }); +}); + +test('fromMe is dropped when forwarding is disabled', () => { + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: false, + recentlySent: makeRecentlySent(), + allowlistMatches: makeAllowlist('*'), + messageId: 'M-OWN-2', + chatId: '6281234567890@s.whatsapp.net', + }); + assert.deepEqual(decision, { action: 'drop_disabled' }); +}); + +test('fromMe is dropped when chatId is not on the allowlist (regression)', () => { + // This is the bug. Before the fix, an owner reply in a non-allowlisted + // chat was still forwarded with fromOwner: true, which made the + // gateway-policy owner-implicit branch create stray handover rows for + // the non-allowlisted contact. + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(), + allowlistMatches: makeAllowlist(['6281234567890@s.whatsapp.net']), + messageId: 'M-OWN-3', + chatId: '111600547700784@lid', + }); + assert.deepEqual(decision, { action: 'drop_allowlist' }); +}); + +test('fromMe is forwarded as owner when chatId is allowlisted', () => { + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(), + allowlistMatches: makeAllowlist(['6281234567890@s.whatsapp.net']), + messageId: 'M-OWN-4', + chatId: '6281234567890@s.whatsapp.net', + }); + assert.deepEqual(decision, { action: 'forward_owner' }); +}); + +test('open-allowlist (matchesAllowedUser short-circuits true) forwards as owner', () => { + // matchesAllowedUser returns true on empty allowlist or "*"; the gate + // must respect that so deployments without an allowlist are unaffected + // by the new check. + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(), + allowlistMatches: () => true, + messageId: 'M-OWN-5', + chatId: '111600547700784@lid', + }); + assert.deepEqual(decision, { action: 'forward_owner' }); +}); + +test('echo check fires before allowlist check', () => { + // A bot-API echo whose chatId happens to be off-allowlist should still + // be dropped as drop_echo, not drop_allowlist, so logging stays + // honest about the actual reason. + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: true, + recentlySent: makeRecentlySent(['M-ECHO-1']), + allowlistMatches: makeAllowlist([]), + messageId: 'M-ECHO-1', + chatId: '111600547700784@lid', + }); + assert.deepEqual(decision, { action: 'drop_echo' }); +}); + +test('disabled flag fires before allowlist check', () => { + // Pre-existing deployments with WHATSAPP_FORWARD_OWNER_MESSAGES unset + // must see drop_disabled regardless of allowlist state, otherwise + // every fromMe message would log a misleading allowlist_mismatch. + const decision = classifyOwnerMessageGate({ + fromMe: true, + fromOwnerEnabled: false, + recentlySent: makeRecentlySent(), + allowlistMatches: makeAllowlist([]), + messageId: 'M-OWN-6', + chatId: '111600547700784@lid', + }); + assert.deepEqual(decision, { action: 'drop_disabled' }); +});