From d28bca5cf07eafd8532df3ba596f6cc90880de37 Mon Sep 17 00:00:00 2001 From: Thomas Joise Date: Fri, 17 Jul 2026 12:38:14 +0530 Subject: [PATCH] fix(settings): validate whole reminders PUT body before writing any setting Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/hq/src/api.ts | 8 +++++- apps/hq/test/settings-reminders.test.ts | 37 ++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/apps/hq/src/api.ts b/apps/hq/src/api.ts index 9de75c5..bd687a1 100644 --- a/apps/hq/src/api.ts +++ b/apps/hq/src/api.ts @@ -993,11 +993,17 @@ export function apiRouter( r.put('/settings/reminders', requireAuth, requireOwner, (req, res) => { const body = req.body as { overdueDays?: unknown; renewalDays?: unknown } try { + // First pass: validate all fields, collect tuples + const toSet: Array<[string, string]> = [] for (const [field, key] of [['overdueDays', 'reminders.overdue_days'], ['renewalDays', 'reminders.renewal_days']] as const) { const v = body[field] if (v === undefined) continue if (typeof v !== 'number' || !Number.isInteger(v) || v < 1) throw new Error(`${field} must be a positive integer`) - setSetting(db, staffId(res), key, String(v)) + toSet.push([key, String(v)]) + } + // Second pass: write all validated settings + for (const [key, value] of toSet) { + setSetting(db, staffId(res), key, value) } res.json({ ok: true }) } catch (err) { diff --git a/apps/hq/test/settings-reminders.test.ts b/apps/hq/test/settings-reminders.test.ts index dc8e3c1..5673622 100644 --- a/apps/hq/test/settings-reminders.test.ts +++ b/apps/hq/test/settings-reminders.test.ts @@ -1,7 +1,10 @@ // apps/hq/test/settings-reminders.test.ts — Task 3: reminder settings + dated schedule inserts -import { describe, expect, it } from 'vitest' +import { describe, expect, it, afterAll } from 'vitest' +import express from 'express' import { openDb } from '../src/db' import { seedIfEmpty } from '../src/seed' +import { createStaff } from '../src/auth' +import { apiRouter } from '../src/api' import { insertSchedule, listSchedules, resolveSchedule } from '../src/repos-reminders' const fresh = () => { const db = openDb(':memory:'); seedIfEmpty(db); return db } @@ -34,3 +37,35 @@ describe('schedule settings', () => { expect(audit).toBeDefined() }) }) + +describe('settings reminders routes', () => { + const appWith = () => { + const db = openDb(':memory:'); seedIfEmpty(db) + createStaff(db, { email: 'owner@test.in', displayName: 'Owner', role: 'owner', password: 'owner-password' }) + const app = express(); app.use(express.json()); app.locals['db'] = db; app.use('/api', apiRouter(db)) + const server = app.listen(0) + const base = `http://localhost:${(server.address() as { port: number }).port}/api` + return { db, server, base } + } + const { db, server, base } = appWith() + afterAll(() => server.close()) + const login = async (email: string, password: string) => + (await (await fetch(`${base}/auth/login`, { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ email, password }) })).json() as any).token + const call = async (token: string, method: string, path: string, body?: unknown) => { + const res = await fetch(base + path, { method, headers: { 'content-type': 'application/json', authorization: `Bearer ${token}` }, ...(body ? { body: JSON.stringify(body) } : {}) }) + return { status: res.status, json: await res.json() as any } + } + + it('validates whole body before writing any setting', async () => { + const token = await login('owner@test.in', 'owner-password') + // Get initial state + const initial = await call(token, 'GET', '/settings/reminders') + const initialOverdue = initial.json.overdueDays + // Attempt to update with mixed valid/invalid fields — should fail + const mixed = await call(token, 'PUT', '/settings/reminders', { overdueDays: 5, renewalDays: -1 }) + expect(mixed.status).toBe(400) + // Verify overdueDays was NOT persisted (no partial write) + const after = await call(token, 'GET', '/settings/reminders') + expect(after.json.overdueDays).toBe(initialOverdue) + }) +})