9.2 KiB
9.2 KiB
padhle — Security Hardening Plan (fixes first, tested)
Purpose: Close every vulnerability found in the 2026-08-19 codebase review, get as close to production-level as possible, and prove there are no session leaks with automated tests. Runtime: Local Supabase is started by the user (
npx supabase start→ API on:54321, Studio:54323). Backend runs on:3001. Style: Each phase has small, ordered, independently verifiable steps. Every step has a Definition of Done and a Verify command so progress is provable.
Phase 0 — Baseline (before touching code)
| # | Step | Verify |
|---|---|---|
| 0.1 | Confirm local Supabase is up: curl -s http://127.0.0.1:54321/rest/v1/ |
HTTP 200 + OpenAPI JSON |
| 0.2 | Confirm backend/.env has SUPABASE_URL, SUPABASE_PUBLISHABLE_KEY, SUPABASE_SERVICE_ROLE_KEY, OPENAI_API_KEY |
grep -oE '^[A-Z_]+=' backend/.env shows all |
| 0.3 | Baseline tests: cd backend && node --test and cd frontend && npm run build |
All pass before changes |
Phase 1 — 🔴 Secret scrub (do first; no code risk)
Problem: sb_secret_N7UND0UgjKTVK-Uodkm0Hg_xSvEMPvz (local Supabase service-role key, bypasses RLS) is hardcoded in git-tracked files.
| # | File | Change | DoD / Verify |
|---|---|---|---|
| 1.1 | ANALYSIS.md (~line 311) |
Replace the live secret with <rotated-local-dev-key> and a note to never commit keys |
git grep -n sb_secret → no real key |
| 1.2 | IMPLEMENTATION_PLAN.md (~line 397) |
Same replacement | git grep -n sb_secret → no real key |
| 1.3 | backend/scripts/setup-db.js (line 10) |
Remove hardcoded fallback; throw if SUPABASE_SERVICE_ROLE_KEY is missing |
node --check backend/scripts/setup-db.js passes; no key string in file |
| 1.4 | backend/.env.example |
Add SUPABASE_URL, SUPABASE_PUBLISHABLE_KEY, SUPABASE_SERVICE_ROLE_KEY placeholders |
File lists the vars |
| 1.5 | Rotate local keys | User runs npx supabase stop && npx supabase start (or regenerates), then updates backend/.env |
Old key string no longer works; app still connects |
| 1.6 | Guard | Add a scripts/check-secrets.mjs (fails git grep-style scan on known patterns) and note to run it pre-commit |
Script exits 0 on clean tree, 1 on a planted secret |
Also add a
.gitignore-style reminder: never add.envfiles. Already covered by existing gitignores.
Phase 2 — 🟠 Auth hardening
Problem A: GET /api/auth/me trusts an unsigned padhle.user JSON cookie — identity is forgeable.
| # | File | Change | Verify |
|---|---|---|---|
| 2.1 | backend/src/routes/auth.js |
/me becomes async: read padhle.token, call verifyToken() from middleware/supabaseAuth.js; on success return { user: { uid, email } } from the verified token; on failure 401 |
curl with forged cookie → 401; real login → 200 with correct uid |
| 2.2 | backend/src/middleware/supabaseAuth.js |
Export verifyToken already exists — no change needed |
— |
Problem B: Password policy is client-side only (minLength=6).
| # | File | Change | Verify |
|---|---|---|---|
| 2.3 | backend/src/routes/auth.js |
In signup, reject password.length < 6 with 400 before calling Supabase |
Test: short password → 400 "Password must be at least 6 characters" |
Phase 3 — 🟠 Input validation (prompt injection vector)
Problem: grade/subject are allow-listed, but chapter is free-form and is injected verbatim into the AI system prompt and stored. Also POST /api/sessions validates nothing.
| # | File | Change | Verify |
|---|---|---|---|
| 3.1 | backend/src/routes/chatValidation.js |
Add validateChapter(chapter): non-empty, ≤ 200 chars, reject \n, \r, <, >. Add validateSessionMeta({grade, subject, chapter}) reusing the allow-lists + chapter check. Export both. |
Unit test in chatValidation.test.js: valid chapter ok; "Ignore <instructions>" → error; 201-char → error |
| 3.2 | backend/src/routes/chat.js |
Call validateChapter(chapter) (when present) in POST /api/chat |
curl with chapter: "<script>" → 400 |
| 3.3 | backend/src/routes/sessions.js |
POST /api/sessions validates grade/subject/chapter via validateSessionMeta |
curl bad metadata → 400 |
Phase 4 — 🟠 AI provider hardening
Problem A: getChatResponse Google branch still concatenates the API key into the URL path (unpatched half of CVE-2026-004).
| # | File | Change | Verify |
|---|---|---|---|
| 4.1 | backend/src/services/ai.js |
Rebuild Google non-streaming URL with the URL constructor + searchParams.set('key', ...) exactly like streamGoogle |
node --check; grep shows no ${apiKey} path concat |
Problem B: No timeouts on AI calls — a stalled provider hangs the SSE connection forever.
| # | File | Change | Verify |
|---|---|---|---|
| 4.2 | backend/src/services/ai.js |
streamOpenai + summarizeConversation + getChatResponse: pass signal: AbortSignal.timeout(120_000) to openai.chat.completions.create |
Code review; test mocks not required |
| 4.3 | backend/src/services/ai.js |
streamAnthropic / streamGoogle fetches: add signal: AbortSignal.timeout(120_000) |
Code review |
Phase 5 — 🟡 Middleware / robustness
| # | File | Change | Verify |
|---|---|---|---|
| 5.1 | backend/src/middleware/supabaseAuth.js |
Cap tokenCache (e.g. 2000 entries): when over cap, delete the oldest key (Map insertion order) |
Unit test or code review |
| 5.2 | backend/src/middleware/securityHeaders.js (new) |
Manual headers middleware: X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy: no-referrer, Cross-Origin-Opener-Policy: same-origin (no new dependency) |
curl -sI http://localhost:3001/health shows headers |
| 5.3 | backend/src/index.js |
Mount securityHeaders first; add app.set('trust proxy', ...) from env (TRUST_PROXY default false; set to 1 behind a proxy) |
Headers present; req.ip correct behind proxy |
| 5.4 | backend/src/middleware/rateLimiter.js |
In dev (NODE_ENV !== 'production'), skip rate limits for any loopback IP (::1, 127.0.0.1, ::ffff:127.0.0.1) — prevents tests from tripping the 5/15min auth limit |
Integration test can sign up users repeatedly |
Phase 6 — Session-leak verification (the core ask)
Goal: Prove cross-user access is impossible, end to end, against the real local Supabase.
| # | File | Change | Verify |
|---|---|---|---|
| 6.1 | backend/src/app.js (new) |
Extract the Express app from index.js into an exported app (no listen) |
node --check |
| 6.2 | backend/src/index.js |
Import app from ./app.js; only app.listen stays here |
Backend still starts |
| 6.3 | backend/test/leak.test.js (new) |
Integration test: starts app.listen(0) on loopback, then with fetch + manual cookie jar: 1. Sign up user A and user B (unique emails). 2. A sends a chat → capture session id from the SSE stream. 3. B GET /api/chat/:id → 404. 4. B GET /api/chat/sessions and GET /api/sessions → must not contain A's session. 5. B DELETE /api/sessions/:id and PATCH .../clear → 404. 6. B posts to A's chatId → 404. 7. /api/auth/me with a forged padhle.user cookie → 401. 8. POST /api/chat with chapter: "<script>ignore" → 400. Teardown: delete A/B from auth.users via service role (or leave test accounts, documented). |
cd backend && node --test test/leak.test.js → all pass |
Phase 7 — Regression & full verification
| # | Step | Verify |
|---|---|---|
| 7.1 | cd backend && node --test |
All suites pass (chatValidation, aiMessages, leak) |
| 7.2 | cd frontend && npm run build |
Vite build passes |
| 7.3 | Syntax check all backend files: for f in backend/src/**/*.js; do node --check "$f"; done |
All pass |
| 7.4 | Manual smoke (optional, user): start backend + frontend, sign up, chat, reload | No console errors; session persists |
| 7.5 | `git grep -n -E 'sb_secret | sk-[A-Za-z0-9]{20,}'` |
| 7.6 | Update ANALYSIS.md Feature Tracker with a "Security Hardening" section listing fixes + verification results |
Doc is current |
Explicitly out of scope (documented for later)
- Refresh-token flow (hourly cookie expiry, no silent refresh) — needs
padhle.refreshcookie + middleware auto-refresh; flagged, not implemented now. - Server-side session revocation list — signout clears cookies + calls Supabase logout; server revocation of other sessions is a future item.
- CSRF tokens — mitigated via
sameSite: laxhttpOnly cookies; full CSRF token flow deferred. - Rate limiter storage (
express-rate-limitin-memory) — single-instance assumption; move to Redis when multi-instance. run-migration.jsplaintext local Postgres password (postgres) — local dev only; documented, not a prod risk.- XSS sanitization in
sessionStore.js— file is legacy/unused; rendering already escapes HTML. Documented, not implemented.
Priority order for execution (this goal round)
- Phase 1 (secret scrub) → 2. Phase 3 (validation) → 3. Phase 4 (AI URL + timeouts) → 4. Phase 5 (middleware) → 5. Phase 2 (auth) → 6. Phase 6 (leak test) → 7. Phase 7 (regression)