BanGUI/Docs/Tasks.md

## TASK-004 — Frontend auth state bootstrapped from `sessionStorage`, not from the backend

**Severity:** Medium

### Where found
`frontend/src/providers/AuthProvider.tsx` line 64 — `useState(() => sessionStorage.getItem(IS_AUTHENTICATED_KEY) === "true")`.

### Why this is needed
On page load, the frontend sets `isAuthenticated = true` from a cached `sessionStorage` flag without contacting the backend. If the session cookie has expired server-side (e.g., server restart, session_duration elapsed), the user briefly sees the authenticated UI before the first API call triggers a 401. This also means that a user with a revoked session (manual DB deletion) appears logged in until they navigate.

### Goal
Validate session validity with the backend on mount so the UI state always reflects reality.

### What to do
1. Add a `GET /api/auth/session` endpoint (or reuse `GET /api/health` with auth) that returns 200 if the session is valid, 401 otherwise.
2. In `AuthProvider`, call this endpoint on mount. While the check is in flight, show a loading spinner or skeleton.
3. On 401, clear `sessionStorage` and set `isAuthenticated = false`, then redirect to `/login`.
4. Keep the `sessionStorage` flag as an optimistic hint to avoid flicker, but always confirm with the backend.

### Possible traps and issues
- Adds one round-trip on every page load — keep the endpoint extremely lightweight (no DB joins).
- Must handle the case where the backend is temporarily unreachable (network error) — do not log the user out in that case, only on an explicit 401.
- The loading state needs a spinner or skeleton so the UI doesn't flash between states.

### Docs changes needed
- `Features.md` — update the authentication flow description.
- `Web-Development.md` — document the mount-time session check pattern.

### Doc references
- [Features.md](Features.md) — authentication and session management
- [Web-Development.md](Web-Development.md) — AuthProvider pattern

---

## TASK-005 — `session_cookie_secure` defaults to `false`

**Severity:** Medium

### Where found
`backend/app/config.py` — `session_cookie_secure: bool = Field(default=False, ...)`.

### Why this is needed
The `Secure` cookie attribute prevents the browser from sending the session cookie over unencrypted HTTP. Defaulting to `false` means that if the production deployment is ever accessed via HTTP (misconfigured nginx, direct backend access, a failed HTTPS redirect), the session token is transmitted in the clear.

### Goal
Default to `true` so production deployments are secure by default. Opt-out explicitly for local development.

### What to do
1. Change `default=False` to `default=True` in the `session_cookie_secure` field.
2. In `Docker/compose.debug.yml`, add `BANGUI_SESSION_COOKIE_SECURE: "false"` explicitly.
3. Document in `compose.debug.yml` comments that `Secure=false` is intentional for local HTTP dev.

### Possible traps and issues
- Browsers reject `Secure` cookies delivered over HTTP — this will break local development unless `compose.debug.yml` is updated.
- Ensure the nginx config in production terminates TLS and passes `X-Forwarded-Proto: https` so FastAPI knows the connection is secure.

### Docs changes needed
- `Backend-Development.md` — document the `session_cookie_secure` config option.

### Doc references
- [Backend-Development.md](Backend-Development.md) — configuration reference

---

## TASK-006 — SPA `*` wildcard redirect hides API 404s

**Severity:** Low

### Where found
`Docker/nginx.conf` — the catch-all `try_files $uri $uri/ /index.html` rule.

### Why this is needed
The SPA wildcard catches every unmatched path, including typos in API paths like `/api/jailss`. The browser receives a 200 with the SPA HTML instead of a 404, masking client-side bugs during development and making API integration harder to debug.

### Goal
Ensure `/api/**` paths that do not match any backend route return 404 from FastAPI, not 200 with HTML from nginx.

### What to do
1. In `nginx.conf`, ensure the `location /api/` block proxies to the backend and does **not** have a `try_files` fallback.
2. Verify that `location /api/` has higher priority than the catch-all `location /` block (nginx uses longest-prefix matching, so `/api/` takes precedence automatically).
3. Remove any `try_files` directives from the `/api/` location block.

### Possible traps and issues
- nginx `try_files` in a `location /` block will not affect `location /api/` as long as `/api/` is defined separately — verify the current config doesn't have an inherited `try_files`.

### Docs changes needed
- `Architekture.md` — document nginx routing rules.

### Doc references
- [Architekture.md](Architekture.md) — nginx / frontend serving

---

## TASK-007 — No rate limiting on the login endpoint

**Severity:** High

### Where found
`backend/app/routers/auth.py` — `POST /api/auth/login`. No rate-limiting middleware, no lockout logic.

### Why this is needed
BanGUI uses a single master password for all access. An attacker with network access to the login endpoint can attempt unlimited passwords per second. With no lockout or slowdown, a 10-character password with the required complexity (upper, digit, special) can be brute-forced offline or via the API.

### Goal
Limit login attempts per IP address to make brute-force attacks infeasible.

