Reauthorize the X display when the server tig has to reach changes

docs/install-macos.md

@@ -113,10 +113,15 @@ which xhost                       # /usr/X11/bin/xhost or /opt/X11/bin/xhost
 
 With XQuartz installed, TIG does the rest of the X11 setup itself on each run:
 it sets `nolisten_tcp` to `false` so XQuartz accepts TCP connections, starts
-XQuartz if it is not already running, and runs `xhost +localhost` to authorize
-the connection. Under Podman the container is given
-`DISPLAY=host.containers.internal:0`, which routes back out of the VM to
-XQuartz on your Mac.
+XQuartz if it is not already running, and runs
+`xhost +localhost +inet:127.0.0.1` to authorize the connection. Under Podman
+the container is given `DISPLAY=host.containers.internal:0`, which routes back
+out of the VM to XQuartz on your Mac.
+
+An XQuartz update or a logout restarts the X server, which drops every `xhost`
+authorization it was holding. TIG reauthorizes before every command, so running
+`xhost + 127.0.0.1` by hand is not part of using `xvd`. Set `TIG_NO_X11=1` if
+you would rather manage the display yourself.
 
 Check a window actually opens:
 
@@ -128,7 +133,9 @@ XQuartz appears in the Dock and displays the 64×64 test image. Quit the window
 to return to the shell.
 
 > If you changed XQuartz's `Allow connections from network clients` setting by
-> hand, restart XQuartz afterwards — the preference is read at startup.
+> hand, restart XQuartz afterwards — the preference is read at startup. When
+> TIG finds XQuartz already running with TCP off, it says so and asks you to
+> quit it, rather than quitting it under your other X clients.
 
 ## 5. Apple Silicon: x86-64 emulation
 
@@ -176,6 +183,7 @@ prefix, see `tig --shim` in the [tig-cli README](../tig-cli/README.md).
 | `Error: no container runtime found` | The machine is not running, or Podman is not on `PATH`. Run `podman machine start`, then `podman info`. |
 | GUI tool prints `Can't open display` or `Error: Can't open display: ` | XQuartz is missing, or you have not logged out and back in since installing it. Check `echo $DISPLAY` and `which xhost` (step 4). |
 | `xhost: unable to open display` | XQuartz is installed but `DISPLAY` points at a stale socket. Quit XQuartz, open a new terminal, and retry. |
+| GUI tools stopped opening after an XQuartz update | XQuartz came back with TCP connections turned off. TIG rewrites the preference, but it is only read at startup: quit XQuartz and run the command again. |
 | A window opens but is blank or freezes | XQuartz was running before its `nolisten_tcp` setting changed. Quit XQuartz entirely and rerun the command; TIG restarts it. |
 | `exec format error`, `no matching manifest` | No x86-64 emulation in the VM on Apple Silicon (step 5). |
 | VICAR says a file does not exist, though it exists on your Mac | The path is outside your home directory and therefore not shared with the VM. Move the data under `$HOME` or recreate the machine with `-v` (step 2). |

tig-cli/README.md

@@ -330,22 +330,33 @@ same reason.
 
 ## GUI tools and X11
 
-GUI tools such as `xvd` and `marsmap` render on your host display. When tig
-creates a container it first authorizes the display, so you do not have to:
+GUI tools such as `xvd` and `marsmap` render on your host display. tig
+authorizes the display for you, so you do not have to:
 
 - **Linux** — runs `xhost +local:`. The broad form is deliberate: with
   `label=disable` the container connects as the `LOCAL:` family, which
   `xhost +local:docker` does not cover. The container shares the host network
   and `/tmp/.X11-unix`, and `DISPLAY` is passed through per command.
 - **macOS** — makes XQuartz listen on TCP
   (`defaults write org.xquartz.X11 nolisten_tcp -bool false`), starts it if it
