fix: break infinite Bad MAC + session recreation loop
Remove aggressive session cleanup from the decryption hot path that caused cascading failures when multiple messages from the same contact arrived simultaneously. The Signal Protocol naturally recovers via retry+pkmsg flow. Changes: - Remove cleanupCorruptedSession() from per-message error handler (hot path) - Move session cleanup to retry-exhausted handler as safety net - Add per-JID retry request deduplication (5s window) - Add 10s cooldown on MAC error session recreation - Export cleanupCorruptedSession/getDecryptionJid for use in messages-recv Tested: corrupted session with fake keys → Bad MAC → retry receipt → pkmsg recovery → all 7 messages delivered, zero infinite loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Renato Alcara
@@ -31,6 +31,7 @@ import {
|
||||
aesDecryptCTR,
|
||||
aesEncryptGCM,
|
||||
cleanMessage,
|
||||
cleanupCorruptedSession,
|
||||
Curve,
|
||||
decodeMediaRetryNode,
|
||||
decodeMessageNode,
|
||||
@@ -41,6 +42,7 @@ import {
|
||||
encodeSignedDeviceIdentity,
|
||||
extractAddressingContext,
|
||||
getCallStatusFromNode,
|
||||
getDecryptionJid,
|
||||
getHistoryMsg,
|
||||
getNextPreKeys,
|
||||
getStatusFromReceiptType,
|
||||
@@ -162,6 +164,12 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
||||
const TC_TOKEN_PRUNE_TS_KEY = '__prune_ts'
|
||||
const tcTokenKnownJids = new Set<string>()
|
||||
const tcTokenRetriedMsgIds = new Set<string>()
|
||||
|
||||
// Deduplicates retry requests per JID within a short window.
|
||||
// When a burst of Bad MAC errors arrives for the same contact,
|
||||
// only the first retry request is sent — the peer resends everything
|
||||
// with a single pkmsg, avoiding the close-session cascade.
|
||||
const retryRequestActiveJids = new Set<string>()
|
||||
let tcTokenIndexSaveTimer: ReturnType<typeof setTimeout> | undefined
|
||||
let lastTcTokenPruneTs = 0
|
||||
|
||||
@@ -1209,12 +1217,49 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
||||
const { key: msgKey } = fullMessage
|
||||
const msgId = msgKey.id!
|
||||
|
||||
// Per-JID deduplication: when multiple messages from the same contact
|
||||
// fail with Bad MAC simultaneously, only send ONE retry request.
|
||||
// The peer will resend all failed messages when it receives the retry receipt.
|
||||
const retryDedupeJid = jidNormalizedUser(node.attrs.from!)
|
||||
if (retryRequestActiveJids.has(retryDedupeJid)) {
|
||||
logger.debug(
|
||||
{ fromJid: retryDedupeJid, msgId },
|
||||
'Skipping duplicate retry request — already in-flight for this JID'
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
retryRequestActiveJids.add(retryDedupeJid)
|
||||
setTimeout(() => retryRequestActiveJids.delete(retryDedupeJid), 5_000)
|
||||
|
||||
if (messageRetryManager) {
|
||||
// Check if we've exceeded max retries using the new system
|
||||
if (messageRetryManager.hasExceededMaxRetries(msgId)) {
|
||||
logger.debug({ msgId }, 'reached retry limit with new retry manager, clearing')
|
||||
messageRetryManager.markRetryFailed(msgId)
|
||||
recordMessageFailure('retry', 'max_retries_reached')
|
||||
|
||||
// Safety net: clean up corrupted sessions only after all retries exhausted.
|
||||
// This avoids the cascading delete loop that occurs when cleanup runs
|
||||
// on every Bad MAC in the hot path (decode-wa-message.ts).
|
||||
// The Signal Protocol recovers naturally via retry+pkmsg for most cases;
|
||||
// this cleanup only runs as a last resort.
|
||||
if (autoCleanCorrupted) {
|
||||
const fromJid = node.attrs.from!
|
||||
try {
|
||||
const decryptionJid = await getDecryptionJid(fromJid, signalRepository)
|
||||
const deletedCount = await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
||||
if (deletedCount > 0) {
|
||||
logger.info(
|
||||
{ msgId, jid: decryptionJid, targetedDevices: deletedCount },
|
||||
`🔄 Session cleanup (retry exhausted) | Targeted: ${deletedCount} devices`
|
||||
)
|
||||
}
|
||||
} catch (cleanupErr) {
|
||||
logger.warn({ msgId, fromJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
@@ -1233,6 +1278,18 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
||||
logger.debug({ retryCount, msgId }, 'reached retry limit, clearing')
|
||||
await msgRetryCache.del(key)
|
||||
recordMessageFailure('retry', 'max_retries_reached')
|
||||
|
||||
// Safety net cleanup (same as new system above)
|
||||
if (autoCleanCorrupted) {
|
||||
const fromJid = node.attrs.from!
|
||||
try {
|
||||
const decryptionJid = await getDecryptionJid(fromJid, signalRepository)
|
||||
await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
||||
} catch (cleanupErr) {
|
||||
logger.warn({ msgId, fromJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -422,34 +422,19 @@ export const decryptMessageNode = (
|
||||
if (isRetryExhausted) {
|
||||
logger.error(
|
||||
errorContext,
|
||||
`⚠️ Session corrupted and recovery failed after ${err.attempts} attempts. Auto-cleanup will attempt to recover.`
|
||||
`⚠️ Session corrupted after ${err.attempts} attempts. Retry+pkmsg flow will recover.`
|
||||
)
|
||||
} else {
|
||||
// First occurrence - log as warning since auto-recovery will attempt
|
||||
logger.warn(errorContext, '⚠️ Corrupted session detected - attempting auto-recovery')
|
||||
}
|
||||
|
||||
// Automatic cleanup of corrupted session (if enabled)
|
||||
// eslint-disable-next-line max-depth
|
||||
if (autoCleanCorrupted) {
|
||||
// eslint-disable-next-line max-depth
|
||||
try {
|
||||
const deletedCount = await cleanupCorruptedSession(decryptionJid, repository, logger)
|
||||
|
||||
// Mask only user portion of JID for privacy (preserve domain info)
|
||||
const { user, server } = jidDecode(decryptionJid) || {}
|
||||
const maskedUser =
|
||||
user && user.length > 8 ? `${user.substring(0, 4)}****${user.substring(user.length - 4)}` : user
|
||||
const maskedJid = maskedUser && server ? `${maskedUser}@${server}` : decryptionJid
|
||||
|
||||
logger.info(
|
||||
{ jid: decryptionJid, maskedJid, targetedDevices: deletedCount },
|
||||
`🔄 Session Reset | JID: ${maskedJid} | Targeted: ${deletedCount} devices | Will recreate on next message`
|
||||
)
|
||||
} catch (cleanupErr) {
|
||||
logger.error({ decryptionJid, err: cleanupErr }, '❌ Failed to cleanup corrupted session')
|
||||
}
|
||||
}
|
||||
// Session cleanup is deferred to retry exhaustion (safety net).
|
||||
// The Signal Protocol handles recovery naturally via retry+pkmsg:
|
||||
// Bad MAC -> retry receipt -> sender re-sends as pkmsg -> new session.
|
||||
// Deleting sessions here (hot path) causes cascading failures when
|
||||
// multiple messages from the same contact arrive simultaneously.
|
||||
// See: messages-recv.ts sendRetryRequest() for deferred cleanup.
|
||||
} else if (isSessionRecord) {
|
||||
// Session record errors are transient - retry should handle them
|
||||
// eslint-disable-next-line max-depth
|
||||
@@ -504,10 +489,14 @@ export function isCorruptedSessionError(error: any): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up corrupted session by deleting all device sessions for a JID
|
||||
* Signal Protocol will automatically recreate the session on next message
|
||||
* Clean up corrupted session by deleting all device sessions for a JID.
|
||||
* Signal Protocol will automatically recreate the session on next message.
|
||||
*
|
||||
* NOTE: This should NOT be called on every Bad MAC error (hot path).
|
||||
* Instead, let the retry+pkmsg flow handle recovery naturally (like WhatsApp does).
|
||||
* Only call this as a safety net when retries are exhausted.
|
||||
*/
|
||||
async function cleanupCorruptedSession(
|
||||
export async function cleanupCorruptedSession(
|
||||
jid: string,
|
||||
repository: SignalRepositoryWithLIDStore,
|
||||
logger: ILogger
|
||||
|
||||
@@ -211,15 +211,33 @@ export class MessageRetryManager {
|
||||
}
|
||||
}
|
||||
|
||||
// MAC errors require IMMEDIATE session recreation regardless of history
|
||||
// This handles the case where contact reinstalled WhatsApp (identity key changed)
|
||||
// MAC errors require session recreation, but rate-limited to prevent loops.
|
||||
// When multiple messages fail with Bad MAC simultaneously, only the first
|
||||
// should trigger recreation; subsequent ones within the cooldown window
|
||||
// piggyback on the same new session established by the retry+pkmsg flow.
|
||||
if (errorCode !== undefined && MAC_ERROR_CODES.has(errorCode)) {
|
||||
this.sessionRecreateHistory.set(jid, Date.now())
|
||||
const now = Date.now()
|
||||
const prevTime = this.sessionRecreateHistory.get(jid)
|
||||
const MAC_ERROR_COOLDOWN_MS = 10_000 // 10 seconds
|
||||
|
||||
if (prevTime && now - prevTime < MAC_ERROR_COOLDOWN_MS) {
|
||||
const reasonName = RetryReason[errorCode] || `code_${errorCode}`
|
||||
this.logger.debug(
|
||||
{ jid, errorCode: reasonName, msSinceLast: now - prevTime },
|
||||
'MAC error session recreation skipped — cooldown active'
|
||||
)
|
||||
return {
|
||||
reason: '',
|
||||
recreate: false
|
||||
}
|
||||
}
|
||||
|
||||
this.sessionRecreateHistory.set(jid, now)
|
||||
this.statistics.sessionRecreations++
|
||||
const reasonName = RetryReason[errorCode] || `code_${errorCode}`
|
||||
metrics.signalMacErrors?.inc({ action: 'session_recreation' })
|
||||
metrics.signalSessionRecreations?.inc({ reason: 'mac_error' })
|
||||
this.logger.warn({ jid, errorCode: reasonName }, 'MAC error detected, forcing immediate session recreation')
|
||||
this.logger.warn({ jid, errorCode: reasonName }, 'MAC error detected, recreating session')
|
||||
return {
|
||||
reason: `MAC error (${reasonName}) - contact may have reinstalled WhatsApp`,
|
||||
recreate: true
|
||||
|
||||
Reference in New Issue
Block a user