Skip to content

ci(node-server-sdk): put in contract test workaround #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joker23 wants to merge 1 commit into from
Closed

ci(node-server-sdk): put in contract test workaround #1085

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion contract-tests/src/sdkClientEntity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,22 @@ export async function newSdkClientEntity(options: any): Promise<SdkClientEntity>
options.configuration.startWaitTimeMs !== undefined
? options.configuration.startWaitTimeMs
: 5000;


// TODO: synchronizer should be a list, but for this is a temporary workaround
// so that we can pass the contract tests in a more reasonable manner.
// SDK-1798
const adaptedConfigs = {
...options.configuration,
};

adaptedConfigs.dataSystem.synchronizers = {
primary: options.configuration.dataSystem.synchronizers?.[0],
secondary: options.configuration.dataSystem.synchronizers?.[1],
}
Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check causes crash when dataSystem undefined

High Severity

The code accesses adaptedConfigs.dataSystem.synchronizers without checking if dataSystem exists first. Since dataSystem is an optional property in SdkConfigOptions, this throws a TypeError when options.configuration.dataSystem is undefined. The makeSdkConfig function properly checks if (options.dataSystem) before accessing it, but this workaround code doesn't perform the same validation, creating an inconsistency that causes runtime crashes.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shallow copy causes unintended input mutation

Medium Severity

The shallow copy of options.configuration means adaptedConfigs.dataSystem still references the original object. When the code assigns adaptedConfigs.dataSystem.synchronizers to a new object structure, it mutates the original options.configuration.dataSystem.synchronizers, changing it from an array to an object. This unexpected side effect modifies the caller's input data, violating the principle that functions shouldn't mutate their parameters unless explicitly intended.

Fix in Cursor Fix in Web

const client: LDClient = ld.init(
options.configuration.credential || 'unknown-sdk-key',
makeSdkConfig(options.configuration, options.tag),
makeSdkConfig(adaptedConfigs, options.tag),
);
try {
await client.waitForInitialization({ timeout });
Expand Down
Loading