Conversation
…nd modify setKey method
|
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates the Node RTC SDK’s E2EE integration to match newer @livekit/rtc-ffi-bindings APIs, including per-track cryptor identification (trackSid) and updated key-setting behavior.
Changes:
- Bump
@livekit/rtc-ffi-bindingsfrom0.12.46-dev.8to0.12.52(and update the lockfile accordingly). - Update
FrameCryptorto carrytrackSidand include it in relevant FFI requests. - Change
KeyProvider.setKeyto require the raw key bytes, and add convenience wrappers onE2EEManager.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pnpm-lock.yaml |
Locks the repo to @livekit/rtc-ffi-bindings@0.12.52 (all platform packages updated). |
packages/livekit-rtc/src/e2ee.ts |
Updates E2EE request shapes (adds trackSid, changes setKey signature, adds manager wrappers). |
packages/livekit-rtc/package.json |
Pins @livekit/rtc-ffi-bindings dependency to 0.12.52. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/livekit-rtc/src/e2ee.ts:41
DEFAULT_RATCHET_WINDOW_SIZEis changed, butKeyProviderOptions(includingratchetWindowSize,ratchetSalt, andfailureTolerance) is currently only stored onKeyProviderand never used to configure the underlying FFI. If the intent is to change ratchet behavior, this file should pass these options into an initialization/config request; otherwise the default values here are effectively dead configuration and can be misleading.
const DEFAULT_RATCHET_SALT = new TextEncoder().encode('LKFrameEncryptionKey');
const DEFAULT_RATCHET_WINDOW_SIZE = 10;
const DEFAULT_FAILURE_TOLERANCE = -1;
export interface KeyProviderOptions {
sharedKey?: Uint8Array;
ratchetSalt?: Uint8Array;
ratchetWindowSize?: number;
failureTolerance?: number;
}
export const defaultKeyProviderOptions: KeyProviderOptions = {
ratchetSalt: DEFAULT_RATCHET_SALT,
ratchetWindowSize: DEFAULT_RATCHET_WINDOW_SIZE,
failureTolerance: DEFAULT_FAILURE_TOLERANCE,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setKey(participantIdentity: string, key: Uint8Array, keyIndex: number) { | ||
| const req = new E2eeRequest({ | ||
| roomHandle: this.roomHandle, | ||
| message: { | ||
| case: 'setKey', | ||
| value: new SetKeyRequest({ | ||
| keyIndex: keyIndex, | ||
| participantIdentity: participantIdentity, | ||
| key, | ||
| }), |
There was a problem hiding this comment.
KeyProvider.setKey is a breaking change for consumers (this class is exported from src/index.ts), since it now requires a Uint8Array key parameter and changes the method signature/semantics. Consider providing an overload/backwards-compatible alternative (or a new method name) and/or documenting the migration path so downstream users don’t unexpectedly break on upgrade.
| setEnabled(enabled: boolean) { | ||
| this.enabled = enabled; | ||
| const req = new E2eeRequest({ | ||
| roomHandle: this.roomHandle, | ||
| message: { | ||
| case: 'cryptorSetEnabled', | ||
| value: new FrameCryptorSetEnabledRequest({ | ||
| participantIdentity: this.participantIdentity, | ||
| trackSid: this.trackSid, | ||
| enabled: this.enabled, | ||
| }), | ||
| }, | ||
| }); | ||
|
|
||
| FfiClient.instance.request({ | ||
| message: { | ||
| case: 'e2ee', | ||
| value: req, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| setKeyIndex(keyIndex: number) { | ||
| this.keyIndex = keyIndex; | ||
| const req = new E2eeRequest({ | ||
| roomHandle: this.roomHandle, | ||
| message: { | ||
| case: 'cryptorSetKeyIndex', | ||
| value: new FrameCryptorSetKeyIndexRequest({ | ||
| participantIdentity: this.participantIdentity, | ||
| trackSid: this.trackSid, | ||
| keyIndex: this.keyIndex, | ||
| }), | ||
| }, |
There was a problem hiding this comment.
There’s no test coverage around the new/changed E2EE request payloads (notably trackSid being included in cryptorSetEnabled/cryptorSetKeyIndex). Since this package already uses Vitest for unit tests, consider adding a small unit test that stubs globalThis._ffiClientInstance and asserts the constructed E2eeRequest includes trackSid (and the expected values) to prevent regressions.
| "@datastructures-js/deque": "1.0.8", | ||
| "@livekit/mutex": "^1.0.0", | ||
| "@livekit/typed-emitter": "^3.0.0", | ||
| "@livekit/rtc-ffi-bindings": "0.12.46-dev.8", | ||
| "@livekit/rtc-ffi-bindings": "0.12.52", | ||
| "pino": "^9.0.0", | ||
| "pino-pretty": "^13.0.0" |
There was a problem hiding this comment.
This PR changes exported public APIs in e2ee.ts (e.g., KeyProvider.setKey signature) but packages/livekit-rtc’s package version isn’t updated here. If this package follows semver, consider bumping the version (and/or adding release notes) to reflect the breaking API change for downstream users.
Update KeyProvider and FrameCryptor to include trackSid and modify setKey method