Skip to content

feat: add support for SSL certificate verification in CLI and connection#47

Open
srimon12 wants to merge 2 commits into
pavanjava:mainfrom
srimon12:feat/ssl-cert-support
Open

feat: add support for SSL certificate verification in CLI and connection#47
srimon12 wants to merge 2 commits into
pavanjava:mainfrom
srimon12:feat/ssl-cert-support

Conversation

@srimon12
Copy link
Copy Markdown
Collaborator

@srimon12 srimon12 commented May 27, 2026

fix #46
fix: support custom TLS verification for Qdrant connections

Add TLS verification controls for HTTPS Qdrant deployments that use
self-signed certificates or internal CAs.

Changes:

  • Add qql connect --no-verify for trusted internal/self-signed setups
  • Add qql connect --ca-cert <path> for custom CA bundles
  • Store the resolved CA bundle path in ~/.qql/config.json
  • Reject conflicting --ca-cert and --no-verify usage
  • Persist verify: bool | str in QQLConfig with backwards-compatible defaults
  • Propagate saved verification settings through connect, execute, dump,
    REPL startup, Connection, and run_query
  • Document CLI, config, and programmatic usage
  • Add regression coverage for connect and execute flows

Closes #46

Summary by CodeRabbit

  • New Features

    • Add TLS verification controls: --verify/--no-verify and --ca-cert, persisted in config.
    • Programmatic/API support for verify (accepts True/False or CA path) for one-shot and persistent connections.
  • Documentation

    • New guides for HTTPS with internal/self-signed certificates and guidance on --no-verify.

Review Change Stack

@srimon12 srimon12 requested a review from pavanjava May 27, 2026 06:33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@src/qql/cli.py`:
- Around line 205-207: The temporary QdrantClient created for connectivity
validation (the instantiation of QdrantClient followed by
client.get_collections()) is never closed and can leak sockets; update the check
to either use a context manager (with QdrantClient(...) as client:
client.get_collections()) or ensure client.close() is called in a finally block
before proceeding to launch the REPL (references: QdrantClient and
client.get_collections()) so the probe client is always cleaned up.
🪄 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 Plus

Run ID: 9b21a542-7185-47e5-b581-57ca791b0f5e

📥 Commits

Reviewing files that changed from the base of the PR and between dbb6713 and 3f5d457.

📒 Files selected for processing (10)
  • README.md
  • docs/getting-started.md
  • docs/programmatic.md
  • docs/reference.md
  • src/qql/__init__.py
  • src/qql/cli.py
  • src/qql/config.py
  • src/qql/connection.py
  • tests/test_cli.py
  • tests/test_connection.py

Comment thread src/qql/cli.py
Repository owner deleted a comment from coderabbitai Bot May 27, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d3e060c3-e2cf-43ff-8e7e-fb3771ded78b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5d457 and 385e161.

📒 Files selected for processing (1)
  • src/qql/cli.py

📝 Walkthrough

Walkthrough

Adds a configurable TLS verification setting: a verify field in persisted config, Connection.__init__ and run_query() accept verify, the CLI connect command gains --verify/--no-verify and --ca-cert, downstream commands use saved verify, and docs/tests updated accordingly.

Changes

TLS Verification Control

Layer / File(s) Summary
Configuration model
src/qql/config.py
QQLConfig gains a new verify field (default True), and load_config() reads the value from persisted JSON with backward-compatible fallback.
Core APIs: Connection and run_query
src/qql/connection.py, src/qql/__init__.py, tests/test_connection.py
Connection.__init__ and run_query() accept a verify parameter (default True) to control SSL verification; the value is threaded through to QdrantClient and stored in config; tests verify default behavior and custom CA bundle paths.
CLI: connect command and propagation
src/qql/cli.py, tests/test_cli.py
connect command adds --verify/--no-verify and --ca-cert options with mutual-exclusivity validation; execute, dump, and REPL bootstrap use the saved config; tests cover flag handling, validation, persistence, and downstream command behavior.
Documentation
README.md, docs/getting-started.md, docs/programmatic.md, docs/reference.md
README adds CLI examples; getting-started documents HTTPS connection patterns; programmatic reference documents the verify parameter and custom CA usage; config reference documents the persistent setting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I nibble through certs both strict and wry,
A path to trust, or skip the check nearby.
--ca-cert to honor what you bring,
--no-verify for trusted, local ring,
Hop—TLS tamed, now safely we comply. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main feature added: SSL certificate verification support in both CLI and connection components.
Linked Issues check ✅ Passed All objectives from issue #46 are implemented: CLI flags for disabling verification (--no-verify) and specifying CA bundles (--ca-cert), plus config persistence and API propagation.
Out of Scope Changes check ✅ Passed All changes directly support TLS verification control as specified in issue #46; no unrelated modifications were introduced outside stated objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSL verification fails with self-signed/internal certs unless verification is manually disabled

1 participant