fix: address all 6 Copilot AI review comments from PR #154
Fixes all valid issues identified by Copilot AI review: 1. 🔴 CRITICAL: Fix deduplication key collision risk - Before: Used maskedJid in dedupe key (e.g., "4680****7890") - Problem: Different JIDs with same first/last 4 digits collide - After: Use original unmasked JID for dedup key - Impact: Prevents legitimate errors from being suppressed 2. 🟡 Fix JID masking logic flaw - Before: Masked full JID string including domain (e.g., "1234****.net") - Problem: Obscures useful info while masking wrong part - After: Decode JID, mask only user portion, preserve domain - Result: "4680****7890@s.whatsapp.net" instead of "1234****.net" 3. 🟡 Add structured logging context - Before: logger.info(string) - loses queryability - After: logger.info({ jid, maskedJid, targetedDevices }, message) - Impact: Maintains observability and log filtering capability 4. 🟡 Fix misleading deletion count wording - Before: "Cleared: X devices" (implies actual deletions) - Problem: deleteSession() returns void, count is targeted - After: "Targeted: X devices" (reflects actual behavior) 5. 🟢 Remove unused variable - Removed _origConsoleLog (dead code after removing console.log interceptor) - Prevents no-unused-vars lint failure 6. 🟢 Fix semicolon format violation - Repository config: semi: false (Prettier) - Removed trailing semicolon from JID extraction line - Prevents eslint-plugin-prettier check failure All changes maintain backward compatibility while fixing correctness, observability, and code quality issues. https://claude.ai/code/session_01TvSrN9JmHZDdKMZDFWEcUS
This commit is contained in:
@@ -427,13 +427,16 @@ export const decryptMessageNode = (
|
||||
try {
|
||||
const deletedCount = await cleanupCorruptedSession(decryptionJid, repository, logger)
|
||||
|
||||
// Mask JID for privacy (show first 4 and last 4 digits)
|
||||
const maskedJid = decryptionJid.length > 8
|
||||
? `${decryptionJid.substring(0, 4)}****${decryptionJid.substring(decryptionJid.length - 4)}`
|
||||
: decryptionJid
|
||||
// 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(
|
||||
`🔄 Session Reset | JID: ${maskedJid} | Cleared: ${deletedCount} devices | Will recreate on next message`
|
||||
{ jid: decryptionJid, maskedJid, targetedDevices: deletedCount },
|
||||
`🔄 Session Reset | JID: ${maskedJid} | Targeted: ${deletedCount} devices | Will recreate on next message`
|
||||
)
|
||||
} catch (cleanupErr) {
|
||||
logger.error(
|
||||
|
||||
+3
-4
@@ -2,7 +2,6 @@
|
||||
// Format session error logs from libsignal BEFORE any imports
|
||||
// This MUST be at the very top to intercept console before libsignal loads
|
||||
// ============================================
|
||||
const _origConsoleLog = console.log
|
||||
const _origConsoleError = console.error
|
||||
|
||||
// Track errors by type + JID to avoid duplicates (using Map for better performance)
|
||||
@@ -36,7 +35,7 @@ console.error = function(...args: unknown[]) {
|
||||
else if (msg.includes('Failed to decrypt')) errorType = '🔌 Decryption Failed'
|
||||
|
||||
// Extract JID from stack trace or message
|
||||
const jidMatch = (msg + String(args[1] ?? '')).match(/(\d{10,}(?:_\d+\.\d+)?)/);
|
||||
const jidMatch = (msg + String(args[1] ?? '')).match(/(\d{10,}(?:_\d+\.\d+)?)/)
|
||||
const jid = jidMatch ? jidMatch[1] : null
|
||||
const maskedJid = jid && jid.length > 8 ? `${jid.substring(0, 4)}****${jid.substring(jid.length - 4)}` : jid
|
||||
|
||||
@@ -45,8 +44,8 @@ console.error = function(...args: unknown[]) {
|
||||
? `${errorType} | JID: ${maskedJid}`
|
||||
: errorType
|
||||
|
||||
// Deduplication key: type + JID (prevents duplicate logs for same error type on same contact)
|
||||
const dedupeKey = `${errorType}:${maskedJid || 'unknown'}`
|
||||
// Deduplication key: type + ORIGINAL JID (use unmasked to prevent collisions)
|
||||
const dedupeKey = `${errorType}:${jid || 'unknown'}`
|
||||
const now = Date.now()
|
||||
const lastTime = _errorTimestamps.get(dedupeKey)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user