clearview/docs/code-review-todo.md

# Code Review TODO — Clearview

**Aangemaakt:** 2026-05-19
**Branch bij review:** `refactor/scanner-package-frontend`
**Scope:** Eerste volledige review (~7.100 regels code)
**Totaal:** 13 CRITICAL · 19 HIGH · 14 MEDIUM · 1 LOW

Werkvolgorde: alle CRITICAL eerst (P0), daarna HIGH (P1), dan MEDIUM/LOW (P2).
Per item staan severity, bestand(en):regel(s), en de gewenste fix.

---

## P0 — CRITICAL (eerst dichten)

### Auth & secrets

- [ ] **Geen authenticatie op enig API-endpoint**
  - `containers/clearview/src/clearview_app/main.py` (alle `/api/` routes)
  - Fix: API-key via `X-API-Key` header met FastAPI `Security()` dependency, of Bearer-token op alle `/api/` routes.

- [ ] **Client secrets staan plaintext in DB**
  - `containers/clearview/src/clearview_app/models.py:21` (`TenantProfile.client_secret`)
  - `containers/clearview/src/clearview_app/models.py:45` (`ScanJob.auth_client_secret`)
  - Fix: encrypt-at-rest met `cryptography.fernet`; key via env var. Decrypt enkel in geheugen bij gebruik.

- [ ] **`.env` niet in `.gitignore`**
  - `.gitignore`, `stack/.env`
  - Fix: voeg `stack/.env` en `**/.env` toe aan `.gitignore`; lever `stack/.env.example` met placeholders; verifieer dat `.env` nog niet in git history zit (anders rotate credentials).

- [ ] **Hardcoded DB-fallback `clearview:clearview`**
  - `containers/clearview/src/clearview_app/config.py:17-19`
  - Fix: verwijder default; `raise RuntimeError("DATABASE_URL required")` als env ontbreekt.

- [ ] **Adminer publiek op `0.0.0.0:8081`**
  - `stack/docker-compose.yml:44-46`
  - Fix: bind aan `127.0.0.1:${ADMINER_PORT:-8081}:8080` of verwijder uit prod-compose.

### Injectie & exfiltratie

- [ ] **XSS via ongescapete velden in `innerHTML`** (3 vindplaatsen)
  - `containers/clearview/site/app.js:658-676` (`job.id`, `job.source_type`, `job.items_scanned`)
  - `containers/clearview/site/app.js:885-894` (`job.status`, `total/processed/successful/failed_targets`, `items_scanned`)
  - `containers/clearview/site/app.js:175-184` (`statusBadge()` zonder escape op `status`)
  - Fix: consequent `escHtml()` op alle API-velden, óók ID's en numerieke. Geen uitzonderingen.

- [ ] **Open redirect via `payload.connect_url`**
  - `containers/clearview/site/app.js:472`
  - Fix: valideer `new URL(payload.connect_url).protocol === 'https:'` + host-allowlist (`login.microsoftonline.com`).

- [ ] **SSRF / token-exfiltratie via `@odata.nextLink`**
  - `containers/clearview/src/clearview_app/scanners/sharepoint.py:547-553`
  - `containers/clearview/src/clearview_app/scanners/entra.py:227-272`
  - Fix: vergelijk `urlparse(next_url).netloc == urlparse(original_url).netloc`; gooi anders een `RuntimeError`.

- [ ] **Header injection in `Content-Disposition`**
  - `containers/clearview/src/clearview_app/main.py:701-705`
  - Fix: type de route-parameter als `uuid.UUID` zodat FastAPI `job_id` valideert; `urllib.parse.quote(filename, safe="")`.

---

## P1 — HIGH

### Correctheid

- [ ] **Token cache zonder TTL — workers crashen na 1 uur**
  - `containers/clearview/src/clearview_app/scanners/sharepoint.py:34, 512-543`
  - Fix: bewaar `expires_at = time.time() + result["expires_in"] - 60`; invalideer in `_get_token_for_host`.