### What to do
1. Add [`slowapi`](https://github.com/laurents/slowapi) (or a simple in-memory counter) to rate-limit `POST /api/auth/login`.
2. Limit to 5 attempts per minute per IP. Return `429 Too Many Requests` with a `Retry-After` header on excess.
3. For the in-memory approach: use a `dict[str, deque[float]]` keyed by IP, storing attempt timestamps, cleaned up by a background task.
4. Consider a 10-second `asyncio.sleep` on wrong password to further slow down attacks (already somewhat mitigated by bcrypt cost factor).

### Possible traps and issues
- Behind nginx, `request.client.host` is the proxy IP. Read the real IP from `X-Forwarded-For` only when the request comes from a trusted proxy (the nginx container's IP). Do not blindly trust `X-Forwarded-For` — it can be spoofed.
- The in-memory rate-limiter is process-local (see TASK-002/003) — in a multi-worker setup, each worker has its own counter. Single-worker constraint limits the blast radius.
- `slowapi` integrates cleanly with FastAPI and handles the `X-Real-IP` / `X-Forwarded-For` header logic.

### Docs changes needed
- `Features.md` — document login rate limiting.
- `Backend-Development.md` — rate limiting conventions.

### Doc references
- [Features.md](Features.md) — authentication
- [Backend-Development.md](Backend-Development.md) — security patterns

---

## TASK-008 — `delete_expired_sessions` never scheduled — sessions table grows unbounded

**Severity:** Medium

### Where found
`backend/app/repositories/session_repo.py` — `delete_expired_sessions()` exists but is never called from any task or lifespan handler.

### Why this is needed
Expired sessions are only removed individually when that specific token is validated and found expired. The bulk cleanup function is never called. Over months of operation, the `sessions` table accumulates every session ever created and is never trimmed, increasing DB size and degrading query performance.

### Goal
Periodically purge expired sessions from the database.

### What to do
1. Create `backend/app/tasks/session_cleanup.py` following the same pattern as `geo_cache_flush.py`.
2. Schedule it as an interval job (e.g., every 6 hours) in `startup_shared_resources`.
3. The task should call `session_repo.delete_expired_sessions(db, now_iso)` and log how many rows were deleted.

### Possible traps and issues
- The task must use `task_db(settings)` (not the request-scoped `get_db`) to open its own connection.
- Log the count of deleted rows at `info` level, not `debug`, so administrators can see the cleanup is running.

### Docs changes needed
- `Architekture.md` — add `session_cleanup` to the scheduled tasks table.
- `Backend-Development.md` — background task patterns.

### Doc references
- [Architekture.md](Architekture.md) — background tasks
- [Backend-Development.md](Backend-Development.md) — scheduled tasks

---

## TASK-009 — Blocklist URL has no scheme/host validation — SSRF risk

**Severity:** High

### Where found
`backend/app/models/blocklist.py` — `BlocklistSourceCreate.url: str = Field(..., min_length=1)`. `backend/app/services/blocklist_service.py` — `preview_source()` and `_download_text_with_retries()`.

### Why this is needed
An authenticated user can supply `file:///etc/passwd`, `http://169.254.169.254/latest/meta-data/` (AWS metadata service), `http://10.0.0.1/admin`, or `http://localhost:8000/api/setup` as a blocklist URL. The backend fetches it and either returns its contents in the preview response or attempts to parse it as an IP list. This is a Server-Side Request Forgery (SSRF) vulnerability.

### Goal
Restrict blocklist URLs to safe, public HTTP/HTTPS endpoints only.

### What to do
1. Change `url: str` to `url: AnyHttpUrl` in `BlocklistSourceCreate` — this rejects `file://`, `ftp://`, and other non-http schemes.
2. Add a `@field_validator("url")` that:
   - Parses the hostname.
   - Resolves it via `socket.getaddrinfo()` (or uses `ipaddress.ip_address()` if it is a bare IP).
   - Rejects RFC 1918 private ranges (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`), loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), and IPv6 equivalents.
3. Add a validator for `http://` URLs — consider requiring `https://` only, or adding a configuration flag.

### Possible traps and issues
- DNS rebinding: the hostname resolves to a public IP at validation time but to a private IP at fetch time. Mitigate by re-validating the final connection IP in aiohttp (custom `TCPConnector` or response callback).
- `AnyHttpUrl` allows ports — ensure `http://evil.com:22/` is also blocked or at least safe.
- `socket.getaddrinfo()` is blocking — use `asyncio.get_event_loop().getaddrinfo()` or run in executor.

### Docs changes needed
- `Features.md` — document URL validation constraints for blocklist sources.
- `Backend-Development.md` — SSRF prevention pattern.

### Doc references
- [Features.md](Features.md) — blocklist management
- [Backend-Development.md](Backend-Development.md) — input validation

---

## TASK-010 — `fail2ban_start_command` split with `.split()` instead of `shlex.split()`

**Severity:** Low

### Where found
`backend/app/config.py` — `fail2ban_start_command` field description says "Split by whitespace to build the argument list". Usages in `backend/app/services/server_service.py` (or similar) call `.split()`.

### Why this is needed
`.split()` splits on any whitespace but does not respect shell quoting. A command like `"/opt/my tools/fail2ban-client" start` is split into three tokens instead of two, breaking execution when the path contains spaces.

### Goal
Use `shlex.split()` to tokenize the start command so quoted arguments are handled correctly.

### What to do
1. Find all call sites of `fail2ban_start_command.split()` and replace with `shlex.split(fail2ban_start_command)`.
2. Add a `@field_validator("fail2ban_start_command")` in `Settings` that calls `shlex.split(v)` and raises `ValueError` if it fails (mismatched quotes), so misconfiguration is caught at startup.

### Possible traps and issues
- `shlex.split()` raises `ValueError` for unmatched quotes — catch this in the validator and convert to a descriptive `ValueError`.
- The validator runs at startup and should include the problematic value in the error message.

### Docs changes needed
- `Backend-Development.md` — document the `fail2ban_start_command` format.

### Doc references
- [Backend-Development.md](Backend-Development.md) — configuration options

---

## TASK-011 — Session token prefix logged on login and logout

**Severity:** Low

### Where found
`backend/app/services/auth_service.py` line ~115: `log.info("bangui_login_success", token_prefix=session.token[:8])` and line ~173: `log.info("bangui_logout", token_prefix=token[:8])`.

### Why this is needed
Logging `token[:8]` (the first 8 hex characters) leaks partial token material into log files. Log files may be forwarded to less-secure log aggregation systems. Even partial token material can aid in token forgery or DB correlation attacks when combined with other information.

### Goal
Remove all token fragments from structured log output. Use a non-sensitive identifier instead.

### What to do
1. In `login()`, replace `token_prefix=session.token[:8]` with `session_id=session.id` (the integer row ID from the DB).
2. In `logout()`, the raw token is available before the session row is fetched. Replace `token_prefix=token[:8]` with `token_hash=hashlib.sha256(token.encode()).hexdigest()[:12]` — a one-way hash fragment that is useful for log correlation without revealing the token.

### Possible traps and issues
- The session ID is only available after `session_repo.create_session()` returns — this is already the case in `login()`.
- In `logout()`, the session row is deleted before logging — use the hash approach instead of the DB ID.

### Docs changes needed
- `Backend-Development.md` — logging conventions (no sensitive data in log fields).

### Doc references
- [Backend-Development.md](Backend-Development.md) — structured logging rules

---

## TASK-012 — `SetupGuard` fires duplicate API calls on mount

**Severity:** Low

### Where found
Frontend setup guard component — the setup status check is performed by multiple consumers independently on mount, resulting in duplicate `GET /api/setup` requests.

### Why this is needed
Duplicate API calls on mount create unnecessary backend load and introduce potential race conditions where both calls return slightly different states. It also increases perceived load time.

### Goal
Deduplicate the setup status check so all consumers share a single in-flight request.

### What to do
1. Move the setup status fetch into a shared React Query (`useQuery`) call keyed by `"setupStatus"`.
2. Any component that needs the setup status reads from the same query cache — React Query deduplicates concurrent requests automatically.
3. Remove any direct `fetch` calls for setup status that bypass the shared query.

### Possible traps and issues
- React Query must be installed and configured at the app root.
- The cache TTL for setup status should be relatively short (e.g., 30 seconds) or invalidated after a successful setup completion.

### Docs changes needed
- `Web-Development.md` — data-fetching conventions (use React Query, not raw `fetch`).

### Doc references
- [Web-Development.md](Web-Development.md) — data fetching patterns

---

## TASK-013 — nginx missing security response headers

**Severity:** High

### Where found
`Docker/nginx.conf` — the server block has no `Content-Security-Policy`, `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, or `Strict-Transport-Security` headers.

### Why this is needed
Without these headers:
- **No CSP** — injected scripts can run freely (XSS).
- **No X-Frame-Options** — the app can be embedded in an iframe on an attacker-controlled page (clickjacking).
- **No X-Content-Type-Options** — browsers may MIME-sniff responses and execute text/plain as JavaScript.
- **No Referrer-Policy** — internal URLs are leaked in the `Referer` header to third-party resources.
- **No HSTS** — even with HTTPS configured, browsers will still attempt HTTP first unless told otherwise.

### Goal
Add all OWASP-recommended security headers to the nginx server block.

### What to do
Add to the `server` block in `nginx.conf`:
```nginx
add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'; frame-ancestors 'none';" always;
add_header X-Frame-Options "DENY" always;
add_header X-Content-Type-Options "nosniff" always;
add_header Referrer-Policy "no-referrer" always;
add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always;
# Only add HSTS when HTTPS is confirmed:
# add_header Strict-Transport-Security "max-age=63072000; includeSubDomains; preload" always;
```

### Possible traps and issues
- Fluent UI v9 uses inline `style` attributes — `style-src 'self' 'unsafe-inline'` is required for now. A stricter CSP using nonces would require server-side rendering of the HTML shell.
- HSTS must only be added when HTTPS is fully configured and working — it is irreversible for the configured `max-age`.
- Use `always` on `add_header` so headers are also included in error responses (4xx, 5xx).

### Docs changes needed
- `Architekture.md` — document the nginx security header configuration.

### Doc references
- [Architekture.md](Architekture.md) — nginx configuration

---

## TASK-014 — `add_log_path` passes arbitrary paths to fail2ban — no allowlist

**Severity:** High

### Where found
`backend/app/services/config_service.py` — `add_log_path()`. `backend/app/models/config.py` — `AddLogPathRequest.log_path: str`.

### Why this is needed
An authenticated user can instruct fail2ban to monitor any file path on the system (e.g., `log_path: "/etc/shadow"`). fail2ban runs as root and opens the file for reading. Even if fail2ban cannot meaningfully parse it, repeated log monitoring of sensitive files can leak their contents via fail2ban's own logging, and the feature represents an unintended read primitive into arbitrary root-readable files.

### Goal
Restrict monitored log paths to a configurable set of safe directories.

### What to do
1. Add a `@field_validator("log_path")` to `AddLogPathRequest` that:
   - Calls `Path(log_path).resolve()` to canonicalize.
   - Checks `resolved.is_relative_to(Path("/var/log"))` or any path in `settings.allowed_log_dirs` (a new configurable list).
   - Raises `ValueError` if the path is outside allowed prefixes.
2. Add `BANGUI_ALLOWED_LOG_DIRS` to `Settings` as `list[str]` defaulting to `["/var/log", "/config/log"]`.
3. Note: use `is_relative_to()`, not `startswith()` — the latter is bypassable with `/var/log_evil`.

### Possible traps and issues
- The validator runs on the Pydantic model before the service is called — the resolved path check happens at request time, not at the OS level. The allowed list must match the actual Docker volume mount paths.
- Custom log file locations (e.g., `/home/app/logs`) need to be added to `BANGUI_ALLOWED_LOG_DIRS`.

### Docs changes needed
- `Features.md` — document the log path restrictions.
- `Backend-Development.md` — input validation for path parameters.

### Doc references
- [Features.md](Features.md) — jail log monitoring
- [Backend-Development.md](Backend-Development.md) — path validation

---

## TASK-015 — `GlobalConfigUpdate.log_target`/`log_level` have no validation

**Severity:** High

### Where found
`backend/app/models/config.py` — `GlobalConfigUpdate`. `backend/app/services/config_service.py` — `update_global_config()`.

### Why this is needed
`log_target` is forwarded raw to fail2ban via the Unix socket. fail2ban (running as root) creates or opens the file at that path if it does not exist. `log_level` is forwarded raw without checking it is a valid fail2ban log level. Both fields represent an injection path into fail2ban's internal state from an authenticated but potentially compromised account.

### Goal
Validate both fields before forwarding to fail2ban.

### What to do
1. Change `log_target` in `GlobalConfigUpdate` to accept only:
   - `Literal["STDOUT", "STDERR", "SYSLOG"]`, or
   - A path validated the same way as `AddLogPathRequest.log_path` (see TASK-014).
2. Change `log_level` to `Literal["CRITICAL", "ERROR", "WARNING", "NOTICE", "INFO", "DEBUG"]`.
3. Apply the same restrictions in `get_global_config` responses for consistency.

### Possible traps and issues
- The allowlist for `log_target` paths must be consistent with TASK-014 (`BANGUI_ALLOWED_LOG_DIRS`).
- Existing deployments using non-standard `log_target` values (e.g., `/var/log/fail2ban.log`) must still work — ensure `/var/log` is in the default allowlist.

### Docs changes needed
- `Features.md` — document valid values for `log_target` and `log_level`.
- `Backend-Development.md` — Pydantic Literal types for constrained string fields.

### Doc references
- [Features.md](Features.md) — global fail2ban configuration
- [Backend-Development.md](Backend-Development.md) — model validation

---

## TASK-016 — `delete_log_path` query parameter unvalidated

**Severity:** Medium

### Where found
`backend/app/routers/jail_config.py` — `DELETE /api/config/jails/{name}/logpath` — `log_path: str = Query(...)`.

### Why this is needed
The `log_path` query parameter is passed directly to the fail2ban socket command `["set", name, "dellogpath", log_path]` without any path validation. An attacker could pass traversal strings or paths to sensitive files, instructing fail2ban to stop monitoring them and potentially confusing fail2ban's internal state.

### Goal
Apply the same allowlist validation as `add_log_path` (TASK-014) to `delete_log_path`.

### What to do
1. Extract the log path validation logic from TASK-014 into a shared helper function in `backend/app/utils/path_utils.py` (e.g., `validate_log_path(path: str, allowed_dirs: list[str]) -> str`).
2. Call the helper from both `AddLogPathRequest` validator and the `delete_log_path` route handler.
3. Return 422 with a descriptive error if validation fails.

### Possible traps and issues
- Query parameters cannot have Pydantic field validators directly in FastAPI — use a `Depends` dependency that validates and returns the resolved path, or validate explicitly at the start of the route handler.

### Docs changes needed
- `Backend-Development.md` — path validation helper usage.

### Doc references
- [Backend-Development.md](Backend-Development.md) — input validation patterns

---

## TASK-017 — `ip LIKE ?` without escaping `%` and `_` wildcards

**Severity:** Medium

### Where found
`backend/app/repositories/fail2ban_db_repo.py` and `backend/app/repositories/history_archive_repo.py` — SQL queries using `ip LIKE ?` with `f"{ip_filter}%"` interpolation.

### Why this is needed
SQLite's `LIKE` operator treats `%` (any sequence of characters) and `_` (any single character) as wildcards. If an IP filter value contains these characters — unusual for well-formed IPs, but possible via crafted input — the query matches unintended rows. For example, `ip_filter = "10.0.0_"` would match `10.0.0.1` through `10.0.0.9`.

### Goal
Escape LIKE metacharacters in all `LIKE` query parameters.

### What to do
1. Escape `%` → `\%` and `_` → `\_` in the filter string before use.
2. Add `ESCAPE '\'` to the SQL: `ip LIKE ? ESCAPE '\'`.
3. Extract this into a helper: `def escape_like(s: str) -> str: return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")`.

### Possible traps and issues
- The backslash escape character itself must also be escaped first to avoid double-escaping. Process in the order: `\` → `\\`, then `%` → `\%`, then `_` → `\_`.
- Test with IPs that contain dots — dots are not LIKE wildcards in SQLite, so they do not need escaping.

### Docs changes needed
- `Backend-Development.md` — database query conventions (LIKE escaping).

### Doc references
- [Backend-Development.md](Backend-Development.md) — database access patterns

---

## TASK-018 — `_write_conf_file` and `_create_conf_file` not atomic

**Severity:** Medium

### Where found
`backend/app/services/config_file_helpers.py` lines ~190 (`_write_conf_file`) and ~226 (`_create_conf_file`) — both use `target.write_text(content, encoding="utf-8")` directly.

### Why this is needed
`Path.write_text()` overwrites the file in place. If the process is killed mid-write, the config file is left in a truncated or corrupt state. fail2ban config files are critical — a corrupt `jail.d/sshd.conf` prevents fail2ban from reloading and may disable active protection.

### Goal
Make all config file writes atomic using write-to-temp + rename.

### What to do
1. Replace `target.write_text(content)` with:
   ```python
   import tempfile, os
   tmp_fd, tmp_path = tempfile.mkstemp(dir=target.parent, suffix=".tmp")
   try:
       with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
           f.write(content)
       os.replace(tmp_path, target)
   except Exception:
       os.unlink(tmp_path)
       raise
   ```
2. Apply to both `_write_conf_file` and `_create_conf_file`.
3. This pattern is already used correctly in `jail_config_service.py` — follow that exact implementation.

### Possible traps and issues
- The temp file must be in the same directory as the target (`dir=target.parent`) so `os.replace()` is atomic (same filesystem, single rename syscall).
- On Windows, `os.replace()` may fail if the target is open — not relevant for Linux containers, but worth noting.

### Docs changes needed
- `Backend-Development.md` — atomic file write conventions.

### Doc references
- [Backend-Development.md](Backend-Development.md) — file I/O patterns

---

## TASK-019 — `session_secret` has no minimum-length enforcement

**Severity:** Medium

### Where found
`backend/app/config.py` — `session_secret: str = Field(..., description="...")`. No `min_length` constraint.

### Why this is needed
`session_secret` is the HMAC key used to sign all session tokens. A secret shorter than 32 characters (256 bits) significantly weakens HMAC-SHA256. The app currently accepts any non-empty string, including a single character.

### Goal
Enforce a minimum secret length of 32 characters at startup.

### What to do
1. Add `min_length=32` to the `session_secret` Field definition.
2. Update the error message to explain: `"session_secret must be at least 32 characters. Generate one with: python -c \"import secrets; print(secrets.token_hex(32))\""`.

### Possible traps and issues
- This is a breaking change for any existing deployment where the secret is shorter than 32 characters. Include a migration note in the changelog.
- The debug compose file uses `dev-secret-do-not-use-in-production` (42 chars) — this already passes the 32-char check, so dev environments are unaffected.

### Docs changes needed
- `Backend-Development.md` — document the `session_secret` constraint.

### Doc references
- [Backend-Development.md](Backend-Development.md) — configuration reference

---

## TASK-020 — `log_target` accepts arbitrary paths — root file write via fail2ban (CRITICAL)

**Severity:** Critical

### Where found
`backend/app/models/config.py` — `GlobalConfigUpdate.log_target: str | None`. `backend/app/services/config_service.py` — `update_global_config()` forwards the value to fail2ban without validation.

### Why this is needed
fail2ban runs as root. When `log_target` is set to a path, fail2ban opens (and if necessary creates) that file for writing. An authenticated user can send `PUT /api/config/global` with `{"log_target": "/etc/cron.d/bangui-pwned"}`, causing fail2ban to create that file as root. With crafted content appended via fail2ban's own logging, this escalates to a root write primitive and potentially to Remote Code Execution.

### Goal
Block all `log_target` values that are not `"STDOUT"`, `"STDERR"`, `"SYSLOG"`, or a path under the configured allowed log directories.

### What to do
1. **Immediate:** Add a strict `@field_validator("log_target")` to `GlobalConfigUpdate` that enforces the allowlist (see TASK-015 — this task and TASK-015 share the same fix).
2. **Defense in depth:** Before sending the command to fail2ban in `update_global_config()`, validate again at the service layer (not just the model layer).
3. Add a regression test: `POST /api/config/global` with `log_target="/etc/passwd"` must return 422.

### Possible traps and issues
- This must be fixed before TASK-015 since it is the more severe variant. The fixes are identical — implement them together.
- Pydantic model validators run before the service receives the value, but an integration test confirming the full request path is essential.

### Docs changes needed
- `Features.md` — document valid log_target values.
- `Backend-Development.md` — critical input validation requirement for config endpoints.

### Doc references
- [Features.md](Features.md) — fail2ban global configuration
- [Backend-Development.md](Backend-Development.md) — input validation

---

## TASK-021 — `set_jail_config_enabled` and `write_jail_config_file` not atomic

**Severity:** Medium

### Where found
`backend/app/services/raw_config_io_service.py` lines ~268 (`set_jail_config_enabled`) and ~344 (`write_jail_config_file`) — both use `path.write_text(updated)` directly.

### Why this is needed
Same root cause as TASK-018. A process kill mid-write leaves the jail config file corrupted, disabling that jail on next fail2ban reload.

### Goal
Atomic writes for `set_jail_config_enabled` and `write_jail_config_file`.

### What to do
Same as TASK-018: replace `path.write_text(content)` with the `NamedTemporaryFile` + `os.replace()` pattern in both functions. This is most efficiently done as part of TASK-018 by extracting a shared `atomic_write(path, content)` helper in `config_file_helpers.py`.

### Possible traps and issues
- Same as TASK-018.
- Extracting the helper makes TASK-018 and TASK-021 a single coordinated change.

### Docs changes needed
- `Backend-Development.md` — atomic write helper documentation.

### Doc references
- [Backend-Development.md](Backend-Development.md) — file I/O conventions

---

## TASK-022 — Session tokens stored in plaintext in SQLite

**Severity:** High

### Where found
`backend/app/db.py` — `sessions` table schema: `token TEXT NOT NULL UNIQUE`. `backend/app/repositories/session_repo.py` — `INSERT INTO sessions (token, ...)` and `SELECT ... WHERE token = ?` both use the raw token value.

### Why this is needed
If the BanGUI SQLite database file is exposed (volume mount misconfiguration, backup leak, path traversal via another vulnerability), all active session tokens are immediately usable — no cracking required. The attacker can directly use the token in the `bangui_session` cookie or `Authorization: Bearer` header.

### Goal
Store a one-way hash of the session token in the database so that the DB file alone is not sufficient to hijack a session.

### What to do
1. In `session_repo.create_session()`, store `hashlib.sha256(token.encode()).hexdigest()` instead of `token` in the `token` column.
2. In `session_repo.get_session()` and `delete_session()`, hash the supplied token before the SQL lookup.
3. The `Session` model's `token` field returned to the service layer still contains the raw token (for use in signing and response) — only the DB column changes.
4. Add a migration (`_MIGRATIONS[2]`) that renames the existing `sessions` table to `sessions_old`, creates a new one, and drops `sessions_old` (or simply truncates all sessions on upgrade, since they are all compromised anyway once the DB was readable in plaintext).

### Possible traps and issues
- Coordinate with TASK-025 (HMAC bypass) — both fixes invalidate all existing sessions. Do them in the same release.
- The migration must be atomic (see TASK-023).
- The `Session.token` field name is slightly misleading once it stores a hash — consider renaming the DB column to `token_hash`.

### Docs changes needed
- `Architekture.md` — update session data model description.
- `Backend-Development.md` — document the session token hashing pattern.

### Doc references
- [Architekture.md](Architekture.md) — authentication and session model
- [Backend-Development.md](Backend-Development.md) — security patterns

---

## TASK-023 — Database migration is non-atomic

**Severity:** Medium

### Where found
`backend/app/db.py` — `_apply_migration()`: calls `db.executescript(migration_script)` (which auto-commits per SQLite Python driver behavior) and then separately `db.execute("INSERT INTO schema_migrations ...")` + `db.commit()`.

### Why this is needed
`executescript()` issues an implicit `COMMIT` before executing the script, so the schema change and the migration record insertion are in two separate transactions. A process crash between them leaves the database in a migrated-but-unrecorded state. On next startup, the migration is re-applied. For a migration that is not idempotent (e.g., `INSERT` without `OR IGNORE`, `ALTER TABLE ADD COLUMN` without `IF NOT EXISTS`), this causes a runtime error or data duplication.

### Goal
Wrap each migration's DDL and its `schema_migrations` record in a single atomic transaction.

### What to do
1. Replace `db.executescript(migration_script)` with individual `await db.execute(stmt)` calls for each DDL statement in the migration (split on `;`).
2. Wrap the entire migration (all DDL statements + the `INSERT INTO schema_migrations`) in an explicit `BEGIN IMMEDIATE` ... `COMMIT` transaction.
3. Test: verify that a simulated crash mid-migration (mocked `execute` that raises on the second statement) leaves the DB at its prior version.

### Possible traps and issues
- SQLite DDL in WAL mode: `CREATE TABLE IF NOT EXISTS` and `CREATE INDEX IF NOT EXISTS` are safe to re-run. `ALTER TABLE ADD COLUMN` is not — it must be guarded with a `PRAGMA table_info` check if used in future migrations.
- Splitting a migration script on `;` must handle semicolons inside string literals and comments. Consider storing each migration as a `list[str]` of individual statements instead of a single script string.

### Docs changes needed
- `Backend-Development.md` — migration authoring guidelines.

### Doc references
- [Backend-Development.md](Backend-Development.md) — database schema and migrations

---

## TASK-024 — No CSRF protection on state-mutating endpoints

**Severity:** High

### Where found
All `POST`, `PUT`, `DELETE` routes in `backend/app/routers/`. Only `SameSite=Lax` on the session cookie provides any CSRF protection.

### Why this is needed
`SameSite=Lax` blocks cross-site `<form>` POST requests but does **not** block `fetch(..., {credentials: "include"})` initiated by JavaScript on a subdomain or a same-origin XSS injection. Without a CSRF token or `Origin` header check, a compromised subdomain can issue authenticated requests on behalf of the logged-in user.

### Goal
Add explicit CSRF protection for all cookie-authenticated state-mutating endpoints.

### What to do
**Option A (recommended — custom header check):**
1. Add a middleware that, for all `POST`/`PUT`/`DELETE`/`PATCH` requests authenticated via cookie (not `Authorization: Bearer`), requires the presence of a custom header: `X-BanGUI-Request: 1`.
2. The frontend API client (`frontend/src/api/client.ts`) already uses a shared `request()` function — add `"X-BanGUI-Request": "1"` to the default headers there.
3. Cross-site `fetch()` calls cannot set custom headers without CORS preflight, which the backend rejects (CORS is only configured for allowed origins).

**Option B — Origin header validation:**
Add middleware that checks `Origin` or `Referer` matches the configured allowed origin for all mutating requests.

### Possible traps and issues
- The Bearer-token path (`Authorization: Bearer`) does not use cookies and is therefore not CSRF-vulnerable — do not apply the check to those requests.
- Detecting cookie-vs-bearer authentication in middleware requires reading request headers before the auth dependency runs — check for `Cookie: bangui_session=` presence.
- Do not apply CSRF checks to `GET`, `HEAD`, `OPTIONS` requests.

### Docs changes needed
- `Architekture.md` — document the CSRF protection mechanism.
- `Backend-Development.md` — CSRF middleware.
- `Web-Development.md` — document the `X-BanGUI-Request` header requirement.

### Doc references
- [Architekture.md](Architekture.md) — security architecture
- [Backend-Development.md](Backend-Development.md) — security patterns
- [Web-Development.md](Web-Development.md) — API client conventions

---

## TASK-025 — `unwrap_session_token` legacy fallback bypasses HMAC check entirely

**Severity:** High

### Where found
`backend/app/services/auth_service.py` — `unwrap_session_token()` lines 44–49:
```python
if SESSION_TOKEN_SIGNATURE_SEPARATOR not in token:
    return token  # HMAC check skipped entirely
```

### Why this is needed
Any token that does not contain the separator character is returned unchanged as a "valid" token — the HMAC signature is never verified. Combined with TASK-022 (plaintext DB), an attacker who reads the database can take a raw token (no separator) and use it directly, bypassing the HMAC layer entirely. The signing mechanism provides zero additional security once the DB is readable.

### Goal
Remove the HMAC bypass. All tokens must carry a valid signature.

### What to do
1. Remove the early-return branch: `if SESSION_TOKEN_SIGNATURE_SEPARATOR not in token: return token`.
2. If the separator is absent, raise `ValueError("Invalid session token.")`.
3. This invalidates all sessions created before HMAC signing was introduced — coordinate with TASK-022 (all sessions should be invalidated during that migration anyway).
4. Update all tests that use raw unsigned tokens.

### Possible traps and issues
- Any test that constructs a raw token without a signature will start failing — this is intentional, update the tests.
- The `unwrap_session_token` docstring mentions "backward compatibility with existing raw session tokens stored in the DB" — remove this rationale once TASK-022 hashes the DB column (raw tokens will no longer be in the DB).

### Docs changes needed
- `Backend-Development.md` — document the session token format (signed only).

### Doc references
- [Backend-Development.md](Backend-Development.md) — authentication internals

---

## TASK-026 — OpenAPI docs (`/docs`, `/redoc`) exposed without authentication in production

**Severity:** Medium

### Where found
`backend/app/main.py` — `FastAPI(title="BanGUI", ...)` with no `docs_url`, `redoc_url`, or `openapi_url` override.

### Why this is needed
FastAPI serves interactive API documentation at `/docs` (Swagger UI) and `/redoc` by default, accessible to any unauthenticated user. These pages expose every endpoint URL, all request and response schemas, and allow direct API invocation from the browser. An attacker can enumerate all available routes, understand input/output models, and attempt attacks without any prior knowledge of the API.

### Goal
Disable API docs in production, or protect them behind authentication.

### What to do
1. Add a `BANGUI_ENABLE_DOCS: bool = Field(default=False, ...)` setting to `Settings`.
2. In `create_app()`:
   ```python
   docs_url = "/api/docs" if resolved_settings.enable_docs else None
   redoc_url = "/api/redoc" if resolved_settings.enable_docs else None
   openapi_url = "/api/openapi.json" if resolved_settings.enable_docs else None
   app = FastAPI(..., docs_url=docs_url, redoc_url=redoc_url, openapi_url=openapi_url)
   ```
3. In `compose.debug.yml`, set `BANGUI_ENABLE_DOCS: "true"`.
4. Production (`compose.prod.yml`) leaves `BANGUI_ENABLE_DOCS` unset (defaults to `false`).

### Possible traps and issues
- The `SetupRedirectMiddleware` must allow `/api/docs` and `/api/openapi.json` in `_ALWAYS_ALLOWED` (or behind auth — a protected docs endpoint requires a custom Starlette route with `require_auth`).
- If you want protected docs (only accessible when logged in), use a custom route that returns the Swagger HTML after `require_auth`.

### Docs changes needed
- `Backend-Development.md` — document the `BANGUI_ENABLE_DOCS` flag.

### Doc references
- [Backend-Development.md](Backend-Development.md) — configuration options

---

## TASK-027 — Debug compose hardcodes a publicly known weak session secret

**Severity:** Medium

### Where found
`Docker/compose.debug.yml` line ~63:
```yaml
BANGUI_SESSION_SECRET: "${BANGUI_SESSION_SECRET:-dev-secret-do-not-use-in-production}"
```

### Why this is needed
The fallback value `dev-secret-do-not-use-in-production` is now publicly visible in the repository. If `compose.debug.yml` is used in any environment where `BANGUI_SESSION_SECRET` is not set (e.g., a CI environment or a staging server that uses the debug compose file), all session tokens can be forged by anyone who has seen this repository.

### Goal
Remove the insecure default. Require the secret to be set explicitly before the container starts.

### What to do
1. Change to `BANGUI_SESSION_SECRET: "${BANGUI_SESSION_SECRET:?BANGUI_SESSION_SECRET must be set — generate with: python -c 'import secrets; print(secrets.token_hex(32))'}"`.
2. Create a `.env.example` file at the project root with placeholder values and generation instructions.
3. Add `.env` to `.gitignore` (verify it is already there).

### Possible traps and issues
- This will break `docker compose -f Docker/compose.debug.yml up` without a `.env` file. Add a clear error message and setup instructions to the README or `Instructions.md`.
- `docker-compose.yml` (the legacy file) already uses the `:?` pattern — follow the same approach.

### Docs changes needed
- `Instructions.md` — add first-run setup instructions for the `.env` file.

### Doc references
- [Instructions.md](Instructions.md) — developer setup

---

## 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

### Where found
`backend/app/exceptions.py` — `Fail2BanConnectionError.__init__()` formats the message as `f"{message} (socket: {socket_path})"`. `backend/app/main.py` — `_fail2ban_connection_handler()` returns `{"detail": f"Cannot reach fail2ban: {exc}"}` verbatim.

### Why this is needed
Every 502 response caused by fail2ban being unreachable includes the full socket path (e.g., `Cannot reach fail2ban: [Errno 2] No such file or directory (socket: /var/run/fail2ban/fail2ban.sock)`) in the JSON error body. This discloses internal infrastructure details to unauthenticated users who can trigger the error. Similarly, `_fail2ban_protocol_handler` includes raw exception details that may expose internal parsing logic.

### Goal
Return generic, user-friendly error messages in HTTP responses. Log full details server-side only.

### What to do
1. In `_fail2ban_connection_handler()`, replace:
   ```python
   content={"detail": f"Cannot reach fail2ban: {exc}"}
   ```
   with:
   ```python
   content={"detail": "Cannot reach the fail2ban service. Check the server status page."}
   ```
2. In `_fail2ban_protocol_handler()`, similarly return a generic message.
3. Both handlers already log `error=str(exc)` server-side — this is correct and should remain.

### Possible traps and issues
- Update any tests that assert the exact `detail` string in 502 responses.
- If the frontend displays this error message directly to the user, ensure it still makes sense after genericizing.

### Docs changes needed
- `Backend-Development.md` — error message hygiene (no internal paths/details in responses).

### Doc references
- [Backend-Development.md](Backend-Development.md) — error handling

---

## TASK-030 — ip-api.com geo lookups use plain HTTP — IP addresses sent unencrypted

**Severity:** Medium

### Where found
`backend/app/services/geo_cache.py` lines ~41–46:
```python
_API_URL = "http://ip-api.com/json/{ip}?fields=..."
_BATCH_API_URL = "http://ip-api.com/batch?fields=..."
```

### Why this is needed
All banned and monitored IP addresses are transmitted to ip-api.com in cleartext over HTTP. These are potentially sensitive data (PII under GDPR/CCPA — IP addresses identify users). Any network path between the BanGUI server and ip-api.com's servers can observe or modify the traffic. Forged responses would corrupt the geo database silently.

### Goal
Use encrypted transport for all geo API calls, or switch to a local resolver.

### What to do
ip-api.com's free tier does not support HTTPS. The recommended approach:
1. Promote the existing `geoip_db_path` setting (MaxMind GeoLite2-Country MMDB) to the **primary** resolver.
2. Use ip-api.com as a secondary fallback only when the MMDB is unavailable or returns no result.
3. Add documentation and compose file examples for downloading and mounting the GeoLite2 MMDB.
4. If ip-api.com HTTP is retained as a fallback, add a config flag `BANGUI_GEOIP_ALLOW_HTTP_FALLBACK` (default `false`) and warn clearly at startup when enabled.

### Possible traps and issues
- The MaxMind GeoLite2 database requires a free account and a license key to download — document the setup process.
- The GeoLite2-Country MMDB does not include ASN or organisation data — these fields will be `null` when using the local resolver. The `GeoInfo` model must handle nullable `asn` and `org`.

### Docs changes needed
- `Features.md` — document the geo resolution mechanism and MMDB setup.
- `Architekture.md` — update the external API dependency section.
- `Backend-Development.md` — configuration for `geoip_db_path`.

### Doc references
- [Features.md](Features.md) — geolocation
- [Architekture.md](Architekture.md) — external API dependencies

---

## TASK-031 — bcrypt 72-byte truncation not enforced — long passwords silently equivalent to their prefix

**Severity:** Medium

### Where found
`backend/app/models/auth.py` — `LoginRequest.password: str = Field(...)` (no `max_length`). `backend/app/models/setup.py` — `SetupRequest.master_password` has `min_length=8` but no `max_length`.

### Why this is needed
bcrypt silently truncates all input at 72 bytes before hashing. A user who sets a 100-character password can be authenticated by supplying only the first 72 characters. The extra characters provide no additional security. An attacker who has reduced the search space to 72 characters can brute-force the password more efficiently than the user intended.

### Goal
Enforce a maximum password length of 72 bytes, or pre-hash before bcrypt to remove the limit entirely.

### What to do
**Option A (simple):**
1. Add `max_length=72` to `SetupRequest.master_password` and `LoginRequest.password`.
2. Update the setup wizard UI to reflect the 72-character maximum.

**Option B (removes the 72-byte limit entirely):**
1. Pre-hash the password with HMAC-SHA256 using the `session_secret` as the key before passing to bcrypt:
   ```python
   pre_hashed = hmac.new(secret.encode(), password.encode(), hashlib.sha256).digest()
   bcrypt.hashpw(pre_hashed, bcrypt.gensalt())
   ```
2. Apply consistently in both `run_setup()` and `_check_password()`.

Option A is recommended as the simpler, lower-risk fix. Option B is architecturally cleaner but requires a stored hash migration.

### Possible traps and issues
- Option A: Users who already have passwords longer than 72 characters will need to reset. For a single-admin app this is acceptable.
- Option B: If the `session_secret` changes, all stored password hashes become invalid (since the pre-hash key changes). This is a hidden coupling — document it explicitly.

### Docs changes needed
- `Features.md` — document the password length constraint.
- `Backend-Development.md` — bcrypt usage notes.

### Doc references
- [Features.md](Features.md) — authentication and setup
- [Backend-Development.md](Backend-Development.md) — password hashing

---

## TASK-032 — `geo_cache` table grows unboundedly — no eviction or purge

**Severity:** Medium

### Where found
`backend/app/repositories/geo_cache_repo.py` — has `upsert_entry`, `bulk_upsert_entries`, `upsert_neg_entry` — but **no DELETE functions**. `backend/app/db.py` — `geo_cache` table has no `last_seen` or `created_at` column.

### Why this is needed
Every unique IP address ever seen by fail2ban gets a row in `geo_cache`. The table is never trimmed. A BanGUI instance monitoring a busy server can accumulate millions of rows over months, increasing the DB file size and degrading query performance on every geo lookup.

### Goal
Implement a retention policy that prunes geo cache entries not referenced recently.

### What to do
1. Add a migration (`_MIGRATIONS[2]`) that adds a `last_seen TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP` column to `geo_cache`.
2. Update `upsert_entry` and `bulk_upsert_entries` to set `last_seen = CURRENT_TIMESTAMP` on every upsert.
3. Add `delete_stale_entries(db: aiosqlite.Connection, cutoff_iso: str) -> int` to `geo_cache_repo.py`.
4. Create `backend/app/tasks/geo_cache_cleanup.py` — a nightly task that calls `delete_stale_entries` with a 90-day cutoff.
5. Register the task in `startup_shared_resources`.

### Possible traps and issues
- Adding a column requires a migration. Coordinate with TASK-023 (migration atomicity) and TASK-022 (session hash migration) — all three migrations must be sequenced correctly as `_MIGRATIONS[2]`, `[3]`, etc.
- IPs that have not been seen in 90 days will lose their geo data — on their next appearance they will be re-resolved from ip-api.com or the MMDB. This is acceptable.

### Docs changes needed
- `Architekture.md` — update the `geo_cache` table description and add the cleanup task.
- `Backend-Development.md` — document the geo cache retention policy.

### Doc references
- [Architekture.md](Architekture.md) — application database schema
- [Backend-Development.md](Backend-Development.md) — background tasks

---

## TASK-033 — Session token returned in JSON body alongside HttpOnly cookie

**Severity:** Medium

### Where found
`backend/app/routers/auth.py` — `login()` returns `LoginResponse(token=signed_token, expires_at=expires_at)` in the JSON body **and** sets the HttpOnly cookie. `backend/app/models/auth.py` — `LoginResponse.token` field.

### Why this is needed
The `LoginResponse` JSON body contains the full signed session token. JavaScript running on the page (including third-party analytics scripts or a future XSS injection) can read the response body from a `fetch()` call and store the token in `localStorage` or a non-HttpOnly cookie. The Bearer-header authentication path (`Authorization: Bearer <token>`) then allows using that extracted token, completely bypassing the protections provided by the HttpOnly cookie.

### Goal
Prevent the session token from being accessible to JavaScript when using cookie-based authentication.

### What to do
1. For browser SPA consumers: Remove the `token` field from `LoginResponse`. The HttpOnly cookie is the only token the browser needs.
2. If an API-first (non-browser) token flow is required, create a separate endpoint `POST /api/auth/token` that returns a token in the body and does **not** set a cookie. Document this endpoint as "for programmatic API clients only, not for browser use".
3. Update the frontend — verify that `AuthProvider` does not use `response.token` (confirmed: it currently does not).

### Possible traps and issues
- Any existing API client that relies on the token in the `LoginResponse` body will break. Check tests.
- The `expires_at` field in `LoginResponse` is useful for the frontend to know when to prompt for re-login — this can remain.
- The Bearer-token path in `require_auth` (`Authorization: Bearer`) remains functional for programmatic clients using the dedicated token endpoint.

### Docs changes needed
- `Features.md` — document the authentication flow (cookie for browser, token endpoint for API clients).
- `Backend-Development.md` — authentication endpoint design.
- `Web-Development.md` — document that the frontend uses only the HttpOnly cookie.

### Doc references
- [Features.md](Features.md) — authentication
- [Backend-Development.md](Backend-Development.md) — auth router design
- [Web-Development.md](Web-Development.md) — AuthProvider