From 80a5f5c9d2f023bb65a43b05be28f918c686a848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= Date: Mon, 6 Apr 2026 10:43:52 -0300 Subject: [PATCH 1/2] fix: invalidate sender key cache on device changes and handle identity notifications Root cause of "Waiting for messages" in group chats: when a participant added/removed a device, the sender key device cache was not invalidated, so SKDM was never sent to the new device on next group message. Device notification fixes (verified against WAWeb/Identity/UpdateDeviceTableApi): - patch_device_add: detect genuinely new devices and invalidate sender key device cache so SKDM is sent on next group message - patch_device_remove: delete Signal sessions for removed device (matching WA Web's deleteRemoteInfo), then invalidate sender key cache - Extract shared delete_sessions_for_devices() helper (DRY with clear_device_record) Identity change handler (verified against WAWeb/Handle/IdentityChange): - Handle type="encrypt" notifications with child (was silently ignored before) - Clear device record (sessions + sender keys) and invalidate device cache - Ignore companion devices (device != 0), matching WA Web - Dispatch new IdentityChange event for application layer --- src/client/device_registry.rs | 256 +++++++++++++++++++++++++++++----- src/handlers/notification.rs | 131 ++++++++++++++++- wacore/src/types/events.rs | 14 ++ 3 files changed, 366 insertions(+), 35 deletions(-) diff --git a/src/client/device_registry.rs b/src/client/device_registry.rs index ec8846824..a6f785930 100644 --- a/src/client/device_registry.rs +++ b/src/client/device_registry.rs @@ -206,6 +206,8 @@ impl Client { /// 4. Replace the full device record /// /// If `signed_bytes` is absent, falls back to simple append (lenient). + /// When a genuinely new device is added, invalidates the sender key device + /// cache so SKDM will be sent on the next group message. pub(crate) async fn patch_device_add( &self, user: &str, @@ -218,6 +220,8 @@ impl Client { return; }; + let devices_before: Vec = record.devices.iter().map(|d| d.device_id).collect(); + let signed_bytes = key_index_info.and_then(|ki| ki.signed_bytes.as_deref()); if let Some(bytes) = signed_bytes { @@ -261,6 +265,16 @@ impl Client { self.append_device_if_new(&mut record, device_id, device.key_index); } + // Detect new devices: any device_id present now that wasn't before. + // Invalidate sender key device cache so SKDM is sent on next group message. + let has_new_device = record + .devices + .iter() + .any(|d| !devices_before.contains(&d.device_id)); + if has_new_device { + self.sender_key_device_cache.invalidate_all(); + } + if let Err(e) = self.update_device_list(record).await { warn!("patch_device_add: failed to persist: {e}"); } @@ -281,47 +295,54 @@ impl Client { } } + /// Delete Signal sessions for specific device IDs under both LID and PN + /// addresses, then flush. Shared by `clear_device_record` and + /// `patch_device_remove`. + async fn delete_sessions_for_devices(&self, user: &str, device_ids: &[u16]) { + let lookup = self.resolve_lookup_keys(user).await; + let servers = [ + wacore_binary::jid::HIDDEN_USER_SERVER, + wacore_binary::jid::DEFAULT_USER_SERVER, + ]; + for &srv in &servers { + for key in lookup.all_keys() { + for &device_id in device_ids { + let mut jid = Jid::new(key, srv); + jid.device = device_id; + let addr = wacore::types::jid::JidExt::to_protocol_address(&jid); + self.signal_cache.delete_session(&addr).await; + } + } + } + if let Err(e) = self.flush_signal_cache().await { + warn!("delete_sessions_for_devices: failed to flush: {e}"); + } + } + /// Clear device record on raw_id mismatch (identity change). /// /// Matches WA Web's `clearDeviceRecord()` in `IdentityUpdateDeviceTableApi`: /// - Deletes Signal sessions for non-primary devices (stale identity) /// - Invalidates sender key device cache so SKDM will be redistributed - /// - Flushes cache to persist session deletions pub(crate) async fn clear_device_record( &self, user: &str, - server: &str, + _server: &str, record: &wacore::store::traits::DeviceListRecord, ) { - let non_primary_count = record.devices.iter().filter(|d| d.device_id != 0).count(); + let non_primary_ids: Vec = record + .devices + .iter() + .filter(|d| d.device_id != 0) + .map(|d| d.device_id as u16) + .collect(); info!( - "Clearing device record for user {user}: removing {non_primary_count} non-primary device(s) due to raw_id change", + "Clearing device record for user {user}: removing {} non-primary device(s) due to raw_id change", + non_primary_ids.len() ); - // Delete Signal sessions under BOTH LID and PN addresses. - // The notification may arrive via one address but sessions can exist - // under either (encrypt path is LID-first, decrypt stores under sender). - let lookup = self.resolve_lookup_keys(user).await; - let servers: &[&str] = match &lookup { - UserLookupKeys::LidWithPn { .. } | UserLookupKeys::PnWithLid { .. } => &[ - wacore_binary::jid::HIDDEN_USER_SERVER, - wacore_binary::jid::DEFAULT_USER_SERVER, - ], - UserLookupKeys::Unknown { .. } => std::slice::from_ref(&server), - }; - for &srv in servers { - for key in lookup.all_keys() { - for device in record.devices.iter().filter(|d| d.device_id != 0) { - let mut jid = Jid::new(key, srv); - jid.device = device.device_id as u16; - let addr = wacore::types::jid::JidExt::to_protocol_address(&jid); - self.signal_cache.delete_session(&addr).await; - } - } - } - if let Err(e) = self.flush_signal_cache().await { - warn!("clear_device_record: failed to flush session deletions: {e}"); - } + self.delete_sessions_for_devices(user, &non_primary_ids) + .await; // Clear persisted SKDM tracking across ALL groups so stale has_key=true // rows don't survive restart. Identity changes are rare so the cost is acceptable. @@ -333,19 +354,28 @@ impl Client { { warn!("clear_device_record: failed to clear persisted sender key devices: {e}"); } - // Also invalidate in-memory cache self.sender_key_device_cache.invalidate_all(); } /// Remove a device from the registry after a device remove notification. + /// + /// Matches WA Web's `bulkApplyDeviceUpdate` cleanup for removed devices + /// (`UpdateDeviceTableApi`): deletes Signal sessions for the device, + /// then invalidates the sender key device cache so SKDM will be + /// redistributed on the next group send. pub(crate) async fn patch_device_remove(&self, user: &str, device_id: u32) { if let Some(mut record) = self.load_device_record(user).await { let before = record.devices.len(); record.devices.retain(|d| d.device_id != device_id); - if record.devices.len() != before - && let Err(e) = self.update_device_list(record).await - { - warn!("patch_device_remove: failed to persist: {e}"); + if record.devices.len() != before { + if device_id != 0 { + self.delete_sessions_for_devices(user, &[device_id as u16]) + .await; + } + self.sender_key_device_cache.invalidate_all(); + if let Err(e) = self.update_device_list(record).await { + warn!("patch_device_remove: failed to persist: {e}"); + } } } } @@ -1197,4 +1227,164 @@ mod tests { assert_eq!(updated.devices.len(), 1); assert_eq!(updated.devices[0].device_id, 0); } + + // ── Sender key device cache invalidation tests ────────────────────── + + #[tokio::test] + async fn test_patch_device_add_invalidates_sender_key_cache() { + use crate::sender_key_device_cache::SenderKeyDeviceMap; + use wacore::store::traits::{DeviceInfo, DeviceListRecord}; + + let client = create_test_client().await; + + // Pre-populate device registry with device 0 only + let record = DeviceListRecord { + user: "15551234567".into(), + devices: vec![DeviceInfo { + device_id: 0, + key_index: None, + }], + timestamp: wacore::time::now_secs(), + phash: None, + raw_id: None, + }; + client + .device_registry_cache + .insert("15551234567".into(), record) + .await; + + // Warm the sender key device cache for a group + let group = "120363000000000001@g.us"; + let map = + SenderKeyDeviceMap::from_db_rows(&[("15551234567:0@s.whatsapp.net".into(), true)]); + client + .sender_key_device_cache + .get_or_init(group, async { std::sync::Arc::new(map) }) + .await; + + // Add device 3 — should invalidate sender key cache + let elem = make_device_element(3, Some(5)); + client.patch_device_add("15551234567", &elem, None).await; + + // Sender key cache should be cleared (get_or_init would need to re-fetch) + // We verify by checking that the cached map doesn't contain the old entry + // anymore through the cache's internal state. Since invalidate_all() was + // called, re-init will produce a fresh map. + let fresh_map = SenderKeyDeviceMap::from_db_rows(&[]); + let result = client + .sender_key_device_cache + .get_or_init(group, async { std::sync::Arc::new(fresh_map) }) + .await; + assert!( + result.is_empty(), + "sender key cache should have been invalidated and re-initialized empty" + ); + } + + #[tokio::test] + async fn test_patch_device_add_no_invalidation_when_device_exists() { + use crate::sender_key_device_cache::SenderKeyDeviceMap; + use wacore::store::traits::{DeviceInfo, DeviceListRecord}; + + let client = create_test_client().await; + + // Pre-populate device registry with device 0 AND device 3 + let record = DeviceListRecord { + user: "15551234567".into(), + devices: vec![ + DeviceInfo { + device_id: 0, + key_index: None, + }, + DeviceInfo { + device_id: 3, + key_index: Some(5), + }, + ], + timestamp: wacore::time::now_secs(), + phash: None, + raw_id: None, + }; + client + .device_registry_cache + .insert("15551234567".into(), record) + .await; + + // Warm the sender key device cache + let group = "120363000000000001@g.us"; + let map = SenderKeyDeviceMap::from_db_rows(&[ + ("15551234567:0@s.whatsapp.net".into(), true), + ("15551234567:3@s.whatsapp.net".into(), true), + ]); + client + .sender_key_device_cache + .get_or_init(group, async { std::sync::Arc::new(map) }) + .await; + + // Re-add device 3 (already exists) — should NOT invalidate cache + let elem = make_device_element(3, Some(5)); + client.patch_device_add("15551234567", &elem, None).await; + + // Cache should still have the old entry + let cached = client + .sender_key_device_cache + .get_or_init(group, async { + panic!("init should not be called — cache should still be warm") + }) + .await; + assert!(!cached.is_empty(), "cache should still be warm"); + } + + #[tokio::test] + async fn test_patch_device_remove_invalidates_sender_key_cache() { + use crate::sender_key_device_cache::SenderKeyDeviceMap; + use wacore::store::traits::{DeviceInfo, DeviceListRecord}; + + let client = create_test_client().await; + + let record = DeviceListRecord { + user: "15551234567".into(), + devices: vec![ + DeviceInfo { + device_id: 0, + key_index: None, + }, + DeviceInfo { + device_id: 3, + key_index: None, + }, + ], + timestamp: wacore::time::now_secs(), + phash: None, + raw_id: None, + }; + client + .device_registry_cache + .insert("15551234567".into(), record) + .await; + + // Warm sender key device cache + let group = "120363000000000001@g.us"; + let map = SenderKeyDeviceMap::from_db_rows(&[ + ("15551234567:0@s.whatsapp.net".into(), true), + ("15551234567:3@s.whatsapp.net".into(), true), + ]); + client + .sender_key_device_cache + .get_or_init(group, async { std::sync::Arc::new(map) }) + .await; + + // Remove device 3 — should invalidate sender key cache + client.patch_device_remove("15551234567", 3).await; + + let fresh_map = SenderKeyDeviceMap::from_db_rows(&[]); + let result = client + .sender_key_device_cache + .get_or_init(group, async { std::sync::Arc::new(fresh_map) }) + .await; + assert!( + result.is_empty(), + "sender key cache should have been invalidated after device removal" + ); + } } diff --git a/src/handlers/notification.rs b/src/handlers/notification.rs index f99df9367..860796386 100644 --- a/src/handlers/notification.rs +++ b/src/handlers/notification.rs @@ -46,8 +46,15 @@ async fn handle_notification_impl(client: &Arc, node: &Node) { match notification_type { "encrypt" => { - if node.attrs.get("from").is_some_and(|v| v == SERVER_JID) { - // Dispatch based on first child tag, matching WA Web's handleEncryptNotification. + // Identity change: + // + // + // WA Web: WAWebHandleIdentityChange — clears device record, deletes sessions, + // marks sender keys for rotation, re-establishes session. + if node.get_optional_child("identity").is_some() { + handle_identity_change(client, node).await; + } else if node.attrs.get("from").is_some_and(|v| v == SERVER_JID) { + // Server-originated encrypt notifications: // "count" → handlePreKeyLow, "digest" → handleDigestKey let first_child_tag = node .children() @@ -304,6 +311,55 @@ fn handle_digest_key(client: &Arc) { .detach(); } +/// Handle identity change notification (user reinstalled WhatsApp). +/// +/// Matches WA Web's `WAWebHandleIdentityChange`: +/// ```xml +/// +/// +/// +/// ``` +/// Clears device record (sessions + sender keys) for the user so that +/// fresh sessions are established and SKDM is redistributed on next send. +async fn handle_identity_change(client: &Arc, node: &Node) { + let Some(from_jid) = node.attrs().optional_jid("from") else { + warn!("Identity change notification missing 'from' attribute"); + return; + }; + + // WA Web ignores companion devices (device != 0) — only primary identity matters + if from_jid.device != 0 { + debug!( + "Ignoring identity change from companion device {}", + from_jid + ); + return; + } + + info!( + "Identity change for user {}: clearing device record", + from_jid.user + ); + + // Load existing record to pass to clear_device_record + if let Some(record) = client.load_device_record(&from_jid.user).await { + client + .clear_device_record(&from_jid.user, &from_jid.server, &record) + .await; + } + + // Invalidate device cache so next send triggers fresh usync + client.invalidate_device_cache(&from_jid.user).await; + + // Dispatch event so application layer can show security code change + client.core.event_bus.dispatch(&Event::IdentityChange( + crate::types::events::IdentityChange { + user: from_jid, + lid_user: node.attrs().optional_jid("lid"), + }, + )); +} + /// Handle device list change notifications. /// Matches WhatsApp Web's WAWebHandleDeviceNotification.handleDevicesNotification(). /// @@ -1863,4 +1919,75 @@ mod tests { "hash-only update without jid should not dispatch events" ); } + + #[tokio::test] + async fn test_identity_change_dispatches_event_and_invalidates_cache() { + let client = create_test_client().await; + let collector = Arc::new(TestEventCollector::default()); + client.register_handler(collector.clone()); + + // Pre-populate device registry so clear_device_record has something to clear + let record = wacore::store::traits::DeviceListRecord { + user: "5511999999999".into(), + devices: vec![wacore::store::traits::DeviceInfo { + device_id: 1, + key_index: None, + }], + timestamp: wacore::time::now_secs(), + phash: None, + raw_id: Some(42), + }; + client + .device_registry_cache + .insert("5511999999999".into(), record) + .await; + + // Simulate identity change notification: type="encrypt" with child + let node = NodeBuilder::new("notification") + .attr("type", "encrypt") + .attr("from", "5511999999999@s.whatsapp.net") + .attr("id", "identity-change-1") + .children([NodeBuilder::new("identity").build()]) + .build(); + handle_notification_impl(&client, &node).await; + + // Should have dispatched IdentityChange event + let events = collector.events(); + assert!( + events.iter().any(|e| matches!(e, Event::IdentityChange(_))), + "should dispatch IdentityChange event, got: {:?}", + events + ); + + // Device registry cache should be invalidated + assert!( + client + .device_registry_cache + .get("5511999999999") + .await + .is_none(), + "device registry cache should be invalidated after identity change" + ); + } + + #[tokio::test] + async fn test_identity_change_ignores_companion_device() { + let client = create_test_client().await; + let collector = Arc::new(TestEventCollector::default()); + client.register_handler(collector.clone()); + + // Companion device (device=5) — should be ignored per WA Web + let node = NodeBuilder::new("notification") + .attr("type", "encrypt") + .attr("from", "5511999999999:5@s.whatsapp.net") + .attr("id", "identity-change-2") + .children([NodeBuilder::new("identity").build()]) + .build(); + handle_notification_impl(&client, &node).await; + + assert!( + collector.events().is_empty(), + "companion device identity change should be ignored" + ); + } } diff --git a/wacore/src/types/events.rs b/wacore/src/types/events.rs index 70d912f9b..c78fb4db8 100644 --- a/wacore/src/types/events.rs +++ b/wacore/src/types/events.rs @@ -304,6 +304,17 @@ pub struct DeviceListUpdate { pub contact_hash: Option, } +/// Identity key changed for a user (e.g., user reinstalled WhatsApp). +/// Emitted after device record cleanup so sessions and sender keys are cleared. +#[derive(Debug, Clone, Serialize)] +pub struct IdentityChange { + /// The user whose identity changed + pub user: Jid, + /// Optional LID for the user + #[serde(skip_serializing_if = "Option::is_none")] + pub lid_user: Option, +} + /// Type of business status update. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] pub enum BusinessUpdateType { @@ -437,6 +448,9 @@ pub enum Event { /// Device list changed for a user (device added/removed/updated) DeviceListUpdate(DeviceListUpdate), + /// Identity key changed (user reinstalled WhatsApp) + IdentityChange(IdentityChange), + /// Business account status changed (verified name, profile, conversion to personal) BusinessStatusUpdate(BusinessStatusUpdate), From 262167e1429f5aeeb370cde8a25dba11bcb92e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= Date: Mon, 6 Apr 2026 10:52:24 -0300 Subject: [PATCH 2/2] fix: skip self-identity change to avoid clearing own device record WA Web's isMePrimary check (IdentityChange.js:58) prevents processing identity change notifications for our own primary device. Without this, we would clear our own sessions and device record. --- src/handlers/notification.rs | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/handlers/notification.rs b/src/handlers/notification.rs index 860796386..291732137 100644 --- a/src/handlers/notification.rs +++ b/src/handlers/notification.rs @@ -336,6 +336,22 @@ async fn handle_identity_change(client: &Arc, node: &Node) { return; } + // WA Web: isMePrimary(h) → handleSelfPrimaryIdentityChange (different flow). + // We must not clear our own device record. + let device_snapshot = client.persistence_manager.get_device_snapshot().await; + let is_me = device_snapshot + .pn + .as_ref() + .is_some_and(|pn| pn.user == from_jid.user) + || device_snapshot + .lid + .as_ref() + .is_some_and(|lid| lid.user == from_jid.user); + if is_me { + debug!("Ignoring self-primary identity change"); + return; + } + info!( "Identity change for user {}: clearing device record", from_jid.user @@ -1970,6 +1986,35 @@ mod tests { ); } + #[tokio::test] + async fn test_identity_change_ignores_self_primary() { + let client = create_test_client().await; + let collector = Arc::new(TestEventCollector::default()); + client.register_handler(collector.clone()); + + // Set our own JID so the self-check works + client + .persistence_manager + .modify_device(|d| { + d.pn = Some("5511999999999@s.whatsapp.net".parse().unwrap()); + }) + .await; + + // Identity change FROM our own JID — should be ignored per WA Web's isMePrimary + let node = NodeBuilder::new("notification") + .attr("type", "encrypt") + .attr("from", "5511999999999@s.whatsapp.net") + .attr("id", "identity-change-self") + .children([NodeBuilder::new("identity").build()]) + .build(); + handle_notification_impl(&client, &node).await; + + assert!( + collector.events().is_empty(), + "self identity change should be ignored" + ); + } + #[tokio::test] async fn test_identity_change_ignores_companion_device() { let client = create_test_client().await;