fix(android): E2EE review follow-ups by santhoshvai · Pull Request #73 · GetStream/react-native-webrtc

santhoshvai · 2026-09-28T12:18:31Z

Follow-ups to the review of #68.

getTrackById() now returns the track wrapper the observer keeps. Before, it used pc.getReceivers(), which disposes the wrappers from its previous call and detaches their sinks. That broke the recorder when E2EE decrypt() or receiver.getStats() ran on the same peer connection. Android only.
E2EE event forwarding on Android now catches exceptions and drops the event, so a bad event cannot crash the app.
The two "dispose E2EE managers first" comments no longer read as an ordering requirement. Dispose order does not matter for safety.
encrypt() docs say to attach before the sender has a track or is negotiated to send.

Tested on dogfood: loopback recording keeps video while receivers are enumerated mid-recording, and E2EE meeting join, leave, rejoin, wrong-key, and bundle reload all pass on both platforms.

🎫 Ticket: https://linear.app/stream/issue/RN-434

Summary by CodeRabbit

Bug Fixes
- Prevented failures while processing encryption events from disrupting event delivery; events that cannot be prepared are dropped safely.
- Improved remote track lookup consistency.
Documentation
- Clarified that encryption must be attached before a sender has a track or is negotiated to send; frames sent beforehand are unencrypted.
- Clarified encryption manager cleanup behavior during app reloads.

getTrackById() looked up remote tracks by enumerating pc.getReceivers() and returning receiver.track(). On Android, PeerConnection.getReceivers() disposes every RtpReceiver wrapper from its previous call before creating new ones, and disposing a receiver disposes its cached VideoTrack, which detaches all of its sinks. So any later getReceivers() call on the same PeerConnection (E2EE decrypt() attaching to another receiver, getStats on a receiver, or getTrackById() itself) silently stopped frames reaching a recorder attached to the returned track. Return the stable wrapper kept in PeerConnectionObserver.remoteTracks instead. It is the same object RTCView renders through and getTrack() already reads, and getReceivers() never touches it.

An exception while converting an encryptionManagerEvent on the crypto worker, or while emitting it on the executor, now gets logged and the event dropped instead of propagating. Keys and transforms are unaffected.

The comments in Android invalidate() and iOS dealloc read as if disposing the E2EE managers before the peer connections were needed for correctness. They are disposed there because JS is gone and nothing else can release them; the order does not matter for safety.

Note in the encrypt() JSDoc that the transform should be attached before the sender has a track or is negotiated to send, since frames sent before attachment go out unencrypted.

coderabbitai · 2026-09-28T12:18:49Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d47803d9-8af3-4db1-87bf-8d47be8fde23

📥 Commits

Reviewing files that changed from the base of the PR and between ae3a128 and bf9781e.

📒 Files selected for processing (4)

android/src/main/java/com/oney/WebRTCModule/EncryptionManagerBridge.java
android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java
ios/RCTWebRTC/WebRTCModule.m
src/RTCEncryptionManager.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

Android event delivery now catches runtime failures, and getTrackById() searches cached remote tracks. Android and iOS lifecycle comments and TypeScript encryption attachment documentation are also updated.

Changes

Native module updates

Layer / File(s)	Summary
Contain E2EE event delivery failures `android/src/main/java/com/oney/WebRTCModule/EncryptionManagerBridge.java`	Event parameter population failures now cause the event to be dropped and logged. Failures from `sendEvent` are caught and logged in the dispatched task.
Search cached remote tracks `android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`	`getTrackById()` searches observers’ `remoteTracks` maps instead of scanning peer connection receiver tracks.
Update E2EE lifecycle and attachment notes `android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`, `ios/RCTWebRTC/WebRTCModule.m`, `src/RTCEncryptionManager.ts`	Lifecycle comments state that JavaScript cannot release E2EE managers after reload and that disposal order relative to peer connections does not affect safety. Encryption documentation says when to call `encrypt` and warns that frames sent before attachment are unencrypted.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: greenfrvr

Merge Risk: ⚪ Minimal · up to bf978

No merge-blocking issue is established. Complete the normal Android and iOS build checks before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to bf978

The changes appear to preserve encryption behavior while fixing a track-lifetime problem. No new security vulnerability was established, but the consequences of a dropped encryption notification are not fully documented.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The evidenced effects are confined to Android peer-connection track lookup and native-to-JavaScript E2EE notifications; no new cross-service or deployment dependency is identified.

Trust Boundaries and Controls

observed — Forwarding still uses the WebRTC module's emitter, which requires an active React instance; delivered events are then filtered by manager ID in the TypeScript receiver. The new catches do not bypass those controls.

Resilience and Maintainability Implications

inferred — Logging instead of delivering an exceptional E2EE notification permits the application to continue without that notification. The available source does not establish attacker control of the exception or whether an application uses the notification as a security-enforcement signal.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies Android E2EE fixes that follow up on review feedback. It accurately summarizes the main changes, including track lookup, event handling, teardown comments, and documentati…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

## [145.4.1](v145.4.0...v145.4.1) (2026-09-28) ### Bug Fixes * **android:** E2EE review follow-ups ([#73](#73)) ([bb6a629](bb6a629))

github-actions · 2026-09-28T12:30:28Z

🎉 This PR is included in version 145.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

santhoshvai added 4 commits September 28, 2026 13:31

fix(android): contain exceptions in E2EE event forwarding

4b8d346

An exception while converting an encryptionManagerEvent on the crypto worker, or while emitting it on the executor, now gets logged and the event dropped instead of propagating. Keys and transforms are unaffected.

docs: document encrypt() attachment timing

bf9781e

Note in the encrypt() JSDoc that the transform should be attached before the sender has a track or is negotiated to send, since frames sent before attachment go out unencrypted.

santhoshvai requested a review from oliverlaz September 28, 2026 12:19

oliverlaz approved these changes Sep 28, 2026

View reviewed changes

santhoshvai merged commit bb6a629 into master Sep 28, 2026
6 checks passed

santhoshvai deleted the e2ee-review branch September 28, 2026 12:28

github-actions Bot pushed a commit that referenced this pull request Sep 28, 2026

chore(release): 145.4.1 [skip ci]

8ce1708

## [145.4.1](v145.4.0...v145.4.1) (2026-09-28) ### Bug Fixes * **android:** E2EE review follow-ups ([#73](#73)) ([bb6a629](bb6a629))

github-actions Bot added the released label Sep 28, 2026

Navigation Menu

fix(android): E2EE review follow-ups - #73

fix(android): E2EE review follow-ups#73
santhoshvai merged 4 commits into
masterfrom
e2ee-review

santhoshvai commented Sep 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 28, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

github-actions Bot commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

santhoshvai commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

github-actions Bot commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

santhoshvai commented Sep 28, 2026 •

edited

Loading

coderabbitai Bot commented Sep 28, 2026 •

edited

Loading