Files
F-r-ldraledigplannerare/SECURITY_PERF_AUDIT_2026-09-18.md
T

60 lines
6.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Security + performance audit — merged findings (2026-09-18)
Detail reports: authz.md, backend-perf.md, scraper-config-ops.md, frontend.md (same dir).
Baseline before fixes: backend 552 passed, ruff clean; frontend 61 files passed, cov 98.75/94.61/92.99/98.75.
## CRITICAL
C1. 14 routes need no auth at all — plan/*, leave-wishes, day-overrides, vacation-days (routers/plan.py:12, leave_wishes.py:12, day_overrides.py:11, vacation_days.py:11). Anonymous read/write/delete of any workspace by id.
C2. Self-invite into any workspace: POST /api/workspace/{any_ws}/invitations + redeem (workspace.py:159-205, workspace_service.py:228-282). Grants full legitimate membership.
C3. Real FK personal data committed: captures/04_*, captures/05_* (child name, payout kronor, tax rate, balances), captures/02_* (BankID QR data-code), test.html (idpx login dump). In history since 4e9fb37. No git remote configured.
## HIGH
H1. IDOR on every authenticated workspace/child/personal_data route — analytics, calendar, settings, planned_days, fetch_data (GET latest/history, start), workspace members/rename/remove. Body ids never reconciled with caller. Only workspace_service._get_member exists and is not caller-scoped.
H2. Scraper: poll/cancel by session_id without owner (fetch_data.py:190-212); session ids logged at INFO (fk_scraper.py:172,198); start targets arbitrary pd/child → data poisoning.
H3. Scraper debug artifacts (logged-in FK HTML + full screenshot) written to disk by default (config.py:121 scraper_debug_dir="./debug/scraper"; fk_scraper.py:245-270,294). Balances logged at DEBUG (fk_scraper.py:443,458,465,479-484,500); raw page text in ValueError surfaced to poll client (:450,:476,:292). Default log_level="DEBUG" (config.py:111).
H4. Default JWT secret silently accepted (config.py:106); no jti/iat/revocation; 7-day stateless refresh; is_active never checked; /api/auth/refresh unthrottled.
H5. No atomicity/isolation in plan apply/sync (planning_service.py:486-513): per-row COMMIT, deletes committed before creates; concurrent syncs corrupt mid-write. Explicit BEGIN on the shared connection is unsafe without a lock.
H6. Per-row commit cost: 300-row apply 89 ms (macOS) / seconds on Linux fsync vs ~1 ms in one txn. No WAL/synchronous/busy_timeout PRAGMAs (connection.py:186-195).
H7. argon2 hash/verify block the event loop 22 ms each (auth_service.py:72,84,88).
H8. No GZipMiddleware; /api/calendar/days ships 183 KB JSON per request, built 3× (calendar_service.py:125, models/calendar.py:63).
H9. Zero indexes anywhere. UNIQUE autoindexes cover the main list paths; full scans on count_by_parent_type, count_dubbeldagar, get_latest_by_parent, delete_by_wish, leave_wish/child/personal_data by workspace, invitation list, all FK-cascade columns. 14-statement DDL in backend-perf.md §4.
H10. Frontend: 15-min access token, no 401→refresh→retry (api.ts:523-579, AuthContext.tsx:28-43) → sessions silently die after 15 min; errors render as banners, isAuthenticated stays true.
H11. Frontend: stale-response race in all 10 hooks (no abort/request-id guard) → wrong child's data shown on fast child switch / year paging.
H12. Frontend cold-load waterfall: refresh → /auth/context → /workspace ×2 (dup) → 6–12 page requests; Calendar auto-POSTs /plan/sync then refetches.
## MEDIUM
M1. revoke_invitation unscoped (workspace_service.py:274-281); invitation_id never exposed → low exploitability.
M2. _sessions never purged; QR PNG + result retained for process lifetime (fk_scraper.py:113,358,432).
M3. OpenAPI docs public in all envs (main.py:81); migration failure swallowed (main.py:61-65).
M4. Rate limiter per-process, IP-keyed, no proxy trust (rate_limit.py:10).
M5. N+1 fan-outs: allocation loader 5+C+C·P+P, analytics overview 2+6P (3·(P−1) redundant), tips 6+P+C·(P+1), dismiss_tip recomputes full pipeline. Measured 0.05 ms/query → not a bottleneck today.
M6. Frontend: logout doesn't clear token if api.logout() throws (AuthContext.tsx:66-70); in-flight WorkspaceContext fetches can repopulate old account's data after logout.
M7. Frontend: every planned-day mutation blanks the Calendar page with a spinner; multi-day save is N sequential POST+GET.
M8. Frontend: useScraperFetch setInterval(1000) async callback, no overlap guard, no unmount cleanup.
M9. localStorage workspaceId fetched blindly (moot once H1 fixed server-side).
## LOW
L1. 40 router sites pass str(exc) to HTTP detail; register 409 echoes email; login timing oracle (no dummy verify); case-sensitive email → duplicate accounts.
L2. personnummer_enc column never written — dead column.
L3. CI: no permissions block, tag-pinned actions, npm install in e2e, playwright-report uploaded always, no uv lock --check, no dependabot.
L4. scripts/demo_plan.py crashes (stale AllocationInput signature, L123).
L5. Frontend: out.js dead artifact; no CSP; grid memoization ~1–3 ms/render (not worth it); bundle 97 kB gzip single chunk (splitting not yet worthwhile); dead /calendar?date= link (Dashboard.tsx:136).
## REFUTED leads
- Allocator propose (0.9 ms) and holidays json.load (0.4 ms) / get_red_days (<0.01 ms) — not worth to_thread.
- Refresh-cookie CSRF — mitigated by SameSite=Lax + explicit origin allowlist; only a footgun if cors_origins=["*"].
- logout delete_cookie attrs — Starlette defaults path="/" on both; cosmetic.
- DevFetch in prod bundle — tree-shaken (import.meta.env.DEV constant-folded).
## Fix scope this run (approved: critical + high)
Wave 1 (parallel, disjoint files): T1 authz (C1,C2,H1,H2,M1) · T2 auth/config/ops hardening (H4,H7,H8,M3, L1 email echo + dummy verify) · T6 frontend (H10,H11,M6,M8)
Wave 2 (after T1): T4 DB layer — UoW + PRAGMAs + batching + indexes (H5,H6,H9) · T3 scraper hygiene (H3,M2) · L4 demo_plan fix
User decision: C3 history purge vs delete-only.
Deferred to follow-up: H12, M4, M5, M7, M9, L2, L3, L5.
## Outcome (2026-09-18)
Fixed in commit "Harden the API and the data layer after a security and performance audit": C1, C2, C3 (history rewritten with git filter-repo; backup bundle at scratchpad/pre-purge-backup.bundle), H1–H11, M1, M2, M3, M6, M8, L1 (email echo + timing), L4; is_active persisted (migration 011).
Final gates: backend 632 passed / ruff clean / alembic head OK; frontend 958 tests, typecheck+lint clean, cov 98.77/94.64/92.04/98.77.
Deferred (follow-up list): H12 cold-load waterfall + no cross-route cache; M4 rate limiter proxy trust / shared store; M5 N+1 query collapsing (0.05 ms/query — not a bottleneck); M7 Calendar spinner on every mutation + sequential multi-day save; M9 (moot server-side); L2 drop personnummer_enc; L3 CI hardening (permissions block, SHA pins, npm ci, failure-only artefact upload, uv lock --check, dependabot); L5 out.js removal, CSP meta, dead /calendar?date= link.
Phase-2 contract change (not started): drop workspace_id from request bodies and derive it from the caller.