diff --git a/extensions/whatsapp/src/direct-peer-owner.test.ts b/extensions/whatsapp/src/direct-peer-owner.test.ts index 2bde2967b64a..1659ab303617 100644 --- a/extensions/whatsapp/src/direct-peer-owner.test.ts +++ b/extensions/whatsapp/src/direct-peer-owner.test.ts @@ -77,13 +77,71 @@ describe("direct-peer owner", () => { setWhatsAppRuntime({ state: { openKeyedStore } } as never); }); - it("keeps mapped-first peers owned by their E164 identity", async () => { + it("persists mapped-first E164 ownership across no-match and runtime reuse", async () => { await expect( resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: mapped }), ).resolves.toMatchObject({ kind: "resolved", peerId: "+15550001111", e164: "+15550001111" }); expect( Array.from(state.stores.values()).flatMap((store) => Array.from(store.values())), - ).toHaveLength(0); + ).toHaveLength(1); + + setWhatsAppRuntime({ state: { openKeyedStore } } as never); + await expect( + resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: noMatch }), + ).resolves.toMatchObject({ + kind: "resolved", + peerId: "+15550001111", + e164: "+15550001111", + }); + + await expect( + resolveWhatsAppDirectPeer({ + accountId: "default", + jid: "999@lid", + mapping: { + kind: "mapped", + e164: "+15550002222", + evidence: [{ source: "baileys", outcome: "mapped" }], + }, + }), + ).resolves.toMatchObject({ + kind: "resolved", + peerId: "+15550001111", + e164: "+15550001111", + }); + + await expect( + resolveWhatsAppDirectPeer({ + accountId: "default", + jid: "999@lid", + mapping: { + kind: "error", + reason: "mapping-conflict", + distinctValueCount: 2, + evidence: [ + { source: "auth", outcome: "mapped" }, + { source: "baileys", outcome: "mapped" }, + ], + }, + }), + ).resolves.toMatchObject({ kind: "error", error: { code: "mapping-error" } }); + }); + + it("atomically keeps one owner when mapped and opaque first observations race", async () => { + const results = await Promise.all([ + resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: mapped }), + resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: noMatch }), + ]); + + const [first, second] = results; + expect(first).toMatchObject({ kind: "resolved" }); + if (!first || first.kind !== "resolved") { + throw new Error("expected the first owner resolution to succeed"); + } + expect(second).toMatchObject({ kind: "resolved", peerId: first.peerId }); + expect( + Array.from(state.stores.values()).flatMap((store) => Array.from(store.values())), + ).toHaveLength(1); }); it("persists an opaque-first LID owner across later mapping and runtime reuse", async () => { @@ -114,6 +172,23 @@ describe("direct-peer owner", () => { ).resolves.toMatchObject({ kind: "resolved", peerId: "999@lid" }); }); + it("keeps explicit mapping failures retryable after LID ownership is stored", async () => { + await resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: noMatch }); + + await expect( + resolveWhatsAppDirectPeer({ + accountId: "default", + jid: "999@lid", + mapping: { + kind: "error", + reason: "mapping-unavailable", + distinctValueCount: 0, + evidence: [{ source: "baileys", outcome: "error", errorKind: "lookup" }], + }, + }), + ).resolves.toMatchObject({ kind: "error", error: { code: "mapping-error" } }); + }); + it("turns mapping failures into retryable owner errors without state writes", async () => { const result = await resolveWhatsAppDirectPeer({ accountId: "default", @@ -138,6 +213,26 @@ describe("direct-peer owner", () => { ).toHaveLength(0); }); + it("rejects malformed persisted owner variants", async () => { + await resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: noMatch }); + const entry = Array.from(state.stores.values()).flatMap((store) => + Array.from(store.values()), + )[0]; + if (!entry) { + throw new Error("expected a persisted owner"); + } + entry.value = { + accountId: "default", + lid: "999@lid", + owner: "e164", + e164: "signal_:+15550001111", + }; + + await expect( + resolveWhatsAppDirectPeer({ accountId: "default", jid: "999@lid", mapping: noMatch }), + ).resolves.toMatchObject({ kind: "error", error: { code: "owner-state-error" } }); + }); + it.each(["lookup", "register", "pairing"] as const)( "fails closed when %s owner state is unavailable", async (failure) => { diff --git a/extensions/whatsapp/src/direct-peer-owner.ts b/extensions/whatsapp/src/direct-peer-owner.ts index 4f78692c4b96..28823ced760f 100644 --- a/extensions/whatsapp/src/direct-peer-owner.ts +++ b/extensions/whatsapp/src/direct-peer-owner.ts @@ -1,7 +1,9 @@ // Whatsapp plugin module owns stable direct-peer compatibility identity. import { createHash } from "node:crypto"; import { readChannelAllowFromStore } from "openclaw/plugin-sdk/channel-pairing"; +import { isRecord } from "openclaw/plugin-sdk/string-coerce-runtime"; import { normalizeWhatsAppLidJid } from "./identity.js"; +import { normalizeWhatsAppDirectIdentity } from "./normalize-target.js"; import { getWhatsAppRuntime } from "./runtime.js"; import type { WhatsAppJidMappingOutcome } from "./targets-runtime.js"; @@ -11,8 +13,7 @@ const DIRECT_PEER_OWNER_MAX_ENTRIES = 50_000; type StoredDirectPeerOwner = { accountId: string; lid: string; - owner: "lid"; -}; +} & ({ owner: "lid" } | { owner: "e164"; e164: string }); type WhatsAppDirectPeerResolution = | { @@ -47,7 +48,7 @@ export class WhatsAppDirectPeerResolutionError extends Error { } function ownerStore() { - return getWhatsAppRuntime().state.openKeyedStore({ + return getWhatsAppRuntime().state.openKeyedStore({ namespace: DIRECT_PEER_OWNER_NAMESPACE, maxEntries: DIRECT_PEER_OWNER_MAX_ENTRIES, overflowPolicy: "reject-new", @@ -84,28 +85,48 @@ function ownerStateError(cause: unknown): WhatsAppDirectPeerResolution { }; } -async function recordLidOwner(params: { - accountId: string; - lid: string; -}): Promise { - const store = ownerStore(); - const key = ownerKey(params.accountId, params.lid); - try { - const created = await store.registerIfAbsent(key, { - accountId: params.accountId, - lid: params.lid, - owner: "lid", - }); - if (created) { - return null; - } - const existing = await store.lookup(key); - return existing?.owner === "lid" && existing.lid === params.lid - ? null - : ownerStateError(new Error("direct-peer owner record is invalid")); - } catch (error) { - return ownerStateError(error); +function readStoredOwner( + value: unknown, + params: { accountId: string; lid: string }, +): StoredDirectPeerOwner | null { + if ( + !isRecord(value) || + value.accountId !== params.accountId || + value.lid !== params.lid || + (value.owner !== "lid" && value.owner !== "e164") + ) { + return null; } + if (value.owner === "lid") { + return { accountId: params.accountId, lid: params.lid, owner: "lid" }; + } + if ( + typeof value.e164 !== "string" || + !value.e164.startsWith("+") || + normalizeWhatsAppDirectIdentity(value.e164) !== value.e164 + ) { + return null; + } + return { accountId: params.accountId, lid: params.lid, owner: "e164", e164: value.e164 }; +} + +function resolveStoredOwner(params: { + owner: StoredDirectPeerOwner; + mapping: WhatsAppJidMappingOutcome; +}): WhatsAppDirectPeerResolution { + const e164 = + params.owner.owner === "e164" + ? params.owner.e164 + : params.mapping.kind === "mapped" + ? params.mapping.e164 + : null; + return { + kind: "resolved", + peerId: params.owner.owner === "e164" ? params.owner.e164 : params.owner.lid, + lid: params.owner.lid, + e164, + mapping: params.mapping, + }; } export async function resolveWhatsAppDirectPeer(params: { @@ -126,27 +147,20 @@ export async function resolveWhatsAppDirectPeer(params: { }), }; } - const store = ownerStore(); - let stored: StoredDirectPeerOwner | undefined; + let storedValue: unknown; try { - stored = await store.lookup(ownerKey(params.accountId, lid)); + storedValue = await store.lookup(ownerKey(params.accountId, lid)); } catch (error) { return ownerStateError(error); } - if (stored) { - if (stored.owner !== "lid" || stored.lid !== lid || stored.accountId !== params.accountId) { + if (storedValue !== undefined) { + const stored = readStoredOwner(storedValue, { accountId: params.accountId, lid }); + if (!stored) { return ownerStateError(new Error("direct-peer owner record is invalid")); } - return { - kind: "resolved", - peerId: lid, - lid, - e164: params.mapping.kind === "mapped" ? params.mapping.e164 : null, - mapping: params.mapping, - }; + return resolveStoredOwner({ owner: stored, mapping: params.mapping }); } - let pairedEntries: string[]; try { pairedEntries = await readChannelAllowFromStore("whatsapp", process.env, params.accountId); @@ -155,27 +169,24 @@ export async function resolveWhatsAppDirectPeer(params: { } const exactLidPairing = pairedEntries.some((entry) => normalizeWhatsAppLidJid(entry) === lid); - if (params.mapping.kind === "mapped" && !exactLidPairing) { - return { - kind: "resolved", - peerId: params.mapping.e164, + const owner: StoredDirectPeerOwner = + params.mapping.kind === "mapped" && !exactLidPairing + ? { accountId: params.accountId, lid, owner: "e164", e164: params.mapping.e164 } + : { accountId: params.accountId, lid, owner: "lid" }; + try { + if (await store.registerIfAbsent(ownerKey(params.accountId, lid), owner)) { + return resolveStoredOwner({ owner, mapping: params.mapping }); + } + const existing = readStoredOwner(await store.lookup(ownerKey(params.accountId, lid)), { + accountId: params.accountId, lid, - e164: params.mapping.e164, - mapping: params.mapping, - }; + }); + return existing + ? resolveStoredOwner({ owner: existing, mapping: params.mapping }) + : ownerStateError(new Error("direct-peer owner record is invalid")); + } catch (error) { + return ownerStateError(error); } - - const writeError = await recordLidOwner({ accountId: params.accountId, lid }); - if (writeError) { - return writeError; - } - return { - kind: "resolved", - peerId: lid, - lid, - e164: params.mapping.kind === "mapped" ? params.mapping.e164 : null, - mapping: params.mapping, - }; } export async function clearWhatsAppDirectPeerOwners(accountId: string): Promise { @@ -183,7 +194,7 @@ export async function clearWhatsAppDirectPeerOwners(accountId: string): Promise< const entries = await store.entries(); await Promise.all( entries - .filter((entry) => entry.value.accountId === accountId) + .filter((entry) => isRecord(entry.value) && entry.value.accountId === accountId) .map(async (entry) => await store.delete(entry.key)), ); } diff --git a/extensions/whatsapp/src/inbound-policy.ts b/extensions/whatsapp/src/inbound-policy.ts index c250efe18d91..2530b83b6e0c 100644 --- a/extensions/whatsapp/src/inbound-policy.ts +++ b/extensions/whatsapp/src/inbound-policy.ts @@ -123,6 +123,8 @@ export async function resolveWhatsAppIngressAccess(params: { senderJid?: string | null; includeCommand?: boolean; }) { + const senderIsSamePhone = + params.policy.isSamePhone(params.senderId) || params.policy.isSamePhone(params.senderE164); return await resolveStableChannelMessageIngress({ channelId: "whatsapp", accountId: params.policy.account.accountId, @@ -169,7 +171,7 @@ export async function resolveWhatsAppIngressAccess(params: { !params.isGroup && params.policy.account.selfChatMode !== false && params.senderId && - params.policy.isSamePhone(params.senderId) + senderIsSamePhone ? [...params.policy.dmAllowFrom, params.senderId] : params.policy.dmAllowFrom, groupAllowFrom: params.policy.groupAllowFrom, diff --git a/extensions/whatsapp/src/inbound/access-control.identity.test.ts b/extensions/whatsapp/src/inbound/access-control.identity.test.ts new file mode 100644 index 000000000000..20a77c9b1f7c --- /dev/null +++ b/extensions/whatsapp/src/inbound/access-control.identity.test.ts @@ -0,0 +1,127 @@ +// Whatsapp tests cover typed direct identity access decisions. +import { beforeAll, describe, expect, it } from "vitest"; +import type { AcceptedInboundAccessControlResult } from "./access-control.js"; +import { + getAccessControlTestConfig, + sendMessageMock, + setAccessControlTestConfig, + setupAccessControlTestHarness, + upsertPairingRequestMock, +} from "./access-control.test-harness.js"; +import { createTestWebInboundMessage } from "./test-message.test-helper.js"; + +setupAccessControlTestHarness(); +let checkInboundAccessControl: typeof import("./access-control.js").checkInboundAccessControl; +let resolveWhatsAppCommandAuthorized: typeof import("../inbound-policy.js").resolveWhatsAppCommandAuthorized; + +beforeAll(async () => { + ({ checkInboundAccessControl } = await import("./access-control.js")); + ({ resolveWhatsAppCommandAuthorized } = await import("../inbound-policy.js")); +}); + +async function checkMappedLidSelf(params: { selfChatMode?: boolean; senderE164: string }) { + setAccessControlTestConfig({ + channels: { + whatsapp: { + dmPolicy: "pairing", + allowFrom: ["+15550001111"], + ...(params.selfChatMode === undefined ? {} : { selfChatMode: params.selfChatMode }), + }, + }, + }); + return await checkInboundAccessControl({ + cfg: getAccessControlTestConfig() as never, + accountId: "default", + from: "999@lid", + selfE164: "+15550009999", + senderE164: params.senderE164, + senderJid: "999@lid", + group: false, + pushName: "Owner", + isFromMe: true, + sock: { sendMessage: sendMessageMock }, + remoteJid: "999@lid", + }); +} + +describe("typed WhatsApp direct identity access", () => { + it.each([ + "telegram:1555", + "sms:+1555", + "signal_:+1555", + "whatsapp:signal_:+1555", + "signal.:+1555", + "signal/:+1555", + "other channel:+1555", + ])("does not authorize +1555 through foreign identity %s", async (allowFrom) => { + setAccessControlTestConfig({ + channels: { whatsapp: { dmPolicy: "allowlist", allowFrom: [allowFrom] } }, + }); + await expect( + checkInboundAccessControl({ + cfg: getAccessControlTestConfig() as never, + accountId: "default", + from: "+1555", + selfE164: "+1999", + senderE164: "+1555", + senderJid: "1555@s.whatsapp.net", + group: false, + isFromMe: false, + sock: { sendMessage: sendMessageMock }, + remoteJid: "1555@s.whatsapp.net", + }), + ).resolves.toMatchObject({ allowed: false }); + }); + + it.each([ + { name: "omitted", selfChatMode: undefined }, + { name: "enabled", selfChatMode: true }, + ])( + "admits a mapped LID self-chat through its phone alias when self-chat is $name", + async ({ selfChatMode }) => { + const result = await checkMappedLidSelf({ selfChatMode, senderE164: "+15550009999" }); + expect(result.allowed).toBe(true); + if (!result.allowed) { + throw new Error("expected mapped LID self-chat admission"); + } + const accepted: AcceptedInboundAccessControlResult = result; + expect(accepted.admission.sender).toEqual({ id: "999@lid", isSamePhone: true }); + await expect( + resolveWhatsAppCommandAuthorized({ + cfg: getAccessControlTestConfig() as never, + msg: createTestWebInboundMessage({ + event: { id: "cmd-mapped-self-lid" }, + payload: { body: "/status" }, + platform: { + chatJid: "999@lid", + recipientJid: "+15550009999", + sender: { lid: "999@lid", e164: "+15550009999" }, + senderJid: "999@lid", + senderE164: "+15550009999", + selfE164: "+15550009999", + }, + admission: { + accountId: "default", + conversation: { kind: "direct", id: "999@lid" }, + sender: { id: "999@lid" }, + }, + }) as never, + }), + ).resolves.toBe(true); + expect(upsertPairingRequestMock).not.toHaveBeenCalled(); + expect(sendMessageMock).not.toHaveBeenCalled(); + }, + ); + + it("does not grant mapped-LID self access through an unrelated phone alias", async () => { + await expect( + checkMappedLidSelf({ selfChatMode: true, senderE164: "+15550008888" }), + ).resolves.toMatchObject({ allowed: false }); + }); + + it("blocks a mapped-LID self-chat when self-chat mode is disabled", async () => { + await expect( + checkMappedLidSelf({ selfChatMode: false, senderE164: "+15550009999" }), + ).resolves.toMatchObject({ allowed: false }); + }); +}); diff --git a/extensions/whatsapp/src/inbound/access-control.test.ts b/extensions/whatsapp/src/inbound/access-control.test.ts index 1fc465fd17fa..799524ec7c78 100644 --- a/extensions/whatsapp/src/inbound/access-control.test.ts +++ b/extensions/whatsapp/src/inbound/access-control.test.ts @@ -210,29 +210,6 @@ describe("checkInboundAccessControl admission contract", () => { ).resolves.toMatchObject({ allowed: false }); }); - it.each(["telegram:1555", "sms:+1555"])( - "does not authorize a WhatsApp phone through foreign provider identity %s", - async (allowFrom) => { - setAccessControlTestConfig({ - channels: { whatsapp: { dmPolicy: "allowlist", allowFrom: [allowFrom] } }, - }); - await expect( - checkInboundAccessControl({ - cfg: getAccessControlTestConfig() as never, - accountId: "default", - from: "+1555", - selfE164: "+1999", - senderE164: "+1555", - senderJid: "1555@s.whatsapp.net", - group: false, - isFromMe: false, - sock: { sendMessage: sendMessageMock }, - remoteJid: "1555@s.whatsapp.net", - }), - ).resolves.toMatchObject({ allowed: false }); - }, - ); - it("authorizes an exact LID through repeated WhatsApp prefixes", async () => { setAccessControlTestConfig({ channels: { diff --git a/extensions/whatsapp/src/inbound/access-control.ts b/extensions/whatsapp/src/inbound/access-control.ts index b582adf87c97..de9a6fef6d04 100644 --- a/extensions/whatsapp/src/inbound/access-control.ts +++ b/extensions/whatsapp/src/inbound/access-control.ts @@ -105,6 +105,8 @@ export async function checkInboundAccessControl(params: { senderE164: params.senderE164, senderJid: params.senderJid, }); + const senderIsSamePhone = + policy.isSamePhone(params.from) || policy.isSamePhone(params.senderE164); const { senderAccess } = access; if (params.group && senderAccess.decision !== "allow") { if (senderAccess.reasonCode === "group_policy_disabled") { @@ -125,10 +127,7 @@ export async function checkInboundAccessControl(params: { // DM access control (secure defaults): "pairing" (default) / "allowlist" / "open" / "disabled". if (!params.group) { - if ( - params.isFromMe && - (policy.account.selfChatMode === false || !policy.isSamePhone(params.from)) - ) { + if (params.isFromMe && (policy.account.selfChatMode === false || !senderIsSamePhone)) { logWhatsAppVerbose(params.verbose, "Skipping outbound DM (fromMe); no pairing reply needed."); return blockedInboundAccess(policy); } @@ -136,7 +135,7 @@ export async function checkInboundAccessControl(params: { logWhatsAppVerbose(params.verbose, "Blocked dm (dmPolicy: disabled)"); return blockedInboundAccess(policy); } - if (senderAccess.decision === "pairing" && !policy.isSamePhone(params.from)) { + if (senderAccess.decision === "pairing" && !senderIsSamePhone) { const candidate = params.from; const candidateIsLid = normalizeWhatsAppLidJid(candidate) !== null; if (suppressPairingReply) { @@ -200,6 +199,7 @@ export async function checkInboundAccessControl(params: { isGroup: params.group, conversationId, senderId: admissionSenderId, + senderE164: params.senderE164, }), }; } diff --git a/extensions/whatsapp/src/inbound/admission.ts b/extensions/whatsapp/src/inbound/admission.ts index ca1b4f857aa6..84cab8df4ebc 100644 --- a/extensions/whatsapp/src/inbound/admission.ts +++ b/extensions/whatsapp/src/inbound/admission.ts @@ -127,6 +127,7 @@ export function buildWhatsAppInboundAdmission(params: { isGroup: boolean; conversationId: string; senderId: string; + senderE164?: string | null; }): WhatsAppInboundAdmission { return { accountId: params.policy.account.accountId, @@ -139,7 +140,8 @@ export function buildWhatsAppInboundAdmission(params: { }, sender: { id: params.senderId, - isSamePhone: params.policy.isSamePhone(params.senderId), + isSamePhone: + params.policy.isSamePhone(params.senderId) || params.policy.isSamePhone(params.senderE164), }, ingress: { admission: params.access.ingress.admission, diff --git a/extensions/whatsapp/src/normalize-target.ts b/extensions/whatsapp/src/normalize-target.ts index 2de9460c8985..bb6b943ed3bd 100644 --- a/extensions/whatsapp/src/normalize-target.ts +++ b/extensions/whatsapp/src/normalize-target.ts @@ -10,8 +10,8 @@ import { normalizeWhatsAppLidJid } from "./identity.js"; const WHATSAPP_USER_JID_RE = /^(\d+)(?::\d+)?@s\.whatsapp\.net$/i; const WHATSAPP_LEGACY_USER_JID_RE = /^(\d+)@c\.us$/i; const WHATSAPP_LID_RE = /^(\d+)(?::\d+)?@(lid|hosted\.lid)$/i; -const NON_WHATSAPP_PROVIDER_PREFIX_RE = /^[a-z][a-z0-9-]*:/i; const WHATSAPP_NEWSLETTER_JID_RE = /^([0-9]+)@newsletter$/i; +const WHATSAPP_PHONE_INPUT_RE = /^\+?[\d\s().-]+$/; function stripWhatsAppTargetPrefixes(value: string): string { let candidate = value.trim(); @@ -99,7 +99,7 @@ export function normalizeWhatsAppTarget(value: string): string | null { if (candidate.includes("@")) { return null; } - if (NON_WHATSAPP_PROVIDER_PREFIX_RE.test(candidate)) { + if (!WHATSAPP_PHONE_INPUT_RE.test(candidate)) { return null; } const normalized = normalizeE164(candidate); diff --git a/extensions/whatsapp/src/resolve-target.test.ts b/extensions/whatsapp/src/resolve-target.test.ts index 53cb40129a56..92f7c5af4002 100644 --- a/extensions/whatsapp/src/resolve-target.test.ts +++ b/extensions/whatsapp/src/resolve-target.test.ts @@ -90,6 +90,17 @@ describe("normalizeWhatsAppDirectIdentity", () => { expect(normalizeWhatsAppDirectIdentity("sms:+1555")).toBeNull(); }); + it.each([ + "signal_:+1555", + "whatsapp:signal_:+1555", + "signal.:+1555", + "signal/:+1555", + "other channel:+1555", + ])("rejects residual non-phone identity syntax in %s", (value) => { + expect(normalizeWhatsAppTarget(value)).toBeNull(); + expect(normalizeWhatsAppDirectIdentity(value)).toBeNull(); + }); + it("accepts formatted phones and exact direct JIDs", () => { expect(normalizeWhatsAppDirectIdentity("+1 (555) 123-4567")).toBe("+15551234567"); expect(normalizeWhatsAppDirectIdentity("15551234567@s.whatsapp.net")).toBe("+15551234567");