`Letsbegin/review.md`

39 lines
## Snapshot
- GoodQuestionRalph / Letsbegin: internal Next.js UI for PRD generation, project scanning, and AI-assisted “Ralph” implementation runs.
- Stack: TypeScript/React, Next.js 16, Node.js, Tailwind 4, PostgreSQL via `pg`, file-backed JSON state in `/tmp` and `data/`.
- How it's run: PM2 (`ecosystem.config.js:1-16`, `CLAUDE.md:5-8`).
- Status: live; production URL/process documented.

## Top Risks (P0 — fix this week)
- `.env.local:1`, `CLAUDE.md:5-6`, `CLAUDE.md:24-27`, `lib/gemini-image.ts:7`, `app/api/prd/generate/route.ts:8`: live API keys/passwords are in repo/docs. Rotate keys/passwords, purge history, require env vars with no fallbacks.
- `middleware.ts:6-15`: `/api/ralph`, `/api/processes`, `/api/live`, `/api/url-info` are public by prefix. Remove these from `publicPaths`; require auth and CSRF for every mutating/admin route.
- `lib/auth.ts:7-10`, `app/api/auth/login/route.ts:54-60`, `middleware.ts:68-80`: sessions are unsigned base64 JSON and cookies are `secure:false`; anyone can forge `{authenticated:true}`. Use signed/encrypted session cookies, `secure:true`, env-only secrets.
- `app/api/ralph/run/route.ts:57-63`, `app/api/ralph/run/route.ts:89-166`, `app/api/ralph/run/route.ts:168-237`: public endpoint accepts arbitrary `projectPath`/story data, writes files, `chown -R`s, creates shell scripts, and spawns them. Restrict project paths to an allowlist and remove public access before reuse.
- `app/api/processes/start/route.ts:83-194`: root-running script path uses unsanitized PRD fields and invokes `claude --dangerously-skip-permissions`. Disable this route or run in a locked sandbox with strict input schema and no root HOME.
- `app/api/processes/stop/route.ts:17-36`, `app/api/live/check/route.ts:123-127`: public process-control routes can kill arbitrary PIDs or pkill by injected project/action data. Authenticate, validate numeric PID ownership, remove shell interpolation.
- `app/api/url-info/route.ts:26-43`: public SSRF fetches any `http*` URL, including localhost/internal services. Block private IPs, redirects to private ranges, non-HTTP(S), and add rate limits.
- `app/api/quickmode/route.ts:4-6`: hardcoded Postgres production credentials. Move to `DATABASE_URL`, rotate DB password, and use least-privileged DB user.

## Notable Issues (P1 — fix this month)
- `app/api/files/route.ts:13-22`: path guard uses string prefix and allows `/tmp`; forged auth can read arbitrary temp/project files. Resolve realpath and enforce a narrow project root allowlist.
- `app/api/directory/route.ts:71-84`: authenticated users can enumerate any path passed in `path`. Same realpath/allowlist fix; cap directory size.
- `app/api/upload/route.ts:15-31`, `app/api/upload/route.ts:49-58`: arbitrary `projectPath` upload and broad `/uploads/` delete. Validate root, file type/size, and delete only files created by this app.
- `app/api/projects/scan/route.ts:14-18`: “admin” check trusts same-origin `referer`, which clients can spoof. Remove referer auth; require real session role/server-side authorization.
- `app/api/projects/scan/route.ts:212-236`, `app/api/projects/scan/route.ts:288-289`, `app/api/projects/scan/route.ts:358-359`: shell commands interpolate `projectPath`. Use `execFile`/spawn args or native git/fs APIs.
- `lib/sheets.ts:32-47`, `app/api/ralph/status/route.ts:41-42`: read-modify-write JSON state can lose updates under concurrent requests. Use atomic writes plus locking, SQLite, or Postgres.
- `app/api/ralph/status/route.ts:72-138`: GET mutates state while polling logs. Split read from state mutation or serialize updates.

## Polish (P2 — when convenient)
- `DEBUG_SUMMARY.md:94-100` is stale/overconfident versus current code risks; update or remove.
- `test_brands.js` is the only observed test-like file; critical auth/process routes have no focused tests.
- Root has many one-off scraper/migration scripts; move archival scripts under `scripts/archive/` with short purpose notes.

## Strengths
- SQL in `app/api/quickmode/route.ts:34-39` uses parameterized queries.
- `scripts/ralph-llm-runner.js:235-244` rejects absolute/`..` model-written paths.
- PM2 deployment metadata is explicit and easy to operate (`ecosystem.config.js:1-16`).

## Open questions for the owner
- Is port `7300` publicly reachable, or always behind trusted nginx/basic auth?
- Which Ralph execution path is still used: `/api/ralph/run` or `/api/processes/start`?
- Should this app ever control projects outside `/root/Projects/Letsbegin`?