TASK-011: Remove session token prefix from log output
Replace sensitive token fragments in structured logs with: - login(): Use session_id=session.id (database row ID) instead of token_prefix - logout(): Use token_hash (SHA256 one-way hash, first 12 chars) instead of token_prefix This prevents partial token material leakage into log aggregation systems while maintaining useful session correlation via hashed tokens or database IDs. Also updated Backend-Development.md to clarify logging conventions for sensitive data handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -246,10 +246,13 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]:
|
|||||||
- Use **structlog** for every log message.
|
- Use **structlog** for every log message.
|
||||||
- Bind contextual key-value pairs — never format strings manually.
|
- Bind contextual key-value pairs — never format strings manually.
|
||||||
- Log levels: `debug` for development detail, `info` for operational events, `warning` for recoverable issues, `error` for failures, `critical` for fatal problems.
|
- Log levels: `debug` for development detail, `info` for operational events, `warning` for recoverable issues, `error` for failures, `critical` for fatal problems.
|
||||||
- Never log sensitive data (passwords, tokens, session IDs).
|
- **Never log sensitive data** (passwords, tokens, session tokens, raw credentials, private keys).
|
||||||
|
- For session correlation without leaking token material, use a one-way hash fragment: `hashlib.sha256(token.encode()).hexdigest()[:12]`.
|
||||||
|
- Use numeric database IDs for entity correlation instead of raw identifiers: `session_id=session.id` instead of `token=session.token`.
|
||||||
|
|
||||||
```python
|
```python
|
||||||
import structlog
|
import structlog
|
||||||
|
import hashlib
|
||||||
|
|
||||||
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||||
|
|
||||||
@@ -261,6 +264,12 @@ async def ban_ip(ip: str, jail: str) -> None:
|
|||||||
except BanError as exc:
|
except BanError as exc:
|
||||||
log.error("ban_failed", ip=ip, jail=jail, error=str(exc))
|
log.error("ban_failed", ip=ip, jail=jail, error=str(exc))
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
async def logout_session(db: aiosqlite.Connection, token: str) -> None:
|
||||||
|
# Use a one-way hash for token correlation in logs
|
||||||
|
token_hash = hashlib.sha256(token.encode()).hexdigest()[:12]
|
||||||
|
await session_repo.delete_session(db, token)
|
||||||
|
log.info("session_terminated", token_hash=token_hash)
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -1,32 +1,3 @@
|
|||||||
## 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
|
## TASK-011 — Session token prefix logged on login and logout
|
||||||
|
|
||||||
**Severity:** Low
|
**Severity:** Low
|
||||||
|
|||||||
@@ -114,7 +114,7 @@ async def login(
|
|||||||
db, token=token, created_at=created_iso, expires_at=expires_iso
|
db, token=token, created_at=created_iso, expires_at=expires_iso
|
||||||
)
|
)
|
||||||
signed_token = sign_session_token(session.token, session_secret)
|
signed_token = sign_session_token(session.token, session_secret)
|
||||||
log.info("bangui_login_success", token_prefix=session.token[:8])
|
log.info("bangui_login_success", session_id=session.id)
|
||||||
return signed_token, session.expires_at
|
return signed_token, session.expires_at
|
||||||
|
|
||||||
|
|
||||||
@@ -175,9 +175,11 @@ async def logout(
|
|||||||
try:
|
try:
|
||||||
token = unwrap_session_token(token, session_secret)
|
token = unwrap_session_token(token, session_secret)
|
||||||
except ValueError:
|
except ValueError:
|
||||||
log.warning("bangui_logout_invalid_token", token_prefix=token[:8])
|
token_hash = hashlib.sha256(token.encode()).hexdigest()[:12]
|
||||||
|
log.warning("bangui_logout_invalid_token", token_hash=token_hash)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
token_hash = hashlib.sha256(token.encode()).hexdigest()[:12]
|
||||||
await session_repo.delete_session(db, token)
|
await session_repo.delete_session(db, token)
|
||||||
log.info("bangui_logout", token_prefix=token[:8])
|
log.info("bangui_logout", token_hash=token_hash)
|
||||||
return token
|
return token
|
||||||
|
|||||||
Reference in New Issue
Block a user