feat(grpc): add explicit TLS toggle to URL bar#8443
Open
KyuboLim wants to merge 1 commit into
Open
Conversation
KyuboLim
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
5 tasks
Contributor
WalkthroughAdds a ChangesgRPC TLS toggle implementation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GrpcQueryUrlComponent
participant grpcUrlHelpers
participant ReduxStore
User->>GrpcQueryUrlComponent: edits URL in SingleLineEditor
GrpcQueryUrlComponent->>grpcUrlHelpers: resolveSecureForInput(storedUrl, inputValue)
grpcUrlHelpers-->>GrpcQueryUrlComponent: secure flag
GrpcQueryUrlComponent->>grpcUrlHelpers: setGrpcUrlSecureScheme(url, secure)
grpcUrlHelpers-->>GrpcQueryUrlComponent: finalUrl
GrpcQueryUrlComponent->>ReduxStore: dispatch requestUrlChanged(finalUrl)
User->>GrpcQueryUrlComponent: clicks TLS lock toggle
GrpcQueryUrlComponent->>grpcUrlHelpers: setGrpcUrlSecureScheme(url, !tlsSecure)
grpcUrlHelpers-->>GrpcQueryUrlComponent: updatedUrl
GrpcQueryUrlComponent->>ReduxStore: dispatch requestUrlChanged(updatedUrl)
GrpcQueryUrlComponent->>GrpcQueryUrlComponent: reload reflection methods (if active)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.js`:
- Around line 21-23: The isLocalGrpcHost helper is matching localhost and
127.0.0.1 as raw substrings, which incorrectly treats remote hosts containing
those strings as local. Update isLocalGrpcHost in grpcUrl.js to parse the host
component of the URL and check loopback only on host boundaries, using proper
hostname comparison in the RequestPane/GrpcQueryUrl logic so the TLS/plaintext
default is inferred correctly.
- Around line 79-83: The secure-state resolver is treating unresolved template
values as plaintext, which causes the scheme-less state to be lost. Update
resolveSecureForInput so that when storedUrl or inputValue is a variable-only
placeholder like {{baseUrl}}, it returns an indeterminate/unchanged state
instead of false, and ensure the caller in grpcUrl.js does not rewrite the value
to grpc:// for unresolved variables. Preserve scheme-less behavior for inputs
without an explicit grpc:// or grpcs:// scheme so transport can remain
backend-inferred until the user makes an explicit TLS choice.
- Line 54: Update getDisplayGrpcUrl in grpcUrl.js to strip http:// and https://
in addition to grpc:// and grpcs:// so the display helper always returns a
host-only value. Keep the normalization consistent with hasExplicitGrpcScheme()
and isSecureGrpcUrl() by extending the existing replace logic, and ensure stored
requests using HTTP(S) no longer show a scheme in the editor.
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/index.js`:
- Around line 132-135: The auto-reflection path in GrpcQueryUrl should not call
handleReflection() for scheme-only values like grpc:// or grpcs://. Update the
guard around the existing !protoFilePath / finalUrl check to verify the
displayed host portion is present before switching into reflection mode, and
apply the same fix in the other matching branch mentioned in the comment so
setIsReflectionMode(true) only runs when there is an actual host.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6ec91040-cafa-4367-870b-ff556efa3f51
📒 Files selected for processing (4)
packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.jspackages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.spec.jspackages/bruno-app/src/components/RequestPane/GrpcQueryUrl/index.js
Comment on lines
+21
to
+23
| export const isLocalGrpcHost = (url = '') => { | ||
| const value = url.toLowerCase(); | ||
| return value.includes('localhost') || value.includes('127.0.0.1'); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match loopback on host boundaries, not substrings.
includes('localhost') / includes('127.0.0.1') also classifies remote hosts like api-localhost.example.com as local, so the inferred default flips from TLS to plaintext for a real remote endpoint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.js` around
lines 21 - 23, The isLocalGrpcHost helper is matching localhost and 127.0.0.1 as
raw substrings, which incorrectly treats remote hosts containing those strings
as local. Update isLocalGrpcHost in grpcUrl.js to parse the host component of
the URL and check loopback only on host boundaries, using proper hostname
comparison in the RequestPane/GrpcQueryUrl logic so the TLS/plaintext default is
inferred correctly.
| * Strip a leading grpc:// or grpcs:// for display in the URL editor, so the | ||
| * scheme is controlled by the lock toggle instead of being typed by hand. | ||
| */ | ||
| export const getDisplayGrpcUrl = (url = '') => url.replace(/^(grpc|grpcs):\/\//i, ''); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Strip http(s):// in the display helper too.
hasExplicitGrpcScheme() and isSecureGrpcUrl() both accept http:// / https://, but getDisplayGrpcUrl() leaves those schemes visible. Existing requests stored with http(s) therefore break the new host-only editor contract until the next edit silently normalizes them to grpc(s)://.
Minimal fix
-export const getDisplayGrpcUrl = (url = '') => url.replace(/^(grpc|grpcs):\/\//i, '');
+export const getDisplayGrpcUrl = (url = '') => url.replace(/^(grpc|grpcs|http|https):\/\//i, '');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.js` at
line 54, Update getDisplayGrpcUrl in grpcUrl.js to strip http:// and https:// in
addition to grpc:// and grpcs:// so the display helper always returns a
host-only value. Keep the normalization consistent with hasExplicitGrpcScheme()
and isSecureGrpcUrl() by extending the existing replace logic, and ensure stored
requests using HTTP(S) no longer show a scheme in the editor.
Comment on lines
+79
to
+83
| export const resolveSecureForInput = (storedUrl = '', inputValue = '') => { | ||
| if (hasExplicitGrpcScheme(inputValue)) return isSecureGrpcUrl(inputValue); | ||
| if (hasExplicitGrpcScheme(storedUrl)) return isSecureGrpcUrl(storedUrl); | ||
| return isSecureGrpcUrl(inputValue); | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the scheme-less state for unresolved variables.
This collapses “unknown until runtime” into false. For storedUrl = '{{baseUrl}}' and inputValue = '{{baseUrl}}', the caller ends up rewriting the value to grpc://{{baseUrl}} on the next edit, which turns backend-inferred transport into explicit plaintext even though the user never touched the TLS toggle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/grpcUrl.js` around
lines 79 - 83, The secure-state resolver is treating unresolved template values
as plaintext, which causes the scheme-less state to be lost. Update
resolveSecureForInput so that when storedUrl or inputValue is a variable-only
placeholder like {{baseUrl}}, it returns an indeterminate/unchanged state
instead of false, and ensure the caller in grpcUrl.js does not rewrite the value
to grpc:// for unresolved variables. Preserve scheme-less behavior for inputs
without an explicit grpc:// or grpcs:// scheme so transport can remain
backend-inferred until the user makes an explicit TLS choice.
Comment on lines
+132
to
135
| if (!protoFilePath && finalUrl) { | ||
| setIsReflectionMode(true); | ||
| handleReflection(finalUrl); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Check for an actual host before auto-refreshing reflection.
finalUrl can be just grpc:// / grpcs:// after toggling an empty field, so these truthiness checks call handleReflection() with a scheme-only URL even though the editor still shows no host. Gate this on the displayed host portion instead of raw string truthiness.
Minimal guard
- if (!protoFilePath && finalUrl) {
+ if (!protoFilePath && getDisplayGrpcUrl(finalUrl)) {
setIsReflectionMode(true);
handleReflection(finalUrl);
}
…
- if (isReflectionMode && finalUrl) {
+ if (isReflectionMode && getDisplayGrpcUrl(finalUrl)) {
handleReflection(finalUrl);
}Also applies to: 256-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/index.js` around
lines 132 - 135, The auto-reflection path in GrpcQueryUrl should not call
handleReflection() for scheme-only values like grpc:// or grpcs://. Update the
guard around the existing !protoFilePath / finalUrl check to verify the
displayed host portion is present before switching into reflection mode, and
apply the same fix in the other matching branch mentioned in the comment so
setIsReflectionMode(true) only runs when there is an actual host.
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
1 participant
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.
feat(grpc): explicit TLS toggle in the gRPC URL bar
What
Adds a lock/unlock toggle to the gRPC URL bar so users can choose TLS
(
grpcs://) vs plaintext (grpc://) explicitly, instead of having to know totype the scheme by hand.
Closes #6950 (the auto-prefix confusion is resolved by making the scheme visible
and controllable).
Why
TLS for gRPC is currently controlled only by the URL scheme, which is
auto-applied and effectively hidden. Users — especially those coming from
Postman — don't discover it and hit failures. #6950 has been open since v2.13.2
and still reproduces on v3.5.0; the community workaround is to manually prepend
grpc://.How
grpcUrl.jswith pure, unit-tested helpers for scheme/TLSderivation.
GrpcQueryUrl:host:portonly; the scheme is toggle-controlled.request.urlas before.grpc-client.jsalready derives TLS and the grpcurl-plaintextflag from the scheme..bruformat change.Behavior (mirrors the existing backend)
localhost/127.0.0.1=plaintext, otherwise TLS), matching
getParsedGrpcUrlObject.{{baseUrl}}…) → toggle stays available; the lockdefaults to plaintext and toggling writes an explicit
grpc://{{var}}/grpcs://{{var}}.Tests
grpcUrl.spec.js— 17 cases covering scheme detection, host inference,display stripping, scheme rewriting, variable handling, and input resolution.
npx jest src/components/RequestPane/GrpcQueryUrl/grpcUrl.spec.js→ 17 passed.Screenshots
Checklist
npm run test(bruno-app) passesUpdated the gRPC URL editor to make TLS/plaintext selection explicit in the UI.
Added a new
grpcUrlhelper module inpackages/bruno-appto:grpc:///grpcs://on demand,Updated
GrpcQueryUrlto use those helpers so:host:portinstead of a user-facing scheme,request.urlstill preserves the selected scheme,Added unit tests covering host inference, variable handling, scheme detection, URL display normalization, scheme rewriting, and secure-state resolution.
Added styling for the new TLS toggle.
No backend, schema, or
.bruformat changes were included.