From 809169b7d00756ebbc79ac7e69b8d7f53cb0ac7e Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 05:53:09 -0700 Subject: [PATCH 1/5] fix(bot-screen): reap the X server a dead launcher leaves behind instead of starting a second one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit status() and stop() keyed only on the launcher pid. When the launcher was SIGKILLed (OOM, a stray kill, a crashed supervisor) its Xvnc kept running in the launcher's session, holding the display number and rfb.sock, while status() reported stopped. The next start() then allocated a fresh display and spawned a second server whose launcher unlinked and re-bound the same rfb.sock path — two X servers, one socket, and the orphan leaked forever because stop() never reached it. The X lock of the recorded display names that server. When no live launcher exists, start() and stop() now check it: a process in the dead launcher's process group, or an Xvnc whose command line binds THIS profile's rfb.sock, is ours and is killed (SIGTERM, then SIGKILL) before the stale state is dropped and a new launch proceeds. A server that merely reused our display number is never touched. _pid_alive also stops treating a zombie as alive: the gateway reaps a killed launcher lazily, and the zombie made status() claim a running screen whose server nobody owned. --- tests/tools/test_bot_desktop_runtime.py | 77 +++++++++++++++++++ tools/bot_desktop/runtime.py | 98 +++++++++++++++++++------ 2 files changed, 153 insertions(+), 22 deletions(-) diff --git a/tests/tools/test_bot_desktop_runtime.py b/tests/tools/test_bot_desktop_runtime.py index fd89a14d6d..d7f1b0ec9b 100644 --- a/tests/tools/test_bot_desktop_runtime.py +++ b/tests/tools/test_bot_desktop_runtime.py @@ -144,3 +144,80 @@ def test_concurrent_starts_of_one_profile_spawn_one_launcher(tmp_path, start_in_ out = _collect([start_in_fresh_process(tmp_path / "a"), start_in_fresh_process(tmp_path / "a")]) assert len({o["pid"] for o in out}) == 1, out assert len(list((tmp_path / "xlocks").glob("spawned.*"))) == 1 + + +_ORPHANING_LAUNCHER = """#!/usr/bin/env bash +# Stands in for launcher.sh whose Xvnc child ("sleep") lives in the launcher's process group and +# outlives a SIGKILL of the launcher itself — the X lock names the child, as the real one does. +: > "$HERMES_BD_XLOCK_DIR/spawned.$$" +sleep 30 & +echo $! > "$HERMES_BD_XLOCK_DIR/.X${HERMES_BD_DISPLAY_NUM}-lock" +: > "$HERMES_BD_SOCKET" +printf 'DISPLAY=:%s\\n' "$HERMES_BD_DISPLAY_NUM" > "$HERMES_BD_ENV_FILE" +wait +""" + + +def _gone(pid: int) -> bool: + import psutil + try: + return psutil.Process(pid).status() == psutil.STATUS_ZOMBIE + except psutil.NoSuchProcess: + return True + + +def _wait_until(pred, timeout=5.0) -> bool: + import time + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if pred(): + return True + time.sleep(0.05) + return pred() + + +@pytest.fixture +def in_process_runtime(tmp_path, monkeypatch): + """runtime.start()/stop() against a scratch state dir and a fake launcher script (set by the test).""" + import os + + home = tmp_path / "home" + (tmp_path / "xlocks").mkdir() + monkeypatch.setattr(runtime, "state_dir", lambda: home / "bot-desktop") + monkeypatch.setattr(runtime, "_LAUNCHER", tmp_path / "launcher.sh") + monkeypatch.setattr(runtime, "_X_LOCK_DIR", tmp_path / "xlocks") + monkeypatch.setattr(runtime, "_ALLOC_LOCK", tmp_path / "alloc.lock") + monkeypatch.setattr(runtime, "missing_binaries", lambda: []) + monkeypatch.setattr(runtime, "geometry", lambda: "800x600") + monkeypatch.setenv("HERMES_BD_XLOCK_DIR", str(tmp_path / "xlocks")) + yield tmp_path + with contextlib.suppress(Exception): + runtime.stop() + for lock in (tmp_path / "xlocks").glob(".X*-lock"): # anything the code under test failed to reap + with contextlib.suppress(OSError, ValueError): + os.kill(int(lock.read_text()), 9) + + +@pytest.mark.linux_only +@pytest.mark.live_system_guard_bypass # the orphan is reparented to init: signalling it is the point +def test_orphaned_x_server_of_a_dead_launcher_is_reaped_on_next_start(in_process_runtime): + """SIGKILL the launcher and its Xvnc survives, holding the display and rfb.sock. status() keys on the + launcher pid and says stopped; start() must find that orphan through the recorded display's X lock + and kill it instead of allocating a second server beside it (two servers, one socket path).""" + import os + import signal + + scratch = in_process_runtime + (scratch / "launcher.sh").write_text(_ORPHANING_LAUNCHER, encoding="utf-8") + first = runtime.start(wait_seconds=10) + lock = scratch / "xlocks" / f".X{first.display.lstrip(':')}-lock" + orphan = int(lock.read_text()) + os.kill(first.pid, signal.SIGKILL) + assert _wait_until(lambda: _gone(first.pid)) + assert not _gone(orphan), "the X server outlives its launcher (that is the bug's precondition)" + assert runtime.status().running is False + + second = runtime.start(wait_seconds=10) + assert second.pid != first.pid and second.running + assert _wait_until(lambda: _gone(orphan)), "the dead launcher's X server must be reaped, not leaked" + assert runtime.stop() is True diff --git a/tools/bot_desktop/runtime.py b/tools/bot_desktop/runtime.py index a9ce1923f0..ace34a9ed6 100644 --- a/tools/bot_desktop/runtime.py +++ b/tools/bot_desktop/runtime.py @@ -125,8 +125,13 @@ def _read(path: Path) -> Optional[str]: def _pid_alive(pid: int) -> bool: + """A zombie is dead for our purposes: a SIGKILLed launcher stays a zombie in the gateway until the next + Popen reaps it, and reporting it as running would hide its orphaned X server behind a live status.""" import psutil - return psutil.pid_exists(pid) + try: + return psutil.Process(pid).status() != psutil.STATUS_ZOMBIE + except psutil.Error: + return False def _create_time(pid: int) -> Optional[float]: @@ -150,21 +155,78 @@ def _launcher_pid() -> Optional[int]: except ValueError: return None actual = _create_time(pid) - return pid if actual is not None and abs(actual - born) < 0.01 else None + return pid if actual is not None and abs(actual - born) < 0.01 and _pid_alive(pid) else None + + +def _recorded_launcher_pid() -> Optional[int]: + """The pid ``launcher.pid`` names, alive or not (the orphan sweep matches process groups against it).""" + pid_s, _, born_s = (_read(state_dir() / "launcher.pid") or "").partition(" ") + return int(pid_s) if pid_s.isdigit() and born_s else None _X_LOCK_DIR = Path("/tmp") # where X servers write .X-lock (tests point it at a scratch dir) +def _x_lock_pid(num: int) -> Optional[int]: + try: + return int((_X_LOCK_DIR / f".X{num}-lock").read_text(encoding="utf-8").strip()) + except (OSError, ValueError): + return None + + def _display_in_use(num: int) -> bool: """A live X server owns ``:num``: its lock file names a running pid. A lock left by a crashed server (dead pid) does not count, so the number can be reclaimed.""" - lock = _X_LOCK_DIR / f".X{num}-lock" - try: - pid = int(lock.read_text(encoding="utf-8").strip()) - except (OSError, ValueError): + pid = _x_lock_pid(num) + return pid is not None and _pid_alive(pid) + + +def _reap_orphaned_server(sd: Path) -> bool: + """Caller holds ``start.lock`` and has established that no live launcher exists. The launcher runs Xvnc in + its own session, so a SIGKILLed launcher leaves the X server alive, holding the display and ``rfb.sock``; + ``status()`` keys on the launcher and says stopped, and a naive restart allocates a second server next + to it and overwrites the socket path both now claim. The X lock of the recorded display names that + server: it is ours when it sits in the dead launcher's process group or its command line binds OUR + socket. Kill it (group first), drop the state it left, and report whether anything was signalled.""" + import psutil + + recorded = _read(sd / "display") + pid = _x_lock_pid(int(recorded)) if recorded and recorded.isdigit() else None + if pid is None or not _pid_alive(pid): return False - return _pid_alive(pid) + launcher = _recorded_launcher_pid() + try: + pgid = os.getpgid(pid) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start/stop) + cmdline = psutil.Process(pid).cmdline() + except (ProcessLookupError, psutil.Error): + return False + binds_our_socket = "Xvnc" in Path(cmdline[0] if cmdline else "").name and str(sd / "rfb.sock") in cmdline + if pgid != launcher and not binds_our_socket: + return False # somebody else's server took the number after we died; never touch it + logger.warning("Bot Desktop launcher %s is gone but its X server (pid %s) survived on :%s; reaping", + launcher, pid, recorded) + _kill_group_then_wait(pgid if pgid == launcher else None, pid) + (_X_LOCK_DIR / f".X{recorded}-lock").unlink(missing_ok=True) + for name in ("launcher.pid", "env", "rfb.sock"): + (sd / name).unlink(missing_ok=True) + return True + + +def _kill_group_then_wait(pgid: Optional[int], pid: int, grace: float = 2.0) -> None: + """SIGTERM the group (or the lone pid), SIGKILL whatever is still there after ``grace``.""" + def _signal(sig: int) -> None: + with contextlib.suppress(ProcessLookupError, PermissionError): + if pgid is not None: + os.killpg(pgid, sig) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start/stop) + else: + os.kill(pid, sig) + _signal(signal.SIGTERM) + deadline = time.monotonic() + grace + while time.monotonic() < deadline: + if not _pid_alive(pid): + return + time.sleep(0.05) + _signal(signal.SIGKILL) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start/stop) _ALLOC_LOCK = Path("/tmp/.hermes-bot-desktop-alloc.lock") # host-wide: profiles allocate from one band @@ -307,6 +369,8 @@ def start(*, wait_seconds: float = 15.0) -> DesktopStatus: with _flocked(sd / "start.lock"): if _launcher_pid() is not None and published_env().get("DISPLAY"): return status() + if _launcher_pid() is None: + _reap_orphaned_server(sd) with _flocked(_ALLOC_LOCK): return _spawn_and_wait(sd, _pick_display(), wait_seconds) @@ -352,7 +416,8 @@ def _spawn_and_wait(sd: Path, num: int, wait_seconds: float) -> DesktopStatus: def stop() -> bool: - """Stop this profile's desktop; True when a running launcher was signalled.""" + """Stop this profile's desktop; True when a running launcher (or the X server a dead one left behind) + was signalled.""" if not is_supported_host(): return False sd = state_dir() @@ -364,22 +429,11 @@ def stop() -> bool: def _stop_locked(sd: Path) -> bool: pid = _launcher_pid() if pid is None: + reaped = _reap_orphaned_server(sd) (sd / "env").unlink(missing_ok=True) - return False + return reaped # The launcher runs in its own session; killing the group takes Xvnc, dbus and Xfce with it. - try: - os.killpg(pid, signal.SIGTERM) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start) - except ProcessLookupError: - pass - for _ in range(50): - if not _pid_alive(pid): - break - time.sleep(0.1) - else: - try: - os.killpg(pid, signal.SIGKILL) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start) - except ProcessLookupError: - pass + _kill_group_then_wait(pid, pid, grace=5.0) (sd / "launcher.pid").unlink(missing_ok=True) (sd / "env").unlink(missing_ok=True) return True From 85d0b1532508773a9ffdade159075f8db41c9600 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 05:53:59 -0700 Subject: [PATCH 2/5] fix(bot-screen): a start() that times out takes its launcher down instead of leaving it to publish later _spawn_and_wait raised on the readiness deadline and walked away from the launcher it had just spawned. Xvnc and Xfce kept coming up; a moment later the launcher published DISPLAY and rfb.sock, and a screen whose start() had reported failure was now "running" under a pid the caller never learned about. A retry then saw it as already up and returned it, so the failure message described a state that no longer existed. On timeout the launch's process group (the launcher is its own session leader, so the group is exactly this launch) gets SIGTERM, then SIGKILL after a grace period, the child is reaped, and only that launch's launcher.pid / env / rfb.sock are removed before the error propagates. --- tests/tools/test_bot_desktop_runtime.py | 33 +++++++++++++++++++++++++ tools/bot_desktop/runtime.py | 7 ++++++ 2 files changed, 40 insertions(+) diff --git a/tests/tools/test_bot_desktop_runtime.py b/tests/tools/test_bot_desktop_runtime.py index d7f1b0ec9b..0d4efe154d 100644 --- a/tests/tools/test_bot_desktop_runtime.py +++ b/tests/tools/test_bot_desktop_runtime.py @@ -221,3 +221,36 @@ def test_orphaned_x_server_of_a_dead_launcher_is_reaped_on_next_start(in_process assert second.pid != first.pid and second.running assert _wait_until(lambda: _gone(orphan)), "the dead launcher's X server must be reaped, not leaked" assert runtime.stop() is True + + +_SLOW_LAUNCHER = """#!/usr/bin/env bash +# Publishes only AFTER runtime.start()'s readiness deadline has passed. +sleep 30 & +echo $! > "$HERMES_BD_XLOCK_DIR/.X${HERMES_BD_DISPLAY_NUM}-lock" +sleep 1 +: > "$HERMES_BD_SOCKET" +printf 'DISPLAY=:%s\\n' "$HERMES_BD_DISPLAY_NUM" > "$HERMES_BD_ENV_FILE" +wait +""" + + +@pytest.mark.linux_only +@pytest.mark.live_system_guard_bypass # the launcher's group must really be signalled +def test_readiness_timeout_terminates_the_launch_it_gave_up_on(in_process_runtime): + """When the launcher misses the readiness deadline start() raises — and must take the launch down with + it. It used to leave the launcher running; the child then published DISPLAY/rfb.sock a moment later and + a screen nobody asked for (and whose start() had reported failure) stayed up behind a 'running' status.""" + import time + + scratch = in_process_runtime + (scratch / "launcher.sh").write_text(_SLOW_LAUNCHER, encoding="utf-8") + sd = runtime.state_dir() + with pytest.raises(RuntimeError, match="did not publish"): + runtime.start(wait_seconds=0.05) + launcher = runtime._recorded_launcher_pid() + assert launcher is None or _gone(launcher), "the timed-out launcher must be reaped, not left to publish later" + time.sleep(1.5) # past the slow launcher's publish time + assert not (sd / "env").exists() and not (sd / "rfb.sock").exists() + assert runtime.status().running is False + for lock in (scratch / "xlocks").glob(".X*-lock"): + assert _gone(int(lock.read_text())), "the launch's X server must die with its launcher" diff --git a/tools/bot_desktop/runtime.py b/tools/bot_desktop/runtime.py index ace34a9ed6..f0b7e293ed 100644 --- a/tools/bot_desktop/runtime.py +++ b/tools/bot_desktop/runtime.py @@ -412,6 +412,13 @@ def _spawn_and_wait(sd: Path, num: int, wait_seconds: float) -> DesktopStatus: logger.info("Bot Desktop for profile %s up on :%s", _profile_name(), num) return status() time.sleep(0.1) + # Giving up must take the launch down: left alone, the launcher publishes DISPLAY and rfb.sock a moment + # later and a screen whose start() reported failure stays up as "running". The launcher is its own + # session leader, so its group is exactly this launch (Xvnc, dbus, Xfce) and nothing else. + _kill_group_then_wait(proc.pid, proc.pid) + proc.wait() + for name in ("launcher.pid", "env", "rfb.sock"): + (sd / name).unlink(missing_ok=True) raise RuntimeError(f"Bot Desktop did not publish its display within {wait_seconds:.0f}s (see {sd / 'launcher.log'})") From ce24f855f65bf8b602e32a9f748464472f527864 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 05:55:57 -0700 Subject: [PATCH 3/5] fix(bot-screen): profile delete/rename stop the profile's Bot Desktop, not only its gateway delete_profile and rename_profile stopped the gateway and stray backends but never the Bot Desktop launcher, which runs in its own session with its own pid file. After a delete, Xvnc + Xfce kept running against a directory that no longer existed, holding a display number and an rfb.sock nobody could reach through status(); after a rename the launcher still pointed at paths under the old name, so the renamed profile's status() saw no screen while the old one lived on. Both ops now call runtime.stop() under a hermes-home override scoped to the OLD profile directory (the runtime reads that profile's bot-desktop/ state whichever profile invoked the command), gated on is_supported_host(). A failure to stop the screen is logged and never aborts the profile operation the user asked for. --- hermes_cli/profiles.py | 23 ++++++++++++++++++++- tests/hermes_cli/test_profiles.py | 34 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 12c98d1048..2043f294a2 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1075,6 +1075,25 @@ def _stop_profile_backends(canon: str, profile_dir: Path) -> None: print(f"✓ Stopped {len(pids)} profile backend process(es)") +def _stop_bot_desktop(profile_dir: Path) -> None: + """Stop the profile's Bot Desktop (Xvnc + Xfce launcher) before its directory is removed or renamed; + gateway shutdown does not reach it (its own session, its own pid file). Scoped through the hermes-home + override so the runtime reads THIS profile's bot-desktop/ state, whichever profile invoked the op. + A failure here is logged, never fatal: the profile op is what the user asked for.""" + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from tools.bot_desktop import runtime + if not runtime.is_supported_host(): + return + token = set_hermes_home_override(profile_dir) + try: + if runtime.stop(): + print("✓ Bot Desktop stopped") + except Exception as e: + logger.warning("Could not stop the Bot Desktop of %s: %s", profile_dir, e) + finally: + reset_hermes_home_override(token) + + def _rmtree_make_writable(func, path, exc): """onexc/onerror handler: add +w on PermissionError so rmtree can proceed. Covers NixOS- style read-only copies where the path itself (0444) or its parent (0555) isn't writable.""" @@ -1168,6 +1187,7 @@ def delete_profile(name: str, yes: bool = False) -> Path: if gw_running: _stop_gateway_process(profile_dir) _stop_profile_backends(canon, profile_dir) + _stop_bot_desktop(profile_dir) # Tombstone before rmtree so a stale serve/logging mkdir cannot relist this name live. mark_named_profile_deleted(profile_dir) @@ -1662,10 +1682,11 @@ def rename_profile(old_name: str, new_name: str) -> Path: if new_dir.exists(): raise FileExistsError(f"Profile '{new_canon}' already exists.") - # 1. Stop gateway if running + # 1. Stop gateway if running, and the screen whose launcher holds paths under the old name if _check_gateway_running(old_dir): _cleanup_gateway_service(old_canon, old_dir) _stop_gateway_process(old_dir) + _stop_bot_desktop(old_dir) # 2. Rename directory old_dir.rename(new_dir) diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index b7c7bf61a4..91d6c03434 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -1161,3 +1161,37 @@ class TestResolveProfileEnvSpelling: assert Path(resolve_profile_env("default")) == _get_default_hermes_home() + + +def _live_bot_desktop_launcher(profile_dir: Path): + """A synthetic Bot Desktop launcher for ``profile_dir``: its own session (like launcher.sh) with the + identity file + env runtime.status() reads, so the profile op sees a running screen.""" + import subprocess + from tools.bot_desktop import runtime + + proc = subprocess.Popen(["sleep", "60"], start_new_session=True) + sd = profile_dir / "bot-desktop" + sd.mkdir() + (sd / "launcher.pid").write_text(f"{proc.pid} {runtime._create_time(proc.pid)}", encoding="utf-8") + (sd / "env").write_text("DISPLAY=:42\n", encoding="utf-8") + return proc + + +@pytest.mark.linux_only +@pytest.mark.parametrize("op", ["delete", "rename"]) +def test_profile_delete_and_rename_stop_the_profiles_bot_desktop(profile_env, op): + """Deleting or renaming a profile stops its gateway, and must stop its Bot Desktop launcher too: the + Xvnc/Xfce session otherwise keeps running against a directory that no longer exists (or now belongs to + another name), holding its display number and an rfb.sock nobody can reach through status().""" + profile_dir = create_profile("coder", no_alias=True) + proc = _live_bot_desktop_launcher(profile_dir) + try: + with patch("hermes_cli.profiles._cleanup_gateway_service"), \ + patch("hermes_cli.profiles.check_alias_collision", return_value="skip"): + if op == "delete": + delete_profile("coder", yes=True) + else: + rename_profile("coder", "hacker") + assert proc.wait(timeout=10) != 0, "the launcher was signalled by the profile op" + finally: + proc.kill() From 3abb4debc740a0dcd596b8f6d5a4b7f4f4ee646c Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 05:56:46 -0700 Subject: [PATCH 4/5] fix(bot-screen): profile export leaves bot-desktop/ (the screen's cookie jar) out of the archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A named-profile export copied bot-desktop/ wholesale: the screen's persistent Chromium profile under bot-desktop/browser-profile carries Cookies and Login Data — every web session the bot is logged into — plus Xauthority, sockets and X state that mean nothing on another machine. Export exists to move a persona (config, memories, skills), and the redaction pass only touches text files, so the SQLite cookie store went through intact. bot-desktop joins the credential exclusion set the named-profile ignore already applies (auth.json, .env). The default profile's export is a root allow-list and never included it; the test pins both so the directory cannot be admitted later. --- hermes_cli/profiles.py | 5 +++-- tests/hermes_cli/test_profiles.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 2043f294a2..5f839710cf 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1492,8 +1492,9 @@ def _default_export_ignore(root_dir: Path): return _ignore -# Credential files dropped from named-profile exports. -_EXPORT_CREDENTIAL_FILES = frozenset({"auth.json", ".env"}) +# Credential files dropped from named-profile exports. ``bot-desktop`` is the screen's runtime state: +# its persistent Chromium profile (Cookies, Login Data — the bot's live web sessions), Xauthority, sockets. +_EXPORT_CREDENTIAL_FILES = frozenset({"auth.json", ".env", "bot-desktop"}) # Text/config suffixes secret-scrubbed on export; binary DBs, images etc. are left alone. _EXPORT_REDACT_SUFFIXES = frozenset({ diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 91d6c03434..3ac037a683 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -1195,3 +1195,21 @@ def test_profile_delete_and_rename_stop_the_profiles_bot_desktop(profile_env, op assert proc.wait(timeout=10) != 0, "the launcher was signalled by the profile op" finally: proc.kill() + + +@pytest.mark.parametrize("name", ["coder", "default"]) +def test_export_leaves_the_bot_desktop_browser_profile_out(profile_env, tmp_path, name): + """bot-desktop/ holds the screen's persistent Chromium profile (Cookies, Login Data: the bot's live web + sessions) plus sockets and X state. None of it belongs in an export archive meant to move a persona.""" + profile_dir = create_profile(name, no_alias=True) if name != "default" else get_profile_dir("default") + (profile_dir / "config.yaml").write_text("model: test") + cookies = profile_dir / "bot-desktop" / "browser-profile" / "Default" / "Cookies" + cookies.parent.mkdir(parents=True) + cookies.write_bytes(b"SQLite format 3\x00") + output = tmp_path / "export" / f"{name}.tar.gz" + output.parent.mkdir(parents=True, exist_ok=True) + export_profile(name, str(output)) + with tarfile.open(str(output), "r:gz") as tf: + names = tf.getnames() + assert f"{name}/config.yaml" in names + assert not [n for n in names if "bot-desktop" in n], names From a047156df74aae80ac765cc41bd58331e84d6887 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 05:59:54 -0700 Subject: [PATCH 5/5] =?UTF-8?q?fix(bot-screen):=20hygiene=20=E2=80=94=20pr?= =?UTF-8?q?ivate=20alloc=20lock,=20private=20lease=20files,=20cookie=20off?= =?UTF-8?q?=20argv,=20real=20Fedora=20packages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small exposures, none a same-UID boundary (which the design does not claim), all about other local users and wrong install hints: - The host-wide display-allocation lock sat at a predictable name in world-writable /tmp, where any local user could pre-create or squat it. It now lives under XDG_RUNTIME_DIR (else ~/.cache), created 0700; still host-wide, outside every profile home, because all profiles allocate from one band. - lease.py hand-rolled mkdir + plain writes, so a takeover recorded before start() ever ran left bot-desktop/ 0755 and lease.json / lease.lock 0644 under the umask — who holds the screen and the lock the RFB bridge serialises on, readable by everyone. It now uses secure_parent_dir and creates every file 0600 via an opener; the fcntl-less (Windows) fallback is untouched. - launcher.sh passed the X cookie on xauth's argv, visible in ps to other local UIDs. The cookie is now fed on stdin through `xauth source -`. - The Fedora map named tigervnc-server-minimal (only a Provides of tigervnc-x11-server, which actually ships Xvnc) and dbus-x11 for dbus-run-session (dbus-daemon owns it; dbus-x11 ships dbus-launch). Both BINARY_PACKAGES and PACKAGES are corrected together. --- tests/tools/test_bot_desktop_lease.py | 23 +++++++++++++++++++++++ tools/bot_desktop/launcher.sh | 5 ++++- tools/bot_desktop/lease.py | 23 ++++++++++++++++++----- tools/bot_desktop/runtime.py | 16 +++++++++++----- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/tests/tools/test_bot_desktop_lease.py b/tests/tools/test_bot_desktop_lease.py index 9bd4d70aa5..4a763ddb6f 100644 --- a/tests/tools/test_bot_desktop_lease.py +++ b/tests/tools/test_bot_desktop_lease.py @@ -178,3 +178,26 @@ def test_lease_works_without_fcntl(tmp_path): out = subprocess.run([sys.executable, "-c", probe], capture_output=True, text=True, encoding="utf-8", timeout=60, stdin=subprocess.DEVNULL, env={**os.environ, "HERMES_HOME": str(tmp_path)}) assert out.stdout.strip() == "OK", out.stderr + + +@pytest.mark.linux_only +def test_lease_files_are_private_even_when_the_lease_is_written_before_the_screen_exists(tmp_path, monkeypatch): + """A takeover can be recorded before start() ever created bot-desktop/ 0700. The lease path then created + the directory and files with the umask (0755 / 0644): who holds the screen, and the lock the RFB bridge + serialises on, readable and clobberable by every other local user. Every piece must be owner-only.""" + import os + import stat + + home = tmp_path / "deep" / "home" + home.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(lease, "get_hermes_home", lambda: home) + old = os.umask(0o022) + try: + lease.acquire("v1") + finally: + os.umask(old) + sd = home / "bot-desktop" + for path in (sd, sd / "lease.json", sd / "lease.lock"): + assert path.exists(), path + assert stat.S_IMODE(path.stat().st_mode) & 0o077 == 0, f"{path.name} is {oct(path.stat().st_mode)}" diff --git a/tools/bot_desktop/launcher.sh b/tools/bot_desktop/launcher.sh index 4efecdaf9c..bfd02b1066 100755 --- a/tools/bot_desktop/launcher.sh +++ b/tools/bot_desktop/launcher.sh @@ -49,7 +49,10 @@ if [[ -e "$xlock" ]] && ! kill -0 "$(tr -d ' ' < "$xlock" 2>/dev/null)" 2>/dev/n rm -f "$xlock" "/tmp/.X11-unix/X${HERMES_BD_DISPLAY_NUM}" fi : > "$XAUTHORITY"; chmod 600 "$XAUTHORITY" -xauth -q -f "$XAUTHORITY" add "$DISPLAY" MIT-MAGIC-COOKIE-1 "$(od -An -N16 -tx1 /dev/urandom | tr -d ' \n')" +# The cookie goes in on stdin, not argv: a command line is readable by every local user via ps. +xauth -q -f "$XAUTHORITY" source - < Lease: return Lease(holder=HUMAN, viewer_id="unreadable-lease", reason="lease file corrupt") -def _write(path: Path, lease: Lease) -> None: +def _private_dir(path: Path) -> None: + """``bot-desktop/`` owner-only even when the lease is the first thing written there (a takeover can be + recorded before start() ever ran, and the umask would otherwise leave it 0755).""" path.parent.mkdir(parents=True, exist_ok=True) + secure_parent_dir(path) + + +def _open_private(path: str | bytes | os.PathLike, flags: int) -> int: + """``open(..., opener=_open_private)``: the file is created 0600 regardless of the umask.""" + return os.open(path, flags, 0o600) + + +def _write(path: Path, lease: Lease) -> None: + _private_dir(path) tmp = path.with_suffix(".json.tmp") - tmp.write_text(json.dumps(lease.as_dict()), encoding="utf-8") + with open(tmp, "w", encoding="utf-8", opener=_open_private) as fh: + fh.write(json.dumps(lease.as_dict())) os.replace(tmp, path) @@ -104,8 +117,8 @@ class _locked: def __enter__(self): if fcntl is None: return self - self._lockfile.parent.mkdir(parents=True, exist_ok=True) - self._fh = open(self._lockfile, "a+", encoding="utf-8") # noqa: SIM115 — closed in __exit__ + _private_dir(self._lockfile) + self._fh = open(self._lockfile, "a+", encoding="utf-8", opener=_open_private) # noqa: SIM115 — closed in __exit__ fcntl.flock(self._fh.fileno(), fcntl.LOCK_EX) return self diff --git a/tools/bot_desktop/runtime.py b/tools/bot_desktop/runtime.py index f0b7e293ed..8f003f8069 100644 --- a/tools/bot_desktop/runtime.py +++ b/tools/bot_desktop/runtime.py @@ -48,8 +48,10 @@ BINARY_PACKAGES = { "apt": {"Xvnc": "tigervnc-standalone-server", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel", "xfdesktop": "xfdesktop4", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-x11", "xauth": "xauth", "xdpyinfo": "x11-utils", "setxkbmap": "x11-xkb-utils", "xprop": "x11-utils"}, - "dnf": {"Xvnc": "tigervnc-server-minimal", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel", - "xfdesktop": "xfdesktop", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-x11", + # tigervnc-x11-server is the real package (tigervnc-server-minimal is only a Provides on it); dbus-run-session + # is in dbus-daemon (dbus-x11 ships dbus-launch only). + "dnf": {"Xvnc": "tigervnc-x11-server", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel", + "xfdesktop": "xfdesktop", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus-daemon", "xauth": "xorg-x11-xauth", "xdpyinfo": "xdpyinfo", "setxkbmap": "setxkbmap", "xprop": "xprop"}, "pacman": {"Xvnc": "tigervnc", "xfwm4": "xfwm4", "xfce4-panel": "xfce4-panel", "xfdesktop": "xfdesktop", "xfsettingsd": "xfce4-settings", "dbus-run-session": "dbus", @@ -60,8 +62,8 @@ PACKAGES = { "apt": ["tigervnc-standalone-server", "xfce4-panel", "xfwm4", "xfdesktop4", "xfce4-settings", "xfce4-terminal", "dbus-x11", "x11-xserver-utils", "x11-utils", "x11-xkb-utils", "xauth", "fonts-dejavu-core"], - "dnf": ["tigervnc-server-minimal", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings", - "xfce4-terminal", "dbus-x11", "xsetroot", "xset", "xdpyinfo", "xprop", "xorg-x11-xauth", "setxkbmap", + "dnf": ["tigervnc-x11-server", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings", + "xfce4-terminal", "dbus-daemon", "xsetroot", "xset", "xdpyinfo", "xprop", "xorg-x11-xauth", "setxkbmap", "dejavu-sans-fonts"], "pacman": ["tigervnc", "xfce4-panel", "xfwm4", "xfdesktop", "xfce4-settings", "xfce4-terminal", "dbus", "xorg-xsetroot", "xorg-xset", "xorg-xdpyinfo", "xorg-xprop", "xorg-xauth", "xorg-setxkbmap", @@ -229,7 +231,9 @@ def _kill_group_then_wait(pgid: Optional[int], pid: int, grace: float = 2.0) -> _signal(signal.SIGKILL) # windows-footgun: ok — Linux-only runtime (is_supported_host gates start/stop) -_ALLOC_LOCK = Path("/tmp/.hermes-bot-desktop-alloc.lock") # host-wide: profiles allocate from one band +# Host-wide (every profile allocates from one band), so it lives outside any profile home — but not in +# world-writable /tmp, where a predictable name lets another local user pre-create or squat the file. +_ALLOC_LOCK = Path(os.environ.get("XDG_RUNTIME_DIR") or Path.home() / ".cache") / "hermes-bot-desktop-alloc.lock" @contextlib.contextmanager @@ -256,6 +260,7 @@ def _pick_display() -> int: def _allocate_display() -> int: + _ALLOC_LOCK.parent.mkdir(parents=True, exist_ok=True, mode=0o700) with _flocked(_ALLOC_LOCK): return _pick_display() @@ -371,6 +376,7 @@ def start(*, wait_seconds: float = 15.0) -> DesktopStatus: return status() if _launcher_pid() is None: _reap_orphaned_server(sd) + _ALLOC_LOCK.parent.mkdir(parents=True, exist_ok=True, mode=0o700) with _flocked(_ALLOC_LOCK): return _spawn_and_wait(sd, _pick_display(), wait_seconds)