mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(whatsapp): gate owner-typed forwards on customer chatId allowlist
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).
This commit is contained in:
parent
a61cf774ce
commit
db52ad0f07
3 changed files with 208 additions and 3 deletions
|
|
@ -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.
|
||||
|
|
|
|||
56
scripts/whatsapp-bridge/owner_message_gate.js
Normal file
56
scripts/whatsapp-bridge/owner_message_gate.js
Normal file
|
|
@ -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' };
|
||||
}
|
||||
126
scripts/whatsapp-bridge/owner_message_gate.test.mjs
Normal file
126
scripts/whatsapp-bridge/owner_message_gate.test.mjs
Normal file
|
|
@ -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' });
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue