feat: add support for ssl cert in websockt #6286
Conversation
WalkthroughAdd WSS/TLS support: URL matching updated for ws/wss; WebSocket event handler fetches certs and proxy config and builds sslOptions; WebSocket client consumes sslOptions for TLS client auth and custom CA; WSS server, fixtures, prefs, and an end-to-end test added. Changes
Sequence DiagramsequenceDiagram
participant UI as User / UI
participant EH as Event Handler\n(ws-event-handlers.js)
participant CU as Cert Utils\n(cert-utils.js)
participant WC as WebSocket Client\n(ws-client.js)
participant WS as WebSocket Server
UI->>EH: Start WSS connection
EH->>CU: getCertsAndProxyConfig()
CU-->>EH: TLS certs + proxy config
EH->>EH: Build sslOptions {ca, cert, key, pfx, passphrase, rejectUnauthorized}
EH->>WC: startConnection(url, { sslOptions, ... })
WC->>WC: Merge sslOptions into wsOptions
WC->>WS: Open WebSocket (wss) using TLS options
WS-->>WC: Accept connection / respond to messages
WC-->>EH: Relay status and messages
EH-->>UI: Connection state and responses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested Reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
🔇 Additional comments (2)
Comment |
pooja-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/bruno-electron/src/ipc/network/cert-utils.js (1)
61-62: Client-cert matching correctly extended to ws/wssIncluding
ws://andwss://in the scheme alternation keeps existing behavior and enables client certificates for secure WebSockets without altering prior HTTP/gRPC matching.packages/bruno-requests/src/ws/ws-client.js (1)
63-73: Consider defaulting/guarding TLS fields and aligning withwsbest practicesRight now all TLS-related fields are passed through directly from
sslOptions, which is fine, but two minor follow-ups are worth considering:
- Explicitly document the
options.sslOptionsshape in the JSDoc forstartConnectionso callers know what is supported.- Optionally default
rejectUnauthorizedwhensslOptionsis omitted or incomplete, to make intended behavior explicit rather than relying on Node/wsdefaults.These are non-blocking but would make the API clearer.
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (1)
256-265: Optional: reuse proxy/certs config more broadlyYou’re calling
getCertsAndProxyConfigbut only usinghttpsAgentRequestFieldsfor the WebSocket handshake. If/when WebSockets also need proxy support or OAuth token flows that respect custom CAs/proxies, consider threadingproxyMode,proxyConfig, andinterpolationOptionsthrough to where they’re needed to avoid duplicating config logic.Not a blocker for this PR, just something to keep in mind.
tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts (1)
7-31: E2E flow is correct; consider adding steps and locator variables for clarityThe test cleanly covers the WSS happy path: open collection, connect, assert connection via the disconnect control, send a message, and assert a headers field in the response. No
waitForTimeoutortest.only, which aligns with the testing guidelines.To make it a bit more robust and readable, you might:
- Introduce locator variables for the request and the final message, e.g.:
const requestItem = page.getByTitle(BRU_REQ_NAME); const lastMessageHeader = locators.messages().nth(2).locator('.text-ellipsis');
- Wrap logical phases in
test.stepblocks (e.g., “open collection”, “connect WSS”, “send message and assert headers”) so the Playwright report is easier to read.These are optional polish items; the core assertions and flow look good.
tests/ssl/custom-ca-certs/server/index.js (2)
46-50: Redundant JSON parsing — useobjdirectly.You've already parsed the JSON into
objon line 30, but line 48 re-parsesBuffer.from(data).toString(). Just use the existingobjvariable.} else { return ws.send(JSON.stringify({ - data: JSON.parse(Buffer.from(data).toString()) + data: obj })); }
24-54: Missing WebSocket error handler.The
wsconnection lacks an'error'event handler. If an error occurs (e.g., client abruptly disconnects), it may throw an unhandled exception and crash the test server.Add an error handler inside the connection callback:
wss.on('connection', function connection(ws, request) { + ws.on('error', function error(err) { + console.error('WebSocket error:', err.message); + }); + ws.on('message', function message(data) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/bruno-electron/src/ipc/network/cert-utils.js(2 hunks)packages/bruno-electron/src/ipc/network/ws-event-handlers.js(1 hunks)packages/bruno-requests/src/ws/ws-client.js(2 hunks)tests/ssl/custom-ca-certs/server/index.js(2 hunks)tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/bruno.json(1 hunks)tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/package.json(1 hunks)tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/ws-ssl-request.bru(1 hunks)tests/ssl/custom-ca-certs/tests/wss-success/init-user-data/preferences.json(1 hunks)tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/ssl/custom-ca-certs/tests/wss-success/init-user-data/preferences.jsontests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.tstests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/bruno.jsontests/ssl/custom-ca-certs/server/index.jstests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/ws-ssl-request.brutests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/package.json
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-electron/src/ipc/network/ws-event-handlers.jstests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.tspackages/bruno-electron/src/ipc/network/cert-utils.jstests/ssl/custom-ca-certs/server/index.jspackages/bruno-requests/src/ws/ws-client.js
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/package.json
📚 Learning: 2025-12-02T07:24:50.299Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.299Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-requests/src/ws/ws-client.js
🧬 Code graph analysis (4)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
packages/bruno-electron/src/ipc/network/index.js (5)
certsAndProxyConfig(116-125)certsAndProxyConfig(146-146)collection(145-145)requestCopy(161-161)requestCopy(162-162)packages/bruno-electron/src/ipc/network/cert-utils.js (3)
getCertsAndProxyConfig(12-119)collection(44-44)httpsAgentRequestFields(26-26)
tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts (2)
tests/utils/page/locators.ts (1)
buildWebsocketCommonLocators(57-77)tests/utils/page/actions.ts (1)
openCollectionAndAcceptSandbox(130-130)
packages/bruno-electron/src/ipc/network/cert-utils.js (1)
packages/bruno-cli/src/runner/run-single-request.js (4)
hostRegex(76-76)hostRegex(261-261)domain(74-74)domain(258-258)
tests/ssl/custom-ca-certs/server/index.js (2)
packages/bruno-tests/src/ws/index.js (1)
ws(1-1)packages/bruno-cli/src/runner/run-single-request.js (3)
request(122-122)request(464-464)url(470-470)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (11)
tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/ws-ssl-request.bru (1)
1-19: Fixture matches test expectationsThe BRU name, WSS URL, and message payload align with the Playwright test and described server behavior; this looks good as the dedicated WSS fixture.
tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/bruno.json (1)
1-6: Collection fixture is minimal and correctCollection metadata is valid and sufficient for the WSS test collection.
tests/ssl/custom-ca-certs/tests/wss-success/init-user-data/preferences.json (1)
1-16: Preferences correctly exercise custom CA behaviorSettings enable SSL verification, use a custom CA, and drop default CAs, which is exactly what this WSS scenario needs.
tests/ssl/custom-ca-certs/tests/wss-success/fixtures/wss-collection/package.json (1)
1-3: Fixture package.json is fineA minimal
nameis sufficient for this collection fixture; nothing else required here.packages/bruno-electron/src/ipc/network/cert-utils.js (1)
118-121: Export style is consistentThe function is still the sole export and the new
module.exports = { getCertsAndProxyConfig };is clear and conventional.packages/bruno-requests/src/ws/ws-client.js (2)
41-44: sslOptions integration keeps API stable and is wired correctlyAccepting
sslOptionsinsideoptionspreserves the publicstartConnectionshape while allowing TLS fields (rejectUnauthorized, ca, cert, key, pfx, passphrase) to flow into the underlyingwsconnection. This is a good extension point for SSL configuration.
108-139: Potential pre-existing bug: use of globalWebSocket.OPEN
queueMessagechecksconnectionMeta.connection.readyState === WebSocket.OPENat line 107, but this module may referencews.WebSocket.OPENelsewhere. In a Node/Electron main-process context,WebSocketis unlikely to be a global, so this can throw or behave inconsistently when the queue is flushed.Even though this is pre-existing, it's worth aligning this line with the rest of the file to avoid runtime issues when messages are queued before the connection opens.
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
247-254: sslOptions mapping is sound and matches TLS expectationsMapping
httpsAgentRequestFieldsintosslOptions(ca, cert, key, pfx, passphrase) and settingrejectUnauthorizedfrompreferencesUtil.shouldVerifyTls()correctly reuses the same CA/client-cert data for WebSockets as for HTTP, while preserving the TLS verification toggle.This is a solid way to centralize TLS behavior across transports.
233-243: FixcollectionPathwhen resolving certificate file paths
getCertsAndProxyConfigexpectscollectionPathto be a directory so it can resolve relativecertFilePath/keyFilePath/pfxFilePathusingpath.join(collectionPath, ...). Here you're passingcollectionPath: collection.pathname, which is likely the full path tobruno.json, not its directory.That will produce paths like
<...>/bruno.json/my-cert.pemfor relative cert paths, causing file reads to fail for WebSocket client certificates even though the same config works for HTTP.Consider aligning this with the HTTP request path and pass the directory instead:
- collectionPath: collection.pathname, + collectionPath: require('node:path').dirname(collection.pathname || ''),This keeps WebSocket certificate resolution consistent with the existing HTTP path.
tests/ssl/custom-ca-certs/server/index.js (2)
56-65: LGTM — upgrade handling is correct.The path check for
/wsand socket destruction for non-matching requests follows the expected pattern fornoServerWebSocket integration.
6-6: Import looks good.Consistent with the existing
requirepattern in this file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/ssl/custom-ca-certs/server/index.js (1)
82-97: Close WebSocket server during shutdown.The shutdown handler closes the HTTPS server but doesn't close the WebSocket server. This could lead to hanging connections or resource leaks in tests.
To properly close the WebSocket server, you'll need to pass it to the shutdown handler. Consider this approach:
In
createServer, return both servers:// At the end of createServer function return new Promise((resolve, reject) => { server.listen(port, (error) => { if (error) { reject(error); } else { resolve({ server, wss }); } }); });Then update
shutdownServer:-function shutdownServer(server, cleanup) { +function shutdownServer(server, wss, cleanup) { const shutdown = (signal) => { console.log(`🛑 Received ${signal}, shutting down`); if (cleanup) cleanup(); + if (wss) { + wss.close(() => { + console.log('WebSocket server closed'); + }); + } + if (server) { server.close(() => process.exit(0)); } else { process.exit(0); } };And update the
startServerfunction accordingly:- const server = await createServer(certsDir, port); + const { server, wss } = await createServer(certsDir, port); - shutdownServer(server, () => { + shutdownServer(server, wss, () => { console.log('✨ Server cleanup completed'); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/ssl/custom-ca-certs/server/index.js(2 hunks)tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
tests/ssl/custom-ca-certs/server/index.js
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/ssl/custom-ca-certs/server/index.js
🧬 Code graph analysis (1)
tests/ssl/custom-ca-certs/server/index.js (1)
packages/bruno-tests/src/ws/index.js (1)
ws(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
tests/ssl/custom-ca-certs/server/index.js (3)
60-69: LGTM!The upgrade handler correctly filters WebSocket connections to
/wspaths and properly integrates the WebSocket server with the HTTPS server using the standardhandleUpgradepattern.
6-6: Verify 'ws' dependency declaration in package.json.The 'ws' package is imported on line 6 but may not be declared in the manifest. Ensure this dependency is added to the appropriate
package.json(root, test directory, or test/ssl directory).
21-58: Replace double quotes with single quotes.The coding guidelines require single quotes for strings. Multiple lines in this segment use double quotes.
Apply this diff:
- // Create WebSocket server for WSS support - const wss = new WebSocket.Server({ noServer: true }); + // Create WebSocket server for WSS support + const wss = new WebSocket.Server({ noServer: true }); wss.on('connection', function connection(ws, request) { ws.on('error', function error(err) { - console.error('WebSocket error:', err.message); + console.error('WebSocket error:', err.message); }); ws.on('message', function message(data) { const msg = Buffer.from(data).toString().trim(); let isJSON = false; let obj = {}; try { obj = JSON.parse(msg); isJSON = true; } catch (err) { // Not a JSON value } if (isJSON) { - if ('func' in obj && obj.func === 'headers') { + if ('func' in obj && obj.func === 'headers') { return ws.send(JSON.stringify({ headers: request.headers })); - } else if ('func' in obj && obj.func === 'query') { - const url = new URL(request.url, `https://${request.headers.host}`); + } else if ('func' in obj && obj.func === 'query') { + const url = new URL(request.url, `https://${request.headers.host}`); const query = Object.fromEntries(url.searchParams.entries()); return ws.send(JSON.stringify({ query: query })); } else { return ws.send(JSON.stringify({ data: obj })); } } return ws.send(Buffer.from(data).toString()); }); });Likely an incorrect or invalid review comment.
1 participant
Description
Add support for ssl cert in websocket
JIRA
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.