From ecbf842b3ba3888e28c4b13c10469a1168371271 Mon Sep 17 00:00:00 2001 From: LaurensBartelds <1+laurensbartelds@noreply@gzmn.nl> Date: Sat, 22 Aug 2026 00:16:13 +0200 Subject: [PATCH] Add detailed implementation plan and tests for 2026 security audit fixes --- .gitea/workflows/deploy.yml | 12 +- backend/src/db/migrations.ts | 15 +-- backend/src/middleware/auth.test.ts | 127 ++++++++++++++++++ backend/src/middleware/auth.ts | 23 +++- backend/src/repositories/userRepo.test.ts | 47 +++++++ backend/src/repositories/userRepo.ts | 34 +++-- backend/src/routes/admin.ts | 2 +- backend/src/routes/ai.ts | 28 ++-- backend/src/routes/auth.ts | 25 +++- backend/src/routes/grade.ts | 17 +-- backend/src/routes/results.ts | 8 +- backend/src/routes/setup.ts | 16 +-- backend/src/routes/share.test.ts | 14 ++ backend/src/routes/share.ts | 9 ++ backend/src/services/authService.test.ts | 106 +++++++++++++++ backend/src/services/authService.ts | 75 ++++++----- backend/src/utils/aiQuota.test.ts | 54 ++++++++ backend/src/utils/aiQuota.ts | 11 ++ backend/src/utils/tokenBlacklist.ts | 3 +- backend/src/utils/tokens.test.ts | 5 +- backend/src/utils/tokens.ts | 9 +- backend/src/validators/aiSchemas.test.ts | 23 ++++ backend/src/validators/aiSchemas.ts | 25 ++++ backend/src/validators/gradeSchemas.test.ts | 65 +++++++++ backend/src/validators/gradeSchemas.ts | 37 +++++ backend/src/validators/resultSchemas.test.ts | 22 ++- backend/src/validators/resultSchemas.ts | 18 +-- .../plans/2026-08-21-security-audit-fixes.md | 104 ++++++++++++++ .../results.json | 2 +- 29 files changed, 827 insertions(+), 109 deletions(-) create mode 100644 backend/src/middleware/auth.test.ts create mode 100644 backend/src/repositories/userRepo.test.ts create mode 100644 backend/src/routes/share.test.ts create mode 100644 backend/src/services/authService.test.ts create mode 100644 backend/src/utils/aiQuota.test.ts create mode 100644 backend/src/validators/aiSchemas.test.ts create mode 100644 backend/src/validators/aiSchemas.ts create mode 100644 backend/src/validators/gradeSchemas.test.ts create mode 100644 backend/src/validators/gradeSchemas.ts create mode 100644 docs/superpowers/plans/2026-08-21-security-audit-fixes.md diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index c97345c..fcc824c 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -26,11 +26,19 @@ jobs: echo "SSH_AUTH_SOCK=$SSH_AUTH_SOCK" >> "$GITHUB_ENV" echo "SSH_AGENT_PID=$SSH_AGENT_PID" >> "$GITHUB_ENV" - - name: Add OrangePi to known_hosts + - name: Pin server host key to known_hosts + env: + HOST_KEY: ${{ vars.SSH_HOST_PUBLIC_KEY }} run: | mkdir -p ~/.ssh chmod 700 ~/.ssh - ssh-keyscan -H "${{ secrets.ANSIBLE_HOST }}" >> ~/.ssh/known_hosts + if [ -z "$HOST_KEY" ]; then + echo "::error::Repository variable SSH_HOST_PUBLIC_KEY is not set." + echo "::error::Pin the server's host key instead of trusting ssh-keyscan over the network:" + echo "::error::run 'ssh-keyscan -t ed25519 ' once from a trusted network and store the full output line in SSH_HOST_PUBLIC_KEY." + exit 1 + fi + printf '%s\n' "$HOST_KEY" >> ~/.ssh/known_hosts - name: Write Inventory run: | diff --git a/backend/src/db/migrations.ts b/backend/src/db/migrations.ts index d2031e2..477e5b8 100644 --- a/backend/src/db/migrations.ts +++ b/backend/src/db/migrations.ts @@ -94,6 +94,11 @@ export function runMigrations(db: Database.Database): void { db.exec(`ALTER TABLE users ADD COLUMN plan TEXT NOT NULL DEFAULT 'free'`); } + // ── Add token_version if missing (JWT revocation on password change/reset) ── + if (!cols.some((c) => c.name === "token_version")) { + db.exec(`ALTER TABLE users ADD COLUMN token_version INTEGER NOT NULL DEFAULT 0`); + } + // ── Add is_favorite to saved_exams if missing ── const examSavedCols = db.prepare(`PRAGMA table_info(saved_exams)`).all() as { name: string }[]; if (!examSavedCols.some((c) => c.name === 'is_favorite')) { @@ -132,16 +137,6 @@ export function runMigrations(db: Database.Database): void { CREATE INDEX IF NOT EXISTS idx_collections_user ON collections(user_id); `); - // ── Promote first user to admin if no admins exist ── - const adminCount = db - .prepare(`SELECT COUNT(*) AS cnt FROM users WHERE role = 'admin'`) - .get() as { cnt: number }; - if (adminCount.cnt === 0) { - db.prepare( - `UPDATE users SET role = 'admin' WHERE id = (SELECT MIN(id) FROM users)`, - ).run(); - } - // ── Seed default settings ── const defaults: Record = { site_name: "BrainWave", diff --git a/backend/src/middleware/auth.test.ts b/backend/src/middleware/auth.test.ts new file mode 100644 index 0000000..a4029a9 --- /dev/null +++ b/backend/src/middleware/auth.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, vi } from 'vitest' +import type { Request, Response, NextFunction } from 'express' +import * as userRepo from '../repositories/userRepo.js' +import { signToken } from '../utils/tokens.js' +import { requireAuth } from './auth.js' + +function mockRes(): Response { + const res = {} as Response + res.status = vi.fn(() => res) as unknown as Response['status'] + res.json = vi.fn(() => res) as unknown as Response['json'] + return res +} + +function makeReq(token?: string): Request { + const headers: Record = {} + if (token) headers.authorization = `Bearer ${token}` + return { headers } as unknown as Request +} + +function createUser(role: 'user' | 'admin' = 'user'): { id: number; email: string; name: string } { + const email = `user-${Math.random().toString(36).slice(2)}@example.com` + const { id } = userRepo.createUser({ + email, + passwordHash: 'x', + displayName: 'Test User', + role, + emailVerified: 1, + }) + return { id, email, name: 'Test User' } +} + +describe('requireAuth', () => { + it('accepts a valid token and loads fresh user data from the DB', () => { + const u = createUser() + const token = signToken({ userId: u.id, email: u.email, displayName: u.name, role: 'user', tv: 0 }) + const req = makeReq(token) + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).toHaveBeenCalledOnce() + expect(res.status).not.toHaveBeenCalled() + expect(req.user?.userId).toBe(u.id) + expect(req.user?.role).toBe('user') + expect(req.user?.tv).toBe(0) + }) + + it('serves the current DB role, not the role claimed by an old JWT', () => { + const u = createUser() + // Token signed while the user was still an admin + const staleAdminToken = signToken({ userId: u.id, email: u.email, displayName: u.name, role: 'admin', tv: 0 }) + userRepo.updateUser(u.id, { role: 'user' }) + const req = makeReq(staleAdminToken) + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).toHaveBeenCalledOnce() + expect(req.user?.role).toBe('user') + }) + + it('rejects tokens for deleted accounts', () => { + const u = createUser() + const token = signToken({ userId: u.id, email: u.email, displayName: u.name, role: 'user', tv: 0 }) + userRepo.deleteUser(u.id) + const req = makeReq(token) + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it('rejects tokens whose token version is stale (e.g. after password change)', () => { + const u = createUser() + const oldToken = signToken({ userId: u.id, email: u.email, displayName: u.name, role: 'user', tv: 0 }) + userRepo.updateUser(u.id, { tokenVersion: 1 }) + const req = makeReq(oldToken) + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it('accepts a token signed with the current token version after rotation', () => { + const u = createUser() + userRepo.updateUser(u.id, { tokenVersion: 2 }) + const freshToken = signToken({ userId: u.id, email: u.email, displayName: u.name, role: 'user', tv: 2 }) + const req = makeReq(freshToken) + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).toHaveBeenCalledOnce() + expect(req.user?.tv).toBe(2) + }) + + it('rejects requests without a Bearer header', () => { + const req = makeReq() + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) + + it('rejects garbage tokens', () => { + const req = makeReq('not.a.jwt') + const res = mockRes() + const next = vi.fn() as NextFunction + + requireAuth(req, res, next) + + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(401) + }) +}) diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts index 8e4d0d6..37c8482 100644 --- a/backend/src/middleware/auth.ts +++ b/backend/src/middleware/auth.ts @@ -1,6 +1,7 @@ import type { Request, Response, NextFunction } from 'express' import { verifyToken, hashToken, type JwtPayload } from '../utils/tokens.js' import { isTokenBlacklisted } from '../utils/tokenBlacklist.js' +import { getUserById } from '../repositories/userRepo.js' declare global { namespace Express { @@ -22,7 +23,7 @@ export function requireAuth(req: Request, res: Response, next: NextFunction): vo } try { const token = header.slice(7) - req.user = verifyToken(token) + const payload = verifyToken(token) // Check if the token has been blacklisted (e.g., after logout) if (isTokenBlacklisted(hashToken(token))) { @@ -30,6 +31,26 @@ export function requireAuth(req: Request, res: Response, next: NextFunction): vo return } + // Revalidate against the database: deleted accounts must not keep working, + // and role/token-version must come from the DB rather than an old JWT claim + // (revocation after demotion or password change/reset). + const user = getUserById(payload.userId) + if (!user) { + res.status(401).json({ error: 'Account no longer exists' }) + return + } + if ((user.token_version ?? 0) !== payload.tv) { + res.status(401).json({ error: 'Session expired, please sign in again' }) + return + } + + req.user = { + userId: user.id, + email: user.email, + displayName: user.display_name, + role: user.role, + tv: payload.tv, + } next() } catch { res.status(401).json({ error: 'Invalid or expired token' }) diff --git a/backend/src/repositories/userRepo.test.ts b/backend/src/repositories/userRepo.test.ts new file mode 100644 index 0000000..4529c92 --- /dev/null +++ b/backend/src/repositories/userRepo.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from 'vitest' +import * as userRepo from './userRepo.js' + +function uniqueEmail(): string { + return `safe-${Math.random().toString(36).slice(2)}@example.com` +} + +describe('userRepo.listUsers', () => { + it('never returns credential or token columns to callers', async () => { + const email = uniqueEmail() + userRepo.createUser({ + email, + passwordHash: await import('bcrypt').then((b) => b.hashSync('SecretPassword1!', 10)), + displayName: 'Safe Listing', + role: 'user', + emailVerified: 0, + verifyToken: 'verify-token-hash', + resetToken: 'reset-token-hash', + }) + + const { users, total } = userRepo.listUsers({ search: email }) + expect(total).toBe(1) + const user = users[0]! + + expect(user.email).toBe(email) + expect(user.display_name).toBe('Safe Listing') + expect(user.role).toBe('user') + + const keys = Object.keys(user) + expect(keys).not.toContain('password_hash') + expect(keys).not.toContain('verify_token') + expect(keys).not.toContain('verify_expires') + expect(keys).not.toContain('reset_token') + expect(keys).not.toContain('reset_expires') + }) + + it('still returns full rows from getUserById for internal auth use', () => { + const email = uniqueEmail() + const { id } = userRepo.createUser({ + email, + passwordHash: 'internal-only', + displayName: 'Internal', + emailVerified: 1, + }) + expect(userRepo.getUserById(id)?.password_hash).toBe('internal-only') + }) +}) diff --git a/backend/src/repositories/userRepo.ts b/backend/src/repositories/userRepo.ts index 7e28928..9ae5b58 100644 --- a/backend/src/repositories/userRepo.ts +++ b/backend/src/repositories/userRepo.ts @@ -13,6 +13,7 @@ export interface DbUser { reset_expires: number | null storage_quota_bytes: number plan: string + token_version: number created_at: number updated_at: number } @@ -40,6 +41,7 @@ export interface UpdateUserData { resetExpires?: number | null storageQuotaBytes?: number plan?: string + tokenVersion?: number } export function createUser(data: CreateUserData): { id: number } { @@ -126,6 +128,10 @@ export function updateUser(id: number, data: UpdateUserData): boolean { fields.push('plan = ?') values.push(data.plan) } + if (data.tokenVersion !== undefined) { + fields.push('token_version = ?') + values.push(data.tokenVersion) + } if (fields.length === 0) return false @@ -154,13 +160,30 @@ export function countAdmins(): number { return row.cnt } -export function listUsers(options: { limit?: number; offset?: number; search?: string } = {}): { users: DbUser[]; total: number } { +/** User row safe for API exposure — no password/token material. */ +export interface SafeUser { + id: number + email: string + display_name: string + role: 'user' | 'admin' + email_verified: number + plan: string + storage_quota_bytes: number + created_at: number +} + +const SAFE_USER_COLUMNS = + 'id, email, display_name, role, email_verified, plan, storage_quota_bytes, created_at' + +export function listUsers(options: { limit?: number; offset?: number; search?: string } = {}): { users: SafeUser[]; total: number } { const db = getDb() const limit = Math.min(options.limit ?? 50, 200) const offset = options.offset ?? 0 const search = options.search ?? '' - let query = 'SELECT * FROM users' + // Explicit column list so credential/token material can never leak into + // API responses, even if new sensitive columns are added later. + let query = `SELECT ${SAFE_USER_COLUMNS} FROM users` let countQuery = 'SELECT COUNT(*) AS total FROM users' const params: unknown[] = [] const countParams: unknown[] = [] @@ -175,13 +198,8 @@ export function listUsers(options: { limit?: number; offset?: number; search?: s query += ' ORDER BY created_at DESC LIMIT ? OFFSET ?' params.push(limit, offset) - const users = db.prepare(query).all(...params) as DbUser[] + const users = db.prepare(query).all(...params) as SafeUser[] const countRow = db.prepare(countQuery).get(...countParams) as { total: number } return { users, total: countRow.total } } - -export function promoteFirstUserToAdmin(): void { - const db = getDb() - db.prepare("UPDATE users SET role = 'admin' WHERE id = (SELECT MIN(id) FROM users)").run() -} diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index 32d6228..2946a8e 100644 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -162,7 +162,7 @@ router.post( }, { provider, apiKey, model, baseURL }, ) - res.json(result) + res.json(result.data) }), ) diff --git a/backend/src/routes/ai.ts b/backend/src/routes/ai.ts index ccdfa27..71577a3 100644 --- a/backend/src/routes/ai.ts +++ b/backend/src/routes/ai.ts @@ -5,6 +5,8 @@ import { asyncHandler } from '../utils/asyncHandler.js' import { requireAuth } from '../middleware/auth.js' import { generateQuestions, getHint, tutorChat } from '../services/aiAssist.js' import { getResultById } from '../repositories/resultRepo.js' +import { enforceDailyAiQuota, recordDailyAiUsage, getUtcDateKey } from '../utils/aiQuota.js' +import { hintSchema } from '../validators/aiSchemas.js' const router = Router() router.use(requireAuth) @@ -39,23 +41,24 @@ router.post( validateBody(generateSchema), asyncHandler(async (req, res) => { const body = req.body as z.infer - const questions = await generateQuestions(body) - res.json({ questions }) + const userId = req.user!.userId + enforceDailyAiQuota(userId) + const result = await generateQuestions(body) + recordDailyAiUsage(userId, getUtcDateKey(), result.usageTokens) + res.json({ questions: result.data }) }), ) -const hintSchema = z.object({ - question: z.record(z.unknown()), - language: z.string().min(1).max(20), -}) - router.post( '/hint', validateBody(hintSchema), asyncHandler(async (req, res) => { - const body = req.body as z.infer - const result = await getHint(body) - res.json(result) + const body = req.body + const userId = req.user!.userId + enforceDailyAiQuota(userId) + const result = await getHint({ question: body.question, language: body.language }) + recordDailyAiUsage(userId, getUtcDateKey(), result.usageTokens) + res.json(result.data) }), ) @@ -130,6 +133,8 @@ router.post( state = {} } + enforceDailyAiQuota(userId) + const reply = await tutorChat({ exam, state, @@ -143,8 +148,9 @@ router.post( messages: body.messages, language: body.language, }) + recordDailyAiUsage(userId, getUtcDateKey(), reply.usageTokens) - res.json(reply) + res.json(reply.data) }), ) diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts index 68bd7bf..4ca9daa 100644 --- a/backend/src/routes/auth.ts +++ b/backend/src/routes/auth.ts @@ -6,8 +6,9 @@ import { requireAuth } from '../middleware/auth.js' import { asyncHandler } from '../utils/asyncHandler.js' import * as authService from '../services/authService.js' import * as userRepo from '../repositories/userRepo.js' -import { hashToken } from '../utils/tokens.js' +import { hashToken, verifyToken } from '../utils/tokens.js' import { blacklistToken } from '../utils/tokenBlacklist.js' +import { AppError } from '../middleware/error.js' import { registerSchema, loginSchema, @@ -44,8 +45,7 @@ router.post( authLimiter, validateBody(registerSchema), asyncHandler(async (req, res) => { - const isFirstUser = userRepo.countUsers() === 0 - const result = await authService.register(req.body, isFirstUser) + const result = await authService.register(req.body) res.json(result) }) ) @@ -145,9 +145,18 @@ router.post( if (header?.startsWith('Bearer ')) { const token = header.slice(7) const tokenHash = hashToken(token) - // Blacklist the token so it cannot be reused after logout - // Expiry is 24h from now (matching JWT expiry) - blacklistToken(tokenHash, Date.now() + 24 * 60 * 60 * 1000) + // Blacklist until the token's real expiry so it cannot be replayed even + // after the in-memory blacklist is reloaded from SQLite on restart + try { + const decoded = verifyToken(token) + const expiresAt = + typeof decoded.exp === 'number' && decoded.exp * 1000 > Date.now() + ? decoded.exp * 1000 + : Date.now() + 60 * 1000 + blacklistToken(tokenHash, expiresAt) + } catch { + // Token already failed verification — nothing worth blacklisting + } } res.json({ success: true }) }) @@ -158,6 +167,10 @@ router.delete( '/me', requireAuth, asyncHandler(async (req: Request, res) => { + const user = userRepo.getUserById(req.user!.userId) + if (user?.role === 'admin' && userRepo.countAdmins() <= 1) { + throw new AppError(400, 'Cannot delete the last admin account') + } authService.deleteUserAccount(req.user!.userId) res.json({ success: true }) }) diff --git a/backend/src/routes/grade.ts b/backend/src/routes/grade.ts index 15906ab..0c72213 100644 --- a/backend/src/routes/grade.ts +++ b/backend/src/routes/grade.ts @@ -1,16 +1,10 @@ import { Router } from 'express' -import { z } from 'zod' import { validateBody } from '../middleware/validate.js' import { asyncHandler } from '../utils/asyncHandler.js' import { requireAuth } from '../middleware/auth.js' import { gradeQuestionWithAI } from '../services/aiGrading.js' - -const gradeRequestSchema = z.object({ - type: z.enum(['essay', 'short-answer', 'translation', 'math', 'ordering', 'match']), - question: z.record(z.unknown()), - answer: z.unknown(), - language: z.string().min(1), -}) +import { enforceDailyAiQuota, recordDailyAiUsage, getUtcDateKey } from '../utils/aiQuota.js' +import { gradeRequestSchema, type GradeRequestBody } from '../validators/gradeSchemas.js' const router = Router() @@ -20,9 +14,12 @@ router.post( '/', validateBody(gradeRequestSchema), asyncHandler(async (req, res) => { - const { type, question, answer, language } = req.body as z.infer + const { type, question, answer, language } = req.body as GradeRequestBody + const userId = req.user!.userId + enforceDailyAiQuota(userId) const result = await gradeQuestionWithAI({ type, question, answer, language }) - res.json(result) + recordDailyAiUsage(userId, getUtcDateKey(), result.usageTokens) + res.json(result.data) }), ) diff --git a/backend/src/routes/results.ts b/backend/src/routes/results.ts index f2dc5c6..687c020 100644 --- a/backend/src/routes/results.ts +++ b/backend/src/routes/results.ts @@ -7,6 +7,7 @@ import * as resultRepo from '../repositories/resultRepo.js' import * as examRepo from '../repositories/examRepo.js' import { submitResultSchema, resultIdParamSchema } from '../validators/resultSchemas.js' import { analyzeResults } from '../services/studyCoach.js' +import { enforceDailyAiQuota, recordDailyAiUsage, getUtcDateKey } from '../utils/aiQuota.js' import type { Request } from 'express' const router = Router() @@ -84,9 +85,12 @@ router.delete( router.get( '/analyze', asyncHandler(async (req: Request, res) => { - const rawResults = resultRepo.getResultsWithExamJson(req.user!.userId, 200) + const userId = req.user!.userId + const rawResults = resultRepo.getResultsWithExamJson(userId, 200) const lang = typeof req.query.lang === 'string' ? req.query.lang : 'en' + enforceDailyAiQuota(userId) const analysis = await analyzeResults(rawResults, lang) + recordDailyAiUsage(userId, getUtcDateKey(), analysis.usageTokens) // Find weakest exam (lowest score that still exists in workspace) let weakestExamId: number | null = null @@ -107,7 +111,7 @@ router.get( } } - res.json({ ...analysis, weakestExamId }) + res.json({ ...analysis.data, weakestExamId }) }) ) diff --git a/backend/src/routes/setup.ts b/backend/src/routes/setup.ts index 92b2919..8e2432e 100644 --- a/backend/src/routes/setup.ts +++ b/backend/src/routes/setup.ts @@ -22,8 +22,11 @@ router.get( '/status', asyncHandler(async (_req, res) => { const title = getSiteTitle() + // Setup is complete once an admin account exists — not merely because + // default settings were seeded + const completed = userRepo.countAdmins() > 0 res.json({ - completed: !!title, + completed, siteTitle: title ?? undefined, }) }) @@ -38,16 +41,12 @@ router.post( // Atomic transaction prevents race conditions between concurrent setup requests const tx = db.transaction(() => { - const title = settingsRepo.getSetting('site_name') - if (title) { + // Gate on admins, not on users or settings: a fresh instance has default + // settings seeded, and an instance that lost its admin can re-bootstrap + if (userRepo.countAdmins() > 0) { throw new AppError(400, 'Setup already completed') } - const userCount = userRepo.countUsers() - if (userCount > 0) { - throw new AppError(400, 'Users already exist') - } - const { email, password, @@ -99,6 +98,7 @@ router.post( email, displayName, role: 'admin', + tv: 0, }) res.status(201).json({ diff --git a/backend/src/routes/share.test.ts b/backend/src/routes/share.test.ts new file mode 100644 index 0000000..de8433a --- /dev/null +++ b/backend/src/routes/share.test.ts @@ -0,0 +1,14 @@ +import { describe, it, expect } from 'vitest' +import { canClone } from './share.js' + +describe('canClone', () => { + it('rejects view-only shares', () => { + expect(canClone('view')).toBe(false) + }) + + it('allows clone-mode shares and legacy rows without a mode', () => { + expect(canClone('clone')).toBe(true) + expect(canClone(null)).toBe(true) + expect(canClone(undefined)).toBe(true) + }) +}) diff --git a/backend/src/routes/share.ts b/backend/src/routes/share.ts index 75e98f8..7c2a3a9 100644 --- a/backend/src/routes/share.ts +++ b/backend/src/routes/share.ts @@ -7,6 +7,12 @@ import type { Request } from 'express' const router = Router() +/** Server-side enforcement of the share mode: 'view' shares must not be + * clonable (the frontend hides the button, but that is not a control). */ +export function canClone(shareMode: string | null | undefined): boolean { + return shareMode !== 'view' +} + // GET /api/share/:token — public, no auth required router.get( '/:token', @@ -37,6 +43,9 @@ router.post( if (!source) { throw new AppError(404, 'Shared exam not found') } + if (!canClone(source.share_mode)) { + throw new AppError(403, 'This exam is shared as view-only and cannot be saved') + } const { id } = examRepo.cloneExam(req.user!.userId, source, token) res.status(201).json({ id, title: source.title + ' (copy)' }) }) diff --git a/backend/src/services/authService.test.ts b/backend/src/services/authService.test.ts new file mode 100644 index 0000000..9054a52 --- /dev/null +++ b/backend/src/services/authService.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect, vi } from 'vitest' +import bcrypt from 'bcrypt' + +vi.mock('../utils/email.js', async (importOriginal) => ({ + ...(await importOriginal()), + sendVerificationEmail: vi.fn(async () => {}), + sendPasswordResetEmail: vi.fn(async () => {}), +})) + +import * as authService from './authService.js' +import { sendPasswordResetEmail } from '../utils/email.js' +import * as userRepo from '../repositories/userRepo.js' + +async function seedUser(overrides: Partial = {}): Promise { + const email = overrides.email ?? `svc-${Math.random().toString(36).slice(2)}@example.com` + const { id } = userRepo.createUser({ + email, + passwordHash: await bcrypt.hash('OldPassword1!', 10), + displayName: 'Svc Test', + emailVerified: 1, + ...overrides, + }) + return id +} + +describe('authService.register', () => { + it('registers new accounts as plain users (admin bootstrap is the setup wizard)', async () => { + const email = `new-${Math.random().toString(36).slice(2)}@example.com` + const result = await authService.register({ + email, + password: 'Sup3rSecure!pass', + displayName: 'New User', + }) + + if (!('token' in result)) throw new Error('expected an AuthResult with a token') + expect(result.token).toBeTruthy() + expect(result.user.role).toBe('user') + + const dbUser = userRepo.getUserByEmail(email) + expect(dbUser?.role).toBe('user') + expect(dbUser?.email_verified).toBe(0) + }) + + it('responds to duplicate emails with a token-less success-shaped body (no enumeration oracle)', async () => { + const id = await seedUser() + const existing = userRepo.getUserById(id)! + + const before = userRepo.countUsers() + const result = await authService.register({ + email: existing.email.toUpperCase(), + password: 'Whatever!123', + displayName: 'Imposter', + }) + + expect('token' in result).toBe(false) + expect('message' in result && typeof result.message).toBe('string') + expect(userRepo.countUsers()).toBe(before) + + // The original account must be untouched + const after = userRepo.getUserById(id)! + expect(after.display_name).toBe(existing.display_name) + }) +}) + +describe('password rotation revokes sessions', () => { + it('changePassword bumps token_version', async () => { + const id = await seedUser() + const before = userRepo.getUserById(id)! + + await authService.changePassword(id, { currentPassword: 'OldPassword1!', newPassword: 'N3wPassword!x' }) + + const after = userRepo.getUserById(id)! + expect(after.password_hash).not.toBe(before.password_hash) + expect(after.token_version).toBe((before.token_version ?? 0) + 1) + }) + + it('changePassword rejects a wrong current password', async () => { + const id = await seedUser() + const before = userRepo.getUserById(id)! + + await expect( + authService.changePassword(id, { currentPassword: 'WrongPassword!', newPassword: 'N3wPassword!x' }), + ).rejects.toThrow() + + expect(userRepo.getUserById(id)!.token_version).toBe(before.token_version) + }) + + it('resetPassword bumps token_version and clears the reset token', async () => { + const id = await seedUser({ emailVerified: 0 }) + const user = userRepo.getUserById(id)! + const email = user.email + + await authService.forgotPassword(email) + const calls = vi.mocked(sendPasswordResetEmail).mock.calls + const rawToken = calls.at(-1)?.[1] as string | undefined + expect(rawToken).toBeTruthy() + + await authService.resetPassword({ token: rawToken!, newPassword: 'BrandNew!123' }) + const after = userRepo.getUserById(id)! + + expect(after.token_version).toBe((user.token_version ?? 0) + 1) + expect(after.reset_token).toBeNull() + expect(after.reset_expires).toBeNull() + expect(bcrypt.compareSync('BrandNew!123', after.password_hash)).toBe(true) + }) +}) diff --git a/backend/src/services/authService.ts b/backend/src/services/authService.ts index 29dc18d..a537577 100644 --- a/backend/src/services/authService.ts +++ b/backend/src/services/authService.ts @@ -15,14 +15,41 @@ export interface AuthResult { user: { id: number; email: string; displayName: string; role: string } } -export async function register(input: RegisterInput, isFirstUser: boolean): Promise { +/** Response for a registration attempt we did not act on (email already exists). + * Deliberately indistinguishable from success so the endpoint cannot be used + * to enumerate registered email addresses. */ +export interface RegistrationPending { + message: string +} + +function toAuthResult(user: userRepo.DbUser): AuthResult { + return { + token: signToken({ + userId: user.id, + email: user.email, + displayName: user.display_name, + role: user.role as 'user' | 'admin', + tv: user.token_version ?? 0, + }), + user: { + id: user.id, + email: user.email, + displayName: user.display_name, + role: user.role, + }, + } +} + +export async function register(input: RegisterInput): Promise { + // Hash before the duplicate check so response timing does not reveal whether + // the email is already registered (user enumeration) + const passwordHash = await bcrypt.hash(input.password, SALT_ROUNDS) + const existing = userRepo.getUserByEmail(input.email) if (existing) { - throw new AppError(400, 'Email already registered') + return { message: 'Check your inbox to finish setting up your account.' } } - const passwordHash = await bcrypt.hash(input.password, SALT_ROUNDS) - const role = isFirstUser ? 'admin' : 'user' const rawVerifyToken = generateVerificationToken() const verifyTokenHash = hashToken(rawVerifyToken) const verifyExpires = Date.now() + VERIFY_TOKEN_EXPIRY_MS @@ -31,7 +58,7 @@ export async function register(input: RegisterInput, isFirstUser: boolean): Prom email: input.email, passwordHash, displayName: input.displayName, - role, + role: 'user', emailVerified: 0, verifyToken: verifyTokenHash, verifyExpires, @@ -42,20 +69,7 @@ export async function register(input: RegisterInput, isFirstUser: boolean): Prom // Don't await — fire and forget so registration isn't blocked by email failures sendVerificationEmail(user.email, rawVerifyToken).catch(() => {}) - return { - token: signToken({ - userId: user.id, - email: user.email, - displayName: user.display_name, - role: user.role as 'user' | 'admin', - }), - user: { - id: user.id, - email: user.email, - displayName: user.display_name, - role: user.role, - }, - } + return toAuthResult(user) } export async function login(input: LoginInput): Promise { @@ -73,20 +87,7 @@ export async function login(input: LoginInput): Promise { throw new AppError(403, 'Please verify your email before logging in') } - return { - token: signToken({ - userId: user.id, - email: user.email, - displayName: user.display_name, - role: user.role as 'user' | 'admin', - }), - user: { - id: user.id, - email: user.email, - displayName: user.display_name, - role: user.role, - }, - } + return toAuthResult(user) } export function getMe(userId: number) { @@ -128,7 +129,11 @@ export async function changePassword(userId: number, input: ChangePasswordInput) } const passwordHash = await bcrypt.hash(input.newPassword, SALT_ROUNDS) - userRepo.updateUser(userId, { passwordHash }) + // Bump token version so tokens issued before the change stop working + userRepo.updateUser(userId, { + passwordHash, + tokenVersion: (user.token_version ?? 0) + 1, + }) } export async function verifyEmail(input: VerifyEmailInput) { @@ -192,10 +197,12 @@ export async function resetPassword(input: ResetPasswordInput) { } const passwordHash = await bcrypt.hash(input.newPassword, SALT_ROUNDS) + // Bump token version so tokens from before the reset stop working userRepo.updateUser(user.id, { passwordHash, resetToken: null, resetExpires: null, + tokenVersion: (user.token_version ?? 0) + 1, }) } diff --git a/backend/src/utils/aiQuota.test.ts b/backend/src/utils/aiQuota.test.ts new file mode 100644 index 0000000..f9a4fe0 --- /dev/null +++ b/backend/src/utils/aiQuota.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect } from 'vitest' +import * as settingsRepo from '../repositories/settingsRepo.js' +import * as aiUsageRepo from '../repositories/aiUsageRepo.js' +import * as userRepo from '../repositories/userRepo.js' +import { enforceDailyAiQuota, getDailyTokenLimit, getUtcDateKey } from './aiQuota.js' +import { AppError } from '../middleware/error.js' + +function seedUser(): number { + const email = `quota-${Math.random().toString(36).slice(2)}@example.com` + return userRepo.createUser({ + email, + passwordHash: 'x', + displayName: 'Quota Test', + emailVerified: 1, + }).id +} + +describe('daily AI token quota', () => { + it('allows everything when no limit is configured', () => { + settingsRepo.setSetting('ai_daily_token_limit', '0') + expect(getDailyTokenLimit()).toBeNull() + expect(() => enforceDailyAiQuota(seedUser())).not.toThrow() + }) + + it('blocks with a 429 once used tokens reach the configured limit', () => { + settingsRepo.setSetting('ai_daily_token_limit', '1000') + const userId = seedUser() + aiUsageRepo.incrementDailyUsage(userId, getUtcDateKey(), 1000, 1) + + expect(() => enforceDailyAiQuota(userId)).toThrowError(AppError) + try { + enforceDailyAiQuota(userId) + } catch (err) { + expect(err).toBeInstanceOf(AppError) + expect((err as AppError).statusCode).toBe(429) + } + }) + + it('allows users under the limit and isolates users from each other', () => { + settingsRepo.setSetting('ai_daily_token_limit', '1000') + const heavy = seedUser() + const fresh = seedUser() + aiUsageRepo.incrementDailyUsage(heavy, getUtcDateKey(), 999, 1) + expect(() => enforceDailyAiQuota(heavy)).not.toThrow() + expect(() => enforceDailyAiQuota(fresh)).not.toThrow() + }) + + it('only counts today’s usage', () => { + settingsRepo.setSetting('ai_daily_token_limit', '1000') + const userId = seedUser() + aiUsageRepo.incrementDailyUsage(userId, '2000-01-01', 999_999, 1) + expect(() => enforceDailyAiQuota(userId)).not.toThrow() + }) +}) diff --git a/backend/src/utils/aiQuota.ts b/backend/src/utils/aiQuota.ts index 77a9a1a..485cf60 100644 --- a/backend/src/utils/aiQuota.ts +++ b/backend/src/utils/aiQuota.ts @@ -1,5 +1,6 @@ import { getSetting } from '../repositories/settingsRepo.js' import { getDailyUsage, incrementDailyUsage } from '../repositories/aiUsageRepo.js' +import { AppError } from '../middleware/error.js' export interface AiQuotaStatus { allowed: boolean @@ -38,3 +39,13 @@ export function checkDailyAiQuota(userId: number): AiQuotaStatus { export function recordDailyAiUsage(userId: number, periodDate: string, tokens: number): void { incrementDailyUsage(userId, periodDate, tokens, 1) } + +/** + * Throws a 429 when the user has exhausted the admin-configured daily token + * limit. Call before every AI provider invocation; pair with recordDailyAiUsage. + */ +export function enforceDailyAiQuota(userId: number): void { + if (!checkDailyAiQuota(userId).allowed) { + throw new AppError(429, 'Daily AI usage limit reached. Please try again tomorrow.') + } +} diff --git a/backend/src/utils/tokenBlacklist.ts b/backend/src/utils/tokenBlacklist.ts index 5e1e1e9..f027552 100644 --- a/backend/src/utils/tokenBlacklist.ts +++ b/backend/src/utils/tokenBlacklist.ts @@ -18,7 +18,8 @@ function initCache(): void { ); CREATE INDEX IF NOT EXISTS idx_token_blacklist_expires ON token_blacklist(expires_at); `) - // Load recent blacklisted tokens into memory + // Purge rows that can never matter again, then load the rest into memory + db.prepare('DELETE FROM token_blacklist WHERE expires_at <= ?').run(Date.now()) const rows = db.prepare('SELECT token_hash FROM token_blacklist WHERE expires_at > ?') .all(Date.now()) as { token_hash: string }[] for (const row of rows) { diff --git a/backend/src/utils/tokens.test.ts b/backend/src/utils/tokens.test.ts index 46278c2..537c2cc 100644 --- a/backend/src/utils/tokens.test.ts +++ b/backend/src/utils/tokens.test.ts @@ -4,13 +4,14 @@ describe('tokens', () => { it('round-trips JWT payload', async () => { process.env.JWT_SECRET = 'test-secret-for-unit-tests-only' const { signToken, verifyToken } = await import('./tokens') - const payload = { userId: 1, email: 'a@b.com', displayName: 'Test', role: 'user' as const } + const payload = { userId: 1, email: 'a@b.com', displayName: 'Test', role: 'user' as const, tv: 3 } const token = signToken(payload) const decoded = verifyToken(token) expect(decoded.userId).toBe(1) expect(decoded.email).toBe('a@b.com') expect(decoded.displayName).toBe('Test') expect(decoded.role).toBe('user') + expect(decoded.tv).toBe(3) }) it('throws on invalid token', async () => { @@ -22,7 +23,7 @@ describe('tokens', () => { it('throws on tampered token', async () => { process.env.JWT_SECRET = 'test-secret-for-unit-tests-only' const { signToken, verifyToken } = await import('./tokens') - const payload = { userId: 1, email: 'a@b.com', displayName: 'Test', role: 'user' as const } + const payload = { userId: 1, email: 'a@b.com', displayName: 'Test', role: 'user' as const, tv: 0 } const token = signToken(payload) const tampered = token.slice(0, -5) + 'xxxxx' expect(() => verifyToken(tampered)).toThrow() diff --git a/backend/src/utils/tokens.ts b/backend/src/utils/tokens.ts index 5298fcf..861642a 100644 --- a/backend/src/utils/tokens.ts +++ b/backend/src/utils/tokens.ts @@ -7,14 +7,19 @@ export interface JwtPayload { email: string displayName: string role: 'user' | 'admin' + /** Token version — must match the user's current token_version or the token is rejected */ + tv: number } export function signToken(payload: JwtPayload): string { return jwt.sign(payload, config.jwtSecret, { expiresIn: config.jwtExpiresIn } as jwt.SignOptions) } -export function verifyToken(token: string): JwtPayload { - return jwt.verify(token, config.jwtSecret) as JwtPayload +/** Verified JWT body: our claims plus the registered exp/iat timestamps. */ +export type VerifiedToken = JwtPayload & { exp?: number; iat?: number } + +export function verifyToken(token: string): VerifiedToken { + return jwt.verify(token, config.jwtSecret) as VerifiedToken } export function generateVerificationToken(): string { diff --git a/backend/src/validators/aiSchemas.test.ts b/backend/src/validators/aiSchemas.test.ts new file mode 100644 index 0000000..171995a --- /dev/null +++ b/backend/src/validators/aiSchemas.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from 'vitest' +import { hintSchema, MAX_HINT_QUESTION_CHARS } from './aiSchemas.js' + +describe('hintSchema', () => { + it('accepts a normal hint request', () => { + const result = hintSchema.safeParse({ + question: { type: 'short-answer', prompt: 'What is 2+2?' }, + language: 'en', + }) + expect(result.success).toBe(true) + }) + + it(`rejects questions whose JSON exceeds ${MAX_HINT_QUESTION_CHARS} chars`, () => { + const result = hintSchema.safeParse({ + question: { prompt: 'x'.repeat(MAX_HINT_QUESTION_CHARS + 1) }, + language: 'en', + }) + expect(result.success).toBe(false) + if (!result.success) { + expect(JSON.stringify(result.error.issues)).toContain('too large') + } + }) +}) diff --git a/backend/src/validators/aiSchemas.ts b/backend/src/validators/aiSchemas.ts new file mode 100644 index 0000000..9bbe201 --- /dev/null +++ b/backend/src/validators/aiSchemas.ts @@ -0,0 +1,25 @@ +import { z } from 'zod' + +export const MAX_HINT_QUESTION_CHARS = 8000 + +function serializedSize(value: unknown): number { + if (value === undefined || value === null) return 0 + return JSON.stringify(value).length +} + +export const hintSchema = z + .object({ + question: z.record(z.unknown()), + language: z.string().min(1).max(20), + }) + .superRefine((val, ctx) => { + if (serializedSize(val.question) > MAX_HINT_QUESTION_CHARS) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Question is too large for AI hints', + path: ['question'], + }) + } + }) + +export type HintRequestBody = z.infer diff --git a/backend/src/validators/gradeSchemas.test.ts b/backend/src/validators/gradeSchemas.test.ts new file mode 100644 index 0000000..6339133 --- /dev/null +++ b/backend/src/validators/gradeSchemas.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect } from 'vitest' +import { + gradeRequestSchema, + MAX_GRADE_QUESTION_CHARS, + MAX_GRADE_ANSWER_CHARS, +} from './gradeSchemas.js' + +const validBase = { + question: { prompt: 'Explain photosynthesis' }, + answer: 'Plants convert light into chemical energy', + language: 'en', +} + +describe('gradeRequestSchema', () => { + it('accepts a well-formed grading request', () => { + const result = gradeRequestSchema.safeParse({ type: 'essay', ...validBase }) + expect(result.success).toBe(true) + }) + + it(`rejects questions whose JSON exceeds ${MAX_GRADE_QUESTION_CHARS} chars`, () => { + const result = gradeRequestSchema.safeParse({ + type: 'essay', + question: { prompt: 'x'.repeat(MAX_GRADE_QUESTION_CHARS + 1) }, + answer: 'irrelevant', + language: 'en', + }) + expect(result.success).toBe(false) + if (!result.success) { + expect(JSON.stringify(result.error.issues)).toContain('too large') + } + }) + + it(`rejects answers whose JSON exceeds ${MAX_GRADE_ANSWER_CHARS} chars`, () => { + const result = gradeRequestSchema.safeParse({ + type: 'essay', + question: { prompt: 'small' }, + answer: 'y'.repeat(MAX_GRADE_ANSWER_CHARS + 1), + language: 'en', + }) + expect(result.success).toBe(false) + if (!result.success) { + expect(JSON.stringify(result.error.issues)).toContain('too large') + } + }) + + it('rejects unknown question types', () => { + const result = gradeRequestSchema.safeParse({ + type: 'flashcard', + question: { prompt: 'hi' }, + answer: 'yo', + language: 'en', + }) + expect(result.success).toBe(false) + }) + + it('bounds the language field', () => { + const result = gradeRequestSchema.safeParse({ + type: 'essay', + question: { prompt: 'hi' }, + answer: 'yo', + language: 'l'.repeat(500), + }) + expect(result.success).toBe(false) + }) +}) diff --git a/backend/src/validators/gradeSchemas.ts b/backend/src/validators/gradeSchemas.ts new file mode 100644 index 0000000..084b454 --- /dev/null +++ b/backend/src/validators/gradeSchemas.ts @@ -0,0 +1,37 @@ +import { z } from 'zod' + +/** Serialized size caps: the grading prompt is built from client-supplied + * question/answer objects, so these bound what can be billed per request. */ +export const MAX_GRADE_QUESTION_CHARS = 8000 +export const MAX_GRADE_ANSWER_CHARS = 4000 + +function serializedSize(value: unknown): number { + if (value === undefined || value === null) return 0 + return JSON.stringify(value).length +} + +const gradeRequestBase = z.object({ + type: z.enum(['essay', 'short-answer', 'translation', 'math', 'ordering', 'match']), + question: z.record(z.unknown()), + answer: z.unknown(), + language: z.string().min(1).max(30), +}) + +export const gradeRequestSchema = gradeRequestBase.superRefine((val, ctx) => { + if (serializedSize(val.question) > MAX_GRADE_QUESTION_CHARS) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Question is too large for AI grading', + path: ['question'], + }) + } + if (serializedSize(val.answer) > MAX_GRADE_ANSWER_CHARS) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Answer is too large for AI grading', + path: ['answer'], + }) + } +}) + +export type GradeRequestBody = z.infer diff --git a/backend/src/validators/resultSchemas.test.ts b/backend/src/validators/resultSchemas.test.ts index 9a9443a..304a883 100644 --- a/backend/src/validators/resultSchemas.test.ts +++ b/backend/src/validators/resultSchemas.test.ts @@ -68,7 +68,7 @@ describe('submitResultSchema', () => { expect(result.success).toBe(true) }) - it('accepts negative score_points because schema allows any number', () => { + it('rejects negative score_points (client-supplied scores are bounded)', () => { const result = submitResultSchema.safeParse({ title: 'My Exam', score_points: -1, @@ -86,7 +86,25 @@ describe('submitResultSchema', () => { }, exam: { title: 'My Exam', questions: [] }, }) - expect(result.success).toBe(true) + expect(result.success).toBe(false) + }) + + it('rejects non-integer counts and absurd magnitudes', () => { + const base = { + title: 'My Exam', + score_points: 8, + total_points: 10, + correct_count: 8, + graded_count: 10, + elapsed_ms: 120000, + state: { currentIdx: 0, answers: {}, flags: {}, notes: '', startedAt: Date.now(), feedbackMode: 'end' }, + exam: { title: 'My Exam', questions: [] }, + } + expect(submitResultSchema.safeParse({ ...base, correct_count: 1.5 }).success).toBe(false) + expect(submitResultSchema.safeParse({ ...base, score_points: 1e12 }).success).toBe(false) + expect(submitResultSchema.safeParse({ ...base, elapsed_ms: -5 }).success).toBe(false) + expect(submitResultSchema.safeParse({ ...base, exam_id: -3 }).success).toBe(false) + expect(submitResultSchema.safeParse({ ...base, title: 't'.repeat(301) }).success).toBe(false) }) it('accepts immediate feedback mode', () => { diff --git a/backend/src/validators/resultSchemas.ts b/backend/src/validators/resultSchemas.ts index 8760a6c..0c258fe 100644 --- a/backend/src/validators/resultSchemas.ts +++ b/backend/src/validators/resultSchemas.ts @@ -1,14 +1,16 @@ import { z } from 'zod' +// Scores/counts arrive from the client, so they are bounded to keep forged +// garbage out of analytics while accepting every value honest clients send. export const submitResultSchema = z.object({ - exam_id: z.number().nullable().optional(), - title: z.string().min(1), - score_points: z.number(), - total_points: z.number(), - correct_count: z.number(), - graded_count: z.number(), - passing_score: z.number().optional(), - elapsed_ms: z.number(), + exam_id: z.number().int().positive().nullable().optional(), + title: z.string().min(1).max(300), + score_points: z.number().finite().min(0).max(1e9), + total_points: z.number().finite().min(0).max(1e9), + correct_count: z.number().int().min(0).max(1e6), + graded_count: z.number().int().min(0).max(1e6), + passing_score: z.number().finite().min(0).max(1e9).optional(), + elapsed_ms: z.number().int().min(0).max(2_147_483_647), state: z.record(z.unknown()), exam: z.record(z.unknown()), }) diff --git a/docs/superpowers/plans/2026-08-21-security-audit-fixes.md b/docs/superpowers/plans/2026-08-21-security-audit-fixes.md new file mode 100644 index 0000000..2abcb1f --- /dev/null +++ b/docs/superpowers/plans/2026-08-21-security-audit-fixes.md @@ -0,0 +1,104 @@ +# 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. diff --git a/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json b/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json index ce9dc33..9345b26 100644 --- a/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json +++ b/node_modules/.vite/vitest/da39a3ee5e6b4b0d3255bfef95601890afd80709/results.json @@ -1 +1 @@ -{"version":"4.1.5","results":[[":frontend/src/utils/grading.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/resultSchemas.test.js",{"duration":0,"failed":true}],[":backend/src/validators/authSchemas.test.ts",{"duration":0,"failed":true}],[":backend/src/validators/resultSchemas.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/authSchemas.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/storage.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/examSchemas.test.js",{"duration":0,"failed":true}],[":backend/dist/config.test.js",{"duration":0,"failed":true}],[":backend/src/validators/adminSchemas.test.ts",{"duration":0,"failed":true}],[":frontend/src/utils/latex.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/adminSchemas.test.js",{"duration":0,"failed":true}],[":backend/src/config.test.ts",{"duration":0,"failed":true}],[":backend/src/validators/examSchemas.test.ts",{"duration":0,"failed":true}],[":backend/dist/utils/tokens.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/validation.test.ts",{"duration":0,"failed":true}],[":backend/src/utils/tokens.test.ts",{"duration":0,"failed":true}],[":frontend/src/utils/slug.test.ts",{"duration":0,"failed":true}],[":backend/src/utils/asyncHandler.test.ts",{"duration":0,"failed":true}],[":backend/src/validators/setupSchemas.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/setupSchemas.test.js",{"duration":0,"failed":true}],[":backend/dist/middleware/error.test.js",{"duration":0,"failed":true}],[":backend/dist/utils/asyncHandler.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/formatting.test.ts",{"duration":0,"failed":true}],[":backend/src/middleware/error.test.ts",{"duration":0,"failed":true}],[":frontend/src/utils/api.test.ts",{"duration":0,"failed":true}],[":backend/src/repositories/storageRepo.test.ts",{"duration":0,"failed":true}],[":backend/dist/repositories/storageRepo.test.js",{"duration":0,"failed":true}]]} \ No newline at end of file +{"version":"4.1.5","results":[[":frontend/src/utils/grading.test.ts",{"duration":9.265338999999983,"failed":false}],[":backend/dist/validators/resultSchemas.test.js",{"duration":0,"failed":true}],[":backend/src/validators/authSchemas.test.ts",{"duration":22.575676000000044,"failed":false}],[":backend/src/validators/resultSchemas.test.ts",{"duration":9.748771000000033,"failed":false}],[":backend/dist/validators/authSchemas.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/storage.test.ts",{"duration":6.310062000000016,"failed":false}],[":backend/dist/validators/examSchemas.test.js",{"duration":0,"failed":true}],[":backend/dist/config.test.js",{"duration":0,"failed":true}],[":backend/src/validators/adminSchemas.test.ts",{"duration":33.639384000000064,"failed":false}],[":frontend/src/utils/latex.test.ts",{"duration":0,"failed":true}],[":backend/dist/validators/adminSchemas.test.js",{"duration":0,"failed":true}],[":backend/src/config.test.ts",{"duration":14.69092599999999,"failed":false}],[":backend/src/validators/examSchemas.test.ts",{"duration":33.61004700000001,"failed":false}],[":backend/dist/utils/tokens.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/validation.test.ts",{"duration":11.680594999999983,"failed":false}],[":backend/src/utils/tokens.test.ts",{"duration":44.34940499999999,"failed":false}],[":frontend/src/utils/slug.test.ts",{"duration":5.085611999999969,"failed":false}],[":backend/src/utils/asyncHandler.test.ts",{"duration":5.737424999999973,"failed":false}],[":backend/src/validators/setupSchemas.test.ts",{"duration":28.528040000000033,"failed":false}],[":backend/dist/validators/setupSchemas.test.js",{"duration":0,"failed":true}],[":backend/dist/middleware/error.test.js",{"duration":0,"failed":true}],[":backend/dist/utils/asyncHandler.test.js",{"duration":0,"failed":true}],[":frontend/src/utils/formatting.test.ts",{"duration":4.583577999999989,"failed":false}],[":backend/src/middleware/error.test.ts",{"duration":8.906761999999958,"failed":false}],[":frontend/src/utils/api.test.ts",{"duration":9.23972999999998,"failed":false}],[":backend/src/repositories/storageRepo.test.ts",{"duration":23.65549999999996,"failed":false}],[":backend/dist/repositories/storageRepo.test.js",{"duration":0,"failed":true}],[":backend/src/validators/gradeSchemas.test.ts",{"duration":16.57176800000002,"failed":false}],[":backend/src/utils/aiQuota.test.ts",{"duration":0,"failed":true}],[":backend/src/validators/collectionSchemas.test.ts",{"duration":15.043968000000007,"failed":false}],[":backend/src/validators/aiSchemas.test.ts",{"duration":14.30532999999997,"failed":false}],[":backend/src/services/authService.test.ts",{"duration":0,"failed":true}],[":backend/src/middleware/auth.test.ts",{"duration":0,"failed":true}]]} \ No newline at end of file