fix: check the debug bind is loopback before claiming the port (#64 item 5)

main.go

@@ -174,11 +174,19 @@ func startDebugServer(cfg config.DebugConfig) (*http.Server, error) {
 		return nil, fmt.Errorf("mutex_profile_fraction must not be negative")
 	}
 
+	if !cfg.AllowPublic {
+		if err := checkLoopbackBind(cfg.Bind); err != nil {
+			return nil, err
+		}
+	}
+
 	listener, err := net.Listen("tcp", cfg.Bind)
 	if err != nil {
 		return nil, fmt.Errorf("listen on %q: %w", cfg.Bind, err)
 	}
 	if !cfg.AllowPublic {
+		// Backstop: a hostname can resolve differently between
+		// checkLoopbackBind and the bind itself.
 		addr, ok := listener.Addr().(*net.TCPAddr)
 		if !ok || !addr.IP.IsLoopback() {
 			listener.Close()
@@ -198,6 +206,36 @@ func startDebugServer(cfg config.DebugConfig) (*http.Server, error) {
 	return srv, nil
 }
 
+// checkLoopbackBind rejects a non-loopback debug bind before any socket is
+// opened.
+//
+// Checking only after net.Listen meant "0.0.0.0:6060" was really bound, and
+// accepting into the backlog, for the moment before the check closed it. Not
+// an exposure, since Serve never started, but a public port should not be
+// claimed at all, and ":6060" deserves a clearer error than "resolves to a
+// non-loopback address". A hostname must resolve to loopback addresses only.
+func checkLoopbackBind(bind string) error {
+	host, _, err := net.SplitHostPort(bind)
+	if err != nil {
+		return fmt.Errorf("debug.bind %q: %w", bind, err)
+	}
+	if host == "" {
+		return fmt.Errorf("bind %q listens on every interface; use 127.0.0.1 or set debug.allow_public = true to acknowledge public pprof exposure", bind)
+	}
+	var ips []net.IP
+	if ip := net.ParseIP(host); ip != nil {
+		ips = []net.IP{ip}
+	} else if ips, err = net.LookupIP(host); err != nil {
+		return fmt.Errorf("debug.bind %q: %w", bind, err)
+	}
+	for _, ip := range ips {
+		if !ip.IsLoopback() {
+			return fmt.Errorf("bind %q resolves to a non-loopback address; set debug.allow_public = true to acknowledge public pprof exposure", bind)
+		}
+	}
+	return nil
+}
+
 // serveDebug runs the pprof listener until it stops.
 //
 // A failure here must not end the process. pprof is opt-in and optional; the

main_test.go

@@ -248,6 +248,63 @@ func TestDebugServerRejectsPublicBindWithoutAcknowledgement(t *testing.T) {
 	}
 }
 
+// The loopback check runs before the port is claimed (#64 item 5).
+//
+// The public port is taken first by the test itself. A server that binds
+// before checking fails with "address already in use"; one that checks first
+// never touches the socket and reports the allow_public guidance instead.
+func TestDebugServerRejectsPublicBindBeforeListening(t *testing.T) {
+	held, err := net.Listen("tcp", "0.0.0.0:0")
+	if err != nil {
+		t.Fatalf("hold a public port: %v", err)
+	}
+	defer held.Close()
+
+	_, err = startDebugServer(config.DebugConfig{Bind: held.Addr().String()})
+	if err == nil {
+		t.Fatal("public debug bind was accepted without allow_public")
+	}
+	if !strings.Contains(err.Error(), "debug.allow_public = true") {
+		t.Fatalf("error = %q; want the allow_public guidance, not a listen error", err)
+	}
+}
+
+func TestDebugServerNamesAnAllInterfacesBind(t *testing.T) {
+	_, err := startDebugServer(config.DebugConfig{Bind: ":0"})
+	if err == nil {
+		t.Fatal(`":0" was accepted without allow_public`)
+	}
+	if !strings.Contains(err.Error(), "every interface") {
+		t.Fatalf("error = %q, want it to say the bind covers every interface", err)
+	}
+}
+
+func TestDebugServerRejectsAMalformedBind(t *testing.T) {
+	_, err := startDebugServer(config.DebugConfig{Bind: "6060"})
+	if err == nil || !strings.Contains(err.Error(), "debug.bind") {
+		t.Fatalf("error = %v, want a debug.bind parse error", err)
+	}
+}
+
+func TestDebugServerAcceptsLoopbackBinds(t *testing.T) {
+	for _, bind := range []string{"127.0.0.1:0", "[::1]:0", "localhost:0"} {
+		t.Run(bind, func(t *testing.T) {
+			if bind == "[::1]:0" {
+				if l, err := net.Listen("tcp", bind); err != nil {
+					t.Skipf("no IPv6 loopback here: %v", err)
+				} else {
+					l.Close()
+				}
+			}
+			srv, err := startDebugServer(config.DebugConfig{Bind: bind})
+			if err != nil {
+				t.Fatalf("startDebugServer(%q): %v", bind, err)
+			}
+			srv.Close()
+		})
+	}
+}
+
 // failingListener returns a permanent Accept error, the shape of a listener
 // that has died under a running server. net/http retries temporary errors, so
 // a plain error is what actually terminates Serve.