-  is not running, and runs `xhost +localhost`; the container uses
-  `DISPLAY=host.docker.internal:0` (`host.containers.internal` under Podman,
-  `host.lima.internal` under Finch).
-
-This happens only when a container is created, not on every command, and is
-skipped silently when there is no `DISPLAY` or no `xhost` (a headless host has
-no display to authorize).
+  is not running, and runs `xhost +localhost +inet:127.0.0.1`; the container
+  uses `DISPLAY=host.docker.internal:0` (`host.containers.internal` under
+  Podman, `host.lima.internal` under Finch).
+
+This happens before every command. An `xhost` authorization is the X server's
+own state, and the container outlives the server — an XQuartz update, a logout,
+an `xhost -` — so authorizing only at container creation left users running
+`xhost` by hand afterwards. `xhost` is idempotent and costs milliseconds, so it
+is simply repeated; the slower macOS work (starting XQuartz, reading its
+preferences) is skipped while the X server is the one tig last saw, which it
+remembers in `~/.cache/tig/x11-authorized`. Setting `TIG_NO_X11=1` leaves the
+host display alone entirely. Everything here is skipped silently when there is
+no `DISPLAY` or no `xhost` (a headless host has no display to authorize).
+
+XQuartz reads `nolisten_tcp` only when it starts. If it is already running with
+TCP turned off — how it comes out of an update that reset the preference — tig
+says so and asks you to quit XQuartz, rather than quitting it under your other
+X clients.
 
 ## SELinux (RHEL, Fedora, CentOS)
 

tig-cli/src/tig_cli/container.py

