MEDIUM: Admin routes use req.params.id without integer validation #79

New Issue

Closed

opened 2026-05-31 12:03:48 -05:00 by null · 1 comment

null commented

2026-05-31 12:03:48 -05:00

Owner

Bug Description

Several admin routes use Number(req.params.id) without parseInt + Number.isInteger() validation:

DELETE /api/admin/users/:id
PUT /api/admin/users/:id/password
PUT /api/admin/users/:id/role
PUT /api/admin/users/:id/active
PUT /api/admin/users/:id/username

SQLite is permissive with type coercion - Number(abc) becomes NaN which SQLite treats as NULL, causing these routes to silently fail to find the target user (returning 404) rather than returning a proper 400 validation error.

Compare with routes/bills.js which properly validates: parseInt(req.params.id, 10) + Number.isInteger() checks.

Impact

Confusing error responses (404 instead of 400) for invalid IDs. Could mask bugs in admin UI code.

Recommended Fix

Add parseInt + Number.isInteger validation for all admin :id parameters, consistent with the pattern used in bills routes.

## Bug Description Several admin routes use Number(req.params.id) without parseInt + Number.isInteger() validation: - DELETE /api/admin/users/:id - PUT /api/admin/users/:id/password - PUT /api/admin/users/:id/role - PUT /api/admin/users/:id/active - PUT /api/admin/users/:id/username SQLite is permissive with type coercion - Number(abc) becomes NaN which SQLite treats as NULL, causing these routes to silently fail to find the target user (returning 404) rather than returning a proper 400 validation error. Compare with routes/bills.js which properly validates: parseInt(req.params.id, 10) + Number.isInteger() checks. ## Impact Confusing error responses (404 instead of 400) for invalid IDs. Could mask bugs in admin UI code. ## Recommended Fix Add parseInt + Number.isInteger validation for all admin :id parameters, consistent with the pattern used in bills routes.

null added the

priority:medium

backend

bug

labels 2026-05-31 12:03:48 -05:00

null commented

2026-06-03 20:00:05 -05:00

Author

Owner

Added at the top of admin.js:

function parseUserId(params) {
const n = parseInt(params.id, 10);
return Number.isInteger(n) && n > 0 ? n : null;
}
Applied to all 5 user routes:

Route Before After
PUT /users/:id/password raw req.params.id in queries parseUserId + early 400
PUT /users/:id/role Number(req.params.id) parseUserId + early 400
PUT /users/:id/active Number(req.params.id) parseUserId + early 400
PUT /users/:id/username Number(req.params.id) parseUserId + early 400
DELETE /users/:id raw req.params.id in query parseUserId + early 400
Backup routes (GET/POST/DELETE /backups/:id) left unchanged — those params are filenames, not integers.

Added at the top of admin.js: function parseUserId(params) { const n = parseInt(params.id, 10); return Number.isInteger(n) && n > 0 ? n : null; } Applied to all 5 user routes: Route Before After PUT /users/:id/password raw req.params.id in queries parseUserId + early 400 PUT /users/:id/role Number(req.params.id) parseUserId + early 400 PUT /users/:id/active Number(req.params.id) parseUserId + early 400 PUT /users/:id/username Number(req.params.id) parseUserId + early 400 DELETE /users/:id raw req.params.id in query parseUserId + early 400 Backup routes (GET/POST/DELETE /backups/:id) left unchanged — those params are filenames, not integers.

null closed this issue

2026-06-03 20:00:05 -05:00