fix: harden auth site access and SSH validation

This commit is contained in:
Gustavo
2026-09-28 14:54:03 -03:00
parent 5b926d637b
commit c4db0dc1e2
33 changed files with 992 additions and 125 deletions
+18 -5
View File
@@ -2,7 +2,8 @@ import { Router, Request, Response } from 'express';
import { randomUUID } from 'crypto';
import { Client as SshClient } from 'ssh2';
import { query, queryOne } from '../config/database';
import { requireAuth, requireWrite } from '../middleware/auth';
import { requireAuth, requireWrite, requireAdmin } from '../middleware/auth';
import { deviceIdSiteGuard } from '../middleware/site';
import { encrypt, decrypt } from '../utils/crypto';
import { RouterOSClient } from '../services/mikrotik/RouterOSClient';
import { DeviceCollector, DeviceRow } from '../services/mikrotik/DeviceCollector';
@@ -12,6 +13,7 @@ import { createDeviceFromBody, type CreateDeviceInput, type CreateDeviceContext
import { parsePort } from '../utils/parsePort';
import { safeConnectionError } from '../utils/safeClientError';
import { detectLockoutRisk } from '../utils/firewallSafety';
import { validateRouterOsUsername } from '../utils/routerOsUsername';
import { withSafeApply, probeCapability, probeUndoCapability, type GuardDevice } from '../services/changeGuard/ChangeGuard';
import { captureSnapshot, resolveManagementPath } from '../services/changeGuard/pathModel';
import { analyzeChange, type PlannedChange } from '../services/changeGuard/analyzeChange';
@@ -22,8 +24,8 @@ import { enqueueBulkAddJob, getBulkAddJobState } from '../services/DeviceBulkAdd
import { logSafe } from '../utils/logSafe';
import { DEVICE_BASE_COLUMNS } from '../services/deviceColumns';
import { buildSegments, summarise, summariseBands } from '../utils/lteDwell';
import { siteScopeDevices, andSite } from '../utils/siteScope';
import { activeSite } from '../middleware/site';
import { siteScopeDevices, andSite, singleSiteOrNull } from '../utils/siteScope';
import { activeSite, enforceSiteAccess } from '../middleware/site';
/** Postgres interval text to milliseconds, for the few windows we offer. */
function intervalToMs(interval: string): number {
@@ -70,6 +72,12 @@ async function loadCredentialPreset(
const router = Router();
router.use(requireAuth);
router.use(enforceSiteAccess);
// SEC-002: every :id in this router is a device id -- refuse it up front if
// the device belongs to a site the caller isn't authorized for, before any
// handler runs. Closes the IDOR of reaching another site's device by guessing
// or enumerating its numeric id.
router.param('id', deviceIdSiteGuard);
let pollerService: PollerService | null = null;
export function setPollerService(p: PollerService): void {
@@ -285,7 +293,7 @@ router.post('/', requireWrite, async (req: Request, res: Response) => {
const result = await createDeviceFromBody(req.body, pollerService, {
requestingUserRole: req.user?.role,
// A device added while viewing a site joins that site (issue #130).
siteId: activeSite(req),
siteId: singleSiteOrNull(activeSite(req)),
});
return res.status(result.status).json(result.body);
});
@@ -316,7 +324,7 @@ router.post('/bulk-add/jobs', requireWrite, async (req: Request, res: Response)
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[], activeSite(req));
await enqueueBulkAddJob(jobId, items as CreateDeviceInput[], singleSiteOrNull(activeSite(req)));
return res.status(202).json({ job_id: jobId, total: items.length });
});
@@ -450,6 +458,11 @@ router.put('/:id', requireWrite, async (req: Request, res: Response) => {
const ssh_username = preset ? preset.ssh_username : req.body.ssh_username;
const ssh_password = preset ? preset.ssh_password : req.body.ssh_password;
// SEC-006: reject an unsafe ssh_username before it can ever reach the
// RouterOS CLI command SshKeyService builds from it.
const sshUsernameError = validateRouterOsUsername(ssh_username);
if (sshUsernameError) return res.status(400).json({ error: sshUsernameError });
// 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.