feat(message_edit): decrypt secretEncryptedMessage MESSAGE_EDIT envelope - #618
Merged
Conversation
WhatsApp wraps message edits in an E2E envelope (`secret_encrypted_message` with `secret_enc_type = MESSAGE_EDIT`) keyed by the original message's `messageContextInfo.messageSecret`. Clients that only listened to the legacy `protocolMessage.editedMessage` path were missing edits from newer senders. Add the receiver primitives, verified against `docs/captured-js/`: - `wacore::secret_enc_addon` — generic HKDF-SHA256 + AES-256-GCM helper covering the full addon family (PollVote, PollEdit, PollAddOption, EventResponse, EventEdit, EncReaction, EncComment, ReportToken, MessageEdit). `ModificationType::aad_mode()` encodes the rule from `WAWebAddonEncryption.js` function `g`: only `PollVote` and `EventResponse` bind `stanzaId\0sender` into AAD, everything else (including all edits) uses empty AAD. - `wacore::message_edit` — typed encrypt/decrypt for the MESSAGE_EDIT envelope plus a 2-attempt LID/PN fallback mirroring `decryptAddOn`. - `MessageEdits` feature surface (`Client::message_edits()`) — JID normalisation via `to_non_ad()`, envelope extraction, and a `rewrap_as_legacy_edit` helper that re-shapes the decrypted inner message into the legacy `protocolMessage.editedMessage` form so downstream consumers handle one shape regardless of envelope. `wacore::poll` now delegates to the generic helper while keeping its public API (a `_with_secret` variant is exposed for the recommended single-step path). Auto-decrypt on the dispatch path is intentionally not wired here — that requires a message-getter callback to look up the parent's `messageSecret`. Consumers call `MessageEdits::decrypt` from their `Event::Message` handler, same pattern as `Polls::decrypt_vote`.
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a shared secret-addon encryption framework (HKDF-SHA256 + AES-256-GCM with AAD modes), implements MESSAGE_EDIT envelope encryption/decryption with JID normalization and optional fallback, migrates poll vote crypto to the addon framework, and exposes high-level feature helpers and module wiring with unit tests. ChangesMessage Edit E2EE with Shared Addon Framework
Sequence Diagram(s)sequenceDiagram
participant Client
participant Features as src::features
participant wacore_message_edit as wacore::message_edit
participant secret_addon as secret_enc_addon
participant AESGCM as AES-256-GCM
Client->>Features: extract_envelope(msg) -> EncryptedEdit
Features->>wacore_message_edit: decrypt_message_edit_with_fallback(enc_payload, iv, message_secret, primary_ctx, fallback?)
wacore_message_edit->>secret_addon: decrypt_addon(enc_payload, iv, message_secret, AddonContext)
secret_addon->>AESGCM: decrypt with derived key + AAD, verify tag
AESGCM-->>secret_addon: plaintext or error
secret_addon-->>wacore_message_edit: plaintext or error
wacore_message_edit->>Client: decoded inner waproto::Message or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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: 8ee71285ad
ℹ️ 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".
Comment on lines
+191
to
+196
| let raw = self | ||
| .target_message_key | ||
| .participant | ||
| .as_deref() | ||
| .or(self.target_message_key.remote_jid.as_deref()) | ||
| .ok_or_else(|| anyhow!("target message key missing participant and remote_jid"))?; |
There was a problem hiding this comment.
When target_message_key.participant is absent, this helper always treats remote_jid as the original sender. In 1:1 chats, keys for messages sent by the local user have from_me=true and remote_jid set to the peer, so using remote_jid here derives the wrong HKDF context and causes MESSAGE_EDIT decryption to fail for edits of self-authored messages unless callers bypass this helper. This new API should branch on from_me (using the local account JID) instead of unconditionally using remote_jid.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wacore/src/poll.rs (1)
74-106: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueLegacy
decrypt_poll_voteduplicatesbuild_aadlogic — acceptable for now.Lines 95-98 hand-roll the PollVote AAD (
stanza_id || 0x00 || voter_jid), which is exactly whatsecret_enc_addon::build_aaddoes. Sincedecrypt_poll_votetakes a pre-derived key (not amessage_secret), it can't just calldecrypt_addon. If you want to kill this duplication without changing the signature, makesecret_enc_addon::build_aadpub(crate)and call it here. Not blocking — but it's a real foot-gun if someone tweaks the AAD format in one place and forgets the other.🤖 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 `@wacore/src/poll.rs` around lines 74 - 106, The decrypt_poll_vote function duplicates the AAD construction (stanza_id || 0x00 || voter_jid), which risks divergence from secret_enc_addon::build_aad; make secret_enc_addon::build_aad pub(crate) and replace the manual AAD assembly in decrypt_poll_vote with a call to secret_enc_addon::build_aad(stanza_id, voter_jid) so the same canonical AAD is used everywhere (do not change decrypt_poll_vote's signature or its use of the pre-derived encryption_key; keep decrypt_addon and message_secret untouched).
🤖 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/message_edit.rs`:
- Around line 126-144: In extract_envelope, keep returning None for invalid IV
lengths but add a diagnostic log before the return: when enc_iv.len() != 12,
call log::warn! (including contextual identifiers such as that the message had
secret_encrypted_message, the secret_enc_type is MessageEdit, and any available
message id or target_message_key) so malformed envelopes are visible in logs;
ensure you import/use the log crate and avoid exposing sensitive payload
contents (do not log enc_payload).
- Around line 31-38: MessageEdits currently just wraps a Client but all public
routines (MessageEdits::decrypt, decrypt_with_fallback, extract_envelope,
rewrap_as_legacy_edit) are static/associated functions, which misleads callers;
either convert the API to actually use the stored client by making the public
methods take &self (e.g. implement decrypt_envelope(&self, ...) that calls
self._client where needed and composes extract_envelope + decrypt +
rewrap_as_legacy_edit), or remove the MessageEdits struct and export those
functions as plain pub fn in the module; pick one approach and update
Client::message_edits() and any call sites accordingly so the surface matches
the implementation intent.
In `@wacore/src/poll.rs`:
- Around line 41-68: The public function encrypt_poll_vote was removed, breaking
downstream callers; restore a public wrapper named pub fn encrypt_poll_vote with
the original signature and behavior, implement it to derive or accept the same
message_secret as before (mirror how decrypt_poll_vote expects inputs) and
delegate to encrypt_poll_vote_with_secret so behavior is identical, keep or
re-add original doc comment and visibility so the API surface matches
decrypt_poll_vote; if you want to signal transition, mark the wrapper deprecated
but keep it present to avoid the breaking change.
In `@wacore/src/secret_enc_addon.rs`:
- Around line 331-342: The test message_edit_aad_is_empty_unlike_poll_vote
currently only does a MessageEdit roundtrip and doesn't exercise AAD-branching;
either remove/update the misleading comment or change the test to derive the
encryption key once and then call the low-level AES-GCM helpers with two
different AAD buffers (MessageEdit vs PollVote) to ensure decryption fails when
AAD differs. To implement the latter, obtain the symmetric key the same way
encrypt_addon does (or expose build_aad/pub(crate) so you can call it) and then
call the AES-GCM encrypt/decrypt helpers directly using the two AADs (or use
aad_mode()/build_aad) to prove decryption fails due to AAD mismatch rather than
key derivation differences; update assertions accordingly. Ensure references to
ModificationType::MessageEdit and PollVote, encrypt_addon, decrypt_addon,
build_aad (or aad_mode) are used to locate the relevant code.
---
Outside diff comments:
In `@wacore/src/poll.rs`:
- Around line 74-106: The decrypt_poll_vote function duplicates the AAD
construction (stanza_id || 0x00 || voter_jid), which risks divergence from
secret_enc_addon::build_aad; make secret_enc_addon::build_aad pub(crate) and
replace the manual AAD assembly in decrypt_poll_vote with a call to
secret_enc_addon::build_aad(stanza_id, voter_jid) so the same canonical AAD is
used everywhere (do not change decrypt_poll_vote's signature or its use of the
pre-derived encryption_key; keep decrypt_addon and message_secret untouched).
🪄 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: 74efce10-a892-4798-94e2-97e6938e5a09
📒 Files selected for processing (8)
src/features/message_edit.rssrc/features/mod.rssrc/features/polls.rssrc/lib.rswacore/src/lib.rswacore/src/message_edit.rswacore/src/poll.rswacore/src/secret_enc_addon.rs
…om_me `EncryptedEdit::original_sender_jid` derived the original sender from `participant || remote_jid`, which is correct for incoming 1:1 and group edits but wrong for self-sent edits that arrive via device-sync: the target message key has `from_me = true` and its `remote_jid` points to the *other* party, not us. The HKDF info would then be keyed under the wrong sender JID and the GCM tag check would fail. WA Web sidesteps this because `MsgGetters.getOriginalSender` reads `originalSelfAuthor || sender` from its materialised msg row, which already carries the resolved self-author. We have no row, so we must reconstruct the same fact from `from_me` plus the caller's own JID. Updated resolution order: 1. `participant` (always set in groups) 2. `my_jid` when `from_me == Some(true)` (self-sent edit sync) 3. `remote_jid` (1:1 incoming edit, chat == other party) `my_jid` is required as an argument. Added tests for the self-sent case and the 1:1 incoming case to lock the resolution in.
Address review on PR #618. - features/message_edit: drop the MessageEdits struct (all methods were associated; the stored Client ref was dead weight). Expose plain pub fn at module level and re-export EncryptedEdit from features. Drop Client::message_edits(); module is reachable via whatsapp_rust::features::message_edit. - features/message_edit::extract_envelope: log::warn on malformed envelopes (missing fields or IV != 12 bytes) instead of silently returning None. Does not log enc_payload. - wacore::poll: restore encrypt_poll_vote with the original pre-derived key signature, symmetric to decrypt_poll_vote. Delegates through the same AES-GCM helper + AAD as decrypt for byte-equivalent behaviour. - wacore::poll: DRY the AAD construction by routing through secret_enc_addon::build_aad (now pub(crate)) for both decrypt_poll_vote and decrypt_poll_vote_with_secret. Extract a poll_vote_addon_ctx helper so the Context lives in one place. - wacore::secret_enc_addon: build_aad is pub(crate). - wacore::secret_enc_addon: rewrite message_edit_aad_is_empty_unlike_poll_vote (renamed aad_mismatch_under_same_key_fails_decrypt) to actually exercise the AAD branch: derive one key, encrypt under the PollVote AAD shape, and prove that decrypting the same ciphertext under the empty MessageEdit AAD fails on tag verification. All workspace lib tests pass (1406 total). Zero clippy warnings.
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/features/message_edit.rs`:
- Around line 73-110: decrypt_with_fallback currently normalizes JIDs and builds
a fallback MessageEditContext that can be byte-identical to the primary context,
causing a wasted second decryption; to fix, after constructing fb_orig/fb_editor
and before creating fallback_ctx compare the normalized fallback values
(fb_orig.as_deref(), fb_editor.as_deref()) against primary.original_sender_jid
and primary.editor_jid and set fallback_ctx to None when both match the primary
(i.e., skip creating a fallback MessageEditContext that equals primary), then
call message_edit::decrypt_message_edit_with_fallback with that possibly None
fallback.
🪄 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: 88917a64-fd66-45f3-8abb-980d16f8a8b3
📒 Files selected for processing (5)
src/features/message_edit.rssrc/features/mod.rssrc/lib.rswacore/src/poll.rswacore/src/secret_enc_addon.rs
`decrypt_with_fallback` previously built a fallback context whenever either fallback JID was Some, including when those values normalised (via `to_non_ad()`) to byte-identical strings as the primary context. That second decrypt was guaranteed to fail the same way as the first. Resolve fallback JIDs once, then short-circuit to `fallback_ctx = None` when both resolved values equal the primary. Covers both the no-fallback-supplied case and the "alternate-form normalises identical" case in one branch. Test: `fallback_normalising_to_primary_jids_is_skipped` proves the no-op was elided by asserting the returned error is the bare primary error rather than the combined `primary=...; fallback=...` shape that `decrypt_message_edit_with_fallback` emits when both attempts run.
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/features/message_edit.rs`:
- Around line 137-152: The match currently accepts Some(enc_payload) of zero
length as valid and constructs EncryptedEdit, which later fails during AES-GCM
tag extraction; update the pattern guard so the success arm only matches when
enc_payload is Some(payload) and payload.len() is at least the AES-GCM tag
length (e.g. >= 16) in addition to iv.len() == 12, so empty/too-short payloads
fall through to the warn! branch; reference the match over (target_key,
enc_payload, enc_iv), the success arm that returns Some(EncryptedEdit {
enc_payload, enc_iv, target_message_key: tk }) and the warn! fallback to ensure
malformed envelopes are logged.
🪄 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: 6df1f323-5aad-4daa-8298-f1a6bcf7cdad
📒 Files selected for processing (1)
src/features/message_edit.rs
Comment on lines
+137
to
+152
| match (target_key, enc_payload, enc_iv) { | ||
| (Some(tk), Some(payload), Some(iv)) if iv.len() == 12 => Some(EncryptedEdit { | ||
| enc_payload: payload, | ||
| enc_iv: iv, | ||
| target_message_key: tk, | ||
| }), | ||
| (tk, payload, iv) => { | ||
| warn!( | ||
| "secret_encrypted_message MESSAGE_EDIT malformed: target_id={:?} has_payload={} iv_len={:?} (expected 12)", | ||
| tk.and_then(|t| t.id.as_deref()), | ||
| payload.is_some(), | ||
| iv.map(|b| b.len()), | ||
| ); | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Optional: empty enc_payload slips past the malformed-envelope warn.
A sender that ships enc_payload = Some(vec![]) (zero-length) with a 12-byte IV will be wrapped as a "valid" EncryptedEdit, then fail downstream in GCM tag extraction with a generic decrypt error — no warn! here, even though the envelope is structurally broken (AES-GCM ciphertext must carry the 16-byte tag). If you want the diagnostic-on-malformed path to cover this too, fold a minimum-length check into the same arm:
♻️ Optional tightening of envelope validation
match (target_key, enc_payload, enc_iv) {
- (Some(tk), Some(payload), Some(iv)) if iv.len() == 12 => Some(EncryptedEdit {
+ (Some(tk), Some(payload), Some(iv)) if iv.len() == 12 && payload.len() >= 16 => Some(EncryptedEdit {
enc_payload: payload,
enc_iv: iv,
target_message_key: tk,
}),
(tk, payload, iv) => {
warn!(
- "secret_encrypted_message MESSAGE_EDIT malformed: target_id={:?} has_payload={} iv_len={:?} (expected 12)",
+ "secret_encrypted_message MESSAGE_EDIT malformed: target_id={:?} payload_len={:?} iv_len={:?} (expected payload>=16, iv=12)",
tk.and_then(|t| t.id.as_deref()),
- payload.is_some(),
+ payload.map(|p| p.len()),
iv.map(|b| b.len()),
);
None
}
}Not blocking — current behaviour is still safe, just diagnostically thinner on this one edge.
🤖 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/message_edit.rs` around lines 137 - 152, The match currently
accepts Some(enc_payload) of zero length as valid and constructs EncryptedEdit,
which later fails during AES-GCM tag extraction; update the pattern guard so the
success arm only matches when enc_payload is Some(payload) and payload.len() is
at least the AES-GCM tag length (e.g. >= 16) in addition to iv.len() == 12, so
empty/too-short payloads fall through to the warn! branch; reference the match
over (target_key, enc_payload, enc_iv), the success arm that returns
Some(EncryptedEdit { enc_payload, enc_iv, target_message_key: tk }) and the
warn! fallback to ensure malformed envelopes are logged.
9 tasks
This was referenced May 27, 2026
This was referenced Jun 8, 2026
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.
Summary
WhatsApp wraps message edits in an E2E envelope (
secret_encrypted_messagewithsecret_enc_type = MESSAGE_EDIT) keyed by the original message'smessageContextInfo.messageSecret. Clients that only listened to the legacyprotocolMessage.editedMessagepath were missing edits from newer senders. The proto already hadSecretEncryptedMessage/MessageEdit = 2but no receiver code consumed the field.This PR adds the receiver primitives, verified against a recent WA Web bundle. It does not auto-decrypt on the dispatch path (see "Out of scope" below) — consumers call the new helper from their
Event::Messagehandler, same pattern asPolls::decrypt_vote.Algorithm
```
info = msgId || origSenderJid || editorJid || "Message Edit"
salt = zeros[32]
key = HKDF-SHA256(salt, ikm = messageSecret, info, L = 32)
aad = (empty)
ct = AES-256-GCM-Encrypt(key, iv12, aad, plaintext)
```
Plaintext is a `Message` proto whose `protocolMessage.editedMessage` carries the new content.
Evidence (WA Web modules)
The Baileys PR #2547 that prompted this work claims the same algorithm; each constant was verified independently against the captured WA Web bundle rather than trusting the PR.
Changes
Out of scope (intentional)
Test plan