diff --git a/HARDENING.md b/HARDENING.md new file mode 100644 index 0000000..923aaec --- /dev/null +++ b/HARDENING.md @@ -0,0 +1,214 @@ +# Denya OneCare — Production Hardening & Review Checklist + +> **Audience:** Abiba (and any agent working the Denya OneCare repo) +> **Status:** Demo → Production hardening +> **Context:** WhatsApp integration lands within a week (once Denya provides credentials). +> We are moving past "demo" toward the final product. This doc is the concrete, +> ordered punch-list to get there. Each item is grounded in the current codebase +> (verified against `main` and the live deployment on `scottdenya`). +> **How to use:** work top-down. P0 items are hard blockers for any real data. +> When a P0/P1 item is done, mark it `[x]` and PR it with a `no-mistakes(review)` pass. + +--- + +## 0. Current state (verified 2026-08-03) + +- **Working & verified:** auth (JWT 30m/7d, bcrypt, RBAC via `require_roles`), ticket + CRUD with 16-status `VALID_TRANSITIONS` state machine, SLA engine, photo uploads, + category/unit hierarchy, 3 role dashboards (CS/FM/CEO), Alembic migrations with + legacy-schema self-heal. **32 pytest tests pass.** +- **Live:** container `denya-onecare` on LXC `scottdenya` (192.168.68.75:8000), + image built 2026-08-02, `restart: unless-stopped`. +- **Known demo-only posture (must change):** `SECRET_KEY=change-me-in-production`, + `CORS_ORIGINS=*`, open `/api/auth/register`, SQLite backend, WhatsApp webhook + code is present but **no real credentials wired**. + +--- + +## 1. P0 — Security blockers (do these FIRST, before any real data) + +### P0.1 Hardcode-safe secrets; never ship the default key +- **Files:** `docker-compose.yml`, `app/core/config.py` +- Replace the hardcoded `SECRET_KEY=change-me-in-production` default with a + fail-closed default: if `SECRET_KEY` is unset/empty or still the well-known + placeholder string, refuse to boot (raise in `Settings` validation or lifespan). +- `docker-compose.yml` must NOT carry a literal secret. Reference an `.env` + (git-ignored) or a runtime secret source. Add `SECRET_KEY` + `WHATSAPP_*` to `.gitignore`. +- **Acceptance:** starting the app without a real key fails loudly; container env + contains a strong random key (≥32 bytes, e.g. `openssl rand -hex 32`). + +### P0.2 Lock down CORS +- **Files:** `app/main.py`, `docker-compose.yml` +- `CORS_ORIGINS=*` + `allow_credentials=True` is an invalid/unsafe combo + (browsers reject `*` with credentials anyway). Replace with an explicit + origin allow-list of the real web origins (e.g. `https://denya.sysloggh.net`, + your NetBird/nomad domain + localhost for dev). +- If credentials are used, origins MUST be explicit — never `*`. +- **Acceptance:** `settings.CORS_ORIGINS` is a comma-separated explicit list; the + middleware builds an allow-list, not `["*"]`. + +### P0.3 Gate user registration +- **File:** `app/routers/auth.py` (`POST /api/auth/register`) +- Today anyone on the network can self-register. Decide the model: + - **Recommended:** require an admin-issued invitation token, or restrict + registration to a seed/allowed list, or remove the open route and create + users only via seed/admin. + - If a public self-service resident/tenant signup is genuinely required + (Phase 2 QR/self-service), it must be a SEPARATE endpoint with a **role + default of the least-privilege role** and rate-limiting — never able to mint + admin/FM roles. +- **Acceptance:** a raw, unauthenticated register call can no longer mint an + `Admin/*` or `Director` account. + +### P0.4 Reconsider SQLite for the final product +- **Files:** `docker-compose.yml`, `app/core/database.py`, `app/core/config.py`, PRD §16 +- PRD Phase 1 calls for PostgreSQL. SQLite is fine for POC but is a write-lock + bottleneck and a data-integrity risk under concurrent FM/CS/WhatsApp writes. +- **Recommended:** switch `DATABASE_URL` to Postgres via async driver + (`postgresql+asyncpg://`). SQLAlchemy 2.0 + SQLAlchemy models are portable — + the migration is mostly: new driver dependency, `DATABASE_URL`, and re-running + Alembic against Postgres. Keep SQLite as the default for local dev/tests only. +- **Acceptance:** `pytest` green against Postgres (tests param via conftest), + Alembic applies cleanly on a fresh Postgres DB. + +### P0.5 WhatsApp webhook auth + hardening (finish wiring, then lock it) +- **File:** `app/routers/whatsapp.py` +- The handler exists but no credentials are set. When wiring this week: + - Verify the `hub.verify_token` check is constant-time (compare with + `secrets.compare_digest`). **The GET verification path currently returns + `{"error": ...}` with HTTP 200** — flip to `403` on token mismatch. + - Validate **inbound messages only from Meta** — the webhook MUST authenticate + Meta's request signature (X-Hub-Signature-256 HMAC over the raw body with your + app secret) before processing, otherwise anyone who discovers the endpoint can + forge tickets. This is the single most important WhatsApp hardening item. + - Add per-sender rate limiting / dedupe on `wa_message_id` (webhook retries can + double-create tickets). Create an idempotency guard keyed on `wa_message_id`. + - Never log the raw access token; redact in `send_whatsapp_reply` error paths. +- **Acceptance:** a forged POST without the Meta signature is rejected; duplicate + `wa_message_id` does not create a second ticket; verify-token mismatch returns 403. + +--- + +## 2. P1 — Operational hardening (before/just after go-live) + +### P1.1 Secrets handling & git hygiene +- Ensure `SECRET_KEY`, `WHATSAPP_*`, and any DB credentials are **not** in the repo + or in the committed `docker-compose.yml`. `.env` is git-ignored. +- On this fleet: align with Syslog's key-off-disk doctrine — inject secrets at + runtime (Infisical) rather than baking into image or compose if feasible. +- Rotate the seed demo users' `denya123` password before production. `seed_users` + is idempotent but the default password is in `app/services/seed.py` — forced-rotate + on first prod login or at seed time. + +### P1.2 Reverse proxy + TLS +- Do not expose the raw uvicorn :8000 behind `CORS_ORIGINS=*` on the WAN. + Terminate TLS at a reverse proxy (Caddy/Traefik/nginx) with a proper domain + (e.g. `denya.sysloggh.net`). +- Configure gunicorn/workers + `--proxy-headers` (or keep uvicorn but behind TLS). +- **Acceptance:** `https://denya.sysloggh.net` serves the app with a valid cert; + `:8000` is not directly reachable from the internet. + +### P1.3 DB backups & persistence +- Postgres change (P0.4) enables sane backups. Wire nightly `pg_dump` (or PBS / + Syslog backup cron) of the persistent volume. The compose already mounts + `app-data` volume — make sure it's on backed-up storage. +- Add an Alembic upgrade step to the deploy runbook (never rely only on + `Base.metadata.create_all` + self-heal for schema changes in prod). + +### P1.4 Logging & observability +- Add structured request logging; route to a location you can actually check + (stdout + a file/volume). Correlate with `ticket_number`. +- Add a minimal `/health` readiness that checks DB connectivity (currently it + returns OK without touching the DB). + +### P1.5 Photo upload hardening +- **File:** `app/routers/tickets.py` +- Uploads already validate MIME + extension and use UUID filenames — good. +- Add: max file-size limit (e.g. 10 MB) and content sniffing (validate magic + bytes, not just `content_type` which is client-supplied). +- Ensure uploaded files are never executable and are served with + `X-Content-Type-Options: nosniff`. + +### P1.6 API hardening & rate limiting +- Add rate limiting on `POST /api/auth/login` (brute-force) — per-IP/IP+account. +- Consider rate limits on ticket creation (spam / mass-creation). +- Normalize/validate `page_size` (already capped `le=200`) and pagination + tie-breaker (`id DESC` present — good). + +--- + +## 3. WhatsApp integration (this week) — concrete wiring runbook + +Assumes Denya provides: **phone number ID, access token, verify token, app secret.** + +1. **Add env vars** (`WHATSAPP_PHONE_NUMBER_ID`, `WHATSAPP_ACCESS_TOKEN`, + `WHATSAPP_VERIFY_TOKEN`, `WHATSAPP_APP_SECRET`, `META_GRAPH_BASE`) to `.env` + (git-ignored) and inject at runtime. Never commit. +2. **Webhook handshake:** in Meta dashboard point the webhook URL at + `/api/whatsapp/webhook`. The GET verify path currently echoes + `hub.challenge` when the verify token matches — confirm this works, then apply + P0.5 (403 on mismatch, HMAC signature validation). +3. **Verify incoming signature** (P0.5) — use `X-Hub-Signature-256` = HMAC-SHA256 + of the raw body with your app secret, compared with `compare_digest`. +4. **Reply flow:** confirm `send_whatsapp_reply` posts correctly to + `graph.facebook.com/v18.0//messages`. The reply template + currently builds a JS string manually — prefer sending the nested object as a + proper JSON body rather than a hand-built string (`{\"body\":\"...\"}`) to avoid + escaping bugs. Test with the Meta "send a test message" tool. +5. **Idempotency:** guard ticket creation on `wa_message_id` (P0.5) to prevent + double-creation on retries. +6. **Standalone test:** use the `mock-log` endpoint to confirm webhook → ticket → + auto-reply path end-to-end in the demo env before pointing Meta's production + webhook at it. + +--- + +## 4. Review recommendations (for the `no-mistakes(review)` pass and final QA) + +- **RBAC coverage:** audit every route for the correct dependency. Currently: + - `POST /api/tickets`, `PATCH`, `POST /{id}/status`, `POST /{id}/photos` → any + authenticated user. Confirm role intent (should a CS Rep push a ticket to + "On-Field Verification"? or only FM/Tech?). + - `GET /api/tickets`, `GET /{id}`, `/transitions`, `/sla`, `/photos` are + **unauthenticated**. For a facilities tool this may be intentional (resident + view), but confirm you're comfortable with public reads of ticket details + (which include reporter/phone). If not, add auth. +- **Phone/tenant data exposure:** ticket detail returns `phone`. Decide who can + see phone numbers and enforce at the API, not just the UI. +- **Test coverage gaps to add:** + - Auth: expired token, malformed token, RBAC denial per role + - WhatsApp: signature validation (valid/invalid/forged), verify-token mismatch, + duplicate `wa_message_id` idempotency + - Pagination boundary: page > last page returns empty items, tie-breaker stable + - Photo upload: bad MIME spoofing, oversize file, `is_before` flag + - SLA: breach boundary exactly at deadline (not just past it) +- **Schema/migration hygiene:** the `ensure_legacy_schema` self-heal in + `app/main.py` exists because of create_all DBs. Once you move to Alembic-only + (P1.3), this becomes dead weight — plan a deprecation. +- **Concurrency:** ticket-number generation reads `max()` then `+1` — fine at + current scale, but under concurrent Postgres writes this can race. If tickets + ever originate from WhatsApp + web + dashboard simultaneously at volume, move to + a sequenced/unique constraint approach. + +--- + +## 5. Definition of Done (production-ready) + +- [ ] No default `SECRET_KEY`; app fails closed without a real key +- [ ] CORS is an explicit origin allow-list +- [ ] Self-registration cannot mint privileged roles (or is admin-gated/removed) +- [ ] Postgres backend; Alembic applies cleanly on fresh DB; nightly backups +- [ ] TLS-terminated reverse proxy with real domain; no raw :8000 on WAN +- [ ] WhatsApp webhook: Meta signature validated, verify-token mismatch → 403, + idempotent on `wa_message_id`, real credentials injected at runtime +- [ ] Login/ticket rate limiting in place +- [ ] Photo uploads size-limited and content-sniffed +- [ ] RBAC audited per-route; phone data access controlled +- [ ] Expanded test suite (auth, WhatsApp, SLA boundary, uploads) — all green +- [ ] Secrets out of repo; demo password rotated +- [ ] Structured logs + DB-aware health check + +--- + +*Prepared by Mumuni (Syslog Falcon) — 2026-08-03, from a hands-on review of the +denya-onecare repo and the live scottdenya deployment.*