Files
brainwave/docs/superpowers/plans/2026-08-21-security-audit-fixes.md
T
LaurensBartelds ecbf842b3b
Test / backend (push) Successful in 15s
Test / frontend (push) Successful in 27s
Add detailed implementation plan and tests for 2026 security audit fixes
2026-08-22 00:16:13 +02:00

7.8 KiB
Raw Blame History

Security Audit Fixes Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Fix all vulnerabilities confirmed by the 2026-08-21 full-codebase security audit (5 confirmed ≥8/10 confidence + 6 sub-threshold verified findings).

Architecture: Three clusters. (A) Session security: JWTs gain a tv (token version) claim; requireAuth revalidates the user against SQLite on every request (deleted accounts, demoted roles, password-rotation revocation); logout blacklists until real exp. (B) AI cost control: wire the existing-but-dead quota helpers into every provider call path and cap attacker-controlled prompt sizes. (C) Bootstrap & hygiene: make the setup wizard the only admin bootstrap, stop register-time auto-admin, sanitize admin user listings, bound result schemas, enforce view-only share mode server-side, pin SSH host keys in CI.

Tech Stack: Express 5, better-sqlite3 (sync), jsonwebtoken 9, zod 3, vitest 4, Gitea Actions.

Spec: Security audit report delivered in this session (2026-08-21). Findings referenced below as Vuln N / sub-threshold N.

Global Constraints

  • Do not alter lines containing redacted numeric literals in userRepo.ts (storageQuotaBytes ?? …) and migrations.ts (storage_quota_bytes … DEFAULT) — edit around them.
  • Keep HTTP response shapes of AI endpoints unchanged ({ data, usageTokens } payloads stay) so the frontend needs no changes.
  • Tests run with cd backend && npx vitest run; vitest.setup stubs JWT_SECRET, DB_PATH=:memory:; getDb() runs full migrations on first use.
  • No git commits — user has not requested them; leave changes in working tree.
  • Existing deployments upgrade transparently; all users are logged out once by the new tv claim (accepted, noted in final report).

Task 1: Token versioning + DB revalidation (Vuln 3, Vuln 4, sub-threshold 1)

Files:

  • Modify: backend/src/db/migrations.ts (add column)
  • Modify: backend/src/utils/tokens.ts (JwtPayload.tv)
  • Modify: backend/src/repositories/userRepo.ts (token_version plumbing)
  • Modify: backend/src/middleware/auth.ts (revalidation)
  • Modify: backend/src/services/authService.ts (sign with tv, bump on password events)
  • Modify: backend/src/routes/setup.ts (sign with tv)
  • Modify: backend/src/routes/auth.ts (logout uses decoded exp)
  • Modify: backend/src/utils/tokenBlacklist.ts (purge expired rows at init)
  • Test: backend/src/utils/tokens.test.ts (update payload fixtures), backend/src/middleware/auth.test.ts (new)

Interfaces:

  • Produces: JwtPayload { userId, email, displayName, role, tv }; DbUser.token_version: number; UpdateUserData.tokenVersion?: number.

  • Step 1: Write failing tests (auth.test.ts): valid token passes and req.user.role comes from DB; deleted user → 401; tv mismatch → 401. Update tokens.test.ts payload fixtures to include tv: 0 (type error until implemented = failing).

  • Step 2: Run npx vitest run src/middleware/auth.test.ts src/utils/tokens.test.ts → FAIL (missing tv / column).

  • Step 3: Implement per code in execution notes: migration guard block; token_version INTEGER NOT NULL DEFAULT 0; JwtPayload gains required tv; requireAuth loads user via getUserById, 401 on missing user, 401 on user.token_version !== payload.tv, rebuilds req.user from the DB row; changePassword/resetPassword bump tokenVersion; logout blacklists (decoded.exp ?? now+86400)*1000; initCache purges expired rows before loading.

  • Step 4: Run → PASS.

  • Step 5: Full suite npx vitest run → PASS.

Task 2: Setup-gated bootstrap, no register auto-admin, enumeration-safe register, last-admin guard (sub-threshold 2, 3, 4)

