fix(api): validate ?days= and guard the onboarding-project routes against a crash (FIX B)

GET /projects/stalled had no try/catch, and a caller-supplied ?days= flowed
straight into `new Date(...).toISOString()`. Confirmed the exact crash:
Number('1e300') passes Number.isFinite (so the old guard let it through),
but the resulting cutoff math overflows the valid Date range and
.toISOString() throws RangeError. Uncaught in an async Express handler this
is an unhandled rejection, which kills the whole process on Node 22 -- any
authenticated role, including plain staff, could take the console down with
one GET.

`days` is now validated (integer, 0-3650) with an explicit 4xx on anything
hostile, falling back to the configured default only when the param is
absent (matches the frontend, which never sends ?days= at all). Audited
every other route this feature added to the same section: /projects/board
and /projects/pending/:key had the identical no-try/catch shape and are now
wrapped too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main
Thomas Joise 1 day ago
parent 31335cd4ad
commit 04aef0aa36

@ -782,18 +782,43 @@ export function apiRouter(
})
// The cross-project board + drill-down (the "who's pending go-live" view).
r.get('/projects/board', requireAuth, async (_req, res) => {
res.json({ ok: true, board: await milestoneBoard(db) })
try {
res.json({ ok: true, board: await milestoneBoard(db) })
} catch (err) {
res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
}
})
r.get('/projects/pending/:key', requireAuth, async (req, res) => {
res.json({ ok: true, projects: await projectsPendingMilestone(db, String(req.params['key'] ?? '')) })
try {
res.json({ ok: true, projects: await projectsPendingMilestone(db, String(req.params['key'] ?? '')) })
} catch (err) {
res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
}
})
// Parked onboarding: active projects whose next step hasn't moved in a while (dashboard/MIS tile).
// FIX B (pre-ship audit): a caller-supplied `days` used to flow straight into `new Date(...)`
// math with no bound and no try/catch — an out-of-range/NaN value threw a RangeError that,
// as an unhandled rejection, crashed the whole process. `days` is now validated (finite
// integer, sane bounds) and the handler is wrapped like its siblings.
r.get('/projects/stalled', requireAuth, async (req, res) => {
const q = req.query['days']
const days = typeof q === 'string' && q !== '' && Number.isFinite(Number(q))
? Number(q) : await getNumberSetting(db, 'project.stall_days', 30)
const today = new Date().toISOString().slice(0, 10)
res.json({ ok: true, projects: await stalledProjects(db, days, today) })
try {
const q = req.query['days']
let days: number
if (typeof q === 'string' && q !== '') {
const parsed = Number(q)
if (!Number.isInteger(parsed) || parsed < 0 || parsed > 3650) {
res.status(400).json({ ok: false, error: 'days must be an integer between 0 and 3650' })
return
}
days = parsed
} else {
days = await getNumberSetting(db, 'project.stall_days', 30)
}
const today = new Date().toISOString().slice(0, 10)
res.json({ ok: true, projects: await stalledProjects(db, days, today) })
} catch (err) {
res.status(400).json({ ok: false, error: err instanceof Error ? err.message : String(err) })
}
})
// The shared onboarding "front" — same lead-in steps for every module's checklist (owner editor).

@ -0,0 +1,75 @@
// apps/hq/test/projects-stalled-route.test.ts — pre-ship audit FIX B: GET /projects/stalled
// had no try/catch, and a caller-supplied `?days=` flowed straight into
// `new Date(...).toISOString()`, which throws a RangeError on an out-of-range/NaN value.
// On Express 4 + Node 22 an unhandled rejection from an async route handler crashes the
// whole process — ANY authenticated role (including plain staff) could take the console
// down with one GET. `days` is now validated and the handler wrapped like its siblings.
import express from 'express'
import { describe, it, expect, afterAll } from 'vitest'
import { openDb } from '../src/db'
import { seedIfEmpty } from '../src/seed'
import { apiRouter } from '../src/api'
import { createStaff } from '../src/auth'
describe('GET /projects/stalled — hostile ?days= must not crash the server', () => {
const servers: { close: () => void }[] = []
afterAll(() => { for (const s of servers) s.close() })
async function httpWorld(): Promise<{ base: string; staffH: Record<string, string> }> {
const db = openDb(':memory:')
await seedIfEmpty(db)
await createStaff(db, { email: 'staff-stalled@t.in', displayName: 'Staff', role: 'staff', password: 'staff-pass-1' })
const app = express(); app.use(express.json()); app.locals['db'] = db
app.use('/api', apiRouter(db))
const server = app.listen(0); servers.push(server)
const base = `http://localhost:${(server.address() as { port: number }).port}/api`
const token = ((await (await fetch(`${base}/auth/login`, {
method: 'POST', headers: { 'content-type': 'application/json' },
body: JSON.stringify({ email: 'staff-stalled@t.in', password: 'staff-pass-1' }),
})).json()) as { token: string }).token
return { base, staffH: { authorization: `Bearer ${token}` } }
}
it('a plain request (no ?days=) works — falls back to the configured default', async () => {
const { base, staffH } = await httpWorld()
const res = await fetch(`${base}/projects/stalled`, { headers: staffH })
expect(res.status).toBe(200)
const body = await res.json() as { ok: boolean; projects: unknown[] }
expect(body.ok).toBe(true)
expect(Array.isArray(body.projects)).toBe(true)
})
it('a sane ?days= still works normally', async () => {
const { base, staffH } = await httpWorld()
const res = await fetch(`${base}/projects/stalled?days=45`, { headers: staffH })
expect(res.status).toBe(200)
expect((await res.json() as { ok: boolean }).ok).toBe(true)
})
const hostileValues = [
'1e300', // finite in JS but overflows Date math -> used to throw RangeError
'-99999999999999', // huge negative -> overflows the other direction
'3.5', // not an integer
'abc', // NaN
'NaN',
'Infinity',
'-Infinity',
'99999999999999999999', // way outside the sane 0-3650 bound
'-1', // out of bound (negative)
]
for (const hostile of hostileValues) {
it(`?days=${hostile} is rejected with a 4xx, never throws / crashes the server`, async () => {
const { base, staffH } = await httpWorld()
const res = await fetch(`${base}/projects/stalled?days=${encodeURIComponent(hostile)}`, { headers: staffH })
expect(res.status).toBeGreaterThanOrEqual(400)
expect(res.status).toBeLessThan(500)
const body = await res.json() as { ok: boolean }
expect(body.ok).toBe(false)
// The server process must still be alive for the NEXT request — the original bug's
// unhandled RangeError rejection killed the whole process.
const health = await fetch(`${base}/health`)
expect(health.status).toBe(200)
})
}
})
Loading…
Cancel
Save