Revert Fastify 5 upgrade to fix WebSocket crash and 415 PATCH errors - #7384
Revert Fastify 5 upgrade to fix WebSocket crash and 415 PATCH errors#7384lucferbux wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
beda146 to
aa6d3c1
Compare
Commit 452e6d2 upgraded Fastify 4→5 and all @fastify/* plugins for security fixes. This introduced two regressions in v2.25.5: 1. @fastify/websocket v8→v11 removed the SocketStream wrapper. The WebSocket proxy code does `connection.socket` which returns undefined in v11, breaking all WebSocket connections and crashing the pod for non-admin users (RHOAISUP-24). 2. Fastify 5 content-type parser behavior change causes FST_ERR_CTP_INVALID_MEDIA_TYPE for application/merge-patch+json, preventing workbench start/stop operations. This reverts to Fastify 4.29.1 (latest 4.x, fixes CVE-2025-32442) and matching @fastify/* plugin versions compatible with Fastify 4. RHOAIENG-59422
aa6d3c1 to
94de8f9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## stable-2.x #7384 +/- ##
==============================================
- Coverage 67.93% 67.92% -0.01%
==============================================
Files 2238 2238
Lines 50847 50847
Branches 14252 14252
==============================================
- Hits 34541 34540 -1
- Misses 16306 16307 +1
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
Closing this in favor of #7387 |
Summary
Reverts the Fastify 4→5 upgrade introduced in commit
452e6d230("Fix security vulnerabilities for rhoai-2.25.4") which shipped in v2.25.5. That upgrade caused two regressions:1. WebSocket crash (RHOAISUP-24, RHOAIENG-59422)
@fastify/websocketv8→v11 removed theSocketStreamwrapper. The WebSocket proxy handler doesconnection.socketwhich returnsundefinedin v11 — every WebSocket connection is broken. For non-admin users, the orphaned target WS connection gets a 403 from K8s API, emitting an unhandlederrorevent that crashes the Node.js process (exit code 1) or causes OOMKill from accumulated leaked connections (exit code 137).Why e2e with cluster-admin didn't catch it: the
TypeErroris inside a.then()(unhandled rejection, doesn't crash), and admin users' WS upgrades succeed (noerrorevent), so the proxy is silently broken but the UI loads fine via REST.2. 415 on PATCH requests
Fastify 5 content-type parser behavior change causes
FST_ERR_CTP_INVALID_MEDIA_TYPEforapplication/merge-patch+json, preventing workbench start/stop operations.Fix
Revert to
fastify@4.29.1(latest 4.x, fixes CVE-2025-32442) and matching@fastify/*plugin versions:fastify^5.8.2^4.29.1@fastify/websocket^11.2.0^8.3.1@fastify/http-proxy^11.4.1^9.5.0@fastify/static^9.0.0^6.10.2@fastify/*Also reverts
loggerInstance→logger(Fastify 4 API) andFastifyHttpProxyOptions→FastifyReplyFromHookstype imports.CVE note
Fastify 4.29.1 fixes CVE-2025-32442 (High). Three Fastify-5-only CVEs remain unpatched (CVE-2026-3635, CVE-2026-25224, CVE-2026-33806) — all are Content-Type validation bypass. The dashboard backend doesn't use Fastify's built-in content-type schema validation and runs behind oauth-proxy, so practical risk is very low.
Test plan
npm run build)npm run type-check)npm run test:jest)npm run test:lint)wss://connections)merge-patch+json)