Files:

  • Modify: backend/src/routes/setup.ts (status + POST gate on countAdmins)

  • Modify: backend/src/routes/auth.ts (drop isFirstUser; last-admin self-delete guard)

  • Modify: backend/src/services/authService.ts (register signature, role always 'user', duplicate → generic success after timing-equalized hash)

  • Modify: backend/src/db/migrations.ts (remove auto-promote block)

  • Modify: backend/src/repositories/userRepo.ts (remove dead promoteFirstUserToAdmin)

  • Test: backend/src/services/authService.test.ts (new)

  • Step 1: Failing tests: duplicate-email register resolves { message } with no token; fresh register returns token with role 'user'; both take ~equal time (hash-before-check asserted structurally, not by timing).

  • Step 2: Run → FAIL.

  • Step 3: Implement: setup status/gate switch to countAdmins() > 0; register drops isFirstUser param, hashes password before duplicate check, returns { message: 'Check your inbox…' } on duplicates; DELETE /api/auth/me refuses when role==='admin' && countAdmins()<=1; migrations/userRepo promotion code deleted.

  • Step 4: Run → PASS. Step 5: Full suite → PASS.

Task 3: AI quota enforcement + prompt size caps (Vuln 1, Vuln 2)

Files:

  • Modify: backend/src/utils/aiQuota.ts (add enforceDailyAiQuota throwing AppError 429)
  • Create: backend/src/validators/gradeSchemas.ts (schema with serialized-size superRefine caps), backend/src/validators/aiSchemas.ts (hintSchema moved + cap)
  • Modify: backend/src/routes/ai.ts, backend/src/routes/grade.ts, backend/src/routes/results.ts (enforce before provider call; recordDailyAiUsage after, using returned usageTokens)
  • Test: backend/src/utils/aiQuota.test.ts, backend/src/validators/gradeSchemas.test.ts

Interfaces:

  • Consumes: recordDailyAiUsage(userId, getUtcDateKey(), tokens); services' existing AiUsageResult.usageTokens.

  • Produces: enforceDailyAiQuota(userId): void (throws AppError(429)).

  • Step 1: Failing tests: quota throws 429 when used ≥ limit; passes under limit; passes when limit unset ("0"/absent → null → unlimited). Grade schema rejects question JSON > 8000 chars / answer > 4000 chars.

  • Step 2: Run → FAIL. Step 3: Implement. Step 4: Run → PASS. Step 5: Full suite → PASS.

Task 4: Admin user DTO + result schema bounds (sub-threshold 5, 6)

Files:

  • Modify: backend/src/repositories/userRepo.ts (SafeUser, explicit SELECT columns in listUsers)

  • Modify: backend/src/validators/resultSchemas.ts (finite/int/min/max bounds, title ≤300)

  • Test: backend/src/validators/resultSchemas.test.ts, repo test for sanitized listing

  • Step 1: Failing tests: listUsers never includes password_hash/verify_token/reset_token/reset_expires keys; schema rejects negative counts, non-integers, huge floats, oversized titles.

  • Step 2–5: implement → PASS → full suite PASS.

Task 5: View-only share clone block (Vuln 5)

Files:

  • Modify: backend/src/routes/share.ts

  • Test: backend/src/routes/share.test.ts

  • Export pure canClone(shareMode?: string | null): boolean ('view' → false; 'clone'/null/undefined → true). Failing test first, then guard if (!canClone(source.share_mode)) throw new AppError(403, 'This exam is shared as view-only and cannot be saved') before cloneExam. PASS + full suite.

Task 6: CI SSH host-key pinning (sub-threshold 7)

Files: Modify .gitea/workflows/deploy.yml

  • Replace keyscan step: write ${{ vars.SSH_HOST_PUBLIC_KEY }} via env-indirection into known_hosts; hard-fail with setup instructions when unset. Validate YAML parses.

Task 7: Verification

  • cd backend && npx tsc --noEmit && npx vitest run — all green.
  • cd frontend && npm run build (or tsc -b) — typecheck green (no frontend changes expected).
  • Ultracode adversarial review workflow over the full diff; fix anything CONFIRMED.
  • Final report mapping each finding → fix → evidence.