backup

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.
+### 2. Remove or use `session_secret`
-- 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.
+- 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.
+### 3. Harden session cookie security
-  - Fix: replace direct `request.app.state.settings` access with `SettingsDep` or other explicit dependencies such as `ServerStatusDep` and `PendingRecoveryDep` in router function signatures.
+- Where found: `backend/app/routers/auth.py`
-  - Expected outcome: routers become easier to unit test, composition is more explicit, and shared state access is only available through documented FastAPI dependencies.
+- 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.
+### 4. Address session cache invalidation semantics
-  - Fix: move service-dependent helpers into `app/services/` or extract shared logic into a new `app/helpers/` layer, keeping `app/utils/` purely independent.
+- Where found: `backend/app/dependencies.py`
-  - Expected outcome: lower coupling between utility and service layers, cleaner dependency direction, and better maintainability.
+- 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`.
+### 5. Improve external HTTP client resilience
-  - 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.
+- Where found: `backend/app/startup.py`
-  - Expected outcome: background jobs have predictable DB lifecycle, avoid hidden bugs from stale connection assumptions, and task code is simpler.
+- 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.
+### 6. Update async socket handling
-  - Fix: document the single-process limitation clearly in code and project docs.
+- Where found: `backend/app/utils/fail2ban_client.py`, `backend/app/startup.py`
-  - Expected outcome: authentication behavior is consistent across deployment modes, and session invalidation works correctly in multi-worker setups.
+- 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:
-- Status: **done** — `backend/app/main.py` uses local imports inside `_lifespan()` to avoid circular dependencies, indicating that startup logic is tightly coupled with services.
+  - `asyncio.get_event_loop()` behavior changed in newer Python versions; this can cause runtime warnings or errors.
-  - 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.
+  - Resource leaks can occur if `startup_shared_resources()` fails before the lifespan shutdown path is reached.
-  - Expected outcome: cleaner startup code with lower coupling, fewer hidden circular import risks, and easier maintenance.
+  - The fail2ban socket client should still handle transient errors and not hide protocol failures behind generic exceptions.
-
-### 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.