`jill-website/review.md`

39 lines
## Snapshot
- Vacation rental/wallcoverings-adjacent marketing + inquiry site for Nosara rentals, with CMS/admin and activity listings.
- Stack: TypeScript, Node.js, Express, EJS, PostgreSQL, Nodemailer, PM2.
- How it's run: PM2 configured in `ecosystem.config.js`; README also documents manual `npm start`.
- Status: stale: route drift, checked-in env, missing local deps.

## Top Risks (P0 — fix this week)
- `.env:31` / `.env:32`: Google API keys are committed. Rotate them, restrict by domain/API, remove `.env` from git history, keep only `.env.example`.
- `src/server.ts:23` / `src/server.ts:24` / `src/middleware/auth.ts:12` / `views/admin.ejs:416`: production auth falls back to hardcoded passwords and the admin UI embeds `admin:admin123`. Require env secrets at boot; remove client-side Basic Auth; use server sessions.
- `src/routes/activities.ts:27` / `src/routes/activities.ts:144` / `src/routes/activities.ts:186` / `src/routes/activities.ts:304`: create/update/delete/bulk-import activity routes have no admin auth beyond site-wide Basic Auth. Mount mutating routes behind `requireAuth` or split public read vs admin write routers.
- `src/routes/linkValidation.ts:230` / `src/services/linkValidationService.ts:23`: `/check-url` fetches arbitrary URLs and follows redirects, enabling SSRF/internal probing. Restrict schemes to http/https, block private/link-local IPs, and allowlist external domains if possible.
- `src/migrations/002_create_cms_tables.sql:13` / `src/migrations/002_create_page_content.sql:2`: duplicate `002` migrations create incompatible `page_content` schemas; fresh migrations can fail or leave columns the model does not use. Collapse/renumber migrations and reconcile `page_title/gallery_images` vs `title/images`.

## Notable Issues (P1 — fix this month)
- `src/server.ts:97` vs `views/admin.ejs:414`: admin router is mounted at `/api/admin`, but UI calls `/admin/api/...`; admin CMS endpoints will 404. Align mount paths and fetch URLs.
- `views/admin-login.ejs:201`: login posts to `/api/cms/auth/login`, but no such TypeScript route exists. Either wire `AdminUserModel` sessions or remove dead login UI.
- `src/server.ts:69`: site-wide Basic Auth protects even public pages. If this is meant to be public marketing, the current app is effectively hidden.
- `src/routes/unsubscribe.ts:10` / `src/server.ts:90`: unsubscribe router is not mounted in `src/server.ts`; email links point to a dead endpoint. Mount it and persist unsubscribe state.
- `src/routes/unsubscribe.ts:89`: raw `email` query is interpolated into HTML. Escape output before rendering.
- `src/services/emailService.ts:164` / `src/services/emailService.ts:205`: inquiry fields are inserted into HTML email without escaping. Escape/sanitize user fields.
- `src/routes/inquiries.ts:21`: invalid date strings become `Invalid Date` and likely produce DB errors instead of 400s. Validate dates explicitly before model insert.
- `src/routes/inquiries.ts:30`: email send failures are only logged after responding; lost confirmations have no durable retry/alert. Persist email status or queue retries.
- `src/routes/properties.ts:15` / `src/routes/inquiries.ts:78`: `limit`/`offset` accept `NaN`, negatives, and huge values. Clamp and validate query params.

## Polish (P2 — when convenient)
- `server.js:745`: legacy root server writes `index.html` at runtime and mounts different routes than `src/server.ts`; remove or quarantine it.
- `coverage/`: generated coverage is committed; ignore generated reports unless intentionally published.
- `npm run typecheck`: failed because `tsc` was not available locally; install deps before relying on repo verification.
- `views/admin.ejs:508`: frontend sends `title/images`, backend expects `page_title/gallery_images`; fix naming drift.

## Strengths
- SQL access is mostly parameterized, reducing injection risk in model queries.
- Models centralize validation and formatting for inquiries, properties, and activities.
- Route factories with dependency injection make API handlers testable.

## Open questions for the owner
- Is the deployed entry point `dist/server.js` or root `server.js`?
- Should the public site be password-protected, or only admin/write actions?
- Are the committed Google keys real and currently enabled?