feat(tctoken): attach and issue tctoken on outgoing VoIP call offers - #970
Conversation
Outgoing 1:1 call offers now carry the callee's stored trusted-contact token as the offer's <privacy> node and issue a fresh token after send, mirroring WhatsApp Web's sendTcToken in StartCall.js. Prevents 463 nacks on call offers to privacy-restricted contacts. Issuance is fire-and-forget and rate-limited by the sender bucket, so it never blocks call setup.
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds trusted-contact token support to outgoing VOIP call offers: ChangesVOIP trusted-contact token flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 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/voip/facade.rs | Attaches stored tctoken to outgoing call offer and spawns fire-and-forget post-send issuance; mirrors the relay-waiter spawn pattern correctly. |
| src/send/tctoken_lifecycle.rs | Extracts is_own_jid helper, widens issue_tc_token_after_send to pub(crate), adds voip-gated should_issue_tc_token with three covering tests. |
| src/features/tctoken.rs | Doc-comment-only change updating the TODO note to reflect the wired 1:1 VoIP path. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as place_call
participant DB as persistence_manager
participant Net as send_node
participant Spawn as spawn_call_tc_token_issuance
participant IQ as issue_tc_token_after_send
Caller->>DB: lookup_tc_token_for_jid(peer)
DB-->>Caller: Option privacy_token
Note over Caller: Build offer with privacy_token
Caller->>Net: send_node(offer)
Net-->>Caller: Ok
Caller->>Spawn: spawn detached
activate Spawn
Spawn->>DB: should_issue_tc_token(peer)
DB-->>Spawn: bool
alt should issue
Spawn->>IQ: issue_tc_token_after_send(peer)
IQ->>DB: record_tc_token_sender_timestamp
end
deactivate Spawn
Caller-->>Caller: return CallHandle
%%{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 Caller as place_call
participant DB as persistence_manager
participant Net as send_node
participant Spawn as spawn_call_tc_token_issuance
participant IQ as issue_tc_token_after_send
Caller->>DB: lookup_tc_token_for_jid(peer)
DB-->>Caller: Option privacy_token
Note over Caller: Build offer with privacy_token
Caller->>Net: send_node(offer)
Net-->>Caller: Ok
Caller->>Spawn: spawn detached
activate Spawn
Spawn->>DB: should_issue_tc_token(peer)
DB-->>Spawn: bool
alt should issue
Spawn->>IQ: issue_tc_token_after_send(peer)
IQ->>DB: record_tc_token_sender_timestamp
end
deactivate Spawn
Caller-->>Caller: return CallHandle
Reviews (4): Last reviewed commit: "refactor(tctoken): extract shared is_own..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Follows AGENTS.md: comments explain why, not what. Removes narration of lookup_tc_token_for_jid / spawn_call_tc_token_issuance at the call sites, the codebase-trivia paragraph on should_issue_tc_token, and the verbose test-block comment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/send/tctoken_lifecycle.rs`:
- Around line 147-174: The VoIP tc-token issuance helper still allows self-calls
to proceed, so add the same self-JID guard used elsewhere before the bot/status
checks in should_issue_tc_token. Update should_issue_tc_token in
tctoken_lifecycle to resolve the peer and return false when it matches our own
PN/LID identity, so a call to ourselves never issues or records a tc token.
🪄 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 (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59c006c7-c55a-4ff3-ad19-cd513c75a71c
📒 Files selected for processing (3)
src/features/tctoken.rssrc/send/tctoken_lifecycle.rssrc/voip/facade.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Changes add token attachment and issuance to call flow, affecting security and privacy. Requires human review to ensure correct token handling and no regression in call setup.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Confidence score: 4/5
- In
src/send/tctoken_lifecycle.rs,should_issue_tc_tokenappears to miss a self-JID check on the outgoing-call path, so a self-resolvedpeercould trigger token issuance/recording for our own account; this could create incorrect token state or noisy background tasks if merged as-is — add an explicit self-JID guard before spawning the issuance task.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
should_issue_tc_token now excludes our own PN/LID before the bot/status check, matching maybe_include_tc_token, so a self-call never issues or records a token for our own account.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/send/tctoken_lifecycle.rs (1)
155-169: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winGood, the self-call hole is patched. We flagged this before and it's fixed now — no more issuing tokens to ourselves. That's the kind of correctness bar we need on privacy-sensitive paths.
One thing though: this exact block is now copy-pasted from
maybe_include_tc_token(lines 30-40). We don't duplicate security-relevant logic like this at scale — if someone tweaks the self-check in one spot and forgets the other, we're back to square one. Pull it into a small private helper both call.♻️ Proposed refactor
+ fn is_own_jid(&self, jid: &Jid) -> bool { + let snapshot = self.persistence_manager.get_device_snapshot(); + snapshot.pn.as_ref().is_some_and(|pn| pn.is_same_user_as(jid)) + || snapshot.lid.as_ref().is_some_and(|lid| lid.is_same_user_as(jid)) + } + pub(super) async fn maybe_include_tc_token( &self, to: &Jid, extra_nodes: &mut Vec<Node>, ) -> bool { ... - let snapshot = self.persistence_manager.get_device_snapshot(); - let is_self = snapshot - .pn - .as_ref() - .is_some_and(|pn| pn.is_same_user_as(to)) - || snapshot - .lid - .as_ref() - .is_some_and(|lid| lid.is_same_user_as(to)); - if is_self { + if self.is_own_jid(to) { return false; }pub(crate) async fn should_issue_tc_token(&self, to: &Jid) -> bool { use wacore::iq::tctoken::should_send_new_tc_token_with; - let snapshot = self.persistence_manager.get_device_snapshot(); - let is_self = snapshot - .pn - .as_ref() - .is_some_and(|pn| pn.is_same_user_as(to)) - || snapshot - .lid - .as_ref() - .is_some_and(|lid| lid.is_same_user_as(to)); - if is_self { + if self.is_own_jid(to) { return false; }🤖 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/send/tctoken_lifecycle.rs` around lines 155 - 169, The self-call guard logic in tctoken_lifecycle is duplicated between the token issuance path and maybe_include_tc_token, so extract the shared self-check into a small private helper and have both call it. Update the existing boolean logic around self detection using persistence_manager.get_device_snapshot, pn/lid, and is_same_user_as so there is one authoritative implementation, then replace both inline copies with that helper.
🤖 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.
Duplicate comments:
In `@src/send/tctoken_lifecycle.rs`:
- Around line 155-169: The self-call guard logic in tctoken_lifecycle is
duplicated between the token issuance path and maybe_include_tc_token, so
extract the shared self-check into a small private helper and have both call it.
Update the existing boolean logic around self detection using
persistence_manager.get_device_snapshot, pn/lid, and is_same_user_as so there is
one authoritative implementation, then replace both inline copies with that
helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e7d4b10e-af80-44b3-8ba2-e1e56a06dbcf
📒 Files selected for processing (1)
src/send/tctoken_lifecycle.rs
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Adds trusted-contact token issuance to VoIP call path, a business logic change in core communication features with production infrastructure impact. Requires human review for correctness and security.
Re-trigger cubic
📦 Binary size report
.text per crate
Baseline: |
The self-JID guard was duplicated between maybe_include_tc_token and should_issue_tc_token. Extract it into a single is_own_jid helper so the message and call paths can't drift apart.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Adds trusted-contact token logic to outgoing VoIP calls, modifying call setup and token issuance. These are business logic changes in a core path that require human review.
Re-trigger cubic
2 participants
Summary
Closes the last documented gap in the trusted-contact token sweep: outgoing VoIP call offers. WhatsApp Web attaches the callee's trusted-contact token to the call offer and issues a fresh token to the callee when starting a call (
sendTcTokeninStartCall.js); the offer builder already had a<privacy>slot butplace_callwas passingNone, and no issuance ran on the call path.Changes
voip::facade::place_call— attaches the callee's stored (non-expired) tctoken to the offer's leading<privacy>node vialookup_tc_token_for_jid, the same token the 1:1 message and presence-subscribe paths already attach. Absent when we hold no valid token.spawn_call_tc_token_issuance), rate-limited by the sender bucket so repeat calls to the same contact within a window don't re-issue. It's detached, so it never blocks call setup on the IQ round-trip. This prevents 463 nacks on later offers to privacy-restricted contacts.should_issue_tc_token— newClienthelper (voip-gated) exposing just the sender-bucket issuance decision the message path inlines inmaybe_include_tc_token.issue_tc_token_after_sendwidened frompub(super)topub(crate)so the call path can reuse it.TODO: VoIP call integrationnote infeatures/tctoken.rsis updated to reflect the wired 1:1 path; group-call initiation isn't implemented yet and should attach/issue per participant the same way when added.Scope
place_callis the only outgoing offer path (1:1); group calls are inbound-only (offer_noticeparsing) today, so there is no group initiation site to wire.Testing
should_issue_tc_tokenreturnstruefor an unknown contact andfalsewithin the current sender bucket.place_call_attaches_stored_tctoken_as_privacy_nodeseeds a token and asserts it rides as the offer's leading<privacy>child; the existing no-token offer test still asserts no<privacy>node.cargo fmt,cargo clippy --features voip, default and--features voipbuilds all clean; fullvoip::facadesuite (30 tests) green.Generated by Claude Code