From 7fd5f59ba228eaea751a287a40cecaed009ca112 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 06:04:43 -0700 Subject: [PATCH] fix(bot-screen): dock Browser entry quotes its Exec= line, so spaced paths work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit launcher.sh received the dock browser as one shell line and recovered the executable with `${3%% *}`: a Chromium under '/opt/Google Chrome/' or a profile dir under a HERMES_HOME with a space split at the first blank, the existence check failed or Exec= became garbage, and the dock had no working Browser icon. Python now hands the launcher the bare executable (HERMES_BD_BROWSER_EXEC, for the `command -v` check) and a ready-made Exec= line (HERMES_BD_BROWSER_EXEC_LINE) built by `browser.dock_exec_line`: each argument double-quoted, reserved characters backslash-escaped inside the quotes and the backslashes string-escaped once more, per the Desktop Entry spec. `dock_argv` is the single source of the dock's arguments (incl. the root sandbox flags). Test: tests/tools/test_bot_desktop_browser.py — a spaced executable and a spaced, quote-bearing profile dir produce a correctly quoted Exec= line (AttributeError on the previous commit); the launcher seed test keeps running the real script (`bash -n` clean). --- tests/tools/test_bot_desktop_browser.py | 19 +++++++++++++++--- tests/tools/test_bot_desktop_launcher_seed.py | 4 ++-- tools/bot_desktop/browser.py | 20 ++++++++++++++----- tools/bot_desktop/launcher.sh | 14 +++++++------ tools/bot_desktop/runtime.py | 6 ++++-- tools/browser_tool_session.py | 2 +- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/tests/tools/test_bot_desktop_browser.py b/tests/tools/test_bot_desktop_browser.py index be1bc8adf3..5fdde7de36 100644 --- a/tests/tools/test_bot_desktop_browser.py +++ b/tests/tools/test_bot_desktop_browser.py @@ -33,7 +33,7 @@ def test_user_pinned_profile_wins(tmp_path, monkeypatch): def test_dock_browser_advertises_a_devtools_port(): """A human-started instance must be attachable, or the agent can never drive it afterwards.""" - assert "--remote-debugging-port=" in browser.dock_command("/opt/chrome", "/p/dir").split()[2] + assert "--remote-debugging-port=" in browser.dock_argv("/opt/chrome", "/p/dir")[2] def _fake_running_instance(user_data_dir, pid: int, port: int) -> None: @@ -129,7 +129,7 @@ def test_unprivileged_user_under_apparmor_userns_restriction_gets_the_system_bro monkeypatch.setattr(browser, "_userns_restricted", lambda: True) exe = browser.executable() assert exe and exe.endswith("chrome-linux/chrome") - assert "--no-sandbox" not in browser.dock_command(exe, "/p/dir") + assert "--no-sandbox" not in browser.dock_argv(exe, "/p/dir") def test_root_dock_browser_starts_with_the_same_sandbox_args_as_the_agents_browser(monkeypatch): @@ -144,7 +144,7 @@ def test_root_dock_browser_starts_with_the_same_sandbox_args_as_the_agents_brows session._apply_chromium_sandbox_args(agent_env) agent_flags = set(agent_env["AGENT_BROWSER_ARGS"].split(",")) assert agent_flags, "root must inject sandbox flags for agent-browser" - assert agent_flags <= set(browser.dock_command("/opt/chrome", "/p/dir").split()) + assert agent_flags <= set(browser.dock_argv("/opt/chrome", "/p/dir")) def test_status_reports_the_headed_browser_or_its_absence(monkeypatch): @@ -157,3 +157,16 @@ def test_status_reports_the_headed_browser_or_its_absence(monkeypatch): assert runtime.status().as_dict()["browser"] is None monkeypatch.setattr(browser, "executable", lambda: "/usr/bin/chromium") assert runtime.status().browser == "/usr/bin/chromium" + + +def test_dock_exec_line_survives_spaces_in_the_executable_and_profile_paths(): + """The launcher used to split the shell line on the first space to find the executable, so a + Chromium under '/opt/Google Chrome/' or a profile under a spaced HERMES_HOME broke the dock icon. + Exec= follows the Desktop Entry spec: each argument double-quoted, with the reserved characters + backslash-escaped inside the quotes.""" + exe = "/opt/Google Chrome/chrome" + profile = '/home/a b/.hermes/browser "x"/profile' + line = browser.dock_exec_line(exe, profile) + assert line.startswith('Exec="/opt/Google Chrome/chrome" ') + assert r'"--user-data-dir=/home/a b/.hermes/browser \\"x\\"/profile"' in line # spec: \" quoted, then \ string-escaped + assert "--remote-debugging-port=0" in line diff --git a/tests/tools/test_bot_desktop_launcher_seed.py b/tests/tools/test_bot_desktop_launcher_seed.py index 261d9d0055..c75296ce30 100644 --- a/tests/tools/test_bot_desktop_launcher_seed.py +++ b/tests/tools/test_bot_desktop_launcher_seed.py @@ -35,7 +35,7 @@ def _seed(tmp_path: Path, fake_bins: list[str], browser_exec: str = "") -> Path: "HERMES_BD_SOCKET": str(tmp_path / "rfb.sock"), "HERMES_BD_XAUTH": str(tmp_path / "Xauthority"), "HERMES_BD_ENV_FILE": str(tmp_path / "env"), "HERMES_BD_CONFIG_HOME": str(cfg), "HERMES_BD_SEED_ONLY": "1", - **({"HERMES_BD_BROWSER_EXEC": browser_exec} if browser_exec else {}), + **({"HERMES_BD_BROWSER_EXEC": browser_exec, "HERMES_BD_BROWSER_EXEC_LINE": f"Exec={browser_exec} --user-data-dir={tmp_path}/bp"} if browser_exec else {}), } subprocess.run(["bash", str(LAUNCHER)], env=env, check=True, stdin=subprocess.DEVNULL, capture_output=True, timeout=30) return cfg @@ -43,7 +43,7 @@ def _seed(tmp_path: Path, fake_bins: list[str], browser_exec: str = "") -> Path: def test_dock_lists_only_programs_present_on_path(tmp_path): chrome = tmp_path / "bin" / "chrome" # the browser is the one runtime.py resolved, never a PATH scan - cfg = _seed(tmp_path, ["xfce4-terminal", "chrome", "firefox"], browser_exec=f"{chrome} --user-data-dir={tmp_path}/bp") + cfg = _seed(tmp_path, ["xfce4-terminal", "chrome", "firefox"], browser_exec=str(chrome)) panel = ET.parse(cfg / "xfce4/xfconf/xfce-perchannel-xml/xfce4-panel.xml") # well-formed or this raises launcher_ids = [str(p.get("name")) for p in panel.iter("property") if p.get("value") == "launcher"] execs = sorted( diff --git a/tools/bot_desktop/browser.py b/tools/bot_desktop/browser.py index c20b90a0e7..f92ea87fa9 100644 --- a/tools/bot_desktop/browser.py +++ b/tools/bot_desktop/browser.py @@ -76,17 +76,27 @@ def dock_launch() -> Optional[Tuple[str, str]]: return (exe, str(profile_dir())) if exe else None -def dock_command(exe: str, user_data_dir: str) -> str: - """Shell line the dock's Browser icon runs. ``--remote-debugging-port=0`` makes a human-started +def dock_argv(exe: str, user_data_dir: str) -> list[str]: + """Command the dock's Browser icon runs. ``--remote-debugging-port=0`` makes a human-started instance attachable (Chromium writes the chosen port to ``/DevToolsActivePort``); first-run / default-browser dialogs would sit between the human and the bot's tabs.""" # --test-type hides the "Chrome for Testing is only for automated testing" and unsupported-flag # (--no-sandbox as root) infobars, which otherwise sit at the top of the human's takeover view. # Root gets the same sandbox-bypass flags agent-browser starts this binary with (one policy). from tools.browser_tool_session import CHROMIUM_SANDBOX_BYPASS_ARGS - root_args = " ".join(("", *CHROMIUM_SANDBOX_BYPASS_ARGS)) if _is_root() else "" - return (f"{exe} --user-data-dir={user_data_dir} --remote-debugging-port=0 --no-first-run " - f"--no-default-browser-check --test-type{root_args}") + return [exe, f"--user-data-dir={user_data_dir}", "--remote-debugging-port=0", "--no-first-run", + "--no-default-browser-check", "--test-type", *(CHROMIUM_SANDBOX_BYPASS_ARGS if _is_root() else ())] + + +def dock_exec_line(exe: str, user_data_dir: str) -> str: + """The ``Exec=`` line of the dock's ``.desktop`` entry. Every argument is double-quoted per the + Desktop Entry spec (a browser under ``/opt/Google Chrome/`` or a profile under a spaced HERMES_HOME + otherwise splits into garbage): inside the quotes ``" ` $ \\`` are backslash-escaped, and because the + value is itself a string field, each of those backslashes is escaped once more.""" + def quote(arg: str) -> str: + quoted = "".join("\\" + ch if ch in '"`$\\' else ch for ch in arg) + return '"' + quoted.replace("\\", "\\\\") + '"' + return "Exec=" + " ".join(quote(arg) for arg in dock_argv(exe, user_data_dir)) def running_instance_cdp_port(user_data_dir: str, *, exclude_session: Optional[str] = None) -> Optional[int]: diff --git a/tools/bot_desktop/launcher.sh b/tools/bot_desktop/launcher.sh index 4efecdaf9c..31133c7fdd 100755 --- a/tools/bot_desktop/launcher.sh +++ b/tools/bot_desktop/launcher.sh @@ -131,18 +131,20 @@ sed -i "s|HERMES_BD_WALLPAPER_PLACEHOLDER|$HERMES_BD_WALLPAPER|" "$X/xfce4-deskt if [[ ! -e "$X/xfce4-panel.xml" ]]; then L="$XDG_CONFIG_HOME/xfce4/panel"; mkdir -p "$L" dock_ids=(); n=20 - add_launcher() { # name icon exec — skipped when the executable is missing - local exe; exe=${3%% *} - command -v "$exe" >/dev/null 2>&1 || return 0 + add_launcher() { # name icon executable [Exec= line] — skipped when the executable is missing. + # The Exec= line is passed ready-made (spec-quoted by Python) so paths with spaces survive; a bare + # program name is its own Exec= value. + command -v "$3" >/dev/null 2>&1 || return 0 n=$((n+1)); mkdir -p "$L/launcher-$n" - printf '[Desktop Entry]\nVersion=1.0\nType=Application\nName=%s\nIcon=%s\nExec=%s\nTerminal=false\nStartupNotify=false\n' \ - "$1" "$2" "$3" > "$L/launcher-$n/hermes.desktop" + printf '[Desktop Entry]\nVersion=1.0\nType=Application\nName=%s\nIcon=%s\n%s\nTerminal=false\nStartupNotify=false\n' \ + "$1" "$2" "${4:-Exec=$3}" > "$L/launcher-$n/hermes.desktop" dock_ids+=("$n") } add_launcher "Terminal" utilities-terminal "xfce4-terminal" # The bot's browser: runtime.py resolves the executable agent-browser drives plus the profile's # persistent user-data-dir, so a human taking over lands in the bot's own cookie jar. - [[ -n "${HERMES_BD_BROWSER_EXEC:-}" ]] && add_launcher "Browser" internet-web-browser "$HERMES_BD_BROWSER_EXEC" + [[ -n "${HERMES_BD_BROWSER_EXEC:-}" ]] && \ + add_launcher "Browser" internet-web-browser "$HERMES_BD_BROWSER_EXEC" "${HERMES_BD_BROWSER_EXEC_LINE:-}" add_launcher "Files" system-file-manager "thunar" add_launcher "Text Editor" accessories-text-editor "mousepad" dock_plugins=""; dock_items="" diff --git a/tools/bot_desktop/runtime.py b/tools/bot_desktop/runtime.py index ea78591ba0..b4167fa305 100644 --- a/tools/bot_desktop/runtime.py +++ b/tools/bot_desktop/runtime.py @@ -338,9 +338,11 @@ def _spawn_and_wait(sd: Path, num: int, wait_seconds: float) -> DesktopStatus: "HERMES_BD_CONFIG_HOME": str(sd / "xdg"), "HERMES_BD_GEOMETRY": geometry(), }) - from tools.bot_desktop.browser import dock_command, dock_launch + from tools.bot_desktop.browser import dock_exec_line, dock_launch if (browser := dock_launch()) is not None: - child_env["HERMES_BD_BROWSER_EXEC"] = dock_command(*browser) + # The bare executable (the launcher checks it exists) and the ready-made, spec-quoted Exec= line. + child_env["HERMES_BD_BROWSER_EXEC"] = browser[0] + child_env["HERMES_BD_BROWSER_EXEC_LINE"] = dock_exec_line(*browser) # Truncated per start: the log is a diagnostic for THIS launch, and nothing rotates it otherwise. log = open(sd / "launcher.log", "wb") # noqa: SIM115 — handed to the child, closed by it proc = subprocess.Popen( # windows-footgun: ok — Linux-only runtime (is_supported_host) diff --git a/tools/browser_tool_session.py b/tools/browser_tool_session.py index 9532fe81dd..1366a415ee 100644 --- a/tools/browser_tool_session.py +++ b/tools/browser_tool_session.py @@ -32,7 +32,7 @@ _CHROMIUM_MISSING_HINT = f"Chromium browser is missing. Install it with: {_CHROM # THE Chromium startup flags for a host where its sandbox cannot work; agent-browser gets them through # AGENT_BROWSER_ARGS and the Bot Desktop dock's Browser icon (same binary, same profile) through -# ``tools.bot_desktop.browser.dock_command`` — one list, or the human's click dies while the agent's works. +# ``tools.bot_desktop.browser.dock_argv`` — one list, or the human's click dies while the agent's works. CHROMIUM_SANDBOX_BYPASS_ARGS = ("--no-sandbox", "--disable-dev-shm-usage")