fix(tools): keep ssh file paths off the hermes host home
Relative writes and a bare ~ were expanded against the container subprocess home and then sent to the SSH target, which does not have that directory. (cherry picked from commit f4a2549d8276762e53ed2d773dbd4de8c7e74b3b)
This commit is contained in:
46
tests/tools/test_ssh_remote_cwd.py
Normal file
46
tests/tools/test_ssh_remote_cwd.py
Normal file
@@ -0,0 +1,46 @@
|
||||
"""SSH file paths must not land on the Hermes host's subprocess home.
|
||||
|
||||
Docker sets that home to ``/opt/data/home``. Expanding ``~`` or a relative
|
||||
path against it, then sending the result to an SSH backend, writes on a
|
||||
directory the remote machine does not have.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
import tools.file_tools_paths as paths
|
||||
import tools.terminal_tool as terminal_tool
|
||||
|
||||
|
||||
def test_ssh_relative_path_stays_off_container_home(monkeypatch):
|
||||
monkeypatch.setattr(paths, "_terminal_env_type_for_task", lambda task_id="default": "ssh")
|
||||
monkeypatch.setattr(terminal_tool, "_session_cwd", {"sess": "/opt/data/home"})
|
||||
with patch("hermes_constants.get_subprocess_home", return_value="/opt/data/home"):
|
||||
resolved = paths._resolve_path_for_task("cwd_probe.txt", task_id="sess")
|
||||
assert str(resolved) == "~/cwd_probe.txt"
|
||||
assert "/opt/data/home" not in str(resolved)
|
||||
|
||||
|
||||
def test_ssh_tilde_is_not_rewritten_to_the_host_home(monkeypatch):
|
||||
monkeypatch.setattr(paths, "_terminal_env_type_for_task", lambda task_id="default": "ssh")
|
||||
monkeypatch.setattr(terminal_tool, "_session_cwd", {})
|
||||
with patch("hermes_constants.get_subprocess_home", return_value="/opt/data/home"):
|
||||
resolved = paths._resolve_path_for_task("~/cwd_probe.txt", task_id="sess")
|
||||
assert str(resolved) == "~/cwd_probe.txt"
|
||||
|
||||
|
||||
def test_ssh_keeps_a_real_remote_directory(monkeypatch):
|
||||
monkeypatch.setattr(paths, "_terminal_env_type_for_task", lambda task_id="default": "ssh")
|
||||
monkeypatch.setattr(terminal_tool, "_session_cwd", {"sess": "/home/ubuntu/COMPRESS"})
|
||||
with patch("hermes_constants.get_subprocess_home", return_value="/opt/data/home"):
|
||||
resolved = paths._resolve_path_for_task("cwd_probe.txt", task_id="sess")
|
||||
assert str(resolved) == "/home/ubuntu/COMPRESS/cwd_probe.txt"
|
||||
|
||||
|
||||
def test_local_tilde_still_uses_subprocess_home(monkeypatch, tmp_path):
|
||||
home = tmp_path / "profile_home"
|
||||
home.mkdir()
|
||||
monkeypatch.setattr(paths, "_terminal_env_type_for_task", lambda task_id="default": "local")
|
||||
monkeypatch.setattr(terminal_tool, "_session_cwd", {})
|
||||
with patch("hermes_constants.get_subprocess_home", return_value=str(home)):
|
||||
resolved = paths._resolve_path_for_task("~/cwd_probe.txt", task_id="sess")
|
||||
assert str(resolved).startswith(str(home))
|
||||
@@ -163,10 +163,56 @@ def _anchor(text: str, base, container_paths: bool) -> Path | PurePosixPath:
|
||||
return p.resolve()
|
||||
|
||||
|
||||
def _ssh_remote_anchor(task_id: str) -> str:
|
||||
"""Working directory on the SSH target, never the Hermes host's home.
|
||||
|
||||
A recorded ``~`` stays ``~``. A recorded copy of the container subprocess
|
||||
home (the host path ``get_subprocess_home()`` returns) is the same idea
|
||||
written out by mistake, so it collapses back to ``~`` for the remote shell.
|
||||
"""
|
||||
from tools.terminal_tool_config import coerce_ssh_remote_cwd
|
||||
|
||||
raw = _authoritative_workspace_root(task_id) or "~"
|
||||
return coerce_ssh_remote_cwd(raw, "ssh") or "~"
|
||||
|
||||
|
||||
def _resolve_ssh_path(filepath: str, task_id: str) -> PurePosixPath:
|
||||
"""Resolve *filepath* in the SSH target's namespace.
|
||||
|
||||
Do not ``Path.resolve()`` or expand ``~`` with ``get_subprocess_home()``:
|
||||
both name directories on the Hermes host (``/opt/data/home`` in Docker),
|
||||
which the remote ``cd`` then rejects.
|
||||
"""
|
||||
text = str(filepath or "").strip()
|
||||
if text == "~" or text.startswith("~/"):
|
||||
return PurePosixPath(text)
|
||||
if posixpath.isabs(text):
|
||||
return _normalize_without_host_deref(text)
|
||||
anchor = _ssh_remote_anchor(task_id)
|
||||
if anchor == "~":
|
||||
joined = f"~/{text}"
|
||||
elif anchor.startswith("~/"):
|
||||
joined = f"{anchor.rstrip('/')}/{text}"
|
||||
else:
|
||||
joined = posixpath.normpath(posixpath.join(anchor, text))
|
||||
if joined == "~" or joined.startswith("~/"):
|
||||
return PurePosixPath(joined)
|
||||
return _normalize_without_host_deref(joined)
|
||||
|
||||
|
||||
def _resolve_base_dir(
|
||||
task_id: str = "default", *, container_paths: bool | None = None) -> Path | PurePosixPath:
|
||||
"""Return the ABSOLUTE base directory for resolving relative paths:
|
||||
``_authoritative_workspace_root``, else the process cwd as a last resort."""
|
||||
``_authoritative_workspace_root``, else the process cwd as a last resort.
|
||||
|
||||
SSH is the exception: the anchor is a remote path (often ``~``), and the
|
||||
process cwd is the Hermes host.
|
||||
"""
|
||||
if _terminal_env_type_for_task(task_id) == "ssh":
|
||||
anchor = _ssh_remote_anchor(task_id)
|
||||
if anchor == "~" or anchor.startswith("~/"):
|
||||
return PurePosixPath(anchor)
|
||||
return _normalize_without_host_deref(anchor)
|
||||
root = _authoritative_workspace_root(task_id)
|
||||
if container_paths is None:
|
||||
container_paths = _uses_container_paths(task_id)
|
||||
@@ -177,6 +223,8 @@ def _resolve_base_dir(
|
||||
def _resolve_path_for_task(filepath: str, task_id: str = "default") -> Path | PurePosixPath:
|
||||
"""Resolve *filepath* against the task's absolute base directory
|
||||
(absolute inputs are returned resolved-but-unanchored)."""
|
||||
if _terminal_env_type_for_task(task_id) == "ssh":
|
||||
return _resolve_ssh_path(filepath, task_id)
|
||||
container_paths = _uses_container_paths(task_id)
|
||||
return _anchor(_host_text(filepath, container_paths),
|
||||
lambda: _resolve_base_dir(task_id, container_paths=container_paths), container_paths)
|
||||
|
||||
@@ -95,6 +95,32 @@ def _get_plugin_env_provider(env_type: str):
|
||||
return _plugin_registry_lookup(env_type, "get_provider", None)
|
||||
|
||||
|
||||
def coerce_ssh_remote_cwd(cwd: str | None, env_type: str | None) -> str | None:
|
||||
"""Cwd to send to an SSH backend.
|
||||
|
||||
``~`` and ``~/...`` stay literal so the remote shell expands them to the
|
||||
SSH user's home. The Hermes process's subprocess home (``/opt/data/home``
|
||||
in the official Docker image) is a directory on the machine running
|
||||
Hermes. Using it as the remote working directory makes ``cd`` exit 126
|
||||
and anchors relative writes on a path that does not exist on the target.
|
||||
Other backends are unchanged.
|
||||
"""
|
||||
if not isinstance(cwd, str) or (env_type or "").strip().lower() != "ssh":
|
||||
return cwd
|
||||
text = cwd.strip()
|
||||
if not text or text == "~" or text.startswith("~/"):
|
||||
return text or "~"
|
||||
try:
|
||||
from hermes_constants import get_subprocess_home
|
||||
|
||||
home = get_subprocess_home()
|
||||
except Exception:
|
||||
home = None
|
||||
if home and os.path.normpath(text) == os.path.normpath(home):
|
||||
return "~"
|
||||
return text
|
||||
|
||||
|
||||
def _is_unusable_container_cwd(cwd: str) -> bool:
|
||||
"""True if *cwd* is a host or relative path that can't be a container
|
||||
workdir: ``docker run -w`` needs an absolute in-sandbox path, otherwise the
|
||||
|
||||
Reference in New Issue
Block a user