vinzenz/wutzcalc
Watch
0
0
Fork
You've already forked wutzcalc
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

admin: close six 500-instead-of-4xx gaps on drink/bar CRUD endpoints

Browse source 
Six defensive-check gaps in server/src/routes/admin.ts, all the same
theme (missing the same check a sibling endpoint already has):

1. PATCH /admin/api/drinks/:id with a taken name 500'd instead of 409 —
   the bar-rename equivalent (and drink POST) already catch the UNIQUE
   constraint, this endpoint didn't.
2. PATCH /admin/api/bars/:id with a nonexistent drink_id 500'd instead of
   400 — drink_ids was only checked for integer-ness, not existence, so a
   bogus id hit an uncaught FK-constraint error. Now validated against the
   drinks table up front (rollback was already correct, this only fixes
   the status code).
3. PATCH /admin/api/bars/:id on a nonexistent bar silently returned
   200 {ok:true} — the drinks-PATCH equivalent checks info.changes === 0
   and 404s, this endpoint checked nothing. Now 404s up front.
4. Non-string name in any of the four drink/bar POST/PATCH endpoints
   500'd (object/numeric name hit an uncaught TypeError calling .trim()
   on a non-string, or an uncaught SQLite type error). All four now
   typeof-guard before use.
5. Drink names weren't trimmed or checked for emptiness, unlike bars —
   an empty/whitespace name showed as a blank tablet tile, and a
   trailing-space variant of an existing name bypassed the UNIQUE index
   and split that drink's stats across two rows. Drink POST/PATCH now
   trim + reject blank, matching bars.

Verified all six against a scratch DB with the real server running
end-to-end (login, each failure case, plus a same-request-shape sanity
check that valid updates still succeed). tsc --noEmit and the server
build both clean.
This commit is contained in:
iris 2026-07-31 11:07:49 +02:00
commit d6e9979439
1 changed files with 68 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

83
server/src/routes/admin.ts
View file

