WebSockets, WebRTC, and Peering - #54
Conversation
… feature/v0.1.0-RC1
left a comment
•
There was a problem hiding this comment.
Stale comment
Security review complete for PR #54.
Validated prior automation findings against current diff:
- The previous bearer-token forgery issue no longer applies:
verifyBearerTokennow fails closed when no secret is configured (missing_secret) and does not use a hardcoded fallback secret.- The prior WebSocket
JSONCallauth-bypass finding no longer applies: theJSONCallpath now checks transport authorization before invoking_handleCall.No high-confidence medium/high/critical vulnerabilities were confirmed in the added/modified code paths reviewed.
Slack summary: PR #54 security pass completed; no medium+ vulnerabilities currently confirmed.
Sent by Cursor Automation: Find vulnerabilities
left a comment
•
There was a problem hiding this comment.
Stale comment
Revalidated prior security findings from earlier automation runs against current HEAD:
- The hardcoded bearer-token fallback secret issue in
middlewares/auth.jsis fixed.- The earlier unconditional WebSocket
JSONCallauth bypass is partially mitigated, but one medium auth-control issue remains.Slack summary: 1 medium security finding remains in WebSocket auth gating (
types/server.js) due fail-open behavior when client-token auth is marked required but no secret is configured.Sent by Cursor Automation: Find vulnerabilities
| const wsCfg = this.settings.websocket || {}; | ||
| const secret = wsCfg.clientToken || wsCfg.sharedSecret || null; | ||
| const required = wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1; | ||
| if (!required || !secret) return true; |
There was a problem hiding this comment.
Medium severity – fail-open WebSocket auth control.
When requireClientToken is enabled but clientToken/sharedSecret is missing, this branch returns true and accepts the connection. That lets unauthenticated clients open WebSocket sessions and hit message handlers without the intended token boundary (including state-changing handlers and JSONCall when RPC auth is off). This should fail closed (return false) when token auth is required but no secret is configured.
left a comment
•
There was a problem hiding this comment.
Stale comment
Security review update after validating prior unresolved security threads against current head.
Findings (current)
- High – Unauthenticated WebSocket
JSONCallcan execute registered RPC methods whenjsonRpcauth mode is not explicitly enabled.- Medium – WebSocket client-token enforcement is fail-open when
requireClientToken=truebut token secret is missing/misconfigured.Validation notes on prior security findings
- Prior hardcoded bearer fallback-secret finding in
middlewares/auth.jsno longer applies on this head.- Remaining issues above are still exploitable with plausible network attack paths.
Slack summary:
PR #54 security review -> REQUEST_CHANGES. 2 active vulns: (1) High unauthenticated WS JSONCall method execution path, (2) Medium fail-open WS token gate on misconfiguration. Hardcoded bearer fallback secret issue appears fixed.Sent by Cursor Automation: Find vulnerabilities
| } | ||
|
|
||
| const kernel = new Actor(jsonCallPayload); | ||
| const result = await server._handleCall({ |
There was a problem hiding this comment.
High: attacker-controlled JSONCall input reaches _handleCall(...) without guaranteed auth. socket._fabricJsonRpcTransportAuthorized is set to true unless jsonRpc.enabled===true && jsonRpc.requireAuth===true, so default/legacy configs allow unauthenticated WebSocket clients to invoke registered RPC handlers. Impact: authz bypass for state-changing/admin methods exposed via _registerMethod.
| const wsCfg = this.settings.websocket || {}; | ||
| const secret = wsCfg.clientToken || wsCfg.sharedSecret || null; | ||
| const required = wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1; | ||
| if (!required || !secret) return true; |
There was a problem hiding this comment.
Medium: fail-open token boundary. When requireClientToken is enabled but clientToken/sharedSecret is absent or misconfigured, this returns true and accepts the connection. Impact: operators can believe WS token auth is enforced while unauthenticated clients are still admitted.
left a comment
•
There was a problem hiding this comment.
Stale comment
Security review update for PR #54:
Validated prior unresolved security findings against the latest head. Previously reported hardcoded bearer fallback-secret and unauthenticated JSON-RPC transport issues have been addressed. However, two authentication-boundary issues still allow unauthenticated WebSocket access in realistic configurations.
Slack summary: Requesting changes due to 1 high and 1 medium auth bypass risk in
types/server.js(WebSocket token enforcement can be bypassed whensharedSecretis used or when token enforcement is enabled without a configured secret).Sent by Cursor Automation: Find vulnerabilities
| }); | ||
| const wsOpts = { server: this.http }; | ||
| const wsCfg = this.settings.websocket || {}; | ||
| if (wsCfg.clientToken && (wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1)) { |
There was a problem hiding this comment.
High severity – WebSocket auth bypass via config mismatch.
Handshake auth is only enabled when wsCfg.clientToken is present, but this PR also treats wsCfg.sharedSecret as a valid token secret in _verifyWebSocketClient and _isJsonRpcTransportAuthorized. A deployment that sets websocket.requireClientToken=true with websocket.sharedSecret will not attach verifyClient, so unauthenticated clients can still complete the WebSocket handshake and reach message handlers (POST/PATCH/Call) without presenting a token.
| const wsCfg = this.settings.websocket || {}; | ||
| const secret = wsCfg.clientToken || wsCfg.sharedSecret || null; | ||
| const required = wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1; | ||
| if (!required || !secret) return true; |
There was a problem hiding this comment.
Medium severity – fail-open token gate when auth is required.
_verifyWebSocketClient returns true when requireClientToken is enabled but the secret is missing (!required || !secret). This fails open instead of denying access, so a misconfigured deployment silently accepts unauthenticated WebSocket clients that can invoke server message handlers and mutate/read application state.
left a comment
•
There was a problem hiding this comment.
Stale comment
Security review result: 1 High severity vulnerability remains after re-validating prior security threads.
- Revalidated and no longer reproducible: earlier hardcoded bearer-secret issue in
middlewares/auth.js(now fails closed when secret is missing).- Still reproducible: WebSocket client-token enforcement can be bypassed when
sharedSecretis configured withoutclientToken.Slack summary:
PR #54 security review -> 1 High finding (WebSocket auth bypass via config mismatch in handshake guard). Requesting changes.Sent by Cursor Automation: Find vulnerabilities
| }); | ||
| const wsOpts = { server: this.http }; | ||
| const wsCfg = this.settings.websocket || {}; | ||
| if (wsCfg.clientToken && (wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1)) { |
There was a problem hiding this comment.
High – authentication bypass: this guard only enables verifyClient when websocket.clientToken is set, but _verifyWebSocketClient() also accepts websocket.sharedSecret. If deployers set requireClientToken: true with only sharedSecret, handshake verification is never attached, so unauthenticated attackers can open WebSocket connections and invoke privileged message handlers (state-changing POST/PATCH, and JSONCall where transport auth is not otherwise required).
left a comment
There was a problem hiding this comment.
Validated prior security-review threads against the current PR head and re-tested the auth paths. One high-confidence vulnerability still applies.
Please address the inline finding before merge; it allows unauthenticated WebSocket access in a common token-auth configuration.
Sent by Cursor Automation: Find vulnerabilities
| }); | ||
| const wsOpts = { server: this.http }; | ||
| const wsCfg = this.settings.websocket || {}; | ||
| if (wsCfg.clientToken && (wsCfg.requireClientToken === true || wsCfg.requireClientToken === '1' || wsCfg.requireClientToken === 1)) { |
There was a problem hiding this comment.
High severity — WebSocket authentication bypass. verifyClient is only enabled when wsCfg.clientToken is set, but the verifier itself accepts either wsCfg.clientToken or wsCfg.sharedSecret as the token secret. With websocket.requireClientToken=true and only websocket.sharedSecret configured, the handshake verifier is never attached, so unauthenticated clients can open WebSocket sessions and reach message handlers (POST/PATCH/Call, and JSONCall when transport auth is not otherwise enforced).


General pass on WebSockets.
Improvements:
MessagespecificiationRelease Gate:
@fabric/browserand/or@fabric/extensionimplement passing tests for auth flowNote
High Risk
Adds new authentication token parsing/verification and role-authorization logic, plus a new payments gate that can block requests with
402; these changes affect security and request handling behavior. CI/runtime upgrade to Node 22 may surface compatibility issues in dependencies/tests.Overview
Upgrades CI/runtime to Node.js
22.14.0by updating.nvmrc,.travis.yml, and moving GitHub Actions toactions/setup-node@v4.Introduces bearer-token based auth context in
middlewares/auth.js: verifies arequest.tokenusing a SHA-256 signature scheme with timing-safe comparison, setsrequest.authenticated, and exposes decodedtokenHeader/tokenPayloadplustokenError.contracts/hasRole.jsis updated to authorize viatokenPayload.roleortokenPayload.roles(and removes noisy debug logging).Adds a new
paymentsmiddleware that creates a Bitcoin invoice and returns402 Payment Required(currently unconditional), and includes minor UI/dev changes: addsAPI.mddocumentation output, cleans a bridge debug log, and adds_handleRemoteReadyinFabricBridgethat fetchesbtc_getbalanceson remote readiness.Written by Cursor Bugbot for commit c692d0b. Configure here.
Summary by CodeRabbit