fix(bot-relay): Windows path SyntaxError in waiter + PATH-less delivery ENOENT
Two failures on a Windows desktop install relaying to a remote gateway (#93590): 1. waiter_command embeds the reply path in generated python -c source with !r. repr escapes each backslash, but the Windows execution layer folds \\ back to \, so \U in C:\Users\... parses as a unicode escape and SyntaxErrors the whole waiter script. Raw-string literals keep the folded single backslash a literal; POSIX paths have no backslashes so the prefix is a no-op there, and \' inside a raw literal still cannot terminate the string, keeping the #93091 injection defense intact. 2. local_delivery_command hardcoded "hermes", relying on PATH — absent in service contexts (systemd units, desktop launchers, non-login SSH shells), so delivery died with ENOENT. It now resolves the CLI next to this gateway's own interpreter (venv bin/Scripts sibling, hermes.exe on Windows) with a bare-name fallback. The #93091 per-profile turn-lock recognition in bot_mode_dm now matches the CLI element by basename (split on both separators) so resolved absolute paths still take the lock instead of silently bypassing it. Fixes #93590
This commit is contained in:
@@ -0,0 +1,147 @@
|
||||
"""Windows-path viability and venv CLI resolution for bot relay (#93590).
|
||||
|
||||
Two failures on a Windows desktop install talking to a remote gateway:
|
||||
|
||||
1. ``waiter_command`` embeds the reply path into generated ``python -c``
|
||||
source with ``!r``. repr escapes each backslash, but the Windows
|
||||
execution layer the waiter runs under folds ``\\`` back to ``\`` —
|
||||
``\\U`` in ``C:\\Users\\...`` then parses as a unicode escape and
|
||||
SyntaxErrors the whole script. The raw-string prefix keeps the folded
|
||||
single backslash a literal; POSIX paths contain no backslashes, so it
|
||||
is a no-op there, and ``\\'`` inside a raw literal still cannot
|
||||
terminate the string, so the injection defense from #93091's
|
||||
python -c hardening is unchanged.
|
||||
|
||||
2. ``local_delivery_command`` hardcoded ``"hermes"``, relying on PATH —
|
||||
which service contexts (systemd units, desktop launchers, non-login
|
||||
SSH shells) do not provide, so delivery died with ENOENT. It now
|
||||
resolves the CLI next to this gateway's own interpreter (the venv
|
||||
bin/Scripts sibling), falling back to the bare name. The #93091
|
||||
turn-lock recognition in bot_mode_dm matches the CLI element by
|
||||
basename so resolved absolute paths (and ``hermes.exe``) still take
|
||||
the per-profile lock.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import shlex
|
||||
from pathlib import Path
|
||||
|
||||
import tools.bot_mode_dm as bot_mode_dm
|
||||
import tools.bot_relay as bot_relay
|
||||
|
||||
|
||||
ENV = {"id": "d" * 32, "target_handle": "researcher", "target_connection": "ssh-vps"}
|
||||
|
||||
|
||||
def _waiter_code(root, env=None) -> str:
|
||||
cmd = bot_relay.waiter_command(root, env or ENV)
|
||||
parts = shlex.split(cmd)
|
||||
return parts[parts.index("-c") + 1]
|
||||
|
||||
|
||||
def test_waiter_windows_path_compiles_after_backslash_folding():
|
||||
"""A Windows reply path must survive the execution layer folding the
|
||||
repr-escaped double backslash back to a single one — the exact shape
|
||||
that SyntaxErrored with ``\\U`` on #93590's reporter setup."""
|
||||
code = _waiter_code("C:\\Users\\joshu\\.hermes")
|
||||
assert "C:" in code # sanity: the Windows path made it into the payload
|
||||
folded = code.replace("\\\\", "\\")
|
||||
# Raw literals: `p = r'C:\Users\joshu\...'` — no unicode-escape crash.
|
||||
compile(folded, "<waiter>", "exec")
|
||||
|
||||
|
||||
def test_waiter_posix_path_and_label_values_roundtrip():
|
||||
"""On POSIX (backslash-free paths) the raw prefix changes nothing."""
|
||||
root = Path("/tmp/hermes-home")
|
||||
code = _waiter_code(root)
|
||||
assigns = {
|
||||
t.targets[0].id: t.value
|
||||
for t in ast.parse(code).body
|
||||
if isinstance(t, ast.Assign) and isinstance(t.targets[0], ast.Name)
|
||||
}
|
||||
expected = str(root / "bot_relay" / "replies" / f"{ENV['id']}.json")
|
||||
assert assigns["p"].value == expected
|
||||
assert assigns["label"].value == "@researcher on ssh-vps"
|
||||
# The literals are raw-prefixed in the generated source.
|
||||
assert "\np = r'" in code
|
||||
assert "\nlabel = r'" in code
|
||||
|
||||
|
||||
def test_waiter_raw_prefix_keeps_injection_defense():
|
||||
"""Hostile roster fields must stay data under the raw prefix too."""
|
||||
inj = {
|
||||
"id": "e" * 32,
|
||||
"target_handle": "researcher",
|
||||
"target_connection": "x'); __import__('sys').exit(2); print('x",
|
||||
}
|
||||
code = _waiter_code(Path("/tmp/hermes-home"), inj)
|
||||
compile(code, "<waiter>", "exec")
|
||||
calls = [
|
||||
n.func.id
|
||||
for n in ast.walk(ast.parse(code))
|
||||
if isinstance(n, ast.Call) and isinstance(n.func, ast.Name)
|
||||
]
|
||||
# The generated waiter only calls str/print/compile-free builtins by
|
||||
# name; the payload's __import__ must remain a string literal, not a
|
||||
# live call — parse it back and confirm it stayed data.
|
||||
assert "__import__" not in calls
|
||||
assert "x'); __import__('sys').exit(2); print('x" in code
|
||||
|
||||
|
||||
def test_local_delivery_resolves_sibling_hermes(tmp_path, monkeypatch):
|
||||
bin_dir = tmp_path / "venv" / "bin"
|
||||
bin_dir.mkdir(parents=True)
|
||||
sibling = bin_dir / "hermes"
|
||||
sibling.touch()
|
||||
sibling.chmod(0o755)
|
||||
monkeypatch.setattr("sys.executable", str(bin_dir / "python"))
|
||||
|
||||
argv = bot_relay.local_delivery_command("ops", "query.json")
|
||||
assert argv[0] == str(sibling)
|
||||
assert argv[1:3] == ["-p", "ops"]
|
||||
assert argv[argv.index("--query-file") + 1] == "query.json"
|
||||
|
||||
|
||||
def test_local_delivery_falls_back_to_bare_name(tmp_path, monkeypatch):
|
||||
empty = tmp_path / "nowhere"
|
||||
empty.mkdir(parents=True)
|
||||
monkeypatch.setattr("sys.executable", str(empty / "python"))
|
||||
|
||||
argv = bot_relay.local_delivery_command("ops", "query.json")
|
||||
assert argv[0] == "hermes"
|
||||
assert argv[1:3] == ["-p", "ops"]
|
||||
|
||||
|
||||
def test_delivery_lock_recognizes_resolved_cli_paths(tmp_path, monkeypatch):
|
||||
"""The #93091 per-profile turn lock must keep matching delivery argvs
|
||||
now that argv[0] may be a resolved absolute path (or hermes.exe)."""
|
||||
acquired = []
|
||||
|
||||
class _Ctx:
|
||||
def __enter__(self):
|
||||
acquired.append("locked")
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
return False
|
||||
|
||||
monkeypatch.setattr(bot_relay, "acquire_turn_lock", lambda root, profile: _Ctx())
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
with bot_mode_dm._delivery_lock(
|
||||
[str(tmp_path / "venv" / "bin" / "hermes"), "-p", "ops", "chat"],
|
||||
stdin_file=False,
|
||||
):
|
||||
pass
|
||||
with bot_mode_dm._delivery_lock(["hermes", "-p", "ops", "chat"], stdin_file=False):
|
||||
pass
|
||||
with bot_mode_dm._delivery_lock(
|
||||
["C:\\venv\\Scripts\\hermes.exe", "-p", "ops", "chat"], stdin_file=False
|
||||
):
|
||||
pass
|
||||
assert acquired == ["locked", "locked", "locked"]
|
||||
|
||||
# Unrelated argvs still bypass the lock entirely.
|
||||
with bot_mode_dm._delivery_lock(["python", "-m", "whatever"], stdin_file=False):
|
||||
pass
|
||||
assert acquired == ["locked", "locked", "locked"]
|
||||
@@ -247,13 +247,17 @@ def test_peer_stdin_delivery_skips_local_lock(root, tmp_path, monkeypatch):
|
||||
|
||||
def test_local_delivery_command_never_reenters_the_lock():
|
||||
"""The gateway deliver handler runs local_delivery_command ALREADY holding
|
||||
the profile lock. That argv must stay a raw `hermes -p … chat` invocation:
|
||||
the profile lock. That argv must stay a raw hermes CLI invocation:
|
||||
routing it through the --run-delivery wrapper would make the child hit
|
||||
_delivery_lock (argv[0]=='hermes' and argv[1]=='-p'), burn the full wait
|
||||
_delivery_lock (hermes CLI + '-p'), burn the full wait
|
||||
budget against its parent's flock, and fail every relay delivery with
|
||||
target_busy."""
|
||||
target_busy. argv[0] may be a resolved venv path (#93590) — the lock
|
||||
matcher and this assertion both go by basename."""
|
||||
from pathlib import Path
|
||||
|
||||
argv = bot_relay.local_delivery_command("ops", "/tmp/q.txt")
|
||||
assert argv[:3] == ["hermes", "-p", "ops"]
|
||||
assert argv[1:3] == ["-p", "ops"]
|
||||
assert Path(argv[0]).name in ("hermes", "hermes.exe")
|
||||
assert "--run-delivery" not in argv
|
||||
assert not any("bot_mode_dm" in part for part in argv)
|
||||
|
||||
|
||||
+13
-1
@@ -527,7 +527,19 @@ def _delivery_lock(argv: list[str], *, stdin_file: bool):
|
||||
lock in ``tools.bot_relay``. Peer transports (stdin mode) run on the
|
||||
remote gateway; their turn is locked THERE by its own deliver path.
|
||||
"""
|
||||
if stdin_file or len(argv) < 3 or argv[0] != "hermes" or argv[1] != "-p":
|
||||
# The CLI element is matched by basename: local_delivery_command now
|
||||
# resolves the venv-relative hermes next to this gateway's interpreter
|
||||
# (#93590 — service contexts lack PATH), so argv[0] may be an absolute
|
||||
# path (and on Windows carries the .exe suffix). Split on both
|
||||
# separators so the shape matches regardless of which platform built
|
||||
# the argv.
|
||||
cli = (argv[0] if argv else "").rsplit("\\", 1)[-1].rsplit("/", 1)[-1]
|
||||
if (
|
||||
stdin_file
|
||||
or len(argv) < 3
|
||||
or cli not in ("hermes", "hermes.exe")
|
||||
or argv[1] != "-p"
|
||||
):
|
||||
return contextlib.nullcontext()
|
||||
from tools.bot_relay import acquire_turn_lock
|
||||
|
||||
|
||||
+27
-3
@@ -496,10 +496,18 @@ def waiter_command(root: Path | str, envelope: dict) -> str:
|
||||
)
|
||||
# Encode label with !r so roster fields cannot break out of the generated
|
||||
# python -c source (quotes, parens, or extra statements in connection_id).
|
||||
# The raw-string prefix keeps Windows paths viable: repr escapes each
|
||||
# backslash ("C:\\Users\\..."), but the Windows execution layer the
|
||||
# waiter runs under folds "\\" back to "\", which turns "\U" into an
|
||||
# invalid unicode escape and SyntaxErrors the whole script (#93590).
|
||||
# With the r prefix the folded single backslash parses as a literal.
|
||||
# POSIX paths contain no backslashes, so the prefix is a no-op there,
|
||||
# and \' inside a raw literal still cannot terminate the string, so
|
||||
# the injection defense above is unchanged.
|
||||
code = (
|
||||
"import json,os,sys,time\n"
|
||||
f"p = {reply_path!r}\n"
|
||||
f"label = {label!r}\n"
|
||||
f"p = r{reply_path!r}\n"
|
||||
f"label = r{label!r}\n"
|
||||
f"deadline = time.time() + {REPLY_WAIT_SECONDS}\n"
|
||||
"while time.time() < deadline:\n"
|
||||
" if os.path.exists(p):\n"
|
||||
@@ -526,10 +534,26 @@ def waiter_command(root: Path | str, envelope: dict) -> str:
|
||||
# ── delivery command (used by the deliver RPC on the TARGET gateway) ────────
|
||||
|
||||
|
||||
def _hermes_cli() -> str:
|
||||
"""Resolve the hermes CLI beside this gateway's own interpreter.
|
||||
|
||||
The deliver RPC runs on the target gateway, whose process is the venv
|
||||
python — its bin/Scripts directory holds the matching ``hermes``
|
||||
entrypoint. A bare ``"hermes"`` relies on PATH, which is exactly what
|
||||
service contexts (systemd units, desktop launchers, non-login SSH
|
||||
shells) do not provide, so delivery died with ENOENT there (#93590).
|
||||
Falls back to the bare name when no sibling exists (e.g. running from
|
||||
a source tree without an installed script), preserving PATH lookup.
|
||||
"""
|
||||
exe = Path(sys.executable or "")
|
||||
sibling = exe.parent / ("hermes.exe" if os.name == "nt" else "hermes")
|
||||
return str(sibling) if sibling.is_file() else "hermes"
|
||||
|
||||
|
||||
def local_delivery_command(profile: str, query_file: str) -> list[str]:
|
||||
"""argv that delivers a DM into ``profile``'s Bot Chat on THIS gateway."""
|
||||
return [
|
||||
"hermes",
|
||||
_hermes_cli(),
|
||||
"-p",
|
||||
profile,
|
||||
"chat",
|
||||
|
||||
Reference in New Issue
Block a user