← back to Lawyer Directory Builder
REVIEW-2026-05-04.md
64 lines
# lawyer-directory-builder — overnight debate-team review (2026-05-04)
Code-reviewer + architect-reviewer parallel run. **5 patches applied** (4 P0 security + 1 launchd-fix). **5 P0 COMPLIANCE issues surfaced — Steve product calls required, not patched.**
## Patches APPLIED (local — NOT deployed)
| # | File:line | Severity | Issue | Fix |
|---|---|---|---|---|
| 1 | `src/server/index.ts:29-32` | **P0** | Wildcard CORS `Access-Control-Allow-Origin: *` on every route — credentialed reads from any origin against authenticated APIs (/api/billing/*, /api/me/*) | Allow-list gated on `PUBLIC_BASE_URL` env + localhost dev; sets `Vary: Origin` + `Allow-Credentials: true` only for matched origin |
| 2 | `src/server/index.ts:179` | **P0** | `/api/firms/:id` did `SELECT * FROM organizations` — leaked future internal columns (`contact_crawled_at`, `site_intel_body`, etc.) to any unauth caller | Enumerated 21-column public allow-list |
| 3 | `src/server/upgrade.ts:306` | **P1** | `/upgrade/thanks?_demo=paid` localhost backdoor used `req.hostname` check (spoofable via `Host:` header with `trust proxy: 1`) | Switched to `process.env.NODE_ENV !== 'production'` |
| 4 | `src/scripts/run_overnight.ts:44` | **launchd-fix** | `spawn('npx', ['tsx', ...])` failed under launchd because launchd's minimal PATH doesn't include npm/nvm/homebrew bin dirs (this is the `com.steve.lawyer-build-nightly` exit=1 with empty stderr). Manual run worked because login shell PATH inherits them. | `spawn(process.execPath, [require.resolve('tsx/cli'), ...])` — uses absolute paths, no PATH dependency |
`tsc --noEmit` exits 0 (pre-existing TS errors in community.ts/leads.ts/portal.ts unrelated to these patches).
## P0 COMPLIANCE — Steve product decisions (NOT patched)
These need your judgment, not a code edit. **All 5 are open action items in `COMPLIANCE.md` and currently violated by live code.**
| # | Where | What's wrong (per Cal §6155 / Rule 7.x / non-lawyer operator standard) |
|---|---|---|
| C1 | `src/server/leads.ts:441-451` | `matched_firm_ids` is still being populated after lead submission. Status enum still includes `'matched'` / `'contacted'` / `'won'` (line 47). This is the active referral workflow that classifies the platform as an LRS under §6155. **You as a non-lawyer cannot operate an LRS.** COMPLIANCE.md §1 says stop. |
| C2 | `src/server/portal.ts:630` | Lawyer Pro pitch reads "Get notified the moment a matched lead is submitted in your practice area + ZIP" — exact LRS-classification language. Replace with self-serve shared-inbox framing. |
| C3 | Multiple | Brand "Counsel & Bar" still rendered publicly. COMPLIANCE.md §3 says rename without "Bar" before public launch. State Bar of California has trademark-policed similar names. |
| C4 | All 11 server files | NO `/privacy-delete`, `/opt-out`, `/data-deletion-request` endpoint. CCPA/CPRA requires one. COMPLIANCE.md §4 promises it. |
| C5 | `src/server/leads.ts:399,680` | Trust block links to `/privacy.html` + `/terms.html` — neither file exists in `/public/`. Dead links on production pages = consumer-protection violation in CA. |
## Other P1 / P2 deferred (track in REVIEW)
- `index.ts:147-149` `/api/attorneys` and `/api/firms` return `phone` + `website` unauth'd. Phone is public State Bar data, but bulk unauth exposure is a CCPA vector for the data marketplace.
- `index.ts:231` `/api/professionals/search` returns `firm_phone` unauth — same issue.
- `data_market.ts:504` `/data/download/:token` no rate limit — buyer can re-download indefinitely. Add cap on `downloads_count` for `one_time` orders.
- `billing.ts:130-133` Stripe webhook missing-secret returns 503 silently — should alert (already in reviewer's own comment, not wired).
- No CSRF protection on any state-changing POST. `sameSite: 'lax'` blocks cross-site links but NOT cross-origin `<form method=POST>`. Add `csurf` or double-submit cookie pattern, especially on `/admin/users/:id/{plan,suspend,delete}` forms in `portal.ts`.
- 6 copies of `esc()` HTML-escape function (drift risk — one in `data_market.ts` omits `'` → `'`). Extract to `src/lib/esc.ts`.
- No `helmet` / CSP anywhere. `npm install helmet` + `app.use(helmet())`.
- `directory.ts:468` `/directory/inquire` hardcodes `consent_to_contact = true` — for CCPA should be explicit opt-in checkbox.
## Architecture roadmap
**Architectural Impact: HIGH.** Project has crossed threshold where ad-hoc per-router conventions are leaking compliance, presentation, and operational risk.
### 1. Codify compliance in code, not COMPLIANCE.md
New `src/lib/compliance_mode.ts` exports `getMode()` (env: `COMPLIANCE_MODE`, default `directory` — fail-closed). Middleware `requireMode('hybrid' | 'lrs_certified')` applied to `matched_firm_ids` write path. Boot-time assert. Surface in `/api/health`. **This is the single change that makes "non-lawyer operator, strictest reading" enforceable.**
### 2. Extract `directory-core` shared TypeScript package — host it here
This project is the only TS-native sibling (doctor is JS, animals/restaurant likely JS). TS gives shared compliance types real teeth. Carve out: `mockup_templates.ts`, `render_mockups.ts`, `site_audit.ts`, `compliance.ts` (rename → `fetch-policy.ts`), `match_firms.ts`, `html-helpers.ts` (shared esc/layout/pagination).
### 3. Split `leads.ts` (802) and `portal.ts` (773) along trust boundaries
- `leads.ts` → `leads_public.ts` (form + results, no DB writes routing to firms) + `leads_admin.ts` (queue, status, notes). The `matched_firm_ids` write at line 446 is the single most legally-loaded line in the codebase and sits buried in the middle of an 800-line render-heavy module.
- `portal.ts` → `auth_routes.ts` (signup/login/logout) + `dashboard.ts` (user dashboard) + `admin_users.ts`.
### 4. Tests
Zero `*.test.ts`. Compliance code has zero regression protection. Add `vitest` + a smoke test asserting `matched_firm_ids` write path is unreachable when `COMPLIANCE_MODE=directory`.
### 5. Ship `scripts/deploy-kamatera.sh` + audit-viewer export
Audit viewer is described as `/audit.html` in MEMORY but no checked-in deploy script. Pattern: rsync `public/audit.html` + static API snapshot, pm2 reload, verify.
## Files touched
- `/Users/stevestudio2/Projects/lawyer-directory-builder/src/server/index.ts`
- `/Users/stevestudio2/Projects/lawyer-directory-builder/src/server/upgrade.ts`
- `/Users/stevestudio2/Projects/lawyer-directory-builder/src/scripts/run_overnight.ts`