Skip to content

chore(deps): update socket.io/client to get downstream security patches #628

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jakemhiller
Copy link

Why

[email protected] (link) introduced an updated version of engine.io-client (link) that includes the updated ws version that fixes this vulnerability CVE-2024-37890

I haven't looked into whether the vulnerability affects this package, but because of the semver used in the socket.io packages, and in these packages, the vulnerable package wasn't able to be updated in our repo that uses snack-sdk.

How

I updated to the latest patch version of socket.io-client that included the ws fix. I had to make two minor TypeScript type changes but otherwise the changes in socket.io-client seemed non-breaking (api additions and bug fixes).

Test Plan

I see that the socket transport is mocked in the jest tests. Is there another way an update like this should be tested?

@jakemhiller jakemhiller requested a review from byCedric as a code owner March 24, 2025 18:19
@@ -42,7 +42,7 @@ export default class TransportImplSocketIO extends TransportImplBase {
this.socket = io(this.snackpubURL, { transports: ['websocket'] });

this.socket.on('connect', () => {
this.socket?.emit('subscribeChannel', { channel: this.channel, sender: this.socket?.id });
this.socket?.emit('subscribeChannel', { channel: this.channel, sender: this.socket?.id! });
Copy link
Author

Choose a reason for hiding this comment

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

Happy to change these to type guards if that is the preference in this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant