# Security GAP Analysis - Geode Rust Client
**Last Updated:** 2026-01-30
**Review Scope:** geode-client-rust v0.1.1-alpha.8
**Commit:** 031d08a (main branch)
**Reviewer:** Security Code Review (Adversarial)
## Executive Summary
This security review identified **18 findings** across the Geode Rust client library.
**All 18 findings have code fixes implemented and verified.**
| Critical | 0 | 0 | 0 |
| High | 2 | 0 | 2 |
| Medium | 3 | 0 | 3 |
| Low | 11 | 0 | 11 |
| Info | 2 | 0 | 2 |
### Remediation Status
| Closed | 18 | All findings remediated and verified |
| Open | 0 | - |
### Top Risks (All Mitigated)
1. **#12 Gitleaks Allowlists .rs Files** (HIGH) - CLOSED: Removed blanket exclusion
2. **#1 Empty Root Certificate Store** (HIGH) - CLOSED: Added rustls-native-certs
3. **#14 Security Scan allow_failure** (MEDIUM) - CLOSED: Removed allow_failure from sbom-scan
4. **#2 TLS Verification Bypass** (MEDIUM) - CLOSED: Added warning log
5. **#3 Unpinned CI Components** (MEDIUM) - CLOSED: Pinned to v1.9.15
6. **#18 ConnectionPool max_size=0 deadlock** (LOW) - CLOSED: Added assert in constructor
## Stack-Ranked Findings
| 1 | High | 60 | Empty Root Certificate Store | TLS/Auth | **CLOSED** | [#1](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/1) | Added rustls-native-certs |
| 2 | High | 48 | Gitleaks Allowlists All .rs Files | Supply Chain | **CLOSED** | [#12](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/12) | Removed blanket exclusion |
| 3 | Medium | 48 | TLS Verification Bypass | TLS/Auth | **CLOSED** | [#2](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/2) | Added warn! log message |
| 4 | Medium | 32 | Unpinned CI Components | CI/CD | **CLOSED** | [#3](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/3) | Pinned to v1.9.15 |
| 5 | Medium | 32 | Security Scan allow_failure:true | CI/CD | **CLOSED** | [#14](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/14) | Removed allow_failure |
| 6 | Low | 24 | Debug Logging Exposure | Ops | **CLOSED** | [#4](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/4) | Replaced with log crate |
| 7 | Low | 24 | Credential Memory | Data | **CLOSED** | [#5](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/5) | Added secrecy crate |
| 8 | Low | 24 | DSN Password Exposure | Data | **CLOSED** | [#7](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/7) | Added redact_dsn() function |
| 9 | Low | 16 | Connection Pool Panics | Availability | **CLOSED** | [#6](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/6) | Added proper error handling |
| 10 | Low | 16 | Docker Runs as Root | Container | **CLOSED** | [#8](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/8) | Added non-root user |
| 11 | Low | 16 | Validation Not Automatic | Input | **CLOSED** | [#9](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/9) | Added validate() and auto-validation |
| 12 | Low | 16 | Unchecked unwrap() Panics | Availability | **CLOSED** | [#13](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/13) | Added error handling + documented expect() |
| 13 | Low | 12 | Connection Pool Lacks Health Check | Availability | **CLOSED** | [#16](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/16) | Added is_healthy() and pool validation |
| 14 | Low | 12 | Blocking sleep in async close() | Performance | **CLOSED** | [#17](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/17) | Removed blocking sleep; QUIC close is async |
| 15 | Low | 8 | GAP_ANALYSIS Status Discrepancy | Docs | **CLOSED** | [#15](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/15) | Synced doc with implementation |
| 16 | Info | 12 | Hardcoded Timeouts | Config | **CLOSED** | [#10](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/10) | Added configurable timeouts |
| 17 | Info | 12 | No cargo-audit in CI | CI/CD | **CLOSED** | [#11](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/11) | Added cargo-audit job |
| 18 | Low | 16 | ConnectionPool max_size=0 deadlock | Availability | **CLOSED** | [#18](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/18) | Added assert!(max_size > 0) |
---
## Findings Detail
### 18. ConnectionPool max_size=0 Causes Infinite Deadlock [LOW] - CLOSED
- **Issue:** [#18](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/18)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 16 (Impact: 2, Likelihood: 2)
- **CWE:** CWE-400 (Uncontrolled Resource Consumption), CWE-835 (Infinite Loop)
**Problem:** Creating a `ConnectionPool` with `max_size=0` causes `acquire()` to block forever. The semaphore is initialized with zero permits, so any call to `acquire_owned()` will wait indefinitely.
**Fix Applied:**
```rust
// src/pool.rs - Added assertion in constructor (lines 17-31)
impl ConnectionPool {
/// Create a new connection pool.
///
/// # Panics
///
/// Panics if `max_size` is 0. A connection pool must have at least one
/// connection slot to function properly. (Gap #18: CWE-400, CWE-835)
pub fn new(host: impl Into<String>, port: u16, max_size: usize) -> Self {
assert!(
max_size > 0,
"ConnectionPool max_size must be at least 1 (was 0). \
A pool with 0 connections would deadlock on acquire()."
);
// ... rest of constructor
}
}
```
**Test Added:**
```rust
#[test]
#[should_panic(expected = "ConnectionPool max_size must be at least 1")]
fn test_connection_pool_max_size_zero_panics() {
// Gap #18: max_size=0 would cause deadlock on acquire() since semaphore
// would have 0 permits. Now properly panics at construction time.
let _pool = ConnectionPool::new("localhost", 3141, 0);
}
```
**Verification:** All 291 unit tests and 28 property-based tests pass.
---
### 12. Gitleaks Allowlists All Rust Files [HIGH] - CLOSED
- **Issue:** [#12](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/12)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 48 (Impact: 4, Likelihood: 3)
**Problem:** The `.gitleaks.toml` had a blanket exclusion for ALL `.rs` files, completely bypassing secret detection for Rust source.
**Fix Applied:**
```toml
# .gitleaks.toml - Removed blanket .rs exclusion
[allowlist]
description = "Global allowlist - only exclude build artifacts and logs"
paths = [
'''target''',
'''.*\.log$''',
]
# Added specific rules for known false positives
[[rules]]
id = "test-fixture-passwords"
description = "Test fixtures with example/placeholder credentials"
[[rules]]
id = "documentation-examples"
description = "Documentation examples showing DSN format"
regex = '''quic://[^:]+:secret@'''
allowlist = { paths = ['''.*\.md$''', '''examples/.*'''] }
```
---
### 13. Unchecked unwrap() Calls Can Cause DoS [LOW] - CLOSED
- **Issue:** [#13](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/13)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 16 (Impact: 2, Likelihood: 2)
**Problem:** Two `unwrap()` calls in client.rs could panic:
1. `Duration.try_into().unwrap()` - panics if timeout exceeds quinn's VarInt range
2. `"0.0.0.0:0".parse().unwrap()` - could theoretically panic
**Fix Applied:**
```rust
// src/client.rs - Timeout now capped to prevent overflow
// Cap idle timeout to quinn's maximum (2^62 - 1 microseconds ≈ 146 years)
// to prevent panic from VarInt overflow
let idle_timeout = Duration::from_secs(idle_timeout_secs.min(146_000 * 365 * 24 * 3600));
transport.max_idle_timeout(Some(
idle_timeout
.try_into()
.map_err(|_| Error::connection("Idle timeout value too large for QUIC protocol"))?,
));
// Socket address parse now uses expect() with documented invariant
let mut endpoint = Endpoint::client(
"0.0.0.0:0"
.parse()
.expect("0.0.0.0:0 is a valid socket address"),
)
```
**Test Added:**
```rust
#[test]
fn test_client_extreme_timeout_values() {
// These should not panic - the actual validation/capping happens at connect time
let _client = Client::new("localhost", 3141)
.connect_timeout(u64::MAX)
.hello_timeout(u64::MAX)
.idle_timeout(u64::MAX);
}
```
---
### 14. Security Scan Jobs Use allow_failure [MEDIUM] - CLOSED
- **Issue:** [#14](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/14)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 32 (Impact: 4, Likelihood: 2)
**Problem:** The sbom-scan job had `allow_failure: true`, allowing vulnerable dependencies to be merged.
**Fix Applied:**
```yaml
# .gitlab-ci.yml - Removed allow_failure from sbom-scan
# SBOM generation and scanning
# Security scan jobs MUST NOT use allow_failure (Gap #14)
- component: gitlab.com/devnw/ci-catalog/sbom-generate@v1.9.15
- component: gitlab.com/devnw/ci-catalog/sbom-scan@v1.9.15
```
---
### 15. GAP_ANALYSIS Status Discrepancy [LOW] - CLOSED
- **Issue:** [#15](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/15)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 8 (Impact: 1, Likelihood: 2)
**Problem:** Documentation claimed issues were closed but GitLab showed them open.
**Fix Applied:** This document has been updated to accurately reflect the implementation status. All code fixes have been verified and implemented.
---
### 16. Connection Pool Lacks Health Checking [LOW] - CLOSED
- **Issue:** [#16](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/16)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 12 (Impact: 2, Likelihood: 1.5)
**Problem:** Stale/broken connections could be returned from the pool without validation.
**Fix Applied:**
```rust
// src/client.rs - Added is_healthy() method
impl Connection {
/// Check if the connection is still healthy.
pub fn is_healthy(&self) -> bool {
self.conn.close_reason().is_none()
}
}
// src/pool.rs - Health check on acquire
let connection = loop {
let conn = { connections.pop() };
match conn {
Some(c) if c.is_healthy() => break c,
Some(_) => continue, // Discard stale connection
None => break client.connect().await?,
}
};
// src/pool.rs - Only return healthy connections to pool
impl Drop for PooledConnection {
fn drop(&mut self) {
if let Some(conn) = self.connection.take() {
if conn.is_healthy() {
// Return to pool
tokio::spawn(async move { connections.push(conn); });
}
// Unhealthy connections are dropped
}
}
}
```
---
### 17. Blocking std::thread::sleep in async Connection::close() [LOW] - CLOSED
- **Issue:** [#17](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/17)
- **Status:** CLOSED
- **Confidence:** Confirmed
- **Risk Score:** 12 (Impact: 2, Likelihood: 1.5)
**Problem:** The `Connection::close()` method used `std::thread::sleep()` which blocked the tokio runtime's worker thread for 100ms. In high-concurrency scenarios with frequent connection churn, this could starve other async tasks.
**Fix Applied:**
```rust
// src/client.rs - Removed blocking sleep (Option 2: backward compatible)
pub fn close(&mut self) -> Result<()> {
// QUIC close is asynchronous and best-effort - the CONNECTION_CLOSE frame
// will be sent by Quinn's internal I/O handling. No blocking delay needed.
// (Gap #17: Removed std::thread::sleep that blocked the async runtime)
self.conn.close(0u32.into(), b"client closing");
Ok(())
}
```
**Rationale:** QUIC's `close()` method initiates the close handshake asynchronously. The CONNECTION_CLOSE frame is sent by Quinn's internal I/O handling without requiring a blocking wait. The previous 100ms delay didn't guarantee frame delivery and unnecessarily blocked the async runtime.
**Tests:** All 291 unit tests and 28 property-based tests pass after this change.
---
## Previously Identified Findings (All Closed)
### 1. Empty Root Certificate Store [HIGH] - CLOSED
- **Issue:** [#1](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/1)
- **Status:** CLOSED
- **Resolution:** Added `rustls-native-certs` v0.8 dependency to load system root certificates
**Fix Applied:**
```rust
// Load system root certificates for proper TLS verification
let cert_result = rustls_native_certs::load_native_certs();
for cert in cert_result.certs {
root_store.add(cert).ok();
}
```
### 2. TLS Verification Bypass [MEDIUM] - CLOSED
- **Issue:** [#2](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/2)
- **Status:** CLOSED
- **Resolution:** Added prominent warning log when skip_verify is enabled
**Fix Applied:**
```rust
warn!(
"TLS certificate verification DISABLED - connection to {}:{} is vulnerable to MITM attacks. \
Do NOT use skip_verify in production!",
host, port
);
```
### 3. Unpinned CI Components [MEDIUM] - CLOSED
- **Issue:** [#3](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/3)
- **Status:** CLOSED
- **Resolution:** All CI components pinned to v1.9.15
**Fix Applied:** Updated `.gitlab-ci.yml` to pin all components:
```yaml
- component: gitlab.com/devnw/ci-catalog/rust-lint@v1.9.15
- component: gitlab.com/devnw/ci-catalog/rust-build@v1.9.15
# ... all components pinned
```
### 4. Debug Logging Exposure [LOW] - CLOSED
- **Issue:** [#4](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/4)
- **Status:** CLOSED
- **Resolution:** Replaced all `eprintln!()` with `log` crate macros (debug!, trace!, warn!)
### 5. Credential Memory Exposure [LOW] - CLOSED
- **Issue:** [#5](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/5)
- **Status:** CLOSED
- **Resolution:** Added `secrecy` v0.10 crate, password now stored as `SecretString`
**Fix Applied:**
```rust
use secrecy::{ExposeSecret, SecretString};
pub struct Client {
// ...
password: Option<SecretString>, // Zeroized on drop
}
```
### 6. Connection Pool Panics [LOW] - CLOSED
- **Issue:** [#6](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/6)
- **Status:** CLOSED
- **Resolution:** Replaced bare `.unwrap()` calls with proper error handling
**Fix Applied:**
```rust
// Before: .acquire_owned().await.unwrap()
// After:
.acquire_owned()
.await
.map_err(|_| Error::pool("Connection pool has been closed"))?;
```
### 7. DSN Password Exposure [LOW] - CLOSED
- **Issue:** [#7](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/7)
- **Status:** CLOSED
- **Resolution:** Added `redact_dsn()` function to sanitize DSN strings before including in error messages
**Tests Added:**
- `test_redact_dsn_url_with_password` - Verifies URL password redaction
- `test_redact_dsn_url_without_password` - Verifies no false positives
- `test_redact_dsn_url_no_auth` - Verifies passthrough when no auth
- `test_redact_dsn_query_param_password` - Verifies query param redaction
- `test_redact_dsn_query_param_pass` - Verifies "pass=" redaction
- `test_redact_dsn_simple_no_password` - Verifies no false positives
- `test_redact_dsn_url_with_query_and_password` - Verifies combined case
### 8. Docker Runs as Root [LOW] - CLOSED
- **Issue:** [#8](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/8)
- **Status:** CLOSED
- **Resolution:** Added non-root user to Dockerfile
**Fix Applied:**
```dockerfile
RUN groupadd --gid 1000 geode && \
useradd --uid 1000 --gid 1000 --shell /bin/false --create-home geode
USER geode
```
### 9. Validation Not Automatic [LOW] - CLOSED
- **Issue:** [#9](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/9)
- **Status:** CLOSED
- **Resolution:** Added `validate()` method to Client builder and automatic validation in `connect()`
**Tests Added:**
- `test_client_validate_valid` - Verifies valid config passes
- `test_client_validate_valid_hostname` - Verifies FQDN passes
- `test_client_validate_valid_ipv4` - Verifies IPv4 passes
- `test_client_validate_invalid_hostname_hyphen_start` - Verifies hostname validation
- `test_client_validate_invalid_hostname_hyphen_end` - Verifies hostname validation
- `test_client_validate_invalid_port_zero` - Verifies port validation
- `test_client_validate_invalid_page_size_zero` - Verifies page size validation
- `test_client_validate_invalid_page_size_too_large` - Verifies page size validation
- `test_client_validate_with_all_options` - Verifies all options together
### 10. Hardcoded Timeouts [INFO] - CLOSED
- **Issue:** [#10](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/10)
- **Status:** CLOSED
- **Resolution:** Added configurable timeout builder methods
**Fix Applied:**
```rust
let client = Client::new("localhost", 3141)
.connect_timeout(15) // default: 10s
.hello_timeout(10) // default: 5s
.idle_timeout(60); // default: 30s
```
### 11. No cargo-audit in CI [INFO] - CLOSED
- **Issue:** [#11](https://gitlab.com/devnw/codepros/geode/geode-client-rust/-/issues/11)
- **Status:** CLOSED
- **Resolution:** Added cargo-audit job to CI pipeline
**Fix Applied:**
```yaml
security:cargo-audit:
stage: security
image: rust:1.85-bookworm
script:
- cargo install cargo-audit --locked
- cargo audit --deny warnings
```
---
## Positive Security Findings
The codebase demonstrates several good security practices:
1. **No unsafe code** - Zero `unsafe` blocks in source
2. **Strong typing** - Rust's type system prevents many bugs
3. **TLS 1.3 only** - Modern, secure protocol version
4. **System root certificates** - Now loads platform CA store
5. **Secure credential handling** - SecretString with zeroization
6. **Password redaction** - DSN passwords redacted from error messages
7. **Automatic input validation** - Validates on connect()
8. **Error handling** - Rich error types with retryable classification
9. **Parameterized queries** - Prevents injection at protocol level
10. **Security.md** - Vulnerability reporting process documented
11. **Multiple security scanners in CI** - SAST, dependency scanning, SBOM, cargo-audit
12. **Configurable timeouts** - Allows tuning for different environments
13. **Non-root container** - Follows least privilege principle
14. **Fuzzing targets** - Property-based and fuzz testing coverage
15. **Connection pool health checking** - Validates connections before reuse
---
## Test Results
All tests pass after implementing security fixes:
```text
cargo test --lib
test result: ok. 291 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
cargo test --test proptest
test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
```
---
## Methodology
### Tools Used
- Manual adversarial code review
- GitLab CLI (glab) for issue tracking
- Pattern searching (ripgrep)
- cargo clippy for linting
- cargo test for validation
### Review Scope
- Application code (`src/*.rs`)
- CI/CD configuration (`.gitlab-ci.yml`)
- Container configuration (`Dockerfile`)
- Security configuration (`.gitleaks.toml`)
- Build scripts (`build.rs`, `build-docker.sh`)
- Examples and tests
### Files Modified During Remediation
- `.gitleaks.toml` - Removed blanket .rs exclusion (#12)
- `src/client.rs` - Fixed timeout overflow, added is_healthy(), removed blocking sleep (#13, #16, #17)
- `.gitlab-ci.yml` - Removed allow_failure from sbom-scan (#14)
- `src/pool.rs` - Added health checking (#16), added max_size validation (#18)
- `docs/GAP_ANALYSIS.md` - Updated status (#15, #17, #18)
---
## References
- [OWASP Top 10](https://owasp.org/Top10/)
- [CWE Database](https://cwe.mitre.org/)
- [RustSec Advisory Database](https://rustsec.org/)
- [Rust Security Guidelines](https://anssi-fr.github.io/rust-guide/)
- [SLSA Supply Chain Levels](https://slsa.dev/)
---
*This analysis was performed as part of an adversarial security code review on 2026-01-30.*
*18 total findings identified and remediated. All findings are now CLOSED.*