`consensus/wholivedthere.md`

41 lines

# Consensus — wholivedthere
Generated 2026-04-30T23:48:03-07:00. Source: codex review/fix/rereview/round-2 artifacts (round-3 was quota-blocked).

## Codex pass-1 review (top risks)
## Top Risks (P0 — fix this week)
- `.env:2-5`, `scripts/deploy-direct.sh:21-22`, `scripts/deploy-pastdoor.sh:28-29`: committed Google/DPLA/GoDaddy API secrets; DNS/domain takeover and API billing exposure. Revoke/rotate now, remove defaults, keep only `.env.example`.
- `scripts/deploy-direct.sh:60`: deploy path drops `pastdoor` database every run; one mistaken rerun destroys production data. Replace with migration/restore into temp DB plus explicit manual confirmation.
- `mcp-la-records/src/arcgis.mjs:83-93`, `mcp-la-records/src/arcgis.mjs:110`: user input is interpolated into ArcGIS SQL-like `where`; quotes/wildcards can alter query semantics. Validate APN with strict regex, escape `%/_/'`, and reject unsupported address chars.
- `mcp-la-records/src/arcgis.mjs:97`: `limit` is unvalidated and passed as `resultRecordCount`; callers can request huge result sets or invalid counts. Clamp to small integer range, e.g. `1..25`.
- `scripts/schema.sql:58`: generated column uses `now()`, which Postgres rejects because generated expressions must be immutable. Replace with a view or query predicate for stale rows.

## Round-1 applied
## Applied
- `scripts/deploy-direct.sh:21` — removed hardcoded GoDaddy credential fallbacks and now reads `GODADDY_API_KEY` / `GODADDY_API_SECRET` from the environment only — prevents exposed source defaults from being used.
- `scripts/deploy-direct.sh:24` — added a fail-fast check when GoDaddy credentials are missing — avoids running DNS updates with empty credentials.
- `scripts/deploy-pastdoor.sh:28` — removed hardcoded GoDaddy credential fallbacks and now reads `GODADDY_API_KEY` / `GODADDY_API_SECRET` from the environment only — prevents exposed source defaults from being used.
- `scripts/deploy-pastdoor.sh:32` — added a fail-fast check when GoDaddy credentials are missing — avoids running DNS/registrar steps with empty credentials.
- `.env.example:3` — added placeholder-only entries for DPLA, GoDaddy, and Cloudflare variables — documents required env vars without copying real secrets.

## Codex re-review verdict: SHIP IT

## New issues introduced
none observed

## Still deferred for Steve
- P0 — `.env:2` — real Google/DPLA/GoDaddy secrets are present according to the review — left `.env` untouched and credential rotation needs account access.
- P0 — `scripts/deploy-direct.sh:65` — deploy script drops the `pastdoor` database — destructive database workflow is outside the safe fix list and needs an owner-approved deployment design.
- P0 — `mcp-la-records/src/arcgis.mjs:83` — ArcGIS `where` query interpolates user input — query semantics and validation rules need product-aware handling.
- P0 — `mcp-la-records/src/arcgis.mjs:97` — unvalidated `limit` reaches ArcGIS result count — API behavior change was not in the approved safe list.
- P0 — `scripts/schema.sql:58` — generated column uses `now()` — SQL/schema changes are explicitly out of scope.
- P1 — `mcp-la-records/src/arcgis.mjs:39` — fetch has no timeout — network behavior change was not in the approved safe list.
- P1 — `mcp-la-records/src/arcgis.mjs:99` — address lookup swallows upstream failures — API error semantics change was not in the approved safe list.
- P1 — `mcp-la-records/src/index.mjs:69` — MCP handler lacks runtime `args` validation — validation contract change was not in the approved safe list.
- P1 — `scripts/deploy-pastdoor.sh:60` — rsync does not exclude `.env*` — deploy behavior change was not one of the allowed safe fix categories.
- P1 — `scripts/smoke-test.sh:65` — smoke test accepts API `404` responses — test expectation change was not in the approved safe list.
- P1 — `PLAN.md:1` / `INTEGRATION.md:1` — docs disagree on architecture — documentation rewrite was not requested for safe fixes.