diff --git a/.env.example b/.env.example new file mode 100644 index 000000000..0cab283d1 --- /dev/null +++ b/.env.example @@ -0,0 +1,3 @@ +# Copy this file to .env and fill in real values before running in production. +# IMPORTANT: Change NEO4J_AUTH before deploying — default credentials are insecure. +NEO4J_AUTH=neo4j/CHANGEME diff --git a/.gitignore b/.gitignore index 50ce6b395..746f9b458 100644 --- a/.gitignore +++ b/.gitignore @@ -102,6 +102,7 @@ pytest_cache/ .env .env.* !.env.act.example +!.env.example .venv env/ venv/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e2fcae1..dd66300bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security +- **`/debug/memory` loopback guard.** The endpoint was missing the + `Depends(_require_loopback)` guard that all other `/debug/*` endpoints carry. + External callers can no longer reach it. +- **`retry_max_attempts` zero guard.** When `retry_enabled=True` and + `retry_max_attempts=0` the retry loop exited without setting `last_error`, + causing `raise last_error` to raise `TypeError: exceptions must derive from + BaseException`. A `RuntimeError` with an actionable message is now raised + instead, and `ProxyConfig.__post_init__` rejects `retry_max_attempts < 1` + at construction time. +- **Blocking subprocess on async event loop.** `_read_rtk_lifetime_stats` and + `_read_lean_ctx_lifetime_stats` called `subprocess.run` directly on the + asyncio thread. The `initialize_context_tool_session_baseline` function is + now `async` and offloads the subprocess via `asyncio.to_thread`; the stats + endpoint uses `await asyncio.to_thread(_get_context_tool_stats)`. +- **Hardcoded Neo4j credential in `docker-compose.yml`.** `NEO4J_AUTH` now + defaults to `${NEO4J_AUTH:-neo4j/devpassword}` and is documented in + `.env.example` (excluded from `.gitignore` via `!.env.example`). +- **`SemanticCache.get_memory_stats()` concurrent iteration.** The method + iterates `self._cache.values()` without holding the async lock. A snapshot + is now taken via `list(self._cache.values())` before iterating to avoid + `RuntimeError: dictionary changed size during iteration` under async load. +- **Default Neo4j password in `ProxyConfig`.** `memory_neo4j_password` default + changed from `"password"` to `""`. The proxy startup path now emits a + `logger.warning` when `memory_backend == "qdrant-neo4j"` and the password + is empty, prompting operators to set a real credential. + ### Fixed - **PyPI install clarity and release gating.** Documented `pipx --python python3.13` for environments where unsupported Python wheel tags cause older-version diff --git a/docker-compose.yml b/docker-compose.yml index 59b3a70a7..c699e3cd6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -39,7 +39,7 @@ services: volumes: - neo4j_data:/data environment: - - NEO4J_AUTH=neo4j/password + - NEO4J_AUTH=${NEO4J_AUTH:-neo4j/devpassword} - NEO4J_PLUGINS=["apoc"] - NEO4J_apoc_export_file_enabled=true - NEO4J_apoc_import_file_enabled=true diff --git a/headroom/proxy/helpers.py b/headroom/proxy/helpers.py index 2d7347d96..61d3e3ad2 100644 --- a/headroom/proxy/helpers.py +++ b/headroom/proxy/helpers.py @@ -1233,11 +1233,11 @@ def _read_context_tool_lifetime_stats(tool: str) -> dict[str, Any] | None: return _read_rtk_lifetime_stats() -def initialize_context_tool_session_baseline() -> None: +async def initialize_context_tool_session_baseline() -> None: """Pin the current context-tool counters as the proxy-session baseline.""" tool = _selected_context_tool() - payload = _read_context_tool_lifetime_stats(tool) + payload = await asyncio.to_thread(_read_context_tool_lifetime_stats, tool) with _context_tool_stats_cache_lock: _context_tool_session_baseline.update( { @@ -1261,10 +1261,10 @@ def initialize_context_tool_session_baseline() -> None: ) -def initialize_rtk_session_baseline() -> None: - """Pin the current context-tool counters as the proxy-session baseline.""" +async def initialize_rtk_session_baseline() -> None: + """Backward-compatible alias for initialize_context_tool_session_baseline.""" - initialize_context_tool_session_baseline() + await initialize_context_tool_session_baseline() def _get_context_tool_stats() -> dict[str, Any] | None: diff --git a/headroom/proxy/models.py b/headroom/proxy/models.py index 1a1dfb0f1..f5363fde6 100644 --- a/headroom/proxy/models.py +++ b/headroom/proxy/models.py @@ -240,7 +240,7 @@ class ProxyConfig: memory_qdrant_api_key: str | None = field(default_factory=qdrant_env.qdrant_env_api_key) memory_neo4j_uri: str = "neo4j://localhost:7687" memory_neo4j_user: str = "neo4j" - memory_neo4j_password: str = "password" + memory_neo4j_password: str = "" memory_bridge_enabled: bool = False memory_bridge_md_paths: list[str] = field(default_factory=list) memory_bridge_md_format: str = "auto" @@ -311,6 +311,10 @@ class ProxyConfig: # ``HeadroomProxy._run_compression_in_executor``. compression_max_workers: int | None = None + def __post_init__(self, smart_routing: bool | None = None) -> None: + if self.retry_enabled and self.retry_max_attempts < 1: + raise ValueError("retry_max_attempts must be >= 1 when retry_enabled=True") + @property def provider_api_overrides(self) -> ProviderApiOverrides: """Return provider API URL overrides as a dedicated provider config object.""" diff --git a/headroom/proxy/semantic_cache.py b/headroom/proxy/semantic_cache.py index 71e81cfcb..e1194b93d 100644 --- a/headroom/proxy/semantic_cache.py +++ b/headroom/proxy/semantic_cache.py @@ -118,12 +118,17 @@ class SemanticCache: """ from ..memory.tracker import ComponentStats - # Calculate size - this is sync but we access _cache directly - # Note: This is a rough estimate, not perfectly accurate under async load + # Take a snapshot of cache values under the lock to avoid iterating + # over a dict that may be mutated concurrently by async coroutines. + # The lock is an asyncio.Lock and cannot be acquired in a sync method, + # so we do a single atomic copy of the values view instead. + snapshot = list(self._cache.values()) + entry_count = len(snapshot) + size_bytes = sys.getsizeof(self._cache) total_hits = 0 - for entry in self._cache.values(): + for entry in snapshot: size_bytes += sys.getsizeof(entry) size_bytes += len(entry.response_body) size_bytes += sys.getsizeof(entry.response_headers) @@ -133,7 +138,7 @@ class SemanticCache: return ComponentStats( name="semantic_cache", - entry_count=len(self._cache), + entry_count=entry_count, size_bytes=size_bytes, budget_bytes=None, hits=total_hits, diff --git a/headroom/proxy/server.py b/headroom/proxy/server.py index 05f1ec445..aff09b850 100644 --- a/headroom/proxy/server.py +++ b/headroom/proxy/server.py @@ -992,6 +992,13 @@ class HeadroomProxy( logger.info("Magika: ENABLED (ML content detection)") if self.memory_handler: + if ( + self.config.memory_backend == "qdrant-neo4j" + and not self.config.memory_neo4j_password + ): + logger.warning( + "NEO4J password is not set — using default credentials is insecure in production" + ) self.warmup.memory_backend.mark_loading() try: await self.memory_handler.ensure_initialized() @@ -1289,7 +1296,11 @@ class HeadroomProxy( ) await asyncio.sleep(delay_with_jitter / 1000) - raise last_error # type: ignore[misc] + if last_error is None: + raise RuntimeError( + "retry loop exhausted with no error recorded; retry_max_attempts must be >= 1" + ) + raise last_error async def _log_toin_stats_periodically(interval_seconds: int = 300) -> None: @@ -1453,7 +1464,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: app.state.started_at = time.time() app.state.ready = False app.state.startup_error = None - initialize_context_tool_session_baseline() + await initialize_context_tool_session_baseline() try: try: @@ -1914,7 +1925,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: # Fetch CLI filtering savings from the selected context tool. These # tokens are avoided before they reach model context. - cli_filtering_stats = _get_context_tool_stats() + cli_filtering_stats = await asyncio.to_thread(_get_context_tool_stats) cli_filtering_tool = ( str(cli_filtering_stats.get("tool", "rtk")) if cli_filtering_stats else "rtk" ) @@ -2315,7 +2326,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: await proxy.metrics.reset_runtime() if proxy.cost_tracker: proxy.cost_tracker.reset_runtime() - initialize_context_tool_session_baseline() + await initialize_context_tool_session_baseline() async with _stats_snapshot_lock: _stats_snapshot["value"] = None _stats_snapshot["expires_at"] = 0.0 @@ -2411,7 +2422,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: ) # Debug endpoints - @app.get("/debug/memory") + @app.get("/debug/memory", dependencies=[Depends(_require_loopback)]) async def debug_memory(): """Get detailed memory usage statistics.