← back to Codex Review 2026 04 30
site-factory/review.md
37 lines
## Snapshot
- Site Factory: PM2-run pipeline for generating/deploying monetized Next.js sites with admin editor, critic worker, viewer.
- Stack: Node/Express, Next.js 14/React/TS/Tailwind, PM2, Postgres, Stripe, Cloudflare/Vercel scripts.
- How it's run: PM2 via `ecosystem.config.js`; scripts/manual for DB/deploy.
- Status: live - PM2 config, logs, fresh 2026 spec.
## Top Risks (P0 — fix this week)
- `.env:10`, `.env:13`, `sites/*/app/.env.local:5-7`: live Cloudflare/Stripe/webhook secrets are in the project tree. Rotate immediately, remove all real `.env*`, add `.gitignore`, move secrets to 1Password/Vercel/PM2 env.
- `ecosystem.config.js:49`, `admin/server.js:43-46`: production PM2 enables `SF_ADMIN_DEV=1`, making every request auto-admin. Set default to `0`, bind admin to `127.0.0.1`, require real OAuth.
- `admin/server.js:15`: production can boot with hardcoded `SESSION_SECRET`, enabling session forgery if dev bypass is later removed. Refuse boot in production unless `SESSION_SECRET` is set and strong.
- `orchestrator/server.js:15`, `orchestrator/server.js:37`, `orchestrator/server.js:96`, `orchestrator/server.js:130`, `orchestrator/server.js:164`, `orchestrator/server.js:180`: orchestrator has open CORS and no auth on write routes. Add shared bearer auth, restrict CORS, expose only behind admin.
- `orchestrator/server.js:195-204`, `stages/runners.js:240-243`: concurrent `/advance` calls can run the same stage twice, duplicating DNS/Stripe/Vercel work. Use DB transaction/advisory lock and only advance `pending` stages.
- `sites/wholivedthere.com/app/lib/db.ts:30-33`, `db/schema.sql:80-89`: webhook writes `site,email` columns that do not exist, then catches/logs and still returns 200. Align schema/query and make persistence failure return 500/retry.
## Notable Issues (P1 — fix this month)
- `orchestrator/server.js:110-112`, `orchestrator/server.js:144-146`, `db/schema.sql:16-25`: code writes `theme_overrides`/`copy_overrides`, but schema lacks both columns. Add migration or editor PATCHes will 500 on fresh DBs.
- `sites/*/app/app/api/webhook/stripe/route.ts:11-13`: missing webhook secret makes the endpoint accept and ignore Stripe events. In production, fail closed unless explicitly `NODE_ENV !== "production"`.
- `orchestrator/server.js:24-35`, `orchestrator/server.js:158-160`: async Express routes lack error wrappers; DB failure can become unhandled rejection/no response. Add centralized async handler.
- `stages/runners.js:76-123`, `stages/runners.js:203-212`: most pipeline stages are stubs but still advance as done. Track `stub/blocked` separately; do not mark launch/deploy/monetize complete.
- `critic/server.js:112-148`, `critic/SELF-CRITIQUE.md:17-22`: `/critique/stub` can post arbitrary findings and has no auth/rate limit. Gate it behind dev-only/shared secret.
- `scripts/deploy-vercel.sh:40-45`: pushes every `.env.local` key to Vercel production, including local DB socket URLs and shared live Stripe key. Use per-site allowlist and production-safe DATABASE_URL.
## Polish (P2 — when convenient)
- No `.gitignore` observed; `node_modules`, `.next`, logs, `.env.local` are present in tree.
- No tests observed; webhook persistence and schema migrations need coverage because current mismatch is silent revenue data loss.
- `PROCESS.md:3` says draft/blocked, while `PROCESS.md:186-191` says resolved; update docs to match reality.
- Hardcoded absolute paths in `ecosystem.config.js:58-80`, `orchestrator/publish.js:6`, scripts reduce portability.
## Strengths
- SQL queries I reviewed use parameterized placeholders; no obvious SQL injection in main routes.
- Stripe webhook signature verification exists when `STRIPE_WEBHOOK_SECRET` is configured.
- Admin UI escapes rendered dynamic content in `admin/public/editor.js:292-297`.
## Open questions for the owner
- Are these `.env`/`.env.local` files ever committed or synced off-machine?
- Should orchestrator/admin be reachable beyond localhost?
- Is Postgres schema managed only by `db/schema.sql`, or are there missing migrations elsewhere?