diff --git a/src/Bridge/__tests__/adapt.test.ts b/src/Bridge/__tests__/adapt.test.ts index d8df2d53..47591314 100644 --- a/src/Bridge/__tests__/adapt.test.ts +++ b/src/Bridge/__tests__/adapt.test.ts @@ -1,6 +1,6 @@ import { describe, it } from 'node:test' import type { MessageWireInfo, WhatsAppEvent } from '@oxidezap/whatsapp-rust-bridge' -import { adaptBridgeEvent, adaptBridgeMessageWire } from '../adapt.ts' +import { adaptBridgeEvent, adaptBridgeMessageWire, KNOWN_BRIDGE_EVENT_TYPES } from '../adapt.ts' import { expect } from '../../__tests__/expect.ts' import { groupAnnouncementWireFixture, @@ -777,5 +777,63 @@ describe('adaptBridgeEvent — anti-corruption layer', () => { it('truly unknown event types return null (caller drops)', () => { expect(adaptBridgeEvent({ type: 'invented_2099', data: {} } as never)).toBe(null) }) + + /** + * The event type is untrusted: it comes from the runtime, which gets it from + * the server, and the adapter table is a plain object literal. + * + * Every one of these resolves on `Object.prototype`. The lookup used to call + * whatever it found and hand the result on as a canonical event — + * `constructor` produced an object and `toString` a string — while + * `__proto__` and `valueOf` threw instead. Found by the bridge fuzzer. + */ + it('does not resolve an event type through the prototype chain', () => { + for (const inherited of ['constructor', 'toString', 'valueOf', '__proto__', 'hasOwnProperty', 'isPrototypeOf']) { + expect(adaptBridgeEvent({ type: inherited, data: {} } as never)).toBe(null) + } + }) + }) + + /** + * An event with no data slot must be adapted, not thrown on. + * + * The contract is "null on unrecoverable shape mismatch, never a throw", and a + * throw here does not stay local — it propagates into the socket's event + * dispatch and takes down the whole event loop rather than the one event. 30 of + * the 58 declared types threw on this before the slot was normalised. + */ + describe('an absent data slot', () => { + it('never throws, for any declared event type', () => { + for (const type of KNOWN_BRIDGE_EVENT_TYPES) { + for (const data of [undefined, null]) { + expect(() => adaptBridgeEvent({ type, data } as never)).not.toThrow() + } + } + }) + + /** + * The dispatch must not answer this question on an adapter's behalf. + * + * Substituting `{}` for a missing slot was tried and reverted: `{}` is a + * perfectly good object, so it walks past the `isObject(data)` guard several + * adapters use to decide a payload is unrecoverable. `history_sync` then + * built an empty final batch and the socket emitted a spurious + * `messaging-history.set` where it had previously dropped the event. + */ + it("leaves an adapter's own object guard the last word", () => { + expect(adaptBridgeEvent({ type: 'history_sync' } as never)).toBe(null) + expect(adaptBridgeEvent({ type: 'history_sync', data: null } as never)).toBe(null) + }) + + it('drops the events that need their data, and keeps the ones that do not', () => { + // `qr` reads a code it has not been given, so there is nothing to emit. + expect(adaptBridgeEvent({ type: 'qr' } as never)).toBe(null) + // `connected` never had a data slot to miss. + expect(adaptBridgeEvent({ type: 'connected' } as never)).toEqual({ type: 'connected' }) + // And an adapter with its own fallback still uses it, rather than the + // event being dropped: this is why the slot is normalised rather than + // rejected outright. + expect(adaptBridgeEvent({ type: 'stream_error' } as never)).toEqual({ type: 'streamError', code: 'unknown' }) + }) }) }) diff --git a/src/Bridge/schema.ts b/src/Bridge/schema.ts index 279c333e..632a7f0b 100644 --- a/src/Bridge/schema.ts +++ b/src/Bridge/schema.ts @@ -123,55 +123,55 @@ const ADAPTERS = { reason: asString(data?.reason) }), - qr: data => (data.code ? { type: 'qr', code: data.code } : null), - pairing_code: data => (data.code ? { type: 'qr', code: data.code } : null), + qr: data => (data?.code ? { type: 'qr', code: data?.code } : null), + pairing_code: data => (data?.code ? { type: 'qr', code: data?.code } : null), pairing_code_refresh: data => ({ type: 'noop', bridgeType: 'pairing_code_refresh', - detail: data.force_manual ? 'force_manual' : 'automatic' + detail: data?.force_manual ? 'force_manual' : 'automatic' }), pair_passkey_request: () => ({ type: 'noop', bridgeType: 'pair_passkey_request' }), pair_passkey_confirmation: data => ({ type: 'noop', bridgeType: 'pair_passkey_confirmation', - detail: data.skip_handoff_ux ? 'handoff_verified' : 'confirmation_required' + detail: data?.skip_handoff_ux ? 'handoff_verified' : 'confirmation_required' }), pair_passkey_error: data => ({ type: 'noop', bridgeType: 'pair_passkey_error', - detail: asString(data.error) + detail: asString(data?.error) }), pair_success: data => { // `id` and `lid` come typed as `Jid` in the bridge .d.ts, but the // bridge actually serializes pair_success.{id,lid} as strings (see // the wire log) — accept both shapes. - const id = typeof data.id === 'string' ? data.id : asJidString(data.id) + const id = typeof data?.id === 'string' ? data?.id : asJidString(data?.id) if (!id) return null return { type: 'pairSuccess', id, - lid: typeof data.lid === 'string' ? data.lid : asJidString(data.lid), - platform: asString(data.platform), - businessName: asString(data.business_name) + lid: typeof data?.lid === 'string' ? data?.lid : asJidString(data?.lid), + platform: asString(data?.platform), + businessName: asString(data?.business_name) } }, pair_error: data => ({ type: 'pairError', - error: asString(data.error) ?? 'Unknown pairing error', - id: typeof data.id === 'string' ? data.id : asJidString(data.id), - lid: typeof data.lid === 'string' ? data.lid : asJidString(data.lid), - businessName: asString(data.business_name), - platform: asString(data.platform) + error: asString(data?.error) ?? 'Unknown pairing error', + id: typeof data?.id === 'string' ? data?.id : asJidString(data?.id), + lid: typeof data?.lid === 'string' ? data?.lid : asJidString(data?.lid), + businessName: asString(data?.business_name), + platform: asString(data?.platform) }), connect_failure: data => isObject(data) - ? { type: 'connectFailure', message: asString(data.message), reason: asNumber(data.reason) } + ? { type: 'connectFailure', message: asString(data?.message), reason: asNumber(data?.reason) } : { type: 'connectFailure' }, - stream_error: data => ({ type: 'streamError', code: asString(data.code) ?? 'unknown' }), + stream_error: data => ({ type: 'streamError', code: asString(data?.code) ?? 'unknown' }), // ── Messages ── message: (data, logger) => adaptMessage(data, logger), @@ -181,15 +181,15 @@ const ADAPTERS = { // while NACKs become `messages.update` with status ERROR. Preserve every // typed field here so the dispatcher can reproduce both surfaces. server_ack: data => { - const id = asString(data.id) + const id = asString(data?.id) if (!id) return null return { type: 'serverAck', id, - class: asString(data.class), - from: asJidString(data.from), - timestamp: asNumber(data.timestamp), - error: asString(data.error) + class: asString(data?.class), + from: asJidString(data?.from), + timestamp: asNumber(data?.timestamp), + error: asString(data?.error) } }, undecryptable_message: data => { @@ -199,7 +199,7 @@ const ADAPTERS = { // can synthesize a CIPHERTEXT stub matching upstream // `messages-recv.ts:1352`. if (!isObject(data)) return null - const info = isObject(data.info) ? data.info : undefined + const info = isObject(data?.info) ? data?.info : undefined if (!info) return null const src = isObject(info.source) ? info.source : undefined const chat = src && asJidString(src.chat) @@ -220,58 +220,58 @@ const ADAPTERS = { pushName: asString(info.push_name), participantAlt: resolveParticipantAlt(senderAlt, isGroup), remoteJidAlt: resolveRemoteJidAlt(senderAlt, recipientAlt, isGroup, isFromMe), - isUnavailable: asBoolOr(data.is_unavailable, false), - unavailableType: asString(data.unavailable_type), - decryptFailMode: asString(data.decrypt_fail_mode), + isUnavailable: asBoolOr(data?.is_unavailable, false), + unavailableType: asString(data?.unavailable_type), + decryptFailMode: asString(data?.decrypt_fail_mode), raw: data } }, // ── Contacts ── push_name_update: data => { - const jid = asJidString(data.jid) - return jid ? { type: 'pushNameUpdate', jid, newPushName: asString(data.new_push_name) } : null + const jid = asJidString(data?.jid) + return jid ? { type: 'pushNameUpdate', jid, newPushName: asString(data?.new_push_name) } : null }, contact_update: data => adaptContactUpdate(data), contact_updated: data => adaptContactUpdate(data), picture_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) if (!jid) return null return { type: 'pictureUpdate', jid, - removed: asBoolOr(data.removed, false), - author: asJidString(data.author), - pictureId: asString(data.picture_id) + removed: asBoolOr(data?.removed, false), + author: asJidString(data?.author), + pictureId: asString(data?.picture_id) } }, // ── Presence ── presence: data => { - const from = asJidString(data.from) + const from = asJidString(data?.from) if (!from) return null return { type: 'presence', from, - unavailable: asBoolOr(data.unavailable, false), - lastSeen: asNumber(data.last_seen) + unavailable: asBoolOr(data?.unavailable, false), + lastSeen: asNumber(data?.last_seen) } }, chat_presence: data => { - const src = isObject(data.source) ? data.source : undefined + const src = isObject(data?.source) ? data?.source : undefined if (!src) return null const chat = asJidString(src.chat) const sender = asJidString(src.sender) if (!chat || !sender) return null // Bridge sends `media: ''` for "no media" — normalize to undefined // so consumers can rely on field omission as the absence signal. - const media = asString(data.media) + const media = asString(data?.media) // Bridge `state` MUST be one of the two canonical values; defaulting // to 'composing' on an unknown/missing value would synthesize a // false typing indicator. Drop the event instead. - const rawState = asString(data.state) + const rawState = asString(data?.state) if (rawState !== 'composing' && rawState !== 'paused') return null return { type: 'chatPresence', @@ -287,40 +287,40 @@ const ADAPTERS = { // ── Chat state ── archive_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) if (!jid) return null return { type: 'archiveUpdate', jid, archived: asBoolOr(extractAction(data)?.archived, true) } }, pin_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) if (!jid) return null return { type: 'pinUpdate', jid, - timestamp: asNumber(data.timestamp), + timestamp: asNumber(data?.timestamp), pinned: asBoolOr(extractAction(data)?.pinned, true) } }, mute_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) if (!jid) return null const action = extractAction(data) return { type: 'muteUpdate', jid, - timestamp: asNumber(data.timestamp), + timestamp: asNumber(data?.timestamp), muted: asBoolOr(action?.muted, true), muteEndTimestamp: asNumber(action?.muteEndTimestamp) ?? asNumber(action?.mute_end_timestamp) } }, star_update: data => adaptStarUpdate(data), mark_chat_as_read_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) if (!jid) return null return { type: 'markChatAsReadUpdate', jid, read: asBoolOr(extractAction(data)?.read, true) } }, label_edit_update: data => { - const labelId = asString(data.label_id) + const labelId = asString(data?.label_id) if (!labelId) return { type: 'noop', bridgeType: 'label_edit_update' } const action = extractAction(data) // `predefinedId` is proto `predefined_id` (a number); upstream `Label` @@ -336,8 +336,8 @@ const ADAPTERS = { } }, label_association_update: data => { - const labelId = asString(data.label_id) - const chatJid = asJidString(data.chat_jid) + const labelId = asString(data?.label_id) + const chatJid = asJidString(data?.chat_jid) if (!labelId || !chatJid) return { type: 'noop', bridgeType: 'label_association_update' } // `action.labeled === true` → label added to the chat, else removed. return { type: 'labelAssociation', labelId, chatJid, labeled: asBoolOr(extractAction(data)?.labeled, true) } @@ -346,27 +346,27 @@ const ADAPTERS = { // ── Calls ── incoming_call: (data, logger) => adaptIncomingCall(data, logger), missed_call: data => { - const from = asJidString(data.from) - const callId = asString(data.call_id) + const from = asJidString(data?.from) + const callId = asString(data?.call_id) if (!from || !callId) return null return { type: 'incomingCall', from, - timestamp: toUnixSeconds(data.timestamp), - offline: data.reason === 'offline', + timestamp: toUnixSeconds(data?.timestamp), + offline: data?.reason === 'offline', action: { type: 'timeout', callId } } }, call_ended_elsewhere: data => { - const from = asJidString(data.from) - const callId = asString(data.call_id) + const from = asJidString(data?.from) + const callId = asString(data?.call_id) if (!from || !callId) return null return { type: 'incomingCall', from, - timestamp: toUnixSeconds(data.timestamp), + timestamp: toUnixSeconds(data?.timestamp), offline: false, - action: { type: data.outcome === 'accepted' ? 'accept' : 'reject', callId } + action: { type: data?.outcome === 'accepted' ? 'accept' : 'reject', callId } } }, @@ -412,32 +412,32 @@ const ADAPTERS = { self_push_name_updated: () => ({ type: 'noop', bridgeType: 'self_push_name_updated' }), offline_sync_completed: data => ({ type: 'offlineSyncCompleted', - count: asNumber(data.count) ?? 0 + count: asNumber(data?.count) ?? 0 }), offline_sync_preview: () => ({ type: 'noop', bridgeType: 'offline_sync_preview' }), dirty_state: data => { - const dirtyType = asString(data.dirty_type) + const dirtyType = asString(data?.dirty_type) if (!dirtyType) return null - return { type: 'dirtyState', dirtyType, timestamp: asNumber(data.timestamp) } + return { type: 'dirtyState', dirtyType, timestamp: asNumber(data?.timestamp) } }, device_list_update: () => ({ type: 'noop', bridgeType: 'device_list_update' }), identity_change: () => ({ type: 'noop', bridgeType: 'identity_change' }), disappearing_mode_changed: data => { - const jid = asJidString(data.from) - const duration = asNumber(data.duration) + const jid = asJidString(data?.from) + const duration = asNumber(data?.duration) if (!jid || duration == null) return { type: 'noop', bridgeType: 'disappearing_mode_changed' } return { type: 'disappearingModeChanged', jid, duration, - settingTimestamp: asNumber(data.setting_timestamp) + settingTimestamp: asNumber(data?.setting_timestamp) } }, business_status_update: () => ({ type: 'noop', bridgeType: 'business_status_update' }), newsletter_live_update: data => { - const newsletterJid = asJidString(data.newsletter_jid) + const newsletterJid = asJidString(data?.newsletter_jid) if (!newsletterJid) return { type: 'noop', bridgeType: 'newsletter_live_update' } - const rawMessages = Array.isArray(data.messages) ? data.messages : [] + const rawMessages = Array.isArray(data?.messages) ? data?.messages : [] const messages = rawMessages .map(m => { if (!isObject(m)) return null @@ -466,10 +466,10 @@ const ADAPTERS = { // (old_lid, old_jid) and (new_lid, new_jid). We learn whatever's // present and let the dispatcher fan out one upstream event per // pair. Mirrors upstream `messages-recv.ts:287`. - const oldJid = asJidString(data.old_jid) - const newJid = asJidString(data.new_jid) - const oldLid = asJidString(data.old_lid) - const newLid = asJidString(data.new_lid) + const oldJid = asJidString(data?.old_jid) + const newJid = asJidString(data?.new_jid) + const oldLid = asJidString(data?.old_lid) + const newLid = asJidString(data?.new_lid) const mappings: { lid: string; pn: string }[] = [] if (oldLid && oldJid) mappings.push({ lid: oldLid, pn: oldJid }) if (newLid && newJid) mappings.push({ lid: newLid, pn: newJid }) @@ -479,29 +479,29 @@ const ADAPTERS = { contact_sync_requested: () => ({ type: 'noop', bridgeType: 'contact_sync_requested' }), user_about_update: () => ({ type: 'noop', bridgeType: 'user_about_update' }), delete_chat_update: data => { - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) return jid ? { type: 'chatDelete', jid } : { type: 'noop', bridgeType: 'delete_chat_update' } }, clear_chat_update: data => { // Clear = drop all messages but keep the chat. Maps to upstream // `messages.delete` `{ jid, all: true }` (the chat-clear surface noted in // the messageDelete dispatcher), distinct from chatDelete (whole chat gone). - const jid = asJidString(data.jid) + const jid = asJidString(data?.jid) return jid ? { type: 'chatClear', jid } : { type: 'noop', bridgeType: 'clear_chat_update' } }, // Muting a contact's status (stories) updates. Forwarded for surface completeness, // but noop'd: upstream Baileys has no status-mute event/chatModify to map it onto. user_status_mute_update: () => ({ type: 'noop', bridgeType: 'user_status_mute_update' }), delete_message_for_me_update: data => { - const chatJid = asJidString(data.chat_jid) - const messageId = asString(data.message_id) + const chatJid = asJidString(data?.chat_jid) + const messageId = asString(data?.message_id) if (!chatJid || !messageId) return { type: 'noop', bridgeType: 'delete_message_for_me_update' } return { type: 'messageDelete', chatJid, messageId, - fromMe: asBoolOr(data.from_me, false), - participantJid: asJidString(data.participant_jid) + fromMe: asBoolOr(data?.from_me, false), + participantJid: asJidString(data?.participant_jid) } }, @@ -514,34 +514,34 @@ const ADAPTERS = { // CanonicalNotification.attrs (typed `Record`). // Drop non-string values, coerce the rest. const attrs: Record = {} - if (isObject(data.attrs)) { - for (const [k, v] of Object.entries(data.attrs)) { + if (isObject(data?.attrs)) { + for (const [k, v] of Object.entries(data?.attrs)) { if (typeof v === 'string') attrs[k] = v else if (typeof v === 'number' || typeof v === 'boolean') attrs[k] = String(v) // Drop nested objects / null silently — they wouldn't make // sense on a flat attrs map anyway. } } - return { type: 'notification', tag: asString(data.tag) ?? 'notification', attrs } + return { type: 'notification', tag: asString(data?.tag) ?? 'notification', attrs } }, raw_node: data => { // `BinaryNode` is shaped exactly like the bridge payload // (`{ tag, attrs, content }`). Minimal sanity check on `tag`. - if (!isObject(data) || typeof data.tag !== 'string') return null + if (!isObject(data) || typeof data?.tag !== 'string') return null return { type: 'rawNode', node: data as never } }, mex_notification: data => { - const opName = asString(data.op_name) + const opName = asString(data?.op_name) if (!opName) return null - const payload = isObject(data.payload) ? (data.payload as Record) : {} + const payload = isObject(data?.payload) ? (data?.payload as Record) : {} return { type: 'mexNotification', opName, - from: asJidString(data.from), - stanzaId: asString(data.stanza_id), - offline: asBoolOr(data.offline, false), + from: asJidString(data?.from), + stanzaId: asString(data?.stanza_id), + offline: asBoolOr(data?.offline, false), payload } } @@ -556,11 +556,35 @@ export const KNOWN_BRIDGE_EVENT_TYPES: ReadonlySet = new Set(Object.keys */ export const adaptBridgeEventViaSchema = (event: WhatsAppEvent, logger?: ILogger): CanonicalEvent | null => { const typed = event as { type: BridgeEventType; data?: unknown } - const adapter = (ADAPTERS as Record>)[typed.type] - if (!adapter) { + // An own property of the table, not anything the prototype chain answers. + // + // The type string comes from the runtime, which gets it from the server, so it + // is untrusted input into a plain-object lookup. `ADAPTERS.constructor` and + // `ADAPTERS.toString` resolve to inherited functions: they were called and + // their return values handed on as canonical events — measured, `constructor` + // produced an object and `toString` a string, neither of which is an event. + // `__proto__` and `valueOf` reached the other failure mode and threw. + if (!Object.hasOwn(ADAPTERS, typed.type)) { logger?.debug({ eventType: typed.type }, 'unknown bridge event (no canonical mapping)') return null } + const adapter = (ADAPTERS as Record>)[typed.type]! + // The slot is passed through exactly as it arrived, nullish included. + // + // The contract these adapters are documented under is "null on unrecoverable + // shape mismatch, never a throw", and a throw here does not stay local: it + // propagates into the socket's event dispatch and takes down the whole event + // loop rather than the one event. 30 of the 58 declared types threw on an + // event that arrived with no data — `data.code` on `undefined` — so every + // adapter in the table now reads its slot optionally. + // + // Substituting an empty object here instead was tried and reverted: `{}` is a + // perfectly good object, so it walks straight past the `isObject(data)` guard + // that several adapters use to decide the payload is unrecoverable. + // `history_sync` then built an empty final batch and the socket emitted a + // spurious `messaging-history.set` where it had previously dropped the event. + // Each adapter owns that decision; the dispatch must not pre-empt it. + // // `data` cast here is the only `as` in the public path — the bridge // runtime is the source of truth that the type matches the discriminator. return adapter(typed.data as never, logger) diff --git a/src/Utils/messages.ts b/src/Utils/messages.ts index ae9aeae9..9f92dda8 100644 --- a/src/Utils/messages.ts +++ b/src/Utils/messages.ts @@ -327,25 +327,49 @@ export const generateForwardMessageContent = (message: WAMessage, forceForward?: } content = normalizeMessageContent(content) - // Shallow clone — only the inner message object gets modified (contextInfo) + // Shallow clone of the outer map — one entry on it is rewritten below. content = { ...content! } let key = Object.keys(content)[0] as keyof proto.IMessage let score = (content?.[key] as { contextInfo: proto.IContextInfo })?.contextInfo?.forwardingScore || 0 score += message.key.fromMe && !forceForward ? 0 : 1 + + const contextInfo: proto.IContextInfo = score > 0 ? { forwardingScore: score, isForwarded: true } : {} + + // The nested message object is the *caller's*, reached through the shallow + // clone above, so writing `contextInfo` onto it wrote into their argument. + // And it replaces rather than merges: a caller who forwarded a quoted message + // found `stanzaId` and `participant` gone from their own object afterwards. + // Upstream leaves the argument untouched. + // + // Rebuilt rather than deep-copied, and only the one object being written. + // Upstream's copy is `proto.Message.decode(proto.Message.encode(content))` — + // a full serialise/parse round trip on a hot send path — which is not worth + // paying to fix an aliasing bug. if (key === 'conversation') { - content.extendedTextMessage = { text: content[key] } + // This object is created here, so nothing of the caller's is aliased and + // the `contextInfo` goes straight in. Same allocation count as before. + content.extendedTextMessage = { text: content[key], contextInfo } delete content.conversation - key = 'extendedTextMessage' + return content } - const key_ = content?.[key] as { contextInfo: proto.IContextInfo } - if (score > 0) { - key_.contextInfo = { forwardingScore: score, isForwarded: true } + const nested = content[key] as { contextInfo: proto.IContextInfo } | undefined + // A plain object is the only thing worth copying, and the only thing that can + // be the caller's to damage. Anything else — an absent slot on an empty + // message, a primitive, an array — keeps the original assignment, which + // throws for exactly the inputs it threw for before and that upstream throws + // for too. Spreading those instead invented a property named `"undefined"` + // where upstream raised a TypeError. + if (typeof nested === 'object' && nested !== null && !Array.isArray(nested)) { + // One shallow spread, of exactly the object being modified. This is the + // whole cost of the fix, and it is unavoidable: not writing into the + // caller's object means writing into a different one. + content[key] = { ...nested, contextInfo } as never } else { - key_.contextInfo = {} + nested!.contextInfo = contextInfo } return content diff --git a/src/__fuzz__/harness/divergence.ts b/src/__fuzz__/harness/divergence.ts index b85967cb..5a941cfd 100644 --- a/src/__fuzz__/harness/divergence.ts +++ b/src/__fuzz__/harness/divergence.ts @@ -598,60 +598,6 @@ const maskInt64Truncations = (local: unknown, upstream: unknown, depth = 0): [un return [maskedLocal, maskedUpstream] } -/** Every `contextInfo` removed, so the rest of a tuple can be compared alone. */ -const withoutContextInfo = (value: unknown, depth = 0): unknown => { - if (depth > 12 || typeof value !== 'object' || value === null) return value - if (Array.isArray(value)) return value.map(item => withoutContextInfo(item, depth + 1)) - const out: Record = {} - for (const [key, nested] of Object.entries(value as Record)) { - if (key === 'contextInfo') continue - out[key] = withoutContextInfo(nested, depth + 1) - } - return out -} - -/** The keys `generateForwardMessageContent` is allowed to leave in a `contextInfo`. */ -const FORWARDING_KEYS: ReadonlySet = new Set(['forwardingScore', 'isForwarded']) - -/** - * True when every `contextInfo` that differs between the two sides is one - * baileyrs wrote the forwarding metadata into, and nothing else differs. - * - * Positional, not global. The mutation oracle reports one finding for the whole - * argument tuple, so "baileyrs mutated the argument" was true of the documented - * write *and* of anything that rode along with it. But requiring *every* - * `contextInfo` to hold only forwarding keys is the opposite error: a - * `contextInfo` the caller supplied deeper in the message is legitimately left - * alone — measured on a `deviceSentMessage` whose inner `extendedTextMessage` - * keeps its own `participant` while the forwarding metadata is written at the - * wrapper level. So each position is compared against upstream's, and only a - * `contextInfo` holding exactly the forwarding keys may differ. - */ -const onlyForwardingContextInfoDiffers = (local: unknown, upstream: unknown, depth = 0): boolean => { - if (depth > 12) return false - if (text(local) === text(upstream)) return true - if (Array.isArray(local) || Array.isArray(upstream)) { - if (!Array.isArray(local) || !Array.isArray(upstream) || local.length !== upstream.length) return false - return local.every((item, index) => onlyForwardingContextInfoDiffers(item, upstream[index], depth + 1)) - } - if (typeof local !== 'object' || typeof upstream !== 'object' || local === null || upstream === null) return false - const a = local as Record - const b = upstream as Record - for (const key of new Set([...Object.keys(a), ...Object.keys(b)])) { - if (text(a[key]) === text(b[key])) continue - if (key === 'contextInfo') { - // Added or replaced, either way it must hold the forwarding keys alone. - const written = a[key] - if (typeof written !== 'object' || written === null) return false - if (!Object.keys(written as Record).every(inner => FORWARDING_KEYS.has(inner))) return false - continue - } - if (!Object.hasOwn(a, key) || !Object.hasOwn(b, key)) return false - if (!onlyForwardingContextInfoDiffers(a[key], b[key], depth + 1)) return false - } - return true -} - /** * What `generateForwardMessageContent`'s two copy strategies are allowed to * differ by, once every documented normalisation has been undone. @@ -840,17 +786,6 @@ const text = (value: unknown): string => { } } -/** - * Every property an empty object inherits. - * - * Enumerated from the prototype itself rather than written out, so the entry that - * relies on it cannot drift from what the runtime actually inherits. - */ -// Object.prototype only: the adapter table is a plain object literal, so that is -// the whole of its prototype chain. Including Function.prototype names would -// excuse a genuine unrecognised event type called `bind` or `name`. -const PROTOTYPE_KEYS: ReadonlySet = new Set(Object.getOwnPropertyNames(Object.prototype)) - /** * An unpaired UTF-16 surrogate, which is what the newsletter encoder differs on. * @@ -860,10 +795,6 @@ const PROTOTYPE_KEYS: ReadonlySet = new Set(Object.getOwnPropertyNames(O const LONE_SURROGATE = /[\ud800-\udbff](?![\udc00-\udfff])|(? { - if (isThrow(divergence.local) || isThrow(divergence.upstream)) return false - const mine = normalise(divergence.local) - const theirs = normalise(divergence.upstream) - // Two conditions rather than one, because either alone is escapable. - // Everything outside `contextInfo` identical rules out a changed body or - // a deleted field; the positional walk then rules out a `contextInfo` - // changed to anything but the forwarding metadata. - if (text(withoutContextInfo(mine)) !== text(withoutContextInfo(theirs))) return false - return onlyForwardingContextInfoDiffers(mine, theirs) - }, - reason: - "baileyrs replaces `contextInfo` on the caller's own message object with `{ forwardingScore, isForwarded }`; upstream leaves the argument untouched and returns new content. Two consequences, and the second is the sharper one: forwarding a message mutates the original in one library and not the other, and because it *replaces* rather than merges, a caller who forwards a quoted message finds `stanzaId` and `participant` gone from their own object afterwards. Measured on `{ extendedTextMessage: { text: 'x', contextInfo: { stanzaId: 'abc', participant: 'a@s.whatsapp.net' } } }`: the baileyrs argument comes back holding only the two forwarding keys, the upstream argument comes back unchanged. Both return values drop the quote metadata, so that part is agreed behaviour; only the write to the caller's object differs. Root cause is the copy: baileyrs shallow-clones with `{ ...content }`, so the nested message object is shared with the caller, where upstream rebuilds it via `proto.Message.decode(proto.Message.encode(content))`.", - review: '2026-11-01' - }, { id: 'forward-message-content-copy-shape', target: 'pure:generateForwardMessageContent', @@ -1194,36 +1095,6 @@ export const KNOWN_DIVERGENCES: readonly KnownDivergence[] = [ 'For a 64-bit integer field holding a number that is not a 64-bit integer, the bridge encoder throws where upstream converts and encodes. Two rejections, one cause: `1.5`, `NaN` and `Infinity` fail the BigInt conversion (`RangeError: The number … cannot be converted to a BigInt`), and `1e300`, `2**63` and `1.5625e19` fail the range check (`Error: invalid int64: …`). Upstream accepts every one of them — `1.5` as 1, `NaN`/`Infinity`/`1e300` as 0, and `1.5625e19` as a wrapped value that is not the number it was given. Safe integers, numeric strings, `null` and `undefined` are byte-identical on both sides. Measured on `pollUpdateMessage.senderTimestampMs`. baileyrs is plainly the more correct side here — upstream silently sends a wrong value where baileyrs refuses — but it is caller-visible either way: the same content sends on Baileys and throws on baileyrs. Same shape as the float32 entry, and the pair should be decided together.', review: '2026-11-01' }, - { - id: 'bridge-adapter-prototype-chain-lookup', - target: /^bridge:adapt-(unknown|total)$/u, - status: 'open', - reason: - 'The adapter table is a plain object literal indexed by the event type string, so a type of "constructor", "toString" or "valueOf" resolves through Object.prototype: the inherited function is called and its return value is handed on as a canonical event, and "__proto__" resolves to a non-function and throws "adapter is not a function". The type comes from the runtime, which gets it from the server, so an untrusted string is indexing a prototype-bearing lookup table. Both outcomes break the layer\'s stated contract of dropping what it does not recognise. A Map, an Object.create(null) table, or an Object.hasOwn guard fixes it.', - review: '2026-10-01', - when: divergence => PROTOTYPE_KEYS.has(String((divergence.input as { type?: unknown })?.type)) - }, - { - id: 'bridge-adapter-throws-on-missing-data', - target: /^bridge:adapt-(total|coverage)$/u, - status: 'open', - reason: - 'Adapters for declared event types read straight into `data` without checking it is there, so an event that arrives with no data slot — or with a slot missing the field the adapter reads — throws a TypeError instead of returning null. `adapt.ts` documents the opposite ("Result is null on unrecoverable shape mismatch"), and the throw does not stay local: it propagates into the socket event dispatch, which takes out the whole event loop rather than the one event.', - review: '2026-10-01', - // The throw shape *and* a `data` slot that is actually missing. On its own - // the regex matches the most common TypeErrors there are — "is not a - // function", "is not iterable" — so an adapter regression on a well-shaped - // event was classified as the known missing-data problem. The per-type - // coverage counter cannot catch that either: it sees a type that never - // adapts at all, not one that fails on one payload in eight. - when: divergence => { - if (!MISSING_DATA_THROWS.test(text(divergence.local))) return false - const data = (divergence.input as { data?: unknown } | undefined)?.data - // Absent, not a record at all, or a record with nothing in it — the three - // shapes an adapter reading straight into `data` cannot survive. - return plainObject(data)?.length !== undefined ? plainObject(data)!.length === 0 : true - } - }, { id: 'event-buffer-merge-precedence', target: 'buffer:differential', diff --git a/src/__tests__/forward-message-content.test.ts b/src/__tests__/forward-message-content.test.ts new file mode 100644 index 00000000..181b782b --- /dev/null +++ b/src/__tests__/forward-message-content.test.ts @@ -0,0 +1,110 @@ +/** + * `generateForwardMessageContent` must not write into its argument. + * + * Found by the pure-helper differential: baileyrs shallow-cloned the outer + * message map with `{ ...content }` and then assigned `contextInfo` onto the + * *nested* message object, which the clone still shares with the caller. And it + * replaced rather than merged, so forwarding a quoted message stripped + * `stanzaId` and `participant` from the caller's own object. Upstream leaves the + * argument untouched, because it copies through + * `proto.Message.decode(proto.Message.encode(content))`. + * + * The fix rebuilds only the one nested object being written, rather than paying + * for that round trip on a hot send path — so these tests pin both halves: the + * argument survives, and the copy stays shallow everywhere else. + */ + +import { describe, it } from 'node:test' +import { generateForwardMessageContent } from '../Utils/messages.ts' +import type { WAMessage } from '../Types/index.ts' +import { expect } from './expect.ts' + +const incoming = (message: unknown): WAMessage => + ({ key: { fromMe: false, remoteJid: 'a@s.whatsapp.net', id: '1' }, message }) as WAMessage + +describe('generateForwardMessageContent — the caller keeps their message', () => { + it('leaves a quoted message intact rather than replacing its contextInfo', () => { + const original = incoming({ + extendedTextMessage: { + text: 'x', + contextInfo: { stanzaId: 'abc', participant: 'a@s.whatsapp.net' } + } + }) + + const forwarded = generateForwardMessageContent(original) + + // The reproducer from the finding: these two used to be gone afterwards. + expect(original.message?.extendedTextMessage?.contextInfo?.stanzaId).toBe('abc') + expect(original.message?.extendedTextMessage?.contextInfo?.participant).toBe('a@s.whatsapp.net') + // And the forwarding metadata went somewhere else entirely. + expect(forwarded.extendedTextMessage?.contextInfo).toEqual({ forwardingScore: 1, isForwarded: true }) + }) + + it('does not hand back the caller’s nested object', () => { + const inner = { text: 'x', contextInfo: { stanzaId: 'abc' } } + const original = incoming({ extendedTextMessage: inner }) + + const forwarded = generateForwardMessageContent(original) + + // Not identity-equal: writing through either one must not reach the other. + expect(forwarded.extendedTextMessage === inner).toBe(false) + expect(inner.contextInfo).toEqual({ stanzaId: 'abc' }) + }) + + it('carries the rest of the nested fields across unchanged', () => { + const original = incoming({ + imageMessage: { url: 'https://x', mimetype: 'image/jpeg', fileLength: 12, caption: 'hi' } + }) + + const forwarded = generateForwardMessageContent(original) + + expect(forwarded.imageMessage?.url).toBe('https://x') + expect(forwarded.imageMessage?.mimetype).toBe('image/jpeg') + expect(forwarded.imageMessage?.caption).toBe('hi') + expect(forwarded.imageMessage?.contextInfo).toEqual({ forwardingScore: 1, isForwarded: true }) + }) + + it('accumulates the forwarding score without touching the source', () => { + const original = incoming({ + extendedTextMessage: { text: 'x', contextInfo: { forwardingScore: 4, isForwarded: true } } + }) + + const forwarded = generateForwardMessageContent(original) + + expect(forwarded.extendedTextMessage?.contextInfo?.forwardingScore).toBe(5) + expect(original.message?.extendedTextMessage?.contextInfo?.forwardingScore).toBe(4) + }) + + it('rewrites a conversation into an extendedTextMessage, leaving the original', () => { + const original = incoming({ conversation: 'hello' }) + + const forwarded = generateForwardMessageContent(original) + + expect(forwarded.extendedTextMessage?.text).toBe('hello') + expect(forwarded.extendedTextMessage?.contextInfo).toEqual({ forwardingScore: 1, isForwarded: true }) + expect(forwarded.conversation).toBe(undefined) + // The caller still has the conversation they passed in. + expect(original.message?.conversation).toBe('hello') + }) + + it('scores an outgoing message at zero unless forwarding is forced', () => { + const own = { key: { fromMe: true, remoteJid: 'a@s.whatsapp.net', id: '1' }, message: { conversation: 'x' } } + + expect(generateForwardMessageContent(own as WAMessage).extendedTextMessage?.contextInfo).toEqual({}) + expect(generateForwardMessageContent(own as WAMessage, true).extendedTextMessage?.contextInfo).toEqual({ + forwardingScore: 1, + isForwarded: true + }) + expect(own.message.conversation).toBe('x') + }) + + /** + * A message with no content key at all reaches the nested assignment with + * nothing to assign to. Upstream raises a TypeError here and so does this — + * an earlier version of the fix spread the absent slot instead and invented a + * property literally named `"undefined"`, which the differential caught. + */ + it('throws on an empty message rather than inventing a key', () => { + expect(() => generateForwardMessageContent(incoming({}))).toThrow() + }) +})