mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-23 02:45:38 -06:00
fix(whatsapp): honor disabled self-chat without widening group access (#118711)
This commit is contained in:
committed by
GitHub
parent
eb76bf499b
commit
09e40d0b74
@@ -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;
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user