feat: support trusted API on separate network/IP/port#4717
Conversation
Coverage Report for CI Build 29564759627Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+4.4%) to 12.359%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
Adds an optional “trusted listener” that exposes the existing Express app on a second bind target (TCP or unix socket) intended for host applications (e.g., Home Assistant) to access Z-Wave JS UI APIs without user-auth, guarded by a TCP peer allowlist or filesystem permissions for unix sockets.
Changes:
- Introduces trusted listener config parsing from
TRUSTED_API_LISTEN/TRUSTED_API_ALLOWED_IPS, including CIDR-based bind-address resolution. - Adds a trusted listener HTTP server that marks allowlisted sockets as “trusted” and integrates it into auth + rate-limiting bypass paths.
- Adds tests and documentation for the new environment variables; adds
ipaddr.jsdependency.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
api/config/app.ts |
Adds trusted listener env var parsing + exported config/types. |
api/lib/trustedListener.ts |
Implements trusted listener server, allowlist enforcement, and CIDR bind resolution. |
api/lib/SocketManager.ts |
Adds helper to attach Socket.IO to an additional HTTP server. |
api/app.ts |
Integrates trusted listener into startup, auth middleware, and rate limiting; closes listener on shutdown. |
test/lib/trustedListenerConfig.test.ts |
Unit tests for trusted listener config parsing + helpers. |
test/lib/trustedListener.test.ts |
Integration tests for trusted listener trust decisions (HTTP + Socket.IO + unix socket). |
docs/guide/env-vars.md |
Documents trusted listener env vars and security model. |
package.json |
Adds ipaddr.js. |
package-lock.json |
Locks ipaddr.js and dependency tree updates. |
robertsLando
left a comment
There was a problem hiding this comment.
Deep review — trusted API listener
Verdict: Ship with minor changes. The approach is sound and the security model holds up under scrutiny. Trust is decided on the kernel-reported TCP peer address in the connection event before any HTTP parsing, X-Forwarded-For/TRUST_PROXY are deliberately excluded, and the WeakSet<net.Socket> keyed on the raw socket cleanly covers keep-alive, HTTP/1.1 pipelining, and websocket-upgrade reuse. Verified fail-closed on remoteAddress === undefined, and the ::ffff: IPv4-mapped divergence between allowlist-parse and remote-match is fail-closed (no over-permit path). Backcompat on the main listener is clean — every new branch (isAuthenticated, /api/auth-enabled, the three rate-limiter skips) evaluates exactly as before.
Review run across 6 lenses (correctness, security, operability, design, tests, DRY). Findings below are robustness / hardening / test-coverage — none challenge the design.
Top 3 risks
- An error on the optional secondary listener hard-kills the primary gateway via
process.exit(1), with divergent failure paths and thin diagnostics (flagged independently by 5 of 6 lenses). - The unix socket is created with no explicit mode — under a permissive container umask it can be world-writable, i.e. unauthenticated full API control for any local process, despite the docs claiming access control is left to filesystem permissions.
- The core auth-bypass wiring in
app.ts(isAuthenticated+ rate-limitskip) is not integration-tested; only the underlying trust primitive is.
Strengths: correct threat model (raw peer address, headers excluded), the WeakSet abstraction reaching both HTTP and the socket.io handshake, fail-fast config validation, and strong helper/config-level test coverage.
Coverage: correctness, security, operability, design, tests, DRY. No changed file left uncovered; no Blockers surfaced by any lens.
…#4717 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This PR adds support for two environment variables that let a host application like Home Assistant establish a secure channel to Z-Wave JS UI's API, without interfering with the user's auth configuration.
This is done by specifying a network/IP/port on which a second API server is started, and an IP allowlist (which IPs may use this). The configuration works entirely through environment variables:
TRUSTED_API_LISTEN: Enables the trusted listener. Three forms are accepted:/: bind a unix socket at that path. Access control is left to filesystem permissions. A stale socket file from an unclean shutdown is removed automatically on startup.host:portwith a literal IP (IPv6 in brackets, e.g.[fd00::1]:8092): bind exactly that address.cidr:port(e.g.172.30.32.0/23:8092): resolve the local interface address inside that network at startup and bind to it. This lets the host name a (docker) network without either side knowing the container's dynamic IP. Startup fails if no (or more than one) local address matches.TRUSTED_API_ALLOWED_IPS: Comma-separated list of IPs/CIDRs (e.g.172.30.32.2,127.0.0.1) allowed to connect to a TCP binding. Required for TCP bindings — startup fails without it. Ignored for unix sockets.