hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 33 Packages Projects Releases Wiki Activity

fix(checkpoints): refuse host rollback and session diff from container sessions on the gateway too

Browse Source 
Widen the container-backend refusal salvaged from #113530 to the sibling
surfaces that render the same host checkpoints: the messaging gateway's
/rollback (restore refused, bare listing prefixed with the reason) and
/diff session, and the CLI's /diff session. The gateway arm follows the
CLI's "default" classification, i.e. the configured terminal backend.

Drop the thin _checkpoint_container_backend wrapper in favour of the
container_backend_for_task predicate it wrapped, trim the salvaged suite
to two invariant tests (one per class: no host store touched by a
container task; every surface refuses a host restore/diff from a
container session, with a local control), and update the docs.

Co-authored-by: fangliquan <fangliquan@qq.com>
This commit is contained in:
teknium1
2026-09-18 01:06:49 -07:00
committed by Teknium
parent 3220b9ed2f
commit dac4c60fbb
7 changed files with 89 additions and 201 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
agent/tool_executor.py
View File

@@ -74,23 +74,17 @@ def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) -
logger.debug("persisted-path record for result stub failed: %s", exc)
def _checkpoint_container_backend(effective_task_id: str) -> Optional[str]:
"""Return the task's backend name when its file paths belong to a container."""
from tools.file_tools_paths import container_backend_for_task
return container_backend_for_task(effective_task_id or "default")
def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effective_task_id: str) -> None:
"""Checkpoint the same workspace path that the file tool will mutate, resolved the way
file tools do (against the task's live cwd, which differs from the process cwd in Docker)."""
file_path = function_args.get("path", "")
if not file_path:
return
if _checkpoint_container_backend(effective_task_id) is not None:
return # container paths: nothing to checkpoint on the host
from agent.file_safety import is_nt_namespace_path
from tools.file_tools_paths import _resolve_path_for_task
from tools.file_tools_paths import _resolve_path_for_task, container_backend_for_task
if container_backend_for_task(effective_task_id or "default") is not None:
return # container paths: nothing to checkpoint on the host
# Resolving an NT-namespace path is itself the NTLM-leak trigger; leave the
# tool's raw-string guard to refuse it without a checkpoint stat.
@@ -989,7 +983,8 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) -
elif function_name == "terminal":
command = function_args.get("command", "")
if _is_destructive_command(command):
if _checkpoint_container_backend(effective_task_id) is None:
from tools.file_tools_paths import container_backend_for_task
if container_backend_for_task(effective_task_id or "default") is None:
from agent.runtime_cwd import scope_terminal_cwd
cwd = function_args.get("workdir") or scope_terminal_cwd() or os.getcwd()
agent._checkpoint_mgr.ensure_checkpoint(cwd, f"before terminal: {command[:60]}")

4
agent/turn_explainers.py
View File

@@ -255,8 +255,8 @@ class TurnExplainersMixin:
if mgr is not None and getattr(mgr, "enabled", False):
backend = None
with suppress(Exception):
from agent.tool_executor import _checkpoint_container_backend
backend = _checkpoint_container_backend(task_id or "default")
from tools.file_tools_paths import container_backend_for_task
backend = container_backend_for_task(task_id or "default")
if backend is None: # container paths carry no host ledger entry
for _p in landed_paths:
with suppress(Exception):

10
gateway/slash_commands.py
View File

@@ -696,9 +696,15 @@ class GatewaySlashCommandsMixin(
tokens = event.get_command_args().strip().split()
restore_all = any(tok.lower() in ("--all", "--force") for tok in tokens)
arg = " ".join(tok for tok in tokens if tok.lower() not in ("--all", "--force"))
# Container-backed session: host checkpoints belong to another tree, so a restore is
# refused; the bare listing stays visible, prefixed with the reason (same as the CLI).
reason = mgr.unsupported_backend_reason()
if reason and arg:
return reason
checkpoints = mgr.list_checkpoints(cwd)
if not arg:
return format_checkpoint_list(checkpoints, cwd)
listing = format_checkpoint_list(checkpoints, cwd)
return f"{reason}\n{listing}" if reason else listing
if not checkpoints:
return t("gateway.rollback.none_found", cwd=cwd)
@@ -736,6 +742,8 @@ class GatewaySlashCommandsMixin(
mgr = self._checkpoint_manager()
if mgr is None:
return t("gateway.diff.not_enabled")
if reason := mgr.unsupported_backend_reason(): # host baseline is not this session's tree
return reason
result = await asyncio.to_thread(mgr.session_diff, cwd)
else:
from tools.working_diff import collect_working_diff

2
hermes_cli/cli_commands_mixin.py
View File

@@ -764,6 +764,8 @@ class CLICommandsMixin:
" (Plain /diff still works — it uses git directly.)"))
if mgr is None:
return
if reason := mgr.unsupported_backend_reason(): # host baseline is not this session's tree
return print(f" {reason}")
result = mgr.session_diff(cwd)
if not result.get("success"):
return print(f" {result.get('error', 'Could not generate diff')}")

