refactor: codebase quality audit — safety, tests, DRY, dep hygiene

Cargo.toml

@@ -70,6 +70,7 @@ sha1 = { version = "0.10.6", default-features = false }
 sha2 = { version = "0.10.6", default-features = false }
 thiserror = "2.0.17"
 tokio = { version = "1.48.0", default-features = false }
+uuid = { version = "1", default-features = false }
 
 # Internal workspace crates
 wacore = { path = "./wacore", default-features = false, version = "0.4.1" }
@@ -132,7 +133,7 @@ whatsapp-rust-tokio-transport = { path = "./transports/tokio-transport", version
 whatsapp-rust-ureq-http-client = { path = "./http_clients/ureq-client", version = "0.4.1", optional = true }
 
 [dev-dependencies]
-uuid = { version = "1.0", features = ["v4"] }
+uuid = { workspace = true, features = ["v4"] }
 
 [[example]]
 name = "benchmark"

src/appstate_sync.rs

@@ -2,12 +2,6 @@
 pub use wacore::appstate::Mutation;
 pub use wacore::appstate_sync::{AppStateProcessor, AppStateSyncDriver, AppStateSyncError};
 
-// The following re-exports are used by the test module
-#[allow(unused_imports)]
-use wacore::appstate::hash::HashState;
-#[allow(unused_imports)]
-use wacore::appstate::patch_decode::{PatchList, WAPatchName};
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/client.rs

@@ -1014,8 +1014,15 @@ impl Client {
         self.signal_cache.clear().await;
         // Reset message processing semaphore to 1 permit (sequential mode for next offline sync).
         // Old workers holding the previous semaphore Arc will finish normally.
-        *self.message_processing_semaphore.lock().unwrap() =
-            Arc::new(async_lock::Semaphore::new(1));
+        // Scoped block ensures MutexGuard is dropped before any .await (MutexGuard is !Send).
+        // Handles poison gracefully — the semaphore is still safe to replace.
+        {
+            let mut guard = match self.message_processing_semaphore.lock() {
+                Ok(g) => g,
+                Err(poisoned) => poisoned.into_inner(),
+            };
+            *guard = Arc::new(async_lock::Semaphore::new(1));
+        }
         // Reset dead-socket timestamps so stale values from the previous
         // connection don't trigger an immediate reconnect on the next one.
         self.last_data_received_ms.store(0, Ordering::Relaxed);

src/features/chat_actions.rs

@@ -19,7 +19,6 @@ use crate::client::Client;
 use anyhow::Result;
 use chrono::DateTime;
 use log::debug;
-use std::sync::Arc;
 use wacore::appstate::patch_decode::WAPatchName;
 use wacore::types::events::{
     ArchiveUpdate, ContactUpdate, Event, MarkChatAsReadUpdate, MuteUpdate, PinUpdate, StarUpdate,
@@ -208,13 +207,13 @@ pub(crate) fn dispatch_chat_mutation(
 
 /// Feature handle for chat management actions.
 ///
-/// Access via `client.chat_actions()` (requires `Arc<Client>`).
+/// Access via `client.chat_actions()`.
 pub struct ChatActions<'a> {
-    client: &'a Arc<Client>,
+    client: &'a Client,
 }
 
 impl<'a> ChatActions<'a> {
-    pub(crate) fn new(client: &'a Arc<Client>) -> Self {
+    pub(crate) fn new(client: &'a Client) -> Self {
         Self { client }
     }
 
@@ -451,8 +450,8 @@ impl<'a> ChatActions<'a> {
 impl Client {
     /// Access chat management actions (archive, pin, mute, star).
     ///
-    /// Requires `Arc<Client>` because app state mutations need key access.
-    pub fn chat_actions(self: &Arc<Self>) -> ChatActions<'_> {
+    /// Access chat management actions (archive, pin, mute, star).
+    pub fn chat_actions(&self) -> ChatActions<'_> {
         ChatActions::new(self)
     }
 }

src/features/media_reupload.rs

@@ -9,7 +9,6 @@
 use crate::client::{Client, ClientError, NodeFilter};
 use anyhow::Result;
 use log::debug;
-use std::sync::Arc;
 use std::time::Duration;
 pub use wacore::media_retry::MediaRetryResult;
 use wacore::media_retry::{
@@ -34,11 +33,11 @@ pub struct MediaReuploadRequest<'a> {
 }
 
 pub struct MediaReupload<'a> {
-    client: &'a Arc<Client>,
+    client: &'a Client,
 }
 
 impl<'a> MediaReupload<'a> {
-    pub(crate) fn new(client: &'a Arc<Client>) -> Self {
+    pub(crate) fn new(client: &'a Client) -> Self {
         Self { client }
     }
 
@@ -115,7 +114,7 @@ impl<'a> MediaReupload<'a> {
 
 impl Client {
     /// Access media reupload operations.
-    pub fn media_reupload(self: &Arc<Self>) -> MediaReupload<'_> {
+    pub fn media_reupload(&self) -> MediaReupload<'_> {
         MediaReupload::new(self)
     }
 }

src/features/profile.rs

@@ -6,7 +6,6 @@ use crate::client::Client;
 use crate::store::commands::DeviceCommand;
 use anyhow::Result;
 use log::{debug, warn};
-use std::sync::Arc;
 use wacore::iq::contacts::SetProfilePictureSpec;
 use wacore::iq::profile::SetStatusTextSpec;
 use wacore_binary::builder::NodeBuilder;
@@ -15,11 +14,11 @@ pub use wacore::iq::contacts::SetProfilePictureResponse;
 
 /// Feature handle for profile operations.
 pub struct Profile<'a> {
-    client: &'a Arc<Client>,
+    client: &'a Client,
 }
 
 impl<'a> Profile<'a> {
-    pub(crate) fn new(client: &'a Arc<Client>) -> Self {
+    pub(crate) fn new(client: &'a Client) -> Self {
         Self { client }
     }
 
@@ -163,8 +162,8 @@ impl<'a> Profile<'a> {
 }
 
 impl Client {
-    /// Access profile operations (requires Arc<Client>).
-    pub fn profile(self: &Arc<Self>) -> Profile<'_> {
+    /// Access profile operations.
+    pub fn profile(&self) -> Profile<'_> {
         Profile::new(self)
     }
 }

src/message.rs

@@ -496,7 +496,10 @@ impl Client {
         // Acquire global processing permit. During offline sync (1 permit), this serializes
         // ALL message processing globally, matching WA Web's allChatQueue pattern.
         // After offline sync completes, permits are increased for parallel processing.
-        let semaphore = self.message_processing_semaphore.lock().unwrap().clone();
+        let semaphore = match self.message_processing_semaphore.lock() {
+            Ok(guard) => guard.clone(),
+            Err(poisoned) => poisoned.into_inner().clone(),
+        };
         let _global_permit = semaphore.acquire_arc().await;
 
         log::debug!(

tests/e2e/Cargo.toml

@@ -14,7 +14,7 @@ dhat-heap = ["dep:dhat"]
 anyhow = { workspace = true }
 dhat = { version = "0.3", optional = true }
 tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] }
-uuid = { version = "1.0", features = ["v4"] }
+uuid = { workspace = true, features = ["v4"] }
 wacore = { path = "../../wacore" }
 whatsapp-rust = { path = "../..", features = ["danger-skip-tls-verify", "debug-diagnostics"] }
 whatsapp-rust-sqlite-storage = { path = "../../storages/sqlite-storage" }

wacore/libsignal/Cargo.toml

@@ -22,7 +22,6 @@ ghash = "0.6.0"
 hex = { workspace = true }
 hkdf = { workspace = true }
 hmac = { workspace = true }
-itertools = { version = "0.14.0", default-features = false, features = ["use_alloc"] }
 log = { workspace = true }
 prost = { workspace = true }
 rand = { workspace = true }
@@ -31,12 +30,12 @@ sha1 = { workspace = true }
 sha2 = { workspace = true }
 subtle = "2.6.1"
 thiserror = { workspace = true }
-uuid = { version = "1.18.1", default-features = false }
+uuid = { workspace = true }
 waproto = { workspace = true }
 x25519-dalek = { version = "2.0.1", features = ["static_secrets"] }
 
 [dev-dependencies]
-futures = { version = "0.3", default-features = false, features = ["executor"] }
+futures = { workspace = true, features = ["executor"] }
 iai-callgrind = { workspace = true }
 
 [[bench]]

wacore/libsignal/src/crypto/aes_cbc.rs

@@ -117,4 +117,140 @@ mod tests {
             .expect("Decryption failed");
         assert_eq!(decrypted, plaintext);
     }
+
+    /// Basic encrypt/decrypt roundtrip
+    #[test]
+    fn test_aes_cbc_roundtrip() {
+        let key = [0x42u8; 32];
+        let iv = [0x11u8; 16];
+        let plaintext = b"hello world, this is a roundtrip test!";
+
+        let mut ciphertext = Vec::new();
+        aes_256_cbc_encrypt_into(plaintext, &key, &iv, &mut ciphertext).unwrap();
+
+        // Ciphertext must differ from plaintext
+        assert_ne!(&ciphertext[..], &plaintext[..]);
+
+        let mut decrypted = Vec::new();
+        aes_256_cbc_decrypt_into(&ciphertext, &key, &iv, &mut decrypted).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    /// NIST AES-256-CBC known-answer vector (from NIST SP 800-38A, Section F.2.5/F.2.6)
+    /// Key: 603deb1015ca71be2b73aef0857d77811f352c073b6108d72d9810a30914dff4
+    /// IV:  000102030405060708090a0b0c0d0e0f
+    /// Plaintext block 1: 6bc1bee22e409f96e93d7e117393172a
+    /// Ciphertext block 1: f58c4c04d6e5f1ba779eabfb5f7bfbd6
+    ///
+    /// We test a single block here. Because our API uses PKCS7 padding and the NIST
+    /// vector does not include padding, we verify only the first ciphertext block.
+    #[test]
+    fn test_aes_256_cbc_nist_vector() {
+        let key: [u8; 32] = [
+            0x60, 0x3d, 0xeb, 0x10, 0x15, 0xca, 0x71, 0xbe, 0x2b, 0x73, 0xae, 0xf0, 0x85, 0x7d,
+            0x77, 0x81, 0x1f, 0x35, 0x2c, 0x07, 0x3b, 0x61, 0x08, 0xd7, 0x2d, 0x98, 0x10, 0xa3,
+            0x09, 0x14, 0xdf, 0xf4,
+        ];
+        let iv: [u8; 16] = [
+            0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d,
+            0x0e, 0x0f,
+        ];
+        let plaintext: [u8; 16] = [
+            0x6b, 0xc1, 0xbe, 0xe2, 0x2e, 0x40, 0x9f, 0x96, 0xe9, 0x3d, 0x7e, 0x11, 0x73, 0x93,
+            0x17, 0x2a,
+        ];
+        let expected_ct_block: [u8; 16] = [
+            0xf5, 0x8c, 0x4c, 0x04, 0xd6, 0xe5, 0xf1, 0xba, 0x77, 0x9e, 0xab, 0xfb, 0x5f, 0x7b,
+            0xfb, 0xd6,
+        ];
+
+        let mut ciphertext = Vec::new();
+        aes_256_cbc_encrypt_into(&plaintext, &key, &iv, &mut ciphertext).unwrap();
+
+        // Our output has PKCS7 padding (2 blocks), but the first block must match NIST
+        assert_eq!(ciphertext.len(), 32, "16-byte input + PKCS7 = 32 bytes");
+        assert_eq!(
+            &ciphertext[..16],
+            &expected_ct_block,
+            "First ciphertext block must match NIST vector"
+        );
+
+        // Verify full roundtrip
+        let mut decrypted = Vec::new();
+        aes_256_cbc_decrypt_into(&ciphertext, &key, &iv, &mut decrypted).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    /// Empty plaintext produces exactly one padding block (16 bytes)
+    #[test]
+    fn test_aes_cbc_empty_plaintext() {
+        let key = [0x42u8; 32];
+        let iv = [0x11u8; 16];
+
+        let mut ciphertext = Vec::new();
+        aes_256_cbc_encrypt_into(b"", &key, &iv, &mut ciphertext).unwrap();
+
+        // PKCS7 on empty input adds a full 16-byte padding block
+        assert_eq!(ciphertext.len(), 16);
+
+        let mut decrypted = Vec::new();
+        aes_256_cbc_decrypt_into(&ciphertext, &key, &iv, &mut decrypted).unwrap();
+        assert!(decrypted.is_empty());
+    }
+
+    /// Exact block-boundary plaintext (16 bytes) must add a full padding block
+    #[test]
+    fn test_aes_cbc_exact_block_boundary() {
+        let key = [0x42u8; 32];
+        let iv = [0x11u8; 16];
+        let plaintext = [0xAA; 16]; // exactly one block
+
+        let mut ciphertext = Vec::new();
+        aes_256_cbc_encrypt_into(&plaintext, &key, &iv, &mut ciphertext).unwrap();
+
+        // 16 bytes plaintext + 16 bytes PKCS7 padding = 32 bytes
+        assert_eq!(ciphertext.len(), 32);
+
+        let mut decrypted = Vec::new();
+        aes_256_cbc_decrypt_into(&ciphertext, &key, &iv, &mut decrypted).unwrap();
+        assert_eq!(decrypted, plaintext);
+    }
+
+    /// Decrypt with wrong key returns error
+    #[test]
+    fn test_aes_cbc_decrypt_wrong_key() {
+        let key = [0x42u8; 32];
+        let wrong_key = [0x43u8; 32];
+        let iv = [0x11u8; 16];
+        let plaintext = b"secret message";
+
+        let mut ciphertext = Vec::new();
+        aes_256_cbc_encrypt_into(plaintext, &key, &iv, &mut ciphertext).unwrap();
+
+        let mut decrypted = Vec::new();
+        let result = aes_256_cbc_decrypt_into(&ciphertext, &wrong_key, &iv, &mut decrypted);
+        assert!(result.is_err());
+    }
+
+    /// Decrypt with bad ciphertext length returns error
+    #[test]
+    fn test_aes_cbc_decrypt_bad_ciphertext_length() {
+        let key = [0x42u8; 32];
+        let iv = [0x11u8; 16];
+
+        // Empty ciphertext
+        let mut output = Vec::new();
+        let result = aes_256_cbc_decrypt_into(&[], &key, &iv, &mut output);
+        assert!(result.is_err());
+
+        // Non-multiple-of-16 ciphertext
+        let bad_ct = vec![0xAA; 15];
+        let result = aes_256_cbc_decrypt_into(&bad_ct, &key, &iv, &mut output);
+        assert!(result.is_err());
+
+        // Another non-multiple-of-16
+        let bad_ct = vec![0xBB; 17];
+        let result = aes_256_cbc_decrypt_into(&bad_ct, &key, &iv, &mut output);
+        assert!(result.is_err());
+    }
 }