-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: update WSSE password digest calculation to use base64-encoded nonce #6566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…once and improve security in prepare-request.js
WalkthroughWSSE authentication digest computation is corrected across CLI and Electron implementations. Nonce generation now uses raw 16-byte buffers, digest calculation concatenates bytes directly, and both nonce and digest are properly base64-encoded. Test expectations updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-cli/src/runner/prepare-request.js (1)
209-224: Critical: Request-level WSSE path was not updated.The request-level WSSE authentication (lines 209-224) still uses the old buggy implementation with hex-encoded nonce and triple-encoding. This will cause authentication failures against standards-compliant WSSE servers when users configure WSSE directly on a request rather than inheriting from collection.
Compare with
packages/bruno-electron/src/ipc/network/prepare-request.jslines 284-301 which correctly updates both paths.🔎 Proposed fix to align with the corrected implementation
if (request.auth.mode === 'wsse') { const username = get(request, 'auth.wsse.username', ''); const password = get(request, 'auth.wsse.password', ''); const ts = new Date().toISOString(); - const nonce = crypto.randomBytes(16).toString('hex'); + const nonceBytes = crypto.randomBytes(16); // Create the password digest using SHA-1 as required for WSSE - const hash = crypto.createHash('sha1'); - hash.update(nonce + ts + password); - const digest = Buffer.from(hash.digest('hex').toString('utf8')).toString('base64'); + // WSSE spec: PasswordDigest = Base64(SHA1(nonce_bytes + created_bytes + password_bytes)) + const passwordDigest = crypto + .createHash('sha1') + .update( + Buffer.concat([nonceBytes, Buffer.from(ts, 'utf8'), Buffer.from(password, 'utf8')]) + ) + .digest('base64'); + + // Nonce must be base64-encoded in the header + const nonce = nonceBytes.toString('base64'); // Construct the WSSE header axiosRequest.headers[ 'X-WSSE' - ] = `UsernameToken Username="${username}", PasswordDigest="${digest}", Nonce="${nonce}", Created="${ts}"`; + ] = `UsernameToken Username="${username}", PasswordDigest="${passwordDigest}", Nonce="${nonce}", Created="${ts}"`; }
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/prepare-request.js (1)
284-301: WSSE request-level auth correctly implements the spec.Both WSSE paths in this file are now consistent and compliant.
The static analysis tool flags
constdeclarations inside switch cases without blocks. Consider wrapping the case body in braces for lexical scoping correctness, though this is a pre-existing pattern in the file.🔎 Optional: wrap case body in block to satisfy linter
case 'wsse': + { const username = get(request, 'auth.wsse.username', ''); const password = get(request, 'auth.wsse.password', ''); // ... rest of implementation axiosRequest.headers[ 'X-WSSE' ] = `UsernameToken Username="${username}", PasswordDigest="${passwordDigest}", Nonce="${nonce}", Created="${ts}"`; break; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-cli/src/runner/prepare-request.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-electron/tests/prepare-request.test.js
🧰 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. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-electron/tests/prepare-request.test.jspackages/bruno-cli/src/runner/prepare-request.js
**/*.test.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Files:
packages/bruno-electron/tests/prepare-request.test.js
🧠 Learnings (2)
📚 Learning: 2025-12-02T07:24:50.311Z
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.311Z
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-electron/src/ipc/network/prepare-request.jspackages/bruno-cli/src/runner/prepare-request.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-electron/tests/prepare-request.test.jspackages/bruno-cli/src/runner/prepare-request.js
🧬 Code graph analysis (1)
packages/bruno-electron/src/ipc/network/prepare-request.js (1)
packages/bruno-cli/src/runner/prepare-request.js (9)
nonceBytes(155-155)crypto(6-6)passwordDigest(159-164)ts(154-154)ts(213-213)password(152-152)password(211-211)nonce(167-167)nonce(214-214)
🪛 Biome (2.1.2)
packages/bruno-electron/src/ipc/network/prepare-request.js
[error] 52-52: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 56-61: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 64-64: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 284-284: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 288-293: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 296-296: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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). (5)
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (4)
packages/bruno-cli/src/runner/prepare-request.js (1)
155-172: WSSE collection-level auth correctly implements the spec.Binary nonce generation, byte-level concatenation via
Buffer.concat, and direct base64 encoding of the SHA-1 digest — this aligns with OASIS WS-Security UsernameToken Profile 1.1.packages/bruno-electron/src/ipc/network/prepare-request.js (1)
52-69: WSSE collection-level auth correctly implements the spec.Binary nonce, byte-level concatenation, and direct base64 encoding — properly aligned with OASIS WS-Security UsernameToken Profile 1.1.
packages/bruno-electron/tests/prepare-request.test.js (2)
184-184: Test expectation correctly updated for base64 nonce.The mock returns
Buffer.from('1234567890abcdef', 'hex')which base64-encodes toEjRWeJCrze8=. This validates that the implementation now outputs base64-encoded nonces in the header.
574-574: Request-level WSSE test expectation correctly updated.Consistent with the collection-level test update.
fixes: #6482
Description
The WSSE implementation had three critical bugs that violated the OASIS WS-Security UsernameToken Profile 1.1 specification, causing authentication to fail with any standards-compliant WSSE server.
Problems
Bruno's WSSE authentication was generating:
3 bugs fixed:
Buffer.concat()Result:
Contribution Checklist:
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.