Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ bench-profiles-*/
/test/
.claude
*.log
___*
___*
47 changes: 34 additions & 13 deletions src/client/coordinators/WaMessageCoordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ import { MEDIA_UPLOAD_PATHS } from '@media/constants'
import type { WaMediaTransferClient } from '@media/transfer/WaMediaTransferClient'
import type { MediaKind } from '@media/types'
import {
buildAddonAdditionalData,
buildAddonSenderPairs,
collectUniqueUserJids,
decodeAddonPlaintext,
decryptAddonPayload,
decryptAddonPayloadWithSenderFallback,
identifyEncryptedAddon,
resolveAddonParentSenderFromKey,
resolveParentMessageSecret,
resolvePollOptionNames,
shouldUseAddonAdditionalData
resolvePollOptionNames
} from '@message/crypto/addon-crypto'
import { unwrapMessage } from '@message/encode/content'
import { encodeGroupHistoryBundle } from '@message/kinds/group-history'
Expand Down Expand Up @@ -848,20 +849,40 @@ export class WaMessageCoordinator {
return
}

const parentMsgOriginalSender = parentEntry.senderJid
const modificationSender = event.key.participant ?? event.key.remoteJid
const modificationSenderRaw = event.key.participant ?? event.key.remoteJid
if (!modificationSenderRaw) return

const modificationSenderCandidates = collectUniqueUserJids(
event.key.fromMe ? event.rawNode.attrs.from : undefined,
modificationSenderRaw,
event.key.participantAlt,
event.key.remoteJidAlt,
event.rawNode.attrs.participant_pn,
event.rawNode.attrs.sender_pn,
event.rawNode.attrs.participant,
event.key.isGroup ? undefined : event.rawNode.attrs.from
)

const keyParentSender = resolveAddonParentSenderFromKey(
addon.targetMessageKey,
event.key.isGroup
)
const parentMsgOriginalSenderCandidates = collectUniqueUserJids(
keyParentSender,
parentEntry.senderJid
)
const senderPairs = buildAddonSenderPairs({
parentCandidates: parentMsgOriginalSenderCandidates,
modificationCandidates: modificationSenderCandidates
})

