fix(send): never memoize own devices in the sender-key map (WA Web parity) - #999
Conversation
…rity) The warm mark (update_sender_key_devices) recorded has_key=true for our own companion devices, but the retry/forget path (mark_forget_sender_key) excludes own devices — a deliberate defense so an inbound retry receipt naming our device can't tear down our own group session. That asymmetry orphans an own companion whose single SKDM encryption failed: it's marked warm (so never re-targeted) yet can never be un-marked (forget skips own), so it can't decrypt our group messages until an unrelated full rotation. WA Web's ParticipantStore guards EVERY markHasSenderKey/markForgetSenderKey mutation with !isMeDevice, so own devices are never memoized at all: they fall into skDistribList on every send and get a fresh SKDM each time — which is exactly why WA Web can't orphan its own companions. Mirror that: exclude own devices from the warm mark too. filter_skdm_targets already re-targets any un-memoized device (a never-marked companion is never device_and_primary_warm), so own companions now re-distribute every send. External devices are unchanged (full-list mark + retry repair); phash is computed from the resolved set, not the marks, so it's unaffected.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR flips the ChangesOwn-Companion Warm Memoization Exclusion
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Listen, this is a small diff but it touches sender-key correctness, which is core infrastructure — we don't move fast and break encryption. One flag flip, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
| Filename | Overview |
|---|---|
| src/send/mod.rs | One-line fix changing exclude_own_devices from false to true in update_sender_key_devices, plus updated doc comment and a new end-to-end test verifying redistribution every send. |
| src/client/sender_keys.rs | Adds warm_mark_excludes_own_devices unit test; no logic changes to production code in this file. |
| wacore/src/send/group.rs | Doc comment update only — clarifies that skdm_devices own-device filtering now happens at persist time; no production logic change. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant S as send_group_stanza
participant U as update_sender_key_devices
participant SK as set_sender_key_status_for_devices
participant DB as DB / sender_key_device_cache
participant F as filter_skdm_targets (next send)
S->>U: skdm_devices (own companion + external members)
U->>SK: "has_key=true, exclude_own_devices=true"
SK->>SK: filter: drop own-user JIDs
SK->>DB: "persist only external members as has_key=true"
SK->>DB: invalidate cache for group
U->>DB: invalidate cache (second call, idempotent)
Note over F: Next send — own companion never in DB
F->>DB: device_and_primary_warm(own_companion) false
F-->>S: own companion included in needs_skdm, fresh SKDM sent
F->>DB: device_and_primary_warm(external_member) true
F-->>S: external member excluded from needs_skdm, no resend
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant S as send_group_stanza
participant U as update_sender_key_devices
participant SK as set_sender_key_status_for_devices
participant DB as DB / sender_key_device_cache
participant F as filter_skdm_targets (next send)
S->>U: skdm_devices (own companion + external members)
U->>SK: "has_key=true, exclude_own_devices=true"
SK->>SK: filter: drop own-user JIDs
SK->>DB: "persist only external members as has_key=true"
SK->>DB: invalidate cache for group
U->>DB: invalidate cache (second call, idempotent)
Note over F: Next send — own companion never in DB
F->>DB: device_and_primary_warm(own_companion) false
F-->>S: own companion included in needs_skdm, fresh SKDM sent
F->>DB: device_and_primary_warm(external_member) true
F-->>S: external member excluded from needs_skdm, no resend
Reviews (1): Last reviewed commit: "fix(send): never memoize own devices in ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR changes sender-key memoization to exclude own companion devices, a deliberate change in group encryption distribution. Despite tests and docs, encryption path changes carry risk and require human verification.
Re-trigger cubic
📦 Binary size report
.text per crate
Baseline: |
1 participant
What
Stop memoizing our own companion devices in the group sender-key
has_keymap, mirroring WA Web. Follow-up to #996 (the residual that a device markedhas_key=truewithout an SKDM actually being encrypted for it could be orphaned).Why (the orphan bug)
Two paths were asymmetric:
update_sender_key_devices→set_sender_key_status_for_devices(..., has_key=true, exclude_own_devices=false): marked the full distribution target, including our own companions, even devices whose per-device SKDM encryption failed.mark_forget_sender_key→set_sender_key_status_for_devices(..., false, exclude_own_devices=true): excludes own devices — a deliberate defense (seecold_mark_excludes_own_devices) so an inbound retry receipt naming our own device can't force us to forget our own sender key and tear down our own group session.So an own companion whose one SKDM encryption failed (or that was marked warm without a node ever reaching it) is marked warm → never re-targeted, yet can never be un-marked (forget skips own). It stays keyless and can't decrypt our group messages until an unrelated wholesale rotation (participant removal / PN↔LID migration). External devices recover via the retry-receipt
mark_forget_sender_key; own companions have no such path.How WA Web does it (source of truth)
docs/captured-js/WAWeb/Api/ParticipantStore.js— the store helperC(record, deviceList, flag)behind bothmarkHasSenderKeyandmarkForgetSenderKeyguards every mutation withif (!isMeDevice(e)). Own devices are never tracked in the sender-key memory at all. The read side (getGroupSenderKeyList) therefore always puts own companions inskDistribList, so they get a fresh SKDM (DSM-wrapped) on every send. That is precisely why WA Web can't orphan its own companions, and why excluding own devices from forget is consistent.The fix
Exclude own devices from the warm mark too (
exclude_own_devices=true), matching WA Web's universal!isMeDeviceguard. Own companions are then never memoized, andfilter_skdm_targets(which already targets any device that isn'tdevice_and_primary_warm— a never-marked device qualifies) re-distributes their SKDM on every send. No orphaning is possible.has_keymarks (build_group_phash_set), so it's unaffected.<enc>nodes per send when the account has companions — exactly the trade-off WA Web accepts (guaranteed delivery over a micro-optimization). Zero change for a bot with no companions.Tests
warm_mark_excludes_own_devices(unit) — the warm mark leaves an external memberhas_key=trueand our own companion unmemoized.own_companion_is_never_memoized_so_it_redistributes_every_send(end-to-end) — after a send marks its targets, the DB shows the external member warm and the own companion absent; the next send'sfilter_skdm_targetsre-targets only the own companion.cargo fmt/clippyclean;wacoresend suite (96) +whatsapp-rustsender-key/skdm suite (24) pass. Existingfailed_device_is_still_marked_has_key(prepare-time full list) unaffected — the full set is still returned; only the persist step excludes own.