fix(whatsapp): upgrade WhatsApp Web channel to upstream whatsapp-rust 0.5#916
Merged
senamakel merged 7 commits intoApr 27, 2026
Merged
Conversation
… 0.5 The previous wa-rs 0.2 fork silently dropped Event::Message for LID-addressed contacts and group sender-key (skmsg) messages — the protocol layer decrypted the payload but never dispatched it to user code, breaking agent dispatch for the bulk of modern WhatsApp traffic (LID is now the default). Upstream whatsapp-rust 0.5 fixed this via PR tinyhumansai#170 (SKDM tracking) + PR tinyhumansai#181 (LID/PN mapping) + sender-key dispatch. Verified locally: 70 Event::Message events fired in 30 minutes including 59 group + LID messages on a real account. Bonus: deletes the 1,345-line custom RusqliteStore. Upstream now ships SqliteStore natively. Breaking change: existing paired sessions need to re-pair on first launch because the SQLite schemas differ between the old fork and upstream's SqliteStore. Pairing is ~30 seconds via QR. Channel trait surface (send/listen/health_check/start_typing/stop_typing) unchanged so runtime/startup.rs and downstream consumers are not affected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the wa-rs dependency set with whatsapp-rust v0.5, removes the custom RusqliteStore, and rewrites the WhatsApp provider to use upstream SqliteStore, TokioRuntime, updated inbound/outbound logic (self-echo filtering, text extraction, reply routing, provenance), AtomicBool health tracking, and integrated shutdown hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant App as OpenHuman App
participant Bot as whatsapp-rust Bot
participant Store as SqliteStore
participant Runtime as TokioRuntime
App->>Bot: init(session_path, allowed_numbers)
App->>Runtime: supply TokioRuntime
App->>Store: supply SqliteStore
Bot->>Runtime: spawn transport & handlers
Bot->>Store: load session & app-state
Bot-->>App: Connected event (connected = true)
Note over Bot,App: Incoming message handling
Bot->>App: inbound(message payload, jid)
App->>App: filter self-echo, extract text, normalize sender, compute reply_target, record provenance
App->>Bot: outbound(send to jid) [should_allow_outbound check]
Note over App,Bot: Shutdown
App->>Bot: core shutdown (connected = false, abort/clear client)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/openhuman/channels/providers/whatsapp_web.rs (3)
240-264:⚠️ Potential issue | 🟠 MajorUse the chat JID as
reply_targetfor group messages.
reply_targetis currently set to the participant number even thoughinfo.source.chatis already available. For@g.ustraffic that will send downstream replies to the author’s DM instead of back to the originating group.Suggested fix
let chat = info.source.chat.to_string(); + let reply_target = if chat.ends_with("@g.us") { + chat.clone() + } else { + normalized.clone() + }; tracing::info!( "📨 WhatsApp message from {} in {}: {}", normalized, chat, text ); if allowed_numbers.is_empty() || allowed_numbers.iter().any(|n| n == "*" || n == &normalized) { if let Err(e) = tx_inner .send(ChannelMessage { id: uuid::Uuid::new_v4().to_string(), channel: "whatsapp".to_string(), sender: normalized.clone(), - reply_target: normalized.clone(), + reply_target, content: text, timestamp: chrono::Utc::now().timestamp_millis() as u64, thread_ts: None, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 240 - 264, The code sets reply_target to the participant number (normalized) which causes replies to be DM'd instead of to the group; change the logic where ChannelMessage is built (look for ChannelMessage creation in this block where sender_user/normalized and info.source.chat are used) to set reply_target to info.source.chat.to_string() when the chat JID indicates a group (e.g., contains "@g.us"), otherwise keep reply_target as the participant normalized number; ensure ChannelMessage.reply_target is updated accordingly before tx_inner.send so group replies go to the group JID.
282-290:⚠️ Potential issue | 🟠 Major
health_check()stays healthy after logout or stream failure.
health_check()is keyed only offbot_handle.is_some(), but that handle is never cleared whenEvent::LoggedOutorEvent::StreamErrorfires. Once the session dies, callers will still seetrueuntil process shutdown.Also applies to: 338-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 282 - 290, health_check() currently returns healthy based solely on bot_handle.is_some(), but bot_handle is never cleared when the session dies; update the event handling for Event::LoggedOut and Event::StreamError to clear/replace the shared bot_handle (e.g., set it to None or mark as dead) so health_check() reflects the disconnected state; modify the same logic at the other occurrence (around the Event handling near lines 338-339) to ensure both LoggedOut and StreamError paths update the same bot_handle state (reference symbols: health_check(), bot_handle, Event::LoggedOut, Event::StreamError).
214-340:⚠️ Potential issue | 🟡 MinorAdd unit coverage for the new event-routing paths.
The changed behavior here—self-echo suppression, text extraction, and group reply-target handling—is not covered by the existing tests, so the next regression will only show up against a live account.
As per coding guidelines "Ship unit tests and coverage for behavior you are adding or changing before building additional features on top" and "Rust code must be unit-tested for new/changed behavior before stacking features; run via
cargo testand extendtests/json_rpc_e2e.rsfor RPC integration tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 214 - 340, Add unit tests that exercise the WhatsApp event-routing closure logic: create tests that construct Event::Message variants and related message metadata to verify self-echo suppression (simulate info.source.is_from_me true -> no ChannelMessage sent), text extraction (cases where msg.conversation is Some, None but msg.extended_text_message.text exists, and both missing -> empty content) and allowed-list behavior (allowed_numbers matching "*" or specific +E.164 sender). Use the same ChannelMessage struct and the event handler's inputs (Event::Message, info, tx_for_handler) to drive the handler and assert whether tx_for_handler receives a message and that its fields (sender, reply_target, content, timestamp/thread_ts) match expectations; add these tests under tests/json_rpc_e2e.rs (or unit tests near whatsapp_web.rs) mocking/simulating the client info and chat types to validate group vs direct reply_target behavior and ensure the closure logic (Event::Message branch, allowed_numbers check, and normalization logic that adds '+') is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 291-301: The Event::PairingCode and Event::PairingQrCode branches
currently log the full pairing payload (variable code), which exposes active
credentials; change those branches in whatsapp_web.rs to log only a
non-sensitive notice that a pairing code/QR was received (and optionally its
type or a redacted placeholder) and remove any logging of the raw code variable;
specifically update the handlers for Event::PairingCode { code, .. } and
Event::PairingQrCode { code, .. } to avoid including the code variable in
tracing::info/debug messages and instead emit a generic message like "pairing
code/QR received" or log a redacted marker.
- Around line 248-253: The tracing::info! call currently logs full sender
identifier (normalized) and message body (text) which can leak PII; update the
log at the tracing::info! invocation to redact sender identifiers and omit
message content by logging only non-sensitive metadata (e.g., chat kind/name
from chat and message length via text.len()), or a deterministic short
hash/partial mask of normalized (not the full ID). Locate the tracing::info!
that uses normalized, chat, and text and replace the interpolation to include
chat and text length (and if needed a masked normalized like first N chars or a
hash) instead of the raw normalized/text values.
---
Outside diff comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 240-264: The code sets reply_target to the participant number
(normalized) which causes replies to be DM'd instead of to the group; change the
logic where ChannelMessage is built (look for ChannelMessage creation in this
block where sender_user/normalized and info.source.chat are used) to set
reply_target to info.source.chat.to_string() when the chat JID indicates a group
(e.g., contains "@g.us"), otherwise keep reply_target as the participant
normalized number; ensure ChannelMessage.reply_target is updated accordingly
before tx_inner.send so group replies go to the group JID.
- Around line 282-290: health_check() currently returns healthy based solely on
bot_handle.is_some(), but bot_handle is never cleared when the session dies;
update the event handling for Event::LoggedOut and Event::StreamError to
clear/replace the shared bot_handle (e.g., set it to None or mark as dead) so
health_check() reflects the disconnected state; modify the same logic at the
other occurrence (around the Event handling near lines 338-339) to ensure both
LoggedOut and StreamError paths update the same bot_handle state (reference
symbols: health_check(), bot_handle, Event::LoggedOut, Event::StreamError).
- Around line 214-340: Add unit tests that exercise the WhatsApp event-routing
closure logic: create tests that construct Event::Message variants and related
message metadata to verify self-echo suppression (simulate
info.source.is_from_me true -> no ChannelMessage sent), text extraction (cases
where msg.conversation is Some, None but msg.extended_text_message.text exists,
and both missing -> empty content) and allowed-list behavior (allowed_numbers
matching "*" or specific +E.164 sender). Use the same ChannelMessage struct and
the event handler's inputs (Event::Message, info, tx_for_handler) to drive the
handler and assert whether tx_for_handler receives a message and that its fields
(sender, reply_target, content, timestamp/thread_ts) match expectations; add
these tests under tests/json_rpc_e2e.rs (or unit tests near whatsapp_web.rs)
mocking/simulating the client info and chat types to validate group vs direct
reply_target behavior and ensure the closure logic (Event::Message branch,
allowed_numbers check, and normalization logic that adds '+') is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59a711bc-e596-4a06-bf80-305e1e72a237
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomlsrc/openhuman/channels/mod.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/whatsapp_storage.rssrc/openhuman/channels/providers/whatsapp_web.rs
💤 Files with no reviewable changes (3)
- src/openhuman/channels/providers/mod.rs
- src/openhuman/channels/mod.rs
- src/openhuman/channels/providers/whatsapp_storage.rs
senamakel
approved these changes
Apr 25, 2026
senamakel
requested changes
Apr 26, 2026
Member
senamakel
left a comment
senamakel
left a comment
There was a problem hiding this comment.
resolve coderabbit issues then good to merge
- Redact PII in inbound message log: info! now carries only chat, masked sender, and text length; raw payload moved to a trace target. - Stop logging WhatsApp pair code / QR at info — they are short-lived link credentials. Surface only a non-sensitive notice; raw values emitted under a dedicated trace target for opt-in debugging. - Use the chat JID as reply_target for group messages so replies go back to the group instead of being DM'd to the participant. - Replace health_check's stale bot_handle.is_some() probe with an AtomicBool driven by Event::Connected / LoggedOut / StreamError so a dropped session no longer reports healthy until process shutdown. - Extract compute_reply_target, redact_phone, and extract_message_text as pure helpers and cover the new event-routing branches with 10 new unit tests (17/17 pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/openhuman/channels/providers/whatsapp_web.rs (2)
353-373:⚠️ Potential issue | 🔴 CriticalRemove the raw pairing credentials from trace logs too.
These
codevalues are live linking credentials. Anyone with trace-log access can pair into the account while they are valid, so they should never be logged at any level.As per coding guidelines "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 353 - 373, The trace calls currently logging the raw variable `code` in the Event::PairingCode and Event::PairingQrCode branches expose live pairing credentials; remove or redact those values so secrets are never written to logs. Locate the branches handling Event::PairingCode and Event::PairingQrCode and either delete the `tracing::trace!` lines that print `code` or change them to log a non-sensitive placeholder (e.g., "<redacted>") or metadata-only message; ensure the `code` variable is not concatenated or formatted into any log call anywhere in those handlers.
301-307:⚠️ Potential issue | 🟠 MajorTrace logging still leaks full sender IDs and message bodies.
Moving the raw payload from
infototracestill writes full PII and message content whenever trace logging is enabled. Keep this log redacted too, or remove it entirely.Based on learnings "Never log secrets or full PII — always redact sensitive information in debug output".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 301 - 307, The trace log is emitting full PII by logging `chat`, `normalized`, and `text` in the `tracing::trace!` call; replace it with a redacted version (or remove it) by passing redacted strings instead of raw values — e.g., call a redact helper (e.g., redact(&normalized) and redact(&text) or redact_id(&chat)) and use those in the `tracing::trace!` invocation so no full sender IDs or message bodies are written, or delete the trace line entirely if trace-level payloads are unnecessary.
🧹 Nitpick comments (1)
src/openhuman/channels/providers/whatsapp_web.rs (1)
527-675: This provider is still well past the repo's Rust file-size guideline.The added docs, helpers, provider logic, and tests leave
whatsapp_web.rsat roughly 675 lines. Please split the event-handling/helpers and/or move the tests into companion modules before this grows again.As per coding guidelines "Source files should be ≤ ~500 lines; split modules when growing to improve maintainability".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 527 - 675, The file is >500 lines; split it by extracting event-handling code and helper functions into separate modules and moving unit tests into a companion test module/file: keep the WhatsAppWebChannel struct, its constructor (WhatsAppWebChannel::new) and public API (name, health_check, connected flag atomic) in the main whatsapp_web module, move event loop / message processing logic into an events module (the code referencing send/receive handlers), and move pure helpers like compute_reply_target, redact_phone, normalize_phone, is_number_allowed, extract_message_text into a helpers module; update the main module to mod events; mod helpers; pub use helpers::{compute_reply_target,redact_phone,normalize_phone,is_number_allowed,extract_message_text} and adjust imports, then relocate the #[cfg(test)] tests into a companion tests module/file that imports the channel and helpers for testing (keeping tests that reference WhatsAppWebChannel, make_channel, and connected flag). Ensure visibility (pub(crate)/pub) is adjusted so tests and events can access needed symbols and update mod declarations so the crate builds unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 117-123: compute_reply_target() correctly returns the group JID
for group chats, but downstream outbound allowlist checks are stripping the
"@g.us" suffix and comparing against allowed_numbers, causing allowed group
replies to be dropped; update the outbound allowlist validation (the function
that checks allowed_numbers for outbound recipients) to special-case JIDs that
end_with("@g.us") by either: 1) skipping the suffix-stripping and validating
against the original sender ID (e.g., keep "+<group-id>@g.us") or 2) explicitly
recognizing and allowing group JIDs when the original message sender was an
allowed number; adjust the outbound check to consult the original
sender_normalized when computing allowlist decisions and add a regression test
that simulates an allowed user replying from a group (ensure
compute_reply_target returns the `@g.us` JID and the outbound allowlist permits
sending back into that group).
- Around line 399-405: The listen() code currently blocks on
tokio::signal::ctrl_c() and force-aborts the bot handle, which bypasses the
app-wide graceful shutdown; replace that ctrl_c wait with the repository’s
coordinated shutdown subscription (use the shared shutdown receiver/subscriber
provided by the app shutdown machinery) and await that instead, and instead of
handle.abort() signal the bot to shut down via the shared shutdown channel (or
call the bot’s graceful shutdown API if one exists) and await its join/cleanup
so the WhatsAppWeb shutdown uses the centralized shutdown flow; update the lines
referencing tokio::signal::ctrl_c(), self.connected.store(...),
*self.client.lock() = None, and the bot_handle .take()/abort() sequence to use
the shared shutdown mechanism and graceful stop of the bot.
---
Duplicate comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 353-373: The trace calls currently logging the raw variable `code`
in the Event::PairingCode and Event::PairingQrCode branches expose live pairing
credentials; remove or redact those values so secrets are never written to logs.
Locate the branches handling Event::PairingCode and Event::PairingQrCode and
either delete the `tracing::trace!` lines that print `code` or change them to
log a non-sensitive placeholder (e.g., "<redacted>") or metadata-only message;
ensure the `code` variable is not concatenated or formatted into any log call
anywhere in those handlers.
- Around line 301-307: The trace log is emitting full PII by logging `chat`,
`normalized`, and `text` in the `tracing::trace!` call; replace it with a
redacted version (or remove it) by passing redacted strings instead of raw
values — e.g., call a redact helper (e.g., redact(&normalized) and redact(&text)
or redact_id(&chat)) and use those in the `tracing::trace!` invocation so no
full sender IDs or message bodies are written, or delete the trace line entirely
if trace-level payloads are unnecessary.
---
Nitpick comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 527-675: The file is >500 lines; split it by extracting
event-handling code and helper functions into separate modules and moving unit
tests into a companion test module/file: keep the WhatsAppWebChannel struct, its
constructor (WhatsAppWebChannel::new) and public API (name, health_check,
connected flag atomic) in the main whatsapp_web module, move event loop /
message processing logic into an events module (the code referencing
send/receive handlers), and move pure helpers like compute_reply_target,
redact_phone, normalize_phone, is_number_allowed, extract_message_text into a
helpers module; update the main module to mod events; mod helpers; pub use
helpers::{compute_reply_target,redact_phone,normalize_phone,is_number_allowed,extract_message_text}
and adjust imports, then relocate the #[cfg(test)] tests into a companion tests
module/file that imports the channel and helpers for testing (keeping tests that
reference WhatsAppWebChannel, make_channel, and connected flag). Ensure
visibility (pub(crate)/pub) is adjusted so tests and events can access needed
symbols and update mod declarations so the crate builds unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77064fa2-4737-4d01-8bf5-1b215e55098b
📒 Files selected for processing (1)
src/openhuman/channels/providers/whatsapp_web.rs
…tdown Round-2 of CodeRabbit review on PR tinyhumansai#916. - Outbound `@g.us` recipients now bypass the per-number allowlist via new `should_allow_outbound` + `is_group_jid` helpers. Without this the previous fix (group reply_target = group JID) would have been silently dropped by `send`/`start_typing`/`stop_typing` on any non-wildcard `allowed_numbers` config: the JID got normalised to `+<group-id>` and never matched. Group membership is governed by WhatsApp itself; the inbound side already gated on the participant. - `listen` no longer blocks on `tokio::signal::ctrl_c()`. It now registers a hook with `core::shutdown` (which honours both SIGINT and SIGTERM and runs the rest of the process's coordinated cleanup) and waits on a local `Notify` that the hook fires after aborting the bot and clearing live state. - 7 new unit tests cover `is_group_jid`, the group-bypass regression, and the existing DM/wildcard/empty-list outbound paths (24/24 pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/channels/providers/whatsapp_web.rs (1)
254-257: Consider adding explicit error context for schema migration failures.When
SqliteStore::new()fails due to an incompatible schema from the oldwa-rs0.2 fork, the error may not clearly indicate that the user should delete the oldwhatsapp-session.dbfile. Adding context would improve the user experience during migration.♻️ Suggested improvement
- let backend = Arc::new(SqliteStore::new(&self.session_path).await?); + let backend = Arc::new( + SqliteStore::new(&self.session_path) + .await + .map_err(|e| anyhow::anyhow!( + "Failed to open WhatsApp session store at {}: {}. \ + If upgrading from wa-rs 0.2, delete the old database and re-pair.", + self.session_path, + e + ))?, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 254 - 257, The call to SqliteStore::new(&self.session_path).await currently returns raw errors that don't explain an incompatible-schema case; wrap or map the error where SqliteStore::new is invoked (the backend initialization that constructs Arc::new(SqliteStore::new(&self.session_path).await?)) to add explicit context suggesting the schema may be from the old wa-rs 0.2 fork and advising the user to delete/replace the whatsapp-session.db file (include the path from self.session_path) when migration fails; ensure the new error preserves the original error details while appending this actionable message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 254-257: The call to SqliteStore::new(&self.session_path).await
currently returns raw errors that don't explain an incompatible-schema case;
wrap or map the error where SqliteStore::new is invoked (the backend
initialization that constructs
Arc::new(SqliteStore::new(&self.session_path).await?)) to add explicit context
suggesting the schema may be from the old wa-rs 0.2 fork and advising the user
to delete/replace the whatsapp-session.db file (include the path from
self.session_path) when migration fails; ensure the new error preserves the
original error details while appending this actionable message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82526cec-6f9c-4e7f-9086-3c65f1591196
📒 Files selected for processing (1)
src/openhuman/channels/providers/whatsapp_web.rs
CodeRabbit nitpick on PR tinyhumansai#916: when the on-disk SQLite store was written by the wa-rs 0.2 fork, `SqliteStore::new` fails with a raw error that doesn't tell the user what to do. Wrap the call with `anyhow!` context that names the path and points at the module-level migration note (delete the old DB and re-pair). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/channels/providers/whatsapp_web.rs (1)
215-219:⚠️ Potential issue | 🟠 MajorRedact recipients in the allowlist-denied warnings.
These branches still emit the full recipient/JID into routine logs when a send or typing request is blocked, which leaks phone numbers on ordinary warning paths. Log a masked identifier or coarse metadata instead. As per coding guidelines "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
Also applies to: 472-476, 497-501
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 215 - 219, The warn logs leak full recipient JIDs (message.recipient) when outbound is blocked; replace direct usage with a redacted identifier by adding a small helper (e.g., redact_recipient or mask_recipient) that returns a coarse token (like "<redacted-xxxx>" using last 2–4 chars or a stable hash) and use that helper wherever should_allow_outbound results in a warning (references: should_allow_outbound and the branches around the send/typing blocks noted). Update the tracing::warn calls at those branches to log the redacted string instead of the raw recipient and ensure the helper is deterministic and does not retain sensitive data.
♻️ Duplicate comments (2)
src/openhuman/channels/providers/whatsapp_web.rs (2)
384-404:⚠️ Potential issue | 🔴 CriticalRemove the raw pair code / QR payload trace logs.
codehere is active linking material. Writing it anywhere in logs makes session hijack possible for anyone with trace access while it is valid. Keep only the non-sensitive “pairing material received” notice and never persist the payload itself. As per coding guidelines "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".Suggested change
- Event::PairingCode { code, .. } => { + Event::PairingCode { .. } => { tracing::info!( "🔑 WhatsApp Web pair code received. Enter the code shown on \ your linking surface into WhatsApp > Linked Devices." ); - tracing::trace!( - target: "openhuman::whatsapp::pairing", - "pair code: {}", - code - ); } - Event::PairingQrCode { code, .. } => { + Event::PairingQrCode { .. } => { tracing::info!( "📱 WhatsApp Web QR code received. Render via QR generator and \ scan with WhatsApp > Linked Devices." ); - tracing::trace!( - target: "openhuman::whatsapp::pairing", - "qr code: {}", - code - ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 384 - 404, Remove any logging that prints the raw linking payload from the Event::PairingCode and Event::PairingQrCode match arms (the variable named code). Delete the tracing::trace! calls that interpolate code, leaving only the non-sensitive tracing::info! messages that say pairing material was received; if you must keep a debug hook, replace the payload with a constant redacted string (e.g., "<redacted>") but do not persist or log the actual code value anywhere in the Event::PairingCode / Event::PairingQrCode handling.
332-338:⚠️ Potential issue | 🟠 MajorTrace logging still leaks full sender IDs and message bodies.
Moving this to
tracedoes not satisfy the repo rule:normalizedandtextare still full PII/user content and will end up in any trace-enabled deployment. Keep trace logs to redacted identifiers, message kind, sizes, or a short hash only. As per coding guidelines "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 332 - 338, The trace log currently emits full PII via the variables chat, normalized, and text in the tracing::trace! call (target: "openhuman::whatsapp::inbound"); change it so only non-PII or redacted values are logged: replace `normalized` with a redacted identifier or short hash (e.g., sha256 or HMAC) and replace `text` with either the message size/length or a short content hash/summary, and keep `chat` redacted as needed; locate the tracing::trace! invocation and compute/redact these safe values before logging and update the log message accordingly.
🧹 Nitpick comments (1)
src/openhuman/channels/providers/whatsapp_web.rs (1)
576-779: Split this module before it grows further.This file is now ~780 lines, well past the repo’s ~500-line target. Moving the pure helper logic and/or the test module into sibling modules would make the provider much easier to review and maintain. As per coding guidelines "src/**/*.rs: Source files should be ≤ ~500 lines; split modules when growing to improve maintainability".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 576 - 779, Current module is too large; split helper logic and tests into separate sibling modules. Move pure helper functions and related statics/methods (normalize_phone, is_number_allowed, compute_reply_target, redact_phone, extract_message_text, is_group_jid, should_allow_outbound and any small utils they depend on) into a new module (e.g., whatsapp_web_helpers) and re-export them into WhatsAppWebChannel; keep the channel struct and async/runtime integration code in the original provider module. Move the #[cfg(test)] mod tests block into a separate test module/file (or a tests-only module) that imports the channel and helpers and retains the same test names (whatsapp_web_channel_name, whatsapp_web_number_allowed_*, whatsapp_web_normalize_phone_*, whatsapp_web_health_check_*, whatsapp_web_compute_reply_target_*, whatsapp_web_redact_phone_*, whatsapp_web_extract_message_text_*, whatsapp_web_is_group_jid_*, whatsapp_web_should_allow_outbound_*), adjusting visibility (pub(crate) or pub) as needed so tests compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 120-130: The current should_allow_outbound(&self, recipient: &str)
unconditionally returns true for any group JID, creating an allowlist bypass;
change it so group JIDs are only allowed when there is explicit provenance that
the bot previously received an allowed inbound message from that same group
(e.g., consult the inbound message store/metadata used elsewhere to verify the
group id), otherwise fall through to normal phone normalization and
is_number_allowed checks; update callers (send, start_typing, stop_typing) to
rely on the tightened should_allow_outbound and add a regression test asserting
that an unrelated group JID (one with no prior allowed inbound message
provenance) is blocked while a group with recorded allowed inbound provenance is
permitted.
---
Outside diff comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 215-219: The warn logs leak full recipient JIDs
(message.recipient) when outbound is blocked; replace direct usage with a
redacted identifier by adding a small helper (e.g., redact_recipient or
mask_recipient) that returns a coarse token (like "<redacted-xxxx>" using last
2–4 chars or a stable hash) and use that helper wherever should_allow_outbound
results in a warning (references: should_allow_outbound and the branches around
the send/typing blocks noted). Update the tracing::warn calls at those branches
to log the redacted string instead of the raw recipient and ensure the helper is
deterministic and does not retain sensitive data.
---
Duplicate comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 384-404: Remove any logging that prints the raw linking payload
from the Event::PairingCode and Event::PairingQrCode match arms (the variable
named code). Delete the tracing::trace! calls that interpolate code, leaving
only the non-sensitive tracing::info! messages that say pairing material was
received; if you must keep a debug hook, replace the payload with a constant
redacted string (e.g., "<redacted>") but do not persist or log the actual code
value anywhere in the Event::PairingCode / Event::PairingQrCode handling.
- Around line 332-338: The trace log currently emits full PII via the variables
chat, normalized, and text in the tracing::trace! call (target:
"openhuman::whatsapp::inbound"); change it so only non-PII or redacted values
are logged: replace `normalized` with a redacted identifier or short hash (e.g.,
sha256 or HMAC) and replace `text` with either the message size/length or a
short content hash/summary, and keep `chat` redacted as needed; locate the
tracing::trace! invocation and compute/redact these safe values before logging
and update the log message accordingly.
---
Nitpick comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 576-779: Current module is too large; split helper logic and tests
into separate sibling modules. Move pure helper functions and related
statics/methods (normalize_phone, is_number_allowed, compute_reply_target,
redact_phone, extract_message_text, is_group_jid, should_allow_outbound and any
small utils they depend on) into a new module (e.g., whatsapp_web_helpers) and
re-export them into WhatsAppWebChannel; keep the channel struct and
async/runtime integration code in the original provider module. Move the
#[cfg(test)] mod tests block into a separate test module/file (or a tests-only
module) that imports the channel and helpers and retains the same test names
(whatsapp_web_channel_name, whatsapp_web_number_allowed_*,
whatsapp_web_normalize_phone_*, whatsapp_web_health_check_*,
whatsapp_web_compute_reply_target_*, whatsapp_web_redact_phone_*,
whatsapp_web_extract_message_text_*, whatsapp_web_is_group_jid_*,
whatsapp_web_should_allow_outbound_*), adjusting visibility (pub(crate) or pub)
as needed so tests compile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99d330e3-3ff5-4580-a80a-98ea52cc1146
📒 Files selected for processing (1)
src/openhuman/channels/providers/whatsapp_web.rs
Round-3 of CodeRabbit review on PR tinyhumansai#916. - `should_allow_outbound` no longer blanket-allows every `@g.us` recipient. The previous fix (round-2) widened the gate so any caller able to set `recipient` could post into arbitrary joined groups via the @g.us suffix. Tightened to require explicit provenance: a new `allowed_groups: Arc<Mutex<HashSet<String>>>` is populated when an allow-listed participant messages in, and the outbound gate consults that set for group recipients. Groups without provenance are blocked. - Outbound warn paths (`send`/`start_typing`/`stop_typing`) no longer emit raw recipient JIDs. New `redact_recipient` helper masks both bare phone numbers and `<digits>@server` JIDs uniformly. - Removed `tracing::trace!` for `Event::PairingCode` and `Event::PairingQrCode` raw payloads. Pair codes and QR strings are active link credentials and must not be logged at any level. - Removed the `target: "openhuman::whatsapp::inbound"` trace log that carried full sender + raw text. Even at trace level it leaked PII in any trace-enabled deployment. - 5 new unit tests cover provenanced/unrelated/no-provenance group paths and `redact_recipient` JID/group/bare-phone shapes (29/29 pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/channels/providers/whatsapp_web.rs (2)
251-255:⚠️ Potential issue | 🟡 MinorRedact recipient in debug log.
The warning log at line 238-240 correctly uses
Self::redact_recipient, but this debug log exposes the fullmessage.recipient. Phone numbers are PII and should be redacted at all log levels.🛡️ Proposed fix
tracing::debug!( "WhatsApp Web: sent message to {} (id: {})", - message.recipient, + Self::redact_recipient(&message.recipient), message_id );Based on learnings: "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 251 - 255, The debug log is emitting full PII by logging message.recipient; change the tracing::debug call that currently prints message.recipient (the one that logs "WhatsApp Web: sent message to {} (id: {})") to pass the redacted value via Self::redact_recipient(&message.recipient) instead, keeping message_id unchanged so the log still shows the id but not the raw phone number.
503-503:⚠️ Potential issue | 🟡 MinorRedact recipient in typing debug logs.
Lines 503 and 528 log the full
recipientat debug level. For consistency with the warning logs and PII protection, these should also use redaction.🛡️ Proposed fix
- tracing::debug!("WhatsApp Web: start typing for {}", recipient); + tracing::debug!("WhatsApp Web: start typing for {}", Self::redact_recipient(recipient));And similarly at line 528:
- tracing::debug!("WhatsApp Web: stop typing for {}", recipient); + tracing::debug!("WhatsApp Web: stop typing for {}", Self::redact_recipient(recipient));Based on learnings: "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` at line 503, Debug logs currently include the full recipient value (tracing::debug! with recipient) which exposes PII; update both debug statements that reference recipient (the tracing::debug! call that logs "WhatsApp Web: start typing for {}", and the other tracing::debug! call later that logs recipient) to redact or mask the recipient before logging (use the same redaction approach used by the warning-level logs—e.g., the project's redact helper or a masked/placeholder representation) so raw recipient data is never emitted.
🧹 Nitpick comments (1)
src/openhuman/channels/providers/whatsapp_web.rs (1)
592-848: Consider extracting tests to a separate file.The file is 849 lines total. While the main implementation (~480 lines) is within the ~500 line guideline, extracting the ~250 lines of tests to
whatsapp_web_tests.rsor atestssubmodule would improve maintainability. This is optional given the tests are well-organized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/whatsapp_web.rs` around lines 592 - 848, The tests module (mod tests) and its helper make_channel plus many test functions referencing WhatsAppWebChannel should be moved out of the large implementation file into a dedicated test file (e.g., whatsapp_web_tests.rs or a tests/whatsapp_web.rs) to improve maintainability; create the new test file as a submodule (or place under crate-level tests) and copy the entire #[cfg(test)] mod tests block there, ensure you import the parent module items (use super::* or crate::openhuman::channels::providers::whatsapp_web::WhatsAppWebChannel as needed), keep the #[cfg(feature = "whatsapp-web")] attributes on tests and helper, and update module declarations (mod whatsapp_web_tests or #[path = "..."]) so the test suite compiles unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 348-353: The log currently prints the raw chat JID (variable chat)
in tracing::info which can expose phone numbers for DMs; update the logging to
redact DM chat JIDs by reusing or adding a helper (e.g., call Self::redact_phone
or implement Self::redact_chat_jid) that detects JIDs like
"12345@s.whatsapp.net" and replaces the local part with a redacted form before
passing into tracing::info; ensure group JIDs remain unchanged and use the
redacted value in the existing tracing::info invocation so no raw PII is logged.
---
Outside diff comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 251-255: The debug log is emitting full PII by logging
message.recipient; change the tracing::debug call that currently prints
message.recipient (the one that logs "WhatsApp Web: sent message to {} (id:
{})") to pass the redacted value via Self::redact_recipient(&message.recipient)
instead, keeping message_id unchanged so the log still shows the id but not the
raw phone number.
- Line 503: Debug logs currently include the full recipient value
(tracing::debug! with recipient) which exposes PII; update both debug statements
that reference recipient (the tracing::debug! call that logs "WhatsApp Web:
start typing for {}", and the other tracing::debug! call later that logs
recipient) to redact or mask the recipient before logging (use the same
redaction approach used by the warning-level logs—e.g., the project's redact
helper or a masked/placeholder representation) so raw recipient data is never
emitted.
---
Nitpick comments:
In `@src/openhuman/channels/providers/whatsapp_web.rs`:
- Around line 592-848: The tests module (mod tests) and its helper make_channel
plus many test functions referencing WhatsAppWebChannel should be moved out of
the large implementation file into a dedicated test file (e.g.,
whatsapp_web_tests.rs or a tests/whatsapp_web.rs) to improve maintainability;
create the new test file as a submodule (or place under crate-level tests) and
copy the entire #[cfg(test)] mod tests block there, ensure you import the parent
module items (use super::* or
crate::openhuman::channels::providers::whatsapp_web::WhatsAppWebChannel as
needed), keep the #[cfg(feature = "whatsapp-web")] attributes on tests and
helper, and update module declarations (mod whatsapp_web_tests or #[path =
"..."]) so the test suite compiles unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1076e0a7-d656-44db-bb6b-725ba8a1d6c3
📒 Files selected for processing (1)
src/openhuman/channels/providers/whatsapp_web.rs
CodeRabbit minor on PR tinyhumansai#916: for DM chats the `chat` variable is `<phone>@s.whatsapp.net` and was being logged raw at info level, re-leaking the participant's phone number even though the sender had been redacted. Reuse `redact_recipient` which masks the user part of any `<digits>@server` JID and leaves the server suffix intact, so DM vs group is still distinguishable in routine logs without carrying a full identifier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…grade-rust-0-5 # Conflicts: # src/openhuman/channels/providers/whatsapp_storage.rs # src/openhuman/channels/providers/whatsapp_web.rs
Contributor
Author
|
@senamakel resolved all CodeRabbit findings across 4 rounds + main merge — ready for re-review when you have a moment. Summary of changes since your "resolve coderabbit issues then good to merge" comment:
29/29 unit tests pass. CI is green on the merge commit (Rust Core Tests + Quality, Rust Quality, Tauri Shell, Frontend, TypeScript, Smoke installs all pass; Build Tauri App still finishing). |
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
… 0.5 (tinyhumansai#916) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The previous
wa-rs0.2 fork silently droppedEvent::Messagefor LID-addressed contacts and group sender-key (skmsg) messages. Protocol-layer decryption succeeded but the events never reached user code, breaking agent dispatch for the bulk of modern WhatsApp traffic (LID is now the default).This PR replaces the fork with upstream
whatsapp-rust0.5, which fixed the dispatch via PRs #170 (SKDM tracking) + #181 (LID/PN mapping) + sender-key handling. As a bonus, upstream ships its ownSqliteStore, so the previous custom 1,345-lineRusqliteStoreis no longer needed.Diff size
Net −1,166 lines.
What changes
Cargo.tomlwa-rs*deps; addwhatsapp-rust 0.5(withsqlite-storage+tokio-runtimefeatures) +whatsapp-rust-tokio-transport+whatsapp-rust-ureq-http-client+wacore.src/openhuman/channels/providers/whatsapp_web.rsChanneltrait surface unchanged (send/listen/health_check/start_typing/stop_typing), soruntime/startup.rsand downstream consumers are unaffected. New: filter self-echoes (info.source.is_from_me) so the agent doesn't react to messages the user typed on another linked device.src/openhuman/channels/providers/whatsapp_storage.rsSqliteStorereplaces all four storage trait impls.src/openhuman/channels/providers/mod.rswhatsapp_storagedeclaration.src/openhuman/channels/mod.rspub use providers::whatsapp_storage.Verification
Tested locally on a real WhatsApp account:
Event::Messagefor direct (@s.whatsapp.net) messagesEvent::Messagefor LID 1:1 (@lid) messagesEvent::Messagefor group messages withskmsgencryptioncargo check --features whatsapp-webcargo fmt -- --checkIn a 30-minute live session, 70
Event::Messageevents fired against the new lib (59 group/LID + 11 direct), versus 0 on the old fork for the same traffic.Breaking change
Existing paired sessions will fail to load on first launch because the SQLite schemas differ between the old fork's
RusqliteStoreand upstream'sSqliteStore. Users will be prompted for a fresh QR scan (~30 seconds). The oldwhatsapp-session.dbcan be deleted afterwards.This affects only Web-mode users; Cloud API mode (webhook-based) is untouched.
Test plan
cargo check --features whatsapp-web,channel-matrix --bin openhuman-corecargo test --features whatsapp-web --lib openhuman::channels::providers::whatsapp_web— 7/7 passcargo fmt -- --check📨 WhatsApp message from ...log +ChannelMessageflows toChannelInboundSubscriberRelated
Event::JoinedGroup→ history-sync ingest andEvent::Message→ real-time ingest intomemory::tree::ingest::ingest_chat.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor
Behavior
Tests