- [ ] **MSAL `ConfidentialClientApplication` per aanroep**
  - `containers/clearview/src/clearview_app/scanners/sharepoint.py:530`
  - Fix: module-level dict `(tenant_id, client_id, auth_method) -> app`. Hergebruik object.

- [ ] **`_TOKEN_CACHE` zonder lock (race in multi-thread)**
  - `containers/clearview/src/clearview_app/scanners/sharepoint.py:34`
  - Fix: `threading.Lock` rond check-then-write, of `functools.lru_cache` + TTL-wrapper.

- [ ] **Race condition in worker: niet-atomaire job-claim**
  - `containers/clearview/src/clearview_app/worker.py:48-68`
  - Fix: één `UPDATE scan_jobs SET status='running' WHERE id=:id AND status='queued' RETURNING id` met `FOR UPDATE SKIP LOCKED`.

- [ ] **Auto-refresh race in frontend**
  - `containers/clearview/site/app.js:1009-1013` + alle `tick()` callsites
  - Fix: `AbortController` per render; vorige request cancelen voordat nieuwe gestart wordt.

- [ ] **Event-listener accumulatie / re-render-pattern**
  - `containers/clearview/site/app.js:238-270, 678-702`
  - Fix: event delegation op stabiele container (`els.jobsTableBody.addEventListener('click', ...)`).

- [ ] **Sequentieel awaiten van onafhankelijke calls**
  - `containers/clearview/site/app.js:554-555, 999, 1281, 1364`
  - Fix: `await Promise.all([refreshJobs(), refreshSelectedJob()])`.

- [ ] **Niet-afgehandelde floating promise**
  - `containers/clearview/site/app.js:1143`
  - Fix: `.catch(err => showFeedback(...))` op `testTargetConnection(...)`.

- [ ] **OAuth state-store is in-memory dict (breekt bij `--workers >1`)**
  - `containers/clearview/src/clearview_app/onboarding.py:31, 134-145`
  - Fix: state opslaan in DB (tabel `oauth_states` met `created_at`, `consumed_at`) of Redis.

- [ ] **`scan_type` ongevalideerd**
  - `containers/clearview/src/clearview_app/schemas.py:36`, `main.py:178, 207`
  - Fix: `Literal["sharepoint","sharepoint_root","mailbox","entra_groups"]` in Pydantic schema.

- [ ] **`datetime.utcnow()` deprecated, timezone-naive** (overal)
  - `main.py`, `worker.py`, `models.py:26-27`, `cert.py:33`
  - Fix: `datetime.now(timezone.utc)`; `DateTime(timezone=True)` in SQLAlchemy-kolommen.

- [ ] **`ThreadPoolExecutor(max_workers=1)` per target**
  - `containers/clearview/src/clearview_app/worker.py:307`
  - Fix: gedeelde executor; documenteer dat `future.cancel()` lopende scan niet onderbreekt.

- [ ] **Geen throttling-respect bij 429 in item-loop**
  - `containers/clearview/src/clearview_app/scanners/sharepoint.py:603-619`
  - Fix: batching via `$expand=RoleAssignments` of exponential backoff op item-niveau.

### Hardening

- [ ] **Container draait als root**
  - `containers/clearview/Dockerfile`
  - Fix: `RUN adduser --system --ingroup clearview clearview` + `USER clearview`.

- [ ] **`.deb` van packages.microsoft.com zonder checksum**
  - `containers/clearview/Dockerfile:17-19`
  - Fix: hardcoded SHA256 + `sha256sum --check`, of officiële GPG-key via `signed-by`.

- [ ] **`Install-Module ExchangeOnlineManagement` zonder versie-pin**
  - `containers/clearview/Dockerfile:24-26`
  - Fix: `-RequiredVersion 3.7.0` (of huidige geteste versie).

- [ ] **Graph-foutberichten gelekt richting frontend**
  - `containers/clearview/src/clearview_app/onboarding.py:170, 188`
  - Fix: volledig log naar server-side DEBUG; aan client alleen generieke code + HTTP-status.