@@ -597,6 +597,10 @@ def execute_vicar_command(
         Returns:
             Exit code from command execution
         """
+        # Per command, not per container: the authorization belongs to the X
+        # server, which restarts far more often than the container does.
+        ensure_x11_ready()
+
         if self.disable_path_translation:
             translated_args = args
             container_cwd = os.getcwd()

tig-cli/src/tig_cli/fast.py

@@ -31,6 +31,7 @@
     build_run_spec,
     container_carries_builds,
     container_display,
+    ensure_x11_ready,
     forwarded_signals,
     get_calibration_path,
     get_container_image,
@@ -80,6 +81,10 @@ def run(argv: list[str]) -> int | None:
     if not container_carries_builds(plan.name):
         return None
 
+    # The container is long-lived and the X server is not, so the display is
+    # authorized per command rather than when the container was created.
+    ensure_x11_ready()
+
     claim = Claim()
     claim.acquire(plan.name)
     try:

tig-cli/src/tig_cli/spec.py

@@ -101,6 +101,13 @@ def __exit__(self, *exc_info) -> bool:
 # How long to wait for XQuartz to come up on macOS before giving up.
 XQUARTZ_START_TIMEOUT = 5.0
 
+# Names the X server that was authorized last, so a command pays for xhost
+# only when the server it has to reach is not that one any more.
+X11_MARKER = "x11-authorized"
+
+# Set to leave the host display alone entirely.
+DISABLE_X11_ENV = "TIG_NO_X11"
+
 # Where calibration files (VISOR / mars_calibration_*) are mounted, matching
 # the layout VICAR's MARS programs expect.
 CALIBRATION_MOUNT = "/usr/local/vicar/mars_calib"
@@ -214,6 +221,21 @@ def _run_quietly(command: list[str], timeout: float = 10.0) -> bool:
     return completed.returncode == 0
 
 
+def _output_of(command: list[str], timeout: float = 10.0) -> str | None:
+    """Run a host helper command; its output, or None if it failed."""
+    import subprocess
+
+    try:
+        completed = subprocess.run(
+            command, capture_output=True, text=True, timeout=timeout
+        )
+    except (OSError, subprocess.SubprocessError):
+        return None
+    if completed.returncode != 0:
+        return None
+    return completed.stdout
+
+
 def selinux_enforcing() -> bool:
     """Whether this host is Linux with SELinux in Enforcing mode.
 
@@ -243,13 +265,38 @@ def selinux_enforcing() -> bool:
     return completed.returncode == 0 and completed.stdout.strip() == "Enforcing"
 
 
-def _ensure_xquartz() -> None:
-    """Make XQuartz running and listening on TCP, as the container needs."""
-    import time
+def _tcp_refused() -> bool:
+    """Whether XQuartz is set to refuse the TCP connections tig needs."""
+    value = _output_of(["defaults", "read", "org.xquartz.X11", "nolisten_tcp"])
+    return value is None or value.strip().lower() not in ("0", "false", "no")
+
 
+def _allow_tcp_connections() -> None:
+    """Have XQuartz listen on TCP, which the container's DISPLAY needs.
+
+    The preference is only read at startup, so a running XQuartz that still
+    refuses TCP - how it comes out of an update that reset the preference -
+    is reported rather than quit out from under the user's other clients.
+    """
+    if not _tcp_refused():
+        return
     _run_quietly(
         ["defaults", "write", "org.xquartz.X11", "nolisten_tcp", "-bool", "false"]
     )
+    if _run_quietly(["pgrep", "-x", "Xquartz"]):
+        print(
+            "tig: XQuartz is running with TCP connections turned off, which "
+            "GUI tools in the container need. Quit XQuartz and run the "
+            "command again.",
+            file=sys.stderr,
+        )
+
+
+def _ensure_xquartz() -> None:
+    """Make XQuartz running and listening on TCP, as the container needs."""
+    import time
+
+    _allow_tcp_connections()
     if _run_quietly(["pgrep", "-x", "Xquartz"]):
         return
     if not _run_quietly(["open", "-a", "XQuartz"]):
@@ -261,26 +308,102 @@ def _ensure_xquartz() -> None:
         time.sleep(0.5)
 
 
+def _x_socket(display: str) -> str | None:
+    """The socket file a local ``display`` names, if it is a local one."""
+    host, separator, number = display.rpartition(":")
+    if not separator or host not in ("", "unix") or not number:
+        return None
+    return f"/tmp/.X11-unix/X{number.split('.')[0]}"
+
+
+def x_server_identity() -> str | None:
+    """Something that differs whenever the X server is a different one.
+
+    Recognizing the server is what lets the expensive XQuartz checks be
+    skipped while it stays the same. ``None`` means no server was
+    recognized, and the caller should do the work rather than assume.
+    """
+    display = os.environ.get("DISPLAY", "")
+    if sys.platform == "darwin":
+        # The process, since the socket DISPLAY names belongs to launchd and
+        # outlives any one XQuartz.
+        running = _output_of(["pgrep", "-x", "Xquartz"]) or ""
+        pids = running.split()
+        return f"{display} xquartz={pids[0]}" if pids else None
+
+    socket = _x_socket(display)
+    if socket is None:
+        # A display reached over the network: nothing local says whether it
+        # is still the server that was authorized.
+        return None
+    try:
+        info = os.stat(socket)
+    except OSError:
+        return None
+    return f"{display} socket={info.st_ino}:{int(info.st_mtime)}"
+
+
+def _x11_marker() -> Path:
+    return Path(home_directory()) / ".cache" / "tig" / X11_MARKER
+
+
+def _authorized_server() -> str | None:
+    """The server the last authorization was for, if it is still recorded."""
+    try:
+        return _x11_marker().read_text()
+    except OSError:
+        return None
+
+
+def _record_authorized_server(identity: str) -> None:
+    marker = _x11_marker()
+    try:
+        marker.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+        marker.write_text(identity)
+    except OSError:
+        pass
+
 
 def ensure_x11_ready() -> None:
     """Authorize the host X server so GUI tools (xvd, marsmap) can connect.
 
+    Done before every command rather than only when a container is created:
+    the authorization is the X server's own state, which the container
+    outlives, so anything that drops it - a restart after an XQuartz update,
+    a logout, an ``xhost -`` - leaves the container unable to connect. That
+    is what has users running ``xhost`` by hand. ``xhost`` is idempotent and
+    costs milliseconds, so it is simply repeated; only the pricier XQuartz
+    checks are skipped while the server is the one already seen.
+
     Silently does nothing when there is no display or no ``xhost``; those
     hosts simply have no GUI to authorize.
     """
     import shutil
 
+    if os.environ.get(DISABLE_X11_ENV):
+        return
     if not os.environ.get("DISPLAY") or shutil.which("xhost") is None:
         return
 
+    identity = x_server_identity()
     if sys.platform == "darwin":
-        # The container reaches XQuartz over TCP via the host gateway name.
-        _ensure_xquartz()
-        _run_quietly(["xhost", "+localhost"])
+        if identity is None or identity != _authorized_server():
+            # The container reaches XQuartz over TCP via the host gateway
+            # name, so XQuartz has to be up and listening on it.
+            _ensure_xquartz()
+            # Asked again, since XQuartz may have only just been started.
+            identity = x_server_identity()
+        # The address explicitly as well as the name: the connection arrives
+        # from the runtime's gateway on IPv4 loopback however localhost
+        # happens to resolve.
+        authorized = _run_quietly(["xhost", "+localhost", "+inet:127.0.0.1"])
     else:
         # The broad form is deliberate: with 'label=disable' the container
         # connects as the LOCAL: family, which '+local:docker' misses.
-        _run_quietly(["xhost", "+local:"])
+        authorized = _run_quietly(["xhost", "+local:"])
+
+    if authorized and identity is not None:
+        _record_authorized_server(identity)
 
 
 def container_display(runtime: str = "docker") -> str: