`lawyer-directory-builder/review.md`

38 lines

## Snapshot
- Compliance-first California lawyer/firm directory with public lead capture, lawyer signup/admin portal, audits, and Stripe upgrade orders.
- Stack: TypeScript, Node/Express 4, PostgreSQL, Playwright/Sharp/Undici, Stripe.
- How it's run: manual `npm run server`; README suggests cron or pm2 for overnight jobs.
- Status: live-ish: prod URL + live Stripe notes.

## 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.

## Notable Issues (P1 — fix this month)
- `package.json:10-16`: scripts reference missing files (`seed_sources.ts`, `run_state_bar.ts`, `export_csv.ts`, `stats.ts`). Remove or restore; current ops commands are misleading.
- `package-lock.json:971-985`, `package-lock.json:1544-1545`: Express 4 runtime with `@types/express` 5. Typecheck fails; align types to Express 4 or upgrade runtime.
- `src/scripts/run_overnight.ts:88-97`: child job exit codes are logged but never fail the orchestrator; nightly can exit 0 after broken imports. Exit nonzero if any required job fails.
- `src/scripts/migrate.ts:28-33`: migration execution and schema_migrations insert are not wrapped in `BEGIN/COMMIT`; failed SQL can leave partial schema unrecorded. Run each migration in a transaction.
- `src/enrich/discover_websites.ts:127-133`: `LIMIT ${LIMIT}` is interpolated from CLI number coercion; `NaN`/odd values can break SQL. Validate positive integer and parameterize where possible.
- `src/enrich/firm_website_contacts.ts:127-143`: check-then-insert for phones/emails races under concurrent runs; duplicate rows possible because schema lacks unique constraints. Add unique indexes and `ON CONFLICT`.
- `src/lib/match_firms.ts:72-93`: correlated `site_audits` subqueries run twice per candidate firm; will get slow as firms/audits grow. Precompute latest audit with lateral join or materialized view.
- Typecheck currently fails: `./node_modules/.bin/tsc --noEmit` reports errors in `pool.ts`, `portal.ts`, `leads.ts`, and `upgrade.ts`.

## Polish (P2 — when convenient)
- `src/server/portal.ts`, `src/server/leads.ts`, `src/server/directory.ts`, `src/server/upgrade.ts`: large inline HTML/CSS duplicates layout/auth chrome; extract shared rendering helpers.
- `public/index.html:263-272`: hardcoded public counts can drift from DB stats. Render from `/api/stats` or label as snapshot.
- `README.md:19-23` says CalBar is blocked, but `src/ingest/calbar_full_scrape.ts:1-27` says plain fetch works. Update docs.

## Strengths
- SQL query parameters are used consistently on user inputs; no SQL injection observed.
- Schema has useful provenance/audit tables and uniqueness for key entities like `professionals.bar_number`.
- Session cookies are httpOnly/secure and server-side sessions are stored in PostgreSQL.

## Open questions for the owner
- Is `lawyers.agentabrams.com` already publicly routed to this server?
- Are the `.env` Stripe keys live production keys currently active?
- Should public lead capture be open-web, or limited to paid/verified acquisition channels?