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.