Harden Hub startup and display-name handling

- Display names starting with '-' were parsed by tmux rename-window as
  flags. The alias was persisted before tmux rejected it, so every later
  start/refresh/open failed in refresh_labels until aliases.json was
  edited by hand. Guard rename-window with '--', apply the name before
  persisting it, and roll back the in-memory table on failure.
- Start the Hub tmux server with -f /dev/null by default. Reading the
  user's ~/.tmux.conf let theme status-formats replace the Sessions bar
  and root-table bindings leak into Herdr; HERDR_HUB_TMUX_CONFIG still
  opts into a custom file.
- An unreadable aliases.json blocked start/refresh/open/choose. Degrade
  to default names with a warning (stderr, or the popup message) and
  leave the file untouched for `doctor` to report.
- `doctor` crashed with a traceback when a *_BIN override pointed at a
  missing executable; report it as a failure instead.
- Refuse to run inside another tmux server before discovery and server
  creation, so no unattached Hub is left behind.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

herdr_hub/cli.py

@@ -47,6 +47,9 @@ from .selector import (
 
 DEFAULT_SOCKET_NAME = "herdr-hub"
 DEFAULT_TMUX_SESSION = "hub"
+# The Hub server is configured entirely by configure(); reading the user's
+# ~/.tmux.conf would let theme status-formats and root-table bindings leak in.
+DEFAULT_TMUX_CONFIG = "/dev/null"
 DEFAULT_SESSION_NAME = "default"
 BOOTSTRAP_WINDOW_NAME = "__herdr_hub_bootstrap__"
 WINDOW_KEY_OPTION = "@herdr_hub_key"
@@ -414,7 +417,7 @@ def discover_remote_processes(
             raw = cmdline_path.read_bytes()
         except (FileNotFoundError, ProcessLookupError):
             continue
-        except (PermissionError, OSError):
+        except OSError:
             unreadable += 1
             continue
         endpoint = parse_remote_argv(decode_cmdline(raw))
@@ -749,7 +752,10 @@ class TmuxHub:
                 truncate_label(alias, 32) if alias else window.endpoint.short_label
             )
         if window.name != display_label:
-            self.run(["rename-window", "-t", window.window_id, display_label])
+            # A display name may start with '-', which tmux would otherwise
+            # parse as a flag. set-option takes the value positionally and
+            # does not accept '--', so only rename-window needs the guard.
+            self.run(["rename-window", "-t", window.window_id, "--", display_label])
         self.run(
             [
                 "set-option",
@@ -763,13 +769,22 @@ class TmuxHub:
 
     @serialized_hub_mutation
     def apply_alias(self, key: str, label: str) -> ManagedWindow | None:
+        previous = self.aliases.get(key)
         if label:
             self.aliases[key] = label
         else:
             self.aliases.pop(key, None)
         window = self.window_for_key(key)
         if window is not None:
-            self._apply_window_label(window)
+            try:
+                self._apply_window_label(window)
+            except HubError:
+                # Keep the in-memory table consistent with what tmux shows.
+                if previous is None:
+                    self.aliases.pop(key, None)
+                else:
+                    self.aliases[key] = previous
+                raise
         return self.window_for_key(key)
 
     def dismissed_keys(self) -> set[str]:
@@ -1065,12 +1080,9 @@ class TmuxHub:
             time.sleep(min(0.05, remaining))
 
     def attach(self) -> None:
-        if inside_any_tmux():
-            if inside_socket(self.socket_name):
-                return
-            raise HubError(
-                "already inside another tmux server; detach from it before attaching to herdr-hub"
-            )
+        if inside_socket(self.socket_name):
+            return
+        ensure_not_nested(self.socket_name)
         result = self.run(
             ["attach-session", "-t", self.session_name],
             check=False,
@@ -1117,20 +1129,50 @@ def invocation_script_path() -> str:
     return str(invoked)
 
 
+def configured_socket_name() -> str:
+    return os.environ.get("HERDR_HUB_SOCKET", DEFAULT_SOCKET_NAME)
+
+
+def configured_tmux_config() -> str:
+    override = os.environ.get("HERDR_HUB_TMUX_CONFIG")
+    return os.path.expanduser(override) if override else DEFAULT_TMUX_CONFIG
+
+
+def load_alias_table() -> tuple[dict[str, str], str | None]:
+    """Load display names, degrading to defaults when the file is unreadable.
+
+    A corrupt optional file must not block start/refresh/open; `doctor`
+    reports the problem and the warning is surfaced to the caller.
+    """
+    registry = make_alias_registry()
+    try:
+        return registry.load(), None
+    except AliasRegistryError as error:
+        return {}, f"display names unavailable, using defaults: {error}"
+
+
 def make_hub(
     arguments: argparse.Namespace,
     *,
     load_aliases: bool = True,
+    warnings: list[str] | None = None,
 ) -> TmuxHub:
     tmux_bin = resolve_binary("tmux", os.environ.get("HERDR_HUB_TMUX_BIN"))
     ssh_bin = os.environ.get("HERDR_HUB_SSH_BIN") or shutil.which("ssh") or "ssh"
-    aliases = make_alias_registry().load() if load_aliases else {}
+    aliases: dict[str, str] = {}
+    if load_aliases:
+        aliases, alias_warning = load_alias_table()
+        if alias_warning is not None:
+            if warnings is not None:
+                warnings.append(alias_warning)
+            else:
+                print(f"herdr-hub: warning: {alias_warning}", file=sys.stderr)
     return TmuxHub(
         tmux_bin=tmux_bin,
         ssh_bin=ssh_bin,
-        socket_name=os.environ.get("HERDR_HUB_SOCKET", DEFAULT_SOCKET_NAME),
+        socket_name=configured_socket_name(),
         session_name=os.environ.get("HERDR_HUB_SESSION", DEFAULT_TMUX_SESSION),
-        config_file=os.environ.get("HERDR_HUB_TMUX_CONFIG"),
+        config_file=configured_tmux_config(),
         script_path=invocation_script_path(),
         aliases=aliases,
     )
@@ -1199,7 +1241,17 @@ def catalog_windows(windows: Iterable[ManagedWindow]) -> list[CatalogWindow]:
     ]
 
 
+def ensure_not_nested(socket_name: str) -> None:
+    if inside_any_tmux() and not inside_socket(socket_name):
+        raise HubError(
+            "already inside another tmux server; detach from it before attaching to herdr-hub"
+        )
+
+
 def command_start(arguments: argparse.Namespace) -> int:
+    # Fail before discovery and server creation: a Hub nobody can attach to
+    # would otherwise be left running in the background.
+    ensure_not_nested(configured_socket_name())
     herdr_bin = resolve_herdr()
     report = discover_endpoints(herdr_bin, configured_proc_root())
     if warning := remember_remote_endpoints(report.endpoints):
@@ -1280,7 +1332,8 @@ def command_open(arguments: argparse.Namespace) -> int:
 
 
 def command_choose(arguments: argparse.Namespace) -> int:
-    hub = make_hub(arguments)
+    startup_warnings: list[str] = []
+    hub = make_hub(arguments, warnings=startup_warnings)
     if not hub.exists():
         raise HubError("herdr-hub is not running")
     windows = hub.managed_windows()
@@ -1298,9 +1351,17 @@ def command_choose(arguments: argparse.Namespace) -> int:
         return catalog_windows(hub.managed_windows())
 
     def rename_session(endpoint_key: str, alias: str) -> None:
-        alias_registry.set(endpoint_key, alias)
+        # Apply to tmux first: a name tmux rejects must never be persisted,
+        # or every later start/refresh would fail while re-applying it.
+        previous = hub.aliases.get(endpoint_key, "")
         if hub.apply_alias(endpoint_key, alias) is None:
+            hub.apply_alias(endpoint_key, previous)
             raise HubError("the selected Session is no longer open")
+        try:
+            alias_registry.set(endpoint_key, alias)
+        except AliasRegistryError:
+            hub.apply_alias(endpoint_key, previous)
+            raise
 
     def delete_session(
         target: str,
@@ -1408,7 +1469,7 @@ def command_choose(arguments: argparse.Namespace) -> int:
             delete_session_handler=delete_session,
             stop_session_handler=stop_session,
         )
-        startup_warnings = [*local_report.warnings]
+        startup_warnings.extend(local_report.warnings)
         if registry_warning:
             startup_warnings.append(registry_warning)
         if startup_warnings:
@@ -1556,12 +1617,18 @@ def command_doctor(arguments: argparse.Namespace) -> int:
             failures += 1
             continue
         version_args = [path, "-V"] if binary in {"tmux", "ssh"} else [path, "--version"]
-        result = subprocess.run(
-            version_args,
-            capture_output=True,
-            text=True,
-            check=False,
-        )
+        try:
+            result = subprocess.run(
+                version_args,
+                capture_output=True,
+                text=True,
+                check=False,
+                timeout=10,
+            )
+        except (OSError, subprocess.SubprocessError) as error:
+            print(f"{binary}: cannot run {path} ({error})")
+            failures += 1
+            continue
         version = (result.stdout or result.stderr).strip()
         print(f"{binary}: {version or path}")
     registry = make_registry()

tests/test_cli_integration.py

@@ -128,6 +128,17 @@ class CliIntegrationTests(unittest.TestCase):
             # Operational recovery must remain possible even if optional
             # display-name configuration is later corrupted.
             (root / "aliases.json").write_text("not json", encoding="utf-8")
+            degraded = subprocess.run(
+                [str(launcher), "refresh"],
+                cwd=repository_root,
+                env=environment,
+                capture_output=True,
+                text=True,
+                check=False,
+            )
+            self.assertEqual(degraded.returncode, 0, degraded.stderr)
+            self.assertIn("display names unavailable", degraded.stderr)
+            self.assertEqual(hub.managed_windows()[0].name, "L default")
             stop = subprocess.run(
                 [str(launcher), "stop"],
                 cwd=repository_root,

tests/test_discovery.py

@@ -1,20 +1,29 @@
 from __future__ import annotations
 
+import argparse
+import contextlib
+import io
 import json
 import os
 from pathlib import Path
 import subprocess
 import tempfile
 import unittest
+from unittest.mock import patch
 
 from herdr_hub.cli import (
     DEFAULT_SESSION_NAME,
+    DEFAULT_TMUX_CONFIG,
     Endpoint,
+    HubError,
+    command_doctor,
+    command_start,
     decode_cmdline,
     dedupe_endpoints,
     deserialize_endpoint,
     discover_local_sessions,
     discover_remote_processes,
+    make_hub,
     parse_remote_argv,
     serialize_endpoint,
 )
@@ -199,5 +208,71 @@ class DiscoveryTests(unittest.TestCase):
         )
 
 
+class CliEnvironmentTests(unittest.TestCase):
+    def test_hub_ignores_user_tmux_conf_unless_overridden(self) -> None:
+        with tempfile.TemporaryDirectory() as directory:
+            environment = {
+                "HERDR_HUB_TMUX_BIN": "/bin/true",
+                "HERDR_HUB_ALIASES_FILE": str(Path(directory) / "aliases.json"),
+            }
+            with patch.dict(os.environ, environment):
+                os.environ.pop("HERDR_HUB_TMUX_CONFIG", None)
+                hub = make_hub(argparse.Namespace())
+                self.assertEqual(hub.config_file, DEFAULT_TMUX_CONFIG)
+                self.assertEqual(hub.base_command()[-2:], ["-f", "/dev/null"])
+            with patch.dict(
+                os.environ, {**environment, "HERDR_HUB_TMUX_CONFIG": "~/hub.conf"}
+            ):
+                hub = make_hub(argparse.Namespace())
+                self.assertEqual(hub.config_file, os.path.expanduser("~/hub.conf"))
+
+    def test_corrupt_alias_file_degrades_to_defaults_with_warning(self) -> None:
+        with tempfile.TemporaryDirectory() as directory:
+            aliases = Path(directory) / "aliases.json"
+            aliases.write_text("not json", encoding="utf-8")
+            environment = {
+                "HERDR_HUB_TMUX_BIN": "/bin/true",
+                "HERDR_HUB_ALIASES_FILE": str(aliases),
+            }
+            warnings: list[str] = []
+            with patch.dict(os.environ, environment):
+                hub = make_hub(argparse.Namespace(), warnings=warnings)
+            self.assertEqual(hub.aliases, {})
+            self.assertEqual(len(warnings), 1)
+            self.assertIn("aliases.json", warnings[0])
+
+            stderr = io.StringIO()
+            with patch.dict(os.environ, environment), contextlib.redirect_stderr(stderr):
+                make_hub(argparse.Namespace())
+            self.assertIn("display names unavailable", stderr.getvalue())
+            # The unreadable file is left untouched for the user to inspect.
+            self.assertEqual(aliases.read_text(encoding="utf-8"), "not json")
+
+    def test_start_refuses_nested_tmux_before_discovery(self) -> None:
+        environment = {"TMUX": "/tmp/tmux-1/other,1,0", "HERDR_HUB_SOCKET": "herdr-hub"}
+        with patch.dict(os.environ, environment), patch(
+            "herdr_hub.cli.discover_endpoints",
+            side_effect=AssertionError("discovery must not run inside another tmux"),
+        ):
+            with self.assertRaisesRegex(HubError, "another tmux server"):
+                command_start(argparse.Namespace())
+
+    def test_doctor_reports_unusable_binary_override_instead_of_crashing(self) -> None:
+        with tempfile.TemporaryDirectory() as directory:
+            root = Path(directory)
+            environment = {
+                "HERDR_HUB_HERDR_BIN": str(root / "missing-herdr"),
+                "HERDR_HUB_REMOTES_FILE": str(root / "remotes.json"),
+                "HERDR_HUB_ALIASES_FILE": str(root / "aliases.json"),
+                "HERDR_HUB_PROC_ROOT": directory,
+            }
+            stdout = io.StringIO()
+            with patch.dict(os.environ, environment), contextlib.redirect_stdout(stdout):
+                status = command_doctor(argparse.Namespace())
+            self.assertGreaterEqual(status, 1)
+            self.assertIn("herdr: cannot run", stdout.getvalue())
+            self.assertIn("discovery failed", stdout.getvalue())
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_tmux_integration.py

@@ -310,6 +310,16 @@ class TmuxIntegrationTests(unittest.TestCase):
         renamed = self.hub.apply_alias(window.key, "临时名称")
         assert renamed is not None
         self.assertEqual(renamed.name, "临时名称")
+        # tmux parses a leading '-' as a flag unless the name is guarded; a
+        # stored name like this must also survive the label refresh that every
+        # later start/refresh performs.
+        dashed = self.hub.apply_alias(window.key, "-dash")
+        assert dashed is not None
+        self.assertEqual(dashed.name, "-dash")
+        self.hub.ensure_server()
+        refreshed = self.hub.window_for_key(window.key)
+        assert refreshed is not None
+        self.assertEqual(refreshed.name, "-dash")
         restored = self.hub.apply_alias(window.key, "")
         assert restored is not None
         self.assertEqual(restored.name, "R orangepi/work")