fix(bot-relay): sweep stale relay artifacts + never leak the deliver tempfile
Widen the DM tempfile-leak fix (#91902/#92407) to the sibling sites PR #92784 introduced: - tools/bot_relay.py: expose the 6h stale sweep as cleanup_bot_relay_artifacts() (cleanup_*_cache contract) and wire it into gateway housekeeping — previously it ran only when the Desktop drained the outbox, so plaintext envelopes/replies queued while the Desktop was away could sit on disk forever. - tui_gateway/methods_bot_relay.py: move the payload write inside the try/finally so a failed write no longer leaks hermes-relay-dm-*.txt. - tools/bot_mode_dm.py: _spawn_delivery takes dm_file=None for relay waiter deliveries, which have no plaintext DM tempfile to reclaim.
This commit is contained in:
@@ -30197,6 +30197,7 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop
|
||||
)
|
||||
from tools.tool_result_storage import cleanup_spillover_cache
|
||||
from tools.bot_mode_dm import cleanup_bot_dm_cache
|
||||
from tools.bot_relay import cleanup_bot_relay_artifacts
|
||||
from hermes_cli.debug import _sweep_expired_pastes
|
||||
|
||||
IMAGE_CACHE_EVERY = 60 # ticks — once per hour at default 60s interval
|
||||
@@ -30217,6 +30218,7 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop
|
||||
("Screenshot", cleanup_screenshot_cache),
|
||||
("Spillover", cleanup_spillover_cache),
|
||||
("Bot DM", cleanup_bot_dm_cache),
|
||||
("Bot relay", cleanup_bot_relay_artifacts),
|
||||
)
|
||||
|
||||
logger.info("Gateway housekeeping started (interval=%ds)", interval)
|
||||
|
||||
@@ -288,3 +288,40 @@ def test_capability_fingerprint_changes_with_relay_roster(tmp_path):
|
||||
])
|
||||
after = bot_mode_probe.capability_fingerprint(home)
|
||||
assert before != after # eternal Bot Chats refresh once on roster change
|
||||
|
||||
|
||||
# ── stale artifact sweep (housekeeping contract) ─────────────────────────────
|
||||
|
||||
|
||||
def test_cleanup_bot_relay_artifacts_sweeps_stale_plaintext(tmp_path, monkeypatch):
|
||||
import os as _os
|
||||
import time as _time
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
target = {"profile": "scout", "handle": "scout", "connection_id": "cloud-1",
|
||||
"connection_label": "", "title": "", "description": ""}
|
||||
stale_env = bot_relay.enqueue_envelope(
|
||||
tmp_path, target=target, message="old secret",
|
||||
sender_profile="default", sender_handle="hermes",
|
||||
)
|
||||
fresh_env = bot_relay.enqueue_envelope(
|
||||
tmp_path, target=target, message="new secret",
|
||||
sender_profile="default", sender_handle="hermes",
|
||||
)
|
||||
base = bot_relay.relay_root(tmp_path)
|
||||
stale_reply = bot_relay.write_reply(tmp_path, stale_env["id"], reply="done")
|
||||
old = _time.time() - bot_relay.STALE_AFTER_SECONDS - 1
|
||||
_os.utime(base / bot_relay.OUTBOX_DIR / f"{stale_env['id']}.json", (old, old))
|
||||
_os.utime(stale_reply, (old, old))
|
||||
|
||||
removed = bot_relay.cleanup_bot_relay_artifacts()
|
||||
|
||||
assert removed == 2
|
||||
assert not (base / bot_relay.OUTBOX_DIR / f"{stale_env['id']}.json").exists()
|
||||
assert not stale_reply.exists()
|
||||
assert (base / bot_relay.OUTBOX_DIR / f"{fresh_env['id']}.json").exists()
|
||||
|
||||
|
||||
def test_cleanup_bot_relay_artifacts_missing_dir_is_zero(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "nope"))
|
||||
assert bot_relay.cleanup_bot_relay_artifacts() == 0
|
||||
|
||||
@@ -105,3 +105,36 @@ def test_reply_roundtrip_and_id_validation(home):
|
||||
|
||||
err = srv._methods["bot_relay.reply"](2, {"id": "../evil"})
|
||||
assert "error" in err
|
||||
|
||||
|
||||
def test_deliver_write_failure_still_removes_tempfile(home, monkeypatch, tmp_path):
|
||||
"""A failed payload write must not leak the relay DM tempfile."""
|
||||
import glob
|
||||
import os
|
||||
import tempfile as _tempfile
|
||||
|
||||
made = []
|
||||
real_mkstemp = _tempfile.mkstemp
|
||||
|
||||
def _tracking_mkstemp(*args, **kwargs):
|
||||
kwargs["dir"] = str(tmp_path)
|
||||
fd, path = real_mkstemp(*args, **kwargs)
|
||||
made.append(path)
|
||||
return fd, path
|
||||
|
||||
class _BrokenWriter:
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc_info):
|
||||
return False
|
||||
|
||||
def write(self, content):
|
||||
raise OSError("disk full")
|
||||
|
||||
monkeypatch.setattr("tempfile.mkstemp", _tracking_mkstemp)
|
||||
monkeypatch.setattr("os.fdopen", lambda *a, **k: _BrokenWriter())
|
||||
err = srv._methods["bot_relay.deliver"](1, {"profile": "ops", "message": "x"})
|
||||
assert "error" in err
|
||||
assert made, "mkstemp was never reached"
|
||||
assert not glob.glob(str(tmp_path / "hermes-relay-dm-*")), "tempfile leaked"
|
||||
|
||||
@@ -270,18 +270,44 @@ def write_reply(
|
||||
return path
|
||||
|
||||
|
||||
def _sweep_stale(base: Path) -> None:
|
||||
cutoff = time.time() - STALE_AFTER_SECONDS
|
||||
def _sweep_stale(base: Path, *, now: float | None = None) -> int:
|
||||
cutoff = (time.time() if now is None else now) - STALE_AFTER_SECONDS
|
||||
removed = 0
|
||||
for sub in (CLAIMED_DIR, REPLIES_DIR, OUTBOX_DIR):
|
||||
try:
|
||||
for path in (base / sub).glob("*.json"):
|
||||
try:
|
||||
if path.stat().st_mtime < cutoff:
|
||||
path.unlink()
|
||||
removed += 1
|
||||
except OSError:
|
||||
continue
|
||||
except OSError:
|
||||
continue
|
||||
return removed
|
||||
|
||||
|
||||
def cleanup_bot_relay_artifacts(max_age_hours: float | None = None) -> int:
|
||||
"""Sweep stale relay artifacts (envelopes/replies hold DM plaintext).
|
||||
|
||||
``_sweep_stale`` otherwise runs only when the Desktop drains the outbox
|
||||
(``claim_pending_envelopes``) — if the Desktop never reconnects, queued
|
||||
plaintext envelopes would sit on disk forever. Same contract as the
|
||||
``cleanup_*_cache`` helpers so the gateway housekeeping loop can call it
|
||||
hourly. ``max_age_hours`` is accepted for signature compatibility but the
|
||||
relay's own ``STALE_AFTER_SECONDS`` governs staleness.
|
||||
"""
|
||||
del max_age_hours # relay staleness is governed by STALE_AFTER_SECONDS
|
||||
try:
|
||||
home = Path(os.getenv("HERMES_HOME") or os.path.expanduser("~/.hermes"))
|
||||
root = home.parent.parent if home.parent.name == "profiles" else home
|
||||
base = relay_root(root)
|
||||
if not base.is_dir():
|
||||
return 0
|
||||
return _sweep_stale(base)
|
||||
except Exception:
|
||||
logger.debug("bot_relay artifact sweep failed", exc_info=True)
|
||||
return 0
|
||||
|
||||
|
||||
# ── waiter (runs on the sender gateway via terminal background process) ─────
|
||||
|
||||
@@ -110,9 +110,9 @@ def _(rid, params: dict) -> dict:
|
||||
return _err(rid, 4092, f"no profile '{profile}' on this gateway")
|
||||
|
||||
fd, tmp = tempfile.mkstemp(prefix="hermes-relay-dm-", suffix=".txt", text=True)
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as f:
|
||||
f.write(message)
|
||||
try:
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as f:
|
||||
f.write(message)
|
||||
proc = subprocess.run(
|
||||
local_delivery_command(resolved, tmp),
|
||||
capture_output=True,
|
||||
|
||||
Reference in New Issue
Block a user