← back to Codex Review 2026 04 30
consensus/Letsbegin.md
58 lines
# Consensus — Letsbegin
Generated 2026-04-30T23:48:03-07:00. Source: codex review/fix/rereview/round-2 artifacts (round-3 was quota-blocked).
## Codex pass-1 review (top risks)
## 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.
## Round-1 applied
## Applied
- `.env.example:1` — added placeholder-only env keys for Gemini, auth, admin scan, database, Ralph, and Shopify — documents required configuration without real secrets.
- `lib/auth.ts:8` — replaced hardcoded auth/session fallbacks with required env lookup — removes fallback credentials from source.
- `lib/gemini-image.ts:7`, `app/api/prd/generate/route.ts:8`, `app/api/prd/clarify/route.ts:4`, `app/api/ralph/convert/route.ts:6`, `app/api/claude/suggest/route.ts:6`, `app/api/url-info/route.ts:4`, `app/api/processes/analyze/route.ts:6`, `app/api/processes/fix/route.ts:7` — replaced Gemini key fallbacks with fail-fast env checks — removes live API key from app source.
- `scripts/ralph-llm-runner.js:45` — removed Gemini fallback key and fails fast if no Ralph/Gemini env key exists — removes live API key from runner.
- `app/api/quickmode/route.ts:4` — replaced hardcoded Postgres URL with required `DATABASE_URL` — removes DB password from API source.
- `app/api/projects/scan/route.ts:6` — replaced admin-password fallback with required `ADMIN_PASSWORD` — removes admin password from source.
- `components/PRDGeneratorStep.tsx:339` — removed hardcoded scan admin header from client fetch — stops exposing the admin key in browser code.
- `CLAUDE.md:6` — replaced real auth/API examples with env-based examples — removes documented secrets.
- `app/api/prd/generate/route.ts:73`, `app/api/url-info/route.ts:31`, `app/api/quickmode/route.ts:32` — added invalid-JSON guards returning 400 — avoids malformed bodies falling through to 500.
- `sync_brands_page.js:16`, `test_brands.js:16`, `fix_rew_products.js:7`, `nina_full_monty.js:24`, `add_ai_desc_thibaut.js:5`, `ani_full_monty.js:17`, `croppy_glm.js:27`, `fix_dwpx_images.js:7`, `fix_x_mfr.js:3`, `glitter_full_monty.js:21`, `nina_crop.js:19`, `nina_names_colors.js:5`, `retro_full_monty.js:24`, `sequin_colors_titles.js:6`, `sequin_full_monty.js:15`, `sequin_monty_noimages.js:6` — replaced script API tokens/keys/DB URLs with env variables and fail-fast checks — removes live secrets from non-migration scripts.
## Codex re-review verdict: FIX BEFORE SHIP
## New issues introduced
`scripts/ralph-llm-runner.js:47` — requires `GEMINI_API_KEY_RALPH`/`GEMINI_API_KEY` even when `RALPH_LLM` defaults to `ollama` at `scripts/ralph-llm-runner.js:110`; this breaks documented/default Ollama runs without Gemini credentials — severity P1
## Round-2 applied
## Round 2 Applied
- `scripts/ralph-llm-runner.js:45` — removed the module-scope Gemini env failure and kept the key lookup passive — fixes CODEX_REREVIEW "STATUS broken" / P1 new issue where default Ollama runs required Gemini credentials.
- `scripts/ralph-llm-runner.js:110` — added the Gemini env failure inside `callGemini()` only — follows CODEX_REREVIEW recommendation to require `GEMINI_API_KEY_RALPH`/`GEMINI_API_KEY` only when the Gemini backend is actually used.
- `app/api/prd/clarify/route.ts:75` — wrapped `request.json()` with a local 400 invalid-JSON response — fixes CODEX_REREVIEW missed call-site at line 75.
- `app/api/ralph/convert/route.ts:141` — wrapped `request.json()` with a local 400 invalid-JSON response — fixes CODEX_REREVIEW missed call-site at line 141.
- `app/api/claude/suggest/route.ts:130` — wrapped `request.json()` with a local 400 invalid-JSON response and kept safe defaults for destructured fields — fixes CODEX_REREVIEW missed call-site at line 130.
- `app/api/processes/analyze/route.ts:33` — wrapped `request.json()` with a local 400 invalid-JSON response — fixes CODEX_REREVIEW missed call-site at line 33.
- `app/api/processes/fix/route.ts:36` — wrapped `request.json()` with a local 400 invalid-JSON response — fixes CODEX_REREVIEW missed call-site at line 36.
## Still deferred for Steve
- P0 — `.env.local:1` — live secrets may remain in local env — left alone per instruction; rotate/purge history needs Steve.
- P0 — `scrape_dg_specs.js:12`, `migrate_global_to_custom.js:29` — remaining hardcoded DB/Shopify secrets — deferred because these are DDL/migration-adjacent scripts.
- P0 — `middleware.ts:6` — admin/process/API prefixes are public — auth/CSRF redesign is beyond safe fixes.
- P0 — `lib/auth.ts:8`, `app/api/auth/login/route.ts:58`, `middleware.ts:70` — sessions are unsigned and cookie `secure` is false — needs session design change.
- P0 — `app/api/ralph/run/route.ts:57`, `app/api/processes/start/route.ts:83`, `app/api/processes/stop/route.ts:17`, `app/api/live/check/route.ts:123` — arbitrary project/process control and shell execution risks — needs sandboxing/authorization.
- P0 — `app/api/url-info/route.ts:29` — public SSRF remains — needs URL/IP validation and rate limiting.
- P1 — `app/api/files/route.ts:14`, `app/api/directory/route.ts:71`, `app/api/upload/route.ts:16` — broad path access/upload/delete guards — needs realpath allowlist design.
- P1 — `app/api/projects/scan/route.ts:12`, `app/api/projects/scan/route.ts:215` — referer trust and shell interpolation remain — needs real authorization plus exec/native API rewrite.
- P1 — `lib/sheets.ts:32`, `app/api/ralph/status/route.ts:41` — concurrent JSON state writes/GET mutation — needs locking or durable state store.