`consensus/lawyer-directory-builder.md`

58 lines
# Consensus — lawyer-directory-builder
Generated 2026-04-30T23:48:02-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:1`, `.env:9-14`: DB/admin/Stripe secrets are present in the project tree; compromise means admin takeover and live charges. Rotate secrets, remove `.env`, commit `.env.example`.
- `src/server/index.ts:19`, `src/server/upgrade.ts:242-249`: global `express.json()` runs before Stripe `express.raw()`, so webhook signature verification will receive parsed JSON, not raw bytes. Mount `/webhooks/stripe` raw parser before JSON middleware.
- `src/server/upgrade.ts:173-199`: Stripe `success_url`/`cancel_url` are built from `Host` / `x-forwarded-host`; hostile headers can create payment sessions redirecting to attacker-controlled domains. Use fixed `PUBLIC_BASE_URL`.
- `src/server/portal.ts:388-395`: login has no rate limit, lockout, or audit trail; leaked/admin emails can be brute-forced. Add per-IP+email throttling and failed-login logging.
- `package-lock.json:1544-1545`, `src/server/index.ts:34-181`: Express 4 async route errors are not wrapped; DB errors can become unhandled rejections/hung requests. Add async handler wrapper and central error middleware.
- `src/server/leads.ts:186-227`, `src/server/directory.ts:468-484`: public lead/inquiry writes have no rate limit, CAPTCHA, or duplicate suppression; easy DB/admin spam. Add throttling and spam controls.


## Round-1 applied
## Applied
- .env.example:1 - added placeholder-only environment template - documents required keys without copying real secrets from `.env`.
- src/server/portal.ts:352 - added malformed/missing body guard on signup - avoids a 500 when `req.body` is absent.
- src/server/portal.ts:391 - added malformed/missing body guard on login - avoids a 500 when `req.body` is absent.
- src/server/portal.ts:534 - added malformed/missing body guard on admin plan updates - avoids a 500 when `req.body` is absent.
- src/server/leads.ts:186 - added malformed/missing body guard on public lead submission - avoids a 500 when `req.body` is absent.
- src/server/leads.ts:337 - added malformed/missing body guards on admin lead updates - avoids a 500 when `req.body` is absent.
- src/server/directory.ts:468 - added malformed/missing body guard on directory inquiries - avoids a 500 when `req.body` is absent.
- src/server/upgrade.ts:149 - added malformed/missing body guard on checkout creation - avoids a 500 when `req.body` is absent.
- src/server/upgrade.ts:314 - added malformed/missing body guard on order status updates - avoids a 500 when `req.body` is absent.
- src/enrich/site_audit.ts:268 - wrapped Playwright browser lifecycle in `try/finally` - ensures the browser closes on worker failures.
- src/scripts/click_through.ts:30 - wrapped Playwright browser lifecycle in `try/finally` - ensures smoke-test browser cleanup.
- src/scripts/probe_calbar.ts:14 - wrapped Playwright browser lifecycle in `try/finally` - ensures probe browser cleanup.
- src/scripts/create_admin.ts:18 - replaced CLI password argument with `CREATE_ADMIN_PASSWORD`/`ADMIN_PASSWORD` env lookup - avoids exposing the password in process args.


## Codex re-review verdict: FIX BEFORE SHIP

## New issues introduced
none observed.


## Round-2 applied
## Round 2 Applied
- src/server/data_market.ts:321 — added malformed/missing body guard before reading `b.kind` and other body fields — fixes CODEX_REREVIEW.md "Missed call-sites" finding for `POST /data/checkout`, where absent/non-object `req.body` could 500.


## Still deferred for Steve
- P0 - .env:1, .env:9-14 - real secrets are in the project tree - rotation/removal requires Steve; `.env` was left untouched.
- P0 - src/server/index.ts:19, src/server/upgrade.ts:242 - Stripe raw webhook parser is mounted after global JSON parser - not in safe list.
- P0 - src/server/upgrade.ts:173 - checkout redirects trust forwarded host headers - needs a fixed `PUBLIC_BASE_URL` decision.
- P0 - src/server/portal.ts:388 - login lacks rate limit/audit trail - auth policy change, not in safe list.
- P0 - src/server/index.ts:34 - async Express handlers lack central error wrapping - broad routing change, not in safe list.
- P0 - src/server/leads.ts:186, src/server/directory.ts:468 - public write endpoints lack spam controls - product/security policy decision.
- P1 - package.json:10 - scripts reference missing files - deferred because restoring/removing ops commands was not in safe list.
- P1 - package-lock.json:971 - Express runtime/types mismatch - package/version change is out of scope.
- P1 - src/scripts/run_overnight.ts:88 - required child job failures do not fail orchestrator - behavior change outside safe list.
- P1 - src/scripts/migrate.ts:28 - migrations not transactional - migration/DDL area is explicitly deferred.
- P1 - src/enrich/discover_websites.ts:127 - CLI `LIMIT` validation/parameterization - not in safe list.
- P1 - src/enrich/firm_website_contacts.ts:127 - duplicate race needs unique indexes/`ON CONFLICT` - schema change is explicitly deferred.
- P1 - src/lib/match_firms.ts:72 - repeated audit subqueries may slow down - query refactor outside safe list.
- P1 - package-lock.json:1544 - existing typecheck failures - not changed; tests/typecheck were not run by request.