From 518063e64fa3226971ea4268370d2b5fb48171fd Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:45:30 -0700 Subject: [PATCH] feat(safety): predict management lockouts before applying a change (v0.19.0-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2a of the change-safety work. Change Guard could already rescue a device after a bad change; this predicts the break beforehand and refuses it. Rather than blocklisting "risky operations" — which is how deleting an *unused* bond and a routine VLAN edit both slipped through — the engine derives the invariants that must hold for the manager to keep reaching the device, simulates the planned change against LIVE device state, and reports any invariant that flips from satisfied to violated. Novel change shapes are covered for free, and each invariant explains itself, so the warning names the real mechanism. - pathModel.ts: captures a live snapshot over the API (no SSH) and resolves the management path — which address, interface, bridge, VLAN and physical ingress port the session depends on, each with the reason it is load-bearing. - invariants.ts: nine checks drawn from MikroTik's documented behaviour, notably that the bridge IS the CPU port and must be a member of the management VLAN. - analyzeChange.ts: simulates the change and diffs invariants, returning safe / warning / critical plus the path used to reach the verdict. - withGuardedChange now runs the analysis first and returns 409 with the verdict unless the caller passes confirm_lockout; analysis failures never block a change. - New GET /:id/management-path and POST /:id/preflight (analyse without applying). Three bugs were found by testing against the live switch rather than fixtures: - Invariants read vlan-filtering from the frozen path instead of the snapshot under evaluation, so "enable vlan-filtering" — the single most important case — was invisible. - VLAN membership is spread across BOTH a static row and a dynamic "added by pvid" row. Simulating only the first match hid the danger; membership now tracks static and effective separately, and a bridge left in the VLAN only by the dynamic row is reported as a warning rather than silently safe. - Requiring the ingress port to be UNTAGGED in the management VLAN flagged a healthy tagged trunk. Because a pre-existing violation is (correctly) not reported as new, that false positive silently suppressed real detections. Verdicts now also list pre-existing violations so this can't hide again. Verified on the production 100G switch: changing the ingress port's PVID off the management VLAN is refused with 409 and the switch is left untouched (pvid still 1); removing the bridge from VLAN 1 is correctly reported as a warning because a dynamic PVID row would still cover it; unrelated VLAN edits stay safe. 29 changeGuard tests, 152 backend tests, full CI preflight green. Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- backend/package.json | 2 +- backend/src/routes/devices.ts | 126 +++++- .../__tests__/analyzeChange.test.ts | 285 ++++++++++++ .../src/services/changeGuard/analyzeChange.ts | 223 +++++++++ .../src/services/changeGuard/invariants.ts | 292 ++++++++++++ backend/src/services/changeGuard/pathModel.ts | 424 ++++++++++++++++++ frontend/package.json | 2 +- frontend/src/version.ts | 2 +- 9 files changed, 1346 insertions(+), 12 deletions(-) create mode 100644 backend/src/services/changeGuard/__tests__/analyzeChange.test.ts create mode 100644 backend/src/services/changeGuard/analyzeChange.ts create mode 100644 backend/src/services/changeGuard/invariants.ts create mode 100644 backend/src/services/changeGuard/pathModel.ts diff --git a/README.md b/README.md index 075be09..d625125 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A self-hosted, full-stack network management platform for MikroTik devices. Monitor, configure, and manage your entire MikroTik infrastructure — routers, switches, and wireless access points — from a single web interface. -![Version](https://img.shields.io/badge/version-0.18.2_Beta-blue) +![Version](https://img.shields.io/badge/version-0.19.0_Beta-blue) ![License](https://img.shields.io/badge/license-AGPLv3-blue) ![Docker](https://img.shields.io/badge/docker-compose-2496ED?logo=docker&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5.3-3178C6?logo=typescript&logoColor=white) diff --git a/backend/package.json b/backend/package.json index 53871c2..ac22a04 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.18.2-beta", + "version": "0.19.0-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/routes/devices.ts b/backend/src/routes/devices.ts index b55c215..dae9032 100644 --- a/backend/src/routes/devices.ts +++ b/backend/src/routes/devices.ts @@ -13,6 +13,8 @@ import { parsePort } from '../utils/parsePort'; import { safeConnectionError } from '../utils/safeClientError'; import { detectLockoutRisk } from '../utils/firewallSafety'; import { withSafeApply, type GuardDevice } from '../services/changeGuard/ChangeGuard'; +import { captureSnapshot, resolveManagementPath } from '../services/changeGuard/pathModel'; +import { analyzeChange, type PlannedChange } from '../services/changeGuard/analyzeChange'; import { redis } from '../config/redis'; import { enqueueBulkAddJob, getBulkAddJobState } from '../services/DeviceBulkAddWorker'; @@ -530,7 +532,17 @@ router.put('/:id/ports/:name/vlan', requireWrite, async (req: Request, res: Resp req.params.id, req, res, - { kind: 'port.vlan', summary: `VLAN config on port ${req.params.name} (pvid ${pvid ?? 1})` }, + { + kind: 'port.vlan', + summary: `VLAN config on port ${req.params.name} (pvid ${pvid ?? 1})`, + change: { + kind: 'port.vlan', + port: req.params.name, + pvid: pvid ?? 1, + tagged: tagged_vlans as number[], + untagged: untagged_vlans as number[], + }, + }, async (c) => { await c.setPortVlanConfig( req.params.name, @@ -922,13 +934,47 @@ async function withGuardedChange( id: string, req: Request, res: Response, - meta: { kind: string; summary: string }, + meta: { kind: string; summary: string; change?: PlannedChange }, fn: (c: DeviceCollector) => Promise, afterConfirmed?: (c: DeviceCollector) => Promise ): Promise { const deviceRow = await queryOne(`SELECT * FROM devices WHERE id = $1`, [id]); if (!deviceRow) { res.status(404).json({ error: 'Device not found' }); return; } + // Pre-flight: simulate the change against live state and refuse a predicted + // lockout unless the user has explicitly accepted it. Analysis failures must + // never block a legitimate change — the auto-revert net still applies. + if (meta.change && req.body?.confirm_lockout !== true) { + try { + const snap = await captureSnapshot(deviceRow as unknown as GuardDevice); + const verdict = analyzeChange(snap, deviceRow as unknown as GuardDevice, meta.change); + if (verdict.severity === 'critical') { + res.status(409).json({ + lockout: true, + reason: verdict.headline, + verdict: { + severity: verdict.severity, + headline: verdict.headline, + violations: verdict.violations, + warnings: verdict.warnings, + path: { + mgmt_interface: verdict.path.mgmtInterface, + bridge: verdict.path.bridge, + mgmt_vlan_id: verdict.path.mgmtVlanId, + tagged_management: verdict.path.taggedManagement, + ingress_port: verdict.path.ingressPort, + ingress_port_source: verdict.path.ingressPortSource, + hops: verdict.path.hops, + }, + }, + }); + return; + } + } catch (err) { + console.warn(`[preflight] analysis skipped for device ${id}: ${(err as Error).message}`); + } + } + const collector = new DeviceCollector(deviceRow); try { const outcome = await withSafeApply( @@ -982,6 +1028,38 @@ async function withCollector( } } +// GET /api/devices/:id/management-path — how the manager reaches this device, and +// which objects are therefore load-bearing. Read-only; useful on its own and as the +// explanation behind a lockout warning. +router.get('/:id/management-path', async (req: Request, res: Response) => { + const deviceRow = await queryOne(`SELECT * FROM devices WHERE id = $1`, [req.params.id]); + if (!deviceRow) return res.status(404).json({ error: 'Device not found' }); + try { + const snap = await captureSnapshot(deviceRow as unknown as GuardDevice); + const path = resolveManagementPath(snap, deviceRow as unknown as GuardDevice); + return res.json(path); + } catch (err) { + return res.status(502).json({ error: `Could not read device state: ${(err as Error).message}` }); + } +}); + +// POST /api/devices/:id/preflight — analyse a change WITHOUT applying it, so the UI +// can warn while the user is still editing. Body: { change: PlannedChange }. +router.post('/:id/preflight', requireWrite, async (req: Request, res: Response) => { + const change = req.body?.change as PlannedChange | undefined; + if (!change || typeof change !== 'object' || !('kind' in change)) { + return res.status(400).json({ error: 'change (with a kind) is required' }); + } + const deviceRow = await queryOne(`SELECT * FROM devices WHERE id = $1`, [req.params.id]); + if (!deviceRow) return res.status(404).json({ error: 'Device not found' }); + try { + const snap = await captureSnapshot(deviceRow as unknown as GuardDevice); + return res.json(analyzeChange(snap, deviceRow as unknown as GuardDevice, change)); + } catch (err) { + return res.status(502).json({ error: `Could not read device state: ${(err as Error).message}` }); + } +}); + router.get('/:id/address-lists', async (req, res) => { await withCollector(req.params.id, res, (c) => c.getAddressLists()); }); @@ -1315,7 +1393,11 @@ router.delete('/:id/ip-addresses/:addrId', requireWrite, async (req: Request, re req.params.id, req, res, - { kind: 'ip.remove', summary: `Remove IP address ${req.params.addrId}` }, + { + kind: 'ip.remove', + summary: `Remove IP address ${req.params.addrId}`, + change: { kind: 'ip.remove', addressId: req.params.addrId }, + }, async (c) => { await c.removeIpAddress(req.params.addrId); return { message: 'IP address removed' }; @@ -1504,7 +1586,11 @@ router.delete('/:id/routing/:routeId', requireWrite, async (req: Request, res: R req.params.id, req, res, - { kind: 'route.remove', summary: `Remove route ${req.params.routeId}` }, + { + kind: 'route.remove', + summary: `Remove route ${req.params.routeId}`, + change: { kind: 'route.remove', routeId: req.params.routeId }, + }, async (c) => { await c.removeRoute(req.params.routeId); return { message: 'Route removed' }; @@ -1723,7 +1809,11 @@ router.post('/:id/vlans', requireWrite, async (req: Request, res: Response) => { req.params.id, req, res, - { kind: 'vlan.add', summary: `Add VLAN ${vlan_id} on ${bridge}` }, + { + kind: 'vlan.add', + summary: `Add VLAN ${vlan_id} on ${bridge}`, + change: { kind: 'vlan.add', bridge, vlanId: vlan_id, tagged: tagged_ports, untagged: untagged_ports }, + }, async (c) => { await c.addBridgeVlan(bridge, vlan_id, tagged_ports, untagged_ports); return { message: `VLAN ${vlan_id} added` }; @@ -1746,7 +1836,14 @@ router.put('/:id/vlans/:vlanDbId', requireWrite, async (req: Request, res: Respo req.params.id, req, res, - { kind: 'vlan.update', summary: `Update VLAN ${vlan.vlan_id} membership on ${vlan.bridge}` }, + { + kind: 'vlan.update', + summary: `Update VLAN ${vlan.vlan_id} membership on ${vlan.bridge}`, + change: { + kind: 'vlan.update', bridge: vlan.bridge, vlanId: vlan.vlan_id, + tagged: tagged_ports, untagged: untagged_ports, + }, + }, async (c) => { await c.updateBridgeVlan(vlan.bridge, vlan.vlan_id, tagged_ports, untagged_ports); return { message: `VLAN ${vlan.vlan_id} updated` }; @@ -1767,7 +1864,11 @@ router.delete('/:id/vlans/:vlanDbId', requireWrite, async (req: Request, res: Re req.params.id, req, res, - { kind: 'vlan.delete', summary: `Remove VLAN ${vlan.vlan_id} from ${vlan.bridge}` }, + { + kind: 'vlan.delete', + summary: `Remove VLAN ${vlan.vlan_id} from ${vlan.bridge}`, + change: { kind: 'vlan.delete', bridge: vlan.bridge, vlanId: vlan.vlan_id }, + }, async (c) => { await c.removeBridgeVlan(vlan.bridge, vlan.vlan_id); return { message: 'VLAN removed' }; @@ -1883,7 +1984,11 @@ router.delete('/:id/bonds/:bondName', requireWrite, async (req: Request, res: Re req.params.id, req, res, - { kind: 'bond.delete', summary: `Delete bond '${req.params.bondName}'` }, + { + kind: 'bond.delete', + summary: `Delete bond '${req.params.bondName}'`, + change: { kind: 'bond.delete', name: req.params.bondName }, + }, async (c) => { await c.deleteBond(req.params.bondName); return { message: 'Bond deleted' }; @@ -1907,6 +2012,11 @@ router.put('/:id/bridge/:bridgeName/vlan-filtering', requireWrite, async (req: R { kind: 'bridge.vlan-filtering', summary: `${enabled ? 'Enable' : 'Disable'} VLAN filtering on bridge '${req.params.bridgeName}'`, + change: { + kind: 'bridge.vlan-filtering', + bridge: req.params.bridgeName, + enabled: Boolean(enabled), + }, }, async (c) => { await c.setBridgeVlanFiltering(req.params.bridgeName, Boolean(enabled)); diff --git a/backend/src/services/changeGuard/__tests__/analyzeChange.test.ts b/backend/src/services/changeGuard/__tests__/analyzeChange.test.ts new file mode 100644 index 0000000..7712663 --- /dev/null +++ b/backend/src/services/changeGuard/__tests__/analyzeChange.test.ts @@ -0,0 +1,285 @@ +import { analyzeChange, simulate, type PlannedChange } from '../analyzeChange'; +import { resolveManagementPath, vlanMembership, expandVlanIds, type DeviceSnapshot } from '../pathModel'; +import type { GuardDevice } from '../ChangeGuard'; + +const device: GuardDevice = { + id: 1, + name: 'test-switch', + ip_address: '192.168.0.40', + api_port: 8728, + api_username: 'admin', + api_password_encrypted: 'x', +}; + +/** + * Mirrors the real switch this was validated against: management is untagged on + * VLAN 1, the address sits on the bridge (the CPU port), and the manager is + * off-subnet so its traffic enters via the default gateway's port. + */ +function baseSnapshot(over: Partial = {}): DeviceSnapshot { + return { + addresses: [{ '.id': '*1', address: '192.168.0.40/24', interface: 'bridge', disabled: 'false' }], + interfaces: [ + { name: 'bridge', type: 'bridge', disabled: 'false' }, + { name: 'sfp28-1', type: 'ether', disabled: 'false' }, + { name: 'ether1', type: 'ether', disabled: 'false' }, + ], + vlanInterfaces: [], + bridges: [{ name: 'bridge', 'vlan-filtering': 'true', pvid: '1' }], + bridgePorts: [ + { interface: 'sfp28-1', bridge: 'bridge', pvid: '1', disabled: 'false' }, + { interface: 'ether1', bridge: 'bridge', pvid: '1', disabled: 'false' }, + ], + bridgeVlans: [ + { bridge: 'bridge', 'vlan-ids': '1', tagged: '', untagged: 'bridge,sfp28-1,ether1', + 'current-tagged': '', 'current-untagged': 'bridge,sfp28-1,ether1' }, + { bridge: 'bridge', 'vlan-ids': '10', tagged: 'sfp28-1', untagged: '', + 'current-tagged': 'sfp28-1', 'current-untagged': '' }, + ], + routes: [ + { '.id': '*A', 'dst-address': '0.0.0.0/0', gateway: '192.168.0.1', active: 'true' }, + { '.id': '*B', 'dst-address': '192.168.0.0/24', gateway: 'bridge', active: 'true' }, + ], + arp: [{ address: '192.168.0.1', 'mac-address': 'B4:FB:E4:0B:0B:8A', interface: 'bridge' }], + bridgeHosts: [{ 'mac-address': 'B4:FB:E4:0B:0B:8A', 'on-interface': 'sfp28-1', local: 'false' }], + services: [{ '.id': '*1', name: 'api', port: '8728', disabled: 'false' }], + firewallFilter: [], + mgmtConnections: [], + ...over, + }; +} + +describe('expandVlanIds', () => { + it('handles single ids, lists and ranges', () => { + expect(expandVlanIds('10')).toEqual([10]); + expect(expandVlanIds('10,20')).toEqual([10, 20]); + expect(expandVlanIds('10-12')).toEqual([10, 11, 12]); + expect(expandVlanIds('1,5-7')).toEqual([1, 5, 6, 7]); + expect(expandVlanIds(undefined)).toEqual([]); + }); +}); + +describe('vlanMembership', () => { + it('prefers effective current-* membership over the static lists', () => { + const snap = baseSnapshot({ + bridgeVlans: [{ bridge: 'bridge', 'vlan-ids': '1', tagged: '', untagged: '', + 'current-tagged': '', 'current-untagged': 'bridge' }], + }); + const m = vlanMembership(snap, 'bridge', 1); + expect(m.found).toBe(true); + expect(m.untagged).toContain('bridge'); + }); +}); + +describe('resolveManagementPath', () => { + it('resolves the untagged-on-bridge case, finding the ingress port via the default gateway', () => { + const path = resolveManagementPath(baseSnapshot(), device); + expect(path.mgmtInterface).toBe('bridge'); + expect(path.bridge).toBe('bridge'); + expect(path.bridgeVlanFiltering).toBe(true); + expect(path.taggedManagement).toBe(false); + expect(path.mgmtVlanId).toBe(1); + expect(path.ingressPort).toBe('sfp28-1'); + expect(path.ingressPortSource).toBe('fdb-default-gw'); + }); + + it('resolves a tagged VLAN sub-interface onto its parent bridge', () => { + const snap = baseSnapshot({ + addresses: [{ '.id': '*1', address: '192.168.0.40/24', interface: 'vlan99', disabled: 'false' }], + vlanInterfaces: [{ name: 'vlan99', 'vlan-id': '99', interface: 'bridge' }], + interfaces: [ + { name: 'bridge', type: 'bridge', disabled: 'false' }, + { name: 'vlan99', type: 'vlan', disabled: 'false' }, + { name: 'sfp28-1', type: 'ether', disabled: 'false' }, + ], + }); + const path = resolveManagementPath(snap, device); + expect(path.taggedManagement).toBe(true); + expect(path.mgmtVlanId).toBe(99); + expect(path.bridge).toBe('bridge'); + }); + + it('reports honestly when the ingress port cannot be determined', () => { + const path = resolveManagementPath(baseSnapshot({ arp: [], bridgeHosts: [] }), device); + expect(path.ingressPort).toBeNull(); + expect(path.ingressPortSource).toBe('unknown'); + expect(path.warnings.join(' ')).toMatch(/could not determine which port/i); + }); +}); + +describe('analyzeChange — the VLAN lockout (golden case)', () => { + it('flags removing the bridge from the management VLAN as critical', () => { + // Exactly the incident: VLAN 1 membership rewritten without the bridge (CPU port). + const change: PlannedChange = { + kind: 'vlan.update', bridge: 'bridge', vlanId: 1, + tagged: [], untagged: ['sfp28-1', 'ether1'], + }; + const v = analyzeChange(baseSnapshot(), device, change); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('bridge-untagged-member'); + expect(v.violations[0].detail).toMatch(/untagged member of VLAN 1/i); + }); + + it('flags moving the ingress port PVID off the management VLAN', () => { + const change: PlannedChange = { + kind: 'port.vlan', port: 'sfp28-1', pvid: 99, tagged: [], untagged: [], + }; + const v = analyzeChange(baseSnapshot(), device, change); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('ingress-port-pvid'); + }); + + it('flags deleting the management VLAN entirely', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'vlan.delete', bridge: 'bridge', vlanId: 1 }); + expect(v.severity).toBe('critical'); + }); + + it('allows an unrelated VLAN edit', () => { + const change: PlannedChange = { + kind: 'vlan.update', bridge: 'bridge', vlanId: 10, tagged: ['sfp28-1', 'ether1'], untagged: [], + }; + expect(analyzeChange(baseSnapshot(), device, change).severity).toBe('safe'); + }); + + it('allows adding a brand-new unused VLAN', () => { + const change: PlannedChange = { + kind: 'vlan.add', bridge: 'bridge', vlanId: 4000, tagged: [], untagged: [], + }; + expect(analyzeChange(baseSnapshot(), device, change).severity).toBe('safe'); + }); +}); + +describe('analyzeChange — real-hardware shapes', () => { + /** + * Taken from the live switch: the ingress port is a TAGGED trunk member of the + * management VLAN while also accepting untagged frames via its PVID, and the + * bridge's untagged membership comes from a dynamic "added by pvid" row + * alongside the static one. An earlier version demanded the ingress port be + * *untagged*, which flagged this healthy config and — because pre-existing + * violations are not re-reported — silently suppressed real detections. + */ + function trunkSnapshot(): DeviceSnapshot { + return baseSnapshot({ + bridgeVlans: [ + { bridge: 'bridge', 'vlan-ids': '1', tagged: 'all', untagged: 'bridge', + 'current-tagged': 'sfp28-1', 'current-untagged': 'bridge', dynamic: 'false' }, + { bridge: 'bridge', 'vlan-ids': '1', tagged: '', untagged: 'bridge', + 'current-tagged': '', 'current-untagged': 'bridge', dynamic: 'true', comment: 'added by pvid' }, + ], + }); + } + + it('treats a tagged trunk ingress port with a matching PVID as healthy', () => { + const v = analyzeChange(trunkSnapshot(), device, { + kind: 'vlan.add', bridge: 'bridge', vlanId: 4001, tagged: [], untagged: [], + }); + expect(v.severity).toBe('safe'); + expect(v.preexisting).toHaveLength(0); + }); + + it('warns when the bridge would keep membership only via the dynamic PVID row', () => { + // Rewriting VLAN 1 drops the static row; the dynamic "added by pvid" row + // survives, so management still works — but only by accident. + const v = analyzeChange(trunkSnapshot(), device, { + kind: 'vlan.update', bridge: 'bridge', vlanId: 1, tagged: [], untagged: ['sfp28-1'], + }); + expect(v.severity).toBe('warning'); + expect(v.violations.map((x) => x.id)).toContain('bridge-untagged-member'); + expect(v.violations[0].severity).toBe('warning'); + expect(v.violations[0].detail).toMatch(/dynamic/i); + }); + + it('is critical when no dynamic row would keep the bridge in the VLAN', () => { + const snap = baseSnapshot({ + bridgeVlans: [ + { bridge: 'bridge', 'vlan-ids': '1', tagged: 'sfp28-1', untagged: 'bridge', + 'current-tagged': 'sfp28-1', 'current-untagged': 'bridge', dynamic: 'false' }, + ], + }); + const v = analyzeChange(snap, device, { + kind: 'vlan.update', bridge: 'bridge', vlanId: 1, tagged: ['sfp28-1'], untagged: [], + }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('bridge-untagged-member'); + }); +}); + +describe('analyzeChange — enabling vlan-filtering', () => { + it('is safe when the bridge is already an untagged member of the management VLAN', () => { + const snap = baseSnapshot({ bridges: [{ name: 'bridge', 'vlan-filtering': 'false', pvid: '1' }] }); + const v = analyzeChange(snap, device, { kind: 'bridge.vlan-filtering', bridge: 'bridge', enabled: true }); + expect(v.severity).toBe('safe'); + }); + + it('is critical when the bridge is missing from the management VLAN', () => { + // vlan-filtering currently off, and VLAN 1's entry omits the bridge — the + // classic "enable filtering, lose the device" case. + const snap = baseSnapshot({ + bridges: [{ name: 'bridge', 'vlan-filtering': 'false', pvid: '1' }], + bridgeVlans: [ + { bridge: 'bridge', 'vlan-ids': '1', tagged: '', untagged: 'sfp28-1,ether1', + 'current-tagged': '', 'current-untagged': 'sfp28-1,ether1' }, + ], + }); + const v = analyzeChange(snap, device, { kind: 'bridge.vlan-filtering', bridge: 'bridge', enabled: true }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('bridge-untagged-member'); + }); +}); + +describe('analyzeChange — other management-path cuts', () => { + it('flags removing the management IP', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'ip.remove', addressId: '*1' }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('mgmt-ip-present'); + }); + + it('flags disabling the ingress port', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'interface.disable', name: 'sfp28-1', disabled: true }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('ingress-port-member'); + }); + + it('allows disabling an unrelated port', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'interface.disable', name: 'ether1', disabled: true }); + expect(v.severity).toBe('safe'); + }); + + it('flags disabling the API service the manager uses', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'service.toggle', serviceId: '*1', disabled: true }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('mgmt-service-open'); + }); + + it('flags removing the default route the manager arrives through', () => { + const v = analyzeChange(baseSnapshot(), device, { kind: 'route.remove', routeId: '*A' }); + expect(v.severity).toBe('critical'); + expect(v.violations.map((x) => x.id)).toContain('route-to-manager'); + }); + + it('flags deleting the bond that carries management', () => { + const snap = baseSnapshot({ + interfaces: [ + { name: 'bridge', type: 'bridge', disabled: 'false' }, + { name: 'bond1', type: 'bond', slaves: 'sfp28-1,sfp28-2', disabled: 'false' }, + ], + bridgePorts: [{ interface: 'bond1', bridge: 'bridge', pvid: '1', disabled: 'false' }], + bridgeVlans: [ + { bridge: 'bridge', 'vlan-ids': '1', tagged: '', untagged: 'bridge,bond1', + 'current-tagged': '', 'current-untagged': 'bridge,bond1' }, + ], + bridgeHosts: [{ 'mac-address': 'B4:FB:E4:0B:0B:8A', 'on-interface': 'bond1', local: 'false' }], + }); + expect(resolveManagementPath(snap, device).ingressBond).toBe('bond1'); + const v = analyzeChange(snap, device, { kind: 'bond.delete', name: 'bond1' }); + expect(v.severity).toBe('critical'); + }); + + it('does not fire when nothing on the path changes', () => { + const snap = baseSnapshot(); + expect(simulate(snap, { kind: 'vlan.add', bridge: 'bridge', vlanId: 4000, tagged: [], untagged: [] }).bridgeVlans) + .toHaveLength(3); + expect(analyzeChange(snap, device, { kind: 'interface.disable', name: 'ether1', disabled: false }).severity) + .toBe('safe'); + }); +}); diff --git a/backend/src/services/changeGuard/analyzeChange.ts b/backend/src/services/changeGuard/analyzeChange.ts new file mode 100644 index 0000000..5e12d37 --- /dev/null +++ b/backend/src/services/changeGuard/analyzeChange.ts @@ -0,0 +1,223 @@ +/** + * Change analysis — "will this change cut the manager off from the device?" + * + * The approach is deliberately a simulation rather than pattern-matching on the + * request: apply the planned change to an in-memory copy of the live device state, + * then re-check the management-path invariants. An invariant that was satisfied + * before and violated after is a predicted lockout, and the invariant explains + * itself, so the warning names the actual mechanism instead of being generic. + */ +import { evaluate, INVARIANTS, type InvariantStatus } from './invariants'; +import { resolveManagementPath, expandVlanIds, type DeviceSnapshot, type ManagementPath, type RosRow } from './pathModel'; +import type { GuardDevice } from './ChangeGuard'; + +export type PlannedChange = + | { kind: 'bridge.vlan-filtering'; bridge: string; enabled: boolean } + | { kind: 'port.vlan'; port: string; pvid: number; tagged: number[]; untagged: number[] } + | { kind: 'vlan.add'; bridge: string; vlanId: number; tagged: string[]; untagged: string[] } + | { kind: 'vlan.update'; bridge: string; vlanId: number; tagged: string[]; untagged: string[] } + | { kind: 'vlan.delete'; bridge: string; vlanId: number } + | { kind: 'ip.remove'; addressId: string } + | { kind: 'interface.disable'; name: string; disabled: boolean } + | { kind: 'bond.delete'; name: string } + | { kind: 'route.remove'; routeId: string } + | { kind: 'service.toggle'; serviceId: string; disabled: boolean }; + +export type Severity = 'safe' | 'warning' | 'critical'; + +export interface ChangeVerdict { + severity: Severity; + /** One-line summary suitable for a dialog heading. */ + headline: string; + /** Invariants that flipped satisfied → violated. */ + violations: InvariantStatus[]; + /** + * Invariants already violated before the change. Not caused by it, but reported + * because they indicate the device is in a fragile state — and because a + * pre-existing violation means that check cannot detect a new break. + */ + preexisting: InvariantStatus[]; + /** Non-fatal concerns (including reduced confidence in the analysis itself). */ + warnings: string[]; + /** The management path the verdict was computed against, for explainability. */ + path: ManagementPath; +} + +const clone = (snap: DeviceSnapshot): DeviceSnapshot => + JSON.parse(JSON.stringify(snap)) as DeviceSnapshot; + +const csv = (v: string | undefined): string[] => + (v || '').split(',').map((s) => s.trim()).filter(Boolean); + +const join = (list: string[]): string => [...new Set(list)].filter(Boolean).join(','); + +const isDynamic = (row: RosRow): boolean => row['dynamic'] === 'true' || row['dynamic'] === 'yes'; + +/** Rewrite one VLAN row's membership, preserving the effective (`current-*`) view. */ +function setMembership(row: RosRow, tagged: string[], untagged: string[]): void { + row['tagged'] = join(tagged); + row['untagged'] = join(untagged); + row['current-tagged'] = row['tagged']; + row['current-untagged'] = row['untagged']; +} + +/** + * Apply a planned change to a copy of the snapshot. Only the fields the invariants + * read need to be accurate — this is a reachability model, not a RouterOS emulator. + */ +export function simulate(snap: DeviceSnapshot, change: PlannedChange): DeviceSnapshot { + const s = clone(snap); + + switch (change.kind) { + case 'bridge.vlan-filtering': { + const b = s.bridges.find((x) => x['name'] === change.bridge); + if (b) b['vlan-filtering'] = change.enabled ? 'true' : 'false'; + break; + } + + case 'port.vlan': { + const port = s.bridgePorts.find((p) => p['interface'] === change.port); + if (port) port['pvid'] = String(change.pvid); + const bridge = port?.['bridge']; + if (!bridge) break; + + // setPortVlanConfig only rewrites rows for the VLANs actually listed in the + // request, and silently skips VLANs that have no row. Crucially it does NOT + // strip the port from VLANs the caller didn't mention — modelling it as a + // full replacement would invent violations that the real change never causes. + for (const row of s.bridgeVlans) { + if (row['bridge'] !== bridge) continue; + const vids = expandVlanIds(row['vlan-ids']); + const wantTagged = vids.some((v) => change.tagged.includes(v)); + const wantUntagged = vids.some((v) => change.untagged.includes(v)); + if (!wantTagged && !wantUntagged) continue; + + const tagged = csv(row['current-tagged'] ?? row['tagged']).filter((p) => p !== change.port); + const untagged = csv(row['current-untagged'] ?? row['untagged']).filter((p) => p !== change.port); + if (wantTagged) tagged.push(change.port); + if (wantUntagged) untagged.push(change.port); + setMembership(row, tagged, untagged); + } + break; + } + + // updateBridgeVlan is remove-then-add, and removeBridgeVlan removes EVERY row + // matching the VID — not just the first. Dynamic rows survive, because RouterOS + // refuses to delete them and the collector swallows that error. + case 'vlan.add': + case 'vlan.update': { + s.bridgeVlans = s.bridgeVlans.filter( + (r) => !(r['bridge'] === change.bridge + && expandVlanIds(r['vlan-ids']).includes(change.vlanId) + && !isDynamic(r)) + ); + const row: RosRow = { bridge: change.bridge, 'vlan-ids': String(change.vlanId), dynamic: 'false' }; + setMembership(row, change.tagged, change.untagged); + s.bridgeVlans.push(row); + break; + } + + case 'vlan.delete': { + s.bridgeVlans = s.bridgeVlans.filter( + (r) => !(r['bridge'] === change.bridge + && expandVlanIds(r['vlan-ids']).includes(change.vlanId) + && !isDynamic(r)) + ); + break; + } + + case 'ip.remove': { + s.addresses = s.addresses.filter((a) => a['.id'] !== change.addressId); + break; + } + + case 'interface.disable': { + const iface = s.interfaces.find((i) => i['name'] === change.name); + if (iface) iface['disabled'] = change.disabled ? 'true' : 'false'; + break; + } + + case 'bond.delete': { + s.interfaces = s.interfaces.filter((i) => i['name'] !== change.name); + s.bridgePorts = s.bridgePorts.filter((p) => p['interface'] !== change.name); + break; + } + + case 'route.remove': { + s.routes = s.routes.filter((r) => r['.id'] !== change.routeId); + break; + } + + case 'service.toggle': { + const svc = s.services.find((x) => x['.id'] === change.serviceId || x['name'] === change.serviceId); + if (svc) svc['disabled'] = change.disabled ? 'true' : 'false'; + break; + } + } + + return s; +} + +/** + * Compare invariants before and after. The management path is resolved once, on the + * *current* state — the question is whether the path the manager is using right now + * survives, not what a hypothetical new path might look like. + */ +export function analyzeChange( + snap: DeviceSnapshot, + device: GuardDevice, + change: PlannedChange +): ChangeVerdict { + const path = resolveManagementPath(snap, device); + const before = evaluate(snap, path, device); + const after = evaluate(simulate(snap, change), path, device); + + const violations: InvariantStatus[] = []; + const preexisting: InvariantStatus[] = []; + for (const inv of INVARIANTS) { + const b = before.get(inv.id); + const a = after.get(inv.id); + if (b && !b.ok) { + preexisting.push({ + id: inv.id, + title: inv.title, + before: false, + after: !!a?.ok, + detail: b.detail, + severity: b.severity ?? 'critical', + }); + continue; // already broken; a "new" break can't be attributed to this change + } + if (b?.ok && a && !a.ok) { + violations.push({ + id: inv.id, + title: inv.title, + before: true, + after: false, + detail: a.detail, + severity: a.severity ?? 'critical', + }); + } + } + + const warnings = [...path.warnings]; + for (const p of preexisting) { + warnings.push(`Pre-existing issue (not caused by this change): ${p.detail}`); + } + // Say so plainly when the analysis itself is running blind, rather than implying + // a clean bill of health. + if (path.ingressPortSource === 'unknown' && path.bridge) { + warnings.push('The ingress port could not be determined, so port-level checks were skipped — treat a "safe" result here with caution.'); + } + + const hasCritical = violations.some((v) => v.severity === 'critical'); + const severity: Severity = hasCritical ? 'critical' : violations.length > 0 ? 'warning' : 'safe'; + const headline = + severity === 'critical' + ? `This change is predicted to cut management access to ${device.name}.` + : severity === 'warning' + ? `This change leaves management on ${device.name} in a fragile state.` + : `No management-path problem predicted for ${device.name}.`; + + return { severity, headline, violations, preexisting, warnings, path }; +} diff --git a/backend/src/services/changeGuard/invariants.ts b/backend/src/services/changeGuard/invariants.ts new file mode 100644 index 0000000..8a5cf2e --- /dev/null +++ b/backend/src/services/changeGuard/invariants.ts @@ -0,0 +1,292 @@ +/** + * Management-path invariants — the conditions that must hold for the manager to + * keep reaching a device. + * + * Checking invariants beats maintaining a blocklist of "dangerous operations": + * a blocklist only catches the change shapes someone thought of, which is exactly + * how deleting an *unused* bond and editing a VLAN both slipped through. Here we + * state what must remain true, then re-evaluate it against the post-change state, + * so novel change shapes are covered for free. + * + * Rules follow MikroTik's own documentation: the bridge interface is the CPU port + * and must be a tagged or untagged member of the management VLAN once + * vlan-filtering is on, and PVID-derived untagged membership is added dynamically + * (which is why effective `current-*` membership is what we test). + */ +import { detectLockoutRisk } from '../../utils/firewallSafety'; +import { vlanMembership, type DeviceSnapshot, type ManagementPath } from './pathModel'; +import type { GuardDevice } from './ChangeGuard'; + +export interface InvariantResult { + ok: boolean; + /** Explanation shown to the user when this flips to violated. */ + detail: string; + /** + * How bad a violation is. 'critical' = management is predicted to break. + * 'warning' = it would survive only on a fragile assumption (e.g. a + * dynamically-added PVID entry that a later change could remove). + */ + severity?: 'critical' | 'warning'; +} + +export interface Invariant { + id: string; + title: string; + check(snap: DeviceSnapshot, path: ManagementPath, device: GuardDevice): InvariantResult; +} + +const isTrue = (v: string | undefined): boolean => v === 'true' || v === 'yes'; +const stripCidr = (a: string): string => (a || '').split('/')[0].trim(); +const ok = (detail = ''): InvariantResult => ({ ok: true, detail }); + +/** + * Read vlan-filtering from the snapshot being evaluated, never from the resolved + * path. The path is frozen at the pre-change state, so keying off it would make + * "enable vlan-filtering" — the most important case to predict — invisible. + */ +const filteringOn = (snap: DeviceSnapshot, bridge: string | null): boolean => + !!bridge && isTrue(snap.bridges.find((b) => b['name'] === bridge)?.['vlan-filtering']); + +export const INVARIANTS: Invariant[] = [ + { + id: 'mgmt-ip-present', + title: 'The management address still exists on the device', + check(snap, path) { + const found = snap.addresses.some( + (a) => !isTrue(a['disabled']) && !isTrue(a['invalid']) && stripCidr(a['address']) === path.mgmtIp + ); + return found + ? ok() + : { ok: false, detail: `No enabled IP address ${path.mgmtIp} would remain on the device — the manager connects to exactly that address.` }; + }, + }, + + { + id: 'mgmt-iface-up', + title: 'The interface holding the management address is enabled', + check(snap, path) { + if (!path.mgmtInterface) return ok(); + const row = snap.interfaces.find((i) => i['name'] === path.mgmtInterface); + if (!row) { + return { ok: false, detail: `Interface ${path.mgmtInterface}, which holds the management address, would no longer exist.` }; + } + return isTrue(row['disabled']) + ? { ok: false, detail: `Interface ${path.mgmtInterface} holds the management address and would be disabled.` } + : ok(); + }, + }, + + { + id: 'vlan-iface-tagged', + title: 'The bridge is a tagged member of the management VLAN', + check(snap, path) { + // Only applies when management rides a tagged VLAN sub-interface on a + // VLAN-filtering bridge. + if (!path.taggedManagement || !path.bridge || !filteringOn(snap, path.bridge) || path.mgmtVlanId == null) { + return ok(); + } + const m = vlanMembership(snap, path.bridge, path.mgmtVlanId); + if (!m.found) { + return { + ok: false, + detail: `VLAN ${path.mgmtVlanId} would have no entry in ${path.bridge}'s VLAN table, so the bridge (the CPU port) could not receive tagged management traffic.`, + }; + } + return m.tagged.includes(path.bridge) + ? ok() + : { + ok: false, + detail: `${path.bridge} would not be a tagged member of VLAN ${path.mgmtVlanId}. Management arrives tagged on that VLAN, and the bridge is the CPU port — without it, traffic never reaches the device's CPU.`, + }; + }, + }, + + { + id: 'bridge-untagged-member', + title: 'The bridge and ingress port are untagged members of the management VLAN', + check(snap, path) { + if (path.taggedManagement || !path.bridge || !filteringOn(snap, path.bridge) || path.mgmtVlanId == null) { + return ok(); + } + const vid = path.mgmtVlanId; + const m = vlanMembership(snap, path.bridge, vid); + + // No entry for the management VLAN. RouterOS *may* re-add untagged membership + // dynamically from the PVID, but that depends on frame-types and is exactly + // the fragile assumption that strands devices — so treat it as a violation. + // A device that already had no entry stays quiet: analyzeChange only reports + // invariants that FLIP, so this fires on removal, not on pre-existing state. + if (!m.found) { + return { + ok: false, + detail: `VLAN ${vid} would have no entry in ${path.bridge}'s VLAN table. Management arrives untagged on that VLAN, and relying on RouterOS to re-add the bridge dynamically is not safe once VLAN filtering is on.`, + }; + } + + if (!m.untagged.includes(path.bridge)) { + return { + ok: false, + severity: 'critical', + detail: `${path.bridge} would not be an untagged member of VLAN ${vid}. Management arrives untagged and the bridge is the CPU port — this is the single most common way to lock a MikroTik out of management.`, + }; + } + + // Present, but only because RouterOS auto-added it from the PVID. That entry + // disappears if the PVID or frame-types change, so management would be + // resting on something the operator did not configure. + if (!m.untaggedStatic.includes(path.bridge)) { + return { + ok: false, + severity: 'warning', + detail: `${path.bridge} would remain an untagged member of VLAN ${vid} only through a dynamic "added by pvid" entry, not a configured one. Management would keep working for now, but any later change to the bridge PVID or frame-types would remove it. Add ${path.bridge} to VLAN ${vid} explicitly.`, + }; + } + + // The ingress port must be a member of the VLAN, but it need NOT be an + // untagged one: a tagged trunk that also accepts untagged frames via its PVID + // is a normal and correct configuration. Requiring "untagged" here produced a + // false positive on real hardware — and because a violation that is already + // present before a change is (rightly) not reported as a new one, that false + // positive silently suppressed genuine detections. + if (path.ingressPort && !m.untagged.includes(path.ingressPort) && !m.tagged.includes(path.ingressPort)) { + return { + ok: false, + severity: 'critical', + detail: `${path.ingressPort} — the port management traffic arrives on — would not be a member of VLAN ${vid} at all.`, + }; + } + return ok(); + }, + }, + + { + id: 'ingress-port-pvid', + title: 'The ingress port still stamps the management VLAN', + check(snap, path) { + if (path.taggedManagement || !path.bridge || !filteringOn(snap, path.bridge)) return ok(); + if (!path.ingressPort || path.mgmtVlanId == null) return ok(); + const port = snap.bridgePorts.find( + (p) => p['interface'] === path.ingressPort && p['bridge'] === path.bridge + ); + if (!port) return ok(); // covered by ingress-port-member + const pvid = parseInt(port['pvid'] || '', 10); + return isNaN(pvid) || pvid === path.mgmtVlanId + ? ok() + : { + ok: false, + detail: `${path.ingressPort} would stamp untagged traffic with VLAN ${pvid} instead of the management VLAN ${path.mgmtVlanId}, moving management into a VLAN the CPU does not receive.`, + }; + }, + }, + + { + id: 'ingress-port-member', + title: 'The ingress port is still a member of the bridge and enabled', + check(snap, path) { + if (!path.ingressPort || !path.bridge) return ok(); + const member = snap.bridgePorts.some( + (p) => p['interface'] === path.ingressPort && p['bridge'] === path.bridge && !isTrue(p['disabled']) + ); + if (!member) { + return { + ok: false, + detail: `${path.ingressPort} would no longer be an enabled member of ${path.bridge}, and that is the port management traffic arrives on.`, + }; + } + const iface = snap.interfaces.find((i) => i['name'] === path.ingressPort); + return iface && isTrue(iface['disabled']) + ? { ok: false, detail: `${path.ingressPort} would be disabled, and that is the port management traffic arrives on.` } + : ok(); + }, + }, + + { + id: 'bond-has-slaves', + title: 'The bond carrying management still has members', + check(snap, path) { + if (!path.ingressBond) return ok(); + const bond = snap.interfaces.find((i) => i['name'] === path.ingressBond); + if (!bond) { + return { ok: false, detail: `Bond ${path.ingressBond} carries management traffic and would no longer exist.` }; + } + const slaves = (bond['slaves'] || '').split(',').map((s) => s.trim()).filter(Boolean); + return slaves.length > 0 + ? ok() + : { ok: false, detail: `Bond ${path.ingressBond} carries management traffic and would be left with no member ports.` }; + }, + }, + + { + id: 'mgmt-service-open', + title: 'The API service the manager uses stays enabled on its port', + check(snap, path, device) { + const wanted = device.api_port === 8729 ? 'api-ssl' : 'api'; + const svc = snap.services.find((s) => s['name'] === wanted); + if (!svc) return ok(); + if (isTrue(svc['disabled'])) { + return { ok: false, detail: `The ${wanted} service would be disabled — that is the service the manager connects through.` }; + } + const port = parseInt(svc['port'] || '', 10); + return !isNaN(port) && port !== device.api_port + ? { ok: false, detail: `The ${wanted} service would move to port ${port}, but the manager connects on ${device.api_port}.` } + : ok(); + }, + }, + + { + id: 'input-chain-permits', + title: 'No input-chain rule blocks the management port', + check(snap, path, device) { + for (const rule of snap.firewallFilter) { + const risk = detectLockoutRisk(rule, { mgmtPorts: [device.api_port] }); + if (risk.risky) { + return { ok: false, detail: `A firewall rule would block management access: ${risk.reason}` }; + } + } + return ok(); + }, + }, + + { + id: 'route-to-manager', + title: 'A route back toward the manager still exists', + check(snap, path) { + // Only meaningful when the manager is reached via a gateway rather than + // being on the same subnet. + if (path.ingressPortSource !== 'fdb-default-gw') return ok(); + const hasDefault = snap.routes.some( + (r) => (r['dst-address'] || '') === '0.0.0.0/0' && r['active'] !== 'false' && !isTrue(r['disabled']) + ); + return hasDefault + ? ok() + : { ok: false, detail: 'The default route would be removed, and the manager reaches this device through the default gateway.' }; + }, + }, +]; + +export interface InvariantStatus { + id: string; + title: string; + before: boolean; + after: boolean; + detail: string; + severity: 'critical' | 'warning'; +} + +/** Evaluate every invariant against a snapshot for a fixed management path. */ +export function evaluate( + snap: DeviceSnapshot, + path: ManagementPath, + device: GuardDevice +): Map { + const out = new Map(); + for (const inv of INVARIANTS) { + try { + out.set(inv.id, inv.check(snap, path, device)); + } catch (err) { + // A broken check must never block a legitimate change. + out.set(inv.id, { ok: true, detail: `check skipped: ${(err as Error).message}` }); + } + } + return out; +} diff --git a/backend/src/services/changeGuard/pathModel.ts b/backend/src/services/changeGuard/pathModel.ts new file mode 100644 index 0000000..915712e --- /dev/null +++ b/backend/src/services/changeGuard/pathModel.ts @@ -0,0 +1,424 @@ +/** + * Management-path model — what the manager's reachability to a device actually + * depends on. + * + * RouterOS performs no cross-object validation: it will happily accept a VLAN, + * bridge or address change that strands its own CPU port, and MikroTik's official + * advice for the most common case is "configure VLAN filtering while using a serial + * console". This module derives, from live device state, the chain of objects the + * management session rides on, so a change can be checked against it beforehand. + * + * Two things make the result precise rather than merely conservative: + * + * - The manager's source IP *as the device sees it* is read from the device's own + * connection tracking (the live API session shows up there), instead of being + * treated as unknowable behind NAT. + * - The ingress port is taken from the bridge host (FDB) table — the port where + * the relevant MAC is actually learned — rather than guessed from topology. + * + * Capture and resolution are deliberately separate so resolution stays a pure + * function over a snapshot and can be unit-tested against fixtures. + */ +import { RouterOSClient } from '../mikrotik/RouterOSClient'; +import { decrypt } from '../../utils/crypto'; +import type { GuardDevice } from './ChangeGuard'; + +export type RosRow = Record; + +export interface DeviceSnapshot { + addresses: RosRow[]; + interfaces: RosRow[]; + vlanInterfaces: RosRow[]; + bridges: RosRow[]; + bridgePorts: RosRow[]; + bridgeVlans: RosRow[]; + routes: RosRow[]; + arp: RosRow[]; + bridgeHosts: RosRow[]; + services: RosRow[]; + firewallFilter: RosRow[]; + /** TCP connections to the management port; reveals the manager's apparent IP. */ + mgmtConnections: RosRow[]; +} + +export type HopKind = 'address' | 'vlan-interface' | 'bridge' | 'bond' | 'port'; + +export interface PathHop { + kind: HopKind; + name: string; + /** Why this object is on the management path — surfaced verbatim in the UI. */ + reason: string; +} + +export interface ManagementPath { + mgmtIp: string; + /** Interface that owns the management address. */ + mgmtInterface: string | null; + /** Bridge in the path, if the management interface is (or sits on) one. */ + bridge: string | null; + bridgeVlanFiltering: boolean; + /** VLAN the management traffic rides in, when VLAN-aware. */ + mgmtVlanId: number | null; + /** True when management arrives tagged (an /interface/vlan), false when untagged. */ + taggedManagement: boolean; + /** Port the manager's traffic actually enters on. */ + ingressPort: string | null; + /** + * How the ingress port was determined: + * fdb — the manager's own next-hop MAC, observed in the forwarding table + * fdb-default-gw — the default gateway's MAC in the forwarding table (used when the + * manager's address isn't observable; correct whenever it is off-subnet) + * inferred — from ARP alone, not confirmed by the forwarding table + * unknown — could not be determined; callers must fall back to conservative checks + */ + ingressPortSource: 'fdb' | 'fdb-default-gw' | 'inferred' | 'unknown'; + /** Manager's source address as the device sees it (may be a NAT gateway). */ + managerIp: string | null; + /** Bond backing the ingress port, if any. */ + ingressBond: string | null; + hops: PathHop[]; + /** Things we could not determine; callers should degrade to conservative checks. */ + warnings: string[]; +} + +const empty = (): RosRow[] => []; + +/** Read everything the analysis needs in one pass. All over the API — no SSH. */ +export async function captureSnapshot(device: GuardDevice): Promise { + const client = new RouterOSClient( + device.ip_address, + device.api_port, + device.api_username, + decrypt(device.api_password_encrypted), + 15_000, + 30_000 + ); + try { + await client.connect(); + const run = (cmd: string, params: Record = {}) => + client.execute(cmd, params).catch(empty); + + const [ + addresses, interfaces, vlanInterfaces, bridges, bridgePorts, + bridgeVlans, routes, arp, bridgeHosts, services, firewallFilter, mgmtConnections, + ] = await Promise.all([ + run('/ip/address/print', { detail: '' }), + run('/interface/print', { detail: '' }), + run('/interface/vlan/print', { detail: '' }), + run('/interface/bridge/print', { detail: '' }), + run('/interface/bridge/port/print', { detail: '' }), + run('/interface/bridge/vlan/print', { detail: '' }), + run('/ip/route/print', { detail: '' }), + run('/ip/arp/print'), + run('/interface/bridge/host/print'), + run('/ip/service/print'), + run('/ip/firewall/filter/print', { detail: '' }), + // Connection tracking may be disabled; treated as optional. + run('/ip/firewall/connection/print', { detail: '' }), + ]); + + return { + addresses, interfaces, vlanInterfaces, bridges, bridgePorts, + bridgeVlans, routes, arp, bridgeHosts, services, firewallFilter, mgmtConnections, + }; + } finally { + client.disconnect(); + } +} + +// ─── helpers ────────────────────────────────────────────────────────────────── + +const stripCidr = (addr: string): string => (addr || '').split('/')[0].trim(); +const isTrue = (v: string | undefined): boolean => v === 'true' || v === 'yes'; +const csv = (v: string | undefined): string[] => + (v || '').split(',').map((s) => s.trim()).filter(Boolean); + +/** RouterOS vlan-ids may be a list and/or ranges: "10", "10,20", "10-12". */ +export function expandVlanIds(spec: string | undefined): number[] { + const out: number[] = []; + for (const part of csv(spec)) { + const range = part.match(/^(\d+)\s*-\s*(\d+)$/); + if (range) { + const from = parseInt(range[1], 10); + const to = parseInt(range[2], 10); + if (!isNaN(from) && !isNaN(to) && to >= from) { + for (let v = from; v <= to; v++) out.push(v); + } + continue; + } + const n = parseInt(part, 10); + if (!isNaN(n)) out.push(n); + } + return out; +} + +/** + * Effective membership for a VLAN on a bridge. RouterOS exposes `current-tagged` + * / `current-untagged`, which include dynamically-added ports (notably the PVID- + * derived untagged entries); those reflect what the switch is really enforcing, so + * they win over the statically configured lists when present. + */ +export function vlanMembership( + snap: DeviceSnapshot, + bridge: string, + vlanId: number +): { + tagged: string[]; + untagged: string[]; + found: boolean; + /** Membership contributed by operator-configured (static) rows only. */ + untaggedStatic: string[]; + taggedStatic: string[]; +} { + const tagged = new Set(); + const untagged = new Set(); + const taggedStatic = new Set(); + const untaggedStatic = new Set(); + let found = false; + + for (const row of snap.bridgeVlans) { + if (row['bridge'] !== bridge) continue; + if (!expandVlanIds(row['vlan-ids']).includes(vlanId)) continue; + found = true; + const t = row['current-tagged'] !== undefined ? row['current-tagged'] : row['tagged']; + const u = row['current-untagged'] !== undefined ? row['current-untagged'] : row['untagged']; + const isDynamic = isTrue(row['dynamic']); + for (const p of csv(t)) { tagged.add(p); if (!isDynamic) taggedStatic.add(p); } + for (const p of csv(u)) { untagged.add(p); if (!isDynamic) untaggedStatic.add(p); } + } + return { + tagged: [...tagged], + untagged: [...untagged], + found, + taggedStatic: [...taggedStatic], + untaggedStatic: [...untaggedStatic], + }; +} + +/** Longest-prefix-ish match: the active route the device would use for `ip`. */ +function routeFor(snap: DeviceSnapshot, ip: string): RosRow | null { + let best: RosRow | null = null; + let bestLen = -1; + for (const r of snap.routes) { + if (isTrue(r['disabled']) || r['active'] === 'false') continue; + const dst = r['dst-address'] || ''; + const [net, lenStr] = dst.split('/'); + const len = parseInt(lenStr ?? '32', 10); + if (isNaN(len)) continue; + if (len === 0 || inNetwork(ip, net, len)) { + if (len > bestLen) { best = r; bestLen = len; } + } + } + return best; +} + +function ipToInt(ip: string): number | null { + const parts = ip.split('.').map(Number); + if (parts.length !== 4 || parts.some((p) => isNaN(p) || p < 0 || p > 255)) return null; + return ((parts[0] << 24) >>> 0) + (parts[1] << 16) + (parts[2] << 8) + parts[3]; +} + +function inNetwork(ip: string, net: string, prefixLen: number): boolean { + const a = ipToInt(ip); + const b = ipToInt(net); + if (a === null || b === null) return false; + if (prefixLen <= 0) return true; + const mask = prefixLen >= 32 ? 0xffffffff : ((0xffffffff << (32 - prefixLen)) >>> 0); + return ((a & mask) >>> 0) === ((b & mask) >>> 0); +} + +/** + * The manager's address as the device sees it, taken from connection tracking on + * the management port. Returns null when conntrack is unavailable. + */ +export function managerIpFromConntrack(snap: DeviceSnapshot, apiPort: number): string | null { + for (const c of snap.mgmtConnections) { + if ((c['protocol'] || '').toLowerCase() !== 'tcp') continue; + const dst = c['dst-address'] || ''; + const port = dst.includes(':') ? parseInt(dst.split(':').pop() || '', 10) : NaN; + if (port !== apiPort) continue; + const src = (c['src-address'] || '').split(':')[0]; + if (src) return src; + } + return null; +} + +// ─── resolution ─────────────────────────────────────────────────────────────── + +/** + * Derive the management path. Pure over the snapshot so it can be exercised with + * fixtures; every hop carries the reason it is load-bearing. + */ +export function resolveManagementPath(snap: DeviceSnapshot, device: GuardDevice): ManagementPath { + const hops: PathHop[] = []; + const warnings: string[] = []; + const mgmtIp = device.ip_address; + + // 1. Which address object, and therefore which interface, serves the manager? + const addrRow = snap.addresses.find( + (a) => !isTrue(a['disabled']) && !isTrue(a['invalid']) && stripCidr(a['address']) === mgmtIp + ); + const mgmtInterface = addrRow + ? (addrRow['actual-interface'] || addrRow['interface'] || null) + : null; + + if (!addrRow) { + warnings.push( + `No enabled /ip/address on the device matches ${mgmtIp}; the manager may be reaching it via NAT or a dynamic address.` + ); + } else { + hops.push({ + kind: 'address', + name: addrRow['address'], + reason: `${addrRow['address']} is the address the manager connects to, on interface ${mgmtInterface}.`, + }); + } + + // 2. Classify that interface and walk down to the bridge, if any. + let bridge: string | null = null; + let mgmtVlanId: number | null = null; + let taggedManagement = false; + + if (mgmtInterface) { + const vlanIface = + snap.vlanInterfaces.find((v) => v['name'] === mgmtInterface) ?? + snap.interfaces.find((i) => i['name'] === mgmtInterface && i['type'] === 'vlan'); + + if (vlanIface) { + // Management rides a tagged VLAN sub-interface. + taggedManagement = true; + const vid = parseInt(vlanIface['vlan-id'] || '', 10); + mgmtVlanId = isNaN(vid) ? null : vid; + const parent = vlanIface['interface'] || null; + hops.push({ + kind: 'vlan-interface', + name: mgmtInterface, + reason: `${mgmtInterface} is a VLAN interface carrying VLAN ${mgmtVlanId ?? '?'} on top of ${parent ?? 'an unknown parent'}.`, + }); + if (parent && snap.bridges.some((b) => b['name'] === parent)) bridge = parent; + else if (parent) { + hops.push({ kind: 'port', name: parent, reason: `${parent} is the physical parent of ${mgmtInterface}.` }); + } + } else if (snap.bridges.some((b) => b['name'] === mgmtInterface)) { + bridge = mgmtInterface; + } else { + hops.push({ + kind: 'port', + name: mgmtInterface, + reason: `${mgmtInterface} directly holds the management address.`, + }); + } + } + + const bridgeRow = bridge ? snap.bridges.find((b) => b['name'] === bridge) : undefined; + const bridgeVlanFiltering = isTrue(bridgeRow?.['vlan-filtering']); + if (bridge) { + hops.push({ + kind: 'bridge', + name: bridge, + reason: bridgeVlanFiltering + ? `${bridge} is the bridge (the CPU port) carrying management, with VLAN filtering ON — it must be a member of the management VLAN.` + : `${bridge} is the bridge (the CPU port) carrying management.`, + }); + } + + // 3. Where does the manager's traffic physically enter? + const managerIp = managerIpFromConntrack(snap, device.api_port); + + // Which next-hop MAC carries the management conversation? + // - manager observed and on-subnet → the manager's own address + // - manager observed and off-subnet → the gateway of its route + // - manager not observable (switches commonly run without connection tracking) + // → the default gateway, which is the return path for any off-subnet manager + let lookupIp: string | null = null; + let viaDefaultGw = false; + + if (managerIp) { + const route = routeFor(snap, managerIp); + const gw = route?.['gateway'] ? route['gateway'].split('%')[0].trim() : null; + lookupIp = snap.arp.some((a) => a['address'] === managerIp) ? managerIp : gw; + } else { + const defaultRoute = snap.routes.find( + (r) => (r['dst-address'] || '') === '0.0.0.0/0' && r['active'] !== 'false' && !isTrue(r['disabled']) + ); + const gw = defaultRoute?.['gateway'] ? defaultRoute['gateway'].split('%')[0].trim() : null; + if (gw && ipToInt(gw) !== null) { lookupIp = gw; viaDefaultGw = true; } + } + + let ingressPort: string | null = null; + let ingressPortSource: ManagementPath['ingressPortSource'] = 'unknown'; + + if (lookupIp) { + const arpRow = snap.arp.find((a) => a['address'] === lookupIp); + const mac = arpRow?.['mac-address']; + if (mac) { + const host = snap.bridgeHosts.find( + (h) => (h['mac-address'] || '').toLowerCase() === mac.toLowerCase() && !isTrue(h['local']) + ); + const port = host?.['on-interface'] || host?.['interface'] || null; + if (port) { + ingressPort = port; + ingressPortSource = viaDefaultGw ? 'fdb-default-gw' : 'fdb'; + } else if (arpRow?.['interface'] && arpRow['interface'] !== bridge) { + // ARP resolves to the bridge itself for anything behind it, which is not a + // port — only useful when the address sits on a real interface. + ingressPort = arpRow['interface']; + ingressPortSource = 'inferred'; + } + } + } + + if (ingressPort) { + const why = + ingressPortSource === 'fdb' + ? `that is where the manager's next-hop MAC is currently learned` + : ingressPortSource === 'fdb-default-gw' + ? `that is where the default gateway (${lookupIp}) is currently learned, and the manager reaches this device through it` + : `inferred from the ARP entry; not confirmed by the forwarding table`; + hops.push({ + kind: 'port', + name: ingressPort, + reason: `Management traffic arrives on ${ingressPort} — ${why}.`, + }); + } else if (bridge) { + warnings.push(`Could not determine which port management arrives on; every port of ${bridge} is treated as potentially load-bearing.`); + } + + // Untagged management: the VLAN is whatever PVID the ingress port stamps on. + if (bridge && !taggedManagement) { + const portRow = ingressPort + ? snap.bridgePorts.find((p) => p['interface'] === ingressPort && p['bridge'] === bridge) + : undefined; + const pvid = parseInt(portRow?.['pvid'] || bridgeRow?.['pvid'] || '', 10); + mgmtVlanId = isNaN(pvid) ? null : pvid; + } + + // 4. Is the ingress port actually a bond? + let ingressBond: string | null = null; + if (ingressPort) { + const asBond = snap.interfaces.find((i) => i['name'] === ingressPort && i['type'] === 'bond'); + if (asBond) { + ingressBond = ingressPort; + hops.push({ + kind: 'bond', + name: ingressPort, + reason: `${ingressPort} is a bond; management depends on it keeping at least one running member.`, + }); + } + } + + return { + mgmtIp, + mgmtInterface, + bridge, + bridgeVlanFiltering, + mgmtVlanId, + taggedManagement, + ingressPort, + ingressPortSource, + managerIp, + ingressBond, + hops, + warnings, + }; +} diff --git a/frontend/package.json b/frontend/package.json index 0b936f9..0a554b3 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.18.2-beta", + "version": "0.19.0-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/version.ts b/frontend/src/version.ts index aa1e38d..123eb8d 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.18.2 Beta'; +export const APP_VERSION = 'v0.19.0 Beta';