Security & stability hardening, collector refactor
Full-codebase review remediation: 17 P1 bugs fixed across security and stability, plus the first round of the planned refactors.
Security
- RBAC:
routers:stats,router:statusreplay and/api/localccnow respectallowedRouterIds— restricted viewers no longer receive host/serial/license/version/cpu (or the WAN IP) for routers outside their allowed set (src/index.js) - Auth mode split-brain fixed: new
_authMode()/_isModern()helpers replace six call sites that defaulted to the removed'basic'mode — a falsyauthModecould previously hand full settings (SMTP/Telegram/router detail) to viewer-role users viaGET /api/settings - Error sanitization: unclassified RouterOS connection errors no longer reach the browser as raw
e.message(wireRosEvents,/api/routers/test);sanitizeErr()additionally redacts email addresses and bot tokens from provider errors - Rate limiting:
TRUSTED_PROXY=trueno longer trusts the whole X-Forwarded-For chain (would let clients spoof IPs past the login limiter); doubleauthLimiteron/loginremoved;/healthzreturns only{ok, starting}to unauthenticated callers (Docker healthcheck unaffected) firewall:tabaccepted only from sockets actually viewing the firewall page/card;router:switchrejects disabled routers;routers.jsonwritten mode 0600
Fixed
- Reconnect loop could die permanently — a throwing
connectionError/closelistener escapedconnectLoop's catch and ended retries forever; listeners are now contained (_safeEmit) and allconnectLoop()call sites log instead of leaking unhandled rejections (src/routeros/client.js) - Listener/memory leak per router hot-swap — collector handlers registered on the global Socket.IO server are now tracked and removed in
teardownSession; each leaked handler retained the entire dead session - Hot-swap orphaned all modern-auth sockets — switching or deleting the active router now relocates every socket watching it (previously only legacy no-auth sockets moved, leaving everyone else in a dead room)
- VPN disconnect alerts never fired — the alerter hook only covered
routerIo.emit(), butvpn:updategoes through.to(); room-scoped emits now feed the alerter, and the alert-session stub gained the.to()method whose absence made its VPN collector throw - Phantom wireless clients / stale tables — RStream's empty-array packets (table emptied) are now handled by wireless, talkers, firewall and dhcpNetworks; departed clients age out and cleared tables actually clear; an empty wifi table now latches the legacy-wireless fallback
- Restart-timer leaks defeating idle gating — bare
setTimeoutrestarts in system/connections/ping (and an uncleared overwrite in traffic) are stored and cancelled on stop/suspend; connections gained a_suspendedflag, its watchdog now recovers a dead stream (previously bailed on exactly that state), andresume()no longer reopens the connection-table stream with zero viewers - 12 unhandled promise rejections on stream teardown —
try/catcharoundstream.stop()cannot catch its promise rejection; all sites use a promise-safe teardown now - PDF/report exports crashed on large ranges —
Math.max(...rows)overflowed the call stack above ~65k rows; replaced with a reduce (ping/traffic/bandwidth/connectivity exports) - Credentials could be permanently blanked — an AES-GCM decrypt failure (key mismatch/corruption) no longer causes the next save to overwrite the stored ciphertext with an empty string; the original ciphertext is preserved until a new credential is explicitly set (
src/settings.js,src/routers.js) - Bad
pingTarget/defaultIfbricked connections — validated at save time inRouters.add/update(previously a persisted bad value made every future browser connection throw, surviving restarts) router:switchingwiped every user's UI — now scoped to the outgoing router's room;switchRouter/disable paths drop stale alert-evaluator state; a rejected interface-list fetch is no longer cached forever; firewall's 60 s heartbeat reaches the dashboard card room (stale-badge fix);/healthzreports real ping/ifstatus errors- Traffic
bindSocketis idempotent (listeners no longer stack on hot-swap/router-switch) andstop()releases all socket references
Changed (refactors)
- New
src/collectors/util.js: sharedclampPoll(),stopStreamSafe(),parseBps()/bpsToMbps()— removes ~10 drifting clamp blocks, 23 hand-rolled stream teardowns and both bps-parser copies; Traffic and Interfaces pages now agree on Mbps precision src/index.js: four_update*Streamsfunctions collapsed into one data-driven_updatePageStream(); O(n²) connectivity outage pairing replaced by a single-pass helper- Collector contract sweep:
lastPayloaddeclared everywhere,traffic.pollMs/netwatch.pollMspresent, netwatch heartbeat idle-gated, interfaceStatus reportslastIfStatusErr, advances its health timestamp on unchanged ticks, keepslastPayloadfresh while idle, and recovers monitor-traffic stream errors in 3 s instead of up to 60 s - Docs:
CLAUDE.md/AI_CONTEXT.mdno longer describe the removed Basic Auth; netwatch documented; traffic idle-gating claim corrected
Tests
- New
test/code-review-remediation.test.js(13 regression tests: connectLoop listener containment, connections suspend/watchdog, traffic bind idempotency, empty-table packets, restart-timer cleanup, router validation, ciphertext preservation) — suite now 247 tests, all passing