ClickHouse / ClickHouse/ClickHouse
`accept-invalid-certificate` config check uses `has()` instead of `getBool()` — explicit 0 still accepts certificates
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
_Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment._
_Retrospective finding from a historical scan of [PR #65238](https://github.com/ClickHouse/ClickHouse/pull/65238) (merged 2024-07-17). Confirmed on current codebase — close with a note if already fixed._
### Describe what's wrong
When a user explicitly sets `0` in their client config file, the client still accepts invalid certificates instead of rejecting them.
**Root cause:** Client.cpp:292: `if (config().has("accept-invalid-certificate"))` only checks if the key exists, not whether its boolean value is true. So `0` triggers the acceptance logic.
**Why we believe this is a bug:** Client::initialize (Client.cpp:292) → config().has("accept-invalid-certificate") checks key existence, not value → Lines 294-295 set AcceptCertificateHandler even when value is 0
**Affected locations:**
- `programs/client/Client.cpp:292` — accept-invalid-certificate config check
**Impact:** Users who explicitly set `0` intending to reject invalid certificates will have their certificates accepted, potentially creating a security vulnerability where self-signed or invalid certificates are trusted when they should not be.
### Does it reproduce on most recent release?
Yes — confirmed on current `master` (commit `5068a189c05`).
### How to reproduce
```python
Run: cd tests/integration && CLICKHOUSE_TESTS_SERVER_BIN_PATH=../../build/programs/clickhouse CLICKHOUSE_TESTS_BASE_CONFIG_DIR=../../programs/server pytest test_accept_invalid_certificate/test_explicit_zero.py -v -s
```
### Expected behavior
```
Exception: certificate verify failed
```
### Error message and/or stacktrace
```
Connection succeeded with output '1\n' when it should have failed with 'certificate verify failed'
```
### Additional context
**Suggested fix:** Change line 292 from `if (config().has("accept-invalid-certificate"))` to `if (config().getBool("accept-invalid-certificate", false))`
**Analysis details:** Confidence HIGH | Severity P2 | Testability: `INTEGRATION_TEST`
Found during automated review of [PR #65238](https://github.com/ClickHouse/ClickHouse/pull/65238).
---
_ClickGapAI · Confidence: HIGH · Severity: P2 · Finding: `h_pr65238_001`_
Contributor guide
Assessment
This issue has not been assessed yet.