Repository navigation
WebSocket backport - #22983
Merged
Merged
WebSocket backport#22983
Conversation
Backport the recovery behavior from 2b3e6cb to Q2 without unrelated Q3 refactors. Report CLOSED and flush the notification queue after catch-up fails, while ignoring failures from aborted connections. Add regression coverage that preserves the socket lock and prevents a false LIVE state.
Backport 040b9a9 without regressing Q2 replacement of CLOSED wrappers. Prevent competing notification connections and cover repeated connect calls with the echo-server regression suite.
Backport Q3 transport recovery through b471752: replace stale wrappers after wake, ignore stale events, recover CONNECTING wrappers with a watchdog started after URL resolution, reconnect after ping timeout, and preserve the browser constructor fallback. Include the PartySocket 1.3.0 migration and deterministic regression coverage. Preserve Q2 constructor defaults and native optional values.
Backport 4e4e3f1, d8f3fa6 and b471752: share in-flight token refreshes, retry transient failures with bounded backoff, stop on invalid sessions, and verify the authenticated cookies endpoint before creating the socket URL. Forward long-running retry events and redact access tokens from URL logging. Preserve existing constructor calls and add regression coverage.
Backport application recovery from dce69a6 and 87d67ff. Track CLOSED notification state, probe on browser visibility and Electron focus, check health periodically, suppress overlapping reconnects and probes, and remove listeners and heartbeat timers on disconnect. Add deterministic orchestration tests. Validation: focused tests and ESLint, full app type-check and production build passed. Skip the pre-commit hook because its project-wide lint includes existing generated dist bundles.
Backport the client-update retry override from dce69a6. Disable automatic retries for capability updates so account listening can fail and app reconnect orchestration can retry setup instead of waiting indefinitely. Cover the outgoing retry configuration and error propagation.
Keep the backported heartbeat from sending protocol-invalid application pings to async notification sockets. Preserve state-based health checks and legacy ping/pong probes. Add a regression test and declare globalThis for the Q2 ESLint environment.
Adapt Q3 f1959ba and 0162f73 to Q2 without the commons migration. Carry attempt and wrapper metadata through transport, authentication, notification processing, and application recovery. Omit untrusted messages, URLs, markers, and payloads instead of relying on partial redaction. Preserve connection control flow and unsupported-ping safeguards.
Adapt eafae9d to Q2 with an explicit WebSocket source and telemetry dependency. Preserve consent, environment and forced-reporting rules, suppress reporting during unconsented device-ID migrations, deduplicate initialization, and remove retry listeners when disabled. Scope account disconnect cleanup to its listeners so application recovery preserves reporting subscriptions. Include focused lifecycle tests without importing HTTP retry telemetry.
Backport the Account payload guards present in the Q3 snapshot via d304d22. Continue processing and acknowledging empty legacy and async notifications without trying to read a missing first event. Cover both notification paths with focused tests.
|
Semgrep found 46
🟠 High severity issue identified in your code: Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. |
![semgrep-code-wireapp[bot]](https://avatars.githubusercontent.com/u/16047324?s=60&v=4){kind=small}
| } | ||
|
|
||
| async function startEchoServer(): Promise<WebSocketServer> { | ||
| const server = new WebSocketServer({host: '127.0.0.1', port: 0}); |
There was a problem hiding this comment.
🟠 High severity and reachable issue identified in your code:
Line 152 has a potentially vulnerable usage of ws, possibly introducing a high severity vulnerability.
ℹ️ Why this is reachable
A reachable issue is a possible security risk because your project actually executes the vulnerable code. This issue is reachable because your code calls vulnerable code in this version of ws.
Affected versions of ws are vulnerable to Allocation of Resources Without Limits or Throttling / Excessive Platform Resource Consumption within a Loop / Uncontrolled Resource Consumption. An attacker can cause memory exhaustion and out-of-memory process termination in ws by sending a high volume of exceptionally small WebSocket fragments and data chunks, forcing excessive allocation of internal frame-buffer wrappers beyond the documented maxPayload limit. The flaw resides in ws internal frame-reassembly logic, so any application that uses ws to send or receive WebSocket traffic is affected regardless of how the API is called.
References: https://euvd.enisa.europa.eu/vulnerability/EUVD-2026-37504, GHSA, CVE
To resolve this comment:
Upgrade this dependency to at least version 8.21.0 at yarn.lock.
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
Contributor
|
🔗 Download Full Report Artifact 🧪 Playwright Test Summary
specs/CriticalFlow/personalAccountLifecycle-TC-8638.spec.ts (❌ 0 failed,
|
Restore the initial LIVE readiness contract with cancellation and a bounded timeout while keeping background recovery asynchronous. Wait for the asynchronous synchronization marker after legacy migration, ignore aborted catch-up, and register and remove only owned listeners. Cover readiness, overlap, setup failures, and external subscriptions. Focused core and app tests, type checks, and lint passed; run checks directly because the existing hook scans generated bundles.
Scope reconnect URL providers to wrapper lifetimes, cancel retry delays and HTTP preflight on supersession, and prevent cancelled attempts from generating synchronization markers. Keep token refresh single-flight across active attempts, abort it on disconnect, and prevent late responses from saving tokens or causing logout. Cover cancellation, reverse authentication completion, marker readiness, and delayed URL callbacks. API-client tests, focused core tests, library type checks, and focused lint passed.
Restore default retries for prekey and MLS public-key uploads. Route capability registration through an explicit API operation with retries disabled, preserving existing ordinary putClient callers. Focused API-client and Account tests, library type checks, and lint passed.
Clarify that ping-disabled OPEN sockets provide local-state evidence only. Preserve the existing protocol safeguard and recovery mechanisms instead of introducing unsupported heartbeat messages. Existing asynchronous no-ping and transport recovery tests pass.
The Q2 workflow resolved ubuntu-latest to 26.04, where Playwright 1.57.0 cannot install ffmpeg. Pin only the browser-execution job to an image supported by the locked Playwright version. Validate YAML locally; hosted execution requires a subsequent CI run.
Replace non-network dummy ws URLs with wss URLs without changing the loopback echo-server protocol or suppressing Semgrep rules. All API-client tests and focused lint pass. The separately reported ws dependency advisory is already present in the Q2 baseline.
Do not react to a late unauthorized response after request cancellation, and propagate the reconnect cancellation signal through automatic token recovery. The regression test failed before the guard and now passes; focused HTTP tests, API-client type checking, and lint passed.
Forward signals supplied through HTTP request configuration when authentication recovery starts a token refresh. Keep cancellation effective through the cookie request adapters without unsafe casts. The regression failed before the fix; focused HTTP tests, API-client type checking, and lint pass.
Keep notification readiness pending across transient closures and recoverable setup failures using the existing heartbeat and online recovery. Cancel superseded setup and distinguish invalid sessions through an owned logout subscription. Retain the bounded five-minute deadline. Focused application and core tests, type checks, and lint pass.
Notify connection invalidation explicitly when connecting, replacing wrappers, and reconnecting in place. Abort the previous processing lifetime, flush queued work, and guard late legacy and async completion before event delivery, acknowledgements, unlock, or LIVE. Cover the already-closing PartySocket case, replacement synchronization, and repeated cleanup without removing external listeners. API-client and core tests, type checks, and focused lint pass.
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.
No description provided.