feat(messages): encrypted CAG reactions and channel comments, both directions - #830
Merged
Conversation
…in both directions Community Announcement Groups reject plaintext reactions: WA Web encrypts them with the target's messageSecret (WAWebReactionEncryptMsgData) and ships an enc_reaction_message envelope. Channel comments use the sibling enc_comment_message envelope. We classified both for send but never produced or decrypted either, so reactions in CAG channels silently broke in both directions and incoming comments surfaced as unreadable envelopes. Incoming: extract_secret_encrypted now recognises the two top-level envelopes alongside secret_encrypted_message, reusing the whole existing secret-lookup and LID/PN fallback machinery. A reaction decrypts to its inner ReactionMessage (WA Web encodes only text + senderTimestampMs) and is surfaced in the plaintext-reaction shape with the key filled from the envelope, so consumers see no difference from a normal reaction. A comment decrypts to its inner body Message; the parent post key surfaces on the new MessageInfo::comment_target, and the comment's own secret is persisted under the comment's id and sender, never the parent's. Outgoing: send_reaction now gates on WA Web's isCag (default_sub_group; the flag is carried on the cached group metadata, with a one-time full metadata query for blobs persisted before the field existed) and emits the encrypted envelope, failing loudly when the parent secret was never captured instead of sending a plaintext reaction the channel drops. A new Comments feature handle authors comments (extended-text body per WAWebSendCommentMessageActionUtils, LID identity, fresh own secret so the comment can itself receive add-ons). The crypto lives in wacore::reaction / wacore::comment as thin wrappers over secret_enc_addon, mirroring the existing event/poll modules.
|
Warning Review limit reached
More reviews will be available in 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds encrypted comment and reaction addon support for Community Announcement Groups: crypto helpers, message extraction/decryption for enc_reaction/enc_comment, inbound persistence/dispatch updates (comment threading), outbound Comments API, and CAG-aware encrypted reaction sending. ChangesEncrypted Comments and Reactions for Community Announcement Groups
Sequence Diagram(s)sequenceDiagram
participant User
participant CommentsAPI
participant MsgSecretResolver
participant CommentCrypto
participant ClientSend
User->>CommentsAPI: send_message(chat, parent_key, body)
CommentsAPI->>MsgSecretResolver: resolve_outgoing_addon_parent(chat, parent_key)
MsgSecretResolver-->>CommentsAPI: parent_jid + parent_secret
CommentsAPI->>CommentCrypto: encrypt_comment_with_secret(inner, parent_secret, parent_id, parent_jid, commenter_jid)
CommentCrypto-->>CommentsAPI: enc_payload + iv
CommentsAPI->>ClientSend: send_message(enc_comment_message with message_secret)
ClientSend->>ClientSend: persist_outbound_msg_secret(comment_id, comment_secret)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Benchmark Results67 unchanged benchmark(s)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d250ed4c7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/message/tests.rs`:
- Around line 9705-9733: The test uses a fixed tokio::time::sleep + try_recv
loop which can race; replace that pattern with the deterministic helper
collect_event(...) to poll rx with a bounded timeout after calling
client.dispatch_parsed_message(msg, &info).await; use collect_event(rx, timeout)
(or the project's equivalent) to retrieve the Event::Message and then assert the
same conditions on the received msg/info (matching COMMENT_ID, checking
extended_text_message text == "great post", enc_comment_message is None, and
info.comment_target id == PARENT_ID). Keep the same assertions but remove the
sleep and manual try_recv loop so the test is deterministic on slow CI.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b6c019d-2d60-487c-8c81-1a7138494499
📒 Files selected for processing (15)
src/features/comments.rssrc/features/groups.rssrc/features/message_edit.rssrc/features/mod.rssrc/features/reaction.rssrc/lib.rssrc/message/dispatch.rssrc/message/msg_secret.rssrc/message/tests.rssrc/pdo.rswacore/src/client/context.rswacore/src/comment.rswacore/src/lib.rswacore/src/reaction.rswacore/src/types/message.rs
…tched body The comment's own messageSecret rides the OUTER envelope (WA Web puts it on the comment msgData), which substitution dropped, so app-managed secret storage (Disabled policy) could never learn it for add-ons targeting the comment. Copy it onto the dispatched body when the inner carries none. The pipeline test now ships the secret in the WA Web shape (outer envelope) and asserts it survives substitution, and polls with a deadline instead of a fixed sleep.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d71e1d3c0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/message/msg_secret.rs`:
- Around line 344-349: The current block only sets inner.message_context_info
when it is None, but if inner.message_context_info exists without a
message_secret we must merge the outer envelope's comment secret into it; update
the logic inside the env.kind == SecretEncKind::EncComment guard to: if
msg.message_context_info.as_ref() yields outer_mci with message_secret, then if
inner.message_context_info is None set inner.message_context_info =
Some(outer_mci.clone()), else if
inner.message_context_info.as_mut().message_secret.is_none() copy
outer_mci.message_secret.clone() into
inner.message_context_info.as_mut().message_secret so the comment secret is
preserved for re-persist. Ensure you reference env.kind,
SecretEncKind::EncComment, inner.message_context_info, msg.message_context_info,
outer_mci and message_secret when making the change.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c2adcb89-d68c-426e-9186-9328a02610a4
📒 Files selected for processing (2)
src/message/msg_secret.rssrc/message/tests.rs
…for CAG addons Three review findings on the addon paths. The outer comment secret now merges into an existing secret-less inner context instead of only filling a missing one. A from_me target key in a group resolves our identity from the group's addressing mode (outbound group secrets are persisted under the group sender identity, and WA Web authors CAG addons under LID), not from the chat server. And an outbound comment persists its own minted secret under the commenter identity and sent id, since the send path only persists reporting-token secrets; without it, add-ons targeting our own comment could never be decrypted.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/comments.rs (1)
91-96: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRefactor to generate secret directly as
[u8; 32]to avoid.expect().Per our coding guidelines,
.expect()shouldn't be used outside tests. Yes, the Vec is provably 32 bytes, but why create a Vec just to convert it? Generate the array directly — cleaner and zero-cost.- let comment_secret: Vec<u8> = { - use rand::Rng; - let mut secret = vec![0u8; 32]; - rand::make_rng::<rand::rngs::StdRng>().fill_bytes(&mut secret); - secret - }; + let comment_secret: [u8; 32] = { + use rand::Rng; + let mut secret = [0u8; 32]; + rand::make_rng::<rand::rngs::StdRng>().fill_bytes(&mut secret); + secret + };Then line 105 becomes
comment_secret.to_vec()and lines 115-118 simplify to just&comment_secret.Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/comments.rs` around lines 91 - 96, Replace the Vec-based secret with a fixed-size array by creating comment_secret as a [u8; 32] and filling it from the RNG (e.g., use rand::rngs::StdRng and RngCore::fill_bytes on a &mut [u8;32]) instead of building a Vec and calling .try_into(). Update subsequent uses: where you previously converted the Vec, call comment_secret.to_vec(), and where you passed a slice or reference, pass &comment_secret (or &comment_secret[..]) so no .expect() or unnecessary allocation/conversion is required; adjust any function signatures or borrows that expected Vec<u8> accordingly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/features/comments.rs`:
- Around line 91-96: Replace the Vec-based secret with a fixed-size array by
creating comment_secret as a [u8; 32] and filling it from the RNG (e.g., use
rand::rngs::StdRng and RngCore::fill_bytes on a &mut [u8;32]) instead of
building a Vec and calling .try_into(). Update subsequent uses: where you
previously converted the Vec, call comment_secret.to_vec(), and where you passed
a slice or reference, pass &comment_secret (or &comment_secret[..]) so no
.expect() or unnecessary allocation/conversion is required; adjust any function
signatures or borrows that expected Vec<u8> accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a991bae-2ff2-4b5e-a55d-7a574dd43c83
📒 Files selected for processing (3)
src/features/comments.rssrc/message/msg_secret.rssrc/message/tests.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 885cf949a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lback helper The public decrypt_secret_encrypted_with_fallback only attempted the combined fallback pair for the reaction/comment kinds, while a migration case can need the alternate identity on only one side of the HKDF. It now tries the mixed combinations too, deduplicated and short-circuiting on success, matching what the receive path already does. Test covers the fallback-modifier-only combination.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e90a1494e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Receivers derive the addon key with the stanza sender, which in a CAG is our LID regardless of the parent author's namespace, so keying the HKDF off the author (a PN participant in a legacy or cross-addressed key) produced reactions other clients could not open. Mirror the comment path: own LID, PN only when no LID is known.
3 tasks
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
1 participant
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.
Problem
Community Announcement Groups never accept plaintext reactions: WA Web (
WAWebReactionEncryptMsgData) fetches the group metadata and, whenisCag, encrypts the reaction with the target message'smessageSecretand emits anenc_reaction_message { targetMessageKey, encPayload, encIv }envelope. Channel comments ship the siblingenc_comment_messageenvelope (WAWebSendCommentMessageAction). whatsapp-rust classified both fields for stanza typing but neither produced nor decrypted them: reactions in CAG channels silently broke in both directions, and incoming comments were dispatched as raw undecryptable envelopes. The crypto primitive (ModificationType::EncReaction/EncCommentinsecret_enc_addon) existed unused.Change
Incoming (both kinds).
extract_secret_encryptednow recognises the two top-level envelopes alongsidesecret_encrypted_message, so they flow through the entire existing machinery: store secret lookup with the LID/PN alternate, app resolver fallback, and the 4-combination identity fallback decrypt.ReactionMessage(WA Web encodes onlytext+senderTimestampMs, confirmed inWAWebAddonEncryption's spec table:Message$ReactionMessageSpec+ENC_REACTION) and is surfaced in the plaintext-reaction shape withkeyfilled from the envelope's target. Consumers see exactly what a plaintext reaction looks like, zero migration.Message(MessageSpec+ENC_COMMENT) and is dispatched as that body. The E2E proto has no slot for the threading link, so the parent post key surfaces on the newMessageInfo::comment_target. The comment's ownmessageSecretis persisted under the comment's id and sender (it keys future add-ons on the comment itself), never under the parent's id.Outgoing reactions.
send_reactionnow gates on WA Web'sisCag(derived fromdefault_sub_group, perWAWebGroupMetadataModel). The flag rides on the cached/persisted group metadata (GroupInfo.is_community_announce,Option<bool>so blobs persisted before the field answer "unknown" and trigger one full metadata query instead of guessing). When the chat is a CAG the reaction is encrypted and shipped as the envelope; if the parent secret was never captured the send fails with a descriptive error rather than emitting a plaintext reaction the channel drops.Outgoing comments. New
client.comments()feature handle:send_text(extended-text body, perWAWebSendCommentMessageActionUtils.encryptExtendedTextComment) andsend_messagefor arbitrary bodies. Comments are authored under the LID identity (getMeLidUserOrThrowparity) and carry a fresh ownmessageSecretso they can themselves receive encrypted add-ons.The crypto lives in
wacore::reaction/wacore::commentas thin wrappers oversecret_enc_addon, mirroring the existingevent/pollmodules. The shared outgoing resolution (parent author from the target key, secret with alternates and resolver) is one helper reused by both send paths, and the envelope key is always stamped with the same identity the HKDF was derived with, so receivers resolve exactly what we keyed.Tests
wacore::reaction/wacore::comment: roundtrips, empty-text removal form, wrong-identity refusal, use-case separation (a comment payload must not decrypt under the reaction use-case), secret-size validation.features::message_edit: envelope recognition for both kinds, malformed-envelope rejection (bad IV, missing key), per-kind dispatch through the public fallback API (primary PN fails, LID fallback decrypts).maybe_decrypt_secret_encrypted_messagewith the key filled from the envelope; an encrypted comment goes throughdispatch_parsed_messageend to end, asserting the dispatchedEvent::Messagecarries the body,comment_targeton the info, the envelope stripped, and the comment's secret persisted under the comment's own id while the parent's row stays untouched.cargo clippy --all-targets -- -D warningsclean.Breaking
MessageInfogains thecomment_target: Option<MessageKey>field (struct literal constructors need the field or..Default::default());GroupInfogainsis_community_announce: Option<bool>(serde-compatible with old persisted blobs);SecretEncKindgains theEncReaction/EncCommentvariants. Pre-1.0 additive surface.