- [ ] **OData-filter injection via displayName/mail**
  - `containers/clearview/src/clearview_app/scanners/entra.py:178-196`
  - Fix: `urllib.parse.quote(cleaned.replace("'", "''"), safe="")`.

- [ ] **PowerShell-args zonder UPN-validatie**
  - `containers/clearview/src/clearview_app/scanners/mailbox.py:181-190`
  - `containers/clearview/src/clearview_app/scanners/exo_scripts/get-permissions.ps1:107`
  - Fix: `re.fullmatch(r"[^@\s]{1,64}@[^@\s]{1,255}", upn)` vóór `_run_pwsh`.

### Architectuur

- [ ] **`main.py` is 1139 regels — splitsen**
  - Fix: `routers/{tenants,jobs,onboarding}.py`, `services/job_service.py`, `export.py`.

- [ ] **`sharepoint.py` is 722 regels — splitsen**
  - Fix: `_auth.py`, `_http.py`, `sharepoint.py` (publieke scanfuncties), `sharing_links.py`.

- [ ] **`_ensure_schema_columns()` met 18 raw `ALTER TABLE`**
  - `containers/clearview/src/clearview_app/main.py:1115-1139`
  - Fix: vervang door Alembic; commit baseline-migratie + history.

---

## P2 — MEDIUM / LOW

- [ ] **`app.js` 1586 regels, geen build step** — splits in ES-modules + esbuild/rollup; verwijder IIFE-wrapper.
- [ ] **Focus management bij route-wissel ontbreekt** — `app.js:1529-1551` — focus naar `<h2>` van nieuwe route na navigatie.
- [ ] **Focus-outline 38% opacity voldoet niet aan WCAG 3:1** — `styles.css:292-296` — `outline: 2px solid var(--cv-accent)`.
- [ ] **Geen debouncing/abort op `jobSiteFilter`** — `app.js:1158-1172` — `AbortController` per fetch.
- [ ] **`els.submitFeedback` gebruikt voor niet-SharePoint feedback** — `app.js:682, 711` — generiek feedback-element of context-specifiek.
- [ ] **Magic string `'__manual__'`** — `app.js:412, 424` — named constant.
- [ ] **Icon-knoppen missen `aria-label`** — `app.js:229-231` — `aria-label` toevoegen.
- [ ] **`scanner.py` is shim zonder waarschuwing** — `DeprecationWarning` of verwijderen.
- [ ] **`except Exception: pass` zonder logging** (meerdere) — minimaal `log.warning(..., exc_info=True)`.
- [ ] **`_resolve_credentials(db, ...)` mist type-hint** — `main.py:935` — `db: Session`.
- [ ] **`CreateScanAppResponse` lekt secret zonder waarschuwing** — `schemas.py:149-155` — `show_once: bool` veld + log-suppression voor dit endpoint.
- [ ] **Deviations hard-capped op 1000** — `main.py:728` — voeg `total_deviations_count` + `truncated: bool` toe.
- [ ] **Geen `logging` in scanners-package** — `import logging; logger = logging.getLogger(__name__)` overal.
- [ ] **`list-mailboxes.ps1` laadt alles in geheugen** — `-ResultSize Unlimited` → paginering of cap.
- [ ] **`cert.py` private key zonder encryptie** — documenteer aanname dat caller encryptie-at-rest doet.
- [ ] **`build-and-push.sh` doet `git add -A`** — `build-and-push.sh:294` — expliciete file-lijst of bevestigingsprompt.
- [ ] **README build-instructies kloppen niet (1/2/3 vs t/r)** — `README.md:80-84`.

---

## Werkwijze

1. Werk per checkbox; verwijder geen items maar zet `[x]` als done.
2. Bij oplevering van een batch: append entry in `docs/changelog-develop.md`.
3. CRITICAL items vereisen handmatige verificatie (auth-tests, secret-rotation, XSS-payload checks).
4. Na P0 + P1: hertest met deze TODO als checklist voordat een nieuwe review wordt aangevraagd.