feat(msg-secret)!: bound messageSecret retention by policy and event-time horizon - #668
Conversation
…dline The msg_secrets store dated rows by insert time (created_at), which is overwritten on every conflict-update and edit re-persist, so no age-based TTL was sound. Replace it with an absolute per-row expires_at deadline (0 = never) carried on MsgSecretEntry and computed by the caller from the parent message's event time plus a per-add-on-kind horizon. - MsgSecretEntry gains expires_at; merge_msg_secret_expiry keeps the later deadline on conflict (0 wins) so redelivery/re-persist never shrinks a window - put_msg_secret becomes a never-expiring convenience wrapper over put_msg_secrets - in-memory backend prunes by expires_at instead of created_at - history-sync records carry the parent messageTimestamp (proto tag 3, added to the partial-decode struct) and an is_poll_or_event flag for horizon selection
Single source of truth for the three previously-scattered msg_secret
decisions (capture / seed / prune):
- MsgSecretPolicy { Managed (default), BotOnly, Full, Disabled }
- MsgSecretRetention with verified per-kind horizons: text/edit 30d (offline
queue can deliver a 20-min-window edit up to 30d late), poll/event 90d
(PollAddOption/EventEdit have no sender-side window), bot 30d
- classify() + expires_at() compute the per-row deadline from the parent's
event time; within_seed_horizon() drives the seed-time age filter
- OriginalMessageResolver trait: app-supplied parent-secret fallback that
makes the Disabled tier workable for a headless core that keeps no messages
- migration adds expires_at column and an (expires_at, device_id) index, replacing the created_at index - legacy rows are backfilled to created_at + 30d so the pre-existing pairing-burst seed ages out under the default text horizon once pruning runs, rather than living forever or being mass-deleted on the first sweep - put_msg_secrets writes the per-row deadline and, on conflict, keeps the later deadline (0 = never wins), so redelivery/re-persist never shortens a window; the scalar put_msg_secret override is dropped in favor of the never-expiring trait default - delete_expired_msg_secrets prunes rows whose non-zero deadline has passed
Replaces the bare msg_secret_ttl_secs knob (which keyed off insert time) with: - CacheConfig.msg_secret_policy (default Managed), msg_secret_retention, and original_message_resolver, re-exported from the crate root - keepalive prune now runs whenever the policy prunes (Managed/BotOnly) and deletes by absolute per-row deadline (cutoff = now), logging the reclaim count Capture/seed still write the prior never-expire deadline; the real per-row deadline + seed filter land in the following commits so each step compiles.
Capture, outbound persist, and the edit re-persist now compute expires_at from the parent's event time and the message's retention class, and honor the policy: - persist_msg_secret_bytes / persist_outbound_msg_secret take a RetentionClass, skip writes under Disabled, and restrict to bot contexts under BotOnly - live capture stamps the message's delivery time and classifies poll/event vs text (bot chats always bot); the edit re-persist keys the next add-on's deadline off the parent kind - the never-expire scalar put path is gone from these sites
…lines This is where the pairing-burst bloat is removed. The seed now: - skips entirely under Disabled, seeds everything (never-expire) under Full - under Managed/BotOnly, drops any history secret whose parent is already past its retention horizon (30d text / 90d poll-event / 30d bot) — those parents can no longer receive a decryptable add-on, so seeding them is pure waste - stamps each seeded row with expires_at from the parent's own messageTimestamp, so it ages out by message age rather than seed time - BotOnly seeds only bot-context secrets (pre-#665 behavior) For a multi-month history this collapses the ~95k-row burst to the recent, still-relevant slice.
…miss When the in-core store and its LID/PN alternate both miss, the edit and msmsg bot decrypt paths now ask the app-supplied resolver (if configured) for the parent secret, trying the primary then the alternate sender. This is what makes the Disabled policy able to decrypt add-ons whose parent the core never kept, and lets a stateful app own retention via its own message store. - bounded by a 5s timeout since it runs inside the per-chat receive lane - preserves the existing graceful-miss behavior: edits surface the raw envelope with no NACK, msmsg still NACKs 495 terminally when nothing resolves - documents that newsletter chats carry no secret (no retention class)
…lver - seed filter: Managed drops text past 30d but keeps recent text and polls within 90d; Full seeds an ancient secret; Disabled seeds nothing - seeded rows carry an expires_at derived from the parent's messageTimestamp, so a prune past msg_ts + 30d removes them while an earlier prune keeps them - OriginalMessageResolver: a secret-encrypted edit decrypts via the resolver when the store is empty under the Disabled policy - adds create_test_client_with_config to inject a custom CacheConfig
|
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 (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds policy-driven message-secret retention: policy types, per-row expires_at/message_ts, batch put_msg_secrets API, backend merge semantics (in-memory and sqlite), history-sync seeding, resolver fallback for misses, send/inbound persistence changes, DB migration, and cleanup wiring. ChangesMessage Secret Retention Policy System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
|
@claude review |
|
Claude finished @jlucaso1's task in 4m 12s —— View job Code ReviewSolid PR. The design — one Findings1. Inconsistent bot-chat detection between call sites (minor bug) Consequence: for a chat against one of those legacy bot phone numbers, classification and 2. 3. 4. Duplicated retention classification (cleanup) 5. Resolver timeout is a hardcoded 5s (informational) 6. Schema index column order (informational) 7. Migration magic number (nit) Things that work well
|
![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: 9a54571d55
ℹ️ 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".
Benchmark Results67 unchanged benchmark(s)
|
Live capture (post-connect add-ons) is the high-value, low-cost path and stays on. Seeding from history only helps the boundary case — an add-on that arrives live yet references a parent delivered via history sync (edits of just- pre-pairing messages, add-options on still-open polls, offline-reconnect replays). Headless consumers that only react to new messages can now skip the pairing-time seed via CacheConfig.seed_msg_secrets_from_history (default true, so behavior is unchanged unless opted out).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/message.rs (1)
628-679:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't skip the app resolver when alternate store lookup errors.
Right now Line 673 goes straight to nack on alternate-lookup failure. That makes
original_message_resolveruseless in the exact degraded-store case where it should still save the decrypt, and it diverges from the secret-encrypted edit path above, which falls through to the resolver on store errors.🤖 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/message.rs` around lines 628 - 679, The code currently bails with a nack when alternate_msg_secret_lookup(...) returns Err(e), which prevents resolve_msg_secret_via_app(...) from being attempted on degraded-store errors; change the Err branch in the Ok(None) arm so that instead of immediately logging and calling spawn_nack(info, NackReason::MissingMessageSecret, None) you log the error (as a warn/debug), then proceed to call alternate_msg_secret_jid(...) and resolve_msg_secret_via_app(...) exactly like the Ok(None) path does; only if the resolver returns None should you then log the missing-secret and call spawn_nack(...). Ensure you reference and reuse the same variables used there (alternate_msg_secret_jid, resolve_msg_secret_via_app, target_sender, target_sender_str, target_id, info) so the behavior matches the secret-encrypted edit path.
🤖 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.rs`:
- Around line 227-249: The code currently calls persist_msg_secret_bytes twice
with separate awaits (in the block using dm_sender_identity_for and the similar
block at 505-523), which can leave only one alias persisted if the second write
fails; change this to build both secret-alias entries (the alias for
info.source.sender and, when chat_is_bot and dm_sender_identity_for(...) yields
a different sender, the alias for that sender) and call a single
put_msg_secrets(...) (or the atomic batch write function) once with both entries
instead of two awaited persist_msg_secret_bytes calls so both aliases are
written in one atomic operation.
- Around line 219-225: The early return is gating on chat_is_bot instead of the
classified retention class; move or change the check so BotOnly consults the
result of wacore::msg_secret::classify(msg, chat_is_bot) (the local variable
class) and only returns when policy.bot_only() && class !=
wacore::msg_secret::RetentionClass::Bot; update the logic around
policy.bot_only(), chat_is_bot, and the classify() call so the retention class
(not chat_is_bot) determines whether to skip storing the secret.
In `@wacore/src/msg_secret.rs`:
- Around line 171-174: Replace the single unconditional trait declaration for
OriginalMessageResolver with two cfg-gated variants: one compiled for wasm32
that uses #[cfg(target_arch = "wasm32")] and #[async_trait(?Send)] and does NOT
require Send + Sync, and a second compiled for non-wasm targets using
#[cfg(not(target_arch = "wasm32"))] and #[async_trait] that retains the Send +
Sync supertraits; keep the async fn resolve_msg_secret signature identical in
both variants so existing callers and CacheConfig::original_message_resolver:
Option<Arc<dyn OriginalMessageResolver>> continue to work.
---
Outside diff comments:
In `@src/message.rs`:
- Around line 628-679: The code currently bails with a nack when
alternate_msg_secret_lookup(...) returns Err(e), which prevents
resolve_msg_secret_via_app(...) from being attempted on degraded-store errors;
change the Err branch in the Ok(None) arm so that instead of immediately logging
and calling spawn_nack(info, NackReason::MissingMessageSecret, None) you log the
error (as a warn/debug), then proceed to call alternate_msg_secret_jid(...) and
resolve_msg_secret_via_app(...) exactly like the Ok(None) path does; only if the
resolver returns None should you then log the missing-secret and call
spawn_nack(...). Ensure you reference and reuse the same variables used there
(alternate_msg_secret_jid, resolve_msg_secret_via_app, target_sender,
target_sender_str, target_id, info) so the behavior matches the secret-encrypted
edit path.
🪄 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: 7bda4dc9-4d5b-4394-9dc9-4ec7e668b36d
📒 Files selected for processing (17)
src/appstate_sync.rssrc/cache_config.rssrc/history_sync.rssrc/keepalive.rssrc/lib.rssrc/message.rssrc/send.rssrc/test_utils.rsstorages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/down.sqlstorages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/up.sqlstorages/sqlite-storage/src/schema.rsstorages/sqlite-storage/src/sqlite_store.rswacore/src/history_sync.rswacore/src/lib.rswacore/src/msg_secret.rswacore/src/store/in_memory.rswacore/src/store/traits.rs
…r on error - Unify bot-chat detection on JidExt::is_bot() across capture/send/seed (was a mix of is_bot() and == Server::Bot, which diverged for legacy bot phone numbers), and classify group bot invocations (botMetadata / bot mention) as the Bot class so BotOnly keeps them and the later reply can decrypt. - Enforce BotOnly at a single chokepoint (build_msg_secret_entry / class != Bot) instead of an early return keyed on chat alone, so the edit re-persist path can no longer bypass it. - Centralize the three-way classification (classify_from_flags) so the seed and live paths can't drift. - Write the primary + alias secret rows in one atomic batch on capture and edit re-persist, so a partial failure can't leave only one alias stored. - On the msmsg bot path, treat a store error as a miss and still try the OriginalMessageResolver (mirrors the edit path) before nacking. Tests: BotOnly keeps a group bot prompt but skips a plain message; unit cover for is_bot_context / classify_from_flags.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- #3 Disabled now reaps legacy rows: prunes() returns true for everything except Full, so switching a pre-existing install to Disabled still ages out the backfilled rows. expires_at gates on a new bounds_retention() (Managed/ BotOnly only) so Full/Disabled keep returning 0. - #5 resolver timeout is now CacheConfig.msg_secret_resolver_timeout (default 5s) instead of a hardcoded constant, and the bound is documented on the trait. - #6 idx_msg_secrets_expires is keyed (device_id, expires_at) so the prune's per-device range scan localizes to one account. - #7 migration comments name 2592000 as 30d (= MsgSecretRetention::text).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@claude review |
|
Claude finished @jlucaso1's task in 3m 48s —— View job Code Review
The previous review pass has been substantively addressed in commits Remaining findings, ordered by severity: Findings1. 2. Edit re-persist uses the edit's arrival time as the parent timestamp (informational) 3. 4. 5. In-memory backend returns Things that work well
LGTM modulo the trait-bound cfg fix; the rest are nits. |
Resolves the WA-Web-fidelity gap surfaced in review: WhatsApp Web's receive path (ProcessEditProtocolMsgs) drops an edit authored outside the parent's edit-processing window, and we decrypted/dispatched secret-encrypted edits unconditionally. - store the parent message's event time (new message_ts column / MsgSecretEntry field, kept across redeliveries via MAX) and expose get_msg_secret_with_ts - on a MESSAGE_EDIT decrypt, drop it when editTs >= parentTs + 1200s (the authored-time check, matching WA Web); permissive when parent_ts is unknown (resolver-supplied secrets carry none) - the edit re-persist now keys the next add-on's deadline off the parent's own event time, not the edit's arrival time (was up to ~60d over-retention) - wasm-relax the OriginalMessageResolver trait (drop Send + Sync on wasm32, the way async_trait is already gated) so a wasm app can supply a !Send resolver - log when the history seed is skipped (opt-out flag or Disabled), and make the text-horizon doc precise that the window is an authored-time check on both send and receive Tests: in-window edit applies, out-of-window edit is dropped, unknown parent_ts stays permissive; get_msg_secret_with_ts round-trips and keeps the parent ts across a 0-ts redelivery.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7525f30e34
ℹ️ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/history_sync.rs (1)
286-304:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBotOnly seeding still drops historical group bot prompts.
This path only classifies from
chat.is_bot()plusrecord.is_poll_or_event. So in a normal group, a bot prompt parent is treated as text/poll here, andMsgSecretPolicy::BotOnlyskips it even though the live path treats that traffic as bot-context. That means a fresh pair can miss parent secrets for historical bot interactions unless the app also provides a resolver. Please thread a bot-context flag throughHistoryMsgSecretRecordand classify from that instead.🤖 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/history_sync.rs` around lines 286 - 304, The seeding logic uses msg_secret::classify_from_flags(chat.is_bot(), record.is_poll_or_event) which misclassifies historical group bot prompts because it relies on chat.is_bot() instead of whether the message itself was bot-context; update the type HistoryMsgSecretRecord to carry a bot_context (or similar) boolean, thread that field through wherever records are created, and replace the call in history_sync.rs to classify_from_flags(record.bot_context, record.is_poll_or_event) so MsgSecretPolicy::bot_only() correctly retains bot-context secrets during seeding.wacore/src/store/in_memory.rs (1)
599-649: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd parity coverage for
message_tsin the reference backend.This backend is the reference implementation used by a lot of higher-level tests, but the new parent-timestamp round-trip and non-clobber behavior never gets asserted here. Add the same kind of
get_msg_secret_with_ts+0-ts redelivery check you added for SQLite so edit-window regressions do not hide behind the in-memory backend.🤖 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/store/in_memory.rs` around lines 599 - 649, The in-memory backend must match SQLite's behavior for parent-timestamp round-trips and redelivery: ensure put_msg_secrets merges message_ts using the later non-zero value (use the max as already intended) and never overwrite a known non-zero existing timestamp with a zero from an incoming MsgSecretEntry; rely on merge_msg_secret_expiry for expiry merging and keep inserting (secret, expires_at, message_ts). Also make get_msg_secret delegate to get_msg_secret_with_ts (or at least return the same secret logic) so callers that rely on get_msg_secret_with_ts/0-ts redelivery behavior see identical results; update put_msg_secrets, get_msg_secret, and get_msg_secret_with_ts accordingly.
🤖 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/send.rs`:
- Around line 1495-1511: The outbound secret uses a wall-clock time taken after
send_node() finishes; capture the outbound event timestamp earlier (e.g. call
wacore::time::now_secs() once in send_message_impl before invoking send_node())
and thread that value through to where MsgSecretEntry is built so message_ts and
the expires_at calculation use the pre-send timestamp instead of the post-send
now; update the send_message_impl signature (and its callers) to accept an
outbound_message_ts and replace the local now usage in the MsgSecretEntry
construction and in the expires_at call so both fields are deterministically
stamped with the pre-send time.
In
`@storages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/up.sql`:
- Around line 6-8: Update the migration comment to state the correct inequality
and variable names used by the receive path: instead of saying "stale edit via
editTs < message_ts + window" mention that a stale edit is dropped when "editTs
>= parentTs + window" (use parentTs to match receive logic) and clarify that
message_ts (the new column on msg_secrets) stores the parent timestamp used in
that comparison; keep the rest of the comment intact.
In `@wacore/src/store/traits.rs`:
- Around line 421-425: The doc for put_msg_secrets is ambiguous about which
non-zero message_ts wins on key conflict; make the merge deterministic by
specifying that when both existing and incoming message_ts are non-zero the
later (max) timestamp wins (i.e., preserve the newest message_ts), and update
the contract comment accordingly; alternatively expose a small helper function
(e.g., merge_msg_secret_message_ts) alongside merge_msg_secret_expiry and
implement backends to call that helper so all implementations use identical
merge rules.
---
Outside diff comments:
In `@src/history_sync.rs`:
- Around line 286-304: The seeding logic uses
msg_secret::classify_from_flags(chat.is_bot(), record.is_poll_or_event) which
misclassifies historical group bot prompts because it relies on chat.is_bot()
instead of whether the message itself was bot-context; update the type
HistoryMsgSecretRecord to carry a bot_context (or similar) boolean, thread that
field through wherever records are created, and replace the call in
history_sync.rs to classify_from_flags(record.bot_context,
record.is_poll_or_event) so MsgSecretPolicy::bot_only() correctly retains
bot-context secrets during seeding.
In `@wacore/src/store/in_memory.rs`:
- Around line 599-649: The in-memory backend must match SQLite's behavior for
parent-timestamp round-trips and redelivery: ensure put_msg_secrets merges
message_ts using the later non-zero value (use the max as already intended) and
never overwrite a known non-zero existing timestamp with a zero from an incoming
MsgSecretEntry; rely on merge_msg_secret_expiry for expiry merging and keep
inserting (secret, expires_at, message_ts). Also make get_msg_secret delegate to
get_msg_secret_with_ts (or at least return the same secret logic) so callers
that rely on get_msg_secret_with_ts/0-ts redelivery behavior see identical
results; update put_msg_secrets, get_msg_secret, and get_msg_secret_with_ts
accordingly.
🪄 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: 323f53f2-2408-4d1b-8e06-f0ddf9a371ef
📒 Files selected for processing (10)
src/history_sync.rssrc/message.rssrc/send.rsstorages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/down.sqlstorages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/up.sqlstorages/sqlite-storage/src/schema.rsstorages/sqlite-storage/src/sqlite_store.rswacore/src/msg_secret.rswacore/src/store/in_memory.rswacore/src/store/traits.rs
- BotOnly now retains a historical group bot prompt: the partial history-sync decode picks up botMetadata (tag 7), so the seed classifies it as a bot context (record.is_bot_invocation) the same way live capture does. Without this a later msmsg bot reply to that prompt nacked as MissingMessageSecret. - add merge_msg_secret_message_ts alongside merge_msg_secret_expiry so both backends share one rule (later non-zero parent ts wins; 0 never clobbers); in-memory uses it and get_msg_secret now delegates to get_msg_secret_with_ts. - migration comment states the actual drop condition (editTs >= parentTs + window) and that message_ts holds parentTs. Test: BotOnly seeds a group bot prompt (botMetadata) but skips a plain message. Skipped the outbound pre-send-timestamp suggestion: post-send vs pre-send differ by sub-second/seconds on 1200s/30d horizons and only ever err permissive, so it doesn't justify threading a timestamp through send_message_impl + all callers.
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)
wacore/src/history_sync.rs (1)
561-568:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCarry parent poll/event retention explicitly.
This metadata contract is too weak for the 90-day add-on rule.
PollAddOptionandEventEditdo not becomeis_poll_or_event = truehere, so the history-seed path downstream will classify them like plain text and skip/prune them after 30 days underManaged/BotOnly. Thread an explicit retention class or parent-retention marker throughHistoryMsgSecretRecordinstead of only the current-message boolean.Also applies to: 638-658
🤖 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/history_sync.rs` around lines 561 - 568, The current boolean field is_poll_or_event on the message metadata is insufficient because add-on messages like PollAddOption and EventEdit inherit the parent's 90-day retention but are not marked true; update HistoryMsgSecretRecord to carry an explicit parent_retention_class (or parent_is_poll_or_event) field and thread it through the code paths that construct/transform HistoryMsgSecretRecord so downstream history-seed logic can consult this parent marker; update any places referencing is_poll_or_event (including creation sites that see PollAddOption, EventEdit, and the code paths around functions that build/serialize HistoryMsgSecretRecord) to set the new parent_retention_class appropriately so PollAddOption/EventEdit get the longer retention.
🤖 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 `@wacore/src/store/traits.rs`:
- Around line 421-425: The docs leave secret bytes merge unspecified, causing
backends to diverge (InMemoryBackend currently replaces secrets). Add a
deterministic secret-merge rule: the stored secret must follow the same
selection as message_ts (use the secret from the row chosen by
merge_msg_secret_message_ts — i.e., the row with the later non-zero message_ts),
and if message_ts ties or both are zero, pick the secret from the row with the
later expires_at; if expires_at also ties, prefer the incoming row's secret.
Update the merge implementation functions (merge_msg_secret_expiry,
merge_msg_secret_message_ts) to apply this rule and change InMemoryBackend
upsert/merge logic to follow it so all backends behave identically.
---
Outside diff comments:
In `@wacore/src/history_sync.rs`:
- Around line 561-568: The current boolean field is_poll_or_event on the message
metadata is insufficient because add-on messages like PollAddOption and
EventEdit inherit the parent's 90-day retention but are not marked true; update
HistoryMsgSecretRecord to carry an explicit parent_retention_class (or
parent_is_poll_or_event) field and thread it through the code paths that
construct/transform HistoryMsgSecretRecord so downstream history-seed logic can
consult this parent marker; update any places referencing is_poll_or_event
(including creation sites that see PollAddOption, EventEdit, and the code paths
around functions that build/serialize HistoryMsgSecretRecord) to set the new
parent_retention_class appropriately so PollAddOption/EventEdit get the longer
retention.
🪄 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: b518102e-4f10-4718-b65d-32ec8ca223c5
📒 Files selected for processing (5)
src/history_sync.rsstorages/sqlite-storage/migrations/2026-05-31-000000_msg_secret_expires_at/up.sqlwacore/src/history_sync.rswacore/src/store/in_memory.rswacore/src/store/traits.rs
Resolve src/message.rs: keep the envelope-frame fix and the oxidezap#667 dispatch regression test on top of main's oxidezap#668 msg-secret retention rewrite of maybe_decrypt_secret_encrypted_message (get_msg_secret_with_ts + edit window) and its window/resolver tests.
1 participant
What
Bounds the per-message
messageSecretstore, which a recent change (#665) made grow without limit: it seeds a secret for every non-forwarded history-sync message and captures one for every live message and every send, and nothing ever prunes them. On a freshly paired headless account that produced ~95k rows accounting for ~99% of the SQLite file.Retention is now one policy plus a per-row, event-time deadline, all on
CacheConfig:msg_secret_policy: MsgSecretPolicy—Managed(default),BotOnly,Full,Disabled— unifies the three previously-scattered decisions (capture on live receive, seed from history, prune).expires_at(absolute unix seconds,0= never) replaces the oldcreated_at-based TTL.created_atwas insert time and was overwritten on every conflict-update and edit re-persist, so no age-based prune was ever sound. The deadline is computed from the parent message's own event time plus a per-add-on-kind horizon, and on conflict the later deadline wins (0= never wins) so a redelivery or edit re-persist never shortens a window.msg_secret_retentionhorizons: text/MESSAGE_EDIT30d, poll/event 90d, bot 30d. An edit is only valid when authored within 20 min of the parent, but the offline queue can deliver that (validly-authored) edit up to ~30 days later, so 30d is the floor that keeps a 30-day-offline receiver able to decrypt.PollAddOption/EventEdithave no window and ride the parent's secret, so poll/event parents get the longer 90d horizon.message_ts) so the receive path can drop a secret-encryptedMESSAGE_EDITauthored outside the window (editTs >= parentTs + 1200s), matching WhatsApp Web'sProcessEditProtocolMsgs. The check is on authored time (not "now"), so a validly-authored edit still applies after an offline gap; it is permissive when the parent's time is unknown (resolver-supplied secrets carry none).seed_msg_secrets_from_history: bool(defaulttrue) — live capture (covering everything received after connect) is independent from history seeding. Seeding only matters for the boundary case: an add-on that arrives live yet references a parent delivered via history sync. Headless consumers that only react to new messages can flip this off.original_message_resolver(+msg_secret_resolver_timeout, default 5s) — an app-supplied fallback consulted when an add-on's parent secret is absent from the store (and its LID/PN alternates), bounded by a timeout because it runs in the per-chat receive lane. This is what lets theDisabledpolicy still decrypt: an app that keeps its own message store can own retention entirely and answer secret lookups on demand. (Send + Syncis dropped on wasm32 so a wasm app can supply a!Sendresolver.)Newsletters arrive as plaintext and never carry a
secret_encrypted_message, so there is no newsletter retention class (documented inline to avoid a future dead branch).Why
The secrets only unlock add-ons that reference a parent by id — secret-encrypted edits, msmsg bot replies, and poll/event edits — all of which are bounded in time. The send path never reads the store, and poll votes/reactions take an app-supplied secret rather than reading it, so the only real consumers are five inbound add-on kinds with finite useful lifetimes. Keeping a secret for every message forever (and re-seeding the entire history on pairing) is therefore pure storage cost with no decrypt benefit beyond the horizon.
The redesign is grounded in verified behavior: the edit window is 1200s (20 min), enforced on both send and receive as an authored-time check (
editTs < parentTs + window) — not relative to "now" — so an offline-delivered but validly-authored edit still applies, which is exactly why retention is sized to the ~30-day delivery window rather than the 20-minute authoring window. PollAddOption/EventEdit have no window at all, and edits by other users (newsletter admins) still key the lookup on the original author's secret, so the existing keying already covers them.Breaking Change
BREAKING CHANGE:
CacheConfig.msg_secret_ttl_secs: u64is removed. Replace it withmsg_secret_policy: MsgSecretPolicy(defaultManaged) plus the additivemsg_secret_retention,seed_msg_secrets_from_history(defaulttrue),original_message_resolver, andmsg_secret_resolver_timeout. The default behavior changes from "seed and keep everything forever" to "seed only the still-relevant slice and prune by event-time horizon".Migration:
msg_secret_policy: MsgSecretPolicy::Full.MsgSecretPolicy::BotOnly.MsgSecretPolicy::Disabledand register anoriginal_message_resolver.MsgSecretStore::put_msg_secretsis now the required method and carriesMsgSecretEntry { expires_at, message_ts }; the scalarput_msg_secretis a never-expiring default wrapper, andget_msg_secret_with_tshas a default returning0for the parent time. Custom backends that implemented the scalar should move toput_msg_secretsand honorexpires_at(keep the later deadline on conflict,0wins) andmessage_ts(keep the non-zero value).Existing installs are not mass-deleted: the SQLite migration backfills legacy rows to
created_at + 30d, so the prior pairing-burst seed ages out gradually once pruning runs (which now includesDisabled, so switching to it also reclaims legacy rows — they drain as each row hits its backfilled deadline, completing within ~30 days, not all at once on the first sweep).Tests
New coverage:
Manageddrops text past 30d but keeps recent text and polls within 90d;Fullseeds an ancient secret;Disabledseeds nothing; the opt-out flag skips seeding underManagedexpires_atderived from the parent'smessageTimestamp— a prune pastmsg_ts + 30dremoves the row while an earlier prune keeps itget_msg_secret_with_tsround-trips and keeps the parent ts across a 0-ts redeliveryBotOnlykeeps a group bot prompt (botMetadata) but skips a plain message; unit cover foris_bot_context/classify_from_flags/ policy predicatesOriginalMessageResolver: a secret-encrypted edit decrypts via the resolver when the store is empty underDisabled0/never wins); prune deletes only passed deadlinesValidation:
cargo fmt --allcargo clippy -p wacore -p whatsapp-rust -p whatsapp-rust-sqlite-storage --all-targets -- -D warningscleancargo test -p wacore -p whatsapp-rust -p whatsapp-rust-sqlite-storage— 819 (wacore lib) + 644 (whatsapp-rust lib) + 31 (sqlite) passing