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

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.

Re-check (2026-09-20)

Verified every item of 2026-09-18 outcome against the tree. Confirmed intact: C1, C2, C3, H5–H11, M1, M2, M3, M6, M8, L1 (partly), L4. Five residual gaps found inside items marked fixed; deferred list untouched.

Fixed this pass:

• R1 (residual H1) — IDOR on /api/analytics/{ws}/overview and /projection: child_id unchecked query param, analytics_service.get_overview() called get_days_left_at_age_4 without workspace_id (guard at analytics_service.py:240 never ran). Member of workspace A could read workspace B's child projection. Fixed: scope.check_child(child_id) in router (matching /age4 route pattern) and workspace_id= passed through service.
• R2 (residual H2) — session ids logged at INFO throughout fk_scraper.py (172,198). All session-id-carrying logger.info() calls downgraded to DEBUG; "scraper session done" kept at INFO without id; debug-artifact warning stays WARNING without id. One WARNING (tier sum mismatch, fk_scraper.py:723) deliberately keeps id.
• R3 (residual H3) — raw str(exc) from persist-callback failure and FK maintenance banner's scraped text reached poll client. Both replaced with fixed strings; full detail stays in ERROR/DEBUG logs.
• R4 (residual L1) — case-sensitive email: Foo@x.se and foo@x.se became two accounts. Lowercasing field_validator on RegisterRequest/LoginRequest; migration 009_lowercase_user_email (aborts with named-rows error if two rows would collide, not opaque UNIQUE constraint failure).
• R5 (residual H11) — usePlanProposal and usePlanSync had no stale-response guard. Both now use shared useLatestRequest helper (as 15 other hooks do).
• M9 — WorkspaceContext seeded workspaceId from localStorage synchronously; PlanSyncProvider's effect did not depend on auth → cold load POST /api/plan/sync before auth resolved, 401'd, raced AuthContext's own refresh. Effect now gated on isAuthenticated.
• M7 — every planned-day mutation blanked Calendar behind skeleton; multi-day save issued 2N requests (write+full refetch per date). usePlannedDays gained background/isRefreshing split (like useLeaveWishes) + skipRefresh option; Calendar's multi-day loop now N writes + one trailing background refresh + one bumpPlanVersion in finally (error path refreshes too). runSync's refresh also made background.
• H12 (partial) — take_pl_on_red_days added to GET /auth/context response; WorkspaceContext.refreshContext now builds workspace state from context alone, drops redundant GET /workspace/:id from every cold load. Workspace.name/createdAt optional (no read-side consumer in src/).
• L3 — CI hardened: top-level permissions: contents: read; all four actions pinned to commit SHAs; e2e npm install → npm ci; both uv sync → uv sync --locked; playwright-report upload if: always() → if: failure(). New .github/dependabot.yml (github-actions, npm, uv).
• L5 (partial) — frontend/out.js dead artifact removed + gitignored. CSP added to nginx.conf.template (from production build). Note: nginx does not merge add_header across levels; four security headers repeated in three location blocks (Cache-Control: /assets/, /fonts/, = /index.html) — verified curl against running container all four present on every route. Three pre-existing headers had silently been absent from those routes before.

Decisions taken (not oversights):

• H4 refresh-token revocation (jti/iat) NOT implemented — stateless refresh stands. is_active checked on refresh → disabled account cut off; only per-token revocation missing.
• Phase-2 contract change (drop workspace_id from bodies) deferred — every route already reconciles body value against caller's scope, defence-in-depth not a live hole.
• H12 step 2 (cross-route cache) re-deferred: of 7 Dashboard fan-out endpoints only 2 safe to cache today; 5 have same-hook or cross-page writers (usePlannedDays, useLeaveWishes, useWorkspaceMembers/Settings, saveManualFetch) not wired to shared invalidation signal. URL-keyed cache would serve pre-mutation data to mutation's own refetch. If revisited, go endpoint-by-endpoint.
• Still deferred: M4 (rate limiter per-process/IP-keyed/proxy trust), M5 (N+1 fan-outs, 0.05 ms/query), L2 (drop personnummer_enc), L5 bundle splitting.
• New follow-up: /auth/context could also carry workspace name/created_at if future feature needs them on cold load.

Gates after this pass: backend 901 passed / ruff clean / alembic head OK / coverage 92%. Frontend 1320 tests in 75 files / typecheck+lint clean / coverage 98.16 lines, 94.79 functions, 92.15 branches, 98.16 statements.

Browser verification (2026-09-20) and two defects it caught

Two rounds of real-browser verification against seeded dev instance caught two bugs the unit suites missed; both fixed + re-verified.

• M7 incomplete on first landing: usePlannedDays' planVersion effect called fetch() with no options, so mutators' bumpPlanVersion() re-ran it as second blocking fetch—2 GETs per save, page blanked on Advanced (direct planned-day) save path. Fixed: effect sole fetcher, mutators only bump, effect backgrounds everything but first load or childId switch.
• isLoading stranded forever on in-app navigation: useLatestRequest pattern latent flaw — setBusy(true) ran unconditional but finally { if (isCurrent(requestId)) setBusy(false) } cleared it only for still-current request, so superseded calls left flag set. React StrictMode double-invokes mount effect; on SPA navigation (childId already known) both invocations fetched—one foreground, one background—leaving isLoading true behind PageSkeleton. Fixed in all 16 hooks: unguarded busy-flag clear (isCurrent decides which response's DATA wins, never whether call releases flag it raised); plus in-flight dedup in usePlannedDays. Regression tests confirmed to fail against pre-fix code.

Verified in-browser post-fix: 4 alternating in-app navigations all render without skeleton; 1 GET /api/planned-days per navigation; Advanced single-day save = 1 write + 1 GET, no blanking; 4-day drag-select = 4 writes back-to-back + exactly 1 trailing GET; cold load issues no GET /api/workspace/{id}; red-days flag round-trips through /auth/context and honored by Calendar's free-day filter; SV and EN both clean.

New follow-ups found (not fixed):

• useCalendarDays mutates isFirstPlanVersionRun ref in effect body; StrictMode double-invoke can fire background: true fetch on page with nothing loaded. Unguarded flag clear stops hangs, can still cause transient extra request / brief flash (dev only).
• PlanSyncProvider handles stale localStorage workspaceId poorly: after workspace it names is gone, background sync 404s and logs error instead of clearing stale id.
• Cold load issues GET /api/workspace/{ws}/members twice.
• Cold load issues POST /api/auth/refresh twice (consistent with StrictMode double-invoke; dev-only, unconfirmed in prod).
• POST /api/plan/sync fires before GET /api/auth/context resolves (safe: auth resolved and server scope-checks; ordering surprise).
• Child-switch behavior could not be browser-tested: demo seed has one child only.

Frontend gates after final fix: 75 files / 1322 tests / 98.17 lines, 94.79 functions, 92.14 branches, 98.17 statements. Branch coverage 0.14 above gate of 92—thin.