diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 0d24af8..1c05e25 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -581,6 +581,36 @@ All domain exceptions raised by services propagate to handlers in `main.py`, ens 2. No duplicated exception-to-HTTP-status mapping logic. 3. Easy to audit all error codes — they are all in one place. +### Error Message Hygiene + +HTTP responses must never leak sensitive internal details that aid attackers or expose infrastructure: + +- **Never include system paths** in HTTP error messages (e.g., `/var/run/fail2ban/fail2ban.sock`, `/etc/fail2ban/`). +- **Never include raw exception messages** that expose internal parsing or implementation logic. +- **Log full details server-side only** — exception handlers must log `error=str(exc)` with full exception context, but return generic user-friendly messages in the HTTP response. + +```python +# ❌ BAD — leaks socket path and internal details to the client +async def _fail2ban_connection_handler(request: Request, exc: Fail2BanConnectionError) -> JSONResponse: + return JSONResponse( + status_code=502, + content={"detail": f"Cannot reach fail2ban: {exc}"}, # exc includes socket path! + ) + +# ✅ GOOD — generic message in response, full details in server logs +async def _fail2ban_connection_handler(request: Request, exc: Fail2BanConnectionError) -> JSONResponse: + log.warning( + "fail2ban_connection_error", + path=request.url.path, + method=request.method, + error=str(exc), # Full details logged server-side + ) + return JSONResponse( + status_code=502, + content={"detail": "Cannot reach the fail2ban service. Check the server status page."}, + ) +``` + --- ## 9. Testing diff --git a/Docs/Tasks.md b/Docs/Tasks.md index ccec9cb..accdb78 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,47 +1,3 @@ -## TASK-028 — Fire-and-forget `asyncio.create_task()` silently discards exceptions - -**Severity:** Low - -### Where found -`backend/app/services/ban_service.py` line ~614: -```python -asyncio.create_task( # noqa: RUF006 - geo_cache.lookup_batch(uncached, http_session, db=app_db), - name="geo_bans_by_country", -) -``` - -### Why this is needed -The task reference is immediately discarded. Any exception raised inside `geo_cache.lookup_batch()` — network errors, aiohttp timeouts, DB write failures — becomes an unhandled task exception. In Python 3.11+ this emits a `RuntimeWarning` to stderr but is otherwise silently swallowed. Errors in background geo resolution are invisible in structured logs. - -### Goal -Ensure exceptions in fire-and-forget tasks are always logged. - -### What to do -1. Wrap the task body in a logging wrapper: - ```python - async def _logged_task(coro: Coroutine[Any, Any, Any], name: str) -> None: - try: - await coro - except Exception: - log.exception("background_task_failed", task_name=name) - - asyncio.create_task(_logged_task(geo_cache.lookup_batch(...), "geo_bans_by_country")) - ``` -2. Extract `_logged_task` into `backend/app/utils/async_utils.py` as a reusable helper so the same pattern is used for all fire-and-forget tasks. - -### Possible traps and issues -- The done callback must not re-raise the exception — only log it. -- `log.exception()` inside a callback/task captures the traceback automatically with structlog. - -### Docs changes needed -- `Backend-Development.md` — fire-and-forget task conventions. - -### Doc references -- [Backend-Development.md](Backend-Development.md) — async patterns - ---- - ## TASK-029 — `Fail2BanConnectionError` leaks socket path in HTTP error responses **Severity:** Medium diff --git a/backend/app/main.py b/backend/app/main.py index 24d65bf..667f95e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -231,7 +231,7 @@ async def _fail2ban_connection_handler( ) return JSONResponse( status_code=502, - content={"detail": f"Cannot reach fail2ban: {exc}"}, + content={"detail": "Cannot reach the fail2ban service. Check the server status page."}, ) @@ -256,7 +256,7 @@ async def _fail2ban_protocol_handler( ) return JSONResponse( status_code=502, - content={"detail": f"fail2ban protocol error: {exc}"}, + content={"detail": "Cannot reach the fail2ban service. Check the server status page."}, )