docs(signal-protocol): document DH ratchet lease rebase and session quarantine - #452
Conversation
…uarantine (whatsapp-rust#1149) Documents the counter-lease-stranding fix from whatsapp-rust PR #1149: a DH ratchet now rebases the sender-chain lease onto the fresh chain instead of leaving a stale ceiling that could refuse to load past MAX_RESERVATION_FAST_FORWARD, and an undecodable session row is now quarantined (reported absent) rather than propagated as a load error, so recovery can replace it automatically. Also adds the new wa_session_record_quarantined_total metric to the counter catalogue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ECfouxpzsKhJ4heK8MUDk
📝 WalkthroughWalkthroughThe documentation describes undecodable session-row quarantine, its metric and self-healing recovery behavior, and how DH ratchet resets rebase sender-chain counter leases. ChangesSession recovery documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@api/signal.mdx`:
- Around line 628-630: Update the reset example near the Note to scope it to
sessions that decode successfully but are logically invalid, such as after an
identity compromise; remove or replace the “corrupted session” wording so
undecodable storage rows are not directed to manual reset.
In `@concepts/storage.mdx`:
- Line 350: Rewrite the quarantine explanation in concepts/storage.mdx lines
350-350 using active voice, concise sentences, and direct second-person phrasing
while preserving the existing behavior and details. Apply the same style to the
lease-reset explanation in advanced/signal-protocol.mdx lines 1303-1312: split
long sentences and address the reader directly; no code changes are required.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b928da98-5291-4d7c-a562-9a1d8291acf0
📒 Files selected for processing (4)
advanced/metrics.mdxadvanced/signal-protocol.mdxapi/signal.mdxconcepts/storage.mdx
![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: 4f838ed2a6
ℹ️ 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".
- Scope the "Reset a broken session" example to logically-invalid sessions instead of "corrupted" ones, since undecodable rows no longer need it (CodeRabbit). - Correct the self-heal claim: a plain load only reports a row absent, it doesn't repair it — recovery happens on the next send or decrypt that establishes a replacement session (Codex). - Note that recovery overwrites the unreadable row, which matters during a version rollback to a build that could still decode it (Codex). - Split the dense quarantine paragraph into bullets and rewrite both new sections in active, second-person voice per AGENTS.md (CodeRabbit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ECfouxpzsKhJ4heK8MUDk
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53598eaad2
ℹ️ 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".
|
|
||
| - `get_session`, `checkout_session`, and `has_session` all report that row as **absent** instead of propagating a decode error. Loading the row doesn't repair it by itself — it only lets the caller treat the address as session-less. | ||
| - That matters because the paths that would otherwise repair the session — decrypting the peer's next pre-key message, the retry-receipt handler — have to load the record first. If the decode error propagated instead, it would strand the address until you deleted the row by hand. | ||
| - Reporting the row absent lets the ordinary no-session recovery run instead: the next send or decrypt for that address fetches a fresh pre-key bundle and persists a replacement session, overwriting the unreadable row. This build never derives key material from bytes it can't decode, so it loses nothing usable — but the overwrite is destructive to the original bytes. If you're rolling back to a build that could still decode that row, back up the database first; the original bytes don't survive the overwrite. |
There was a problem hiding this comment.
When the next use is an inbound ciphertext, decryption does not fetch a fresh peer pre-key bundle: an ordinary message without a usable session triggers a retry, and only a later pre-key message can establish the replacement session. The revised wording correctly says that loading alone does not repair the row, but this sentence still promises that the next decrypt both fetches a bundle and recovers automatically; distinguish the outbound send's server-side bundle fetch from the inbound retry/pre-key-message flow.
Useful? React with 👍 / 👎.
| | `wa_appstate_mutations_total` | — | App-state mutations applied | | ||
| | `wa_identity_change_total` | — | Peer identity changes that triggered a session reset | | ||
| | `wa_prekey_upload_total` | `outcome` = `ok`, `fail` | Pre-key uploads by outcome | | ||
| | `wa_session_record_quarantined_total` | — | Stored session rows that failed to decode and were treated as absent for recovery. Steady state is zero — see [session row quarantine](/concepts/storage#signalstorecache) | |
There was a problem hiding this comment.
Because this is a monotonic _total counter, its value remains non-zero after the first quarantined row and does not return to zero during a healthy steady state. Saying that steady state is zero can lead operators to alert on the cumulative value and keep an alert firing indefinitely; document that the expected steady-state increase or rate is zero and that a positive rate is the signal to investigate.
Useful? React with 👍 / 👎.
2 participants
Documents whatsapp-rust#1149, "fix(signal): stop a DH ratchet stranding the counter lease".
What changed upstream
A DH ratchet replaces a session's sender chain in place (fresh key material, counter reset to zero) without archiving the retired chain. The record-level sender-chain counter lease wasn't rebased when this happened, so the ceiling could keep describing a chain that no longer existed. For a peer you mostly monologue at (e.g. your own other device, which receives a copy of every outgoing message but rarely replies), this could strand the ceiling thousands of counters above the live chain — and on the next restart/lossy cache reset, recovery would refuse to fast-forward past
MAX_RESERVATION_FAST_FORWARDand fail to load the session record at all, permanently stranding that address (breaking decrypt, group-send fan-out, and retry-receipt handling for it).The fix has two parts:
SessionRecord::rebase_lease_after_sender_chain_reset()lowers the lease ceiling to at most one reservation batch as part of the same mutation that swaps in the fresh chain, preventing new occurrences.SignalStoreCachenow reports an undecodable/corrupted session row as absent rather than surfacing a load error, letting the ordinary no-session recovery path replace it automatically. This self-heals rows that were already stranded by builds predating fix 1. A newwa_session_record_quarantined_totalcounter tracks this.Doc changes
advanced/signal-protocol.mdx— new "DH ratchet resets rebase the lease" subsection under Flush scheduling, explaining the bug and the rebase fix, includingMAX_RESERVATION_FAST_FORWARD(previously undocumented).concepts/storage.mdx— new paragraph underSignalStoreCachedocumenting the undecodable-row quarantine behavior and thehas_session()semantics change (it now decodes rather than only checking existence).advanced/metrics.mdx— addedwa_session_record_quarantined_totalto the counters table.api/signal.mdx— added a note to "Reset a broken session" clarifying that an undecodable row no longer needs manualdelete_sessions/assert_sessionsintervention.No changelog entries were added, per standing instructions that changelog entries are human-authored only.
Generated by Claude Code
Summary by cubic
Document DH ratchet resets rebasing the sender-chain lease and the new quarantine behavior for undecodable session rows.
Adds the
wa_session_record_quarantined_totalmetric, clarifies thatSignalStoreCachetreats undecodable rows as absent and thathas_sessiondecodes, notes that recovery happens on the next send or decrypt and overwrites the unreadable row, and scopes the “Reset a broken session” example to logically invalid sessions.Written for commit 53598ea. Summary will update on new commits.
Summary by CodeRabbit