mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(security): patch loopback guard, retry None raise, blocking subprocess, and cache stats race
- Add `Depends(_require_loopback)` to `/debug/memory` endpoint (was missing
while /debug/tasks, /debug/ws-sessions, /debug/warmup all had it)
- Guard `raise last_error` when last_error is None (retry_max_attempts=0 path
raised TypeError); add ProxyConfig.__post_init__ validation rejecting
retry_max_attempts < 1 when retry_enabled=True
- Make initialize_context_tool_session_baseline async; offload subprocess via
asyncio.to_thread so the blocking rtk/lean-ctx subprocess does not stall the
event loop; update call sites in server.py
- Take snapshot list() of SemanticCache._cache.values() before iterating in
get_memory_stats() to avoid dict-size-changed RuntimeError under async load
- Change memory_neo4j_password default from 'password' to '' and emit a
logger.warning at startup when backend=qdrant-neo4j and password is empty
- Replace hardcoded NEO4J_AUTH=neo4j/password in docker-compose.yml with
${NEO4J_AUTH:-neo4j/devpassword}; add .env.example with CHANGEME placeholder
- Format tests/test_provider_proxy_routes.py (pre-existing ruff format drift)
This commit is contained in:
parent
d8ac7472bf
commit
78f3a4dd3e
8 changed files with 67 additions and 16 deletions
3
.env.example
Normal file
3
.env.example
Normal file
|
|
@ -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
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -102,6 +102,7 @@ pytest_cache/
|
|||
.env
|
||||
.env.*
|
||||
!.env.act.example
|
||||
!.env.example
|
||||
.venv
|
||||
env/
|
||||
venv/
|
||||
|
|
|
|||
27
CHANGELOG.md
27
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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue