feat(client): add E2EE support via WebRTC Encoded Transforms#2198
Open
feat(client): add E2EE support via WebRTC Encoded Transforms#2198
Conversation
Add end-to-end encryption for media tracks using a symmetric XOR transform applied to every encoded frame in a dedicated Web Worker. Uses RTCRtpScriptTransform (W3C standard) on browsers that support it (Safari, Firefox) and falls back to Insertable Streams (createEncodedStreams) on Chrome where RTCRtpScriptTransform is unreliable.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces end-to-end encryption (E2EE) support for WebRTC media streams using encoded transforms. The implementation adds type definitions, encryption key configuration, publisher/subscriber encryption setup, E2EE compatibility detection, and a worker-based frame transform system with codec-specific handling. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Pub as Publisher
participant E2EE as E2EE Module
participant Worker as E2EE Worker
participant RTC as RTCPeerConnection
participant Net as Network
App->>Pub: addTransceiver(track, { encryptionKey })
Pub->>E2EE: Check supportsE2EE()
E2EE-->>Pub: true
Pub->>E2EE: Import & createEncryptor(sender, key)
E2EE->>E2EE: Check shouldUseInsertableStreams()
E2EE->>RTC: Get createEncodedStreams()
RTC-->>E2EE: readable, writable streams
E2EE->>Worker: postMessage({ operation: 'encode', readable, writable, key })
Worker->>Worker: Setup encode transform
Worker-->>E2EE: Ready
Pub->>RTC: Attach transceiver with sender
RTC->>RTC: Media frames flow to sender
RTC->>Worker: Send encoded frames
Worker->>Worker: Encrypt frames (codec-aware)
Worker->>Net: Send encrypted frames
sequenceDiagram
participant Net as Network
participant Worker as E2EE Worker
participant Sub as Subscriber
participant E2EE as E2EE Module
participant RTC as RTCPeerConnection
participant App as Application
Net->>RTC: Receive encrypted frames
RTC->>Sub: handleOnTrack(TrackEvent)
Sub->>E2EE: Check supportsE2EE()
E2EE-->>Sub: true
Sub->>E2EE: Import & createDecryptor(receiver, key)
E2EE->>RTC: Get createEncodedStreams()
RTC-->>E2EE: readable, writable streams
E2EE->>Worker: postMessage({ operation: 'decode', readable, writable, key })
Worker->>Worker: Setup decode transform
Worker-->>E2EE: Ready
Sub->>RTC: Attach receiver with transform
RTC->>Worker: Send encrypted frames
Worker->>Worker: Validate checksum & decrypt
Worker->>RTC: Send decrypted frames
RTC->>App: Emit decrypted MediaStreamTrack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/client/src/rtc/Publisher.ts (1)
141-149: Consider wrappingcreateEncryptorin a try-catch.If
createEncryptorthrows (e.g., Worker instantiation failure, invalid key), the entireaddTransceiverflow will fail and prevent track publishing. A defensive try-catch with a warning log would allow graceful degradation.🛡️ Proposed defensive handling
const { encryptionKey } = this.clientPublishOptions || {}; if (encryptionKey) { if (supportsE2EE()) { - createEncryptor(transceiver.sender, encryptionKey); - this.logger.debug('E2EE encryptor attached to sender'); + try { + createEncryptor(transceiver.sender, encryptionKey); + this.logger.debug('E2EE encryptor attached to sender'); + } catch (err) { + this.logger.warn('Failed to attach E2EE encryptor', err); + } } else { this.logger.warn(`E2EE requested but not supported`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/Publisher.ts` around lines 141 - 149, Wrap the call to createEncryptor(transceiver.sender, encryptionKey) inside a try-catch so that exceptions from encryptor creation (e.g., Worker instantiation or invalid key) do not abort the addTransceiver/publish flow; on catch, log a warning via this.logger.warn including the error and continue without attaching the encryptor, leaving behavior unchanged when supportsE2EE() is false, and keep the debug log only on successful creation.packages/client/src/rtc/Subscriber.ts (1)
97-105: Consider wrappingcreateDecryptorin a try-catch for consistency with Publisher.Same reasoning as Publisher: if
createDecryptorthrows, the track handling will fail entirely. A defensive try-catch would allow the track to still be processed (unencrypted) rather than dropping it.🛡️ Proposed defensive handling
const { encryptionKey } = this.clientPublishOptions || {}; if (encryptionKey) { if (supportsE2EE()) { - createDecryptor(e.receiver, encryptionKey); - this.logger.debug('E2EE decryptor attached to receiver'); + try { + createDecryptor(e.receiver, encryptionKey); + this.logger.debug('E2EE decryptor attached to receiver'); + } catch (err) { + this.logger.warn('Failed to attach E2EE decryptor', err); + } } else { this.logger.warn(`E2EE requested but not supported`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/Subscriber.ts` around lines 97 - 105, The createDecryptor invocation in Subscriber (using createDecryptor(e.receiver, encryptionKey) guarded by supportsE2EE()) should be wrapped in a try-catch like Publisher to avoid losing the entire track if decryption initialization throws; update the block where this.clientPublishOptions?.encryptionKey is checked to call createDecryptor inside a try, log a warning or debug on failure via this.logger (e.g., this.logger.warn('E2EE decryptor failed to attach', error)), and continue processing the track if an exception occurs so unencrypted handling still proceeds.sample-apps/react/react-dogfood/lib/queryConfigParams.ts (1)
19-19: Security note: encryption key in URL query string.Exposing the encryption key via
?encryption_key=makes it visible in browser history, server logs, and referrer headers. This is acceptable for a dogfood/testing app, but ensure this pattern isn't replicated in production integrations where the key should be exchanged out-of-band.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/react-dogfood/lib/queryConfigParams.ts` at line 19, The code currently reads the encryption key from the URL query (encryptionKey: query['encryption_key']) which exposes secrets; change this to not pull sensitive keys from the query string — remove or default encryptionKey to undefined in queryConfigParams.ts and instead accept the key via a secure channel (e.g., an explicit config parameter passed from server-side code or process.env when in safe dev/dogfood only); if you must keep a dev-only shortcut, gate it behind a strict environment check (e.g., NODE_ENV === 'development' or a DOGFOOD flag) and document that production must supply keys out-of-band.packages/client/src/rtc/e2ee/index.ts (2)
31-63: Add error handling for pipe operations and consider key encoding.Two concerns with the worker implementation:
Missing error handling:
pipeTo()returns a Promise that can reject on stream errors. Silent failures during encryption/decryption would be difficult to debug.Key encoding:
charCodeAt()returns values > 255 for non-ASCII characters, whichsetInt8truncates. While this works symmetrically, it's worth documenting that keys should be ASCII-only, or encoding the key to bytes explicitly.♻️ Suggested improvement for error handling
function handleTransform({ readable, writable, key }) { - readable.pipeThrough(xorTransform(key)).pipeTo(writable); + readable.pipeThrough(xorTransform(key)).pipeTo(writable).catch((err) => { + console.error('[stream-video-e2ee] transform pipeline error:', err); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/e2ee/index.ts` around lines 31 - 63, The worker lacks stream error handling and uses charCodeAt which mishandles non-ASCII key bytes; update xorTransform to explicitly encode the key to a byte array (e.g., via TextEncoder) and use unsigned byte access (getUint8/setUint8 or Uint8Array) so key bytes are 0-255, and in handleTransform where readable.pipeTo(writable) is used (and in the onrtctransform/onmessage dispatch paths) attach a rejection handler (pipeTo(...).catch(err => { /* report via postMessage or console.error */ })) or await and try/catch to surface errors back to the main thread for debugging; reference functions: xorTransform, handleTransform, the self.onrtctransform handler, and self.onmessage handler.
83-99: Theoperationparameter is passed but unused in the worker.The
operationvalue ('encode'/'decode') is included in the message but the worker'shandleTransformfunction ignores it since XOR is symmetric. Consider either:
- Removing it to avoid confusion
- Adding a comment explaining it's reserved for future asymmetric transforms
This is cosmetic since the current implementation works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/e2ee/index.ts` around lines 83 - 99, The worker message includes an unused 'operation' field which is confusing; update attachTransform (and its getWorker/w.postMessage call) to stop sending the operation value and remove it from the function signature, and also remove or stop depending on operation elsewhere; alternatively, if you prefer to keep the API, add a concise comment in attachTransform and in the worker's handleTransform noting that XOR is symmetric so 'operation' is currently unused and reserved for potential future asymmetric transforms (reference attachTransform, w.postMessage, and handleTransform).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/client/src/rtc/e2ee/index.ts`:
- Around line 94-95: The current code initializes piped with a Set (if ((piped
??= new Set()).has(target) return; piped.add(target);) but it should be a
WeakSet to avoid memory leaks for object targets; change the initializer from
new Set() to new WeakSet() and update the piped variable's declared type (where
it's defined) to WeakSet<typeof target> (or WeakSet<object>) so has/add calls on
piped remain valid.
- Around line 65-68: Change piped from a Set to a WeakSet to avoid retaining
strong references to RTCRtpSender/RTCRtpReceiver (declare piped as
WeakSet<RTCRtpSender | RTCRtpReceiver> and initialize it accordingly), and add
explicit cleanup to release worker resources by terminating/disposing the Worker
and clearing worker and workerUrl when the associated Call/Publisher/Subscriber
is disposed; specifically, in the code paths for Call.dispose, Publisher.dispose
and Subscriber.dispose (or any e2ee teardown function), call worker.terminate()
or the worker's disposal method if available, then set worker = undefined and
workerUrl = undefined and set piped = undefined to allow GC.
---
Nitpick comments:
In `@packages/client/src/rtc/e2ee/index.ts`:
- Around line 31-63: The worker lacks stream error handling and uses charCodeAt
which mishandles non-ASCII key bytes; update xorTransform to explicitly encode
the key to a byte array (e.g., via TextEncoder) and use unsigned byte access
(getUint8/setUint8 or Uint8Array) so key bytes are 0-255, and in handleTransform
where readable.pipeTo(writable) is used (and in the onrtctransform/onmessage
dispatch paths) attach a rejection handler (pipeTo(...).catch(err => { /* report
via postMessage or console.error */ })) or await and try/catch to surface errors
back to the main thread for debugging; reference functions: xorTransform,
handleTransform, the self.onrtctransform handler, and self.onmessage handler.
- Around line 83-99: The worker message includes an unused 'operation' field
which is confusing; update attachTransform (and its getWorker/w.postMessage
call) to stop sending the operation value and remove it from the function
signature, and also remove or stop depending on operation elsewhere;
alternatively, if you prefer to keep the API, add a concise comment in
attachTransform and in the worker's handleTransform noting that XOR is symmetric
so 'operation' is currently unused and reserved for potential future asymmetric
transforms (reference attachTransform, w.postMessage, and handleTransform).
In `@packages/client/src/rtc/Publisher.ts`:
- Around line 141-149: Wrap the call to createEncryptor(transceiver.sender,
encryptionKey) inside a try-catch so that exceptions from encryptor creation
(e.g., Worker instantiation or invalid key) do not abort the
addTransceiver/publish flow; on catch, log a warning via this.logger.warn
including the error and continue without attaching the encryptor, leaving
behavior unchanged when supportsE2EE() is false, and keep the debug log only on
successful creation.
In `@packages/client/src/rtc/Subscriber.ts`:
- Around line 97-105: The createDecryptor invocation in Subscriber (using
createDecryptor(e.receiver, encryptionKey) guarded by supportsE2EE()) should be
wrapped in a try-catch like Publisher to avoid losing the entire track if
decryption initialization throws; update the block where
this.clientPublishOptions?.encryptionKey is checked to call createDecryptor
inside a try, log a warning or debug on failure via this.logger (e.g.,
this.logger.warn('E2EE decryptor failed to attach', error)), and continue
processing the track if an exception occurs so unencrypted handling still
proceeds.
In `@sample-apps/react/react-dogfood/lib/queryConfigParams.ts`:
- Line 19: The code currently reads the encryption key from the URL query
(encryptionKey: query['encryption_key']) which exposes secrets; change this to
not pull sensitive keys from the query string — remove or default encryptionKey
to undefined in queryConfigParams.ts and instead accept the key via a secure
channel (e.g., an explicit config parameter passed from server-side code or
process.env when in safe dev/dogfood only); if you must keep a dev-only
shortcut, gate it behind a strict environment check (e.g., NODE_ENV ===
'development' or a DOGFOOD flag) and document that production must supply keys
out-of-band.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34706f24-48cc-4daa-a7dc-7f51973b58e7
📒 Files selected for processing (9)
packages/client/src/rtc/BasePeerConnection.tspackages/client/src/rtc/Publisher.tspackages/client/src/rtc/Subscriber.tspackages/client/src/rtc/__tests__/Publisher.test.tspackages/client/src/rtc/__tests__/Subscriber.test.tspackages/client/src/rtc/__tests__/mocks/webrtc.mocks.tspackages/client/src/rtc/e2ee/index.tspackages/client/src/types.tssample-apps/react/react-dogfood/lib/queryConfigParams.ts
Comment on lines
+65
to
+68
| /** Tracks senders/receivers that already have encoded streams piped. */ | ||
| let piped: Set<RTCRtpSender | RTCRtpReceiver> | undefined; | ||
| let worker: Worker | undefined; | ||
| let workerUrl: string | undefined; |
Contributor
There was a problem hiding this comment.
Use WeakSet instead of Set to prevent memory leaks.
The piped variable holds strong references to RTCRtpSender/RTCRtpReceiver objects. These won't be garbage collected even after the peer connection is closed, causing a memory leak that grows with each call.
Additionally, there's no disposal mechanism for worker and workerUrl, which violates the coding guideline: "Always call dispose() on Call, Publisher, Subscriber when done for proper memory management."
🐛 Proposed fix using WeakSet and adding disposal
/** Tracks senders/receivers that already have encoded streams piped. */
-let piped: Set<RTCRtpSender | RTCRtpReceiver> | undefined;
+let piped: WeakSet<RTCRtpSender | RTCRtpReceiver> | undefined;
let worker: Worker | undefined;
let workerUrl: string | undefined;
+
+/**
+ * Disposes E2EE resources. Call when encryption is no longer needed.
+ */
+export const disposeE2EE = () => {
+ if (worker) {
+ worker.terminate();
+ worker = undefined;
+ }
+ if (workerUrl) {
+ URL.revokeObjectURL(workerUrl);
+ workerUrl = undefined;
+ }
+ piped = undefined;
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** Tracks senders/receivers that already have encoded streams piped. */ | |
| let piped: Set<RTCRtpSender | RTCRtpReceiver> | undefined; | |
| let worker: Worker | undefined; | |
| let workerUrl: string | undefined; | |
| /** Tracks senders/receivers that already have encoded streams piped. */ | |
| let piped: WeakSet<RTCRtpSender | RTCRtpReceiver> | undefined; | |
| let worker: Worker | undefined; | |
| let workerUrl: string | undefined; | |
| /** | |
| * Disposes E2EE resources. Call when encryption is no longer needed. | |
| */ | |
| export const disposeE2EE = () => { | |
| if (worker) { | |
| worker.terminate(); | |
| worker = undefined; | |
| } | |
| if (workerUrl) { | |
| URL.revokeObjectURL(workerUrl); | |
| workerUrl = undefined; | |
| } | |
| piped = undefined; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/e2ee/index.ts` around lines 65 - 68, Change piped
from a Set to a WeakSet to avoid retaining strong references to
RTCRtpSender/RTCRtpReceiver (declare piped as WeakSet<RTCRtpSender |
RTCRtpReceiver> and initialize it accordingly), and add explicit cleanup to
release worker resources by terminating/disposing the Worker and clearing worker
and workerUrl when the associated Call/Publisher/Subscriber is disposed;
specifically, in the code paths for Call.dispose, Publisher.dispose and
Subscriber.dispose (or any e2ee teardown function), call worker.terminate() or
the worker's disposal method if available, then set worker = undefined and
workerUrl = undefined and set piped = undefined to allow GC.
Comment on lines
+94
to
+95
| if ((piped ??= new Set()).has(target)) return; | ||
| piped.add(target); |
Contributor
There was a problem hiding this comment.
Update to WeakSet here as well.
Per the fix suggested above, this line should create a WeakSet instead of Set.
🐛 Proposed fix
- if ((piped ??= new Set()).has(target)) return;
+ if ((piped ??= new WeakSet()).has(target)) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ((piped ??= new Set()).has(target)) return; | |
| piped.add(target); | |
| if ((piped ??= new WeakSet()).has(target)) return; | |
| piped.add(target); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/e2ee/index.ts` around lines 94 - 95, The current code
initializes piped with a Set (if ((piped ??= new Set()).has(target) return;
piped.add(target);) but it should be a WeakSet to avoid memory leaks for object
targets; change the initializer from new Set() to new WeakSet() and update the
piped variable's declared type (where it's defined) to WeakSet<typeof target>
(or WeakSet<object>) so has/add calls on piped remain valid.
Split the monolithic e2ee/index.ts into three files: - compatibility.ts: supportsE2EE() stays in the main bundle - worker.ts: worker source string + lifecycle management - e2ee.ts: createEncryptor/createDecryptor API Publisher and Subscriber now dynamically import the heavy E2EE code, so consumers who don't use E2EE pay zero bundle cost. The browser build emits a separate e2ee chunk; node builds inline it.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/client/src/rtc/e2ee/e2ee.ts (1)
7-11: Consider broadening the Chromium detection comment.The
navigator.userAgent?.includes('Chrome')check will match all Chromium-based browsers (Edge, Opera, Brave, etc.), not just Chrome. This is likely intentional since they share the same WebRTC implementation quirks, but a clarifying comment would help future maintainers.📝 Suggested documentation improvement
/** - * Chrome exposes RTCRtpScriptTransform, but it doesn't seem to work reliably. - * Use Insertable Streams (createEncodedStreams) there instead. + * Chromium-based browsers (Chrome, Edge, Opera, etc.) expose RTCRtpScriptTransform, + * but it doesn't work reliably. Use Insertable Streams (createEncodedStreams) instead. + * The "Chrome" UA substring check intentionally matches all Chromium derivatives. */ const shouldUseInsertableStreams = (): boolean =>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/e2ee/e2ee.ts` around lines 7 - 11, The userAgent check in shouldUseInsertableStreams() currently uses navigator.userAgent?.includes('Chrome') which also matches other Chromium-based browsers (Edge, Brave, Opera); update the code by adding a clarifying comment above shouldUseInsertableStreams() stating that this check intentionally targets Chromium-based browsers (not only Google Chrome) because they share the same WebRTC/RTCRtpSender behavior (specifically the presence of createEncodedStreams on RTCRtpSender.prototype), so future maintainers understand the broader match and the rationale for keeping the existing navigator.userAgent check.packages/client/src/rtc/e2ee/worker.ts (1)
223-237: Add worker disposal mechanism to prevent memory leaks.The worker is lazily created but never cleaned up. Per coding guidelines, cleanup/disposal should be implemented for proper memory management. Also,
workerUrlis never revoked.♻️ Proposed addition for worker cleanup
let worker: Worker | undefined; let workerUrl: string | undefined; export const getWorker = () => { if (!worker) { if (!workerUrl) { const blob = new Blob([WORKER_SOURCE], { type: 'application/javascript', }); workerUrl = URL.createObjectURL(blob); } worker = new Worker(workerUrl, { name: 'stream-video-e2ee' }); } return worker; }; + +export const disposeWorker = () => { + if (worker) { + worker.terminate(); + worker = undefined; + } + if (workerUrl) { + URL.revokeObjectURL(workerUrl); + workerUrl = undefined; + } +};The
disposeWorkerfunction should then be called fromPublisher.dispose()/Subscriber.dispose()or a higher-level cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/e2ee/worker.ts` around lines 223 - 237, The module lazily creates a Worker in getWorker but never disposes it or revokes workerUrl, causing memory leaks; add a disposeWorker function that checks the module-scoped worker and workerUrl and if present calls worker.terminate(), URL.revokeObjectURL(workerUrl), and sets both worker and workerUrl to undefined, and invoke disposeWorker from higher-level cleanup such as Publisher.dispose() / Subscriber.dispose() to ensure proper teardown of the WORKER_SOURCE-backed worker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/client/src/rtc/e2ee/worker.ts`:
- Around line 126-132: The transform method currently bypasses encryption for
AV1 frames (check: transform function, condition codec === 'av1') which can
leave AV1 streams in plaintext; modify transform to emit a clear runtime warning
via the module's logger (or console.warn if no logger exists) whenever codec ===
'av1' and E2EE is enabled, and update the public API docs/comments to explicitly
state that AV1 frames are not encrypted by this E2EE implementation so users are
aware of the caveat.
- Around line 115-119: Add a disposal and recovery mechanism for the E2EE
worker: implement a dispose() that unregisters event listeners registered on the
worker (rtctransform, message), calls worker.terminate(), and calls
URL.revokeObjectURL(workerUrl); add error and 'onerror'/'onmessageerror'
handling in getWorker() so the worker can be reset/recreated on failure;
document via JSDoc on the module/getWorker()/xorPayload() that this
XOR-with-repeating-key implementation (and the 0xDEADBEEF frame marker) is a
reference/demo only and not cryptographically secure and recommend authenticated
encryption (e.g., IETF SFrame / AES‑GCM per RFC 9605) for production; finally
add a small recovery API (e.g., resetWorker()/ensureWorker()) to recreate the
worker after errors.
In `@packages/client/src/rtc/Subscriber.ts`:
- Around line 97-107: The dynamic import in Subscriber (where supportsE2EE()
leads to import('./e2ee/e2ee').then(({ createDecryptor }) => {
createDecryptor(e.receiver, encryptionKey); this.logger.debug('E2EE decryptor
attached to receiver'); })) lacks error handling; add a .catch handler on the
import promise to log the failure via this.logger.error (include the error and
context like "failed to load E2EE module" or "failed to initialize decryptor")
and, if appropriate, call any fallback or cleanup (e.g., notify caller or avoid
attaching decryptor) so unhandled promise rejections are prevented and failures
are visible.
---
Nitpick comments:
In `@packages/client/src/rtc/e2ee/e2ee.ts`:
- Around line 7-11: The userAgent check in shouldUseInsertableStreams()
currently uses navigator.userAgent?.includes('Chrome') which also matches other
Chromium-based browsers (Edge, Brave, Opera); update the code by adding a
clarifying comment above shouldUseInsertableStreams() stating that this check
intentionally targets Chromium-based browsers (not only Google Chrome) because
they share the same WebRTC/RTCRtpSender behavior (specifically the presence of
createEncodedStreams on RTCRtpSender.prototype), so future maintainers
understand the broader match and the rationale for keeping the existing
navigator.userAgent check.
In `@packages/client/src/rtc/e2ee/worker.ts`:
- Around line 223-237: The module lazily creates a Worker in getWorker but never
disposes it or revokes workerUrl, causing memory leaks; add a disposeWorker
function that checks the module-scoped worker and workerUrl and if present calls
worker.terminate(), URL.revokeObjectURL(workerUrl), and sets both worker and
workerUrl to undefined, and invoke disposeWorker from higher-level cleanup such
as Publisher.dispose() / Subscriber.dispose() to ensure proper teardown of the
WORKER_SOURCE-backed worker.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72ec27ec-5586-4e79-9399-0b0d70c779d6
📒 Files selected for processing (7)
packages/client/rollup.config.mjspackages/client/src/rtc/Publisher.tspackages/client/src/rtc/Subscriber.tspackages/client/src/rtc/__tests__/Subscriber.test.tspackages/client/src/rtc/e2ee/compatibility.tspackages/client/src/rtc/e2ee/e2ee.tspackages/client/src/rtc/e2ee/worker.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/client/src/rtc/Publisher.ts
- packages/client/src/rtc/tests/Subscriber.test.ts
Comment on lines
+115
to
+119
| function xorPayload(src, dst, offset, key, keyLen, len) { | ||
| for (let i = 0; i < len; ++i) { | ||
| dst[i] = src[i] ^ key.charCodeAt((offset + i) % keyLen); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WebRTC E2EE best practices authenticated encryption AES-GCM
💡 Result:
WebRTC E2EE + AES‑GCM (authenticated encryption): best practices
- Use a standard E2EE framing scheme (SFrame) rather than ad‑hoc frame crypto. SFrame is the IETF’s lightweight AEAD-based approach designed for real-time media and for use on top of WebRTC when you don’t want intermediaries (e.g., SFUs) to see media. (datatracker.ietf.org)
- In browsers, prefer the built-in “SFrame transform” via WebRTC Encoded Transforms / Insertable Streams. The W3C Encoded Transform spec explicitly defines setting SFrame key material (key + keyId) per RFC 9605. (w3.org)
- Pick an SFrame cipher suite that uses AEAD AES‑GCM (and keep a full tag unless you have a strong reason). RFC 9605 defines an AES‑GCM suite (e.g.,
AES_128_GCM_SHA256_128) with a 96‑bit nonce and 128‑bit tag. (datatracker.ietf.org) - Never reuse an (AES‑GCM key, nonce/IV) pair. With GCM, nonce reuse under the same key is catastrophic; ensure your construction guarantees uniqueness (e.g., per-sender counters + key separation, or SFrame’s mechanisms). (datatracker.ietf.org)
- Rotate/ratchet keys for forward secrecy and compromise recovery. RFC 9605 describes key identifiers and ratcheting concepts intended to integrate with an E2E key management framework. (ietf.org)
- Treat signaling and identity/authentication as part of E2EE. WebRTC encrypts media hop-by-hop with SRTP (typically set up via DTLS-SRTP), but true E2EE requires that your key agreement/authentication is not subvertible by the signaling path. (webrtc.googlesource.com)
- If you’re comparing “SRTP AES‑GCM” vs “E2EE AES‑GCM”: don’t confuse them. SRTP AES‑GCM (RFC 7714) protects media on the wire between endpoints/SFUs, while SFrame/Encoded Transforms add an application-controlled E2EE layer so intermediaries can’t decrypt. (ietf.org)
If you share your architecture (P2P vs SFU, number of participants, and how you do key agreement—PSK/MLS/other), I can suggest a concrete nonce/key schedule that avoids GCM misuse and fits WebRTC frame semantics.
Citations:
- 1: https://datatracker.ietf.org/doc/html/rfc9605?utm_source=openai
- 2: https://www.w3.org/TR/webrtc-encoded-transform/?utm_source=openai
- 3: https://datatracker.ietf.org/doc/html/rfc9605?utm_source=openai
- 4: https://datatracker.ietf.org/doc/html/rfc9605?utm_source=openai
- 5: https://www.ietf.org/rfc/rfc9605.pdf?utm_source=openai
- 6: https://webrtc.googlesource.com/src/%2B/main/pc/g3doc/srtp.md?utm_source=openai
- 7: https://www.ietf.org/ietf-ftp/rfc/rfc7714.txt.pdf?utm_source=openai
🏁 Script executed:
# First, let's examine the file size and structure
wc -l packages/client/src/rtc/e2ee/worker.tsRepository: GetStream/stream-video-js
Length of output: 111
🏁 Script executed:
# Now let's read the relevant sections of the file
head -n 240 packages/client/src/rtc/e2ee/worker.ts | tail -n +1Repository: GetStream/stream-video-js
Length of output: 8059
🏁 Script executed:
# Let's also check for any comments or documentation about the source/purpose
grep -n "reference\|demo\|webrtc\|sample\|implementation\|TODO\|FIXME" packages/client/src/rtc/e2ee/worker.tsRepository: GetStream/stream-video-js
Length of output: 261
🏁 Script executed:
# Check if there are any related configuration or documentation files
find packages/client/src/rtc/e2ee -type f -name "*.ts" -o -name "*.tsx" -o -name "*.md" | head -20Repository: GetStream/stream-video-js
Length of output: 190
🏁 Script executed:
# Check e2ee.ts for any notes about maturity, demo status, or TODOs
head -100 packages/client/src/rtc/e2ee/e2ee.ts | grep -i "todo\|fixme\|demo\|experimental\|production\|note\|warning" -B2 -A2Repository: GetStream/stream-video-js
Length of output: 51
🏁 Script executed:
# Look for any overall E2EE documentation or README
find packages/client/src/rtc/e2ee -name "*.md" -o -name "README*"Repository: GetStream/stream-video-js
Length of output: 51
🏁 Script executed:
# Check if there are any tests or comments indicating maturity
find packages/client -path "*/e2ee*" -name "*.test.ts" -o -name "*.spec.ts" 2>/dev/nullRepository: GetStream/stream-video-js
Length of output: 51
🏁 Script executed:
# Check compatibility.ts for any relevant context
head -50 packages/client/src/rtc/e2ee/compatibility.tsRepository: GetStream/stream-video-js
Length of output: 334
Add worker disposal mechanism and clarify E2EE security properties.
This E2EE implementation has three issues:
-
Missing worker cleanup: No
dispose()method or cleanup mechanism. The worker's event listeners (rtctransform,message) are never unregistered, andworkerUrlis never passed toURL.revokeObjectURL(), causing memory leaks. Add a cleanup function that revokes the Blob URL and terminates the worker. -
Cryptographic limitations: XOR with a repeating key is not cryptographically secure—it's vulnerable to known-plaintext attacks and provides no authentication. The
0xDEADBEEFmarker is useful for frame detection but not a MAC. While this follows the webrtc/samples reference implementation, production E2EE should use authenticated encryption (IETF SFrame with AES-GCM per RFC 9605). Add JSDoc clearly stating this is a reference implementation, not production-ready E2EE. -
Unguarded worker lifecycle:
getWorker()creates a shared worker but has no recovery path if the worker errors or needs recreation. Consider adding error handling and a mechanism to reset the worker.
Per coding guidelines: "Always call dispose() on Call, Publisher, Subscriber when done for proper memory management" and "Unregister event listeners in cleanup/disposal code to prevent memory leaks."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/e2ee/worker.ts` around lines 115 - 119, Add a
disposal and recovery mechanism for the E2EE worker: implement a dispose() that
unregisters event listeners registered on the worker (rtctransform, message),
calls worker.terminate(), and calls URL.revokeObjectURL(workerUrl); add error
and 'onerror'/'onmessageerror' handling in getWorker() so the worker can be
reset/recreated on failure; document via JSDoc on the
module/getWorker()/xorPayload() that this XOR-with-repeating-key implementation
(and the 0xDEADBEEF frame marker) is a reference/demo only and not
cryptographically secure and recommend authenticated encryption (e.g., IETF
SFrame / AES‑GCM per RFC 9605) for production; finally add a small recovery API
(e.g., resetWorker()/ensureWorker()) to recreate the worker after errors.
Comment on lines
+126
to
+132
| transform(frame, controller) { | ||
| // https://groups.google.com/g/discuss-webrtc/c/5CMOZ4JtERo | ||
| // https://issues.chromium.org/issues/40287616 | ||
| if (codec === 'av1') { | ||
| controller.enqueue(frame); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
AV1 frames pass through unencrypted - ensure this is clearly communicated to users.
When codec === 'av1', frames are enqueued without encryption. While this is documented in the JSDoc header, users configuring E2EE might not realize AV1 streams remain in plaintext. Consider:
- Logging a warning when AV1 is detected with E2EE enabled
- Adding this caveat to the public API documentation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/e2ee/worker.ts` around lines 126 - 132, The transform
method currently bypasses encryption for AV1 frames (check: transform function,
condition codec === 'av1') which can leave AV1 streams in plaintext; modify
transform to emit a clear runtime warning via the module's logger (or
console.warn if no logger exists) whenever codec === 'av1' and E2EE is enabled,
and update the public API docs/comments to explicitly state that AV1 frames are
not encrypted by this E2EE implementation so users are aware of the caveat.
Comment on lines
+97
to
+107
| const { encryptionKey } = this.clientPublishOptions || {}; | ||
| if (encryptionKey) { | ||
| if (supportsE2EE()) { | ||
| import('./e2ee/e2ee').then(({ createDecryptor }) => { | ||
| createDecryptor(e.receiver, encryptionKey); | ||
| this.logger.debug('E2EE decryptor attached to receiver'); | ||
| }); | ||
| } else { | ||
| this.logger.warn(`E2EE requested but not supported`); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add error handling for the dynamic import.
The dynamic import lacks a .catch() handler. If the import fails (e.g., network error loading the chunk, or an exception in module initialization), it will result in an unhandled promise rejection with no indication to the user that E2EE failed to initialize.
🛡️ Proposed fix
const { encryptionKey } = this.clientPublishOptions || {};
if (encryptionKey) {
if (supportsE2EE()) {
- import('./e2ee/e2ee').then(({ createDecryptor }) => {
- createDecryptor(e.receiver, encryptionKey);
- this.logger.debug('E2EE decryptor attached to receiver');
- });
+ import('./e2ee/e2ee')
+ .then(({ createDecryptor }) => {
+ createDecryptor(e.receiver, encryptionKey);
+ this.logger.debug('E2EE decryptor attached to receiver');
+ })
+ .catch((err) => {
+ this.logger.error('Failed to attach E2EE decryptor', err);
+ });
} else {
this.logger.warn(`E2EE requested but not supported`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { encryptionKey } = this.clientPublishOptions || {}; | |
| if (encryptionKey) { | |
| if (supportsE2EE()) { | |
| import('./e2ee/e2ee').then(({ createDecryptor }) => { | |
| createDecryptor(e.receiver, encryptionKey); | |
| this.logger.debug('E2EE decryptor attached to receiver'); | |
| }); | |
| } else { | |
| this.logger.warn(`E2EE requested but not supported`); | |
| } | |
| } | |
| const { encryptionKey } = this.clientPublishOptions || {}; | |
| if (encryptionKey) { | |
| if (supportsE2EE()) { | |
| import('./e2ee/e2ee') | |
| .then(({ createDecryptor }) => { | |
| createDecryptor(e.receiver, encryptionKey); | |
| this.logger.debug('E2EE decryptor attached to receiver'); | |
| }) | |
| .catch((err) => { | |
| this.logger.error('Failed to attach E2EE decryptor', err); | |
| }); | |
| } else { | |
| this.logger.warn(`E2EE requested but not supported`); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/Subscriber.ts` around lines 97 - 107, The dynamic
import in Subscriber (where supportsE2EE() leads to
import('./e2ee/e2ee').then(({ createDecryptor }) => {
createDecryptor(e.receiver, encryptionKey); this.logger.debug('E2EE decryptor
attached to receiver'); })) lacks error handling; add a .catch handler on the
import promise to log the failure via this.logger.error (include the error and
context like "failed to load E2EE module" or "failed to initialize decryptor")
and, if appropriate, call any fallback or cleanup (e.g., notify caller or avoid
attaching decryptor) so unhandled promise rejections are prevented and failures
are visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Overview
Add end-to-end encryption for media tracks using a symmetric XOR transform applied to every encoded frame in a dedicated Web Worker.
📝 Implementation notes
packages/client/src/rtc/e2ee/index.tsmodule with a single shared Worker that handles both encode and decode transformscreateEncodedStreams) on Chrome whereRTCRtpScriptTransformis unreliableonrtctransformevent for the standard API,onmessagefor Insertable StreamsDataView+charCodeAt, following the webrtc/samples referenceBasePeerConnectionconditionally setsencodedInsertableStreams: trueon the PC config for ChromePublisher.addTransceiverattaches encryptor to sender,Subscriber.handleOnTrackattaches decryptor to receiverWeakSetguard prevents double-piping oncreateEncodedStreams(which can only be called once per sender/receiver)encryptionKeyoption added toClientPublishOptionsand wired through the dogfood app via?encryption_key=query param🎫 Ticket: https://linear.app/stream/issue/XYZ-123
Summary by CodeRabbit
New Features
Tests