feat(newsletter): plaintext channel edit/revoke + reject newsletter on the E2E send path - #725
Merged
Merged
Conversation
Newsletters are plaintext channels: their only valid send is the <plaintext> branch in send_message_with_options, which returns before send_message_impl. But pin_message/unpin_message (via send_pin), edit_message, and revoke_message call send_message_impl directly with no newsletter guard, so a newsletter JID fell into the DM/E2E branch and tried to build encrypted device fanout (Signal prekeys) against a channel that never uses Signal, producing an invalid stanza or a prekey-fetch error. Add a single is_newsletter() guard at the top of send_message_impl so every E2E-direct producer fails fast with a clear error instead of mis-routing. WA Web does support newsletter edit/revoke as plaintext operations; emitting those plaintext stanzas is left as a follow-up. Pin-in-chat is not a channel operation, so rejecting it is correct. Resolves the mis-routing for pin/unpin, edit, and revoke at the root.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughNewsletter plaintext edit/revoke node builder and Newsletter API methods were added, ChangesNewsletter edit/revoke and send-path guard
Sequence Diagram(s)sequenceDiagram
participant NewsletterAPI as Newsletter::edit_message/revoke_message
participant Builder as build_newsletter_edit_node
participant Client as Client::send_node
participant Server as XMPP/WhatsAppServer
NewsletterAPI->>Builder: build_newsletter_edit_node(to, message_id, Edit/Revoke)
Builder->>Client: return <message> Node (to,id,type,edit,plaintext)
Client->>Server: send_node(<message> Node)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels: api-design 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Benchmark Results67 unchanged benchmark(s)
|
jlucaso1
changed the title
![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: f5cdddc07b
ℹ️ 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".
jlucaso1
force-pushed
the
fix/newsletter-jid-reject-e2e-send
branch
from
June 5, 2026 04:58
f5cdddc to
ce464fe
Compare
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/features/newsletter.rs (1)
618-622:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse
editbefore falling back totype.This is going to misclassify the wire shape we just added.
build_newsletter_edit_node()emits edits astype="text|media" edit="3"and revokes astype="text" edit="8", but this parser only readstype, so history fetches will come back asText/Mediainstead ofEdit/Revoke. That makesNewsletterMessageType::Edit/Revokeunreachable for messages produced by the new API.Suggested fix
- let message_type = msg_node - .get_attr("type") - .map(|v| v.as_str()) - .map(|s| NewsletterMessageType::from(s.as_ref())) - .unwrap_or(NewsletterMessageType::Text); + let message_type = match msg_node.get_attr("edit").map(|v| v.as_str().into_owned()) { + Some(edit) if edit == "3" => NewsletterMessageType::Edit, + Some(edit) if edit == "8" => NewsletterMessageType::Revoke, + _ => msg_node + .get_attr("type") + .map(|v| v.as_str()) + .map(|s| NewsletterMessageType::from(s.as_ref())) + .unwrap_or(NewsletterMessageType::Text), + };🤖 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/newsletter.rs` around lines 618 - 622, The parser currently assigns message_type from msg_node.get_attr("type") which ignores the new edit/revoke wire shape; change the logic to first inspect msg_node.get_attr("edit") and map known edit codes (e.g. "3" -> NewsletterMessageType::Edit, "8" -> NewsletterMessageType::Revoke) to produce Edit/Revoke, and only if no edit attribute is present fall back to the existing type-based mapping (the current NewsletterMessageType::from path). Update the code where message_type is computed (the msg_node / NewsletterMessageType usage shown) so edit takes precedence over type while preserving the existing Text/Media fallback behavior.
🤖 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/features/newsletter.rs`:
- Around line 394-434: Both edit_message and revoke_message should reject an
empty message_id at the public API boundary: check the message_id (the id
variable produced from message_id.into()) for empty string and return an
Err(anyhow::anyhow!(...)) with a clear error when it is empty (mentioning
NewsletterMessage.message_id may be empty) before calling
crate::send::build_newsletter_edit_node or self.client.send_node; update both
functions (edit_message and revoke_message) to perform this validation and
short-circuit on empty id.
---
Outside diff comments:
In `@src/features/newsletter.rs`:
- Around line 618-622: The parser currently assigns message_type from
msg_node.get_attr("type") which ignores the new edit/revoke wire shape; change
the logic to first inspect msg_node.get_attr("edit") and map known edit codes
(e.g. "3" -> NewsletterMessageType::Edit, "8" -> NewsletterMessageType::Revoke)
to produce Edit/Revoke, and only if no edit attribute is present fall back to
the existing type-based mapping (the current NewsletterMessageType::from path).
Update the code where message_type is computed (the msg_node /
NewsletterMessageType usage shown) so edit takes precedence over type while
preserving the existing Text/Media fallback behavior.
🪄 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: bc7560d0-d14e-4e6c-9b37-ebc8b5f265b0
📒 Files selected for processing (2)
src/features/newsletter.rssrc/send.rs
Newsletter (channel) edit and revoke are real WhatsApp Web plaintext operations
(OutMessagePublishNewsletterEditMixin / RevokeMixin), not E2E. Add
Client::newsletter().edit_message(jid, message_id, new_content) and
revoke_message(jid, message_id).
Both reference the target by the original message's stanza id (the wire `id`),
matching WA Web (mergeNewsletterClientIDMixin sets only `id`) and whatsmeow
(sendNewsletter, req.ID = protocolMessage.key.id), NOT by server_id (that attr is
reaction-only). NewsletterMessage now exposes the wire `id` (message_id) so
callers can pass it. One builder (build_newsletter_edit_node) keyed off a
NewsletterEdit { Edit(&Message), Revoke } enum so invalid edit/revoke+content
combinations are unrepresentable:
edit: <message id={message_id} type={content} edit="3"><plaintext [mediatype]>{new content}</plaintext></message>
revoke: <message id={message_id} type="text" edit="8"><plaintext/></message>
The methods reject non-newsletter JIDs so a misuse cannot send plaintext to a
non-channel chat. Pin-in-chat is not a channel op and stays rejected.
jlucaso1
force-pushed
the
fix/newsletter-jid-reject-e2e-send
branch
from
June 5, 2026 05:08
ce464fe to
ec5a804
Compare
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.
Makes newsletter (channel) message operations compliant with WhatsApp Web. Channels are plaintext and never use the E2E/Signal path, so this both stops the mis-routing and implements the real plaintext edit/revoke operations.
Problem
pin_message/unpin_message(viasend_pin),edit_message, andrevoke_messagecallsend_message_impldirectly, bypassing the plaintext newsletter branch insend_message_with_options. A newsletter JID therefore fell into the DM/E2E branch and tried to build encrypted device fanout (Signal prekeys) against a channel that never uses Signal.Changes
Reject guard at the top of
send_message_impl: a newsletter JID on the E2E path fails fast with an error pointing at the newsletter methods. Covers every E2E-direct producer (revoke/pin/edit) at the root.Plaintext newsletter edit/revoke:
Client::newsletter().edit_message(jid, message_id, new_content)Client::newsletter().revoke_message(jid, message_id)These reference the target by the original message's stanza id (the wire
id), exposed onNewsletterMessage.message_id. A singlebuild_newsletter_edit_nodekeyed off aNewsletterEdit { Edit(&Message), Revoke }enum builds:<message id={message_id} type={content type} edit="3"><plaintext [mediatype]>{new content}</plaintext></message><message id={message_id} type="text" edit="8"><plaintext/></message>The methods reject non-newsletter JIDs so a misuse cannot send plaintext to a non-channel chat. Pin-in-chat is not a channel operation and stays rejected.
WA Web / whatsmeow cross-reference
id, NOTserver_id: WA WebmergeNewsletterClientIDMixinsetssmax("message", { id: STANZA_ID(messageId) })only;server_idappears only on the reaction/pollVote path (ClientAndServerIDMixin). whatsmeowsendNewslettersetsattrs["id"] = req.IDwherereq.ID = protocolMessage.key.id(the original message's stanza id string), and emits noserver_id.AdminEdit = "3",AdminRevoke = "8",ContentTypeText->type="text". whatsmeowEditAttributeAdminEdit/Revokematch.Tests
newsletter_jid_rejected_on_e2e_send_path,pin_message_rejects_newsletter.build_newsletter_edit_node_emits_plaintext_edit(text edit, id string, no mediatype, content round-trip),build_newsletter_edit_node_media_edit(type="media" + mediatype="image"),build_newsletter_edit_node_revoke_is_empty_plaintext.newsletter_edit_message_wrapper_sends_plaintext_edit(end-to-end via the sent-node intercept),newsletter_edit_revoke_reject_non_newsletter_jid.cargo fmt,cargo clippy --all-targets -- -D warnings, and the whatsapp-rust + wacore suites are green.Breaking
None on stable surface. The new
NewsletterMessage.message_idfield is additive.