fix(qss): track endpoint identity in QSSClient connection coalescing#3210
Open
holmesworcester wants to merge 1 commit into
Open
fix(qss): track endpoint identity in QSSClient connection coalescing#3210holmesworcester wants to merge 1 commit into
holmesworcester wants to merge 1 commit into
Conversation
adrastaea
reviewed
May 7, 2026
Collaborator
adrastaea
left a comment
There was a problem hiding this comment.
This doesn't fit into the current release. Adds too much complexity for an unrealistic case.
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
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.
Summary
This fixes a QSS client connection race where one in-flight endpoint connection can incorrectly satisfy a later request for a different endpoint.
Changes:
ws://qss-astill share the same in-flight promise.Code Path
The vulnerable primitive in 7.1.0 is
QSSClient.createSocketAndConnect().In the base branch:
packages/backend/src/nest/qss/qss.client.ts:82-89stores one global_connectPromise._connectPromiseexists, the method returns it without checking what endpoint it belongs to.packages/backend/src/nest/qss/qss.client.ts:100-129then creates a socket for the first requested endpoint and waits for that socket to connect.That means this sequence is possible inside one client process:
This does not require multiple active communities or users in one Quiet client. It only requires one client to replace an in-flight QSS connection attempt before the first attempt settles.
Fix
The fix adds endpoint identity to the in-flight connection state:
packages/backend/src/nest/qss/qss.client.ts:34-36now tracks_connectPromiseEndpointand an abort controller for the in-flight connect.packages/backend/src/nest/qss/qss.client.ts:84-101still returns the existing promise when the requested endpoint matches.packages/backend/src/nest/qss/qss.client.ts:87-98aborts the stale in-flight connect when the requested endpoint differs.packages/backend/src/nest/qss/qss.client.ts:104-118records and clears the promise/endpoint/abort state together.packages/backend/src/nest/qss/qss.client.ts:166-229makes_waitForConnect()operate on the specific socket it was given and reject promptly when that connect is aborted.The key invariant is now: coalesce only when the endpoint is the same; replace when the endpoint changes.
Why This Is A Real Bug Class
This is not a "multiple communities active at once" bug. The realistic version is stale state during transition:
The previous implementation had no way to distinguish those requests. That is a real correctness issue because QSS endpoint identity is part of the community/join context. Returning a socket for endpoint A to a caller that requested endpoint B can make subsequent auth/sign-in/log-sync traffic go to the wrong QSS server.
Honest likelihood assessment:
Regression Test
The new test file is
packages/backend/src/nest/qss/qss.client.spec.ts.It covers two cases:
packages/backend/src/nest/qss/qss.client.spec.ts:106-128: concurrent requests for the same endpoint still coalesce onto one socket.packages/backend/src/nest/qss/qss.client.spec.ts:130-160: a newer request forws://new-qssmust not be satisfied by an abandoned pending connection tows://old-qss.Red on raw 7.1.0 after adding the test only:
Green after this fix:
Validation
Result:
Note
Replaces holmesworcester/quiet#2, which targeted the workflow-stripped mirror at
holmesworcester/quiet:7.1.0. This PR retargets the same patch directly at upstreamTryQuiet/quiet:7.1.0.