From cdd0bf8cbd288b354dfd6dbcc5a02364452cbdfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 19 Jul 2026 10:36:44 +0100 Subject: [PATCH] feat: bind HTTP server to multiple addresses (#1792) * feat: bind HTTP server to multiple addresses Support a list of bind addresses so the backend can serve on loopback and a tailnet IP at once, without exposing every interface via 0.0.0.0. - Config: server.host now parses a comma-separated list; a new server.hosts YAML array takes precedence when set. Entries are trimmed, deduped, and validated (IP or hostname); a wildcard entry collapses the set. - Startup: one net.Listener per resolved address serving one shared http.Server; the single-listener path is the one-element case. - Bind-failure policy: all-fail is fatal; partial-fail warns, serves the good addresses, and runs a cancellable background retry loop that binds late-arriving addresses (e.g. a tailnet IP after tailscaled starts). - Graceful shutdown stops the retry loop before closing all listeners; covered by goleak. - Readiness probe picks a reachable bound address, preferring loopback. - Add ServerConfig.NonLoopbackBinds and config.IsLoopbackHost so the sibling app-auth task can gate fail-closed auth on off-box binds. Co-Authored-By: Claude Opus 4.8 (1M context) * fix: preserve KANDEV_SERVER_HOST over config server.hosts server.host now wins over the server.hosts array when set, because Host carries the KANDEV_SERVER_HOST env override and env must beat a config-file server.hosts (env-over-file precedence and the desktop loopback contract). The viper default for server.host is empty so an unset host is distinguishable from an explicit one; server.hosts is used only when host is unset. Adds regression tests covering the env-override and hosts-when-unset paths. Co-Authored-By: Claude Opus 4.8 (1M context) * fix: harden multi-bind host parsing and listener lifecycle Address review feedback: - Validate every host before wildcard collapse so an invalid entry is rejected regardless of order; canonicalize IPs so equivalent forms (::1 and 0:0:0:0:0:0:0:1) dedupe and unspecified longhand IPs count as the wildcard. - Probe the IPv6 loopback (::1) for an IPv6 wildcard listener instead of 127.0.0.1, so readiness doesn't stall on an IPv6-only socket. - Make serverListeners.Stop race-free via sync.Once. - Register t.Cleanup for the blocker listener before the fatal-assertion path in the partial-failure test. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 4.8 (1M context) --- apps/backend/internal/backendapp/helpers.go | 5 + .../backend/internal/backendapp/httpserver.go | 182 ++++++++++++++ .../internal/backendapp/httpserver_test.go | 224 ++++++++++++++++++ apps/backend/internal/backendapp/main.go | 53 ++--- .../internal/backendapp/server_test.go | 33 +-- apps/backend/internal/common/config/config.go | 184 +++++++++++++- .../internal/common/config/config_test.go | 150 ++++++++++++ 7 files changed, 762 insertions(+), 69 deletions(-) create mode 100644 apps/backend/internal/backendapp/httpserver.go create mode 100644 apps/backend/internal/backendapp/httpserver_test.go diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index ca64eaeef4..0fee686bf1 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1257,6 +1257,7 @@ func externalMCPOpenMiddleware() gin.HandlerFunc { // runGracefulShutdown gracefully stops all services and runs cleanups. func runGracefulShutdown( server *http.Server, + listeners *serverListeners, orchestratorSvc *orchestrator.Service, lifecycleMgr *lifecycle.Manager, runCleanups func(), @@ -1269,6 +1270,10 @@ func runGracefulShutdown( zap.Int("agent_timeout_seconds", int(agentShutdownTimeout/time.Second)), zap.Int("tracing_timeout_seconds", int(tracingShutdownTimeout/time.Second))) + // Stop the background bind-retry loop before Shutdown so no new listener + // can be created after the server begins closing its listeners. + listeners.Stop() + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), httpShutdownTimeout) defer shutdownCancel() diff --git a/apps/backend/internal/backendapp/httpserver.go b/apps/backend/internal/backendapp/httpserver.go new file mode 100644 index 0000000000..ea578b83f4 --- /dev/null +++ b/apps/backend/internal/backendapp/httpserver.go @@ -0,0 +1,182 @@ +package backendapp + +import ( + "context" + "net" + "net/http" + "sync" + "time" + + "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/common/logger" + "go.uber.org/zap" +) + +// serverBindRetryInterval is how often the background retry loop re-attempts a +// bind for an address that failed the initial bind (e.g. a tailnet IP that only +// exists once tailscaled comes up). It is a var so tests can shorten it. +var serverBindRetryInterval = 5 * time.Second + +// serverListeners owns the set of net.Listeners that serve a single shared +// http.Server, plus a background retry loop that keeps trying hosts whose +// initial bind failed. The single-listener case is just the one-element form. +// +// All active listeners are closed by the shared http.Server on Shutdown; the +// retry loop is stopped explicitly via Stop() before that Shutdown so no new +// listener can appear after shutdown begins. +type serverListeners struct { + server *http.Server + port int + log *logger.Logger + + mu sync.Mutex + bound []string // ln.Addr().String() for each active listener + + retryCancel context.CancelFunc + retryDone chan struct{} + stopOnce sync.Once +} + +// startHTTPServers binds one listener per host and serves the shared handler on +// each. Bind-failure policy: if EVERY bind fails it returns false (fatal); if +// SOME fail it logs a prominent warning naming the failed hosts, serves the +// rest, and launches a background retry loop that binds the failed hosts once +// they become available. Returns the manager so shutdown can stop the retry +// loop and the readiness probe can pick a reachable address. +func startHTTPServers(server *http.Server, hosts []string, port int, log *logger.Logger) (*serverListeners, bool) { + sl := &serverListeners{server: server, port: port, log: log} + + var failed []string + for _, h := range hosts { + if err := sl.bind(h); err != nil { + failed = append(failed, h) + } + } + + if len(sl.boundAddrs()) == 0 { + log.Error("Server failed to bind any configured address", zap.Strings("hosts", hosts)) + return nil, false + } + + if len(failed) > 0 { + log.Warn("Server bound only some addresses; retrying the rest in the background", + zap.Strings("failed", failed), + zap.Strings("bound", sl.boundAddrs())) + sl.startRetry(failed) + } + + return sl, true +} + +// bind listens on host:port and, on success, serves the shared handler on the +// new listener. A failure is logged and returned so the caller can decide the +// all-fail vs. partial-fail policy. +func (sl *serverListeners) bind(host string) error { + addr := serverListenAddr(host, sl.port) + ln, err := net.Listen("tcp", addr) + if err != nil { + sl.log.Warn("Server listen error", zap.String("addr", addr), zap.Error(err)) + return err + } + + sl.mu.Lock() + sl.bound = append(sl.bound, ln.Addr().String()) + sl.mu.Unlock() + + sl.serve(ln) + return nil +} + +// serve runs the shared http.Server on ln in its own goroutine. http.Server +// supports Serve being called concurrently on multiple listeners and closes +// all of them on Shutdown. The defensive ln.Close() covers the narrow race +// where a retry bind completes after Shutdown has already started (Serve then +// returns ErrServerClosed without closing the listener). +func (sl *serverListeners) serve(ln net.Listener) { + go func() { + defer func() { _ = ln.Close() }() + sl.log.Info("WebSocket server listening", + zap.String("addr", ln.Addr().String()), zap.Int("port", sl.port)) + if err := sl.server.Serve(ln); err != nil && err != http.ErrServerClosed { + sl.log.Error("Server serve error", + zap.String("addr", ln.Addr().String()), zap.Error(err)) + } + }() +} + +func (sl *serverListeners) boundAddrs() []string { + sl.mu.Lock() + defer sl.mu.Unlock() + return append([]string(nil), sl.bound...) +} + +// startRetry launches the background retry loop for the given failed hosts. +func (sl *serverListeners) startRetry(hosts []string) { + ctx, cancel := context.WithCancel(context.Background()) + sl.retryCancel = cancel + sl.retryDone = make(chan struct{}) + go sl.retryLoop(ctx, hosts) +} + +// retryLoop periodically re-attempts binding the still-failed hosts until all +// succeed or the context is cancelled (graceful shutdown). Each successful bind +// starts serving immediately, so a late-arriving tailnet IP self-heals. +func (sl *serverListeners) retryLoop(ctx context.Context, hosts []string) { + defer close(sl.retryDone) + + ticker := time.NewTicker(serverBindRetryInterval) + defer ticker.Stop() + + pending := append([]string(nil), hosts...) + for len(pending) > 0 { + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + + var still []string + for _, h := range pending { + if err := sl.bind(h); err != nil { + still = append(still, h) + continue + } + sl.log.Info("Server bound previously-failed address", zap.String("host", h)) + } + pending = still + } +} + +// Stop cancels the background retry loop and waits for it to drain. Safe to +// call concurrently and when no retry loop was started; the cancel+drain runs +// at most once via stopOnce. Must run before server.Shutdown so no new listener +// appears after shutdown begins. +func (sl *serverListeners) Stop() { + if sl == nil { + return + } + sl.stopOnce.Do(func() { + if sl.retryCancel != nil { + sl.retryCancel() + <-sl.retryDone + } + }) +} + +// probeAddr returns an address the readiness probe can dial. It prefers a bound +// address that resolves to loopback (always reachable from the box without +// depending on a routable interface), falling back to the first bound address. +func (sl *serverListeners) probeAddr() string { + sl.mu.Lock() + defer sl.mu.Unlock() + if len(sl.bound) == 0 { + return "" + } + for _, a := range sl.bound { + probe := serverProbeAddr(a) + if host, _, err := net.SplitHostPort(probe); err == nil && config.IsLoopbackHost(host) { + return probe + } + } + return serverProbeAddr(sl.bound[0]) +} diff --git a/apps/backend/internal/backendapp/httpserver_test.go b/apps/backend/internal/backendapp/httpserver_test.go new file mode 100644 index 0000000000..901df176a1 --- /dev/null +++ b/apps/backend/internal/backendapp/httpserver_test.go @@ -0,0 +1,224 @@ +package backendapp + +import ( + "context" + "fmt" + "net" + "net/http" + "testing" + "time" + + "github.com/kandev/kandev/internal/common/logger" + "go.uber.org/goleak" + "go.uber.org/zap" +) + +func testLogger(t *testing.T) *logger.Logger { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatalf("NewFromZap: %v", err) + } + return log +} + +// freePort grabs a currently-free TCP port on loopback and releases it. There +// is an inherent race between release and re-bind, but it is acceptable for +// tests and keeps the shared-port multi-listener scenarios simple. +func freePort(t *testing.T) int { + t.Helper() + ln := listenOnFreePort(t) + port := listenerPort(t, ln) + if err := ln.Close(); err != nil { + t.Fatalf("close temp listener: %v", err) + } + return port +} + +func noContentServer() *http.Server { + return &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }), + } +} + +// getStatus dials host:port over HTTP and returns the status code. Keep-alives +// are disabled so no idle client connection lingers for goleak to flag. +func getStatus(host string, port int) (int, error) { + url := fmt.Sprintf("http://%s/", net.JoinHostPort(host, fmt.Sprint(port))) + client := &http.Client{ + Timeout: 500 * time.Millisecond, + Transport: &http.Transport{DisableKeepAlives: true}, + } + defer client.CloseIdleConnections() + resp, err := client.Get(url) + if err != nil { + return 0, err + } + defer func() { _ = resp.Body.Close() }() + return resp.StatusCode, nil +} + +// waitForStatus polls host:port until it returns wantStatus or the deadline +// expires. +func waitForStatus(t *testing.T, host string, port, wantStatus int, within time.Duration) { + t.Helper() + deadline := time.Now().Add(within) + for { + if got, err := getStatus(host, port); err == nil && got == wantStatus { + return + } + if time.Now().After(deadline) { + t.Fatalf("address %s:%d never returned %d within %s", host, port, wantStatus, within) + } + time.Sleep(10 * time.Millisecond) + } +} + +// shutdown stops the retry loop and the shared server, then verifies no +// goroutines leaked. Registered first via t.Cleanup so it runs last. +func shutdown(t *testing.T, listeners *serverListeners, server *http.Server) { + t.Helper() + if listeners != nil { + listeners.Stop() + } + if server != nil { + if err := server.Shutdown(context.Background()); err != nil { + t.Errorf("server.Shutdown: %v", err) + } + } + goleak.VerifyNone(t) +} + +func TestStartHTTPServersMultipleLoopbackAddresses(t *testing.T) { + log := testLogger(t) + port := freePort(t) + server := noContentServer() + + listeners, ok := startHTTPServers(server, []string{"127.0.0.1", "127.0.0.2"}, port, log) + if !ok { + t.Fatal("startHTTPServers returned not-ok for two bindable loopback addresses") + } + defer shutdown(t, listeners, server) + + for _, host := range []string{"127.0.0.1", "127.0.0.2"} { + got, err := getStatus(host, port) + if err != nil { + t.Fatalf("GET %s:%d: %v", host, port, err) + } + if got != http.StatusNoContent { + t.Fatalf("GET %s:%d status = %d, want %d", host, port, got, http.StatusNoContent) + } + } +} + +func TestStartHTTPServersAllFailIsFatal(t *testing.T) { + defer goleak.VerifyNone(t) + log := testLogger(t) + + // Occupy the port on 127.0.0.1 so the only requested bind fails. + blocker := listenOnFreePort(t) + t.Cleanup(func() { _ = blocker.Close() }) + port := listenerPort(t, blocker) + + server := noContentServer() + listeners, ok := startHTTPServers(server, []string{"127.0.0.1"}, port, log) + if ok { + listeners.Stop() + _ = server.Shutdown(context.Background()) + t.Fatal("startHTTPServers returned ok when every bind failed; want fatal") + } + if listeners != nil { + t.Fatalf("startHTTPServers returned non-nil manager on all-fail: %v", listeners) + } +} + +func TestStartHTTPServersPartialFailSelfHeals(t *testing.T) { + defer goleak.VerifyNone(t) + + // Shorten the retry cadence for the test and restore it after. + prev := serverBindRetryInterval + serverBindRetryInterval = 15 * time.Millisecond + t.Cleanup(func() { serverBindRetryInterval = prev }) + + log := testLogger(t) + port := freePort(t) + + // Block 127.0.0.2:port so its initial bind fails; 127.0.0.1 succeeds. + blocker, err := net.Listen("tcp", net.JoinHostPort("127.0.0.2", fmt.Sprint(port))) + if err != nil { + t.Fatalf("occupy 127.0.0.2:%d: %v", port, err) + } + t.Cleanup(func() { _ = blocker.Close() }) + + server := noContentServer() + listeners, ok := startHTTPServers(server, []string{"127.0.0.1", "127.0.0.2"}, port, log) + if !ok { + _ = blocker.Close() + t.Fatal("startHTTPServers returned not-ok on partial failure; want serve-the-good-one") + } + defer shutdown(t, listeners, server) + + // The bound address serves immediately. + if got, err := getStatus("127.0.0.1", port); err != nil || got != http.StatusNoContent { + t.Fatalf("GET 127.0.0.1:%d = %d (err %v), want %d", port, got, err, http.StatusNoContent) + } + + // Release the blocked address; the background retry should bind it. + if err := blocker.Close(); err != nil { + t.Fatalf("release blocker: %v", err) + } + waitForStatus(t, "127.0.0.2", port, http.StatusNoContent, 2*time.Second) +} + +func TestServerListenersStopClosesListenersAndDrains(t *testing.T) { + defer goleak.VerifyNone(t) + + log := testLogger(t) + port := freePort(t) + server := noContentServer() + + listeners, ok := startHTTPServers(server, []string{"127.0.0.1", "127.0.0.2"}, port, log) + if !ok { + t.Fatal("startHTTPServers not-ok") + } + waitForStatus(t, "127.0.0.1", port, http.StatusNoContent, time.Second) + + listeners.Stop() + // Stop is idempotent. + listeners.Stop() + if err := server.Shutdown(context.Background()); err != nil { + t.Fatalf("server.Shutdown: %v", err) + } + + // After shutdown, both listeners are closed. + if _, err := getStatus("127.0.0.1", port); err == nil { + t.Fatal("expected connection failure on 127.0.0.1 after shutdown") + } + if _, err := getStatus("127.0.0.2", port); err == nil { + t.Fatal("expected connection failure on 127.0.0.2 after shutdown") + } +} + +func TestProbeAddrPrefersLoopback(t *testing.T) { + sl := &serverListeners{bound: []string{"100.64.0.1:8080", "127.0.0.1:8080"}} + if got := sl.probeAddr(); got != "127.0.0.1:8080" { + t.Fatalf("probeAddr() = %q, want 127.0.0.1:8080", got) + } + + wildcard := &serverListeners{bound: []string{"0.0.0.0:8080"}} + if got := wildcard.probeAddr(); got != "127.0.0.1:8080" { + t.Fatalf("probeAddr() wildcard = %q, want 127.0.0.1:8080", got) + } + + wildcard6 := &serverListeners{bound: []string{"[::]:8080"}} + if got := wildcard6.probeAddr(); got != "[::1]:8080" { + t.Fatalf("probeAddr() ipv6 wildcard = %q, want [::1]:8080", got) + } + + tailnetOnly := &serverListeners{bound: []string{"100.64.0.1:8080"}} + if got := tailnetOnly.probeAddr(); got != "100.64.0.1:8080" { + t.Fatalf("probeAddr() tailnet-only = %q, want 100.64.0.1:8080", got) + } +} diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index b7932c6f1d..9b90e404fa 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -764,7 +764,13 @@ func startGatewayAndServe( if port == 0 { port = ports.Backend } - if !startHTTPServer(server, port, log) { + hosts, err := cfg.Server.ResolvedBinds() + if err != nil { + log.Error("Invalid server bind configuration", zap.Error(err)) + return false + } + listeners, ok := startHTTPServers(server, hosts, port, log) + if !ok { return false } @@ -776,33 +782,12 @@ func startGatewayAndServe( // Flip the readiness flag once the HTTP listener is actually // accepting connections, not just "spawned". Serve runs in a goroutine - // after we bind the socket above; probe the local listener with a short - // retry loop — once a single connect succeeds, the kernel queue is up and - // any subsequent /health call will land on a wired route. - go waitListenerThenMarkReady(server.Addr, log) - - awaitShutdown(server, orchestratorSvc, lifecycleMgr, runCleanups, log) - return true -} - -func startHTTPServer(server *http.Server, port int, log *logger.Logger) bool { - addr := strings.TrimSpace(server.Addr) - if addr == "" { - addr = serverListenAddr("", port) - } - ln, err := net.Listen("tcp", addr) - if err != nil { - log.Error("Server listen error", zap.Error(err)) - return false - } - - go func() { - log.Info("WebSocket server listening", zap.String("addr", ln.Addr().String()), zap.Int("port", port)) - if err := server.Serve(ln); err != nil && err != http.ErrServerClosed { - log.Error("Server serve error", zap.Error(err)) - } - }() + // after we bind the socket above; probe a reachable local listener with a + // short retry loop — once a single connect succeeds, the kernel queue is up + // and any subsequent /health call will land on a wired route. + go waitListenerThenMarkReady(listeners.probeAddr(), log) + awaitShutdown(server, listeners, orchestratorSvc, lifecycleMgr, runCleanups, log) return true } @@ -820,8 +805,12 @@ func serverProbeAddr(listenAddr string) string { return listenAddr } switch host { - case "", "0.0.0.0", "::": + case "", "0.0.0.0": host = "127.0.0.1" + case "::": + // Preserve the address family: an IPv6-only wildcard listener isn't + // reachable via 127.0.0.1, so probe the IPv6 loopback instead. + host = "::1" } return net.JoinHostPort(host, port) } @@ -1693,8 +1682,11 @@ func buildHTTPServer( log: log, }) + // Addr is intentionally left unset: bind addresses are resolved from + // cfg.Server.ResolvedBinds() and served via startHTTPServers, which may + // create several listeners on one shared handler. server.Shutdown closes + // all of them regardless of Addr. return &http.Server{ - Addr: serverListenAddr(cfg.Server.Host, port), Handler: router, ReadTimeout: cfg.Server.ReadTimeoutDuration(), WriteTimeout: cfg.Server.WriteTimeoutDuration(), @@ -1704,6 +1696,7 @@ func buildHTTPServer( // awaitShutdown waits for an OS signal then performs graceful shutdown. func awaitShutdown( server *http.Server, + listeners *serverListeners, orchestratorSvc *orchestrator.Service, lifecycleMgr *lifecycle.Manager, runCleanups func(), @@ -1730,5 +1723,5 @@ func awaitShutdown( log.Info("Received shutdown signal", zap.String("signal", sig.String()), zap.Int("pid", os.Getpid())) - runGracefulShutdown(server, orchestratorSvc, lifecycleMgr, runCleanups, log) + runGracefulShutdown(server, listeners, orchestratorSvc, lifecycleMgr, runCleanups, log) } diff --git a/apps/backend/internal/backendapp/server_test.go b/apps/backend/internal/backendapp/server_test.go index ee8b4f1e14..4777371409 100644 --- a/apps/backend/internal/backendapp/server_test.go +++ b/apps/backend/internal/backendapp/server_test.go @@ -1,41 +1,10 @@ package backendapp import ( - "context" "net" - "net/http" "testing" - - "github.com/kandev/kandev/internal/common/logger" - "go.uber.org/zap" ) -func TestStartHTTPServerUsesServerAddr(t *testing.T) { - log, err := logger.NewFromZap(zap.NewNop()) - if err != nil { - t.Fatalf("NewFromZap: %v", err) - } - - blocked := listenOnFreePort(t) - t.Cleanup(func() { - if err := blocked.Close(); err != nil { - t.Errorf("close blocked listener: %v", err) - } - }) - - server := &http.Server{ - Addr: "127.0.0.1:0", - Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) }), - } - defer func() { - _ = server.Shutdown(context.Background()) - }() - - if !startHTTPServer(server, listenerPort(t, blocked), log) { - t.Fatalf("expected server to listen on configured Addr %q", server.Addr) - } -} - func TestServerListenAddr(t *testing.T) { tests := []struct { name string @@ -74,7 +43,7 @@ func TestServerProbeAddr(t *testing.T) { }{ {name: "port-only address probes loopback", listenAddr: ":38429", want: "127.0.0.1:38429"}, {name: "wildcard ipv4 probes loopback", listenAddr: "0.0.0.0:38429", want: "127.0.0.1:38429"}, - {name: "wildcard ipv6 probes loopback", listenAddr: "[::]:38429", want: "127.0.0.1:38429"}, + {name: "wildcard ipv6 probes ipv6 loopback", listenAddr: "[::]:38429", want: "[::1]:38429"}, {name: "loopback address probes itself", listenAddr: "127.0.0.1:38429", want: "127.0.0.1:38429"}, {name: "ipv6 loopback probes itself", listenAddr: "[::1]:38429", want: "[::1]:38429"}, } diff --git a/apps/backend/internal/common/config/config.go b/apps/backend/internal/common/config/config.go index 50f69737bc..bab8e95131 100644 --- a/apps/backend/internal/common/config/config.go +++ b/apps/backend/internal/common/config/config.go @@ -4,6 +4,7 @@ package config import ( "fmt" + "net" "os" "path/filepath" "runtime" @@ -82,13 +83,175 @@ func (c *Config) ResolvedDataDir() string { return filepath.Join(c.ResolvedHomeDir(), "data") } +// defaultServerHost is the wildcard host the server binds to when no host is +// configured — every interface. +const defaultServerHost = "0.0.0.0" + // ServerConfig holds HTTP server configuration. type ServerConfig struct { - Host string `mapstructure:"host"` - Port int `mapstructure:"port"` - ReadTimeout int `mapstructure:"readTimeout"` // in seconds - WriteTimeout int `mapstructure:"writeTimeout"` // in seconds - WebInternalURL string `mapstructure:"webInternalUrl"` + // Host is the bind address. A single value (e.g. "127.0.0.1") behaves as + // it always has; a comma-separated list (e.g. "127.0.0.1,100.64.0.1") + // binds one listener per address. Empty means the wildcard default. + // + // Host carries the KANDEV_SERVER_HOST env override, so when it is set it + // wins over Hosts to preserve env-over-file precedence (see ResolvedBinds). + Host string `mapstructure:"host"` + // Hosts is the YAML-array form of Host, for config files that prefer an + // array to a comma-separated string. It is used only when Host is unset. + Hosts []string `mapstructure:"hosts"` + Port int `mapstructure:"port"` + ReadTimeout int `mapstructure:"readTimeout"` // in seconds + WriteTimeout int `mapstructure:"writeTimeout"` // in seconds + WebInternalURL string `mapstructure:"webInternalUrl"` +} + +// splitHosts splits a comma-separated host string, trimming whitespace and +// dropping empty entries. +func splitHosts(s string) []string { + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out +} + +// normalizeHost validates h and returns its canonical form plus whether it is a +// wildcard (binds every interface). IP addresses are canonicalized via +// net.ParseIP so equivalent forms dedupe (e.g. 0:0:0:0:0:0:0:1 and ::1); any +// unspecified IP (0.0.0.0, ::, or a longhand form) is treated as a wildcard. +// Hostnames are returned as-is. An invalid entry returns an error. +func normalizeHost(h string) (canonical string, wildcard bool, err error) { + if ip := net.ParseIP(h); ip != nil { + if ip.IsUnspecified() { + return ip.String(), true, nil + } + return ip.String(), false, nil + } + if isValidHostname(h) { + return h, false, nil + } + return "", false, fmt.Errorf("server bind host %q is not a valid IP address or hostname", h) +} + +// isValidHostname reports whether h is a syntactically valid RFC 1123 hostname. +func isValidHostname(h string) bool { + if len(h) == 0 || len(h) > 253 { + return false + } + for _, label := range strings.Split(strings.TrimSuffix(h, "."), ".") { + if !isValidHostnameLabel(label) { + return false + } + } + return true +} + +// isValidHostnameLabel reports whether label is a valid single hostname label +// (letters, digits, and interior hyphens, 1–63 chars). +func isValidHostnameLabel(label string) bool { + if len(label) == 0 || len(label) > 63 { + return false + } + if label[0] == '-' || label[len(label)-1] == '-' { + return false + } + for i := 0; i < len(label); i++ { + c := label[i] + if (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') && + (c < '0' || c > '9') && c != '-' { + return false + } + } + return true +} + +// IsLoopbackHost reports whether h refers only to the local loopback +// interface. A wildcard (empty/0.0.0.0/::) is NOT loopback because it also +// binds routable interfaces. Unresolvable hostnames are treated as +// non-loopback (fail-closed) so callers gating on "any non-loopback bind" +// don't under-report exposure. +func IsLoopbackHost(h string) bool { + h = strings.TrimSpace(h) + if h == "" { + return false + } + if strings.EqualFold(h, "localhost") { + return true + } + if ip := net.ParseIP(h); ip != nil { + return ip.IsLoopback() + } + return false +} + +// ResolvedBinds returns the de-duplicated, validated list of hosts the HTTP +// server should bind to. server.host (comma-separated string) takes precedence +// over server.hosts (YAML array) when set, because Host carries the +// KANDEV_SERVER_HOST env override and env must beat a config-file server.hosts +// (env-over-file precedence, and the desktop loopback contract). server.hosts +// is used only when host is unset. Whitespace is trimmed and empty/duplicate +// entries dropped, and IP addresses are canonicalized so equivalent forms +// dedupe. Every entry is validated before the set is collapsed, so an invalid +// host is rejected regardless of its position relative to a wildcard. If any +// entry is a wildcard (empty/0.0.0.0/:: or an unspecified IP) the whole set +// collapses to that single wildcard, since it already binds every interface. An +// empty result falls back to the wildcard default. +func (c *ServerConfig) ResolvedBinds() ([]string, error) { + raw := splitHosts(c.Host) + if len(raw) == 0 { + for _, h := range c.Hosts { + raw = append(raw, splitHosts(h)...) + } + } + + seen := make(map[string]struct{}, len(raw)) + out := make([]string, 0, len(raw)) + wildcard := "" + for _, h := range raw { + norm, isWild, err := normalizeHost(h) + if err != nil { + return nil, err + } + if isWild { + if wildcard == "" { + wildcard = norm + } + continue + } + if _, dup := seen[norm]; dup { + continue + } + seen[norm] = struct{}{} + out = append(out, norm) + } + if wildcard != "" { + return []string{wildcard}, nil + } + if len(out) == 0 { + return []string{defaultServerHost}, nil + } + return out, nil +} + +// NonLoopbackBinds returns the resolved bind hosts that are not loopback-only. +// The sibling app-auth work uses this to decide whether to require auth (fail +// closed when the server is reachable off-box). A wildcard bind counts as +// non-loopback. +func (c *ServerConfig) NonLoopbackBinds() ([]string, error) { + binds, err := c.ResolvedBinds() + if err != nil { + return nil, err + } + out := make([]string, 0, len(binds)) + for _, h := range binds { + if !IsLoopbackHost(h) { + out = append(out, h) + } + } + return out, nil } // DatabaseConfig holds database connection configuration. @@ -288,8 +451,12 @@ func detectDefaultLogFormat() string { // setDefaults configures default values for all configuration options. func setDefaults(v *viper.Viper) { - // Server defaults - v.SetDefault("server.host", "0.0.0.0") + // Server defaults. Host defaults to empty (not the wildcard) so an unset + // host is distinguishable from an explicit one: ResolvedBinds falls back to + // server.hosts only when host is unset, and empty resolves to the wildcard + // default. This keeps KANDEV_SERVER_HOST winning over a config-file + // server.hosts. + v.SetDefault("server.host", "") v.SetDefault("server.port", ports.Backend) v.SetDefault("server.readTimeout", 30) v.SetDefault("server.writeTimeout", 30) @@ -497,6 +664,9 @@ func validate(cfg *Config) error { if cfg.Server.Port <= 0 || cfg.Server.Port > 65535 { errs = append(errs, "server.port must be between 1 and 65535") } + if _, err := cfg.Server.ResolvedBinds(); err != nil { + errs = append(errs, err.Error()) + } // Database validation. Normalize the driver in place so downstream // case-sensitive comparisons (and the postgres-only branch below) see diff --git a/apps/backend/internal/common/config/config_test.go b/apps/backend/internal/common/config/config_test.go index d1c41263c8..33aaa39edb 100644 --- a/apps/backend/internal/common/config/config_test.go +++ b/apps/backend/internal/common/config/config_test.go @@ -218,6 +218,156 @@ func TestServerHostFromEnv(t *testing.T) { } } +func TestResolvedBinds(t *testing.T) { + tests := []struct { + name string + host string + hosts []string + want []string + wantErr bool + }{ + {name: "single host", host: "127.0.0.1", want: []string{"127.0.0.1"}}, + {name: "empty falls back to wildcard default", host: "", want: []string{"0.0.0.0"}}, + {name: "comma separated list", host: "127.0.0.1,100.64.0.1", want: []string{"127.0.0.1", "100.64.0.1"}}, + {name: "whitespace trimmed", host: " 127.0.0.1 , 100.64.0.1 ", want: []string{"127.0.0.1", "100.64.0.1"}}, + {name: "duplicates dropped", host: "127.0.0.1,127.0.0.1,::1", want: []string{"127.0.0.1", "::1"}}, + {name: "wildcard collapses set", host: "127.0.0.1,0.0.0.0", want: []string{"0.0.0.0"}}, + {name: "ipv6 wildcard collapses set", host: "::,127.0.0.1", want: []string{"::"}}, + {name: "hostname allowed", host: "my-tailnet-host", want: []string{"my-tailnet-host"}}, + {name: "hosts array used when host unset", host: "", hosts: []string{"127.0.0.1", "100.64.0.1"}, want: []string{"127.0.0.1", "100.64.0.1"}}, + {name: "explicit host wins over hosts array", host: "127.0.0.1", hosts: []string{"0.0.0.0"}, want: []string{"127.0.0.1"}}, + {name: "hosts array as comma string", hosts: []string{"127.0.0.1,100.64.0.1"}, want: []string{"127.0.0.1", "100.64.0.1"}}, + {name: "equivalent ipv6 forms dedupe", host: "::1,0:0:0:0:0:0:0:1", want: []string{"::1"}}, + {name: "longhand unspecified ipv6 is wildcard", host: "0:0:0:0:0:0:0:0,127.0.0.1", want: []string{"::"}}, + {name: "invalid entry errors", host: "127.0.0.1,not a host!!", wantErr: true}, + {name: "invalid entry after wildcard still errors", host: "0.0.0.0,not a host!!", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sc := ServerConfig{Host: tt.host, Hosts: tt.hosts} + got, err := sc.ResolvedBinds() + if tt.wantErr { + if err == nil { + t.Fatalf("ResolvedBinds() expected error, got %v", got) + } + if !strings.Contains(err.Error(), "not a host!!") { + t.Fatalf("ResolvedBinds() error = %q, want it to name the bad entry", err) + } + return + } + if err != nil { + t.Fatalf("ResolvedBinds() unexpected error: %v", err) + } + if len(got) != len(tt.want) { + t.Fatalf("ResolvedBinds() = %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("ResolvedBinds() = %v, want %v", got, tt.want) + } + } + }) + } +} + +func TestIsLoopbackHost(t *testing.T) { + tests := []struct { + host string + want bool + }{ + {"127.0.0.1", true}, + {"127.0.0.53", true}, + {"::1", true}, + {"localhost", true}, + {"LocalHost", true}, + {"0.0.0.0", false}, + {"::", false}, + {"", false}, + {"100.64.0.1", false}, + {"my-tailnet-host", false}, + } + for _, tt := range tests { + t.Run(tt.host, func(t *testing.T) { + if got := IsLoopbackHost(tt.host); got != tt.want { + t.Fatalf("IsLoopbackHost(%q) = %v, want %v", tt.host, got, tt.want) + } + }) + } +} + +func TestNonLoopbackBinds(t *testing.T) { + sc := ServerConfig{Host: "127.0.0.1,100.64.0.1,::1"} + got, err := sc.NonLoopbackBinds() + if err != nil { + t.Fatalf("NonLoopbackBinds() error: %v", err) + } + if len(got) != 1 || got[0] != "100.64.0.1" { + t.Fatalf("NonLoopbackBinds() = %v, want [100.64.0.1]", got) + } + + loopOnly := ServerConfig{Host: "127.0.0.1"} + if got, err := loopOnly.NonLoopbackBinds(); err != nil || len(got) != 0 { + t.Fatalf("NonLoopbackBinds() = %v (err %v), want empty", got, err) + } + + wildcard := ServerConfig{Host: "0.0.0.0"} + if got, err := wildcard.NonLoopbackBinds(); err != nil || len(got) != 1 || got[0] != "0.0.0.0" { + t.Fatalf("NonLoopbackBinds() = %v (err %v), want [0.0.0.0]", got, err) + } +} + +// TestServerHostEnvOverridesConfigHosts verifies env-over-file precedence: a +// KANDEV_SERVER_HOST env override must win over a config-file server.hosts +// array, so launching with a loopback host binds only loopback even if the +// config file left non-loopback addresses in server.hosts. Regression for the +// desktop/headless loopback contract. +func TestServerHostEnvOverridesConfigHosts(t *testing.T) { + dir := t.TempDir() + cfgYAML := "server:\n hosts:\n - 0.0.0.0\n - 100.64.0.1\n" + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(cfgYAML), 0o600); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + t.Setenv("KANDEV_SERVER_HOST", "127.0.0.1") + + cfg, err := LoadWithPath(dir) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + binds, err := cfg.Server.ResolvedBinds() + if err != nil { + t.Fatalf("ResolvedBinds: %v", err) + } + if len(binds) != 1 || binds[0] != "127.0.0.1" { + t.Fatalf("ResolvedBinds() = %v, want [127.0.0.1] (env host must override config server.hosts)", binds) + } +} + +// TestServerHostsFromConfigWhenHostUnset confirms server.hosts is honored when +// no host/env override is present. +func TestServerHostsFromConfigWhenHostUnset(t *testing.T) { + dir := t.TempDir() + cfgYAML := "server:\n hosts:\n - 127.0.0.1\n - 100.64.0.1\n" + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(cfgYAML), 0o600); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + // Ensure no ambient KANDEV_SERVER_HOST override leaks into the test. + t.Setenv("KANDEV_SERVER_HOST", "") + + cfg, err := LoadWithPath(dir) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + binds, err := cfg.Server.ResolvedBinds() + if err != nil { + t.Fatalf("ResolvedBinds: %v", err) + } + want := []string{"127.0.0.1", "100.64.0.1"} + if len(binds) != len(want) || binds[0] != want[0] || binds[1] != want[1] { + t.Fatalf("ResolvedBinds() = %v, want %v", binds, want) + } +} + // TestFeaturesConfig_JSONShape pins the wire format of GET /api/v1/features. // The handler in helpers.go serializes FeaturesConfig directly so new // fields flow through without an extra edit; this test guarantees the