feat: non-blocking bulk-add, CORS hardening, credential preset access control (#31)

* devices: hide discovered neighbors that are already managed

Discovered Devices showed every unresolved MikroTik neighbor, even when
the neighbor was obviously the same device a managed node is reporting
through a different link / MAC / alias. Since we already know every
managed device interface MAC, primary IP, and system identity, we can
flag those rows.

- /api/devices/discovered now annotates each row with duplicate_of_device_id
  and duplicate_of_device_name when the neighbor matches a managed device
  by interface MAC, primary IP, or system identity.
- Add a Hide duplicates of managed devices toggle (default on) above the
  table, with a count of how many matches were found. When enabled, the
  list hides matched rows; when disabled, matches stay visible but are
  dimmed and tagged Already managed so the origin is clear.
- Suppress the Add to Manager action for rows that are already a
  managed device, since adding them would create a second record for the
  same hardware.

Co-authored-by: Denis <denis-ev@users.noreply.github.com>

* devices: safer duplicate handling, try-all safeguards, and timezone/firmware fixes

Co-authored-by: Denis <denis-ev@users.noreply.github.com>

* Non-blocking follow-ups: parsePort util, tests, bulk-add queue, preset rate limit

Co-authored-by: Denis <denis-ev@users.noreply.github.com>

* fix(security): tighten CORS, Redis rate limits, and preset access controls

Require explicit CORS origins in production, align Socket.IO CORS with HTTP,
default Docker Compose to local HTTPS origins, and remove the in-memory
mutation limiter in favor of Redis. Also add operator-use flags for credential
presets in the UI and harden bulk-add job access and client error messages.

Co-authored-by: Denis <denis-ev@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>

backend/src/routes/__tests__/credentialPresets.test.ts

@@ -0,0 +1,129 @@
+jest.mock('../../config/database');
+jest.mock('../../config/redis', () => ({
+  redis: {
+    incr: jest.fn().mockResolvedValue(1),
+    expire: jest.fn().mockResolvedValue(1),
+    ttl: jest.fn().mockResolvedValue(60),
+  },
+}));
+jest.mock('../../middleware/auth', () => ({
+  requireAuth: (_req: unknown, _res: unknown, next: () => void) => next(),
+  requireAdmin: (
+    req: { user?: { role: string } },
+    res: { status: (c: number) => { json: (b: unknown) => void } },
+    next: () => void
+  ) => {
+    if (req.user?.role !== 'admin') {
+      res.status(403).json({ error: 'Admin access required' });
+      return;
+    }
+    next();
+  },
+}));
+
+jest.mock('../../utils/crypto', () => ({
+  encrypt: (s: string) => `enc:${s}`,
+}));
+
+import request from 'supertest';
+import express from 'express';
+import credentialPresetsRoutes from '../credentialPresets';
+import { query, queryOne } from '../../config/database';
+
+const mockedQuery = jest.mocked(query);
+const mockedQueryOne = jest.mocked(queryOne);
+
+function makeApp(role: string) {
+  const app = express();
+  app.use(express.json());
+  app.use((req, _res, next) => {
+    (req as express.Request & { user?: { userId: number; username: string; role: string } }).user = {
+      userId: 1,
+      username: 'test',
+      role,
+    };
+    next();
+  });
+  app.use('/', credentialPresetsRoutes);
+  return app;
+}
+
+describe('credentialPresets routes', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('GET / returns public list', async () => {
+    mockedQuery.mockResolvedValueOnce([
+      {
+        id: 1,
+        name: 'Default',
+        api_username: 'admin',
+        api_password_encrypted: 'x',
+        api_port: 8728,
+        ssh_username: null,
+        ssh_password_encrypted: null,
+        ssh_port: null,
+        notes: null,
+        allow_operator_use: true,
+        created_at: 't',
+        updated_at: 't',
+      },
+    ]);
+    const app = makeApp('viewer');
+    const res = await request(app).get('/');
+    expect(res.status).toBe(200);
+    expect(res.body[0]).toMatchObject({
+      id: 1,
+      name: 'Default',
+      has_api_password: true,
+      has_ssh_password: false,
+    });
+    expect(res.body[0].api_password_encrypted).toBeUndefined();
+  });
+
+  it('POST / rejects non-admin', async () => {
+    const app = makeApp('operator');
+    const res = await request(app).post('/').send({
+      name: 'x',
+      api_username: 'a',
+      api_password: 'p',
+    });
+    expect(res.status).toBe(403);
+  });
+
+  it('POST / requires name, api_username, api_password', async () => {
+    const app = makeApp('admin');
+    const res = await request(app).post('/').send({ name: 'only' });
+    expect(res.status).toBe(400);
+    expect(String(res.body.error)).toMatch(/required/);
+  });
+
+  it('POST / creates preset when admin', async () => {
+    mockedQueryOne.mockResolvedValueOnce(null);
+    mockedQuery.mockResolvedValueOnce([
+      {
+        id: 2,
+        name: 'Lab',
+        api_username: 'admin',
+        api_password_encrypted: 'enc:p',
+        api_port: 8728,
+        ssh_username: null,
+        ssh_password_encrypted: null,
+        ssh_port: null,
+        notes: null,
+        allow_operator_use: true,
+        created_at: 't',
+        updated_at: 't',
+      },
+    ]);
+    const app = makeApp('admin');
+    const res = await request(app).post('/').send({
+      name: 'Lab',
+      api_username: 'admin',
+      api_password: 'secret',
+    });
+    expect(res.status).toBe(201);
+    expect(res.body).toMatchObject({ id: 2, name: 'Lab', has_api_password: true });
+  });
+});

