refactor(backend): hoist parseIntParam helper to utils (#798)

Adds backend/src/utils/parseIntParam.ts with a shared parseIntParam
helper that writes a 400 'Invalid <label>' response and returns null on
non-numeric route params. Consolidates the parseInt + isNaN + 400 shape
that was inlined or duplicated across multiple routers.

Updated:
- routes/fleet.ts: replaced the local parseIdParam wrapper.
- routes/autoHeal.ts: replaced the local parsePolicyId wrapper.
- routes/notifications.ts: replaced parseRouteId wrapper plus an inline
  notification-id site.
- routes/apiTokens.ts, routes/labels.ts, routes/registries.ts,
  routes/scheduledTasks.ts, routes/users.ts: replaced inline copies.

Out of scope (route handlers without an existing isNaN check, kept
intentionally untouched to avoid introducing new 400 responses): alerts,
nodes, webhooks, and several user-routes handlers that rely on a
downstream 404 instead.

Closes #748
This commit is contained in:
Anso
2026-04-27 00:39:28 -04:00
committed by GitHub
parent add3abaece
commit 4c352c74c8
9 changed files with 70 additions and 84 deletions
+3 -2
View File
@@ -6,6 +6,7 @@ import { authMiddleware } from '../middleware/auth';
import { requireAdmin, requireAdmiral } from '../middleware/tierGates';
import { rejectApiTokenScope } from '../middleware/apiTokenScope';
import { isDebugEnabled } from '../utils/debug';
import { parseIntParam } from '../utils/parseIntParam';
// JWT ceiling exceeds the longest user-selectable expiry (365d) so the DB
// check (expires_at) is always the tighter bound.
@@ -109,8 +110,8 @@ apiTokensRouter.delete('/:id', authMiddleware, async (req: Request, res: Respons
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid token ID.' }); return; }
const id = parseIntParam(req, res, 'id', 'token ID');
if (id === null) return;
const apiToken = DatabaseService.getInstance().getApiTokenById(id);
if (!apiToken) { res.status(404).json({ error: 'API token not found.' }); return; }
+4 -12
View File
@@ -4,6 +4,7 @@ import { DatabaseService } from '../services/DatabaseService';
import { authMiddleware } from '../middleware/auth';
import { requirePaid, requireAdmin } from '../middleware/tierGates';
import { getErrorMessage } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
const AutoHealPolicyCreateSchema = z.object({
stack_name: z.string().min(1).max(255),
@@ -15,15 +16,6 @@ const AutoHealPolicyCreateSchema = z.object({
});
const AutoHealPolicyUpdateSchema = AutoHealPolicyCreateSchema.partial().omit({ stack_name: true });
function parsePolicyId(req: Request, res: Response): number | null {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) {
res.status(400).json({ error: 'Invalid id' });
return null;
}
return id;
}
export const autoHealRouter = Router();
autoHealRouter.get('/policies', authMiddleware, (req: Request, res: Response): void => {
@@ -71,7 +63,7 @@ autoHealRouter.post('/policies', authMiddleware, (req: Request, res: Response):
autoHealRouter.patch('/policies/:id', authMiddleware, (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
const id = parsePolicyId(req, res);
const id = parseIntParam(req, res, 'id');
if (id === null) return;
const parsed = AutoHealPolicyUpdateSchema.safeParse(req.body);
if (!parsed.success) {
@@ -92,7 +84,7 @@ autoHealRouter.patch('/policies/:id', authMiddleware, (req: Request, res: Respon
autoHealRouter.delete('/policies/:id', authMiddleware, (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
const id = parsePolicyId(req, res);
const id = parseIntParam(req, res, 'id');
if (id === null) return;
try {
const db = DatabaseService.getInstance();
@@ -107,7 +99,7 @@ autoHealRouter.delete('/policies/:id', authMiddleware, (req: Request, res: Respo
autoHealRouter.get('/policies/:id/history', authMiddleware, (req: Request, res: Response): void => {
if (!requirePaid(req, res)) return;
const id = parsePolicyId(req, res);
const id = parseIntParam(req, res, 'id');
if (id === null) return;
const limit = Math.min(parseInt(String(req.query.limit ?? '50'), 10) || 50, 100);
try {
+8 -23
View File
@@ -20,6 +20,7 @@ import { getLatestVersion } from '../utils/version-check';
import { isValidStackName } from '../utils/validation';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
import { POLICY_SEVERITIES } from '../utils/severity';
import { CloudBackupService } from '../services/CloudBackupService';
import { NotificationService } from '../services/NotificationService';
@@ -28,22 +29,6 @@ import { LicenseService } from '../services/LicenseService';
const updateTracker = FleetUpdateTrackerService.getInstance();
const UPDATE_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
/**
* Parse a numeric route param. Writes a 400 response and returns null when
* the value isn't a valid integer; callers early-return on null. Collapses
* the 7 copies of the `parseInt ... isNaN ... 400 'Invalid X ID'` shape
* across the fleet router.
*/
function parseIdParam(req: Request, res: Response, paramName: string, label: string): number | null {
const raw = req.params[paramName] as string | undefined;
const parsed = parseInt(raw ?? '', 10);
if (isNaN(parsed)) {
res.status(400).json({ error: `Invalid ${label}` });
return null;
}
return parsed;
}
const UPDATE_TIMEOUT_MSG = 'Node did not come back online within 5 minutes.';
const EARLY_FAIL_MS = 180 * 1000; // 3 minutes before declaring a probable pull failure
@@ -399,7 +384,7 @@ fleetRouter.get('/node/:nodeId/stacks', authMiddleware, async (req: Request, res
if (!requirePaid(req, res)) return;
try {
const nodeId = parseIdParam(req, res, 'nodeId', 'node ID');
const nodeId = parseIntParam(req, res, 'nodeId', 'node ID');
if (nodeId === null) return;
const node = DatabaseService.getInstance().getNode(nodeId);
if (!node) {
@@ -439,7 +424,7 @@ fleetRouter.get('/node/:nodeId/stacks/:stackName/containers', authMiddleware, as
if (!requirePaid(req, res)) return;
try {
const nodeId = parseIdParam(req, res, 'nodeId', 'node ID');
const nodeId = parseIntParam(req, res, 'nodeId', 'node ID');
if (nodeId === null) return;
const stackName = req.params.stackName as string;
if (!isValidStackName(stackName)) {
@@ -633,7 +618,7 @@ fleetRouter.post('/nodes/:nodeId/update', authMiddleware, async (req: Request, r
if (!requirePaid(req, res)) return;
if (!requireAdmin(req, res)) return;
try {
const nodeId = parseIdParam(req, res, 'nodeId', 'node ID');
const nodeId = parseIntParam(req, res, 'nodeId', 'node ID');
if (nodeId === null) return;
const db = DatabaseService.getInstance();
const node = db.getNode(nodeId);
@@ -786,7 +771,7 @@ fleetRouter.post('/update-all', authMiddleware, async (req: Request, res: Respon
fleetRouter.delete('/nodes/:nodeId/update-status', authMiddleware, async (req: Request, res: Response): Promise<void> => {
if (!requirePaid(req, res)) return;
try {
const nodeId = parseIdParam(req, res, 'nodeId', 'node ID');
const nodeId = parseIntParam(req, res, 'nodeId', 'node ID');
if (nodeId === null) return;
const node = DatabaseService.getInstance().getNode(nodeId);
if (!node) {
@@ -934,7 +919,7 @@ fleetRouter.get('/snapshots/:id', authMiddleware, async (req: Request, res: Resp
if (!requirePaid(req, res)) return;
try {
const id = parseIdParam(req, res, 'id', 'snapshot ID');
const id = parseIntParam(req, res, 'id', 'snapshot ID');
if (id === null) return;
const db = DatabaseService.getInstance();
const snapshot = db.getSnapshot(id);
@@ -980,7 +965,7 @@ fleetRouter.post('/snapshots/:id/restore', authMiddleware, async (req: Request,
if (!requirePaid(req, res)) return;
try {
const snapshotId = parseIdParam(req, res, 'id', 'snapshot ID');
const snapshotId = parseIntParam(req, res, 'id', 'snapshot ID');
if (snapshotId === null) return;
const { nodeId, stackName, redeploy = false } = req.body;
@@ -1093,7 +1078,7 @@ fleetRouter.delete('/snapshots/:id', authMiddleware, async (req: Request, res: R
if (!requirePaid(req, res)) return;
try {
const id = parseIdParam(req, res, 'id', 'snapshot ID');
const id = parseIntParam(req, res, 'id', 'snapshot ID');
if (id === null) return;
const db = DatabaseService.getInstance();
const snapshot = db.getSnapshot(id);
+7 -6
View File
@@ -12,6 +12,7 @@ import { VALID_LABEL_COLORS, MAX_LABELS_PER_NODE } from '../helpers/constants';
import { isValidStackName } from '../utils/validation';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage, isSqliteUniqueViolation } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
const activeBulkActions = new Set<string>();
@@ -105,8 +106,8 @@ labelsRouter.put('/:id', authMiddleware, async (req: Request, res: Response): Pr
if (!requirePaid(req, res)) return;
if (!requireBody(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid label ID' }); return; }
const id = parseIntParam(req, res, 'id', 'label ID');
if (id === null) return;
const nodeId = req.nodeId ?? 0;
const { name, color } = req.body;
@@ -148,8 +149,8 @@ labelsRouter.put('/:id', authMiddleware, async (req: Request, res: Response): Pr
labelsRouter.delete('/:id', authMiddleware, async (req: Request, res: Response): Promise<void> => {
if (!requirePaid(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid label ID' }); return; }
const id = parseIntParam(req, res, 'id', 'label ID');
if (id === null) return;
const nodeId = req.nodeId ?? 0;
if (isDebugEnabled()) console.debug('[Labels:debug] Delete label:', { id, nodeId });
DatabaseService.getInstance().deleteLabel(id, nodeId);
@@ -165,8 +166,8 @@ labelsRouter.post('/:id/action', authMiddleware, async (req: Request, res: Respo
if (!requireAdmin(req, res)) return;
if (!requireBody(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid label ID' }); return; }
const id = parseIntParam(req, res, 'id', 'label ID');
if (id === null) return;
const { action } = req.body;
const validActions = ['deploy', 'stop', 'restart'];
if (!action || !validActions.includes(action)) {
+6 -14
View File
@@ -12,18 +12,10 @@ import {
} from '../helpers/notificationChannels';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
const VALID_CATEGORIES: ReadonlySet<NotificationCategory> = new Set(ALL_NOTIFICATION_CATEGORIES);
function parseRouteId(req: Request, res: Response): number | null {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) {
res.status(400).json({ error: 'Invalid route ID' });
return null;
}
return id;
}
function validateNodeId(nodeId: unknown, res: Response): number | null | false {
if (nodeId === undefined || nodeId === null) return null;
if (typeof nodeId !== 'number' || !Number.isInteger(nodeId)) {
@@ -81,8 +73,8 @@ notificationsRouter.post('/read', authMiddleware, async (req: Request, res: Resp
notificationsRouter.delete('/:id', authMiddleware, async (req: Request, res: Response): Promise<void> => {
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid notification ID' }); return; }
const id = parseIntParam(req, res, 'id', 'notification ID');
if (id === null) return;
const nodeId = req.nodeId ?? 0;
DatabaseService.getInstance().deleteNotification(nodeId, id);
res.json({ success: true });
@@ -193,7 +185,7 @@ notificationRoutesRouter.put('/:id', authMiddleware, async (req: Request, res: R
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseRouteId(req, res);
const id = parseIntParam(req, res, 'id', 'route ID');
if (id === null) return;
const existing = DatabaseService.getInstance().getNotificationRoute(id);
@@ -269,7 +261,7 @@ notificationRoutesRouter.delete('/:id', authMiddleware, (req: Request, res: Resp
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseRouteId(req, res);
const id = parseIntParam(req, res, 'id', 'route ID');
if (id === null) return;
const changes = DatabaseService.getInstance().deleteNotificationRoute(id);
@@ -286,7 +278,7 @@ notificationRoutesRouter.post('/:id/test', authMiddleware, async (req: Request,
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseRouteId(req, res);
const id = parseIntParam(req, res, 'id', 'route ID');
if (id === null) return;
const route = DatabaseService.getInstance().getNotificationRoute(id);
+7 -6
View File
@@ -2,6 +2,7 @@ import { Router, type Request, type Response } from 'express';
import { RegistryService } from '../services/RegistryService';
import { requireAdmin, requireAdmiral } from '../middleware/tierGates';
import { rejectApiTokenScope } from '../middleware/apiTokenScope';
import { parseIntParam } from '../utils/parseIntParam';
const VALID_REGISTRY_TYPES = ['dockerhub', 'ghcr', 'ecr', 'custom'] as const;
const REGISTRY_SCOPE_MESSAGE = 'API tokens cannot manage registry credentials.';
@@ -80,8 +81,8 @@ registriesRouter.put('/:id', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid registry ID' }); return; }
const id = parseIntParam(req, res, 'id', 'registry ID');
if (id === null) return;
const existing = RegistryService.getInstance().getById(id);
if (!existing) { res.status(404).json({ error: 'Registry not found' }); return; }
@@ -118,8 +119,8 @@ registriesRouter.delete('/:id', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid registry ID' }); return; }
const id = parseIntParam(req, res, 'id', 'registry ID');
if (id === null) return;
const existing = RegistryService.getInstance().getById(id);
if (!existing) { res.status(404).json({ error: 'Registry not found' }); return; }
@@ -137,8 +138,8 @@ registriesRouter.post('/:id/test', async (req: Request, res: Response): Promise<
if (!requireAdmin(req, res)) return;
if (!requireAdmiral(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) { res.status(400).json({ error: 'Invalid registry ID' }); return; }
const id = parseIntParam(req, res, 'id', 'registry ID');
if (id === null) return;
const result = await RegistryService.getInstance().testConnection(id);
res.json(result);
+8 -16
View File
@@ -6,6 +6,7 @@ import { SchedulerService } from '../services/SchedulerService';
import { requirePaid, requireAdmin, requireScheduledTaskTier } from '../middleware/tierGates';
import { escapeCsvField } from '../utils/csv';
import { getErrorMessage } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
const VALID_TARGET_TYPES = ['stack', 'fleet', 'system'] as const;
const VALID_ACTIONS = ['restart', 'snapshot', 'prune', 'update', 'scan', 'auto_backup', 'auto_stop', 'auto_down', 'auto_start'] as const;
@@ -17,15 +18,6 @@ type ScheduledAction = typeof VALID_ACTIONS[number];
const STACK_ONLY_ACTIONS = new Set<ScheduledAction>(['auto_backup', 'auto_stop', 'auto_down', 'auto_start']);
function parseTaskId(req: Request, res: Response): number | null {
const id = parseInt(req.params.id as string, 10);
if (isNaN(id)) {
res.status(400).json({ error: 'Invalid task ID' });
return null;
}
return id;
}
/**
* Validate that the target_type is compatible with the action. Each action
* has exactly one allowed target_type; the helper returns an error message
@@ -190,7 +182,7 @@ scheduledTasksRouter.get('/:id', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const task = DatabaseService.getInstance().getScheduledTask(id);
if (!task) { res.status(404).json({ error: 'Scheduled task not found' }); return; }
@@ -206,7 +198,7 @@ scheduledTasksRouter.put('/:id', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
@@ -286,7 +278,7 @@ scheduledTasksRouter.delete('/:id', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
@@ -307,7 +299,7 @@ scheduledTasksRouter.patch('/:id/toggle', (req: Request, res: Response): void =>
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
@@ -337,7 +329,7 @@ scheduledTasksRouter.post('/:id/run', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
@@ -368,7 +360,7 @@ scheduledTasksRouter.get('/:id/runs/export', (req: Request, res: Response): void
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
@@ -404,7 +396,7 @@ scheduledTasksRouter.get('/:id/runs', (req: Request, res: Response): void => {
if (!requireAdmin(req, res)) return;
if (!requirePaid(req, res)) return;
try {
const id = parseTaskId(req, res);
const id = parseIntParam(req, res, 'id', 'task ID');
if (id === null) return;
const db = DatabaseService.getInstance();
+3 -5
View File
@@ -8,6 +8,7 @@ import { rejectApiTokenScope } from '../middleware/apiTokenScope';
import { BCRYPT_SALT_ROUNDS, MIN_PASSWORD_LENGTH } from '../helpers/constants';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage, isSqliteUniqueViolation } from '../utils/errors';
import { parseIntParam } from '../utils/parseIntParam';
const USERS_SCOPE_MESSAGE = 'API tokens cannot access user management.';
const VALID_USER_ROLES: UserRole[] = ['admin', 'viewer', 'deployer', 'node-admin', 'auditor'];
@@ -218,11 +219,8 @@ usersRouter.post('/:id/mfa/reset', authMiddleware, (req: Request, res: Response)
if (rejectApiTokenScope(req, res, USERS_SCOPE_MESSAGE)) return;
if (!requireAdmin(req, res)) return;
try {
const id = parseInt(req.params.id as string, 10);
if (!Number.isFinite(id)) {
res.status(400).json({ error: 'Invalid user id' });
return;
}
const id = parseIntParam(req, res, 'id', 'user id');
if (id === null) return;
const db = DatabaseService.getInstance();
const target = db.getUser(id);
if (!target) {
+24
View File
@@ -0,0 +1,24 @@
import type { Request, Response } from 'express';
/**
* Parse a numeric route param. Writes a 400 response and returns null when
* the value isn't a valid integer; callers early-return on null.
*
* @param paramName the key in req.params (e.g. 'id', 'nodeId')
* @param label optional human-readable label used in the error body.
* Falls back to paramName when omitted (e.g. 'Invalid id').
*/
export function parseIntParam(
req: Request,
res: Response,
paramName: string,
label?: string,
): number | null {
const raw = req.params[paramName] as string | undefined;
const parsed = parseInt(raw ?? '', 10);
if (isNaN(parsed)) {
res.status(400).json({ error: `Invalid ${label ?? paramName}` });
return null;
}
return parsed;
}