fix(settings): validate whole reminders PUT body before writing any setting

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 5 days ago
parent 36fcf795a8
commit d28bca5cf0

@ -993,11 +993,17 @@ export function apiRouter(
r.put('/settings/reminders', requireAuth, requireOwner, (req, res) => { r.put('/settings/reminders', requireAuth, requireOwner, (req, res) => {
const body = req.body as { overdueDays?: unknown; renewalDays?: unknown } const body = req.body as { overdueDays?: unknown; renewalDays?: unknown }
try { 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) { for (const [field, key] of [['overdueDays', 'reminders.overdue_days'], ['renewalDays', 'reminders.renewal_days']] as const) {
const v = body[field] const v = body[field]
if (v === undefined) continue if (v === undefined) continue
if (typeof v !== 'number' || !Number.isInteger(v) || v < 1) throw new Error(`${field} must be a positive integer`) 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 }) res.json({ ok: true })
} catch (err) { } catch (err) {

@ -1,7 +1,10 @@
// apps/hq/test/settings-reminders.test.ts — Task 3: reminder settings + dated schedule inserts // 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 { openDb } from '../src/db'
import { seedIfEmpty } from '../src/seed' import { seedIfEmpty } from '../src/seed'
import { createStaff } from '../src/auth'
import { apiRouter } from '../src/api'
import { insertSchedule, listSchedules, resolveSchedule } from '../src/repos-reminders' import { insertSchedule, listSchedules, resolveSchedule } from '../src/repos-reminders'
const fresh = () => { const db = openDb(':memory:'); seedIfEmpty(db); return db } const fresh = () => { const db = openDb(':memory:'); seedIfEmpty(db); return db }
@ -34,3 +37,35 @@ describe('schedule settings', () => {
expect(audit).toBeDefined() 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)
})
})

Loading…
Cancel
Save