ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-26 20:35:39 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(whatsapp): harden direct identity ownership

Browse Source
This commit is contained in:
joshavant
2026-08-11 20:42:46 -05:00
parent 156e385898
commit 40797ccd45
9 changed files with 316 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/whatsapp/src/direct-peer-owner.test.ts
+97 -2
View File
@@ -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) => {
extensions/whatsapp/src/direct-peer-owner.ts
+68 -57
View File
@@ -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<StoredDirectPeerOwner>({
return getWhatsAppRuntime().state.openKeyedStore<unknown>({
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<WhatsAppDirectPeerResolution | null> {
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<void> {
@@ -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)),
);
}
extensions/whatsapp/src/inbound-policy.ts
+3 -1
View File
@@ -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,
extensions/whatsapp/src/inbound/access-control.identity.test.ts
+127
View File
@@ -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 });
});
});
extensions/whatsapp/src/inbound/access-control.test.ts
-23
View File
@@ -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: {
extensions/whatsapp/src/inbound/access-control.ts
+5 -5
View File
@@ -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,
}),
};
}
extensions/whatsapp/src/inbound/admission.ts
+3 -1
View File
@@ -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,
extensions/whatsapp/src/normalize-target.ts
+2 -2
View File
@@ -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);
extensions/whatsapp/src/resolve-target.test.ts
+11
View File
@@ -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");