From 8f3660a099f7689d41d144481c914fa1d1a15d09 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 23:04:49 +0000 Subject: [PATCH] fix(history): use OR instead of AND for JID validation in extractLidPnFromMessage Address PR #20 code review feedback: - Fix logical bug: changed `&&` to `||` in isPersonJid validation This ensures BOTH JIDs must be person JIDs before extracting a LID-PN mapping, preventing "poisoned" mappings where one side is a group/newsletter/broadcast - Add isPersonJid() helper to validate JID types - Add extractLidPnFromConversation() to extract mappings from conversation lidJid/pnJid properties - Add extractLidPnFromMessage() to extract mappings from message remoteJidAlt/participantAlt fields - Update processHistoryMessage() to use new extraction functions - Add comprehensive tests covering edge cases for mixed JID types (person + group, person + broadcast, person + newsletter) Note: Returning `undefined` is the correct TypeScript convention for indicating absence of value (as per TypeScript team guidelines) --- src/Utils/history.ts | 156 ++++++++++++- src/__tests__/Utils/history.test.ts | 327 +++++++++++++++++++++++++++- 2 files changed, 477 insertions(+), 6 deletions(-) diff --git a/src/Utils/history.ts b/src/Utils/history.ts index b8c1a504..a9845bca 100644 --- a/src/Utils/history.ts +++ b/src/Utils/history.ts @@ -6,9 +6,134 @@ import { WAMessageStubType } from '../Types' import { toNumber } from './generics' import { normalizeMessageContent } from './messages' import { downloadContentFromMessage } from './messages-media' +import { + isHostedLidUser, + isHostedPnUser, + isJidBroadcast, + isJidGroup, + isJidNewsletter, + isLidUser, + isPnUser, + jidDecode, + jidNormalizedUser, +} from '../WABinary/jid-utils' const inflatePromise = promisify(inflate) +/** + * Checks if a JID represents a person (individual user) rather than a group, broadcast, or newsletter. + * Only person JIDs (LID or PN formats) can have LID-PN mappings. + */ +export function isPersonJid(jid: string | undefined): boolean { + if(!jid) { + return false + } + + // Groups, broadcasts, and newsletters don't have LID-PN mappings + if(isJidGroup(jid) || isJidBroadcast(jid) || isJidNewsletter(jid)) { + return false + } + + // Only person JIDs (LID or PN formats) can have mappings + return !!( + isLidUser(jid) || + isHostedLidUser(jid) || + isPnUser(jid) || + isHostedPnUser(jid) + ) +} + +/** + * Extracts a LID-PN mapping from conversation data. + * Returns undefined if the conversation doesn't represent a valid person-to-person mapping. + */ +export function extractLidPnFromConversation( + chatId: string, + lidJid: string | undefined | null, + pnJid: string | undefined | null +): LIDMapping | undefined { + // Skip non-person JIDs (groups, broadcasts, newsletters) + if(!isPersonJid(chatId)) { + return undefined + } + + const chatIsLid = isLidUser(chatId) || isHostedLidUser(chatId) + const chatIsPn = isPnUser(chatId) || isHostedPnUser(chatId) + + if(chatIsLid && pnJid) { + return { + lid: jidNormalizedUser(chatId), + pn: jidNormalizedUser(pnJid) + } + } + + if(chatIsPn && lidJid) { + return { + lid: jidNormalizedUser(lidJid), + pn: jidNormalizedUser(chatId) + } + } + + return undefined +} + +/** + * Extracts a LID-PN mapping from message data (remoteJidAlt/participantAlt fields). + * Returns undefined if the message doesn't contain a valid person-to-person mapping. + * + * IMPORTANT: Uses || (OR) to ensure BOTH JIDs are person JIDs before extracting. + * This prevents "poisoned" mappings where one side is a group/newsletter/broadcast. + */ +export function extractLidPnFromMessage( + remoteJid: string | undefined | null, + remoteJidAlt: string | undefined | null, + participant: string | undefined | null, + participantAlt: string | undefined | null +): LIDMapping | undefined { + const primaryJid = participant || remoteJid + const altJid = participantAlt || remoteJidAlt + + if(!primaryJid || !altJid) { + return undefined + } + + // FIXED: Use || (OR) instead of && (AND) + // Both JIDs MUST be person JIDs to create a valid mapping. + // Using && would allow mixed scenarios (e.g., person + group) to proceed, + // resulting in invalid "poisoned" mappings. + if(!isPersonJid(primaryJid) || !isPersonJid(altJid)) { + return undefined + } + + const primaryDecoded = jidDecode(primaryJid) + const altDecoded = jidDecode(altJid) + + if(!primaryDecoded || !altDecoded) { + return undefined + } + + const primaryIsLid = primaryDecoded.server === 'lid' || + primaryDecoded.server === 'hosted.lid' + const altIsLid = altDecoded.server === 'lid' || + altDecoded.server === 'hosted.lid' + + if(primaryIsLid && !altIsLid) { + return { + lid: jidNormalizedUser(primaryJid), + pn: jidNormalizedUser(altJid) + } + } + + if(!primaryIsLid && altIsLid) { + return { + lid: jidNormalizedUser(altJid), + pn: jidNormalizedUser(primaryJid) + } + } + + return undefined +} + export const downloadHistory = async (msg: proto.Message.IHistorySyncNotification, options: RequestInit) => { const stream = await downloadContentFromMessage(msg, 'md-msg-hist', { options }) const bufferArray: Buffer[] = [] @@ -42,7 +167,7 @@ export const processHistoryMessage = (item: proto.IHistorySync) => { case proto.HistorySync.HistorySyncType.RECENT: case proto.HistorySync.HistorySyncType.FULL: case proto.HistorySync.HistorySyncType.ON_DEMAND: - for (const chat of item.conversations! as Chat[]) { + for(const chat of item.conversations! as Chat[]) { contacts.push({ id: chat.id!, name: chat.name || undefined, @@ -50,23 +175,33 @@ export const processHistoryMessage = (item: proto.IHistorySync) => { phoneNumber: chat.pnJid || undefined }) + // Extract LID-PN mapping from conversation (lidJid/pnJid properties) + const convMapping = extractLidPnFromConversation( + chat.id!, + chat.lidJid, + chat.pnJid + ) + if(convMapping) { + lidPnMappings.push(convMapping) + } + const msgs = chat.messages || [] delete chat.messages - for (const item of msgs) { + for(const item of msgs) { const message = item.message! as WAMessage messages.push(message) - if (!chat.messages?.length) { + if(!chat.messages?.length) { // keep only the most recent message in the chat array chat.messages = [{ message }] } - if (!message.key.fromMe && !chat.lastMessageRecvTimestamp) { + if(!message.key.fromMe && !chat.lastMessageRecvTimestamp) { chat.lastMessageRecvTimestamp = toNumber(message.messageTimestamp) } - if ( + if( (message.messageStubType === WAMessageStubType.BIZ_PRIVACY_MODE_TO_BSP || message.messageStubType === WAMessageStubType.BIZ_PRIVACY_MODE_TO_FB) && message.messageStubParameters?.[0] @@ -76,6 +211,17 @@ export const processHistoryMessage = (item: proto.IHistorySync) => { verifiedName: message.messageStubParameters?.[0] }) } + + // Extract LID-PN mapping from message (remoteJidAlt/participantAlt fields) + const msgMapping = extractLidPnFromMessage( + message.key.remoteJid, + (message.key as { remoteJidAlt?: string }).remoteJidAlt, + message.key.participant, + (message.key as { participantAlt?: string }).participantAlt + ) + if(msgMapping) { + lidPnMappings.push(msgMapping) + } } chats.push({ ...chat }) diff --git a/src/__tests__/Utils/history.test.ts b/src/__tests__/Utils/history.test.ts index 2676fcb6..a900e275 100644 --- a/src/__tests__/Utils/history.test.ts +++ b/src/__tests__/Utils/history.test.ts @@ -1,5 +1,10 @@ import { proto } from '../../../WAProto/index.js' -import { processHistoryMessage } from '../../Utils/history' +import { + extractLidPnFromConversation, + extractLidPnFromMessage, + isPersonJid, + processHistoryMessage +} from '../../Utils/history' describe('processHistoryMessage', () => { describe('phoneNumberToLidMappings extraction', () => { @@ -90,5 +95,325 @@ describe('processHistoryMessage', () => { phoneNumber: '1234567890123@s.whatsapp.net' }) }) + + it('should extract LID-PN mappings from conversation lidJid/pnJid properties', () => { + const historySync: proto.IHistorySync = { + syncType: proto.HistorySync.HistorySyncType.INITIAL_BOOTSTRAP, + conversations: [ + { + id: '1234567890123@s.whatsapp.net', + name: 'Test User', + lidJid: '11111111111111@lid', + pnJid: '1234567890123@s.whatsapp.net' + } + ] + } + + const result = processHistoryMessage(historySync) + + // Should include mapping extracted from conversation + expect(result.lidPnMappings).toContainEqual({ + lid: '11111111111111@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should NOT extract LID-PN mappings from group conversations', () => { + const historySync: proto.IHistorySync = { + syncType: proto.HistorySync.HistorySyncType.INITIAL_BOOTSTRAP, + conversations: [ + { + id: '123456789@g.us', + name: 'Test Group', + lidJid: '11111111111111@lid', + pnJid: '1234567890123@s.whatsapp.net' + } + ] + } + + const result = processHistoryMessage(historySync) + + // Should NOT include mapping from group conversation + expect(result.lidPnMappings).toEqual([]) + }) + }) +}) + +describe('isPersonJid', () => { + it('should return true for LID users', () => { + expect(isPersonJid('11111111111111@lid')).toBe(true) + }) + + it('should return true for hosted LID users', () => { + expect(isPersonJid('11111111111111@hosted.lid')).toBe(true) + }) + + it('should return true for PN users (s.whatsapp.net)', () => { + expect(isPersonJid('1234567890123@s.whatsapp.net')).toBe(true) + }) + + it('should return true for hosted PN users', () => { + expect(isPersonJid('1234567890123@hosted')).toBe(true) + }) + + it('should return false for groups', () => { + expect(isPersonJid('123456789@g.us')).toBe(false) + }) + + it('should return false for broadcasts', () => { + expect(isPersonJid('status@broadcast')).toBe(false) + }) + + it('should return false for newsletters', () => { + expect(isPersonJid('123456789@newsletter')).toBe(false) + }) + + it('should return false for undefined', () => { + expect(isPersonJid(undefined)).toBe(false) + }) + + it('should return false for empty string', () => { + expect(isPersonJid('')).toBe(false) + }) +}) + +describe('extractLidPnFromConversation', () => { + it('should extract mapping when chat is PN and lidJid is provided', () => { + const result = extractLidPnFromConversation( + '1234567890123@s.whatsapp.net', + '11111111111111@lid', + null + ) + + expect(result).toEqual({ + lid: '11111111111111@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should extract mapping when chat is LID and pnJid is provided', () => { + const result = extractLidPnFromConversation( + '11111111111111@lid', + null, + '1234567890123@s.whatsapp.net' + ) + + expect(result).toEqual({ + lid: '11111111111111@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should return undefined for group chats', () => { + const result = extractLidPnFromConversation( + '123456789@g.us', + '11111111111111@lid', + '1234567890123@s.whatsapp.net' + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined for broadcast chats', () => { + const result = extractLidPnFromConversation( + 'status@broadcast', + '11111111111111@lid', + '1234567890123@s.whatsapp.net' + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined for newsletter chats', () => { + const result = extractLidPnFromConversation( + '123456789@newsletter', + '11111111111111@lid', + '1234567890123@s.whatsapp.net' + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when no mapping data is provided', () => { + const result = extractLidPnFromConversation( + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toBeUndefined() + }) +}) + +describe('extractLidPnFromMessage', () => { + it('should extract mapping when primary is LID and alt is PN', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toEqual({ + lid: '11111111111111@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should extract mapping when primary is PN and alt is LID', () => { + const result = extractLidPnFromMessage( + '1234567890123@s.whatsapp.net', + '11111111111111@lid', + null, + null + ) + + expect(result).toEqual({ + lid: '11111111111111@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should prefer participant over remoteJid', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + '22222222222222@lid', + '33333333333333@lid', + '1234567890123@s.whatsapp.net' + ) + + // participant (33333333333333@lid) and participantAlt (1234567890123@s.whatsapp.net) should be used + expect(result).toEqual({ + lid: '33333333333333@lid', + pn: '1234567890123@s.whatsapp.net' + }) + }) + + it('should return undefined when primaryJid is missing', () => { + const result = extractLidPnFromMessage( + null, + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when altJid is missing', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + null, + null, + null + ) + + expect(result).toBeUndefined() + }) + + // CRITICAL: Edge cases for the && vs || fix + describe('edge cases for mixed JID types (bug fix validation)', () => { + it('should return undefined when primary is person but alt is GROUP', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + '123456789@g.us', // group JID + null, + null + ) + + // With the fix (using ||), this should return undefined + // because group JIDs cannot be part of a valid LID-PN mapping + expect(result).toBeUndefined() + }) + + it('should return undefined when primary is person but alt is BROADCAST', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + 'status@broadcast', // broadcast JID + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when primary is person but alt is NEWSLETTER', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + '123456789@newsletter', // newsletter JID + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when alt is person but primary is GROUP', () => { + const result = extractLidPnFromMessage( + '123456789@g.us', // group JID + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when alt is person but primary is BROADCAST', () => { + const result = extractLidPnFromMessage( + 'status@broadcast', // broadcast JID + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when alt is person but primary is NEWSLETTER', () => { + const result = extractLidPnFromMessage( + '123456789@newsletter', // newsletter JID + '1234567890123@s.whatsapp.net', + null, + null + ) + + expect(result).toBeUndefined() + }) + + it('should return undefined when BOTH are non-person JIDs', () => { + const result = extractLidPnFromMessage( + '123456789@g.us', // group JID + 'status@broadcast', // broadcast JID + null, + null + ) + + expect(result).toBeUndefined() + }) + }) + + it('should return undefined when both are same type (both LID)', () => { + const result = extractLidPnFromMessage( + '11111111111111@lid', + '22222222222222@lid', + null, + null + ) + + // Both are LID, no PN - can't determine mapping + expect(result).toBeUndefined() + }) + + it('should return undefined when both are same type (both PN)', () => { + const result = extractLidPnFromMessage( + '1234567890123@s.whatsapp.net', + '9876543210987@s.whatsapp.net', + null, + null + ) + + // Both are PN, no LID - can't determine mapping + expect(result).toBeUndefined() }) })