backend/src/routes/credentialPresets.ts

@@ -2,10 +2,17 @@ import { Router, Request, Response } from 'express';
 import { query, queryOne } from '../config/database';
 import { requireAuth, requireAdmin } from '../middleware/auth';
 import { encrypt } from '../utils/crypto';
+import { rateLimitRedis } from '../middleware/rateLimitRedis';
 
 const router = Router();
 router.use(requireAuth);
 
+const presetMutationLimiter = rateLimitRedis({
+  windowSec: 60,
+  max: 30,
+  keyPrefix: 'credential-preset',
+});
+
 export interface CredentialPresetRow {
   id: number;
   name: string;
@@ -16,6 +23,8 @@ export interface CredentialPresetRow {
   ssh_password_encrypted: string | null;
   ssh_port: number | null;
   notes: string | null;
+  /** When false, only admins may use this preset when adding/updating devices. */
+  allow_operator_use: boolean;
   created_at: string;
   updated_at: string;
 }
@@ -30,6 +39,7 @@ interface CredentialPresetPublic {
   ssh_username: string | null;
   ssh_port: number | null;
   notes: string | null;
+  allow_operator_use: boolean;
   has_api_password: boolean;
   has_ssh_password: boolean;
   created_at: string;
@@ -45,6 +55,7 @@ function toPublic(row: CredentialPresetRow): CredentialPresetPublic {
     ssh_username: row.ssh_username,
     ssh_port: row.ssh_port,
     notes: row.notes,
+    allow_operator_use: row.allow_operator_use !== false,
     has_api_password: !!row.api_password_encrypted,
     has_ssh_password: !!row.ssh_password_encrypted,
     created_at: row.created_at,
@@ -54,15 +65,20 @@ function toPublic(row: CredentialPresetRow): CredentialPresetPublic {
 
 // GET /api/credential-presets — any authenticated user can list (the Add
 // Device modal needs this for its picker), but secrets are never exposed.
-router.get('/', async (_req: Request, res: Response) => {
+router.get('/', async (req: Request, res: Response) => {
+  const role = req.user?.role;
   const rows = await query<CredentialPresetRow>(
     `SELECT * FROM credential_presets ORDER BY name ASC`
   );
-  res.json(rows.map(toPublic));
+  const filtered =
+    role === 'admin'
+      ? rows
+      : rows.filter((r) => (r as CredentialPresetRow).allow_operator_use !== false);
+  res.json(filtered.map(toPublic));
 });
 
 // POST /api/credential-presets — admin only
-router.post('/', requireAdmin, async (req: Request, res: Response) => {
+router.post('/', requireAdmin, presetMutationLimiter, async (req: Request, res: Response) => {
   const {
     name,
     api_username,
@@ -72,6 +88,7 @@ router.post('/', requireAdmin, async (req: Request, res: Response) => {
     ssh_password,
     ssh_port,
     notes,
+    allow_operator_use,
   } = req.body as {
     name?: string;
     api_username?: string;
@@ -81,6 +98,7 @@ router.post('/', requireAdmin, async (req: Request, res: Response) => {
     ssh_password?: string | null;
     ssh_port?: number | null;
     notes?: string | null;
+    allow_operator_use?: boolean;
   };
 
   if (!name || !api_username || !api_password) {
@@ -100,11 +118,13 @@ router.post('/', requireAdmin, async (req: Request, res: Response) => {
   const encApi = encrypt(api_password);
   const encSsh = ssh_password ? encrypt(ssh_password) : null;
 
+  const allowOp = allow_operator_use !== false;
+
   const rows = await query<CredentialPresetRow>(
     `INSERT INTO credential_presets
        (name, api_username, api_password_encrypted, api_port,
-        ssh_username, ssh_password_encrypted, ssh_port, notes)
-     VALUES ($1,$2,$3,$4,$5,$6,$7,$8)
+        ssh_username, ssh_password_encrypted, ssh_port, notes, allow_operator_use)
+     VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9)
      RETURNING *`,
     [
       name,
@@ -115,13 +135,14 @@ router.post('/', requireAdmin, async (req: Request, res: Response) => {
       encSsh,
       ssh_port ?? null,
       notes || null,
+      allowOp,
     ]
   );
   return res.status(201).json(toPublic(rows[0]));
 });
 
 // PUT /api/credential-presets/:id — admin only
-router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
+router.put('/:id', requireAdmin, presetMutationLimiter, async (req: Request, res: Response) => {
   const id = parseInt(req.params.id, 10);
   if (!Number.isFinite(id)) return res.status(400).json({ error: 'Invalid id' });
 
@@ -141,6 +162,7 @@ router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
     ssh_port,
     notes,
     clear_ssh_password,
+    allow_operator_use,
   } = req.body as {
     name?: string;
     api_username?: string;
@@ -152,6 +174,7 @@ router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
     notes?: string | null;
     // Explicit flag to remove a previously-saved SSH password
     clear_ssh_password?: boolean;
+    allow_operator_use?: boolean;
   };
 
   if (typeof name === 'string' && name && name !== existing.name) {
@@ -177,8 +200,9 @@ router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
        ssh_password_encrypted = $6,
        ssh_port               = $7,
        notes                  = $8,
+       allow_operator_use     = COALESCE($9, allow_operator_use),
        updated_at             = NOW()
-     WHERE id = $9`,
+     WHERE id = $10`,
     [
       name ?? null,
       api_username ?? null,
@@ -188,6 +212,7 @@ router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
       newSshPass,
       ssh_port === undefined ? existing.ssh_port : ssh_port,
       notes === undefined ? existing.notes : (notes || null),
+      allow_operator_use === undefined ? null : allow_operator_use,
       id,
     ]
   );
@@ -200,7 +225,7 @@ router.put('/:id', requireAdmin, async (req: Request, res: Response) => {
 });
 
 // DELETE /api/credential-presets/:id — admin only
-router.delete('/:id', requireAdmin, async (req: Request, res: Response) => {
+router.delete('/:id', requireAdmin, presetMutationLimiter, async (req: Request, res: Response) => {
   const id = parseInt(req.params.id, 10);
   if (!Number.isFinite(id)) return res.status(400).json({ error: 'Invalid id' });
   const result = await query(

backend/src/routes/devices.ts

@@ -1,4 +1,5 @@
 import { Router, Request, Response } from 'express';
+import { randomUUID } from 'crypto';
 import { query, queryOne } from '../config/database';
 import { requireAuth, requireWrite } from '../middleware/auth';
 import { encrypt, decrypt } from '../utils/crypto';
@@ -6,11 +7,17 @@ import { RouterOSClient } from '../services/mikrotik/RouterOSClient';
 import { DeviceCollector, DeviceRow } from '../services/mikrotik/DeviceCollector';
 import { PollerService } from '../services/PollerService';
 import type { CredentialPresetRow } from './credentialPresets';
+import { createDeviceFromBody, type CreateDeviceInput, type CreateDeviceContext } from '../services/deviceCreation';
+import { parsePort } from '../utils/parsePort';
+import { safeConnectionError } from '../utils/safeClientError';
+import { redis } from '../config/redis';
+import { enqueueBulkAddJob, getBulkAddJobState } from '../services/DeviceBulkAddWorker';
 
 // Resolve a credential preset id into decrypted credentials. Returns null if
 // no id was provided; throws a readable error if the id is invalid.
 async function loadCredentialPreset(
-  id: number | null | undefined
+  id: number | null | undefined,
+  ctx?: CreateDeviceContext
 ): Promise<{
   api_username: string;
   api_password: string;
@@ -25,6 +32,12 @@ async function loadCredentialPreset(
     [id]
   );
   if (!preset) throw new Error(`Credential preset ${id} not found`);
+  const allowOp = preset.allow_operator_use !== false;
+  if (ctx?.requestingUserRole === 'operator' && !allowOp) {
+    const err = new Error('This credential preset is restricted to administrators');
+    (err as Error & { statusCode?: number }).statusCode = 403;
+    throw err;
+  }
   return {
     api_username: preset.api_username,
     api_password: decrypt(preset.api_password_encrypted),
@@ -165,165 +178,71 @@ router.get('/discovered', async (_req: Request, res: Response) => {
 
 // POST /api/devices
 router.post('/', requireWrite, async (req: Request, res: Response) => {
-  const {
-    name, ip_address, device_type = 'router', notes,
-    credential_preset_id,
-  } = req.body as { name?: string; ip_address?: string; device_type?: string; notes?: string; credential_preset_id?: number | null };
+  const result = await createDeviceFromBody(req.body, pollerService, {
+    requestingUserRole: req.user?.role,
+  });
+  return res.status(result.status).json(result.body);
+});
 
-  // If a preset is supplied, its credentials win over anything else in the
-  // body — the client just needs to say "use preset 3". Anything the body
-  // also sends (like ssh_username) is ignored in favor of preset values, so
-  // "deploying" a preset always produces a consistent device record.
-  let preset: Awaited<ReturnType<typeof loadCredentialPreset>> = null;
-  try {
-    preset = await loadCredentialPreset(credential_preset_id ?? null);
-  } catch (err) {
-    return res.status(400).json({ error: (err as Error).message });
+const BULK_ADD_META_TTL_SEC = 86400;
+
+// POST /api/devices/bulk-add/jobs — enqueue Try-All style adds (survives tab close)
+router.post('/bulk-add/jobs', requireWrite, async (req: Request, res: Response) => {
+  const { items } = req.body as { items?: unknown };
+  if (!Array.isArray(items) || items.length === 0) {
+    return res.status(400).json({ error: 'items must be a non-empty array' });
   }
-
-  const api_username: string | undefined = preset?.api_username ?? req.body.api_username;
-  const api_password: string | undefined = preset?.api_password ?? req.body.api_password;
-  const parsePort = (v: unknown, fallback: number): number => {
-    if (v == null || v === '') return fallback;
-    const n = typeof v === 'number' ? v : parseInt(String(v), 10);
-    return Number.isFinite(n) ? n : fallback;
-  };
-  const api_port: number = preset?.api_port ?? parsePort(req.body.api_port, 8728);
-  const ssh_username: string | null = preset ? preset.ssh_username : (req.body.ssh_username ?? null);
-  const ssh_password: string | null = preset ? preset.ssh_password : (req.body.ssh_password ?? null);
-  const ssh_port: number = preset?.ssh_port ?? parsePort(req.body.ssh_port, 22);
-  const combineWithDeviceId =
-    typeof req.body.combine_with_device_id === 'number' ? req.body.combine_with_device_id : null;
-  const forceReplaceBySerial = req.body.force_replace_existing_by_serial === true;
-
-  if (!name || !ip_address || !api_username || !api_password) {
-    return res.status(400).json({ error: 'name, ip_address, api_username, api_password are required' });
+  if (items.length > 500) {
+    return res.status(400).json({ error: 'items array exceeds maximum of 500' });
   }
-
-  // Test connection and detect hardware serial before saving
-  const testClient = new RouterOSClient(ip_address, api_port, api_username, api_password, 10_000);
-  let detectedSerial: string | null = null;
-  try {
-    await testClient.connect();
-    const rb = await testClient.execute('/system/routerboard/print').catch(() => [] as Record<string, string>[]);
-    detectedSerial = (rb[0]?.['serial-number'] || '').trim() || null;
-  } catch (err) {
-    return res.status(422).json({
-      error: `Cannot connect to device: ${(err as Error).message}`,
-    });
-  } finally {
-    testClient.disconnect();
-  }
-
-  if (detectedSerial) {
-    const existingBySerial = await queryOne<{
-      id: number;
-      name: string;
-      ip_address: string;
-      serial_number: string;
-    }>(
-      `SELECT id, name, ip_address, serial_number
-         FROM devices
-        WHERE serial_number = $1`,
-      [detectedSerial]
-    );
-
-    if (existingBySerial) {
-      const shouldCombine =
-        combineWithDeviceId != null && combineWithDeviceId === existingBySerial.id;
-      if (!shouldCombine && !forceReplaceBySerial) {
-        return res.status(409).json({
-          error: 'duplicate_serial',
-          code: 'duplicate_serial',
-          existing_device: existingBySerial,
-          candidate: {
-            serial_number: detectedSerial,
-            identity: name,
-            ip_address,
-          },
-        });
-      }
-
-      const encryptedPass = encrypt(api_password);
-      const encryptedSshPass = ssh_password ? encrypt(ssh_password) : null;
-      await query(
-        `UPDATE devices SET
-           name=COALESCE($1,name),
-           ip_address=$2,
-           api_port=$3,
-           api_username=$4,
-           api_password_encrypted=$5,
-           ssh_port=$6,
-           ssh_username=COALESCE($7,ssh_username),
-           ssh_password_encrypted=COALESCE($8,ssh_password_encrypted),
-           device_type=COALESCE($9,device_type),
-           notes=COALESCE($10,notes),
-           updated_at=NOW()
-         WHERE id = $11`,
-        [
-          name,
-          ip_address,
-          api_port,
-          api_username,
-          encryptedPass,
-          ssh_port,
-          ssh_username,
-          encryptedSshPass,
-          device_type,
-          notes || null,
-          existingBySerial.id,
-        ]
-      );
-
-      if (pollerService) {
-        await pollerService.scheduleDeviceSync(existingBySerial.id, 'full');
-      }
-
-      const updatedExisting = await queryOne(
-        `SELECT id, name, ip_address, api_port, api_username, model, serial_number,
-                firmware_version, ros_version, device_type, status, last_seen, notes, created_at
-         FROM devices WHERE id = $1`,
-        [existingBySerial.id]
-      );
-      if (!updatedExisting) {
-        return res.status(500).json({
-          error: 'Duplicate merge succeeded but device row could not be reloaded',
-          device_id: existingBySerial.id,
-        });
-      }
-      return res.status(200).json({
-        ...updatedExisting,
-        merged_from_duplicate: true,
-      });
-    }
-  }
-
-  const encryptedPass = encrypt(api_password);
-  const encryptedSshPass = ssh_password ? encrypt(ssh_password) : null;
-
-  const rows = await query<{ id: number }>(
-    `INSERT INTO devices (name, ip_address, api_port, api_username, api_password_encrypted,
-                          ssh_port, ssh_username, ssh_password_encrypted, device_type, notes, status)
-     VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,'unknown') RETURNING id`,
-    [name, ip_address, api_port, api_username, encryptedPass,
-     ssh_port, ssh_username || null, encryptedSshPass, device_type, notes || null]
+  const jobId = randomUUID();
+  const ownerUserId = req.user!.userId;
+  await redis.set(
+    `device-bulk-add:${jobId}:meta`,
+    JSON.stringify({
+      status: 'queued',
+      total: items.length,
+      processed: 0,
+      owner_user_id: ownerUserId,
+      created_at: new Date().toISOString(),
+    }),
+    'EX',
+    BULK_ADD_META_TTL_SEC
   );
+  await redis.set(`device-bulk-add:${jobId}:results`, '[]', 'EX', BULK_ADD_META_TTL_SEC);
+  await enqueueBulkAddJob(jobId, items as CreateDeviceInput[]);
+  return res.status(202).json({ job_id: jobId, total: items.length });
+});
 
-  const newId = rows[0].id;
-
-  // Trigger full sync in background
-  if (pollerService) {
-    await pollerService.scheduleDeviceSync(newId, 'full');
+// POST /api/devices/bulk-add/jobs/:jobId/cancel — request cooperative cancel
+router.post('/bulk-add/jobs/:jobId/cancel', requireWrite, async (req: Request, res: Response) => {
+  const state = await getBulkAddJobState(req.params.jobId);
+  if (!state.found) {
+    return res.status(404).json({ error: 'Job not found or expired' });
   }
+  const ownerId = state.meta.owner_user_id;
+  if (typeof ownerId !== 'number' || ownerId !== req.user!.userId) {
+    return res.status(403).json({ error: 'Forbidden' });
+  }
+  await redis.set(`device-bulk-add:${req.params.jobId}:cancel`, '1', 'EX', BULK_ADD_META_TTL_SEC);
+  return res.json({ message: 'Cancel requested' });
+});
 
-  const device = await queryOne(
-    `SELECT id, name, ip_address, api_port, api_username, model, serial_number,
-            firmware_version, ros_version, device_type, status, last_seen, notes, created_at
-     FROM devices WHERE id = $1`,
-    [newId]
-  );
-
-  return res.status(201).json(device);
+// GET /api/devices/bulk-add/jobs/:jobId — poll job status and incremental results
+router.get('/bulk-add/jobs/:jobId', requireWrite, async (req: Request, res: Response) => {
+  const state = await getBulkAddJobState(req.params.jobId);
+  if (!state.found) {
+    return res.status(404).json({ error: 'Job not found or expired' });
+  }
+  const ownerId = state.meta.owner_user_id;
+  if (typeof ownerId !== 'number' || ownerId !== req.user!.userId) {
+    return res.status(403).json({ error: 'Forbidden' });
+  }
+  return res.json({
+    job_id: req.params.jobId,
+    ...state.meta,
+    results: state.results,
+  });
 });
 
 // GET /api/devices/:id
@@ -392,35 +311,39 @@ router.put('/:id', requireWrite, async (req: Request, res: Response) => {
     credential_preset_id?: number | null;
   };
 
-  let preset: Awaited<ReturnType<typeof loadCredentialPreset>> = null;
-  try {
-    preset = await loadCredentialPreset(credential_preset_id ?? null);
-  } catch (err) {
-    return res.status(400).json({ error: (err as Error).message });
-  }
-
-  // When a preset is applied, its values take precedence over anything else
-  // in the body so "apply preset" has a single unambiguous meaning.
-  const api_port = preset?.api_port ?? req.body.api_port;
-  const api_username = preset?.api_username ?? req.body.api_username;
-  const api_password = preset?.api_password ?? req.body.api_password;
-  const ssh_port = preset?.ssh_port ?? req.body.ssh_port;
-  const ssh_username = preset ? preset.ssh_username : req.body.ssh_username;
-  const ssh_password = preset ? preset.ssh_password : req.body.ssh_password;
-
   const existing = await queryOne<{
     id: number;
     ip_address: string;
     api_port: number;
     api_username: string;
     api_password_encrypted: string;
+    ssh_port: number | null;
   }>(
-    `SELECT id, ip_address, api_port, api_username, api_password_encrypted
+    `SELECT id, ip_address, api_port, api_username, api_password_encrypted, ssh_port
        FROM devices WHERE id = $1`,
     [req.params.id]
   );
   if (!existing) return res.status(404).json({ error: 'Device not found' });
 
+  let preset: Awaited<ReturnType<typeof loadCredentialPreset>> = null;
+  try {
+    preset = await loadCredentialPreset(credential_preset_id ?? null, {
+      requestingUserRole: req.user?.role,
+    });
+  } catch (err) {
+    const status = (err as Error & { statusCode?: number }).statusCode ?? 400;
+    return res.status(status).json({ error: (err as Error).message });
+  }
+
+  // When a preset is applied, its values take precedence over anything else
+  // in the body so "apply preset" has a single unambiguous meaning.
+  const api_port = preset?.api_port ?? parsePort(req.body.api_port, existing.api_port);
+  const api_username = preset?.api_username ?? req.body.api_username;
+  const api_password = preset?.api_password ?? req.body.api_password;
+  const ssh_port = preset?.ssh_port ?? parsePort(req.body.ssh_port, existing.ssh_port ?? 22);
+  const ssh_username = preset ? preset.ssh_username : req.body.ssh_username;
+  const ssh_password = preset ? preset.ssh_password : req.body.ssh_password;
+
   // If the user is changing the IP (or port / username / password), verify the
   // RouterOS API is still reachable with the new values before persisting —
   // otherwise we could silently brick the device record.
@@ -430,7 +353,7 @@ router.put('/:id', requireWrite, async (req: Request, res: Response) => {
   const presetReplacesApiCreds = !!preset;
   if (ipChanged || portChanged || userChanged || api_password || presetReplacesApiCreds) {
     const testIp = ip_address ?? existing.ip_address;
-    const testPort = (typeof api_port === 'number' ? api_port : undefined) ?? existing.api_port;
+    const testPort = api_port;
     const testUser = api_username ?? existing.api_username;
     const testPass = api_password ? api_password : decrypt(existing.api_password_encrypted);
     const testClient = new RouterOSClient(testIp, testPort, testUser, testPass, 10_000);
@@ -439,7 +362,7 @@ router.put('/:id', requireWrite, async (req: Request, res: Response) => {
       testClient.disconnect();
     } catch (err) {
       return res.status(422).json({
-        error: `Cannot connect to device with new settings: ${(err as Error).message}`,
+        error: safeConnectionError('PUT /devices/:id', err),
       });
     }
   }