const plaintext = await decryptAddonPayload({
const plaintext = await decryptAddonPayloadWithSenderFallback({
messageSecret: parentEntry.secret,
stanzaId: targetMessageId,
parentMsgOriginalSender,
modificationSender,
senderPairs,
modificationType: addon.modificationType,
ciphertext: addon.encPayload,
iv: addon.encIv,
additionalData: shouldUseAddonAdditionalData(addon.modificationType)
? buildAddonAdditionalData(targetMessageId, modificationSender)
: undefined
iv: addon.encIv
})

let decrypted = decodeAddonPlaintext(addon.kind, plaintext)
Expand Down
40 changes: 38 additions & 2 deletions src/client/coordinators/WaMessageDispatchCoordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,11 @@ export class WaMessageDispatchCoordinator {
sendOptions.id &&
(this.deps.persistAllMessageSecrets || needsSecretPersistence(messageWithSecret))
) {
const meJid = this.deps.getCurrentCredentials()?.meJid ?? ''
void this.deps.messageSecretStore
.set(sendOptions.id, { secret: rawSecret, senderJid: meJid })
.set(sendOptions.id, {
secret: rawSecret,
senderJid: this.resolveOutgoingSecretSenderJid()
})
.catch((error) => {
this.deps.logger.warn('failed to persist outgoing message secret', {
id: sendOptions.id,
Expand Down Expand Up @@ -1480,6 +1482,40 @@ export class WaMessageDispatchCoordinator {
return WA_ADDRESSING_MODES.PN
}

/**
* Parent-author JID stored with the outgoing message secret. Peers address
* us by LID in `pollCreationMessageKey` during the PN→LID migration, so a
* valid `meLid` wins. A malformed LID must not abort the send: fall back to
* a normalized `meJid`, then to the empty sender (same recoverable path as
* {@link resolveSenderForAddressingMode}).
*/
private resolveOutgoingSecretSenderJid(): string {
const credentials = this.deps.getCurrentCredentials()
const meLid = credentials?.meLid
if (meLid && meLid.includes('@')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: resolveOutgoingSecretSenderJid only checks that meLid contains '@' before normalizing and returning it, without verifying it is actually a LID JID (e.g. via isLidJid). If credentials.meLid is ever populated with a PN-shaped value, this will persist the PN as the parent sender instead of falling back to meJid, which can cause the stored sender fallback used during addon decryption to be wrong. Consider normalizing meLid first and only returning it when isLidJid() is true, falling back to meJid otherwise.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/client/coordinators/WaMessageDispatchCoordinator.ts, line 1495:

<comment>resolveOutgoingSecretSenderJid only checks that `meLid` contains '@' before normalizing and returning it, without verifying it is actually a LID JID (e.g. via `isLidJid`). If `credentials.meLid` is ever populated with a PN-shaped value, this will persist the PN as the parent sender instead of falling back to `meJid`, which can cause the stored sender fallback used during addon decryption to be wrong. Consider normalizing `meLid` first and only returning it when `isLidJid()` is true, falling back to `meJid` otherwise.</comment>

<file context>
@@ -1487,6 +1482,40 @@ export class WaMessageDispatchCoordinator {
+    private resolveOutgoingSecretSenderJid(): string {
+        const credentials = this.deps.getCurrentCredentials()
+        const meLid = credentials?.meLid
+        if (meLid && meLid.includes('@')) {
+            try {
+                return toUserJid(meLid)
</file context>

try {
return toUserJid(meLid)
} catch (error) {
this.deps.logger.trace('ignoring malformed me lid jid', {
meLid,
message: toError(error).message
})
}
}
const meJid = credentials?.meJid
if (meJid && meJid.includes('@')) {
try {
return toUserJid(meJid)
Comment on lines +1495 to +1508

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require a LID before preferring meLid.

toUserJid() accepts PN JIDs. A PN-shaped credentials.meLid can therefore win over
credentials.meJid and persist the wrong parent sender. Later addon decryption can then
lack the correct stored sender fallback.

  • src/client/coordinators/WaMessageDispatchCoordinator.ts#L1495-L1508: Normalize
    meLid, then return it only when isLidJid() is true. Otherwise fall back to meJid.
  • src/client/coordinators/__tests__/coordinators.test.ts#L243-L271: Add a case where
    meLid is a valid PN JID that differs from meJid, and assert that the normalized
    meJid is returned.
📍 Affects 2 files
  • src/client/coordinators/WaMessageDispatchCoordinator.ts#L1495-L1508 (this comment)
  • src/client/coordinators/__tests__/coordinators.test.ts#L243-L271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/client/coordinators/WaMessageDispatchCoordinator.ts` around lines 1495 -
1508, Update the meLid handling near the parent-sender resolution logic in
WaMessageDispatchCoordinator.ts: normalize meLid, return it only when isLidJid()
confirms it is a LID, and otherwise fall back to meJid. In
src/client/coordinators/__tests__/coordinators.test.ts lines 243-271, add
coverage for a valid PN-shaped meLid differing from meJid and assert the
normalized meJid is returned.

} catch (error) {
this.deps.logger.trace('ignoring malformed me jid', {
meJid,
message: toError(error).message
})
}
}
return ''
}

private resolveSenderForAddressingMode(
addressingMode: GroupAddressingMode,
meJid: string
Expand Down
43 changes: 42 additions & 1 deletion src/client/coordinators/__tests__/coordinators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ function createMessageDispatchCoordinator(
groupMetadataStore: WaGroupMetadataMemoryStore,
overrides?: {
readonly meJid?: string
readonly meLid?: string
readonly mobileMessageIdFormat?: () => boolean
readonly serverClock?: ServerClock
readonly signalDeviceSync?: {
Expand Down Expand Up @@ -177,7 +178,9 @@ function createMessageDispatchCoordinator(
set: async (_id: string, _entry: { secret: Uint8Array; senderJid: string }) => {}
} as never,
getCurrentCredentials: () =>
overrides?.meJid ? ({ meJid: overrides.meJid } as never) : null,
overrides?.meJid || overrides?.meLid
? ({ meJid: overrides.meJid, meLid: overrides.meLid } as never)
: null,
resolvePrivacyTokenNode: async () => null,
onDirectMessageSent: () => undefined,
mobileMessageIdFormat: overrides?.mobileMessageIdFormat,
Expand All @@ -201,6 +204,14 @@ function buildAppStateSyncResult(
return { collections }
}

function callResolveOutgoingSecretSenderJid(coordinator: WaMessageDispatchCoordinator): string {
return (
coordinator as unknown as {
resolveOutgoingSecretSenderJid(): string
}
).resolveOutgoingSecretSenderJid()
}

function callResolvePeerRecipientPn(
coordinator: WaMessageDispatchCoordinator,
recipientUserJid: string,
Expand Down Expand Up @@ -229,6 +240,36 @@ test('message dispatch emits message_send with the outbound proto and destinatio
assert.ok(events[0]?.message)
})

test('outgoing secret sender prefers meLid and recedes to meJid when lid is malformed', () => {
const store = new WaGroupMetadataMemoryStore()
const withLid = createMessageDispatchCoordinator(store, {
meJid: '5511000000000:12@s.whatsapp.net',
meLid: '123456789:3@lid'
})
assert.equal(callResolveOutgoingSecretSenderJid(withLid), '123456789@lid')

const malformedLid = createMessageDispatchCoordinator(store, {
meJid: '5511000000000:12@s.whatsapp.net',
meLid: 'not-a-jid'
})
assert.equal(callResolveOutgoingSecretSenderJid(malformedLid), '5511000000000@s.whatsapp.net')

const malformedLidWithAt = createMessageDispatchCoordinator(store, {
meJid: '5511000000000@s.whatsapp.net',
meLid: '@lid'
})
assert.equal(
callResolveOutgoingSecretSenderJid(malformedLidWithAt),
'5511000000000@s.whatsapp.net'
)

const bothMalformed = createMessageDispatchCoordinator(store, {
meJid: 'not-a-jid',
meLid: 'also-bad'
})
assert.equal(callResolveOutgoingSecretSenderJid(bothMalformed), '')
})

test('resolvePeerRecipientPn: a PN-addressed caller on a LID envelope stamps that PN', async () => {
const coordinator = createMessageDispatchCoordinator(new WaGroupMetadataMemoryStore())
const pn = await callResolvePeerRecipientPn(
Expand Down
Loading
Loading