feat(client): add E2EE support via WebRTC Encoded Transforms - #2198
Open
oliverlaz wants to merge 111 commits into
Open
feat(client): add E2EE support via WebRTC Encoded Transforms#2198oliverlaz wants to merge 111 commits into
oliverlaz wants to merge 111 commits into
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end encryption (E2EE) for WebRTC encoded frames: introduces an inline worker and Changes
Sequence DiagramssequenceDiagram
participant Client
participant Publisher
participant BasePeerConnection
participant RTCPeerConnection
participant EncryptionManager
participant Worker
participant RTCRtpSender
Client->>Publisher: publish(track, codec)
Publisher->>BasePeerConnection: createPeerConnection(config, e2ee?)
BasePeerConnection->>BasePeerConnection: clone RTCConfiguration
alt e2ee present and Chrome
BasePeerConnection->>BasePeerConnection: set encodedInsertableStreams = true
end
BasePeerConnection->>RTCPeerConnection: new RTCPeerConnection(modified config)
Publisher->>RTCRtpSender: addTransceiver -> sender
Publisher->>EncryptionManager: encrypt(sender, codec)
EncryptionManager->>Worker: postMessage(attach encode streams / transfer streams)
Worker->>RTCRtpSender: attach encode TransformStream
RTCRtpSender->>Worker: encoded frames -> Worker (transform)
Worker->>Worker: AES-128-GCM encrypt, append trailer
Worker->>RTCRtpSender: output encrypted frames
sequenceDiagram
participant Network
participant RTCRtpReceiver
participant Worker
participant EncryptionManager
participant Subscriber
participant Client
Network->>RTCRtpReceiver: RTP packets (encrypted payload)
RTCRtpReceiver->>Worker: encoded frames -> Worker (transform)
Worker->>Worker: validate trailer, extract keyIndex/counter
Worker->>Worker: AES-128-GCM decrypt, RBSP unescape if needed
Worker->>RTCRtpReceiver: decrypted frames
RTCRtpReceiver->>Subscriber: ontrack(receiver, stream)
Subscriber->>EncryptionManager: decrypt(receiver, userId)
EncryptionManager->>Worker: postMessage(attach decode streams)
Subscriber->>Client: deliver media track for playback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
+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
Replace standalone E2EE function exports and ClientPublishOptions.e2ee config with a single EncryptionManager class that owns the full E2EE lifecycle: key distribution, transform attachment, and worker management. - Add EncryptionManager with factory create(), static isSupported(), and instance methods: setKey, setSharedKey, removeKeys, encrypt, decrypt - Upgrade worker crypto from XOR to AES-128-GCM with per-user key store, shared key fallback, counter-based IVs, and codec-aware clear bytes - Add Call.setE2EEManager() for externally-provided manager injection - Pass manager reference through BasePeerConnectionOpts to Publisher/Subscriber - Delete e2ee.ts and compatibility.ts (absorbed into EncryptionManager) - Remove e2ee field from ClientPublishOptions - worker.ts now exports only WORKER_SOURCE (manager owns worker lifecycle)
- Drop frames when encryption key is missing instead of leaking plaintext - Drop frames when decryption key is missing instead of feeding ciphertext to the decoder - Expand clearBytes trailer field to 2 bytes (max 32767) to support H.264 frames with large SPS/PPS/SEI headers - Add key material length validation (must be exactly 16 bytes for AES-128) - Add worker error reporting to main thread via postMessage - Use scoped logger (videoLoggerSystem) instead of console.error - Add dispose guard to prevent double-termination - Block AV1 codec in encrypt() instead of silently passing through - Make dogfood applyQueryConfigParams async to ensure E2EE is initialized before join() - Upgrade sample app key derivation from naive byte cycling to PBKDF2 - Remove unused sharedKeyIndex variable - Simplify frame counter back to single uint32
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/client/src/rtc/e2ee/EncryptionManager.ts (1)
139-144: Document that rawKey is transferred and becomes unusable after the call.The
[rawKey]transfer array moves ownership to the worker, making the original ArrayBuffer detached (zero-length). Callers who try to reuse the buffer will get unexpected behavior.📝 Suggested JSDoc addition
* `@param` rawKey - 16-byte raw AES-128 key material. Transferred to the worker (zero-copy). + * The ArrayBuffer becomes detached after this call and cannot be reused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/rtc/e2ee/EncryptionManager.ts` around lines 139 - 144, The setKey method transfers ownership of rawKey via this.worker.postMessage(..., [rawKey]) which detaches the original ArrayBuffer; update the JSDoc for setKey (and mention validateKeyLength behavior) to clearly state that rawKey is transferred and will become unusable after the call and advise callers to pass a clone if they need to reuse it (or remove the transfer if non-destructive behavior is required); ensure the comment references setKey and the postMessage transfer semantics so future maintainers understand the detachment risk.packages/client/src/Call.ts (1)
2001-2011: Consider adding runtime validation for timing constraint.The JSDoc correctly documents that
setE2EEManagermust be called beforejoin(), but there's no runtime enforcement. If called afterjoin(), the manager won't be passed to the peer connections (they're already created), and E2EE silently won't work.Consider adding a warning or error when called after joining:
🛡️ Suggested defensive check
setE2EEManager = (e2ee: EncryptionManager) => { + if (this.state.callingState === CallingState.JOINED || + this.state.callingState === CallingState.JOINING) { + this.logger.warn( + 'setE2EEManager called after join - E2EE will not be active for this session' + ); + } this.e2eeManager = e2ee; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/Call.ts` around lines 2001 - 2011, setE2EEManager can be called after the call has already joined which silently makes E2EE ineffective; add a runtime check in the setE2EEManager method that detects whether the call has already created peer connections (e.g. check a join flag like this.joined or existing peer connection map/array such as this.peerConnections) and when true, either throw an Error or at minimum log a clear warning (e.g. "setE2EEManager must be called before join() — E2EE will not be applied") and avoid silently failing; still assign this.e2eeManager when safe, but ensure the check references setE2EEManager, join, this.e2eeManager and the join/peer-connection indicator so future callers get immediate feedback.
🤖 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 105-118: Frame counter resets on worker recreation causing
potential IV reuse for the same key; update buildIV and nextFrameCounter to
prevent reuse by adding a random nonce component to the IV (e.g., make IV_LEN
12, fill bytes 0-3 with crypto.getRandomValues(4) and bytes 4-7 or 8-11 with the
4-byte frame counter) so each worker instantiation produces distinct (key, IV)
pairs even if counters restart, and ensure buildIV uses the same byte layout as
nextFrameCounter; alternatively consider persisting frameCounters externally or
requiring key rotation on worker recreation and document that constraint in
comments near buildIV/nextFrameCounter.
In `@sample-apps/react/react-dogfood/pages/bare/join/`[callId].tsx:
- Around line 65-67: The call to applyQueryConfigParams using router.query
currently swallows E2EE initialization errors by only logging them; change this
to await applyQueryConfigParams(router.query) inside a try/catch (mirroring
MeetingUI.tsx) and on catch either set a visible error state (e.g., setInitError
/ render an error screen) or prevent proceeding to join (e.g., navigate away or
disable join) so the call does not continue without requested E2EE; ensure the
error handling path surfaces the error to the user instead of only
console.error.
---
Nitpick comments:
In `@packages/client/src/Call.ts`:
- Around line 2001-2011: setE2EEManager can be called after the call has already
joined which silently makes E2EE ineffective; add a runtime check in the
setE2EEManager method that detects whether the call has already created peer
connections (e.g. check a join flag like this.joined or existing peer connection
map/array such as this.peerConnections) and when true, either throw an Error or
at minimum log a clear warning (e.g. "setE2EEManager must be called before
join() — E2EE will not be applied") and avoid silently failing; still assign
this.e2eeManager when safe, but ensure the check references setE2EEManager,
join, this.e2eeManager and the join/peer-connection indicator so future callers
get immediate feedback.
In `@packages/client/src/rtc/e2ee/EncryptionManager.ts`:
- Around line 139-144: The setKey method transfers ownership of rawKey via
this.worker.postMessage(..., [rawKey]) which detaches the original ArrayBuffer;
update the JSDoc for setKey (and mention validateKeyLength behavior) to clearly
state that rawKey is transferred and will become unusable after the call and
advise callers to pass a clone if they need to reuse it (or remove the transfer
if non-destructive behavior is required); ensure the comment references setKey
and the postMessage transfer semantics so future maintainers understand the
detachment risk.
🪄 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: f60c8fa0-f27a-4082-b311-018d76a5c700
📒 Files selected for processing (13)
packages/client/index.tspackages/client/src/Call.tspackages/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/e2ee/EncryptionManager.tspackages/client/src/rtc/e2ee/worker.tspackages/client/src/rtc/types.tssample-apps/react/react-dogfood/components/MeetingUI.tsxsample-apps/react/react-dogfood/lib/queryConfigParams.tssample-apps/react/react-dogfood/pages/bare/join/[callId].tsx
✅ Files skipped from review due to trivial changes (3)
- packages/client/src/rtc/types.ts
- packages/client/src/rtc/tests/Publisher.test.ts
- packages/client/src/rtc/tests/Subscriber.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/client/src/rtc/Publisher.ts
- packages/client/src/rtc/BasePeerConnection.ts
- packages/client/src/rtc/Subscriber.ts
Add a standalone Vite + React sample app for testing E2EE with multiple participants in a single browser window. Each participant has their own StreamVideoClient, Call, and EncryptionManager with per-user keys. Demo features: - Add/remove up to 4 participants sharing the same call - Per-user key generation, manual key input (hex or passphrase), rotation - Automatic cross-participant key distribution - "Local only" mode to simulate key mismatch - Decryption failure toast notification with manual dismiss - Event log showing all key operations in real time SDK changes: - Subscriber uses participant.userId for decrypt key lookup instead of trackLookupPrefix, fixing per-user key matching - Worker posts throttled decryptionFailed messages on decrypt failure - EncryptionManager exposes onDecryptionFailed callback
…tices Centralize all E2EE key operations into e2ee/keys.ts with a pluggable SendKeyFn transport abstraction. Extract React state management into a useE2EEDemo hook, leaving App.tsx as a thin layout shell. - Make Call.e2eeManager public for direct access - Add E2EEParticipant interface (pure key state, no SDK refs) - Wrap ParticipantPanel in React.memo with stable callbacks - Rename ParticipantState to ParticipantSession - Fix side-effects-in-state-updater anti-pattern - Delete crypto.ts (superseded by e2ee/keys.ts)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
sample-apps/react/e2ee-demo/src/e2ee/keys.ts (1)
119-140: Consider documenting the hardcoded salt limitation.The PBKDF2 derivation uses a static salt
'stream-e2ee'. While acceptable for a demo, a production implementation should use a unique salt per user or session. Consider adding a brief comment noting this.📝 Suggested documentation addition
export const deriveKeyFromPassphrase = async ( passphrase: string, ): Promise<ArrayBuffer> => { const enc = new TextEncoder(); const baseKey = await crypto.subtle.importKey( 'raw', enc.encode(passphrase), 'PBKDF2', false, ['deriveBits'], ); + // Note: Static salt is acceptable for this demo. In production, use a + // unique salt per user/session stored alongside the derived key metadata. return crypto.subtle.deriveBits( { name: 'PBKDF2', salt: enc.encode('stream-e2ee'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/e2ee/keys.ts` around lines 119 - 140, The deriveKeyFromPassphrase function currently uses a hardcoded salt ('stream-e2ee') in the PBKDF2 parameters; add a concise comment above deriveKeyFromPassphrase noting that the static salt is only suitable for demo purposes and that production should use a unique, per-user or per-session random salt (persisted or transmitted alongside the derived key material) to prevent cross-user attacks and rainbow-table vulnerabilities; reference the function name deriveKeyFromPassphrase and the literal 'stream-e2ee' in the comment so reviewers can quickly find and understand the limitation.sample-apps/react/e2ee-demo/src/components/KeyControls.tsx (1)
13-29: Consider memoizingtoHexcomputation and stabilizing handlers.For consistency with coding guidelines,
toHex(currentKey)could be memoized withuseMemo, andhandleSetKeycould useuseCallback. However, since this is a demo app with limited re-render frequency, this is a minor optimization.♻️ Optional optimization
-import { useState } from 'react'; +import { useState, useMemo, useCallback } from 'react'; import { toHex } from '../e2ee/keys'; import './KeyControls.css'; // ... props interface ... export const KeyControls = ({ currentKey, keyIndex, color, onRotate, onSetKey, }: KeyControlsProps) => { const [input, setInput] = useState(''); const [localOnly, setLocalOnly] = useState(false); - const hex = toHex(currentKey); + const hex = useMemo(() => toHex(currentKey), [currentKey]); - const handleSetKey = () => { + const handleSetKey = useCallback(() => { const trimmed = input.trim(); if (!trimmed) return; onSetKey(trimmed, localOnly); setInput(''); - }; + }, [input, localOnly, onSetKey]);As per coding guidelines: "Use useMemo for expensive computations in React components" and "Use useCallback hook to stabilize function references passed to child components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/components/KeyControls.tsx` around lines 13 - 29, The component KeyControls computes hex with toHex(currentKey) on every render and defines handleSetKey inline; memoize the hex value using React's useMemo referencing currentKey and stabilize handleSetKey with useCallback referencing input, localOnly and onSetKey so the function identity is stable when passed to children; ensure imports include useMemo and useCallback and keep existing behavior (trim input, early return, call onSetKey(trimmed, localOnly), then setInput('')) while referencing the memoized hex variable.sample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.ts (2)
30-36: Token fetch lacks explicit error handling.If the fetch fails or returns non-JSON, the error will propagate up. While the caller has a try/catch, consider adding basic validation for clearer error messages in the demo.
♻️ Suggested improvement
const createTokenProvider = (userId: string) => async () => { const url = new URL(TOKEN_ENDPOINT); url.searchParams.set('api_key', API_KEY); url.searchParams.set('user_id', userId); - const { token } = await fetch(url).then((r) => r.json()); + const response = await fetch(url); + if (!response.ok) { + throw new Error(`Token fetch failed: ${response.status}`); + } + const { token } = await response.json(); return token as string; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.ts` around lines 30 - 36, The createTokenProvider function lacks explicit error handling for network failures and non-OK/non-JSON responses; wrap the fetch call in a try/catch, validate response.ok and content-type before parsing, and throw descriptive errors including the URL/userId and status/text or parse error so callers get clearer messages (update createTokenProvider to catch fetch/network exceptions, check response.ok, inspect headers for application/json, attempt json() inside a try and throw a clear error if parsing fails).
270-273: Minor inconsistency in accessinge2eeManager.Line 272 accesses
target.call.e2eeManager?.dispose(), while the cleanup effect (line 61) accessesp.e2eeManager.dispose()directly. Sincetargetalready has thee2eeManagerreference, using it directly is clearer and avoids the optional chaining.♻️ Suggested fix
// SDK cleanup target.call.leave().catch(() => {}); - target.call.e2eeManager?.dispose(); + target.e2eeManager.dispose(); target.client.disconnectUser().catch(() => {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.ts` around lines 270 - 273, The cleanup block is inconsistently accessing the E2EE manager; change the call from using optional chaining on the call object (target.call.e2eeManager?.dispose()) to use the direct reference already present on the target (target.e2eeManager.dispose()), mirroring the earlier cleanup pattern (p.e2eeManager.dispose()); ensure you call dispose() on target.e2eeManager and remove the unnecessary optional chaining so the code consistently uses the same E2EE manager reference.sample-apps/react/e2ee-demo/src/components/ParticipantPanel.tsx (2)
23-37: Consider memoizingCallUIto avoid unnecessary re-renders.
CallUIis rendered inside eachParticipantPanel. Since the parent is memoized and this component relies only on hook state, wrapping it withmemoensures it won't re-render when parent props change.♻️ Suggested refactor
-const CallUI = () => { +const CallUI = memo(function CallUI() { const { useCallCallingState } = useCallStateHooks(); const callingState = useCallCallingState(); if (callingState !== CallingState.JOINED) { return <div className="participant-panel__loading">Connecting...</div>; } return ( <StreamTheme> <PaginatedGridLayout /> <CallControls /> </StreamTheme> ); -}; +});Also update the import:
-import { memo, useCallback } from 'react'; +import { memo, useCallback } from 'react';Based on learnings: "Use React.memo for components rendered in large participant lists to optimize re-render performance"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/components/ParticipantPanel.tsx` around lines 23 - 37, Wrap the CallUI functional component with React.memo so it only re-renders when its hook-derived state changes: import React, { memo } (or add memo to the existing React import) and export/define CallUI as memo(CallUI) (or assign const MemoizedCallUI = memo(CallUI) and use that). Target the CallUI definition that uses useCallCallingState and is rendered inside ParticipantPanel so it doesn't re-render when parent props change; no other behavior changes to StreamTheme, PaginatedGridLayout, or CallControls are needed.
72-74: Truncation logic may produce misleading UI for short user IDs.If
userIdis shorter than 24 characters, the display will still append...even though nothing was truncated. Consider conditional truncation.♻️ Suggested fix
<span className="participant-panel__user-id" title={userId}> - {userId.slice(0, 24)}... + {userId.length > 24 ? `${userId.slice(0, 24)}...` : userId} </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/e2ee-demo/src/components/ParticipantPanel.tsx` around lines 72 - 74, The span in ParticipantPanel.tsx currently always appends "..." to the displayed userId via {userId.slice(0, 24)}..., which is misleading when userId.length <= 24; update the rendering in the component (look for the element with className "participant-panel__user-id" in ParticipantPanel) to conditionally truncate and append "..." only when userId.length > 24, otherwise render the full userId (and preserve the title={userId} behavior).
🤖 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/__tests__/Subscriber.test.ts`:
- Around line 218-247: The test assumes the '123' trackLookupPrefix is absent in
shared state; make it deterministic by using a unique prefix that cannot be
registered elsewhere in tests (e.g. 'orphan-<unique>' or a random string) when
setting mediaStream.id and in the expected decrypt call. Concretely, update the
mediaStream.id assignment in the test that calls Subscriber and handleOnTrack to
use a unique prefix (replace '123:TRACK_TYPE_VIDEO' with something like
'orphan-XYZ:TRACK_TYPE_VIDEO') and update the
expect(e2eeMock.decrypt).toHaveBeenCalledWith(...) assertion to expect that same
unique prefix ('orphan-XYZ') instead of '123', so the fallback-to-trackId path
is deterministic; alternatively clear or reset the relevant lookup in shared
state before invoking subscriber['handleOnTrack'] if you prefer to keep a fixed
id.
In `@packages/client/src/rtc/e2ee/worker.ts`:
- Around line 52-54: The sharedKey fallback reuses the same AES-GCM key across
participants while buildIV() only uses the frame counter, causing nonce reuse;
fix by deriving a participant-scoped subkey or adding a sender-unique IV prefix
that both sender and receiver can reconstruct. Locate the sharedKey variable and
the buildIV() function and implement one of these: 1) derive per-participant AES
keys from the shared secret (e.g., HKDF using userId or senderId as info) and
replace use of sharedKey with that derived CryptoKey, or 2) prepend a
sender-unique prefix (derived from the shared secret and senderId) to the IV
generated by buildIV() so the full IV includes both the prefix and the frame
counter; ensure the receiver performs the same derivation when decrypting so IVs
and keys match.
- Around line 56-63: importKey is async and can complete out of order, causing
latestKeyIndex to be rolled back; serialize imports per participant by adding a
per-user pending promise/lock (e.g., a Map like pendingImports keyed by userId)
and chain await the previous import before executing the body of importKey (or
acquire a per-user mutex) so imports for the same user run sequentially;
additionally, when updating latestKeyIndex inside importKey, only advance it if
the incoming keyIndex is newer than the stored value (or use the serialized
ordering guarantee so simple assignment is safe), referencing importKey,
latestKeyIndex, and keyStore to locate where to apply the serialization and
conditional update.
- Around line 46-54: The three maps keyStore, latestKeyIndex, and frameCounters
(and their comments) must be keyed by sessionId instead of userId to avoid
cross-session collisions when a user has multiple devices; change their usage so
every get/set/delete uses sessionId as the unique identifier, update any code
that installs/reads keys or increments frame counters to pass sessionId, and
ensure session-scoped cleanup when a session ends; keep sharedKey semantics as a
true fallback but document it explicitly as shared across sessions only when
intended.
In `@sample-apps/react/e2ee-demo/src/App.css`:
- Line 11: The CSS custom property --font-mono in App.css uses mixed case font
family names which triggers Stylelint's value-keyword-case rule; update the
value of --font-mono (the font-family string assigned to the --font-mono
variable) to use lowercase keywords (e.g., 'sf mono', 'cascadia code', 'fira
code', consolas, monospace) so all font names are lowercase and the linter
passes.
In `@sample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.ts`:
- Around line 230-258: The async call to setE2EEKeyFromInput inside
setKeyFromInput can reject but has no error handling; update setKeyFromInput to
add a .catch handler (or use try/catch if converting to async) for the Promise
returned by setE2EEKeyFromInput(target.e2eeManager, ...), and in that handler
log the error via logEvent (or console/error logger), surface a user-visible
error state for the target (e.g., update participants state to reflect failure
or set an error message), and ensure any partial UI updates are not applied on
failure so the UI remains consistent.
---
Nitpick comments:
In `@sample-apps/react/e2ee-demo/src/components/KeyControls.tsx`:
- Around line 13-29: The component KeyControls computes hex with
toHex(currentKey) on every render and defines handleSetKey inline; memoize the
hex value using React's useMemo referencing currentKey and stabilize
handleSetKey with useCallback referencing input, localOnly and onSetKey so the
function identity is stable when passed to children; ensure imports include
useMemo and useCallback and keep existing behavior (trim input, early return,
call onSetKey(trimmed, localOnly), then setInput('')) while referencing the
memoized hex variable.
In `@sample-apps/react/e2ee-demo/src/components/ParticipantPanel.tsx`:
- Around line 23-37: Wrap the CallUI functional component with React.memo so it
only re-renders when its hook-derived state changes: import React, { memo } (or
add memo to the existing React import) and export/define CallUI as memo(CallUI)
(or assign const MemoizedCallUI = memo(CallUI) and use that). Target the CallUI
definition that uses useCallCallingState and is rendered inside ParticipantPanel
so it doesn't re-render when parent props change; no other behavior changes to
StreamTheme, PaginatedGridLayout, or CallControls are needed.
- Around line 72-74: The span in ParticipantPanel.tsx currently always appends
"..." to the displayed userId via {userId.slice(0, 24)}..., which is misleading
when userId.length <= 24; update the rendering in the component (look for the
element with className "participant-panel__user-id" in ParticipantPanel) to
conditionally truncate and append "..." only when userId.length > 24, otherwise
render the full userId (and preserve the title={userId} behavior).
In `@sample-apps/react/e2ee-demo/src/e2ee/keys.ts`:
- Around line 119-140: The deriveKeyFromPassphrase function currently uses a
hardcoded salt ('stream-e2ee') in the PBKDF2 parameters; add a concise comment
above deriveKeyFromPassphrase noting that the static salt is only suitable for
demo purposes and that production should use a unique, per-user or per-session
random salt (persisted or transmitted alongside the derived key material) to
prevent cross-user attacks and rainbow-table vulnerabilities; reference the
function name deriveKeyFromPassphrase and the literal 'stream-e2ee' in the
comment so reviewers can quickly find and understand the limitation.
In `@sample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.ts`:
- Around line 30-36: The createTokenProvider function lacks explicit error
handling for network failures and non-OK/non-JSON responses; wrap the fetch call
in a try/catch, validate response.ok and content-type before parsing, and throw
descriptive errors including the URL/userId and status/text or parse error so
callers get clearer messages (update createTokenProvider to catch fetch/network
exceptions, check response.ok, inspect headers for application/json, attempt
json() inside a try and throw a clear error if parsing fails).
- Around line 270-273: The cleanup block is inconsistently accessing the E2EE
manager; change the call from using optional chaining on the call object
(target.call.e2eeManager?.dispose()) to use the direct reference already present
on the target (target.e2eeManager.dispose()), mirroring the earlier cleanup
pattern (p.e2eeManager.dispose()); ensure you call dispose() on
target.e2eeManager and remove the unnecessary optional chaining so the code
consistently uses the same E2EE manager reference.
🪄 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: 2330f332-3438-4070-82fe-411a79bdab61
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (28)
packages/client/src/Call.tspackages/client/src/rtc/Subscriber.tspackages/client/src/rtc/__tests__/Subscriber.test.tspackages/client/src/rtc/e2ee/EncryptionManager.tspackages/client/src/rtc/e2ee/worker.tssample-apps/react/e2ee-demo/index.htmlsample-apps/react/e2ee-demo/package.jsonsample-apps/react/e2ee-demo/src/App.csssample-apps/react/e2ee-demo/src/App.tsxsample-apps/react/e2ee-demo/src/components/EventLog.csssample-apps/react/e2ee-demo/src/components/EventLog.tsxsample-apps/react/e2ee-demo/src/components/Header.csssample-apps/react/e2ee-demo/src/components/Header.tsxsample-apps/react/e2ee-demo/src/components/KeyControls.csssample-apps/react/e2ee-demo/src/components/KeyControls.tsxsample-apps/react/e2ee-demo/src/components/ParticipantGrid.csssample-apps/react/e2ee-demo/src/components/ParticipantGrid.tsxsample-apps/react/e2ee-demo/src/components/ParticipantPanel.csssample-apps/react/e2ee-demo/src/components/ParticipantPanel.tsxsample-apps/react/e2ee-demo/src/config.tssample-apps/react/e2ee-demo/src/e2ee/keys.tssample-apps/react/e2ee-demo/src/hooks/useE2EEDemo.tssample-apps/react/e2ee-demo/src/main.tsxsample-apps/react/e2ee-demo/src/types.tssample-apps/react/e2ee-demo/src/vite-env.d.tssample-apps/react/e2ee-demo/tsconfig.jsonsample-apps/react/e2ee-demo/tsconfig.node.jsonsample-apps/react/e2ee-demo/vite.config.ts
✅ Files skipped from review due to trivial changes (13)
- sample-apps/react/e2ee-demo/src/vite-env.d.ts
- sample-apps/react/e2ee-demo/vite.config.ts
- sample-apps/react/e2ee-demo/tsconfig.node.json
- sample-apps/react/e2ee-demo/index.html
- sample-apps/react/e2ee-demo/tsconfig.json
- sample-apps/react/e2ee-demo/src/components/EventLog.css
- sample-apps/react/e2ee-demo/src/components/ParticipantGrid.css
- sample-apps/react/e2ee-demo/src/components/Header.css
- sample-apps/react/e2ee-demo/package.json
- sample-apps/react/e2ee-demo/src/main.tsx
- sample-apps/react/e2ee-demo/src/components/KeyControls.css
- sample-apps/react/e2ee-demo/src/types.ts
- sample-apps/react/e2ee-demo/src/components/ParticipantPanel.css
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/client/src/rtc/Subscriber.ts
- packages/client/src/Call.ts
- packages/client/src/rtc/e2ee/EncryptionManager.ts
When participant B joins, sendKey could not deliver A's key to B because B wasn't in participantsRef yet. Split the exchange: use sendKey for the outbound direction (A gets B's key) and set existing keys on B's e2eeManager directly.
…iliation When a track arrives before the participantJoined event, the receiver was never wired to the E2EE decryptor, leaving frames permanently encrypted. Store the RTCRtpReceiver with orphaned tracks and call decrypt() when the participant identity becomes known during reconciliation in the participant event handlers. Also adds a comprehensive test suite for EncryptionManager.
…porting - Derive IV prefix from SHA-256(rawKey + userId) to prevent IV reuse across users with shared keys and across key rotations - Add version byte to frame trailer (v1) for forward compatibility - Add perf-report toggle: worker reports encode/decode FPS when enabled - Expose setPerfReport() and onPerfReport callback on EncryptionManager Also updates the E2EE demo app: - Use ?environment=pronto instead of hardcoded mmhfdzb5evj2 API key - Add E2EE on/off toggle in header - Show perf metrics in the event log - Fix stale closure in addParticipant
Runtime toggle: - Add EncryptionManager.setEnabled() to toggle E2EE without reconnecting - Worker e2eeActive flag gates encoder only; decoder always decrypts encrypted frames (valid trailer + keys) and passes plaintext through - Demo app: per-participant and global E2EE toggles with key revocation Performance optimizations: - Synchronous IV prefix lookup on the hot path (eliminates one await per frame — no more unnecessary Promise/microtask overhead) - Pre-compute shared key IV prefixes at transform creation time - Zero-copy AAD via subarray instead of slice - Single-pass RBSP escape (removed counting pass)
- Rename worker.ts → e2ee-worker.ts and worker/ → e2ee-worker/ - Export bundled worker as a function instead of a template literal string - EncryptionManager creates Worker from e2eeWorker.toString() - Remove unnecessary tsconfig exclusion for worker directory
…rospection - Add codec selector (VP8/VP9/H.264/AV1) in e2ee-demo header - Add shared key support with passphrase input and per-user key overrides - Auto-hide decryption error banner on E2EE disable or decryption resume - Add onDecryptionResumed callback to EncryptionManager - Fix lazy IV prefix recomputation in encode/decode transforms - Add requestKeyDump() for worker key state introspection - Export KeyStateReport type from client package
- Frame counter fails closed at the AES-GCM 32-bit ceiling and fires
`rekeyRequested` once per user at 2^31, preventing IV reuse.
- Surface persistent decryption failures via `onE2EEBroken` so hosts
can terminate the session instead of silently passing through.
- Add `onRotationNeeded` callback for counter-threshold rekey signals.
- Opt-in AES-256-GCM via `EncryptionManager.create(userId,
{ algorithm: 'AES-256-GCM' })`; default stays AES-128-GCM.
- Two-pass RBSP escape (exact sizing, no over-allocation) and explicit
codec allowlist (opus/vp8/vp9/h264).
- Test coverage for rollover, rekey signalling, AES-256 opt-in,
codec/utils modules, and crypto key state.
- Demo: rename `keyHex` field to `fingerprint` to match the SDK's
non-reversible introspection surface.
`auto-on` makes E2EE mandatory, so the backend rejects a join whose e2ee flag does not match the call. `available` only permits E2EE, which would let a keyless, manager-less client into the call and undercut what the harness sets out to demonstrate.
Add a `?call_type=` query param, resolved the same way as `?environment=`, so the harness can exercise call types other than `default`. It is fixed for the session like the call id, so it stays out of the runtime config patch, and the header now reads `<type>:<id>`. Expose the live Call instances on `window` for quicker console access. The harness runs several calls at once, so `window.calls` is keyed by lowercased participant name and `window.call` is a shortcut for the first one. Both are republished whenever the participant list changes, so removing a participant does not leave a torn-down call behind on `window`.
Add an Encryption selector to the control bar. It is seeded from the mode the
backend resolves, so joining a call someone else created shows that call's real
mode, and the selected mode is sent as the settings override on create.
The selector locks once the call exists rather than while someone is joined: the
backend refuses to change a call's encryption after creation ("call encryption
cannot be changed after creation"), and removing every participant does not
un-create the call, so re-picking a mode there would silently do nothing. Use a
fresh call id to try another mode.
Rename `Snapshot.encryptionMode` to `resolvedEncryptionMode` to keep it distinct
from the new `HarnessConfig.encryptionMode`: the selector is what the harness
asks for, the `mode:` badge is what the server resolved, and the two disagreeing
is a signal worth seeing rather than hiding.
Drop the `settings_override` from call creation: whether a call is end-to-end encrypted is now entirely the call type's server-side configuration, and the harness only reads the resolved mode back for the header badge. The encryption dropdown goes with it, along with the config field, the mode option type and the seeding flag that existed only to serve it. E2EE becomes a per-participant choice rather than a session-wide mode, which is the axis the SFU actually cares about: `addParticipant(e2ee)` either attaches an EncryptionManager or does not. The participant button splits into "Join (E2EE)" (green) and "Join (plain)" (neutral). A plain join needs no Encoded Transforms, so browser support does not gate it. Default to the dedicated `e2ee` call type, still overridable with `?call_type=`. Note that a call type permits exactly one of the two joins unless it is set to `available`: `auto-on` refuses a plain join with error 109, and a non-encrypted call refuses an e2ee join with error 110.
`CallUI` shows its "Connecting..." fallback for any non-JOINED state, so leaving through the SDK's own hang-up button left the panel stuck there: the harness only dropped a participant via its own remove button, and nothing watched for a call that ended on its own. Subscribe to `callingState$` per participant and remove it on LEFT, which also covers the SFU dropping the participant. The subscription is set up after the join, so the replayed initial value is JOINED and cannot tear down a participant mid-spawn. Re-entrancy is safe: teardown unsubscribes the emitting subscription and the follow-up `leave()` rejects on the already-left call, which teardown already catches.
…r contract The RTC layer no longer asks a manager which Encoded Transform API it intends to attach itself with. It enables the non-standard `encodedInsertableStreams` flag whenever a manager is attached and the browser exposes the legacy `createEncodedStreams` API, so a custom manager built on Insertable Streams can no longer break silently by omitting an optional interface method. `E2EEManager` is now exactly `encrypt` + `decrypt`. `EncryptionManager.preferredTransform` moves to an internal module for the same reason: which pipeline the SDK picks is an implementation detail, not part of the public contract. `isSupported()` stays public and delegates to it. The e2ee-demo selector now mirrors the one knob that is public, `forceRtpScriptTransform`, rather than naming a pipeline it can no longer know, and reports raw browser capabilities instead of predicting the pick. Adds coverage for the peer connection flag, which had none.
ChaosControls listed every other participant under "Revoke my key from", but revokeKey only acts on participants with an E2EE manager. Clicking the keyless spy did nothing and logged nothing, and clicking a plain joiner logged "Revoked <user>'s key" even though the removeKeys call was skipped on the missing manager. The event log is the whole point of the harness, so it must not claim a revocation that never happened. The list now filters on `enabled`, and revokeKey skips managerless holders instead of logging past them. The empty state reads "no key-holding peers", which stays true when a spy or plain joiner is in the call. Also drops dead code found while auditing the app: - E2EEHarness.dispose(), which had no callers. Wiring it to an unmount effect would misfire under StrictMode's double mount, and page teardown already does the real cleanup. - Three unused CSS blocks in ParticipantPanel.css, left over from a removed error toast and a user-id label. - An orphaned comment in ControlBar describing the encryption-mode lock that went away with the mode dropdown. Refreshes comments that outlived their code: the mode badge tooltip no longer points at the removed Encryption selector, and the harness no longer claims to request an encryption mode.
Addresses a focused review of the E2EE internals. - A malformed or forged H264 frame could kill a track's decode pipeline for good. readTrailer sizes clearBytes against the raw frame, but un-escaping shrinks the unit by a byte per escape sequence, so readTrailerIv could be asked to read at a negative offset. That throw escaped transform(), errored the TransformStream and left no path to rebuild it: the participant's media froze for the session. Both clearBytes and the RBSP flag are plaintext, so a relay could forge exactly this shape. The unit is now length-checked and the frame dropped instead. - registerEventHandlers captured call.e2eeManager at setup() time. An app that reads call settings before deciding to encrypt (get()/getOrCreate() then setE2EEManager then join) captured undefined for the whole call, so orphaned tracks - the very race that code handles - never got a decryptor. The manager is now read per event. - Every EncryptionManager method now rejects use after dispose(). The worker is terminated by then, so postMessage was a silent no-op and an attached transform stalled forever: rejoining a Call that kept a disposed manager published nothing, with no error and no log. - A frame that arrives unencrypted is still forwarded, since a peer may publish plain when the call's encryption mode is `available`, but it now raises `e2ee.unencrypted_frame` so a downgrade cannot pass unnoticed. - Not holding a key is reported as `e2ee.missing_key` with the keyIndex rather than as `e2ee.decryption_failed`, so key-distribution lag is distinguishable from a mismatched or tampered frame. - AV1 decrypt now consumes a tileIdx for every coded OBU, matching encrypt. Skipping the bump on a header-less OBU shifted the salt of later same-layer OBUs and failed their GCM check. - The stats key in the worker uses an escaped NUL instead of a literal 0x00 byte, which had made git classify the whole file as binary and hid it from diffs and blame. Also includes comment and log trimming across these files, and replaces the per-event log table with a single dispatch log line.
`e2ee.decryption_failed`, `e2ee.decryption_resumed` and `e2ee.broken` now carry the `trackType` alongside the userId. Failures are counted per track, so a peer publishing audio and video reports them independently: without the label a host cannot tell two failing tracks of one peer from the same track reported twice, and cannot tell that a peer's video recovered while their audio is still broken. Uses it in react-dogfood to explain a wrong shared key, which until now looked like a broken call rather than a wrong key: frames arrive, fail their authentication tag and are dropped, so tiles stay black and audio silent with nothing said about why. There is no direct "your key is wrong" signal and there cannot be. A wrong key still encrypts happily, so the local encoder never complains and nothing reports that others cannot decrypt us. The only evidence is inbound and per peer, so the verdict comes from the breadth of the failures: with a shared key, the peer holding the wrong one fails against every publisher while everyone else fails against only them. `e2ee.broken` is the trigger rather than `e2ee.decryption_failed`, which fires once a second for any transient mismatch and would cry wolf. The banner doubles as the fix. `updateEncryptionKey` previously only touched React state and the URL, so a key corrected after joining never reached the worker and the old key stayed installed. It now pushes the derived key to a live manager, reusing the same key index so the worker replaces it in place and clears its failure count on the first frame that decrypts. Blind spots, since the UI promises no more than this: alone in the call, or with every peer muted and camera-off, a wrong key stays undetectable, and two peers sharing the same wrong key decrypt each other so neither sees a full sweep.
AV1 is out of scope for the initial E2EE release, so the inline-OBU
framing scheme it needed is removed rather than shipped unused. Our SFU
must not negotiate AV1 when E2EE is enabled.
Deletes av1.ts and av1-obu.ts plus their tests and golden snapshot
(1025 lines). None of it was exported from index.ts, so the public API is
unchanged. With one framing scheme left, CodecProfile.scheme became
single-valued and is gone too, along with AV1_VERSION and
AV1_INLINE_HEADER_LEN.
Fail-closed is the safety net: av1 is no longer in CODEC_PROFILES, so
isSupportedCodec('av1') is false and the encode path installs a
drop-everything transform that emits e2ee.encryption_failed. An AV1 track
can never be published in the clear on an encrypted call, it publishes
nothing. Pinned by a test.
Two AV1 tests covered non-AV1 behavior and were re-based rather than
deleted:
- the encode-failure re-arm test now uses an h264 frame whose clear
header exceeds the trailer's 15-bit clearBytes field
- the AV1 round-trip is replaced by an opus one asserting the TOC byte
stays clear. That required dropping the `= 'delta'` default from the
frame() test helper, since `undefined` is how the worker recognizes an
audio frame and the default was silently turning audio cases into
delta video ones
Also fixed in passing:
- vitest coverage include narrowed to src/**/*.ts; the v8 remapper threw
a PARSE_ERROR on a non-TS file under src/
- the worker docblock claimed "VP9: 0 bytes"; the code has always used
VP8's 10/3 clear-byte rule
- the e2ee-demo no longer offers AV1 in its codec picker
The 2^31 soft threshold asked the host to rotate keys before the frame counter reached its 32-bit ceiling, but rotating cannot help. The counter is scoped to the worker rather than to a key, so neither setKey nor removeKeys resets it and the ceiling arrives at the same time regardless. Only a new EncryptionManager restores the budget. The event was also unreachable in practice, needing roughly 6 months of continuous publishing in one session, and its false remedy had already spread into the demo app, which claimed that installing fresh key material restarts the counter. Remove the event, the RotationEvent type, COUNTER_REKEY_THRESHOLD and the rekeyRequested set. The fail-closed check at 2^32 stays, without the misleading "rekey required" wording. Tests now cover that the counter stays pinned after exhaustion and that a rekey does not recover it.
e2ee.decryption_resumed could be lost permanently. recordSuccess consumes the recovery edge by clearing the failure count, so a throttled-away resumed was never retried and left the host latched on decryption_failed for a healthy track. It was also gated on the failing keyIndex's own count, but decryption_failed names a track rather than a key epoch, so a track that recovered by rotating to a new keyIndex never cleared either. Emit the recovery unthrottled and pair it with delivered failures instead: a new failureReported flag is set only when a decryption_failed actually reached the host, and cleared by the recovery. A recovery can only fire for a failure that fired, and those stay throttled, so the rate stays bounded without discarding transitions. Carry track identity on the events that are raised per track: encryption_failed gains userId and trackType, and the decode-side missing_key and unencrypted_frame gain trackType. Without it a peer's audio, video and screen share produced identical messages a host could not tell apart. The encode-side missing_key stays user-scoped, since holding no key stalls every outgoing track at once. Also correct FAILURE_TOLERANCE's comment, which claimed keys are marked invalid when it only gates a notification, and fix the demo's missing_key handler, which reported a remote key still in flight as a local encoder error.
Cut comments that restate the code, and shorten the rest to the reasoning they carry. Comment lines drop by roughly a third: 208 to 136 in EncryptionManager, 171 to 123 in the worker, 154 to 109 in crypto, 138 to 75 in events. The largest single source of noise was events.ts, where every event was documented twice: once on its payload type and again on its E2EEEventMap entry. The substance now lives on the type, and the map entries are one-liners. Elsewhere this removes per-field docs that repeat the field name, the duplicate trailer layout above writeTrailer, and the restatement wrapped around the load-bearing rationale in the replay window and trust ordering blocks. That reasoning is kept. No behavior change.
The marker is a heuristic, not a guarantee: any 4-byte value collides with random data at 2^-32. What it can do is avoid being a likely accident. 0xDEADBEEF is a widespread debug fill, so a non-E2EE frame ending in it is far more plausible than one ending in a value nothing else uses, and every such frame reaches a decrypt attempt. The new value keeps the property the H264 path depends on: no byte of it is 0x00 or <= 0x03, so the start-code-safe trailer tail still survives RBSP escaping untouched. Noted on the constant, so a future value cannot be picked without it. Pre-release wire change, so no version bump. Conformance vectors are regenerated in the spec; only the trailing 4 bytes move, since the magic feeds neither the AAD nor the IV.
Without a codec the encoder fell back to a profile that left 0 clear bytes and did not escape. For video that is silently unsafe: the SFU cannot read the frame headers it needs for keyframe detection and layer selection, and an unescaped H264 payload is split by the packetizer at the first random start code in the ciphertext. Nothing reported it. It also inverted the safety ordering. A named codec with no rule, say h265, fails closed and says so, while the same stream published with the codec omitted shipped corrupt frames quietly. Add `audioOnly` to CodecProfile, set on opus and on the default profile, and reject a frame carrying a key/delta type under it. The check sits in the encode transform rather than at attach time, because attach time cannot tell audio from video without reading trackType, which is documented as grouping perf stats only. It runs before the counter, so a dropped frame costs no IV. Unlabeled audio is unchanged and still keeps its 1 clear byte: the TOC byte rule holds for any audio codec. Tests cover both directions, so the guard cannot later be widened to reject all unlabeled frames.
The worker entry point had grown to 522 lines holding both transforms, the notification rules and the perf counters. Split it into encode.ts, decode.ts, notifications.ts and perf.ts, leaving a 143-line entry point that does what an entry point should: select a transform, pipe it, and dispatch commands. No file in the worker now exceeds crypto.ts at 381. notifications.ts is the one that earns its keep beyond tidiness. The SPEC section 10 delivery rules - throttled levels, the unthrottled decryption_resumed edge paired one-to-one with delivered failures, the per-track encryption_failed latch - were enforced across four closures inside decodeTransform and three loose postMessage calls elsewhere. They now live in one file, as EncodeNotifier and DecodeNotifier, so the contract can be read against the spec instead of reassembled from call sites. Also in this change: - Remove the forceRtpScriptTransform escape hatch. Chrome stays on Insertable Streams and the selection is no longer configurable, so re-enabling the standard API means changing preferredTransform rather than threading a flag through the manager, the peer connection and the demo harness. The demo's Transform selector goes with it. - Remove the cmd.dispose worker command and its pipeline teardown. dispose() posted it and then immediately called Worker.terminate(), which races the message and reclaims everything anyway. crypto.dispose stays as an explicitly marked test-only reset seam. - Resolve the transform in create() instead of falling back to 'script' in the constructor, so the unsupported-browser branch is unreachable by construction rather than by a defensive default. - Use Object.hasOwn for codec profile lookup: `in` walks the prototype chain, so a codec named 'toString' resolved to a function. - Export the create() options type, and fix a comment in the H264 encode branch that had been mangled into three interleaved drafts.
…undary The escaper started its zero-run count at 0, so it never saw the clear header's trailing bytes. On the wire the header's tail and the escaped unit are contiguous, so a header ending in 0x00 followed by ciphertext starting 00 01 shipped a fake Annex-B start code spanning the boundary. libwebrtc's H264 packetizer splits on it and the frame is destroyed. Browser encoders cannot reach this today: the last clear byte is the first slice-header byte, and first_mb_in_slice = 0 forces its top bit on. Multi-slice hardware encoders can, which matters because the iOS and Android SDKs are about to implement this format - so it is fixed now rather than recorded as a known limitation. Encode and decode both derive the seed from the same clear bytes, so nothing extra travels in the frame, and every frame that was already escaped correctly is byte-identical. rbspUnescape is rewritten as a state-machine mirror of the escaper so it can recognize an escape byte sitting at the boundary; for a zero seed it is equivalent to the previous index-scanning version. Also pins the SPEC section 11 conformance vectors byte-for-byte. Nothing guarded the wire format: the round-trip tests encrypt and decrypt with the same code, so a change to the trailer, the IV derivation or the escaping would pass them and break interop with other SDKs silently. The new test asserts the exact bytes on encode, and decodes the pinned bytes back - the direction that catches a decoder which only agrees with its own encoder. The H.264 vector exercises the RBSP path, so it also guards this fix.
Seven of the manager's worker-message tests asserted one generic forwarder with different string literals, three more duplicated the browser matrix that transformSupport.test.ts already owns, and the E2EEManager contract test asserted that vi.fn() records calls. Replaced with tables and type-level assertions. The module split left notifications.ts and perf.ts with no direct tests: perf.ts sat at 46% statements and createThrottle - the mechanism behind every SPEC section 10 rule - had no test at all. Both are now covered directly, under fake timers, which is what the throttle windows and the reporting interval need. Also covers the worker's command dispatch, and the Object.prototype codec lookup that the hasOwn fix guards but nothing asserted. Worker coverage 87.6 -> 95.4% statements, 79.6 -> 89.4% branches.
A second pass over the suite, after the per-module split. The remaining noise was not duplicated tests but granularity: dozens of single- assertion cases that were rows of an implicit table, so a new case meant copying a test rather than adding a row. Key validation is one code path behind setKey and setSharedKey, but was covered by seven tests spread over three describes; the browser matrix in transformSupport was five tests over one function; the clear-byte rules were six; readTrailer's rejection cases were four. Each is now a single table that states the property once. Removed outright: a perf test asserting startCrypto's sentinel return rather than any behaviour, a trailer test now redundant with the SPEC conformance vectors, and several pairs of tests that were two halves of one claim (replay-window prefix partitioning, peek-is-read-only, the fingerprint properties, removeKeys isolation). Coverage is unchanged in every digit - 95.38% statements, 89.72% branches - which is the evidence the removed cases were re-running paths other tests already covered. Collapsing the command table also surfaced enablePerformanceReporting, which had no test at all.
createReplayWindow, createFailureTracker and createThrottle each closed
over per-instance state and returned an object literal, which is what a
class expresses directly. Matches the convention the rest of the
codebase follows, and the perf registries converted earlier.
The replay window gains a second class rather than a straight
translation. Its epochs were plain {prefix, state} objects whose bitmap
was driven by four free functions taking the bitmap as an argument, so
the high-water mark and the bits it indexes were state with no owner,
and the advance logic appeared twice inside commit. ReplayEpoch now owns
both behind accepts()/record(), leaving ReplayWindow with just the epoch
list and the eviction policy.
Extracting that surfaced an untested branch, so this also adds the
regression test for it. Slots repeat every REPLAY_WINDOW counters, so
counter N and counter N + REPLAY_WINDOW share one bit; when the mark
advances past counters that never arrived, their slots must be cleared
or a later genuine out-of-order frame landing on one is rejected as a
replay and its media silently dropped. Deleting the clearing entirely
kept every existing test green - the "counter jump" test takes the
whole-bitmap-wipe branch and never reuses a slot. The new test advances
in sub-window steps and then delivers a colliding counter, and fails
without the clearing. The behaviour was always correct; only the
coverage was missing.
crypto.ts and utils.ts had grown into grab bags holding key management, IV derivation, the trailer codec, the replay window, the frame counter and the failure tracker. Dissolve both into modules named after what they own: - keyStore.ts: KeyStore class plus the worker's single instance - frameCounter.ts: the SPEC section 9 IV-uniqueness counter - replayWindow.ts: ReplayEpoch and ReplayWindow - failureTracker.ts: FailureTracker - trailer.ts: fillIV, writeTrailer, readTrailer, readTrailerIv - queue.ts: the serialization queue, which cannot live in the worker entry module because the inline-worker plugin forbids exports there Throttle moves into notifications.ts, its only consumer. Behaviour changes, both narrowing a footgun: - the frame counter is one number, not a Map keyed by userId. A worker serves a single local user and only the encode path draws from it, so keying by user could hand out a fresh counter at 1 on a changed id, which under the same key and IV prefix is exactly the IV reuse the counter exists to prevent. - key import failures report through reportError, so notifications.ts is the only module that talks to the host. Tests mirror the new layout: crypto.test.ts and utils.test.ts split into keyStore, frameCounter, replayWindow, failureTracker, trailer and queue suites. No coverage lost.
SPEC.md is the wire-format contract the iOS, Android, Flutter and Unity SDKs port against: trailer layout, IV derivation, clear-byte rules per codec, H.264 RBSP escaping, receiver hardening, event delivery rules and the section 11 conformance vectors that packages/client pins in conformance.test.ts. It had drifted from the implementation in four places, all corrected here: - section 7 and section 8 described `decryption_resumed` as throttled and as gated on the failure count. Section 10 already says the opposite, and the code agrees with section 10. An SDK following the old pseudocode would strand the host on `decryption_failed` for any track that recovers on a new keyIndex, which is the exact bug section 10 warns about. - the frame counter is one value per manager, not a map keyed by userId. Sections 4, 6 and 9 still showed the map, so section 9 now spells out why keying it is a footgun rather than merely redundant. - the clear-byte rules clamp to the frame length. Section 5.4 documented this for H.264 but section 5.1 did not, and an SDK that skips the clamp builds an AAD of a different length, which fails the tag every time. - section 2 did not mention that `create` throws when no Encoded Transform API exists or the worker cannot be constructed. Section 9 asks implementers to name the remedy in the exhaustion error, and the reference implementation had stopped doing so. The message now names a new EncryptionManager, which is what recovers the track; rotating keys, the thing an integrator would try first, does not. The existing rekey test asserts the message names it, so this cannot regress silently.
The constructor took a scope string and always built its own logger, so a
subclass that already had one ended up with two logger objects under the
same scope. EncryptionManager was doing exactly that: `super('Encryption
Manager')` plus its own `getLogger('EncryptionManager')`.
The constructor now takes either a `ScopedLogger` or a scope, and only
creates one when given a scope. The field is `protected` and renamed from
`emitterLogger` to `logger`, so subclasses can drop their own field and
use the inherited one rather than passing an instance down.
EncryptionManager does that here: its `logger` field, its `getLogger`
call and both logger imports are gone, and its five call sites resolve to
the inherited field unchanged.
Note for future subclasses: `protected` makes the field part of the
inheritance contract, so a subclass declaring its own `logger` now fails
to compile instead of silently shadowing it.
Removes the file-level comment blocks from notifications.ts and perf.ts. The delivery rules they described are documented in SPEC.md section 10, which is now in the repo, and the per-symbol JSDoc in both files already covers what each one does.
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/REACT-1050/e2ee-implementation-for-web
Docs: https://github.com/GetStream/docs-content/pull/1399
Summary by CodeRabbit
New Features
Tests
Chores