fix: cli proxy config updates#6846
Conversation
WalkthroughCentralizes proxy config migration into a new exported Changes
Sequence Diagram(s)(Skipped — changes are internal refactor and do not introduce multi-component sequential flows warranting a diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
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/run-single-request.js (1)
316-327: Avoid sending empty proxy credentials when auth isn’t configured.With old configs that omit auth,
auth.disabledis undefined, soproxyAuthEnabledbecomes true and the URI includes empty credentials. This can unintentionally trigger proxy auth with blank values. Gate auth on the presence of credentials (or explicit disablement).✅ Suggested fix
- const proxyAuthEnabled = !get(proxyConfig, 'auth.disabled', false); + const proxyAuthUsername = interpolateString(get(proxyConfig, 'auth.username', ''), interpolationOptions); + const proxyAuthPassword = interpolateString(get(proxyConfig, 'auth.password', ''), interpolationOptions); + const hasAuthCredentials = Boolean(proxyAuthUsername || proxyAuthPassword); + const proxyAuthEnabled = !get(proxyConfig, 'auth.disabled', false) && hasAuthCredentials; const socksEnabled = proxyProtocol.includes('socks'); let uriPort = isUndefined(proxyPort) || isNull(proxyPort) ? '' : `:${proxyPort}`; let proxyUri; if (proxyAuthEnabled) { - const proxyAuthUsername = encodeURIComponent(interpolateString(get(proxyConfig, 'auth.username'), interpolationOptions)); - const proxyAuthPassword = encodeURIComponent(interpolateString(get(proxyConfig, 'auth.password'), interpolationOptions)); - - proxyUri = `${proxyProtocol}://${proxyAuthUsername}:${proxyAuthPassword}@${proxyHostname}${uriPort}`; + const encodedUsername = encodeURIComponent(proxyAuthUsername); + const encodedPassword = encodeURIComponent(proxyAuthPassword); + proxyUri = `${proxyProtocol}://${encodedUsername}:${encodedPassword}@${proxyHostname}${uriPort}`; } else { proxyUri = `${proxyProtocol}://${proxyHostname}${uriPort}`; }
3960757 to
6025bfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/bruno-cli/src/runner/run-single-request.js`:
- Around line 354-357: The code logs the raw http_proxy value (which may contain
credentials) before setting request.httpAgent; remove the console.log({
http_proxy }) to avoid leaking secrets, or if logging is required use a
sanitized form (e.g., mask credentials) before logging. Update the block that
references http_proxy, new URL(http_proxy), and request.httpAgent = new
HttpProxyAgent(http_proxy) to no longer print the full http_proxy value to
stdout.
🧹 Nitpick comments (1)
packages/bruno-requests/src/utils/proxy-util.ts (1)
40-44: Guard against legacy configs missingenabled.Migration only runs when
enabledexists. If any persisted legacy configs omitenabled, they’ll bypass migration and downstream code will treat old shape as new (e.g.,auth.disabledwon’t be set). Please verify whetherenabledis always present in stored legacy configs; if not, consider broadening old-format detection (e.g., treat configs withoutconfigas old).🔧 Possible adjustment
-export const transformProxyConfig = (proxy: OldProxyConfig | NewProxyConfig | null | undefined): NewProxyConfig | OldProxyConfig => { +export const transformProxyConfig = (proxy: OldProxyConfig | NewProxyConfig | null | undefined): NewProxyConfig | OldProxyConfig => { proxy = proxy || {}; // Check if this is an old format (has 'enabled' property) - if (proxy.hasOwnProperty('enabled')) { + const isOldFormat = Object.prototype.hasOwnProperty.call(proxy, 'enabled') + || !Object.prototype.hasOwnProperty.call(proxy, 'config'); + if (isOldFormat) { const oldProxy = proxy as OldProxyConfig; const enabled = oldProxy.enabled;
* fix: cli `proxy config` updates * fix: review comment fixes
Description
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
✏️ Tip: You can customize this high-level summary in your review settings.