Repository navigation
Conversation
Co-authored-by: sandstrom <122287+sandstrom@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 707aed3. Configure here.
| sidecar = URI(@sidecar_url) | ||
| connection = ::Net::HTTP.new(sidecar.hostname, sidecar.port, nil) | ||
| connection.use_ssl = false | ||
| connection.use_ssl = sidecar.scheme == "https" |
There was a problem hiding this comment.
HTTPS skips transport SSL configuration
Medium Severity
conn enables SSL for https sidecar URLs but never applies ssl_configuration. ssl_verification, ssl_ca_file, and transport.ssl are ignored, so the connection keeps Net::HTTP's default peer verification and system trust store. Private CAs and disabled verification, typical for Docker or an intercepting proxy, fail for Spotlight while the main transport honors the same settings.
Reviewed by Cursor Bugbot for commit 707aed3. Configure here.
There was a problem hiding this comment.
I'm not sure I see the security issue here:
Private CAs and disabled verification, typical for Docker or an intercepting proxy, fail for Spotlight while the main transport honors the same settings.
While this is true, it's not a security issue (on the contrary, disabling verification would be), and more of a feature limitation.
This change would only make vanilla HTTPS work, not special cases with Private CAs.
That said, if you as maintainers prefer another approach, let me know or feel free to amend this PR.


Description
Add support for HTTPs in spotlight transport. Useful when running spotlight in docker container or behind proxy for example.
Fixes: #3128