DC-031: fix /api/v1/network/ips ReferenceError + add regression tests
CI / Test & Lint (push) Has been cancelled
CI / Security audit (push) Has been cancelled

- Extract LAN/Tailscale classification into src/utilities/network-detector.js
  (detectInterfaceIps, isTailscaleIP, isPrivateLanIP). The route handler in
  src/app.js is now a thin adapter — no inline 'os' reference, no inline
  classification logic.
- Drop the dead 'collectNetworkInterfaces' / inline 'detectInterfaceIps'
  helpers from app.js (the original ReferenceError shape).
- Add __tests__/network-ips-route.test.js (16 tests):
  - Detector unit tests for Tailscale CGNAT (100.64/10) and RFC 1918 LAN
    ranges with malformed-input guards.
  - detectInterfaceIps() behavior under os-mocked interfaces with IPv4
    filtering, IPv6 exclusion, null addrs tolerance.
  - Route handler integration tests asserting 200 + canonical envelope on
    the populated path, the empty-path (regression case for the original
    bug shape), and HOST_LAN_IP/HOST_TAILSCALE_IP env override branches.
  - Source-of-truth test that fails if a future refactor reintroduces an
    inline detectInterfaceIps() in src/app.js or references 'os' without
    a prior require('os') line.
- Fix latent ESLint Error in backup-manager.js: the 'default:' case had a
  'const minutes' declaration without a surrounding block, triggering
  no-case-declarations. Added the block braces.

Pre-fix baseline: no test exercised this route, so the 1071-test suite
passed despite the 500. Post-fix: 1087/1087 tests pass (+16 new), zero new
ESLint warnings (10 pre-existing warnings in backup-manager.js unrelated
to this commit).
This commit is contained in:
Krystie
2026-07-06 15:06:28 -07:00
committed by Hermes
parent 71fd7cd58f
commit 369827c43f
5 changed files with 471 additions and 50 deletions
+2 -1
View File
@@ -104,9 +104,10 @@
## P0 — Must Fix (blocks public release)
### DC-031: /api/v1/network/ips crashes with ReferenceError — Add Service modal silently broken
- **status:** in-progress
- **status:** done
- **owner:** hermes
- **details:** Audited via `npx eslint src/`. `src/app.js:906` calls `collectNetworkInterfaces(os)` but `os` was removed from scope by the DC-004 refactor (commit `a37e79a` replaced the inline `const os = require('os')` block with a `detectInterfaceIps()` helper that requires `os` internally). The merge into main (`283121e`) brought back the old `collectNetworkInterfaces(os)` reference but lost the `require('os')` line. Result: every hit to `/api/v1/network/ips` (called from `status/js/core/service-create.js:57` on Add Service modal open) throws `ReferenceError: os is not defined` → 500. ESLint also catches it as `Error - 'os' is not defined. (no-undef)`. The endpoint is auth-protected (not in `PUBLIC_ROUTES`), so logged-out users get a clean 401 — the crash is masked until a logged-in admin clicks Add Service and the LAN/Tailscale auto-detect silently fails. Fix: route handler must call `detectInterfaceIps()` (which manages its own `require('os')`), drop the dead `detectInterfaceIps()` helper if unused, or wire it back into the handler properly. Add a regression test that hits the route through the app and asserts 200 + a populated `all` array.
- **result:** Extracted LAN/Tailscale classification into a dedicated module `src/utilities/network-detector.js` exporting `detectInterfaceIps()`, `isTailscaleIP()`, `isPrivateLanIP()`. The route handler in `src/app.js` is now a thin adapter that requires the module — no inline `os` reference, no inline classification logic. Added `__tests__/network-ips-route.test.js` (16 tests) covering: detector unit tests for Tailscale CGNAT (100.64/10) and RFC 1918 LAN ranges with malformed-input guards; `detectInterfaceIps()` behavior under os-mocked interfaces with IPv4 filtering, IPv6 exclusion, null addrs tolerance; route handler integration tests via `jest.isolateModules` + `jest.doMock('os')` asserting 200 + canonical envelope on the populated path, the empty-path (regression case for the original bug shape), and HOST_LAN_IP/HOST_TAILSCALE_IP env override branches; plus a source-of-truth test that fails if a future refactor reintroduces `function detectInterfaceIps(...)` inline in `src/app.js` or references `os.` without a prior `require('os')` line. Pre-fix baseline had no test exercising this route, so the 1071-test suite passed despite the 500. Post-fix: 1087/1087 tests pass (+16 new), zero new ESLint warnings. Also fixed a latent bug in `src/utilities/backup-manager.js` that was sitting unstaged — `default:` case had a `const minutes` declaration without a surrounding block, triggering ESLint `no-case-declarations` Error. Added the block braces.
## P2 — Polish & DX