Files

T

root 0d79a582f6 no-mistakes(document): drop stale hand-copied test count from HARDENING.md

2026-09-09 13:06:19 +00:00

13 KiB

Raw Blame History

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. pytest suite passes.
Live: container denya-onecare on LXC scottdenya (192.168.68.75:8000), image built 2026-08-02, restart: unless-stopped.
Demo-only posture resolved (PR #10 + P0 batch): SECRET_KEY now fails closed without a real key, CORS is an explicit origin allow-list, and open self-registration was removed — POST /api/auth/register → 404, users are admin-managed only (see AGENTS.md).
Known demo-only posture (must change): 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 — DONE (PR #10)

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 — DONE (PR #10)

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 — DONE (P0 batch, 2026-09)

Open POST /api/auth/register was removed entirely (404) — there is no sign-up UI.
Users are admin-managed: POST /api/auth/users (Admin/Jerome + Admin/Wahab only) creates users with a forced canonical role (unknown roles → 422); PATCH /api/auth/users/{id} changes role / deactivates (self-modification → 400); DELETE /api/auth/users/{id} is guarded (users referenced by tickets/timeline/escalations → 409); duplicate email → 409.
Emails are normalized (strip + lowercase) on every write path; a startup self-heal lowercases legacy mixed-case rows so pre-P0 accounts can't be locked out of login. Unified role model lives in app/core/roles.py.
Regression tests: tests/test_p0_auth_admin_batch.py.
If a public self-service resident/tenant signup is genuinely required later (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.

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 — PARTIAL (P0 batch, 2026-09)

File: app/routers/whatsapp.py
Done: the GET handshake validates hub.verify_token with a constant-time compare and returns 403 on mismatch; inbound POSTs are gated by the X-Webhook-Secret header matching WHATSAPP_WEBHOOK_SECRET (fail-closed 403 when the env var is unset — see .env.example); the debug GET /api/whatsapp/mock-log now requires Bearer auth.
Still open: Meta request-signature validation (X-Hub-Signature-256 HMAC over the raw body with the app secret — the shared-secret header above is the interim gate); per-sender rate limiting / dedupe idempotency keyed on wa_message_id (retries can still double-create tickets); redacting the raw access token in send_whatsapp_reply error paths.
Acceptance (open items): a forged POST without the Meta signature is rejected; duplicate wa_message_id does not create a second ticket; verify-token mismatch returns 403 (done).

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

Done (P0 batch): POST /api/auth/login is rate-limited in-process — ~5 failures / 15 min per IP+email → 429 (env-tunable LOGIN_RATE_LIMIT_*, a successful login resets the window). Open: ticket-creation rate limiting (spam / mass-creation).
Done (P0 batch): ticket-list pagination — page/page_size (default 50, cap 200), limit alias for page_size also capped; passing both with different values → 422. Tie-breaker id DESC present.

3. WhatsApp integration (this week) — concrete wiring runbook

Assumes Denya provides: phone number ID, access token, verify token, app secret.

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.
Webhook handshake: in Meta dashboard point the webhook URL at <domain>/api/whatsapp/webhook. The GET verify path echoes hub.challenge when the verify token matches (constant-time compare; mismatch → 403). The still-open P0.5 work is the Meta signature validation in step 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.
Reply flow: confirm send_whatsapp_reply posts correctly to graph.facebook.com/v18.0/<PHONE_NUMBER_ID>/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.
Idempotency: guard ticket creation on wa_message_id (P0.5) to prevent double-creation on retries.
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 (PR #10)
CORS is an explicit origin allow-list (PR #10)
Self-registration removed (register → 404); users are admin-managed only with forced canonical roles (P0 batch)
Postgres backend; Alembic applies cleanly on fresh DB; nightly backups
TLS-terminated reverse proxy with real domain; no raw :8000 on WAN
Webhook POST gated by X-Webhook-Secret (fail-closed); verify-token mismatch → 403; mock-log requires auth
WhatsApp webhook: Meta X-Hub-Signature-256 HMAC validated; idempotent on wa_message_id; real credentials injected at runtime
Login rate limiting in place (~5 fails / 15 min per IP+email → 429)
Ticket-creation 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.

13 KiB Raw Blame History