`docs/phase-1-codex-findings.md`

37 lines
# Phase 1 — Codex debate findings (2026-05-01)

## Status

The Codex side of the adversarial debate failed all 6 rounds with **"Quota exceeded. Check your plan and billing details."** — the codex CLI's API quota is exhausted. Steve needs to refill before re-running.

Claude's side ran fully and produced real findings on the **existing codebase** (since the debate ran `git review --uncommitted` against current diff, not the phase-1-design.md). The findings are real bugs to address before/during Phase 1.

## CRITICAL — fix before Phase 1 ships

1. **`agents/api-agent/server.js openPitchModal` textarea XSS** — `body` string-concats raw `d.name`/`d.suggested_domain` into `<textarea>` via `card.innerHTML`. Org name containing `</textarea><script>` breaks out and runs in admin origin. Fix: run every `d.*` field through `escapeHtml`.

2. **`renderPreview` + grid card + pitch modal — javascript: URL in href** — `escHtml(o.website)` HTML-escapes but does not validate scheme. A `javascript:` URL survives and fires when staff click. Multiple sites: server.js:587, server.js:1071, server.js:1385. Fix: allowlist `^https?:` before emitting href.

3. **`POST /organizations/:id/touch` unauthenticated** — Phase 1 was already going to add auth here. Confirmed.

## HIGH — fix in Phase 1

- `zip-heat` builder: `data-zip="' + z.zip + '"` unescaped → wrap in escapeHtml
- `mapHref` interpolates `${o.lat},${o.lng}` directly into href; numeric columns return as strings → encodeURIComponent each
- `load()` / `renderMap()`: concurrent fetches with no AbortController → reqId++ pattern
- `/outreach.csv` filename: `req.query.category` flows into Content-Disposition → restrict to `/^[a-z_]{1,32}$/`
- `data_market.js:651-664` CSV download: string-matches `where[]` array via `startsWith('LOWER(o.city)')` to swap in EXISTS; future predicate edit silently disables filter → emit EXISTS form on first build

## MEDIUM — Phase 1 polish

- `appendChild(card.parentElement === back ? back : back)` — dead ternary; both branches identical
- Geolocation prompt fires on every public `/preview/:id` view with no explanatory copy → gate behind "Show distance" button
- Dashboard `<select>` controls have only `title=`, no `<label for=>` → add visually-hidden labels
- `/map.json LIMIT 80000` is a DoS amplifier → cap at 20k or stream NDJSON
- `addPins()` is dead code — `renderMap` only calls `addPinsFromMapJson` → delete
- `themeWord` falls back to "beautifully themed" when category empty → reads like LLM tell; per-org-type fallback instead

## Action

I'll patch the 3 CRITICAL findings now (~15 min) before scheduling the actual Phase 1 build. The HIGH/MEDIUM items get pulled into the Phase 1 task list for Ralph.