252
tests/agent/test_tool_executor_checkpoint_paths.py
View File

@@ -2,6 +2,7 @@
import json
import os
import threading
from types import SimpleNamespace
import pytest
@@ -14,15 +15,13 @@ from tools.terminal_tool import _active_environments, _env_lock
@pytest.fixture
def manager(tmp_path, monkeypatch):
monkeypatch.setattr(
"tools.checkpoint_manager.CHECKPOINT_BASE", tmp_path / "checkpoints"
)
monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", tmp_path / "checkpoints")
return CheckpointManager(enabled=True)
@pytest.fixture
def container_task_id(monkeypatch):
class FakeDockerEnvironment:
class FakeDockerEnvironment: # class-name hint is how file_tools_paths classifies live envs
pass
task_id = "container-checkpoint-task"
@@ -67,159 +66,79 @@ def test_relative_file_checkpoint_uses_task_workspace(tmp_path, monkeypatch):
assert manager.list_checkpoints(str(process_cwd)) == []
def _assert_container_skipped(manager, task_id):
assert manager.list_all_checkpoints() == []
assert "docker" in manager.unsupported_backend_reason(task_id)
def test_container_backend_file_checkpoint_is_not_taken_on_missing_host_path(
manager, container_task_id,
):
agent = SimpleNamespace(_checkpoint_mgr=manager)
_ensure_file_checkpoint(
agent, "write_file", {"path": "/workspace/project/a.txt"}, container_task_id
)
_assert_container_skipped(manager, container_task_id)
@pytest.mark.skipif(os.name == "nt", reason="POSIX absolute container path")
def test_container_backend_file_checkpoint_does_not_snapshot_a_colliding_host_tree(
tmp_path, manager, container_task_id,
):
host_dir = tmp_path / "workspace" / "project"
host_dir.mkdir(parents=True)
(host_dir / "pyproject.toml").write_text("[project]\n", encoding="utf-8")
(host_dir / "a.txt").write_text("host content\n", encoding="utf-8")
_ensure_file_checkpoint(
SimpleNamespace(_checkpoint_mgr=manager),
"write_file",
{"path": str(host_dir / "a.txt")},
container_task_id,
)
assert manager.list_checkpoints(str(host_dir)) == []
_assert_container_skipped(manager, container_task_id)
def test_container_backend_destructive_terminal_checkpoint_is_not_taken(
tmp_path, monkeypatch, manager, container_task_id,
):
# Pin the host cwd the branch would snapshot on main to a small tree, not the test process cwd.
host_cwd = tmp_path / "host-cwd"
host_cwd.mkdir()
(host_cwd / "keep.txt").write_text("host", encoding="utf-8")
monkeypatch.setenv("TERMINAL_CWD", str(host_cwd))
agent = SimpleNamespace(
quiet_mode=True,
tool_progress_callback=None,
tool_start_callback=None,
_checkpoint_mgr=manager,
_touch_activity=lambda *_: None,
)
ref = _ToolCallRef(
"terminal",
{"command": "rm -f /workspace/project/a.txt"},
container_task_id,
"call-1",
[],
)
_begin_tool_execution(agent, ref, None)
_assert_container_skipped(manager, container_task_id)
def test_container_backend_post_write_ledger_is_not_recorded(
tmp_path, manager, container_task_id,
):
# A real host file at the container path's spelling: on main the ledger loop hashes it
# on the host and persists an entry, which is exactly the false attribution to prevent.
def test_container_backend_task_leaves_host_store_untouched(tmp_path, manager, container_task_id):
"""A container-backed task's paths are container paths: no host snapshot from the file or
destructive-terminal hooks and no host ledger entry, even when an unrelated host tree shares
the spelling. A local task on the same manager still gets its checkpoint (control)."""
host_file = tmp_path / "workspace" / "project" / "a.txt"
host_file.parent.mkdir(parents=True)
host_file.write_text("after\n", encoding="utf-8")
host_file.write_text("host content\n", encoding="utf-8")
path = str(host_file)
agent = SimpleNamespace(
_turn_failed_file_mutations={},
_turn_file_mutation_paths=set(),
_checkpoint_mgr=manager,
_checkpoint_mgr=manager, _turn_failed_file_mutations={}, _turn_file_mutation_paths=set(),
quiet_mode=True, tool_progress_callback=None, tool_start_callback=None, _touch_activity=lambda *_: None,
)
_ensure_file_checkpoint(agent, "write_file", {"path": path}, container_task_id)
ref = _ToolCallRef("terminal", {"command": f"rm -rf {host_file.parent}"}, container_task_id, "call-1", [])
_begin_tool_execution(agent, ref, None)
TurnExplainersMixin._record_file_mutation_result(
agent,
"write_file",
{"path": path, "content": "after\n"},
json.dumps({"bytes_written": 6, "resolved_path": path}),
False,
task_id=container_task_id,
agent, "write_file", {"path": path, "content": "x"},
json.dumps({"bytes_written": 1, "resolved_path": path}), False, task_id=container_task_id,
)
assert not (tmp_path / "checkpoints" / "store" / "ledgers").exists()
_assert_container_skipped(manager, container_task_id)
assert manager.list_checkpoints(str(host_file.parent)) == []
assert not (tmp_path / "checkpoints").exists()
_ensure_file_checkpoint(agent, "write_file", {"path": path}, "local-task")
assert len(manager.list_checkpoints(str(host_file.parent))) == 1
def test_backend_reason_follows_the_session_backend(monkeypatch, manager):
"""Nothing is remembered: the same manager answers for the backend configured right now,
even after a container-backed mutation went through the checkpoint hook."""
monkeypatch.setenv("TERMINAL_ENV", "docker")
_ensure_file_checkpoint(
SimpleNamespace(_checkpoint_mgr=manager), "write_file", {"path": "/workspace/project/a.txt"}, "default"
)
assert "docker" in manager.unsupported_backend_reason()
monkeypatch.setenv("TERMINAL_ENV", "local")
assert manager.unsupported_backend_reason() is None
monkeypatch.setenv("TERMINAL_ENV", "docker")
assert "docker" in manager.unsupported_backend_reason()
def test_local_backend_behaviour_unchanged(tmp_path, monkeypatch, manager):
workspace = tmp_path / "local-project"
workspace.mkdir()
(workspace / "pyproject.toml").write_text("[project]\n", encoding="utf-8")
(workspace / "existing.txt").write_text("before\n", encoding="utf-8")
monkeypatch.setenv("TERMINAL_ENV", "local")
monkeypatch.setenv("TERMINAL_CWD", str(workspace))
_ensure_file_checkpoint(
SimpleNamespace(_checkpoint_mgr=manager),
"write_file",
{"path": "new.txt"},
"local-checkpoint-task",
)
assert manager.list_checkpoints(str(workspace))
def test_container_session_rollback_restore_is_refused(tmp_path, monkeypatch, manager, container_task_id, capsys):
"""A host checkpoint that predates the container session must not be restored from it."""
@pytest.mark.asyncio
async def test_container_session_refuses_host_rollback_on_every_surface(tmp_path, monkeypatch, manager, capsys):
"""A host checkpoint left by an earlier local session is never restored or diffed from a
container-backed session: CLI /rollback + /diff session, gateway /rollback + /diff session,
and the rollback.restore RPC all answer with the backend reason instead."""
from gateway import run as gateway_run
from gateway.config import Platform
from gateway.platforms.event import MessageEvent
from gateway.session import SessionSource
from hermes_cli.cli_commands_mixin import CLICommandsMixin
host_dir = tmp_path / "workspace" / "project"
host_dir.mkdir(parents=True)
(host_dir / "a.txt").write_text("before\n", encoding="utf-8")
monkeypatch.setenv("TERMINAL_ENV", "docker") # the configured backend, as the product bridges it
monkeypatch.setenv("TERMINAL_CWD", str(host_dir))
manager.ensure_checkpoint(str(host_dir), "earlier local session")
assert manager.list_checkpoints(str(host_dir))
# Fresh session: no mutation has been observed, so only the backend classification can refuse.
def refuse(*_args, **_kwargs):
raise AssertionError("host restore reached from a container-backed session")
cli = SimpleNamespace(
_checkpoint_manager=lambda _lines: manager,
_resolve_checkpoint_ref=lambda ref, cps: cps[int(ref) - 1]["hash"],
_rollback_restore=refuse,
_rollback_diff=refuse,
)
for command in ("/rollback 1 --all", "/rollback diff 1"):
CLICommandsMixin._handle_rollback_command(cli, command)
assert "docker" in capsys.readouterr().out
assert len(manager.list_checkpoints(str(host_dir))) == 1 # no pre-rollback snapshot either
def test_container_session_rpc_restore_is_refused(tmp_path, monkeypatch, manager, container_task_id):
import threading
from tui_gateway import server
host_dir = tmp_path / "workspace" / "project"
host_dir.mkdir(parents=True)
(host_dir / "a.txt").write_text("before\n", encoding="utf-8")
monkeypatch.setenv("TERMINAL_ENV", "docker")
manager.ensure_checkpoint(str(host_dir), "earlier local session")
monkeypatch.setenv("TERMINAL_ENV", "docker") # the configured backend, as the product bridges it
monkeypatch.setenv("TERMINAL_CWD", str(host_dir))
def refuse(*_args, **_kwargs):
raise AssertionError("host checkpoint operation reached from a container-backed session")
monkeypatch.setattr(manager, "restore", refuse)
monkeypatch.setattr(manager, "diff", refuse)
monkeypatch.setattr(manager, "session_diff", refuse)
cli = SimpleNamespace(
_checkpoint_manager=lambda _lines: manager,
_resolve_checkpoint_ref=lambda ref, cps: cps[int(ref) - 1]["hash"],
_rollback_restore=refuse, _rollback_diff=refuse,
)
for command in ("/rollback 1 --all", "/rollback diff 1"):
CLICommandsMixin._handle_rollback_command(cli, command)
assert "docker" in "".join(capsys.readouterr())
CLICommandsMixin._print_session_diff(cli, str(host_dir), False)
assert "docker" in "".join(capsys.readouterr())
runner = object.__new__(gateway_run.GatewayRunner)
runner._checkpoint_manager = lambda: manager
source = SessionSource(platform=Platform.TELEGRAM, user_id="u", chat_id="c", user_name="t", chat_type="dm")
for text in ("/rollback 1", "/rollback 1 --all"):
assert "docker" in await runner._handle_rollback_command(MessageEvent(text=text, source=source))
assert "docker" in await runner._handle_diff_command(MessageEvent(text="/diff session", source=source))
listing = await runner._handle_rollback_command(MessageEvent(text="/rollback", source=source))
assert "docker" in listing and "earlier local session" in listing # listing stays visible
session = {
"agent": SimpleNamespace(_checkpoint_mgr=manager), "cwd": str(host_dir), "running": False,
"session_key": "container-key", "history": [], "history_lock": threading.Lock(), "history_version": 0,
@@ -228,52 +147,13 @@ def test_container_session_rpc_restore_is_refused(tmp_path, monkeypatch, manager
resp = server.handle_request(
{"id": "1", "method": "rollback.restore", "params": {"session_id": "container-sid", "hash": "1"}}
)
assert resp["result"]["success"] is False
assert "docker" in resp["result"]["error"]
assert len(manager.list_checkpoints(str(host_dir))) == 1
assert resp["result"]["success"] is False and "docker" in resp["result"]["error"]
def test_local_session_rollback_restore_still_dispatches(tmp_path, monkeypatch, manager, capsys):
"""The other direction: a local session with a host checkpoint restores as before."""
import threading
from hermes_cli.cli_commands_mixin import CLICommandsMixin
from tui_gateway import server
host_dir = tmp_path / "local-project"
host_dir.mkdir()
(host_dir / "a.txt").write_text("before", encoding="utf-8")
monkeypatch.setenv("TERMINAL_ENV", "local")
monkeypatch.setenv("TERMINAL_CWD", str(host_dir))
manager.ensure_checkpoint(str(host_dir), "earlier local session")
calls = []
cli = SimpleNamespace(
_checkpoint_manager=lambda _lines: manager,
_resolve_checkpoint_ref=lambda ref, cps: cps[int(ref) - 1]["hash"],
_rollback_restore=lambda *args: calls.append(args),
)
CLICommandsMixin._handle_rollback_command(cli, "/rollback 1 --all")
assert len(calls) == 1 and "Checkpoints are not taken" not in capsys.readouterr().out
session = {
"agent": SimpleNamespace(_checkpoint_mgr=manager), "cwd": str(host_dir), "running": False,
"session_key": "local-key", "history": [], "history_lock": threading.Lock(), "history_version": 0,
}
monkeypatch.setitem(server._sessions, "local-sid", session)
monkeypatch.setenv("TERMINAL_ENV", "local") # control: a local session still dispatches
monkeypatch.setattr(manager, "restore", lambda *a, **k: {"success": True, "restored_to": "x", "reason": "r"})
resp = server.handle_request(
{"id": "1", "method": "rollback.restore", "params": {"session_id": "local-sid", "hash": "1"}}
)
assert resp["result"]["success"] is True
# A persistent docker container of the launch profile already occupies the "default" registry
# slot; this local session must still be classified by its own key, not by that cached env.
class FakeDockerEnvironment:
pass
with _env_lock:
monkeypatch.setitem(_active_environments, "default", FakeDockerEnvironment())
resp = server.handle_request(
{"id": "2", "method": "rollback.restore", "params": {"session_id": "local-sid", "hash": "1"}}
{"id": "2", "method": "rollback.restore", "params": {"session_id": "container-sid", "hash": "1"}}
)
assert resp["result"]["success"] is True
assert (host_dir / "a.txt").read_text(encoding="utf-8") == "before\n"
assert os.environ["TERMINAL_ENV"] == "local"

3
tests/hermes_cli/test_diff_command.py
View File

@@ -36,6 +36,9 @@ class _Mgr:
self._result = result
self.calls = []
def unsupported_backend_reason(self, task_id="default"):
return None
def session_diff(self, cwd):
self.calls.append(cwd)
return self._result

2
website/docs/user-guide/checkpoints-and-rollback.md
View File

@@ -236,7 +236,7 @@ Restore just one file from a checkpoint without affecting the rest of the direct
### Container Backends
With a container terminal backend (`docker`, `singularity`, `modal`, `daytona`, `vercel_sandbox`, or a container plugin), file paths belong to the sandbox rather than the host. Hermes therefore does not take checkpoints or record the agent-write ledger for those paths, and `/rollback` explains the limitation: it still lists existing host checkpoints but refuses diff and restore. Over the gateway RPC, `rollback.list` and `rollback.diff` remain available for inspecting host checkpoints while `rollback.restore` is refused. Local and SSH backends are unaffected.
With a container terminal backend (`docker`, `singularity`, `modal`, `daytona`, `vercel_sandbox`, or a container plugin), file paths belong to the sandbox rather than the host. Hermes therefore does not take checkpoints or record the agent-write ledger for those paths, and `/rollback` explains the limitation: it still lists existing host checkpoints but refuses diff and restore, on the CLI and in messaging-gateway chats alike; `/diff session` answers with the same reason. Over the TUI/Desktop RPC, `rollback.list` and `rollback.diff` remain available for inspecting host checkpoints while `rollback.restore` is refused. Local and SSH backends are unaffected. To point `terminal.cwd` at the container-side view of a mounted directory see [`terminal.docker_mount_cwd_to_workspace`](./configuration.md).
- **Git availability** — if `git` is not found on `PATH`, checkpoints are transparently disabled.
- **Directory scope** — Hermes skips overly broad directories (root `/`, home `$HOME`).