From be46547114ca9ce5ef074961c1b020ef4e0cd372 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 7 Apr 2026 21:15:41 +0200 Subject: [PATCH] backup --- Docs/Tasks.md | 77 ++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index e8cc377..0e16aa0 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -8,44 +8,51 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. ## Open Issues -### Backend Architecture Review Findings +### 1. Fix setup persistence +- Where found: `backend/app/config.py`, `backend/app/startup.py`, `backend/app/services/setup_service.py`, `backend/app/routers/setup.py` +- Goal: runtime configuration should use the values persisted during setup for `database_path`, `fail2ban_socket`, `timezone`, and `session_duration_minutes` rather than only environment defaults. +- Possible traps and issues: + - Setup may appear successful but later use a different DB/socket on restart. + - A partially persisted setup run must not leave the app in a broken or half-configured state. + - Using both env vars and persisted settings requires a clear precedence model. -- Status: **done** — `backend/app/routers/config.py` now uses explicit dependency injection for fail2ban settings and no longer reads `request.app.state.settings` directly. -- Status: **done** — `backend/app/routers/config.py` now uses an explicit `AppState` dependency for pending recovery and activation state instead of writing `request.app.state` directly. +### 2. Remove or use `session_secret` +- Where found: `backend/app/config.py` +- Goal: either eliminate the unused `BANGUI_SESSION_SECRET` requirement or use it for session token generation / signing so the setting has purpose. +- Possible traps and issues: + - Keeping it required without use is misleading and burdens deployments. + - Introducing a new crypto dependency for session tokens must preserve backward compatibility with existing sessions. + - If switched to signed tokens, ensure token revocation / logout still works correctly. -- Status: **done** — `backend/app/routers/*` often reads config directly from `request.app.state.settings` instead of using dependency injection. This bypasses the dependency layer and creates hidden coupling between routers and application state. - - Fix: replace direct `request.app.state.settings` access with `SettingsDep` or other explicit dependencies such as `ServerStatusDep` and `PendingRecoveryDep` in router function signatures. - - Expected outcome: routers become easier to unit test, composition is more explicit, and shared state access is only available through documented FastAPI dependencies. +### 3. Harden session cookie security +- Where found: `backend/app/routers/auth.py` +- Goal: auth cookies should be set with `secure=True` in HTTPS production deployments and `SameSite`/`HttpOnly` behavior should be explicit and configurable. +- Possible traps and issues: + - Hardcoding `secure=False` makes production deployment insecure. + - Switching to `secure=True` can break local development unless there is an explicit dev override. + - The frontend API may need matching CORS and same-site handling when served from a different origin. -- Status: **done** — Several utility modules under `backend/app/utils/` import service layer code (`app.services.*`). Utilities should remain low-level helpers and not depend on higher-level service logic. - - Fix: move service-dependent helpers into `app/services/` or extract shared logic into a new `app/helpers/` layer, keeping `app/utils/` purely independent. - - Expected outcome: lower coupling between utility and service layers, cleaner dependency direction, and better maintainability. +### 4. Address session cache invalidation semantics +- Where found: `backend/app/dependencies.py` +- Goal: make session caching safe or remove it, and document that cache invalidation is not cluster-safe if the app is run with multiple workers. +- Possible traps and issues: + - Process-local cache can keep revoked sessions alive in other worker processes. + - Implementing a shared cache is a larger architectural change; a safer short-term fix is to disable caching by default. + - Need to ensure `logout()` and session expiry remain consistent across requests. -- Status: **done** — background task modules in `backend/app/tasks/` no longer rely on the dead `app.state.db` fast-path and now open/close dedicated task-local DB connections using `app.state.settings.database_path`. - - Fix: remove the unused `app.state.db` branch and always open/close a dedicated task-local connection, or intentionally add a shared DB connection to `app.state` and manage its lifecycle. - - Expected outcome: background jobs have predictable DB lifecycle, avoid hidden bugs from stale connection assumptions, and task code is simpler. +### 5. Improve external HTTP client resilience +- Where found: `backend/app/startup.py` +- Goal: create `aiohttp.ClientSession()` with sensible global timeouts, connection limit settings, and optional retry policy for geo/blocklist API calls. +- Possible traps and issues: + - Without timeouts, external lookups can hang request handling or background tasks. + - Connection limits must be chosen carefully to avoid underutilization or overload. + - A retry policy should avoid retry storms and should respect API rate limits. -- Status: **done** — `backend/app/dependencies.py` contains an in-memory process-local session cache for auth tokens. This optimization is valid for a single-process server, but it is not cluster-safe for multi-worker or distributed deployments. - - Fix: document the single-process limitation clearly in code and project docs. - - Expected outcome: authentication behavior is consistent across deployment modes, and session invalidation works correctly in multi-worker setups. - -- Status: **done** — `backend/app/main.py` uses local imports inside `_lifespan()` to avoid circular dependencies, indicating that startup logic is tightly coupled with services. - - Fix: evaluate whether the startup initialization can be moved into a dedicated `startup.py` or split service initialization into smaller modules; keep import order simple and explicit. - - Expected outcome: cleaner startup code with lower coupling, fewer hidden circular import risks, and easier maintenance. - -### Recommended refactors for an AI agent - -- Status: **done** — Standardise dependency injection in routers by using `SettingsDep`, `ServerStatusDep`, `PendingRecoveryDep`, and other dependency definitions from `backend/app/dependencies.py`. -- Refactor `backend/app/utils/` so it does not import business-layer services. Move cross-layer helpers to the appropriate layer or introduce a shared helper package if needed. -- Simplify background task DB management in `backend/app/tasks/`: remove the dead `app.state.db` logic or implement a real shared connection and document its lifecycle. -- Document auth cache semantics in the code and project docs. If cluster deployments are intended, replace the process-local cache with a shared cache or remove it. -- Inspect `backend/app/main.py` startup wiring and reduce local import usage by extracting startup responsibilities into clearer components. - -### Goals and expectations for the fix - -- Preserve existing functionality while reducing hidden coupling. -- Improve testability of routers and background tasks by making dependencies explicit. -- Make the application startup and shared-state behavior easy to reason about. -- Ensure backend architecture is stable for future refactors, especially around authentication, config handling, and scheduled jobs. -- Provide enough detail so an AI agent can make the changes safely without altering business behavior. +### 6. Update async socket handling +- Where found: `backend/app/utils/fail2ban_client.py`, `backend/app/startup.py` +- Goal: use modern asyncio APIs (`get_running_loop()`), avoid blocking operations on the event loop, and ensure startup resources are cleaned up if initialization fails. +- Possible traps and issues: + - `asyncio.get_event_loop()` behavior changed in newer Python versions; this can cause runtime warnings or errors. + - Resource leaks can occur if `startup_shared_resources()` fails before the lifespan shutdown path is reached. + - The fail2ban socket client should still handle transient errors and not hide protocol failures behind generic exceptions.