@ -322,7 +322,12 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
'/admin/api/drinks',
async (req, reply) => {
if (!requireAuth(req, reply)) return;
const { name, price_cents } = req.body ?? ({} as any);
const { name: rawName, price_cents } = req.body ?? ({} as any);
// Trim + reject non-strings/blank, same as bars already do — an
// untrimmed or empty name shows as a blank tile on the tablet grid,
// and a trailing-space variant of an existing name bypasses the
// UNIQUE index and splits that drink's stats across two rows.
const name = typeof rawName === 'string' ? rawName.trim() : '';
if (!name || !isValidCents(price_cents)) return sendProblem(req, reply, 400, 'Bad Request', 'invalid');
try {
const info = db
@ -344,10 +349,18 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
if (!requireAuth(req, reply)) return;
const id = parseId(req.params.id, req, reply);
if (id === null) return;
const { name, price_cents, archived } = req.body ?? {};
const { name: rawName, price_cents, archived } = req.body ?? {};
if (price_cents !== undefined && !isValidCents(price_cents)) {
return sendProblem(req, reply, 400, 'Bad Request', 'invalid price_cents');
}
let name: string | undefined;
if (rawName !== undefined) {
if (typeof rawName !== 'string') {
return sendProblem(req, reply, 400, 'Bad Request', 'invalid name');
}
name = rawName.trim();
if (!name) return sendProblem(req, reply, 400, 'Bad Request', 'name empty');
}
const sets: string[] = [];
const args: any[] = [];
if (name !== undefined) { sets.push('name = ?'); args.push(name); }
@ -355,9 +368,18 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
if (archived !== undefined) { sets.push('archived = ?'); args.push(archived ? 1 : 0); }
if (sets.length === 0) return { ok: true };
args.push(id);
const info = db.prepare(`UPDATE drinks SET ${sets.join(', ')} WHERE id = ?`).run(...args);
if (info.changes === 0) return sendProblem(req, reply, 404, 'Not Found', 'drink not found');
return { ok: true };
try {
const info = db.prepare(`UPDATE drinks SET ${sets.join(', ')} WHERE id = ?`).run(...args);
if (info.changes === 0) return sendProblem(req, reply, 404, 'Not Found', 'drink not found');
return { ok: true };
} catch (e: any) {
// Matches the POST /admin/api/drinks UNIQUE-name handling below —
// this endpoint was the one place that gap hadn't been closed yet.
if (String(e.message).includes('UNIQUE') && String(e.message).includes('drinks.name')) {
return sendProblem(req, reply, 409, 'Conflict', 'name already exists');
}
throw e;
}
}
);
@ -402,7 +424,14 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
'/admin/api/bars',
async (req, reply) => {
if (!requireAuth(req, reply)) return;
const name = req.body?.name?.trim();
const rawName = req.body?.name;
// typeof-guard before .trim() — a non-string name (number/object)
// used to reach `.trim()` and throw an uncaught TypeError, surfacing
// as a raw 500 instead of a 400.
if (typeof rawName !== 'string') {
return sendProblem(req, reply, 400, 'Bad Request', 'name required');
}
const name = rawName.trim();
const pfand_cents = req.body?.pfand_cents ?? 0;
if (!name) return sendProblem(req, reply, 400, 'Bad Request', 'name required');
if (!isValidCents(pfand_cents)) {
@ -428,7 +457,22 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
if (!requireAuth(req, reply)) return;
const id = parseId(req.params.id, req, reply);
if (id === null) return;
const { name, pfand_cents, drink_ids } = req.body ?? {};
// The drinks-PATCH equivalent checks info.changes === 0 and 404s; a
// PATCH to this multi-statement transaction had no such check at all,
// so a stale admin tab "fixing" a Pfand rate on an already-deleted bar
// got a silent 200 {ok:true} with nothing actually changed.
const bar = db.prepare('SELECT 1 FROM bars WHERE id = ?').get(id);
if (!bar) return sendProblem(req, reply, 404, 'Not Found', 'bar not found');
const { name: rawName, pfand_cents, drink_ids } = req.body ?? {};
let name: string | undefined;
if (rawName !== undefined) {
if (typeof rawName !== 'string') {
return sendProblem(req, reply, 400, 'Bad Request', 'invalid name');
}
name = rawName.trim();
if (!name) return sendProblem(req, reply, 400, 'Bad Request', 'name empty');
}
if (pfand_cents !== undefined && !isValidCents(pfand_cents)) {
return sendProblem(req, reply, 400, 'Bad Request', 'invalid pfand_cents');
}
@ -436,13 +480,25 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
if (!Array.isArray(drink_ids) || !drink_ids.every(did => Number.isInteger(did))) {
return sendProblem(req, reply, 400, 'Bad Request', 'invalid drink_ids');
}
// drink_ids was only checked for integer-ness, not existence — a
// bogus id threw an uncaught FK-constraint error inside the
// transaction below (rollback held correctly, but surfaced as a
// raw 500 instead of a 400).
const uniqueIds = [...new Set(drink_ids)];
if (uniqueIds.length > 0) {
const placeholders = uniqueIds.map(() => '?').join(',');
const found = db
.prepare(`SELECT id FROM drinks WHERE id IN (${placeholders})`)
.all(...uniqueIds) as { id: number }[];
if (found.length !== uniqueIds.length) {
return sendProblem(req, reply, 400, 'Bad Request', 'unknown drink_id');
}
}
}
try {
db.transaction(() => {
if (name !== undefined) {
const trimmed = name.trim();
if (!trimmed) throw new Error('name empty');
db.prepare('UPDATE bars SET name = ? WHERE id = ?').run(trimmed, id);
db.prepare('UPDATE bars SET name = ? WHERE id = ?').run(name, id);
}
if (pfand_cents !== undefined) {
db.prepare('UPDATE bars SET pfand_cents = ? WHERE id = ?').run(pfand_cents, id);
@ -453,17 +509,14 @@ export function registerAdminRoutes(app: FastifyInstance, db: DB) {
'INSERT INTO bar_drinks (bar_id, drink_id, sort_order) VALUES (?, ?, ?)'
);
// Dedupe: a repeated id would otherwise trip bar_drinks' own
// (bar_id, drink_id) primary key and get mis-reported below as
// "name already exists" via the UNIQUE substring match.
// (bar_id, drink_id) primary key.
[...new Set(drink_ids)].forEach((did, idx) => ins.run(id, did, idx));
}
})();
} catch (e: any) {
const msg = String(e.message);
if (msg.includes('UNIQUE') && msg.includes('bars.name')) {
if (String(e.message).includes('UNIQUE') && String(e.message).includes('bars.name')) {
return sendProblem(req, reply, 409, 'Conflict', 'name already exists');
}
if (msg === 'name empty') return sendProblem(req, reply, 400, 'Bad Request', 'name empty');
throw e;
}
return { ok: true };