chore: support running multiple clients with same client id#1253
chore: support running multiple clients with same client id#1253
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
joker23
force-pushed
the
skz/sdk-1907/electron-multi-env
branch
3 times, most recently
from
4c9b14f to
5809629
Compare
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5809629. Configure here.
| ipcRenderer.sendSync(getIPCChannelName(namespace, 'removeEventHandler'), callbackId), | ||
| }); | ||
| const ldClientBridge = (credential: string, namespace?: string): LDClientBridge => { | ||
| const ipcNs = deriveNamespace(credential, namespace); |
There was a problem hiding this comment.
Bridge preload now requires Node.js crypto module
Medium Severity
The bridge module (used as an Electron preload script) now transitively imports createHash from Node.js crypto via deriveNamespace in ElectronIPC.ts. Previously, the bridge only depended on electron APIs (ipcRenderer, contextBridge) which are available in sandboxed preload scripts. The crypto module is not in the limited set of Node.js modules available in Electron's sandboxed preload context (only events, timers, url are). Since sandbox is Electron's default, this could break the bridge for users who don't further bundle their preload scripts with a polyfill.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5809629. Configure here.
| */ | ||
| export function deriveNamespace(credential: string, userNamespace?: string): string { | ||
| const input = userNamespace ? `${userNamespace}:${credential}` : credential; | ||
| return createHash('sha256').update(input).digest('base64url'); |
There was a problem hiding this comment.
Empty string namespace silently treated as no namespace
Low Severity
deriveNamespace uses a JavaScript truthiness check (userNamespace ? ...) which treats an empty string '' identically to undefined. Since TypeValidators.String accepts empty strings, a user can set namespace: '' and it silently behaves like no namespace was set. Two clients with the same credential — one with namespace: '' and one with no namespace — would collide on storage and IPC channels.
Reviewed by Cursor Bugbot for commit 5809629. Configure here.
joker23
force-pushed
the
skz/sdk-1907/electron-multi-env
branch
from
5809629 to
cc53988
Compare
1 participant
This PR will implement support for running multiple LDClients with the same mobile key. We do this by adding an optional
namespaceoption that will prefix the persistent storage path so there won't be any collisions.I also reduce some redundant code in the
__test__repo as it was getting unwieldyNote
Medium Risk
Changes how storage paths and IPC channel names are derived (hashed namespace, optional user namespace), which can affect multi-client behavior and any code assuming the old channel naming/bridge signature. Backward compatibility is intended when
namespaceis omitted, but misconfiguration could lead to unexpected client isolation or collisions.Overview
Adds an optional
namespaceoption to the Electron SDK to allow multiple client instances using the same credential to coexist without colliding on persisted cache or IPC handler/channel names.Introduces
deriveNamespace()(SHA-256 ofcredentialornamespace:credential) and threads the derived value through main-process storage (ElectronPlatform/ElectronStorage), main-process IPC registration (ElectronClient), and renderer IPC bridge creation (ldClientBridge,ElectronRendererClient,createRendererClient).Updates/extends tests (including new
ElectronIPCunit tests and contract-test entity wiring) and refactors test utilities (sharedcreateMockLogger+ table-driven variation tests); docs add migration guidance for usingnamespacewith multiple clients.Reviewed by Cursor Bugbot for commit 5809629. Bugbot is set up for automated code reviews on this repo. Configure here.